[U-Boot] [PATCH] Fix bootm to work on powerpc again (compressed uImage)

[U-Boot] [PATCH] Fix bootm to work on powerpc again (compressed uImage)

[Stefan Roese]

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

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

[U-Boot] [PATCH] Fix bootm to work on powerpc again (compressed uImage)

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130628/6f81433d/attachment.pgp>

[U-Boot] [PATCH] Fix bootm to work on powerpc again (compressed uImage)

[Stefan Roese]

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

[U-Boot] [PATCH] Fix bootm to work on powerpc again (compressed uImage)

[Simon Glass]

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

[U-Boot] [PATCH] Fix bootm to work on powerpc again (compressed uImage)

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130628/ed02a9b2/attachment.pgp>