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

[Maud Spierings | GOcontroll]

From: Guenter Roeck <groeck7@xxxxxxxxx> on behalf of Guenter Roeck <linux@xxxxxxxxxxxx>
Sent: Tuesday, March 4, 2025 12:54 PM
 
>On 3/4/25 03:33, Maud Spierings | GOcontroll wrote:
>> From: Guenter Roeck <groeck7@xxxxxxxxx> on behalf of Guenter Roeck <linux@xxxxxxxxxxxx>
>> Sent: Tuesday, March 4, 2025 12:12 PM
>>  
>>> On 3/4/25 00:24, Maud Spierings via B4 Relay wrote:
>>>> From: Maud Spierings <maudspierings@xxxxxxxxxxxxxx>
>>>>
>>>> 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.

That is unfortunate that such an attribute does not exist

>> 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.

That sounds good to me, I'll give it a test and send a new patch if it
works out.

>Guenter