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

Scott Wood
2015-07-16T19:30:29Z

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@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