Re: [PATCH v1 01/16] thermal: core: Introduce .trip_crossed() callback for thermal governors

[Daniel Lezcano]

On 10/04/2024 18:10, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Introduce a new thermal governor callback called .trip_crossed()
> that will be invoked whenever a trip point is crossed by the zone
> temperature, either on the way up or on the way down.
>
> The trip crossing direction information will be passed to it and if
> multiple trips are crossed in the same direction during one thermal zone
> update, the new callback will be invoked for them in temperature order,
> either ascending or descending, depending on the trip crossing
> direction.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
>   drivers/thermal/thermal_core.c |   19 +++++++++++++++++--
>   drivers/thermal/thermal_core.h |    4 ++++
>   2 files changed, 21 insertions(+), 2 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -302,11 +302,21 @@ static void monitor_thermal_zone(struct
>   		thermal_zone_device_set_polling(tz, tz->polling_delay_jiffies);
>   }
>   
> +static struct thermal_governor *thermal_get_tz_governor(struct thermal_zone_device *tz)
> +{
> +	if (tz->governor)
> +		return tz->governor;
> +
> +	return def_governor;
> +}
> +
>   static void handle_non_critical_trips(struct thermal_zone_device *tz,
>   				      const struct thermal_trip *trip)
>   {
> -	tz->governor ? tz->governor->throttle(tz, trip) :
> -		       def_governor->throttle(tz, trip);
> +	struct thermal_governor *governor = thermal_get_tz_governor(tz);
> +
> +	if (governor->throttle)
> +		governor->throttle(tz, trip);
>   }
>   
>   void thermal_governor_update_tz(struct thermal_zone_device *tz,
> @@ -470,6 +480,7 @@ static int thermal_trip_notify_cmp(void
>   void __thermal_zone_device_update(struct thermal_zone_device *tz,
>   				  enum thermal_notify_event event)
>   {
> +	struct thermal_governor *governor = thermal_get_tz_governor(tz);
>   	struct thermal_trip_desc *td;
>   	LIST_HEAD(way_down_list);
>   	LIST_HEAD(way_up_list);
> @@ -493,12 +504,16 @@ void __thermal_zone_device_update(struct
>   	list_for_each_entry(td, &way_up_list, notify_list_node) {
>   		thermal_notify_tz_trip_up(tz, &td->trip);
>   		thermal_debug_tz_trip_up(tz, &td->trip);
> +		if (governor->trip_crossed)
> +			governor->trip_crossed(tz, &td->trip, true);

Is it possible to wrap this into a function ? So we keep the calls at 
the same level in this block

>   	}
>   
>   	list_sort(NULL, &way_down_list, thermal_trip_notify_cmp);
>   	list_for_each_entry(td, &way_down_list, notify_list_node) {
>   		thermal_notify_tz_trip_down(tz, &td->trip);
>   		thermal_debug_tz_trip_down(tz, &td->trip);
> +		if (governor->trip_crossed)
> +			governor->trip_crossed(tz, &td->trip, false);

idem

>   	}
>   
>   	monitor_thermal_zone(tz);
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -30,6 +30,7 @@ struct thermal_trip_desc {
>    *		otherwise it fails.
>    * @unbind_from_tz:	callback called when a governor is unbound from a
>    *			thermal zone.
> + * @trip_crossed:	called for trip points that have just been crossed
>    * @throttle:	callback called for every trip point even if temperature is
>    *		below the trip point temperature
>    * @update_tz:	callback called when thermal zone internals have changed, e.g.
> @@ -40,6 +41,9 @@ struct thermal_governor {
>   	const char *name;
>   	int (*bind_to_tz)(struct thermal_zone_device *tz);
>   	void (*unbind_from_tz)(struct thermal_zone_device *tz);
> +	void (*trip_crossed)(struct thermal_zone_device *tz,
> +			     const struct thermal_trip *trip,
> +			     bool crossed_up);
>   	int (*throttle)(struct thermal_zone_device *tz,
>   			const struct thermal_trip *trip);
>   	void (*update_tz)(struct thermal_zone_device *tz,

--
<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