On 1/26/07, Wolfgang Denk wd@denx.de wrote:
In message 528646bc0701261111i6f0dcb0i96f81259b3a4311d@mail.gmail.com you wrote:
My vote is to treat DataFlash like a block device, and make sure that it supports byte-wide access.
Treat DataFlash like a block device? That sounds wrong to me.
Perhaps using the term block device is too loaded. DataFlash is not the same as memory mapped storage, but it's not the same as a CF card or an IDE disk (in that it's not broken up into sectors). However, being non-memmapped makes it more similar to those devices than it is to memmapped flash.
OTOH, the argument could be made that memory mapped flash isn't the same either because while memmapped flash can be read from directly, writing requires extra twiddling. I'd be mad to suggest that memmapped flash should be removed from the memory manipulation commands.
Flash devices aren't too bad because all of them use the same flash_write and addr2info hooks. Right after them, however, there is an mmc hook which does essentially the exact same thing. DataFlash falls into the same category. (What kicked off this whole topic is that I'm working on the md command which only has the DataFlash hook, not the MMC or Flash hooks (and a flash hook wouldn't even make sense for 'md').
Does anyone see any reason why these hooks couldn't be merged into a single addr2info type call? If the flash_info_t was modified to include read/write function pointers and a private data pointer (move a bunch of the parameters into a private data structure). Boards can provide a table including the various flash and non-memmapped devices that are available. A single hook would be enough to get the entry for a particular address.
If I took this approach; it could be implemented in a staged approach, starting with memmapped flash, and then merging in mmc and dataflash support once the architecture is show to work. It still leaves the issue that it pretends mmc and dataflash devices are memory mapped which I really don't like, but with a standard set of hooks, it may be possible to just pass the raw string into addr2info instead of an integer address, and collect all the parsing code in one place (ie. use a "dev:addr" format as previously suggested)
I understand your concerns about the #ifdef mess, but this suggestion does not sound tempting either.
That particular approach happens to be/was my preference, but it is certainly not the only approach.
Cheers, g.