Re: [PATCH] thermal: core: Send a sysfs notification on trip points

[Vincent Whitchurch <vincent.whitchurch@axis.com>]

On Thu, Apr 02, 2020 at 04:21:15PM +0200, Daniel Lezcano wrote:
> Currently the userspace has no easy way to get notified when a
> specific trip point was crossed. There are a couple of approaches:
> 
> - the userspace polls the sysfs temperature with usually an
>   unacceptable delay between the trip temperature point crossing and
>   the moment it is detected, or a high polling rate with an
>   unacceptable number of wakeup events.
> 
> - the thermal zone is set to be managed by an userspace governor in
>   order to receive the uevent even if the thermal zone needs to be
>   managed by another governor.
> 
> These changes allow to send a sysfs notification on the
> trip_point_*_temp when the temperature is getting higher than the trip
> point temperature. By this way, the userspace can be notified
> everytime when the trip point is crossed, this is useful for the
> thermal Android HAL or for notification to be sent via d-bus.
> 
> That allows the userspace to manage the applications based on specific
> alerts on different thermal zones to mitigate the skin temperature,
> letting the kernel governors handle the high temperature for hardware
> like the CPU, the GPU or the modem.
> 
> The temperature can be oscillating around a trip point and the event
> will be sent multiple times. It is up to the userspace to deal with
> this situation.

The actual temperature value would also be interesting.  Is there a way
for userspace to obtain it in a race-free manner when it is notified
that the trip point has been crossed?

> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index c06550930979..3cbdd20252ab 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -407,6 +407,19 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
>  	}
>  }
>  
> +static int thermal_trip_crossed(struct thermal_zone_device *tz, int trip)
> +{
> +	int trip_temp;
> +
> +	tz->ops->get_trip_temp(tz, trip, &trip_temp);
> +
> +	if (tz->last_temperature == THERMAL_TEMP_INVALID)
> +		return 0;
> +
> +	return ((tz->last_temperature < trip_temp)) &&
> +		(tz->temperature >= trip_temp));

 drivers/thermal/thermal_core.c: In function ‘thermal_trip_crossed’:
 drivers/thermal/thermal_core.c:425:33: error: expected ‘;’ before ‘)’ token
    (tz->temperature >= trip_temp));
                                  ^
 drivers/thermal/thermal_core.c:425:33: error: expected statement before ‘)’ token

> +}
> +
>  static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
>  {
>  	enum thermal_trip_type type;
> @@ -417,6 +430,16 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
>  
>  	tz->ops->get_trip_type(tz, trip, &type);
>  
> +	/*
> +	 * This condition will be true everytime the temperature is
> +	 * greater than the trip point and the previous temperature
> +	 * was below. In this case notify the userspace via a sysfs
> +	 * event on the trip point.
> +	 */
> +	if (thermal_trip_crossed(tz, trip))
> +		sysfs_notify(&tz->device.kobj, NULL,
> +			     tz->trip_temp_attrs[trip].attr.attr.name);

Normally sysfs_notify() is used to notify userspace that the value of
the sysfs file has changed, but in this case it's being used on a sysfs
file whose value never changes.  I don't know if there are other drivers
that do something similar.