Re: [PATCH] thinkpad_acpi: support new BIOS version string pattern

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 09, 2015 at 08:35:22PM -0800, Darren Hart wrote:
> >  static bool __pure __init tpacpi_is_valid_fw_id(const char * const s,
> >  						const char t)
> >  {
> > -	return s && strlen(s) >= 8 &&
> > +	bool is_most = 0;
> > +	bool is_new = 0;
> 
> The new variables aren't really necessary, you certainly do not need two of
> them.
> 
> > +
> > +	/*
> > +	 * Most models: xxyTkkWW (#.##c)
> > +	 * Ancient 570/600 and -SL lacks (#.##c)
> > +	 */
> > +	is_most = s && strlen(s) >= 8 &&
> 
> Try:
> 
> if (s && strlen(s) >=8 &&
> 	...
> 	)
> 	return true;

Yes, you are right.

> >  		tpacpi_is_fw_digit(s[0]) &&
> >  		tpacpi_is_fw_digit(s[1]) &&
> >  		s[2] == t &&
> >  		(s[3] == 'T' || s[3] == 'N') &&
> >  		tpacpi_is_fw_digit(s[4]) &&
> >  		tpacpi_is_fw_digit(s[5]);
> > +
> > +	if (is_most)
> > +		return is_most;
> > +
> > +	/* New models: xxxyTkkW (#.##c); T550 and some others */
> 
> Is the second W intentionally left off in xxxyTkkW ? We don't test for it, so I
> wasn't sure.

Yes, we have the latest models, tested it.

> > +	is_new = s && strlen(s) >= 8 &&
> > +		tpacpi_is_fw_digit(s[0]) &&
> > +		tpacpi_is_fw_digit(s[1]) &&
> > +		tpacpi_is_fw_digit(s[2]) &&
> 
> As far as I can tell, the only significant difference here (compared to above)
> is an additional leading digit and the subsequent string indices?
> 
> Seems to me this could be done fairly easily in the same block with only a minor
> adjustment at the beginning and the use of an incrementing index. Should be
> cleaner than duplicating 90% of the block?

I'd like to separate them still, it will be more readable and
extendible(Lenovo may release some BIOSes with other patterns for
non-classic ThinkPad models in the near future).

Thanks, Darren, will send the v2.

-- 
Adam Lee

------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel




[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux