Messages in this thread | | | Date | Fri, 2 Jun 2023 11:17:27 +0200 | Subject | Re: [PATCH v2 5/6] thermal/drivers/mediatek/lvts_thermal: Don't leave threshold zeroed | From | Daniel Lezcano <> |
| |
On 04/05/2023 02:48, Nícolas F. R. A. Prado wrote: > The thermal framework might leave the low threshold unset if there > aren't any lower trip points. This leaves the register zeroed, which > translates to a very high temperature for the low threshold. The > interrupt for this threshold is then immediately triggered, and the > state machine gets stuck, preventing any other temperature monitoring > interrupts to ever trigger. > > (The same happens by not setting the Cold or Hot to Normal thresholds > when using those) > > Set the unused threshold to a valid low value. This value was chosen so > that for any valid golden temperature read from the efuse, when the > value is converted to raw and back again to milliCelsius, the result > doesn't underflow. > > Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver") > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > --- > > Changes in v2: > - Added this commit > > drivers/thermal/mediatek/lvts_thermal.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c > index efd1e938e1c2..951a4cb75ef6 100644 > --- a/drivers/thermal/mediatek/lvts_thermal.c > +++ b/drivers/thermal/mediatek/lvts_thermal.c > @@ -82,6 +82,8 @@ > #define LVTS_HW_SHUTDOWN_MT8195 105000 > #define LVTS_HW_SHUTDOWN_MT8192 105000 > > +#define LVTS_MINIUM_THRESHOLD 20000
MINIMUM
So if the thermal zone reaches 20°C, the interrupt fires, the set_trips sets again 20°C but the interrupt won't fire until the temperature goes above 20°C and then crosses the temperature low threshold the way down again?
> static int golden_temp = LVTS_GOLDEN_TEMP_DEFAULT; > static int coeff_b = LVTS_COEFF_B; > > @@ -309,6 +311,11 @@ static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high) > pr_debug("%s: Setting low limit temperature interrupt: %d\n", > thermal_zone_device_type(tz), low); > writel(raw_low, LVTS_OFFSETL(base)); > + } else { > + pr_debug("%s: Setting low limit temperature to minimum\n", > + thermal_zone_device_type(tz)); > + raw_low = lvts_temp_to_raw(LVTS_MINIUM_THRESHOLD); > + writel(raw_low, LVTS_OFFSETL(base));
That's duplicate code:
u32 raw_low = lvts_temp_to_raw(low != -INT_MAX ? low : LVTS_MINIMUM_THRESHOLD);
And then the condition in the code goes away: if (low != -INT_MAX) { }
> } > > /*
-- <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
| |