diff mbox

[U-Boot,RFC,v2] bootm: fix passing argc to standalone apps

Message ID 1473409138-54696-1-git-send-email-Zubair.Kakakhel@imgtec.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Zubair Lutfullah Kakakhel Sept. 9, 2016, 8:18 a.m. UTC
This bug appears in b6396403 which makes u-boot unable to pass
arguments via bootm to a standalone application without this patch.

Steps to reproduce.

Compile a u-boot. Use mkimage to package the standalone hello_world.bin
file.

e.g. For the MIPS Boston platform

mkimage -n "hello" -A mips -O u-boot -C none -T standalone \
     -a 0xffffffff80200000 -d hello_world.bin \
     -ep 0xffffffff80200000 hello_out

Then tftp hello_out and run it using

boston # dhcp 192.168.154.45:hello_out
...
boston # bootm $loadaddr 123 321

Without the patch the following output is observed.

boston # bootm $loadaddr 123 321
   Image Name:   hello
   Image Type:   MIPS U-Boot Standalone Program (uncompressed)
   Data Size:    1240 Bytes = 1.2 KiB
   Load Address: 80200000
   Entry Point:  80200000
   Verifying Checksum ... OK
   Loading Standalone Program ... OK
Example expects ABI version 8
Actual U-Boot ABI version 8
Hello World
argc = 0
argv[0] = "0xffffffff88000000"

With the patch, you see the following.

boston # bootm $loadaddr 123 321
   Image Name:   hello
   Image Type:   MIPS U-Boot Standalone Program (uncompressed)
   Data Size:    1240 Bytes = 1.2 KiB
   Load Address: 80200000
   Entry Point:  80200000
   Verifying Checksum ... OK
   Loading Standalone Program ... OK
Example expects ABI version 8
Actual U-Boot ABI version 8
Hello World
argc = 3
argv[0] = "0xffffffff88000000"
argv[1] = "123"
argv[2] = "321"
argv[3] = "<NULL>"

Without the patch, the go command at the entry point seems to work.

boston # go 0xffffffff80200000 123 321
Example expects ABI version 8
Actual U-Boot ABI version 8
Hello World
argc = 3
argv[0] = "0xffffffff80200000"
argv[1] = "123"
argv[2] = "321"
argv[3] = "<NULL>"
Hit any key to exit ...

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---

Tested on the MIPS Boston platform.
---
 common/bootm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Simon Glass Sept. 19, 2016, 12:57 a.m. UTC | #1
