Re: NAND technical review

[Jonathan Larmour <jifl@jifvik.org>]

Rutger Hofman wrote:
> Jonathan Larmour wrote:
> 
>> In E's case, in the EA LPC2468 port example, they have the following 
>> in the platform HAL for a port (although it could be a package instead):
>>
>> [various functions/macros defined which are used by k9fxx08x0x.inl]
>> #include <cyg/devs/nand/k9fxx08x0x.inl>
>> CYG_NAND_DEVICE(ea_nand, "onboard", &k9f8_funs, &_k9_ea_lpc2468_priv,
>>                 &linux_mtd_ecc, &nand_mtd_oob_64);
>>
>> which succinctly brings together the chip driver, accessor functions, 
>> ECC algorithm, and OOB layout. It becomes easy for a board port to 
>> choose some different chips/layouts/ECC. There's flexibility for the 
>> future in that.
> 
> Yes, in R that is all in the board's CDL. I am unsure what that means 
> w.r.t. flexibility.

But in R that CDL doesn't allow for linking together rather more arbitrary 
implementations - instead it's one of a set of existing implementations 
provided by R. Although from what you said in the other mail, the spare 
layout at least has now been tackled by allowing a custom layout.

>> With R's implementation, there seems to be much more code involved. 
>> And I sort of see why there's more code, and I sort of don't. Not just 
>> in the generic layer, but in the drivers as well, at least looking at 
>> the bfin chip, and I don't think the differences are completely 
>> explained by the hardware properties of each NFC (but I'm very willing 
>> to be corrected!). Comparing E's k9_read_page() along with everything 
>> it calls, with R's bfin_nfc_data_read() along with everything it calls 
>> (and those call etc. not just in bfin_nfc.c but also 
>> nand_ez_kit_bf548.inc[1]) there's a huge difference. If nothing else 
>> from what I can tell this may then require a much larger porting 
>> effort, compared to E's.
> 
> The BlackFin nfc reads/writes in small sub-pages so doing a 512B or 2KB 
> read/write needs a loop to traverse the sub-pages.

Ok.

> More complexity in 
> bfin_nfc_data_read is added because there is support for random 
> sub-small-page reads too - ultimately a consequence of the outer API 
> capability to do random reads.

And it's true that E does not support partial reads, although it would be 
more reasonable to make partial read support an optional driver feature 
(and synthesise it otherwise, or make it a property the user can detect) - 
if nothing else I'd believe some NFCs won't support it.

> And things are complicated because the 
> BFin NFC wants a handshake with each data byte/word read.

Ok.

> The code in the .inc file (chip select) is more generic than it should 
> be. It has support for multiple, possibly heterogeneous, chips, while 
> there is only one NAND chip on the EZ-Kit. It figures out at run-time 
> though that the only thing to do is handle the CHIP_ENABLE pin.

I think it's bound up with the layering really - R's controller code has 
to able to handle any chip. In E it can be bound to the platform and so 
it's known.

> Shameless plug: I think you might also consider to take a look at the 
> example GPIO controller driver that I bundled. That is intended to show 
> how small a device-specific controller driver can actually be.

That does take quite a lot of shortcuts, such as not ensuring lines are 
asserted for long enough, nor having timeouts. But yes if the bfin driver 
is odd, and this driver is more typical then this would make a device 
driver roughly the same size as E. But I'm not entirely sure how 
consistently true that would be in general for NFCs due to that layer not 
knowing about properties of the underlying chip driver, and instead having 
to determine things at runtime, e.g. 8 vs 16-bit access, etc. For example, 
E has a much simpler mechanism to handle what R requires both 
data_read_8() and data_read() methods in the nand controller functions to 
supply.

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