From 44844db91397d3d94589f3c0c855be02daeebdb3 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 9 Nov 2023 16:01:48 +0100 Subject: [PATCH 01/64] thermal: core: Add trip thresholds for trip crossing detection The trip crossing detection in handle_thermal_trip() does not work correctly in the cases when a trip point is crossed on the way up and then the zone temperature stays above its low temperature (that is, its temperature decreased by its hysteresis). The trip temperature may be passed by the zone temperature subsequently in that case, even multiple times, but that does not count as the trip crossing as long as the zone temperature does not fall below the trip's low temperature or, in other words, until the trip is crossed on the way down. |-----------low--------high------------| |<--------->| | hyst | | | | -|--> crossed on the way up | <---|-- crossed on the way down However, handle_thermal_trip() will invoke thermal_notify_tz_trip_up() every time the trip temperature is passed by the zone temperature on the way up regardless of whether or not the trip has been crossed on the way down yet. Moreover, it will not call thermal_notify_tz_trip_down() if the last zone temperature was between the trip's temperature and its low temperature, so some "trip crossed on the way down" events may not be reported. To address this issue, introduce trip thresholds equal to either the temperature of the given trip, or its low temperature, such that if the trip's threshold is passed by the zone temperature on the way up, its value will be set to the trip's low temperature and thermal_notify_tz_trip_up() will be called, and if the trip's threshold is passed by the zone temperature on the way down, its value will be set to the trip's temperature (high) and thermal_notify_tz_trip_down() will be called. Accordingly, if the threshold is passed on the way up, it cannot be passed on the way up again until its passed on the way down and if it is passed on the way down, it cannot be passed on the way down again until it is passed on the way up which guarantees correct triggering of trip crossing notifications. If the last temperature of the zone is invalid, the trip's threshold will be set depending of the zone's current temperature: If that temperature is above the trip's temperature, its threshold will be set to its low temperature or otherwise its threshold will be set to its (high) temperature. Because the zone temperature is initially set to invalid and tz->last_temperature is only updated by update_temperature(), this is sufficient to set the correct initial threshold values for all trips. Link: https://lore.kernel.org/all/20220718145038.1114379-4-daniel.lezcano@linaro.org Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_core.c | 43 ++++++++++++++++++++++++++++------ include/linux/thermal.h | 2 ++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 9c17d35ccbbd..625ba07cbe2f 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -345,22 +345,51 @@ static void handle_critical_trips(struct thermal_zone_device *tz, } static void handle_thermal_trip(struct thermal_zone_device *tz, - const struct thermal_trip *trip) + struct thermal_trip *trip) { if (trip->temperature == THERMAL_TEMP_INVALID) return; - if (tz->last_temperature != THERMAL_TEMP_INVALID) { - if (tz->last_temperature < trip->temperature && - tz->temperature >= trip->temperature) + if (tz->last_temperature == THERMAL_TEMP_INVALID) { + /* Initialization. */ + trip->threshold = trip->temperature; + if (tz->temperature >= trip->threshold) + trip->threshold -= trip->hysteresis; + } else if (tz->last_temperature < trip->threshold) { + /* + * The trip threshold is equal to the trip temperature, unless + * the latter has changed in the meantime. In either case, + * the trip is crossed if the current zone temperature is at + * least equal to its temperature, but otherwise ensure that + * the threshold and the trip temperature will be equal. + */ + if (tz->temperature >= trip->temperature) { thermal_notify_tz_trip_up(tz->id, thermal_zone_trip_id(tz, trip), tz->temperature); - if (tz->last_temperature >= trip->temperature && - tz->temperature < trip->temperature - trip->hysteresis) + trip->threshold = trip->temperature - trip->hysteresis; + } else { + trip->threshold = trip->temperature; + } + } else { + /* + * The previous zone temperature was above or equal to the trip + * threshold, which would be equal to the "low temperature" of + * the trip (its temperature minus its hysteresis), unless the + * trip temperature or hysteresis had changed. In either case, + * the trip is crossed if the current zone temperature is below + * the low temperature of the trip, but otherwise ensure that + * the trip threshold will be equal to the low temperature of + * the trip. + */ + if (tz->temperature < trip->temperature - trip->hysteresis) { thermal_notify_tz_trip_down(tz->id, thermal_zone_trip_id(tz, trip), tz->temperature); + trip->threshold = trip->temperature; + } else { + trip->threshold = trip->temperature - trip->hysteresis; + } } if (trip->type == THERMAL_TRIP_CRITICAL || trip->type == THERMAL_TRIP_HOT) @@ -403,7 +432,7 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) void __thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event) { - const struct thermal_trip *trip; + struct thermal_trip *trip; if (atomic_read(&in_suspend)) return; diff --git a/include/linux/thermal.h b/include/linux/thermal.h index cee814d5d1ac..1f9ee869f9f9 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -57,12 +57,14 @@ enum thermal_notify_event { * struct thermal_trip - representation of a point in temperature domain * @temperature: temperature value in miliCelsius * @hysteresis: relative hysteresis in miliCelsius + * @threshold: trip crossing notification threshold miliCelsius * @type: trip point type * @priv: pointer to driver data associated with this trip */ struct thermal_trip { int temperature; int hysteresis; + int threshold; enum thermal_trip_type type; void *priv; }; From 4e6d4687f7645e3b4a83d915974e8749c24bf2e2 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 25 Oct 2023 20:22:19 +0100 Subject: [PATCH 02/64] thermal: gov_power_allocator: Rename trip_max_desired_temperature Refactor the code and rename the last passive trip point field. There is a comment describing the field properly. Use shorter field name so as to allow to clarify the code. This change is not expected to alter the general functionality. Signed-off-by: Lukasz Luba [ rjw: Changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 40 ++++++++++++--------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 83d4f451b1a9..97a8a6e4e1b0 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -59,9 +59,8 @@ static inline s64 div_frac(s64 x, s64 y) * governor switches on when this trip point is crossed. * If the thermal zone only has one passive trip point, * @trip_switch_on should be NULL. - * @trip_max_desired_temperature: last passive trip point of the thermal - * zone. The temperature we are - * controlling for. + * @trip_max: last passive trip point of the thermal zone. The + * temperature we are controlling for. */ struct power_allocator_params { bool allocated_tzp; @@ -69,7 +68,7 @@ struct power_allocator_params { s32 prev_err; u32 sustainable_power; const struct thermal_trip *trip_switch_on; - const struct thermal_trip *trip_max_desired_temperature; + const struct thermal_trip *trip_max; }; /** @@ -93,7 +92,7 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz) struct thermal_cooling_device *cdev = instance->cdev; u32 min_power; - if (instance->trip != params->trip_max_desired_temperature) + if (instance->trip != params->trip_max) continue; if (!cdev_is_power_actor(cdev)) @@ -379,8 +378,7 @@ static int allocate_power(struct thermal_zone_device *tz, { struct thermal_instance *instance; struct power_allocator_params *params = tz->governor_data; - const struct thermal_trip *trip_max_desired_temperature = - params->trip_max_desired_temperature; + const struct thermal_trip *trip_max = params->trip_max; u32 *req_power, *max_power, *granted_power, *extra_actor_power; u32 *weighted_req_power; u32 total_req_power, max_allocatable_power, total_weighted_req_power; @@ -390,7 +388,7 @@ static int allocate_power(struct thermal_zone_device *tz, num_actors = 0; total_weight = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - if ((instance->trip == trip_max_desired_temperature) && + if ((instance->trip == trip_max) && cdev_is_power_actor(instance->cdev)) { num_actors++; total_weight += instance->weight; @@ -429,7 +427,7 @@ static int allocate_power(struct thermal_zone_device *tz, int weight; struct thermal_cooling_device *cdev = instance->cdev; - if (instance->trip != trip_max_desired_temperature) + if (instance->trip != trip_max) continue; if (!cdev_is_power_actor(cdev)) @@ -465,7 +463,7 @@ static int allocate_power(struct thermal_zone_device *tz, total_granted_power = 0; i = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - if (instance->trip != trip_max_desired_temperature) + if (instance->trip != trip_max) continue; if (!cdev_is_power_actor(instance->cdev)) @@ -531,13 +529,13 @@ static void get_governor_trips(struct thermal_zone_device *tz, if (last_passive) { params->trip_switch_on = first_passive; - params->trip_max_desired_temperature = last_passive; + params->trip_max = last_passive; } else if (first_passive) { params->trip_switch_on = NULL; - params->trip_max_desired_temperature = first_passive; + params->trip_max = first_passive; } else { params->trip_switch_on = NULL; - params->trip_max_desired_temperature = last_active; + params->trip_max = last_active; } } @@ -556,8 +554,8 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) list_for_each_entry(instance, &tz->thermal_instances, tz_node) { struct thermal_cooling_device *cdev = instance->cdev; - if (instance->trip != params->trip_max_desired_temperature || - (!cdev_is_power_actor(instance->cdev))) + if (instance->trip != params->trip_max || + !cdev_is_power_actor(instance->cdev)) continue; instance->target = 0; @@ -642,12 +640,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz) get_governor_trips(tz, params); - if (params->trip_max_desired_temperature) { - int temp = params->trip_max_desired_temperature->temperature; - + if (params->trip_max) estimate_pid_constants(tz, tz->tzp->sustainable_power, - params->trip_switch_on, temp); - } + params->trip_switch_on, + params->trip_max->temperature); reset_pid_controller(params); @@ -688,7 +684,7 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, * We get called for every trip point but we only need to do * our calculations once */ - if (trip != params->trip_max_desired_temperature) + if (trip != params->trip_max) return 0; trip = params->trip_switch_on; @@ -702,7 +698,7 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, tz->passive = 1; - return allocate_power(tz, params->trip_max_desired_temperature->temperature); + return allocate_power(tz, params->trip_max->temperature); } static struct thermal_governor thermal_gov_power_allocator = { From e83747c2f8e3cc5e284e37a8921099f1901d79d8 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 25 Oct 2023 20:22:20 +0100 Subject: [PATCH 03/64] thermal: gov_power_allocator: Set up trip points earlier Set up the trip points at the beginning of the binding function. This simplifies the code a bit and allows for further cleanups. Also add a check to fail the binding if the last passive trip point is not found. Signed-off-by: Lukasz Luba [ rjw: Changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 97a8a6e4e1b0..0dfc5b5ab523 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -617,14 +617,24 @@ static int power_allocator_bind(struct thermal_zone_device *tz) int ret; struct power_allocator_params *params; - ret = check_power_actors(tz); - if (ret) - return ret; - params = kzalloc(sizeof(*params), GFP_KERNEL); if (!params) return -ENOMEM; + get_governor_trips(tz, params); + if (!params->trip_max) { + dev_warn(&tz->device, "power_allocator: missing trip_max\n"); + kfree(params); + return -EINVAL; + } + + ret = check_power_actors(tz); + if (ret) { + dev_warn(&tz->device, "power_allocator: binding failed\n"); + kfree(params); + return ret; + } + if (!tz->tzp) { tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL); if (!tz->tzp) { @@ -638,12 +648,9 @@ static int power_allocator_bind(struct thermal_zone_device *tz) if (!tz->tzp->sustainable_power) dev_warn(&tz->device, "power_allocator: sustainable_power will be estimated\n"); - get_governor_trips(tz, params); - - if (params->trip_max) - estimate_pid_constants(tz, tz->tzp->sustainable_power, - params->trip_switch_on, - params->trip_max->temperature); + estimate_pid_constants(tz, tz->tzp->sustainable_power, + params->trip_switch_on, + params->trip_max->temperature); reset_pid_controller(params); From c7568e78411a67b28fe23acda934cd26d9dc42a3 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 25 Oct 2023 20:22:21 +0100 Subject: [PATCH 04/64] thermal: gov_power_allocator: Check the cooling devices only for trip_max The throttling logic only cares about the last passive trip point and the cooling devices attached to it. Therefore, there is no need to bail out if other trip points have cooling devices which are not a supported by the IPA. Check the cooling devices only for 'trip_max' during the binding. Signed-off-by: Lukasz Luba [ rjw: Changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 0dfc5b5ab523..4b9ef04577a9 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -578,6 +578,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) * check_power_actors() - Check all cooling devices and warn when they are * not power actors * @tz: thermal zone to operate on + * @params: power allocator private data * * Check all cooling devices in the @tz and warn every time they are missing * power actor API. The warning should help to investigate the issue, which @@ -586,12 +587,16 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) * Return: 0 on success, -EINVAL if any cooling device does not implement * the power actor API. */ -static int check_power_actors(struct thermal_zone_device *tz) +static int check_power_actors(struct thermal_zone_device *tz, + struct power_allocator_params *params) { struct thermal_instance *instance; int ret = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { + if (instance->trip != params->trip_max) + continue; + if (!cdev_is_power_actor(instance->cdev)) { dev_warn(&tz->device, "power_allocator: %s is not a power actor\n", instance->cdev->type); @@ -628,7 +633,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz) return -EINVAL; } - ret = check_power_actors(tz); + ret = check_power_actors(tz, params); if (ret) { dev_warn(&tz->device, "power_allocator: binding failed\n"); kfree(params); From 499cc391b41c8ccf5f5eae4c85e1725a037f138f Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 25 Oct 2023 20:22:22 +0100 Subject: [PATCH 05/64] thermal: gov_power_allocator: Rearrange local variables Rearrange the order of local variable definitions in multiple functions so as to follow the kernel coding style in that respect. Also, move local variable definitions located in nested code blocks to the beginning of each function to improve the visibility of all local variables in use. This change is not expected to alter the general functionality. Signed-off-by: Lukasz Luba [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 39 ++++++++++++++------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 4b9ef04577a9..79621b42ead3 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -84,13 +84,14 @@ struct power_allocator_params { */ static u32 estimate_sustainable_power(struct thermal_zone_device *tz) { - u32 sustainable_power = 0; - struct thermal_instance *instance; struct power_allocator_params *params = tz->governor_data; + struct thermal_cooling_device *cdev; + struct thermal_instance *instance; + u32 sustainable_power = 0; + u32 min_power; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - struct thermal_cooling_device *cdev = instance->cdev; - u32 min_power; + cdev = instance->cdev; if (instance->trip != params->trip_max) continue; @@ -211,10 +212,10 @@ static u32 pid_controller(struct thermal_zone_device *tz, int control_temp, u32 max_allocatable_power) { + struct power_allocator_params *params = tz->governor_data; s64 p, i, d, power_range; s32 err, max_power_frac; u32 sustainable_power; - struct power_allocator_params *params = tz->governor_data; max_power_frac = int_to_frac(max_allocatable_power); @@ -373,20 +374,20 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, } } -static int allocate_power(struct thermal_zone_device *tz, - int control_temp) +static int allocate_power(struct thermal_zone_device *tz, int control_temp) { - struct thermal_instance *instance; + u32 total_req_power, max_allocatable_power, total_weighted_req_power; + u32 *req_power, *max_power, *granted_power, *extra_actor_power; struct power_allocator_params *params = tz->governor_data; const struct thermal_trip *trip_max = params->trip_max; - u32 *req_power, *max_power, *granted_power, *extra_actor_power; - u32 *weighted_req_power; - u32 total_req_power, max_allocatable_power, total_weighted_req_power; u32 total_granted_power, power_range; - int i, num_actors, total_weight, ret = 0; + struct thermal_cooling_device *cdev; + struct thermal_instance *instance; + u32 *weighted_req_power; + int i, weight, ret = 0; + int total_weight = 0; + int num_actors = 0; - num_actors = 0; - total_weight = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { if ((instance->trip == trip_max) && cdev_is_power_actor(instance->cdev)) { @@ -424,8 +425,7 @@ static int allocate_power(struct thermal_zone_device *tz, max_allocatable_power = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - int weight; - struct thermal_cooling_device *cdev = instance->cdev; + cdev = instance->cdev; if (instance->trip != trip_max) continue; @@ -547,12 +547,13 @@ static void reset_pid_controller(struct power_allocator_params *params) static void allow_maximum_power(struct thermal_zone_device *tz, bool update) { - struct thermal_instance *instance; struct power_allocator_params *params = tz->governor_data; + struct thermal_cooling_device *cdev; + struct thermal_instance *instance; u32 req_power; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - struct thermal_cooling_device *cdev = instance->cdev; + cdev = instance->cdev; if (instance->trip != params->trip_max || !cdev_is_power_actor(instance->cdev)) @@ -619,8 +620,8 @@ static int check_power_actors(struct thermal_zone_device *tz, */ static int power_allocator_bind(struct thermal_zone_device *tz) { - int ret; struct power_allocator_params *params; + int ret; params = kzalloc(sizeof(*params), GFP_KERNEL); if (!params) From 30e1178c100df8c8560f4d338fdb6f4fcd27e676 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 25 Oct 2023 20:22:23 +0100 Subject: [PATCH 06/64] thermal: gov_power_allocator: Use shorter paths to access data when possible The 'cdev' pointer in allow_maximum_power() is valid, so there is no need to use 'instance->cdev' instead of it. This change is not expected to alter the general functionality. Signed-off-by: Lukasz Luba [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 79621b42ead3..0f7f8278eacc 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -560,7 +560,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) continue; instance->target = 0; - mutex_lock(&instance->cdev->lock); + mutex_lock(&cdev->lock); /* * Call for updating the cooling devices local stats and avoid * periods of dozen of seconds when those have not been @@ -569,9 +569,9 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) cdev->ops->get_requested_power(cdev, &req_power); if (update) - __thermal_cdev_update(instance->cdev); + __thermal_cdev_update(cdev); - mutex_unlock(&instance->cdev->lock); + mutex_unlock(&cdev->lock); } } From 0458d536ae97c5107b81810778d050da04d83fa2 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 25 Oct 2023 20:22:24 +0100 Subject: [PATCH 07/64] thermal: gov_power_allocator: Remove excessive local variables Local variable 'ret' in allocate_power() is only used in the return statement, so drop it. Local variable 'trip_max' in allocate_power() is only used for caching the params->trip_max value which may as well be accessed directly as needed, so drop it either. This change is not expected to alter the general functionality. Signed-off-by: Lukasz Luba [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 0f7f8278eacc..e6d2f0fe8d2f 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -379,17 +379,16 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) u32 total_req_power, max_allocatable_power, total_weighted_req_power; u32 *req_power, *max_power, *granted_power, *extra_actor_power; struct power_allocator_params *params = tz->governor_data; - const struct thermal_trip *trip_max = params->trip_max; u32 total_granted_power, power_range; struct thermal_cooling_device *cdev; struct thermal_instance *instance; u32 *weighted_req_power; - int i, weight, ret = 0; int total_weight = 0; int num_actors = 0; + int i, weight; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - if ((instance->trip == trip_max) && + if ((instance->trip == params->trip_max) && cdev_is_power_actor(instance->cdev)) { num_actors++; total_weight += instance->weight; @@ -427,7 +426,7 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) list_for_each_entry(instance, &tz->thermal_instances, tz_node) { cdev = instance->cdev; - if (instance->trip != trip_max) + if (instance->trip != params->trip_max) continue; if (!cdev_is_power_actor(cdev)) @@ -463,7 +462,7 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) total_granted_power = 0; i = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - if (instance->trip != trip_max) + if (instance->trip != params->trip_max) continue; if (!cdev_is_power_actor(instance->cdev)) @@ -484,7 +483,7 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) kfree(req_power); - return ret; + return 0; } /** From 401888e7206778db54b79e6b3c25a2f1461413e6 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 25 Oct 2023 20:22:25 +0100 Subject: [PATCH 08/64] thermal: gov_power_allocator: Rearrange initialization of local variables Rearrange the initialization of local variables in allocate_power() so as to improve code clarity and the visibility of the initial values. This change is not expected to alter the general functionality. Signed-off-by: Lukasz Luba [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index e6d2f0fe8d2f..785fff14223d 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -376,16 +376,19 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, static int allocate_power(struct thermal_zone_device *tz, int control_temp) { - u32 total_req_power, max_allocatable_power, total_weighted_req_power; u32 *req_power, *max_power, *granted_power, *extra_actor_power; struct power_allocator_params *params = tz->governor_data; - u32 total_granted_power, power_range; struct thermal_cooling_device *cdev; struct thermal_instance *instance; + u32 total_weighted_req_power = 0; + u32 max_allocatable_power = 0; + u32 total_granted_power = 0; + u32 total_req_power = 0; u32 *weighted_req_power; + u32 power_range, weight; int total_weight = 0; int num_actors = 0; - int i, weight; + int i = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { if ((instance->trip == params->trip_max) && @@ -418,11 +421,6 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) extra_actor_power = &req_power[3 * num_actors]; weighted_req_power = &req_power[4 * num_actors]; - i = 0; - total_weighted_req_power = 0; - total_req_power = 0; - max_allocatable_power = 0; - list_for_each_entry(instance, &tz->thermal_instances, tz_node) { cdev = instance->cdev; @@ -459,7 +457,6 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) total_weighted_req_power, power_range, granted_power, extra_actor_power); - total_granted_power = 0; i = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { if (instance->trip != params->trip_max) From 59730241647287c0ab64bf0bc7449308392b7ea4 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 29 Nov 2023 14:36:07 +0100 Subject: [PATCH 09/64] thermal: trip: Drop a redundant check from thermal_zone_set_trip() After recent changes in the thermal framework, a trip points array is required for registering a thermal zone that is not tripless, so the tz->trips pointer in thermal_zone_set_trip() is never NULL and the check involving it is redundant. Drop that check. No functional impact. Signed-off-by: Rafael J. Wysocki Reviewed-by: Lukasz Luba Acked-by: Daniel Lezcano --- drivers/thermal/thermal_trip.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index e42456442c68..e3dd583234dd 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -153,9 +153,6 @@ int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id, struct thermal_trip t; int ret; - if (!tz->ops->set_trip_temp && !tz->ops->set_trip_hyst && !tz->trips) - return -EINVAL; - ret = __thermal_zone_get_trip(tz, trip_id, &t); if (ret) return ret; From be0a3600aa1ebe9d23243c91d41ab1a2d5091a9b Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Dec 2023 13:24:08 +0100 Subject: [PATCH 10/64] thermal: sysfs: Rework the handling of trip point updates Both trip_point_temp_store() and trip_point_hyst_store() use thermal_zone_set_trip() to update a given trip point, but none of them actually needs to change more than one field in struct thermal_trip representing it. However, each of them effectively calls __thermal_zone_get_trip() twice in a row for the same trip index value, once directly and once via thermal_zone_set_trip(), which is not particularly efficient, and the way in which thermal_zone_set_trip() carries out the update is not particularly straightforward. Moreover, input processing need not be done under the thermal zone lock in any of these functions. Rework trip_point_temp_store() and trip_point_hyst_store() to address the above, move the part of thermal_zone_set_trip() that is still useful to a new function called thermal_zone_trip_updated() and drop the rest of it. While at it, make trip_point_hyst_store() reject negative hysteresis values. Signed-off-by: Rafael J. Wysocki Reviewed-by: Daniel Lezcano --- drivers/thermal/thermal_core.h | 2 ++ drivers/thermal/thermal_sysfs.c | 52 +++++++++++++++++++++++---------- drivers/thermal/thermal_trip.c | 45 ++++++---------------------- include/linux/thermal.h | 4 --- 4 files changed, 47 insertions(+), 56 deletions(-) diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 0a3b3ec5120b..7dfe6c8deb8e 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -124,6 +124,8 @@ int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, struct thermal_trip *trip); int thermal_zone_trip_id(struct thermal_zone_device *tz, const struct thermal_trip *trip); +void thermal_zone_trip_updated(struct thermal_zone_device *tz, + const struct thermal_trip *trip); int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp); /* sysfs I/F */ diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index eef40d4f3063..06202aa50060 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -120,8 +120,13 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct thermal_zone_device *tz = to_thermal_zone(dev); - struct thermal_trip trip; + struct thermal_trip *trip; int trip_id, ret; + int temp; + + ret = kstrtoint(buf, 10, &temp); + if (ret) + return -EINVAL; if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1) return -EINVAL; @@ -133,15 +138,20 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, goto unlock; } - ret = __thermal_zone_get_trip(tz, trip_id, &trip); - if (ret) - goto unlock; + trip = &tz->trips[trip_id]; - ret = kstrtoint(buf, 10, &trip.temperature); - if (ret) - goto unlock; + if (temp != trip->temperature) { + if (tz->ops->set_trip_temp) { + ret = tz->ops->set_trip_temp(tz, trip_id, temp); + if (ret) + goto unlock; + } + + trip->temperature = temp; + + thermal_zone_trip_updated(tz, trip); + } - ret = thermal_zone_set_trip(tz, trip_id, &trip); unlock: mutex_unlock(&tz->lock); @@ -179,8 +189,13 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct thermal_zone_device *tz = to_thermal_zone(dev); - struct thermal_trip trip; + struct thermal_trip *trip; int trip_id, ret; + int hyst; + + ret = kstrtoint(buf, 10, &hyst); + if (ret || hyst < 0) + return -EINVAL; if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1) return -EINVAL; @@ -192,15 +207,20 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, goto unlock; } - ret = __thermal_zone_get_trip(tz, trip_id, &trip); - if (ret) - goto unlock; + trip = &tz->trips[trip_id]; - ret = kstrtoint(buf, 10, &trip.hysteresis); - if (ret) - goto unlock; + 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); + } - ret = thermal_zone_set_trip(tz, trip_id, &trip); unlock: mutex_unlock(&tz->lock); diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index e3dd583234dd..90861dec7eb0 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -147,42 +147,6 @@ int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, } EXPORT_SYMBOL_GPL(thermal_zone_get_trip); -int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id, - const struct thermal_trip *trip) -{ - struct thermal_trip t; - int ret; - - ret = __thermal_zone_get_trip(tz, trip_id, &t); - if (ret) - return ret; - - if (t.type != trip->type) - return -EINVAL; - - if (t.temperature != trip->temperature && tz->ops->set_trip_temp) { - ret = tz->ops->set_trip_temp(tz, trip_id, trip->temperature); - if (ret) - return ret; - } - - if (t.hysteresis != trip->hysteresis && tz->ops->set_trip_hyst) { - ret = tz->ops->set_trip_hyst(tz, trip_id, trip->hysteresis); - if (ret) - return ret; - } - - if (tz->trips && (t.temperature != trip->temperature || t.hysteresis != trip->hysteresis)) - tz->trips[trip_id] = *trip; - - thermal_notify_tz_trip_change(tz->id, trip_id, trip->type, - trip->temperature, trip->hysteresis); - - __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED); - - return 0; -} - int thermal_zone_trip_id(struct thermal_zone_device *tz, const struct thermal_trip *trip) { @@ -192,3 +156,12 @@ int thermal_zone_trip_id(struct thermal_zone_device *tz, */ return trip - tz->trips; } + +void thermal_zone_trip_updated(struct thermal_zone_device *tz, + const struct thermal_trip *trip) +{ + thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip), + trip->type, trip->temperature, + trip->hysteresis); + __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED); +} diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 1f9ee869f9f9..0ea99f50d57c 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -282,10 +282,6 @@ int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, struct thermal_trip *trip); int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, struct thermal_trip *trip); - -int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id, - const struct thermal_trip *trip); - int for_each_thermal_trip(struct thermal_zone_device *tz, int (*cb)(struct thermal_trip *, void *), void *data); From 18dfb0e4c3c3cd5654182833bcf3dfdcef754e6e Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Dec 2023 13:26:59 +0100 Subject: [PATCH 11/64] thermal: sysfs: Rework the reading of trip point attributes Rework the _show() callback functions for the trip point temperature, hysteresis and type attributes to avoid copying the values of struct thermal_trip fields that they do not use and make them carry out the same validation checks as the corresponding _store() callback functions. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Reviewed-by: Daniel Lezcano --- drivers/thermal/thermal_sysfs.c | 52 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index 06202aa50060..9e3d8fa01eea 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -83,25 +83,24 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, char *buf) { struct thermal_zone_device *tz = to_thermal_zone(dev); - struct thermal_trip trip; - int trip_id, result; + enum thermal_trip_type type; + int trip_id; if (sscanf(attr->attr.name, "trip_point_%d_type", &trip_id) != 1) return -EINVAL; mutex_lock(&tz->lock); - if (device_is_registered(dev)) - result = __thermal_zone_get_trip(tz, trip_id, &trip); - else - result = -ENODEV; + if (!device_is_registered(dev)) { + mutex_unlock(&tz->lock); + return -ENODEV; + } + + type = tz->trips[trip_id].type; mutex_unlock(&tz->lock); - if (result) - return result; - - switch (trip.type) { + switch (type) { case THERMAL_TRIP_CRITICAL: return sprintf(buf, "critical\n"); case THERMAL_TRIP_HOT: @@ -163,25 +162,23 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr, char *buf) { struct thermal_zone_device *tz = to_thermal_zone(dev); - struct thermal_trip trip; - int trip_id, ret; + int trip_id, temp; if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1) return -EINVAL; mutex_lock(&tz->lock); - if (device_is_registered(dev)) - ret = __thermal_zone_get_trip(tz, trip_id, &trip); - else - ret = -ENODEV; + if (!device_is_registered(dev)) { + mutex_unlock(&tz->lock); + return -ENODEV; + } + + temp = tz->trips[trip_id].temperature; mutex_unlock(&tz->lock); - if (ret) - return ret; - - return sprintf(buf, "%d\n", trip.temperature); + return sprintf(buf, "%d\n", temp); } static ssize_t @@ -232,22 +229,23 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr, char *buf) { struct thermal_zone_device *tz = to_thermal_zone(dev); - struct thermal_trip trip; - int trip_id, ret; + int trip_id, hyst; if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1) return -EINVAL; mutex_lock(&tz->lock); - if (device_is_registered(dev)) - ret = __thermal_zone_get_trip(tz, trip_id, &trip); - else - ret = -ENODEV; + if (!device_is_registered(dev)) { + mutex_unlock(&tz->lock); + return -ENODEV; + } + + hyst = tz->trips[trip_id].hysteresis; mutex_unlock(&tz->lock); - return ret ? ret : sprintf(buf, "%d\n", trip.hysteresis); + return sprintf(buf, "%d\n", hyst); } static ssize_t From 4649620d9404d3aceb25891c24bab77143e3f21c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 8 Dec 2023 20:13:44 +0100 Subject: [PATCH 12/64] thermal: core: Make thermal_zone_device_unregister() return after freeing the zone Make thermal_zone_device_unregister() wait until all of the references to the given thermal zone object have been dropped and free it before returning. This guarantees that when thermal_zone_device_unregister() returns, there is no leftover activity regarding the thermal zone in question which is required by some of its callers (for instance, modular driver code that wants to know when it is safe to let the module go away). Subsequently, this will allow some confusing device_is_registered() checks to be dropped from the thermal sysfs and core code. Signed-off-by: Rafael J. Wysocki Reviewed-and-tested-by: Lukasz Luba Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.c | 6 +++++- include/linux/thermal.h | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 625ba07cbe2f..70a294d12187 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -822,7 +822,7 @@ static void thermal_release(struct device *dev) tz = to_thermal_zone(dev); thermal_zone_destroy_device_groups(tz); mutex_destroy(&tz->lock); - kfree(tz); + complete(&tz->removal); } else if (!strncmp(dev_name(dev), "cooling_device", sizeof("cooling_device") - 1)) { cdev = to_cooling_device(dev); @@ -1315,6 +1315,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t INIT_LIST_HEAD(&tz->thermal_instances); ida_init(&tz->ida); mutex_init(&tz->lock); + init_completion(&tz->removal); id = ida_alloc(&thermal_tz_ida, GFP_KERNEL); if (id < 0) { result = id; @@ -1494,6 +1495,9 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz) put_device(&tz->device); thermal_notify_tz_delete(tz_id); + + wait_for_completion(&tz->removal); + kfree(tz); } EXPORT_SYMBOL_GPL(thermal_zone_device_unregister); diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 0ea99f50d57c..bedbaec9a42e 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -117,6 +117,7 @@ struct thermal_cooling_device { * @id: unique id number for each thermal zone * @type: the thermal zone device type * @device: &struct device for this thermal zone + * @removal: removal completion * @trip_temp_attrs: attributes for trip points for sysfs: trip temperature * @trip_type_attrs: attributes for trip points for sysfs: trip type * @trip_hyst_attrs: attributes for trip points for sysfs: trip hysteresis @@ -156,6 +157,7 @@ struct thermal_zone_device { int id; char type[THERMAL_NAME_LENGTH]; struct device device; + struct completion removal; struct attribute_group trips_attribute_group; struct thermal_attr *trip_temp_attrs; struct thermal_attr *trip_type_attrs; From c3ffdfff978a089f2d678571664eecd41953253d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 8 Dec 2023 20:19:03 +0100 Subject: [PATCH 13/64] thermal: Drop redundant and confusing device_is_registered() checks Multiple places in the thermal subsystem (most importantly, sysfs attribute callback functions) check if the given thermal zone device is still registered in order to return early in case the device_del() in thermal_zone_device_unregister() has run already. However, after thermal_zone_device_unregister() has been made wait for all of the zone-related activity to complete before returning, it is not necessary to do that any more, because all of the code holding a reference to the thermal zone device object will be waited for even if it does not do anything special to enforce this. Accordingly, drop all of the device_is_registered() checks that are now redundant and get rid of the zone locking that is not necessary any more after dropping them. Signed-off-by: Rafael J. Wysocki Reviewed-and-tested-by: Lukasz Luba Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.c | 9 ----- drivers/thermal/thermal_helpers.c | 5 +-- drivers/thermal/thermal_hwmon.c | 5 +-- drivers/thermal/thermal_sysfs.c | 60 +++---------------------------- 4 files changed, 7 insertions(+), 72 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 70a294d12187..2cf6caff6784 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -203,9 +203,6 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *tz, mutex_lock(&thermal_governor_lock); mutex_lock(&tz->lock); - if (!device_is_registered(&tz->device)) - goto exit; - gov = __find_governor(strim(policy)); if (!gov) goto exit; @@ -471,12 +468,6 @@ static int thermal_zone_device_set_mode(struct thermal_zone_device *tz, return ret; } - if (!device_is_registered(&tz->device)) { - mutex_unlock(&tz->lock); - - return -ENODEV; - } - if (tz->ops->change_mode) ret = tz->ops->change_mode(tz, mode); diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c index 69e8ea4aa908..d0afb623e475 100644 --- a/drivers/thermal/thermal_helpers.c +++ b/drivers/thermal/thermal_helpers.c @@ -139,10 +139,7 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp) goto unlock; } - if (device_is_registered(&tz->device)) - ret = __thermal_zone_get_temp(tz, temp); - else - ret = -ENODEV; + ret = __thermal_zone_get_temp(tz, temp); unlock: mutex_unlock(&tz->lock); diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c index c3ae44659b81..252116f1e535 100644 --- a/drivers/thermal/thermal_hwmon.c +++ b/drivers/thermal/thermal_hwmon.c @@ -80,10 +80,7 @@ temp_crit_show(struct device *dev, struct device_attribute *attr, char *buf) mutex_lock(&tz->lock); - if (device_is_registered(&tz->device)) - ret = tz->ops->get_crit_temp(tz, &temperature); - else - ret = -ENODEV; + ret = tz->ops->get_crit_temp(tz, &temperature); mutex_unlock(&tz->lock); diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index 9e3d8fa01eea..f52af8a3b4b5 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -83,24 +83,12 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr, char *buf) { struct thermal_zone_device *tz = to_thermal_zone(dev); - enum thermal_trip_type type; int trip_id; if (sscanf(attr->attr.name, "trip_point_%d_type", &trip_id) != 1) return -EINVAL; - mutex_lock(&tz->lock); - - if (!device_is_registered(dev)) { - mutex_unlock(&tz->lock); - return -ENODEV; - } - - type = tz->trips[trip_id].type; - - mutex_unlock(&tz->lock); - - switch (type) { + switch (tz->trips[trip_id].type) { case THERMAL_TRIP_CRITICAL: return sprintf(buf, "critical\n"); case THERMAL_TRIP_HOT: @@ -132,11 +120,6 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, mutex_lock(&tz->lock); - if (!device_is_registered(dev)) { - ret = -ENODEV; - goto unlock; - } - trip = &tz->trips[trip_id]; if (temp != trip->temperature) { @@ -162,23 +145,12 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr, char *buf) { struct thermal_zone_device *tz = to_thermal_zone(dev); - int trip_id, temp; + int trip_id; if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1) return -EINVAL; - mutex_lock(&tz->lock); - - if (!device_is_registered(dev)) { - mutex_unlock(&tz->lock); - return -ENODEV; - } - - temp = tz->trips[trip_id].temperature; - - mutex_unlock(&tz->lock); - - return sprintf(buf, "%d\n", temp); + return sprintf(buf, "%d\n", tz->trips[trip_id].temperature); } static ssize_t @@ -199,11 +171,6 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, mutex_lock(&tz->lock); - if (!device_is_registered(dev)) { - ret = -ENODEV; - goto unlock; - } - trip = &tz->trips[trip_id]; if (hyst != trip->hysteresis) { @@ -229,23 +196,12 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr, char *buf) { struct thermal_zone_device *tz = to_thermal_zone(dev); - int trip_id, hyst; + int trip_id; if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1) return -EINVAL; - mutex_lock(&tz->lock); - - if (!device_is_registered(dev)) { - mutex_unlock(&tz->lock); - return -ENODEV; - } - - hyst = tz->trips[trip_id].hysteresis; - - mutex_unlock(&tz->lock); - - return sprintf(buf, "%d\n", hyst); + return sprintf(buf, "%d\n", tz->trips[trip_id].hysteresis); } static ssize_t @@ -294,11 +250,6 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, mutex_lock(&tz->lock); - if (!device_is_registered(dev)) { - ret = -ENODEV; - goto unlock; - } - if (!tz->ops->set_emul_temp) tz->emul_temperature = temperature; else @@ -307,7 +258,6 @@ emul_temp_store(struct device *dev, struct device_attribute *attr, if (!ret) __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); -unlock: mutex_unlock(&tz->lock); return ret ? ret : count; From b38aa87f67931e23ebc32c0ca00a86dfa4688719 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 8 Dec 2023 20:20:00 +0100 Subject: [PATCH 14/64] thermal: core: Rework thermal zone availability check In order to avoid running __thermal_zone_device_update() for thermal zones going away, the thermal zone lock is held around device_del() in thermal_zone_device_unregister() and thermal_zone_device_update() passes the given thermal zone device to device_is_registered(). This allows thermal_zone_device_update() to skip the __thermal_zone_device_update() if device_del() has already run for the thermal zone at hand. However, instead of looking at driver core internals, the thermal subsystem may as well rely on its own data structures for this purpose. Namely, if the thermal zone is not present in thermal_tz_list, it can be regarded as unavailable, which in fact is already the case in thermal_zone_device_unregister(). Accordingly, the device_is_registered() check in thermal_zone_device_update() can be replaced with checking whether or not the node list_head in struct thermal_zone_device is empty, in which case it is not there in thermal_tz_list. To make this work, though, it is necessary to initialize tz->node in thermal_zone_device_register_with_trips() before registering the thermal zone device and it needs to be added to thermal_tz_list and deleted from it under its zone lock. After the above modifications, the zone lock does not need to be held around device_del() in thermal_zone_device_unregister() any more. Signed-off-by: Rafael J. Wysocki Reviewed-and-tested-by: Lukasz Luba Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 2cf6caff6784..e5434cdbf23b 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -505,11 +505,16 @@ int thermal_zone_device_is_enabled(struct thermal_zone_device *tz) return tz->mode == THERMAL_DEVICE_ENABLED; } +static bool thermal_zone_is_present(struct thermal_zone_device *tz) +{ + return !list_empty(&tz->node); +} + void thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event) { mutex_lock(&tz->lock); - if (device_is_registered(&tz->device)) + if (thermal_zone_is_present(tz)) __thermal_zone_device_update(tz, event); mutex_unlock(&tz->lock); } @@ -1304,6 +1309,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t } INIT_LIST_HEAD(&tz->thermal_instances); + INIT_LIST_HEAD(&tz->node); ida_init(&tz->ida); mutex_init(&tz->lock); init_completion(&tz->removal); @@ -1369,7 +1375,9 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t } mutex_lock(&thermal_list_lock); + mutex_lock(&tz->lock); list_add_tail(&tz->node, &thermal_tz_list); + mutex_unlock(&tz->lock); mutex_unlock(&thermal_list_lock); /* Bind cooling devices for this zone */ @@ -1460,7 +1468,10 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz) mutex_unlock(&thermal_list_lock); return; } + + mutex_lock(&tz->lock); list_del(&tz->node); + mutex_unlock(&tz->lock); /* Unbind all cdevs associated with 'this' thermal zone */ list_for_each_entry(cdev, &thermal_cdev_list, node) @@ -1477,9 +1488,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz) ida_free(&thermal_tz_ida, tz->id); ida_destroy(&tz->ida); - mutex_lock(&tz->lock); device_del(&tz->device); - mutex_unlock(&tz->lock); kfree(tz->tzp); From b6515a88baf4628e93fcc39c2b81fc1740eb3c3f Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 4 Dec 2023 20:36:14 +0100 Subject: [PATCH 15/64] thermal: trip: Drop redundant __thermal_zone_get_trip() header The __thermal_zone_get_trip() header in drivers/thermal/thermal_core.h is redundant, because there is one already in thermal.h, so drop it. No functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/thermal/thermal_core.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 7dfe6c8deb8e..fe2917a74054 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -120,8 +120,6 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, for (__trip = __tz->trips; __trip - __tz->trips < __tz->num_trips; __trip++) void __thermal_zone_set_trips(struct thermal_zone_device *tz); -int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, - struct thermal_trip *trip); int thermal_zone_trip_id(struct thermal_zone_device *tz, const struct thermal_trip *trip); void thermal_zone_trip_updated(struct thermal_zone_device *tz, From 0c0c4740c9d2668a234f7743ba50d54acab0821c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 4 Dec 2023 20:41:30 +0100 Subject: [PATCH 16/64] thermal: trip: Use for_each_trip() in __thermal_zone_set_trips() Make __thermal_zone_set_trips() use for_each_trip() instead of an open- coded loop over trip indices. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano Reviewed-by: Lukasz Luba --- drivers/thermal/thermal_trip.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 90861dec7eb0..581f1cd883e5 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -63,25 +63,21 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips); */ void __thermal_zone_set_trips(struct thermal_zone_device *tz) { - struct thermal_trip trip; + const struct thermal_trip *trip; int low = -INT_MAX, high = INT_MAX; bool same_trip = false; - int i, ret; + int ret; lockdep_assert_held(&tz->lock); if (!tz->ops->set_trips) return; - for (i = 0; i < tz->num_trips; i++) { + for_each_trip(tz, trip) { bool low_set = false; int trip_low; - ret = __thermal_zone_get_trip(tz, i , &trip); - if (ret) - return; - - trip_low = trip.temperature - trip.hysteresis; + trip_low = trip->temperature - trip->hysteresis; if (trip_low < tz->temperature && trip_low > low) { low = trip_low; @@ -89,9 +85,9 @@ void __thermal_zone_set_trips(struct thermal_zone_device *tz) same_trip = false; } - if (trip.temperature > tz->temperature && - trip.temperature < high) { - high = trip.temperature; + if (trip->temperature > tz->temperature && + trip->temperature < high) { + high = trip->temperature; same_trip = low_set; } } From 2e3e7dad4bf5ad869e8dc9fa09151913154318b0 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 4 Dec 2023 20:46:35 +0100 Subject: [PATCH 17/64] thermal: helpers: Use for_each_trip() in __thermal_zone_get_temp() Make __thermal_zone_get_temp() use for_each_trip() instead of an open- coded loop over trip indices. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano Reviewed-by: Lukasz Luba --- drivers/thermal/thermal_helpers.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c index d0afb623e475..c3982e0f0075 100644 --- a/drivers/thermal/thermal_helpers.c +++ b/drivers/thermal/thermal_helpers.c @@ -82,20 +82,18 @@ EXPORT_SYMBOL(get_thermal_instance); */ int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp) { - int ret = -EINVAL; - int count; + const struct thermal_trip *trip; int crit_temp = INT_MAX; - struct thermal_trip trip; + int ret = -EINVAL; lockdep_assert_held(&tz->lock); ret = tz->ops->get_temp(tz, temp); if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) { - for (count = 0; count < tz->num_trips; count++) { - ret = __thermal_zone_get_trip(tz, count, &trip); - if (!ret && trip.type == THERMAL_TRIP_CRITICAL) { - crit_temp = trip.temperature; + for_each_trip(tz, trip) { + if (trip->type == THERMAL_TRIP_CRITICAL) { + crit_temp = trip->temperature; break; } } From 183b64132f9692b15545c1aad755e71a53bcfb94 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 4 Dec 2023 20:49:03 +0100 Subject: [PATCH 18/64] thermal: netlink: Use for_each_trip() in thermal_genl_cmd_tz_get_trip() Make thermal_genl_cmd_tz_get_trip() use for_each_trip() instead of an open- coded loop over trip indices. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano Reviewed-by: Lukasz Luba --- drivers/thermal/thermal_netlink.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c index 08bc46c3ec7b..21f00d73acb7 100644 --- a/drivers/thermal/thermal_netlink.c +++ b/drivers/thermal/thermal_netlink.c @@ -450,10 +450,10 @@ out_cancel_nest: static int thermal_genl_cmd_tz_get_trip(struct param *p) { struct sk_buff *msg = p->msg; + const struct thermal_trip *trip; struct thermal_zone_device *tz; struct nlattr *start_trip; - struct thermal_trip trip; - int ret, i, id; + int id; if (!p->attrs[THERMAL_GENL_ATTR_TZ_ID]) return -EINVAL; @@ -470,16 +470,12 @@ static int thermal_genl_cmd_tz_get_trip(struct param *p) mutex_lock(&tz->lock); - for (i = 0; i < tz->num_trips; i++) { - - ret = __thermal_zone_get_trip(tz, i, &trip); - if (ret) - goto out_cancel_nest; - - if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, i) || - nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, trip.type) || - nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP, trip.temperature) || - nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST, trip.hysteresis)) + for_each_trip(tz, trip) { + if (nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_ID, + thermal_zone_trip_id(tz, trip)) || + nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TYPE, trip->type) || + nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_TEMP, trip->temperature) || + nla_put_u32(msg, THERMAL_GENL_ATTR_TZ_TRIP_HYST, trip->hysteresis)) goto out_cancel_nest; } From bdc22c8d52d70fc5655ab4dbf72fa79b034bb7b5 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 5 Dec 2023 20:18:39 +0100 Subject: [PATCH 19/64] thermal: trip: Send trip change notifications on all trip updates 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. Signed-off-by: Rafael J. Wysocki Acked-by: Daniel Lezcano --- drivers/acpi/thermal.c | 7 +++++-- .../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(-) diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index f74d81abdbfc..3e679e9feec5 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -297,6 +297,7 @@ static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data) 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; @@ -307,9 +308,11 @@ static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data) 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; } diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c index a03b67579dd9..3e4bfe817fac 100644 --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c @@ -225,7 +225,8 @@ EXPORT_SYMBOL_GPL(int340x_thermal_zone_remove); 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(struct thermal_trip *trip, void *arg) 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); diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index f52af8a3b4b5..d8ff74a4338a 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -129,9 +129,9 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, 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: diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index 581f1cd883e5..a1ad345c0741 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -152,7 +152,6 @@ int thermal_zone_trip_id(struct thermal_zone_device *tz, */ 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 thermal_zone_device *tz, 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); diff --git a/include/linux/thermal.h b/include/linux/thermal.h index bedbaec9a42e..09f6eb82c191 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -291,6 +291,8 @@ int thermal_zone_for_each_trip(struct thermal_zone_device *tz, 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); From 404f62cd6407f163e03cfaca97e27c1c4c62eb3c Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 13 Dec 2023 13:13:22 +0100 Subject: [PATCH 20/64] thermal/core: Check get_temp ops is present when registering a tz Initially the check against the get_temp ops in the thermal_zone_device_update() was put in there in order to catch drivers not providing this method. Instead of checking again and again the function if the ops exists in the update function, let's do the check at registration time, so it is checked one time and for all. Signed-off-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_core.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index e5434cdbf23b..2415dc50c31d 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -434,11 +434,6 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, if (atomic_read(&in_suspend)) return; - if (WARN_ONCE(!tz->ops->get_temp, - "'%s' must not be called without 'get_temp' ops set\n", - __func__)) - return; - if (!thermal_zone_device_is_enabled(tz)) return; @@ -1285,7 +1280,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t return ERR_PTR(-EINVAL); } - if (!ops) { + if (!ops || !ops->get_temp) { pr_err("Thermal zone device ops not defined\n"); return ERR_PTR(-EINVAL); } From 04e6ccfc93c5a1aa1d75a537cf27e418895e20ea Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 14 Dec 2023 11:52:25 +0100 Subject: [PATCH 21/64] thermal: core: Fix NULL pointer dereference in zone registration error path If device_register() in thermal_zone_device_register_with_trips() returns an error, the tz variable is set to NULL and subsequently dereferenced in kfree(tz->tzp). Commit adc8749b150c ("thermal/drivers/core: Use put_device() if device_register() fails") added the tz = NULL assignment in question to avoid a possible double-free after dropping the reference to the zone device. However, after commit 4649620d9404 ("thermal: core: Make thermal_zone_device_unregister() return after freeing the zone"), that assignment has become redundant, because dropping the reference to the zone device does not cause the zone object to be freed any more. Drop it to address the NULL pointer dereference. Fixes: 3d439b1a2ad3 ("thermal/core: Alloc-copy-free the thermal zone parameters structure") Signed-off-by: Rafael J. Wysocki Reviewed-by: Lukasz Luba --- drivers/thermal/thermal_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 2415dc50c31d..5e5fcbd81dda 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1393,7 +1393,6 @@ unregister: device_del(&tz->device); release_device: put_device(&tz->device); - tz = NULL; remove_id: ida_free(&thermal_tz_ida, id); free_tzp: From 5f70413a85056db04050604a76b52e3f39a37f21 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Wed, 20 Dec 2023 21:51:44 -0800 Subject: [PATCH 22/64] thermal: cpuidle_cooling: fix kernel-doc warning and a spello Correct one misuse of kernel-doc notation and one spelling error as reported by codespell. cpuidle_cooling.c:152: warning: cannot understand function prototype: 'struct thermal_cooling_device_ops cpuidle_cooling_ops = ' For the kernel-doc warning, don't use "/**" for a comment on data. kernel-doc can be used for structure declarations but not definitions. Signed-off-by: Randy Dunlap Signed-off-by: Rafael J. Wysocki --- drivers/thermal/cpuidle_cooling.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/cpuidle_cooling.c b/drivers/thermal/cpuidle_cooling.c index 69f4c0a8dfcc..f678c1281862 100644 --- a/drivers/thermal/cpuidle_cooling.c +++ b/drivers/thermal/cpuidle_cooling.c @@ -66,7 +66,7 @@ static unsigned int cpuidle_cooling_runtime(unsigned int idle_duration_us, * @state : a pointer to the state variable to be filled * * The function always returns 100 as the injection ratio. It is - * percentile based for consistency accross different platforms. + * percentile based for consistency across different platforms. * * Return: The function can not fail, it is always zero */ @@ -146,7 +146,7 @@ static int cpuidle_cooling_set_cur_state(struct thermal_cooling_device *cdev, return 0; } -/** +/* * cpuidle_cooling_ops - thermal cooling device ops */ static struct thermal_cooling_device_ops cpuidle_cooling_ops = { From 4e814173a8c4f432fd068b1c796f0416328c9d99 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 18 Dec 2023 20:25:02 +0100 Subject: [PATCH 23/64] thermal: core: Fix thermal zone suspend-resume synchronization There are 3 synchronization issues with thermal zone suspend-resume during system-wide transitions: 1. The resume code runs in a PM notifier which is invoked after user space has been thawed, so it can run concurrently with user space which can trigger a thermal zone device removal. If that happens, the thermal zone resume code may use a stale pointer to the next list element and crash, because it does not hold thermal_list_lock while walking thermal_tz_list. 2. The thermal zone resume code calls thermal_zone_device_init() outside the zone lock, so user space or an update triggered by the platform firmware may see an inconsistent state of a thermal zone leading to unexpected behavior. 3. Clearing the in_suspend global variable in thermal_pm_notify() allows __thermal_zone_device_update() to continue for all thermal zones and it may as well run before the thermal_tz_list walk (or at any point during the list walk for that matter) and attempt to operate on a thermal zone that has not been resumed yet. It may also race destructively with thermal_zone_device_init(). To address these issues, add thermal_list_lock locking to thermal_pm_notify(), especially arount the thermal_tz_list, make it call thermal_zone_device_init() back-to-back with __thermal_zone_device_update() under the zone lock and replace in_suspend with per-zone bool "suspend" indicators set and unset under the given zone's lock. Link: https://lore.kernel.org/linux-pm/20231218162348.69101-1-bo.ye@mediatek.com/ Reported-by: Bo Ye Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_core.c | 30 +++++++++++++++++++++++------- include/linux/thermal.h | 2 ++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 5e5fcbd81dda..7456335efaaa 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -37,8 +37,6 @@ static LIST_HEAD(thermal_governor_list); static DEFINE_MUTEX(thermal_list_lock); static DEFINE_MUTEX(thermal_governor_lock); -static atomic_t in_suspend; - static struct thermal_governor *def_governor; /* @@ -431,7 +429,7 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, { struct thermal_trip *trip; - if (atomic_read(&in_suspend)) + if (tz->suspended) return; if (!thermal_zone_device_is_enabled(tz)) @@ -1542,17 +1540,35 @@ static int thermal_pm_notify(struct notifier_block *nb, case PM_HIBERNATION_PREPARE: case PM_RESTORE_PREPARE: case PM_SUSPEND_PREPARE: - atomic_set(&in_suspend, 1); + mutex_lock(&thermal_list_lock); + + list_for_each_entry(tz, &thermal_tz_list, node) { + mutex_lock(&tz->lock); + + tz->suspended = true; + + mutex_unlock(&tz->lock); + } + + mutex_unlock(&thermal_list_lock); break; case PM_POST_HIBERNATION: case PM_POST_RESTORE: case PM_POST_SUSPEND: - atomic_set(&in_suspend, 0); + mutex_lock(&thermal_list_lock); + list_for_each_entry(tz, &thermal_tz_list, node) { + mutex_lock(&tz->lock); + + tz->suspended = false; + thermal_zone_device_init(tz); - thermal_zone_device_update(tz, - THERMAL_EVENT_UNSPECIFIED); + __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); + + mutex_unlock(&tz->lock); } + + mutex_unlock(&thermal_list_lock); break; default: break; diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 09f6eb82c191..d00622b64d50 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -152,6 +152,7 @@ struct thermal_cooling_device { * @node: node in thermal_tz_list (in thermal_core.c) * @poll_queue: delayed work for polling * @notify_event: Last notification event + * @suspended: thermal zone suspend indicator */ struct thermal_zone_device { int id; @@ -185,6 +186,7 @@ struct thermal_zone_device { struct list_head node; struct delayed_work poll_queue; enum thermal_notify_event notify_event; + bool suspended; }; /** From 33fcb595dc14678717274c270d02c7d7e0a3c404 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 18 Dec 2023 20:26:47 +0100 Subject: [PATCH 24/64] thermal: core: Initialize poll_queue in thermal_zone_device_init() In preparation for a subsequent change, move the initialization of the poll_queue delayed work from thermal_zone_device_register_with_trips() to thermal_zone_device_init() which is called by the former. However, because thermal_zone_device_init() is also called by thermal_pm_notify(), make the latter call cancel_delayed_work() on poll_queue before invoking the former, so as to allow the work item to be re-initialized safely. Also move thermal_zone_device_check() which needs to be defined before thermal_zone_device_init(), so the latter can pass it to the INIT_DELAYED_WORK() macro. Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_core.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 7456335efaaa..94e5e353b40e 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -414,9 +414,20 @@ static void update_temperature(struct thermal_zone_device *tz) thermal_genl_sampling_temp(tz->id, temp); } +static void thermal_zone_device_check(struct work_struct *work) +{ + struct thermal_zone_device *tz = container_of(work, struct + thermal_zone_device, + poll_queue.work); + thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); +} + static void thermal_zone_device_init(struct thermal_zone_device *tz) { struct thermal_instance *pos; + + INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check); + tz->temperature = THERMAL_TEMP_INVALID; tz->prev_low_trip = -INT_MAX; tz->prev_high_trip = INT_MAX; @@ -513,14 +524,6 @@ void thermal_zone_device_update(struct thermal_zone_device *tz, } EXPORT_SYMBOL_GPL(thermal_zone_device_update); -static void thermal_zone_device_check(struct work_struct *work) -{ - struct thermal_zone_device *tz = container_of(work, struct - thermal_zone_device, - poll_queue.work); - thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); -} - int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *), void *data) { @@ -1376,8 +1379,6 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t /* Bind cooling devices for this zone */ bind_tz(tz); - INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check); - thermal_zone_device_init(tz); /* Update the new thermal zone and mark it as already updated. */ if (atomic_cmpxchg(&tz->need_update, 1, 0)) @@ -1560,6 +1561,8 @@ static int thermal_pm_notify(struct notifier_block *nb, list_for_each_entry(tz, &thermal_tz_list, node) { mutex_lock(&tz->lock); + cancel_delayed_work(&tz->poll_queue); + tz->suspended = false; thermal_zone_device_init(tz); From 5a5efdaffda5d23717d9117cf36cda9eafcf2fae Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 18 Dec 2023 20:28:31 +0100 Subject: [PATCH 25/64] thermal: core: Resume thermal zones asynchronously The resume of thermal zones in thermal_pm_notify() is carried out sequentially, which may be a problem if __thermal_zone_device_update() takes a significant time to run for some thermal zones, because some other thermal zones may need to wait for them to resume then and if any other PM notifiers are going to be invoked after the thermal one, they will need to wait for it either. To address this, make thermal_pm_notify() switch the poll_queue delayed work over to a one-shot thermal_zone_device_resume() work function that will restore the original one during the thermal zone resume and queue up poll_queue without a delay for each thermal zone. Link: https://lore.kernel.org/linux-pm/20231120234015.3273143-1-radusolea@google.com/ Reported-by: Radu Solea Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_core.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 94e5e353b40e..3ffccd73b19e 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1532,6 +1532,22 @@ exit: } EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_name); +static void thermal_zone_device_resume(struct work_struct *work) +{ + struct thermal_zone_device *tz; + + tz = container_of(work, struct thermal_zone_device, poll_queue.work); + + mutex_lock(&tz->lock); + + tz->suspended = false; + + thermal_zone_device_init(tz); + __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); + + mutex_unlock(&tz->lock); +} + static int thermal_pm_notify(struct notifier_block *nb, unsigned long mode, void *_unused) { @@ -1563,10 +1579,16 @@ static int thermal_pm_notify(struct notifier_block *nb, cancel_delayed_work(&tz->poll_queue); - tz->suspended = false; - - thermal_zone_device_init(tz); - __thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED); + /* + * Replace the work function with the resume one, which + * will restore the original work function and schedule + * the polling work if needed. + */ + INIT_DELAYED_WORK(&tz->poll_queue, + thermal_zone_device_resume); + /* Queue up the work without a delay. */ + mod_delayed_work(system_freezable_power_efficient_wq, + &tz->poll_queue, 0); mutex_unlock(&tz->lock); } From 5eb4f413ad60db7c4b11c4d331b04f2909c8ba14 Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Thu, 28 Dec 2023 11:02:47 +0100 Subject: [PATCH 26/64] thermal: netlink: Add enum for mutlicast groups indexes Use enum instead of hard-coded numbers for indexing multicast groups. Signed-off-by: Stanislaw Gruszka Acked-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_netlink.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c index 21f00d73acb7..aca36c4ddbf3 100644 --- a/drivers/thermal/thermal_netlink.c +++ b/drivers/thermal/thermal_netlink.c @@ -13,9 +13,14 @@ #include "thermal_core.h" +enum thermal_genl_multicast_groups { + THERMAL_GENL_SAMPLING_GROUP = 0, + THERMAL_GENL_EVENT_GROUP = 1, +}; + static const struct genl_multicast_group thermal_genl_mcgrps[] = { - { .name = THERMAL_GENL_SAMPLING_GROUP_NAME, }, - { .name = THERMAL_GENL_EVENT_GROUP_NAME, }, + [THERMAL_GENL_SAMPLING_GROUP] = { .name = THERMAL_GENL_SAMPLING_GROUP_NAME, }, + [THERMAL_GENL_EVENT_GROUP] = { .name = THERMAL_GENL_EVENT_GROUP_NAME, }, }; static const struct nla_policy thermal_genl_policy[THERMAL_GENL_ATTR_MAX + 1] = { @@ -95,7 +100,7 @@ int thermal_genl_sampling_temp(int id, int temp) genlmsg_end(skb, hdr); - genlmsg_multicast(&thermal_gnl_family, skb, 0, 0, GFP_KERNEL); + genlmsg_multicast(&thermal_gnl_family, skb, 0, THERMAL_GENL_SAMPLING_GROUP, GFP_KERNEL); return 0; out_cancel: @@ -290,7 +295,7 @@ static int thermal_genl_send_event(enum thermal_genl_event event, genlmsg_end(msg, hdr); - genlmsg_multicast(&thermal_gnl_family, msg, 0, 1, GFP_KERNEL); + genlmsg_multicast(&thermal_gnl_family, msg, 0, THERMAL_GENL_EVENT_GROUP, GFP_KERNEL); return 0; From 04c3b03044034ce50886f2c0d1c595ff25f45085 Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Thu, 28 Dec 2023 11:02:48 +0100 Subject: [PATCH 27/64] thermal: netlink: Add thermal_group_has_listeners() helper Add a helper function to check if there are listeners for thermal_gnl_family multicast groups. For now use it to avoid unnecessary allocations and sending thermal genl messages when there are no recipients. In the future, in conjunction with (not yet implemented) notification of change in the netlink socket group membership, this helper can be used to open/close hardware interfaces based on the presence of user space subscribers. Signed-off-by: Stanislaw Gruszka Acked-by: Daniel Lezcano Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_netlink.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/thermal/thermal_netlink.c b/drivers/thermal/thermal_netlink.c index aca36c4ddbf3..332052e24a86 100644 --- a/drivers/thermal/thermal_netlink.c +++ b/drivers/thermal/thermal_netlink.c @@ -76,6 +76,11 @@ typedef int (*cb_t)(struct param *); static struct genl_family thermal_gnl_family; +static int thermal_group_has_listeners(enum thermal_genl_multicast_groups group) +{ + return genl_has_listeners(&thermal_gnl_family, &init_net, group); +} + /************************** Sampling encoding *******************************/ int thermal_genl_sampling_temp(int id, int temp) @@ -83,6 +88,9 @@ int thermal_genl_sampling_temp(int id, int temp) struct sk_buff *skb; void *hdr; + if (!thermal_group_has_listeners(THERMAL_GENL_SAMPLING_GROUP)) + return 0; + skb = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL); if (!skb) return -ENOMEM; @@ -280,6 +288,9 @@ static int thermal_genl_send_event(enum thermal_genl_event event, int ret = -EMSGSIZE; void *hdr; + if (!thermal_group_has_listeners(THERMAL_GENL_EVENT_GROUP)) + return 0; + msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL); if (!msg) return -ENOMEM; From a8c959402d4dd6823918b33828d79900ae58c700 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:45 +0000 Subject: [PATCH 28/64] thermal: core: Add governor callback for thermal zone change Add a new callback to the struct thermal_governor. It can be used for updating governors when there is a change in the thermal zone internals, e.g. thermal cooling device is bind to the thermal zone. That makes possible to move some heavy operations like memory allocations related to the number of cooling instances out of the throttle() callback. Both callback code paths (throttle() and update_tz()) are protected with the same thermal zone lock, which guaranties the consistency. Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_core.c | 14 ++++++++++++++ drivers/thermal/thermal_core.h | 2 ++ include/linux/thermal.h | 6 ++++++ 3 files changed, 22 insertions(+) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 3ffccd73b19e..58958288b559 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -309,6 +309,15 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz, def_governor->throttle(tz, trip); } +void thermal_governor_update_tz(struct thermal_zone_device *tz, + enum thermal_notify_event reason) +{ + if (!tz->governor || !tz->governor->update_tz) + return; + + tz->governor->update_tz(tz, reason); +} + void thermal_zone_device_critical(struct thermal_zone_device *tz) { /* @@ -715,6 +724,8 @@ int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz, list_add_tail(&dev->tz_node, &tz->thermal_instances); list_add_tail(&dev->cdev_node, &cdev->thermal_instances); atomic_set(&tz->need_update, 1); + + thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV); } mutex_unlock(&cdev->lock); mutex_unlock(&tz->lock); @@ -773,6 +784,9 @@ int thermal_unbind_cdev_from_trip(struct thermal_zone_device *tz, if (pos->tz == tz && pos->trip == trip && pos->cdev == cdev) { list_del(&pos->tz_node); list_del(&pos->cdev_node); + + thermal_governor_update_tz(tz, THERMAL_TZ_UNBIND_CDEV); + mutex_unlock(&cdev->lock); mutex_unlock(&tz->lock); goto unbind; diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index fe2917a74054..479c3b6917e4 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -114,6 +114,8 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *, char *); int thermal_build_list_of_policies(char *buf); void __thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event); +void thermal_governor_update_tz(struct thermal_zone_device *tz, + enum thermal_notify_event reason); /* Helpers */ #define for_each_trip(__tz, __trip) \ diff --git a/include/linux/thermal.h b/include/linux/thermal.h index d00622b64d50..4d96fefb2767 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -51,6 +51,8 @@ enum thermal_notify_event { THERMAL_DEVICE_POWER_CAPABILITY_CHANGED, /* power capability changed */ THERMAL_TABLE_CHANGED, /* Thermal table(s) changed */ THERMAL_EVENT_KEEP_ALIVE, /* Request for user space handler to respond */ + THERMAL_TZ_BIND_CDEV, /* Cooling dev is bind to the thermal zone */ + THERMAL_TZ_UNBIND_CDEV, /* Cooling dev is unbind from the thermal zone */ }; /** @@ -199,6 +201,8 @@ struct thermal_zone_device { * thermal zone. * @throttle: callback called for every trip point even if temperature is * below the trip point temperature + * @update_tz: callback called when thermal zone internals have changed, e.g. + * thermal cooling instance was added/removed * @governor_list: node in thermal_governor_list (in thermal_core.c) */ struct thermal_governor { @@ -207,6 +211,8 @@ struct thermal_governor { void (*unbind_from_tz)(struct thermal_zone_device *tz); int (*throttle)(struct thermal_zone_device *tz, const struct thermal_trip *trip); + void (*update_tz)(struct thermal_zone_device *tz, + enum thermal_notify_event reason); struct list_head governor_list; }; From 2c06456f656f7093077b4df958ed86a6554bc917 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:46 +0000 Subject: [PATCH 29/64] thermal: gov_power_allocator: Refactor check_power_actors() In preparation for a subsequent change, rearrange check_power_actors(). No intentional functional impact. Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 785fff14223d..d9175d9f5e3f 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -581,8 +581,9 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) * power actor API. The warning should help to investigate the issue, which * could be e.g. lack of Energy Model for a given device. * - * Return: 0 on success, -EINVAL if any cooling device does not implement - * the power actor API. + * If all of the cooling devices currently attached to @tz implement the power + * actor API, return the number of them (which may be 0, because some cooling + * devices may be attached later). Otherwise, return -EINVAL. */ static int check_power_actors(struct thermal_zone_device *tz, struct power_allocator_params *params) @@ -597,8 +598,9 @@ static int check_power_actors(struct thermal_zone_device *tz, if (!cdev_is_power_actor(instance->cdev)) { dev_warn(&tz->device, "power_allocator: %s is not a power actor\n", instance->cdev->type); - ret = -EINVAL; + return -EINVAL; } + ret++; } return ret; @@ -631,7 +633,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz) } ret = check_power_actors(tz, params); - if (ret) { + if (ret < 0) { dev_warn(&tz->device, "power_allocator: binding failed\n"); kfree(params); return ret; From 3d827317b17febba02b6b976fa910364221fecaf Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:47 +0000 Subject: [PATCH 30/64] thermal: gov_power_allocator: Refactor checks in divvy_up_power() Simplify the code and remove one extra 'if' block. No intentional functional impact. Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index d9175d9f5e3f..9e35ebd7cb03 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -332,7 +332,8 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, u32 total_req_power, u32 power_range, u32 *granted_power, u32 *extra_actor_power) { - u32 extra_power, capped_extra_power; + u32 capped_extra_power = 0; + u32 extra_power = 0; int i; /* @@ -341,8 +342,6 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, if (!total_req_power) total_req_power = 1; - capped_extra_power = 0; - extra_power = 0; for (i = 0; i < num_actors; i++) { u64 req_range = (u64)req_power[i] * power_range; @@ -358,7 +357,7 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, capped_extra_power += extra_actor_power[i]; } - if (!extra_power) + if (!extra_power || !capped_extra_power) return; /* @@ -366,12 +365,13 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, * how far they are from the max */ extra_power = min(extra_power, capped_extra_power); - if (capped_extra_power > 0) - for (i = 0; i < num_actors; i++) { - u64 extra_range = (u64)extra_actor_power[i] * extra_power; - granted_power[i] += DIV_ROUND_CLOSEST_ULL(extra_range, - capped_extra_power); - } + + for (i = 0; i < num_actors; i++) { + u64 extra_range = (u64)extra_actor_power[i] * extra_power; + + granted_power[i] += DIV_ROUND_CLOSEST_ULL(extra_range, + capped_extra_power); + } } static int allocate_power(struct thermal_zone_device *tz, int control_temp) From 792c3dc08ddcf29a514156bb72b3d2ad4998c69f Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:48 +0000 Subject: [PATCH 31/64] thermal: gov_power_allocator: Change trace functions Change trace event trace_thermal_power_allocator() to not use dynamic array for requested power and granted power for all power actors. Instead, simplify the trace event and print other simple values. Add new trace event to print power actor information of requested power and granted power. That trace event would be called in a loop for each power actor. The trace data would be easier to parse comparing to the dynamic array implementation. Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 5 +-- drivers/thermal/thermal_trace_ipa.h | 50 ++++++++++++++++----------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 9e35ebd7cb03..53283fd8a944 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -469,11 +469,12 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) granted_power[i]); total_granted_power += granted_power[i]; + trace_thermal_power_actor(tz, i, req_power[i], + granted_power[i]); i++; } - trace_thermal_power_allocator(tz, req_power, total_req_power, - granted_power, total_granted_power, + trace_thermal_power_allocator(tz, total_req_power, total_granted_power, num_actors, power_range, max_allocatable_power, tz->temperature, control_temp - tz->temperature); diff --git a/drivers/thermal/thermal_trace_ipa.h b/drivers/thermal/thermal_trace_ipa.h index 84568db5421b..b16b5dd863d9 100644 --- a/drivers/thermal/thermal_trace_ipa.h +++ b/drivers/thermal/thermal_trace_ipa.h @@ -8,19 +8,14 @@ #include TRACE_EVENT(thermal_power_allocator, - TP_PROTO(struct thermal_zone_device *tz, u32 *req_power, - u32 total_req_power, u32 *granted_power, - u32 total_granted_power, size_t num_actors, - u32 power_range, u32 max_allocatable_power, - int current_temp, s32 delta_temp), - TP_ARGS(tz, req_power, total_req_power, granted_power, - total_granted_power, num_actors, power_range, - max_allocatable_power, current_temp, delta_temp), + TP_PROTO(struct thermal_zone_device *tz, u32 total_req_power, + u32 total_granted_power, int num_actors, u32 power_range, + u32 max_allocatable_power, int current_temp, s32 delta_temp), + TP_ARGS(tz, total_req_power, total_granted_power, num_actors, + power_range, max_allocatable_power, current_temp, delta_temp), TP_STRUCT__entry( __field(int, tz_id ) - __dynamic_array(u32, req_power, num_actors ) __field(u32, total_req_power ) - __dynamic_array(u32, granted_power, num_actors) __field(u32, total_granted_power ) __field(size_t, num_actors ) __field(u32, power_range ) @@ -30,11 +25,7 @@ TRACE_EVENT(thermal_power_allocator, ), TP_fast_assign( __entry->tz_id = tz->id; - memcpy(__get_dynamic_array(req_power), req_power, - num_actors * sizeof(*req_power)); __entry->total_req_power = total_req_power; - memcpy(__get_dynamic_array(granted_power), granted_power, - num_actors * sizeof(*granted_power)); __entry->total_granted_power = total_granted_power; __entry->num_actors = num_actors; __entry->power_range = power_range; @@ -43,18 +34,35 @@ TRACE_EVENT(thermal_power_allocator, __entry->delta_temp = delta_temp; ), - TP_printk("thermal_zone_id=%d req_power={%s} total_req_power=%u granted_power={%s} total_granted_power=%u power_range=%u max_allocatable_power=%u current_temperature=%d delta_temperature=%d", - __entry->tz_id, - __print_array(__get_dynamic_array(req_power), - __entry->num_actors, 4), - __entry->total_req_power, - __print_array(__get_dynamic_array(granted_power), - __entry->num_actors, 4), + TP_printk("thermal_zone_id=%d total_req_power=%u total_granted_power=%u power_range=%u max_allocatable_power=%u current_temperature=%d delta_temperature=%d", + __entry->tz_id, __entry->total_req_power, __entry->total_granted_power, __entry->power_range, __entry->max_allocatable_power, __entry->current_temp, __entry->delta_temp) ); +TRACE_EVENT(thermal_power_actor, + TP_PROTO(struct thermal_zone_device *tz, int actor_id, u32 req_power, + u32 granted_power), + TP_ARGS(tz, actor_id, req_power, granted_power), + TP_STRUCT__entry( + __field(int, tz_id) + __field(int, actor_id) + __field(u32, req_power) + __field(u32, granted_power) + ), + TP_fast_assign( + __entry->tz_id = tz->id; + __entry->actor_id = actor_id; + __entry->req_power = req_power; + __entry->granted_power = granted_power; + ), + + TP_printk("thermal_zone_id=%d actor_id=%d req_power=%u granted_power=%u", + __entry->tz_id, __entry->actor_id, __entry->req_power, + __entry->granted_power) +); + TRACE_EVENT(thermal_power_allocator_pid, TP_PROTO(struct thermal_zone_device *tz, s32 err, s32 err_integral, s64 p, s64 i, s64 d, s32 output), From 912e97c67cc3f333c4c5df8f51498c651792e658 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:49 +0000 Subject: [PATCH 32/64] thermal: gov_power_allocator: Move memory allocation out of throttle() The new thermal callback allows to react to the change of cooling instances in the thermal zone. Move the memory allocation to that new callback and save CPU cycles in the throttle() code path. Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 207 +++++++++++++++++--------- 1 file changed, 136 insertions(+), 71 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 53283fd8a944..626c635f137f 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -46,6 +46,22 @@ static inline s64 div_frac(s64 x, s64 y) return div_s64(x << FRAC_BITS, y); } +/** + * struct power_actor - internal power information for power actor + * @req_power: requested power value (not weighted) + * @max_power: max allocatable power for this actor + * @granted_power: granted power for this actor + * @extra_actor_power: extra power that this actor can receive + * @weighted_req_power: weighted requested power as input to IPA + */ +struct power_actor { + u32 req_power; + u32 max_power; + u32 granted_power; + u32 extra_actor_power; + u32 weighted_req_power; +}; + /** * struct power_allocator_params - parameters for the power allocator governor * @allocated_tzp: whether we have allocated tzp for this thermal zone and @@ -61,6 +77,9 @@ static inline s64 div_frac(s64 x, s64 y) * @trip_switch_on should be NULL. * @trip_max: last passive trip point of the thermal zone. The * temperature we are controlling for. + * @num_actors: number of cooling devices supporting IPA callbacks + * @buffer_size: internal buffer size, to avoid runtime re-calculation + * @power: buffer for all power actors internal power information */ struct power_allocator_params { bool allocated_tzp; @@ -69,6 +88,9 @@ struct power_allocator_params { u32 sustainable_power; const struct thermal_trip *trip_switch_on; const struct thermal_trip *trip_max; + unsigned int num_actors; + unsigned int buffer_size; + struct power_actor *power; }; /** @@ -303,15 +325,10 @@ power_actor_set_power(struct thermal_cooling_device *cdev, /** * divvy_up_power() - divvy the allocated power between the actors - * @req_power: each actor's requested power - * @max_power: each actor's maximum available power - * @num_actors: size of the @req_power, @max_power and @granted_power's array - * @total_req_power: sum of @req_power + * @power: buffer for all power actors internal power information + * @num_actors: number of power actors in this thermal zone + * @total_req_power: sum of all weighted requested power for all actors * @power_range: total allocated power - * @granted_power: output array: each actor's granted power - * @extra_actor_power: an appropriately sized array to be used in the - * function as temporary storage of the extra power given - * to the actors * * This function divides the total allocated power (@power_range) * fairly between the actors. It first tries to give each actor a @@ -324,13 +341,9 @@ power_actor_set_power(struct thermal_cooling_device *cdev, * If any actor received more than their maximum power, then that * surplus is re-divvied among the actors based on how far they are * from their respective maximums. - * - * Granted power for each actor is written to @granted_power, which - * should've been allocated by the calling function. */ -static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, - u32 total_req_power, u32 power_range, - u32 *granted_power, u32 *extra_actor_power) +static void divvy_up_power(struct power_actor *power, int num_actors, + u32 total_req_power, u32 power_range) { u32 capped_extra_power = 0; u32 extra_power = 0; @@ -343,18 +356,19 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, total_req_power = 1; for (i = 0; i < num_actors; i++) { - u64 req_range = (u64)req_power[i] * power_range; + struct power_actor *pa = &power[i]; + u64 req_range = (u64)pa->req_power * power_range; - granted_power[i] = DIV_ROUND_CLOSEST_ULL(req_range, - total_req_power); + pa->granted_power = DIV_ROUND_CLOSEST_ULL(req_range, + total_req_power); - if (granted_power[i] > max_power[i]) { - extra_power += granted_power[i] - max_power[i]; - granted_power[i] = max_power[i]; + if (pa->granted_power > pa->max_power) { + extra_power += pa->granted_power - pa->max_power; + pa->granted_power = pa->max_power; } - extra_actor_power[i] = max_power[i] - granted_power[i]; - capped_extra_power += extra_actor_power[i]; + pa->extra_actor_power = pa->max_power - pa->granted_power; + capped_extra_power += pa->extra_actor_power; } if (!extra_power || !capped_extra_power) @@ -367,61 +381,44 @@ static void divvy_up_power(u32 *req_power, u32 *max_power, int num_actors, extra_power = min(extra_power, capped_extra_power); for (i = 0; i < num_actors; i++) { - u64 extra_range = (u64)extra_actor_power[i] * extra_power; + struct power_actor *pa = &power[i]; + u64 extra_range = pa->extra_actor_power; - granted_power[i] += DIV_ROUND_CLOSEST_ULL(extra_range, - capped_extra_power); + extra_range *= extra_power; + pa->granted_power += DIV_ROUND_CLOSEST_ULL(extra_range, + capped_extra_power); } } static int allocate_power(struct thermal_zone_device *tz, int control_temp) { - u32 *req_power, *max_power, *granted_power, *extra_actor_power; struct power_allocator_params *params = tz->governor_data; + unsigned int num_actors = params->num_actors; + struct power_actor *power = params->power; struct thermal_cooling_device *cdev; struct thermal_instance *instance; u32 total_weighted_req_power = 0; u32 max_allocatable_power = 0; u32 total_granted_power = 0; u32 total_req_power = 0; - u32 *weighted_req_power; u32 power_range, weight; int total_weight = 0; - int num_actors = 0; - int i = 0; - - list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - if ((instance->trip == params->trip_max) && - cdev_is_power_actor(instance->cdev)) { - num_actors++; - total_weight += instance->weight; - } - } + int i = 0, ret; if (!num_actors) return -ENODEV; - /* - * We need to allocate five arrays of the same size: - * req_power, max_power, granted_power, extra_actor_power and - * weighted_req_power. They are going to be needed until this - * function returns. Allocate them all in one go to simplify - * the allocation and deallocation logic. - */ - BUILD_BUG_ON(sizeof(*req_power) != sizeof(*max_power)); - BUILD_BUG_ON(sizeof(*req_power) != sizeof(*granted_power)); - BUILD_BUG_ON(sizeof(*req_power) != sizeof(*extra_actor_power)); - BUILD_BUG_ON(sizeof(*req_power) != sizeof(*weighted_req_power)); - req_power = kcalloc(num_actors * 5, sizeof(*req_power), GFP_KERNEL); - if (!req_power) - return -ENOMEM; + list_for_each_entry(instance, &tz->thermal_instances, tz_node) + if ((instance->trip == params->trip_max) && + cdev_is_power_actor(instance->cdev)) + total_weight += instance->weight; - max_power = &req_power[num_actors]; - granted_power = &req_power[2 * num_actors]; - extra_actor_power = &req_power[3 * num_actors]; - weighted_req_power = &req_power[4 * num_actors]; + /* Clean all buffers for new power estimations */ + memset(power, 0, params->buffer_size); list_for_each_entry(instance, &tz->thermal_instances, tz_node) { + struct power_actor *pa = &power[i]; + cdev = instance->cdev; if (instance->trip != params->trip_max) @@ -430,7 +427,8 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) if (!cdev_is_power_actor(cdev)) continue; - if (cdev->ops->get_requested_power(cdev, &req_power[i])) + ret = cdev->ops->get_requested_power(cdev, &pa->req_power); + if (ret) continue; if (!total_weight) @@ -438,27 +436,29 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) else weight = instance->weight; - weighted_req_power[i] = frac_to_int(weight * req_power[i]); + pa->weighted_req_power = frac_to_int(weight * pa->req_power); - if (cdev->ops->state2power(cdev, instance->lower, - &max_power[i])) + ret = cdev->ops->state2power(cdev, instance->lower, + &pa->max_power); + if (ret) continue; - total_req_power += req_power[i]; - max_allocatable_power += max_power[i]; - total_weighted_req_power += weighted_req_power[i]; + total_req_power += pa->req_power; + max_allocatable_power += pa->max_power; + total_weighted_req_power += pa->weighted_req_power; i++; } power_range = pid_controller(tz, control_temp, max_allocatable_power); - divvy_up_power(weighted_req_power, max_power, num_actors, - total_weighted_req_power, power_range, granted_power, - extra_actor_power); + divvy_up_power(power, num_actors, total_weighted_req_power, + power_range); i = 0; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { + struct power_actor *pa = &power[i]; + if (instance->trip != params->trip_max) continue; @@ -466,11 +466,11 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) continue; power_actor_set_power(instance->cdev, instance, - granted_power[i]); - total_granted_power += granted_power[i]; + pa->granted_power); + total_granted_power += pa->granted_power; - trace_thermal_power_actor(tz, i, req_power[i], - granted_power[i]); + trace_thermal_power_actor(tz, i, pa->req_power, + pa->granted_power); i++; } @@ -479,8 +479,6 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) max_allocatable_power, tz->temperature, control_temp - tz->temperature); - kfree(req_power); - return 0; } @@ -607,6 +605,63 @@ static int check_power_actors(struct thermal_zone_device *tz, return ret; } +static int allocate_actors_buffer(struct power_allocator_params *params, + int num_actors) +{ + int ret; + + kfree(params->power); + + /* There might be no cooling devices yet. */ + if (!num_actors) { + ret = -EINVAL; + goto clean_state; + } + + params->power = kcalloc(num_actors, sizeof(struct power_actor), + GFP_KERNEL); + if (!params->power) { + ret = -ENOMEM; + goto clean_state; + } + + params->num_actors = num_actors; + params->buffer_size = num_actors * sizeof(struct power_actor); + + return 0; + +clean_state: + params->num_actors = 0; + params->buffer_size = 0; + params->power = NULL; + return ret; +} + +static void power_allocator_update_tz(struct thermal_zone_device *tz, + enum thermal_notify_event reason) +{ + struct power_allocator_params *params = tz->governor_data; + struct thermal_instance *instance; + int num_actors = 0; + + switch (reason) { + case THERMAL_TZ_BIND_CDEV: + case THERMAL_TZ_UNBIND_CDEV: + list_for_each_entry(instance, &tz->thermal_instances, tz_node) + if ((instance->trip == params->trip_max) && + cdev_is_power_actor(instance->cdev)) + num_actors++; + + if (num_actors == params->num_actors) + return; + + allocate_actors_buffer(params, num_actors); + break; + default: + break; + } +} + /** * power_allocator_bind() - bind the power_allocator governor to a thermal zone * @tz: thermal zone to bind it to @@ -640,6 +695,13 @@ static int power_allocator_bind(struct thermal_zone_device *tz) return ret; } + ret = allocate_actors_buffer(params, ret); + if (ret) { + dev_warn(&tz->device, "power_allocator: allocation failed\n"); + kfree(params); + return ret; + } + if (!tz->tzp) { tz->tzp = kzalloc(sizeof(*tz->tzp), GFP_KERNEL); if (!tz->tzp) { @@ -664,6 +726,7 @@ static int power_allocator_bind(struct thermal_zone_device *tz) return 0; free_params: + kfree(params->power); kfree(params); return ret; @@ -680,6 +743,7 @@ static void power_allocator_unbind(struct thermal_zone_device *tz) tz->tzp = NULL; } + kfree(params->power); kfree(tz->governor_data); tz->governor_data = NULL; } @@ -718,5 +782,6 @@ static struct thermal_governor thermal_gov_power_allocator = { .bind_to_tz = power_allocator_bind, .unbind_from_tz = power_allocator_unbind, .throttle = power_allocator_throttle, + .update_tz = power_allocator_update_tz, }; THERMAL_GOVERNOR_DECLARE(thermal_gov_power_allocator); From e3ecd5716b957ff0e558e853d34be8d1e8173f64 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:50 +0000 Subject: [PATCH 33/64] thermal: gov_power_allocator: Simplify checks for valid power actor There is a need to check if the cooling device in the thermal zone supports IPA callback and is set for control trip point. Refactor the code which validates the power actor capabilities and make it more consistent in all places. No intentional functional impact. Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 44 ++++++++++++--------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 626c635f137f..b5ec60ae7efd 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -93,6 +93,13 @@ struct power_allocator_params { struct power_actor *power; }; +static bool power_actor_is_valid(struct power_allocator_params *params, + struct thermal_instance *instance) +{ + return (instance->trip == params->trip_max && + cdev_is_power_actor(instance->cdev)); +} + /** * estimate_sustainable_power() - Estimate the sustainable power of a thermal zone * @tz: thermal zone we are operating in @@ -113,14 +120,10 @@ static u32 estimate_sustainable_power(struct thermal_zone_device *tz) u32 min_power; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { + if (!power_actor_is_valid(params, instance)) + continue; + cdev = instance->cdev; - - if (instance->trip != params->trip_max) - continue; - - if (!cdev_is_power_actor(cdev)) - continue; - if (cdev->ops->state2power(cdev, instance->upper, &min_power)) continue; @@ -409,8 +412,7 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) return -ENODEV; list_for_each_entry(instance, &tz->thermal_instances, tz_node) - if ((instance->trip == params->trip_max) && - cdev_is_power_actor(instance->cdev)) + if (power_actor_is_valid(params, instance)) total_weight += instance->weight; /* Clean all buffers for new power estimations */ @@ -419,14 +421,11 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) list_for_each_entry(instance, &tz->thermal_instances, tz_node) { struct power_actor *pa = &power[i]; + if (!power_actor_is_valid(params, instance)) + continue; + cdev = instance->cdev; - if (instance->trip != params->trip_max) - continue; - - if (!cdev_is_power_actor(cdev)) - continue; - ret = cdev->ops->get_requested_power(cdev, &pa->req_power); if (ret) continue; @@ -459,10 +458,7 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) list_for_each_entry(instance, &tz->thermal_instances, tz_node) { struct power_actor *pa = &power[i]; - if (instance->trip != params->trip_max) - continue; - - if (!cdev_is_power_actor(instance->cdev)) + if (!power_actor_is_valid(params, instance)) continue; power_actor_set_power(instance->cdev, instance, @@ -548,12 +544,11 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update) u32 req_power; list_for_each_entry(instance, &tz->thermal_instances, tz_node) { - cdev = instance->cdev; - - if (instance->trip != params->trip_max || - !cdev_is_power_actor(instance->cdev)) + if (!power_actor_is_valid(params, instance)) continue; + cdev = instance->cdev; + instance->target = 0; mutex_lock(&cdev->lock); /* @@ -648,8 +643,7 @@ static void power_allocator_update_tz(struct thermal_zone_device *tz, case THERMAL_TZ_BIND_CDEV: case THERMAL_TZ_UNBIND_CDEV: list_for_each_entry(instance, &tz->thermal_instances, tz_node) - if ((instance->trip == params->trip_max) && - cdev_is_power_actor(instance->cdev)) + if (power_actor_is_valid(params, instance)) num_actors++; if (num_actors == params->num_actors) From 879c9dc511732b74a04f11336e00f12783337a8a Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:51 +0000 Subject: [PATCH 34/64] thermal/sysfs: Update instance->weight under tz lock User space can change the weight of a thermal instance via sysfs while the .throttle() callback is running for a governor, because weight_store() does not use the zone lock. The IPA governor uses instance weight values for power calculations and caches the sum of them as total_weight, so it gets confused when one of them changes while its .throttle() callback is running. To prevent that from happening, use thermal zone locking in weight_store(). Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_sysfs.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index d8ff74a4338a..299c0fb16593 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -936,7 +936,11 @@ ssize_t weight_store(struct device *dev, struct device_attribute *attr, return ret; instance = container_of(attr, struct thermal_instance, weight_attr); + + /* Don't race with governors using the 'weight' value */ + mutex_lock(&instance->tz->lock); instance->weight = weight; + mutex_unlock(&instance->tz->lock); return count; } From bfc57bd1685981730bfe9802d9de7603a0a43bc4 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:52 +0000 Subject: [PATCH 35/64] thermal/sysfs: Update governors when the 'weight' has changed Support governors update when the thermal instance's weight has changed. This allows to adjust internal state for the governor. Signed-off-by: Lukasz Luba [ rjw: Add two empty code lines aroung the locking ] Signed-off-by: Rafael J. Wysocki --- drivers/thermal/thermal_sysfs.c | 5 +++++ include/linux/thermal.h | 1 + 2 files changed, 6 insertions(+) diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index 299c0fb16593..f4033865b093 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -939,7 +939,12 @@ ssize_t weight_store(struct device *dev, struct device_attribute *attr, /* Don't race with governors using the 'weight' value */ mutex_lock(&instance->tz->lock); + instance->weight = weight; + + thermal_governor_update_tz(instance->tz, + THERMAL_INSTANCE_WEIGHT_CHANGED); + mutex_unlock(&instance->tz->lock); return count; diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 4d96fefb2767..9d0427da32af 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -53,6 +53,7 @@ enum thermal_notify_event { THERMAL_EVENT_KEEP_ALIVE, /* Request for user space handler to respond */ THERMAL_TZ_BIND_CDEV, /* Cooling dev is bind to the thermal zone */ THERMAL_TZ_UNBIND_CDEV, /* Cooling dev is unbind from the thermal zone */ + THERMAL_INSTANCE_WEIGHT_CHANGED, /* Thermal instance weight changed */ }; /** From a3cd6db4cc2ed70fc3468cdb5eb20745e7fefba9 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Wed, 20 Dec 2023 23:17:53 +0000 Subject: [PATCH 36/64] thermal: gov_power_allocator: Support new update callback of weights When the thermal instance's weight is updated from the sysfs the governor update_tz() callback is triggered. Implement proper reaction to this event in the IPA, which would save CPU cycles spent in throttle(). This will speed-up the main throttle() IPA function and clean it up a bit. Signed-off-by: Lukasz Luba Signed-off-by: Rafael J. Wysocki --- drivers/thermal/gov_power_allocator.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index b5ec60ae7efd..7b6aa265ff6a 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -77,6 +77,7 @@ struct power_actor { * @trip_switch_on should be NULL. * @trip_max: last passive trip point of the thermal zone. The * temperature we are controlling for. + * @total_weight: Sum of all thermal instances weights * @num_actors: number of cooling devices supporting IPA callbacks * @buffer_size: internal buffer size, to avoid runtime re-calculation * @power: buffer for all power actors internal power information @@ -88,6 +89,7 @@ struct power_allocator_params { u32 sustainable_power; const struct thermal_trip *trip_switch_on; const struct thermal_trip *trip_max; + int total_weight; unsigned int num_actors; unsigned int buffer_size; struct power_actor *power; @@ -405,16 +407,11 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) u32 total_granted_power = 0; u32 total_req_power = 0; u32 power_range, weight; - int total_weight = 0; int i = 0, ret; if (!num_actors) return -ENODEV; - list_for_each_entry(instance, &tz->thermal_instances, tz_node) - if (power_actor_is_valid(params, instance)) - total_weight += instance->weight; - /* Clean all buffers for new power estimations */ memset(power, 0, params->buffer_size); @@ -430,7 +427,7 @@ static int allocate_power(struct thermal_zone_device *tz, int control_temp) if (ret) continue; - if (!total_weight) + if (!params->total_weight) weight = 1 << FRAC_BITS; else weight = instance->weight; @@ -651,6 +648,12 @@ static void power_allocator_update_tz(struct thermal_zone_device *tz, allocate_actors_buffer(params, num_actors); break; + case THERMAL_INSTANCE_WEIGHT_CHANGED: + params->total_weight = 0; + list_for_each_entry(instance, &tz->thermal_instances, tz_node) + if (power_actor_is_valid(params, instance)) + params->total_weight += instance->weight; + break; default: break; } From 788494ba09992f6bc1c50327afdd1c861113fbe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Fri, 17 Nov 2023 06:22:14 +0100 Subject: [PATCH 37/64] dt-bindings: thermal: convert Mediatek Thermal to the json-schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This helps validating DTS files. Introduced changes: 1. Improved title 2. Simplified description (dropped "This describes the device tree...") 3. Dropped undocumented "reset-names" from example Signed-off-by: Rafał Miłecki Reviewed-by: Rob Herring Reviewed-by: AngeloGioacchino Del Regno Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231117052214.24554-1-zajec5@gmail.com --- .../bindings/thermal/mediatek,thermal.yaml | 99 +++++++++++++++++++ .../bindings/thermal/mediatek-thermal.txt | 52 ---------- 2 files changed, 99 insertions(+), 52 deletions(-) create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml delete mode 100644 Documentation/devicetree/bindings/thermal/mediatek-thermal.txt diff --git a/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml new file mode 100644 index 000000000000..d96a2e32bd8f --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/thermal/mediatek,thermal.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Mediatek thermal controller for on-SoC temperatures + +maintainers: + - Sascha Hauer + +description: + This device does not have its own ADC, instead it directly controls the AUXADC + via AHB bus accesses. For this reason it needs phandles to the AUXADC. Also it + controls a mux in the apmixedsys register space via AHB bus accesses, so a + phandle to the APMIXEDSYS is also needed. + +allOf: + - $ref: thermal-sensor.yaml# + +properties: + compatible: + enum: + - mediatek,mt2701-thermal + - mediatek,mt2712-thermal + - mediatek,mt7622-thermal + - mediatek,mt7981-thermal + - mediatek,mt7986-thermal + - mediatek,mt8173-thermal + - mediatek,mt8183-thermal + - mediatek,mt8365-thermal + - mediatek,mt8516-thermal + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + items: + - description: Main clock needed for register access + - description: The AUXADC clock + + clock-names: + items: + - const: therm + - const: auxadc + + mediatek,auxadc: + $ref: /schemas/types.yaml#/definitions/phandle + description: A phandle to the AUXADC which the thermal controller uses + + mediatek,apmixedsys: + $ref: /schemas/types.yaml#/definitions/phandle + description: A phandle to the APMIXEDSYS controller + + resets: + description: Reset controller controlling the thermal controller + + nvmem-cells: + items: + - description: + NVMEM cell with EEPROMA phandle to the calibration data provided by an + NVMEM device. If unspecified default values shall be used. + + nvmem-cell-names: + items: + - const: calibration-data + +required: + - reg + - interrupts + - clocks + - clock-names + - mediatek,auxadc + - mediatek,apmixedsys + +unevaluatedProperties: false + +examples: + - | + #include + #include + #include + + thermal@1100b000 { + compatible = "mediatek,mt8173-thermal"; + reg = <0x1100b000 0x1000>; + interrupts = <0 70 IRQ_TYPE_LEVEL_LOW>; + clocks = <&pericfg CLK_PERI_THERM>, <&pericfg CLK_PERI_AUXADC>; + clock-names = "therm", "auxadc"; + resets = <&pericfg MT8173_PERI_THERM_SW_RST>; + mediatek,auxadc = <&auxadc>; + mediatek,apmixedsys = <&apmixedsys>; + nvmem-cells = <&thermal_calibration_data>; + nvmem-cell-names = "calibration-data"; + #thermal-sensor-cells = <1>; + }; diff --git a/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt deleted file mode 100644 index ac39c7156fde..000000000000 --- a/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt +++ /dev/null @@ -1,52 +0,0 @@ -* Mediatek Thermal - -This describes the device tree binding for the Mediatek thermal controller -which measures the on-SoC temperatures. This device does not have its own ADC, -instead it directly controls the AUXADC via AHB bus accesses. For this reason -this device needs phandles to the AUXADC. Also it controls a mux in the -apmixedsys register space via AHB bus accesses, so a phandle to the APMIXEDSYS -is also needed. - -Required properties: -- compatible: - - "mediatek,mt8173-thermal" : For MT8173 family of SoCs - - "mediatek,mt2701-thermal" : For MT2701 family of SoCs - - "mediatek,mt2712-thermal" : For MT2712 family of SoCs - - "mediatek,mt7622-thermal" : For MT7622 SoC - - "mediatek,mt7981-thermal", "mediatek,mt7986-thermal" : For MT7981 SoC - - "mediatek,mt7986-thermal" : For MT7986 SoC - - "mediatek,mt8183-thermal" : For MT8183 family of SoCs - - "mediatek,mt8365-thermal" : For MT8365 family of SoCs - - "mediatek,mt8516-thermal", "mediatek,mt2701-thermal : For MT8516 family of SoCs -- reg: Address range of the thermal controller -- interrupts: IRQ for the thermal controller -- clocks, clock-names: Clocks needed for the thermal controller. required - clocks are: - "therm": Main clock needed for register access - "auxadc": The AUXADC clock -- mediatek,auxadc: A phandle to the AUXADC which the thermal controller uses -- mediatek,apmixedsys: A phandle to the APMIXEDSYS controller. -- #thermal-sensor-cells : Should be 0. See Documentation/devicetree/bindings/thermal/thermal-sensor.yaml for a description. - -Optional properties: -- resets: Reference to the reset controller controlling the thermal controller. -- nvmem-cells: A phandle to the calibration data provided by a nvmem device. If - unspecified default values shall be used. -- nvmem-cell-names: Should be "calibration-data" - -Example: - - thermal: thermal@1100b000 { - #thermal-sensor-cells = <1>; - compatible = "mediatek,mt8173-thermal"; - reg = <0 0x1100b000 0 0x1000>; - interrupts = <0 70 IRQ_TYPE_LEVEL_LOW>; - clocks = <&pericfg CLK_PERI_THERM>, <&pericfg CLK_PERI_AUXADC>; - clock-names = "therm", "auxadc"; - resets = <&pericfg MT8173_PERI_THERM_SW_RST>; - reset-names = "therm"; - mediatek,auxadc = <&auxadc>; - mediatek,apmixedsys = <&apmixedsys>; - nvmem-cells = <&thermal_calibration_data>; - nvmem-cell-names = "calibration-data"; - }; From 88071e31e994ee23356674e0c5461b25e2a95cdc Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Fri, 24 Nov 2023 17:57:44 +0800 Subject: [PATCH 38/64] dt-bindings: thermal: loongson,ls2k-thermal: Fix binding check issues Add the missing 'thermal-sensor-cells' property which is required for every thermal sensor as it's used when using phandles. And add the thermal-sensor.yaml reference. In fact, it was a careless mistake when submitting the driver that caused it to not work properly. So the fix is necessary, although it will result in the ABI break. Fixes: 72684d99a854 ("thermal: dt-bindings: add loongson-2 thermal") Cc: Yinbo Zhu Signed-off-by: Binbin Zhou Reviewed-by: Conor Dooley Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/6d69362632271ab0af9a5fbfa3bc46a0894f1d54.1700817227.git.zhoubinbin@loongson.cn --- .../bindings/thermal/loongson,ls2k-thermal.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml index 7538469997f9..b634f57cd011 100644 --- a/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml +++ b/Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml @@ -10,6 +10,9 @@ maintainers: - zhanghongchen - Yinbo Zhu +allOf: + - $ref: /schemas/thermal/thermal-sensor.yaml# + properties: compatible: oneOf: @@ -26,12 +29,16 @@ properties: interrupts: maxItems: 1 + '#thermal-sensor-cells': + const: 1 + required: - compatible - reg - interrupts + - '#thermal-sensor-cells' -additionalProperties: false +unevaluatedProperties: false examples: - | @@ -41,4 +48,5 @@ examples: reg = <0x1fe01500 0x30>; interrupt-parent = <&liointc0>; interrupts = <7 IRQ_TYPE_LEVEL_LOW>; + #thermal-sensor-cells = <1>; }; From 15ef92e9c41124ee9d88b01208364f3fe1f45f84 Mon Sep 17 00:00:00 2001 From: Binbin Zhou Date: Fri, 24 Nov 2023 17:57:45 +0800 Subject: [PATCH 39/64] drivers/thermal/loongson2_thermal: Fix incorrect PTR_ERR() judgment PTR_ERR() returns -ENODEV when thermal-zones are undefined, and we need -ENODEV as the right value for comparison. Otherwise, tz->type is NULL when thermal-zones is undefined, resulting in the following error: [ 12.290030] CPU 1 Unable to handle kernel paging request at virtual address fffffffffffffff1, era == 900000000355f410, ra == 90000000031579b8 [ 12.302877] Oops[#1]: [ 12.305190] CPU: 1 PID: 181 Comm: systemd-udevd Not tainted 6.6.0-rc7+ #5385 [ 12.312304] pc 900000000355f410 ra 90000000031579b8 tp 90000001069e8000 sp 90000001069eba10 [ 12.320739] a0 0000000000000000 a1 fffffffffffffff1 a2 0000000000000014 a3 0000000000000001 [ 12.329173] a4 90000001069eb990 a5 0000000000000001 a6 0000000000001001 a7 900000010003431c [ 12.337606] t0 fffffffffffffff1 t1 54567fd5da9b4fd4 t2 900000010614ec40 t3 00000000000dc901 [ 12.346041] t4 0000000000000000 t5 0000000000000004 t6 900000010614ee20 t7 900000000d00b790 [ 12.354472] t8 00000000000dc901 u0 54567fd5da9b4fd4 s9 900000000402ae10 s0 900000010614ec40 [ 12.362916] s1 90000000039fced0 s2 ffffffffffffffed s3 ffffffffffffffed s4 9000000003acc000 [ 12.362931] s5 0000000000000004 s6 fffffffffffff000 s7 0000000000000490 s8 90000001028b2ec8 [ 12.362938] ra: 90000000031579b8 thermal_add_hwmon_sysfs+0x258/0x300 [ 12.386411] ERA: 900000000355f410 strscpy+0xf0/0x160 [ 12.391626] CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE) [ 12.397898] PRMD: 00000004 (PPLV0 +PIE -PWE) [ 12.403678] EUEN: 00000000 (-FPE -SXE -ASXE -BTE) [ 12.409859] ECFG: 00071c1c (LIE=2-4,10-12 VS=7) [ 12.415882] ESTAT: 00010000 [PIL] (IS= ECode=1 EsubCode=0) [ 12.415907] BADV: fffffffffffffff1 [ 12.415911] PRID: 0014a000 (Loongson-64bit, Loongson-2K1000) [ 12.415917] Modules linked in: loongson2_thermal(+) vfat fat uio_pdrv_genirq uio fuse zram zsmalloc [ 12.415950] Process systemd-udevd (pid: 181, threadinfo=00000000358b9718, task=00000000ace72fe3) [ 12.415961] Stack : 0000000000000dc0 54567fd5da9b4fd4 900000000402ae10 9000000002df9358 [ 12.415982] ffffffffffffffed 0000000000000004 9000000107a10aa8 90000001002a3410 [ 12.415999] ffffffffffffffed ffffffffffffffed 9000000107a11268 9000000003157ab0 [ 12.416016] 9000000107a10aa8 ffffff80020fc0c8 90000001002a3410 ffffffffffffffed [ 12.416032] 0000000000000024 ffffff80020cc1e8 900000000402b2a0 9000000003acc000 [ 12.416048] 90000001002a3410 0000000000000000 ffffff80020f4030 90000001002a3410 [ 12.416065] 0000000000000000 9000000002df6808 90000001002a3410 0000000000000000 [ 12.416081] ffffff80020f4030 0000000000000000 90000001002a3410 9000000002df2ba8 [ 12.416097] 00000000000000b4 90000001002a34f4 90000001002a3410 0000000000000002 [ 12.416114] ffffff80020f4030 fffffffffffffff0 90000001002a3410 9000000002df2f30 [ 12.416131] ... [ 12.416138] Call Trace: [ 12.416142] [<900000000355f410>] strscpy+0xf0/0x160 [ 12.416167] [<90000000031579b8>] thermal_add_hwmon_sysfs+0x258/0x300 [ 12.416183] [<9000000003157ab0>] devm_thermal_add_hwmon_sysfs+0x50/0xe0 [ 12.416200] [] loongson2_thermal_probe+0x128/0x200 [loongson2_thermal] [ 12.416232] [<9000000002df6808>] platform_probe+0x68/0x140 [ 12.416249] [<9000000002df2ba8>] really_probe+0xc8/0x3c0 [ 12.416269] [<9000000002df2f30>] __driver_probe_device+0x90/0x180 [ 12.416286] [<9000000002df3058>] driver_probe_device+0x38/0x160 [ 12.416302] [<9000000002df33a8>] __driver_attach+0xa8/0x200 [ 12.416314] [<9000000002deffec>] bus_for_each_dev+0x8c/0x120 [ 12.416330] [<9000000002df198c>] bus_add_driver+0x10c/0x2a0 [ 12.416346] [<9000000002df46b4>] driver_register+0x74/0x160 [ 12.416358] [<90000000022201a4>] do_one_initcall+0x84/0x220 [ 12.416372] [<90000000022f3ab8>] do_init_module+0x58/0x2c0 [ 12.416386] [<90000000022f6538>] init_module_from_file+0x98/0x100 [ 12.416399] [<90000000022f67f0>] sys_finit_module+0x230/0x3c0 [ 12.416412] [<900000000358f7c8>] do_syscall+0x88/0xc0 [ 12.416431] [<900000000222137c>] handle_syscall+0xbc/0x158 Fixes: e7e3a7c35791 ("thermal/drivers/loongson-2: Add thermal management support") Cc: Yinbo Zhu Signed-off-by: Binbin Zhou Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/343c14de98216636a47b43e8bfd47b70d0a8e068.1700817227.git.zhoubinbin@loongson.cn --- drivers/thermal/loongson2_thermal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/loongson2_thermal.c b/drivers/thermal/loongson2_thermal.c index 133098dc0854..99ca0c7bc41c 100644 --- a/drivers/thermal/loongson2_thermal.c +++ b/drivers/thermal/loongson2_thermal.c @@ -127,7 +127,7 @@ static int loongson2_thermal_probe(struct platform_device *pdev) if (!IS_ERR(tzd)) break; - if (PTR_ERR(tzd) != ENODEV) + if (PTR_ERR(tzd) != -ENODEV) continue; return dev_err_probe(dev, PTR_ERR(tzd), "failed to register"); From 748b49c7dfe59d026c2e44cfb63cf66659aceb50 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Tue, 28 Nov 2023 09:44:48 +0100 Subject: [PATCH 40/64] dt-bindings: thermal: qcom-tsens: document the SM8650 Temperature Sensor Document the Temperature Sensor (TSENS) on the SM8650 Platform. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Neil Armstrong Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231128-topic-sm8650-upstream-bindings-tsens-v3-1-54179e6646d3@linaro.org --- Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml index 437b74732886..99d9c526c0b6 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml @@ -66,6 +66,7 @@ properties: - qcom,sm8350-tsens - qcom,sm8450-tsens - qcom,sm8550-tsens + - qcom,sm8650-tsens - const: qcom,tsens-v2 - description: v2 of TSENS with combined interrupt From 87f67d1747bc3ce8ace14be99b47d7731041ff03 Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Wed, 29 Nov 2023 09:43:27 -0300 Subject: [PATCH 41/64] dt-bindings: thermal-zones: Document critical-action Document the critical-action property to describe the thermal action the OS should perform after the critical temperature is reached. The possible values are "shutdown" and "reboot". The motivation for introducing the critical-action property is that different systems may need different thermal actions when the critical temperature is reached. For example, in a desktop PC, it is desired that a shutdown happens after the critical temperature is reached. However, in some embedded cases, such behavior does not suit well, as the board may be unattended in the field and rebooting may be a better approach. The bootloader may also benefit from this new property as it can check the SoC temperature and in case the temperature is above the critical point, it can trigger a shutdown or reboot accordingly. Signed-off-by: Fabio Estevam Reviewed-by: Krzysztof Kozlowski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231129124330.519423-1-festevam@gmail.com --- .../bindings/thermal/thermal-zones.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml index 4a8dabc48170..dbd52620d293 100644 --- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml +++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml @@ -75,6 +75,22 @@ patternProperties: framework and assumes that the thermal sensors in this zone support interrupts. + critical-action: + $ref: /schemas/types.yaml#/definitions/string + description: | + The action the OS should perform after the critical temperature is reached. + By default the system will shutdown as a safe action to prevent damage + to the hardware, if the property is not set. + The shutdown action should be always the default and preferred one. + Choose 'reboot' with care, as the hardware may be in thermal stress, + thus leading to infinite reboots that may cause damage to the hardware. + Make sure the firmware/bootloader will act as the last resort and take + over the thermal control. + + enum: + - shutdown + - reboot + thermal-sensors: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 From 5a0e241003b80247de59727c945bc94c848f893d Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Wed, 29 Nov 2023 09:43:28 -0300 Subject: [PATCH 42/64] thermal/core: Prepare for introduction of thermal reboot Add some helper functions to make it easier introducing the support for thermal reboot. No functional change. Signed-off-by: Fabio Estevam Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231129124330.519423-2-festevam@gmail.com --- drivers/thermal/thermal_core.c | 14 ++++++++++---- include/linux/reboot.h | 7 ++++++- kernel/reboot.c | 8 ++++---- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 5e5fcbd81dda..859f62e9d779 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -311,18 +311,24 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz, def_governor->throttle(tz, trip); } -void thermal_zone_device_critical(struct thermal_zone_device *tz) +static void thermal_zone_device_halt(struct thermal_zone_device *tz, bool shutdown) { /* * poweroff_delay_ms must be a carefully profiled positive value. * Its a must for forced_emergency_poweroff_work to be scheduled. */ int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS; + const char *msg = "Temperature too high"; - dev_emerg(&tz->device, "%s: critical temperature reached, " - "shutting down\n", tz->type); + dev_emerg(&tz->device, "%s: critical temperature reached\n", tz->type); - hw_protection_shutdown("Temperature too high", poweroff_delay_ms); + if (shutdown) + hw_protection_shutdown(msg, poweroff_delay_ms); +} + +void thermal_zone_device_critical(struct thermal_zone_device *tz) +{ + thermal_zone_device_halt(tz, true); } EXPORT_SYMBOL(thermal_zone_device_critical); diff --git a/include/linux/reboot.h b/include/linux/reboot.h index c4cc3b89ced1..4586c663884e 100644 --- a/include/linux/reboot.h +++ b/include/linux/reboot.h @@ -177,7 +177,12 @@ void ctrl_alt_del(void); extern void orderly_poweroff(bool force); extern void orderly_reboot(void); -void hw_protection_shutdown(const char *reason, int ms_until_forced); +void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shutdown); + +static inline void hw_protection_shutdown(const char *reason, int ms_until_forced) +{ + __hw_protection_shutdown(reason, ms_until_forced, true); +} /* * Emergency restart, callable from an interrupt handler. diff --git a/kernel/reboot.c b/kernel/reboot.c index 395a0ea3c7a8..b236c4c06bb3 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -957,7 +957,7 @@ static void hw_failure_emergency_poweroff(int poweroff_delay_ms) } /** - * hw_protection_shutdown - Trigger an emergency system poweroff + * __hw_protection_shutdown - Trigger an emergency system poweroff * * @reason: Reason of emergency shutdown to be printed. * @ms_until_forced: Time to wait for orderly shutdown before tiggering a @@ -971,7 +971,7 @@ static void hw_failure_emergency_poweroff(int poweroff_delay_ms) * if the previous request has given a large timeout for forced shutdown. * Can be called from any context. */ -void hw_protection_shutdown(const char *reason, int ms_until_forced) +void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shutdown) { static atomic_t allow_proceed = ATOMIC_INIT(1); @@ -986,9 +986,9 @@ void hw_protection_shutdown(const char *reason, int ms_until_forced) * orderly_poweroff failure */ hw_failure_emergency_poweroff(ms_until_forced); - orderly_poweroff(true); + if (shutdown) + orderly_poweroff(true); } -EXPORT_SYMBOL_GPL(hw_protection_shutdown); static int __init reboot_setup(char *str) { From 79fa723ba84c2b1b3124c72df8a3b07b851a5477 Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Wed, 29 Nov 2023 09:43:29 -0300 Subject: [PATCH 43/64] reboot: Introduce thermal_zone_device_critical_reboot() Introduce thermal_zone_device_critical_reboot() to trigger an emergency reboot. It is a counterpart of thermal_zone_device_critical() with the difference that it will force a reboot instead of shutdown. The motivation for doing this is to allow the thermal subystem to trigger a reboot when the temperature reaches the critical temperature. Signed-off-by: Fabio Estevam Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231129124330.519423-3-festevam@gmail.com --- drivers/thermal/thermal_core.c | 7 +++++++ drivers/thermal/thermal_core.h | 1 + include/linux/reboot.h | 5 +++++ kernel/reboot.c | 28 +++++++++++++++++----------- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 859f62e9d779..0d761afb7cbc 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -324,6 +324,8 @@ static void thermal_zone_device_halt(struct thermal_zone_device *tz, bool shutdo if (shutdown) hw_protection_shutdown(msg, poweroff_delay_ms); + else + hw_protection_reboot(msg, poweroff_delay_ms); } void thermal_zone_device_critical(struct thermal_zone_device *tz) @@ -332,6 +334,11 @@ void thermal_zone_device_critical(struct thermal_zone_device *tz) } EXPORT_SYMBOL(thermal_zone_device_critical); +void thermal_zone_device_critical_reboot(struct thermal_zone_device *tz) +{ + thermal_zone_device_halt(tz, false); +} + static void handle_critical_trips(struct thermal_zone_device *tz, const struct thermal_trip *trip) { diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index fe2917a74054..b5e6743bd157 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -114,6 +114,7 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *, char *); int thermal_build_list_of_policies(char *buf); void __thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event); +void thermal_zone_device_critical_reboot(struct thermal_zone_device *tz); /* Helpers */ #define for_each_trip(__tz, __trip) \ diff --git a/include/linux/reboot.h b/include/linux/reboot.h index 4586c663884e..abcdde4df697 100644 --- a/include/linux/reboot.h +++ b/include/linux/reboot.h @@ -179,6 +179,11 @@ extern void orderly_poweroff(bool force); extern void orderly_reboot(void); void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shutdown); +static inline void hw_protection_reboot(const char *reason, int ms_until_forced) +{ + __hw_protection_shutdown(reason, ms_until_forced, false); +} + static inline void hw_protection_shutdown(const char *reason, int ms_until_forced) { __hw_protection_shutdown(reason, ms_until_forced, true); diff --git a/kernel/reboot.c b/kernel/reboot.c index b236c4c06bb3..35d5e0b67993 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -957,19 +957,22 @@ static void hw_failure_emergency_poweroff(int poweroff_delay_ms) } /** - * __hw_protection_shutdown - Trigger an emergency system poweroff + * __hw_protection_shutdown - Trigger an emergency system shutdown or reboot * - * @reason: Reason of emergency shutdown to be printed. - * @ms_until_forced: Time to wait for orderly shutdown before tiggering a - * forced shudown. Negative value disables the forced - * shutdown. + * @reason: Reason of emergency shutdown or reboot to be printed. + * @ms_until_forced: Time to wait for orderly shutdown or reboot before + * triggering it. Negative value disables the forced + * shutdown or reboot. + * @shutdown: If true, indicates that a shutdown will happen + * after the critical tempeature is reached. + * If false, indicates that a reboot will happen + * after the critical tempeature is reached. * - * Initiate an emergency system shutdown in order to protect hardware from - * further damage. Usage examples include a thermal protection or a voltage or - * current regulator failures. - * NOTE: The request is ignored if protection shutdown is already pending even - * if the previous request has given a large timeout for forced shutdown. - * Can be called from any context. + * Initiate an emergency system shutdown or reboot in order to protect + * hardware from further damage. Usage examples include a thermal protection. + * NOTE: The request is ignored if protection shutdown or reboot is already + * pending even if the previous request has given a large timeout for forced + * shutdown/reboot. */ void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shutdown) { @@ -988,7 +991,10 @@ void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shut hw_failure_emergency_poweroff(ms_until_forced); if (shutdown) orderly_poweroff(true); + else + orderly_reboot(); } +EXPORT_SYMBOL_GPL(__hw_protection_shutdown); static int __init reboot_setup(char *str) { From 62e79e38b257a59f1e3d8aff801ae8590e2e45b4 Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Wed, 29 Nov 2023 09:43:30 -0300 Subject: [PATCH 44/64] thermal/thermal_of: Allow rebooting after critical temp Currently, the default mechanism is to trigger a shutdown after the critical temperature is reached. In some embedded cases, such behavior does not suit well, as the board may be unattended in the field and rebooting may be a better approach. The bootloader may also check the temperature and only allow the boot to proceed when the temperature is below a certain threshold. Introduce support for allowing a reboot to be triggered after the critical temperature is reached. If the "critical-action" devicetree property is not found, fall back to the shutdown action to preserve the existing default behavior. If a custom ops->critical exists, then it takes preference over critical-actions. Tested on a i.MX8MM board with the following devicetree changes: thermal-zones { cpu-thermal { critical-action = "reboot"; }; }; Signed-off-by: Fabio Estevam Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231129124330.519423-4-festevam@gmail.com --- drivers/thermal/thermal_of.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 1e0655b63259..4d6c22e0ed85 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -475,6 +475,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * struct thermal_zone_params tzp = {}; struct thermal_zone_device_ops *of_ops; struct device_node *np; + const char *action; int delay, pdelay; int ntrips, mask; int ret; @@ -511,6 +512,11 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * 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, pdelay, delay); From 720f8db834a31009d8d11893278ca3f8072a575e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 16 Nov 2023 12:26:35 +0100 Subject: [PATCH 45/64] thermal: amlogic: Make amlogic_thermal_disable() return void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit amlogic_thermal_disable() returned zero unconditionally and amlogic_thermal_remove() already ignores the return value. Make it return no value and modify amlogic_thermal_suspend to not check the value. This patch introduces no semantic changes, but makes it more obvious for a human reader that amlogic_thermal_suspend() cannot fail. Signed-off-by: Uwe Kleine-König Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231116112633.668826-2-u.kleine-koenig@pengutronix.de --- drivers/thermal/amlogic_thermal.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c index 5877cde25b79..562f63b7bf27 100644 --- a/drivers/thermal/amlogic_thermal.c +++ b/drivers/thermal/amlogic_thermal.c @@ -167,13 +167,11 @@ static int amlogic_thermal_enable(struct amlogic_thermal *data) return 0; } -static int amlogic_thermal_disable(struct amlogic_thermal *data) +static void amlogic_thermal_disable(struct amlogic_thermal *data) { regmap_update_bits(data->regmap, TSENSOR_CFG_REG1, TSENSOR_CFG_REG1_ENABLE, 0); clk_disable_unprepare(data->clk); - - return 0; } static int amlogic_thermal_get_temp(struct thermal_zone_device *tz, int *temp) @@ -302,7 +300,9 @@ static int __maybe_unused amlogic_thermal_suspend(struct device *dev) { struct amlogic_thermal *data = dev_get_drvdata(dev); - return amlogic_thermal_disable(data); + amlogic_thermal_disable(data); + + return 0; } static int __maybe_unused amlogic_thermal_resume(struct device *dev) From ac99b129630efa14efe176e968045cde9d442e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Thu, 16 Nov 2023 12:26:36 +0100 Subject: [PATCH 46/64] thermal: amlogic: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This macro has the advantage over SIMPLE_DEV_PM_OPS that we don't have to care about when the functions are actually used, so the corresponding __maybe_unused can be dropped. Also make use of pm_ptr() to discard all PM related stuff if CONFIG_PM isn't enabled. Signed-off-by: Uwe Kleine-König Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231116112633.668826-3-u.kleine-koenig@pengutronix.de --- drivers/thermal/amlogic_thermal.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/thermal/amlogic_thermal.c b/drivers/thermal/amlogic_thermal.c index 562f63b7bf27..df7a5ed55385 100644 --- a/drivers/thermal/amlogic_thermal.c +++ b/drivers/thermal/amlogic_thermal.c @@ -296,7 +296,7 @@ static void amlogic_thermal_remove(struct platform_device *pdev) amlogic_thermal_disable(data); } -static int __maybe_unused amlogic_thermal_suspend(struct device *dev) +static int amlogic_thermal_suspend(struct device *dev) { struct amlogic_thermal *data = dev_get_drvdata(dev); @@ -305,20 +305,21 @@ static int __maybe_unused amlogic_thermal_suspend(struct device *dev) return 0; } -static int __maybe_unused amlogic_thermal_resume(struct device *dev) +static int amlogic_thermal_resume(struct device *dev) { struct amlogic_thermal *data = dev_get_drvdata(dev); return amlogic_thermal_enable(data); } -static SIMPLE_DEV_PM_OPS(amlogic_thermal_pm_ops, - amlogic_thermal_suspend, amlogic_thermal_resume); +static DEFINE_SIMPLE_DEV_PM_OPS(amlogic_thermal_pm_ops, + amlogic_thermal_suspend, + amlogic_thermal_resume); static struct platform_driver amlogic_thermal_driver = { .driver = { .name = "amlogic_thermal", - .pm = &amlogic_thermal_pm_ops, + .pm = pm_ptr(&amlogic_thermal_pm_ops), .of_match_table = of_amlogic_thermal_match, }, .probe = amlogic_thermal_probe, From 20bf6262d518c5d3bf38c805be1bbda36acb9506 Mon Sep 17 00:00:00 2001 From: Maxim Kiselev Date: Mon, 18 Dec 2023 00:06:22 +0300 Subject: [PATCH 47/64] dt-bindings: thermal: sun8i: Add binding for D1/T113s THS controller Add a binding for D1/T113s thermal sensor controller. Signed-off-by: Maxim Kiselev Reviewed-by: Conor Dooley Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231217210629.131486-2-bigunclemax@gmail.com --- .../bindings/thermal/allwinner,sun8i-a83t-ths.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml index fbd4212285e2..9b2272a9ec15 100644 --- a/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml +++ b/Documentation/devicetree/bindings/thermal/allwinner,sun8i-a83t-ths.yaml @@ -16,6 +16,7 @@ properties: - allwinner,sun8i-a83t-ths - allwinner,sun8i-h3-ths - allwinner,sun8i-r40-ths + - allwinner,sun20i-d1-ths - allwinner,sun50i-a64-ths - allwinner,sun50i-a100-ths - allwinner,sun50i-h5-ths @@ -61,6 +62,7 @@ allOf: compatible: contains: enum: + - allwinner,sun20i-d1-ths - allwinner,sun50i-a100-ths - allwinner,sun50i-h6-ths @@ -84,7 +86,9 @@ allOf: properties: compatible: contains: - const: allwinner,sun8i-h3-ths + enum: + - allwinner,sun8i-h3-ths + - allwinner,sun20i-d1-ths then: properties: @@ -103,6 +107,7 @@ allOf: enum: - allwinner,sun8i-h3-ths - allwinner,sun8i-r40-ths + - allwinner,sun20i-d1-ths - allwinner,sun50i-a64-ths - allwinner,sun50i-a100-ths - allwinner,sun50i-h5-ths From ebbf19e36d021f253425344b4d4b987f3b7d9be5 Mon Sep 17 00:00:00 2001 From: Maxim Kiselev Date: Mon, 18 Dec 2023 00:06:23 +0300 Subject: [PATCH 48/64] thermal/drivers/sun8i: Add D1/T113s THS controller support This patch adds a thermal sensor controller support for the D1/T113s, which is similar to the one on H6, but with only one sensor and different scale and offset values. Signed-off-by: Maxim Kiselev Acked-by: Jernej Skrabec Reviewed-by: Andre Przywara Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231217210629.131486-3-bigunclemax@gmail.com --- drivers/thermal/sun8i_thermal.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c index f989b55a8aa8..6a8e386dbc8d 100644 --- a/drivers/thermal/sun8i_thermal.c +++ b/drivers/thermal/sun8i_thermal.c @@ -606,6 +606,18 @@ static const struct ths_thermal_chip sun50i_h6_ths = { .calc_temp = sun8i_ths_calc_temp, }; +static const struct ths_thermal_chip sun20i_d1_ths = { + .sensor_num = 1, + .has_bus_clk_reset = true, + .offset = 188552, + .scale = 673, + .temp_data_base = SUN50I_H6_THS_TEMP_DATA, + .calibrate = sun50i_h6_ths_calibrate, + .init = sun50i_h6_thermal_init, + .irq_ack = sun50i_h6_irq_ack, + .calc_temp = sun8i_ths_calc_temp, +}; + static const struct of_device_id of_ths_match[] = { { .compatible = "allwinner,sun8i-a83t-ths", .data = &sun8i_a83t_ths }, { .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths }, @@ -614,6 +626,7 @@ static const struct of_device_id of_ths_match[] = { { .compatible = "allwinner,sun50i-a100-ths", .data = &sun50i_a100_ths }, { .compatible = "allwinner,sun50i-h5-ths", .data = &sun50i_h5_ths }, { .compatible = "allwinner,sun50i-h6-ths", .data = &sun50i_h6_ths }, + { .compatible = "allwinner,sun20i-d1-ths", .data = &sun20i_d1_ths }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, of_ths_match); From 7ec597ba25a3d942e13870cc27848e301c26c561 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Thu, 30 Nov 2023 18:41:13 +0100 Subject: [PATCH 49/64] dt-bindings: thermal: qcom-spmi-adc-tm5/hc: Fix example node names The ADC Thermal Monitor is part of an SPMI PMIC, which in turn sits on an SPMI bus. Fixes: db03874b8543 ("dt-bindings: thermal: qcom: add HC variant of adc-thermal monitor bindings") Fixes: e8ffd6c0756b ("dt-bindings: thermal: qcom: add adc-thermal monitor bindings") Signed-off-by: Johan Hovold Acked-by: Krzysztof Kozlowski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231130174114.13122-2-johan+linaro@kernel.org --- .../devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml | 3 ++- .../devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml index 01253d58bf9f..82f8f25885c0 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml @@ -114,7 +114,8 @@ examples: - | #include #include - spmi_bus { + + pmic { #address-cells = <1>; #size-cells = <0>; pm8998_adc: adc@3100 { diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml index 3c81def03c84..02347cee6c6f 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml @@ -167,7 +167,8 @@ examples: - | #include #include - spmi_bus { + + pmic { #address-cells = <1>; #size-cells = <0>; pm8150b_adc: adc@3100 { @@ -207,7 +208,8 @@ examples: #include #include #include - spmi_bus { + + pmic { #address-cells = <1>; #size-cells = <0>; pmk8350_vadc: adc@3100 { From 4bddb0cdfad9148a08b2bda7c3d479b1da715929 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Thu, 30 Nov 2023 18:41:14 +0100 Subject: [PATCH 50/64] dt-bindings: thermal: qcom-spmi-adc-tm5/hc: Clean up examples Clean up the examples by adding newline separators, moving 'reg' properties after 'compatible' and dropping unused labels. Signed-off-by: Johan Hovold Acked-by: Krzysztof Kozlowski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231130174114.13122-3-johan+linaro@kernel.org --- .../bindings/thermal/qcom-spmi-adc-tm-hc.yaml | 5 +++-- .../devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml index 82f8f25885c0..7541e27704ca 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm-hc.yaml @@ -118,9 +118,10 @@ examples: pmic { #address-cells = <1>; #size-cells = <0>; + pm8998_adc: adc@3100 { - reg = <0x3100>; compatible = "qcom,spmi-adc-rev2"; + reg = <0x3100>; #address-cells = <1>; #size-cells = <0>; #io-channel-cells = <1>; @@ -131,7 +132,7 @@ examples: }; }; - pm8998_adc_tm: adc-tm@3400 { + adc-tm@3400 { compatible = "qcom,spmi-adc-tm-hc"; reg = <0x3400>; interrupts = <0x2 0x34 0x0 IRQ_TYPE_EDGE_RISING>; diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml index 02347cee6c6f..d9d2657287cb 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml @@ -171,9 +171,10 @@ examples: pmic { #address-cells = <1>; #size-cells = <0>; + pm8150b_adc: adc@3100 { - reg = <0x3100>; compatible = "qcom,spmi-adc5"; + reg = <0x3100>; #address-cells = <1>; #size-cells = <0>; #io-channel-cells = <1>; @@ -187,7 +188,7 @@ examples: }; }; - pm8150b_adc_tm: adc-tm@3500 { + adc-tm@3500 { compatible = "qcom,spmi-adc-tm5"; reg = <0x3500>; interrupts = <0x2 0x35 0x0 IRQ_TYPE_EDGE_RISING>; @@ -212,9 +213,10 @@ examples: pmic { #address-cells = <1>; #size-cells = <0>; + pmk8350_vadc: adc@3100 { - reg = <0x3100>; compatible = "qcom,spmi-adc7"; + reg = <0x3100>; #address-cells = <1>; #size-cells = <0>; #io-channel-cells = <1>; @@ -235,7 +237,7 @@ examples: }; }; - pmk8350_adc_tm: adc-tm@3400 { + adc-tm@3400 { compatible = "qcom,spmi-adc-tm5-gen2"; reg = <0x3400>; interrupts = <0x0 0x34 0x0 IRQ_TYPE_EDGE_RISING>; From 9da39ef332c417ce52732564c1c682a6e1209302 Mon Sep 17 00:00:00 2001 From: Florian Eckert Date: Mon, 4 Dec 2023 15:13:35 +0100 Subject: [PATCH 51/64] tools/thermal/tmon: Fix compilation warning for wrong format The following warnings are shown during compilation: tui.c: In function 'show_cooling_device': tui.c:216:40: warning: format '%d' expects argument of type 'int', but argument 7 has type 'long unsigned int' [-Wformat=] 216 | "%02d %12.12s%6d %6d", | ~~^ | | | int | %6ld ...... 219 | ptdata.cdi[j].cur_state, | ~~~~~~~~~~~~~~~~~~~~~~~ | | | long unsigned int tui.c:216:44: warning: format '%d' expects argument of type 'int', but argument 8 has type 'long unsigned int' [-Wformat=] 216 | "%02d %12.12s%6d %6d", | ~~^ | | | int | %6ld ...... 220 | ptdata.cdi[j].max_state); | ~~~~~~~~~~~~~~~~~~~~~~~ | | | long unsigned int To fix this, the correct string format must be used for printing. Signed-off-by: Florian Eckert Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231204141335.2798194-1-fe@dev.tdt.de --- tools/thermal/tmon/tui.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/thermal/tmon/tui.c b/tools/thermal/tmon/tui.c index 031b258667d8..7f5dd2b87f15 100644 --- a/tools/thermal/tmon/tui.c +++ b/tools/thermal/tmon/tui.c @@ -213,7 +213,7 @@ void show_cooling_device(void) * cooling device instances. skip unused idr. */ mvwprintw(cooling_device_window, j + 2, 1, - "%02d %12.12s%6d %6d", + "%02d %12.12s%6lu %6lu", ptdata.cdi[j].instance, ptdata.cdi[j].type, ptdata.cdi[j].cur_state, From 0cefaf6c89c016e9eae9f8881ecaf50e836869a9 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:17 +0100 Subject: [PATCH 52/64] thermal/drivers/exynos: Remove an unnecessary field description It seems that the field has been removed in one of the previous commits, but the description has been forgotten. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-2-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 123ec81e1943..187086658e8f 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -160,7 +160,6 @@ enum soc_type { * in the positive-TC generator block * 0 < reference_voltage <= 31 * @regulator: pointer to the TMU regulator structure. - * @reg_conf: pointer to structure to register with core thermal. * @tzd: pointer to thermal_zone_device structure * @ntrip: number of supported trip points. * @enabled: current status of TMU device From 0ac3e1cf37367aea1fef26569b793b5e57eb7a51 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:18 +0100 Subject: [PATCH 53/64] thermal/drivers/exynos: Drop id field We do not use the value, and only Exynos 7 defines this alias anyway. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-3-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 187086658e8f..4ff32245d2a9 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -138,7 +138,6 @@ enum soc_type { /** * struct exynos_tmu_data : A structure to hold the private data of the TMU * driver - * @id: identifier of the one instance of the TMU controller. * @base: base address of the single instance of the TMU controller. * @base_second: base address of the common registers of the TMU controller. * @irq: irq number of the TMU controller. @@ -172,7 +171,6 @@ enum soc_type { * @tmu_clear_irqs: SoC specific TMU interrupts clearing method */ struct exynos_tmu_data { - int id; void __iomem *base; void __iomem *base_second; int irq; @@ -865,10 +863,6 @@ static int exynos_map_dt_data(struct platform_device *pdev) if (!data || !pdev->dev.of_node) return -ENODEV; - data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl"); - if (data->id < 0) - data->id = 0; - data->irq = irq_of_parse_and_map(pdev->dev.of_node, 0); if (data->irq <= 0) { dev_err(&pdev->dev, "failed to get IRQ\n"); From 20009a8137eefbeebb86402d04cae8955795385a Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:19 +0100 Subject: [PATCH 54/64] thermal/drivers/exynos: Wwitch from workqueue-driven interrupt handling to threaded interrupts The workqueue boilerplate is mostly one-to-one what the threaded interrupts do. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-4-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 29 +++++++++------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 4ff32245d2a9..c144592d4584 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -142,7 +142,6 @@ enum soc_type { * @base_second: base address of the common registers of the TMU controller. * @irq: irq number of the TMU controller. * @soc: id of the SOC type. - * @irq_work: pointer to the irq work structure. * @lock: lock to implement synchronization. * @clk: pointer to the clock structure. * @clk_sec: pointer to the clock structure for accessing the base_second. @@ -175,7 +174,6 @@ struct exynos_tmu_data { void __iomem *base_second; int irq; enum soc_type soc; - struct work_struct irq_work; struct mutex lock; struct clk *clk, *clk_sec, *sclk; u32 cal_type; @@ -763,10 +761,9 @@ static int exynos7_tmu_read(struct exynos_tmu_data *data) EXYNOS7_TMU_TEMP_MASK; } -static void exynos_tmu_work(struct work_struct *work) +static irqreturn_t exynos_tmu_threaded_irq(int irq, void *id) { - struct exynos_tmu_data *data = container_of(work, - struct exynos_tmu_data, irq_work); + struct exynos_tmu_data *data = id; thermal_zone_device_update(data->tzd, THERMAL_EVENT_UNSPECIFIED); @@ -778,7 +775,8 @@ static void exynos_tmu_work(struct work_struct *work) clk_disable(data->clk); mutex_unlock(&data->lock); - enable_irq(data->irq); + + return IRQ_HANDLED; } static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data) @@ -812,16 +810,6 @@ static void exynos4210_tmu_clear_irqs(struct exynos_tmu_data *data) writel(val_irq, data->base + tmu_intclear); } -static irqreturn_t exynos_tmu_irq(int irq, void *id) -{ - struct exynos_tmu_data *data = id; - - disable_irq_nosync(irq); - schedule_work(&data->irq_work); - - return IRQ_HANDLED; -} - static const struct of_device_id exynos_tmu_match[] = { { .compatible = "samsung,exynos3250-tmu", @@ -1023,8 +1011,6 @@ static int exynos_tmu_probe(struct platform_device *pdev) if (ret) goto err_sensor; - INIT_WORK(&data->irq_work, exynos_tmu_work); - data->clk = devm_clk_get(&pdev->dev, "tmu_apbif"); if (IS_ERR(data->clk)) { dev_err(&pdev->dev, "Failed to get clock\n"); @@ -1093,8 +1079,11 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_sclk; } - ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq, - IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data); + ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL, + exynos_tmu_threaded_irq, + IRQF_TRIGGER_RISING + | IRQF_SHARED | IRQF_ONESHOT, + dev_name(&pdev->dev), data); if (ret) { dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq); goto err_sclk; From 52ef6f567e6b9a878a8e0e5d6367aa65a08227a5 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:20 +0100 Subject: [PATCH 55/64] thermal/drivers/exynos: Handle devm_regulator_get_optional return value correctly Currently, if regulator is required in the SoC, but devm_regulator_get_optional fails for whatever reason, the execution will proceed without propagating the error. Meanwhile there is no reason to output the error in case of -ENODEV. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-5-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index c144592d4584..8bcad8a70dc5 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -1002,9 +1002,17 @@ static int exynos_tmu_probe(struct platform_device *pdev) return ret; } } else { - if (PTR_ERR(data->regulator) == -EPROBE_DEFER) + ret = PTR_ERR(data->regulator); + switch (ret) { + case -ENODEV: + break; + case -EPROBE_DEFER: return -EPROBE_DEFER; - dev_info(&pdev->dev, "Regulator node (vtmu) not found\n"); + default: + dev_err(&pdev->dev, "Failed to get regulator: %d\n", + ret); + return ret; + } } ret = exynos_map_dt_data(pdev); From 5d6976d01414f23af4b81d7f91cfd59839c8b1fe Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:21 +0100 Subject: [PATCH 56/64] thermal/drivers/exynos: Simplify regulator (de)initialization We rewrite the initialization to enable the regulator as part of devm, which allows us to not handle the struct instance manually. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-6-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 49 +++++++++------------------- 1 file changed, 15 insertions(+), 34 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 8bcad8a70dc5..3bdcbab7466f 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -157,7 +157,6 @@ enum soc_type { * @reference_voltage: reference voltage of amplifier * in the positive-TC generator block * 0 < reference_voltage <= 31 - * @regulator: pointer to the TMU regulator structure. * @tzd: pointer to thermal_zone_device structure * @ntrip: number of supported trip points. * @enabled: current status of TMU device @@ -183,7 +182,6 @@ struct exynos_tmu_data { u16 temp_error1, temp_error2; u8 gain; u8 reference_voltage; - struct regulator *regulator; struct thermal_zone_device *tzd; unsigned int ntrip; bool enabled; @@ -994,50 +992,40 @@ static int exynos_tmu_probe(struct platform_device *pdev) * TODO: Add regulator as an SOC feature, so that regulator enable * is a compulsory call. */ - data->regulator = devm_regulator_get_optional(&pdev->dev, "vtmu"); - if (!IS_ERR(data->regulator)) { - ret = regulator_enable(data->regulator); - if (ret) { - dev_err(&pdev->dev, "failed to enable vtmu\n"); - return ret; - } - } else { - ret = PTR_ERR(data->regulator); - switch (ret) { - case -ENODEV: - break; - case -EPROBE_DEFER: - return -EPROBE_DEFER; - default: - dev_err(&pdev->dev, "Failed to get regulator: %d\n", - ret); - return ret; - } + ret = devm_regulator_get_enable_optional(&pdev->dev, "vtmu"); + switch (ret) { + case 0: + case -ENODEV: + break; + case -EPROBE_DEFER: + return -EPROBE_DEFER; + default: + dev_err(&pdev->dev, "Failed to get enabled regulator: %d\n", + ret); + return ret; } ret = exynos_map_dt_data(pdev); if (ret) - goto err_sensor; + return ret; data->clk = devm_clk_get(&pdev->dev, "tmu_apbif"); if (IS_ERR(data->clk)) { dev_err(&pdev->dev, "Failed to get clock\n"); - ret = PTR_ERR(data->clk); - goto err_sensor; + return PTR_ERR(data->clk); } data->clk_sec = devm_clk_get(&pdev->dev, "tmu_triminfo_apbif"); if (IS_ERR(data->clk_sec)) { if (data->soc == SOC_ARCH_EXYNOS5420_TRIMINFO) { dev_err(&pdev->dev, "Failed to get triminfo clock\n"); - ret = PTR_ERR(data->clk_sec); - goto err_sensor; + return PTR_ERR(data->clk_sec); } } else { ret = clk_prepare(data->clk_sec); if (ret) { dev_err(&pdev->dev, "Failed to get clock\n"); - goto err_sensor; + return ret; } } @@ -1107,10 +1095,6 @@ err_clk: err_clk_sec: if (!IS_ERR(data->clk_sec)) clk_unprepare(data->clk_sec); -err_sensor: - if (!IS_ERR(data->regulator)) - regulator_disable(data->regulator); - return ret; } @@ -1124,9 +1108,6 @@ static void exynos_tmu_remove(struct platform_device *pdev) clk_unprepare(data->clk); if (!IS_ERR(data->clk_sec)) clk_unprepare(data->clk_sec); - - if (!IS_ERR(data->regulator)) - regulator_disable(data->regulator); } #ifdef CONFIG_PM_SLEEP From d7a5b431911c5d9da7fbff852433e6f99a4c6616 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:22 +0100 Subject: [PATCH 57/64] thermal/drivers/exynos: Stop using the threshold mechanism on Exynos 4210 Exynos 4210 supports setting a base threshold value, which is added to all trip points. This might be useful, but is not really necessary in our usecase, so we always set it to 0 to simplify the code a bit. Additionally, this change makes it so that we convert the value to the calibrated one in a slightly different place. This is more correct morally, though it does not make any change when single-point calibration is being used (which is the case currently). Reviewed-by: Krzysztof Kozlowski Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-7-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 3bdcbab7466f..d918bf6d5359 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -343,20 +343,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on) static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data, int trip_id, u8 temp) { - struct thermal_trip trip; - u8 ref, th_code; - - if (thermal_zone_get_trip(data->tzd, 0, &trip)) - return; - - ref = trip.temperature / MCELSIUS; - - if (trip_id == 0) { - th_code = temp_to_code(data, ref); - writeb(th_code, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP); - } - - temp -= ref; + temp = temp_to_code(data, temp); writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + trip_id * 4); } @@ -371,6 +358,8 @@ static void exynos4210_tmu_initialize(struct platform_device *pdev) struct exynos_tmu_data *data = platform_get_drvdata(pdev); sanitize_temp_error(data, readl(data->base + EXYNOS_TMU_REG_TRIMINFO)); + + writeb(0, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP); } static void exynos4412_tmu_set_trip_temp(struct exynos_tmu_data *data, From b72ba67baec1d8ff67edc6e70c371ab6b2f7d31c Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:23 +0100 Subject: [PATCH 58/64] thermal/drivers/exynos: Split initialization of TMU and the thermal zone This will be needed in the future, as the thermal zone subsystem might call our callbacks right after devm_thermal_of_zone_register. Currently we just make get_temp return EAGAIN in such case, but this will not be possible with state-modifying callbacks, for instance set_trips. Reviewed-by: Krzysztof Kozlowski Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-8-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 104 +++++++++++++++------------ 1 file changed, 60 insertions(+), 44 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index d918bf6d5359..970bada90f2f 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -251,25 +251,8 @@ static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info) static int exynos_tmu_initialize(struct platform_device *pdev) { struct exynos_tmu_data *data = platform_get_drvdata(pdev); - struct thermal_zone_device *tzd = data->tzd; - int num_trips = thermal_zone_get_num_trips(tzd); unsigned int status; - int ret = 0, temp; - - ret = thermal_zone_get_crit_temp(tzd, &temp); - if (ret && data->soc != SOC_ARCH_EXYNOS5433) { /* FIXME */ - dev_err(&pdev->dev, - "No CRITICAL trip point defined in device tree!\n"); - goto out; - } - - if (num_trips > data->ntrip) { - dev_info(&pdev->dev, - "More trip points than supported by this TMU.\n"); - dev_info(&pdev->dev, - "%d trip points should be configured in polling mode.\n", - num_trips - data->ntrip); - } + int ret = 0; mutex_lock(&data->lock); clk_enable(data->clk); @@ -280,32 +263,63 @@ static int exynos_tmu_initialize(struct platform_device *pdev) if (!status) { ret = -EBUSY; } else { - int i, ntrips = - min_t(int, num_trips, data->ntrip); - data->tmu_initialize(pdev); - - /* Write temperature code for rising and falling threshold */ - for (i = 0; i < ntrips; i++) { - - struct thermal_trip trip; - - ret = thermal_zone_get_trip(tzd, i, &trip); - if (ret) - goto err; - - data->tmu_set_trip_temp(data, i, trip.temperature / MCELSIUS); - data->tmu_set_trip_hyst(data, i, trip.temperature / MCELSIUS, - trip.hysteresis / MCELSIUS); - } - data->tmu_clear_irqs(data); } + + if (!IS_ERR(data->clk_sec)) + clk_disable(data->clk_sec); + clk_disable(data->clk); + mutex_unlock(&data->lock); + + return ret; +} + +static int exynos_thermal_zone_configure(struct platform_device *pdev) +{ + struct exynos_tmu_data *data = platform_get_drvdata(pdev); + struct thermal_zone_device *tzd = data->tzd; + int i, num_trips = thermal_zone_get_num_trips(tzd); + int ret = 0, temp; + + ret = thermal_zone_get_crit_temp(tzd, &temp); + + if (ret && data->soc != SOC_ARCH_EXYNOS5433) { /* FIXME */ + dev_err(&pdev->dev, + "No CRITICAL trip point defined in device tree!\n"); + goto out; + } + + mutex_lock(&data->lock); + + if (num_trips > data->ntrip) { + dev_info(&pdev->dev, + "More trip points than supported by this TMU.\n"); + dev_info(&pdev->dev, + "%d trip points should be configured in polling mode.\n", + num_trips - data->ntrip); + } + + clk_enable(data->clk); + + num_trips = min_t(int, num_trips, data->ntrip); + + /* Write temperature code for rising and falling threshold */ + for (i = 0; i < num_trips; i++) { + struct thermal_trip trip; + + ret = thermal_zone_get_trip(tzd, i, &trip); + if (ret) + goto err; + + data->tmu_set_trip_temp(data, i, trip.temperature / MCELSIUS); + data->tmu_set_trip_hyst(data, i, trip.temperature / MCELSIUS, + trip.hysteresis / MCELSIUS); + } + err: clk_disable(data->clk); mutex_unlock(&data->lock); - if (!IS_ERR(data->clk_sec)) - clk_disable(data->clk_sec); out: return ret; } @@ -1044,10 +1058,12 @@ static int exynos_tmu_probe(struct platform_device *pdev) break; } - /* - * data->tzd must be registered before calling exynos_tmu_initialize(), - * requesting irq and calling exynos_tmu_control(). - */ + ret = exynos_tmu_initialize(pdev); + if (ret) { + dev_err(&pdev->dev, "Failed to initialize TMU\n"); + goto err_sclk; + } + data->tzd = devm_thermal_of_zone_register(&pdev->dev, 0, data, &exynos_sensor_ops); if (IS_ERR(data->tzd)) { @@ -1058,9 +1074,9 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_sclk; } - ret = exynos_tmu_initialize(pdev); + ret = exynos_thermal_zone_configure(pdev); if (ret) { - dev_err(&pdev->dev, "Failed to initialize TMU\n"); + dev_err(&pdev->dev, "Failed to configure the thermal zone\n"); goto err_sclk; } From af00d488339aee7bf42b07057053ef919bedee6f Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:24 +0100 Subject: [PATCH 59/64] thermal/drivers/exynos: Use BIT wherever possible The original driver did not use that macro and it allows us to make our intentions slightly clearer. Reviewed-by: Lukasz Luba Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-9-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 970bada90f2f..ca1b1cec0300 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -388,7 +388,7 @@ static void exynos4412_tmu_set_trip_temp(struct exynos_tmu_data *data, if (trip == 3) { con = readl(data->base + EXYNOS_TMU_REG_CONTROL); - con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT); + con |= BIT(EXYNOS_TMU_THERM_TRIP_EN_SHIFT); writel(con, data->base + EXYNOS_TMU_REG_CONTROL); } } @@ -559,16 +559,16 @@ static void exynos4210_tmu_control(struct platform_device *pdev, bool on) continue; interrupt_en |= - (1 << (EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4)); + BIT(EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4); } if (data->soc != SOC_ARCH_EXYNOS4210) interrupt_en |= interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT; - con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT); + con |= BIT(EXYNOS_TMU_CORE_EN_SHIFT); } else { - con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT); + con &= ~BIT(EXYNOS_TMU_CORE_EN_SHIFT); } writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN); @@ -590,15 +590,15 @@ static void exynos5433_tmu_control(struct platform_device *pdev, bool on) continue; interrupt_en |= - (1 << (EXYNOS7_TMU_INTEN_RISE0_SHIFT + i)); + BIT(EXYNOS7_TMU_INTEN_RISE0_SHIFT + i); } interrupt_en |= interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT; - con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT); + con |= BIT(EXYNOS_TMU_CORE_EN_SHIFT); } else - con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT); + con &= ~BIT(EXYNOS_TMU_CORE_EN_SHIFT); pd_det_en = on ? EXYNOS5433_PD_DET_EN : 0; @@ -622,17 +622,17 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on) continue; interrupt_en |= - (1 << (EXYNOS7_TMU_INTEN_RISE0_SHIFT + i)); + BIT(EXYNOS7_TMU_INTEN_RISE0_SHIFT + i); } interrupt_en |= interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT; - con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT); - con |= (1 << EXYNOS7_PD_DET_EN_SHIFT); + con |= BIT(EXYNOS_TMU_CORE_EN_SHIFT); + con |= BIT(EXYNOS7_PD_DET_EN_SHIFT); } else { - con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT); - con &= ~(1 << EXYNOS7_PD_DET_EN_SHIFT); + con &= ~BIT(EXYNOS_TMU_CORE_EN_SHIFT); + con &= ~BIT(EXYNOS7_PD_DET_EN_SHIFT); } writel(interrupt_en, data->base + EXYNOS7_TMU_REG_INTEN); From 5314b1543787e6cd5d248186fcfd5c5fc4ca2146 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Fri, 1 Dec 2023 10:56:25 +0100 Subject: [PATCH 60/64] thermal/drivers/exynos: Use set_trips ops Currently, each trip point defined in the device tree corresponds to a single hardware interrupt. This commit instead switches to using two hardware interrupts, whose values are set dynamically using the set_trips callback. Additionally, the critical temperature threshold is handled specifically. Setting interrupts in this way also fixes a long-standing lockdep warning, which was caused by calling thermal_zone_get_trips with our lock being held. Do note that this requires TMU initialization to be split into two parts, as done by the parent commit: parts of the initialization call into the thermal_zone_device structure and so must be done after its registration, but the initialization is also responsible for setting up calibration, which must be done before thermal_zone_device registration, which will call set_trips for the first time; if the calibration is not done in time, the interrupt values will be silently wrong! Reviewed-by: Lukasz Luba Signed-off-by: Mateusz Majewski Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20231201095625.301884-10-m.majewski2@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 393 ++++++++++++++------------- 1 file changed, 209 insertions(+), 184 deletions(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index ca1b1cec0300..6482513bfe66 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -158,10 +158,12 @@ enum soc_type { * in the positive-TC generator block * 0 < reference_voltage <= 31 * @tzd: pointer to thermal_zone_device structure - * @ntrip: number of supported trip points. * @enabled: current status of TMU device - * @tmu_set_trip_temp: SoC specific method to set trip (rising threshold) - * @tmu_set_trip_hyst: SoC specific to set hysteresis (falling threshold) + * @tmu_set_low_temp: SoC specific method to set trip (falling threshold) + * @tmu_set_high_temp: SoC specific method to set trip (rising threshold) + * @tmu_set_crit_temp: SoC specific method to set critical temperature + * @tmu_disable_low: SoC specific method to disable an interrupt (falling threshold) + * @tmu_disable_high: SoC specific method to disable an interrupt (rising threshold) * @tmu_initialize: SoC specific TMU initialization method * @tmu_control: SoC specific TMU control method * @tmu_read: SoC specific TMU temperature read method @@ -183,13 +185,13 @@ struct exynos_tmu_data { u8 gain; u8 reference_voltage; struct thermal_zone_device *tzd; - unsigned int ntrip; bool enabled; - void (*tmu_set_trip_temp)(struct exynos_tmu_data *data, int trip, - u8 temp); - void (*tmu_set_trip_hyst)(struct exynos_tmu_data *data, int trip, - u8 temp, u8 hyst); + void (*tmu_set_low_temp)(struct exynos_tmu_data *data, u8 temp); + void (*tmu_set_high_temp)(struct exynos_tmu_data *data, u8 temp); + void (*tmu_set_crit_temp)(struct exynos_tmu_data *data, u8 temp); + void (*tmu_disable_low)(struct exynos_tmu_data *data); + void (*tmu_disable_high)(struct exynos_tmu_data *data); void (*tmu_initialize)(struct platform_device *pdev); void (*tmu_control)(struct platform_device *pdev, bool on); int (*tmu_read)(struct exynos_tmu_data *data); @@ -279,49 +281,28 @@ static int exynos_thermal_zone_configure(struct platform_device *pdev) { struct exynos_tmu_data *data = platform_get_drvdata(pdev); struct thermal_zone_device *tzd = data->tzd; - int i, num_trips = thermal_zone_get_num_trips(tzd); - int ret = 0, temp; + int ret, temp; ret = thermal_zone_get_crit_temp(tzd, &temp); + if (ret) { + /* FIXME: Remove this special case */ + if (data->soc == SOC_ARCH_EXYNOS5433) + return 0; - if (ret && data->soc != SOC_ARCH_EXYNOS5433) { /* FIXME */ dev_err(&pdev->dev, "No CRITICAL trip point defined in device tree!\n"); - goto out; + return ret; } mutex_lock(&data->lock); - - if (num_trips > data->ntrip) { - dev_info(&pdev->dev, - "More trip points than supported by this TMU.\n"); - dev_info(&pdev->dev, - "%d trip points should be configured in polling mode.\n", - num_trips - data->ntrip); - } - clk_enable(data->clk); - num_trips = min_t(int, num_trips, data->ntrip); + data->tmu_set_crit_temp(data, temp / MCELSIUS); - /* Write temperature code for rising and falling threshold */ - for (i = 0; i < num_trips; i++) { - struct thermal_trip trip; - - ret = thermal_zone_get_trip(tzd, i, &trip); - if (ret) - goto err; - - data->tmu_set_trip_temp(data, i, trip.temperature / MCELSIUS); - data->tmu_set_trip_hyst(data, i, trip.temperature / MCELSIUS, - trip.hysteresis / MCELSIUS); - } - -err: clk_disable(data->clk); mutex_unlock(&data->lock); -out: - return ret; + + return 0; } static u32 get_con_reg(struct exynos_tmu_data *data, u32 con) @@ -354,17 +335,74 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on) mutex_unlock(&data->lock); } -static void exynos4210_tmu_set_trip_temp(struct exynos_tmu_data *data, - int trip_id, u8 temp) +static void exynos_tmu_update_bit(struct exynos_tmu_data *data, int reg_off, + int bit_off, bool enable) { - temp = temp_to_code(data, temp); - writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + trip_id * 4); + u32 interrupt_en; + + interrupt_en = readl(data->base + reg_off); + if (enable) + interrupt_en |= BIT(bit_off); + else + interrupt_en &= ~BIT(bit_off); + writel(interrupt_en, data->base + reg_off); } -/* failing thresholds are not supported on Exynos4210 */ -static void exynos4210_tmu_set_trip_hyst(struct exynos_tmu_data *data, - int trip, u8 temp, u8 hyst) +static void exynos_tmu_update_temp(struct exynos_tmu_data *data, int reg_off, + int bit_off, u8 temp) { + u16 tmu_temp_mask; + u32 th; + + tmu_temp_mask = + (data->soc == SOC_ARCH_EXYNOS7) ? EXYNOS7_TMU_TEMP_MASK + : EXYNOS_TMU_TEMP_MASK; + + th = readl(data->base + reg_off); + th &= ~(tmu_temp_mask << bit_off); + th |= temp_to_code(data, temp) << bit_off; + writel(th, data->base + reg_off); +} + +static void exynos4210_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp) +{ + /* + * Failing thresholds are not supported on Exynos 4210. + * We use polling instead. + */ +} + +static void exynos4210_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp) +{ + temp = temp_to_code(data, temp); + writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + 4); + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_RISE0_SHIFT + 4, true); +} + +static void exynos4210_tmu_disable_low(struct exynos_tmu_data *data) +{ + /* Again, this is handled by polling. */ +} + +static void exynos4210_tmu_disable_high(struct exynos_tmu_data *data) +{ + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_RISE0_SHIFT + 4, false); +} + +static void exynos4210_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp) +{ + /* + * Hardware critical temperature handling is not supported on Exynos 4210. + * We still set the critical temperature threshold, but this is only to + * make sure it is handled as soon as possible. It is just a normal interrupt. + */ + + temp = temp_to_code(data, temp); + writeb(temp, data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + 12); + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_RISE0_SHIFT + 12, true); } static void exynos4210_tmu_initialize(struct platform_device *pdev) @@ -376,33 +414,31 @@ static void exynos4210_tmu_initialize(struct platform_device *pdev) writeb(0, data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP); } -static void exynos4412_tmu_set_trip_temp(struct exynos_tmu_data *data, - int trip, u8 temp) +static void exynos4412_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp) { - u32 th, con; - - th = readl(data->base + EXYNOS_THD_TEMP_RISE); - th &= ~(0xff << 8 * trip); - th |= temp_to_code(data, temp) << 8 * trip; - writel(th, data->base + EXYNOS_THD_TEMP_RISE); - - if (trip == 3) { - con = readl(data->base + EXYNOS_TMU_REG_CONTROL); - con |= BIT(EXYNOS_TMU_THERM_TRIP_EN_SHIFT); - writel(con, data->base + EXYNOS_TMU_REG_CONTROL); - } + exynos_tmu_update_temp(data, EXYNOS_THD_TEMP_FALL, 0, temp); + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_FALL0_SHIFT, true); } -static void exynos4412_tmu_set_trip_hyst(struct exynos_tmu_data *data, - int trip, u8 temp, u8 hyst) +static void exynos4412_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp) { - u32 th; + exynos_tmu_update_temp(data, EXYNOS_THD_TEMP_RISE, 8, temp); + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_RISE0_SHIFT + 4, true); +} - th = readl(data->base + EXYNOS_THD_TEMP_FALL); - th &= ~(0xff << 8 * trip); - if (hyst) - th |= temp_to_code(data, temp - hyst) << 8 * trip; - writel(th, data->base + EXYNOS_THD_TEMP_FALL); +static void exynos4412_tmu_disable_low(struct exynos_tmu_data *data) +{ + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_FALL0_SHIFT, false); +} + +static void exynos4412_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp) +{ + exynos_tmu_update_temp(data, EXYNOS_THD_TEMP_RISE, 24, temp); + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_CONTROL, + EXYNOS_TMU_THERM_TRIP_EN_SHIFT, true); } static void exynos4412_tmu_initialize(struct platform_device *pdev) @@ -432,44 +468,39 @@ static void exynos4412_tmu_initialize(struct platform_device *pdev) sanitize_temp_error(data, trim_info); } -static void exynos5433_tmu_set_trip_temp(struct exynos_tmu_data *data, - int trip, u8 temp) +static void exynos5433_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp) { - unsigned int reg_off, j; - u32 th; - - if (trip > 3) { - reg_off = EXYNOS5433_THD_TEMP_RISE7_4; - j = trip - 4; - } else { - reg_off = EXYNOS5433_THD_TEMP_RISE3_0; - j = trip; - } - - th = readl(data->base + reg_off); - th &= ~(0xff << j * 8); - th |= (temp_to_code(data, temp) << j * 8); - writel(th, data->base + reg_off); + exynos_tmu_update_temp(data, EXYNOS5433_THD_TEMP_FALL3_0, 0, temp); + exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_FALL0_SHIFT, true); } -static void exynos5433_tmu_set_trip_hyst(struct exynos_tmu_data *data, - int trip, u8 temp, u8 hyst) +static void exynos5433_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp) { - unsigned int reg_off, j; - u32 th; + exynos_tmu_update_temp(data, EXYNOS5433_THD_TEMP_RISE3_0, 8, temp); + exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN, + EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, true); +} - if (trip > 3) { - reg_off = EXYNOS5433_THD_TEMP_FALL7_4; - j = trip - 4; - } else { - reg_off = EXYNOS5433_THD_TEMP_FALL3_0; - j = trip; - } +static void exynos5433_tmu_disable_low(struct exynos_tmu_data *data) +{ + exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_FALL0_SHIFT, false); +} - th = readl(data->base + reg_off); - th &= ~(0xff << j * 8); - th |= (temp_to_code(data, temp - hyst) << j * 8); - writel(th, data->base + reg_off); +static void exynos5433_tmu_disable_high(struct exynos_tmu_data *data) +{ + exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN, + EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, false); +} + +static void exynos5433_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp) +{ + exynos_tmu_update_temp(data, EXYNOS5433_THD_TEMP_RISE7_4, 24, temp); + exynos_tmu_update_bit(data, EXYNOS_TMU_REG_CONTROL, + EXYNOS_TMU_THERM_TRIP_EN_SHIFT, true); + exynos_tmu_update_bit(data, EXYNOS5433_TMU_REG_INTEN, + EXYNOS7_TMU_INTEN_RISE0_SHIFT + 7, true); } static void exynos5433_tmu_initialize(struct platform_device *pdev) @@ -505,34 +536,41 @@ static void exynos5433_tmu_initialize(struct platform_device *pdev) cal_type ? 2 : 1); } -static void exynos7_tmu_set_trip_temp(struct exynos_tmu_data *data, - int trip, u8 temp) +static void exynos7_tmu_set_low_temp(struct exynos_tmu_data *data, u8 temp) { - unsigned int reg_off, bit_off; - u32 th; - - reg_off = ((7 - trip) / 2) * 4; - bit_off = ((8 - trip) % 2); - - th = readl(data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off); - th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off)); - th |= temp_to_code(data, temp) << (16 * bit_off); - writel(th, data->base + EXYNOS7_THD_TEMP_RISE7_6 + reg_off); + exynos_tmu_update_temp(data, EXYNOS7_THD_TEMP_FALL7_6 + 12, 0, temp); + exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_FALL0_SHIFT + 0, true); } -static void exynos7_tmu_set_trip_hyst(struct exynos_tmu_data *data, - int trip, u8 temp, u8 hyst) +static void exynos7_tmu_set_high_temp(struct exynos_tmu_data *data, u8 temp) { - unsigned int reg_off, bit_off; - u32 th; + exynos_tmu_update_temp(data, EXYNOS7_THD_TEMP_RISE7_6 + 12, 16, temp); + exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN, + EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, true); +} - reg_off = ((7 - trip) / 2) * 4; - bit_off = ((8 - trip) % 2); +static void exynos7_tmu_disable_low(struct exynos_tmu_data *data) +{ + exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN, + EXYNOS_TMU_INTEN_FALL0_SHIFT + 0, false); +} - th = readl(data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off); - th &= ~(EXYNOS7_TMU_TEMP_MASK << (16 * bit_off)); - th |= temp_to_code(data, temp - hyst) << (16 * bit_off); - writel(th, data->base + EXYNOS7_THD_TEMP_FALL7_6 + reg_off); +static void exynos7_tmu_disable_high(struct exynos_tmu_data *data) +{ + exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN, + EXYNOS7_TMU_INTEN_RISE0_SHIFT + 1, false); +} + +static void exynos7_tmu_set_crit_temp(struct exynos_tmu_data *data, u8 temp) +{ + /* + * Like Exynos 4210, Exynos 7 does not seem to support critical temperature + * handling in hardware. Again, we still set a separate interrupt for it. + */ + exynos_tmu_update_temp(data, EXYNOS7_THD_TEMP_RISE7_6 + 0, 16, temp); + exynos_tmu_update_bit(data, EXYNOS7_TMU_REG_INTEN, + EXYNOS7_TMU_INTEN_RISE0_SHIFT + 7, true); } static void exynos7_tmu_initialize(struct platform_device *pdev) @@ -547,87 +585,44 @@ static void exynos7_tmu_initialize(struct platform_device *pdev) static void exynos4210_tmu_control(struct platform_device *pdev, bool on) { struct exynos_tmu_data *data = platform_get_drvdata(pdev); - struct thermal_zone_device *tz = data->tzd; - struct thermal_trip trip; - unsigned int con, interrupt_en = 0, i; + unsigned int con; con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL)); - if (on) { - for (i = 0; i < data->ntrip; i++) { - if (thermal_zone_get_trip(tz, i, &trip)) - continue; - - interrupt_en |= - BIT(EXYNOS_TMU_INTEN_RISE0_SHIFT + i * 4); - } - - if (data->soc != SOC_ARCH_EXYNOS4210) - interrupt_en |= - interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT; - + if (on) con |= BIT(EXYNOS_TMU_CORE_EN_SHIFT); - } else { + else con &= ~BIT(EXYNOS_TMU_CORE_EN_SHIFT); - } - writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN); writel(con, data->base + EXYNOS_TMU_REG_CONTROL); } static void exynos5433_tmu_control(struct platform_device *pdev, bool on) { struct exynos_tmu_data *data = platform_get_drvdata(pdev); - struct thermal_zone_device *tz = data->tzd; - struct thermal_trip trip; - unsigned int con, interrupt_en = 0, pd_det_en, i; + unsigned int con, pd_det_en; con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL)); - if (on) { - for (i = 0; i < data->ntrip; i++) { - if (thermal_zone_get_trip(tz, i, &trip)) - continue; - - interrupt_en |= - BIT(EXYNOS7_TMU_INTEN_RISE0_SHIFT + i); - } - - interrupt_en |= - interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT; - + if (on) con |= BIT(EXYNOS_TMU_CORE_EN_SHIFT); - } else + else con &= ~BIT(EXYNOS_TMU_CORE_EN_SHIFT); pd_det_en = on ? EXYNOS5433_PD_DET_EN : 0; writel(pd_det_en, data->base + EXYNOS5433_TMU_PD_DET_EN); - writel(interrupt_en, data->base + EXYNOS5433_TMU_REG_INTEN); writel(con, data->base + EXYNOS_TMU_REG_CONTROL); } static void exynos7_tmu_control(struct platform_device *pdev, bool on) { struct exynos_tmu_data *data = platform_get_drvdata(pdev); - struct thermal_zone_device *tz = data->tzd; - struct thermal_trip trip; - unsigned int con, interrupt_en = 0, i; + unsigned int con; con = get_con_reg(data, readl(data->base + EXYNOS_TMU_REG_CONTROL)); if (on) { - for (i = 0; i < data->ntrip; i++) { - if (thermal_zone_get_trip(tz, i, &trip)) - continue; - - interrupt_en |= - BIT(EXYNOS7_TMU_INTEN_RISE0_SHIFT + i); - } - - interrupt_en |= - interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT; - con |= BIT(EXYNOS_TMU_CORE_EN_SHIFT); con |= BIT(EXYNOS7_PD_DET_EN_SHIFT); } else { @@ -635,7 +630,6 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on) con &= ~BIT(EXYNOS7_PD_DET_EN_SHIFT); } - writel(interrupt_en, data->base + EXYNOS7_TMU_REG_INTEN); writel(con, data->base + EXYNOS_TMU_REG_CONTROL); } @@ -873,13 +867,15 @@ static int exynos_map_dt_data(struct platform_device *pdev) switch (data->soc) { case SOC_ARCH_EXYNOS4210: - data->tmu_set_trip_temp = exynos4210_tmu_set_trip_temp; - data->tmu_set_trip_hyst = exynos4210_tmu_set_trip_hyst; + data->tmu_set_low_temp = exynos4210_tmu_set_low_temp; + data->tmu_set_high_temp = exynos4210_tmu_set_high_temp; + data->tmu_disable_low = exynos4210_tmu_disable_low; + data->tmu_disable_high = exynos4210_tmu_disable_high; + data->tmu_set_crit_temp = exynos4210_tmu_set_crit_temp; data->tmu_initialize = exynos4210_tmu_initialize; data->tmu_control = exynos4210_tmu_control; data->tmu_read = exynos4210_tmu_read; data->tmu_clear_irqs = exynos4210_tmu_clear_irqs; - data->ntrip = 4; data->gain = 15; data->reference_voltage = 7; data->efuse_value = 55; @@ -892,14 +888,16 @@ static int exynos_map_dt_data(struct platform_device *pdev) case SOC_ARCH_EXYNOS5260: case SOC_ARCH_EXYNOS5420: case SOC_ARCH_EXYNOS5420_TRIMINFO: - data->tmu_set_trip_temp = exynos4412_tmu_set_trip_temp; - data->tmu_set_trip_hyst = exynos4412_tmu_set_trip_hyst; + data->tmu_set_low_temp = exynos4412_tmu_set_low_temp; + data->tmu_set_high_temp = exynos4412_tmu_set_high_temp; + data->tmu_disable_low = exynos4412_tmu_disable_low; + data->tmu_disable_high = exynos4210_tmu_disable_high; + data->tmu_set_crit_temp = exynos4412_tmu_set_crit_temp; data->tmu_initialize = exynos4412_tmu_initialize; data->tmu_control = exynos4210_tmu_control; data->tmu_read = exynos4412_tmu_read; data->tmu_set_emulation = exynos4412_tmu_set_emulation; data->tmu_clear_irqs = exynos4210_tmu_clear_irqs; - data->ntrip = 4; data->gain = 8; data->reference_voltage = 16; data->efuse_value = 55; @@ -911,14 +909,16 @@ static int exynos_map_dt_data(struct platform_device *pdev) data->max_efuse_value = 100; break; case SOC_ARCH_EXYNOS5433: - data->tmu_set_trip_temp = exynos5433_tmu_set_trip_temp; - data->tmu_set_trip_hyst = exynos5433_tmu_set_trip_hyst; + data->tmu_set_low_temp = exynos5433_tmu_set_low_temp; + data->tmu_set_high_temp = exynos5433_tmu_set_high_temp; + data->tmu_disable_low = exynos5433_tmu_disable_low; + data->tmu_disable_high = exynos5433_tmu_disable_high; + data->tmu_set_crit_temp = exynos5433_tmu_set_crit_temp; data->tmu_initialize = exynos5433_tmu_initialize; data->tmu_control = exynos5433_tmu_control; data->tmu_read = exynos4412_tmu_read; data->tmu_set_emulation = exynos4412_tmu_set_emulation; data->tmu_clear_irqs = exynos4210_tmu_clear_irqs; - data->ntrip = 8; data->gain = 8; if (res.start == EXYNOS5433_G3D_BASE) data->reference_voltage = 23; @@ -929,14 +929,16 @@ static int exynos_map_dt_data(struct platform_device *pdev) data->max_efuse_value = 150; break; case SOC_ARCH_EXYNOS7: - data->tmu_set_trip_temp = exynos7_tmu_set_trip_temp; - data->tmu_set_trip_hyst = exynos7_tmu_set_trip_hyst; + data->tmu_set_low_temp = exynos7_tmu_set_low_temp; + data->tmu_set_high_temp = exynos7_tmu_set_high_temp; + data->tmu_disable_low = exynos7_tmu_disable_low; + data->tmu_disable_high = exynos7_tmu_disable_high; + data->tmu_set_crit_temp = exynos7_tmu_set_crit_temp; data->tmu_initialize = exynos7_tmu_initialize; data->tmu_control = exynos7_tmu_control; data->tmu_read = exynos7_tmu_read; data->tmu_set_emulation = exynos4412_tmu_set_emulation; data->tmu_clear_irqs = exynos4210_tmu_clear_irqs; - data->ntrip = 8; data->gain = 9; data->reference_voltage = 17; data->efuse_value = 75; @@ -972,9 +974,32 @@ static int exynos_map_dt_data(struct platform_device *pdev) return 0; } +static int exynos_set_trips(struct thermal_zone_device *tz, int low, int high) +{ + struct exynos_tmu_data *data = thermal_zone_device_priv(tz); + + mutex_lock(&data->lock); + clk_enable(data->clk); + + if (low > INT_MIN) + data->tmu_set_low_temp(data, low / MCELSIUS); + else + data->tmu_disable_low(data); + if (high < INT_MAX) + data->tmu_set_high_temp(data, high / MCELSIUS); + else + data->tmu_disable_high(data); + + clk_disable(data->clk); + mutex_unlock(&data->lock); + + return 0; +} + static const struct thermal_zone_device_ops exynos_sensor_ops = { .get_temp = exynos_get_temp, .set_emul_temp = exynos_tmu_set_emulation, + .set_trips = exynos_set_trips, }; static int exynos_tmu_probe(struct platform_device *pdev) From 8a8b6bb93c704776c4b05cb517c3fa8baffb72f5 Mon Sep 17 00:00:00 2001 From: Ricardo Neri Date: Tue, 2 Jan 2024 20:14:56 -0800 Subject: [PATCH 61/64] thermal: intel: hfi: Refactor enabling code into helper functions In preparation for the addition of a suspend notifier, wrap the logic to enable HFI and program its memory buffer into helper functions. Both the CPU hotplug callback and the suspend notifier will use them. This refactoring does not introduce functional changes. Signed-off-by: Ricardo Neri Signed-off-by: Rafael J. Wysocki --- drivers/thermal/intel/intel_hfi.c | 43 ++++++++++++++++--------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c index c69db6c90869..820613e293cd 100644 --- a/drivers/thermal/intel/intel_hfi.c +++ b/drivers/thermal/intel/intel_hfi.c @@ -347,6 +347,26 @@ static void init_hfi_instance(struct hfi_instance *hfi_instance) hfi_instance->data = hfi_instance->hdr + hfi_features.hdr_size; } +/* Caller must hold hfi_instance_lock. */ +static void hfi_enable(void) +{ + u64 msr_val; + + rdmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val); + msr_val |= HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT; + wrmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val); +} + +static void hfi_set_hw_table(struct hfi_instance *hfi_instance) +{ + phys_addr_t hw_table_pa; + u64 msr_val; + + hw_table_pa = virt_to_phys(hfi_instance->hw_table); + msr_val = hw_table_pa | HW_FEEDBACK_PTR_VALID_BIT; + wrmsrl(MSR_IA32_HW_FEEDBACK_PTR, msr_val); +} + /** * intel_hfi_online() - Enable HFI on @cpu * @cpu: CPU in which the HFI will be enabled @@ -364,8 +384,6 @@ void intel_hfi_online(unsigned int cpu) { struct hfi_instance *hfi_instance; struct hfi_cpu_info *info; - phys_addr_t hw_table_pa; - u64 msr_val; u16 die_id; /* Nothing to do if hfi_instances are missing. */ @@ -409,8 +427,6 @@ void intel_hfi_online(unsigned int cpu) if (!hfi_instance->hw_table) goto unlock; - hw_table_pa = virt_to_phys(hfi_instance->hw_table); - /* * Allocate memory to keep a local copy of the table that * hardware generates. @@ -420,16 +436,6 @@ void intel_hfi_online(unsigned int cpu) if (!hfi_instance->local_table) goto free_hw_table; - /* - * Program the address of the feedback table of this die/package. On - * some processors, hardware remembers the old address of the HFI table - * even after having been reprogrammed and re-enabled. Thus, do not free - * the pages allocated for the table or reprogram the hardware with a - * new base address. Namely, program the hardware only once. - */ - msr_val = hw_table_pa | HW_FEEDBACK_PTR_VALID_BIT; - wrmsrl(MSR_IA32_HW_FEEDBACK_PTR, msr_val); - init_hfi_instance(hfi_instance); INIT_DELAYED_WORK(&hfi_instance->update_work, hfi_update_work_fn); @@ -438,13 +444,8 @@ void intel_hfi_online(unsigned int cpu) cpumask_set_cpu(cpu, hfi_instance->cpus); - /* - * Enable the hardware feedback interface and never disable it. See - * comment on programming the address of the table. - */ - rdmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val); - msr_val |= HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT; - wrmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val); + hfi_set_hw_table(hfi_instance); + hfi_enable(); unlock: mutex_unlock(&hfi_instance_lock); From ac1f9230d92a04619331c600dbcead0e32b3e80e Mon Sep 17 00:00:00 2001 From: Ricardo Neri Date: Tue, 2 Jan 2024 20:14:57 -0800 Subject: [PATCH 62/64] thermal: intel: hfi: Enable an HFI instance from its first online CPU Previously, HFI instances were never disabled once enabled. A CPU in an instance only had to check during boot whether another CPU had previously initialized the instance and its corresponding data structure. A subsequent changeset will add functionality to disable instances to support hibernation. Such change will also make possible to disable an HFI instance during runtime via CPU hotplug. Enable an HFI instance from the first of its CPUs that comes online. This covers the boot, CPU hotplug, and resume-from-suspend cases. It also covers systems with one or more HFI instances (i.e., packages). Signed-off-by: Ricardo Neri Signed-off-by: Rafael J. Wysocki --- drivers/thermal/intel/intel_hfi.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c index 820613e293cd..713da8befd40 100644 --- a/drivers/thermal/intel/intel_hfi.c +++ b/drivers/thermal/intel/intel_hfi.c @@ -410,13 +410,12 @@ void intel_hfi_online(unsigned int cpu) /* * Now check if the HFI instance of the package/die of @cpu has been * initialized (by checking its header). In such case, all we have to - * do is to add @cpu to this instance's cpumask. + * do is to add @cpu to this instance's cpumask and enable the instance + * if needed. */ mutex_lock(&hfi_instance_lock); - if (hfi_instance->hdr) { - cpumask_set_cpu(cpu, hfi_instance->cpus); - goto unlock; - } + if (hfi_instance->hdr) + goto enable; /* * Hardware is programmed with the physical address of the first page @@ -442,10 +441,14 @@ void intel_hfi_online(unsigned int cpu) raw_spin_lock_init(&hfi_instance->table_lock); raw_spin_lock_init(&hfi_instance->event_lock); +enable: cpumask_set_cpu(cpu, hfi_instance->cpus); - hfi_set_hw_table(hfi_instance); - hfi_enable(); + /* Enable this HFI instance if this is its first online CPU. */ + if (cpumask_weight(hfi_instance->cpus) == 1) { + hfi_set_hw_table(hfi_instance); + hfi_enable(); + } unlock: mutex_unlock(&hfi_instance_lock); From 1c53081d773c2cb4461636559b0d55b46559ceec Mon Sep 17 00:00:00 2001 From: Ricardo Neri Date: Tue, 2 Jan 2024 20:14:58 -0800 Subject: [PATCH 63/64] thermal: intel: hfi: Disable an HFI instance when all its CPUs go offline In preparation to support hibernation, add functionality to disable an HFI instance during CPU offline. The last CPU of an instance that goes offline will disable such instance. The Intel Software Development Manual states that the operating system must wait for the hardware to set MSR_IA32_PACKAGE_THERM_STATUS[26] after disabling an HFI instance to ensure that it will no longer write on the HFI memory. Some processors, however, do not ever set such bit. Wait a minimum of 2ms to give time hardware to complete any pending memory writes. Signed-off-by: Ricardo Neri Signed-off-by: Rafael J. Wysocki --- drivers/thermal/intel/intel_hfi.c | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c index 713da8befd40..22445403b520 100644 --- a/drivers/thermal/intel/intel_hfi.c +++ b/drivers/thermal/intel/intel_hfi.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -367,6 +368,32 @@ static void hfi_set_hw_table(struct hfi_instance *hfi_instance) wrmsrl(MSR_IA32_HW_FEEDBACK_PTR, msr_val); } +/* Caller must hold hfi_instance_lock. */ +static void hfi_disable(void) +{ + u64 msr_val; + int i; + + rdmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val); + msr_val &= ~HW_FEEDBACK_CONFIG_HFI_ENABLE_BIT; + wrmsrl(MSR_IA32_HW_FEEDBACK_CONFIG, msr_val); + + /* + * Wait for hardware to acknowledge the disabling of HFI. Some + * processors may not do it. Wait for ~2ms. This is a reasonable + * time for hardware to complete any pending actions on the HFI + * memory. + */ + for (i = 0; i < 2000; i++) { + rdmsrl(MSR_IA32_PACKAGE_THERM_STATUS, msr_val); + if (msr_val & PACKAGE_THERM_STATUS_HFI_UPDATED) + break; + + udelay(1); + cpu_relax(); + } +} + /** * intel_hfi_online() - Enable HFI on @cpu * @cpu: CPU in which the HFI will be enabled @@ -420,6 +447,10 @@ void intel_hfi_online(unsigned int cpu) /* * Hardware is programmed with the physical address of the first page * frame of the table. Hence, the allocated memory must be page-aligned. + * + * Some processors do not forget the initial address of the HFI table + * even after having been reprogrammed. Keep using the same pages. Do + * not free them. */ hfi_instance->hw_table = alloc_pages_exact(hfi_features.nr_table_pages, GFP_KERNEL | __GFP_ZERO); @@ -488,6 +519,10 @@ void intel_hfi_offline(unsigned int cpu) mutex_lock(&hfi_instance_lock); cpumask_clear_cpu(cpu, hfi_instance->cpus); + + if (!cpumask_weight(hfi_instance->cpus)) + hfi_disable(); + mutex_unlock(&hfi_instance_lock); } From f380846462b2d8341be303db954d4305d419b883 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 15 Dec 2023 20:53:52 +0100 Subject: [PATCH 64/64] thermal: trip: Constify thermal zone argument of thermal_zone_trip_id() Because thermal_zone_trip_id() does not update the thermal zone object passed to it, its pointer argument representing the thermal zone can be const, so adjust its definition accordingly. No functional impact. Signed-off-by: Rafael J. Wysocki Reviewed-by: Lukasz Luba --- drivers/thermal/thermal_core.h | 2 +- drivers/thermal/thermal_trip.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index e6a2b6f97be8..4e023d54fd27 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -123,7 +123,7 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz, for (__trip = __tz->trips; __trip - __tz->trips < __tz->num_trips; __trip++) void __thermal_zone_set_trips(struct thermal_zone_device *tz); -int thermal_zone_trip_id(struct thermal_zone_device *tz, +int thermal_zone_trip_id(const struct thermal_zone_device *tz, const struct thermal_trip *trip); void thermal_zone_trip_updated(struct thermal_zone_device *tz, const struct thermal_trip *trip); diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index a1ad345c0741..8bffa1e5e206 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -143,7 +143,7 @@ int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, } EXPORT_SYMBOL_GPL(thermal_zone_get_trip); -int thermal_zone_trip_id(struct thermal_zone_device *tz, +int thermal_zone_trip_id(const struct thermal_zone_device *tz, const struct thermal_trip *trip) { /*