On 9 September 2016 at 02:18, Zubair Lutfullah Kakakhel
<Zubair.Kakakhel@imgtec.com> wrote:
> This bug appears in b6396403 which makes u-boot unable to pass
> arguments via bootm to a standalone application without this patch.
>
> Steps to reproduce.
>
> Compile a u-boot. Use mkimage to package the standalone hello_world.bin
> file.
>
> e.g. For the MIPS Boston platform
>
> mkimage -n "hello" -A mips -O u-boot -C none -T standalone \
>      -a 0xffffffff80200000 -d hello_world.bin \
>      -ep 0xffffffff80200000 hello_out
>
> Then tftp hello_out and run it using
>
> boston # dhcp 192.168.154.45:hello_out
> ...
> boston # bootm $loadaddr 123 321
>
> Without the patch the following output is observed.
>
> boston # bootm $loadaddr 123 321
>    Image Name:   hello
>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>    Data Size:    1240 Bytes = 1.2 KiB
>    Load Address: 80200000
>    Entry Point:  80200000
>    Verifying Checksum ... OK
>    Loading Standalone Program ... OK
> Example expects ABI version 8
> Actual U-Boot ABI version 8
> Hello World
> argc = 0
> argv[0] = "0xffffffff88000000"
>
> With the patch, you see the following.
>
> boston # bootm $loadaddr 123 321
>    Image Name:   hello
>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>    Data Size:    1240 Bytes = 1.2 KiB
>    Load Address: 80200000
>    Entry Point:  80200000
>    Verifying Checksum ... OK
>    Loading Standalone Program ... OK
> Example expects ABI version 8
> Actual U-Boot ABI version 8
> Hello World
> argc = 3
> argv[0] = "0xffffffff88000000"
> argv[1] = "123"
> argv[2] = "321"
> argv[3] = "<NULL>"
>
> Without the patch, the go command at the entry point seems to work.
>
> boston # go 0xffffffff80200000 123 321
> Example expects ABI version 8
> Actual U-Boot ABI version 8
> Hello World
> argc = 3
> argv[0] = "0xffffffff80200000"
> argv[1] = "123"
> argv[2] = "321"
> argv[3] = "<NULL>"
> Hit any key to exit ...
>
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>
> ---
>
> Tested on the MIPS Boston platform.
> ---
>  common/bootm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index e6da551..a26ada4 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -602,10 +602,8 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>         if (!ret && (states & BOOTM_STATE_FINDOS))
>                 ret = bootm_find_os(cmdtp, flag, argc, argv);
>
> -       if (!ret && (states & BOOTM_STATE_FINDOTHER)) {
> +       if (!ret && (states & BOOTM_STATE_FINDOTHER))
>                 ret = bootm_find_other(cmdtp, flag, argc, argv);
> -               argc = 0;       /* consume the args */
> -       }
>
>         /* Load the OS */
>         if (!ret && (states & BOOTM_STATE_LOADOS)) {
> --
> 1.9.1
>

Reviewed-by: Simon Glass <sjg@chromium.org>
Zubair Lutfullah Kakakhel Sept. 23, 2016, 10:17 a.m. UTC | #2
Hi,

comments at end

On 09/19/2016 01:57 AM, Simon Glass wrote:
> On 9 September 2016 at 02:18, Zubair Lutfullah Kakakhel
> <Zubair.Kakakhel@imgtec.com> wrote:
>> This bug appears in b6396403 which makes u-boot unable to pass
>> arguments via bootm to a standalone application without this patch.
>>
>> Steps to reproduce.
>>
>> Compile a u-boot. Use mkimage to package the standalone hello_world.bin
>> file.
>>
>> e.g. For the MIPS Boston platform
>>
>> mkimage -n "hello" -A mips -O u-boot -C none -T standalone \
>>      -a 0xffffffff80200000 -d hello_world.bin \
>>      -ep 0xffffffff80200000 hello_out
>>
>> Then tftp hello_out and run it using
>>
>> boston # dhcp 192.168.154.45:hello_out
>> ...
>> boston # bootm $loadaddr 123 321
>>
>> Without the patch the following output is observed.
>>
>> boston # bootm $loadaddr 123 321
>>    Image Name:   hello
>>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>>    Data Size:    1240 Bytes = 1.2 KiB
>>    Load Address: 80200000
>>    Entry Point:  80200000
>>    Verifying Checksum ... OK
>>    Loading Standalone Program ... OK
>> Example expects ABI version 8
>> Actual U-Boot ABI version 8
>> Hello World
>> argc = 0
>> argv[0] = "0xffffffff88000000"
>>
>> With the patch, you see the following.
>>
>> boston # bootm $loadaddr 123 321
>>    Image Name:   hello
>>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>>    Data Size:    1240 Bytes = 1.2 KiB
>>    Load Address: 80200000
>>    Entry Point:  80200000
>>    Verifying Checksum ... OK
>>    Loading Standalone Program ... OK
>> Example expects ABI version 8
>> Actual U-Boot ABI version 8
>> Hello World
>> argc = 3
>> argv[0] = "0xffffffff88000000"
>> argv[1] = "123"
>> argv[2] = "321"
>> argv[3] = "<NULL>"
>>
>> Without the patch, the go command at the entry point seems to work.
>>
>> boston # go 0xffffffff80200000 123 321
>> Example expects ABI version 8
>> Actual U-Boot ABI version 8
>> Hello World
>> argc = 3
>> argv[0] = "0xffffffff80200000"
>> argv[1] = "123"
>> argv[2] = "321"
>> argv[3] = "<NULL>"
>> Hit any key to exit ...
>>
>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>>
>> ---
>>
>> Tested on the MIPS Boston platform.
>> ---
>>  common/bootm.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/common/bootm.c b/common/bootm.c
>> index e6da551..a26ada4 100644
>> --- a/common/bootm.c
>> +++ b/common/bootm.c
>> @@ -602,10 +602,8 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>>         if (!ret && (states & BOOTM_STATE_FINDOS))
>>                 ret = bootm_find_os(cmdtp, flag, argc, argv);
>>
>> -       if (!ret && (states & BOOTM_STATE_FINDOTHER)) {
>> +       if (!ret && (states & BOOTM_STATE_FINDOTHER))
>>                 ret = bootm_find_other(cmdtp, flag, argc, argv);
>> -               argc = 0;       /* consume the args */
>> -       }
>>
>>         /* Load the OS */
>>         if (!ret && (states & BOOTM_STATE_LOADOS)) {
>> --
>> 1.9.1
>>
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>

Thanks for the review.

Can this be applied as is or do I need to send it again without the RFC
and as a Patch?

Regards,
ZubairLK
Simon Glass Sept. 28, 2016, 5:51 p.m. UTC | #3
Hi,

On 23 September 2016 at 04:17, Zubair Lutfullah Kakakhel
<Zubair.Kakakhel@imgtec.com> wrote:
> Hi,
>
> comments at end
>
>
> On 09/19/2016 01:57 AM, Simon Glass wrote:
>>
>> On 9 September 2016 at 02:18, Zubair Lutfullah Kakakhel
>> <Zubair.Kakakhel@imgtec.com> wrote:
>>>
>>> This bug appears in b6396403 which makes u-boot unable to pass
>>> arguments via bootm to a standalone application without this patch.
>>>
>>> Steps to reproduce.
>>>
>>> Compile a u-boot. Use mkimage to package the standalone hello_world.bin
>>> file.
>>>
>>> e.g. For the MIPS Boston platform
>>>
>>> mkimage -n "hello" -A mips -O u-boot -C none -T standalone \
>>>      -a 0xffffffff80200000 -d hello_world.bin \
>>>      -ep 0xffffffff80200000 hello_out
>>>
>>> Then tftp hello_out and run it using
>>>
>>> boston # dhcp 192.168.154.45:hello_out
>>> ...
>>> boston # bootm $loadaddr 123 321
>>>
>>> Without the patch the following output is observed.
>>>
>>> boston # bootm $loadaddr 123 321
>>>    Image Name:   hello
>>>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>>>    Data Size:    1240 Bytes = 1.2 KiB
>>>    Load Address: 80200000
>>>    Entry Point:  80200000
>>>    Verifying Checksum ... OK
>>>    Loading Standalone Program ... OK
>>> Example expects ABI version 8
>>> Actual U-Boot ABI version 8
>>> Hello World
>>> argc = 0
>>> argv[0] = "0xffffffff88000000"
>>>
>>> With the patch, you see the following.
>>>
>>> boston # bootm $loadaddr 123 321
>>>    Image Name:   hello
>>>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>>>    Data Size:    1240 Bytes = 1.2 KiB
>>>    Load Address: 80200000
>>>    Entry Point:  80200000
>>>    Verifying Checksum ... OK
>>>    Loading Standalone Program ... OK
>>> Example expects ABI version 8
>>> Actual U-Boot ABI version 8
>>> Hello World
>>> argc = 3
>>> argv[0] = "0xffffffff88000000"
>>> argv[1] = "123"
>>> argv[2] = "321"
>>> argv[3] = "<NULL>"
>>>
>>> Without the patch, the go command at the entry point seems to work.
>>>
>>> boston # go 0xffffffff80200000 123 321
>>> Example expects ABI version 8
>>> Actual U-Boot ABI version 8
>>> Hello World
>>> argc = 3
>>> argv[0] = "0xffffffff80200000"
>>> argv[1] = "123"
>>> argv[2] = "321"
>>> argv[3] = "<NULL>"
>>> Hit any key to exit ...
>>>
>>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>>>
>>> ---
>>>
>>> Tested on the MIPS Boston platform.
>>> ---
>>>  common/bootm.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/common/bootm.c b/common/bootm.c
>>> index e6da551..a26ada4 100644
>>> --- a/common/bootm.c
>>> +++ b/common/bootm.c
>>> @@ -602,10 +602,8 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int
>>> argc, char * const argv[],
>>>         if (!ret && (states & BOOTM_STATE_FINDOS))
>>>                 ret = bootm_find_os(cmdtp, flag, argc, argv);
>>>
>>> -       if (!ret && (states & BOOTM_STATE_FINDOTHER)) {
>>> +       if (!ret && (states & BOOTM_STATE_FINDOTHER))
>>>                 ret = bootm_find_other(cmdtp, flag, argc, argv);
>>> -               argc = 0;       /* consume the args */
>>> -       }
>>>
>>>         /* Load the OS */
>>>         if (!ret && (states & BOOTM_STATE_LOADOS)) {
>>> --
>>> 1.9.1
>>>
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>
> Thanks for the review.
>
> Can this be applied as is or do I need to send it again without the RFC
> and as a Patch?

I think it is fine as is.

Regards,
Simon
Tom Rini Oct. 3, 2016, 1:36 p.m. UTC | #4
On Fri, Sep 09, 2016 at 09:18:58AM +0100, Zubair Lutfullah Kakakhel wrote:

> This bug appears in b6396403 which makes u-boot unable to pass
> arguments via bootm to a standalone application without this patch.
> 
> Steps to reproduce.
> 
> Compile a u-boot. Use mkimage to package the standalone hello_world.bin
> file.
> 
> e.g. For the MIPS Boston platform
> 
> mkimage -n "hello" -A mips -O u-boot -C none -T standalone \
>      -a 0xffffffff80200000 -d hello_world.bin \
>      -ep 0xffffffff80200000 hello_out
> 
> Then tftp hello_out and run it using
> 
> boston # dhcp 192.168.154.45:hello_out
> ...
> boston # bootm $loadaddr 123 321
> 
> Without the patch the following output is observed.
> 
> boston # bootm $loadaddr 123 321
>    Image Name:   hello
>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>    Data Size:    1240 Bytes = 1.2 KiB
>    Load Address: 80200000
>    Entry Point:  80200000
>    Verifying Checksum ... OK
>    Loading Standalone Program ... OK
> Example expects ABI version 8
> Actual U-Boot ABI version 8
> Hello World
> argc = 0
> argv[0] = "0xffffffff88000000"
> 
> With the patch, you see the following.
> 
> boston # bootm $loadaddr 123 321
>    Image Name:   hello
>    Image Type:   MIPS U-Boot Standalone Program (uncompressed)
>    Data Size:    1240 Bytes = 1.2 KiB
>    Load Address: 80200000
>    Entry Point:  80200000
>    Verifying Checksum ... OK
>    Loading Standalone Program ... OK
> Example expects ABI version 8
> Actual U-Boot ABI version 8
> Hello World
> argc = 3
> argv[0] = "0xffffffff88000000"
> argv[1] = "123"
> argv[2] = "321"
> argv[3] = "<NULL>"
> 
> Without the patch, the go command at the entry point seems to work.
> 
> boston # go 0xffffffff80200000 123 321
> Example expects ABI version 8
> Actual U-Boot ABI version 8
> Hello World
> argc = 3
> argv[0] = "0xffffffff80200000"
> argv[1] = "123"
> argv[2] = "321"
> argv[3] = "<NULL>"
> Hit any key to exit ...
> 
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/common/bootm.c b/common/bootm.c
index e6da551..a26ada4 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -602,10 +602,8 @@  int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 	if (!ret && (states & BOOTM_STATE_FINDOS))
 		ret = bootm_find_os(cmdtp, flag, argc, argv);
 
-	if (!ret && (states & BOOTM_STATE_FINDOTHER)) {
+	if (!ret && (states & BOOTM_STATE_FINDOTHER))
 		ret = bootm_find_other(cmdtp, flag, argc, argv);
-		argc = 0;	/* consume the args */
-	}
 
 	/* Load the OS */
 	if (!ret && (states & BOOTM_STATE_LOADOS)) {