* [PATCH v2 1/9] thermal: Get rid of CONFIG_THERMAL_WRITABLE_TRIPS
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
@ 2024-02-12 18:26 ` Rafael J. Wysocki
2024-02-22 13:50 ` Daniel Lezcano
2024-02-12 18:31 ` [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip Rafael J. Wysocki
` (8 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:26 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The only difference made by CONFIG_THERMAL_WRITABLE_TRIPS is whether or
not the writable trips mask passed during thermal zone registration
will take any effect, but whoever passes a non-zero writable trips mask
to thermal_zone_device_register_with_trips() can be forgiven thinking
that it will always work.
Moreover, some thermal drivers expect user space to set trip temperature
values, so they select CONFIG_THERMAL_WRITABLE_TRIPS, possibly overriding
a manual choice to unset it and going against the design purportedly
allowing system integrators to decide on the writability of trip points
for the given kernel build. It is also set in one platform's defconfig.
Forthermore, CONFIG_THERMAL_WRITABLE_TRIPS only affects trip temperature,
because trip hysteresis is writable as long as the thermal zone provides
a callback to update it, regardless of the CONFIG_THERMAL_WRITABLE_TRIPS
value.
The above means that the symbol in question is used inconsistently and
its purpose is at least moot, so remove it and always take the writable
trip mask passed to thermal_zone_device_register_with_trips() into
account.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2: No changes
---
arch/arm/configs/imx_v6_v7_defconfig | 1 -
drivers/thermal/Kconfig | 11 -----------
drivers/thermal/intel/Kconfig | 2 --
drivers/thermal/thermal_sysfs.c | 3 +--
4 files changed, 1 insertion(+), 16 deletions(-)
Index: linux-pm/drivers/thermal/Kconfig
===================================================================
--- linux-pm.orig/drivers/thermal/Kconfig
+++ linux-pm/drivers/thermal/Kconfig
@@ -83,17 +83,6 @@ config THERMAL_OF
Say 'Y' here if you need to build thermal infrastructure
based on device tree.
-config THERMAL_WRITABLE_TRIPS
- bool "Enable writable trip points"
- help
- This option allows the system integrator to choose whether
- trip temperatures can be changed from userspace. The
- writable trips need to be specified when setting up the
- thermal zone but the choice here takes precedence.
-
- Say 'Y' here if you would like to allow userspace tools to
- change trip temperatures.
-
choice
prompt "Default Thermal governor"
default THERMAL_DEFAULT_GOV_STEP_WISE
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -458,8 +458,7 @@ static int create_trip_attrs(struct ther
tz->trip_temp_attrs[indx].name;
tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
- if (IS_ENABLED(CONFIG_THERMAL_WRITABLE_TRIPS) &&
- mask & (1 << indx)) {
+ if (mask & (1 << indx)) {
tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_temp_attrs[indx].attr.store =
trip_point_temp_store;
Index: linux-pm/drivers/thermal/intel/Kconfig
===================================================================
--- linux-pm.orig/drivers/thermal/intel/Kconfig
+++ linux-pm/drivers/thermal/intel/Kconfig
@@ -23,7 +23,6 @@ config X86_PKG_TEMP_THERMAL
tristate "X86 package temperature thermal driver"
depends on X86_THERMAL_VECTOR
select THERMAL_GOV_USER_SPACE
- select THERMAL_WRITABLE_TRIPS
select INTEL_TCC
default m
help
@@ -47,7 +46,6 @@ config INTEL_SOC_DTS_THERMAL
tristate "Intel SoCs DTS thermal driver"
depends on X86 && PCI && ACPI
select INTEL_SOC_DTS_IOSF_CORE
- select THERMAL_WRITABLE_TRIPS
help
Enable this to register Intel SoCs (e.g. Bay Trail) platform digital
temperature sensor (DTS). These SoCs have two additional DTSs in
Index: linux-pm/arch/arm/configs/imx_v6_v7_defconfig
===================================================================
--- linux-pm.orig/arch/arm/configs/imx_v6_v7_defconfig
+++ linux-pm/arch/arm/configs/imx_v6_v7_defconfig
@@ -228,7 +228,6 @@ CONFIG_SENSORS_IIO_HWMON=y
CONFIG_SENSORS_PWM_FAN=y
CONFIG_SENSORS_SY7636A=y
CONFIG_THERMAL_STATISTICS=y
-CONFIG_THERMAL_WRITABLE_TRIPS=y
CONFIG_CPU_THERMAL=y
CONFIG_IMX_THERMAL=y
CONFIG_WATCHDOG=y
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 1/9] thermal: Get rid of CONFIG_THERMAL_WRITABLE_TRIPS
2024-02-12 18:26 ` [PATCH v2 1/9] thermal: Get rid of CONFIG_THERMAL_WRITABLE_TRIPS Rafael J. Wysocki
@ 2024-02-22 13:50 ` Daniel Lezcano
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 13:50 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:26, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The only difference made by CONFIG_THERMAL_WRITABLE_TRIPS is whether or
> not the writable trips mask passed during thermal zone registration
> will take any effect, but whoever passes a non-zero writable trips mask
> to thermal_zone_device_register_with_trips() can be forgiven thinking
> that it will always work.
>
> Moreover, some thermal drivers expect user space to set trip temperature
> values, so they select CONFIG_THERMAL_WRITABLE_TRIPS, possibly overriding
> a manual choice to unset it and going against the design purportedly
> allowing system integrators to decide on the writability of trip points
> for the given kernel build. It is also set in one platform's defconfig.
>
> Forthermore, CONFIG_THERMAL_WRITABLE_TRIPS only affects trip temperature,
> because trip hysteresis is writable as long as the thermal zone provides
> a callback to update it, regardless of the CONFIG_THERMAL_WRITABLE_TRIPS
> value.
>
> The above means that the symbol in question is used inconsistently and
> its purpose is at least moot, so remove it and always take the writable
> trip mask passed to thermal_zone_device_register_with_trips() into
> account.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
2024-02-12 18:26 ` [PATCH v2 1/9] thermal: Get rid of CONFIG_THERMAL_WRITABLE_TRIPS Rafael J. Wysocki
@ 2024-02-12 18:31 ` Rafael J. Wysocki
2024-02-22 14:36 ` Daniel Lezcano
2024-02-22 19:48 ` [PATCH v2.1 " Rafael J. Wysocki
2024-02-12 18:32 ` [PATCH v2 3/9] thermal: core: Drop the .set_trip_hyst() thermal zone operation Rafael J. Wysocki
` (7 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:31 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
In order to allow thermal zone creators to specify the writability of
trip point temperature and hysteresis on a per-trip basis, add a flags
field to struct thermal_trip and define flags to represent the desired
trip properties.
Also make thermal_zone_device_register_with_trips() set the
THERMAL_TRIP_FLAG_RW_TEMP flag for all trips covered by the writable
trips mask passed to it and modify the thermal sysfs code to look at
the trip flags instead of using the writable trips mask directly or
checking the presence of the .set_trip_hyst() zone callback.
Additionally, make trip_point_temp_store() and trip_point_hyst_store()
fail with an error code if the trip passed to one of them has
THERMAL_TRIP_FLAG_RW_TEMP or THERMAL_TRIP_FLAG_RW_HYST,
respectively, clear in its flags.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2:
* Rename trip flags (Stanislaw).
---
drivers/thermal/thermal_core.c | 12 +++++++++++-
drivers/thermal/thermal_core.h | 2 +-
drivers/thermal/thermal_sysfs.c | 28 +++++++++++++++++++---------
include/linux/thermal.h | 7 +++++++
4 files changed, 38 insertions(+), 11 deletions(-)
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -64,15 +64,23 @@ enum thermal_notify_event {
* @threshold: trip crossing notification threshold miliCelsius
* @type: trip point type
* @priv: pointer to driver data associated with this trip
+ * @flags: flags representing binary properties of the trip
*/
struct thermal_trip {
int temperature;
int hysteresis;
int threshold;
enum thermal_trip_type type;
+ u8 flags;
void *priv;
};
+#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
+#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
+
+#define THERMAL_TRIP_FLAG_MASK_RW (THERMAL_TRIP_FLAG_RW_TEMP | \
+ THERMAL_TRIP_FLAG_RW_HYST)
+
struct thermal_zone_device_ops {
int (*bind) (struct thermal_zone_device *,
struct thermal_cooling_device *);
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1356,13 +1356,23 @@ thermal_zone_device_register_with_trips(
tz->devdata = devdata;
tz->trips = trips;
tz->num_trips = num_trips;
+ if (num_trips > 0) {
+ struct thermal_trip *trip;
+
+ for_each_trip(tz, trip) {
+ if (mask & 1)
+ trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
+
+ mask >>= 1;
+ }
+ }
thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
/* sys I/F */
/* Add nodes that are always present via .groups */
- result = thermal_zone_create_device_groups(tz, mask);
+ result = thermal_zone_create_device_groups(tz);
if (result)
goto remove_id;
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -131,7 +131,7 @@ void thermal_zone_trip_updated(struct th
int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
/* sysfs I/F */
-int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
+int thermal_zone_create_device_groups(struct thermal_zone_device *tz);
void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
void thermal_cooling_device_setup_sysfs(struct thermal_cooling_device *);
void thermal_cooling_device_destroy_sysfs(struct thermal_cooling_device *cdev);
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -122,6 +122,11 @@ trip_point_temp_store(struct device *dev
trip = &tz->trips[trip_id];
+ if (!(trip->flags & THERMAL_TRIP_FLAG_RW_TEMP)) {
+ ret = -EPERM;
+ goto unlock;
+ }
+
if (temp != trip->temperature) {
if (tz->ops->set_trip_temp) {
ret = tz->ops->set_trip_temp(tz, trip_id, temp);
@@ -173,6 +178,11 @@ trip_point_hyst_store(struct device *dev
trip = &tz->trips[trip_id];
+ if (!(trip->flags & THERMAL_TRIP_FLAG_RW_HYST)) {
+ ret = -EPERM;
+ goto unlock;
+ }
+
if (hyst != trip->hysteresis) {
if (tz->ops->set_trip_hyst) {
ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);
@@ -392,17 +402,16 @@ static const struct attribute_group *the
/**
* create_trip_attrs() - create attributes for trip points
* @tz: the thermal zone device
- * @mask: Writeable trip point bitmap.
*
* helper function to instantiate sysfs entries for every trip
* point and its properties of a struct thermal_zone_device.
*
* Return: 0 on success, the proper error value otherwise.
*/
-static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
+static int create_trip_attrs(struct thermal_zone_device *tz)
{
+ const struct thermal_trip *trip;
struct attribute **attrs;
- int indx;
/* This function works only for zones with at least one trip */
if (tz->num_trips <= 0)
@@ -437,7 +446,9 @@ static int create_trip_attrs(struct ther
return -ENOMEM;
}
- for (indx = 0; indx < tz->num_trips; indx++) {
+ for_each_trip(tz, trip) {
+ int indx = thermal_zone_trip_id(tz, trip);
+
/* create trip type attribute */
snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
"trip_point_%d_type", indx);
@@ -458,7 +469,7 @@ static int create_trip_attrs(struct ther
tz->trip_temp_attrs[indx].name;
tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
- if (mask & (1 << indx)) {
+ if (trip->flags & THERMAL_TRIP_FLAG_RW_TEMP) {
tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_temp_attrs[indx].attr.store =
trip_point_temp_store;
@@ -473,7 +484,7 @@ static int create_trip_attrs(struct ther
tz->trip_hyst_attrs[indx].name;
tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
- if (tz->ops->set_trip_hyst) {
+ if (trip->flags & THERMAL_TRIP_FLAG_RW_HYST) {
tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_hyst_attrs[indx].attr.store =
trip_point_hyst_store;
@@ -505,8 +516,7 @@ static void destroy_trip_attrs(struct th
kfree(tz->trips_attribute_group.attrs);
}
-int thermal_zone_create_device_groups(struct thermal_zone_device *tz,
- int mask)
+int thermal_zone_create_device_groups(struct thermal_zone_device *tz)
{
const struct attribute_group **groups;
int i, size, result;
@@ -522,7 +532,7 @@ int thermal_zone_create_device_groups(st
groups[i] = thermal_zone_attribute_groups[i];
if (tz->num_trips) {
- result = create_trip_attrs(tz, mask);
+ result = create_trip_attrs(tz);
if (result) {
kfree(groups);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip
2024-02-12 18:31 ` [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip Rafael J. Wysocki
@ 2024-02-22 14:36 ` Daniel Lezcano
2024-02-22 15:36 ` Daniel Lezcano
2024-02-22 15:51 ` Rafael J. Wysocki
2024-02-22 19:48 ` [PATCH v2.1 " Rafael J. Wysocki
1 sibling, 2 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 14:36 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:31, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> In order to allow thermal zone creators to specify the writability of
> trip point temperature and hysteresis on a per-trip basis, add a flags
> field to struct thermal_trip and define flags to represent the desired
> trip properties.
>
> Also make thermal_zone_device_register_with_trips() set the
> THERMAL_TRIP_FLAG_RW_TEMP flag for all trips covered by the writable
> trips mask passed to it and modify the thermal sysfs code to look at
> the trip flags instead of using the writable trips mask directly or
> checking the presence of the .set_trip_hyst() zone callback.
>
> Additionally, make trip_point_temp_store() and trip_point_hyst_store()
> fail with an error code if the trip passed to one of them has
> THERMAL_TRIP_FLAG_RW_TEMP or THERMAL_TRIP_FLAG_RW_HYST,
> respectively, clear in its flags.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> v1 -> v2:
> * Rename trip flags (Stanislaw).
>
> ---
> drivers/thermal/thermal_core.c | 12 +++++++++++-
> drivers/thermal/thermal_core.h | 2 +-
> drivers/thermal/thermal_sysfs.c | 28 +++++++++++++++++++---------
> include/linux/thermal.h | 7 +++++++
> 4 files changed, 38 insertions(+), 11 deletions(-)
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -64,15 +64,23 @@ enum thermal_notify_event {
> * @threshold: trip crossing notification threshold miliCelsius
> * @type: trip point type
> * @priv: pointer to driver data associated with this trip
> + * @flags: flags representing binary properties of the trip
> */
> struct thermal_trip {
> int temperature;
> int hysteresis;
> int threshold;
> enum thermal_trip_type type;
> + u8 flags;
> void *priv;
> };
>
> +#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
> +#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
> +
> +#define THERMAL_TRIP_FLAG_MASK_RW (THERMAL_TRIP_FLAG_RW_TEMP | \
> + THERMAL_TRIP_FLAG_RW_HYST)
What about THERMAL_TRIP_FLAG_RW instead ?
> struct thermal_zone_device_ops {
> int (*bind) (struct thermal_zone_device *,
> struct thermal_cooling_device *);
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1356,13 +1356,23 @@ thermal_zone_device_register_with_trips(
> tz->devdata = devdata;
> tz->trips = trips;
> tz->num_trips = num_trips;
> + if (num_trips > 0) {
Is this check really necessary? for_each_trip() should exit immediately
if there is no trip points.
> + struct thermal_trip *trip;
> +
> + for_each_trip(tz, trip) {
> + if (mask & 1)
> + trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
> +
> + mask >>= 1;
> + }
> + }
>
> thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
> thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
>
> /* sys I/F */
> /* Add nodes that are always present via .groups */
> - result = thermal_zone_create_device_groups(tz, mask);
> + result = thermal_zone_create_device_groups(tz);
> if (result)
> goto remove_id;
>
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -131,7 +131,7 @@ void thermal_zone_trip_updated(struct th
> int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
>
> /* sysfs I/F */
> -int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
> +int thermal_zone_create_device_groups(struct thermal_zone_device *tz);
> void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
> void thermal_cooling_device_setup_sysfs(struct thermal_cooling_device *);
> void thermal_cooling_device_destroy_sysfs(struct thermal_cooling_device *cdev);
> Index: linux-pm/drivers/thermal/thermal_sysfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> +++ linux-pm/drivers/thermal/thermal_sysfs.c
> @@ -122,6 +122,11 @@ trip_point_temp_store(struct device *dev
>
> trip = &tz->trips[trip_id];
>
> + if (!(trip->flags & THERMAL_TRIP_FLAG_RW_TEMP)) {
> + ret = -EPERM;
> + goto unlock;
> + }
Does it really happen?
If the sysfs file is created with the right permission regarding the
trip->flags then this condition can never be true.
> if (temp != trip->temperature) {
> if (tz->ops->set_trip_temp) {
> ret = tz->ops->set_trip_temp(tz, trip_id, temp);
> @@ -173,6 +178,11 @@ trip_point_hyst_store(struct device *dev
>
> trip = &tz->trips[trip_id];
>
> + if (!(trip->flags & THERMAL_TRIP_FLAG_RW_HYST)) {
> + ret = -EPERM;
> + goto unlock;
> + }
Ditto
> if (hyst != trip->hysteresis) {
> if (tz->ops->set_trip_hyst) {
> ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);
> @@ -392,17 +402,16 @@ static const struct attribute_group *the
> /**
> * create_trip_attrs() - create attributes for trip points
> * @tz: the thermal zone device
> - * @mask: Writeable trip point bitmap.
> *
> * helper function to instantiate sysfs entries for every trip
> * point and its properties of a struct thermal_zone_device.
> *
> * Return: 0 on success, the proper error value otherwise.
> */
> -static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
> +static int create_trip_attrs(struct thermal_zone_device *tz)
> {
> + const struct thermal_trip *trip;
> struct attribute **attrs;
> - int indx;
>
> /* This function works only for zones with at least one trip */
> if (tz->num_trips <= 0)
> @@ -437,7 +446,9 @@ static int create_trip_attrs(struct ther
> return -ENOMEM;
> }
>
> - for (indx = 0; indx < tz->num_trips; indx++) {
> + for_each_trip(tz, trip) {
> + int indx = thermal_zone_trip_id(tz, trip);
> +
> /* create trip type attribute */
> snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
> "trip_point_%d_type", indx);
> @@ -458,7 +469,7 @@ static int create_trip_attrs(struct ther
> tz->trip_temp_attrs[indx].name;
> tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
> tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
> - if (mask & (1 << indx)) {
> + if (trip->flags & THERMAL_TRIP_FLAG_RW_TEMP) {
> tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
> tz->trip_temp_attrs[indx].attr.store =
> trip_point_temp_store;
> @@ -473,7 +484,7 @@ static int create_trip_attrs(struct ther
> tz->trip_hyst_attrs[indx].name;
> tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
> tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
> - if (tz->ops->set_trip_hyst) {
> + if (trip->flags & THERMAL_TRIP_FLAG_RW_HYST) {
> tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
> tz->trip_hyst_attrs[indx].attr.store =
> trip_point_hyst_store;
> @@ -505,8 +516,7 @@ static void destroy_trip_attrs(struct th
> kfree(tz->trips_attribute_group.attrs);
> }
>
> -int thermal_zone_create_device_groups(struct thermal_zone_device *tz,
> - int mask)
> +int thermal_zone_create_device_groups(struct thermal_zone_device *tz)
> {
> const struct attribute_group **groups;
> int i, size, result;
> @@ -522,7 +532,7 @@ int thermal_zone_create_device_groups(st
> groups[i] = thermal_zone_attribute_groups[i];
>
> if (tz->num_trips) {
> - result = create_trip_attrs(tz, mask);
> + result = create_trip_attrs(tz);
> if (result) {
> kfree(groups);
>
>
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip
2024-02-22 14:36 ` Daniel Lezcano
@ 2024-02-22 15:36 ` Daniel Lezcano
2024-02-22 15:51 ` Rafael J. Wysocki
1 sibling, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 15:36 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 22/02/2024 15:36, Daniel Lezcano wrote:
> On 12/02/2024 19:31, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> In order to allow thermal zone creators to specify the writability of
>> trip point temperature and hysteresis on a per-trip basis, add a flags
>> field to struct thermal_trip and define flags to represent the desired
>> trip properties.
>>
>> Also make thermal_zone_device_register_with_trips() set the
>> THERMAL_TRIP_FLAG_RW_TEMP flag for all trips covered by the writable
>> trips mask passed to it and modify the thermal sysfs code to look at
>> the trip flags instead of using the writable trips mask directly or
>> checking the presence of the .set_trip_hyst() zone callback.
>>
>> Additionally, make trip_point_temp_store() and trip_point_hyst_store()
>> fail with an error code if the trip passed to one of them has
>> THERMAL_TRIP_FLAG_RW_TEMP or THERMAL_TRIP_FLAG_RW_HYST,
>> respectively, clear in its flags.
>>
>> No intentional functional impact.
>>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> ---
>> ===================================================================
>> --- linux-pm.orig/drivers/thermal/thermal_core.c
>> +++ linux-pm/drivers/thermal/thermal_core.c
>> @@ -1356,13 +1356,23 @@ thermal_zone_device_register_with_trips(
>> tz->devdata = devdata;
>> tz->trips = trips;
>> tz->num_trips = num_trips;
[ ... ]
>> + if (num_trips > 0) {
>
> Is this check really necessary? for_each_trip() should exit immediately
> if there is no trip points.
Given that is removed in patch 9/9, please ignore this comment
>> + struct thermal_trip *trip;
>> +
>> + for_each_trip(tz, trip) {
>> + if (mask & 1)
>> + trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
>> +
>> + mask >>= 1;
>> + }
>> + }
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip
2024-02-22 14:36 ` Daniel Lezcano
2024-02-22 15:36 ` Daniel Lezcano
@ 2024-02-22 15:51 ` Rafael J. Wysocki
2024-02-22 16:13 ` Rafael J. Wysocki
1 sibling, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-22 15:51 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rafael J. Wysocki, Linux PM, Lukasz Luba, LKML,
Stanislaw Gruszka, Srinivas Pandruvada, Zhang Rui, netdev,
Ido Schimmel, Petr Machata, Miri Korenblit, linux-wireless,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On Thu, Feb 22, 2024 at 3:36 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 12/02/2024 19:31, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > In order to allow thermal zone creators to specify the writability of
> > trip point temperature and hysteresis on a per-trip basis, add a flags
> > field to struct thermal_trip and define flags to represent the desired
> > trip properties.
> >
> > Also make thermal_zone_device_register_with_trips() set the
> > THERMAL_TRIP_FLAG_RW_TEMP flag for all trips covered by the writable
> > trips mask passed to it and modify the thermal sysfs code to look at
> > the trip flags instead of using the writable trips mask directly or
> > checking the presence of the .set_trip_hyst() zone callback.
> >
> > Additionally, make trip_point_temp_store() and trip_point_hyst_store()
> > fail with an error code if the trip passed to one of them has
> > THERMAL_TRIP_FLAG_RW_TEMP or THERMAL_TRIP_FLAG_RW_HYST,
> > respectively, clear in its flags.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > v1 -> v2:
> > * Rename trip flags (Stanislaw).
> >
> > ---
> > drivers/thermal/thermal_core.c | 12 +++++++++++-
> > drivers/thermal/thermal_core.h | 2 +-
> > drivers/thermal/thermal_sysfs.c | 28 +++++++++++++++++++---------
> > include/linux/thermal.h | 7 +++++++
> > 4 files changed, 38 insertions(+), 11 deletions(-)
> >
> > Index: linux-pm/include/linux/thermal.h
> > ===================================================================
> > --- linux-pm.orig/include/linux/thermal.h
> > +++ linux-pm/include/linux/thermal.h
> > @@ -64,15 +64,23 @@ enum thermal_notify_event {
> > * @threshold: trip crossing notification threshold miliCelsius
> > * @type: trip point type
> > * @priv: pointer to driver data associated with this trip
> > + * @flags: flags representing binary properties of the trip
> > */
> > struct thermal_trip {
> > int temperature;
> > int hysteresis;
> > int threshold;
> > enum thermal_trip_type type;
> > + u8 flags;
> > void *priv;
> > };
> >
> > +#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
> > +#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
> > +
> > +#define THERMAL_TRIP_FLAG_MASK_RW (THERMAL_TRIP_FLAG_RW_TEMP | \
> > + THERMAL_TRIP_FLAG_RW_HYST)
>
> What about THERMAL_TRIP_FLAG_RW instead ?
Fine with me.
> > struct thermal_zone_device_ops {
> > int (*bind) (struct thermal_zone_device *,
> > struct thermal_cooling_device *);
> > Index: linux-pm/drivers/thermal/thermal_core.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_core.c
> > +++ linux-pm/drivers/thermal/thermal_core.c
> > @@ -1356,13 +1356,23 @@ thermal_zone_device_register_with_trips(
> > tz->devdata = devdata;
> > tz->trips = trips;
> > tz->num_trips = num_trips;
> > + if (num_trips > 0) {
>
> Is this check really necessary?
No, it isn't.
> for_each_trip() should exit immediately if there is no trip points.
Right.
> > + struct thermal_trip *trip;
> > +
> > + for_each_trip(tz, trip) {
> > + if (mask & 1)
> > + trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
> > +
> > + mask >>= 1;
> > + }
> > + }
> >
> > thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
> > thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
> >
> > /* sys I/F */
> > /* Add nodes that are always present via .groups */
> > - result = thermal_zone_create_device_groups(tz, mask);
> > + result = thermal_zone_create_device_groups(tz);
> > if (result)
> > goto remove_id;
> >
> > Index: linux-pm/drivers/thermal/thermal_core.h
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_core.h
> > +++ linux-pm/drivers/thermal/thermal_core.h
> > @@ -131,7 +131,7 @@ void thermal_zone_trip_updated(struct th
> > int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
> >
> > /* sysfs I/F */
> > -int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
> > +int thermal_zone_create_device_groups(struct thermal_zone_device *tz);
> > void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
> > void thermal_cooling_device_setup_sysfs(struct thermal_cooling_device *);
> > void thermal_cooling_device_destroy_sysfs(struct thermal_cooling_device *cdev);
> > Index: linux-pm/drivers/thermal/thermal_sysfs.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> > +++ linux-pm/drivers/thermal/thermal_sysfs.c
> > @@ -122,6 +122,11 @@ trip_point_temp_store(struct device *dev
> >
> > trip = &tz->trips[trip_id];
> >
> > + if (!(trip->flags & THERMAL_TRIP_FLAG_RW_TEMP)) {
> > + ret = -EPERM;
> > + goto unlock;
> > + }
>
> Does it really happen?
>
> If the sysfs file is created with the right permission regarding the
> trip->flags then this condition can never be true.
But the permissions can be changed after the file has been created, can't they?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip
2024-02-22 15:51 ` Rafael J. Wysocki
@ 2024-02-22 16:13 ` Rafael J. Wysocki
0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-22 16:13 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rafael J. Wysocki, Linux PM, Lukasz Luba, LKML,
Stanislaw Gruszka, Srinivas Pandruvada, Zhang Rui, netdev,
Ido Schimmel, Petr Machata, Miri Korenblit, linux-wireless,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On Thu, Feb 22, 2024 at 4:51 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Feb 22, 2024 at 3:36 PM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
> >
> > On 12/02/2024 19:31, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > In order to allow thermal zone creators to specify the writability of
> > > trip point temperature and hysteresis on a per-trip basis, add a flags
> > > field to struct thermal_trip and define flags to represent the desired
> > > trip properties.
> > >
> > > Also make thermal_zone_device_register_with_trips() set the
> > > THERMAL_TRIP_FLAG_RW_TEMP flag for all trips covered by the writable
> > > trips mask passed to it and modify the thermal sysfs code to look at
> > > the trip flags instead of using the writable trips mask directly or
> > > checking the presence of the .set_trip_hyst() zone callback.
> > >
> > > Additionally, make trip_point_temp_store() and trip_point_hyst_store()
> > > fail with an error code if the trip passed to one of them has
> > > THERMAL_TRIP_FLAG_RW_TEMP or THERMAL_TRIP_FLAG_RW_HYST,
> > > respectively, clear in its flags.
> > >
> > > No intentional functional impact.
> > >
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > >
> > > v1 -> v2:
> > > * Rename trip flags (Stanislaw).
> > >
> > > ---
> > > drivers/thermal/thermal_core.c | 12 +++++++++++-
> > > drivers/thermal/thermal_core.h | 2 +-
> > > drivers/thermal/thermal_sysfs.c | 28 +++++++++++++++++++---------
> > > include/linux/thermal.h | 7 +++++++
> > > 4 files changed, 38 insertions(+), 11 deletions(-)
> > >
> > > Index: linux-pm/include/linux/thermal.h
> > > ===================================================================
> > > --- linux-pm.orig/include/linux/thermal.h
> > > +++ linux-pm/include/linux/thermal.h
> > > @@ -64,15 +64,23 @@ enum thermal_notify_event {
> > > * @threshold: trip crossing notification threshold miliCelsius
> > > * @type: trip point type
> > > * @priv: pointer to driver data associated with this trip
> > > + * @flags: flags representing binary properties of the trip
> > > */
> > > struct thermal_trip {
> > > int temperature;
> > > int hysteresis;
> > > int threshold;
> > > enum thermal_trip_type type;
> > > + u8 flags;
> > > void *priv;
> > > };
> > >
> > > +#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
> > > +#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
> > > +
> > > +#define THERMAL_TRIP_FLAG_MASK_RW (THERMAL_TRIP_FLAG_RW_TEMP | \
> > > + THERMAL_TRIP_FLAG_RW_HYST)
> >
> > What about THERMAL_TRIP_FLAG_RW instead ?
>
> Fine with me.
>
> > > struct thermal_zone_device_ops {
> > > int (*bind) (struct thermal_zone_device *,
> > > struct thermal_cooling_device *);
> > > Index: linux-pm/drivers/thermal/thermal_core.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/thermal/thermal_core.c
> > > +++ linux-pm/drivers/thermal/thermal_core.c
> > > @@ -1356,13 +1356,23 @@ thermal_zone_device_register_with_trips(
> > > tz->devdata = devdata;
> > > tz->trips = trips;
> > > tz->num_trips = num_trips;
> > > + if (num_trips > 0) {
> >
> > Is this check really necessary?
>
> No, it isn't.
>
> > for_each_trip() should exit immediately if there is no trip points.
>
> Right.
>
> > > + struct thermal_trip *trip;
> > > +
> > > + for_each_trip(tz, trip) {
> > > + if (mask & 1)
> > > + trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
> > > +
> > > + mask >>= 1;
> > > + }
> > > + }
> > >
> > > thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
> > > thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
> > >
> > > /* sys I/F */
> > > /* Add nodes that are always present via .groups */
> > > - result = thermal_zone_create_device_groups(tz, mask);
> > > + result = thermal_zone_create_device_groups(tz);
> > > if (result)
> > > goto remove_id;
> > >
> > > Index: linux-pm/drivers/thermal/thermal_core.h
> > > ===================================================================
> > > --- linux-pm.orig/drivers/thermal/thermal_core.h
> > > +++ linux-pm/drivers/thermal/thermal_core.h
> > > @@ -131,7 +131,7 @@ void thermal_zone_trip_updated(struct th
> > > int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
> > >
> > > /* sysfs I/F */
> > > -int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
> > > +int thermal_zone_create_device_groups(struct thermal_zone_device *tz);
> > > void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
> > > void thermal_cooling_device_setup_sysfs(struct thermal_cooling_device *);
> > > void thermal_cooling_device_destroy_sysfs(struct thermal_cooling_device *cdev);
> > > Index: linux-pm/drivers/thermal/thermal_sysfs.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> > > +++ linux-pm/drivers/thermal/thermal_sysfs.c
> > > @@ -122,6 +122,11 @@ trip_point_temp_store(struct device *dev
> > >
> > > trip = &tz->trips[trip_id];
> > >
> > > + if (!(trip->flags & THERMAL_TRIP_FLAG_RW_TEMP)) {
> > > + ret = -EPERM;
> > > + goto unlock;
> > > + }
> >
> > Does it really happen?
> >
> > If the sysfs file is created with the right permission regarding the
> > trip->flags then this condition can never be true.
>
> But the permissions can be changed after the file has been created, can't they?
Even so, the .store() callback will not be set then anyway.
I'll send an update of this patch.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2.1 2/9] thermal: core: Add flags to struct thermal_trip
2024-02-12 18:31 ` [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip Rafael J. Wysocki
2024-02-22 14:36 ` Daniel Lezcano
@ 2024-02-22 19:48 ` Rafael J. Wysocki
2024-02-27 11:21 ` Rafael J. Wysocki
1 sibling, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-22 19:48 UTC (permalink / raw)
To: Linux PM, Daniel Lezcano
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
In order to allow thermal zone creators to specify the writability of
trip point temperature and hysteresis on a per-trip basis, add a flags
field to struct thermal_trip and define flags to represent the desired
trip properties.
Also make thermal_zone_device_register_with_trips() set the
THERMAL_TRIP_FLAG_RW_TEMP flag for all trips covered by the writable
trips mask passed to it and modify the thermal sysfs code to look at
the trip flags instead of using the writable trips mask directly or
checking the presence of the .set_trip_hyst() zone callback.
Additionally, make trip_point_temp_store() and trip_point_hyst_store()
fail with an error code if the trip passed to one of them has
THERMAL_TRIP_FLAG_RW_TEMP or THERMAL_TRIP_FLAG_RW_HYST,
respectively, clear in its flags.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v2 -> v2.1:
* Don't add redundant checks in 3 places (Daniel).
* Define THERMAL_TRIP_FLAG_RW as the combination of the _RW_ trip flags (Daniel).
v1 -> v2:
* Rename trip flags (Stanislaw).
---
drivers/thermal/thermal_core.c | 9 ++++++++-
drivers/thermal/thermal_core.h | 2 +-
drivers/thermal/thermal_sysfs.c | 18 +++++++++---------
include/linux/thermal.h | 8 ++++++++
4 files changed, 26 insertions(+), 11 deletions(-)
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -64,15 +64,23 @@ enum thermal_notify_event {
* @threshold: trip crossing notification threshold miliCelsius
* @type: trip point type
* @priv: pointer to driver data associated with this trip
+ * @flags: flags representing binary properties of the trip
*/
struct thermal_trip {
int temperature;
int hysteresis;
int threshold;
enum thermal_trip_type type;
+ u8 flags;
void *priv;
};
+#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
+#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
+
+#define THERMAL_TRIP_FLAG_RW (THERMAL_TRIP_FLAG_RW_TEMP | \
+ THERMAL_TRIP_FLAG_RW_HYST)
+
struct thermal_zone_device_ops {
int (*bind) (struct thermal_zone_device *,
struct thermal_cooling_device *);
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1278,6 +1278,7 @@ thermal_zone_device_register_with_trips(
int passive_delay, int polling_delay)
{
struct thermal_zone_device *tz;
+ struct thermal_trip *trip;
int id;
int result;
struct thermal_governor *governor;
@@ -1356,13 +1357,19 @@ thermal_zone_device_register_with_trips(
tz->devdata = devdata;
memcpy(tz->trips, trips, num_trips * sizeof(*trips));
tz->num_trips = num_trips;
+ for_each_trip(tz, trip) {
+ if (mask & 1)
+ trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
+
+ mask >>= 1;
+ }
thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
/* sys I/F */
/* Add nodes that are always present via .groups */
- result = thermal_zone_create_device_groups(tz, mask);
+ result = thermal_zone_create_device_groups(tz);
if (result)
goto remove_id;
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -131,7 +131,7 @@ void thermal_zone_trip_updated(struct th
int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
/* sysfs I/F */
-int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
+int thermal_zone_create_device_groups(struct thermal_zone_device *tz);
void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
void thermal_cooling_device_setup_sysfs(struct thermal_cooling_device *);
void thermal_cooling_device_destroy_sysfs(struct thermal_cooling_device *cdev);
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -392,17 +392,16 @@ static const struct attribute_group *the
/**
* create_trip_attrs() - create attributes for trip points
* @tz: the thermal zone device
- * @mask: Writeable trip point bitmap.
*
* helper function to instantiate sysfs entries for every trip
* point and its properties of a struct thermal_zone_device.
*
* Return: 0 on success, the proper error value otherwise.
*/
-static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
+static int create_trip_attrs(struct thermal_zone_device *tz)
{
+ const struct thermal_trip *trip;
struct attribute **attrs;
- int indx;
/* This function works only for zones with at least one trip */
if (tz->num_trips <= 0)
@@ -437,7 +436,9 @@ static int create_trip_attrs(struct ther
return -ENOMEM;
}
- for (indx = 0; indx < tz->num_trips; indx++) {
+ for_each_trip(tz, trip) {
+ int indx = thermal_zone_trip_id(tz, trip);
+
/* create trip type attribute */
snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
"trip_point_%d_type", indx);
@@ -458,7 +459,7 @@ static int create_trip_attrs(struct ther
tz->trip_temp_attrs[indx].name;
tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
- if (mask & (1 << indx)) {
+ if (trip->flags & THERMAL_TRIP_FLAG_RW_TEMP) {
tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_temp_attrs[indx].attr.store =
trip_point_temp_store;
@@ -473,7 +474,7 @@ static int create_trip_attrs(struct ther
tz->trip_hyst_attrs[indx].name;
tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
- if (tz->ops.set_trip_hyst) {
+ if (trip->flags & THERMAL_TRIP_FLAG_RW_HYST) {
tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_hyst_attrs[indx].attr.store =
trip_point_hyst_store;
@@ -505,8 +506,7 @@ static void destroy_trip_attrs(struct th
kfree(tz->trips_attribute_group.attrs);
}
-int thermal_zone_create_device_groups(struct thermal_zone_device *tz,
- int mask)
+int thermal_zone_create_device_groups(struct thermal_zone_device *tz)
{
const struct attribute_group **groups;
int i, size, result;
@@ -522,7 +522,7 @@ int thermal_zone_create_device_groups(st
groups[i] = thermal_zone_attribute_groups[i];
if (tz->num_trips) {
- result = create_trip_attrs(tz, mask);
+ result = create_trip_attrs(tz);
if (result) {
kfree(groups);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2.1 2/9] thermal: core: Add flags to struct thermal_trip
2024-02-22 19:48 ` [PATCH v2.1 " Rafael J. Wysocki
@ 2024-02-27 11:21 ` Rafael J. Wysocki
0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-27 11:21 UTC (permalink / raw)
To: Linux PM, Daniel Lezcano
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On Thu, Feb 22, 2024 at 8:48 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> In order to allow thermal zone creators to specify the writability of
> trip point temperature and hysteresis on a per-trip basis, add a flags
> field to struct thermal_trip and define flags to represent the desired
> trip properties.
>
> Also make thermal_zone_device_register_with_trips() set the
> THERMAL_TRIP_FLAG_RW_TEMP flag for all trips covered by the writable
> trips mask passed to it and modify the thermal sysfs code to look at
> the trip flags instead of using the writable trips mask directly or
> checking the presence of the .set_trip_hyst() zone callback.
>
> Additionally, make trip_point_temp_store() and trip_point_hyst_store()
> fail with an error code if the trip passed to one of them has
> THERMAL_TRIP_FLAG_RW_TEMP or THERMAL_TRIP_FLAG_RW_HYST,
> respectively, clear in its flags.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
AFAICS this patch addresses all of the review comments and because the
rest of the series (depending on it) has been approved, I'm applying
the whole series including this patch.
> ---
>
> v2 -> v2.1:
> * Don't add redundant checks in 3 places (Daniel).
> * Define THERMAL_TRIP_FLAG_RW as the combination of the _RW_ trip flags (Daniel).
>
> v1 -> v2:
> * Rename trip flags (Stanislaw).
>
> ---
> drivers/thermal/thermal_core.c | 9 ++++++++-
> drivers/thermal/thermal_core.h | 2 +-
> drivers/thermal/thermal_sysfs.c | 18 +++++++++---------
> include/linux/thermal.h | 8 ++++++++
> 4 files changed, 26 insertions(+), 11 deletions(-)
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -64,15 +64,23 @@ enum thermal_notify_event {
> * @threshold: trip crossing notification threshold miliCelsius
> * @type: trip point type
> * @priv: pointer to driver data associated with this trip
> + * @flags: flags representing binary properties of the trip
> */
> struct thermal_trip {
> int temperature;
> int hysteresis;
> int threshold;
> enum thermal_trip_type type;
> + u8 flags;
> void *priv;
> };
>
> +#define THERMAL_TRIP_FLAG_RW_TEMP BIT(0)
> +#define THERMAL_TRIP_FLAG_RW_HYST BIT(1)
> +
> +#define THERMAL_TRIP_FLAG_RW (THERMAL_TRIP_FLAG_RW_TEMP | \
> + THERMAL_TRIP_FLAG_RW_HYST)
> +
> struct thermal_zone_device_ops {
> int (*bind) (struct thermal_zone_device *,
> struct thermal_cooling_device *);
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1278,6 +1278,7 @@ thermal_zone_device_register_with_trips(
> int passive_delay, int polling_delay)
> {
> struct thermal_zone_device *tz;
> + struct thermal_trip *trip;
> int id;
> int result;
> struct thermal_governor *governor;
> @@ -1356,13 +1357,19 @@ thermal_zone_device_register_with_trips(
> tz->devdata = devdata;
> memcpy(tz->trips, trips, num_trips * sizeof(*trips));
> tz->num_trips = num_trips;
> + for_each_trip(tz, trip) {
> + if (mask & 1)
> + trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
> +
> + mask >>= 1;
> + }
>
> thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
> thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
>
> /* sys I/F */
> /* Add nodes that are always present via .groups */
> - result = thermal_zone_create_device_groups(tz, mask);
> + result = thermal_zone_create_device_groups(tz);
> if (result)
> goto remove_id;
>
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -131,7 +131,7 @@ void thermal_zone_trip_updated(struct th
> int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
>
> /* sysfs I/F */
> -int thermal_zone_create_device_groups(struct thermal_zone_device *, int);
> +int thermal_zone_create_device_groups(struct thermal_zone_device *tz);
> void thermal_zone_destroy_device_groups(struct thermal_zone_device *);
> void thermal_cooling_device_setup_sysfs(struct thermal_cooling_device *);
> void thermal_cooling_device_destroy_sysfs(struct thermal_cooling_device *cdev);
> Index: linux-pm/drivers/thermal/thermal_sysfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> +++ linux-pm/drivers/thermal/thermal_sysfs.c
> @@ -392,17 +392,16 @@ static const struct attribute_group *the
> /**
> * create_trip_attrs() - create attributes for trip points
> * @tz: the thermal zone device
> - * @mask: Writeable trip point bitmap.
> *
> * helper function to instantiate sysfs entries for every trip
> * point and its properties of a struct thermal_zone_device.
> *
> * Return: 0 on success, the proper error value otherwise.
> */
> -static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
> +static int create_trip_attrs(struct thermal_zone_device *tz)
> {
> + const struct thermal_trip *trip;
> struct attribute **attrs;
> - int indx;
>
> /* This function works only for zones with at least one trip */
> if (tz->num_trips <= 0)
> @@ -437,7 +436,9 @@ static int create_trip_attrs(struct ther
> return -ENOMEM;
> }
>
> - for (indx = 0; indx < tz->num_trips; indx++) {
> + for_each_trip(tz, trip) {
> + int indx = thermal_zone_trip_id(tz, trip);
> +
> /* create trip type attribute */
> snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
> "trip_point_%d_type", indx);
> @@ -458,7 +459,7 @@ static int create_trip_attrs(struct ther
> tz->trip_temp_attrs[indx].name;
> tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
> tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
> - if (mask & (1 << indx)) {
> + if (trip->flags & THERMAL_TRIP_FLAG_RW_TEMP) {
> tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
> tz->trip_temp_attrs[indx].attr.store =
> trip_point_temp_store;
> @@ -473,7 +474,7 @@ static int create_trip_attrs(struct ther
> tz->trip_hyst_attrs[indx].name;
> tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
> tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
> - if (tz->ops.set_trip_hyst) {
> + if (trip->flags & THERMAL_TRIP_FLAG_RW_HYST) {
> tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
> tz->trip_hyst_attrs[indx].attr.store =
> trip_point_hyst_store;
> @@ -505,8 +506,7 @@ static void destroy_trip_attrs(struct th
> kfree(tz->trips_attribute_group.attrs);
> }
>
> -int thermal_zone_create_device_groups(struct thermal_zone_device *tz,
> - int mask)
> +int thermal_zone_create_device_groups(struct thermal_zone_device *tz)
> {
> const struct attribute_group **groups;
> int i, size, result;
> @@ -522,7 +522,7 @@ int thermal_zone_create_device_groups(st
> groups[i] = thermal_zone_attribute_groups[i];
>
> if (tz->num_trips) {
> - result = create_trip_attrs(tz, mask);
> + result = create_trip_attrs(tz);
> if (result) {
> kfree(groups);
>
>
>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 3/9] thermal: core: Drop the .set_trip_hyst() thermal zone operation
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
2024-02-12 18:26 ` [PATCH v2 1/9] thermal: Get rid of CONFIG_THERMAL_WRITABLE_TRIPS Rafael J. Wysocki
2024-02-12 18:31 ` [PATCH v2 2/9] thermal: core: Add flags to struct thermal_trip Rafael J. Wysocki
@ 2024-02-12 18:32 ` Rafael J. Wysocki
2024-02-22 14:48 ` Daniel Lezcano
2024-02-12 18:34 ` [PATCH v2 4/9] thermal: intel: Set THERMAL_TRIP_FLAG_RW_TEMP directly Rafael J. Wysocki
` (6 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:32 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
None of the users of the thermal core provides a .set_trip_hyst()
thermal zone operation, so drop that callback from struct
thermal_zone_device_ops and update trip_point_hyst_store()
accordingly.
No functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/thermal/thermal_sysfs.c | 6 ------
include/linux/thermal.h | 1 -
2 files changed, 7 deletions(-)
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -184,12 +184,6 @@ trip_point_hyst_store(struct device *dev
}
if (hyst != trip->hysteresis) {
- if (tz->ops->set_trip_hyst) {
- ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);
- if (ret)
- goto unlock;
- }
-
trip->hysteresis = hyst;
thermal_zone_trip_updated(tz, trip);
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -91,7 +91,6 @@ struct thermal_zone_device_ops {
int (*change_mode) (struct thermal_zone_device *,
enum thermal_device_mode);
int (*set_trip_temp) (struct thermal_zone_device *, int, int);
- int (*set_trip_hyst) (struct thermal_zone_device *, int, int);
int (*get_crit_temp) (struct thermal_zone_device *, int *);
int (*set_emul_temp) (struct thermal_zone_device *, int);
int (*get_trend) (struct thermal_zone_device *,
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 3/9] thermal: core: Drop the .set_trip_hyst() thermal zone operation
2024-02-12 18:32 ` [PATCH v2 3/9] thermal: core: Drop the .set_trip_hyst() thermal zone operation Rafael J. Wysocki
@ 2024-02-22 14:48 ` Daniel Lezcano
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 14:48 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:32, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> None of the users of the thermal core provides a .set_trip_hyst()
> thermal zone operation, so drop that callback from struct
> thermal_zone_device_ops and update trip_point_hyst_store()
> accordingly.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 4/9] thermal: intel: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
` (2 preceding siblings ...)
2024-02-12 18:32 ` [PATCH v2 3/9] thermal: core: Drop the .set_trip_hyst() thermal zone operation Rafael J. Wysocki
@ 2024-02-12 18:34 ` Rafael J. Wysocki
2024-02-22 15:11 ` Daniel Lezcano
2024-02-12 18:35 ` [PATCH v2 5/9] mlxsw: core_thermal: " Rafael J. Wysocki
` (5 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:34 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Some Intel thermal drivers need/want the temperature of their trip
points to be set by user space via sysfs and so they pass nonzero
writable trip masks during thermal zone registration for this purpose.
It is now possible to achieve the same result by setting the
THERMAL_TRIP_FLAG_RW_TEMP trip flag directly, so modify the drivers
in question to do that instead of using a nonzero writable trips mask.
No intentional functional impact.
Note that this change is requisite for dropping the mask argument from
thermal_zone_device_register_with_trips() going forward.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2:
* Rename trip flags (Stanislaw).
* Fix build issue in alloc_soc_dts().
---
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 8 -
drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c | 3
drivers/thermal/intel/intel_quark_dts_thermal.c | 32 +---
drivers/thermal/intel/intel_soc_dts_iosf.c | 73 ++++------
drivers/thermal/intel/intel_soc_dts_iosf.h | 1
drivers/thermal/intel/x86_pkg_temp_thermal.c | 4
6 files changed, 50 insertions(+), 71 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
@@ -130,7 +130,6 @@ struct int34x_thermal_zone *int340x_ther
struct thermal_trip *zone_trips;
unsigned long long trip_cnt = 0;
unsigned long long hyst;
- int trip_mask = 0;
acpi_status status;
int i, ret;
@@ -151,10 +150,8 @@ struct int34x_thermal_zone *int340x_ther
int34x_zone->ops->get_temp = get_temp;
status = acpi_evaluate_integer(adev->handle, "PATC", NULL, &trip_cnt);
- if (ACPI_SUCCESS(status)) {
+ if (ACPI_SUCCESS(status))
int34x_zone->aux_trip_nr = trip_cnt;
- trip_mask = BIT(trip_cnt) - 1;
- }
zone_trips = kzalloc(sizeof(*zone_trips) * (trip_cnt + INT340X_THERMAL_MAX_TRIP_COUNT),
GFP_KERNEL);
@@ -166,6 +163,7 @@ struct int34x_thermal_zone *int340x_ther
for (i = 0; i < trip_cnt; i++) {
zone_trips[i].type = THERMAL_TRIP_PASSIVE;
zone_trips[i].temperature = THERMAL_TEMP_INVALID;
+ zone_trips[i].flags |= THERMAL_TRIP_FLAG_RW_TEMP;
}
trip_cnt = int340x_thermal_read_trips(adev, zone_trips, trip_cnt);
@@ -186,7 +184,7 @@ struct int34x_thermal_zone *int340x_ther
int34x_zone->zone = thermal_zone_device_register_with_trips(
acpi_device_bid(adev),
zone_trips, trip_cnt,
- trip_mask, int34x_zone,
+ 0, int34x_zone,
int34x_zone->ops,
&int340x_thermal_params,
0, 0);
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
@@ -235,6 +235,7 @@ static int get_trip_temp(struct proc_the
static struct thermal_trip psv_trip = {
.type = THERMAL_TRIP_PASSIVE,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP,
};
static struct thermal_zone_device_ops tzone_ops = {
@@ -290,7 +291,7 @@ static int proc_thermal_pci_probe(struct
psv_trip.temperature = get_trip_temp(pci_info);
pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
- 1, 1, pci_info,
+ 1, 0, pci_info,
&tzone_ops,
&tzone_params, 0, 0);
if (IS_ERR(pci_info->tzone)) {
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
@@ -93,10 +93,6 @@
/* Quark DTS has 2 trip points: hot & catastrophic */
#define QRK_MAX_DTS_TRIPS 2
-/* If DTS not locked, all trip points are configurable */
-#define QRK_DTS_WR_MASK_SET 0x3
-/* If DTS locked, all trip points are not configurable */
-#define QRK_DTS_WR_MASK_CLR 0
#define DEFAULT_POLL_DELAY 2000
@@ -321,15 +317,16 @@ static void free_soc_dts(struct soc_sens
static struct soc_sensor_entry *alloc_soc_dts(void)
{
struct soc_sensor_entry *aux_entry;
+ struct thermal_trip *trips;
int err;
u32 out;
- int wr_mask;
aux_entry = kzalloc(sizeof(*aux_entry), GFP_KERNEL);
if (!aux_entry) {
err = -ENOMEM;
return ERR_PTR(-ENOMEM);
}
+ trips = aux_entry->trips;
/* Check if DTS register is locked */
err = iosf_mbi_read(QRK_MBI_UNIT_RMU, MBI_REG_READ,
@@ -337,13 +334,7 @@ static struct soc_sensor_entry *alloc_so
if (err)
goto err_ret;
- if (out & QRK_DTS_LOCK_BIT) {
- aux_entry->locked = true;
- wr_mask = QRK_DTS_WR_MASK_CLR;
- } else {
- aux_entry->locked = false;
- wr_mask = QRK_DTS_WR_MASK_SET;
- }
+ aux_entry->locked = !!(out & QRK_DTS_LOCK_BIT);
/* Store DTS default state if DTS registers are not locked */
if (!aux_entry->locked) {
@@ -360,19 +351,22 @@ static struct soc_sensor_entry *alloc_so
&aux_entry->store_ptps);
if (err)
goto err_ret;
+
+ trips[QRK_DTS_ID_TP_CRITICAL].flags |= THERMAL_TRIP_FLAG_RW_TEMP;
+ trips[QRK_DTS_ID_TP_HOT].flags |= THERMAL_TRIP_FLAG_RW_TEMP;
}
- 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,
+ 0, aux_entry,
+ &tzone_ops,
NULL, 0, polling_delay);
if (IS_ERR(aux_entry->tzone)) {
err = PTR_ERR(aux_entry->tzone);
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)
{
@@ -217,16 +201,10 @@ static void remove_dts_thermal_zone(stru
thermal_zone_device_unregister(dts->tzone);
}
-static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,
- bool critical_trip)
+static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts)
{
- int writable_trip_cnt = SOC_MAX_DTS_TRIPS;
char name[10];
- unsigned long trip;
- int trip_mask;
- unsigned long ptps;
u32 store_ptps;
- unsigned long i;
int ret;
/* Store status to restor on exit */
@@ -237,27 +215,21 @@ static int add_dts_thermal_zone(int id,
dts->id = id;
- if (critical_trip)
- writable_trip_cnt--;
-
- trip_mask = GENMASK(writable_trip_cnt - 1, 0);
-
/* Check if the writable trip we provide is not used by BIOS */
ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
SOC_DTS_OFFSET_PTPS, &store_ptps);
- if (ret)
- trip_mask = 0;
- else {
- ptps = store_ptps;
- for_each_set_clump8(i, trip, &ptps, writable_trip_cnt * 8)
- trip_mask &= ~BIT(i / 8);
+ if (!ret) {
+ int i;
+
+ for (i = 0; i <= 1; i++) {
+ if (store_ptps & (0xFFU << i * 8))
+ dts->trips[i].flags &= ~THERMAL_TRIP_FLAG_RW_TEMP;
+ }
}
- dts->trip_mask = trip_mask;
snprintf(name, sizeof(name), "soc_dts%d", id);
dts->tzone = thermal_zone_device_register_with_trips(name, dts->trips,
SOC_MAX_DTS_TRIPS,
- trip_mask,
- dts, &tzone_ops,
+ 0, dts, &tzone_ops,
NULL, 0, 0);
if (IS_ERR(dts->tzone)) {
ret = PTR_ERR(dts->tzone);
@@ -315,8 +287,16 @@ 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);
+}
+
+static void set_trip(struct thermal_trip *trip, enum thermal_trip_type type,
+ u8 flags, int temp)
+{
+ trip->type = type;
+ trip->flags = flags;
+ trip->temperature = temp;
}
struct intel_soc_dts_sensors *
@@ -346,29 +326,36 @@ intel_soc_dts_iosf_init(enum intel_soc_d
for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) {
enum thermal_trip_type trip_type;
+ u8 trip_flags;
int temp;
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;
+ set_trip(&sensors->soc_dts[i].trips[0], THERMAL_TRIP_PASSIVE,
+ THERMAL_TRIP_FLAG_RW_TEMP, 0);
+
if (critical_trip) {
trip_type = THERMAL_TRIP_CRITICAL;
+ trip_flags = 0;
temp = sensors->tj_max - crit_offset;
} else {
trip_type = THERMAL_TRIP_PASSIVE;
+ trip_flags = THERMAL_TRIP_FLAG_RW_TEMP;
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;
+
+ set_trip(&sensors->soc_dts[i].trips[1], trip_type, trip_flags, 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]);
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
@@ -28,7 +28,6 @@ struct intel_soc_dts_sensors;
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
@@ -302,6 +302,7 @@ static struct thermal_trip *pkg_temp_the
tj_max - thres_reg_value * 1000 : THERMAL_TEMP_INVALID;
trips[i].type = THERMAL_TRIP_PASSIVE;
+ trips[i].flags |= THERMAL_TRIP_FLAG_RW_TEMP;
pr_debug("%s: cpu=%d, trip=%d, temp=%d\n",
__func__, cpu, i, trips[i].temperature);
@@ -345,8 +346,7 @@ static int pkg_temp_thermal_device_add(u
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,
- (thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
+ zonedev->trips, thres_count, 0,
zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
if (IS_ERR(zonedev->tzone)) {
err = PTR_ERR(zonedev->tzone);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/9] thermal: intel: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:34 ` [PATCH v2 4/9] thermal: intel: Set THERMAL_TRIP_FLAG_RW_TEMP directly Rafael J. Wysocki
@ 2024-02-22 15:11 ` Daniel Lezcano
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 15:11 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:34, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Some Intel thermal drivers need/want the temperature of their trip
> points to be set by user space via sysfs and so they pass nonzero
> writable trip masks during thermal zone registration for this purpose.
>
> It is now possible to achieve the same result by setting the
> THERMAL_TRIP_FLAG_RW_TEMP trip flag directly, so modify the drivers
> in question to do that instead of using a nonzero writable trips mask.
>
> No intentional functional impact.
>
> Note that this change is requisite for dropping the mask argument from
> thermal_zone_device_register_with_trips() going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
I've reviewed the changes. Some changes in the DTS are opaque for me, so
I can not give my reviewed-by tag but the acked-by
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 5/9] mlxsw: core_thermal: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
` (3 preceding siblings ...)
2024-02-12 18:34 ` [PATCH v2 4/9] thermal: intel: Set THERMAL_TRIP_FLAG_RW_TEMP directly Rafael J. Wysocki
@ 2024-02-12 18:35 ` Rafael J. Wysocki
2024-02-22 15:13 ` Daniel Lezcano
2024-02-12 18:38 ` [PATCH v2 6/9] wifi: iwlwifi: mvm: " Rafael J. Wysocki
` (4 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:35 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
to allow their temperature to be set from user space via sysfs instead
of using a nonzero writable trips mask during thermal zone registration,
so make the mlxsw code do that.
No intentional functional impact.
Note that this change is requisite for dropping the mask argument from
thermal_zone_device_register_with_trips() going forward.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
v1 -> v2:
* Rename trip flag (Stanislaw).
* Add R-by from Ido.
---
drivers/net/ethernet/mellanox/mlxsw/core_thermal.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
Index: linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
===================================================================
--- linux-pm.orig/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -44,16 +44,19 @@ static const struct thermal_trip default
.type = THERMAL_TRIP_ACTIVE,
.temperature = MLXSW_THERMAL_ASIC_TEMP_NORM,
.hysteresis = MLXSW_THERMAL_HYSTERESIS_TEMP,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP,
},
{
/* In range - 40-100% PWM */
.type = THERMAL_TRIP_ACTIVE,
.temperature = MLXSW_THERMAL_ASIC_TEMP_HIGH,
.hysteresis = MLXSW_THERMAL_HYSTERESIS_TEMP,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP,
},
{ /* Warning */
.type = THERMAL_TRIP_HOT,
.temperature = MLXSW_THERMAL_ASIC_TEMP_HOT,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP,
},
};
@@ -62,16 +65,19 @@ static const struct thermal_trip default
.type = THERMAL_TRIP_ACTIVE,
.temperature = MLXSW_THERMAL_MODULE_TEMP_NORM,
.hysteresis = MLXSW_THERMAL_HYSTERESIS_TEMP,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP,
},
{
/* In range - 40-100% PWM */
.type = THERMAL_TRIP_ACTIVE,
.temperature = MLXSW_THERMAL_MODULE_TEMP_HIGH,
.hysteresis = MLXSW_THERMAL_HYSTERESIS_TEMP,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP,
},
{ /* Warning */
.type = THERMAL_TRIP_HOT,
.temperature = MLXSW_THERMAL_MODULE_TEMP_HOT,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP,
},
};
@@ -92,9 +98,6 @@ static const struct mlxsw_cooling_states
#define MLXSW_THERMAL_NUM_TRIPS ARRAY_SIZE(default_thermal_trips)
-/* Make sure all trips are writable */
-#define MLXSW_THERMAL_TRIP_MASK (BIT(MLXSW_THERMAL_NUM_TRIPS) - 1)
-
struct mlxsw_thermal;
struct mlxsw_thermal_module {
@@ -420,7 +423,7 @@ mlxsw_thermal_module_tz_init(struct mlxs
module_tz->tzdev = thermal_zone_device_register_with_trips(tz_name,
module_tz->trips,
MLXSW_THERMAL_NUM_TRIPS,
- MLXSW_THERMAL_TRIP_MASK,
+ 0,
module_tz,
&mlxsw_thermal_module_ops,
&mlxsw_thermal_params,
@@ -548,7 +551,7 @@ mlxsw_thermal_gearbox_tz_init(struct mlx
gearbox_tz->tzdev = thermal_zone_device_register_with_trips(tz_name,
gearbox_tz->trips,
MLXSW_THERMAL_NUM_TRIPS,
- MLXSW_THERMAL_TRIP_MASK,
+ 0,
gearbox_tz,
&mlxsw_thermal_gearbox_ops,
&mlxsw_thermal_params, 0,
@@ -773,7 +776,7 @@ int mlxsw_thermal_init(struct mlxsw_core
thermal->tzdev = thermal_zone_device_register_with_trips("mlxsw",
thermal->trips,
MLXSW_THERMAL_NUM_TRIPS,
- MLXSW_THERMAL_TRIP_MASK,
+ 0,
thermal,
&mlxsw_thermal_ops,
&mlxsw_thermal_params, 0,
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 5/9] mlxsw: core_thermal: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:35 ` [PATCH v2 5/9] mlxsw: core_thermal: " Rafael J. Wysocki
@ 2024-02-22 15:13 ` Daniel Lezcano
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 15:13 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:35, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
> to allow their temperature to be set from user space via sysfs instead
> of using a nonzero writable trips mask during thermal zone registration,
> so make the mlxsw code do that.
>
> No intentional functional impact.
>
> Note that this change is requisite for dropping the mask argument from
> thermal_zone_device_register_with_trips() going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 6/9] wifi: iwlwifi: mvm: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
` (4 preceding siblings ...)
2024-02-12 18:35 ` [PATCH v2 5/9] mlxsw: core_thermal: " Rafael J. Wysocki
@ 2024-02-12 18:38 ` Rafael J. Wysocki
2024-02-15 18:04 ` Rafael J. Wysocki
2024-02-22 15:14 ` Daniel Lezcano
2024-02-12 18:39 ` [PATCH v2 7/9] thermal: imx: " Rafael J. Wysocki
` (3 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:38 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
to allow their temperature to be set from user space via sysfs instead
of using a nonzero writable trips mask during thermal zone registration,
so make the iwlwifi code do that.
No intentional functional impact.
Note that this change is requisite for dropping the mask argument from
thermal_zone_device_register_with_trips() going forward.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2:
* Rename trip flag (Stanislaw).
* Fix coding mistake in iwl_mvm_thermal_zone_register().
* Add "wifi:" prefix to the subject (Kalle).
---
drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
Index: linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
===================================================================
--- linux-pm.orig/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -667,9 +667,6 @@ static struct thermal_zone_device_ops t
.set_trip_temp = iwl_mvm_tzone_set_trip_temp,
};
-/* make all trips writable */
-#define IWL_WRITABLE_TRIPS_MSK (BIT(IWL_MAX_DTS_TRIPS) - 1)
-
static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
{
int i, ret;
@@ -692,11 +689,12 @@ static void iwl_mvm_thermal_zone_registe
for (i = 0 ; i < IWL_MAX_DTS_TRIPS; i++) {
mvm->tz_device.trips[i].temperature = THERMAL_TEMP_INVALID;
mvm->tz_device.trips[i].type = THERMAL_TRIP_PASSIVE;
+ mvm->tz_device.trips[i].flags = THERMAL_TRIP_FLAG_RW_TEMP;
}
mvm->tz_device.tzone = thermal_zone_device_register_with_trips(name,
mvm->tz_device.trips,
IWL_MAX_DTS_TRIPS,
- IWL_WRITABLE_TRIPS_MSK,
+ 0,
mvm, &tzone_ops,
NULL, 0, 0);
if (IS_ERR(mvm->tz_device.tzone)) {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] wifi: iwlwifi: mvm: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:38 ` [PATCH v2 6/9] wifi: iwlwifi: mvm: " Rafael J. Wysocki
@ 2024-02-15 18:04 ` Rafael J. Wysocki
2024-02-22 15:14 ` Daniel Lezcano
1 sibling, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-15 18:04 UTC (permalink / raw)
To: Linux PM, Miri Korenblit
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Rafael J. Wysocki,
Ido Schimmel, Petr Machata, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi, Kalle Valo, Johannes Berg
On Mon, Feb 12, 2024 at 7:42 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
> to allow their temperature to be set from user space via sysfs instead
> of using a nonzero writable trips mask during thermal zone registration,
> so make the iwlwifi code do that.
>
> No intentional functional impact.
>
> Note that this change is requisite for dropping the mask argument from
> thermal_zone_device_register_with_trips() going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> v1 -> v2:
> * Rename trip flag (Stanislaw).
> * Fix coding mistake in iwl_mvm_thermal_zone_register().
> * Add "wifi:" prefix to the subject (Kalle).
I think that all of the feedback on the v1 of this patch has been
addressed, so are there any more concerns regarding it?
If not, it would be nice to get an ACK for it, so it can be routed
through the PM tree.
> ---
> drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> ===================================================================
> --- linux-pm.orig/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> +++ linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
> @@ -667,9 +667,6 @@ static struct thermal_zone_device_ops t
> .set_trip_temp = iwl_mvm_tzone_set_trip_temp,
> };
>
> -/* make all trips writable */
> -#define IWL_WRITABLE_TRIPS_MSK (BIT(IWL_MAX_DTS_TRIPS) - 1)
> -
> static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
> {
> int i, ret;
> @@ -692,11 +689,12 @@ static void iwl_mvm_thermal_zone_registe
> for (i = 0 ; i < IWL_MAX_DTS_TRIPS; i++) {
> mvm->tz_device.trips[i].temperature = THERMAL_TEMP_INVALID;
> mvm->tz_device.trips[i].type = THERMAL_TRIP_PASSIVE;
> + mvm->tz_device.trips[i].flags = THERMAL_TRIP_FLAG_RW_TEMP;
> }
> mvm->tz_device.tzone = thermal_zone_device_register_with_trips(name,
> mvm->tz_device.trips,
> IWL_MAX_DTS_TRIPS,
> - IWL_WRITABLE_TRIPS_MSK,
> + 0,
> mvm, &tzone_ops,
> NULL, 0, 0);
> if (IS_ERR(mvm->tz_device.tzone)) {
>
>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] wifi: iwlwifi: mvm: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:38 ` [PATCH v2 6/9] wifi: iwlwifi: mvm: " Rafael J. Wysocki
2024-02-15 18:04 ` Rafael J. Wysocki
@ 2024-02-22 15:14 ` Daniel Lezcano
1 sibling, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 15:14 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:38, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
> to allow their temperature to be set from user space via sysfs instead
> of using a nonzero writable trips mask during thermal zone registration,
> so make the iwlwifi code do that.
>
> No intentional functional impact.
>
> Note that this change is requisite for dropping the mask argument from
> thermal_zone_device_register_with_trips() going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 7/9] thermal: imx: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
` (5 preceding siblings ...)
2024-02-12 18:38 ` [PATCH v2 6/9] wifi: iwlwifi: mvm: " Rafael J. Wysocki
@ 2024-02-12 18:39 ` Rafael J. Wysocki
2024-02-22 15:32 ` Daniel Lezcano
2024-02-12 18:40 ` [PATCH v2 8/9] thermal: of: " Rafael J. Wysocki
` (2 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:39 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
to allow their temperature to be set from user space via sysfs instead
of using a nonzero writable trips mask during thermal zone registration,
so make the imx thermal code do that.
No intentional functional impact.
Note that this change is requisite for dropping the mask argument from
thermal_zone_device_register_with_trips() going forward.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2: Rename trip flag (Stanislaw).
---
drivers/thermal/imx_thermal.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/thermal/imx_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/imx_thermal.c
+++ linux-pm/drivers/thermal/imx_thermal.c
@@ -115,7 +115,8 @@ struct thermal_soc_data {
};
static struct thermal_trip trips[] = {
- [IMX_TRIP_PASSIVE] = { .type = THERMAL_TRIP_PASSIVE },
+ [IMX_TRIP_PASSIVE] = { .type = THERMAL_TRIP_PASSIVE,
+ .flags = THERMAL_TRIP_FLAG_RW_TEMP },
[IMX_TRIP_CRITICAL] = { .type = THERMAL_TRIP_CRITICAL },
};
@@ -699,7 +700,7 @@ static int imx_thermal_probe(struct plat
data->tz = thermal_zone_device_register_with_trips("imx_thermal_zone",
trips,
ARRAY_SIZE(trips),
- BIT(IMX_TRIP_PASSIVE), data,
+ 0, data,
&imx_tz_ops, NULL,
IMX_PASSIVE_DELAY,
IMX_POLLING_DELAY);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 7/9] thermal: imx: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:39 ` [PATCH v2 7/9] thermal: imx: " Rafael J. Wysocki
@ 2024-02-22 15:32 ` Daniel Lezcano
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 15:32 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:39, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
> to allow their temperature to be set from user space via sysfs instead
> of using a nonzero writable trips mask during thermal zone registration,
> so make the imx thermal code do that.
>
> No intentional functional impact.
>
> Note that this change is requisite for dropping the mask argument from
> thermal_zone_device_register_with_trips() going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 8/9] thermal: of: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
` (6 preceding siblings ...)
2024-02-12 18:39 ` [PATCH v2 7/9] thermal: imx: " Rafael J. Wysocki
@ 2024-02-12 18:40 ` Rafael J. Wysocki
2024-02-22 13:48 ` Daniel Lezcano
2024-02-22 15:33 ` Daniel Lezcano
2024-02-12 18:42 ` [PATCH v2 9/9] thermal: core: Eliminate writable trip points masks Rafael J. Wysocki
2024-02-15 17:56 ` [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
9 siblings, 2 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:40 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
to allow their temperature to be set from user space via sysfs instead
of using a nonzero writable trips mask during thermal zone registration,
so make the OF thermal code do that.
No intentional functional impact.
Note that this change is requisite for dropping the mask argument from
thermal_zone_device_register_with_trips() going forward.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2: Rename trip flag (Stanislaw).
---
drivers/thermal/thermal_of.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Index: linux-pm/drivers/thermal/thermal_of.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_of.c
+++ linux-pm/drivers/thermal/thermal_of.c
@@ -117,6 +117,8 @@ static int thermal_of_populate_trip(stru
return ret;
}
+ trip->flags = THERMAL_TRIP_FLAG_RW_TEMP;
+
return 0;
}
@@ -477,7 +479,7 @@ static struct thermal_zone_device *therm
struct device_node *np;
const char *action;
int delay, pdelay;
- int ntrips, mask;
+ int ntrips;
int ret;
of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
@@ -510,15 +512,13 @@ static struct thermal_zone_device *therm
of_ops->bind = thermal_of_bind;
of_ops->unbind = thermal_of_unbind;
- mask = GENMASK_ULL((ntrips) - 1, 0);
-
ret = of_property_read_string(np, "critical-action", &action);
if (!ret)
if (!of_ops->critical && !strcasecmp(action, "reboot"))
of_ops->critical = thermal_zone_device_critical_reboot;
tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips,
- mask, data, of_ops, &tzp,
+ 0, data, of_ops, &tzp,
pdelay, delay);
if (IS_ERR(tz)) {
ret = PTR_ERR(tz);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 8/9] thermal: of: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:40 ` [PATCH v2 8/9] thermal: of: " Rafael J. Wysocki
@ 2024-02-22 13:48 ` Daniel Lezcano
2024-02-22 13:59 ` Rafael J. Wysocki
2024-02-22 15:33 ` Daniel Lezcano
1 sibling, 1 reply; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 13:48 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:40, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
> to allow their temperature to be set from user space via sysfs instead
> of using a nonzero writable trips mask during thermal zone registration,
> so make the OF thermal code do that.
>
> No intentional functional impact.
>
> Note that this change is requisite for dropping the mask argument from
> thermal_zone_device_register_with_trips() going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> v1 -> v2: Rename trip flag (Stanislaw).
>
> ---
> drivers/thermal/thermal_of.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_of.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_of.c
> +++ linux-pm/drivers/thermal/thermal_of.c
> @@ -117,6 +117,8 @@ static int thermal_of_populate_trip(stru
> return ret;
> }
>
> + trip->flags = THERMAL_TRIP_FLAG_RW_TEMP;
> +
> return 0;
> }
Even if you are not at the origin of this default behavior. I'm
wondering if we should be more protective against changes from userspace
when the firmware is telling us to protect the silicon at a specific
temperature.
What do you think if we set the THERMAL_TRIP_FLAG_RW_TEMP only if the
trip point is not bound to a cooling device?
So trip points without associated cooling device can be writable but
others can be considered as managed by the kernel and no modifiable.
(This comment does not put in question this patch BTW)
> @@ -477,7 +479,7 @@ static struct thermal_zone_device *therm
> struct device_node *np;
> const char *action;
> int delay, pdelay;
> - int ntrips, mask;
> + int ntrips;
> int ret;
>
> of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> @@ -510,15 +512,13 @@ static struct thermal_zone_device *therm
> of_ops->bind = thermal_of_bind;
> of_ops->unbind = thermal_of_unbind;
>
> - mask = GENMASK_ULL((ntrips) - 1, 0);
> -
> ret = of_property_read_string(np, "critical-action", &action);
> if (!ret)
> if (!of_ops->critical && !strcasecmp(action, "reboot"))
> of_ops->critical = thermal_zone_device_critical_reboot;
>
> tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips,
> - mask, data, of_ops, &tzp,
> + 0, data, of_ops, &tzp,
> pdelay, delay);
> if (IS_ERR(tz)) {
> ret = PTR_ERR(tz);
>
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 8/9] thermal: of: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-22 13:48 ` Daniel Lezcano
@ 2024-02-22 13:59 ` Rafael J. Wysocki
0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-22 13:59 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rafael J. Wysocki, Linux PM, Lukasz Luba, LKML,
Stanislaw Gruszka, Srinivas Pandruvada, Zhang Rui, netdev,
Ido Schimmel, Petr Machata, Miri Korenblit, linux-wireless,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On Thu, Feb 22, 2024 at 2:48 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 12/02/2024 19:40, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
> > to allow their temperature to be set from user space via sysfs instead
> > of using a nonzero writable trips mask during thermal zone registration,
> > so make the OF thermal code do that.
> >
> > No intentional functional impact.
> >
> > Note that this change is requisite for dropping the mask argument from
> > thermal_zone_device_register_with_trips() going forward.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > v1 -> v2: Rename trip flag (Stanislaw).
> >
> > ---
> > drivers/thermal/thermal_of.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > Index: linux-pm/drivers/thermal/thermal_of.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_of.c
> > +++ linux-pm/drivers/thermal/thermal_of.c
> > @@ -117,6 +117,8 @@ static int thermal_of_populate_trip(stru
> > return ret;
> > }
> >
> > + trip->flags = THERMAL_TRIP_FLAG_RW_TEMP;
> > +
> > return 0;
> > }
>
> Even if you are not at the origin of this default behavior. I'm
> wondering if we should be more protective against changes from userspace
> when the firmware is telling us to protect the silicon at a specific
> temperature.
>
> What do you think if we set the THERMAL_TRIP_FLAG_RW_TEMP only if the
> trip point is not bound to a cooling device?
>
> So trip points without associated cooling device can be writable but
> others can be considered as managed by the kernel and no modifiable.
This sounds reasonable to me.
This is mostly relevant to thermal_of anyway, because the other
drivers asking for writable trip temperature seem to want it
regardless.
> (This comment does not put in question this patch BTW)
OK
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 8/9] thermal: of: Set THERMAL_TRIP_FLAG_RW_TEMP directly
2024-02-12 18:40 ` [PATCH v2 8/9] thermal: of: " Rafael J. Wysocki
2024-02-22 13:48 ` Daniel Lezcano
@ 2024-02-22 15:33 ` Daniel Lezcano
1 sibling, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 15:33 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:40, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> It is now possible to flag trip points with THERMAL_TRIP_FLAG_RW_TEMP
> to allow their temperature to be set from user space via sysfs instead
> of using a nonzero writable trips mask during thermal zone registration,
> so make the OF thermal code do that.
>
> No intentional functional impact.
>
> Note that this change is requisite for dropping the mask argument from
> thermal_zone_device_register_with_trips() going forward.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 9/9] thermal: core: Eliminate writable trip points masks
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
` (7 preceding siblings ...)
2024-02-12 18:40 ` [PATCH v2 8/9] thermal: of: " Rafael J. Wysocki
@ 2024-02-12 18:42 ` Rafael J. Wysocki
2024-02-22 15:37 ` Daniel Lezcano
2024-02-15 17:56 ` [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
9 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-12 18:42 UTC (permalink / raw)
To: Linux PM
Cc: Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
All of the thermal_zone_device_register_with_trips() callers pass zero
writable trip points masks to it, so drop the mask argument from that
function and update all of its callers accordingly.
This also removes the artificial trip points per zone limit of 32,
related to using writable trip points masks.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2: Rename trip flag (Stanislaw).
---
drivers/acpi/thermal.c | 2
drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c | 2
drivers/net/ethernet/mellanox/mlxsw/core_thermal.c | 3 -
drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 1
drivers/platform/x86/acerhdf.c | 2
drivers/thermal/da9062-thermal.c | 2
drivers/thermal/imx_thermal.c | 2
drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 2
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
drivers/thermal/rcar_thermal.c | 2
drivers/thermal/st/st_thermal.c | 2
drivers/thermal/thermal_core.c | 30 +---------
drivers/thermal/thermal_of.c | 2
include/linux/thermal.h | 6 --
18 files changed, 19 insertions(+), 49 deletions(-)
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -323,8 +323,7 @@ int thermal_zone_get_crit_temp(struct th
struct thermal_zone_device *thermal_zone_device_register_with_trips(
const char *type,
struct thermal_trip *trips,
- int num_trips, int mask,
- void *devdata,
+ int num_trips, void *devdata,
struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp,
int passive_delay, int polling_delay);
@@ -383,8 +382,7 @@ void thermal_zone_device_critical(struct
static inline struct thermal_zone_device *thermal_zone_device_register_with_trips(
const char *type,
struct thermal_trip *trips,
- int num_trips, int mask,
- void *devdata,
+ int num_trips, void *devdata,
struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp,
int passive_delay, int polling_delay)
Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1251,7 +1251,6 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
* @type: the thermal zone device type
* @trips: a pointer to an array of thermal trips
* @num_trips: the number of trip points the thermal zone support
- * @mask: a bit string indicating the writeablility of trip points
* @devdata: private device data
* @ops: standard thermal zone device callbacks
* @tzp: thermal zone platform parameters
@@ -1272,7 +1271,7 @@ 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,
+thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *trips, int num_trips,
void *devdata, struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp, int passive_delay,
int polling_delay)
@@ -1293,20 +1292,7 @@ thermal_zone_device_register_with_trips(
return ERR_PTR(-EINVAL);
}
- /*
- * Max trip count can't exceed 31 as the "mask >> num_trips" condition.
- * For example, shifting by 32 will result in compiler warning:
- * warning: right shift count >= width of type [-Wshift-count- overflow]
- *
- * Also "mask >> num_trips" will always be true with 32 bit shift.
- * E.g. mask = 0x80000000 for trip id 31 to be RW. Then
- * mask >> 32 = 0x80000000
- * This will result in failure for the below condition.
- *
- * Check will be true when the bit 31 of the mask is set.
- * 32 bit shift will cause overflow of 4 byte integer.
- */
- if (num_trips > (BITS_PER_TYPE(int) - 1) || num_trips < 0 || mask >> num_trips) {
+ if (num_trips < 0) {
pr_err("Incorrect number of thermal trips\n");
return ERR_PTR(-EINVAL);
}
@@ -1356,16 +1342,6 @@ thermal_zone_device_register_with_trips(
tz->devdata = devdata;
tz->trips = trips;
tz->num_trips = num_trips;
- if (num_trips > 0) {
- struct thermal_trip *trip;
-
- for_each_trip(tz, trip) {
- if (mask & 1)
- trip->flags |= THERMAL_TRIP_FLAG_RW_TEMP;
-
- mask >>= 1;
- }
- }
thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
@@ -1450,7 +1426,7 @@ struct thermal_zone_device *thermal_trip
struct thermal_zone_device_ops *ops,
const struct thermal_zone_params *tzp)
{
- return thermal_zone_device_register_with_trips(type, NULL, 0, 0, devdata,
+ return thermal_zone_device_register_with_trips(type, NULL, 0, devdata,
ops, tzp, 0, 0);
}
EXPORT_SYMBOL_GPL(thermal_tripless_zone_device_register);
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -670,7 +670,7 @@ static int acpi_thermal_register_thermal
tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
tz->trip_table,
trip_count,
- 0, tz,
+ tz,
&acpi_thermal_zone_ops,
NULL,
passive_delay,
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
@@ -184,7 +184,7 @@ struct int34x_thermal_zone *int340x_ther
int34x_zone->zone = thermal_zone_device_register_with_trips(
acpi_device_bid(adev),
zone_trips, trip_cnt,
- 0, int34x_zone,
+ int34x_zone,
int34x_zone->ops,
&int340x_thermal_params,
0, 0);
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
@@ -291,7 +291,7 @@ static int proc_thermal_pci_probe(struct
psv_trip.temperature = get_trip_temp(pci_info);
pci_info->tzone = thermal_zone_device_register_with_trips("TCPU_PCI", &psv_trip,
- 1, 0, pci_info,
+ 1, pci_info,
&tzone_ops,
&tzone_params, 0, 0);
if (IS_ERR(pci_info->tzone)) {
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
@@ -235,7 +235,7 @@ read_trips:
ptd->tzd = thermal_zone_device_register_with_trips(board_names[board_id],
ptd->trips, nr_trips,
- 0, ptd, &tzd_ops,
+ ptd, &tzd_ops,
NULL, 0, 0);
if (IS_ERR(ptd->tzd)) {
dev_err(&pdev->dev, "Failed to register thermal zone %s\n",
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
@@ -365,7 +365,7 @@ static struct soc_sensor_entry *alloc_so
aux_entry->tzone = thermal_zone_device_register_with_trips("quark_dts",
trips,
QRK_MAX_DTS_TRIPS,
- 0, aux_entry,
+ aux_entry,
&tzone_ops,
NULL, 0, polling_delay);
if (IS_ERR(aux_entry->tzone)) {
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
@@ -229,7 +229,7 @@ static int add_dts_thermal_zone(int id,
snprintf(name, sizeof(name), "soc_dts%d", id);
dts->tzone = thermal_zone_device_register_with_trips(name, dts->trips,
SOC_MAX_DTS_TRIPS,
- 0, dts, &tzone_ops,
+ dts, &tzone_ops,
NULL, 0, 0);
if (IS_ERR(dts->tzone)) {
ret = PTR_ERR(dts->tzone);
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
@@ -346,7 +346,7 @@ static int pkg_temp_thermal_device_add(u
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, 0,
+ zonedev->trips, thres_count,
zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
if (IS_ERR(zonedev->tzone)) {
err = PTR_ERR(zonedev->tzone);
Index: linux-pm/drivers/thermal/thermal_of.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_of.c
+++ linux-pm/drivers/thermal/thermal_of.c
@@ -518,7 +518,7 @@ static struct thermal_zone_device *therm
of_ops->critical = thermal_zone_device_critical_reboot;
tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips,
- 0, data, of_ops, &tzp,
+ data, of_ops, &tzp,
pdelay, delay);
if (IS_ERR(tz)) {
ret = PTR_ERR(tz);
Index: linux-pm/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
===================================================================
--- linux-pm.orig/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
+++ linux-pm/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
@@ -60,7 +60,7 @@ int cxgb4_thermal_init(struct adapter *a
snprintf(ch_tz_name, sizeof(ch_tz_name), "cxgb4_%s", adap->name);
ch_thermal->tzdev = thermal_zone_device_register_with_trips(ch_tz_name, &trip, num_trip,
- 0, adap,
+ adap,
&cxgb4_thermal_ops,
NULL, 0, 0);
if (IS_ERR(ch_thermal->tzdev)) {
Index: linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
===================================================================
--- linux-pm.orig/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ linux-pm/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -423,7 +423,6 @@ mlxsw_thermal_module_tz_init(struct mlxs
module_tz->tzdev = thermal_zone_device_register_with_trips(tz_name,
module_tz->trips,
MLXSW_THERMAL_NUM_TRIPS,
- 0,
module_tz,
&mlxsw_thermal_module_ops,
&mlxsw_thermal_params,
@@ -551,7 +550,6 @@ mlxsw_thermal_gearbox_tz_init(struct mlx
gearbox_tz->tzdev = thermal_zone_device_register_with_trips(tz_name,
gearbox_tz->trips,
MLXSW_THERMAL_NUM_TRIPS,
- 0,
gearbox_tz,
&mlxsw_thermal_gearbox_ops,
&mlxsw_thermal_params, 0,
@@ -776,7 +774,6 @@ int mlxsw_thermal_init(struct mlxsw_core
thermal->tzdev = thermal_zone_device_register_with_trips("mlxsw",
thermal->trips,
MLXSW_THERMAL_NUM_TRIPS,
- 0,
thermal,
&mlxsw_thermal_ops,
&mlxsw_thermal_params, 0,
Index: linux-pm/drivers/platform/x86/acerhdf.c
===================================================================
--- linux-pm.orig/drivers/platform/x86/acerhdf.c
+++ linux-pm/drivers/platform/x86/acerhdf.c
@@ -678,7 +678,7 @@ static int __init acerhdf_register_therm
return -EINVAL;
thz_dev = thermal_zone_device_register_with_trips("acerhdf", trips, ARRAY_SIZE(trips),
- 0, NULL, &acerhdf_dev_ops,
+ NULL, &acerhdf_dev_ops,
&acerhdf_zone_params, 0,
(kernelmode) ? interval*1000 : 0);
if (IS_ERR(thz_dev))
Index: linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
===================================================================
--- linux-pm.orig/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ linux-pm/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -694,7 +694,6 @@ static void iwl_mvm_thermal_zone_registe
mvm->tz_device.tzone = thermal_zone_device_register_with_trips(name,
mvm->tz_device.trips,
IWL_MAX_DTS_TRIPS,
- 0,
mvm, &tzone_ops,
NULL, 0, 0);
if (IS_ERR(mvm->tz_device.tzone)) {
Index: linux-pm/drivers/thermal/da9062-thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/da9062-thermal.c
+++ linux-pm/drivers/thermal/da9062-thermal.c
@@ -197,7 +197,7 @@ static int da9062_thermal_probe(struct p
mutex_init(&thermal->lock);
thermal->zone = thermal_zone_device_register_with_trips(thermal->config->name,
- trips, ARRAY_SIZE(trips), 0, thermal,
+ trips, ARRAY_SIZE(trips), thermal,
&da9062_thermal_ops, NULL, pp_tmp,
0);
if (IS_ERR(thermal->zone)) {
Index: linux-pm/drivers/thermal/imx_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/imx_thermal.c
+++ linux-pm/drivers/thermal/imx_thermal.c
@@ -700,7 +700,7 @@ static int imx_thermal_probe(struct plat
data->tz = thermal_zone_device_register_with_trips("imx_thermal_zone",
trips,
ARRAY_SIZE(trips),
- 0, data,
+ data,
&imx_tz_ops, NULL,
IMX_PASSIVE_DELAY,
IMX_POLLING_DELAY);
Index: linux-pm/drivers/thermal/rcar_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/rcar_thermal.c
+++ linux-pm/drivers/thermal/rcar_thermal.c
@@ -489,7 +489,7 @@ static int rcar_thermal_probe(struct pla
&rcar_thermal_zone_ops);
} else {
priv->zone = thermal_zone_device_register_with_trips(
- "rcar_thermal", trips, ARRAY_SIZE(trips), 0, priv,
+ "rcar_thermal", trips, ARRAY_SIZE(trips), priv,
&rcar_thermal_zone_ops, NULL, 0,
idle);
Index: linux-pm/drivers/thermal/st/st_thermal.c
===================================================================
--- linux-pm.orig/drivers/thermal/st/st_thermal.c
+++ linux-pm/drivers/thermal/st/st_thermal.c
@@ -203,7 +203,7 @@ int st_thermal_register(struct platform_
trip.type = THERMAL_TRIP_CRITICAL;
sensor->thermal_dev =
- thermal_zone_device_register_with_trips(dev_name(dev), &trip, 1, 0, sensor,
+ thermal_zone_device_register_with_trips(dev_name(dev), &trip, 1, sensor,
&st_tz_ops, NULL, 0, polling_delay);
if (IS_ERR(sensor->thermal_dev)) {
dev_err(dev, "failed to register thermal zone device\n");
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 9/9] thermal: core: Eliminate writable trip points masks
2024-02-12 18:42 ` [PATCH v2 9/9] thermal: core: Eliminate writable trip points masks Rafael J. Wysocki
@ 2024-02-22 15:37 ` Daniel Lezcano
0 siblings, 0 replies; 28+ messages in thread
From: Daniel Lezcano @ 2024-02-22 15:37 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Lukasz Luba, LKML, Stanislaw Gruszka, Srinivas Pandruvada,
Zhang Rui, netdev, Ido Schimmel, Petr Machata, Miri Korenblit,
linux-wireless, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On 12/02/2024 19:42, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> All of the thermal_zone_device_register_with_trips() callers pass zero
> writable trip points masks to it, so drop the mask argument from that
> function and update all of its callers accordingly.
>
> This also removes the artificial trip points per zone limit of 32,
> related to using writable trip points masks.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/9] thermal: Writable trip points handling rework
2024-02-12 18:25 [PATCH v2 0/9] thermal: Writable trip points handling rework Rafael J. Wysocki
` (8 preceding siblings ...)
2024-02-12 18:42 ` [PATCH v2 9/9] thermal: core: Eliminate writable trip points masks Rafael J. Wysocki
@ 2024-02-15 17:56 ` Rafael J. Wysocki
9 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2024-02-15 17:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Lukasz Luba, LKML, Daniel Lezcano, Stanislaw Gruszka,
Srinivas Pandruvada, Zhang Rui, netdev, Ido Schimmel,
Petr Machata, Miri Korenblit, linux-wireless, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team,
Manaf Meethalavalappu Pallikunhi
On Mon, Feb 12, 2024 at 7:44 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Hi Everyone,
>
> This is an update of
>
> https://lore.kernel.org/linux-pm/3232442.5fSG56mABF@kreacher/
>
> fixing a few bugs and renaming the new trip point flags introduced by it.
>
> The original description of the patch series is still applicable:
>
> "The purpose of this patch series is to allow thermal zone creators
> to specify which properties (temperature or hysteresis) of which
> trip points can be set from user space via sysfs on a per-trip basis
> instead of passing writable trips masks to the thermal zone registration
> function which is both cumbersome and error prone and it doesn't even
> allow to request different treatment of different trip properties.
>
> The writable trip masks used today only affect trip temperatures (that is, if
> a trip point is in a writable trips mask, its temperature can be set via
> sysfs) and they only take effect if the CONFIG_THERMAL_WRITABLE_TRIPS kernel
> configuration option is set, which appears to be assumed by at least some
> of the drivers using writable trips masks. Some other drivers using them
> simply select CONFIG_THERMAL_WRITABLE_TRIPS which pretty much defeats its
> purpose (and imx even sets this option in its defconfig).
>
> For this reasons, patch [1/9] removes CONFIG_THERMAL_WRITABLE_TRIPS and makes
> the writable trips masks always work.
>
> Moreover, trip hysteresis, which is not affected either by the writable trips
> masks or by CONFIG_THERMAL_WRITABLE_TRIPS, can only be set via sysfs if the
> .set_trip_hyst() operation is provided by the given thermal zone, but currently
> this thermal zone operation is used by no one, so effectively trip hysteresis
> cannot be set via sysfs at all. This is not a problem for the majority of
> drivers that want trip temperatures to be set via sysfs, because they also
> don't want trip hysteresis to be changed for any trips (at least as far as I
> can say), but there are use cases in which it is desirable to be able to
> update trip hysteresis as well as trip temperature (for example see
> https://lore.kernel.org/linux-pm/20240106191502.29126-1-quic_manafm@quicinc.com/).
> Those use cases are not addressed here directly, but after this series
> addressing them should be relatively straightforward.
>
> Namely, patch [2/9] adds flags to struct thermal_trip and defines two of them
> to indicate whether or not setting the temperature or hysteresis of the given
> trip via sysfs is allowed. If a writable trips mask is passed to
> thermal_zone_device_register_with_trips(), is it is used to set the
> "writable temperature" flag for the trips covered by it and that flag is
> then consulted by the thermal sysfs code. The "writable hysteresis" trip
> flag is also taken into account by the thermal sysfs code, but it is not set
> automatically in any case.
>
> Patch [3/9] is based on the observation that the .set_trip_hyst() thermal zone
> operation is never used - it simply drops that callback from struct
> thermal_zone_device_ops and adjusts the code checking its presence.
>
> Patches [4-8/9] update drivers using writable trips masks to set the new
> "writable temperature" flag directly instead and some of them are simplified
> a bit as a result. After these patches, all of the callers of
> thermal_zone_device_register_with_trips() pass a zero writable trips mask
> to it, so patch [9/9] drops that mask from the functions argument list and
> adjusts all of its callers accordingly.
>
> After all of the changes in this series, allowing the hysteresis value to be
> set via sysfs for a given trip is a matter of setting its "writable
> hysteresis" flag (and analogously for trip temperature)."
By the lack of comments I gather that this series is not controversial.
It unlocks further development and it should be run through linux-next
for a couple of weeks before the merge window, so reviews are welcome.
Thanks!
^ permalink raw reply [flat|nested] 28+ messages in thread