* [PATCH v1 1/6] thermal: core: Store zone trips table in struct thermal_zone_device
2024-02-05 21:12 [PATCH v1 0/6] thermal: Store trips table and ops in thermal_zone_device Rafael J. Wysocki
@ 2024-02-05 21:14 ` Rafael J. Wysocki
2024-02-08 8:22 ` Stanislaw Gruszka
2024-02-05 21:15 ` [PATCH v1 2/6] thermal: ACPI: Discard trip table after zone registration Rafael J. Wysocki
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-02-05 21:14 UTC (permalink / raw)
To: Linux PM
Cc: Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba, Zhang Rui,
Srinivas Pandruvada, Stanislaw Gruszka,
AngeloGioacchino Del Regno
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The current code requires thermal zone creators to pass a pointer to a
writable trips table to thermal_zone_device_register_with_trips() and
that trips table is then used by the thermal core going forward.
Consequently, the callers of thermal_zone_device_register_with_trips()
are required to hold on to the trips table passed to it until the given
thermal zone is unregistered, at which point the trips table can be
freed, but at the same time they are not allowed to access the cells in
that table directly. This is both error prone and confusing.
To address it, turn the trips table pointer in struct thermal_zone_device
into a flex array (counted by its num_trips field), allocate it during
thermal zone device allocation and copy the contents of the trips table
supplied by the zone creator (which can be const now) into it.
This allows the callers of thermal_zone_device_register_with_trips() to
drop their trip tables right after the zone registration.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/thermal/thermal_core.c | 16 +++++++++-------
include/linux/thermal.h | 10 +++++-----
2 files changed, 14 insertions(+), 12 deletions(-)
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -130,7 +130,6 @@ struct thermal_cooling_device {
* @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
* @mode: current mode of this thermal zone
* @devdata: private pointer for device private data
- * @trips: an array of struct thermal_trip
* @num_trips: number of trip points the thermal zone supports
* @passive_delay_jiffies: number of jiffies to wait between polls when
* performing passive cooling.
@@ -160,6 +159,7 @@ struct thermal_cooling_device {
* @poll_queue: delayed work for polling
* @notify_event: Last notification event
* @suspended: thermal zone suspend indicator
+ * @trips: array of struct thermal_trip objects
*/
struct thermal_zone_device {
int id;
@@ -172,7 +172,6 @@ struct thermal_zone_device {
struct thermal_attr *trip_hyst_attrs;
enum thermal_device_mode mode;
void *devdata;
- struct thermal_trip *trips;
int num_trips;
unsigned long passive_delay_jiffies;
unsigned long polling_delay_jiffies;
@@ -193,10 +192,11 @@ struct thermal_zone_device {
struct list_head node;
struct delayed_work poll_queue;
enum thermal_notify_event notify_event;
+ bool suspended;
#ifdef CONFIG_THERMAL_DEBUGFS
struct thermal_debugfs *debugfs;
#endif
- bool suspended;
+ struct thermal_trip trips[] __counted_by(num_trips);
};
/**
@@ -315,7 +315,7 @@ int thermal_zone_get_crit_temp(struct th
#ifdef CONFIG_THERMAL
struct thermal_zone_device *thermal_zone_device_register_with_trips(
const char *type,
- struct thermal_trip *trips,
+ const struct thermal_trip *trips,
int num_trips, int mask,
void *devdata,
struct thermal_zone_device_ops *ops,
@@ -375,7 +375,7 @@ void thermal_zone_device_critical(struct
#else
static inline struct thermal_zone_device *thermal_zone_device_register_with_trips(
const char *type,
- struct thermal_trip *trips,
+ const struct thermal_trip *trips,
int num_trips, int mask,
void *devdata,
struct thermal_zone_device_ops *ops,
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1272,10 +1272,13 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
* IS_ERR*() helpers.
*/
struct thermal_zone_device *
-thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int num_trips, int mask,
- void *devdata, struct thermal_zone_device_ops *ops,
- const struct thermal_zone_params *tzp, int passive_delay,
- int polling_delay)
+thermal_zone_device_register_with_trips(const char *type,
+ const struct thermal_trip *trips,
+ int num_trips, int mask,
+ void *devdata,
+ struct thermal_zone_device_ops *ops,
+ const struct thermal_zone_params *tzp,
+ int passive_delay, int polling_delay)
{
struct thermal_zone_device *tz;
int id;
@@ -1322,7 +1325,7 @@ thermal_zone_device_register_with_trips(
if (!thermal_class)
return ERR_PTR(-ENODEV);
- tz = kzalloc(sizeof(*tz), GFP_KERNEL);
+ tz = kzalloc(struct_size(tz, trips, num_trips), GFP_KERNEL);
if (!tz)
return ERR_PTR(-ENOMEM);
@@ -1344,7 +1347,6 @@ thermal_zone_device_register_with_trips(
result = id;
goto free_tzp;
}
-
tz->id = id;
strscpy(tz->type, type, sizeof(tz->type));
@@ -1354,7 +1356,7 @@ thermal_zone_device_register_with_trips(
tz->ops = ops;
tz->device.class = thermal_class;
tz->devdata = devdata;
- tz->trips = trips;
+ memcpy(tz->trips, trips, num_trips * sizeof(trips[0]));
tz->num_trips = num_trips;
thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/6] thermal: core: Store zone trips table in struct thermal_zone_device
2024-02-05 21:14 ` [PATCH v1 1/6] thermal: core: Store zone trips table in struct thermal_zone_device Rafael J. Wysocki
@ 2024-02-08 8:22 ` Stanislaw Gruszka
2024-02-08 19:25 ` Rafael J. Wysocki
0 siblings, 1 reply; 14+ messages in thread
From: Stanislaw Gruszka @ 2024-02-08 8:22 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba,
Zhang Rui, Srinivas Pandruvada, AngeloGioacchino Del Regno
On Mon, Feb 05, 2024 at 10:14:31PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The current code requires thermal zone creators to pass a pointer to a
> writable trips table to thermal_zone_device_register_with_trips() and
> that trips table is then used by the thermal core going forward.
>
> Consequently, the callers of thermal_zone_device_register_with_trips()
> are required to hold on to the trips table passed to it until the given
> thermal zone is unregistered, at which point the trips table can be
> freed, but at the same time they are not allowed to access the cells in
> that table directly. This is both error prone and confusing.
>
> To address it, turn the trips table pointer in struct thermal_zone_device
> into a flex array (counted by its num_trips field), allocate it during
> thermal zone device allocation and copy the contents of the trips table
> supplied by the zone creator (which can be const now) into it.
>
> This allows the callers of thermal_zone_device_register_with_trips() to
> drop their trip tables right after the zone registration.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/thermal/thermal_core.c | 16 +++++++++-------
> include/linux/thermal.h | 10 +++++-----
> 2 files changed, 14 insertions(+), 12 deletions(-)
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -130,7 +130,6 @@ struct thermal_cooling_device {
> * @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis
> * @mode: current mode of this thermal zone
> * @devdata: private pointer for device private data
> - * @trips: an array of struct thermal_trip
> * @num_trips: number of trip points the thermal zone supports
> * @passive_delay_jiffies: number of jiffies to wait between polls when
> * performing passive cooling.
> @@ -160,6 +159,7 @@ struct thermal_cooling_device {
> * @poll_queue: delayed work for polling
> * @notify_event: Last notification event
> * @suspended: thermal zone suspend indicator
> + * @trips: array of struct thermal_trip objects
> */
> struct thermal_zone_device {
> int id;
> @@ -172,7 +172,6 @@ struct thermal_zone_device {
> struct thermal_attr *trip_hyst_attrs;
> enum thermal_device_mode mode;
> void *devdata;
> - struct thermal_trip *trips;
> int num_trips;
> unsigned long passive_delay_jiffies;
> unsigned long polling_delay_jiffies;
> @@ -193,10 +192,11 @@ struct thermal_zone_device {
> struct list_head node;
> struct delayed_work poll_queue;
> enum thermal_notify_event notify_event;
> + bool suspended;
> #ifdef CONFIG_THERMAL_DEBUGFS
> struct thermal_debugfs *debugfs;
> #endif
> - bool suspended;
> + struct thermal_trip trips[] __counted_by(num_trips);
> };
>
> /**
> @@ -315,7 +315,7 @@ int thermal_zone_get_crit_temp(struct th
> #ifdef CONFIG_THERMAL
> struct thermal_zone_device *thermal_zone_device_register_with_trips(
> const char *type,
> - struct thermal_trip *trips,
> + const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> struct thermal_zone_device_ops *ops,
> @@ -375,7 +375,7 @@ void thermal_zone_device_critical(struct
> #else
> static inline struct thermal_zone_device *thermal_zone_device_register_with_trips(
> const char *type,
> - struct thermal_trip *trips,
> + const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> struct thermal_zone_device_ops *ops,
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1272,10 +1272,13 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
> * IS_ERR*() helpers.
> */
> struct thermal_zone_device *
> -thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int num_trips, int mask,
> - void *devdata, struct thermal_zone_device_ops *ops,
> - const struct thermal_zone_params *tzp, int passive_delay,
> - int polling_delay)
> +thermal_zone_device_register_with_trips(const char *type,
> + const struct thermal_trip *trips,
> + int num_trips, int mask,
> + void *devdata,
> + struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_params *tzp,
> + int passive_delay, int polling_delay)
> {
> struct thermal_zone_device *tz;
> int id;
> @@ -1322,7 +1325,7 @@ thermal_zone_device_register_with_trips(
> if (!thermal_class)
> return ERR_PTR(-ENODEV);
>
> - tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> + tz = kzalloc(struct_size(tz, trips, num_trips), GFP_KERNEL);
> if (!tz)
> return ERR_PTR(-ENOMEM);
>
> @@ -1344,7 +1347,6 @@ thermal_zone_device_register_with_trips(
> result = id;
> goto free_tzp;
> }
> -
> tz->id = id;
> strscpy(tz->type, type, sizeof(tz->type));
>
> @@ -1354,7 +1356,7 @@ thermal_zone_device_register_with_trips(
> tz->ops = ops;
> tz->device.class = thermal_class;
> tz->devdata = devdata;
> - tz->trips = trips;
> + memcpy(tz->trips, trips, num_trips * sizeof(trips[0]));
> tz->num_trips = num_trips;
>
> thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/6] thermal: core: Store zone trips table in struct thermal_zone_device
2024-02-08 8:22 ` Stanislaw Gruszka
@ 2024-02-08 19:25 ` Rafael J. Wysocki
0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-02-08 19:25 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Rafael J. Wysocki, Linux PM, Daniel Lezcano, LKML, Linux ACPI,
Lukasz Luba, Zhang Rui, Srinivas Pandruvada,
AngeloGioacchino Del Regno
On Thu, Feb 8, 2024 at 9:22 AM Stanislaw Gruszka
<stanislaw.gruszka@linux.intel.com> wrote:
>
> On Mon, Feb 05, 2024 at 10:14:31PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > The current code requires thermal zone creators to pass a pointer to a
> > writable trips table to thermal_zone_device_register_with_trips() and
> > that trips table is then used by the thermal core going forward.
> >
> > Consequently, the callers of thermal_zone_device_register_with_trips()
> > are required to hold on to the trips table passed to it until the given
> > thermal zone is unregistered, at which point the trips table can be
> > freed, but at the same time they are not allowed to access the cells in
> > that table directly. This is both error prone and confusing.
> >
> > To address it, turn the trips table pointer in struct thermal_zone_device
> > into a flex array (counted by its num_trips field), allocate it during
> > thermal zone device allocation and copy the contents of the trips table
> > supplied by the zone creator (which can be const now) into it.
> >
> > This allows the callers of thermal_zone_device_register_with_trips() to
> > drop their trip tables right after the zone registration.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Thanks a lot for all of the reviews, much appreciated, especially
regarding the Intel drivers changes.
Unfortunately, this patch series, and the first half of it in
particular, is somewhat premature, because a couple of thermal drivers
do unexpected things to their trip point tables and they need to be
modified to stop accessing the trip tables directly before the core
can start using internal copies of them.
I'm going to save the tags for the future, however.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 2/6] thermal: ACPI: Discard trip table after zone registration
2024-02-05 21:12 [PATCH v1 0/6] thermal: Store trips table and ops in thermal_zone_device Rafael J. Wysocki
2024-02-05 21:14 ` [PATCH v1 1/6] thermal: core: Store zone trips table in struct thermal_zone_device Rafael J. Wysocki
@ 2024-02-05 21:15 ` Rafael J. Wysocki
2024-02-08 8:22 ` Stanislaw Gruszka
2024-02-05 21:16 ` [PATCH v1 3/6] thermal: intel: Discard trip tables " Rafael J. Wysocki
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-02-05 21:15 UTC (permalink / raw)
To: Linux PM
Cc: Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba, Zhang Rui,
Srinivas Pandruvada, Stanislaw Gruszka,
AngeloGioacchino Del Regno
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Because the thermal core creates and uses its own copy of the trips
table passed to thermal_zone_device_register_with_trips(), it is not
necessary to hold on to a local copy of it any more after the given
thermal zone has been registered.
Accordingly, modify the ACPI thermal driver to store the trips table
passed to thermal_zone_device_register_with_trips() in a local variable
which is automatically discarded after the zone registration.
Also make some additional code simplifications unlocked by the above
change.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/thermal.c | 57 +++++++++++++++++--------------------------------
1 file changed, 20 insertions(+), 37 deletions(-)
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -47,6 +47,8 @@
#define ACPI_THERMAL_TRIP_PASSIVE (-1)
+#define ACPI_THERMAL_MAX_NR_TRIPS (ACPI_THERMAL_MAX_ACTIVE + 3)
+
/*
* This exception is thrown out in two cases:
* 1.An invalid trip point becomes invalid or a valid trip point becomes invalid
@@ -112,7 +114,6 @@ struct acpi_thermal {
unsigned long polling_frequency;
volatile u8 zombie;
struct acpi_thermal_trips trips;
- struct thermal_trip *trip_table;
struct thermal_zone_device *thermal_zone;
int kelvin_offset; /* in millidegrees */
struct work_struct thermal_check_work;
@@ -451,26 +452,19 @@ fail:
return false;
}
-static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
+static void acpi_thermal_get_trip_points(struct acpi_thermal *tz)
{
- unsigned int count = 0;
int i;
- if (acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE))
- count++;
+ acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
- if (acpi_thermal_init_trip(tz, i))
- count++;
- else
+ if (!acpi_thermal_init_trip(tz, i))
break;
-
}
while (++i < ACPI_THERMAL_MAX_ACTIVE)
tz->trips.active[i].trip.temp_dk = THERMAL_TEMP_INVALID;
-
- return count;
}
/* sys I/F for generic thermal sysfs support */
@@ -662,13 +656,14 @@ static void acpi_thermal_zone_sysfs_remo
}
static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
+ const struct thermal_trip *trip_table,
unsigned int trip_count,
int passive_delay)
{
int result;
tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
- tz->trip_table,
+ trip_table,
trip_count,
0, tz,
&acpi_thermal_zone_ops,
@@ -823,10 +818,10 @@ static void acpi_thermal_free_thermal_zo
static int acpi_thermal_add(struct acpi_device *device)
{
+ struct thermal_trip trip_table[ACPI_THERMAL_MAX_NR_TRIPS] = { 0 };
struct acpi_thermal_trip *acpi_trip;
struct thermal_trip *trip;
struct acpi_thermal *tz;
- unsigned int trip_count;
int crit_temp, hot_temp;
int passive_delay = 0;
int result;
@@ -848,21 +843,10 @@ static int acpi_thermal_add(struct acpi_
acpi_thermal_aml_dependency_fix(tz);
/* Get trip points [_CRT, _PSV, etc.] (required). */
- trip_count = acpi_thermal_get_trip_points(tz);
+ acpi_thermal_get_trip_points(tz);
crit_temp = acpi_thermal_get_critical_trip(tz);
- if (crit_temp != THERMAL_TEMP_INVALID)
- trip_count++;
-
hot_temp = acpi_thermal_get_hot_trip(tz);
- if (hot_temp != THERMAL_TEMP_INVALID)
- trip_count++;
-
- if (!trip_count) {
- pr_warn(FW_BUG "No valid trip points!\n");
- result = -ENODEV;
- goto free_memory;
- }
/* Get temperature [_TMP] (required). */
result = acpi_thermal_get_temperature(tz);
@@ -881,13 +865,7 @@ static int acpi_thermal_add(struct acpi_
acpi_thermal_guess_offset(tz, crit_temp);
- trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
- if (!trip) {
- result = -ENOMEM;
- goto free_memory;
- }
-
- tz->trip_table = trip;
+ trip = trip_table;
if (crit_temp != THERMAL_TEMP_INVALID) {
trip->type = THERMAL_TRIP_CRITICAL;
@@ -923,9 +901,17 @@ static int acpi_thermal_add(struct acpi_
trip++;
}
- result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
+ if (trip == trip_table) {
+ pr_warn(FW_BUG "No valid trip points!\n");
+ result = -ENODEV;
+ goto free_memory;
+ }
+
+ result = acpi_thermal_register_thermal_zone(tz, trip_table,
+ trip - trip_table,
+ passive_delay);
if (result)
- goto free_trips;
+ goto free_memory;
refcount_set(&tz->thermal_check_count, 3);
mutex_init(&tz->thermal_check_lock);
@@ -944,8 +930,6 @@ static int acpi_thermal_add(struct acpi_
flush_wq:
flush_workqueue(acpi_thermal_pm_queue);
acpi_thermal_unregister_thermal_zone(tz);
-free_trips:
- kfree(tz->trip_table);
free_memory:
acpi_thermal_free_thermal_zone(tz);
@@ -966,7 +950,6 @@ static void acpi_thermal_remove(struct a
flush_workqueue(acpi_thermal_pm_queue);
acpi_thermal_unregister_thermal_zone(tz);
- kfree(tz->trip_table);
acpi_thermal_free_thermal_zone(tz);
}
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 2/6] thermal: ACPI: Discard trip table after zone registration
2024-02-05 21:15 ` [PATCH v1 2/6] thermal: ACPI: Discard trip table after zone registration Rafael J. Wysocki
@ 2024-02-08 8:22 ` Stanislaw Gruszka
0 siblings, 0 replies; 14+ messages in thread
From: Stanislaw Gruszka @ 2024-02-08 8:22 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba,
Zhang Rui, Srinivas Pandruvada, AngeloGioacchino Del Regno
On Mon, Feb 05, 2024 at 10:15:50PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Because the thermal core creates and uses its own copy of the trips
> table passed to thermal_zone_device_register_with_trips(), it is not
> necessary to hold on to a local copy of it any more after the given
> thermal zone has been registered.
>
> Accordingly, modify the ACPI thermal driver to store the trips table
> passed to thermal_zone_device_register_with_trips() in a local variable
> which is automatically discarded after the zone registration.
>
> Also make some additional code simplifications unlocked by the above
> change.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/acpi/thermal.c | 57 +++++++++++++++++--------------------------------
> 1 file changed, 20 insertions(+), 37 deletions(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -47,6 +47,8 @@
>
> #define ACPI_THERMAL_TRIP_PASSIVE (-1)
>
> +#define ACPI_THERMAL_MAX_NR_TRIPS (ACPI_THERMAL_MAX_ACTIVE + 3)
> +
> /*
> * This exception is thrown out in two cases:
> * 1.An invalid trip point becomes invalid or a valid trip point becomes invalid
> @@ -112,7 +114,6 @@ struct acpi_thermal {
> unsigned long polling_frequency;
> volatile u8 zombie;
> struct acpi_thermal_trips trips;
> - struct thermal_trip *trip_table;
> struct thermal_zone_device *thermal_zone;
> int kelvin_offset; /* in millidegrees */
> struct work_struct thermal_check_work;
> @@ -451,26 +452,19 @@ fail:
> return false;
> }
>
> -static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
> +static void acpi_thermal_get_trip_points(struct acpi_thermal *tz)
> {
> - unsigned int count = 0;
> int i;
>
> - if (acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE))
> - count++;
> + acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
>
> for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> - if (acpi_thermal_init_trip(tz, i))
> - count++;
> - else
> + if (!acpi_thermal_init_trip(tz, i))
> break;
> -
> }
>
> while (++i < ACPI_THERMAL_MAX_ACTIVE)
> tz->trips.active[i].trip.temp_dk = THERMAL_TEMP_INVALID;
> -
> - return count;
> }
>
> /* sys I/F for generic thermal sysfs support */
> @@ -662,13 +656,14 @@ static void acpi_thermal_zone_sysfs_remo
> }
>
> static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
> + const struct thermal_trip *trip_table,
> unsigned int trip_count,
> int passive_delay)
> {
> int result;
>
> tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
> - tz->trip_table,
> + trip_table,
> trip_count,
> 0, tz,
> &acpi_thermal_zone_ops,
> @@ -823,10 +818,10 @@ static void acpi_thermal_free_thermal_zo
>
> static int acpi_thermal_add(struct acpi_device *device)
> {
> + struct thermal_trip trip_table[ACPI_THERMAL_MAX_NR_TRIPS] = { 0 };
> struct acpi_thermal_trip *acpi_trip;
> struct thermal_trip *trip;
> struct acpi_thermal *tz;
> - unsigned int trip_count;
> int crit_temp, hot_temp;
> int passive_delay = 0;
> int result;
> @@ -848,21 +843,10 @@ static int acpi_thermal_add(struct acpi_
> acpi_thermal_aml_dependency_fix(tz);
>
> /* Get trip points [_CRT, _PSV, etc.] (required). */
> - trip_count = acpi_thermal_get_trip_points(tz);
> + acpi_thermal_get_trip_points(tz);
>
> crit_temp = acpi_thermal_get_critical_trip(tz);
> - if (crit_temp != THERMAL_TEMP_INVALID)
> - trip_count++;
> -
> hot_temp = acpi_thermal_get_hot_trip(tz);
> - if (hot_temp != THERMAL_TEMP_INVALID)
> - trip_count++;
> -
> - if (!trip_count) {
> - pr_warn(FW_BUG "No valid trip points!\n");
> - result = -ENODEV;
> - goto free_memory;
> - }
>
> /* Get temperature [_TMP] (required). */
> result = acpi_thermal_get_temperature(tz);
> @@ -881,13 +865,7 @@ static int acpi_thermal_add(struct acpi_
>
> acpi_thermal_guess_offset(tz, crit_temp);
>
> - trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
> - if (!trip) {
> - result = -ENOMEM;
> - goto free_memory;
> - }
> -
> - tz->trip_table = trip;
> + trip = trip_table;
>
> if (crit_temp != THERMAL_TEMP_INVALID) {
> trip->type = THERMAL_TRIP_CRITICAL;
> @@ -923,9 +901,17 @@ static int acpi_thermal_add(struct acpi_
> trip++;
> }
>
> - result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
> + if (trip == trip_table) {
> + pr_warn(FW_BUG "No valid trip points!\n");
> + result = -ENODEV;
> + goto free_memory;
> + }
> +
> + result = acpi_thermal_register_thermal_zone(tz, trip_table,
> + trip - trip_table,
> + passive_delay);
> if (result)
> - goto free_trips;
> + goto free_memory;
>
> refcount_set(&tz->thermal_check_count, 3);
> mutex_init(&tz->thermal_check_lock);
> @@ -944,8 +930,6 @@ static int acpi_thermal_add(struct acpi_
> flush_wq:
> flush_workqueue(acpi_thermal_pm_queue);
> acpi_thermal_unregister_thermal_zone(tz);
> -free_trips:
> - kfree(tz->trip_table);
> free_memory:
> acpi_thermal_free_thermal_zone(tz);
>
> @@ -966,7 +950,6 @@ static void acpi_thermal_remove(struct a
>
> flush_workqueue(acpi_thermal_pm_queue);
> acpi_thermal_unregister_thermal_zone(tz);
> - kfree(tz->trip_table);
> acpi_thermal_free_thermal_zone(tz);
> }
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 3/6] thermal: intel: Discard trip tables after zone registration
2024-02-05 21:12 [PATCH v1 0/6] thermal: Store trips table and ops in thermal_zone_device Rafael J. Wysocki
2024-02-05 21:14 ` [PATCH v1 1/6] thermal: core: Store zone trips table in struct thermal_zone_device Rafael J. Wysocki
2024-02-05 21:15 ` [PATCH v1 2/6] thermal: ACPI: Discard trip table after zone registration Rafael J. Wysocki
@ 2024-02-05 21:16 ` Rafael J. Wysocki
2024-02-08 8:36 ` Stanislaw Gruszka
2024-02-05 21:18 ` [PATCH v1 4/6] thermal: core: Store zone ops in struct thermal_zone_device Rafael J. Wysocki
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-02-05 21:16 UTC (permalink / raw)
To: Linux PM
Cc: Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba, Zhang Rui,
Srinivas Pandruvada, Stanislaw Gruszka,
AngeloGioacchino Del Regno
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Because the thermal core creates and uses its own copy of the trips
table passed to thermal_zone_device_register_with_trips(), it is not
necessary to hold on to a local copy of it any more after the given
thermal zone has been registered.
Accordingly, modify Intel thermal drivers to discard the trips tables
passed to thermal_zone_device_register_with_trips() after thermal zone
registration, for example by storing them in local variables which are
automatically discarded when the zone registration is complete.
Also make some additional code simplifications unlocked by the above
changes.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 6 -
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h | 1
drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c | 6 -
drivers/thermal/intel/intel_pch_thermal.c | 21 ++---
drivers/thermal/intel/intel_quark_dts_thermal.c | 12 +--
drivers/thermal/intel/intel_soc_dts_iosf.c | 38 +++-------
drivers/thermal/intel/intel_soc_dts_iosf.h | 1
drivers/thermal/intel/x86_pkg_temp_thermal.c | 35 +++------
8 files changed, 48 insertions(+), 72 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
@@ -179,8 +179,6 @@ struct int34x_thermal_zone *int340x_ther
for (i = 0; i < trip_cnt; ++i)
zone_trips[i].hysteresis = hyst;
- int34x_zone->trips = zone_trips;
-
int34x_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
int34x_zone->zone = thermal_zone_device_register_with_trips(
@@ -190,6 +188,8 @@ struct int34x_thermal_zone *int340x_ther
int34x_zone->ops,
&int340x_thermal_params,
0, 0);
+ kfree(zone_trips);
+
if (IS_ERR(int34x_zone->zone)) {
ret = PTR_ERR(int34x_zone->zone);
goto err_thermal_zone;
@@ -203,7 +203,6 @@ struct int34x_thermal_zone *int340x_ther
err_enable:
thermal_zone_device_unregister(int34x_zone->zone);
err_thermal_zone:
- kfree(int34x_zone->trips);
acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
err_trips_alloc:
kfree(int34x_zone->ops);
@@ -217,7 +216,6 @@ void int340x_thermal_zone_remove(struct
{
thermal_zone_device_unregister(int34x_zone->zone);
acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
- kfree(int34x_zone->trips);
kfree(int34x_zone->ops);
kfree(int34x_zone);
}
Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
@@ -20,7 +20,6 @@ struct active_trip {
struct int34x_thermal_zone {
struct acpi_device *adev;
- struct thermal_trip *trips;
int aux_trip_nr;
struct thermal_zone_device *zone;
struct thermal_zone_device_ops *ops;
Index: linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
@@ -233,10 +233,6 @@ static int get_trip_temp(struct proc_the
return temp;
}
-static struct thermal_trip psv_trip = {
- .type = THERMAL_TRIP_PASSIVE,
-};
-
static struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
.set_trip_temp = sys_set_trip_temp,
@@ -251,6 +247,7 @@ static int proc_thermal_pci_probe(struct
{
struct proc_thermal_device *proc_priv;
struct proc_thermal_pci *pci_info;
+ struct thermal_trip psv_trip = { 0 };
int irq_flag = 0, irq, ret;
bool msi_irq = false;
@@ -288,6 +285,7 @@ static int proc_thermal_pci_probe(struct
}
psv_trip.temperature = get_trip_temp(pci_info);
+ psv_trip.type = THERMAL_TRIP_PASSIVE;
pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
1, 1, pci_info,
Index: linux-pm/drivers/thermal/intel/intel_pch_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_pch_thermal.c
+++ linux-pm/drivers/thermal/intel/intel_pch_thermal.c
@@ -84,7 +84,6 @@ struct pch_thermal_device {
void __iomem *hw_base;
struct pci_dev *pdev;
struct thermal_zone_device *tzd;
- struct thermal_trip trips[PCH_MAX_TRIPS];
bool bios_enabled;
};
@@ -94,7 +93,8 @@ struct pch_thermal_device {
* passive trip temperature using _PSV method. There is no specific
* passive temperature setting in MMIO interface of this PCI device.
*/
-static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd, int trip)
+static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd,
+ struct thermal_trip *trip)
{
struct acpi_device *adev;
int temp;
@@ -106,8 +106,8 @@ static int pch_wpt_add_acpi_psv_trip(str
if (thermal_acpi_passive_trip_temp(adev, &temp) || temp <= 0)
return 0;
- ptd->trips[trip].type = THERMAL_TRIP_PASSIVE;
- ptd->trips[trip].temperature = temp;
+ trip->type = THERMAL_TRIP_PASSIVE;
+ trip->temperature = temp;
return 1;
}
#else
@@ -159,6 +159,7 @@ static const char *board_names[] = {
static int intel_pch_thermal_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
+ struct thermal_trip ptd_trips[PCH_MAX_TRIPS] = { 0 };
enum pch_board_ids board_id = id->driver_data;
struct pch_thermal_device *ptd;
int nr_trips = 0;
@@ -220,21 +221,21 @@ read_trips:
trip_temp = readw(ptd->hw_base + WPT_CTT);
trip_temp &= 0x1FF;
if (trip_temp) {
- ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
- ptd->trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
+ ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
+ ptd_trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
}
trip_temp = readw(ptd->hw_base + WPT_PHL);
trip_temp &= 0x1FF;
if (trip_temp) {
- ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
- ptd->trips[nr_trips++].type = THERMAL_TRIP_HOT;
+ ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
+ ptd_trips[nr_trips++].type = THERMAL_TRIP_HOT;
}
- nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
+ nr_trips += pch_wpt_add_acpi_psv_trip(ptd, &ptd_trips[nr_trips]);
ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
- ptd->trips, nr_trips,
+ ptd_trips, nr_trips,
0, ptd, &tzd_ops,
NULL, 0, 0);
if (IS_ERR(ptd->tzd)) {
Index: linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -105,7 +105,6 @@ struct soc_sensor_entry {
u32 store_ptps;
u32 store_dts_enable;
struct thermal_zone_device *tzone;
- struct thermal_trip trips[QRK_MAX_DTS_TRIPS];
};
static struct soc_sensor_entry *soc_dts;
@@ -320,6 +319,7 @@ static void free_soc_dts(struct soc_sens
static struct soc_sensor_entry *alloc_soc_dts(void)
{
+ struct thermal_trip trips[QRK_MAX_DTS_TRIPS] = { 0 };
struct soc_sensor_entry *aux_entry;
int err;
u32 out;
@@ -362,14 +362,14 @@ static struct soc_sensor_entry *alloc_so
goto err_ret;
}
- aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
- aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
+ trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
+ trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
- aux_entry->trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
- aux_entry->trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
+ trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
+ trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
aux_entry->tzone = thermal_zone_device_register_with_trips("quark_dts",
- aux_entry->trips,
+ trips,
QRK_MAX_DTS_TRIPS,
wr_mask,
aux_entry, &tzone_ops,
Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
+++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
@@ -129,22 +129,6 @@ err_restore_ptps:
return status;
}
-static int configure_trip(struct intel_soc_dts_sensor_entry *dts,
- int thres_index, enum thermal_trip_type trip_type,
- int temp)
-{
- int ret;
-
- ret = update_trip_temp(dts->sensors, thres_index, temp);
- if (ret)
- return ret;
-
- dts->trips[thres_index].temperature = temp;
- dts->trips[thres_index].type = trip_type;
-
- return 0;
-}
-
static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
int temp)
{
@@ -218,6 +202,7 @@ static void remove_dts_thermal_zone(stru
}
static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
+ struct thermal_trip *trips,
bool critical_trip)
{
int writable_trip_cnt = SOC_MAX_DTS_TRIPS;
@@ -254,7 +239,7 @@ static int add_dts_thermal_zone(int id,
}
dts->trip_mask = trip_mask;
snprintf(name, sizeof(name), "soc_dts%d", id);
- dts->tzone = thermal_zone_device_register_with_trips(name, dts->trips,
+ dts->tzone = thermal_zone_device_register_with_trips(name, trips,
SOC_MAX_DTS_TRIPS,
trip_mask,
dts, &tzone_ops,
@@ -315,14 +300,15 @@ EXPORT_SYMBOL_GPL(intel_soc_dts_iosf_int
static void dts_trips_reset(struct intel_soc_dts_sensors *sensors, int dts_index)
{
- configure_trip(&sensors->soc_dts[dts_index], 0, 0, 0);
- configure_trip(&sensors->soc_dts[dts_index], 1, 0, 0);
+ update_trip_temp(sensors, 0, 0);
+ update_trip_temp(sensors, 1, 0);
}
struct intel_soc_dts_sensors *
intel_soc_dts_iosf_init(enum intel_soc_dts_interrupt_type intr_type,
bool critical_trip, int crit_offset)
{
+ struct thermal_trip trips[SOC_MAX_DTS_SENSORS][SOC_MAX_DTS_TRIPS] = { 0 };
struct intel_soc_dts_sensors *sensors;
int tj_max;
int ret;
@@ -350,11 +336,13 @@ intel_soc_dts_iosf_init(enum intel_soc_d
sensors->soc_dts[i].sensors = sensors;
- ret = configure_trip(&sensors->soc_dts[i], 0,
- THERMAL_TRIP_PASSIVE, 0);
+ ret = update_trip_temp(sensors, 0, 0);
if (ret)
goto err_reset_trips;
+ trips[i][0].type = THERMAL_TRIP_PASSIVE;
+ trips[i][0].temperature = 0;
+
if (critical_trip) {
trip_type = THERMAL_TRIP_CRITICAL;
temp = sensors->tj_max - crit_offset;
@@ -362,13 +350,17 @@ intel_soc_dts_iosf_init(enum intel_soc_d
trip_type = THERMAL_TRIP_PASSIVE;
temp = 0;
}
- ret = configure_trip(&sensors->soc_dts[i], 1, trip_type, temp);
+ ret = update_trip_temp(sensors, 1, temp);
if (ret)
goto err_reset_trips;
+
+ trips[i][1].type = trip_type;
+ trips[i][1].temperature = temp;
}
for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
- ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], critical_trip);
+ ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], trips[i],
+ critical_trip);
if (ret)
goto err_remove_zone;
}
Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.h
+++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
@@ -29,7 +29,6 @@ struct intel_soc_dts_sensor_entry {
int id;
u32 store_status;
u32 trip_mask;
- struct thermal_trip trips[SOC_MAX_DTS_TRIPS];
struct thermal_zone_device *tzone;
struct intel_soc_dts_sensors *sensors;
};
Index: linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -53,7 +53,6 @@ struct zone_device {
u32 msr_pkg_therm_high;
struct delayed_work work;
struct thermal_zone_device *tzone;
- struct thermal_trip *trips;
struct cpumask cpumask;
};
@@ -268,17 +267,13 @@ static int pkg_thermal_notify(u64 msr_va
return 0;
}
-static struct thermal_trip *pkg_temp_thermal_trips_init(int cpu, int tj_max, int num_trips)
+static int pkg_temp_thermal_trips_init(int cpu, int tj_max,
+ struct thermal_trip *trips, int num_trips)
{
- struct thermal_trip *trips;
unsigned long thres_reg_value;
u32 mask, shift, eax, edx;
int ret, i;
- trips = kzalloc(sizeof(*trips) * num_trips, GFP_KERNEL);
- if (!trips)
- return ERR_PTR(-ENOMEM);
-
for (i = 0; i < num_trips; i++) {
if (i) {
@@ -291,10 +286,8 @@ static struct thermal_trip *pkg_temp_the
ret = rdmsr_on_cpu(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
&eax, &edx);
- if (ret < 0) {
- kfree(trips);
- return ERR_PTR(ret);
- }
+ if (ret < 0)
+ return ret;
thres_reg_value = (eax & mask) >> shift;
@@ -307,11 +300,12 @@ static struct thermal_trip *pkg_temp_the
__func__, cpu, i, trips[i].temperature);
}
- return trips;
+ return 0;
}
static int pkg_temp_thermal_device_add(unsigned int cpu)
{
+ struct thermal_trip trips[MAX_NUMBER_OF_TRIPS] = { 0 };
int id = topology_logical_die_id(cpu);
u32 eax, ebx, ecx, edx;
struct zone_device *zonedev;
@@ -336,21 +330,19 @@ static int pkg_temp_thermal_device_add(u
if (!zonedev)
return -ENOMEM;
- zonedev->trips = pkg_temp_thermal_trips_init(cpu, tj_max, thres_count);
- if (IS_ERR(zonedev->trips)) {
- err = PTR_ERR(zonedev->trips);
+ err = pkg_temp_thermal_trips_init(cpu, tj_max, trips, thres_count);
+ if (err)
goto out_kfree_zonedev;
- }
INIT_DELAYED_WORK(&zonedev->work, pkg_temp_thermal_threshold_work_fn);
zonedev->cpu = cpu;
zonedev->tzone = thermal_zone_device_register_with_trips("x86_pkg_temp",
- zonedev->trips, thres_count,
+ trips, thres_count,
(thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
if (IS_ERR(zonedev->tzone)) {
err = PTR_ERR(zonedev->tzone);
- goto out_kfree_trips;
+ goto out_kfree_zonedev;
}
err = thermal_zone_device_enable(zonedev->tzone);
if (err)
@@ -369,8 +361,6 @@ static int pkg_temp_thermal_device_add(u
out_unregister_tz:
thermal_zone_device_unregister(zonedev->tzone);
-out_kfree_trips:
- kfree(zonedev->trips);
out_kfree_zonedev:
kfree(zonedev);
return err;
@@ -457,10 +447,9 @@ static int pkg_thermal_cpu_offline(unsig
raw_spin_unlock_irq(&pkg_temp_lock);
/* Final cleanup if this is the last cpu */
- if (lastcpu) {
- kfree(zonedev->trips);
+ if (lastcpu)
kfree(zonedev);
- }
+
return 0;
}
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 3/6] thermal: intel: Discard trip tables after zone registration
2024-02-05 21:16 ` [PATCH v1 3/6] thermal: intel: Discard trip tables " Rafael J. Wysocki
@ 2024-02-08 8:36 ` Stanislaw Gruszka
0 siblings, 0 replies; 14+ messages in thread
From: Stanislaw Gruszka @ 2024-02-08 8:36 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba,
Zhang Rui, Srinivas Pandruvada, AngeloGioacchino Del Regno
On Mon, Feb 05, 2024 at 10:16:59PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Because the thermal core creates and uses its own copy of the trips
> table passed to thermal_zone_device_register_with_trips(), it is not
> necessary to hold on to a local copy of it any more after the given
> thermal zone has been registered.
>
> Accordingly, modify Intel thermal drivers to discard the trips tables
> passed to thermal_zone_device_register_with_trips() after thermal zone
> registration, for example by storing them in local variables which are
> automatically discarded when the zone registration is complete.
>
> Also make some additional code simplifications unlocked by the above
> changes.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 6 -
> drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h | 1
> drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c | 6 -
> drivers/thermal/intel/intel_pch_thermal.c | 21 ++---
> drivers/thermal/intel/intel_quark_dts_thermal.c | 12 +--
> drivers/thermal/intel/intel_soc_dts_iosf.c | 38 +++-------
> drivers/thermal/intel/intel_soc_dts_iosf.h | 1
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 35 +++------
> 8 files changed, 48 insertions(+), 72 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
> @@ -179,8 +179,6 @@ struct int34x_thermal_zone *int340x_ther
> for (i = 0; i < trip_cnt; ++i)
> zone_trips[i].hysteresis = hyst;
>
> - int34x_zone->trips = zone_trips;
> -
> int34x_zone->lpat_table = acpi_lpat_get_conversion_table(adev->handle);
>
> int34x_zone->zone = thermal_zone_device_register_with_trips(
> @@ -190,6 +188,8 @@ struct int34x_thermal_zone *int340x_ther
> int34x_zone->ops,
> &int340x_thermal_params,
> 0, 0);
> + kfree(zone_trips);
> +
> if (IS_ERR(int34x_zone->zone)) {
> ret = PTR_ERR(int34x_zone->zone);
> goto err_thermal_zone;
> @@ -203,7 +203,6 @@ struct int34x_thermal_zone *int340x_ther
> err_enable:
> thermal_zone_device_unregister(int34x_zone->zone);
> err_thermal_zone:
> - kfree(int34x_zone->trips);
> acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
> err_trips_alloc:
> kfree(int34x_zone->ops);
> @@ -217,7 +216,6 @@ void int340x_thermal_zone_remove(struct
> {
> thermal_zone_device_unregister(int34x_zone->zone);
> acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
> - kfree(int34x_zone->trips);
> kfree(int34x_zone->ops);
> kfree(int34x_zone);
> }
> Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> +++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> @@ -20,7 +20,6 @@ struct active_trip {
>
> struct int34x_thermal_zone {
> struct acpi_device *adev;
> - struct thermal_trip *trips;
> int aux_trip_nr;
> struct thermal_zone_device *zone;
> struct thermal_zone_device_ops *ops;
> Index: linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> +++ linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> @@ -233,10 +233,6 @@ static int get_trip_temp(struct proc_the
> return temp;
> }
>
> -static struct thermal_trip psv_trip = {
> - .type = THERMAL_TRIP_PASSIVE,
> -};
> -
> static struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> .set_trip_temp = sys_set_trip_temp,
> @@ -251,6 +247,7 @@ static int proc_thermal_pci_probe(struct
> {
> struct proc_thermal_device *proc_priv;
> struct proc_thermal_pci *pci_info;
> + struct thermal_trip psv_trip = { 0 };
> int irq_flag = 0, irq, ret;
> bool msi_irq = false;
>
> @@ -288,6 +285,7 @@ static int proc_thermal_pci_probe(struct
> }
>
> psv_trip.temperature = get_trip_temp(pci_info);
> + psv_trip.type = THERMAL_TRIP_PASSIVE;
>
> pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
> 1, 1, pci_info,
> Index: linux-pm/drivers/thermal/intel/intel_pch_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_pch_thermal.c
> +++ linux-pm/drivers/thermal/intel/intel_pch_thermal.c
> @@ -84,7 +84,6 @@ struct pch_thermal_device {
> void __iomem *hw_base;
> struct pci_dev *pdev;
> struct thermal_zone_device *tzd;
> - struct thermal_trip trips[PCH_MAX_TRIPS];
> bool bios_enabled;
> };
>
> @@ -94,7 +93,8 @@ struct pch_thermal_device {
> * passive trip temperature using _PSV method. There is no specific
> * passive temperature setting in MMIO interface of this PCI device.
> */
> -static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd, int trip)
> +static int pch_wpt_add_acpi_psv_trip(struct pch_thermal_device *ptd,
> + struct thermal_trip *trip)
> {
> struct acpi_device *adev;
> int temp;
> @@ -106,8 +106,8 @@ static int pch_wpt_add_acpi_psv_trip(str
> if (thermal_acpi_passive_trip_temp(adev, &temp) || temp <= 0)
> return 0;
>
> - ptd->trips[trip].type = THERMAL_TRIP_PASSIVE;
> - ptd->trips[trip].temperature = temp;
> + trip->type = THERMAL_TRIP_PASSIVE;
> + trip->temperature = temp;
> return 1;
> }
> #else
> @@ -159,6 +159,7 @@ static const char *board_names[] = {
> static int intel_pch_thermal_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> {
> + struct thermal_trip ptd_trips[PCH_MAX_TRIPS] = { 0 };
> enum pch_board_ids board_id = id->driver_data;
> struct pch_thermal_device *ptd;
> int nr_trips = 0;
> @@ -220,21 +221,21 @@ read_trips:
> trip_temp = readw(ptd->hw_base + WPT_CTT);
> trip_temp &= 0x1FF;
> if (trip_temp) {
> - ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> - ptd->trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
> + ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> + ptd_trips[nr_trips++].type = THERMAL_TRIP_CRITICAL;
> }
>
> trip_temp = readw(ptd->hw_base + WPT_PHL);
> trip_temp &= 0x1FF;
> if (trip_temp) {
> - ptd->trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> - ptd->trips[nr_trips++].type = THERMAL_TRIP_HOT;
> + ptd_trips[nr_trips].temperature = GET_WPT_TEMP(trip_temp);
> + ptd_trips[nr_trips++].type = THERMAL_TRIP_HOT;
> }
>
> - nr_trips += pch_wpt_add_acpi_psv_trip(ptd, nr_trips);
> + nr_trips += pch_wpt_add_acpi_psv_trip(ptd, &ptd_trips[nr_trips]);
>
> ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
> - ptd->trips, nr_trips,
> + ptd_trips, nr_trips,
> 0, ptd, &tzd_ops,
> NULL, 0, 0);
> if (IS_ERR(ptd->tzd)) {
> Index: linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -105,7 +105,6 @@ struct soc_sensor_entry {
> u32 store_ptps;
> u32 store_dts_enable;
> struct thermal_zone_device *tzone;
> - struct thermal_trip trips[QRK_MAX_DTS_TRIPS];
> };
>
> static struct soc_sensor_entry *soc_dts;
> @@ -320,6 +319,7 @@ static void free_soc_dts(struct soc_sens
>
> static struct soc_sensor_entry *alloc_soc_dts(void)
> {
> + struct thermal_trip trips[QRK_MAX_DTS_TRIPS] = { 0 };
> struct soc_sensor_entry *aux_entry;
> int err;
> u32 out;
> @@ -362,14 +362,14 @@ static struct soc_sensor_entry *alloc_so
> goto err_ret;
> }
>
> - aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
> - aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
> + trips[QRK_DTS_ID_TP_CRITICAL].temperature = get_trip_temp(QRK_DTS_ID_TP_CRITICAL);
> + trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
>
> - aux_entry->trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
> - aux_entry->trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
> + trips[QRK_DTS_ID_TP_HOT].temperature = get_trip_temp(QRK_DTS_ID_TP_HOT);
> + trips[QRK_DTS_ID_TP_HOT].type = THERMAL_TRIP_HOT;
>
> aux_entry->tzone = thermal_zone_device_register_with_trips("quark_dts",
> - aux_entry->trips,
> + trips,
> QRK_MAX_DTS_TRIPS,
> wr_mask,
> aux_entry, &tzone_ops,
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> @@ -129,22 +129,6 @@ err_restore_ptps:
> return status;
> }
>
> -static int configure_trip(struct intel_soc_dts_sensor_entry *dts,
> - int thres_index, enum thermal_trip_type trip_type,
> - int temp)
> -{
> - int ret;
> -
> - ret = update_trip_temp(dts->sensors, thres_index, temp);
> - if (ret)
> - return ret;
> -
> - dts->trips[thres_index].temperature = temp;
> - dts->trips[thres_index].type = trip_type;
> -
> - return 0;
> -}
> -
> static int sys_set_trip_temp(struct thermal_zone_device *tzd, int trip,
> int temp)
> {
> @@ -218,6 +202,7 @@ static void remove_dts_thermal_zone(stru
> }
>
> static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
> + struct thermal_trip *trips,
> bool critical_trip)
> {
> int writable_trip_cnt = SOC_MAX_DTS_TRIPS;
> @@ -254,7 +239,7 @@ static int add_dts_thermal_zone(int id,
> }
> dts->trip_mask = trip_mask;
> snprintf(name, sizeof(name), "soc_dts%d", id);
> - dts->tzone = thermal_zone_device_register_with_trips(name, dts->trips,
> + dts->tzone = thermal_zone_device_register_with_trips(name, trips,
> SOC_MAX_DTS_TRIPS,
> trip_mask,
> dts, &tzone_ops,
> @@ -315,14 +300,15 @@ EXPORT_SYMBOL_GPL(intel_soc_dts_iosf_int
>
> static void dts_trips_reset(struct intel_soc_dts_sensors *sensors, int dts_index)
> {
> - configure_trip(&sensors->soc_dts[dts_index], 0, 0, 0);
> - configure_trip(&sensors->soc_dts[dts_index], 1, 0, 0);
> + update_trip_temp(sensors, 0, 0);
> + update_trip_temp(sensors, 1, 0);
> }
>
> struct intel_soc_dts_sensors *
> intel_soc_dts_iosf_init(enum intel_soc_dts_interrupt_type intr_type,
> bool critical_trip, int crit_offset)
> {
> + struct thermal_trip trips[SOC_MAX_DTS_SENSORS][SOC_MAX_DTS_TRIPS] = { 0 };
> struct intel_soc_dts_sensors *sensors;
> int tj_max;
> int ret;
> @@ -350,11 +336,13 @@ intel_soc_dts_iosf_init(enum intel_soc_d
>
> sensors->soc_dts[i].sensors = sensors;
>
> - ret = configure_trip(&sensors->soc_dts[i], 0,
> - THERMAL_TRIP_PASSIVE, 0);
> + ret = update_trip_temp(sensors, 0, 0);
> if (ret)
> goto err_reset_trips;
>
> + trips[i][0].type = THERMAL_TRIP_PASSIVE;
> + trips[i][0].temperature = 0;
> +
> if (critical_trip) {
> trip_type = THERMAL_TRIP_CRITICAL;
> temp = sensors->tj_max - crit_offset;
> @@ -362,13 +350,17 @@ intel_soc_dts_iosf_init(enum intel_soc_d
> trip_type = THERMAL_TRIP_PASSIVE;
> temp = 0;
> }
> - ret = configure_trip(&sensors->soc_dts[i], 1, trip_type, temp);
> + ret = update_trip_temp(sensors, 1, temp);
> if (ret)
> goto err_reset_trips;
> +
> + trips[i][1].type = trip_type;
> + trips[i][1].temperature = temp;
> }
>
> for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
> - ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], critical_trip);
> + ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], trips[i],
> + critical_trip);
> if (ret)
> goto err_remove_zone;
> }
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.h
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.h
> @@ -29,7 +29,6 @@ struct intel_soc_dts_sensor_entry {
> int id;
> u32 store_status;
> u32 trip_mask;
> - struct thermal_trip trips[SOC_MAX_DTS_TRIPS];
> struct thermal_zone_device *tzone;
> struct intel_soc_dts_sensors *sensors;
> };
> Index: linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -53,7 +53,6 @@ struct zone_device {
> u32 msr_pkg_therm_high;
> struct delayed_work work;
> struct thermal_zone_device *tzone;
> - struct thermal_trip *trips;
> struct cpumask cpumask;
> };
>
> @@ -268,17 +267,13 @@ static int pkg_thermal_notify(u64 msr_va
> return 0;
> }
>
> -static struct thermal_trip *pkg_temp_thermal_trips_init(int cpu, int tj_max, int num_trips)
> +static int pkg_temp_thermal_trips_init(int cpu, int tj_max,
> + struct thermal_trip *trips, int num_trips)
> {
> - struct thermal_trip *trips;
> unsigned long thres_reg_value;
> u32 mask, shift, eax, edx;
> int ret, i;
>
> - trips = kzalloc(sizeof(*trips) * num_trips, GFP_KERNEL);
> - if (!trips)
> - return ERR_PTR(-ENOMEM);
> -
> for (i = 0; i < num_trips; i++) {
>
> if (i) {
> @@ -291,10 +286,8 @@ static struct thermal_trip *pkg_temp_the
>
> ret = rdmsr_on_cpu(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
> &eax, &edx);
> - if (ret < 0) {
> - kfree(trips);
> - return ERR_PTR(ret);
> - }
> + if (ret < 0)
> + return ret;
>
> thres_reg_value = (eax & mask) >> shift;
>
> @@ -307,11 +300,12 @@ static struct thermal_trip *pkg_temp_the
> __func__, cpu, i, trips[i].temperature);
> }
>
> - return trips;
> + return 0;
> }
>
> static int pkg_temp_thermal_device_add(unsigned int cpu)
> {
> + struct thermal_trip trips[MAX_NUMBER_OF_TRIPS] = { 0 };
> int id = topology_logical_die_id(cpu);
> u32 eax, ebx, ecx, edx;
> struct zone_device *zonedev;
> @@ -336,21 +330,19 @@ static int pkg_temp_thermal_device_add(u
> if (!zonedev)
> return -ENOMEM;
>
> - zonedev->trips = pkg_temp_thermal_trips_init(cpu, tj_max, thres_count);
> - if (IS_ERR(zonedev->trips)) {
> - err = PTR_ERR(zonedev->trips);
> + err = pkg_temp_thermal_trips_init(cpu, tj_max, trips, thres_count);
> + if (err)
> goto out_kfree_zonedev;
> - }
>
> INIT_DELAYED_WORK(&zonedev->work, pkg_temp_thermal_threshold_work_fn);
> zonedev->cpu = cpu;
> zonedev->tzone = thermal_zone_device_register_with_trips("x86_pkg_temp",
> - zonedev->trips, thres_count,
> + trips, thres_count,
> (thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
> zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
> if (IS_ERR(zonedev->tzone)) {
> err = PTR_ERR(zonedev->tzone);
> - goto out_kfree_trips;
> + goto out_kfree_zonedev;
> }
> err = thermal_zone_device_enable(zonedev->tzone);
> if (err)
> @@ -369,8 +361,6 @@ static int pkg_temp_thermal_device_add(u
>
> out_unregister_tz:
> thermal_zone_device_unregister(zonedev->tzone);
> -out_kfree_trips:
> - kfree(zonedev->trips);
> out_kfree_zonedev:
> kfree(zonedev);
> return err;
> @@ -457,10 +447,9 @@ static int pkg_thermal_cpu_offline(unsig
> raw_spin_unlock_irq(&pkg_temp_lock);
>
> /* Final cleanup if this is the last cpu */
> - if (lastcpu) {
> - kfree(zonedev->trips);
> + if (lastcpu)
> kfree(zonedev);
> - }
> +
> return 0;
> }
>
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 4/6] thermal: core: Store zone ops in struct thermal_zone_device
2024-02-05 21:12 [PATCH v1 0/6] thermal: Store trips table and ops in thermal_zone_device Rafael J. Wysocki
` (2 preceding siblings ...)
2024-02-05 21:16 ` [PATCH v1 3/6] thermal: intel: Discard trip tables " Rafael J. Wysocki
@ 2024-02-05 21:18 ` Rafael J. Wysocki
2024-02-08 8:48 ` Stanislaw Gruszka
2024-02-05 21:18 ` [PATCH v1 5/6] thermal: ACPI: Constify acpi_thermal_zone_ops Rafael J. Wysocki
2024-02-05 21:20 ` [PATCH v1 6/6] thermal: intel: Adjust ops handling during thermal zone registration Rafael J. Wysocki
5 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-02-05 21:18 UTC (permalink / raw)
To: Linux PM
Cc: Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba, Zhang Rui,
Srinivas Pandruvada, Stanislaw Gruszka,
AngeloGioacchino Del Regno
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The current code requires thermal zone creators to pass pointers to
writable ops to thermal_zone_device_register_with_trips() which needs
to modify the target struct thermal_zone_device_ops object if the
"critical" operation in it is NULL.
Moreover, the callers of thermal_zone_device_register_with_trips() are
required to hold on to the struct thermal_zone_device_ops object passed
to it until the given thermal zone is unregistered.
Both of these requirements are quite inconvenient, so modify struct
thermal_zone_device to contain struct thermal_zone_device_ops as field and
make thermal_zone_device_register_with_trips() copy the contents of the
struct thermal_zone_device_ops passed to it via a pointer (which can be
const now) to that field. Also adjust the code using thermal zone ops
accordingly.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/thermal/thermal_core.c | 40 +++++++++++++++++++-------------------
drivers/thermal/thermal_helpers.c | 10 ++++-----
drivers/thermal/thermal_hwmon.c | 4 +--
drivers/thermal/thermal_sysfs.c | 12 +++++------
drivers/thermal/thermal_trip.c | 4 +--
include/linux/thermal.h | 6 ++---
6 files changed, 38 insertions(+), 38 deletions(-)
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -182,7 +182,7 @@ struct thermal_zone_device {
int prev_low_trip;
int prev_high_trip;
atomic_t need_update;
- struct thermal_zone_device_ops *ops;
+ struct thermal_zone_device_ops ops;
struct thermal_zone_params *tzp;
struct thermal_governor *governor;
void *governor_data;
@@ -318,14 +318,14 @@ struct thermal_zone_device *thermal_zone
const struct thermal_trip *trips,
int num_trips, int mask,
void *devdata,
- struct thermal_zone_device_ops *ops,
+ const struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp,
int passive_delay, int polling_delay);
struct thermal_zone_device *thermal_tripless_zone_device_register(
const char *type,
void *devdata,
- struct thermal_zone_device_ops *ops,
+ const struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp);
void thermal_zone_device_unregister(struct thermal_zone_device *tz);
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -356,9 +356,9 @@ static void handle_critical_trips(struct
trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type);
if (trip->type == THERMAL_TRIP_CRITICAL)
- tz->ops->critical(tz);
- else if (tz->ops->hot)
- tz->ops->hot(tz);
+ tz->ops.critical(tz);
+ else if (tz->ops.hot)
+ tz->ops.hot(tz);
}
static void handle_thermal_trip(struct thermal_zone_device *tz,
@@ -493,8 +493,8 @@ static int thermal_zone_device_set_mode(
return ret;
}
- if (tz->ops->change_mode)
- ret = tz->ops->change_mode(tz, mode);
+ if (tz->ops.change_mode)
+ ret = tz->ops.change_mode(tz, mode);
if (!ret)
tz->mode = mode;
@@ -867,8 +867,8 @@ static void bind_cdev(struct thermal_coo
struct thermal_zone_device *pos = NULL;
list_for_each_entry(pos, &thermal_tz_list, node) {
- if (pos->ops->bind) {
- ret = pos->ops->bind(pos, cdev);
+ if (pos->ops.bind) {
+ ret = pos->ops.bind(pos, cdev);
if (ret)
print_bind_err_msg(pos, cdev, ret);
}
@@ -1184,8 +1184,8 @@ void thermal_cooling_device_unregister(s
/* Unbind all thermal zones associated with 'this' cdev */
list_for_each_entry(tz, &thermal_tz_list, node) {
- if (tz->ops->unbind)
- tz->ops->unbind(tz, cdev);
+ if (tz->ops.unbind)
+ tz->ops.unbind(tz, cdev);
}
mutex_unlock(&thermal_list_lock);
@@ -1199,13 +1199,13 @@ static void bind_tz(struct thermal_zone_
int ret;
struct thermal_cooling_device *pos = NULL;
- if (!tz->ops->bind)
+ if (!tz->ops.bind)
return;
mutex_lock(&thermal_list_lock);
list_for_each_entry(pos, &thermal_cdev_list, node) {
- ret = tz->ops->bind(tz, pos);
+ ret = tz->ops.bind(tz, pos);
if (ret)
print_bind_err_msg(tz, pos, ret);
}
@@ -1224,8 +1224,8 @@ int thermal_zone_get_crit_temp(struct th
{
int i, ret = -EINVAL;
- if (tz->ops->get_crit_temp)
- return tz->ops->get_crit_temp(tz, temp);
+ if (tz->ops.get_crit_temp)
+ return tz->ops.get_crit_temp(tz, temp);
if (!tz->trips)
return -EINVAL;
@@ -1276,7 +1276,7 @@ thermal_zone_device_register_with_trips(
const struct thermal_trip *trips,
int num_trips, int mask,
void *devdata,
- struct thermal_zone_device_ops *ops,
+ const struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp,
int passive_delay, int polling_delay)
{
@@ -1350,10 +1350,10 @@ thermal_zone_device_register_with_trips(
tz->id = id;
strscpy(tz->type, type, sizeof(tz->type));
- if (!ops->critical)
- ops->critical = thermal_zone_device_critical;
+ tz->ops = *ops;
+ if (!tz->ops.critical)
+ tz->ops.critical = thermal_zone_device_critical;
- tz->ops = ops;
tz->device.class = thermal_class;
tz->devdata = devdata;
memcpy(tz->trips, trips, num_trips * sizeof(trips[0]));
@@ -1439,7 +1439,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_device_re
struct thermal_zone_device *thermal_tripless_zone_device_register(
const char *type,
void *devdata,
- struct thermal_zone_device_ops *ops,
+ const struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp)
{
return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
@@ -1501,8 +1501,8 @@ void thermal_zone_device_unregister(stru
/* Unbind all cdevs associated with 'this' thermal zone */
list_for_each_entry(cdev, &thermal_cdev_list, node)
- if (tz->ops->unbind)
- tz->ops->unbind(tz, cdev);
+ if (tz->ops.unbind)
+ tz->ops.unbind(tz, cdev);
mutex_unlock(&thermal_list_lock);
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -123,8 +123,8 @@ trip_point_temp_store(struct device *dev
trip = &tz->trips[trip_id];
if (temp != trip->temperature) {
- if (tz->ops->set_trip_temp) {
- ret = tz->ops->set_trip_temp(tz, trip_id, temp);
+ if (tz->ops.set_trip_temp) {
+ ret = tz->ops.set_trip_temp(tz, trip_id, temp);
if (ret)
goto unlock;
}
@@ -174,8 +174,8 @@ trip_point_hyst_store(struct device *dev
trip = &tz->trips[trip_id];
if (hyst != trip->hysteresis) {
- if (tz->ops->set_trip_hyst) {
- ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);
+ if (tz->ops.set_trip_hyst) {
+ ret = tz->ops.set_trip_hyst(tz, trip_id, hyst);
if (ret)
goto unlock;
}
@@ -250,10 +250,10 @@ emul_temp_store(struct device *dev, stru
mutex_lock(&tz->lock);
- if (!tz->ops->set_emul_temp)
+ if (!tz->ops.set_emul_temp)
tz->emul_temperature = temperature;
else
- ret = tz->ops->set_emul_temp(tz, temperature);
+ ret = tz->ops.set_emul_temp(tz, temperature);
if (!ret)
__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -26,8 +26,8 @@ int get_tz_trend(struct thermal_zone_dev
{
enum thermal_trend trend;
- if (tz->emul_temperature || !tz->ops->get_trend ||
- tz->ops->get_trend(tz, trip, &trend)) {
+ if (tz->emul_temperature || !tz->ops.get_trend ||
+ tz->ops.get_trend(tz, trip, &trend)) {
if (tz->temperature > tz->last_temperature)
trend = THERMAL_TREND_RAISING;
else if (tz->temperature < tz->last_temperature)
@@ -75,7 +75,7 @@ EXPORT_SYMBOL(get_thermal_instance);
* temperature and fill @temp.
*
* Both tz and tz->ops must be valid pointers when calling this function,
- * and the tz->ops->get_temp callback must be provided.
+ * and the tz->ops.get_temp callback must be provided.
* The function must be called under tz->lock.
*
* Return: On success returns 0, an error code otherwise
@@ -88,7 +88,7 @@ int __thermal_zone_get_temp(struct therm
lockdep_assert_held(&tz->lock);
- ret = tz->ops->get_temp(tz, temp);
+ ret = tz->ops.get_temp(tz, temp);
if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
for_each_trip(tz, trip) {
@@ -132,7 +132,7 @@ int thermal_zone_get_temp(struct thermal
mutex_lock(&tz->lock);
- if (!tz->ops->get_temp) {
+ if (!tz->ops.get_temp) {
ret = -EINVAL;
goto unlock;
}
Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_trip.c
+++ linux-pm/drivers/thermal/thermal_trip.c
@@ -70,7 +70,7 @@ void __thermal_zone_set_trips(struct the
lockdep_assert_held(&tz->lock);
- if (!tz->ops->set_trips)
+ if (!tz->ops.set_trips)
return;
for_each_trip(tz, trip) {
@@ -114,7 +114,7 @@ void __thermal_zone_set_trips(struct the
* Set a temperature window. When this window is left the driver
* must inform the thermal core via thermal_zone_device_update.
*/
- ret = tz->ops->set_trips(tz, low, high);
+ ret = tz->ops.set_trips(tz, low, high);
if (ret)
dev_err(&tz->device, "Failed to set trips: %d\n", ret);
}
Index: linux-pm/drivers/thermal/thermal_hwmon.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_hwmon.c
+++ linux-pm/drivers/thermal/thermal_hwmon.c
@@ -80,7 +80,7 @@ temp_crit_show(struct device *dev, struc
mutex_lock(&tz->lock);
- ret = tz->ops->get_crit_temp(tz, &temperature);
+ ret = tz->ops.get_crit_temp(tz, &temperature);
mutex_unlock(&tz->lock);
@@ -132,7 +132,7 @@ thermal_hwmon_lookup_temp(const struct t
static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz)
{
int temp;
- return tz->ops->get_crit_temp && !tz->ops->get_crit_temp(tz, &temp);
+ return tz->ops.get_crit_temp && !tz->ops.get_crit_temp(tz, &temp);
}
int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 4/6] thermal: core: Store zone ops in struct thermal_zone_device
2024-02-05 21:18 ` [PATCH v1 4/6] thermal: core: Store zone ops in struct thermal_zone_device Rafael J. Wysocki
@ 2024-02-08 8:48 ` Stanislaw Gruszka
0 siblings, 0 replies; 14+ messages in thread
From: Stanislaw Gruszka @ 2024-02-08 8:48 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba,
Zhang Rui, Srinivas Pandruvada, AngeloGioacchino Del Regno
On Mon, Feb 05, 2024 at 10:18:02PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The current code requires thermal zone creators to pass pointers to
> writable ops to thermal_zone_device_register_with_trips() which needs
> to modify the target struct thermal_zone_device_ops object if the
> "critical" operation in it is NULL.
>
> Moreover, the callers of thermal_zone_device_register_with_trips() are
> required to hold on to the struct thermal_zone_device_ops object passed
> to it until the given thermal zone is unregistered.
>
> Both of these requirements are quite inconvenient, so modify struct
> thermal_zone_device to contain struct thermal_zone_device_ops as field and
> make thermal_zone_device_register_with_trips() copy the contents of the
> struct thermal_zone_device_ops passed to it via a pointer (which can be
> const now) to that field. Also adjust the code using thermal zone ops
> accordingly.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/thermal/thermal_core.c | 40 +++++++++++++++++++-------------------
> drivers/thermal/thermal_helpers.c | 10 ++++-----
> drivers/thermal/thermal_hwmon.c | 4 +--
> drivers/thermal/thermal_sysfs.c | 12 +++++------
> drivers/thermal/thermal_trip.c | 4 +--
> include/linux/thermal.h | 6 ++---
> 6 files changed, 38 insertions(+), 38 deletions(-)
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -182,7 +182,7 @@ struct thermal_zone_device {
> int prev_low_trip;
> int prev_high_trip;
> atomic_t need_update;
> - struct thermal_zone_device_ops *ops;
> + struct thermal_zone_device_ops ops;
> struct thermal_zone_params *tzp;
> struct thermal_governor *governor;
> void *governor_data;
> @@ -318,14 +318,14 @@ struct thermal_zone_device *thermal_zone
> const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> - struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_device_ops *ops,
> const struct thermal_zone_params *tzp,
> int passive_delay, int polling_delay);
>
> struct thermal_zone_device *thermal_tripless_zone_device_register(
> const char *type,
> void *devdata,
> - struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_device_ops *ops,
> const struct thermal_zone_params *tzp);
>
> void thermal_zone_device_unregister(struct thermal_zone_device *tz);
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -356,9 +356,9 @@ static void handle_critical_trips(struct
> trace_thermal_zone_trip(tz, thermal_zone_trip_id(tz, trip), trip->type);
>
> if (trip->type == THERMAL_TRIP_CRITICAL)
> - tz->ops->critical(tz);
> - else if (tz->ops->hot)
> - tz->ops->hot(tz);
> + tz->ops.critical(tz);
> + else if (tz->ops.hot)
> + tz->ops.hot(tz);
> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz,
> @@ -493,8 +493,8 @@ static int thermal_zone_device_set_mode(
> return ret;
> }
>
> - if (tz->ops->change_mode)
> - ret = tz->ops->change_mode(tz, mode);
> + if (tz->ops.change_mode)
> + ret = tz->ops.change_mode(tz, mode);
>
> if (!ret)
> tz->mode = mode;
> @@ -867,8 +867,8 @@ static void bind_cdev(struct thermal_coo
> struct thermal_zone_device *pos = NULL;
>
> list_for_each_entry(pos, &thermal_tz_list, node) {
> - if (pos->ops->bind) {
> - ret = pos->ops->bind(pos, cdev);
> + if (pos->ops.bind) {
> + ret = pos->ops.bind(pos, cdev);
> if (ret)
> print_bind_err_msg(pos, cdev, ret);
> }
> @@ -1184,8 +1184,8 @@ void thermal_cooling_device_unregister(s
>
> /* Unbind all thermal zones associated with 'this' cdev */
> list_for_each_entry(tz, &thermal_tz_list, node) {
> - if (tz->ops->unbind)
> - tz->ops->unbind(tz, cdev);
> + if (tz->ops.unbind)
> + tz->ops.unbind(tz, cdev);
> }
>
> mutex_unlock(&thermal_list_lock);
> @@ -1199,13 +1199,13 @@ static void bind_tz(struct thermal_zone_
> int ret;
> struct thermal_cooling_device *pos = NULL;
>
> - if (!tz->ops->bind)
> + if (!tz->ops.bind)
> return;
>
> mutex_lock(&thermal_list_lock);
>
> list_for_each_entry(pos, &thermal_cdev_list, node) {
> - ret = tz->ops->bind(tz, pos);
> + ret = tz->ops.bind(tz, pos);
> if (ret)
> print_bind_err_msg(tz, pos, ret);
> }
> @@ -1224,8 +1224,8 @@ int thermal_zone_get_crit_temp(struct th
> {
> int i, ret = -EINVAL;
>
> - if (tz->ops->get_crit_temp)
> - return tz->ops->get_crit_temp(tz, temp);
> + if (tz->ops.get_crit_temp)
> + return tz->ops.get_crit_temp(tz, temp);
>
> if (!tz->trips)
> return -EINVAL;
> @@ -1276,7 +1276,7 @@ thermal_zone_device_register_with_trips(
> const struct thermal_trip *trips,
> int num_trips, int mask,
> void *devdata,
> - struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_device_ops *ops,
> const struct thermal_zone_params *tzp,
> int passive_delay, int polling_delay)
> {
> @@ -1350,10 +1350,10 @@ thermal_zone_device_register_with_trips(
> tz->id = id;
> strscpy(tz->type, type, sizeof(tz->type));
>
> - if (!ops->critical)
> - ops->critical = thermal_zone_device_critical;
> + tz->ops = *ops;
> + if (!tz->ops.critical)
> + tz->ops.critical = thermal_zone_device_critical;
>
> - tz->ops = ops;
> tz->device.class = thermal_class;
> tz->devdata = devdata;
> memcpy(tz->trips, trips, num_trips * sizeof(trips[0]));
> @@ -1439,7 +1439,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_device_re
> struct thermal_zone_device *thermal_tripless_zone_device_register(
> const char *type,
> void *devdata,
> - struct thermal_zone_device_ops *ops,
> + const struct thermal_zone_device_ops *ops,
> const struct thermal_zone_params *tzp)
> {
> return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
> @@ -1501,8 +1501,8 @@ void thermal_zone_device_unregister(stru
>
> /* Unbind all cdevs associated with 'this' thermal zone */
> list_for_each_entry(cdev, &thermal_cdev_list, node)
> - if (tz->ops->unbind)
> - tz->ops->unbind(tz, cdev);
> + if (tz->ops.unbind)
> + tz->ops.unbind(tz, cdev);
>
> mutex_unlock(&thermal_list_lock);
>
> Index: linux-pm/drivers/thermal/thermal_sysfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> +++ linux-pm/drivers/thermal/thermal_sysfs.c
> @@ -123,8 +123,8 @@ trip_point_temp_store(struct device *dev
> trip = &tz->trips[trip_id];
>
> if (temp != trip->temperature) {
> - if (tz->ops->set_trip_temp) {
> - ret = tz->ops->set_trip_temp(tz, trip_id, temp);
> + if (tz->ops.set_trip_temp) {
> + ret = tz->ops.set_trip_temp(tz, trip_id, temp);
> if (ret)
> goto unlock;
> }
> @@ -174,8 +174,8 @@ trip_point_hyst_store(struct device *dev
> trip = &tz->trips[trip_id];
>
> if (hyst != trip->hysteresis) {
> - if (tz->ops->set_trip_hyst) {
> - ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);
> + if (tz->ops.set_trip_hyst) {
> + ret = tz->ops.set_trip_hyst(tz, trip_id, hyst);
> if (ret)
> goto unlock;
> }
> @@ -250,10 +250,10 @@ emul_temp_store(struct device *dev, stru
>
> mutex_lock(&tz->lock);
>
> - if (!tz->ops->set_emul_temp)
> + if (!tz->ops.set_emul_temp)
> tz->emul_temperature = temperature;
> else
> - ret = tz->ops->set_emul_temp(tz, temperature);
> + ret = tz->ops.set_emul_temp(tz, temperature);
>
> if (!ret)
> __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> Index: linux-pm/drivers/thermal/thermal_helpers.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_helpers.c
> +++ linux-pm/drivers/thermal/thermal_helpers.c
> @@ -26,8 +26,8 @@ int get_tz_trend(struct thermal_zone_dev
> {
> enum thermal_trend trend;
>
> - if (tz->emul_temperature || !tz->ops->get_trend ||
> - tz->ops->get_trend(tz, trip, &trend)) {
> + if (tz->emul_temperature || !tz->ops.get_trend ||
> + tz->ops.get_trend(tz, trip, &trend)) {
> if (tz->temperature > tz->last_temperature)
> trend = THERMAL_TREND_RAISING;
> else if (tz->temperature < tz->last_temperature)
> @@ -75,7 +75,7 @@ EXPORT_SYMBOL(get_thermal_instance);
> * temperature and fill @temp.
> *
> * Both tz and tz->ops must be valid pointers when calling this function,
> - * and the tz->ops->get_temp callback must be provided.
> + * and the tz->ops.get_temp callback must be provided.
> * The function must be called under tz->lock.
> *
> * Return: On success returns 0, an error code otherwise
> @@ -88,7 +88,7 @@ int __thermal_zone_get_temp(struct therm
>
> lockdep_assert_held(&tz->lock);
>
> - ret = tz->ops->get_temp(tz, temp);
> + ret = tz->ops.get_temp(tz, temp);
>
> if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
> for_each_trip(tz, trip) {
> @@ -132,7 +132,7 @@ int thermal_zone_get_temp(struct thermal
>
> mutex_lock(&tz->lock);
>
> - if (!tz->ops->get_temp) {
> + if (!tz->ops.get_temp) {
> ret = -EINVAL;
> goto unlock;
> }
> Index: linux-pm/drivers/thermal/thermal_trip.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_trip.c
> +++ linux-pm/drivers/thermal/thermal_trip.c
> @@ -70,7 +70,7 @@ void __thermal_zone_set_trips(struct the
>
> lockdep_assert_held(&tz->lock);
>
> - if (!tz->ops->set_trips)
> + if (!tz->ops.set_trips)
> return;
>
> for_each_trip(tz, trip) {
> @@ -114,7 +114,7 @@ void __thermal_zone_set_trips(struct the
> * Set a temperature window. When this window is left the driver
> * must inform the thermal core via thermal_zone_device_update.
> */
> - ret = tz->ops->set_trips(tz, low, high);
> + ret = tz->ops.set_trips(tz, low, high);
> if (ret)
> dev_err(&tz->device, "Failed to set trips: %d\n", ret);
> }
> Index: linux-pm/drivers/thermal/thermal_hwmon.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_hwmon.c
> +++ linux-pm/drivers/thermal/thermal_hwmon.c
> @@ -80,7 +80,7 @@ temp_crit_show(struct device *dev, struc
>
> mutex_lock(&tz->lock);
>
> - ret = tz->ops->get_crit_temp(tz, &temperature);
> + ret = tz->ops.get_crit_temp(tz, &temperature);
>
> mutex_unlock(&tz->lock);
>
> @@ -132,7 +132,7 @@ thermal_hwmon_lookup_temp(const struct t
> static bool thermal_zone_crit_temp_valid(struct thermal_zone_device *tz)
> {
> int temp;
> - return tz->ops->get_crit_temp && !tz->ops->get_crit_temp(tz, &temp);
> + return tz->ops.get_crit_temp && !tz->ops.get_crit_temp(tz, &temp);
> }
>
> int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 5/6] thermal: ACPI: Constify acpi_thermal_zone_ops
2024-02-05 21:12 [PATCH v1 0/6] thermal: Store trips table and ops in thermal_zone_device Rafael J. Wysocki
` (3 preceding siblings ...)
2024-02-05 21:18 ` [PATCH v1 4/6] thermal: core: Store zone ops in struct thermal_zone_device Rafael J. Wysocki
@ 2024-02-05 21:18 ` Rafael J. Wysocki
2024-02-08 8:49 ` Stanislaw Gruszka
2024-02-05 21:20 ` [PATCH v1 6/6] thermal: intel: Adjust ops handling during thermal zone registration Rafael J. Wysocki
5 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-02-05 21:18 UTC (permalink / raw)
To: Linux PM
Cc: Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba, Zhang Rui,
Srinivas Pandruvada, Stanislaw Gruszka,
AngeloGioacchino Del Regno
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Because thermal zone operations are now stored directly in struct
thermal_zone_device, acpi_thermal_zone_ops need not be modified by
the thermal core and so it can be const.
Adjust the code accordingly.
No functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -620,7 +620,7 @@ acpi_thermal_unbind_cooling_device(struc
return acpi_thermal_bind_unbind_cdev(thermal, cdev, false);
}
-static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
+static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
.bind = acpi_thermal_bind_cooling_device,
.unbind = acpi_thermal_unbind_cooling_device,
.get_temp = thermal_get_temp,
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 5/6] thermal: ACPI: Constify acpi_thermal_zone_ops
2024-02-05 21:18 ` [PATCH v1 5/6] thermal: ACPI: Constify acpi_thermal_zone_ops Rafael J. Wysocki
@ 2024-02-08 8:49 ` Stanislaw Gruszka
0 siblings, 0 replies; 14+ messages in thread
From: Stanislaw Gruszka @ 2024-02-08 8:49 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba,
Zhang Rui, Srinivas Pandruvada, AngeloGioacchino Del Regno
On Mon, Feb 05, 2024 at 10:18:50PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Because thermal zone operations are now stored directly in struct
> thermal_zone_device, acpi_thermal_zone_ops need not be modified by
> the thermal core and so it can be const.
>
> Adjust the code accordingly.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/acpi/thermal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -620,7 +620,7 @@ acpi_thermal_unbind_cooling_device(struc
> return acpi_thermal_bind_unbind_cdev(thermal, cdev, false);
> }
>
> -static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> +static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> .bind = acpi_thermal_bind_cooling_device,
> .unbind = acpi_thermal_unbind_cooling_device,
> .get_temp = thermal_get_temp,
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 6/6] thermal: intel: Adjust ops handling during thermal zone registration
2024-02-05 21:12 [PATCH v1 0/6] thermal: Store trips table and ops in thermal_zone_device Rafael J. Wysocki
` (4 preceding siblings ...)
2024-02-05 21:18 ` [PATCH v1 5/6] thermal: ACPI: Constify acpi_thermal_zone_ops Rafael J. Wysocki
@ 2024-02-05 21:20 ` Rafael J. Wysocki
2024-02-08 8:51 ` Stanislaw Gruszka
5 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2024-02-05 21:20 UTC (permalink / raw)
To: Linux PM
Cc: Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba, Zhang Rui,
Srinivas Pandruvada, Stanislaw Gruszka,
AngeloGioacchino Del Regno
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Because thermal zone operations are now stored directly in struct
thermal_zone_device, thermal zone creators can discard the operations
structure after the zone registration is complete, or it can be made
read-only.
Accordingly, make int340x_thermal_zone_add() use a local variable to
represent thermal zone operations, so it is freed automatically upon the
function exit, and make the other Intel thermal drivers use const zone
operations structures.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 26 ++--------
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h | 1
drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c | 2
drivers/thermal/intel/intel_pch_thermal.c | 2
drivers/thermal/intel/intel_quark_dts_thermal.c | 2
drivers/thermal/intel/intel_soc_dts_iosf.c | 2
drivers/thermal/intel/x86_pkg_temp_thermal.c | 2
7 files changed, 11 insertions(+), 26 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
@@ -61,12 +61,6 @@ static void int340x_thermal_critical(str
dev_dbg(&zone->device, "%s: critical temperature reached\n", zone->type);
}
-static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
- .get_temp = int340x_thermal_get_zone_temp,
- .set_trip_temp = int340x_thermal_set_trip_temp,
- .critical = int340x_thermal_critical,
-};
-
static inline void *int_to_trip_priv(int i)
{
return (void *)(long)i;
@@ -126,6 +120,11 @@ static struct thermal_zone_params int340
struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
int (*get_temp) (struct thermal_zone_device *, int *))
{
+ const struct thermal_zone_device_ops zone_ops = {
+ .set_trip_temp = int340x_thermal_set_trip_temp,
+ .critical = int340x_thermal_critical,
+ .get_temp = get_temp ? get_temp : int340x_thermal_get_zone_temp,
+ };
struct int34x_thermal_zone *int34x_zone;
struct thermal_trip *zone_trips;
unsigned long long trip_cnt = 0;
@@ -140,16 +139,6 @@ struct int34x_thermal_zone *int340x_ther
int34x_zone->adev = adev;
- int34x_zone->ops = kmemdup(&int340x_thermal_zone_ops,
- sizeof(int340x_thermal_zone_ops), GFP_KERNEL);
- if (!int34x_zone->ops) {
- ret = -ENOMEM;
- goto err_ops_alloc;
- }
-
- if (get_temp)
- int34x_zone->ops->get_temp = get_temp;
-
status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
if (ACPI_SUCCESS(status)) {
int34x_zone->aux_trip_nr = trip_cnt;
@@ -185,7 +174,7 @@ struct int34x_thermal_zone *int340x_ther
acpi_device_bid(adev),
zone_trips, trip_cnt,
trip_mask, int34x_zone,
- int34x_zone->ops,
+ &zone_ops,
&int340x_thermal_params,
0, 0);
kfree(zone_trips);
@@ -205,8 +194,6 @@ err_enable:
err_thermal_zone:
acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
err_trips_alloc:
- kfree(int34x_zone->ops);
-err_ops_alloc:
kfree(int34x_zone);
return ERR_PTR(ret);
}
@@ -216,7 +203,6 @@ void int340x_thermal_zone_remove(struct
{
thermal_zone_device_unregister(int34x_zone->zone);
acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
- kfree(int34x_zone->ops);
kfree(int34x_zone);
}
EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
+++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
@@ -22,7 +22,6 @@ struct int34x_thermal_zone {
struct acpi_device *adev;
int aux_trip_nr;
struct thermal_zone_device *zone;
- struct thermal_zone_device_ops *ops;
void *priv_data;
struct acpi_lpat_conversion_table *lpat_table;
};
Index: linux-pm/drivers/thermal/intel/intel_pch_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_pch_thermal.c
+++ linux-pm/drivers/thermal/intel/intel_pch_thermal.c
@@ -131,7 +131,7 @@ static void pch_critical(struct thermal_
thermal_zone_device_type(tzd));
}
-static struct thermal_zone_device_ops tzd_ops = {
+static const struct thermal_zone_device_ops tzd_ops = {
.get_temp = pch_thermal_get_temp,
.critical = pch_critical,
};
Index: linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_quark_dts_thermal.c
+++ linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
@@ -292,7 +292,7 @@ static int sys_change_mode(struct therma
return ret;
}
-static struct thermal_zone_device_ops tzone_ops = {
+static const struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
.set_trip_temp = sys_set_trip_temp,
.change_mode = sys_change_mode,
Index: linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
+++ linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
@@ -233,7 +233,7 @@ static int get_trip_temp(struct proc_the
return temp;
}
-static struct thermal_zone_device_ops tzone_ops = {
+static const struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
.set_trip_temp = sys_set_trip_temp,
};
Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
+++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
@@ -168,7 +168,7 @@ static int sys_get_curr_temp(struct ther
return 0;
}
-static struct thermal_zone_device_ops tzone_ops = {
+static const struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
.set_trip_temp = sys_set_trip_temp,
};
Index: linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/intel/x86_pkg_temp_thermal.c
+++ linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
@@ -166,7 +166,7 @@ sys_set_trip_temp(struct thermal_zone_de
}
/* Thermal zone callback registry */
-static struct thermal_zone_device_ops tzone_ops = {
+static const struct thermal_zone_device_ops tzone_ops = {
.get_temp = sys_get_curr_temp,
.set_trip_temp = sys_set_trip_temp,
};
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 6/6] thermal: intel: Adjust ops handling during thermal zone registration
2024-02-05 21:20 ` [PATCH v1 6/6] thermal: intel: Adjust ops handling during thermal zone registration Rafael J. Wysocki
@ 2024-02-08 8:51 ` Stanislaw Gruszka
0 siblings, 0 replies; 14+ messages in thread
From: Stanislaw Gruszka @ 2024-02-08 8:51 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Daniel Lezcano, LKML, Linux ACPI, Lukasz Luba,
Zhang Rui, Srinivas Pandruvada, AngeloGioacchino Del Regno
On Mon, Feb 05, 2024 at 10:20:32PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Because thermal zone operations are now stored directly in struct
> thermal_zone_device, thermal zone creators can discard the operations
> structure after the zone registration is complete, or it can be made
> read-only.
>
> Accordingly, make int340x_thermal_zone_add() use a local variable to
> represent thermal zone operations, so it is freed automatically upon the
> function exit, and make the other Intel thermal drivers use const zone
> operations structures.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
> drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 26 ++--------
> drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h | 1
> drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c | 2
> drivers/thermal/intel/intel_pch_thermal.c | 2
> drivers/thermal/intel/intel_quark_dts_thermal.c | 2
> drivers/thermal/intel/intel_soc_dts_iosf.c | 2
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 2
> 7 files changed, 11 insertions(+), 26 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
> @@ -61,12 +61,6 @@ static void int340x_thermal_critical(str
> dev_dbg(&zone->device, "%s: critical temperature reached\n", zone->type);
> }
>
> -static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
> - .get_temp = int340x_thermal_get_zone_temp,
> - .set_trip_temp = int340x_thermal_set_trip_temp,
> - .critical = int340x_thermal_critical,
> -};
> -
> static inline void *int_to_trip_priv(int i)
> {
> return (void *)(long)i;
> @@ -126,6 +120,11 @@ static struct thermal_zone_params int340
> struct int34x_thermal_zone *int340x_thermal_zone_add(struct acpi_device *adev,
> int (*get_temp) (struct thermal_zone_device *, int *))
> {
> + const struct thermal_zone_device_ops zone_ops = {
> + .set_trip_temp = int340x_thermal_set_trip_temp,
> + .critical = int340x_thermal_critical,
> + .get_temp = get_temp ? get_temp : int340x_thermal_get_zone_temp,
> + };
> struct int34x_thermal_zone *int34x_zone;
> struct thermal_trip *zone_trips;
> unsigned long long trip_cnt = 0;
> @@ -140,16 +139,6 @@ struct int34x_thermal_zone *int340x_ther
>
> int34x_zone->adev = adev;
>
> - int34x_zone->ops = kmemdup(&int340x_thermal_zone_ops,
> - sizeof(int340x_thermal_zone_ops), GFP_KERNEL);
> - if (!int34x_zone->ops) {
> - ret = -ENOMEM;
> - goto err_ops_alloc;
> - }
> -
> - if (get_temp)
> - int34x_zone->ops->get_temp = get_temp;
> -
> status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
> if (ACPI_SUCCESS(status)) {
> int34x_zone->aux_trip_nr = trip_cnt;
> @@ -185,7 +174,7 @@ struct int34x_thermal_zone *int340x_ther
> acpi_device_bid(adev),
> zone_trips, trip_cnt,
> trip_mask, int34x_zone,
> - int34x_zone->ops,
> + &zone_ops,
> &int340x_thermal_params,
> 0, 0);
> kfree(zone_trips);
> @@ -205,8 +194,6 @@ err_enable:
> err_thermal_zone:
> acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
> err_trips_alloc:
> - kfree(int34x_zone->ops);
> -err_ops_alloc:
> kfree(int34x_zone);
> return ERR_PTR(ret);
> }
> @@ -216,7 +203,6 @@ void int340x_thermal_zone_remove(struct
> {
> thermal_zone_device_unregister(int34x_zone->zone);
> acpi_lpat_free_conversion_table(int34x_zone->lpat_table);
> - kfree(int34x_zone->ops);
> kfree(int34x_zone);
> }
> EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove);
> Index: linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> +++ linux-pm/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.h
> @@ -22,7 +22,6 @@ struct int34x_thermal_zone {
> struct acpi_device *adev;
> int aux_trip_nr;
> struct thermal_zone_device *zone;
> - struct thermal_zone_device_ops *ops;
> void *priv_data;
> struct acpi_lpat_conversion_table *lpat_table;
> };
> Index: linux-pm/drivers/thermal/intel/intel_pch_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_pch_thermal.c
> +++ linux-pm/drivers/thermal/intel/intel_pch_thermal.c
> @@ -131,7 +131,7 @@ static void pch_critical(struct thermal_
> thermal_zone_device_type(tzd));
> }
>
> -static struct thermal_zone_device_ops tzd_ops = {
> +static const struct thermal_zone_device_ops tzd_ops = {
> .get_temp = pch_thermal_get_temp,
> .critical = pch_critical,
> };
> Index: linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ linux-pm/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -292,7 +292,7 @@ static int sys_change_mode(struct therma
> return ret;
> }
>
> -static struct thermal_zone_device_ops tzone_ops = {
> +static const struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> .set_trip_temp = sys_set_trip_temp,
> .change_mode = sys_change_mode,
> Index: linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> +++ linux-pm/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> @@ -233,7 +233,7 @@ static int get_trip_temp(struct proc_the
> return temp;
> }
>
> -static struct thermal_zone_device_ops tzone_ops = {
> +static const struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> .set_trip_temp = sys_set_trip_temp,
> };
> Index: linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_soc_dts_iosf.c
> +++ linux-pm/drivers/thermal/intel/intel_soc_dts_iosf.c
> @@ -168,7 +168,7 @@ static int sys_get_curr_temp(struct ther
> return 0;
> }
>
> -static struct thermal_zone_device_ops tzone_ops = {
> +static const struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> .set_trip_temp = sys_set_trip_temp,
> };
> Index: linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ linux-pm/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -166,7 +166,7 @@ sys_set_trip_temp(struct thermal_zone_de
> }
>
> /* Thermal zone callback registry */
> -static struct thermal_zone_device_ops tzone_ops = {
> +static const struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> .set_trip_temp = sys_set_trip_temp,
> };
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread