[U-Boot] [PATCH v3 0/3] mtd: nand: omap: fix ecc-layout
*changes v2 -> v3* - re-ordered the patch-set so that eccpos[] is fixed before 'oobfree' - marked for stable 3.13.x+
*changes v1 -> v2* [PATCH 1/3] fix oobfree->offset calculation + adjust for reserved-marker of last sector [PATCH 2/3] <new patch> [PATCH 3/3] refactor code as suggested by Brian Norris computersforpeace@gmail.com
*original v1* [PATCH 1/2] http://lists.infradead.org/pipermail/linux-mtd/2013-December/050946.html [PATCH 2/2] http://lists.infradead.org/pipermail/linux-mtd/2013-December/050945.html
This patch-set is tested on AM335x-EVM using following end-to-end boot sequence with appropriate u-boot configs [1]
(BCH8_HW) (HAM1_HW) (HAM1_HW) (HAM1_HW) (UBIFS) ROM ---------> SPL ---------> U-Boot ---------> Kernel ---------> File-System
(BCH8_HW) (BCH8_SW) (BCH8_SW) (BCH8_SW) (UBIFS) ROM ---------> SPL ---------> U-Boot ---------> Kernel ---------> File-System
(BCH8_HW) (BCH8_HW) (BCH8_HW) (BCH8_HW) (UBIFS) ROM ---------> SPL ---------> U-Boot ---------> Kernel ---------> File-System
Test1: flash ubi image from u-boot and boot the kernel u-boot> mw 0x82000000 0xff <u-boot.img size aligned to NAND block boundary> u-boot> fatload mmc 0 0x82000000 u-boot.img u-boot> nand erase <u-boot_offset> <u-boot.img size> u-boot> nand write 0x82000000 <u-boot_offset> <u-boot.img size> u-boot> setenv bootargs 'console=ttyO0,115200n8 noinitrd mem=256M \ root=ubi0 rw rootfstype=ubifs ubi.mtd=<mtdpart-of-rootfs>,\ <page-size> ip=off init=/init' u-boot> bootm <kernel_offset>
Test2: update u-boot.img from kernel and re-boot kernel> flash_erase /dev/<mtdpart-of-u-boot> 0 0 kernel> nandwrite -s 0 /dev/<mtdpart-of-u-boot> u-boot.img kernel> reboot
[1] u-boot configurations to match above ecc-layout are documented at https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide
Pekon Gupta (3): mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver mtd: nand: omap: fix ecclayout->oobfree->offset mtd: nand: omap: fix ecclayout->oobfree->length
drivers/mtd/nand/omap2.c | 61 +++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 21 deletions(-)
Fixes: commit a919e51161b58ed7e6e663daba99ab7d558808f3 mtd: nand: omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe
Fixes ecclayout mismatch introduced in above commit for following ecc-schemes: - OMAP_ECC_BCH4_CODE_HW_DETECTION_SW - OMAP_ECC_BCH8_CODE_HW_DETECTION_SW However, this patch also touches other ecc-schemes as the fix required refactoring common code, into ecc-scheme specific code.
This patch aligns ecc-layout for below ecc-schemes as per reference [1],[2],[3]
+---+------------+-------------++-------------+-------------+ |OOB|BCH8_CODE_HW|BCH8_CODE_HW_||HAM1_CODE_HW |HAM1_CODE_HW | |pos| | DETECTION_SW||(x8 device) |(x16 device) | +---+------------+-------------++-------------+-------------+ | 0 |BADBLK_MARK | BADBLK_MARK || BADBLK_MARK | BADBLK_MARK | | 1 |BADBLK_MARK | BADBLK_MARK || eccpos[0] | BADBLK_MARK | | 2 | eccpos[0] | eccpos[0] || eccpos[1] | eccpos[0] | | 3 | eccpos[1] | eccpos[1] || eccpos[2] | eccpos[1] | | 4 | eccpos[2] | eccpos[2] || eccpos[3] | eccpos[2] | | 5 | eccpos[3] | eccpos[3] || eccpos[4] | eccpos[3] | | 6 | eccpos[4] | eccpos[4] || eccpos[5] | eccpos[4] | | 7 | eccpos[5] | eccpos[5] || eccpos[6] | eccpos[5] | | 8 | eccpos[6] | eccpos[6] || eccpos[7] | eccpos[6] | | 9 | eccpos[7] | eccpos[7] || eccpos[8] | eccpos[7] | |10 | eccpos[8] | eccpos[8] || eccpos[9] | eccpos[8] | |11 | eccpos[9] | eccpos[9] || eccpos[10] | eccpos[9] | |12 | eccpos[10] | eccpos[10] || eccpos[11] | eccpos[10] | |13 | eccpos[11] | eccpos[11] || oobfree[0] | eccpos[11] | |14 | eccpos[12] | eccpos[12] || oobfree[1] | oobfree[0] | |15 | eccpos[13] | <reserved> || oobfree[2] | oobfree[1] | +---+------------+-------------++-------------+-------------+ |16 | eccpos[14] | eccpos[13] || oobfree[3] | oobfree[2] | |...| [...] | [...] || [...] | [...] | |56 | eccpos[54] | eccpos[51] || oobfree[43] | oobfree[42] | |57 | eccpos[55] | <reserved> || oobfree[44] | oobfree[43] | +===+============+=============+==============+=============+ |58 | oobfree[0] | oobfree[0] || oobfree[45] | oobfree[44] | |59 | oobfree[1] | oobfree[1] || oobfree[46] | oobfree[45] | |60 | oobfree[2] | oobfree[2] || oobfree[47] | oobfree[46] | |61 | oobfree[3] | oobfree[3] || oobfree[48] | oobfree[47] | |62 | oobfree[4] | oobfree[4] || oobfree[49] | oobfree[48] | |63 | oobfree[5] | oobfree[5] || oobfree[50] | oobfree[49] | +---+------------+-------------+--------------+-------------+
[1] ecc-layout expected by ROM code, as specified in SoC TRM under: Chapter="Initialization" Section="Device Initialization by ROM code" Sub-Section="Memory Booting" Heading="NAND" Figure="ECC Locations in NAND Spare Areas"
[2] ecc-layout updates in u-boot http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
[3] u-boot configurations to match above ecc-layout are documented at https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide
CC: stable@vger.kernel.org # 3.13.x+ Reported-by: Enric Balletbo Serra eballetbo@iseebcn.com Tested-by: Enric Balletbo i Serra eballetbo@gmail.com Tested-by: Stefan Roese sr@denx.de Signed-off-by: Pekon Gupta pekon@ti.com --- drivers/mtd/nand/omap2.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index ef4190a..34ef941 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1633,6 +1633,7 @@ static int omap_nand_probe(struct platform_device *pdev) int i; dma_cap_mask_t mask; unsigned sig; + unsigned oob_index; struct resource *res; struct mtd_part_parser_data ppdata = {};
@@ -1826,9 +1827,11 @@ static int omap_nand_probe(struct platform_device *pdev) (mtd->writesize / nand_chip->ecc.size); if (nand_chip->options & NAND_BUSWIDTH_16) - ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH; + oob_index = BADBLOCK_MARKER_LENGTH; else - ecclayout->eccpos[0] = 1; + oob_index = 1; + for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) + ecclayout->eccpos[i] = oob_index; ecclayout->oobfree->offset = ecclayout->eccpos[0] + ecclayout->eccbytes; break; @@ -1847,7 +1850,12 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size); - ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH; + oob_index = BADBLOCK_MARKER_LENGTH; + for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) { + ecclayout->eccpos[i] = oob_index; + if (((i + 1) % nand_chip->ecc.bytes) == 0) + oob_index++; + } ecclayout->oobfree->offset = ecclayout->eccpos[0] + ecclayout->eccbytes; /* software bch library is used for locating errors */ @@ -1883,7 +1891,9 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size); - ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH; + oob_index = BADBLOCK_MARKER_LENGTH; + for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) + ecclayout->eccpos[i] = oob_index; ecclayout->oobfree->offset = ecclayout->eccpos[0] + ecclayout->eccbytes; /* This ECC scheme requires ELM H/W block */ @@ -1913,7 +1923,12 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size); - ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH; + oob_index = BADBLOCK_MARKER_LENGTH; + for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) { + ecclayout->eccpos[i] = oob_index; + if (((i + 1) % nand_chip->ecc.bytes) == 0) + oob_index++; + } ecclayout->oobfree->offset = ecclayout->eccpos[0] + ecclayout->eccbytes; /* software bch library is used for locating errors */ @@ -1956,7 +1971,9 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size); - ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH; + oob_index = BADBLOCK_MARKER_LENGTH; + for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) + ecclayout->eccpos[i] = oob_index; ecclayout->oobfree->offset = ecclayout->eccpos[0] + ecclayout->eccbytes; break; @@ -1975,8 +1992,6 @@ static int omap_nand_probe(struct platform_device *pdev) /* populate remaining ECC layout data */ ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH + ecclayout->eccbytes); - for (i = 1; i < ecclayout->eccbytes; i++) - ecclayout->eccpos[i] = ecclayout->eccpos[0] + i; /* check if NAND device's OOB is enough to store ECC signatures */ if (mtd->oobsize < (ecclayout->eccbytes + BADBLOCK_MARKER_LENGTH)) { pr_err("not enough OOB bytes required = %d, available=%d\n",
removing stable@vger.kernel.org from CC list. (git send-email --suppress-cc=cc din't work for me)
From: Gupta, Pekon
Fixes: commit a919e51161b58ed7e6e663daba99ab7d558808f3 mtd: nand: omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe
Fixes ecclayout mismatch introduced in above commit for following ecc-schemes:
- OMAP_ECC_BCH4_CODE_HW_DETECTION_SW
- OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
However, this patch also touches other ecc-schemes as the fix required refactoring common code, into ecc-scheme specific code.
This patch aligns ecc-layout for below ecc-schemes as per reference [1],[2],[3]
+---+------------+-------------++-------------+-------------+ |OOB|BCH8_CODE_HW|BCH8_CODE_HW_||HAM1_CODE_HW |HAM1_CODE_HW | |pos| | DETECTION_SW||(x8 device) |(x16 device) | +---+------------+-------------++-------------+-------------+ | 0 |BADBLK_MARK | BADBLK_MARK || BADBLK_MARK | BADBLK_MARK | | 1 |BADBLK_MARK | BADBLK_MARK || eccpos[0] | BADBLK_MARK | | 2 | eccpos[0] | eccpos[0] || eccpos[1] | eccpos[0] | | 3 | eccpos[1] | eccpos[1] || eccpos[2] | eccpos[1] | | 4 | eccpos[2] | eccpos[2] || eccpos[3] | eccpos[2] | | 5 | eccpos[3] | eccpos[3] || eccpos[4] | eccpos[3] | | 6 | eccpos[4] | eccpos[4] || eccpos[5] | eccpos[4] | | 7 | eccpos[5] | eccpos[5] || eccpos[6] | eccpos[5] | | 8 | eccpos[6] | eccpos[6] || eccpos[7] | eccpos[6] | | 9 | eccpos[7] | eccpos[7] || eccpos[8] | eccpos[7] | |10 | eccpos[8] | eccpos[8] || eccpos[9] | eccpos[8] | |11 | eccpos[9] | eccpos[9] || eccpos[10] | eccpos[9] | |12 | eccpos[10] | eccpos[10] || eccpos[11] | eccpos[10] | |13 | eccpos[11] | eccpos[11] || oobfree[0] | eccpos[11] | |14 | eccpos[12] | eccpos[12] || oobfree[1] | oobfree[0] | |15 | eccpos[13] | <reserved> || oobfree[2] | oobfree[1] | +---+------------+-------------++-------------+-------------+ |16 | eccpos[14] | eccpos[13] || oobfree[3] | oobfree[2] | |...| [...] | [...] || [...] | [...] | |56 | eccpos[54] | eccpos[51] || oobfree[43] | oobfree[42] | |57 | eccpos[55] | <reserved> || oobfree[44] | oobfree[43] | +===+============+=============+==============+=============+ |58 | oobfree[0] | oobfree[0] || oobfree[45] | oobfree[44] | |59 | oobfree[1] | oobfree[1] || oobfree[46] | oobfree[45] | |60 | oobfree[2] | oobfree[2] || oobfree[47] | oobfree[46] | |61 | oobfree[3] | oobfree[3] || oobfree[48] | oobfree[47] | |62 | oobfree[4] | oobfree[4] || oobfree[49] | oobfree[48] | |63 | oobfree[5] | oobfree[5] || oobfree[50] | oobfree[49] | +---+------------+-------------+--------------+-------------+
[1] ecc-layout expected by ROM code, as specified in SoC TRM under: Chapter="Initialization" Section="Device Initialization by ROM code" Sub-Section="Memory Booting" Heading="NAND" Figure="ECC Locations in NAND Spare Areas"
[2] ecc-layout updates in u-boot http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
[3] u-boot configurations to match above ecc-layout are documented at https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide
CC: stable@vger.kernel.org # 3.13.x+ Reported-by: Enric Balletbo Serra eballetbo@iseebcn.com Tested-by: Enric Balletbo i Serra eballetbo@gmail.com Tested-by: Stefan Roese sr@denx.de Signed-off-by: Pekon Gupta pekon@ti.com
drivers/mtd/nand/omap2.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index ef4190a..34ef941 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1633,6 +1633,7 @@ static int omap_nand_probe(struct platform_device *pdev) int i; dma_cap_mask_t mask; unsigned sig;
- unsigned oob_index; struct resource *res; struct mtd_part_parser_data ppdata = {};
@@ -1826,9 +1827,11 @@ static int omap_nand_probe(struct platform_device *pdev) (mtd->writesize / nand_chip->ecc.size); if (nand_chip->options & NAND_BUSWIDTH_16)
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
oob_index = BADBLOCK_MARKER_LENGTH;
ecclayout->eccpos[0] = 1;
oob_index = 1;for (i = 0; i < ecclayout->eccbytes; i++, oob_index++)ecclayout->eccpos[i] = oob_index;@@ -1847,7 +1850,12 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
oob_index = BADBLOCK_MARKER_LENGTH;for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) {ecclayout->eccpos[i] = oob_index;if (((i + 1) % nand_chip->ecc.bytes) == 0)oob_index++;}@@ -1883,7 +1891,9 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
oob_index = BADBLOCK_MARKER_LENGTH;for (i = 0; i < ecclayout->eccbytes; i++, oob_index++)ecclayout->eccpos[i] = oob_index;@@ -1913,7 +1923,12 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
oob_index = BADBLOCK_MARKER_LENGTH;for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) {ecclayout->eccpos[i] = oob_index;if (((i + 1) % nand_chip->ecc.bytes) == 0)oob_index++;}@@ -1956,7 +1971,9 @@ static int omap_nand_probe(struct platform_device *pdev) ecclayout->eccbytes = nand_chip->ecc.bytes * (mtd->writesize / nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
oob_index = BADBLOCK_MARKER_LENGTH;for (i = 0; i < ecclayout->eccbytes; i++, oob_index++)ecclayout->eccpos[i] = oob_index;@@ -1975,8 +1992,6 @@ static int omap_nand_probe(struct platform_device *pdev) /* populate remaining ECC layout data */ ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH + ecclayout->eccbytes);
- for (i = 1; i < ecclayout->eccbytes; i++)
ecclayout->eccpos[i] = ecclayout->eccpos[0] + i;-- 1.8.5.1.163.gd7aced9
with regards, pekon
Hi Pekon,
On Mon, 17 Feb 2014 13:11:23 +0530, Pekon Gupta pekon@ti.com wrote:
Fixes: commit a919e51161b58ed7e6e663daba99ab7d558808f3 mtd: nand: omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe
Fixes ecclayout mismatch introduced in above commit for following ecc-schemes:
- OMAP_ECC_BCH4_CODE_HW_DETECTION_SW
- OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
However, this patch also touches other ecc-schemes as the fix required refactoring common code, into ecc-scheme specific code.
This patch aligns ecc-layout for below ecc-schemes as per reference [1],[2],[3]
+---+------------+-------------++-------------+-------------+ |OOB|BCH8_CODE_HW|BCH8_CODE_HW_||HAM1_CODE_HW |HAM1_CODE_HW | |pos| | DETECTION_SW||(x8 device) |(x16 device) | +---+------------+-------------++-------------+-------------+ | 0 |BADBLK_MARK | BADBLK_MARK || BADBLK_MARK | BADBLK_MARK | | 1 |BADBLK_MARK | BADBLK_MARK || eccpos[0] | BADBLK_MARK | | 2 | eccpos[0] | eccpos[0] || eccpos[1] | eccpos[0] | | 3 | eccpos[1] | eccpos[1] || eccpos[2] | eccpos[1] | | 4 | eccpos[2] | eccpos[2] || eccpos[3] | eccpos[2] | | 5 | eccpos[3] | eccpos[3] || eccpos[4] | eccpos[3] | | 6 | eccpos[4] | eccpos[4] || eccpos[5] | eccpos[4] | | 7 | eccpos[5] | eccpos[5] || eccpos[6] | eccpos[5] | | 8 | eccpos[6] | eccpos[6] || eccpos[7] | eccpos[6] | | 9 | eccpos[7] | eccpos[7] || eccpos[8] | eccpos[7] | |10 | eccpos[8] | eccpos[8] || eccpos[9] | eccpos[8] | |11 | eccpos[9] | eccpos[9] || eccpos[10] | eccpos[9] | |12 | eccpos[10] | eccpos[10] || eccpos[11] | eccpos[10] | |13 | eccpos[11] | eccpos[11] || oobfree[0] | eccpos[11] | |14 | eccpos[12] | eccpos[12] || oobfree[1] | oobfree[0] | |15 | eccpos[13] | <reserved> || oobfree[2] | oobfree[1] | +---+------------+-------------++-------------+-------------+ |16 | eccpos[14] | eccpos[13] || oobfree[3] | oobfree[2] | |...| [...] | [...] || [...] | [...] | |56 | eccpos[54] | eccpos[51] || oobfree[43] | oobfree[42] | |57 | eccpos[55] | <reserved> || oobfree[44] | oobfree[43] | +===+============+=============+==============+=============+ |58 | oobfree[0] | oobfree[0] || oobfree[45] | oobfree[44] | |59 | oobfree[1] | oobfree[1] || oobfree[46] | oobfree[45] | |60 | oobfree[2] | oobfree[2] || oobfree[47] | oobfree[46] | |61 | oobfree[3] | oobfree[3] || oobfree[48] | oobfree[47] | |62 | oobfree[4] | oobfree[4] || oobfree[49] | oobfree[48] | |63 | oobfree[5] | oobfree[5] || oobfree[50] | oobfree[49] | +---+------------+-------------+--------------+-------------+
[1] ecc-layout expected by ROM code, as specified in SoC TRM under: Chapter="Initialization" Section="Device Initialization by ROM code" Sub-Section="Memory Booting" Heading="NAND" Figure="ECC Locations in NAND Spare Areas"
[2] ecc-layout updates in u-boot http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
[3] u-boot configurations to match above ecc-layout are documented at https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide
CC: stable@vger.kernel.org # 3.13.x+ Reported-by: Enric Balletbo Serra eballetbo@iseebcn.com Tested-by: Enric Balletbo i Serra eballetbo@gmail.com Tested-by: Stefan Roese sr@denx.de Signed-off-by: Pekon Gupta pekon@ti.com
Seems to me that the commit message above could actually be placed in a doc/README.* file, making the commit message itself less bulky.
drivers/mtd/nand/omap2.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)
Amicalement,
On Mon, Feb 17, 2014 at 10:11:52AM +0100, Albert ARIBAUD wrote:
On Mon, 17 Feb 2014 13:11:23 +0530, Pekon Gupta pekon@ti.com wrote:
Fixes: commit a919e51161b58ed7e6e663daba99ab7d558808f3 mtd: nand: omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe
Fixes ecclayout mismatch introduced in above commit for following ecc-schemes:
- OMAP_ECC_BCH4_CODE_HW_DETECTION_SW
- OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
However, this patch also touches other ecc-schemes as the fix required refactoring common code, into ecc-scheme specific code.
This patch aligns ecc-layout for below ecc-schemes as per reference [1],[2],[3] Figure="ECC Locations in NAND Spare Areas"
[snip]
Seems to me that the commit message above could actually be placed in a doc/README.* file, making the commit message itself less bulky.
Yes, I have suggested Pekon add this as a proper documentation file. But I will take the patches as-is anyway.
Brian
1) In current implementation, ecclayout->oobfree->offset is calculated with respect to ecclayout->eccpos[0] which is incorrect because ECC bytes may not be stored contiguously in OOB. So, this patch calculates ecclayout->oobfree->offset with respect to last ECC byte-position 'eccpos[ecclayout->eccbytes-1]'.
2) ECC layout of some ecc-schemes expects reserved-markers at specific eccpos[] which should not be over-written by any file-system metadata. So this patch aligns oobfree->offset taking into account of such markers.
CC: stable@vger.kernel.org # 3.13.x+ Tested-by: Enric Balletbo i Serra eballetbo@gmail.com Tested-by: Stefan Roese sr@denx.de Signed-off-by: Pekon Gupta pekon@ti.com --- drivers/mtd/nand/omap2.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index 34ef941..58685ab 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1832,8 +1832,9 @@ static int omap_nand_probe(struct platform_device *pdev) oob_index = 1; for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) ecclayout->eccpos[i] = oob_index; - ecclayout->oobfree->offset = ecclayout->eccpos[0] + - ecclayout->eccbytes; + /* no reserved-marker in ecclayout for this ecc-scheme */ + ecclayout->oobfree->offset = + ecclayout->eccpos[ecclayout->eccbytes - 1] + 1; break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW: @@ -1856,8 +1857,9 @@ static int omap_nand_probe(struct platform_device *pdev) if (((i + 1) % nand_chip->ecc.bytes) == 0) oob_index++; } - ecclayout->oobfree->offset = ecclayout->eccpos[0] + - ecclayout->eccbytes; + /* include reserved-marker in ecclayout->oobfree calculation */ + ecclayout->oobfree->offset = 1 + + ecclayout->eccpos[ecclayout->eccbytes - 1] + 1; /* software bch library is used for locating errors */ nand_chip->ecc.priv = nand_bch_init(mtd, nand_chip->ecc.size, @@ -1894,8 +1896,9 @@ static int omap_nand_probe(struct platform_device *pdev) oob_index = BADBLOCK_MARKER_LENGTH; for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) ecclayout->eccpos[i] = oob_index; - ecclayout->oobfree->offset = ecclayout->eccpos[0] + - ecclayout->eccbytes; + /* reserved marker already included in ecclayout->eccbytes */ + ecclayout->oobfree->offset = + ecclayout->eccpos[ecclayout->eccbytes - 1] + 1; /* This ECC scheme requires ELM H/W block */ if (is_elm_present(info, pdata->elm_of_node, BCH4_ECC) < 0) { pr_err("nand: error: could not initialize ELM\n"); @@ -1929,8 +1932,9 @@ static int omap_nand_probe(struct platform_device *pdev) if (((i + 1) % nand_chip->ecc.bytes) == 0) oob_index++; } - ecclayout->oobfree->offset = ecclayout->eccpos[0] + - ecclayout->eccbytes; + /* include reserved-marker in ecclayout->oobfree calculation */ + ecclayout->oobfree->offset = 1 + + ecclayout->eccpos[ecclayout->eccbytes - 1] + 1; /* software bch library is used for locating errors */ nand_chip->ecc.priv = nand_bch_init(mtd, nand_chip->ecc.size, @@ -1974,8 +1978,9 @@ static int omap_nand_probe(struct platform_device *pdev) oob_index = BADBLOCK_MARKER_LENGTH; for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) ecclayout->eccpos[i] = oob_index; - ecclayout->oobfree->offset = ecclayout->eccpos[0] + - ecclayout->eccbytes; + /* reserved marker already included in ecclayout->eccbytes */ + ecclayout->oobfree->offset = + ecclayout->eccpos[ecclayout->eccbytes - 1] + 1; break; #else pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
removing stable@vger.kernel.org from CC list. (git send-email --suppress-cc=cc din't work for me)
From: Gupta, Pekon
- In current implementation, ecclayout->oobfree->offset is calculated with
respect to ecclayout->eccpos[0] which is incorrect because ECC bytes may not be stored contiguously in OOB. So, this patch calculates ecclayout->oobfree->offset with respect to last ECC byte-position 'eccpos[ecclayout->eccbytes-1]'.
- ECC layout of some ecc-schemes expects reserved-markers at specific eccpos[]
which should not be over-written by any file-system metadata. So this patch aligns oobfree->offset taking into account of such markers.
CC: stable@vger.kernel.org # 3.13.x+ Tested-by: Enric Balletbo i Serra eballetbo@gmail.com Tested-by: Stefan Roese sr@denx.de Signed-off-by: Pekon Gupta pekon@ti.com
drivers/mtd/nand/omap2.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index 34ef941..58685ab 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1832,8 +1832,9 @@ static int omap_nand_probe(struct platform_device *pdev) oob_index = 1; for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) ecclayout->eccpos[i] = oob_index;
ecclayout->oobfree->offset = ecclayout->eccpos[0] +ecclayout->eccbytes;
/* no reserved-marker in ecclayout for this ecc-scheme */ecclayout->oobfree->offset =ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1856,8 +1857,9 @@ static int omap_nand_probe(struct platform_device *pdev) if (((i + 1) % nand_chip->ecc.bytes) == 0) oob_index++; }
ecclayout->oobfree->offset = ecclayout->eccpos[0] +ecclayout->eccbytes;
/* include reserved-marker in ecclayout->oobfree calculation */ecclayout->oobfree->offset = 1 +ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;@@ -1894,8 +1896,9 @@ static int omap_nand_probe(struct platform_device *pdev) oob_index = BADBLOCK_MARKER_LENGTH; for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) ecclayout->eccpos[i] = oob_index;
ecclayout->oobfree->offset = ecclayout->eccpos[0] +ecclayout->eccbytes;
/* reserved marker already included in ecclayout->eccbytes */ecclayout->oobfree->offset =ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;@@ -1929,8 +1932,9 @@ static int omap_nand_probe(struct platform_device *pdev) if (((i + 1) % nand_chip->ecc.bytes) == 0) oob_index++; }
ecclayout->oobfree->offset = ecclayout->eccpos[0] +ecclayout->eccbytes;
/* include reserved-marker in ecclayout->oobfree calculation */ecclayout->oobfree->offset = 1 +ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;@@ -1974,8 +1978,9 @@ static int omap_nand_probe(struct platform_device *pdev) oob_index = BADBLOCK_MARKER_LENGTH; for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) ecclayout->eccpos[i] = oob_index;
ecclayout->oobfree->offset = ecclayout->eccpos[0] +ecclayout->eccbytes;
/* reserved marker already included in ecclayout->eccbytes */ecclayout->oobfree->offset =ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;#else pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n"); -- 1.8.5.1.163.gd7aced9
With regards, pekon
This patch excludes reserved-marker byte-position from oobfree->length calculation. Thus all bytes from oobfree->offset till end of OOB are free.
CC: stable@vger.kernel.org # 3.13.x+ Signed-off-by: Pekon Gupta pekon@ti.com --- drivers/mtd/nand/omap2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index 58685ab..bf642ce 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1994,9 +1994,8 @@ static int omap_nand_probe(struct platform_device *pdev) goto return_error; }
- /* populate remaining ECC layout data */ - ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH + - ecclayout->eccbytes); + /* all OOB bytes from oobfree->offset till end off OOB are free */ + ecclayout->oobfree->length = mtd->oobsize - ecclayout->oobfree->offset; /* check if NAND device's OOB is enough to store ECC signatures */ if (mtd->oobsize < (ecclayout->eccbytes + BADBLOCK_MARKER_LENGTH)) { pr_err("not enough OOB bytes required = %d, available=%d\n",
removing stable@vger.kernel.org from CC list. (git send-email --suppress-cc=cc din't work, may be it should have been --supress-cc=bodycc)
From: Gupta, Pekon
This patch excludes reserved-marker byte-position from oobfree->length calculation. Thus all bytes from oobfree->offset till end of OOB are free.
CC: stable@vger.kernel.org # 3.13.x+ Signed-off-by: Pekon Gupta pekon@ti.com
drivers/mtd/nand/omap2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index 58685ab..bf642ce 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1994,9 +1994,8 @@ static int omap_nand_probe(struct platform_device *pdev) goto return_error; }
- /* populate remaining ECC layout data */
- ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
ecclayout->eccbytes);
- /* all OOB bytes from oobfree->offset till end off OOB are free */
- ecclayout->oobfree->length = mtd->oobsize - ecclayout->oobfree->offset; /* check if NAND device's OOB is enough to store ECC signatures */ if (mtd->oobsize < (ecclayout->eccbytes + BADBLOCK_MARKER_LENGTH)) { pr_err("not enough OOB bytes required = %d, available=%d\n",
-- 1.8.5.1.163.gd7aced9
with regards, pekon
Hi Pekon,
On Mon, Feb 17, 2014 at 01:11:22PM +0530, Pekon Gupta wrote:
*changes v2 -> v3*
- re-ordered the patch-set so that eccpos[] is fixed before 'oobfree'
- marked for stable 3.13.x+
*changes v1 -> v2* [PATCH 1/3] fix oobfree->offset calculation + adjust for reserved-marker of last sector [PATCH 2/3] <new patch> [PATCH 3/3] refactor code as suggested by Brian Norris computersforpeace@gmail.com
*original v1* [PATCH 1/2] http://lists.infradead.org/pipermail/linux-mtd/2013-December/050946.html [PATCH 2/2] http://lists.infradead.org/pipermail/linux-mtd/2013-December/050945.html
This patch-set is tested on AM335x-EVM using following end-to-end boot sequence with appropriate u-boot configs [1]
(BCH8_HW) (HAM1_HW) (HAM1_HW) (HAM1_HW) (UBIFS)ROM ---------> SPL ---------> U-Boot ---------> Kernel ---------> File-System
(BCH8_HW) (BCH8_SW) (BCH8_SW) (BCH8_SW) (UBIFS)ROM ---------> SPL ---------> U-Boot ---------> Kernel ---------> File-System
(BCH8_HW) (BCH8_HW) (BCH8_HW) (BCH8_HW) (UBIFS)ROM ---------> SPL ---------> U-Boot ---------> Kernel ---------> File-System
Test1: flash ubi image from u-boot and boot the kernel u-boot> mw 0x82000000 0xff <u-boot.img size aligned to NAND block boundary> u-boot> fatload mmc 0 0x82000000 u-boot.img u-boot> nand erase <u-boot_offset> <u-boot.img size> u-boot> nand write 0x82000000 <u-boot_offset> <u-boot.img size> u-boot> setenv bootargs 'console=ttyO0,115200n8 noinitrd mem=256M \ root=ubi0 rw rootfstype=ubifs ubi.mtd=<mtdpart-of-rootfs>,\ <page-size> ip=off init=/init' u-boot> bootm <kernel_offset>
Test2: update u-boot.img from kernel and re-boot kernel> flash_erase /dev/<mtdpart-of-u-boot> 0 0 kernel> nandwrite -s 0 /dev/<mtdpart-of-u-boot> u-boot.img kernel> reboot
[1] u-boot configurations to match above ecc-layout are documented at https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide
Pushed to linux-mtd.git. I plan to send this to Linus in the 3.14-rc. Thanks!
If you get the time, can you submit some documentation for Documentation/mtd/nand/?
Brian
participants (4)
-
Albert ARIBAUD -
Brian Norris -
Gupta, Pekon -
Pekon Gupta