1. Patchew
  2. linux
  3. [PATCH 0/3] hwmon/adt7470: fix 'fan*_{min,max}', 'temp*_alarm' and add 'temp_fan_norm_alarm'
  4. View patch

[PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output

Adrian DC posted 3 patches 1 year, 1 month ago
[PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output
Posted by Adrian DC 1 year, 1 month ago 
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(-)

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;
-- 
2.43.0
Re: [PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output
Posted by Guenter Roeck 1 year, 1 month ago 
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;

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.