* thermal/drivers/airoha: Fix low trip temperature setting
@ 2025-11-22 22:07 david laight
2025-11-25 14:27 ` Christian Marangi
0 siblings, 1 reply; 2+ messages in thread
From: david laight @ 2025-11-22 22:07 UTC (permalink / raw)
To: rafael, rafael, linux-pm, linux-kernel, ansuelsmth
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,
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: thermal/drivers/airoha: Fix low trip temperature setting
2025-11-22 22:07 thermal/drivers/airoha: Fix low trip temperature setting david laight
@ 2025-11-25 14:27 ` Christian Marangi
0 siblings, 0 replies; 2+ messages in thread
From: Christian Marangi @ 2025-11-25 14:27 UTC (permalink / raw)
To: david laight; +Cc: rafael, linux-pm, linux-kernel
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-11-25 14:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-22 22:07 thermal/drivers/airoha: Fix low trip temperature setting david laight
2025-11-25 14:27 ` Christian Marangi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).