Re: NAND technical review

[Jonathan Larmour <jifl@jifvik.org>]

Rutger Hofman wrote:
> Jonathan Larmour wrote:
> 
>> Rutger Hofman wrote:
>> I was referring to the "layout" field of cyg_nand_chip_id_t. Whereas 
>> spare_scatter_fill()/extract() use the spare_layout field of the 
>> cyg_nand_chip_info_t. I haven't yet found where the "layout" field of 
>> cyg_nand_chip_id_t is used. 
> 
> 
> This represents the fourth byte of the 'regular' read_ID response. 
> (First is manufacturer, second is device type, third is often 0, but may 
> be defined to give further architectural details.) The specx for this 
> layout byte:
> 
[snip]

Okay. So the fact the byte is in the table is for completeness, even 
though read_id reads it out and doesn't need to compare it. Got it, thanks.

>> So I believe that means if you want to rewrite the boot program on 
>> Blackfin _and_ use a normal layout, you wouldn't be able to use R as 
>> it stands at the moment. Ack, OK.
> 
> 
> Hot-swapping is necessary to pull this tric. It would be doable in my 
> current svn version (which is coming up one of these days). It would 
> imply crossing through all layers though, and it would indeed be 
> horribly risky.

Okay.

[snip]
>>> That guarded guess was correct. OneNAND is no raw NAND chip so R's 
>>> common controller code won't fit. The thing to do is make a driver 
>>> that comes in place of the common controller, as suggested above. 
>>> Once ANC supports a pluggable controller (which I will do if it makes 
>>> a difference), adding a OneNAND will take the same amount of effort 
>>> as for E's implementation.
>>
>> 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.

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

There still seems to me to be challenges in how spare layout is managed in 
the abstracted NFC case.

I'm not saying it can't be done - it's all software so it definitely can. 
But it seems to me there would be quite an upheaval to get from here to there.

> In conclusion: a driver in R for a different class of chip than 'raw 
> NAND' would indeed replace R's controller-common, any 
> controller-device-specific, and any chip, because these are all code for 
> raw NAND. I would hope that linker magic remove the code as unused from 
> the binary.

It depends how you would envisage it is selected. What I would have 
thought would probably be a CDL option (which lives in CYGPKG_IO_NAND) 
which is required by either the platform NAND package or platform HAL 
package. Enabling this CDL option would build the existing high level 
controller layer (in CYGPKG_IO_NAND) into extras.o to force inclusion in 
the image (probably via a new HAL table). I doubt linker magic would help 
in particular. So when that option is not required by the platform it 
remains disabled and the code is not built.

> A driver for OneNAND would still need to be written obviously, and it 
> would share no or little code with controller-common and raw chip. My 
> guess is that would hold for E too - although I don't really know 
> because I have had no time to review E's code.

That's correct as I see it.

Jifl
-- 
--["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine