I've been thinking about folding all of the duplicate functions. There
are quite a few because both temperature sensors behave exactly the
same, and so they can be handled with one code execution line. Is there
a dev in lm_sensors to allow the change in read frequencies?
On Sat, 2006-08-19 at 09:04 -0600, Jim Cromie wrote:
> Brian Beardall wrote:
> > This is the patch for the f75383/4 driver. The f75383.diff applies to
> > the kernel, and the lm_sensors-f75383.diff applies to lm_sensors. I
> > would just like to have the code reviewed so that changes can be made.
> > Hopefully within this week because school starts again on Aug. 28th, and
> > that will make the schedule tight for hacking on this driver. Both
> > patches are -p1 patches. If you have an ECS mainboard that is only about
> > 2 years old it will most likely have this temperature sensor chip.
> >
> >
>
>
> These and other repeated callbacks can be folded down to 1 copy -
> theyre already extracting the index, which you can pre-initialize below..
>
> > +
> > +static ssize_t show_temp1(struct device *dev, struct device_attribute *devattr,
> > + char *buf)
> > +{
> > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > + struct f75383_data *data = f75383_update_device(dev);
> > + return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp1[attr->index]));
> > +}
> > +
> > +static ssize_t show_temp2(struct device *dev, struct device_attribute *devattr,
> > + char *buf)
> > +{
> > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > + struct f75383_data *data = f75383_update_device(dev);
> > + return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp2[attr->index]));
> > +}
> >
>
> Youre already taking care of indexing in declarations too !
>
> > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp1, NULL, 0);
> > +static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp1,
> > + set_temp1, 1);
> > +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp1,
> > + set_temp1, 2);
> >
>
>
> > +static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp2,
> > + set_temp2, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp2,
> > + set_temp2, 2);
> >
>
> Unless Ive missed something, above should be following: dropped 1,2 on
> function-names
>
>
> > +static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp,
> > + set_temp, 1);
> > +static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp,
> > + set_temp, 2);
> >
>
>
> Maybe I'll be able to take another look later..
>
> hth
> jimc
