This is the mail archive of the
ecos-devel@sourceware.org
mailing list for the eCos project.
NAND review
- From: Simon Kallweit <simon dot kallweit at intefo dot ch>
- To: "ecos-devel at ecos dot sourceware dot org" <ecos-devel at ecos dot sourceware dot org>
- Date: Tue, 19 May 2009 10:27:05 +0200
- Subject: NAND review
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