Re: [PATCH v2 02/12] thermal/of: Move OF code where it belongs to

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] A syntax error is introduced in `include/linux/thermal.h` when `CONFIG_THERMAL` is disabled, breaking compilation. Note: I found the fix later in the series in commit fb0063667bcf5 ("thermal/core: Remove node pointer parameter when registering a tz").
- [High] The patch intentionally breaks cooling device registration for non-OF drivers on systems without `CONFIG_THERMAL_OF` enabled, causing runtime regressions. Note: I found the fix later in the series in commit b79ce9cd7a188 ("hwmon:: Use non-OF thermal cooling device register function").
--

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

thermal/of: Move OF code where it belongs to

This commit moves thermal_of_cooling_device_register and
devm_thermal_of_cooling_device_register from thermal-core to thermal-of.
It adds temporary stubs returning errors for non-OF drivers when
CONFIG_THERMAL_OF is disabled.

> As the thermal-of file depends on CONFIG_THERMAL_OF=y, by moving these
> two functions in it, when non-OF drivers are using this API that
> results in a link issue. In order to prevent unnecessary
> circumvolutions with the changes, the stubs have been added in the
> header file resulting in having the non-OF drivers compiled without
> the CONFIG_THERMAL_OF=y using these stubs and failing to register
> their cooling devices.
> 
> This is temporary as the next changes introduces the functions for
> non-OF drivers and returning to the initial state.

Is it acceptable to temporarily break cooling device registration for
non-OF drivers when CONFIG_THERMAL_OF is disabled?

Since the stubs return ERR_PTR(-ENODEV), it looks like non-OF drivers will
fail to register their cooling devices during this window.

I noticed this is fixed later in the series in commit b79ce9cd7a188, but
does introducing an intentional functional regression here break git bisect?

> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h

[ ... ]

> @@ -305,18 +323,7 @@ thermal_cooling_device_register(const char *type, void *devdata,
>  	const struct thermal_cooling_device_ops *ops)
>  { return ERR_PTR(-ENODEV); }
>  static inline struct thermal_cooling_device *
> -thermal_of_cooling_device_register(struct device_node *np,
> -	const char *type, void *devdata,
> -	const struct thermal_cooling_device_ops *ops)
> -{ return ERR_PTR(-ENODEV); }
> -static inline 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)
> -{
> -	return ERR_PTR(-ENODEV);
> -}
> +
>  static inline void thermal_cooling_device_unregister(
>  	struct thermal_cooling_device *cdev)
>  { }

Does this leave a dangling return type that causes a compiler syntax error
when CONFIG_THERMAL is disabled?

I see this was also fixed later in the series by commit fb0063667bcf5, but
does leaving this invalid C syntax here hurt bisectability for builds
without CONFIG_THERMAL?

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