Messages in this thread | | | Date | Tue, 24 Jan 2023 12:04:58 +0100 | From | Miquel Raynal <> | Subject | Re: [PATCH] thermal/drivers/armada: Use the thermal_zone_get_crit_temp() |
| |
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.
> + armada_set_overheat_thresholds(priv, temperature, 0); > + priv->overheat_sensor = tz; > + priv->interrupt_source = sensor_id; > + armada_enable_overheat_interrupt(priv); > > - return -EINVAL; > + return 0; > } > > static int armada_thermal_probe(struct platform_device *pdev)
LGTM so,
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Thanks, Miquèl
| |