On 30/11/2020 12:49, Niklas Söderlund wrote: > Hi Daniel, > > Thanks for your comments. > > On 2020-11-30 09:15:00 +0100, Daniel Lezcano wrote: >> On 26/11/2020 23:09, Niklas Söderlund wrote: >>> Remove the usage of interrupts for the normal temperature operation and >>> depend on the polling performed by the thermal core. This is done to >>> prepare to use the interrupts as they are intended to trigger once >>> specific trip points are passed and not to react to temperature changes >>> in the normal operational range. >> >> I'm not sure to understand the change. Is it not more interesting to >> have the polling mode disabled for PM reasons and let the interrupt to >> fire at the first trip point so the mitigation happens then with the >> polling passive ? > > I agree and this is one of two goals I have with this change, in the > long run. The other is to be able support SoC models where the > interrupts may not be accessible. > > The change in this patch is to stop using the interrupts to fire as soon > as the temp moves +/- 1 degree C, see rcar_gen3_thermal_update_range(). > When I wrote that code I had misunderstood how things should be done and > thought I should use the interrupts as a substitute to the polling done > by the core and generate a THERMAL_EVENT_UNSPECIFIED as soon as the temp > changed, see rcar_gen3_thermal_irq(). > > The way I understand it today is that I should instead setup the > interrupts to fire if the temp moves over a trip point, in my case > described in DT to allow the system to be informed of this as you > describe above. > > In this firs change I'm simply removing my incorrect usage of interrupts > that I in future changes will add back in an correct usage pattern while > at the same time making interrupts optional to support SoCs where the > may not be available. > > Does this make sens or have I got the idea wrong? Ah, ok. I understand better now, thanks for the clarification. The interrupt mode implementation is wrong, so you remove it and switch to the polling mode until the interrupt is re-implemented in the correct way. -- <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