thermal/drivers/airoha: Fix low trip temperature setting
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.)
Signed-off by: David Laight <[email protected]>
---
This is a diff because I've not even contemplated committing the nearly 500 files I changed.
drivers/thermal/airoha_thermal.c +4 -4
@@ -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,
Quoted 10 lines · david laight
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 toclamp_t(). Additionally (and probably accidentally) it has a 64bit unsigned type which propagates throughRAW_TO_TEMP()causing a 'signedness error'. Hit it at the root cause with an (int) cast ofFIELD_MAX(). (Alternatively arrange forRAW_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?
Quoted 35 lines
Fixes 42de37f40e1b ("thermal/drivers: Add support for Airoha EN7581 thermal sensor")Signed-off by: David Laight <[email protected]>
---
This is a diff because I've not even contemplated committing the nearly 500 files I changed.drivers/thermal/airoha_thermal.c +4
-4@@ -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,
Signature · 1 line
Ansuel