Message-ID: <d3e8a814-1dce-4054-87f9-3bd3f7b7c6dc@roeck-us.net>
Date: Tue, 4 Mar 2025 03:54:52 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Maud Spierings | GOcontroll <maudspierings@...ontroll.com>,
 Jean Delvare <jdelvare@...e.com>
Cc: "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] hwmon: (ntc_thermistor) Add min and max temperature
 attributes

On 3/4/25 03:33, Maud Spierings | GOcontroll wrote:
> From: Guenter Roeck <groeck7@...il.com> on behalf of Guenter Roeck <linux@...ck-us.net>
> Sent: Tuesday, March 4, 2025 12:12 PM
>   
>> 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.
> 
> Regarding the driver not reporting values outside the table values:
> 
> That does seem to be true and is good in my opinion, however currently
> 125 can mean 125 or something higher, with an indication of a max
> measurable temperature it can be determined that this is a max value and
> thus might have extra considerations.
> 
> Regarding the meaning of attribues:
> 
> It is difficult that the attributes do not have descriptions in
> include/linux/hwmon.h
> 
> Is there an attribute that should be used to indicate this maximum
> measurable value to userspace? HWMON_T_HIGHEST/LOWEST?
> HWMON_T_RATED_MIN/MAX?
>

There is no attribute providing the temperature (or any other) range
supported by a chip. highest/lowest are highest/lowest measurements, and
the rated attributes are rated min/max values for the system, not for
the chip. This applies to all chips, not just this one. Misusing the
ABI to report "limits supported by the chip" would be inappropriate and
misleading.

> Some extra ramblings:
> 
> I want to have some indication of what the lowest and highest
> temperatures that the sensor can measure are. Imagine I set my trip point
> at 140 degrees, but the sensor can only measure up to 125, I would like
> there to be some feedback that this trip point can never be measured.
> 
That would be a problem which applies to every chip. Unfortunately, it is
all but impossible to try to express that in sysfs attributes. Many chips
have different temperature ratings based on the chip model/variant
(commercial vs. car vs. mil), but the valid range can not be determined
from register values.

The same really applies to this driver: The tables in the driver are for
specific thermistors, but the thermistor will still provide a reading if the
temperature is out of range. It will just be out of spec.

On top of that, thermal limits should be based on board limits, not on chip
limits. A board which supports a temperature up to 140 degrees C but
utilizes a temperature sensor which can only measure up to 125 degrees C
would be a badly designed board. Trying to address that in a driver
would not add any value.

> Some kind of plausibility check may also be interesting. For example I
> have an ntc in an lvds display, if this display is disconnected it shuts
> down because the ADC reads zero, which means temp==temp_max.
> 

The best we could do would be to return -ENODATA for temperature values
if resistor values are out of range.

Guenter

Powered by blists - more mailing lists