Re: [PATCH v8 8/8] Loongson: YeeLoong: add input/hotkey driver

[Wu Zhangjin <wuzhangjin@gmail.com>]

On Sun, 2009-12-06 at 22:48 -0800, Dmitry Torokhov wrote:
> Hi Wu,
> 
> On Sun, Dec 06, 2009 at 03:01:48PM +0800, Wu Zhangjin wrote:
> > +
> > +#define EC_VER_LEN 64
> > +
> > +static int black_screen_handler(int status)
> > +{
> > +	char *p, ec_ver[EC_VER_LEN];
> > +
> > +	p = strstr(loongson_cmdline, "EC_VER=");
> > +	if (!p)
> > +		memset(ec_ver, 0, EC_VER_LEN);
> > +	else {
> > +		strncpy(ec_ver, p, EC_VER_LEN);
> > +		p = strstr(ec_ver, " ");
> > +		if (p)
> > +			*p = '\0';
> > +	}
> > +
> 
> Hmm, why do you copy and parse command lineinstead of using module
> param and also doing it just once?
> 

The kernel command line (EC_VER=blabla) is passed by bootloader(PMON)
automatically, so, I think it's better to use it.

Using an extra module param is good, but may make the users confused and
will also add extra jobs to the distribution developers ;) in the
future, the information of the EC and the other information of the
devices will be passed to kernel from BIOS via something like
FDT(Flattened Device Tree, used on PowerPC machines) as Ralf introduced,
so, the above method is a temp solution.

Thanks & Regards,
	Wu Zhangjin

> > +	/* Seems EC(>=PQ1D26) does this job for us, we can not do it again,
> > +	 * otherwise, the brightness will not resume to the normal level! */
> > +	if (strncasecmp(ec_ver, "EC_VER=PQ1D26", 64) < 0)
> > +		yeeloong_lcd_vo_set(status);
> > +
> > +	return status;
> > +}
> 
> Thanks.
>