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!
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
---
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;
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
---
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++)
On 03/10/2023 15:18, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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 <[email protected]>
> ---
Acked-by: Daniel Lezcano <[email protected]>
--
<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
On 03/10/2023 15:21, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> 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 <[email protected]>
> ---
Reviewed-by: Daniel Lezcano <[email protected]>
--
<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
On Wed, Oct 4, 2023 at 6:30 PM Daniel Lezcano <[email protected]> wrote:
>
> On 03/10/2023 15:21, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > 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 <[email protected]>
> > ---
>
> Reviewed-by: Daniel Lezcano <[email protected]>
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!