[U-Boot] [PATCHv2 1/3] nand_spl_simple: Add a simple flash read function

[Ladislav Michl]

On Mon, Jan 04, 2016 at 12:23:36PM -0600, Scott Wood wrote:
> On Mon, 2016-01-04 at 16:54 +0100, Ladislav Michl wrote:
> > From: Thomas Gleixner <tglx at linutronix.de>
> > 
> > To support UBI in SPL we need a simple flash read function. Add one to
> > nand_spl_simple and keep it as simple as it goes.
> > 
> > Signed-off-by: Thomas Gleixner <tglx at linutronix.de>
> 
> Where is your signoff?

I'm only messenger and have nothing to do with this part of patch. The other
part which I touched has my signoff.

> Did Thomas really write this patch for U-Boot (if so, why isn't he on CC?) or
> is it taken from some other project?

Ha! It is my mailer which broke Cc. Thomas was on Cc list and still is in my
sent-mail folder. Also apologize to Marek, let's see how Cc will end this time.
And yes, patch was written for U-Boot: https://patchwork.ozlabs.org/patch/367305/

> > ---
> >  drivers/mtd/nand/nand_spl_simple.c | 64
> > ++++++++++++++++++++++++++++++++++++++
> >  include/nand.h                     |  1 +
> >  2 files changed, 65 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/nand_spl_simple.c
> > b/drivers/mtd/nand/nand_spl_simple.c
> > index e69f662..2e1af53 100644
> > --- a/drivers/mtd/nand/nand_spl_simple.c
> > +++ b/drivers/mtd/nand/nand_spl_simple.c
> > @@ -209,6 +209,70 @@ static int nand_read_page(int block, int page, void
> > *dst)
> >  }
> >  #endif
> >  
> > +#ifdef CONFIG_SPL_UBI
> > +/*
> > + * Temporary storage for non NAND page aligned and non NAND page sized
> > + * reads. Note: This does not support runtime detected FLASH yet, but
> > + * that should be reasonably easy to fix by making the buffer large
> > + * enough :)
> > + */
> > +static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
> > +
> > +/**
> > + * nand_spl_read_flash - Read data from flash into a buffer
> > + * @pnum:	Number of the physical eraseblock
> 
> s/pnum/block/g
> 
> > + * @offset:	Data offset from the start of @pnum
> > + * @len:	Data size to read
> > + * @dest:	Address of the destination buffer
> > + *
> > + * This could be further optimized if we'd have a subpage read
> > + * function in the simple code. On NAND which allows subpage reads
> > + * this would spare quite some time to readout e.g. the VID header of
> > + * UBI.
> > + *
> > + * Notes:
> > + *
> > + *	@offset + @len are not allowed to be larger than a physical
> > + *	erase block. No sanity check done for simplicity reasons.
> > + *
> > + * To support runtime detected flash this needs to be extended by
> > + * information about the actual flash geometry, but thats beyond the
> > + * scope of this effort and for most applications where fast boot is
> > + * required its a non issue anyway.
> > + */
> > +int nand_spl_read_flash(u32 pnum, u32 offset, u32 len, void *dest)
> 
> This name is too generic and doesn't suggest what's different compared to
> nand_spl_load_image (they both read data from flash into a buffer).
> 
> How about nand_spl_read_block()?

ok.

> > +{
> > +	u32 offs, page, read, toread = len;
> > +
> > +	/* Calculate the page number */
> > +	page = offset / CONFIG_SYS_NAND_PAGE_SIZE;
> > +
> > +	/* Offset to the start of a flash page */
> > +	offs = offset % CONFIG_SYS_NAND_PAGE_SIZE;
> > +
> > +	while (toread) {
> > +		/*
> > +		 * Non page aligned reads go to the scratch buffer.
> > +		 * Page aligned reads go directly to the destination.
> > +		 */
> > +		if (offs || toread < CONFIG_SYS_NAND_PAGE_SIZE) {
> > +			nand_read_page(pnum, page, scratch_buf);
> > +			read = min(len, toread);
> 
> toread is always <= len, so this is pointless.  As this is the only use of len
> after the toread init, once this is gone you could also eliminate toread and
> just use len.

ok, will change that.

> -Scott