[U-Boot] [PATCH] nand/mxc: set host->page_addr for NAND_CMD_READOOB

[Benoît Thébaudeau]

On Thursday, January 31, 2013 11:36:11 PM, Scott Wood wrote:
> On 01/31/2013 02:45:02 PM, Benoît Thébaudeau wrote:
> > Hi Scott,
> > 
> > On Thursday, January 31, 2013 8:47:55 PM, Scott Wood wrote:
> > > Without this, all OOB reads are from the last page normally read
> > > (or zero at boot).  This results in bad block scans failing to look
> > > in the right place, and so no bad blocks are found.
> 
> Looking more closely, the calls to send_addr() use page_addr rather
> than host->page_addr, and the oob page read uses "page" in the call to
> cmdfunc, so maybe it's just the debug output that was wrong?

Looks like.

> > > Signed-off-by: Scott Wood <scottwood at freescale.com>
> > > ---
> > > From IRC discussion with a2cypher.  Compile-tested only; testing
> > > would be appreciated.
> > >
> > >  drivers/mtd/nand/mxc_nand.c |    1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/mtd/nand/mxc_nand.c
> > b/drivers/mtd/nand/mxc_nand.c
> > > index d0ded48..32ba340 100644
> > > --- a/drivers/mtd/nand/mxc_nand.c
> > > +++ b/drivers/mtd/nand/mxc_nand.c
> > > @@ -1021,6 +1021,7 @@ void mxc_nand_command(struct mtd_info *mtd,
> > unsigned
> > > command,
> > >  		break;
> > >
> > >  	case NAND_CMD_READOOB:
> > > +		host->page_addr = page_addr;
> > >  		host->col_addr = column;
> > >  		host->spare_only = true;
> > >  		if (host->pagesize_2k)
> > 
> > For which NFC version and NAND Flash page size is this?
> 
> All, presumably.

My i.MX platforms with NFC v2.1 and v3 can detect bad blocks (both true ones,
and the fake ones created at the end of the NAND for the BBT), so there is
something else going on if a2cypher had an issue.

> > Do you have a means of duplicating the issue?
> 
> Not personally; it was found during an IRC discussion with a2cypher.

OK.

> > I wonder if the appropriate fix would not rather be to replace all
> > occurrences
> > of "host->page_addr" with "page", except in mxc_nand_correct_data()
> > and
> > mxc_nand_command(). Otherwise, it looks like there will still be
> > weird things
> > going on with this variable.
> 
> Hmm, what's going on in mxc_nand_read_page_raw_syndrome()?  The caller
> of that calls cmdfunc, then the mxc_nand_read_page_raw_syndrome() calls
> it again, using host->page_addr that was set the first time cmdfunc was
> called? :-P

Indeed. Whatever the caller does, it's supposed to pass the right page to
mxc_nand_read_page_raw_syndrome(), so let's use the passed "page" everywhere,
and use "host->page_addr" only to keep track of accessed pages for
mxc_nand_correct_data(), so that there is no dependency between calls?

Best regards,
Benoît