[PATCH v2 0/6] thermal: Improve iteration over trip points

[PATCH v2 0/6] thermal: Improve iteration over trip points

[Rafael J. Wysocki]

Hi All,

This is a new version of

https://lore.kernel.org/linux-acpi/4871671.31r3eYUQgx@kreacher

which is being posted, because I've realized that the ACPI piece could be
somewhat simpler.  Namely, it is not really necessary to store the indices of
active trip points in the ACPI thermal driver (so as to use them to invoke the
appropriate ACPI methods), because they can be readily computed, but IMO it is
better to make changes in more steps in order to use this observation.

This is still true:

 It turns out that the notification-handling code in the ACPI thermal driver
 can be rearranged to iterate over trip points once, with the help of
 for_each_thermal_trip() called directly under the zone lock, so patch [1/6]
 adds a helper function for that.

This time, however, more changes are made in order to use this new function in
the ACPI thermal driver:
 * One function is relocated (so that the subsequent changes look cleaner)
   in patch [2/6].
 * Two functions are merged into one (so as to prepare the code for the next
   change) in patch [3/4]
 * Patch [4/6] changes the ACPI thermal driver to use the function introduced
   in patch [1/6] and to reduce the number of trip point walks after a
   notification from the platform firmware from 2 to 1.

Next, patch [5/6] drops thermal_zone_device_exec() that is not used any more
and patch [6/6] changes the int340x thermal driver to also use the new helper
to iterate over trip points, so it need not make risky assumptions regarding
the core functionality.

Please see the individual patch changelogs for details.

Thanks!

[PATCH v2 1/6] thermal: core: Add function to walk trips under zone lock

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add a wrapper around for_each_thermal_trip(), called
thermal_zone_for_each_trip(), that will invoke the former under the
thermal zone lock and pass its return value to the caller.

Two drivers will be modified subsequently to use this new function.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2: Mo changes

---
 drivers/thermal/thermal_trip.c |   14 ++++++++++++++
 include/linux/thermal.h        |    3 +++
 2 files changed, 17 insertions(+)

Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -27,6 +27,20 @@ int for_each_thermal_trip(struct thermal
 }
 EXPORT_SYMBOL_GPL(for_each_thermal_trip);
 
