hwmon: (adt7473) Initialize
PATCH 1/1 hwmon: (adt7473) Initialize
data->max_duty_at_overheat is not updated in adt7473_update_device, so it might be used before it is initialized (if the user reads from sysfs file max_duty_at_crit before writing to it.)
--- drivers/hwmon/adt7473.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/hwmon/adt7473.c +3 -0
@@ -298,6 +298,9 @@ no_sensor_update: ADT7473_REG_PWM_BHVR(i)); } + i =i2c_smbus_read_byte_data (client,ADT7473_REG_CFG4 ); + data->max_duty_at_overheat = !!(i &ADT7473_CFG4_MAX_DUTY_AT_OVT ); + data->limits_last_updated = local_jiffies; data->limits_valid =1 ;
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
drivers/hwmon/adt7473.c no_sensor_update: · line 302
Quoted 8 lines · Jean Delvare
drivers/hwmon/adt7473.c +2
-0@@ -298,6 +298,9 @@ no_sensor_update: ADT7473_REG_PWM_BHVR(i)); } + i =i2c_smbus_read_byte_data (client,ADT7473_REG_CFG4 ); + data->max_duty_at_overheat = !!(i &ADT7473_CFG4_MAX_DUTY_AT_OVT );
Maybe it would be more understandable to use:data->max_duty_at_overheat = (i & ADT7473_CFG4_MAX_DUTY_AT_OVT) ? 1 : 0;
Although I don't know if gcc optimizes both to the same instructions.
drivers/hwmon/adt7473.c no_sensor_update: · line 305
Quoted 3 lines
drivers/hwmon/adt7473.c +1
-0+ data->limits_last_updated = local_jiffies; data->limits_valid =1 ;
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi Paulius,
Thanks for the review.
Quoted 7 lines · Paulius Zaleckas
Quoted 6 lines · Jean Delvare
drivers/hwmon/adt7473.c +0
-0@@ -298,6 +298,9 @@ no_sensor_update: ADT7473_REG_PWM_BHVR(i)); }
Quoted 5 lines
drivers/hwmon/adt7473.c no_sensor_update: · line 302
Quoted 2 lines
drivers/hwmon/adt7473.c +2
-0+ i =i2c_smbus_read_byte_data (client,ADT7473_REG_CFG4 ); + data->max_duty_at_overheat = !!(i &ADT7473_CFG4_MAX_DUTY_AT_OVT );Maybe it would be more understandable to use:
data->max_duty_at_overheat= (i &ADT7473_CFG4_MAX_DUTY_AT_OVT) ? 1 : 0;
The !! construct seems to be widely used in the kernel, I found 800 occurrences, so I guess it's OK to use it here. But if Darrick (as the driver author) or Mark (as the subsystem maintainer) prefer your variant, I can update the patch.
Quoted 1 line
Although I don't know if gcc optimizes both to the same instructions.
It seems so, yes (gcc 4.1.2).
Quoted 4 lines
Quoted 4 lines
drivers/hwmon/adt7473.c +1
-0+ data->limits_last_updated = local_jiffies; data->limits_valid =1 ;
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 6 lines · Jean Delvare
data->max_duty_at_overheatis not updated inadt7473_update_device, so it might be used before it is initialized (if the user reads from sysfs filemax_duty_at_critbefore writing to it.)Signed-off-by Jean Delvare <[email protected]>Cc Darrick J. Wong <[email protected]>
Thanks for catching this. Looks/compiles/seems to run ok, so:
As for !! vs (?:), I don't have any particular preference.
--D
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi all:
Quoted 5 lines · Darrick J. Wong
Quoted 4 lines · Jean Delvare
data->max_duty_at_overheatis not updated inadt7473_update_device, so it might be used before it is initialized (if the user reads from sysfs filemax_duty_at_critbefore writing to it.)
Quoted 7 lines
Quoted 2 lines
Signed-off-by Jean Delvare <[email protected]>Cc Darrick J. Wong <[email protected]>Thanks for catching this. Looks/compiles/seems to run ok, so:
Acked-by Darrick J. Wong <[email protected]>As for !! vs (?:), I don't have any particular preference.
Applied to hwmon-2.6.git/testing, thanks.
Signature · 8 lines
Mark M. Hoffman [email protected] _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors