Nick Thompson wrote:
Improve read performance from Large Page NAND devices.
This patch employs the following concepts to produce a ~37% improvement in oob_first read speed (on a 300MHz ARM9). The time for a mid-buffer 2k page read is now 260us, 7.88MB/s (was 357us, 5.74MB/s). oob_first is probably the best case improvement.
Provides a new config option to allow building for large page devices only. reducing code size by ~800 bytes. [CONFIG_SYS_NAND_NO_SMALL_PAGE] This almost exactly compensates for the code increase due to other changes.
Could we make it more orthogonal? I.e. CONFIG_NAND_512B_PAGE, CONFIG_NAND_2K_PAGE, CONFIG_NAND_4K_PAGE? As is, it does nothing to keep things from growing for small-page-only boards.
As it would determine what support is present rather than what the hardware actually is, I don't think it would go in CONFIG_SYS.
- /* The chip might be ready by now, don't lose anymore time */
- if (this->dev_ready) {
if (this->dev_ready(mtd))goto ready;- } else {
if (this->read_byte(mtd) & NAND_STATUS_READY)goto ready;- }
Does it really take a noticeable amount of time to do reset_timer() and get_timer() once?
- Wait for cache ready after read request.
- Returns to read state before returning.
- @mtd: mtd info structure
- @chip: nand chip info structure
- */
+static int nand_wait_cache_load(struct mtd_info *mtd, struct nand_chip *chip) +{
- int state = nand_wait(mtd, chip);
- chip->cmd_ctrl(mtd, NAND_CMD_READSTART, NAND_CLE | NAND_CTRL_CLE |
NAND_CTRL_CHANGE);
NAND_CTRL_CLE includes NAND_CLE.
Why nand_wait() before READSTART? The existing nand_command_lp() doesn't do this AFAICT.
This change will break drivers that support large page and use the default read_page functions, but do not implement cmd_ctrl (they replace cmdfunc instead). This includes fsl_elbc_nand, mxc_nand, and mpc5121_nfc. While I'd like to move them to implementing their own read_page-type functions instead of cmdfunc, is there any way to make it a smoother transition?
- chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_CLE | NAND_CTRL_CHANGE);
Shouldn't this be NAND_NCE | NAND_CTRL_CHANGE? Don't we want to drop CLE here?
- if (nand_next_page_req(*rstate))
chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page+1);
Spaces around binary operators.
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index cb7c19a..85b7c3c 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -269,17 +269,20 @@ struct nand_ecc_ctrl { uint8_t *calc_ecc); int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
uint8_t *buf, int page);
uint8_t *buf, int page,uint32_t *rstate);
uint8_t *buf, int page);
uint8_t *buf, int page,uint32_t *rstate);
uint8_t *buf);
uint8_t *buf, int page,uint32_t *rstate);
Does rstate really need to be a parameter, or could it be part of the mtd struct? I'd really like nand_do_read_ops() to not have to know about any of this, and have it be internal to the read_page functions -- other than perhaps clearing the value on exit, or some other way to let read_page know that its context has changed.
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.
-Scott