Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <829788a5-3da4-8638-a587-9e80e2fd3fea@linaro.org>
Date:   Tue, 9 Aug 2022 10:53:22 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v5 00/33] New thermal OF code
Content-Language: en-US
To:     Guenter Roeck <linux@roeck-us.net>,
        Michael Walle <michael@walle.cc>
Cc:     daniel.lezcano@linexp.org, abailon@baylibre.com,
        anarsoul@gmail.com, baolin.wang7@gmail.com,
        bjorn.andersson@linaro.org, broonie@kernel.org,
        damien.lemoal@opensource.wdc.com, digetx@gmail.com,
        f.fainelli@gmail.com, glaroque@baylibre.com,
        hayashi.kunihiko@socionext.com, heiko@sntech.de, j-keerthy@ti.com,
        jonathanh@nvidia.com, khilman@baylibre.com,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
        lukasz.luba@arm.com, matthias.bgg@gmail.com,
        mcoquelin.stm32@gmail.com, mhiramat@kernel.org,
        miquel.raynal@bootlin.com, niklas.soderlund@ragnatech.se,
        rafael@kernel.org, rui.zhang@intel.com, shawnguo@kernel.org,
        talel@amazon.com, thierry.reding@gmail.com, tiny.windzz@gmail.com
References: <20220804224349.1926752-1-daniel.lezcano@linexp.org>
 <20220808094216.928018-1-michael@walle.cc>
 <20220808102610.GA1969424@roeck-us.net>
From:   Daniel Lezcano <daniel.lezcano@linaro.org>
In-Reply-To: <20220808102610.GA1969424@roeck-us.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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 |
<http://www.linaro.org/linaro-blog/> Blog