Messages in this thread | | | Date | Tue, 24 Jan 2023 12:36:22 +0100 | Subject | Re: [PATCH] thermal/drivers/armada: Use the thermal_zone_get_crit_temp() | From | Daniel Lezcano <> |
| |
On 24/01/2023 12:04, Miquel Raynal wrote: > Hi Daniel, > > daniel.lezcano@linaro.org wrote on Wed, 18 Jan 2023 23:26:10 +0100: > >> The driver browses the trip point to find out the critical trip >> temperature. However the function thermal_zone_get_crit_temp() does >> already that, so the routine is pointless in the driver. >> >> Use thermal_zone_get_crit_temp() instead of inspecting all the trip >> points. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- >> drivers/thermal/armada_thermal.c | 38 +++++++++++++------------------- >> 1 file changed, 15 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c >> index db040dbdaa0a..c6d51d8acbf0 100644 >> --- a/drivers/thermal/armada_thermal.c >> +++ b/drivers/thermal/armada_thermal.c >> @@ -784,34 +784,26 @@ static int armada_configure_overheat_int(struct armada_thermal_priv *priv, >> int sensor_id) >> { >> /* Retrieve the critical trip point to enable the overheat interrupt */ >> - struct thermal_trip trip; >> + int temperature; >> int ret; >> - int i; >> - >> - for (i = 0; i < thermal_zone_get_num_trips(tz); i++) { >> - >> - ret = thermal_zone_get_trip(tz, i, &trip); >> - if (ret) >> - return ret; >> - >> - if (trip.type != THERMAL_TRIP_CRITICAL) >> - continue; >> - >> - ret = armada_select_channel(priv, sensor_id); >> - if (ret) >> - return ret; >> >> - armada_set_overheat_thresholds(priv, trip.temperature, >> - trip.hysteresis); >> - priv->overheat_sensor = tz; >> - priv->interrupt_source = sensor_id; >> + ret = thermal_zone_get_crit_temp(tz, &temperature); >> + if (ret) >> + return ret; >> >> - armada_enable_overheat_interrupt(priv); >> + ret = armada_select_channel(priv, sensor_id); >> + if (ret) >> + return ret; >> >> - return 0; >> - } >> + /* >> + * A critical temperature does not have a hysteresis >> + */ > > Makes sense. > > Nit: I would actually put that comment in the commit log rather than > keeping it in the code, but whatever, that's a nice simplification.
Oh, actually, I added the comment because of the third parameter change for armada_set_overheat_thresholds(..., 0);
>> + armada_set_overheat_thresholds(priv, temperature, 0);
I think it makes sense to keep the comment to clarify why we pass this zero temperature argument.
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |