Re: [PATCH] hwmon: (ntc_thermistor) Add min and max temperature attributes

[Guenter Roeck <linux@roeck-us.net>]

On 3/4/25 00:24, Maud Spierings via B4 Relay wrote:
> From: Maud Spierings <maudspierings@gocontroll.com>
> 
> Add the min and max temperature attributes as it is trivial for this
> driver.
> 
> This can help with detecting implausible readings and indicates to users
> which range they can actually measure, so they will not set a trip point
> at a temperature higher than max or lower than min.
> 
Unless I misunderstand the driver code, readings outside the table values
are never reported. Also, min/max are supposed to be alarm temperatures.
The reported values for min/max would be between -55 and +155 degrees C,
which does not make sense and has zero value for trip point usage.

NACK.

Guenter

> This implementation only works for ntcs, if a ptc gets added this will
> have to be changed, but I'm not sure if that is ever the intention.
> 
> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
> ---
>   drivers/hwmon/ntc_thermistor.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/ntc_thermistor.c b/drivers/hwmon/ntc_thermistor.c
> index 0d29c8f97ba7c2f264588b6309b91ca494012ad6..fecee177be5daf304c0521a74f2733f6e42567e0 100644
> --- a/drivers/hwmon/ntc_thermistor.c
> +++ b/drivers/hwmon/ntc_thermistor.c
> @@ -522,6 +522,12 @@ static int ntc_read(struct device *dev, enum hwmon_sensor_types type,
>   		case hwmon_temp_type:
>   			*val = 4;
>   			return 0;
> +		case hwmon_temp_min:
> +			*val = data->comp[0].temp_c*1000;
> +			return 0;
> +		case hwmon_temp_max:
> +			*val = data->comp[data->n_comp-1].temp_c*1000;
> +			return 0;
>   		default:
>   			break;
>   		}
> @@ -539,6 +545,8 @@ static umode_t ntc_is_visible(const void *data, enum hwmon_sensor_types type,
>   		switch (attr) {
>   		case hwmon_temp_input:
>   		case hwmon_temp_type:
> +		case hwmon_temp_min:
> +		case hwmon_temp_max:
>   			return 0444;
>   		default:
>   			break;
> @@ -549,7 +557,8 @@ static umode_t ntc_is_visible(const void *data, enum hwmon_sensor_types type,
>   
>   static const struct hwmon_channel_info * const ntc_info[] = {
>   	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> -	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_TYPE),
> +	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_TYPE | HWMON_T_MIN |
> +					 HWMON_T_MAX),
>   	NULL
>   };
>   
> 
> ---
> base-commit: cd3215bbcb9d4321def93fea6cfad4d5b42b9d1d
> change-id: 20250304-ntc_min_max-a925e87d0da7
> 
> Best regards,