[U-Boot] [PATCH 4/4 V3] PXA: Adapt Voipac PXA270 to OneNAND SPL

[Marek Vasut]

> On 11/03/2011 04:52 PM, Marek Vasut wrote:
> >> On 11/02/2011 08:56 PM, Marek Vasut wrote:
> >>> +	onenand_spl_load_image(CONFIG_SPL_ONENAND_LOAD_ADDR,
> >>> +				CONFIG_SPL_ONENAND_LOAD_SIZE,
> >>> +				(void *)CONFIG_SYS_TEXT_BASE);
> >> 
> >> If we make it "nand_spl_load_image", and make the #defines conform, we
> >> can have the same code call the function for nand and onenand.  I don't
> >> see any reason why onenand is a completely different subsystem in
> >> general, rather than just another NAND driver.  The NAND subsystem's
> >> driver interface is lower level than it should be, but that affects
> >> other NAND controllers as well (such as fsl_elbc).
> >> 
> >> Switching to the generic nand_boot() in
> >> http://patchwork.ozlabs.org/patch/123219/ would get you the ability to
> >> load the environment during the SPL.
> >> 
> >> -Scott
> > 
> > I don't think I understand. Why do you want to mix onenand and nand ?
> 
> Why do we want to separate them?  What is the fundamental difference
> between OneNAND, and a high-level NAND controller such as fsl_elbc?

Honestly, I'm not the author of the subsystem, but please check the 
documentation. The way we retrieve data from onenand is different to NAND.

> Maybe there would be some differences on init if we can't produce
> "normal" ID data, but that doesn't justify duplicating the whole subsystem.

Where do you see such duplication? cmd_onenand ?
> 
> Why should the code that just wants to use an API to move data around
> need to care which it is?  Why should there be behavioral differences
> that aren't rooted in the actual hardware?  Another approach might be to
> use MTD as the common interface, but factor out common code into
> libraries that drivers can use, and avoid the main nand_base.c code even
> for things like fsl_elbc.

I think you're mistaken here. OneNAND != NAND.
> 
> This is not a new complaint -- I've asked for this before but nobody's
> put the time into sorting out the mess (and I have neither time nor
> hardware nor documentation).  The SPL load_image function is a simple
> enough interface to start with, though. :-)

Well, it seems what you are proposing is way beyond the scope of this patchset.
> 
> In fact, it should probably just be spl_load_image() with whatever boot
> source has been configured into this SPL build.

What if you have two boot sources?

> 
> > Also, will your approach still allow me to squeeze the important code
> > into the first 1kb for the initial copying of SPL?
> 
> If you can't fit the common load sequence in, then of course don't use
> it, but there's no need for the function name to be different.
> 
> -Scott