Re: NAND technical review

Jonathan Larmour wrote:
> Rutger Hofman wrote:
>> Jonathan Larmour wrote:
[How would different hardware, like OneNAND, fit into R's model?]
>>> Would that not require a significant reworking and relayering of 
>>> code? It seems to me that controller drivers and the chip drivers 
>>> used by controller drivers under this system will still want to be 
>>> able to access the infrastructure support for BBTs, ECCs and spare 
>>> layout. From what I can see, preserving that without large amounts of 
>>> indirection (imposing further performance and size hits) would pose 
>>> quite some challenge. The result would be something really quite 
>>> different to what R is like today.
>>
>> The reworking would just be to replace the calls in the ANC of the 
>> controller-common API with indirect calls, and supporting this 
>> configurability in the CDL. OneNAND doesn't need ECC code because 
>> things are handled in hardware (the datasheet recommends not even 
>> checking the ECC status register for 2-bit failures because they are 
>> so rare).
> 
> As I mentioned when I brought up OneNAND, my concern was really more 
> general: that the layer is at present only intended for a particular 
> access model. OneNAND is only a current example of where this assumption 
> breaks, there may be more in the future (or now).
> 
>> Maybe BBT is necessary too for OneNAND; I didn't think that through yet,
> 
> I would have thought so personally.
> 
>> but I would hope the BBT implementation would support different 
>> controllers without a lot of reworking - right now, accessing the chip 
>> already goes through controller calls. The indirections would be few: 
>> one for each top-level API call (unless the ANC must redistribute 
>> application pages to chip pages, which is only in case of 
>> heterogeneous chips).
> 
> It's not just indirecting the functions, but checking what may need 
> changing for anything which accesses the controller data, e.g. the 
> contents of struct CYG_NAND_CTL.

OK, to answer this question, the level of detail will go up considerably.

Lots of CYG_NAND_CTL is pointers to higher layers (anc) and lower layers 
(funs, priv, chip stuff). These would remain, although the function 
dispatch table would be reused (or unused, I don't know about OneNAND 
varieties). The ECC fields would remain too (if applicable, but nowadays 
that is a CDL option) and likewise mutex. I would guess that any state 
for the different class of controller/chip could be incorporated into priv.

So, (surprisingly to me because I didn't consider anything else than raw 
NAND), CYG_NAND_CTL seems generic enough to incorporate other types of 
NAND chip. I'd say the controller-common API must stay -- if it doesn't, 
I would be doubtful to fit it into a NAND harness. Reminder to self: ANC 
must call the controller over a function dispatcher.

CYG_NAND_CHIP would need to be split into a generic part that has page 
size, block size, num blocks, and type-specific stuff like timing and 
like the bucket-full of ONFI parameters.

Required code refactoring: a number of functions must just be lifted out 
for common usage. For spare layout, my guess is that just the 
scatter/gather functions from controller-common would be shared (small 
though they are). If ECC is desired: the configurable part of it is 
already separate; some more (cyg_nand_ctl_ecc_repair, debug stuff) can 
be usefully factored out. One more thing that should be common is the 
verification step; it had best move one level up, to ANC.

I think the rest of controller-common is tied to raw NAND, and would 
have no place in a driver for non-raw NAND hardware.

> It's not clear how the stuff in src/chip/ would map onto a different 
> controller model.

The interrogation parts would not fit at all, they would have no place 
in a driver for different hardware. Interrogation is the majority of the 
code. Common code would be BBT and bad-block queries.


If this all is going to happen, the code would be partitioned like:

top-level = anc (with verify and function dispatch)
controller = shared (ECC, spare), raw NAND, OneNAND, ...
chip = shared (BBT, bad-block query), raw NAND, OneNAND, ...
... and unmodified device-specific controller, platform stuff, ...

The CDL would piece together the desired components, like you suggested. 
I think that both the public API and the interface to device-specific 
drivers will remain intact, so clients won't really notice (except 
possiby in their CDL).

I guess that this refactoring will take something like one or a few 
days' work, including having ANC call the controller over a dispatch 
table. I'll be glad to do it (ETA: somewhere in the next 1 to 1.5 months).

Personal note: I am glad with this kind of detailed feedback. Still, I 
would have preferred to get it when I put up the NAND design for 
discussion, about a year ago.

Rutger