Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4206 bytes --]

On Fri, Nov 12, 2021 at 05:40:42PM +0100, Pali Rohár wrote:
> On Friday 12 November 2021 11:31:38 Tom Rini wrote:
> > On Fri, Nov 12, 2021 at 02:44:15PM +0100, Stefan Roese wrote:
> > > On 11/9/21 17:14, Marek Behún wrote:
> > > > From: Pali Rohár <pali@kernel.org>
> > > > 
> > > > Commit aeb0ca64dbb5 ("arm: mvebu: turris_omnia: disable MCU watchdog in
> > > > SPL when booting over UART") disabled MCU watchdog when booting over
> > > > UART to ensure that watchdog does not reboot the board before UART
> > > > transfer finishes.
> > > > 
> > > > But if UART transfer fails for some reason, or if U-Boot binary crashes,
> > > > then board hangs forever as there is no watchdog running which could
> > > > reset it.
> > > > 
> > > > To fix this issue, enable A385 watchdog with very high timeout before
> > > > disabling MCU watchdog to ensure that even slow transfer can finish
> > > > successfully before watchdog timer expires and also to ensure that if
> > > > board hangs for some reason, watchdog will reset it.
> > > > 
> > > > Omnia's MCU watchdog has fixed 120 seconds timer and it cannot be
> > > > changed (without updating MCU firmware). A385 watchdog by default uses
> > > > 25 MHz input clock and so the largest timeout value (2^32-1) can be
> > > > just 171 seconds. But A385 watchdog can be switched to use NBCLK (L2) as
> > > > input clock (on Turris Omnia it is 800 MHz clock) and in this case final
> > > > watchdog clock frequency is calculated as:
> > > > 
> > > >    freq = NBCLK / 2 / (2 ^ R)
> > > > 
> > > > So A385 watchdog on Turris Omnia can be configured to at most 1374
> > > > seconds (about 22 minutes). We set it to 10 minutes, which should be
> > > > enough even for bigger U-Boot binaries or slower UART transfers.
> > > > 
> > > > Both U-Boot and Linux kernel, when initializing A385 watchdog, switch
> > > > watchdog timer to 25 MHz input clock, so usage of NBCLK input clock in
> > > > U-Boot SPL does not cause any issues.
> > > > 
> > > > Fixes: aeb0ca64dbb5 ("arm: mvebu: turris_omnia: disable MCU watchdog in SPL when booting over UART")
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > > 
> > > I'm wondering if it makes sense to add this "slow" watchdog support to
> > > the common watchdog driver instead (orion_wdt.c). So that it dynamically
> > > configures the needed input clock for the desired timeout.
> > > 
> > > Would this be possible?
> > 
> > I know I'm late to the thread here, but why can't / aren't we servicing
> > the 120s watchdog during UART loading?
> 
> UART loading is not done by U-Boot. It is done by BootROM.

Ah, that's the bit of the system I didn't know, ok.

> BootROM first loads SPL. Then it starts executing SPL and SPL must
> return control back to BootROM. BootROM then continue loading of proper
> U-Boot via UART and after successful transfer it executes proper U-Boot.
> 
> BootROM does not enable watchdog and does not kick watchdog during UART
> transfer.
> 
> So if we enable 120s watchdog in SPL and BootROM does not finish loading
> of proper U-Boot then board is rebooted.
> 
> Turris Omnia has additional MCU watchdog, which is activated after
> reset, but there is no function to kick this watchdog. It can be only
> stopped. Commit mentioned in Fixes: tag turned this MCU watchdog in SPL
> and this new commit enable one-shot A385 watchdog with large timeout
> which should be enough for loading proper U-Boot.

I'm not sure I can think of anything better to handle this than what
you're describing off-hand, but I'll keep thinking about it in case I
do..

> > Or have I gone crazy and we don't do that on say OMAP?
> 
> I do not know how is loading over UART implemented on OMAP. On OMAP3 I
> always used only loading over USB and watchdog was not enabled in
> X-Loader / SPL.

Right, OK, it wasn't until AM33xx and later that I started enabling SPL
watchdog support (which could be done on omap3 too) and since we
implement the normal servicing mechanisms and that U-Boot SPL is what's
doing the load over UART of main U-Boot, this case is fine there.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]