lkml.org 
[lkml]   [2020]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/5] thermal/core: Add critical and hot ops
From
Date


On 12/10/20 1:37 PM, Daniel Lezcano wrote:
> On 10/12/2020 13:44, Lukasz Luba wrote:
>>
>>
>> On 12/10/20 12:15 PM, Daniel Lezcano wrote:
>>> Currently there is no way to the sensors to directly call an ops in
>>> interrupt mode without calling thermal_zone_device_update assuming all
>>> the trip points are defined.
>>>
>>> A sensor may want to do something special if a trip point is hot or
>>> critical.
>>>
>>> This patch adds the critical and hot ops to the thermal zone device,
>>> so a sensor can directly invoke them or let the thermal framework to
>>> call the sensor specific ones.
>>>
>>> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
>>>   drivers/thermal/thermal_core.c | 43 +++++++++++++++++++++-------------
>>>   include/linux/thermal.h        |  3 +++
>>>   2 files changed, 30 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/thermal/thermal_core.c
>>> b/drivers/thermal/thermal_core.c
>>> index e6771e5aeedb..cee0b31b5cd7 100644
>>> --- a/drivers/thermal/thermal_core.c
>>> +++ b/drivers/thermal/thermal_core.c
>>> @@ -375,6 +375,25 @@ static void thermal_emergency_poweroff(void)
>>>                     msecs_to_jiffies(poweroff_delay_ms));
>>>   }
>>>   +void thermal_zone_device_critical(struct thermal_zone_device *tz)
>>> +{
>>> +    dev_emerg(&tz->device, "%s: critical temperature reached, "
>>> +          "shutting down\n", tz->type);
>>> +
>>> +    mutex_lock(&poweroff_lock);
>>> +    if (!power_off_triggered) {
>>> +        /*
>>> +         * Queue a backup emergency shutdown in the event of
>>> +         * orderly_poweroff failure
>>> +         */
>>> +        thermal_emergency_poweroff();
>>> +        orderly_poweroff(true);
>>> +        power_off_triggered = true;
>>> +    }
>>> +    mutex_unlock(&poweroff_lock);
>>> +}
>>> +EXPORT_SYMBOL(thermal_zone_device_critical);
>>> +
>>>   static void handle_critical_trips(struct thermal_zone_device *tz,
>>>                     int trip, enum thermal_trip_type trip_type)
>>>   {
>>> @@ -391,22 +410,10 @@ static void handle_critical_trips(struct
>>> thermal_zone_device *tz,
>>>       if (tz->ops->notify)
>>>           tz->ops->notify(tz, trip, trip_type);
>>>   -    if (trip_type == THERMAL_TRIP_CRITICAL) {
>>> -        dev_emerg(&tz->device,
>>> -              "critical temperature reached (%d C), shutting down\n",
>>> -              tz->temperature / 1000);
>>> -        mutex_lock(&poweroff_lock);
>>> -        if (!power_off_triggered) {
>>> -            /*
>>> -             * Queue a backup emergency shutdown in the event of
>>> -             * orderly_poweroff failure
>>> -             */
>>> -            thermal_emergency_poweroff();
>>> -            orderly_poweroff(true);
>>> -            power_off_triggered = true;
>>> -        }
>>> -        mutex_unlock(&poweroff_lock);
>>> -    }
>>> +    if (trip_type == THERMAL_TRIP_HOT && tz->ops->hot)
>>> +        tz->ops->hot(tz);
>>> +    else if (trip_type == THERMAL_TRIP_CRITICAL)
>>> +        tz->ops->critical(tz);
>>
>> I can see that in the patch 3/5 there driver .critical() callback
>> calls framework thermal_zone_device_critical() at the end.
>> I wonder if we could always call this framework function.
>
> It is actually done on purpose, we want to let the driver to handle the
> critical routine which may not end up with an emergency shutdown.

I see.

>
> [ ... ]
>
>>>   #else
>>>   static inline struct thermal_zone_device *thermal_zone_device_register(
>>>       const char *type, int trips, int mask, void *devdata,
>>>
>>
>> I am just concerned about drivers which provide own .critical() callback
>> but forgot to call thermal_zone_device_critical() at the end and
>> framework could skip it.
>>
>> Or we can make sure during the review that it's not an issue (and ignore
>> out of tree drivers)?
>
> Yes, the framework guarantees if the critical trip point is crossed we
> call the emergency shutdown by default. If the driver choose to override
> it, it takes responsibility of the change.
>

Fair enough. Thus, the patch LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Regards,
Lukasz


\
 
 \ /
  Last update: 2020-12-10 14:54    [W:0.058 / U:1.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site