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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
