| Message ID | 1372406523-5726-1-git-send-email-sr@denx.de |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On Fri, Jun 28, 2013 at 10:02:03AM +0200, Stefan Roese wrote: > Patch 35fc84fa1 [Refactor the bootm command to reduce code duplication] > breaks booting Linux (compressed uImage with fdt) on powerpc. > > boot_jump_linux() mustn't be called before boot_prep_linux() and > boot_body_linux() have been called. So remove the superfluous call > to boot_jump_linux() in arch/powerpc/lib/bootm.c as its called later on > in this function. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@ti.com> > --- > Simon, I'm not 100% sure why this is triggered by your patch. > > As you see, my patch only fixes powerpc. ARM might have the same problem > as it has the same code duplication in bootm.c. Progress! On PowerPC, does bootm work like before? I still have to issue a separate 'bootm go', but that now boots, rather than hangs.
On 06/28/2013 03:03 PM, Tom Rini wrote: >> Patch 35fc84fa1 [Refactor the bootm command to reduce code duplication] >> breaks booting Linux (compressed uImage with fdt) on powerpc. >> >> boot_jump_linux() mustn't be called before boot_prep_linux() and >> boot_body_linux() have been called. So remove the superfluous call >> to boot_jump_linux() in arch/powerpc/lib/bootm.c as its called later on >> in this function. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Tom Rini <trini@ti.com> >> --- >> Simon, I'm not 100% sure why this is triggered by your patch. >> >> As you see, my patch only fixes powerpc. ARM might have the same problem >> as it has the same code duplication in bootm.c. > > Progress! On PowerPC, does bootm work like before? Yes. I'm using the same environment (boot_cmd) as before. And without this patch "bootm kernel_addr - fdt_addr' hangs. I might only affect DT booting, as without this patch boot_jump_linux() is called without calling boot_prep_linux() and boot_body_linux() before. One of those (I didn't check which one) is responsible for this line: Loading Device Tree to 007f8000, end 007ffdd4 ... OK And this is missing when this patch is not applied. Thanks, Stefan
Hi Stefan, On Fri, Jun 28, 2013 at 1:02 AM, Stefan Roese <sr@denx.de> wrote: > Patch 35fc84fa1 [Refactor the bootm command to reduce code duplication] > breaks booting Linux (compressed uImage with fdt) on powerpc. > > boot_jump_linux() mustn't be called before boot_prep_linux() and > boot_body_linux() have been called. So remove the superfluous call > to boot_jump_linux() in arch/powerpc/lib/bootm.c as its called later on > in this function. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@ti.com> > Acked-by: Simon Glass <sjg@chromium.org> > --- > Simon, I'm not 100% sure why this is triggered by your patch. > I'm very grateful for your time spent looking at this and getting to the bottom of it! Maybe because do_bootm_linux() is a funny function - you can call it with a mask in which case it will do that step, but if you call it with 0 it will do all steps. Seems like something that should be tidied up. > > As you see, my patch only fixes powerpc. ARM might have the same problem > as it has the same code duplication in bootm.c. > > Thanks, > Stefan > > arch/powerpc/lib/bootm.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c > index dd6c98c..d4ad323 100644 > --- a/arch/powerpc/lib/bootm.c > +++ b/arch/powerpc/lib/bootm.c > @@ -256,11 +256,6 @@ int do_bootm_linux(int flag, int argc, char * const > argv[], bootm_headers_t *ima > return 0; > } > > - if (flag & BOOTM_STATE_OS_GO) { > - boot_jump_linux(images); > - return 0; > - } > - > boot_prep_linux(images); > ret = boot_body_linux(images); > if (ret) > -- > 1.8.3.1 > > Regards, Simon
On Fri, Jun 28, 2013 at 10:02:03AM +0200, Stefan Roese wrote: > Patch 35fc84fa1 [Refactor the bootm command to reduce code duplication] > breaks booting Linux (compressed uImage with fdt) on powerpc. > > boot_jump_linux() mustn't be called before boot_prep_linux() and > boot_body_linux() have been called. So remove the superfluous call > to boot_jump_linux() in arch/powerpc/lib/bootm.c as its called later on > in this function. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Simon Glass <sjg@chromium.org> > Cc: Tom Rini <trini@ti.com> Applied to u-boot/master, thanks!
diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c index dd6c98c..d4ad323 100644 --- a/arch/powerpc/lib/bootm.c +++ b/arch/powerpc/lib/bootm.c @@ -256,11 +256,6 @@ int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *ima return 0; } - if (flag & BOOTM_STATE_OS_GO) { - boot_jump_linux(images); - return 0; - } - boot_prep_linux(images); ret = boot_body_linux(images); if (ret)
Patch 35fc84fa1 [Refactor the bootm command to reduce code duplication] breaks booting Linux (compressed uImage with fdt) on powerpc. boot_jump_linux() mustn't be called before boot_prep_linux() and boot_body_linux() have been called. So remove the superfluous call to boot_jump_linux() in arch/powerpc/lib/bootm.c as its called later on in this function. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Simon Glass <sjg@chromium.org> Cc: Tom Rini <trini@ti.com> --- Simon, I'm not 100% sure why this is triggered by your patch. As you see, my patch only fixes powerpc. ARM might have the same problem as it has the same code duplication in bootm.c. Thanks, Stefan arch/powerpc/lib/bootm.c | 5 ----- 1 file changed, 5 deletions(-)