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

Fri Nov 12 16:23:50 CET 2021

On Friday 12 November 2021 14:44:15 Stefan Roese wrote:
> On 11/9/21 17:14, Marek Behún wrote:
> > From: Pali Rohár <pali at 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 at kernel.org>
> > Signed-off-by: Marek Behún <marek.behun at 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?

It should be possible to extend orion_wdt.c driver to add support for
L2/NBCLK as a clock source. Currently neither kernel nor U-Boot has this
support. There is an issue how to get this clock property in watchdog
driver as it should be described in DTS. And name "orion" says that this
driver is compatible back to the old Marvell Orion products. I have no
idea how this "slow" clock source is supported on older products.

But I think that the main issue here is how the watchdog in this board
code is used. It is there just to start it and letting it as is. No
period kicking, no other actions.

It is activated here only to ensure that when U-Boot SPL returns to
bootrom, watchdog is activated and resets board in case bootrom fails to
load proper U-Boot.

So for using it, following steps are needed:
* extend U-Boot watchdog code to allow initializing watchdog drivers
  without periodic reset, but allowing to start them with specific
  timeout
* extend that code, so there can be different configurations per
  different watchdogs and different between SPL and proper U-Boot
* figure out how can be that watchdog configured on non-A385 platforms
* extend DTS structure in a way that be used also in Linux kernel
  and figure out how to specify correct clock in DTS

I think that this is too much work, just for one specific usage for one
board. I do not see how can be such long time period (20 minutes) used
as watchdog timeout on other boards. Is there any use case for other
board? If not, I think it is overkill to implement it, if there is no
benefit from it. I think that watchdog timeout is generally set to 60s.

> Thanks,
> Stefan
> 
> > ---
> >   board/CZ.NIC/turris_omnia/turris_omnia.c | 65 +++++++++++++++++++++++-
> >   1 file changed, 63 insertions(+), 2 deletions(-)
> > 
> > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > index 36c596efc5..ae24d14b76 100644
> > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > @@ -43,6 +43,23 @@ DECLARE_GLOBAL_DATA_PTR;
> >   #define OMNIA_I2C_EEPROM_CHIP_LEN	2
> >   #define OMNIA_I2C_EEPROM_MAGIC		0x0341a034
> > +#define SYS_RSTOUT_MASK			MVEBU_REGISTER(0x18260)
> > +#define   SYS_RSTOUT_MASK_WD		BIT(10)
> > +
> > +#define A385_WDT_GLOBAL_CTRL		MVEBU_REGISTER(0x20300)
> > +#define   A385_WDT_GLOBAL_RATIO_MASK	GENMASK(18, 16)
> > +#define   A385_WDT_GLOBAL_RATIO_SHIFT	16
> > +#define   A385_WDT_GLOBAL_25MHZ		BIT(10)
> > +#define   A385_WDT_GLOBAL_ENABLE	BIT(8)
> > +
> > +#define A385_WDT_GLOBAL_STATUS		MVEBU_REGISTER(0x20304)
> > +#define   A385_WDT_GLOBAL_EXPIRED	BIT(31)
> > +
> > +#define A385_WDT_DURATION		MVEBU_REGISTER(0x20334)
> > +
> > +#define A385_WD_RSTOUT_UNMASK		MVEBU_REGISTER(0x20704)
> > +#define   A385_WD_RSTOUT_UNMASK_GLOBAL	BIT(8)
> > +
> >   enum mcu_commands {
> >   	CMD_GET_STATUS_WORD	= 0x01,
> >   	CMD_GET_RESET		= 0x09,
> > @@ -141,6 +158,47 @@ static int omnia_mcu_write(u8 cmd, const void *buf, int len)
> >   	return dm_i2c_write(chip, cmd, buf, len);
> >   }
> > +static void enable_a385_watchdog(unsigned int timeout_minutes)
> > +{
> > +	struct sar_freq_modes sar_freq;
> > +	u32 watchdog_freq;
> > +
> > +	printf("Enabling A385 watchdog with %u minutes timeout...\n",
> > +	       timeout_minutes);
> > +
> > +	/*
> > +	 * Use NBCLK clock (a.k.a. L2 clock) as watchdog input clock with
> > +	 * its maximal ratio 7 instead of default fixed 25 MHz clock.
> > +	 * It allows to set watchdog duration up to the 22 minutes.
> > +	 */
> > +	clrsetbits_32(A385_WDT_GLOBAL_CTRL,
> > +		      A385_WDT_GLOBAL_25MHZ | A385_WDT_GLOBAL_RATIO_MASK,
> > +		      7 << A385_WDT_GLOBAL_RATIO_SHIFT);
> > +
> > +	/*
> > +	 * Calculate watchdog clock frequency. It is defined by formula:
> > +	 *   freq = NBCLK / 2 / (2 ^ ratio)
> > +	 * We set ratio to the maximal possible value 7.
> > +	 */
> > +	get_sar_freq(&sar_freq);
> > +	watchdog_freq = sar_freq.nb_clk * 1000000 / 2 / (1 << 7);
> > +
> > +	/* Set watchdog duration */
> > +	writel(timeout_minutes * 60 * watchdog_freq, A385_WDT_DURATION);
> > +
> > +	/* Clear the watchdog expiration bit */
> > +	clrbits_32(A385_WDT_GLOBAL_STATUS, A385_WDT_GLOBAL_EXPIRED);
> > +
> > +	/* Enable watchdog timer */
> > +	setbits_32(A385_WDT_GLOBAL_CTRL, A385_WDT_GLOBAL_ENABLE);
> > +
> > +	/* Enable reset on watchdog */
> > +	setbits_32(A385_WD_RSTOUT_UNMASK, A385_WD_RSTOUT_UNMASK_GLOBAL);
> > +
> > +	/* Unmask reset for watchdog */
> > +	clrbits_32(SYS_RSTOUT_MASK, SYS_RSTOUT_MASK_WD);
> > +}
> > +
> >   static bool disable_mcu_watchdog(void)
> >   {
> >   	int ret;
> > @@ -423,10 +481,13 @@ void spl_board_init(void)
> >   {
> >   	/*
> >   	 * If booting from UART, disable MCU watchdog in SPL, since uploading
> > -	 * U-Boot proper can take too much time and trigger it.
> > +	 * U-Boot proper can take too much time and trigger it. Instead enable
> > +	 * A385 watchdog with very high timeout (10 minutes) to prevent hangup.
> >   	 */
> > -	if (get_boot_device() == BOOT_DEVICE_UART)
> > +	if (get_boot_device() == BOOT_DEVICE_UART) {
> > +		enable_a385_watchdog(10);
> >   		disable_mcu_watchdog();
> > +	}
> >   }
> >   int board_init(void)
> > 
> 
> Viele Grüße,
> Stefan Roese
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de