[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <12386481.O9o76ZdvQC@kreacher>
Date: Wed, 31 Jan 2024 20:01:53 +0100
From: "Rafael J. Wysocki" <rjw@...ysocki.net>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Linux PM <linux-pm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
Lukasz Luba <lukasz.luba@....com>, Zhang Rui <rui.zhang@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Manaf Meethalavalappu Pallikunhi <quic_manafm@...cinc.com>
Subject:
Re: [PATCH v1] thermal: sysfs: Make trip hysteresis writable along with trip
temperature
On Wednesday, January 31, 2024 7:41:52 PM CET Rafael J. Wysocki wrote:
> On Wed, Jan 31, 2024 at 7:18 PM Daniel Lezcano
> <daniel.lezcano@...aro.org> wrote:
> >
> > On 29/01/2024 21:40, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > >
> > > Trip point temperature can be modified via sysfs if
> > > CONFIG_THERMAL_WRITABLE_TRIPS is enabled and the thermal
> > > zone creator requested that the given trip be writable
> > > in the writable trips mask passed to the registration
> > > function.
> > >
> > > However, trip point hysteresis is treated differently - it is only
> > > writable if the thermal zone has a .set_trip_hyst() operation defined
> > > and neither CONFIG_THERMAL_WRITABLE_TRIPS, nor the writable trips mask
> > > supplied by the zone creator has any bearing on this. That is
> > > inconsistent and confusing, and it generally does not meet user
> > > expectations.
> > >
> > > For this reason, modify create_trip_attrs() to handle trip point
> > > hysteresis in the same way as trip point temperature, so they both
> > > are writable at the same time regardless of what trip point operations
> > > are defined for the thermal zone.
> > >
> > > Link: https://lore.kernel.org/linux-pm/20240106191502.29126-1-quic_manafm@quicinc.com
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > ---
> > >
> > > Notes:
> > >
> > > * I don't think that CONFIG_THERMAL_WRITABLE_TRIPS is very useful.
> > > The only thing controlled by it is whether or not the writable trip
> > > mask used during registration will have any effect and this is quite
> > > confusing. Some drivers select it for this reason which seems a bit
> > > odd to me.
> > >
> > > Maybe it can be dropped after the patch below?
> >
> > Actually it is used from an userspace daemon to get threshold crossing
> > temperature which is then changed on the fly.
>
> I mean to drop CONFIG_THERMAL_WRITABLE_TRIPS and make the writable
> trip masks used during zone registration always work. Sorry for the
> confusion.
So for the record, this (and note that the symbol is clearly not used as
intended, because drivers select it and one platform sets it in defconfig):
---
arch/arm/configs/imx_v6_v7_defconfig | 1 -
drivers/thermal/Kconfig | 11 -----------
drivers/thermal/intel/Kconfig | 2 --
drivers/thermal/thermal_sysfs.c | 8 +++-----
4 files changed, 3 insertions(+), 19 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;
@@ -474,8 +473,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 (IS_ENABLED(CONFIG_THERMAL_WRITABLE_TRIPS) &&
- mask & (1 << indx)) {
+ if (mask & (1 << indx)) {
tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
tz->trip_hyst_attrs[indx].attr.store =
trip_point_hyst_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
Powered by blists - more mailing lists