Re: [PATCH v2 05/12] thermal/of: Move the node pointer assignation in the OF code file

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] The patch introduces a double call to `thermal_cooling_device_init_complete()` in `devm_thermal_of_cooling_device_register()`, leading to linked list corruption.
--

commit 1b7fe952320202b48dbc54a3f4bad412d0bfc910
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>

thermal/of: Move the node pointer assignation in the OF code file

This commit moves the assignment of the device node pointer from the thermal
core framework to the thermal OF code. It updates
devm_thermal_of_cooling_device_register() to use
thermal_of_cooling_device_register() instead of
__thermal_cooling_device_register(), creating a more consistent API design.

> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 3ba8c970f11f3..ee9776d0e5be7 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -537,6 +537,7 @@ thermal_of_cooling_device_register(struct device_node *np,
>  	if (IS_ERR(cdev))
>  		return cdev;
>  
> +	cdev->np = np;
>  	thermal_cooling_device_init_complete(cdev);
>  
>  	return cdev;

[ ... ]

> @@ -578,7 +579,7 @@ devm_thermal_of_cooling_device_register(struct device *dev,
>  	if (!ptr)
>  		return ERR_PTR(-ENOMEM);
>  
> -	tcd = __thermal_cooling_device_register(np, type, devdata, ops);
> +	tcd = thermal_of_cooling_device_register(np, type, devdata, ops);

Does this result in thermal_cooling_device_init_complete() being called
twice for the same cooling device?

When thermal_of_cooling_device_register() is called, it successfully
initializes the device and calls thermal_cooling_device_init_complete(),
which adds the device to the list.

Then devm_thermal_of_cooling_device_register() explicitly calls
thermal_cooling_device_init_complete() again:

drivers/thermal/thermal_of.c:devm_thermal_of_cooling_device_register() {
    ...
	tcd = thermal_of_cooling_device_register(np, type, devdata, ops);
	if (IS_ERR(tcd)) {
		devres_free(ptr);
		return tcd;
	}

	*ptr = tcd;
	devres_add(dev, ptr);

	thermal_cooling_device_init_complete(tcd);
    ...
}

Could this cause list_add() to be called twice on the same node, corrupting
the doubly-linked list structure?

While this issue is fixed later in the series by commit 61302b0ce72b9
(thermal/of: Pass the of_index and add a function to register with an
index), should it be fixed here to avoid breaking bisectability?

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