NAND review

[Simon Kallweit <simon dot kallweit at intefo dot ch>]

Hi there

I took a quick look at the NAND code released by Ross. I'll just give 
you my thoughts in no particular order:

* Ross's code uses the directory 'io/nand' for the NAND framework and 
adds the NAND flash device drivers in 'devs/nand'. This is contrary to 
what was discussed on the mailing list a few months ago:

http://ecos.sourceware.org/ml/ecos-discuss/2008-09/msg00172.html

I still think that the naming scheme by Ross (and what Rutger originally 
intended to do) is the better approach. Because when we mix the flash 
chip drivers for NOR and NAND chips in one directory (devs/flash) it's 
rather easy to get confused. For example we would get something like 
synth, synth_v2 and synth_nand all in the same directory. Or we would 
have to move that to synth/v1 synth/v2 synth/nand or something. Anyway I 
would suggest we leave the flash v1/v2 stuff alone and add a new devs 
directory 'nand' for the NAND stuff.

* Ross added very primitive support for partitions. Each partition is 
defined by a first and a last block. Reading/writing pages and erasing 
sectors is always done in the context of a partition. BUT the page and 
block addresses are still absolute addresses to the flash chip base, not 
relative to the given partition. This is slightly confusing and I wonder 
if the better approach would be to change this to relative addresses. 
Absolute addresses could still be used in the absence of a partition (NULL).

I also wonder if the current approach for initializing the partition 
table is wise. Currently the flash device driver uses a macro 
NAND_DEV_PARTITION_INIT to call a potential function defined the HAL 
platform to initialize the partition table. This is flexible but we will 
have a lot of code duplication in HAL platforms if the partition table 
is going to be defined via CDL options. Couldn't this be implemented in 
a more generic fashion?

* Ross's code is not layered as heavily as Rutgers code. Rutger has a 
clear interface between the application, the NAND flash controller and 
the NAND flash chips. In Ross's code this is all much more loosly 
coupled. Both approaches have their pros and cons but I would like to 
discuss Ross's approach here. The interface between the common NAND code 
and the flash devices is very high-level (initialization, 
reading/writing pages, erasing blocks, checking factory bad blocks). 
This makes for example the implementation of a synthetic NAND device 
very, very easy. I see some issues with the interface between the NAND 
chip driver and the NAND flash controller. In the current design the 
NAND flash controller is implemented in the HAL platform. The interface 
between the chip driver and the controller is defined in the chip driver 
itself (it just calls to a few functions which the platforms needs to 
implement). Is it intended that future drivers use the same interface or 
are drivers free to define their own? I think a common interface should 
be used for simplicity, and this should probably be defined in a more 
rigid manner. I also wonder if the NAND flash controller functionality 
belongs to the platform. In my opinion this should go to the HAL variant 
instead. What should remain in the platform is the initialization of the 
flash controller as well as the definition of the flash chips used.

* The current code does not read the flash chip timings from the chip 
signature. The timings are hardcoded into the driver. This might or 
might not be the best approach. Again, when adding a generic interface 
between the chip driver and the controller, these timings could be used 
to setup the controller (as some NAND flash controllers can take core of 
these timings). Some of the timings don't belong into the driver but 
into the controller I think, but I didn't look at it in detail.

* The current synth driver is rather limited in its configurability. It 
does not yet support small page flash chips for example. This should be 
improved but I think that's pretty easy to do.

Well these are my first thoughts on the prereleased code. I hope more 
people take a look at it and we can have a discussion and soon decide 
which NAND framework we're going to use.

Simon