thermal/drivers/airoha: Fix low trip temperature setting

Posted by david laight 1 week, 2 days ago

There is a 'cut&paste' error in the code that sets the 'low' limit.
The clamp() calls contains 'high' instead of 'low'.

While FEILD_MAX() ought to be (and probably is) a compile-time constant.
it isn't constant enough for the range check I'm adding to clamp_t().
Additionally (and probably accidentally) it has a 64bit unsigned type
which propagates through RAW_TO_TEMP() causing a 'signedness error'.
Hit it at the root cause with an (int) cast of FIELD_MAX().
(Alternatively arrange for RAW_TO_TEMP() to always return a signed value.)

Fixes: 42de37f40e1b ("thermal/drivers: Add support for Airoha EN7581 thermal sensor")

Signed-off by: David Laight <david.laight.linux@gmail.com>

---
This is a diff because I've not even contemplated committing the nearly 500
files I changed.

diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
index b9fd6bfc88e5..75afc94fea5f 100644
--- a/drivers/thermal/airoha_thermal.c
+++ b/drivers/thermal/airoha_thermal.c
@@ -261,8 +261,8 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
 
 	if (high != INT_MAX) {
 		/* Validate high and clamp it a supported value */
-		high = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
-			       RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
+		high = clamp(high, RAW_TO_TEMP(priv, 0),
+			       RAW_TO_TEMP(priv, (int)FIELD_MAX(EN7581_DOUT_TADC_MASK)));
 
 		/* We offset the high temp of 1°C to trigger correct event */
 		writel(TEMP_TO_RAW(priv, high) >> 4,
@@ -273,8 +273,8 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
 
 	if (low != -INT_MAX) {
 		/* Validate low and clamp it to a supported value */
-		low = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
-			      RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
+		low = clamp(low, RAW_TO_TEMP(priv, 0),
+			      RAW_TO_TEMP(priv, (int)FIELD_MAX(EN7581_DOUT_TADC_MASK)));
 
 		/* We offset the low temp of 1°C to trigger correct event */
 		writel(TEMP_TO_RAW(priv, low) >> 4,

Re: thermal/drivers/airoha: Fix low trip temperature setting

Posted by Christian Marangi 6 days, 9 hours ago

On Sat, Nov 22, 2025 at 10:07:47PM +0000, david laight wrote:
> There is a 'cut&paste' error in the code that sets the 'low' limit.
> The clamp() calls contains 'high' instead of 'low'.
> 
> While FEILD_MAX() ought to be (and probably is) a compile-time constant.
> it isn't constant enough for the range check I'm adding to clamp_t().
> Additionally (and probably accidentally) it has a 64bit unsigned type
> which propagates through RAW_TO_TEMP() causing a 'signedness error'.
> Hit it at the root cause with an (int) cast of FIELD_MAX().
> (Alternatively arrange for RAW_TO_TEMP() to always return a signed value.)
>

Sorry but I'm not very convinced by this section.

clamp_t in theory cast everything to the requested type. FIELD_MAX will
correctly result in the max value for the register and then RAW_TO_TEMP
should convert the value to a temp and be an int.

If it wasn't the case it should have been a BUILD_BUG error.

Am I missing something?

> Fixes: 42de37f40e1b ("thermal/drivers: Add support for Airoha EN7581 thermal sensor")
> 
> Signed-off by: David Laight <david.laight.linux@gmail.com>
> 
> ---
> This is a diff because I've not even contemplated committing the nearly 500
> files I changed.
> 
> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index b9fd6bfc88e5..75afc94fea5f 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c
> @@ -261,8 +261,8 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
>  
>  	if (high != INT_MAX) {
>  		/* Validate high and clamp it a supported value */
> -		high = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
> -			       RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
> +		high = clamp(high, RAW_TO_TEMP(priv, 0),
> +			       RAW_TO_TEMP(priv, (int)FIELD_MAX(EN7581_DOUT_TADC_MASK)));
>  
>  		/* We offset the high temp of 1°C to trigger correct event */
>  		writel(TEMP_TO_RAW(priv, high) >> 4,
> @@ -273,8 +273,8 @@ static int airoha_thermal_set_trips(struct thermal_zone_device *tz, int low,
>  
>  	if (low != -INT_MAX) {
>  		/* Validate low and clamp it to a supported value */
> -		low = clamp_t(int, high, RAW_TO_TEMP(priv, 0),
> -			      RAW_TO_TEMP(priv, FIELD_MAX(EN7581_DOUT_TADC_MASK)));
> +		low = clamp(low, RAW_TO_TEMP(priv, 0),
> +			      RAW_TO_TEMP(priv, (int)FIELD_MAX(EN7581_DOUT_TADC_MASK)));
>  
>  		/* We offset the low temp of 1°C to trigger correct event */
>  		writel(TEMP_TO_RAW(priv, low) >> 4,
> 

-- 
	Ansuel