[U-Boot] [PATCH] NAND: fix "raw" reads with ECC syndrome layouts
Paulraj, Sandeep
s-paulraj at ti.com
Sat Nov 7 20:37:20 CET 2009
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 at 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 at users.sourceforge.net>
> Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
> Signed-off-by: David Woodhouse <David.Woodhouse at 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;
> --
> 1.6.0.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list