[U-Boot,3/3] bootm: use genimg_get_kernel_addr()

Use the new API which is originally taken out from boot_get_kernel
of bootm.c

Signed-off-by: Bryan Wu <pengw@nvidia.com>
---
 common/bootm.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

On 07/31/2014 06:40 PM, Bryan Wu wrote:
> Use the new API which is originally taken out from boot_get_kernel
> of bootm.c

> diff --git a/common/bootm.c b/common/bootm.c

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

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,
Tested-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>

Hi Bryan,

On 31 July 2014 18:40, Bryan Wu <cooloney@gmail.com> wrote:
> Use the new API which is originally taken out from boot_get_kernel
> of bootm.c
>
> Signed-off-by: Bryan Wu <pengw@nvidia.com>
> ---
>  common/bootm.c | 25 +++++--------------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index 7ec2ed8..aee68cd 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -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 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.

>
>         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,
> --
> 1.9.1
>

Regards,
Simon

On Fri, Aug 1, 2014 at 12:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/31/2014 06:40 PM, Bryan Wu wrote:
>>
>> Use the new API which is originally taken out from boot_get_kernel
>> of bootm.c
>
>
>> diff --git a/common/bootm.c b/common/bootm.c
>
>
>>         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);
>
>
> 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