Messages in this thread | | | Subject | Re: [PATCH 2/5] thermal/core: Add critical and hot ops | From | Lukasz Luba <> | Date | Thu, 10 Dec 2020 13:51:58 +0000 |
| |
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
| |