Messages in this thread | | | Date | Thu, 29 Jun 2023 17:10:34 -0400 | From | Nícolas F. R. A. Prado <> | Subject | Re: [PATCH v2 5/6] thermal/drivers/mediatek/lvts_thermal: Don't leave threshold zeroed |
| |
On Fri, Jun 02, 2023 at 11:17:27AM +0200, Daniel Lezcano wrote: > 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?
Well, actually, set_trips won't even be called since from the thermal framework's perspective we haven't crossed trip points, ie in __thermal_zone_set_trips():
/* No need to change trip points */ if (tz->prev_low_trip == low && tz->prev_high_trip == high) return;
But in any case, yes, the interrupt will fire, the temperature will get updated in the framework, and that's it. It will only fire again when a threshold is crossed again (either by the temperature rising and falling again below this minimum, or rising beyond the high treshold).
So basically at most this will cause a spurious interrupt when the temperature gets low enough. I do get 34-36C on all sensors when idling though, so I doubt that temperature is even reachable. Besides, we don't really have another option here if we want working interrupts, the threshold needs to be set to a valid value, and this is the lowest I've found.
And thanks for all the feedback! I'll prepare a v3 based on your comments.
Thanks, Nícolas
| |