Re: [PATCH v1 10/14] thermal/of: Rename the devm_thermal_of_cooling_device_register() function

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [Low] Semantic mismatch in API naming for drivers not using child nodes.
- [Low] Incomplete refactoring leaves `thermal_of_cooling_device_register()` with a generic name.
- [Low] Kerneldoc for the renamed function does not document the new specialized semantics.
--

commit d1ab585391fe2969c56e5221cba69749aaa4c5c5
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
Subject: thermal/of: Rename the devm_thermal_of_cooling_device_register() function

This commit renames devm_thermal_of_cooling_device_register() to
devm_thermal_of_child_cooling_device_register() across the kernel. This
name change is intended to distinguish the old bindings using child nodes from
an incoming new binding format for cooling devices.

> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 37269db2de84f..e6a567d58579d 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
[ ... ]
> @@ -685,8 +685,9 @@ static int pwm_fan_probe(struct platform_device *pdev)
>  
>  	ctx->pwm_fan_state = ctx->pwm_fan_max_state;
>  	if (IS_ENABLED(CONFIG_THERMAL)) {
> -		cdev = devm_thermal_of_cooling_device_register(dev,
> -			dev->of_node, "pwm-fan", ctx, &pwm_fan_cooling_ops);
> +		cdev = devm_thermal_of_child_cooling_device_register(dev, dev->of_node,
> +								     "pwm-fan", ctx,
> +								     &pwm_fan_cooling_ops);

Since this driver is passing dev->of_node rather than a child node, does this
renaming create a semantic mismatch for this and other similar drivers (such
as tc654.c, max6650.c, and gpio-fan.c)?

[ ... ]
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 3d2fb8f37b9c5..ba5093f612d0e 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -550,7 +550,7 @@ static void thermal_cooling_device_release(struct device *dev, void *res)
>  }
>  
>  /**
> - * devm_thermal_of_cooling_device_register() - register an OF thermal cooling
> + * devm_thermal_of_child_cooling_device_register() - register an OF thermal cooling
>   *					       device
>   * @dev:	a valid struct device pointer of a sensor device.
>   * @np:		a pointer to a device tree node.

Since the function is now explicitly meant for legacy bindings utilizing child
nodes, could this kerneldoc description be updated to clarify its specialized
semantics instead of leaving it generically registering an OF thermal cooling
device?

[ ... ]
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a8e870ca2e271..6535353ae83ca 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -206,10 +206,10 @@ thermal_of_cooling_device_register(struct device_node *np,
>  				   const struct thermal_cooling_device_ops *ops);
>  
>  struct thermal_cooling_device *
> -devm_thermal_of_cooling_device_register(struct device *dev,
> -					struct device_node *np,
> -					const char *type, void *devdata,
> -					const struct thermal_cooling_device_ops *ops);
> +devm_thermal_of_child_cooling_device_register(struct device *dev,
> +					      struct device_node *np,
> +					      const char *type, void *devdata,
> +					      const struct thermal_cooling_device_ops *ops);

Should the underlying unmanaged thermal_of_cooling_device_register() also
be renamed to maintain a consistent API surface between the managed and
unmanaged variants?

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