Quoting "Mark M. Hoffman" <mhoffman at lightlink.com>:
> Hi Alexandre:
>
> Just a couple nits to pick...
>
> * Alexandre d'Alton <alex at alexdalton.org> [2004-06-22 19:52:09 +0200]:
>
> (liberal snips)
>
> > +#define AUTO_TEMP_MAX_FROM_REG(reg) \
> > + AUTO_TEMP_RANGE_FROM_REG(reg) + \
> > + AUTO_TEMP_MIN_FROM_REG(reg)
>
> Should add outer parens there.
I agree on that point. I will have to re-send the patch updated.
>
> > +#define fan_auto_channel_offset(offset) \
> > +static ssize_t show_fan_auto_channel_##offset (struct device *dev, char
> *buf) \
> > +{ \
> > + return show_fan_auto_channel(dev, buf, 0x##offset - 1); \
> > +} \
> > +static ssize_t set_fan_auto_channel_##offset (struct device *dev, \
> > + const char *buf, size_t count) \
> > +{ \
> > + return set_fan_auto_channel(dev, buf, count, 0x##offset - 1); \
> > +} \
> > +static DEVICE_ATTR(auto_fan##offset##_channel, S_IRUGO | S_IWUSR, \
> > + show_fan_auto_channel_##offset, \
> > + set_fan_auto_channel_##offset)
>
> A recent driver core patch *necessitates* the trailing semicolon for
> DEVICE_ATTR().
In fact, I already put the semicolons on the iner calls to DEVICE_ATTR(), but as
when I "instanciate"
the macros I put a semicolon, I thought that it was unnecessary on the last one.
If you confirm that I have to add them, I'll be glad to do it.
Thanks,
Alex.
>
> > +static DEVICE_ATTR(auto_temp##offset##_off, S_IRUGO, \
> > + show_auto_temp_##offset##_off, NULL); \
> > +static DEVICE_ATTR(auto_temp##offset##_min, S_IRUGO | S_IWUSR, \
> > + show_auto_temp_##offset##_min, set_auto_temp_##offset##_min);\
> > +static DEVICE_ATTR(auto_temp##offset##_max, S_IRUGO | S_IWUSR, \
> > + show_auto_temp_##offset##_max, set_auto_temp_##offset##_max)
>
> Likewise for the last of these three.
>
> > +static DEVICE_ATTR(fan##offset##_pwm, S_IRUGO | S_IWUSR, \
> > + show_pwm_##offset, set_pwm_##offset)
>
> And here... eh, there are more but you get the point.
>
> Regards,
>
> --
> Mark M. Hoffman
> mhoffman at lightlink.com
>
>
--
-------------------------------------------------
Alexandre d'ALTON
alex at alexdalton.org
