All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
@ 2021-11-09 16:14 Marek Behún
  2021-11-12 13:44 ` Stefan Roese
  2021-11-30 13:38 ` Stefan Roese
  0 siblings, 2 replies; 8+ messages in thread
From: Marek Behún @ 2021-11-09 16:14 UTC (permalink / raw)
  To: Stefan Roese; +Cc: u-boot, Pali Rohár, Marek Behún

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>
---
 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)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
  2021-11-09 16:14 [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog Marek Behún
@ 2021-11-12 13:44 ` Stefan Roese
  2021-11-12 15:23   ` Pali Rohár
  2021-11-12 16:31   ` Tom Rini
  2021-11-30 13:38 ` Stefan Roese
  1 sibling, 2 replies; 8+ messages in thread
From: Stefan Roese @ 2021-11-12 13:44 UTC (permalink / raw)
  To: Marek Behún; +Cc: u-boot, Pali Rohár, Marek Behún

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?

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@denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
  2021-11-12 13:44 ` Stefan Roese
@ 2021-11-12 15:23   ` Pali Rohár
  2021-11-12 16:19     ` Stefan Roese
  2021-11-12 16:31   ` Tom Rini
  1 sibling, 1 reply; 8+ messages in thread
From: Pali Rohár @ 2021-11-12 15:23 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot, Marek Behún

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@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?

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@denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
  2021-11-12 15:23   ` Pali Rohár
@ 2021-11-12 16:19     ` Stefan Roese
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Roese @ 2021-11-12 16:19 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot, Marek Behún

On 11/12/21 16:23, Pali Rohár wrote:
> 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@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?
> 
> 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.

This was my feeling as well. But I wanted to be sure here, before adding
this code to one board only, that would "look better" in the watchdog
driver (no defined for the reg offsets etc). But if this will never be
used in other board, of which I'm pretty sure of, then let's stick with
this current board specific implementation. Especially if it's also
quite complex to configure this specific setup in the common WDT driver.

Thanks anyways for looking into it,
Stefan

>> 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@denx.de

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@denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
  2021-11-12 13:44 ` Stefan Roese
  2021-11-12 15:23   ` Pali Rohár
@ 2021-11-12 16:31   ` Tom Rini
  2021-11-12 16:40     ` Pali Rohár
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Rini @ 2021-11-12 16:31 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot, Pali Rohár, Marek Behún

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

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?  Or have I gone crazy and we
don't do that on say OMAP?

-- 
Tom

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
  2021-11-12 16:31   ` Tom Rini
@ 2021-11-12 16:40     ` Pali Rohár
  2021-11-12 16:48       ` Tom Rini
  0 siblings, 1 reply; 8+ messages in thread
From: Pali Rohár @ 2021-11-12 16:40 UTC (permalink / raw)
  To: Tom Rini; +Cc: Stefan Roese, Marek Behún, u-boot, Marek Behún

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.

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.

> 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.

> -- 
> Tom



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
  2021-11-12 16:40     ` Pali Rohár
@ 2021-11-12 16:48       ` Tom Rini
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Rini @ 2021-11-12 16:48 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Stefan Roese, Marek Behún, u-boot, Marek Behún

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog
  2021-11-09 16:14 [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog Marek Behún
  2021-11-12 13:44 ` Stefan Roese
@ 2021-11-30 13:38 ` Stefan Roese
  1 sibling, 0 replies; 8+ messages in thread
From: Stefan Roese @ 2021-11-30 13:38 UTC (permalink / raw)
  To: Marek Behún; +Cc: u-boot, Pali Rohár, Marek Behún

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>

Applied to u-boot-marvell/master

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@denx.de

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-11-30 13:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 16:14 [PATCH u-boot-marvell] arm: mvebu: turris_omnia: enable A385 watchdog before disabling MCU watchdog Marek Behún
2021-11-12 13:44 ` Stefan Roese
2021-11-12 15:23   ` Pali Rohár
2021-11-12 16:19     ` Stefan Roese
2021-11-12 16:31   ` Tom Rini
2021-11-12 16:40     ` Pali Rohár
2021-11-12 16:48       ` Tom Rini
2021-11-30 13:38 ` Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.