[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9c2971dd-2f2d-426f-9107-eae93d5dd554@linaro.org>
Date: Mon, 1 Jul 2024 17:13:47 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: linux-pm@...r.kernel.org, Zhang Rui <rui.zhang@...el.com>,
Lukasz Luba <lukasz.luba@....com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] thermal/core: Introduce user trip points
On 28/06/2024 15:56, Rafael J. Wysocki wrote:
> On Thu, Jun 27, 2024 at 10:55 AM Daniel Lezcano
> <daniel.lezcano@...aro.org> wrote:
>>
>> Currently the thermal framework has 4 trip point types:
>>
>> - active : basically for fans (or anything requiring energy to cool
>> down)
>>
>> - passive : a performance limiter
>>
>> - hot : for a last action before reaching critical
>>
>> - critical : a without return threshold leading to a system shutdown
>>
>> A thermal zone monitors the temperature regarding these trip
>> points. The old way to do that is actively polling the temperature
>> which is very bad for embedded systems, especially mobile and it is
>> even worse today as we can have more than fifty thermal zones. The
>> modern way is to rely on the driver to send an interrupt when the trip
>> points are crossed, so the system can sleep while the temperature
>> monitoring is offloaded to a dedicated hardware.
>>
>> However, the thermal aspect is also managed from userspace to protect
>> the user, especially tracking down the skin temperature sensor. The
>> logic is more complex than what we found in the kernel because it
>> needs multiple sources indicating the thermal situation of the entire
>> system.
>>
>> For this reason it needs to setup trip points at different levels in
>> order to get informed about what is going on with some thermal zones
>> when running some specific application.
>>
>> For instance, the skin temperature must be limited to 43°C on a long
>> run but can go to 48°C for 10 minutes, or 60°C for 1 minute.
>>
>> The thermal engine must then rely on trip points to monitor those
>> temperatures. Unfortunately, today there is only 'active' and
>> 'passive' trip points which has a specific meaning for the kernel, not
>> the userspace. That leads to hacks in different platforms for mobile
>> and embedded systems where 'active' trip points are used to send
>> notification to the userspace. This is obviously not right because
>> these trip are handled by the kernel.
>>
>> This patch introduces the 'user' trip point type where its semantic is
>> simple: do nothing at the kernel level, just send a notification to
>> the user space.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
>> ---
>> .../devicetree/bindings/thermal/thermal-zones.yaml | 1 +
>> drivers/thermal/thermal_core.c | 8 ++++++++
>> drivers/thermal/thermal_of.c | 1 +
>> drivers/thermal/thermal_trace.h | 4 +++-
>> drivers/thermal/thermal_trip.c | 1 +
>> include/uapi/linux/thermal.h | 1 +
>> 6 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
>> index 68398e7e8655..cb9ea54a192e 100644
>> --- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
>> +++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
>> @@ -153,6 +153,7 @@ patternProperties:
>> type:
>> $ref: /schemas/types.yaml#/definitions/string
>> enum:
>> + - user # enable user notification
>> - active # enable active cooling e.g. fans
>> - passive # enable passive cooling e.g. throttling cpu
>> - hot # send notification to driver
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index 2aa04c46a425..506f880d9aa9 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -734,6 +734,14 @@ int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz,
>> if (tz != pos1 || cdev != pos2)
>> return -EINVAL;
>>
>> + /*
>> + * It is not allowed to bind a cooling device with a trip
>> + * point user type because no mitigation should happen from
>> + * the kernel with these trip points
>> + */
>> + if (trip->type == THERMAL_TRIP_USER)
>> + return -EINVAL;
>
> Maybe print a debug message when bailing out here?
After thinking a bit about the message, it sounds to me that is a really
an error in the firmware if we end up binding an 'user' trip point.
What about the following message:
dev_err(tz->device, "Trying to bind the cooling device '%s' with an
'user' trip point id=%d", cdev->type, trip->id);
--
<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
Powered by blists - more mailing lists