[U-Boot] [PATCH RFC] NAND: Improve read performance from Large Page NAND devices

[Scott Wood]

Nick Thompson wrote:
> I could put in a CONFIG_NAND_NO_LARGE_PAGE as well, but since small
> page devices are EOL now I was only looking to make it easier to
> remove legacy code (a bit like the Museum IDs in nand_ids.c).

I don't think small page support is going away any time soon.

>> Does it really take a noticeable amount of time to do reset_timer() and 
>> get_timer() once?
> 
> On ARM the timer functions use divides to adjust the timer values.

Yay mandatory 1000 Hz clock rate. :-P

Does ARM have a 64-bit multiply that can be used to speed up 
division-by-constant?  If not, can the division be pulled out of 
reset_timer by doing (new - old) / rate instead of new/rate - old/rate?

I assume it's OK for reset_timer to reset completely to zero, including 
the remainder.

> it takes several microseconds longer to spot the ready status when
> the chip is in fact already ready. It's not major, but it is easily
> measurable.

Wow, I know divide is expensive, but several microseconds?

>> Why nand_wait() before READSTART?  The existing nand_command_lp() 
>> doesn't do this AFAICT.
> 
> In fact I'm only after the functionality in nand_wait. nand_wait always
> sends a "read status" command. To get out of read status state you have
> to send a new command and, if waiting for a read to complete, the obvious
> command is read start as this puts the chip back into read state.
> 
> The full sequence is "read", "read start", (chip goes busy), "read
> status", (polling status reads...), (chip goes ready), "read start",
> (data reads...).

Ah, I see.  So this is specifically to help the case where you don't 
have access to the busy pin?  I'm wondering if it will slow things down 
when the busy pin is hooked up, versus not having to issue the extra 
commands and read the status.

>> If we need to communicate to the read_page function whether this is the 
>> last page, then that can be a separate flag that isn't tied up with what 
>> the hardware is capable of, or whether a boundary is being spanned.
> 
> Only do_read_ops knows if this is the last page read, so that does need
> to be passed to read page.

Right, the "if" was wondering what the impact would be of just 
unconditionally fetching the next page.

> I didn't want to add such low level stuff into
> mtd struct as it is very transitory information and bloats the struct.

It's internal state of functions supplied by the generic code; that 
seems the place for such things.

Though I don't really care that much, as long as nand_do_read_ops() 
doesn't have to manage the state beyond saying "start" and "end".

> The page read function also needs to know if this is the first page read.

Clearing the state on entry to the read ops function would allow that, 
with only minor dictation of how read_page uses the field.

> The page read functions can handle the boundaries and autoinc status, but
> only at the expense of repeating the same tests in each read page function.

The tests can be factored out into helper functions -- but it should be 
the replaceable read_page functions that contain such low-level 
knowledge.  nand_do_read_ops() should just be a high-level loop around 
read_page(), plus the OOB and partial page shuffling.

-Scott