[U-Boot] [PATCH] NAND: fix "raw" reads with ECC syndrome layouts
From: David Brownell dbrownell@users.sourceforge.net
The syndrome based page read/write routines store ECC, and possibly other "OOB" data, right after each chunk of ECC'd data. With ECC chunk size of 512 bytes and a large page (2KiB) NAND, the layout is:
data-0 OOB-0 data-1 OOB-1 data-2 OOB-2 data-3 OOB-3 OOB-leftover
Where OOBx is (prepad, ECC, postpad). However, the current "raw" routines use a traditional layout -- data OOB, disregarding the prepad and postpad values -- so when they're used with that type of ECC hardware, those calls mix up the data and OOB. Which means, in particular, that bad block tables won't be found on startup, with data corruption and related chaos ensuing.
The current syndrome-based drivers in mainline all seem to use one chunk per page; presumably they haven't noticed such bugs.
Fix this, by adding read/write page_raw_syndrome() routines as siblings of the existing non-raw routines; "raw" just means to bypass the ECC computations, not change data and OOB layout.
Signed-off-by: David Brownell dbrownell@users.sourceforge.net Signed-off-by: Andrew Morton akpm@linux-foundation.org Signed-off-by: David Woodhouse David.Woodhouse@intel.com --- Adding this patch to the U-Boot NAND driver to sync up with the kernel NAND driver. There are 2 checkpatch warnings. I will let Scott Wood(u-boot-nand-flash custodian) decide on whether to fix this or not. E-mails exchanged on the u-boot mailing list semed to me that this might not be a big issue especially as we are merely copying from the kernel. drivers/mtd/nand/nand_base.c | 100 ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index cf032a6..6da261c 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -894,6 +894,8 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *this) * @chip: nand chip info structure * @buf: buffer to store read data * @page: page number to read + * + * Not for syndrome calculating ecc controllers, which use a special oob layout */ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, uint8_t *buf, int page) @@ -904,6 +906,48 @@ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, }
/** + * nand_read_page_raw_syndrome - [Intern] read raw page data without ecc + * @mtd: mtd info structure + * @chip: nand chip info structure + * @buf: buffer to store read data + * @page: page number to read + * + * We need a special oob layout and handling even when OOB isn't used. + */ +static int nand_read_page_raw_syndrome(struct mtd_info *mtd, struct nand_chip *chip, + uint8_t *buf, int page) +{ + int eccsize = chip->ecc.size; + int eccbytes = chip->ecc.bytes; + uint8_t *oob = chip->oob_poi; + int steps, size; + + for (steps = chip->ecc.steps; steps > 0; steps--) { + chip->read_buf(mtd, buf, eccsize); + buf += eccsize; + + if (chip->ecc.prepad) { + chip->read_buf(mtd, oob, chip->ecc.prepad); + oob += chip->ecc.prepad; + } + + chip->read_buf(mtd, oob, eccbytes); + oob += eccbytes; + + if (chip->ecc.postpad) { + chip->read_buf(mtd, oob, chip->ecc.postpad); + oob += chip->ecc.postpad; + } + } + + size = mtd->oobsize - (oob - chip->oob_poi); + if (size) + chip->read_buf(mtd, oob, size); + + return 0; +} + +/** * nand_read_page_swecc - [REPLACABLE] software ecc based page read function * @mtd: mtd info structure * @chip: nand chip info structure @@ -1682,6 +1726,8 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from, * @mtd: mtd info structure * @chip: nand chip info structure * @buf: data buffer + * + * Not for syndrome calculating ecc controllers, which use a special oob layout */ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, const uint8_t *buf) @@ -1691,6 +1737,44 @@ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, }
/** + * nand_write_page_raw_syndrome - [Intern] raw page write function + * @mtd: mtd info structure + * @chip: nand chip info structure + * @buf: data buffer + * + * We need a special oob layout and handling even when ECC isn't checked. + */ +static void nand_write_page_raw_syndrome(struct mtd_info *mtd, struct nand_chip *chip, + const uint8_t *buf) +{ + int eccsize = chip->ecc.size; + int eccbytes = chip->ecc.bytes; + uint8_t *oob = chip->oob_poi; + int steps, size; + + for (steps = chip->ecc.steps; steps > 0; steps--) { + chip->write_buf(mtd, buf, eccsize); + buf += eccsize; + + if (chip->ecc.prepad) { + chip->write_buf(mtd, oob, chip->ecc.prepad); + oob += chip->ecc.prepad; + } + + chip->read_buf(mtd, oob, eccbytes); + oob += eccbytes; + + if (chip->ecc.postpad) { + chip->write_buf(mtd, oob, chip->ecc.postpad); + oob += chip->ecc.postpad; + } + } + + size = mtd->oobsize - (oob - chip->oob_poi); + if (size) + chip->write_buf(mtd, oob, size); +} +/** * nand_write_page_swecc - [REPLACABLE] software ecc based page write function * @mtd: mtd info structure * @chip: nand chip info structure @@ -2781,10 +2865,6 @@ int nand_scan_tail(struct mtd_info *mtd) * check ECC mode, default to software if 3byte/512byte hardware ECC is * selected and we have 256 byte pagesize fallback to software ECC */ - if (!chip->ecc.read_page_raw) - chip->ecc.read_page_raw = nand_read_page_raw; - if (!chip->ecc.write_page_raw) - chip->ecc.write_page_raw = nand_write_page_raw;
switch (chip->ecc.mode) { case NAND_ECC_HW_OOB_FIRST: @@ -2804,6 +2884,10 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_hwecc; if (!chip->ecc.write_page) chip->ecc.write_page = nand_write_page_hwecc; + if (!chip->ecc.read_page_raw) + chip->ecc.read_page_raw = nand_read_page_raw; + if (!chip->ecc.write_page_raw) + chip->ecc.write_page_raw = nand_write_page_raw; if (!chip->ecc.read_oob) chip->ecc.read_oob = nand_read_oob_std; if (!chip->ecc.write_oob) @@ -2825,6 +2909,10 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_syndrome; if (!chip->ecc.write_page) chip->ecc.write_page = nand_write_page_syndrome; + if (!chip->ecc.read_page_raw) + chip->ecc.read_page_raw = nand_read_page_raw_syndrome; + if (!chip->ecc.write_page_raw) + chip->ecc.write_page_raw = nand_write_page_raw_syndrome; if (!chip->ecc.read_oob) chip->ecc.read_oob = nand_read_oob_syndrome; if (!chip->ecc.write_oob) @@ -2843,6 +2931,8 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_swecc; chip->ecc.read_subpage = nand_read_subpage; chip->ecc.write_page = nand_write_page_swecc; + chip->ecc.read_page_raw = nand_read_page_raw; + chip->ecc.write_page_raw = nand_write_page_raw; chip->ecc.read_oob = nand_read_oob_std; chip->ecc.write_oob = nand_write_oob_std; chip->ecc.size = 256; @@ -2855,6 +2945,8 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_raw; chip->ecc.write_page = nand_write_page_raw; chip->ecc.read_oob = nand_read_oob_std; + chip->ecc.read_page_raw = nand_read_page_raw; + chip->ecc.write_page_raw = nand_write_page_raw; chip->ecc.write_oob = nand_write_oob_std; chip->ecc.size = mtd->writesize; chip->ecc.bytes = 0;
Quick head up. I submitted this patch to the u-boot mailing list on David Brownell's behalf.
Sandeep
Subject: [U-Boot] [PATCH] NAND: fix "raw" reads with ECC syndrome layouts
From: David Brownell dbrownell@users.sourceforge.net
The syndrome based page read/write routines store ECC, and possibly other "OOB" data, right after each chunk of ECC'd data. With ECC chunk size of 512 bytes and a large page (2KiB) NAND, the layout is:
data-0 OOB-0 data-1 OOB-1 data-2 OOB-2 data-3 OOB-3 OOB-leftover
Where OOBx is (prepad, ECC, postpad). However, the current "raw" routines use a traditional layout -- data OOB, disregarding the prepad and postpad values -- so when they're used with that type of ECC hardware, those calls mix up the data and OOB. Which means, in particular, that bad block tables won't be found on startup, with data corruption and related chaos ensuing.
The current syndrome-based drivers in mainline all seem to use one chunk per page; presumably they haven't noticed such bugs.
Fix this, by adding read/write page_raw_syndrome() routines as siblings of the existing non-raw routines; "raw" just means to bypass the ECC computations, not change data and OOB layout.
Signed-off-by: David Brownell dbrownell@users.sourceforge.net Signed-off-by: Andrew Morton akpm@linux-foundation.org Signed-off-by: David Woodhouse David.Woodhouse@intel.com
Adding this patch to the U-Boot NAND driver to sync up with the kernel NAND driver. There are 2 checkpatch warnings. I will let Scott Wood(u-boot-nand-flash custodian) decide on whether to fix this or not. E-mails exchanged on the u-boot mailing list semed to me that this might not be a big issue especially as we are merely copying from the kernel. drivers/mtd/nand/nand_base.c | 100 ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index cf032a6..6da261c 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -894,6 +894,8 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *this)
- @chip: nand chip info structure
- @buf: buffer to store read data
- @page: page number to read
- Not for syndrome calculating ecc controllers, which use a special oob
layout */ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, uint8_t *buf, int page) @@ -904,6 +906,48 @@ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, }
/**
- nand_read_page_raw_syndrome - [Intern] read raw page data without ecc
- @mtd: mtd info structure
- @chip: nand chip info structure
- @buf: buffer to store read data
- @page: page number to read
- We need a special oob layout and handling even when OOB isn't used.
- */
+static int nand_read_page_raw_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
uint8_t *buf, int page)+{
- int eccsize = chip->ecc.size;
- int eccbytes = chip->ecc.bytes;
- uint8_t *oob = chip->oob_poi;
- int steps, size;
- for (steps = chip->ecc.steps; steps > 0; steps--) {
chip->read_buf(mtd, buf, eccsize);buf += eccsize;if (chip->ecc.prepad) {chip->read_buf(mtd, oob, chip->ecc.prepad);oob += chip->ecc.prepad;}chip->read_buf(mtd, oob, eccbytes);oob += eccbytes;if (chip->ecc.postpad) {chip->read_buf(mtd, oob, chip->ecc.postpad);oob += chip->ecc.postpad;}- }
- size = mtd->oobsize - (oob - chip->oob_poi);
- if (size)
chip->read_buf(mtd, oob, size);- return 0;
+}
+/**
- nand_read_page_swecc - [REPLACABLE] software ecc based page read
function
- @mtd: mtd info structure
- @chip: nand chip info structure
@@ -1682,6 +1726,8 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
- @mtd: mtd info structure
- @chip: nand chip info structure
- @buf: data buffer
- Not for syndrome calculating ecc controllers, which use a special oob
layout */ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, const uint8_t *buf) @@ -1691,6 +1737,44 @@ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip, }
/**
- nand_write_page_raw_syndrome - [Intern] raw page write function
- @mtd: mtd info structure
- @chip: nand chip info structure
- @buf: data buffer
- We need a special oob layout and handling even when ECC isn't checked.
- */
+static void nand_write_page_raw_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
const uint8_t *buf)+{
- int eccsize = chip->ecc.size;
- int eccbytes = chip->ecc.bytes;
- uint8_t *oob = chip->oob_poi;
- int steps, size;
- for (steps = chip->ecc.steps; steps > 0; steps--) {
chip->write_buf(mtd, buf, eccsize);buf += eccsize;if (chip->ecc.prepad) {chip->write_buf(mtd, oob, chip->ecc.prepad);oob += chip->ecc.prepad;}chip->read_buf(mtd, oob, eccbytes);oob += eccbytes;if (chip->ecc.postpad) {chip->write_buf(mtd, oob, chip->ecc.postpad);oob += chip->ecc.postpad;}- }
- size = mtd->oobsize - (oob - chip->oob_poi);
- if (size)
chip->write_buf(mtd, oob, size);+} +/**
- nand_write_page_swecc - [REPLACABLE] software ecc based page write
function
- @mtd: mtd info structure
- @chip: nand chip info structure
@@ -2781,10 +2865,6 @@ int nand_scan_tail(struct mtd_info *mtd) * check ECC mode, default to software if 3byte/512byte hardware ECC is * selected and we have 256 byte pagesize fallback to software ECC */
if (!chip->ecc.read_page_raw)
chip->ecc.read_page_raw = nand_read_page_raw;if (!chip->ecc.write_page_raw)
chip->ecc.write_page_raw = nand_write_page_raw;switch (chip->ecc.mode) { case NAND_ECC_HW_OOB_FIRST:
@@ -2804,6 +2884,10 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_hwecc; if (!chip->ecc.write_page) chip->ecc.write_page = nand_write_page_hwecc;
if (!chip->ecc.read_page_raw)chip->ecc.read_page_raw = nand_read_page_raw;if (!chip->ecc.write_page_raw)chip->ecc.write_page_raw = nand_write_page_raw;@@ -2825,6 +2909,10 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_syndrome; if (!chip->ecc.write_page) chip->ecc.write_page = nand_write_page_syndrome;
if (!chip->ecc.read_page_raw)chip->ecc.read_page_raw = nand_read_page_raw_syndrome;if (!chip->ecc.write_page_raw)chip->ecc.write_page_raw = nand_write_page_raw_syndrome;@@ -2843,6 +2931,8 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_swecc; chip->ecc.read_subpage = nand_read_subpage; chip->ecc.write_page = nand_write_page_swecc;
chip->ecc.read_page_raw = nand_read_page_raw;chip->ecc.write_page_raw = nand_write_page_raw;@@ -2855,6 +2945,8 @@ int nand_scan_tail(struct mtd_info *mtd) chip->ecc.read_page = nand_read_page_raw; chip->ecc.write_page = nand_write_page_raw; chip->ecc.read_oob = nand_read_oob_std;
chip->ecc.read_page_raw = nand_read_page_raw;chip->ecc.write_page_raw = nand_write_page_raw;-- 1.6.0.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Subject: Re: [U-Boot] [PATCH] NAND: fix "raw" reads with ECC syndrome layouts
On Saturday 07 November 2009, Paulraj, Sandeep wrote:
Quick head up. I submitted this patch to the u-boot mailing list on David Brownell's
behalf.
Sandeep
You should *NEVER* forge "From" addresses like spammers do.
Sorry about that. I gave the wrong --from address while using git-send-email.
Could you be kind enough to take a look at v2 that I just sent?
Sandeep
participants (3)
-
David Brownell -
dbrownellļ¼ users.sourceforge.net -
Paulraj, Sandeep