[PATCH v2 3/3] mtd: nand: introduce a READMODE command

[Gerhard Sittig]

On Tue, 2014-04-15 at 04:10 +0000, Gupta, Pekon wrote:
> 
> Hi Gerhard,
> 
> >From: Gerhard Sittig [mailto:gsi at denx.de]
> >
> >the nand_command_lp() implementation derives a "READPAGE" sequence from
> >a passed in READ0 opcode, i.e. emits a sequence of READ0 _and_ READSTART
> >commands in this case
> >
> >introduce a "READMODE" command which sends the READ0 opcode to the chip
> >exclusively and doesn't send the READSTART opcode
> >
> >such a "READMODE" command is useful in the context of on-die-ECC support
> >where a sequence of READ0, READSTART, STATUS, READ0 is required; having
> >support for READMODE in the common nand_command_lp() routine avoids the
> >need for duplication and open coded cmd_ctrl() calls
> >
> >for the non-"large page" setup (i.e. the nand_command() routine) both
> >commands "READMODE" and "READ0" are identical, "READSTART" exclusively
> >applies to large page configurations
> >
> >Signed-off-by: Gerhard Sittig <gsi at denx.de>
> >---
> >changes in v2:
> >- update the commmit message to discuss that for the nand_command()
> >  routine READMODE results in identical behaviour as READ0
> >- rephrase the NAND_CMD_READMODE command to better reflect that it
> >  re-uses the NAND_CMD_READ0 opcode plus has high bits set
> >
> > drivers/mtd/nand/nand_base.c |    4 +++-
> > include/linux/mtd/nand.h     |   11 +++++++++++
> > 2 files changed, 14 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> >index 7108191b1598..50b8a2a93b4f 100644
> >--- a/drivers/mtd/nand/nand_base.c
> >+++ b/drivers/mtd/nand/nand_base.c
> >@@ -711,7 +711,8 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> >
> > 	/*
> > 	 * Program and erase have their own busy handlers status, sequential
> >-	 * in and status need no delay.
> >+	 * in and status need no delay, read mode just reverts back to
> >+	 * data output after a status command and needs no read start.
> > 	 */
> > 	switch (command) {
> >
> >@@ -722,6 +723,7 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
> > 	case NAND_CMD_SEQIN:
> > 	case NAND_CMD_RNDIN:
> > 	case NAND_CMD_STATUS:
> >+	case NAND_CMD_READMODE:
> > 		return;
> >
> > 	case NAND_CMD_RESET:
> 
> You probably missed doing same change in nand_command() also.

Thank you for looking at the patches, again!

AFAICS nothing is missing, the difference is there by purpose.

The READMODE command does not require special handling in the
nand_command() case, as the sequence of READ0, READSTART, STATUS,
READ0 does not apply there (it's only relevant to large page
setups).

In the nand_command() case the sequence would be READ0, STATUS,
READ0 -- and so READ0 and READMODE are handled identically,
READSTART does not exist in that scenario.  And the STATUS and
READ0 only would apply in the on-die-ECC scenario, which might as
well not occur at all without large pages.

I tried to reason about this situation in the commit message, but
might not have come up with the best phrases there.  (That's the
reason for the excessive quotation above.)

There is a difference between nand_command_lp() and
nand_command() in that for the latter the re-emitted READMODE
will result in another busy check (or a delay in the absence of
an R/B pin condition).  So we fail on the safe side, which should
be OK.

Would an improved commit message help?  Or shall I put an
explicit comment into nand_command() about why it's different
from nand_command_lp()?  Although it already is, the situation is
not new ...


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de