2024-04-23 19:01:30

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1] thermal: core: Introduce thermal_governor_trip_crossed()

From: Rafael J. Wysocki <[email protected]>

Add a wrapper around the .trip_crossed() governor callback invocation
to reduce code duplications slightly and improve the code layout in
__thermal_zone_device_update().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---

As requested in https://lore.kernel.org/linux-pm/[email protected]/

---
drivers/thermal/thermal_core.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -450,6 +450,15 @@ static void thermal_zone_device_init(str
pos->initialized = false;
}

+static void thermal_governor_trip_crossed(struct thermal_governor *governor,
+ struct thermal_zone_device *tz,
+ const struct thermal_trip *trip,
+ bool crossed_up)
+{
+ if (governor->trip_crossed)
+ governor->trip_crossed(tz, trip, crossed_up);
+}
+
static int thermal_trip_notify_cmp(void *ascending, const struct list_head *a,
const struct list_head *b)
{
@@ -489,16 +498,14 @@ 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);
+ thermal_governor_trip_crossed(governor, tz, &td->trip, true);
}

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);
+ thermal_governor_trip_crossed(governor, tz, &td->trip, false);
}

if (governor->manage)





2024-04-24 07:28:04

by Lukasz Luba

[permalink] [raw]
Subject: Re: [PATCH v1] thermal: core: Introduce thermal_governor_trip_crossed()



On 4/23/24 20:01, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Add a wrapper around the .trip_crossed() governor callback invocation
> to reduce code duplications slightly and improve the code layout in
> __thermal_zone_device_update().
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> As requested in https://lore.kernel.org/linux-pm/[email protected]/
>
> ---
> drivers/thermal/thermal_core.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -450,6 +450,15 @@ static void thermal_zone_device_init(str
> pos->initialized = false;
> }
>
> +static void thermal_governor_trip_crossed(struct thermal_governor *governor,
> + struct thermal_zone_device *tz,
> + const struct thermal_trip *trip,
> + bool crossed_up)
> +{
> + if (governor->trip_crossed)
> + governor->trip_crossed(tz, trip, crossed_up);
> +}
> +
> static int thermal_trip_notify_cmp(void *ascending, const struct list_head *a,
> const struct list_head *b)
> {
> @@ -489,16 +498,14 @@ 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);
> + thermal_governor_trip_crossed(governor, tz, &td->trip, true);
> }
>
> 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);
> + thermal_governor_trip_crossed(governor, tz, &td->trip, false);
> }
>
> if (governor->manage)
>
>
>

Good suggestion from Daniel. That code would probably be inlined anyway
by most of compilers today, so no harm & LGTM.

Reviewed-by: Lukasz Luba <[email protected]>