introduce genimg_get_kernel_addr()
cover introduce genimg_get_kernel_addr()
When trying to fix the error message issue in pxe sysboot, we found it's need a real kernel address for further image format detection.
So I take out such code from boot_get_kernel() of bootm.c and create the new API functin genimg_get_kernel_addr(). Then convert pxe/sysboot and bootm.c to use it.
Bryan Wu (3):
image: introduce genimg_get_kernel_addr()
pxe: detect image format before calling bootm/bootz
bootm: use genimg_get_kernel_addr()
common/bootm.c | 25 +++++-------------------- common/cmd_pxe.c | 15 +++++++++++---- common/image.c | 43 +++++++++++++++++++++++++++++++++++++++++++ include/image.h | 1 + 4 files changed, 60 insertions(+), 24 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/3 image: introduce genimg_get_kernel_addr()
Kernel address is normally stored as a string argument of bootm or bootz. This function is taken out from boot_get_kernel() of bootm.c, which can be reused by others.
--- common/image.c | 43 +++++++++++++++++++++++++++++++++++++++++++ include/image.h | 1 + 2 files changed, 44 insertions(+)
common/image.c +43 -0
@@ -643,6 +643,49 @@ int genimg_get_comp_id(const char *name)#ifndef USE_HOSTCC /** +* genimg_get_kernel_addr - get the real kernel address +* @img_addr: a string might contain real image address +* +* genimg_get_kernel_addr() get the real kernel start address from a string +* which is normally the first argv of bootm/bootz +* +* returns: +* kernel start address +*/ +ulong genimg_get_kernel_addr (char *const img_addr) +{ +#if defined(CONFIG_FIT ) +const char *fit_uname_config =NULL ; +const char *fit_uname_kernel =NULL ; +#endif + +ulong kernel_addr; + +/* find out kernel image address */ +if (!img_addr) { + kernel_addr = load_addr; +debug ("* kernel: default image load address = 0x%08lx\n" , + load_addr); +#if defined(CONFIG_FIT ) + } else if (fit_parse_conf(img_addr, load_addr, &kernel_addr, + fit_uname_config)) { +debug ("* kernel: config '%s' from image at 0x%08lx\n" , + *fit_uname_config, kernel_addr); + }else if (fit_parse_subimage (img_addr, load_addr, &kernel_addr, + fit_uname_kernel)) { +debug ("* kernel: subimage '%s' from image at 0x%08lx\n" , + *fit_uname_kernel, kernel_addr); +#endif + }else { + kernel_addr =simple_strtoul (img_addr,NULL ,16 ); +debug ("* kernel: cmdline image address = 0x%08lx\n" , + kernel_addr); + } + +return kernel_addr; +} + +/** * genimg_get_format - get image format type * @img_addr: image start address *
include/image.h +1 -0
@@ -424,6 +424,7 @@ enum fit_load_op {#define IMAGE_FORMAT_FIT 0x02/* new, libfdt based format */ #define IMAGE_FORMAT_ANDROID 0x03/* Android boot image */ +ulong genimg_get_kernel_addr (char *const img_addr);int genimg_get_format (const void *img_addr);int genimg_has_config (bootm_headers_t *images);ulong genimg_get_image (ulong img_addr);
Signature · 1 line
1.9.1
PATCH 1/3 image: introduce genimg_get_kernel_addr()
Hi Bryan,
Quoted 17 lines · Bryan Wu
Kernel address is normally stored as a string argument of bootm or bootz. This function is taken out from
boot_get_kernel()ofbootm.c, which can be reused by others.Signed-off-by Bryan Wu <[email protected]>--- common/image.c | 43 +++++++++++++++++++++++++++++++++++++++++++ include/image.h | 1 + 2 files changed, 44 insertions(+)common/image.c +0
-0@@ -643,6 +643,49 @@ int genimg_get_comp_id(const char *name)#ifndef USE_HOSTCC
common/image.c int genimg_get_comp_id(const char *name) · line 654
Quoted 10 lines
common/image.c +9
-0/** +* genimg_get_kernel_addr - get the real kernel address +* @img_addr: a string might contain real image address +* +* genimg_get_kernel_addr() get the real kernel start address from a string +* which is normally the first argv of bootm/bootz +* +* returns: +* kernel start address +*/
I know you are being consistent, but actually I think this should be in the header file - you could move some of the other functions comments there too if you like (in a separate patch).
Also you should document the behaviour when @img_addr is NULL.
common/image.c int genimg_get_comp_id(const char *name) · line 659
Quoted 5 lines
common/image.c +5
-0+ulong genimg_get_kernel_addr (char *const img_addr) +{ +#if defined(CONFIG_FIT ) +const char *fit_uname_config =NULL ; +const char *fit_uname_kernel =NULL ;
I don't think we need the tabs after 'char'.
common/image.c int genimg_get_comp_id(const char *name) · line 661
Quoted 2 lines
common/image.c +2
-0+#endif +
nit: Remove this blank line.
Quoted 43 lines
common/image.c +27
-0+ulong kernel_addr; + +/* find out kernel image address */ +if (!img_addr) { + kernel_addr = load_addr; +debug ("* kernel: default image load address = 0x%08lx\n" , + load_addr); +#if defined(CONFIG_FIT ) + } else if (fit_parse_conf(img_addr, load_addr, &kernel_addr, + fit_uname_config)) { +debug ("* kernel: config '%s' from image at 0x%08lx\n" , + *fit_uname_config, kernel_addr); + }else if (fit_parse_subimage (img_addr, load_addr, &kernel_addr, + fit_uname_kernel)) { +debug ("* kernel: subimage '%s' from image at 0x%08lx\n" , + *fit_uname_kernel, kernel_addr); +#endif + }else { + kernel_addr =simple_strtoul (img_addr,NULL ,16 ); +debug ("* kernel: cmdline image address = 0x%08lx\n" , + kernel_addr); + } + +return kernel_addr; +} + +/** * genimg_get_format - get image format type * @img_addr: image start address * include/image.h +1
-1@@ -424,6 +424,7 @@ enum fit_load_op {#define IMAGE_FORMAT_FIT 0x02/* new, libfdt based format */ #define IMAGE_FORMAT_ANDROID 0x03/* Android boot image */ +ulong genimg_get_kernel_addr (char *const img_addr);int genimg_get_format (const void *img_addr);int genimg_has_config (bootm_headers_t *images);ulong genimg_get_image (ulong img_addr); --
Quoted 2 lines
1.9.1
Regards,
Simon
PATCH 1/3 image: introduce genimg_get_kernel_addr()
In reply to Bryan WuQuoted 5 lines · Bryan Wu
Kernel address is normally stored as a string argument of bootm or bootz. This function is taken out from
boot_get_kernel()ofbootm.c, which can be reused by others.Signed-off-by Bryan Wu <[email protected]>
After doing the following to fix warnings:
common/image.c +4 -4
@@ -668,13 +668,13 @@ ulong genimg_get_kernel_addr(char * const img_addr) load_addr);#if defined(CONFIG_FIT ) } else if (fit_parse_conf(img_addr, load_addr, &kernel_addr, - fit_uname_config)) { + &fit_uname_config)) {debug ("* kernel: config '%s' from image at 0x%08lx\n" , - *fit_uname_config, kernel_addr); + fit_uname_config, kernel_addr); }else if (fit_parse_subimage (img_addr, load_addr, &kernel_addr, - fit_uname_kernel)) { + &fit_uname_kernel)) {debug ("* kernel: subimage '%s' from image at 0x%08lx\n" , - *fit_uname_kernel, kernel_addr); + fit_uname_kernel, kernel_addr);#endif }else { kernel_addr =simple_strtoul (img_addr,NULL ,16 );
Applied to u-boot/master, thanks!
Signature · 8 lines
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/20140810/74876541/attachment.pgp>
PATCH 1/3 image: introduce genimg_get_kernel_addr()
Quoted 14 lines · Tom Rini
Quoted 5 lines · Bryan Wu
Kernel address is normally stored as a string argument of bootm or bootz. This function is taken out from
boot_get_kernel()ofbootm.c, which can be reused by others.Signed-off-by Bryan Wu <[email protected]>After doing the following to fix warnings:
common/image.c +0
-0
common/image.c ulong genimg_get_kernel_addr(char * const img_addr) · line 671
Quoted 5 lines
common/image.c +1
-1@@ -668,13 +668,13 @@ ulong genimg_get_kernel_addr(char * const img_addr) load_addr);#if defined(CONFIG_FIT ) } else if (fit_parse_conf(img_addr, load_addr, &kernel_addr, - fit_uname_config)) { + &fit_uname_config)) {
My bad, thanks for fixing this.
-Bryan
Quoted 17 lines
common/image.c +3
-3debug ("* kernel: config '%s' from image at 0x%08lx\n" , - *fit_uname_config, kernel_addr); + fit_uname_config, kernel_addr); }else if (fit_parse_subimage (img_addr, load_addr, &kernel_addr, - fit_uname_kernel)) { + &fit_uname_kernel)) {debug ("* kernel: subimage '%s' from image at 0x%08lx\n" , - *fit_uname_kernel, kernel_addr); + fit_uname_kernel, kernel_addr);#endif }else { kernel_addr =simple_strtoul (img_addr,NULL ,16 );Applied to u-boot/master, thanks!
--
Tom
PATCH 2/3 pxe: detect image format before calling bootm/bootz
In reply to Bryan WuTrying bootm for zImage will print out several error message which is not necessary for this case. So detect image format firstly, only try bootm for legacy and FIT format image then try bootz for others.
This patch needs new function genimg_get_kernel_addr().
--- common/cmd_pxe.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
common/cmd_pxe.c +11 -4
@@ -1,5 +1,6 @@/* * Copyright 2010-2011 Calxeda, Inc. +* Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved. * * SPDX-License-Identifier: GPL-2.0+ */
@@ -609,6 +610,8 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)char *bootargs;int bootm_argc =3 ;int len =0 ; +ulong kernel_addr; +void *buf;label_print (label);
@@ -771,11 +774,15 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)if (bootm_argv[3 ]) bootm_argc =4 ; -do_bootm (cmdtp,0 , bootm_argc, bootm_argv); - + kernel_addr =genimg_get_kernel_addr (bootm_argv[1 ]); + buf =map_sysmem (kernel_addr,0 ); +/* Try bootm for legacy and FIT format image */ +if (genimg_get_format (buf) !=IMAGE_FORMAT_INVALID ) +do_bootm (cmdtp,0 , bootm_argc, bootm_argv);#ifdef CONFIG_CMD_BOOTZ -/* Try booting a zImage if do_bootm returns */ -do_bootz (cmdtp,0 , bootm_argc, bootm_argv); +/* Try booting a zImage */ + else + do_bootz(cmdtp, 0, bootm_argc, bootm_argv);#endif return 1 ; }
Signature · 1 line
1.9.1
PATCH 2/3 pxe: detect image format before calling bootm/bootz
Quoted 7 lines · Bryan Wu
Trying bootm for zImage will print out several error message which is not necessary for this case. So detect image format firstly, only try bootm for legacy and FIT format image then try bootz for others.
This patch needs new function
genimg_get_kernel_addr().Signed-off-by Bryan Wu <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
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/20140810/535482d9/attachment.pgp>
PATCH 3/3 bootm: use genimg_get_kernel_addr()
In reply to Bryan WuUse the new API which is originally taken out from boot_get_kernel of bootm.c
--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
common/bootm.c +5 -20
@@ -731,26 +731,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,int os_noffset;#endif -/* find out kernel image address */ -if (argc <1 ) { - img_addr = load_addr; -debug ("* kernel: default image load address = 0x%08lx\n" , - load_addr); -#if defined(CONFIG_FIT ) - } else if (fit_parse_conf(argv[0], load_addr, &img_addr, - &fit_uname_config)) { -debug ("* kernel: config '%s' from image at 0x%08lx\n" , - fit_uname_config, img_addr); - }else if (fit_parse_subimage (argv[0 ], load_addr, &img_addr, - &fit_uname_kernel)) { -debug ("* kernel: subimage '%s' from image@0x%08lx\n" , - fit_uname_kernel, img_addr); -#endif - }else { - img_addr =simple_strtoul (argv[0 ],NULL ,16 ); -debug ("* kernel: cmdline image address = 0x%08lx\n" , - img_addr); - } + img_addr =genimg_get_kernel_addr (argv[0 ]);bootstage_mark (BOOTSTAGE_ID_CHECK_MAGIC );
@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
Signature · 1 line
1.9.1
PATCH 3/3 bootm: use genimg_get_kernel_addr()
common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 794
Quoted 10 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.ccommon/bootm.c +4
-0case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel);
I'd be tempted to try and rework patch 1/3 so that it could "return" the fit_uname_* values too. That way, you wouldn't need to call fit_parse_*() a second time here. This probably isn't a big issue though.
The series,
PATCH 3/3 bootm: use genimg_get_kernel_addr()
Quoted 16 lines · Stephen Warren
common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 794
Quoted 13 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.ccommon/bootm.c +4
-0case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel);
Quoted 4 lines
I'd be tempted to try and rework patch 1/3 so that it could "return" the fit_uname_* values too. That way, you wouldn't need to call fit_parse_*() a second time here. This probably isn't a big issue though.
I actually tried to do that at beginning but it makes a little bit complicated for the caller like pxe code. I believe most of users don't care about those 2 fit uname arguments but only the kernel address. So I think we can make it simpler firstly.
Thanks,
-Bryan
PATCH 3/3 bootm: use genimg_get_kernel_addr()
In reply to Bryan WuHi Bryan,
Quoted 27 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-11@@ -731,26 +731,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,int os_noffset;#endif -/* find out kernel image address */ -if (argc <1 ) { - img_addr = load_addr; -debug ("* kernel: default image load address = 0x%08lx\n" , - load_addr); -#if defined(CONFIG_FIT ) - } else if (fit_parse_conf(argv[0], load_addr, &img_addr, - &fit_uname_config)) { -debug ("* kernel: config '%s' from image at 0x%08lx\n" , - fit_uname_config, img_addr); - } else if (fit_parse_subimage(argv[0], load_addr, &img_addr,
common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 734
Quoted 10 lines
common/bootm.c +1
-9- &fit_uname_kernel)) { - debug("* kernel: subimage '%s' from image at 0x%08lx\n", - fit_uname_kernel, img_addr); -#endif - } else { - img_addr =simple_strtoul (argv[0 ],NULL ,16 ); -debug ("* kernel: cmdline image address = 0x%08lx\n" , - img_addr); - } + img_addr =genimg_get_kernel_addr (argv[0 ]);
Do you think it would be clearer to say
img_addr = genimg_get_kernel_addr(arg < 1 ? NULL : argv[0]);
I don't think it matters in practice, just a little nervous about people going off the end of the args.
Quoted 15 lines
common/bootm.c +4
-1bootstage_mark (BOOTSTAGE_ID_CHECK_MAGIC );@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL, --
Quoted 2 lines
1.9.1
Regards,
Simon
PATCH 3/3 bootm: use genimg_get_kernel_addr()
In reply to Bryan WuQuoted 6 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>
After doing the following to fix warnings:
common/bootm.c +2 -2
@@ -789,9 +789,9 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT :if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, - fit_uname_config)) + &fit_uname_config))fit_parse_subimage (argv[0 ], load_addr, &img_addr, - fit_uname_kernel); + &fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
Applied to u-boot/master, thanks!
Signature · 8 lines
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/20140810/838245b3/attachment.pgp>