[U-Boot] [PATCH v3 1/2] image: fix bootm failure for FIT image

[Bryan Wu]

Hi Simon, Tom and Stephen,

Could you guys review this patch which solved the issue York reported?

Thanks,
-Bryan

On Fri, Aug 15, 2014 at 4:51 PM, Bryan Wu <cooloney at gmail.com> wrote:
> Commit b3dd64f5d537 "bootm: use genimg_get_kernel_addr()" introduced
> a bug for booting FIT image. It's because calling fit_parse_config()
> twice will give us wrong value in img_addr.
>
> Add a new function genimg_get_kernel_addr_fit() whichl will always
> return fit_uname_config and fit_uname_kernel for CONFIG_FIT.
> genimg_get_kernel_addr() will ignore those to parameters.
>
> Reported-by: York Sun <yorksun at freescale.com>
> Signed-off-by: Bryan Wu <pengw at nvidia.com>
> ---
>  common/bootm.c  |  9 +++------
>  common/image.c  | 39 +++++++++++++++++++++++++++------------
>  include/image.h |  3 +++
>  3 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index 76d811c..245c82a 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -725,13 +725,14 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
>  #endif
>         ulong           img_addr;
>         const void *buf;
> -#if defined(CONFIG_FIT)
>         const char      *fit_uname_config = NULL;
>         const char      *fit_uname_kernel = NULL;
> +#if defined(CONFIG_FIT)
>         int             os_noffset;
>  #endif
>
> -       img_addr = genimg_get_kernel_addr(argv[0]);
> +       img_addr = genimg_get_kernel_addr_fit(argv[0], &fit_uname_config,
> +                                         &fit_uname_kernel);
>
>         bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
>
> @@ -788,10 +789,6 @@ 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,
> diff --git a/common/image.c b/common/image.c
> index a2999c0..d4ccff0 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -643,22 +643,24 @@ int genimg_get_comp_id(const char *name)
>
>  #ifndef USE_HOSTCC
>  /**
> - * genimg_get_kernel_addr - get the real kernel address
> + * genimg_get_kernel_addr_fit - get the real kernel address and return 2
> + *                              FIT strings
>   * @img_addr: a string might contain real image address
> + * @fit_uname_config: double pointer to a char, will hold pointer to a
> + *                    configuration unit name
> + * @fit_uname_kernel: double pointer to a char, will hold pointer to a subimage
> + *                    name
>   *
> - * genimg_get_kernel_addr() get the real kernel start address from a string
> + * genimg_get_kernel_addr_fit 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)
> +ulong genimg_get_kernel_addr_fit(char * const img_addr,
> +                            const char **fit_uname_config,
> +                            const char **fit_uname_kernel)
>  {
> -#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 */
> @@ -668,13 +670,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);
> @@ -686,6 +688,19 @@ ulong genimg_get_kernel_addr(char * const img_addr)
>  }
>
>  /**
> + * genimg_get_kernel_addr() is the simple version of
> + * genimg_get_kernel_addr_fit(). It ignores those return FIT strings
> + */
> +ulong genimg_get_kernel_addr(char * const img_addr)
> +{
> +       const char *fit_uname_config = NULL;
> +       const char *fit_uname_kernel = NULL;
> +
> +       return genimg_get_kernel_addr_fit(img_addr, &fit_uname_config,
> +                                         &fit_uname_kernel);
> +}
> +
> +/**
>   * genimg_get_format - get image format type
>   * @img_addr: image start address
>   *
> diff --git a/include/image.h b/include/image.h
> index ca2fe86..69f86ad 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -424,6 +424,9 @@ 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_fit(char * const img_addr,
> +                                const char **fit_uname_config,
> +                                const char **fit_uname_kernel);
>  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);
> --
> 1.9.1
>