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
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" 
in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to