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