Re: [PATCH v1 02/14] thermal/driver/tegra/soctherm: Use devm_ variant when registering a cooling device

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Teardown asymmetry / use-after-disable causing kernel panic on probe failure or driver unbind.
--

commit 7cd0558154b37412cd530ca6d0e191a67317cc1e
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>

thermal/driver/tegra/soctherm: Use devm_ variant when registering a cooling device

This commit modifies the driver to use the devm_ variant for cooling
device registration, aiming to fix a resource leak at removal time.

> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
> --- a/drivers/thermal/tegra/soctherm.c
> +++ b/drivers/thermal/tegra/soctherm.c
[ ... ]
> @@ -1700,9 +1700,9 @@ static void soctherm_init_hw_throt_cdev(struct platform_device *pdev)
>  			stc->init = true;
>  		} else {
>  
> -			tcd = thermal_of_cooling_device_register(np_stcc,
> -							 (char *)name, ts,
> -							 &throt_cooling_ops);
> +			tcd = devm_thermal_of_cooling_device_register(dev, np_stcc,
> +								      (char *)name, ts,
> +								      &throt_cooling_ops);

Could this devm transition create a teardown asymmetry that leads to a kernel
panic?

While this prevents the cooling device from leaking, the unregistration is now
deferred until devres_release_all() runs (after probe() returns an error or
remove() completes).

The driver still manually disables the hardware clocks during the probe error
path and in tegra_soctherm_remove(). This creates a window where the cooling
device remains registered and exposed to the thermal core and sysfs while the
underlying hardware is powered down.

If there is a concurrent sysfs read of the cooling device state during this
window, it will invoke throt_get_cdev_cur_state(), which executes:

readl(ts->regs + THROT_STATUS)

Accessing IP block registers with the clocks disabled causes a synchronous
external abort on Tegra.

Should the clock disable operations also be managed via devres (e.g., using
devm_add_action_or_reset()) to guarantee LIFO teardown ordering?

>  			if (IS_ERR_OR_NULL(tcd)) {
>  				dev_err(dev,
>  					"throttle-cfg: %s: failed to register cooling device\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260419182203.4083985-1-daniel.lezcano@oss.qualcomm.com?part=2