From: Christian Marangi <ansuelsmth@gmail.com>
To: david laight <david.laight@runbox.com>
Cc: rafael@kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: thermal/drivers/airoha: Fix low trip temperature setting
Date: Tue, 25 Nov 2025 15:27:02 +0100 [thread overview]
Message-ID: <6925bcba.df0a0220.925a7.ca27@mx.google.com> (raw)
In-Reply-To: <20251122220747.70c51d9d@pumpkin>
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
prev parent reply other threads:[~2025-11-25 14:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-22 22:07 thermal/drivers/airoha: Fix low trip temperature setting david laight
2025-11-25 14:27 ` Christian Marangi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6925bcba.df0a0220.925a7.ca27@mx.google.com \
--to=ansuelsmth@gmail.com \
--cc=david.laight@runbox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).