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


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.

Something like a helper function always called:
void _handle_critical( *tz)
{
if (tz->ops->critical)
tz->ops->critical(tz);

thermal_zone_device_critical(tz);
}

and then called:

else if (trip_type == THERMAL_TRIP_CRITICAL)
_handle_critical(tz);

> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
> @@ -1331,6 +1338,10 @@ thermal_zone_device_register(const char *type, int trips, int mask,
>
> tz->id = id;
> strlcpy(tz->type, type, sizeof(tz->type));
> +
> + if (!ops->critical)
> + ops->critical = thermal_zone_device_critical;

Then we could drop this default assignment.

> +
> tz->ops = ops;
> tz->tzp = tzp;
> tz->device.class = &thermal_class;
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index f23a388ded15..125c8a4d52e6 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -79,6 +79,8 @@ struct thermal_zone_device_ops {
> enum thermal_trend *);
> int (*notify) (struct thermal_zone_device *, int,
> enum thermal_trip_type);
> + void (*hot)(struct thermal_zone_device *);
> + void (*critical)(struct thermal_zone_device *);
> };
>
> struct thermal_cooling_device_ops {
> @@ -399,6 +401,7 @@ void thermal_cdev_update(struct thermal_cooling_device *);
> void thermal_notify_framework(struct thermal_zone_device *, int);
> int thermal_zone_device_enable(struct thermal_zone_device *tz);
> int thermal_zone_device_disable(struct thermal_zone_device *tz);
> +void thermal_zone_device_critical(struct thermal_zone_device *tz);
> #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)?

Regards,
Lukasz

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