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

[Guenter Roeck]

On 8/9/22 01:53, Daniel Lezcano wrote:
> 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?

In the hwmon source (you point to it below):

        if (IS_ERR(tzd)) {
                if (PTR_ERR(tzd) != -ENODEV)
                        return PTR_ERR(tzd);
                dev_info(dev, "temp%d_input not attached to any thermal zone\n",
                         index + 1);
                devm_kfree(dev, tdata);
                return 0;
        }

That contradicts "if the thermal zone is not described, the hwmon driver won't initialize".
Now I must be missing something, since you mention that yourself below, and your new patch
series fixes the problem, at least AFAICS. Confused.

Guenter

> > 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