[U-Boot] [PATCH 1/4] spl: nand: simple: replace readb() with chip specific read_buf()

[Scott Wood]

On Thu, 2015-07-16 at 16:12 +0200, Albert ARIBAUD wrote:
> Hello Vladimir,
> 
> On Thu, 16 Jul 2015 16:48:26 +0300, Vladimir Zapolskiy <vz at mleia.com>
> wrote:
> 
> (cutting short to the essential point remaining)
> 
> > > I personally don't think this is the right way; nand_simple.c should be
> > > left unchanged and the board should simply not use it since it does not
> > > have a simple NAND controller, and instead it should provide its own
> > > nand_spl_load_image().

I don't have a problem with expanding the definition of "simple" to include 
custom read_byte and such, as long as we make sure that existing targets 
don't break.  For drivers that don't supply these functions, where are the 
default functions coming from?  I don't see nand_spl_simple's nand_init() 
initializing them.

> > For me an alternative change to the proposed one is to duplicate
> > nand_spl_simple.c functionality in LPC32xx SLC NAND driver. From
> > maintenance point of view this is not the best thing to do IMHO.
> 
> You're right that some of the functionality present in nand_simple.c is
> duplicated elsewhere; however, that functionality is not the one specific
> to nand_simple; actually, it is nand_spl_load_image(), the overall load
> functionality which should be in no driver at all but of which I see ten
> incarnations in ten drivers.
> 
> There should be only one nand_spl_load_image(), in its own file, with
> placeholders for driver-specific actions such as page read, bad page
> check, etc. One day, maybe, there will be a patch for that.

You'll never get it down to just one, since there are some targets that rely 
on the tight integration within a single file to squeak in under a 4 KiB size 
limit.  Maybe some of the implementations could be replaced with an optional 
generic version, though.

-Scott