Messages in this thread | | | Subject | Re: [PATCH] thermal/core: Emit a warning if the thermal zone is updated without ops | From | Daniel Lezcano <> | Date | Tue, 8 Dec 2020 14:51:41 +0100 |
| |
Hi Lukasz,
On 08/12/2020 10:36, Lukasz Luba wrote: > Hi Daniel,
[ ... ]
>> static void thermal_zone_device_init(struct thermal_zone_device *tz) >> @@ -553,11 +555,9 @@ void thermal_zone_device_update(struct >> thermal_zone_device *tz, >> if (atomic_read(&in_suspend)) >> return; >> - if (!tz->ops->get_temp) >> + if (update_temperature(tz)) >> return; >> - update_temperature(tz); >> - > > I think the patch does a bit more. Previously we continued running the > code below even when the thermal_zone_get_temp() returned an error (due > to various reasons). Now we stop and probably would not schedule next > polling, not calling: > handle_thermal_trip() and monitor_thermal_zone()
I agree there is a change in the behavior.
> I would left update_temperature(tz) as it was and not check the return. > The function thermal_zone_get_temp() can protect itself from missing > tz->ops->get_temp(), so we should be safe. > > What do you think?
Does it make sense to handle the trip point if we are unable to read the temperature?
The lines following the update_temperature() are:
- thermal_zone_set_trips() which needs a correct tz->temperature
- handle_thermal_trip() which needs a correct tz->temperature to compare with
- monitor_thermal_zone() which needs a consistent tz->passive. This one is updated by the governor which is in an inconsistent state because the temperature is not updated.
The problem I see here is how the interrupt mode and the polling mode are existing in the same code path.
The interrupt mode can call thermal_notify_framework() for critical/hot trip points without being followed by a monitoring. But for the other trip points, the get_temp is needed.
IMHO, we should return if update_temperature() is failing.
Perhaps, it would make sense to simply prevent to register a thermal zone if the get_temp ops is not defined.
AFAICS, if the interrupt mode without get_temp callback are for hot and critical trip points which can be directly invoked from the sensor via a specified callback, no thermal zone would be needed in this case.
-- <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
| |