[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f2b58aab-6664-4968-86d9-1e761a41b065@roeck-us.net>
Date: Tue, 4 Mar 2025 03:12:18 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: maudspierings@...ontroll.com, Jean Delvare <jdelvare@...e.com>
Cc: linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: (ntc_thermistor) Add min and max temperature
attributes
On 3/4/25 00:24, Maud Spierings via B4 Relay wrote:
> From: Maud Spierings <maudspierings@...ontroll.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@...ontroll.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,
Powered by blists - more mailing lists