hwmon: (nct7802) Add fan output control
PATCH 1/1 hwmon: (nct7802) Add fan output control
Notes:
Some lines intentionally more than 80 characters because wrapping this lines decreases readability.
The patch was checked with scripts/checkpatch.pl --max-line-lengthˆ
Following code is not typecasting.
It is inline struct attribute_group definition using compound literal. (See https://gcc.gnu.org/onlinedocs/gcc-3.2.2/gcc/Compound-Literals.html)
static const struct attribute_group *nct7802_groups[] = {
...
&(const struct attribute_group){ .attrs = fan_output_attributes },
...Reference:
Nuvoton Hardware Monitoring IC NCT7802Y
7.2.91 Fan Control 1 Output Value
Location : Index 60h
7.2.92 Fan Control 2 Output Value
Location : Index 61h
7.2.93 Fan Control 3 Output Value
Location : Index 62h
--- drivers/hwmon/nct7802.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
drivers/hwmon/nct7802.c +45 -0
@@ -102,6 +102,38 @@ static ssize_t store_temp_type(struct device *dev,return err ? : count; } +static ssize_t show_dec (struct device *dev,struct device_attribute *devattr,char *buf) +{ +int ret =-1 ; +struct sensor_device_attribute *attr =to_sensor_dev_attr (devattr); +struct nct7802_data *data =dev_get_drvdata (dev); +unsigned int val =0 ; + + ret =regmap_read (data->regmap, attr->index, &val); +if (ret <0 ) +return ret; + + ret =sprintf (buf,"%d\n" , val); +return ret; +} + +static ssize_t store_u8 (struct device *dev,struct device_attribute *devattr, +const char *buf,size_t count) +{ +struct sensor_device_attribute *attr =to_sensor_dev_attr (devattr); +struct nct7802_data *data =dev_get_drvdata (dev); +int err; +unsigned int val; + + err =kstrtouint (buf,0 , &val); +if (err <0 ) +return err; +if (val >0xFF ) +return -EINVAL ; + + err =regmap_write (data->regmap, attr->index, val); +return err ? : count; +}static int nct7802_read_temp (struct nct7802_data *data,u8 reg_temp,u8 reg_temp_low,int *temp)
@@ -735,6 +767,11 @@ static SENSOR_DEVICE_ATTR_2(fan3_alarm, S_IRUGO, show_alarm, NULL, 0x1a, 2); static SENSOR_DEVICE_ATTR_2(fan3_beep, S_IRUGO | S_IWUSR, show_beep, store_beep, 0x5b, 2); +/* 7.2.91... Fan Control Output Value */ +static SENSOR_DEVICE_ATTR(fan1_output, S_IRUGO | S_IWUSR, show_dec, store_u8, 0x60); +static SENSOR_DEVICE_ATTR(fan2_output, S_IRUGO | S_IWUSR, show_dec, store_u8, 0x61); +static SENSOR_DEVICE_ATTR(fan3_output, S_IRUGO | S_IWUSR, show_dec, store_u8, 0x62); +static struct attribute *nct7802_fan_attrs[] = { &sensor_dev_attr_fan1_input.dev_attr.attr, &sensor_dev_attr_fan1_min.dev_attr.attr,
@@ -773,10 +810,18 @@ static struct attribute_group nct7802_fan_group = { .is_visible = nct7802_fan_is_visible, }; +static struct attribute *fan_output_attributes[] = { + &sensor_dev_attr_fan1_output.dev_attr.attr, + &sensor_dev_attr_fan2_output.dev_attr.attr, + &sensor_dev_attr_fan3_output.dev_attr.attr, +NULL +}; +static const struct attribute_group *nct7802_groups[] = { &nct7802_temp_group, &nct7802_in_group, &nct7802_fan_group, + &(const struct attribute_group ){ .attrs = fan_output_attributes },NULL };
Signature · 7 lines
1.9.1 _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Guenter Roeck
maintainerHi Constantine,
Quoted 6 lines · Constantine Shulyupin
Notes:
Some lines intentionally more than 80 characters because wrapping this lines decreases readability.
The patch was checked withscripts/checkpatch.pl--max-line-lengthˆ
Quoted 9 lines
Following code is not typecasting.
It is inline structattribute_groupdefinition using compound literal. (See https://gcc.gnu.org/onlinedocs/gcc-3.2.2/gcc/Compound-Literals.html)static const struct attribute_group *nct7802_groups[] = { ... &(const struct attribute_group){ .attrs = fan_output_attributes }, ...
I still don't like it. See below.
Quoted 6 lines
Reference:
Nuvoton Hardware Monitoring IC NCT7802Y
7.2.91 Fan Control 1 Output Value
Location : Index 60h
Quoted 6 lines
7.2.92 Fan Control 2 Output Value
Location : Index 61h7.2.93 Fan Control 3 Output Value
Location : Index 62h
This all doesn't really belong into the change log.
Quoted 10 lines
Signed-off-by Constantine Shulyupin <[email protected]>--- drivers/hwmon/nct7802.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)drivers/hwmon/nct7802.c +0
-0
drivers/hwmon/nct7802.c static ssize_t store_temp_type(struct device *dev, · line 105
Quoted 4 lines
drivers/hwmon/nct7802.c +1
-0@@ -102,6 +102,38 @@ static ssize_t store_temp_type(struct device *dev,return err ? : count; } +static ssize_t show_dec (struct device *dev,struct device_attribute *devattr,char *buf)
Please make this show_pwm.
drivers/hwmon/nct7802.c static ssize_t store_temp_type(struct device *dev, · line 117
Quoted 12 lines
drivers/hwmon/nct7802.c +12
-0+{ +int ret =-1 ; +struct sensor_device_attribute *attr =to_sensor_dev_attr (devattr); +struct nct7802_data *data =dev_get_drvdata (dev); +unsigned int val =0 ; + + ret =regmap_read (data->regmap, attr->index, &val); +if (ret <0 ) +return ret; + + ret =sprintf (buf,"%d\n" , val); +return ret;
return sprintf(buf, "%d\n", val);
drivers/hwmon/nct7802.c static ssize_t store_temp_type(struct device *dev, · line 121
Quoted 4 lines
drivers/hwmon/nct7802.c +4
-0+} + +static ssize_t store_u8 (struct device *dev,struct device_attribute *devattr, +const char *buf,size_t count)
store_pwm, and please align the continuation line with '('.
drivers/hwmon/nct7802.c static ssize_t store_temp_type(struct device *dev, · line 132
Quoted 11 lines
drivers/hwmon/nct7802.c +11
-0+{ +struct sensor_device_attribute *attr =to_sensor_dev_attr (devattr); +struct nct7802_data *data =dev_get_drvdata (dev); +int err; +unsigned int val; + + err =kstrtouint (buf,0 , &val); +if (err <0 ) +return err; +if (val >0xFF ) +return -EINVAL ;
You might consider using kstrtou8 instead.
Quoted 8 lines
drivers/hwmon/nct7802.c +4
-0+ + err =regmap_write (data->regmap, attr->index, val); +return err ? : count; +}static int nct7802_read_temp (struct nct7802_data *data,u8 reg_temp,u8 reg_temp_low,int *temp)
drivers/hwmon/nct7802.c static SENSOR_DEVICE_ATTR_2(fan3_alarm, S_IRUGO, show_alarm, NULL, 0x1a, 2); · line 774
Quoted 8 lines
drivers/hwmon/nct7802.c +5
-0@@ -735,6 +767,11 @@ static SENSOR_DEVICE_ATTR_2(fan3_alarm, S_IRUGO, show_alarm, NULL, 0x1a, 2); static SENSOR_DEVICE_ATTR_2(fan3_beep, S_IRUGO | S_IWUSR, show_beep, store_beep, 0x5b, 2); +/* 7.2.91... Fan Control Output Value */ +static SENSOR_DEVICE_ATTR(fan1_output, S_IRUGO | S_IWUSR, show_dec, store_u8, 0x60); +static SENSOR_DEVICE_ATTR(fan2_output, S_IRUGO | S_IWUSR, show_dec, store_u8, 0x61); +static SENSOR_DEVICE_ATTR(fan3_output, S_IRUGO | S_IWUSR, show_dec, store_u8, 0x62); +
Please use pwm[1-3] for the attribute names.
Quoted 4 lines
drivers/hwmon/nct7802.c +0
-0static struct attribute *nct7802_fan_attrs[] = { &sensor_dev_attr_fan1_input.dev_attr.attr, &sensor_dev_attr_fan1_min.dev_attr.attr,
drivers/hwmon/nct7802.c static struct attribute_group nct7802_fan_group = { · line 819
Quoted 10 lines
drivers/hwmon/nct7802.c +7
-0@@ -773,10 +810,18 @@ static struct attribute_group nct7802_fan_group = { .is_visible = nct7802_fan_is_visible, }; +static struct attribute *fan_output_attributes[] = { + &sensor_dev_attr_fan1_output.dev_attr.attr, + &sensor_dev_attr_fan2_output.dev_attr.attr, + &sensor_dev_attr_fan3_output.dev_attr.attr, +NULL +}; +
Please use an explicit variable for the group, similar to the other groups. I would suggest to use nct7802_pwm_attrs and nct7802_pwm_group.
_attrs is a naming convention used by macros such as ATTRIBUTE_GROUPS, so it makes sense to stick with it.
drivers/hwmon/nct7802.c static struct attribute_group nct7802_fan_group = { · line 824
Quoted 5 lines
drivers/hwmon/nct7802.c +1
-0static const struct attribute_group *nct7802_groups[] = { &nct7802_temp_group, &nct7802_in_group, &nct7802_fan_group, + &(const struct attribute_group ){ .attrs = fan_output_attributes },
... as I am not in favor of such constructs.
Thanks,
Guenter
drivers/hwmon/nct7802.c static struct attribute_group nct7802_fan_group = { · line 827
Quoted 4 lines
drivers/hwmon/nct7802.c +0
-0NULL };
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors