[PATCH] thermal/drivers/exynos: fix clock ordering race and shared IRQ handling

[Alexey Klimov]

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 at 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