nand/mxc: set host->page_addr for NAND_CMD_READOOB
PATCH 1/1 nand/mxc: set host->page_addr for NAND_CMD_READOOB
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.
---
PATCH 1/1 nand/mxc: set host->page_addr for NAND_CMD_READOOB
Hi Scott,
Quoted 24 lines · Scott Wood
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 <[email protected]>---
From IRC discussion with a2cypher. Compile-tested only; testing would be appreciated.drivers/mtd/nand/mxc_nand.c | 1 + 1 file changed, 1 insertion(+)drivers/mtd/nand/mxc_nand.c +1
-0@@ -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;
Quoted 1 line
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
PATCH 1/1 nand/mxc: set host->page_addr for NAND_CMD_READOOB
Quoted 6 lines · Beno?t Th?baudeau
Hi Scott,
Quoted 3 lines · Scott Wood
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?
Quoted 18 lines
Review
Quoted 9 lines
Signed-off-by Scott Wood <[email protected]>---
From IRC discussion with a2cypher. Compile-tested only; testing would be appreciated.drivers/mtd/nand/mxc_nand.c | 1 + 1 file changed, 1 insertion(+)(quoted diff) +0
-0
b/drivers/mtd/nand/mxc_nand.cdrivers/mtd/nand/mxc_nand.c void mxc_nand_command(struct mtd_info *mtd,
Quoted 4 lines
index d0ded48..32ba340 100644
drivers/mtd/nand/mxc_nand.c +0
-0@@ -1021,6 +1021,7 @@ void mxc_nand_command(struct mtd_info *mtd,unsigned
Quoted 3 lines
drivers/mtd/nand/mxc_nand.c +0
-0command, break;
Quoted 7 lines
Quoted 5 lines
drivers/mtd/nand/mxc_nand.c +1
-0case 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.
Quoted 1 line
Do you have a means of duplicating the issue?
Not personally; it was found during an IRC discussion with a2cypher.
Quoted 7 lines
I wonder if the appropriate fix would not rather be to replace all occurrences
of "host->page_addr" with "page", except inmxc_nand_correct_data()andmxc_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
PATCH 1/1 nand/mxc: set host->page_addr for NAND_CMD_READOOB
Quoted 11 lines · Scott Wood
Quoted 6 lines · Beno?t Th?baudeau
Hi Scott,
Quoted 3 lines · Scott Wood
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()usepage_addrrather thanhost->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.
Quoted 26 lines
Quoted 18 lines
Review
Quoted 9 lines
Signed-off-by Scott Wood <[email protected]>---
From IRC discussion with a2cypher. Compile-tested only; testing would be appreciated.drivers/mtd/nand/mxc_nand.c | 1 + 1 file changed, 1 insertion(+)(quoted diff) +0
-0
b/drivers/mtd/nand/mxc_nand.cdrivers/mtd/nand/mxc_nand.c void mxc_nand_command(struct mtd_info *mtd,
Quoted 4 lines
index d0ded48..32ba340 100644
drivers/mtd/nand/mxc_nand.c +0
-0@@ -1021,6 +1021,7 @@ void mxc_nand_command(struct mtd_info *mtd,unsigned
Quoted 3 lines
drivers/mtd/nand/mxc_nand.c +0
-0command, break;Quoted 7 lines
Quoted 5 lines
drivers/mtd/nand/mxc_nand.c +1
-0case 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?
Quoted 1 line
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.
Quoted 3 lines
Quoted 1 line
Do you have a means of duplicating the issue?
Not personally; it was found during an IRC discussion with a2cypher.
OK.
Quoted 8 lines
Quoted 7 lines
I wonder if the appropriate fix would not rather be to replace all occurrences
of "host->page_addr" with "page", except inmxc_nand_correct_data()andmxc_nand_command(). Otherwise, it looks like there will still be weird things
going on with this variable.
Quoted 4 lines
Hmm, what's going on in
mxc_nand_read_page_raw_syndrome()? The caller of that calls cmdfunc, then themxc_nand_read_page_raw_syndrome()calls it again, usinghost->page_addrthat 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