+int thermal_zone_for_each_trip(struct thermal_zone_device *tz,
+			       int (*cb)(struct thermal_trip *, void *),
+			       void *data)
+{
+	int ret;
+
+	mutex_lock(&tz->lock);
+	ret = for_each_thermal_trip(tz, cb, data);
+	mutex_unlock(&tz->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_for_each_trip);
+
 int thermal_zone_get_num_trips(struct thermal_zone_device *tz)
 {
 	return tz->num_trips;
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -287,6 +287,9 @@ int thermal_zone_set_trip(struct thermal
 int for_each_thermal_trip(struct thermal_zone_device *tz,
 			  int (*cb)(struct thermal_trip *, void *),
 			  void *data);
+int thermal_zone_for_each_trip(struct thermal_zone_device *tz,
+			       int (*cb)(struct thermal_trip *, void *),
+			       void *data);
 int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
 
 int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp);

Re: [PATCH v2 1/6] thermal: core: Add function to walk trips under zone lock

[Daniel Lezcano]

On 03/10/2023 15:17, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Add a wrapper around for_each_thermal_trip(), called
> thermal_zone_for_each_trip(), that will invoke the former under the
> thermal zone lock and pass its return value to the caller.
> 
> Two drivers will be modified subsequently to use this new function.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

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

[PATCH v2 2/6] ACPI: thermal: Move get_active_temp()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Put the get_active_temp() function next to the analogous
get_passive_temp() one to allow subsequent changes to be easier to
follow.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/thermal.c |   46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -189,6 +189,29 @@ static long get_passive_temp(struct acpi
 	return tmp;
 }
 
+static long get_active_temp(struct acpi_thermal *tz, int index)
+{
+	char method[] = { '_', 'A', 'C', '0' + index, '\0' };
+	unsigned long long tmp;
+	acpi_status status;
+
+	status = acpi_evaluate_integer(tz->device->handle, method, NULL, &tmp);
+	if (ACPI_FAILURE(status))
+		return THERMAL_TEMP_INVALID;
+
+	/*
+	 * If an override has been provided, apply it so there are no active
+	 * trips with thresholds greater than the override.
+	 */
+	if (act > 0) {
+		unsigned long long override = celsius_to_deci_kelvin(act);
+
+		if (tmp > override)
+			tmp = override;
+	}
+	return tmp;
+}
+
 static void acpi_thermal_update_passive_trip(struct acpi_thermal *tz)
 {
 	struct acpi_thermal_trip *acpi_trip = &tz->trips.passive.trip;
@@ -250,29 +273,6 @@ static void acpi_thermal_update_trip_dev
 	ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
-static long get_active_temp(struct acpi_thermal *tz, int index)
-{
-	char method[] = { '_', 'A', 'C', '0' + index, '\0' };
-	unsigned long long tmp;
-	acpi_status status;
-
-	status = acpi_evaluate_integer(tz->device->handle, method, NULL, &tmp);
-	if (ACPI_FAILURE(status))
-		return THERMAL_TEMP_INVALID;
-
-	/*
-	 * If an override has been provided, apply it so there are no active
-	 * trips with thresholds greater than the override.
-	 */
-	if (act > 0) {
-		unsigned long long override = celsius_to_deci_kelvin(act);
-
-		if (tmp > override)
-			tmp = override;
-	}
-	return tmp;
-}
-
 static void acpi_thermal_update_active_trip(struct acpi_thermal *tz, int index)
 {
 	struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;

Re: [PATCH v2 2/6] ACPI: thermal: Move get_active_temp()

[Daniel Lezcano]

On 03/10/2023 15:18, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Put the get_active_temp() function next to the analogous
> get_passive_temp() one to allow subsequent changes to be easier to
> follow.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


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

[PATCH v2 3/6] ACPI: thermal: Combine passive and active trip update functions

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Combine acpi_thermal_update_passive_trip() and
acpi_thermal_update_active_trip() into one common function called
acpi_thermal_update_trip(), so as to reduce code duplication and
prepare the code in question for subsequent changes.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/thermal.c |   35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -212,14 +212,25 @@ static long get_active_temp(struct acpi_
 	return tmp;
 }
 
-static void acpi_thermal_update_passive_trip(struct acpi_thermal *tz)
+static void acpi_thermal_update_trip(struct acpi_thermal *tz,
+				     int index)
 {
-	struct acpi_thermal_trip *acpi_trip = &tz->trips.passive.trip;
+	struct acpi_thermal_trip *acpi_trip;
 
-	if (!acpi_thermal_trip_valid(acpi_trip) || psv > 0)
+	acpi_trip = index == ACPI_THERMAL_TRIP_PASSIVE ?
+			&tz->trips.passive.trip : &tz->trips.active[index].trip;
+	if (!acpi_thermal_trip_valid(acpi_trip))
 		return;
 
-	acpi_trip->temp_dk = get_passive_temp(tz);
+	if (index == ACPI_THERMAL_TRIP_PASSIVE) {
+		if (psv > 0)
+			return;
+
+		acpi_trip->temp_dk = get_passive_temp(tz);
+	} else {
+		acpi_trip->temp_dk = get_active_temp(tz, index);
+	}
+
 	if (!acpi_thermal_trip_valid(acpi_trip))
 		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
@@ -273,18 +284,6 @@ static void acpi_thermal_update_trip_dev
 	ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
-static void acpi_thermal_update_active_trip(struct acpi_thermal *tz, int index)
-{
-	struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip;
-
-	if (!acpi_thermal_trip_valid(acpi_trip))
-		return;
-
-	acpi_trip->temp_dk = get_active_temp(tz, index);
-	if (!acpi_thermal_trip_valid(acpi_trip))
-		ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
-}
-
 static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data)
 {
 	struct acpi_thermal_trip *acpi_trip = trip->priv;
@@ -308,9 +307,9 @@ static void acpi_thermal_adjust_thermal_
 	int i;
 
 	if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) {
-		acpi_thermal_update_passive_trip(tz);
+		acpi_thermal_update_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
 		for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
-			acpi_thermal_update_active_trip(tz, i);
+			acpi_thermal_update_trip(tz, i);
 	} else {
 		acpi_thermal_update_trip_devices(tz, ACPI_THERMAL_TRIP_PASSIVE);
 		for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)

Re: [PATCH v2 3/6] ACPI: thermal: Combine passive and active trip update functions

[Daniel Lezcano]

On 03/10/2023 15:21, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Combine acpi_thermal_update_passive_trip() and
> acpi_thermal_update_active_trip() into one common function called
> acpi_thermal_update_trip(), so as to reduce code duplication and
> prepare the code in question for subsequent changes.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

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

Re: [PATCH v2 3/6] ACPI: thermal: Combine passive and active trip update functions

[Rafael J. Wysocki]

On Wed, Oct 4, 2023 at 6:30 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 03/10/2023 15:21, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Combine acpi_thermal_update_passive_trip() and
> > acpi_thermal_update_active_trip() into one common function called
> > acpi_thermal_update_trip(), so as to reduce code duplication and
> > prepare the code in question for subsequent changes.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Thanks for all of the reviews, much appreciated!

I'm going to apply the series now unless you have strong objections
against any of the remaining patches [4,6/6].

Cheers!

[PATCH v2 4/6] ACPI: thermal: Use thermal_zone_for_each_trip() for updating trips

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Rearrange the code handling notifications from the platform firmware
regarding trip point updates to carry out one loop over trip points
instead of two of them by using thermal_zone_for_each_trip() for that,
which is more straightforward than using a combination of
thermal_zone_device_exec() and for_each_thermal_trip(), each with its
own callback function.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Rebase on top of the previous patches.
   * Add active_trip_index() for computing active trip point indices instead
     of storing them.

---
 drivers/acpi/thermal.c |   78 +++++++++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 41 deletions(-)

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -177,6 +177,15 @@ static bool acpi_thermal_trip_valid(stru
 	return acpi_trip->temp_dk != THERMAL_TEMP_INVALID;
 }
 
+static int active_trip_index(struct acpi_thermal *tz,
+			     struct acpi_thermal_trip *acpi_trip)
+{
+	struct acpi_thermal_active *active;
+
+	active = container_of(acpi_trip, struct acpi_thermal_active, trip);
+	return active - tz->trips.active;
+}
+
 static long get_passive_temp(struct acpi_thermal *tz)
 {
 	unsigned long long tmp;
@@ -213,21 +222,18 @@ static long get_active_temp(struct acpi_
 }
 
 static void acpi_thermal_update_trip(struct acpi_thermal *tz,
-				     int index)
+				     const struct thermal_trip *trip)
 {
-	struct acpi_thermal_trip *acpi_trip;
-
-	acpi_trip = index == ACPI_THERMAL_TRIP_PASSIVE ?
-			&tz->trips.passive.trip : &tz->trips.active[index].trip;
-	if (!acpi_thermal_trip_valid(acpi_trip))
-		return;
+	struct acpi_thermal_trip *acpi_trip = trip->priv;
 
-	if (index == ACPI_THERMAL_TRIP_PASSIVE) {
+	if (trip->type == THERMAL_TRIP_PASSIVE) {
 		if (psv > 0)
 			return;
 
 		acpi_trip->temp_dk = get_passive_temp(tz);
 	} else {
+		int index = active_trip_index(tz, acpi_trip);
+
 		acpi_trip->temp_dk = get_active_temp(tz, index);
 	}
 
@@ -267,31 +273,39 @@ static bool update_trip_devices(struct a
 	return true;
 }
 
-static void acpi_thermal_update_trip_devices(struct acpi_thermal *tz, int index)
+static void acpi_thermal_update_trip_devices(struct acpi_thermal *tz,
+					     struct thermal_trip *trip)
 {
-	struct acpi_thermal_trip *acpi_trip;
-
-	acpi_trip = index == ACPI_THERMAL_TRIP_PASSIVE ?
-			&tz->trips.passive.trip : &tz->trips.active[index].trip;
-	if (!acpi_thermal_trip_valid(acpi_trip))
-		return;
+	struct acpi_thermal_trip *acpi_trip = trip->priv;
+	int index = trip->type == THERMAL_TRIP_PASSIVE ?
+			ACPI_THERMAL_TRIP_PASSIVE : active_trip_index(tz, acpi_trip);
 
-	if (update_trip_devices(tz, acpi_trip, index, true)) {
+	if (update_trip_devices(tz, acpi_trip, index, true))
 		return;
-	}
 
 	acpi_trip->temp_dk = THERMAL_TEMP_INVALID;
 	ACPI_THERMAL_TRIPS_EXCEPTION(tz, "state");
 }
 
+struct adjust_trip_data {
+	struct acpi_thermal *tz;
+	u32 event;
+};
+
 static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data)
 {
 	struct acpi_thermal_trip *acpi_trip = trip->priv;
-	struct acpi_thermal *tz = data;
+	struct adjust_trip_data *atd = data;
+	struct acpi_thermal *tz = atd->tz;
 
-	if (!acpi_trip)
+	if (!acpi_trip || !acpi_thermal_trip_valid(acpi_trip))
 		return 0;
 
+	if (atd->event == ACPI_THERMAL_NOTIFY_THRESHOLDS)
+		acpi_thermal_update_trip(tz, trip);
+	else
+		acpi_thermal_update_trip_devices(tz, trip);
+
 	if (acpi_thermal_trip_valid(acpi_trip))
 		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk);
 	else
@@ -300,25 +314,6 @@ static int acpi_thermal_adjust_trip(stru
 	return 0;
 }
 
-static void acpi_thermal_adjust_thermal_zone(struct thermal_zone_device *thermal,
-					     unsigned long data)
-{
-	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
-	int i;
-
-	if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) {
-		acpi_thermal_update_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
-		for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
-			acpi_thermal_update_trip(tz, i);
-	} else {
-		acpi_thermal_update_trip_devices(tz, ACPI_THERMAL_TRIP_PASSIVE);
-		for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
-			acpi_thermal_update_trip_devices(tz, i);
-	}
-
-	for_each_thermal_trip(tz->thermal_zone, acpi_thermal_adjust_trip, tz);
-}
-
 static void acpi_queue_thermal_check(struct acpi_thermal *tz)
 {
 	if (!work_pending(&tz->thermal_check_work))
@@ -327,17 +322,18 @@ static void acpi_queue_thermal_check(str
 
 static void acpi_thermal_trips_update(struct acpi_thermal *tz, u32 event)
 {
+	struct adjust_trip_data atd = { .tz = tz, .event = event };
 	struct acpi_device *adev = tz->device;
 
 	/*
-	 * Use thermal_zone_device_exec() to carry out the trip points
+	 * Use thermal_zone_for_each_trip() to carry out the trip points
 	 * update, so as to protect thermal_get_trend() from getting stale
 	 * trip point temperatures and to prevent thermal_zone_device_update()
 	 * invoked from acpi_thermal_check_fn() from producing inconsistent
 	 * results.
 	 */
-	thermal_zone_device_exec(tz->thermal_zone,
-				 acpi_thermal_adjust_thermal_zone, event);
+	thermal_zone_for_each_trip(tz->thermal_zone,
+				   acpi_thermal_adjust_trip, &atd);
 	acpi_queue_thermal_check(tz);
 	acpi_bus_generate_netlink_event(adev->pnp.device_class,
 					dev_name(&adev->dev), event, 0);

Re: [PATCH v2 4/6] ACPI: thermal: Use thermal_zone_for_each_trip() for updating trips

[Daniel Lezcano]

On 03/10/2023 15:24, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Rearrange the code handling notifications from the platform firmware
> regarding trip point updates to carry out one loop over trip points
> instead of two of them by using thermal_zone_for_each_trip() for that,
> which is more straightforward than using a combination of
> thermal_zone_device_exec() and for_each_thermal_trip(), each with its
> own callback function.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

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

[PATCH v2 5/6] thermal: core: Drop thermal_zone_device_exec()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because thermal_zone_device_exec() has no users any more and there are
no plans to use it anywhere, revert commit 9a99a996d1ec ("thermal: core:
Introduce thermal_zone_device_exec()") that introduced it.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2: No changes

---
 drivers/thermal/thermal_core.c |   19 -------------------
 include/linux/thermal.h        |    4 ----
 2 files changed, 23 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -493,25 +493,6 @@ void thermal_zone_device_update(struct t
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device_update);
 
-/**
- * thermal_zone_device_exec - Run a callback under the zone lock.
- * @tz: Thermal zone.
- * @cb: Callback to run.
- * @data: Data to pass to the callback.
- */
-void thermal_zone_device_exec(struct thermal_zone_device *tz,
-			      void (*cb)(struct thermal_zone_device *,
-					 unsigned long),
-			      unsigned long data)
-{
-	mutex_lock(&tz->lock);
-
-	cb(tz, data);
-
-	mutex_unlock(&tz->lock);
-}
-EXPORT_SYMBOL_GPL(thermal_zone_device_exec);
-
 static void thermal_zone_device_check(struct work_struct *work)
 {
 	struct thermal_zone_device *tz = container_of(work, struct
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -340,10 +340,6 @@ int thermal_zone_unbind_cooling_device(s
 				       struct thermal_cooling_device *);
 void thermal_zone_device_update(struct thermal_zone_device *,
 				enum thermal_notify_event);
-void thermal_zone_device_exec(struct thermal_zone_device *tz,
-			      void (*cb)(struct thermal_zone_device *,
-					 unsigned long),
-			      unsigned long data);
 
 struct thermal_cooling_device *thermal_cooling_device_register(const char *,
 		void *, const struct thermal_cooling_device_ops *);

Re: [PATCH v2 5/6] thermal: core: Drop thermal_zone_device_exec()

[Daniel Lezcano]

On 03/10/2023 15:25, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Because thermal_zone_device_exec() has no users any more and there are
> no plans to use it anywhere, revert commit 9a99a996d1ec ("thermal: core:
> Introduce thermal_zone_device_exec()") that introduced it.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


> ---
> 
> v1 -> v2: No changes
> 
> ---
>   drivers/thermal/thermal_core.c |   19 -------------------
>   include/linux/thermal.h        |    4 ----
>   2 files changed, 23 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -493,25 +493,6 @@ void thermal_zone_device_update(struct t
>   }
>   EXPORT_SYMBOL_GPL(thermal_zone_device_update);
>   
> -/**
> - * thermal_zone_device_exec - Run a callback under the zone lock.
> - * @tz: Thermal zone.
> - * @cb: Callback to run.
> - * @data: Data to pass to the callback.
> - */
> -void thermal_zone_device_exec(struct thermal_zone_device *tz,
> -			      void (*cb)(struct thermal_zone_device *,
> -					 unsigned long),
> -			      unsigned long data)
> -{
> -	mutex_lock(&tz->lock);
> -
> -	cb(tz, data);
> -
> -	mutex_unlock(&tz->lock);
> -}
> -EXPORT_SYMBOL_GPL(thermal_zone_device_exec);
> -
>   static void thermal_zone_device_check(struct work_struct *work)
>   {
>   	struct thermal_zone_device *tz = container_of(work, struct
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -340,10 +340,6 @@ int thermal_zone_unbind_cooling_device(s
>   				       struct thermal_cooling_device *);
>   void thermal_zone_device_update(struct thermal_zone_device *,
>   				enum thermal_notify_event);
> -void thermal_zone_device_exec(struct thermal_zone_device *tz,
> -			      void (*cb)(struct thermal_zone_device *,
> -					 unsigned long),
> -			      unsigned long data);
>   
>   struct thermal_cooling_device *thermal_cooling_device_register(const char *,
>   		void *, const struct thermal_cooling_device_ops *);
> 
> 
> 

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

[PATCH v2 6/6] thermal: int340x: Use thermal_zone_for_each_trip()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Modify int340x_thermal_update_trips() to use thermal_zone_for_each_trip()
for walking trips instead of using the trips[] table passed to the
thermal zone registration function.

For this purpose, store active trip point indices in the priv fieids of
the corresponding thermal_trip structures.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2: Mo changes

---
 drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c |   76 +++++------
 1 file changed, 41 insertions(+), 35 deletions(-)

Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -67,6 +67,16 @@ static struct thermal_zone_device_ops in
 	.critical	= int340x_thermal_critical,
 };
 
+static inline void *int_to_trip_priv(int i)
+{
+	return (void *)(long)i;
+}
+
+static inline int trip_priv_to_int(const struct thermal_trip *trip)
+{
+	return (long)trip->priv;
+}
+
 static int int340x_thermal_read_trips(struct acpi_device *zone_adev,
 				      struct thermal_trip *zone_trips,
 				      int trip_cnt)
@@ -101,6 +111,7 @@ static int int340x_thermal_read_trips(st
 			break;
 
 		zone_trips[trip_cnt].type = THERMAL_TRIP_ACTIVE;
+		zone_trips[trip_cnt].priv = int_to_trip_priv(i);
 		trip_cnt++;
 	}
 
@@ -212,45 +223,40 @@ void int340x_thermal_zone_remove(struct
 }
 EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
 
-void int340x_thermal_update_trips(struct int34x_thermal_zone *int34x_zone)
+static int int340x_update_one_trip(struct thermal_trip *trip, void *arg)
 {
-	struct acpi_device *zone_adev = int34x_zone->adev;
-	struct thermal_trip *zone_trips = int34x_zone->trips;
-	int trip_cnt = int34x_zone->zone->num_trips;
-	int act_trip_nr = 0;
-	int i;
-
-	mutex_lock(&int34x_zone->zone->lock);
-
-	for (i = int34x_zone->aux_trip_nr; i < trip_cnt; i++) {
-		int temp, err;
-
-		switch (zone_trips[i].type) {
-		case THERMAL_TRIP_CRITICAL:
-			err = thermal_acpi_critical_trip_temp(zone_adev, &temp);
-			break;
-		case THERMAL_TRIP_HOT:
-			err = thermal_acpi_hot_trip_temp(zone_adev, &temp);
-			break;
-		case THERMAL_TRIP_PASSIVE:
-			err = thermal_acpi_passive_trip_temp(zone_adev, &temp);
-			break;
-		case THERMAL_TRIP_ACTIVE:
-			err = thermal_acpi_active_trip_temp(zone_adev, act_trip_nr++,
-							    &temp);
-			break;
-		default:
-			err = -ENODEV;
-		}
-		if (err) {
-			zone_trips[i].temperature = THERMAL_TEMP_INVALID;
-			continue;
-		}
+	struct acpi_device *zone_adev = arg;
+	int temp, err;
 
-		zone_trips[i].temperature = temp;
+	switch (trip->type) {
+	case THERMAL_TRIP_CRITICAL:
+		err = thermal_acpi_critical_trip_temp(zone_adev, &temp);
+		break;
+	case THERMAL_TRIP_HOT:
+		err = thermal_acpi_hot_trip_temp(zone_adev, &temp);
+		break;
+	case THERMAL_TRIP_PASSIVE:
+		err = thermal_acpi_passive_trip_temp(zone_adev, &temp);
+		break;
+	case THERMAL_TRIP_ACTIVE:
+		err = thermal_acpi_active_trip_temp(zone_adev,
+						    trip_priv_to_int(trip),
+						    &temp);
+		break;
+	default:
+		err = -ENODEV;
 	}
+	if (err)
+		temp = THERMAL_TEMP_INVALID;
 
-	mutex_unlock(&int34x_zone->zone->lock);
+	trip->temperature = temp;
+	return 0;
+}
+
+void int340x_thermal_update_trips(struct int34x_thermal_zone *int34x_zone)
+{
+	thermal_zone_for_each_trip(int34x_zone->zone, int340x_update_one_trip,
+				   int34x_zone->adev);
 }
 EXPORT_SYMBOL_GPL(int340x_thermal_update_trips);
 

Re: [PATCH v2 6/6] thermal: int340x: Use thermal_zone_for_each_trip()

[Daniel Lezcano]

On 03/10/2023 15:26, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Modify int340x_thermal_update_trips() to use thermal_zone_for_each_trip()
> for walking trips instead of using the trips[] table passed to the
> thermal zone registration function.
> 
> For this purpose, store active trip point indices in the priv fieids of
> the corresponding thermal_trip structures.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> v1 -> v2: Mo changes
> 
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

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