[U-Boot] [PATCH 1/3] bootm: Handle errors consistently
A recent bootm fix left the error path incomplete. Reinstate this so that failures in bootm stages are handled properly.
Signed-off-by: Simon Glass sjg@chromium.org --- common/cmd_bootm.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 02a5013..f4df0a0 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -684,12 +684,8 @@ static int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, if (!ret && (states & BOOTM_STATE_OS_GO)) { ret = boot_selected_os(argc, argv, BOOTM_STATE_OS_GO, images, boot_fn); - if (ret) - goto err; }
- return ret; - /* Deal with any fallout */ err: if (iflag)
With the move of the interrupt code to earlier in the sequence, we exposed a problem where the interrupts are disabled at each bootm stage. This is not correct - it should be done only once. Let's disable interrupts in the LOAD stage. Put the code in a function for clarity.
Also, bootz lost its interrupt code altogether, so reinstate it.
Signed-off-by: Simon Glass sjg@chromium.org --- common/cmd_bootm.c | 67 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 24 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index f4df0a0..401055d 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -539,6 +539,42 @@ static int boot_selected_os(int argc, char * const argv[], int state, }
/** + * bootm_disable_interrupts() - Disable interrupts in preparation for load/boot + * + * @return interrupt flag (0 if interrupts were disabled, non-zero if they were + * enabled) + */ +static ulong bootm_disable_interrupts(void) +{ + ulong iflag; + + /* + * We have reached the point of no return: we are going to + * overwrite all exception vector code, so we cannot easily + * recover from any failures any more... + */ + iflag = disable_interrupts(); +#ifdef CONFIG_NETCONSOLE + /* Stop the ethernet stack if NetConsole could have left it up */ + eth_halt(); +#endif + +#if defined(CONFIG_CMD_USB) + /* + * turn off USB to prevent the host controller from writing to the + * SDRAM while Linux is booting. This could happen (at least for OHCI + * controller), because the HCCA (Host Controller Communication Area) + * lies within the SDRAM and the host controller writes continously to + * this area (as busmaster!). The HccaFrameNumber is for example + * updated every 1 ms within the HCCA structure in SDRAM! For more + * details see the OpenHCI specification. + */ + usb_stop(); +#endif + return iflag; +} + +/** * Execute selected states of the bootm command. * * Note the arguments to this state must be the first argument, Any 'bootm' @@ -588,34 +624,11 @@ static int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, argc = 0; /* consume the args */ }
- /* - * We have reached the point of no return: we are going to - * overwrite all exception vector code, so we cannot easily - * recover from any failures any more... - */ - iflag = disable_interrupts(); -#ifdef CONFIG_NETCONSOLE - /* Stop the ethernet stack if NetConsole could have left it up */ - eth_halt(); -#endif - -#if defined(CONFIG_CMD_USB) - /* - * turn off USB to prevent the host controller from writing to the - * SDRAM while Linux is booting. This could happen (at least for OHCI - * controller), because the HCCA (Host Controller Communication Area) - * lies within the SDRAM and the host controller writes continously to - * this area (as busmaster!). The HccaFrameNumber is for example - * updated every 1 ms within the HCCA structure in SDRAM! For more - * details see the OpenHCI specification. - */ - usb_stop(); -#endif - /* Load the OS */ if (!ret && (states & BOOTM_STATE_LOADOS)) { ulong load_end;
+ iflag = bootm_disable_interrupts(); ret = bootm_load_os(images, &load_end, 0); if (ret && ret != BOOTM_ERR_OVERLAP) goto err; @@ -1774,6 +1787,12 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (bootz_start(cmdtp, flag, argc, argv, &images)) return 1;
+ /* + * We are doing the BOOTM_STATE_LOADOS state ourselves, so must + * disable interrupts ourselves + */ + bootm_disable_interrupts(); + ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO, &images, 1);
In the recent bootm refactor, the PREP stage was missing in the bootz command. This causes unpredictable behaviour on platforms which need this stage to operate correctly (e.g. ARM).
Signed-off-by: Simon Glass sjg@chromium.org --- common/cmd_bootm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 401055d..26ed7d8 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1794,7 +1794,8 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) bootm_disable_interrupts();
ret = do_bootm_states(cmdtp, flag, argc, argv, - BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO, + BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO | + BOOTM_STATE_OS_GO, &images, 1);
return ret;
On Wed, Jul 3, 2013 at 9:12 AM, Simon Glass sjg@chromium.org wrote:
In the recent bootm refactor, the PREP stage was missing in the bootz command. This causes unpredictable behaviour on platforms which need this stage to operate correctly (e.g. ARM).
Signed-off-by: Simon Glass sjg@chromium.org
common/cmd_bootm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 401055d..26ed7d8 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1794,7 +1794,8 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) bootm_disable_interrupts();
ret = do_bootm_states(cmdtp, flag, argc, argv,
BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO,
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |BOOTM_STATE_OS_GO, &images, 1); return ret;-- 1.8.3
Still no dice.. v2013.07-rc2 + these 3 + Tom's "cmd_bootm.c: Correct check/return for unsupported sub-command"
Tested with the Panda/Wand..
Panda: bootz test: load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage run mmcargs bootz ${loadaddr}
Panda: bootm test: (this still works fine..) load mmc ${mmcdev}:${mmcpart} ${loadaddr} uImage run mmcargs bootm ${loadaddr}
U-Boot SPL 2013.07-rc2-00004-gb3e6fff-dirty (Jul 03 2013 - 09:33:36) OMAP4430 ES2.1 OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img
U-Boot 2013.07-rc2-00004-gb3e6fff-dirty (Jul 03 2013 - 09:33:36)
CPU : OMAP4430 ES2.1 Board: OMAP4 Panda I2C: ready DRAM: 1 GiB MMC: OMAP SD/MMC: 0 Using default environment
In: serial Out: serial Err: serial Net: No ethernet found. Hit any key to stop autoboot: 0 Panda # load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage reading zImage 3413152 bytes read in 160 ms (20.3 MiB/s) Panda # run mmcargs Panda # bootz ${loadaddr} prefetch abort pc : [<10da7a5c>] lr : [<bff813f1>] sp : bfefdba0 ip : 7fe00fa8 fp : 00000000 r10: bfefe6a0 r9 : 00000002 r8 : bfefdf38 r7 : 80000000 r6 : 00000700 r5 : 10da7a5a r4 : bfefdc18 r3 : bfefdc18 r2 : bfefe6a0 r1 : 00000002 r0 : 00000100 Flags: NzCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
Regards,
On Wed, Jul 3, 2013 at 9:46 AM, Robert Nelson robertcnelson@gmail.com wrote:
On Wed, Jul 3, 2013 at 9:12 AM, Simon Glass sjg@chromium.org wrote:
In the recent bootm refactor, the PREP stage was missing in the bootz command. This causes unpredictable behaviour on platforms which need this stage to operate correctly (e.g. ARM).
Signed-off-by: Simon Glass sjg@chromium.org
common/cmd_bootm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 401055d..26ed7d8 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1794,7 +1794,8 @@ int do_bootz(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) bootm_disable_interrupts();
ret = do_bootm_states(cmdtp, flag, argc, argv,
BOOTM_STATE_OS_FAKE_GO | BOOTM_STATE_OS_GO,
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |BOOTM_STATE_OS_GO, &images, 1); return ret;-- 1.8.3
Still no dice.. v2013.07-rc2 + these 3 + Tom's "cmd_bootm.c: Correct check/return for unsupported sub-command"
Tested with the Panda/Wand..
Panda: bootz test: load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage run mmcargs bootz ${loadaddr}
Panda: bootm test: (this still works fine..) load mmc ${mmcdev}:${mmcpart} ${loadaddr} uImage run mmcargs bootm ${loadaddr}
U-Boot SPL 2013.07-rc2-00004-gb3e6fff-dirty (Jul 03 2013 - 09:33:36) OMAP4430 ES2.1 OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img
U-Boot 2013.07-rc2-00004-gb3e6fff-dirty (Jul 03 2013 - 09:33:36)
CPU : OMAP4430 ES2.1 Board: OMAP4 Panda I2C: ready DRAM: 1 GiB MMC: OMAP SD/MMC: 0 Using default environment
In: serial Out: serial Err: serial Net: No ethernet found. Hit any key to stop autoboot: 0 Panda # load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage reading zImage 3413152 bytes read in 160 ms (20.3 MiB/s) Panda # run mmcargs Panda # bootz ${loadaddr} prefetch abort pc : [<10da7a5c>] lr : [<bff813f1>] sp : bfefdba0 ip : 7fe00fa8 fp : 00000000 r10: bfefe6a0 r9 : 00000002 r8 : bfefdf38 r7 : 80000000 r6 : 00000700 r5 : 10da7a5a r4 : bfefdc18 r3 : bfefdc18 r2 : bfefe6a0 r1 : 00000002 r0 : 00000100 Flags: NzCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
Okay, so my quick hack from yesterday now works on top of these 3 patches for the Panda case:
U-Boot SPL 2013.07-rc2-00005-g8d900ea-dirty (Jul 03 2013 - 09:52:08) OMAP4430 ES2.1 OMAP SD/MMC: 0 reading u-boot.img reading u-boot.img
U-Boot 2013.07-rc2-00005-g8d900ea-dirty (Jul 03 2013 - 09:52:08)
CPU : OMAP4430 ES2.1 Board: OMAP4 Panda I2C: ready DRAM: 1 GiB MMC: OMAP SD/MMC: 0 Using default environment
In: serial Out: serial Err: serial Net: No ethernet found. Hit any key to stop autoboot: 0 Panda # load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage reading zImage 3413152 bytes read in 159 ms (20.5 MiB/s) Panda # run mmcargs Panda # bootz ${loadaddr}
Starting kernel ...
Uncompressing Linux... done, booting the kernel. [ 0.000000] Booting Linux on physical CPU 0 [ 0.000000] Initializing cgroup subsys cpuset [ 0.000000] Initializing cgroup subsys cpu [ 0.000000] Linux version 3.7.10-x12 (root@imx6q-sabrelite-1gb-0) (gcc version 4.6.3 (Debian 4.6.3-14) ) #1 SMP Sun Jun 9 03:19:23 UTC 2013
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 7b3e459..5749057 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1758,6 +1758,10 @@ static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, int ret; void *zi_start, *zi_end;
+ memset(images, 0, sizeof(bootm_headers_t)); + boot_start_lmb(images); + images->os.os = IH_OS_LINUX; + ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START, images, 1);
So I'm going to quickly re-test the wand, which is a device tree: 'bootz zImage - ftd" boot...
Regards,
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 7b3e459..5749057 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1758,6 +1758,10 @@ static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, int ret; void *zi_start, *zi_end;
memset(images, 0, sizeof(bootm_headers_t));boot_start_lmb(images);images->os.os = IH_OS_LINUX;ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START, images, 1);So I'm going to quickly re-test the wand, which is a device tree: 'bootz zImage - ftd" boot...
Nope close half way there.. Still broken on Wand (device tree)
Board file boot fine..
bootz zImage bootz zImage initrd.img
The device tree cases, lock up.. bootz zImage - device.dtb bootz zImage initrd.img device.dtb
Environment size: 2316/8188 bytes => load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage 4109672 bytes read in 310 ms (12.6 MiB/s) => load mmc ${mmcdev}:${mmcpart} ${fdt_addr} /dtbs/${fdt_file} 22150 bytes read in 259 ms (83 KiB/s) => run mmcargs => bootz ${loadaddr} - ${fdt_addr}
Starting kernel ...
Regards,
Hi Robert,
On Thu, Jul 4, 2013 at 12:06 AM, Robert Nelson robertcnelson@gmail.comwrote:
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 7b3e459..5749057 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1758,6 +1758,10 @@ static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc, int ret; void *zi_start, *zi_end;
memset(images, 0, sizeof(bootm_headers_t));boot_start_lmb(images);images->os.os = IH_OS_LINUX;ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START, images, 1);So I'm going to quickly re-test the wand, which is a device tree: 'bootz zImage - ftd" boot...
Nope close half way there.. Still broken on Wand (device tree)
Board file boot fine..
bootz zImage bootz zImage initrd.img
The device tree cases, lock up.. bootz zImage - device.dtb bootz zImage initrd.img device.dtb
Environment size: 2316/8188 bytes => load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage 4109672 bytes read in 310 ms (12.6 MiB/s) => load mmc ${mmcdev}:${mmcpart} ${fdt_addr} /dtbs/${fdt_file} 22150 bytes read in 259 ms (83 KiB/s) => run mmcargs => bootz ${loadaddr} - ${fdt_addr}
Starting kernel ...
I hope to be able to test this on an ARM platform tomorrow (unfortunately I am travelling and don't have the right setup).
In the meantime I have done some testing with sandbox and will send out some v2 patches which correct two more errors. I finally realised that when I tested this I was probably using zboot instead of bootz. Despite the obvious difference it somehow escaped me.
Regards, Simon
participants (2)
-
Robert Nelson -
Simon Glass