* [U-Boot] [PATCH] nand/mxc: set host->page_addr for NAND_CMD_READOOB @ 2013-01-31 19:47 Scott Wood 2013-01-31 20:45 ` Benoît Thébaudeau 0 siblings, 1 reply; 4+ messages in thread From: Scott Wood @ 2013-01-31 19:47 UTC (permalink / raw) To: u-boot 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. Signed-off-by: Scott Wood <scottwood@freescale.com> --- ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] nand/mxc: set host->page_addr for NAND_CMD_READOOB 2013-01-31 19:47 [U-Boot] [PATCH] nand/mxc: set host->page_addr for NAND_CMD_READOOB Scott Wood @ 2013-01-31 20:45 ` Benoît Thébaudeau 2013-01-31 22:36 ` Scott Wood 0 siblings, 1 reply; 4+ messages in thread From: Benoît Thébaudeau @ 2013-01-31 20:45 UTC (permalink / raw) To: u-boot 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. > > Signed-off-by: Scott Wood <scottwood@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? Do you have a means of duplicating the issue? 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. Best regards, Beno?t ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] nand/mxc: set host->page_addr for NAND_CMD_READOOB 2013-01-31 20:45 ` Benoît Thébaudeau @ 2013-01-31 22:36 ` Scott Wood 2013-01-31 23:01 ` Benoît Thébaudeau 0 siblings, 1 reply; 4+ messages in thread From: Scott Wood @ 2013-01-31 22:36 UTC (permalink / raw) To: u-boot 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? > > Signed-off-by: Scott Wood <scottwood@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. > Do you have a means of duplicating the issue? Not personally; it was found during an IRC discussion with a2cypher. > 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 -Scott ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] nand/mxc: set host->page_addr for NAND_CMD_READOOB 2013-01-31 22:36 ` Scott Wood @ 2013-01-31 23:01 ` Benoît Thébaudeau 0 siblings, 0 replies; 4+ messages in thread From: Benoît Thébaudeau @ 2013-01-31 23:01 UTC (permalink / raw) To: u-boot 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@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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-01-31 23:01 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-31 19:47 [U-Boot] [PATCH] nand/mxc: set host->page_addr for NAND_CMD_READOOB Scott Wood 2013-01-31 20:45 ` Benoît Thébaudeau 2013-01-31 22:36 ` Scott Wood 2013-01-31 23:01 ` Benoît Thébaudeau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.