Message-ID: <4d12b57c-3dbb-e290-3f82-eb30aefa7dd4@gmail.com>
Date:   Sun, 20 Jun 2021 23:35:31 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Zhang Rui <rui.zhang@...el.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Amit Kucheria <amitk@...nel.org>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-tegra@...r.kernel.org
Subject: Re: [PATCH v1 1/2] hwmon: Support set_trips() of thermal device ops

20.06.2021 22:21, Guenter Roeck пишет:
> On Sun, Jun 20, 2021 at 08:38:27PM +0300, Dmitry Osipenko wrote:
>> 20.06.2021 20:23, Guenter Roeck пишет:
>>> On Sun, Jun 20, 2021 at 07:12:22PM +0300, Dmitry Osipenko wrote:
>>>> Support set_trips() callback of thermal device ops. This allows HWMON
>>>> device to operatively notify thermal core about temperature changes, which
>>>> is very handy to have in a case where HWMON sensor is used by CPU thermal
>>>> zone that performs passive cooling and emergency shutdown on overheat.
>>>> Thermal core will be able to react faster to temperature changes.
>>>>
>>>
>>> Why would this require a driver callback, and why can it not be handled
>>> in the hwmon core alone ? The hwmon core could register a set_trip function
>>> if the chip (driver) supports setting low and high limits, and it could
>>> call the appropriate driver functions when hwmon_thermal_set_trips()
>>> is called.
>>
>> I wasn't sure about what other hwmon drivers may need and want to do for
>> programming of the trips, so decided to start with this variant. I'll
>> prepare v2 since you're suggesting that the universal callback should
>> work okay for all drivers, thanks.
> 
> It will require some checks during probe to make sure that writeable limits
> exist, but that is still better than per-driver code. If for whatever
> reason some platform expects a different set of registers (say,
> critical limits instead of warning limits to attach to trip points),
> or if some platform expects that limits are _not_ used as trip points,
> that would not be driver but platform specific. You would not be able
> to address that on driver level with a single callback either (after all,
> lm90 compatible chips support up to three sets of limits).
> That means you already made an implementation specific choice with your
> code, by selecting one of those three sets of limits to act as trip
> points, and by making trip point support mandatory for all lm90 compatible
> chips. If we need to make that configurable, we'll need a better solution
> than a single driver callback, and that solution may as well be generic
> and driver independent.

Thank you for the clarification! If device makes a special use of lm90,
then very likely that it won't attach sensor to thermal zone. At least
all devices supported by mainline kernel should be okay here.

I think other sensors should be in a similar position. If a more complex
solution will be needed, then indeed hwmon API could be improved
further. The thermal device is created only for hwmon sensors that are
attached to thermal zone in a device-tree, so the scope of potentially
affected device should be small. Seems lm90 is actually the only hwmon
sensor that is used by thermal zones today.

AFAICS, all drivers return -EOPNOTSUPP if limits can't be changed, so we
could equal this error code to success in a case of set_trips(). The
set_trips() is very optional, if driver can't set limits, then the trips
won't trigger and thermal core will continue to work like set_trips()
wasn't hooked up. I'll implement this in v2.

Powered by blists - more mailing lists