[PATCH] watchdog: use time_after_eq() in watchdog_reset()

[Stefan Roese]

On 15.04.21 08:54, Rasmus Villemoes wrote:
> On 15/04/2021 07.38, Stefan Roese wrote:
>> On 13.04.21 16:43, Rasmus Villemoes wrote:
>>> Some boards don't work with the rate-limiting done in the generic
>>> watchdog_reset() provided by wdt-uclass.
>>>
>>> For example, on powerpc, get_timer() ceases working during bootm since
>>> interrupts are disabled before the kernel image gets decompressed, and
>>> when the decompression takes longer than the watchdog device
>>> allows (or enough of the budget that the kernel doesn't get far enough
>>> to assume responsibility for petting the watchdog), the result is a
>>> non-booting board.
>>>
>>> As a somewhat hacky workaround (because DT is supposed to describe
>>> hardware), allow specifying hw_margin_ms=0 in device tree to
>>> effectively disable the ratelimiting and actually ping the watchdog
>>> every time watchdog_reset() is called. For that to work, the "has
>>> enough time passed" check just needs to be tweaked a little to allow
>>> the now==next_reset case as well.
>>>
>>> Suggested-by: Christophe Leroy <christophe.leroy at csgroup.eu>
>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
>>> ---
>>>
>>> It's the option I dislike the most (because of the DT abuse), but I
>>> also do accept that it's the one with the minimal code impact, and
>>> apparently the path of least resistance. So here it is.
>>
>> Right. An alternative way would have been to add a new Kconfig symbol
>> to define the default value of "reset_period" so that it can be
>> configured to different values via Kconfig as well.
> 
> No, I don't think we should not go in that direction.
> 
> Another thing I have on my todo-list is to rewrite the watchdog_reset()
> in wdt-uclass to handle _all_ DM watchdogs, not just the first one.

Great. This is a current flaw in the U-Boot WDT implementation.

> Some
> boards make use of both the one in the CPU/SOC as well as some
> gpio-triggered one. Both the hw_margin_ms/reset_period and the
> last_reset data would then have to live with the device, not be static
> variables. Last I looked, it does seem that the DM code supports having
> a piece of class-owned, per-device data, so it shouldn't be too hard to do.

Thanks for looking into this.

Thanks,
Stefan