Re: Trip points crossed not detected when no cooling device bound

Rafael J. Wysocki
2024-06-27T18:23:57Z

On Thu, Jun 27, 2024 at 6:30 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 27/06/2024 11:54, Rafael J. Wysocki wrote:
> > On Thu, Jun 27, 2024 at 12:24 AM Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:
> >>
> >> On 26/06/2024 23:21, Daniel Lezcano wrote:
> >>
> >> [ ... ]
> >>
> >>>> Oh, I see where the problem can be. If the zone is polling only, it
> >>>> will not rearm the timer when the current zone temperature is invalid
> >>>> after the above commit, so does the attached patch help?
> >>>
> >>> At this point, I went far when bisecting another problem and I ended up
> >>> screwing my config file. So I had to generate a new one from the default
> >>> config. Since then the issue is no longer happening which sounds very
> >>> strange to me.
> >>>
> >>> I'm still investigating but if you have a suggestion coming in mind, it
> >>> would be welcome because I'm failing to find out what is going on ... :/
> >>
> >> I finally reproduced the issue. That happens when there is *no* cooling
> >> device bound on *any* thermal zones.
> >
> > Interesting.
> >
> >> Your patch seems to fix the problem but I'm not sure to understand the
> >> conditions of the bug
> >
> > It's probably the same as for commit 202aa0d4bb53:
> > thermal_zone_device_init() sets tz->temperature to
> > THERMAL_TEMP_INVALID and if the first invocation of
> > __thermal_zone_get_temp() returns an error (because the .get_temp()
> > callback returns an error), monitor_thermal_zone(). If polling is the
> > only way in which the zone temperature can be updated, things go south
> > because the timer is not set and there is no other way to set it. No
> > updates will be coming
>
> If there is no polling delay (aka interrupt driven), the routine will
> skip the _set_trips function and the monitor_thermal_zone() will do
> nothing in this case, right ?

_set_trips() looks at tz->temperature, however, and it doesn't make
sense to call it if the latter is invalid.

Same for handle_thermal_trip() and governor callbacks.

> Even setting a label jump to "monitor:" the routine is broken AFAICT

I beg to differ.

Yes, monitor_thermal_zone() does nothing if there is no polling, but
it needs to be called anyway because it checks whether or not polling
is there in the first place.

And if there is no polling, it is assumed that
__thermal_zone_device_update() will be called by other means.

> > The reason why the presence of cooling devices can "fix" this is
> > because thermal_bind_cdev_to_trip() sets tz->need_update to 1 which
> > then causes the thermal_zone_device_update() in
> > __thermal_cooling_device_register() to trigger and that will update
> > the temperature.
>
> IIUC, the first time get_temp() fails and then when the tz is bound, the
> update triggers a new call with get_temp() which returns a valid
> temperature ?

Yes.