This is the mail archive of the mailing list for the eCos project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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:

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]