public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] hwmon/adt7470: fix 'fan*_{min,max}', 'temp*_alarm' and add 'temp_fan_norm_alarm'
@ 2025-01-05 19:55 Adrian DC
  2025-01-05 19:55 ` [PATCH 1/3] hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0' Adrian DC
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Adrian DC @ 2025-01-05 19:55 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-kernel; +Cc: Adrian DC

Adrian DC (3):
  hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0'
  hwmon/adt7470: resolve faulty 'temp*_alarm' values read output
  hwmon/adt7470: create 'temp_fan_norm_alarm' attribute for 'NORM' alarm

 drivers/hwmon/adt7470.c | 77 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 5 deletions(-)


base-commit: ab75170520d4964f3acf8bb1f91d34cbc650688e
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0'
  2025-01-05 19:55 [PATCH 0/3] hwmon/adt7470: fix 'fan*_{min,max}', 'temp*_alarm' and add 'temp_fan_norm_alarm' Adrian DC
@ 2025-01-05 19:55 ` Adrian DC
  2025-01-06 19:02   ` Guenter Roeck
  2025-01-05 19:55 ` [PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output Adrian DC
  2025-01-05 19:55 ` [PATCH 3/3] hwmon/adt7470: create 'temp_fan_norm_alarm' attribute for 'NORM' alarm Adrian DC
  2 siblings, 1 reply; 7+ messages in thread
From: Adrian DC @ 2025-01-05 19:55 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-kernel; +Cc: Adrian DC

Tested with the following script and values
---

{
  # Access hwmon
  cd /sys/class/hwmon/hwmon1/

  # Set to 1 => 82
  echo -n ' [TEST] Set to 1 : '
  echo '1' >./fan1_max
  cat ./fan1_max

  # Set to 1234 => 1234
  echo -n ' [TEST] Set to 1234 : '
  echo '1234' >./fan1_max
  cat ./fan1_max

  # Reset to 0 => 0
  echo -n ' [TEST] Set to 0 : '
  echo '0' >./fan1_max
  cat ./fan1_max
}
---

Signed-off-by: Adrian DC <radian.dc@gmail.com>
---
 drivers/hwmon/adt7470.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index dbee6926fa05..712bc41b4a0d 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -662,11 +662,15 @@ static int adt7470_fan_write(struct device *dev, u32 attr, int channel, long val
 	struct adt7470_data *data = dev_get_drvdata(dev);
 	int err;
 
-	if (val <= 0)
+	if (val < 0)
 		return -EINVAL;
 
-	val = FAN_RPM_TO_PERIOD(val);
-	val = clamp_val(val, 1, 65534);
+	if (val) {
+		val = FAN_RPM_TO_PERIOD(val);
+		val = clamp_val(val, 1, 65534);
+	} else {
+		val = FAN_PERIOD_INVALID;
+	}
 
 	switch (attr) {
 	case hwmon_fan_min:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output
  2025-01-05 19:55 [PATCH 0/3] hwmon/adt7470: fix 'fan*_{min,max}', 'temp*_alarm' and add 'temp_fan_norm_alarm' Adrian DC
  2025-01-05 19:55 ` [PATCH 1/3] hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0' Adrian DC
@ 2025-01-05 19:55 ` Adrian DC
  2025-01-06 18:59   ` Guenter Roeck
  2025-01-05 19:55 ` [PATCH 3/3] hwmon/adt7470: create 'temp_fan_norm_alarm' attribute for 'NORM' alarm Adrian DC
  2 siblings, 1 reply; 7+ messages in thread
From: Adrian DC @ 2025-01-05 19:55 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-kernel; +Cc: Adrian DC

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] hwmon/adt7470: create 'temp_fan_norm_alarm' attribute for 'NORM' alarm
  2025-01-05 19:55 [PATCH 0/3] hwmon/adt7470: fix 'fan*_{min,max}', 'temp*_alarm' and add 'temp_fan_norm_alarm' Adrian DC
  2025-01-05 19:55 ` [PATCH 1/3] hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0' Adrian DC
  2025-01-05 19:55 ` [PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output Adrian DC
@ 2025-01-05 19:55 ` Adrian DC
  2025-01-06 19:10   ` Guenter Roeck
  2 siblings, 1 reply; 7+ messages in thread
From: Adrian DC @ 2025-01-05 19:55 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, linux-hwmon, linux-kernel; +Cc: Adrian DC

Default: 0 in all normal use cases
Test: Raises to 1 if all FANs are in automatic mode and below threshold
---

Signed-off-by: Adrian DC <radian.dc@gmail.com>
---
 drivers/hwmon/adt7470.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index afb881385dbb..1ec4c0807053 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -54,6 +54,7 @@ static const unsigned short normal_i2c[] = { 0x2C, 0x2E, 0x2F, I2C_CLIENT_END };
 #define		ADT7470_R8T_ALARM		0x01
 #define		ADT7470_R9T_ALARM		0x02
 #define		ADT7470_R10T_ALARM		0x04
+#define		ADT7470_NORM_ALARM		0x08
 #define		ADT7470_FAN1_ALARM		0x10
 #define		ADT7470_FAN2_ALARM		0x20
 #define		ADT7470_FAN3_ALARM		0x40
@@ -533,6 +534,18 @@ static ssize_t num_temp_sensors_store(struct device *dev,
 	return count;
 }
 
+static ssize_t temp_fan_norm_alarm_show(struct device *dev,
+				     struct device_attribute *devattr,
+				     char *buf)
+{
+	struct adt7470_data *data = adt7470_update_device(dev);
+
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	return sprintf(buf, "%d\n", !!(data->alarm & (ADT7470_NORM_ALARM << 8)));
+}
+
 static int adt7470_temp_read(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct adt7470_data *data = adt7470_update_device(dev);
@@ -1080,6 +1093,7 @@ static ssize_t pwm_auto_temp_store(struct device *dev,
 static DEVICE_ATTR_RW(alarm_mask);
 static DEVICE_ATTR_RW(num_temp_sensors);
 static DEVICE_ATTR_RW(auto_update_interval);
+static DEVICE_ATTR_RO(temp_fan_norm_alarm);
 
 static SENSOR_DEVICE_ATTR_RW(force_pwm_max, force_pwm_max, 0);
 
@@ -1112,6 +1126,7 @@ static struct attribute *adt7470_attrs[] = {
 	&dev_attr_alarm_mask.attr,
 	&dev_attr_num_temp_sensors.attr,
 	&dev_attr_auto_update_interval.attr,
+	&dev_attr_temp_fan_norm_alarm.attr,
 	&sensor_dev_attr_force_pwm_max.dev_attr.attr,
 	&sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr.attr,
 	&sensor_dev_attr_pwm2_auto_point1_pwm.dev_attr.attr,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output
  2025-01-05 19:55 ` [PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output Adrian DC
@ 2025-01-06 18:59   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2025-01-06 18:59 UTC (permalink / raw)
  To: Adrian DC, Jean Delvare, linux-hwmon, linux-kernel

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;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0'
  2025-01-05 19:55 ` [PATCH 1/3] hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0' Adrian DC
@ 2025-01-06 19:02   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2025-01-06 19:02 UTC (permalink / raw)
  To: Adrian DC, Jean Delvare, linux-hwmon, linux-kernel

On 1/5/25 11:55, Adrian DC wrote:
> Tested with the following script and values

The patch description is supposed to explain the reason for the changes,
not test results. Test results are useful, but only appropriate after "---".

Guenter

> ---
> 
> {
>    # Access hwmon
>    cd /sys/class/hwmon/hwmon1/
> 
>    # Set to 1 => 82
>    echo -n ' [TEST] Set to 1 : '
>    echo '1' >./fan1_max
>    cat ./fan1_max
> 
>    # Set to 1234 => 1234
>    echo -n ' [TEST] Set to 1234 : '
>    echo '1234' >./fan1_max
>    cat ./fan1_max
> 
>    # Reset to 0 => 0
>    echo -n ' [TEST] Set to 0 : '
>    echo '0' >./fan1_max
>    cat ./fan1_max
> }
> ---
> 
> Signed-off-by: Adrian DC <radian.dc@gmail.com>
> ---
>   drivers/hwmon/adt7470.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index dbee6926fa05..712bc41b4a0d 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -662,11 +662,15 @@ static int adt7470_fan_write(struct device *dev, u32 attr, int channel, long val
>   	struct adt7470_data *data = dev_get_drvdata(dev);
>   	int err;
>   
> -	if (val <= 0)
> +	if (val < 0)
>   		return -EINVAL;
>   
> -	val = FAN_RPM_TO_PERIOD(val);
> -	val = clamp_val(val, 1, 65534);
> +	if (val) {
> +		val = FAN_RPM_TO_PERIOD(val);
> +		val = clamp_val(val, 1, 65534);
> +	} else {
> +		val = FAN_PERIOD_INVALID;
> +	}
>   
>   	switch (attr) {
>   	case hwmon_fan_min:


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] hwmon/adt7470: create 'temp_fan_norm_alarm' attribute for 'NORM' alarm
  2025-01-05 19:55 ` [PATCH 3/3] hwmon/adt7470: create 'temp_fan_norm_alarm' attribute for 'NORM' alarm Adrian DC
@ 2025-01-06 19:10   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2025-01-06 19:10 UTC (permalink / raw)
  To: Adrian DC, Jean Delvare, linux-hwmon, linux-kernel

On 1/5/25 11:55, Adrian DC wrote:
> Default: 0 in all normal use cases
> Test: Raises to 1 if all FANs are in automatic mode and below threshold

As with the other patches, this is not an acceptable patch description.

On top of that, it introduces a non-standard attribute without documenting
it nor providing a rationale explaining the need for it. Also, this is not
an alarm. The bit says "everything is normal". I fail to see the value of
such an attribute, much less as an "alarm" attribute.

Guenter


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-01-06 19:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-05 19:55 [PATCH 0/3] hwmon/adt7470: fix 'fan*_{min,max}', 'temp*_alarm' and add 'temp_fan_norm_alarm' Adrian DC
2025-01-05 19:55 ` [PATCH 1/3] hwmon/adt7470: allow 'fan*_{min,max}' to be reset to '0' Adrian DC
2025-01-06 19:02   ` Guenter Roeck
2025-01-05 19:55 ` [PATCH 2/3] hwmon/adt7470: resolve faulty 'temp*_alarm' values read output Adrian DC
2025-01-06 18:59   ` Guenter Roeck
2025-01-05 19:55 ` [PATCH 3/3] hwmon/adt7470: create 'temp_fan_norm_alarm' attribute for 'NORM' alarm Adrian DC
2025-01-06 19:10   ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox