On 11/03/2011 07:55 PM, Marek Vasut wrote:
>> On 11/03/2011 04:52 PM, Marek Vasut wrote:
>> 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.

What documentation?  How is it different?  There are substantial
differences in how we "retrieve data" between drivers that use the NAND
subsystem.  Surely you've seen that in the mxs_nand driver. :-)

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

cmd_onenand and env_onenand are the most irritating, since they're at a
layer that really shouldn't care about the differences -- we should
probably have a plain "mtd" command instead, for most of the functionality.

There's also onenand_bbt.c -- what are the hardware-based differences in
how the bad block table is managed?

nand_base.c/onenand_base.c are less clear.  Obviously much of what is in
onenand_base.c would be in the controller driver if it used the NAND
subsystem.  But it looks like some of it is duplication.

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

Well, last I tried I couldn't find any public documentation, so all I
have to go on is the code, some marketing-type info, and asking
questions of people that appear to know more about OneNAND than I do. :-)

>From what I can see, it looks like NAND with an integrated controller
that exposes an unusual command set, but still for the most part
provides the same operations.  Several of our existing NAND-subsystem
drivers have to fake the command set for the generic layer as well.
Initialization/identification might be a problem area that current
drivers don't have to deal with, though.  Actually integrating OneNAND
with NAND would likely involve an already-needed restructuring of the
subsystem.

If the answer really is that it makes sense to consider OneNAND to be a
totally different thing from NAND, then it's outside my jurisdiction as
NAND custodian -- which is fine with me.  Frankly, even if it does make
sense to merge them, I'd rather not be custodian of the OneNAND stuff
unless someone is actually willing to do the merge.  I don't have access
to hardware or documentation, and it's an entirely separate codebase.
People just started sending me the patches a few years back.

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

Traditionally SPL has been small and purpose-built to do exactly one
thing -- so we decide at compile-time things that we might otherwise
decide at runtime.

If there's a requirement for multiple boot sources decided at runtime,
then we'll obviously need a runtime mechanism. -- but it seems a bit
hackish to why does it matter whether it's two different types of
device, or two of the same type of device (possibly with different
controller types)?  If the answer is that, for example, NAND versus USB
versus MMC is a likely use case, but two different NANDs is not likely,
is NAND versus OneNAND any more likely?

Maybe spl_load_image should be a function pointer that board code sets,
with each implementation being distinctly named (in which case
nand_spl_load_image would become nand_spl_simple_load_image, unless we
move it to nand_spl_load.c and make nand_read_page a function pointer).
 If needed to save a few bytes, we could use #defines to eliminate the
function pointers in a single-target SPL build.

For now, until we decide to do something SPL-wide, call it what you want.

-Scott

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to