From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Thu, 15 Apr 2021 08:57:11 +0200 Subject: [PATCH] watchdog: use time_after_eq() in watchdog_reset() In-Reply-To: <733cb132-f952-aebc-c467-769bc69dcff9@prevas.dk> References: <20210413144320.2504774-1-rasmus.villemoes@prevas.dk> <7b583359-b326-a435-2f14-48119b9ce8f8@denx.de> <733cb132-f952-aebc-c467-769bc69dcff9@prevas.dk> Message-ID: <8371533b-c5d7-7797-5ed9-4052ae630160@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>> Signed-off-by: Rasmus Villemoes >>> --- >>> >>> 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