Re: [PATCH v2 7/7] hwmon: (lm90) Add support for max6695 and max6696

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 15 Sep 2010 07:29:27 -0700, Guenter Roeck wrote:
> On Wed, Sep 15, 2010 at 07:20:29AM -0400, Jean Delvare wrote:
> > Would it be possible to simply place these functions at the right
> > location so that forward declarations are not needed? Ideally I would
> > like to get rid of all these forward declarations.
> > 
> I moved lm90_select_remote_channel() so the lm90_read_reg() declaration
> is no longer needed.
> 
> Code structure is such that sensor show/set functions come first, followed by
> "real" code. Unfortunately, that implies that local functions called from
> those functions have to be forward declarated.
> 
> We can move that around to avoid forward declarations, but I would prefer
> to do that in a separate patch if we decide to go that way.

Sure, fine with me.

> [...]
> > > +/*
> > > + * Additional attributes for devices with 3 temperature sensors
> > > + */
> > > +static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp11, NULL, 0, 5);
> > > +static SENSOR_DEVICE_ATTR_2(temp3_min, S_IWUSR | S_IRUGO, show_temp11,
> > > +     set_temp11, 3, 6);
> > > +static SENSOR_DEVICE_ATTR_2(temp3_max, S_IWUSR | S_IRUGO, show_temp11,
> > > +     set_temp11, 4, 7);
> > > +static SENSOR_DEVICE_ATTR(temp3_crit, S_IWUSR | S_IRUGO, show_temp8,
> > > +     set_temp8, 6);
> > > +static SENSOR_DEVICE_ATTR(temp3_crit_hyst, S_IRUGO, show_temphyst, NULL, 4);
> > 
> > 4, really? Remote 2 critical limit is 6.
> > 
> I am impressed. Good catch.

Now you understand why it takes me so long to review patches. I
actually read them ;)

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux