| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <cf055d45-8970-4657-ab86-d28636645c81@linaro.org>
Date: Wed, 6 Dec 2023 15:38:50 +0100
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linux PM <linux-pm@...r.kernel.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Zhang Rui <rui.zhang@...el.com>,
Linux ACPI <linux-acpi@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Lukasz Luba <lukasz.luba@....com>
Subject: Re: [PATCH v1] thermal: trip: Send trip change notifications on all
trip updates
Hi Rafael,
On 05/12/2023 20:18, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> The _store callbacks of the trip point temperature and hysteresis sysfs
> attributes invoke thermal_notify_tz_trip_change() to send a notification
> regarding the trip point change, but when trip points are updated by the
> platform firmware, trip point change notifications are not sent.
>
> To make the behavior after a trip point change more consistent,
> modify all of the 3 places where trip point temperature is updated
> to use a new function called thermal_zone_set_trip_temp() for this
> purpose and make that function call thermal_notify_tz_trip_change().
>
> Note that trip point hysteresis can only be updated via sysfs and
> trip_point_hyst_store() calls thermal_notify_tz_trip_change() already,
> so this code path need not be changed.
Why the ACPI driver is not calling thermal_zone_device_update() after
changing the trip point like the other drivers?
It would make sense to have the thermal framework to be notified about
this change and check if there is a trip violation, no ?
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>
> Depends on https://lore.kernel.org/linux-pm/12337662.O9o76ZdvQC@kreacher/
>
> ---
> drivers/acpi/thermal.c | 7 +++--
> drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c | 8 +++---
> drivers/thermal/thermal_sysfs.c | 4 +--
> drivers/thermal/thermal_trip.c | 14 ++++++++++-
> include/linux/thermal.h | 2 +
> 5 files changed, 27 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_sysfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> +++ linux-pm/drivers/thermal/thermal_sysfs.c
> @@ -146,9 +146,9 @@ trip_point_temp_store(struct device *dev
> goto unlock;
> }
>
> - trip->temperature = temp;
> + thermal_zone_set_trip_temp(tz, trip, temp);
>
> - thermal_zone_trip_updated(tz, trip);
> + __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
> }
>
> unlock:
> Index: linux-pm/drivers/thermal/thermal_trip.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_trip.c
> +++ linux-pm/drivers/thermal/thermal_trip.c
> @@ -152,7 +152,6 @@ int thermal_zone_trip_id(struct thermal_
> */
> return trip - tz->trips;
> }
> -
> void thermal_zone_trip_updated(struct thermal_zone_device *tz,
> const struct thermal_trip *trip)
> {
> @@ -161,3 +160,16 @@ void thermal_zone_trip_updated(struct th
> trip->hysteresis);
> __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);
> }
> +
> +void thermal_zone_set_trip_temp(struct thermal_zone_device *tz,
> + struct thermal_trip *trip, int temp)
> +{
> + if (trip->temperature == temp)
> + return;
> +
> + trip->temperature = temp;
> + thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),
> + trip->type, trip->temperature,
> + trip->hysteresis);
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_set_trip_temp);
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -289,6 +289,8 @@ int thermal_zone_for_each_trip(struct th
> int (*cb)(struct thermal_trip *, void *),
> void *data);
> int thermal_zone_get_num_trips(struct thermal_zone_device *tz);
> +void thermal_zone_set_trip_temp(struct thermal_zone_device *tz,
> + struct thermal_trip *trip, int temp);
>
> int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int *temp);
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -294,6 +294,7 @@ static int acpi_thermal_adjust_trip(stru
> struct acpi_thermal_trip *acpi_trip = trip->priv;
> struct adjust_trip_data *atd = data;
> struct acpi_thermal *tz = atd->tz;
> + int temp;
>
> if (!acpi_trip || !acpi_thermal_trip_valid(acpi_trip))
> return 0;
> @@ -304,9 +305,11 @@ static int acpi_thermal_adjust_trip(stru
> acpi_thermal_update_trip_devices(tz, trip);
>
> if (acpi_thermal_trip_valid(acpi_trip))
> - trip->temperature = acpi_thermal_temp(tz, acpi_trip->temp_dk);
> + temp = acpi_thermal_temp(tz, acpi_trip->temp_dk);
> else
> - trip->temperature = THERMAL_TEMP_INVALID;
> + temp = THERMAL_TEMP_INVALID;
> +
> + thermal_zone_set_trip_temp(tz->thermal_zone, trip, temp);
>
> return 0;
> }
> 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
> @@ -225,7 +225,8 @@ EXPORT_SYMBOL_GPL(int340x_thermal_zone_r
>
> static int int340x_update_one_trip(struct thermal_trip *trip, void *arg)
> {
> - struct acpi_device *zone_adev = arg;
> + struct int34x_thermal_zone *int34x_zone = arg;
> + struct acpi_device *zone_adev = int34x_zone->adev;
> int temp, err;
>
> switch (trip->type) {
> @@ -249,14 +250,15 @@ static int int340x_update_one_trip(struc
> if (err)
> temp = THERMAL_TEMP_INVALID;
>
> - trip->temperature = temp;
> + thermal_zone_set_trip_temp(int34x_zone->zone, trip, temp);
> +
> return 0;
> }
>
> void int340x_thermal_update_trips(struct int34x_thermal_zone *int34x_zone)
> {
> thermal_zone_for_each_trip(int34x_zone->zone, int340x_update_one_trip,
> - int34x_zone->adev);
> + int34x_zone);
> }
> EXPORT_SYMBOL_GPL(int340x_thermal_update_trips);
>
>
>
>
--
<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
Powered by blists - more mailing lists