From: "Alexey Klimov" <alexey.klimov@linaro•org>
To: "Rosen Penev" <rosenp@gmail•com>, <linux-pm@vger•kernel.org>
Cc: "Bartlomiej Zolnierkiewicz" <bzolnier@gmail•com>,
"Krzysztof Kozlowski" <krzk@kernel•org>,
"Rafael J. Wysocki" <rafael@kernel•org>,
"Daniel Lezcano" <daniel.lezcano@kernel•org>,
"Zhang Rui" <rui.zhang@intel•com>,
"Lukasz Luba" <lukasz.luba@arm•com>,
"Peter Griffin" <peter.griffin@linaro•org>,
"Alim Akhtar" <alim.akhtar@samsung•com>,
"open list:SAMSUNG THERMAL DRIVER"
<linux-samsung-soc@vger•kernel.org>,
"moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES"
<linux-arm-kernel@lists•infradead.org>,
"open list" <linux-kernel@vger•kernel.org>
Subject: Re: [PATCH] thermal/drivers/exynos: fix clock ordering race and shared IRQ handling
Date: Wed, 03 Jun 2026 09:29:02 +0100 [thread overview]
Message-ID: <DIZ9U0RE11CQ.3QJET21NQ2QJL@linaro.org> (raw)
In-Reply-To: <20260603013018.239703-1-rosenp@gmail.com>
On Wed Jun 3, 2026 at 2:30 AM BST, Rosen Penev wrote:
> Fix two pre-existing issues in exynos_tmu_probe/remove:
>
> 1. Clock ordering race: The driver manually unprepares clocks in
> exynos_tmu_remove() and the probe error path, but the IRQ handler and
> thermal zone are devm-managed and remain active until after the manual
> cleanup. If the shared IRQ fires or the thermal zone is polled in that
> window, clk_enable() is called on an unprepared clock, which is illegal.
> Replace devm_clk_get() + manual clk_prepare() with devm_clk_get_prepared(),
> and devm_clk_get() + manual clk_prepare_enable() with
> devm_clk_get_enabled(), so clock unprepare is tied to the devm lifetime
> and happens after the IRQ and thermal zone are released. Remove the
> now-redundant manual cleanup from the error path and remove function.
>
> 2. Shared IRQ handling: The driver requests a shared IRQ (IRQF_SHARED) with
> NULL as the hardirq handler, causing the kernel to wake the threaded
> handler for every interrupt on the shared line. The threaded handler
> unconditionally returned IRQ_HANDLED without verifying that the TMU
> actually generated the interrupt, which could cause other shared-IRQ
> devices to be starved. Change tmu_clear_irqs to return the interrupt
> status register value, and return IRQ_NONE from the handler if no TMU
> interrupt was pending.
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail•com>
> ---
> drivers/thermal/samsung/exynos_tmu.c | 81 +++++++++-------------------
> 1 file changed, 24 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index 47a99b3c5395..5dc22006d7f8 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -196,7 +196,7 @@ struct exynos_tmu_data {
> void (*tmu_control)(struct platform_device *pdev, bool on);
> int (*tmu_read)(struct exynos_tmu_data *data);
> void (*tmu_set_emulation)(struct exynos_tmu_data *data, int temp);
> - void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
> + u32 (*tmu_clear_irqs)(struct exynos_tmu_data *data);
> };
>
> /*
> @@ -760,24 +760,28 @@ static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id)
> {
> struct exynos_tmu_data *data = id;
>
> - thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);
> -
> mutex_lock(&data->lock);
> clk_enable(data->clk);
>
> + if (!data->tmu_clear_irqs(data)) {
> + clk_disable(data->clk);
> + mutex_unlock(&data->lock);
> + return IRQ_NONE;
> + }
> +
> /* TODO: take action based on particular interrupt */
> - data->tmu_clear_irqs(data);
After that change the TODO comment now feels misplaced.
From a quick glance, it seems it should be moved as well to just
before if (!data->tmu_clear_irqs(data))
>
> + thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED);
> +
> return IRQ_HANDLED;
Best regards,
Alexey
prev parent reply other threads:[~2026-06-03 8:29 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-03 1:30 [PATCH] thermal/drivers/exynos: fix clock ordering race and shared IRQ handling Rosen Penev
2026-06-03 8:29 ` Alexey Klimov [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=DIZ9U0RE11CQ.3QJET21NQ2QJL@linaro.org \
--to=alexey.klimov@linaro$(echo .)org \
--cc=alim.akhtar@samsung$(echo .)com \
--cc=bzolnier@gmail$(echo .)com \
--cc=daniel.lezcano@kernel$(echo .)org \
--cc=krzk@kernel$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-pm@vger$(echo .)kernel.org \
--cc=linux-samsung-soc@vger$(echo .)kernel.org \
--cc=lukasz.luba@arm$(echo .)com \
--cc=peter.griffin@linaro$(echo .)org \
--cc=rafael@kernel$(echo .)org \
--cc=rosenp@gmail$(echo .)com \
--cc=rui.zhang@intel$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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