On 20/11/09 20:17, Scott Wood wrote:
On Fri, Nov 20, 2009 at 03:30:46PM +0000, Nick Thompson wrote:
In the case of a nand controller that needs the OOB data before it can read the page data, an unnecessary read sequence is sent to the nand. This reduces read performance.
By how much? Is a similar patch going into Linux?
"How much" is difficult to answer. My platform is an 300MHz ARM9, with a 100MHz bus clock speed (to the 8 bit NAND). I have optimised the nand_read_buf function in a architecture specific way that makes it almost 120% faster than the default (4.3MB/s -> 9.3MB/s).
In that context the overhead of the extra read operation is about 10%. Not huge, but I have 13MBytes to pull out of NAND at boot time and every little helps. There is another potential 10% in the OOB first read function, which I intend to go after next. There may be another ~10% that can be squeezed out, but that needs more analysis.
I do intend to address these same issues in the Linux driver later.
This sequence is sent by default before all page reads, but the OOB first page read function immediately issues a new command, a simulated READOOB command, which overrides the previous sequence.
Is it just a performance issue, or could some NAND chips get confused by the aborted read command?
I'm not aware that the sequence is invalid. It seems not to cause an issue on the device I'm using. I can only present it as a performance issue - and that it looks confusing on a bus analyser.
This patch (fragment) prevents the initial read sequence from being sent if chip->ecc.mode indicates OOB first operation.
I can apply this if it's needed, but I'm hesitant to keep adding workarounds for layering problems. Is there any way we can push all cmdfunc invocations
That was my main concern as well. Also I wanted to guage reaction to submitting patches for code that essentially is copied from Linux, before I start with any major monkeying around in there.
into replaceable functions? Have nand_do_read_ops just be a loop around high-level "read page" functions that are replaceable, and which can keep their own state to determine whether autoinc is applicable.
Good questions. Needs more thought, but sounds reasonable to me. I will look into the implications of this. I can only test OOB first page read and only 2k page devices.
The code does spend a lot of time waiting around for the NAND to be ready when it could actually be doing something useful at the same time. Autoinc should be maintained and also sequential-read commands could help performance, rather than falling back to read0 all the time. [read0 has 25us busy time, read sequential has 3us]
Ideally a high-level driver like fsl_elbc_nand wouldn't have to implement cmdfunc at all.
I'm not sure I know what you mean here. I'll have a look at that code. cmdfunc should probably be simplified to only send command sequences and not invoke any nand wait functions. This gets in the way of some useful optimisation. If some drivers are replacing it, it will be more difficult to coordinate changes.
Nick.