| Message ID | 20201206123753.28440-1-daniel.lezcano@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFD] thermal: core: Browse the trip points in reverse order and exit | expand |
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 9d6a7b7f8ab4..d745b62a63af 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -562,8 +562,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz, tz->notify_event = event; - for (count = 0; count < tz->trips; count++) - handle_thermal_trip(tz, count); + for (count = tz->trips - 1; count >= 0; count--) + if (handle_thermal_trip(tz, count)) + break; monitor_thermal_zone(tz); }
When reading the code it is unclear if the loop is there to handle one trip point or all the trip points above a certain temperature. With the current code, the throttle function is called for every trip point crossed and it is ambiguous if that is made on purpose. Even digging into the history up to April, 16th 2015, the initial git import of the acpi implementation of the loop before being converted into a generic code, it is unclear if all trip points must be handled or not. When the code was intially written, was it assuming there can be only one passive or active trip point ? The cooling effect of the system will be very different depending on how this loop is considered. Even the IPA governor is filtering out multiple trip points to stick to one value. Was the code to accomodate with a bogus loop and based on the multiple non-critical trip points ? Another example: arch/arm64/boot/dts/exynos/exynos5433-tmu.dtsi defines 7 passive trip points, each of them mapped to the *same* cooling device but with different min-max states. That means the handle trip points loop will go through all these seven trip points and change the state of the cooling device seven times. On the other hand, a thermal zone can have two different cooling devices mapped to two trip points, shall we consider having both cooling devices acting together when the second trip point is crossed (eg. 1st trip point: cpufreq cooling device + 2nd trip point: fan), or the second cooling device takes over the first one ? IOW, combine the cooling effects or not ? Depending on the expected behavior, the loop can be done in the reverse order and exit after processing the highest trip point. Cc: Lukasz Luba <lukasz.luba@arm.com> Cc: Kukjin Kim <kgene@kernel.org> Cc: Krzysztof Kozlowski <krzk@kernel.org> Cc: Zhang Rui <rui.zhang@intel.com> Cc: Thara Gopinath <thara.gopinath@linaro.org> Cc: Amit Kucheria <amitk@kernel.org> Cc: Len Brown <lenb@kernel.org> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/thermal/thermal_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.17.1