Re: [PATCH v6 3/7] thermal/drivers: airoha: Convert to regmap API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The device interrupt is requested before the thermal zone is registered, and hardware interrupts are not disabled on unbind.
- [High] The interrupt handler silently drops events if both high and low threshold interrupts are pending simultaneously.
--

commit ddea8884dc559f65dc6c31d8d784b42996724da4
Author: Christian Marangi <ansuelsmth@gmail.com>

thermal/drivers: airoha: Convert to regmap API

Convert the Airoha thermal driver to use the regmap API instead of direct MMIO
accesses. This refactoring prepares the driver for Airoha AN7583 support,
which relies on a syscon regmap.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index 829a7327fc403..b63893a8997a3 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c

[ ... ]

> @@ -300,9 +300,9 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
>  	struct airoha_thermal_priv *priv = data;
>  	enum thermal_notify_event event;
>  	bool update = false;
> -	u32 status;
> +	u32 status = 0;
>  
> -	status = readl(priv->base + EN7581_TEMPMONINTSTS);
> +	regmap_read(priv->map, EN7581_TEMPMONINTSTS, &status);
>  	switch (status & (EN7581_HOFSINTSTS0 | EN7581_LOFSINTSTS0)) {
>  	case EN7581_HOFSINTSTS0:
>  		event = THERMAL_TRIP_VIOLATED;
> @@ -318,7 +318,7 @@ static irqreturn_t airoha_thermal_irq(int irq, void *data)
>  	}

[Severity: High]
This is a pre-existing issue, but does this switch statement drop events if
both HOFSINTSTS0 and LOFSINTSTS0 are set?

If the hardware latches both the high (EN7581_HOFSINTSTS0) and low
(EN7581_LOFSINTSTS0) threshold interrupt bits simultaneously, the bitwise AND
operation will yield both bits set. The switch statement will attempt an
exact match and fall through to the default case. The interrupts are then
cleared via regmap_write(), but thermal_zone_device_update() is never
called because update remains false.

>  
>  	/* Reset Interrupt */
> -	writel(status, priv->base + EN7581_TEMPMONINTSTS);
> +	regmap_write(priv->map, EN7581_TEMPMONINTSTS, status);
>  
>  	if (update)
>  		thermal_zone_device_update(priv->tz, event);

[ ... ]

> @@ -462,8 +475,8 @@ static int airoha_thermal_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, priv);
>  
>  	/* Enable LOW and HIGH interrupt */
> -	writel(EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0,
> -	       priv->base + EN7581_TEMPMONINT);
> +	regmap_write(priv->map, EN7581_TEMPMONINT,
> +		     EN7581_HOFSINTEN0 | EN7581_LOFSINTEN0);
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the probe function
request the interrupt before the thermal zone is registered?

It appears that devm_request_threaded_irq() is called earlier in
airoha_thermal_probe(), enabling the interrupt before
devm_thermal_of_zone_register() initializes priv->tz. If an interrupt fires
during this window, the ISR will dereference a NULL priv->tz.

Additionally, since there is no .remove callback to mask the hardware
interrupt, could this cause a use-after-free during driver unbind? Devres
executes cleanup in reverse order of creation, so priv->tz is destroyed
first, leaving the IRQ handler active until the IRQ is freed. An incoming
interrupt during this window might also lead to a use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702094846.17325-1-ansuelsmth@gmail.com?part=3