[PATCH u-boot-mvebu 0/3] turris_omnia: Disable MCU watchdog in SPL when booting over UART
Hello Stefan,
while debugging Omnia, when sending U-Boot over UART, some USB-TTY controllers are too slow and Omnia's MCU watchdog resets the board before U-Boot proper is sent.
This patch series changes Omnia board code so that when booting via UART, we disable MCU watchdog in SPL, prior loading U-Boot proper.
Marek
Marek Behún (3): arm: mvebu: Move get_boot_device() to cpu.c and make visible arm: mvebu: turris_omnia: don't guard by CONFIG_SPL_BUILD macro arm: mvebu: turris_omnia: disable MCU watchdog in SPL when booting over UART
arch/arm/mach-mvebu/cpu.c | 60 ++++++++++++++++++ arch/arm/mach-mvebu/include/mach/cpu.h | 2 + arch/arm/mach-mvebu/spl.c | 77 +++--------------------- board/CZ.NIC/turris_omnia/turris_omnia.c | 25 +++++--- configs/turris_omnia_defconfig | 1 + 5 files changed, 88 insertions(+), 77 deletions(-)
Move the function get_boot_device() from spl.c to cpu.c.
Make it visible, so that it may be used from other files.
Signed-off-by: Marek Behún marek.behun@nic.cz --- arch/arm/mach-mvebu/cpu.c | 60 ++++++++++++++++++++ arch/arm/mach-mvebu/include/mach/cpu.h | 2 + arch/arm/mach-mvebu/spl.c | 77 +++----------------------- 3 files changed, 71 insertions(+), 68 deletions(-)
diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c index 0b935c46fb..daf8bd66a0 100644 --- a/arch/arm/mach-mvebu/cpu.c +++ b/arch/arm/mach-mvebu/cpu.c @@ -14,6 +14,7 @@ #include <asm/pl310.h> #include <asm/arch/cpu.h> #include <asm/arch/soc.h> +#include <asm/spl.h> #include <sdhci.h>
#define DDR_BASE_CS_OFF(n) (0x0000 + ((n) << 3)) @@ -80,6 +81,65 @@ int mvebu_soc_family(void) return MVEBU_SOC_UNKNOWN; }
+u32 get_boot_device(void) +{ + u32 val; + u32 boot_device; + + /* + * First check, if UART boot-mode is active. This can only + * be done, via the bootrom error register. Here the + * MSB marks if the UART mode is active. + */ + val = readl(CONFIG_BOOTROM_ERR_REG); + boot_device = (val & BOOTROM_ERR_MODE_MASK) >> BOOTROM_ERR_MODE_OFFS; + debug("BOOTROM_REG=0x%08x boot_device=0x%x\n", val, boot_device); + if (boot_device == BOOTROM_ERR_MODE_UART) + return BOOT_DEVICE_UART; + +#ifdef CONFIG_ARMADA_38X + /* + * If the bootrom error code contains any other than zeros it's an + * error condition and the bootROM has fallen back to UART boot + */ + boot_device = (val & BOOTROM_ERR_CODE_MASK) >> BOOTROM_ERR_CODE_OFFS; + if (boot_device) + return BOOT_DEVICE_UART; +#endif + + /* + * Now check the SAR register for the strapped boot-device + */ + val = readl(CONFIG_SAR_REG); /* SAR - Sample At Reset */ + boot_device = (val & BOOT_DEV_SEL_MASK) >> BOOT_DEV_SEL_OFFS; + debug("SAR_REG=0x%08x boot_device=0x%x\n", val, boot_device); + switch (boot_device) { +#ifdef BOOT_FROM_NAND + case BOOT_FROM_NAND: + return BOOT_DEVICE_NAND; +#endif +#ifdef BOOT_FROM_MMC + case BOOT_FROM_MMC: + case BOOT_FROM_MMC_ALT: + return BOOT_DEVICE_MMC1; +#endif + case BOOT_FROM_UART: +#ifdef BOOT_FROM_UART_ALT + case BOOT_FROM_UART_ALT: +#endif + return BOOT_DEVICE_UART; +#ifdef BOOT_FROM_SATA + case BOOT_FROM_SATA: + case BOOT_FROM_SATA_ALT: + return BOOT_DEVICE_SATA; +#endif + case BOOT_FROM_SPI: + return BOOT_DEVICE_SPI; + default: + return BOOT_DEVICE_BOOTROM; + }; +} + #if defined(CONFIG_DISPLAY_CPUINFO)
#if defined(CONFIG_ARMADA_375) diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h index 79858858c2..a7a62c7e7d 100644 --- a/arch/arm/mach-mvebu/include/mach/cpu.h +++ b/arch/arm/mach-mvebu/include/mach/cpu.h @@ -148,6 +148,8 @@ void __noreturn return_to_bootrom(void); int mv_sdh_init(unsigned long regbase, u32 max_clk, u32 min_clk, u32 quirks); #endif
+u32 get_boot_device(void); + void get_sar_freq(struct sar_freq_modes *sar_freq);
/* diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c index f0cf60bb14..8d6d4902f6 100644 --- a/arch/arm/mach-mvebu/spl.c +++ b/arch/arm/mach-mvebu/spl.c @@ -171,74 +171,6 @@ int spl_parse_board_header(struct spl_image_info *spl_image, return 0; }
-static u32 get_boot_device(void) -{ - u32 val; - u32 boot_device; - - /* - * First check, if UART boot-mode is active. This can only - * be done, via the bootrom error register. Here the - * MSB marks if the UART mode is active. - */ - val = readl(CONFIG_BOOTROM_ERR_REG); - boot_device = (val & BOOTROM_ERR_MODE_MASK) >> BOOTROM_ERR_MODE_OFFS; - debug("BOOTROM_REG=0x%08x boot_device=0x%x\n", val, boot_device); - if (boot_device == BOOTROM_ERR_MODE_UART) - return BOOT_DEVICE_UART; - -#ifdef CONFIG_ARMADA_38X - /* - * If the bootrom error code contains any other than zeros it's an - * error condition and the bootROM has fallen back to UART boot - */ - boot_device = (val & BOOTROM_ERR_CODE_MASK) >> BOOTROM_ERR_CODE_OFFS; - if (boot_device) - return BOOT_DEVICE_UART; -#endif - - /* - * Now check the SAR register for the strapped boot-device - */ - val = readl(CONFIG_SAR_REG); /* SAR - Sample At Reset */ - boot_device = (val & BOOT_DEV_SEL_MASK) >> BOOT_DEV_SEL_OFFS; - debug("SAR_REG=0x%08x boot_device=0x%x\n", val, boot_device); - switch (boot_device) { -#ifdef BOOT_FROM_NAND - case BOOT_FROM_NAND: - return BOOT_DEVICE_NAND; -#endif -#ifdef BOOT_FROM_MMC - case BOOT_FROM_MMC: - case BOOT_FROM_MMC_ALT: - return BOOT_DEVICE_MMC1; -#endif - case BOOT_FROM_UART: -#ifdef BOOT_FROM_UART_ALT - case BOOT_FROM_UART_ALT: -#endif - return BOOT_DEVICE_UART; -#ifdef BOOT_FROM_SATA - case BOOT_FROM_SATA: - case BOOT_FROM_SATA_ALT: - return BOOT_DEVICE_SATA; -#endif - case BOOT_FROM_SPI: - return BOOT_DEVICE_SPI; - default: - return BOOT_DEVICE_BOOTROM; - }; -} - -#else - -static u32 get_boot_device(void) -{ - return BOOT_DEVICE_BOOTROM; -} - -#endif - u32 spl_boot_device(void) { u32 boot_device = get_boot_device(); @@ -285,6 +217,15 @@ u32 spl_boot_device(void) } }
+#else + +u32 spl_boot_device(void) +{ + return BOOT_DEVICE_BOOTROM; +} + +#endif + int board_return_to_bootrom(struct spl_image_info *spl_image, struct spl_boot_device *bootdev) {
We do not need to guard code in board_init() and board_late_init() functions with the CONFIG_SPL_BUILD macro, since these functions are not called in SPL.
Signed-off-by: Marek Behún marek.behun@nic.cz --- board/CZ.NIC/turris_omnia/turris_omnia.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index a7e5f56eed..a84a409f43 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -129,7 +129,6 @@ static int omnia_mcu_read(u8 cmd, void *buf, int len) return dm_i2c_read(chip, cmd, buf, len); }
-#ifndef CONFIG_SPL_BUILD static int omnia_mcu_write(u8 cmd, const void *buf, int len) { struct udevice *chip; @@ -158,7 +157,6 @@ static bool disable_mcu_watchdog(void)
return true; } -#endif
static bool omnia_detect_sata(void) { @@ -325,7 +323,6 @@ struct mv_ddr_topology_map *mv_ddr_topology_map_get(void) return &board_topology_map_1g; }
-#ifndef CONFIG_SPL_BUILD static int set_regdomain(void) { struct omnia_eeprom oep; @@ -394,7 +391,6 @@ static void handle_reset_button(void) } } } -#endif
int board_early_init_f(void) { @@ -428,19 +424,15 @@ int board_init(void) /* address of boot parameters */ gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100;
-#ifndef CONFIG_SPL_BUILD disable_mcu_watchdog(); -#endif
return 0; }
int board_late_init(void) { -#ifndef CONFIG_SPL_BUILD set_regdomain(); handle_reset_button(); -#endif pci_init();
return 0;
When booting over UART, sending U-Boot proper may take too much time and MCU watchdog will reset the board before U-Boot proper is loaded.
Better disable MCU watchdog in SPL when booting over UART.
Signed-off-by: Marek Behún marek.behun@nic.cz --- board/CZ.NIC/turris_omnia/turris_omnia.c | 17 ++++++++++++++++- configs/turris_omnia_defconfig | 1 + 2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index a84a409f43..b0391c973d 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -419,12 +419,27 @@ int board_early_init_f(void) return 0; }
+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. + */ + if (get_boot_device() == BOOT_DEVICE_UART) + disable_mcu_watchdog(); +} + int board_init(void) { /* address of boot parameters */ gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100;
- disable_mcu_watchdog(); + /* + * If not booting from UART, MCU watchdog was not disabled in SPL, + * disable it now. + */ + if (get_boot_device() != BOOT_DEVICE_UART) + disable_mcu_watchdog();
return 0; } diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig index cd443ceb30..67f8bdadaf 100644 --- a/configs/turris_omnia_defconfig +++ b/configs/turris_omnia_defconfig @@ -33,6 +33,7 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_MISC_INIT_R=y +CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_I2C=y CONFIG_CMD_MEMTEST=y CONFIG_SYS_ALT_MEMTEST=y
Hi Marek,
On 16.08.21 14:27, Marek Behún wrote:
When booting over UART, sending U-Boot proper may take too much time and MCU watchdog will reset the board before U-Boot proper is loaded.
Better disable MCU watchdog in SPL when booting over UART.
Signed-off-by: Marek Behún marek.behun@nic.cz
board/CZ.NIC/turris_omnia/turris_omnia.c | 17 ++++++++++++++++- configs/turris_omnia_defconfig | 1 + 2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index a84a409f43..b0391c973d 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -419,12 +419,27 @@ int board_early_init_f(void) return 0; }
+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.*/- if (get_boot_device() == BOOT_DEVICE_UART)
disable_mcu_watchdog();+}
- int board_init(void) { /* address of boot parameters */ gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100;
- disable_mcu_watchdog();
- /*
* If not booting from UART, MCU watchdog was not disabled in SPL,* disable it now.*/- if (get_boot_device() != BOOT_DEVICE_UART)
disable_mcu_watchdog();
Why do you disable the MCU watchdog here in U-Boot proper?
Thanks, Stefan
return 0; } diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig index cd443ceb30..67f8bdadaf 100644 --- a/configs/turris_omnia_defconfig +++ b/configs/turris_omnia_defconfig @@ -33,6 +33,7 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_MISC_INIT_R=y +CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_I2C=y CONFIG_CMD_MEMTEST=y CONFIG_SYS_ALT_MEMTEST=y
Viele Grüße, Stefan
On Mon, 16 Aug 2021 14:50:15 +0200 Stefan Roese sr@denx.de wrote:
- disable_mcu_watchdog();
- /*
* If not booting from UART, MCU watchdog was not disabledin SPL,
* disable it now.*/- if (get_boot_device() != BOOT_DEVICE_UART)
disable_mcu_watchdog();Why do you disable the MCU watchdog here in U-Boot proper?
Hi Stefan,
this is just where it was before. But looking at the prompt SoC: MV88F6820-A0 at 1600 MHz DRAM: 2 GiB (800 MHz, 32-bit, 2T, ECC not enabled) Disabling MCU watchdog... disabled WDT: Started with servicing (60s timeout)
it looks like U-Boot's watchdog is being enabled after MCU watchdog. It would be better to enable U-Boot's WDT before disabling MCU watchdog. I will send another patch that moves it to another function, board_late_init for example.
Shall I resend this series with this new patch?
Marek
On 16.08.21 15:01, Marek Behún wrote:
On Mon, 16 Aug 2021 14:50:15 +0200 Stefan Roese sr@denx.de wrote:
- disable_mcu_watchdog();
- /*
* If not booting from UART, MCU watchdog was not disabledin SPL,
* disable it now.*/- if (get_boot_device() != BOOT_DEVICE_UART)
disable_mcu_watchdog();Why do you disable the MCU watchdog here in U-Boot proper?
Hi Stefan,
this is just where it was before. But looking at the prompt SoC: MV88F6820-A0 at 1600 MHz DRAM: 2 GiB (800 MHz, 32-bit, 2T, ECC not enabled) Disabling MCU watchdog... disabled WDT: Started with servicing (60s timeout)
it looks like U-Boot's watchdog is being enabled after MCU watchdog. It would be better to enable U-Boot's WDT before disabling MCU watchdog. I will send another patch that moves it to another function, board_late_init for example.
So the MCU watchdog is some kind of very early WD and only used until the SoC WD is used / enabled?
Shall I resend this series with this new patch?
Yes, please do.
Thanks, Stefan
On Mon, 16 Aug 2021 15:07:33 +0200 Stefan Roese sr@denx.de wrote:
So the MCU watchdog is some kind of very early WD and only used until the SoC WD is used / enabled?
Yes, this is how it was first thought of for Omnia. Looking at the MCU code it seems that the watchdog can be started again after being disabled, so it could theoretically be used as a normal watchdog. The problem is that it cannot be pinged to reset the counter. It has to be stopped and started again, meaning there is a small window when it is disabled. If the system used this watchdog, this small window would make it vulnerable, so I don't think we should use it.
Shall I resend this series with this new patch?
Yes, please do.
Will do.
Marek
On 16.08.21 15:11, Marek Behún wrote:
On Mon, 16 Aug 2021 15:07:33 +0200 Stefan Roese sr@denx.de wrote:
So the MCU watchdog is some kind of very early WD and only used until the SoC WD is used / enabled?
Yes, this is how it was first thought of for Omnia. Looking at the MCU code it seems that the watchdog can be started again after being disabled, so it could theoretically be used as a normal watchdog. The problem is that it cannot be pinged to reset the counter. It has to be stopped and started again, meaning there is a small window when it is disabled. If the system used this watchdog, this small window would make it vulnerable, so I don't think we should use it.
Agreed. With such a WD concept including this "window of vulnerability", it really shouldn't be used.
Thanks, Stefan
Shall I resend this series with this new patch?
Yes, please do.
Will do.
Marek
Viele Grüße, Stefan
participants (2)
-
Marek Behún -
Stefan Roese