[U-Boot] [PATCH] ML: SPL: NOR: Add CONFIG_SPL_NOR_COPY_ENTIRE_IMAGE define to enable whole image copy from NOR

[Lukasz Majewski]

Hi Tom,

> On Fri, Sep 16, 2016 at 10:43:51AM +0200, Lukasz Majewski wrote:
> 
> > This define gives the possibility to copy entire image (including
> > header) from NOR parallel memory to e.g. SDRAM.
> > 
> > The legacy behavior is preserved, since other board don't enabled
> > this option.
> 
> What's the use case?

This code is used on some boards (ppc [1]) in which we copy only
payload of the image (i.e. we drop the package metadata).

In my particular case I would like to copy the image with its meta
data (in similar way works spl_nand.c and spl_mmc.c).

Since, I wanted to avoid code duplication, I've decided to add this
flag.

> 
> > Signed-off-by: Lukasz Majewski <l.majewski at majess.pl>
> > ---
> >  Kconfig              | 10 ++++++++++
> 
> This should now get moved to common/spl/Kconfig

OK.

As a side note - I do have some other patches which move some SPL_NOR
options to Kconfig.

However, I omitted them when I saw great work done by Simon to move a
lot of SPL options to Kconfig. I hope them will be merged soon :-)

> 
> >  README               |  4 ++++
> 
> This can be dropped.

So, we don't add any new entries to README and now rely solely on
Kconfig "help" sections?

> 
> [snip]
> > diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
> > index 8ea874c..1e0f739 100644
> > --- a/common/spl/spl_nor.c
> > +++ b/common/spl/spl_nor.c
> > @@ -9,13 +9,15 @@
> >  
> >  int spl_nor_load_image(void)
> >  {
> > +	void *img_src;
> >  	int ret;
> > +#ifndef CONFIG_SPL_NOR_COPY_ENTIRE_IMAGE
> >  	/*
> >  	 * Loading of the payload to SDRAM is done with skipping of
> >  	 * the mkimage header in this SPL NOR driver
> >  	 */
> >  	spl_image.flags |= SPL_COPY_PAYLOAD_ONLY;
> > -
> > +#endif
> >  #ifdef CONFIG_SPL_OS_BOOT
> >  	if (!spl_start_uboot()) {
> >  		const struct image_header *header;
> > @@ -64,9 +66,13 @@ int spl_nor_load_image(void)
> >  	if (ret)
> >  		return ret;
> >  
> 
> We should have a comment here about how we're calculating the base of
> where to copy from.

I will be more verbose with v2 :-).

Thanks for review.

> 
> > +	img_src = (void *)CONFIG_SYS_UBOOT_BASE;
> > +#ifndef CONFIG_SPL_NOR_COPY_ENTIRE_IMAGE
> > +	img_src += sizeof(struct image_header));
> > +#endif
> > +
> >  	memcpy((void *)(unsigned long)spl_image.load_addr,
> > -	       (void *)(CONFIG_SYS_UBOOT_BASE + sizeof(struct
> > image_header)),
> > -	       spl_image.size);
> > +	       img_src, spl_image.size);
> >  
> >  	return 0;
> >  }
> 
> Thanks!
> 

Best regards,
Łukasz Majewski


Notes:
[1] - Extract from source code (spl_nor.c):

/*
 * On some system (e.g. powerpc), the load-address and
 * entry-point is located at address 0. We can't load
 * to 0-0x40. So skip header in this case.
 */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160918/377aa428/attachment.sig>