Re: [PATCH v1 06/14] thermal/core: Register cooling device non-OF drivers

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Low] Missing `static inline` in the fallback dummy implementation of `devm_thermal_cooling_device_register`.
- [Low] Incorrect kerneldoc comment for `devm_thermal_cooling_device_register`.
--

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

thermal/core: Register cooling device non-OF drivers

This commit provides a non-OF function to register a cooling device, allowing
a clear separation between OF and non-OF code. Drivers not using a devicetree
can then migrate to this new function.

> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 2f5d214d51a10..356a49e541fd5 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
[ ... ]
> @@ -1164,6 +1164,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 the comment states this registers a cooling device
with a device tree node reference. Since this is the non-OF version,
should this part of the comment be updated to reflect that it does not use a
devicetree?

> + * 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,
[ ... ]
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 0ddc77aeeca26..d9332b0371884 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
[ ... ]
> @@ -317,6 +322,14 @@ devm_thermal_of_cooling_device_register(struct device *dev,
>  {
>  	return ERR_PTR(-ENODEV);
>  }
> +
> +struct thermal_cooling_device *
> +devm_thermal_cooling_device_register(struct device *dev, const char *type,
> +				     void *devdata, const struct thermal_cooling_device_ops *ops)
> +{
> +	return ERR_PTR(-ENODEV);
> +}

Does this fallback implementation need the static inline keywords? 
Without them, it looks like this will be defined as a strong global symbol in
every translation unit that includes the thermal header when CONFIG_THERMAL is
disabled, which could cause multiple definition linker errors.

> +
>  static inline void thermal_cooling_device_unregister(
>  	struct thermal_cooling_device *cdev)
>  { }

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