| Message ID | 20250105195521.3263193-3-radian.dc@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon/adt7470: fix 'fan*_{min,max}', 'temp*_alarm' and add 'temp_fan_norm_alarm' | expand |
On 1/5/25 11:55, Adrian DC wrote: > Tested with the following script: The patch description is supposed to explain what is changed, and why. Test results are appropriate after "---", but not as patch description. > --- > > { > # Access hwmon > cd /sys/class/hwmon/hwmon1/ > > # Check alarms > alarms() { > echo -n ' [READ]' > echo -n " fan[1234]_alarm = $(cat ./fan[1234]_alarm | tr '\n' ' ')" > echo -n " temp{[123456789],10}_alarm = $(cat ./temp{[123456789],10}_alarm | tr '\n' ' ')" > echo '' > } > > # Configure hardware > echo '0' >./alarm_mask > echo '10' >./num_temp_sensors > > # Test fans > for fan in $(seq 1 4); do > echo " [TEST] fan${fan}_min : Min FAN speed ${fan} under limit" > echo '65535' >"./fan${fan}_min" > sleep 4 > alarms > echo '0' >"./fan${fan}_min" > sleep 5 > done > > # Test temperatures > for temp in $(seq 1 10); do > echo " [TEST] temp${temp}_max : Max temperature ${temp} over limit" > echo '-126000' >"./temp${temp}_max" > sleep 5 > alarms > echo '127000' >"./temp${temp}_max" > sleep 5 > done > > # Test clean > echo ' [TEST] Final state' > alarms > } > --- > > Faulty values: > [TEST] temp1_max : Max temperature 1 over limit > [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 1 0 1 0 1 0 1 0 1 > [TEST] temp2_max : Max temperature 2 over limit > [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 > --- > > Fixed values: > [TEST] fan1_min : Min FAN speed 1 under limit > [READ] fan[1234]_alarm = 1 0 0 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 > [TEST] fan2_min : Min FAN speed 2 under limit > [READ] fan[1234]_alarm = 0 1 0 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 > [TEST] fan3_min : Min FAN speed 3 under limit > [READ] fan[1234]_alarm = 0 0 1 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 > [TEST] fan4_min : Min FAN speed 4 under limit > [READ] fan[1234]_alarm = 0 0 0 1 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 > [TEST] temp1_max : Max temperature 1 over limit > [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 1 0 0 0 0 0 0 0 0 0 > [TEST] temp2_max : Max temperature 2 over limit > [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 1 0 0 0 0 0 0 0 0 > [TEST] temp3_max : Max temperature 3 over limit > [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 0 1 0 0 0 0 0 0 0 > ... > --- > > Signed-off-by: Adrian DC <radian.dc@gmail.com> > --- > drivers/hwmon/adt7470.c | 52 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 50 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c > index 712bc41b4a0d..afb881385dbb 100644 > --- a/drivers/hwmon/adt7470.c > +++ b/drivers/hwmon/adt7470.c > @@ -551,7 +551,40 @@ static int adt7470_temp_read(struct device *dev, u32 attr, int channel, long *va > *val = 1000 * data->temp_max[channel]; > break; > case hwmon_temp_alarm: > - *val = !!(data->alarm & channel); > + switch (channel) { > + case 0: > + *val = !!(data->alarm & ADT7470_R1T_ALARM); > + break; > + case 1: > + *val = !!(data->alarm & ADT7470_R2T_ALARM); > + break; > + case 2: > + *val = !!(data->alarm & ADT7470_R3T_ALARM); > + break; > + case 3: > + *val = !!(data->alarm & ADT7470_R4T_ALARM); > + break; > + case 4: > + *val = !!(data->alarm & ADT7470_R5T_ALARM); > + break; > + case 5: > + *val = !!(data->alarm & ADT7470_R6T_ALARM); > + break; > + case 6: > + *val = !!(data->alarm & ADT7470_R7T_ALARM); > + break; > + case 7: > + *val = !!(data->alarm & (ADT7470_R8T_ALARM << 8)); > + break; > + case 8: > + *val = !!(data->alarm & (ADT7470_R9T_ALARM << 8)); > + break; > + case 9: > + *val = !!(data->alarm & (ADT7470_R10T_ALARM << 8)); > + break; > + default: > + return -EOPNOTSUPP; > + } This can be simplified as if (channel < 7) *val = !!(data->alarm & BIT(channel)); else *val = !!(data->alarm & BIT(channel + 1); > break; > default: > return -EOPNOTSUPP; > @@ -648,7 +681,22 @@ static int adt7470_fan_read(struct device *dev, u32 attr, int channel, long *val > *val = 0; > break; > case hwmon_fan_alarm: > - *val = !!(data->alarm & (1 << (12 + channel))); > + switch (channel) { > + case 0: > + *val = !!(data->alarm & (ADT7470_FAN1_ALARM << 8)); > + break; > + case 1: > + *val = !!(data->alarm & (ADT7470_FAN2_ALARM << 8)); > + break; > + case 2: > + *val = !!(data->alarm & (ADT7470_FAN3_ALARM << 8)); > + break; > + case 3: > + *val = !!(data->alarm & (ADT7470_FAN4_ALARM << 8)); > + break; > + default: > + return -EOPNOTSUPP; > + } I don't immediately see the problem with the current code, nor do I see a functional difference. Please explain. Either case, the switch statement is unnecessary. > break; > default: > return -EOPNOTSUPP;
diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c index 712bc41b4a0d..afb881385dbb 100644 --- a/drivers/hwmon/adt7470.c +++ b/drivers/hwmon/adt7470.c @@ -551,7 +551,40 @@ static int adt7470_temp_read(struct device *dev, u32 attr, int channel, long *va *val = 1000 * data->temp_max[channel]; break; case hwmon_temp_alarm: - *val = !!(data->alarm & channel); + switch (channel) { + case 0: + *val = !!(data->alarm & ADT7470_R1T_ALARM); + break; + case 1: + *val = !!(data->alarm & ADT7470_R2T_ALARM); + break; + case 2: + *val = !!(data->alarm & ADT7470_R3T_ALARM); + break; + case 3: + *val = !!(data->alarm & ADT7470_R4T_ALARM); + break; + case 4: + *val = !!(data->alarm & ADT7470_R5T_ALARM); + break; + case 5: + *val = !!(data->alarm & ADT7470_R6T_ALARM); + break; + case 6: + *val = !!(data->alarm & ADT7470_R7T_ALARM); + break; + case 7: + *val = !!(data->alarm & (ADT7470_R8T_ALARM << 8)); + break; + case 8: + *val = !!(data->alarm & (ADT7470_R9T_ALARM << 8)); + break; + case 9: + *val = !!(data->alarm & (ADT7470_R10T_ALARM << 8)); + break; + default: + return -EOPNOTSUPP; + } break; default: return -EOPNOTSUPP; @@ -648,7 +681,22 @@ static int adt7470_fan_read(struct device *dev, u32 attr, int channel, long *val *val = 0; break; case hwmon_fan_alarm: - *val = !!(data->alarm & (1 << (12 + channel))); + switch (channel) { + case 0: + *val = !!(data->alarm & (ADT7470_FAN1_ALARM << 8)); + break; + case 1: + *val = !!(data->alarm & (ADT7470_FAN2_ALARM << 8)); + break; + case 2: + *val = !!(data->alarm & (ADT7470_FAN3_ALARM << 8)); + break; + case 3: + *val = !!(data->alarm & (ADT7470_FAN4_ALARM << 8)); + break; + default: + return -EOPNOTSUPP; + } break; default: return -EOPNOTSUPP;
Tested with the following script: --- { # Access hwmon cd /sys/class/hwmon/hwmon1/ # Check alarms alarms() { echo -n ' [READ]' echo -n " fan[1234]_alarm = $(cat ./fan[1234]_alarm | tr '\n' ' ')" echo -n " temp{[123456789],10}_alarm = $(cat ./temp{[123456789],10}_alarm | tr '\n' ' ')" echo '' } # Configure hardware echo '0' >./alarm_mask echo '10' >./num_temp_sensors # Test fans for fan in $(seq 1 4); do echo " [TEST] fan${fan}_min : Min FAN speed ${fan} under limit" echo '65535' >"./fan${fan}_min" sleep 4 alarms echo '0' >"./fan${fan}_min" sleep 5 done # Test temperatures for temp in $(seq 1 10); do echo " [TEST] temp${temp}_max : Max temperature ${temp} over limit" echo '-126000' >"./temp${temp}_max" sleep 5 alarms echo '127000' >"./temp${temp}_max" sleep 5 done # Test clean echo ' [TEST] Final state' alarms } --- Faulty values: [TEST] temp1_max : Max temperature 1 over limit [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 1 0 1 0 1 0 1 0 1 [TEST] temp2_max : Max temperature 2 over limit [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 --- Fixed values: [TEST] fan1_min : Min FAN speed 1 under limit [READ] fan[1234]_alarm = 1 0 0 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 [TEST] fan2_min : Min FAN speed 2 under limit [READ] fan[1234]_alarm = 0 1 0 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 [TEST] fan3_min : Min FAN speed 3 under limit [READ] fan[1234]_alarm = 0 0 1 0 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 [TEST] fan4_min : Min FAN speed 4 under limit [READ] fan[1234]_alarm = 0 0 0 1 temp{[123456789],10}_alarm = 0 0 0 0 0 0 0 0 0 0 [TEST] temp1_max : Max temperature 1 over limit [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 1 0 0 0 0 0 0 0 0 0 [TEST] temp2_max : Max temperature 2 over limit [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 1 0 0 0 0 0 0 0 0 [TEST] temp3_max : Max temperature 3 over limit [READ] fan[1234]_alarm = 0 0 0 0 temp{[123456789],10}_alarm = 0 0 1 0 0 0 0 0 0 0 ... --- Signed-off-by: Adrian DC <radian.dc@gmail.com> --- drivers/hwmon/adt7470.c | 52 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-)