Re: [PATCH v5 00/33] New thermal OF code

Daniel Lezcano
2022-08-09T08:53:40Z

Hi Guenter,

On 08/08/2022 12:26, Guenter Roeck wrote:

[ ... ]

>> But I guess even if that is fixed, the driver will not probe due to the
>> missing trip points? Are they now mandatory? Does it mean we'd need to
>> update our device trees? But that will then mean older devices trees
>> don't work anymore.
>
> It would also mean that all hwmon drivers registering a thermal zone sensor
> would fail to register unless such a thermal zone actually exists.

Probably missing something but if the thermal zone is not described, the
hwmon driver won't initialize. And except if I'm wrong, that was already
the case before these changes, no?

> This
> would make the whole concept of having the hwmon core register thermal
> zone sensors impossible.

No, only the way the thermal OF is implemented changed. No functional
changes. So AFAICT, you can still create thermal zones with the hwmon.

> I have no idea how this is expected to work now,
> but there is an apparent flaw in the logic. That means I withdraw my
> Acked-by: for the hwmon patches in this series until it is guaranteed
> that hwmon registration does not fail as above if there is no thermal
> zone associated with a sensor.

If the thermal zone creation fails with -ENODEV, then it is no
considered as an error when creating the hwmon [1]

The function [devm]_thermal_zone_of_sensor_register() checks if there is
a thermal zone description, if not it bails out with -ENODEV [2]

Otherwise it checks all the thermal zones if the device passed as
parameter matches a sensor in the thermal zone [3][4]

If there is no match, then it returns -ENODEV which is the default error
code [5]

My understanding is there is no thermal zone creation if there is no
description in the device tree for such a device in the thermal zone.

The issue we had here was the confusing error message when -ENODEV
(before was -EINVAL) is returning while before the code was silently
continuing without creating the thermal zone.

We are talking here about what is in under CONFIG_THERMAL_OF in the
hwmon code path. The rest is untouched.

Am I missing something?

[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux/+/refs/tags/v5.18/drivers/hwmon/hwmon.c#230

[2]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux/+/refs/tags/v5.18/drivers/thermal/thermal_of.c#499

[3]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux/+/refs/tags/v5.18/drivers/thermal/thermal_of.c#510

[4]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux/+/refs/tags/v5.18/drivers/thermal/thermal_of.c#428

[5]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux/+/refs/tags/v5.18/drivers/thermal/thermal_of.c#497
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |