[U-Boot] [PATCH 1/3] image: introduce genimg_get_kernel_addr()

[Simon Glass]

Hi Bryan,

On 31 July 2014 18:39, Bryan Wu <cooloney at gmail.com> wrote:
> 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.
>
> Signed-off-by: Bryan Wu <pengw at nvidia.com>
> ---
>  common/image.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
>  include/image.h |  1 +
>  2 files changed, 44 insertions(+)
>
> diff --git a/common/image.c b/common/image.c
> index 11b3cf5..4e2816a 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -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
> + */

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.

> +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'.

> +#endif
> +

nit: Remove this blank line.

> +       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
>   *
> diff --git a/include/image.h b/include/image.h
> index 3e8f78d..ca2fe86 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -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);
> --
> 1.9.1
>

Regards,
Simon