[lm-sensors] [PATCH] f75383 /f75384 driver

[jim.cromie@gmail.com (Jim Cromie)]

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