On 12.04.21 09:04, Rasmus Villemoes wrote:
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'm not sure if I explicitly rejected any patches, sorry it's been too
long. ;)
But I also find this idea from Christophe quite applealing, as it's
impact on the common code is minimal. Either you set the DT property
to 0. Or if you don't have the possibility to do this (no DT on your
platform?), then add a new Kconfig option to configure the default
value for "reset_period" via Kconfig.
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.
"hw_margin_ms=0" would be my favorite way right now (see above). If this
does not work for you, I'm also fine with the latest patch you sent to
the list.
Thanks,
Stefan
