On Wed, Feb 22, 2012 at 06:26:00PM -0800, Guenter Roeck wrote:
> On Wed, Feb 22, 2012 at 05:18:48PM -0500, Nikolaus Schulz wrote:
> > +static bool duty_mode_enabled(u8 pwm_enable)
> > +{
> > + switch (pwm_enable) {
> > + case 0: /* Manual, duty mode (full speed) */
> > + case 1: /* Manual, duty mode */
> > + case 4: /* Auto, duty mode */
> > + return true;
> > + case 2: /* Auto, speed mode */
> > + case 3: /* Manual, speed mode */
> > + return false;
> > + default:
> > + BUG();
> > + }
> > +}
>
> This keeps bugging me ... I think it would make sense to simplify it to
>
> return pwm_enable != 2 && pwm_enable != 3;
Well, I used that verbose switch statement because
a) I feel uncomfortable using these magic numbers instead of a
well-defined enum - by the way, is there a reason for not using an enum for
that? - and
b) a simple logical expression like you suggested may lead to hidden
bugs once the range of valid values changes.
But if you insist, I can change that.
Nikolaus
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
