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

[Scott Wood]

On Mon, 2016-01-04 at 22:17 +0100, Ladislav Michl wrote:
> On Mon, Jan 04, 2016 at 07:38:23PM +0100, Ladislav Michl wrote:
> > 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/
> 
> Well, seems to be mailman configuration:
> https://mail.python.org/pipermail/mailman-users/2006-May/051194.html
> so those who received post via mailman are seeing striped Cc line. Thanks to
> Nathan Lynch for pointing it out.
> Full Cc list:
> Cc: Scott Wood <oss at buserror.net>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: Heiko Schocher <hs at denx.de>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Richard Weinberger <richard at nod.at>
> Cc: Marek Vasut <marex at denx.de>

Oh right, I forgot about that mailman bug.

> > > +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.
> 
> Thinking about it more, I'd rather see nand_spl_read_peb(int peb, ...
> but that is not consistent with other nand reading functions.

"peb" is cryptic, and what sort of non-physical erase block does the core NAND
code deal with that warrants distinction?

> 
> > > > +{
> > > > +	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.
> 
> What about something like this? Changelog will be appended to v3.
> Btw, any comments to other patches in this serie?
> 
> -- >8 --
> 
> From: Thomas Gleixner <tglx at linutronix.de>
> Subject: nand_spl_simple: Add a simple NAND read function
>     
> To support UBI in SPL we need a simple NAND 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>
> Signed-off-by: Ladislav Michl <ladis at linux-mips.org>

Acked-by: Scott Wood <oss at buserror.net>

-Scott