Re: [PATCH v2 04/12] thermal/core: Remove node pointer parameter when registering a tz

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [High] Compilation failure due to undeclared identifier `np` in `__thermal_cooling_device_register`.
- [High] Compilation failure due to argument count mismatch in `devm_thermal_of_cooling_device_register`.
- [High] Device tree (OF) cooling devices silently fail to bind to thermal zones.
- [Low] Misleading kerneldoc for `devm_thermal_cooling_device_register`.
--

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

thermal/core: Remove node pointer parameter when registering a tz

This patch removes the device_node pointer parameter in the core function
registering a thermal cooling device, which is inconsistent with non-OF code.

> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 35cf170f3fa1c..113719466dc21 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
[ ... ]
> @@ -1050,15 +1049,12 @@ void thermal_cooling_device_init_complete(struct thermal_cooling_device *cdev)
>   * This interface function adds a new thermal cooling device (fan/processor/...)
>   * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
>   * to all the thermal zone devices registered at the same time.
> - * It also gives the opportunity to link the cooling device to a device tree
> - * node, so that it can be bound to a thermal zone created out of device tree.
>   *
>   * Return: a pointer to the created struct thermal_cooling_device or an
>   * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
>   */
>  struct thermal_cooling_device *
> -__thermal_cooling_device_register(struct device_node *np,
> -				  const char *type, void *devdata,
> +__thermal_cooling_device_register(const char *type, void *devdata,
>  				  const struct thermal_cooling_device_ops *ops)
>  {
>  	struct thermal_cooling_device *cdev;

Does removing the np parameter here cause a compilation failure?

The assignment cdev->np = np; was left untouched further down in the function
body, which results in an undeclared identifier error.

(Note: This regression is fixed later in the series by commit 1b7fe95232020)

[ ... ]
> @@ -1167,6 +1163,50 @@ thermal_cooling_device_register(const char *type, void *devdata,
>  }
>  EXPORT_SYMBOL_GPL(thermal_cooling_device_register);
>  
> +static void thermal_cooling_device_release(struct device *dev, void *res)
> +{
> +	thermal_cooling_device_unregister(*(struct thermal_cooling_device **)res);
> +}
> +
> +/**
> + * devm_thermal_cooling_device_register() - register a thermal cooling device
> + * @dev:	a valid struct device pointer of a sensor device.
> + * @type:	the thermal cooling device type.
> + * @devdata:	device private data.
> + * @ops:	standard thermal cooling devices callbacks.
> + *
> + * This function will register a cooling device with device tree node reference.

This isn't a bug, but is this kerneldoc accurate?

This function is the generic (non-OF) devres variant and does not extract or
assign any device tree node reference. It appears to be a copy-paste error
from the OF-specific variant.

> + * This interface function adds a new thermal cooling device (fan/processor/...)
> + * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
> + * to all the thermal zone devices registered at the same time.
> + *
> + * Return: a pointer to the created struct thermal_cooling_device or an
> + * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
> + */
> +struct thermal_cooling_device *
> +devm_thermal_cooling_device_register(struct device *dev, const char *type,
> +				     void *devdata, const struct thermal_cooling_device_ops *ops)

[ ... ]
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 6e44bcf37b003..d45455bb9e9e6 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -272,8 +272,7 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz,
>  void thermal_cooling_device_init_complete(struct thermal_cooling_device *cdev);
>  
>  struct thermal_cooling_device *
> -__thermal_cooling_device_register(struct device_node *np,
> -				  const char *type, void *devdata,
> +__thermal_cooling_device_register(const char *type, void *devdata,
>  				  const struct thermal_cooling_device_ops *ops);

Does changing this prototype cause a compilation failure due to an argument
count mismatch in devm_thermal_of_cooling_device_register()?

The existing call inside devm_thermal_of_cooling_device_register() in
drivers/thermal/thermal_of.c was not updated and still passes 4 arguments.

(Note: This regression is fixed later in the series by commit 1b7fe95232020)

[ ... ]
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index c4b67554df447..3ba8c970f11f3 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -533,7 +533,7 @@ thermal_of_cooling_device_register(struct device_node *np,
>  {
>  	struct thermal_cooling_device *cdev;
>  
> -	cdev = __thermal_cooling_device_register(np, type, devdata, ops);
> +	cdev = __thermal_cooling_device_register(type, devdata, ops);
>  	if (IS_ERR(cdev))
>  		return cdev;

Does this silently break device tree thermal zone cooling bindings?

Since the np parameter is removed from the core registration function,
cdev->np is no longer populated there. The np parameter is passed here, but
not manually assigned to cdev->np.

Because cdev is zero-allocated, cdev->np remains NULL. When
thermal_cooling_device_init_complete() runs to bind the cooling device,
the check if (cooling_spec.np != cdev->np) will unconditionally fail.

(Note: This regression is fixed later in the series by commit 1b7fe95232020)

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