[RFC PATCH 1/3] spl: spl_nor: Move legacy image loading into spl_legacy.c

[Daniel Schwierzeck]

Am 10.04.20 um 13:04 schrieb Stefan Roese:
> Move the legacy image loading into spl_legacy.c. This makes it easier
> to extend the legacy image handling with new features that other
> SPL loaders might use (e.g. spl_spi.c etc).
> 
> No functional change intended.
> 
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Weijie Gao <weijie.gao at mediatek.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> ---
> RFC comment: I'm sendig these 3 patches as RFC and once we've come to an
> agreement on these (Acked etc), I'll integrate them into the mtmips SPL
> patchset from Weijie and will send v7.
> 
> Thanks,
> Stefan
> 
>  common/spl/spl_legacy.c | 20 ++++++++++++++++++++
>  common/spl/spl_nor.c    | 27 +++++++++++++--------------
>  include/spl.h           | 13 +++++++++++++
>  3 files changed, 46 insertions(+), 14 deletions(-)

Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>

nits below

> 
> diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
> index 772135193e..7f00fc8885 100644
> --- a/common/spl/spl_legacy.c
> +++ b/common/spl/spl_legacy.c
> @@ -51,3 +51,23 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image,
>  
>  	return 0;
>  }
> +
> +int spl_load_legacy_img(struct spl_image_info *spl_image,
> +			struct spl_load_info *load, ulong header)
> +{
> +	struct image_header hdr;
> +	int ret;
> +
> +	/* Read header into local struct */
> +	load->read(load, header, sizeof(hdr), &hdr);
> +
> +	ret = spl_parse_image_header(spl_image, &hdr);
> +	if (ret)
> +		return ret;
> +
> +	/* Read image */
> +	load->read(load, header + sizeof(hdr), spl_image->size,
> +		   (void *)(unsigned long)spl_image->load_addr);
> +
> +	return 0;
> +}
> diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
> index b1e79b9ded..3f03ffe6a3 100644
> --- a/common/spl/spl_nor.c
> +++ b/common/spl/spl_nor.c
> @@ -24,7 +24,6 @@ unsigned long __weak spl_nor_get_uboot_base(void)
>  static int spl_nor_load_image(struct spl_image_info *spl_image,
>  			      struct spl_boot_device *bootdev)
>  {
> -	int ret;
>  	__maybe_unused const struct image_header *header;
>  	__maybe_unused struct spl_load_info load;
>  
> @@ -43,6 +42,8 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
>  		header = (const struct image_header *)CONFIG_SYS_OS_BASE;
>  #ifdef CONFIG_SPL_LOAD_FIT
>  		if (image_get_magic(header) == FDT_MAGIC) {
> +			int ret;
> +
>  			debug("Found FIT\n");
>  			load.bl_len = 1;
>  			load.read = spl_nor_load_read;
> @@ -61,6 +62,7 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
>  #endif
>  		if (image_get_os(header) == IH_OS_LINUX) {
>  			/* happy - was a Linux */
> +			int ret;
>  
>  			ret = spl_parse_image_header(spl_image, header);
>  			if (ret)
> @@ -93,11 +95,9 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
>  		debug("Found FIT format U-Boot\n");
>  		load.bl_len = 1;
>  		load.read = spl_nor_load_read;
> -		ret = spl_load_simple_fit(spl_image, &load,
> -					  spl_nor_get_uboot_base(),
> -					  (void *)header);
> -
> -		return ret;
> +		return spl_load_simple_fit(spl_image, &load,
> +					   spl_nor_get_uboot_base(),
> +					   (void *)header);

looks like an unrelated optimisation

>  	}
>  #endif
>  	if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) {
> @@ -107,14 +107,13 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
>  					      spl_nor_get_uboot_base());
>  	}
>  
> -	ret = spl_parse_image_header(spl_image,
> -			(const struct image_header *)spl_nor_get_uboot_base());
> -	if (ret)
> -		return ret;
> -
> -	memcpy((void *)(unsigned long)spl_image->load_addr,
> -	       (void *)(spl_nor_get_uboot_base() + sizeof(struct image_header)),
> -	       spl_image->size);
> +	/* Legacy image handling */
> +	if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) {
> +		load.bl_len = 1;
> +		load.read = spl_nor_load_read;
> +		return spl_load_legacy_img(spl_image, &load,
> +					   spl_nor_get_uboot_base());
> +	}
>  
>  	return 0;
>  }
> diff --git a/include/spl.h b/include/spl.h
> index 6087cd793c..c6c64b6a72 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -219,6 +219,19 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>  #define SPL_COPY_PAYLOAD_ONLY	1
>  #define SPL_FIT_FOUND		2
>  
> +/**
> + * spl_load_legacy_img() - Loads a legacy image from a device.
> + * @spl_image:	Image description to set up
> + * @load:	Structure containing the information required to load data.
> + * @header:	Pointer to image header (including appended image)
> + *
> + * Reads an legacy image from the device. Loads u-boot image to
> + * specified load address.
> + * Returns 0 on success.
> + */
> +int spl_load_legacy_img(struct spl_image_info *spl_image,
> +			struct spl_load_info *load, ulong header);
> +
>  /**
>   * spl_load_imx_container() - Loads a imx container image from a device.
>   * @spl_image:	Image description to set up
> 

-- 
- Daniel