Messages in this thread Patch in this message | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH v1] thermal: sysfs: Make trip hysteresis writable along with trip temperature | Date | Wed, 31 Jan 2024 20:01:53 +0100 |
| |
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@linaro.org> wrote: > > > > On 29/01/2024 21:40, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.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@intel.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
| |