| Message ID | 1423439.CION9zEpRf@p2130 |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Dear Christian Eggers Christian Eggers <ceggers <at> gmx.de> writes: > > do_bootm_standanlone() calls ntohl(images->ep) which is wrong because > endianess conversion has already been done: > > do_bootm() > \-do_bootm_states() > +-bootm_find_os() > | \-images.ep = image_get_ep(); > | \-uimage_to_cpu(hdr->ih_ep); > \-boot_selected_os() > \-do_bootm_standanlone() > > Without this conversion the code works correctly at least on ARM9. What is the endian of your ARM9 testbed? I've tested the hello_world standalone image with and without your patch on a T5040 (BE). Behaviour of "bootm" does not change. So I am not sure what problem this patch fixes on PowerPC, if any. The patch might be applicable only to certain archs so you should test it on other archs as well (otherwise this patch might break them). As a reference, the standalone image was created using the commandline: ../../tools/mkimage -A powerpc -O u-boot -T standalone -C none -a 0x10040000 -e 0x10040000 -n test -d hello_world.bin hello_world.img (Yes, I modified CONFIG_STANDALONE_LOAD_ADDR for this test.) (Yes, hello_world was modified to not return for this test.) And executed, on the board: tftp 100000 hello_world.img bootm 0x100000 > Addtionally "appl" need not be dereferenced with the "*" operator. -snipped- > common/cmd_bootm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c > index a59ee95..c507e1d 100644 > --- a/common/cmd_bootm.c > +++ b/common/cmd_bootm.c Also, your patch does not apply on master. -snipped- All the best, Rommel
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index a59ee95..c507e1d 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -514,8 +514,8 @@ static int do_bootm_standalone(int flag, int argc, char * const argv[], setenv_hex("filesize", images->os.image_len); return 0; } - appl = (int (*)(int, char * const []))(ulong)ntohl(images->ep); - (*appl)(argc, argv); + appl = (int (*)(int, char * const []))images->ep; + appl(argc, argv); return 0; }
do_bootm_standanlone() calls ntohl(images->ep) which is wrong because endianess conversion has already been done: do_bootm() \-do_bootm_states() +-bootm_find_os() | \-images.ep = image_get_ep(); | \-uimage_to_cpu(hdr->ih_ep); \-boot_selected_os() \-do_bootm_standanlone() Without this conversion the code works correctly at least on ARM9. Addtionally "appl" need not be dereferenced with the "*" operator. Signed-off-by: Christian Eggers <ceggers@gmx.de> --- Changes in v2: - Improve description why the patch is required - (appl)(...) --> appl(...) Further remarks: It seems there's no real difference between doing "(*func_ptr)(args)" and "func_ptr(args)": --- arm --- kernel_entry = (void (*)(int, int, uint))images->ep; kernel_entry(0, machid, r2) ---/arm --- --- powerpc --- kernel = (void (*)(bd_t *, ulong, ulong, ulong, ulong, ulong, ulong))images->ep; (*kernel)(kbd, initrd_start, initrd_end, cmd_start, cmd_end, 0, 0); ---/powerpc --- See also: http://stackoverflow.com/questions/7518815/function-pointer-automatic-dereferencing common/cmd_bootm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)