On 09/04/2021 16.37, Christophe Leroy wrote:
Le 09/04/2021 à 16:12, Rasmus Villemoes a écrit :
The ratelimiting isn't really strictly needed (prior to DM WDT, no such thing existed), so just disable it when we know that time no longer passes and have watchdog_reset() (e.g. called from decompression loop) unconditionally reset the watchdog timer.
Do we need to make it that complicated ? I think before the generic implementation, powerpc didn't have a rate limitation at all for pinging the watchdog, why not go back this direction, all the time ?
I mean we could simply set reset_period to 0 at all time for powerpc ( and change the test to time_after_eq() instead of time_after() ).
Maybe. I think I'd still prefer the one that changes the rate-limiting to be based on get_ticks() instead of get_timer(), which did seem to work everywhere.
IIRC, Stefan previously rejected having a config knob or board hook for disabling the ratelimiting - changing the code [the s/after/after_eq/] to effectively allow that by passing hw_margin_ms=0 in DT would probably work, but seems quite hacky (especially when DT is supposed to describe hardware).
I've also floated the option of making get_timer() return something sensible even with interrupts off on ppc - that would also solve the problem, and would benefit other generic code that uses get_timer() without knowing its limitation on ppc. https://lists.denx.de/pipermail/u-boot/2020-June/414842.html .
So, there are lots of ways of solving it. Which direction to take really depends on which viewpoint one has:
(1) the watchdog code should, if rate-limiting at all, use an implementation that works on all platforms [the get_ticks option] (2) the watchdog code should, if rate-limiting at all, provide a way for a board/platform to override or disable that (3) get_timer() on ppc is broken, it needs to provide sensible results at all times, even when interrupts are disabled for tens or hundreds of milliseconds.
Rasmus