* [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2013-12-13 9:12 ` Pekon Gupta
0 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: Brian Norris, Artem Bityutskiy, Scott Wood, Tom Rini
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
avinashphilipk, jp.francois, Pekon Gupta, r.meier, balbi, u-boot,
Enric Balletbo Serra, linux-mtd, Igor Grinberg, Ezequiel Garcia,
Javier Martinez Canillas, linux-omap, ivan.djelic,
Nikita Kiryanov
As there were parallel set of patches running between u-boot and kernel.
hence, some patch-sets caused regression for OMAP3x platforms when booting
using u-boot specifically for ecc-schemes (like BCH4_SW).
Hence this patch series fixes those regressions, and tests complete
NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
(following configurations are required for building u-boot driver which is
compatible to kernel ecclayout for selected ecc-scheme)
(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
*Configurations used to build u-boot and kernel for end-to-end NAND boot*
+------------+--------------------------------------------+------------------+
| ecc-scheme | u-boot/SPL configs | kernel DTS |
+------------+--------------------------------------------+------------------+
| | | |
| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
| | 10, 11, 12 } | |
| | (for NAND page-size=2048) | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
| using s/w | #define CONFIG_BCH | |
|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
| | 11, 12, 13, 14, \ | |
| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
| | 25, 26, 27, 28, \ | |
| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
| | 39, 40, 41, 42, \ | |
| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
| | 53, 54, 55, 56, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
#* In addition following patches need to be pulled for u-boot:
http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
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
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
Pekon Gupta (2):
mtd: nand: omap: fix ecclayout->oobfree->offset
mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
drivers/mtd/nand/omap2.c | 50 ++++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 21 deletions(-)
--
1.8.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2013-12-13 9:12 ` Pekon Gupta
0 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: Brian Norris, Artem Bityutskiy, Scott Wood, Tom Rini
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
avinashphilipk, jp.francois, Pekon Gupta, r.meier, balbi, u-boot,
Enric Balletbo Serra, linux-mtd, Igor Grinberg, Ezequiel Garcia,
Javier Martinez Canillas, linux-omap, ivan.djelic,
Nikita Kiryanov
As there were parallel set of patches running between u-boot and kernel.
hence, some patch-sets caused regression for OMAP3x platforms when booting
using u-boot specifically for ecc-schemes (like BCH4_SW).
Hence this patch series fixes those regressions, and tests complete
NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
(following configurations are required for building u-boot driver which is
compatible to kernel ecclayout for selected ecc-scheme)
(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
*Configurations used to build u-boot and kernel for end-to-end NAND boot*
+------------+--------------------------------------------+------------------+
| ecc-scheme | u-boot/SPL configs | kernel DTS |
+------------+--------------------------------------------+------------------+
| | | |
| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
| | 10, 11, 12 } | |
| | (for NAND page-size=2048) | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
| using s/w | #define CONFIG_BCH | |
|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
| | 11, 12, 13, 14, \ | |
| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
| | 25, 26, 27, 28, \ | |
| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
| | 39, 40, 41, 42, \ | |
| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
| | 53, 54, 55, 56, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
#* In addition following patches need to be pulled for u-boot:
http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
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
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
Pekon Gupta (2):
mtd: nand: omap: fix ecclayout->oobfree->offset
mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
drivers/mtd/nand/omap2.c | 50 ++++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 21 deletions(-)
--
1.8.1
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2013-12-13 9:12 ` Pekon Gupta
0 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: u-boot
As there were parallel set of patches running between u-boot and kernel.
hence, some patch-sets caused regression for OMAP3x platforms when booting
using u-boot specifically for ecc-schemes (like BCH4_SW).
Hence this patch series fixes those regressions, and tests complete
NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
(following configurations are required for building u-boot driver which is
compatible to kernel ecclayout for selected ecc-scheme)
(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
*Configurations used to build u-boot and kernel for end-to-end NAND boot*
+------------+--------------------------------------------+------------------+
| ecc-scheme | u-boot/SPL configs | kernel DTS |
+------------+--------------------------------------------+------------------+
| | | |
| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
| | 10, 11, 12 } | |
| | (for NAND page-size=2048) | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
| using s/w | #define CONFIG_BCH | |
|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
| | 11, 12, 13, 14, \ | |
| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
| | 25, 26, 27, 28, \ | |
| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
| | 39, 40, 41, 42, \ | |
| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
| | 53, 54, 55, 56, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
#* In addition following patches need to be pulled for u-boot:
http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
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
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
Pekon Gupta (2):
mtd: nand: omap: fix ecclayout->oobfree->offset
mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
drivers/mtd/nand/omap2.c | 50 ++++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 21 deletions(-)
--
1.8.1
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
2013-12-13 9:12 ` Pekon Gupta
(?)
@ 2013-12-13 9:12 ` Pekon Gupta
-1 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: Brian Norris, Artem Bityutskiy, Scott Wood, Tom Rini
Cc: u-boot, linux-mtd, linux-omap, Enric Balletbo Serra,
Stefan Roese, Javier Martinez Canillas, Thomas Petazzoni,
Igor Grinberg, Michael Trimarchi, r.meier, r-woodruff2,
Nikita Kiryanov, jp.francois, ivan.djelic, Ezequiel Garcia,
avinashphilipk, balbi, Pekon Gupta
This patch updates starting offset for free bytes in OOB which can be used by
file-systems to store their metadata (like clean-marker in case of JFFS2).
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f777250..bbdb5e8 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1835,8 +1835,6 @@ static int omap_nand_probe(struct platform_device *pdev)
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
else
ecclayout->eccpos[0] = 1;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1854,8 +1852,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1890,8 +1886,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* 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");
@@ -1920,8 +1914,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1963,8 +1955,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
break;
#else
pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
@@ -1978,9 +1968,6 @@ 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);
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 */
@@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
err = -EINVAL;
goto return_error;
}
+ /* populate remaining ECC layout data */
+ ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
+ ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
+ ecclayout->eccbytes);
/* second phase scan */
if (nand_scan_tail(mtd)) {
--
1.8.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
@ 2013-12-13 9:12 ` Pekon Gupta
0 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: Brian Norris, Artem Bityutskiy, Scott Wood, Tom Rini
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
avinashphilipk, jp.francois, Pekon Gupta, r.meier, balbi, u-boot,
Enric Balletbo Serra, linux-mtd, Igor Grinberg, Ezequiel Garcia,
Javier Martinez Canillas, linux-omap, ivan.djelic,
Nikita Kiryanov
This patch updates starting offset for free bytes in OOB which can be used by
file-systems to store their metadata (like clean-marker in case of JFFS2).
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f777250..bbdb5e8 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1835,8 +1835,6 @@ static int omap_nand_probe(struct platform_device *pdev)
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
else
ecclayout->eccpos[0] = 1;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1854,8 +1852,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1890,8 +1886,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* 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");
@@ -1920,8 +1914,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1963,8 +1955,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
break;
#else
pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
@@ -1978,9 +1968,6 @@ 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);
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 */
@@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
err = -EINVAL;
goto return_error;
}
+ /* populate remaining ECC layout data */
+ ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
+ ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
+ ecclayout->eccbytes);
/* second phase scan */
if (nand_scan_tail(mtd)) {
--
1.8.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
@ 2013-12-13 9:12 ` Pekon Gupta
0 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: u-boot
This patch updates starting offset for free bytes in OOB which can be used by
file-systems to store their metadata (like clean-marker in case of JFFS2).
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f777250..bbdb5e8 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1835,8 +1835,6 @@ static int omap_nand_probe(struct platform_device *pdev)
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
else
ecclayout->eccpos[0] = 1;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1854,8 +1852,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1890,8 +1886,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* 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");
@@ -1920,8 +1914,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1963,8 +1955,6 @@ static int omap_nand_probe(struct platform_device *pdev)
(mtd->writesize /
nand_chip->ecc.size);
ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
- ecclayout->oobfree->offset = ecclayout->eccpos[0] +
- ecclayout->eccbytes;
break;
#else
pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
@@ -1978,9 +1968,6 @@ 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);
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 */
@@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
err = -EINVAL;
goto return_error;
}
+ /* populate remaining ECC layout data */
+ ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
+ ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
+ ecclayout->eccbytes);
/* second phase scan */
if (nand_scan_tail(mtd)) {
--
1.8.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v1 2/2] mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
2013-12-13 9:12 ` Pekon Gupta
(?)
@ 2013-12-13 9:12 ` Pekon Gupta
-1 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: Brian Norris, Artem Bityutskiy, Scott Wood, Tom Rini
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
avinashphilipk, jp.francois, Pekon Gupta, r.meier, balbi, u-boot,
Enric Balletbo Serra, linux-mtd, Igor Grinberg, Ezequiel Garcia,
Javier Martinez Canillas, linux-omap, ivan.djelic,
Nikita Kiryanov
This patch mainly fixes ecc-layout for following ecc-schemes, to bring them
in sync with u-boot omap_gpmc NAND driver:
- BCH4_SW: OMAP_ECC_BCH4_CODE_HW_DETECTION_SW
This ecc-scheme is mainly used on AM35xx and other legacy platforms.
- BCH8_SW: OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
This ecc-scheme is mainly used on OMAP3x and other legacy platforms.
Apart from above, this patch also touches other ecc-schemes as the fix
required refactoring common code, into ecc-scheme specific code.
Hence, end-to-end NAND boot sequence was tested on AM335x-EVM to avoid
any further regression on legacy or current platforms.
(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
*Configurations used to build u-boot and kernel for end-to-end NAND boot*
+------------+--------------------------------------------+------------------+
| ecc-scheme | u-boot/SPL configs | kernel DTS |
+------------+--------------------------------------------+------------------+
| | | |
| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
| | 10, 11, 12 } | |
| | (for NAND page-size=2048) | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
| using s/w | #define CONFIG_BCH | |
|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
| | 11, 12, 13, 14, \ | |
| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
| | 25, 26, 27, 28, \ | |
| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
| | 39, 40, 41, 42, \ | |
| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
| | 53, 54, 55, 56, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
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
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index bbdb5e8..e7836bf 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 = {};
@@ -1832,9 +1833,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;
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1851,7 +1854,13 @@ 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++) {
+ if ((i % nand_chip->ecc.bytes) || (i == 0))
+ ecclayout->eccpos[i] = oob_index;
+ else
+ ecclayout->eccpos[i] = ++oob_index;
+ }
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1885,7 +1894,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;
/* 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");
@@ -1913,7 +1924,13 @@ 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++) {
+ if ((i % nand_chip->ecc.bytes) || (i == 0))
+ ecclayout->eccpos[i] = oob_index;
+ else
+ ecclayout->eccpos[i] = ++oob_index;
+ }
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1954,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;
break;
#else
pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
@@ -1968,8 +1987,6 @@ static int omap_nand_probe(struct platform_device *pdev)
goto return_error;
}
- 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",
--
1.8.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH v1 2/2] mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
@ 2013-12-13 9:12 ` Pekon Gupta
0 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: Brian Norris, Artem Bityutskiy, Scott Wood, Tom Rini
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
avinashphilipk, jp.francois, Pekon Gupta, r.meier, balbi, u-boot,
Enric Balletbo Serra, linux-mtd, Igor Grinberg, Ezequiel Garcia,
Javier Martinez Canillas, linux-omap, ivan.djelic,
Nikita Kiryanov
This patch mainly fixes ecc-layout for following ecc-schemes, to bring them
in sync with u-boot omap_gpmc NAND driver:
- BCH4_SW: OMAP_ECC_BCH4_CODE_HW_DETECTION_SW
This ecc-scheme is mainly used on AM35xx and other legacy platforms.
- BCH8_SW: OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
This ecc-scheme is mainly used on OMAP3x and other legacy platforms.
Apart from above, this patch also touches other ecc-schemes as the fix
required refactoring common code, into ecc-scheme specific code.
Hence, end-to-end NAND boot sequence was tested on AM335x-EVM to avoid
any further regression on legacy or current platforms.
(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
*Configurations used to build u-boot and kernel for end-to-end NAND boot*
+------------+--------------------------------------------+------------------+
| ecc-scheme | u-boot/SPL configs | kernel DTS |
+------------+--------------------------------------------+------------------+
| | | |
| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
| | 10, 11, 12 } | |
| | (for NAND page-size=2048) | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
| using s/w | #define CONFIG_BCH | |
|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
| | 11, 12, 13, 14, \ | |
| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
| | 25, 26, 27, 28, \ | |
| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
| | 39, 40, 41, 42, \ | |
| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
| | 53, 54, 55, 56, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
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
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index bbdb5e8..e7836bf 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 = {};
@@ -1832,9 +1833,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;
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1851,7 +1854,13 @@ 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++) {
+ if ((i % nand_chip->ecc.bytes) || (i == 0))
+ ecclayout->eccpos[i] = oob_index;
+ else
+ ecclayout->eccpos[i] = ++oob_index;
+ }
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1885,7 +1894,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;
/* 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");
@@ -1913,7 +1924,13 @@ 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++) {
+ if ((i % nand_chip->ecc.bytes) || (i == 0))
+ ecclayout->eccpos[i] = oob_index;
+ else
+ ecclayout->eccpos[i] = ++oob_index;
+ }
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1954,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;
break;
#else
pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
@@ -1968,8 +1987,6 @@ static int omap_nand_probe(struct platform_device *pdev)
goto return_error;
}
- 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",
--
1.8.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 2/2] mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
@ 2013-12-13 9:12 ` Pekon Gupta
0 siblings, 0 replies; 45+ messages in thread
From: Pekon Gupta @ 2013-12-13 9:12 UTC (permalink / raw)
To: u-boot
This patch mainly fixes ecc-layout for following ecc-schemes, to bring them
in sync with u-boot omap_gpmc NAND driver:
- BCH4_SW: OMAP_ECC_BCH4_CODE_HW_DETECTION_SW
This ecc-scheme is mainly used on AM35xx and other legacy platforms.
- BCH8_SW: OMAP_ECC_BCH8_CODE_HW_DETECTION_SW
This ecc-scheme is mainly used on OMAP3x and other legacy platforms.
Apart from above, this patch also touches other ecc-schemes as the fix
required refactoring common code, into ecc-scheme specific code.
Hence, end-to-end NAND boot sequence was tested on AM335x-EVM to avoid
any further regression on legacy or current platforms.
(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
*Configurations used to build u-boot and kernel for end-to-end NAND boot*
+------------+--------------------------------------------+------------------+
| ecc-scheme | u-boot/SPL configs | kernel DTS |
+------------+--------------------------------------------+------------------+
| | | |
| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
| | 10, 11, 12 } | |
| | (for NAND page-size=2048) | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
| using s/w | #define CONFIG_BCH | |
|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
| | 11, 12, 13, 14, \ | |
| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
| | 25, 26, 27, 28, \ | |
| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
| | 39, 40, 41, 42, \ | |
| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
| | 53, 54, 55, 56, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
| | | |
| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
| | (for NAND page-size=2048) | |
| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
| | | |
+------------+--------------------------------------------+------------------+
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
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index bbdb5e8..e7836bf 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 = {};
@@ -1832,9 +1833,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;
break;
case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
@@ -1851,7 +1854,13 @@ 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++) {
+ if ((i % nand_chip->ecc.bytes) || (i == 0))
+ ecclayout->eccpos[i] = oob_index;
+ else
+ ecclayout->eccpos[i] = ++oob_index;
+ }
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1885,7 +1894,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;
/* 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");
@@ -1913,7 +1924,13 @@ 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++) {
+ if ((i % nand_chip->ecc.bytes) || (i == 0))
+ ecclayout->eccpos[i] = oob_index;
+ else
+ ecclayout->eccpos[i] = ++oob_index;
+ }
/* software bch library is used for locating errors */
nand_chip->ecc.priv = nand_bch_init(mtd,
nand_chip->ecc.size,
@@ -1954,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;
break;
#else
pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
@@ -1968,8 +1987,6 @@ static int omap_nand_probe(struct platform_device *pdev)
goto return_error;
}
- 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",
--
1.8.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* RE: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2013-12-13 9:12 ` Pekon Gupta
(?)
@ 2014-01-06 7:40 ` Gupta, Pekon
-1 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-06 7:40 UTC (permalink / raw)
To: Enric Balletbo Serra, Stefan Roese, Javier Martinez Canillas,
Thomas Petazzoni, Igor Grinberg, Michael Trimarchi,
Nikita Kiryanov, jp.francois, ivan.djelic
Cc: Woodruff, Richard, Artem Bityutskiy, Brian Norris, Rini, Tom,
avinashphilipk, r.meier, u-boot, linux-mtd, Balbi, Felipe,
Ezequiel Garcia, Scott Wood, linux-omap
Hello Enric, Nikita, and other OMAP3 users,
>
>As there were parallel set of patches running between u-boot and kernel.
>hence, some patch-sets caused regression for OMAP3x platforms when booting
>using u-boot specifically for ecc-schemes (like BCH4_SW).
>
>Hence this patch series fixes those regressions, and tests complete
>NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>(following configurations are required for building u-boot driver which is
> compatible to kernel ecclayout for selected ecc-scheme)
>
>
> (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
>
>*Configurations used to build u-boot and kernel for end-to-end NAND boot*
>+------------+--------------------------------------------+------------------+
>| ecc-scheme | u-boot/SPL configs | kernel DTS |
>+------------+--------------------------------------------+------------------+
>| | | |
>| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>| | 10, 11, 12 } | |
>| | (for NAND page-size=2048) | |
>| | | |
>+------------+--------------------------------------------+------------------+
>| | | |
>| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>| using s/w | #define CONFIG_BCH | |
>|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>| | 11, 12, 13, 14, \ | |
>| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>| | 25, 26, 27, 28, \ | |
>| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>| | 39, 40, 41, 42, \ | |
>| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>| | 53, 54, 55, 56, } | |
>| | (for NAND page-size=2048) | |
>| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>| | | |
>+------------+--------------------------------------------+------------------+
>| | | |
>| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>| | (for NAND page-size=2048) | |
>| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>| | | |
>+------------+--------------------------------------------+------------------+
>
>#* In addition following patches need to be pulled for u-boot:
> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>
>
>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
>
>Signed-off-by: Pekon Gupta <pekon@ti.com>
>---
Though I have done initial level of testing on AM335x as mentioned above,
But will it be possible for you to test and confirm if these set of patches
solve regressions on your OMAP3 boards ?
with regards, pekon
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-06 7:40 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-06 7:40 UTC (permalink / raw)
To: Enric Balletbo Serra, Stefan Roese, Javier Martinez Canillas,
Thomas Petazzoni, Igor Grinberg, Michael Trimarchi,
Nikita Kiryanov, jp.francois, ivan.djelic
Cc: Woodruff, Richard, Artem Bityutskiy, Brian Norris, Rini, Tom,
avinashphilipk, r.meier, u-boot, linux-mtd, Balbi, Felipe,
Ezequiel Garcia, Scott Wood, linux-omap
Hello Enric, Nikita, and other OMAP3 users,
>
>As there were parallel set of patches running between u-boot and kernel.
>hence, some patch-sets caused regression for OMAP3x platforms when booting
>using u-boot specifically for ecc-schemes (like BCH4_SW).
>
>Hence this patch series fixes those regressions, and tests complete
>NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>(following configurations are required for building u-boot driver which is
> compatible to kernel ecclayout for selected ecc-scheme)
>
>
> (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
>
>*Configurations used to build u-boot and kernel for end-to-end NAND boot*
>+------------+--------------------------------------------+------------------+
>| ecc-scheme | u-boot/SPL configs | kernel DTS |
>+------------+--------------------------------------------+------------------+
>| | | |
>| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>| | 10, 11, 12 } | |
>| | (for NAND page-size=2048) | |
>| | | |
>+------------+--------------------------------------------+------------------+
>| | | |
>| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>| using s/w | #define CONFIG_BCH | |
>|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>| | 11, 12, 13, 14, \ | |
>| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>| | 25, 26, 27, 28, \ | |
>| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>| | 39, 40, 41, 42, \ | |
>| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>| | 53, 54, 55, 56, } | |
>| | (for NAND page-size=2048) | |
>| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>| | | |
>+------------+--------------------------------------------+------------------+
>| | | |
>| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>| | (for NAND page-size=2048) | |
>| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>| | | |
>+------------+--------------------------------------------+------------------+
>
>#* In addition following patches need to be pulled for u-boot:
> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>
>
>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
>
>Signed-off-by: Pekon Gupta <pekon@ti.com>
>---
Though I have done initial level of testing on AM335x as mentioned above,
But will it be possible for you to test and confirm if these set of patches
solve regressions on your OMAP3 boards ?
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-06 7:40 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-06 7:40 UTC (permalink / raw)
To: u-boot
Hello Enric, Nikita, and other OMAP3 users,
>
>As there were parallel set of patches running between u-boot and kernel.
>hence, some patch-sets caused regression for OMAP3x platforms when booting
>using u-boot specifically for ecc-schemes (like BCH4_SW).
>
>Hence this patch series fixes those regressions, and tests complete
>NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>(following configurations are required for building u-boot driver which is
> compatible to kernel ecclayout for selected ecc-scheme)
>
>
> (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
>
>*Configurations used to build u-boot and kernel for end-to-end NAND boot*
>+------------+--------------------------------------------+------------------+
>| ecc-scheme | u-boot/SPL configs | kernel DTS |
>+------------+--------------------------------------------+------------------+
>| | | |
>| HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>| (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>| Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>| using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>| | 10, 11, 12 } | |
>| | (for NAND page-size=2048) | |
>| | | |
>+------------+--------------------------------------------+------------------+
>| | | |
>| BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>| using s/w | #define CONFIG_BCH | |
>|library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>|for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>| error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>|correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>| | 11, 12, 13, 14, \ | |
>| | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>| | 25, 26, 27, 28, \ | |
>| | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>| | 39, 40, 41, 42, \ | |
>| | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>| | 53, 54, 55, 56, } | |
>| | (for NAND page-size=2048) | |
>| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>| | | |
>+------------+--------------------------------------------+------------------+
>| | | |
>| BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>| | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>|(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>| using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>| h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>|for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>| error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>|correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>| | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>| | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>| | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>| | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>| | (for NAND page-size=2048) | |
>| | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>| | | |
>+------------+--------------------------------------------+------------------+
>
>#* In addition following patches need to be pulled for u-boot:
> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>
>
>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
>
>Signed-off-by: Pekon Gupta <pekon@ti.com>
>---
Though I have done initial level of testing on AM335x as mentioned above,
But will it be possible for you to test and confirm if these set of patches
solve regressions on your OMAP3 boards ?
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-06 7:40 ` Gupta, Pekon
(?)
@ 2014-01-06 11:48 ` Stefan Roese
-1 siblings, 0 replies; 45+ messages in thread
From: Stefan Roese @ 2014-01-06 11:48 UTC (permalink / raw)
To: Gupta, Pekon, Enric Balletbo Serra, Javier Martinez Canillas,
Thomas Petazzoni, Igor Grinberg, Michael Trimarchi,
Nikita Kiryanov, jp.francois, ivan.djelic
Cc: Woodruff, Richard, Artem Bityutskiy, Brian Norris, Rini, Tom,
avinashphilipk, r.meier, u-boot, linux-mtd, Balbi, Felipe,
Ezequiel Garcia, Scott Wood, linux-omap
Hi Pekon,
On 06.01.2014 08:40, Gupta, Pekon wrote:
> Hello Enric, Nikita, and other OMAP3 users,
>
>>
>> As there were parallel set of patches running between u-boot and kernel.
>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>
>> Hence this patch series fixes those regressions, and tests complete
>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>> (following configurations are required for building u-boot driver which is
>> compatible to kernel ecclayout for selected ecc-scheme)
>>
>>
>> (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
>>
>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>> +------------+--------------------------------------------+------------------+
>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>> | | 10, 11, 12 } | |
>> | | (for NAND page-size=2048) | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>> | using s/w | #define CONFIG_BCH | |
>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>> | | 11, 12, 13, 14, \ | |
>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>> | | 25, 26, 27, 28, \ | |
>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>> | | 39, 40, 41, 42, \ | |
>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>> | | 53, 54, 55, 56, } | |
>> | | (for NAND page-size=2048) | |
>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>> | | (for NAND page-size=2048) | |
>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>>
>> #* In addition following patches need to be pulled for u-boot:
>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>
>>
>> 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
>>
>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>> ---
>
> Though I have done initial level of testing on AM335x as mentioned above,
> But will it be possible for you to test and confirm if these set of patches
> solve regressions on your OMAP3 boards ?
Those patches work fine on our custom AM335x board. So:
Tested-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-06 11:48 ` Stefan Roese
0 siblings, 0 replies; 45+ messages in thread
From: Stefan Roese @ 2014-01-06 11:48 UTC (permalink / raw)
To: Gupta, Pekon, Enric Balletbo Serra, Javier Martinez Canillas,
Thomas Petazzoni, Igor Grinberg, Michael Trimarchi,
Nikita Kiryanov, jp.francois, ivan.djelic
Cc: Woodruff, Richard, Artem Bityutskiy, Brian Norris, Rini, Tom,
avinashphilipk, r.meier, u-boot, linux-mtd, Balbi, Felipe,
Ezequiel Garcia, Scott Wood, linux-omap
Hi Pekon,
On 06.01.2014 08:40, Gupta, Pekon wrote:
> Hello Enric, Nikita, and other OMAP3 users,
>
>>
>> As there were parallel set of patches running between u-boot and kernel.
>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>
>> Hence this patch series fixes those regressions, and tests complete
>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>> (following configurations are required for building u-boot driver which is
>> compatible to kernel ecclayout for selected ecc-scheme)
>>
>>
>> (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
>>
>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>> +------------+--------------------------------------------+------------------+
>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>> | | 10, 11, 12 } | |
>> | | (for NAND page-size=2048) | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>> | using s/w | #define CONFIG_BCH | |
>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>> | | 11, 12, 13, 14, \ | |
>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>> | | 25, 26, 27, 28, \ | |
>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>> | | 39, 40, 41, 42, \ | |
>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>> | | 53, 54, 55, 56, } | |
>> | | (for NAND page-size=2048) | |
>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>> | | (for NAND page-size=2048) | |
>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>>
>> #* In addition following patches need to be pulled for u-boot:
>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>
>>
>> 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
>>
>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>> ---
>
> Though I have done initial level of testing on AM335x as mentioned above,
> But will it be possible for you to test and confirm if these set of patches
> solve regressions on your OMAP3 boards ?
Those patches work fine on our custom AM335x board. So:
Tested-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-06 11:48 ` Stefan Roese
0 siblings, 0 replies; 45+ messages in thread
From: Stefan Roese @ 2014-01-06 11:48 UTC (permalink / raw)
To: u-boot
Hi Pekon,
On 06.01.2014 08:40, Gupta, Pekon wrote:
> Hello Enric, Nikita, and other OMAP3 users,
>
>>
>> As there were parallel set of patches running between u-boot and kernel.
>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>
>> Hence this patch series fixes those regressions, and tests complete
>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>> (following configurations are required for building u-boot driver which is
>> compatible to kernel ecclayout for selected ecc-scheme)
>>
>>
>> (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
>>
>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>> +------------+--------------------------------------------+------------------+
>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>> | | 10, 11, 12 } | |
>> | | (for NAND page-size=2048) | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>> | using s/w | #define CONFIG_BCH | |
>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>> | | 11, 12, 13, 14, \ | |
>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>> | | 25, 26, 27, 28, \ | |
>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>> | | 39, 40, 41, 42, \ | |
>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>> | | 53, 54, 55, 56, } | |
>> | | (for NAND page-size=2048) | |
>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>> | | | |
>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>> | | (for NAND page-size=2048) | |
>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>> | | | |
>> +------------+--------------------------------------------+------------------+
>>
>> #* In addition following patches need to be pulled for u-boot:
>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>
>>
>> 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
>>
>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>> ---
>
> Though I have done initial level of testing on AM335x as mentioned above,
> But will it be possible for you to test and confirm if these set of patches
> solve regressions on your OMAP3 boards ?
Those patches work fine on our custom AM335x board. So:
Tested-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-06 11:48 ` Stefan Roese
@ 2014-01-14 14:27 ` Enric Balletbo Serra
-1 siblings, 0 replies; 45+ messages in thread
From: Enric Balletbo Serra @ 2014-01-14 14:27 UTC (permalink / raw)
To: Stefan Roese
Cc: Gupta, Pekon, Javier Martinez Canillas, Thomas Petazzoni,
Igor Grinberg, Michael Trimarchi, Nikita Kiryanov, jp.francois,
ivan.djelic, Woodruff, Richard, Artem Bityutskiy, Brian Norris,
Rini, Tom, avinashphilipk, r.meier, u-boot, linux-mtd, Balbi,
Felipe, Ezequiel Garcia, Scott Wood
Hi Pekon,
2014/1/6 Stefan Roese <sr@denx.de>:
> Hi Pekon,
>
> On 06.01.2014 08:40, Gupta, Pekon wrote:
>> Hello Enric, Nikita, and other OMAP3 users,
>>
>>>
>>> As there were parallel set of patches running between u-boot and kernel.
>>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>>
>>> Hence this patch series fixes those regressions, and tests complete
>>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>>> (following configurations are required for building u-boot driver which is
>>> compatible to kernel ecclayout for selected ecc-scheme)
>>>
>>>
>>> (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
>>>
>>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>>> +------------+--------------------------------------------+------------------+
>>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>>> +------------+--------------------------------------------+------------------+
>>> | | | |
>>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>> | | 10, 11, 12 } | |
>>> | | (for NAND page-size=2048) | |
>>> | | | |
>>> +------------+--------------------------------------------+------------------+
>>> | | | |
>>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>>> | using s/w | #define CONFIG_BCH | |
>>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>>> | | 11, 12, 13, 14, \ | |
>>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>>> | | 25, 26, 27, 28, \ | |
>>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>>> | | 39, 40, 41, 42, \ | |
>>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>>> | | 53, 54, 55, 56, } | |
>>> | | (for NAND page-size=2048) | |
>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>> | | | |
>>> +------------+--------------------------------------------+------------------+
>>> | | | |
>>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>>> | | (for NAND page-size=2048) | |
>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>> | | | |
>>> +------------+--------------------------------------------+------------------+
>>>
>>> #* In addition following patches need to be pulled for u-boot:
>>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>>
>>>
>>> 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
>>>
>>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>>> ---
>>
>> Though I have done initial level of testing on AM335x as mentioned above,
>> But will it be possible for you to test and confirm if these set of patches
>> solve regressions on your OMAP3 boards ?
>
> Those patches work fine on our custom AM335x board. So:
>
> Tested-by: Stefan Roese <sr@denx.de>
>
> Thanks,
> Stefan
>
Sorry for this long delay, I'm doing this in my free time and
sometimes it's difficult to find the time. Those patches also worked
on my OMAP3 boards so
Tested-by: Enric Balletbo i Serra <eballetbo@gmail.com>
Many thanks for your great work.
Best Regards,
Enric
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-14 14:27 ` Enric Balletbo Serra
0 siblings, 0 replies; 45+ messages in thread
From: Enric Balletbo Serra @ 2014-01-14 14:27 UTC (permalink / raw)
To: Stefan Roese
Cc: Thomas Petazzoni, Woodruff, Richard, Nikita Kiryanov,
Artem Bityutskiy, Brian Norris, avinashphilipk, jp.francois,
r.meier, Scott Wood, Balbi, Felipe, u-boot, linux-omap,
linux-mtd, Gupta, Pekon, Ezequiel Garcia, Rini, Tom,
Javier Martinez Canillas, Michael Trimarchi, ivan.djelic,
Igor Grinberg
Hi Pekon,
2014/1/6 Stefan Roese <sr@denx.de>:
> Hi Pekon,
>
> On 06.01.2014 08:40, Gupta, Pekon wrote:
>> Hello Enric, Nikita, and other OMAP3 users,
>>
>>>
>>> As there were parallel set of patches running between u-boot and kernel.
>>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>>
>>> Hence this patch series fixes those regressions, and tests complete
>>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>>> (following configurations are required for building u-boot driver which is
>>> compatible to kernel ecclayout for selected ecc-scheme)
>>>
>>>
>>> (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
>>>
>>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>>> +------------+--------------------------------------------+------------------+
>>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>>> +------------+--------------------------------------------+------------------+
>>> | | | |
>>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>> | | 10, 11, 12 } | |
>>> | | (for NAND page-size=2048) | |
>>> | | | |
>>> +------------+--------------------------------------------+------------------+
>>> | | | |
>>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>>> | using s/w | #define CONFIG_BCH | |
>>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>>> | | 11, 12, 13, 14, \ | |
>>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>>> | | 25, 26, 27, 28, \ | |
>>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>>> | | 39, 40, 41, 42, \ | |
>>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>>> | | 53, 54, 55, 56, } | |
>>> | | (for NAND page-size=2048) | |
>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>> | | | |
>>> +------------+--------------------------------------------+------------------+
>>> | | | |
>>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>>> | | (for NAND page-size=2048) | |
>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>> | | | |
>>> +------------+--------------------------------------------+------------------+
>>>
>>> #* In addition following patches need to be pulled for u-boot:
>>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>>
>>>
>>> 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
>>>
>>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>>> ---
>>
>> Though I have done initial level of testing on AM335x as mentioned above,
>> But will it be possible for you to test and confirm if these set of patches
>> solve regressions on your OMAP3 boards ?
>
> Those patches work fine on our custom AM335x board. So:
>
> Tested-by: Stefan Roese <sr@denx.de>
>
> Thanks,
> Stefan
>
Sorry for this long delay, I'm doing this in my free time and
sometimes it's difficult to find the time. Those patches also worked
on my OMAP3 boards so
Tested-by: Enric Balletbo i Serra <eballetbo@gmail.com>
Many thanks for your great work.
Best Regards,
Enric
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-14 14:27 ` Enric Balletbo Serra
(?)
@ 2014-01-15 4:01 ` Gupta, Pekon
-1 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-15 4:01 UTC (permalink / raw)
To: Enric Balletbo Serra, Stefan Roese
Cc: Javier Martinez Canillas, Thomas Petazzoni, Igor Grinberg,
Michael Trimarchi, Nikita Kiryanov, jp.francois, ivan.djelic,
Woodruff, Richard, Artem Bityutskiy, Brian Norris, Rini, Tom,
avinashphilipk, r.meier, u-boot, linux-mtd, Balbi, Felipe,
Ezequiel Garcia, Scott Wood, linux-omap
Hi Brian,
>From: Enric Balletbo Serra [mailto:eballetbo@gmail.com]
>>2014/1/6 Stefan Roese <sr@denx.de>:
...
>>>> As there were parallel set of patches running between u-boot and kernel.
>>>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>>>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>>>
>>>> Hence this patch series fixes those regressions, and tests complete
>>>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>>>> (following configurations are required for building u-boot driver which is
>>>> compatible to kernel ecclayout for selected ecc-scheme)
>>>>
>>>>
>>>> (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
>>>>
>>>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>>>> +------------+--------------------------------------------+------------------+
>>>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>>>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>>>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>>> | | 10, 11, 12 } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>>>> | using s/w | #define CONFIG_BCH | |
>>>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>>>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>>>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>>>> | | 11, 12, 13, 14, \ | |
>>>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>>>> | | 25, 26, 27, 28, \ | |
>>>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>>>> | | 39, 40, 41, 42, \ | |
>>>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>>>> | | 53, 54, 55, 56, } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>>>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>>>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>>>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>>>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>>>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>>>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>>>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>>>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>>
>>>> #* In addition following patches need to be pulled for u-boot:
>>>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>>>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>>>
>>>>
>>>> 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
>>>>
>>>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>>>> ---
>>>
>>> Though I have done initial level of testing on AM335x as mentioned above,
>>> But will it be possible for you to test and confirm if these set of patches
>>> solve regressions on your OMAP3 boards ?
>>
>> Those patches work fine on our custom AM335x board. So:
>>
>> Tested-by: Stefan Roese <sr@denx.de>
>>
>> Thanks,
>> Stefan
>>
>
>Sorry for this long delay, I'm doing this in my free time and
>sometimes it's difficult to find the time. Those patches also worked
>on my OMAP3 boards so
>
>Tested-by: Enric Balletbo i Serra <eballetbo@gmail.com>
>
>Many thanks for your great work.
>
>Best Regards,
> Enric
>
This patch-set fixes pending regression on OMAP3 platforms and now it
has received Tested-by from:
- Stefan Roese <sr@denx.de> for AM335x platform
- Enric Balletbo i Serra <eballetbo@gmail.com> for OMAP3 platform
So request you to please queue this on priority.
Also, please add following as I missed this while preparing the patches.
Reported-by: Enric Balletbo i Serra <eballetbo@gmail.com>
Thank you.
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-15 4:01 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-15 4:01 UTC (permalink / raw)
To: Enric Balletbo Serra, Stefan Roese, Brian Norris
Cc: Thomas Petazzoni, Woodruff, Richard, Nikita Kiryanov,
Artem Bityutskiy, Brian Norris, avinashphilipk, jp.francois,
r.meier, Scott Wood, u-boot, linux-omap, linux-mtd,
Igor Grinberg, Ezequiel Garcia, Rini, Tom,
Javier Martinez Canillas, Michael Trimarchi, ivan.djelic, Balbi,
Felipe
Hi Brian,
>From: Enric Balletbo Serra [mailto:eballetbo@gmail.com]
>>2014/1/6 Stefan Roese <sr@denx.de>:
...
>>>> As there were parallel set of patches running between u-boot and kernel.
>>>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>>>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>>>
>>>> Hence this patch series fixes those regressions, and tests complete
>>>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>>>> (following configurations are required for building u-boot driver which is
>>>> compatible to kernel ecclayout for selected ecc-scheme)
>>>>
>>>>
>>>> (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
>>>>
>>>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>>>> +------------+--------------------------------------------+------------------+
>>>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>>>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>>>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>>> | | 10, 11, 12 } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>>>> | using s/w | #define CONFIG_BCH | |
>>>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>>>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>>>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>>>> | | 11, 12, 13, 14, \ | |
>>>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>>>> | | 25, 26, 27, 28, \ | |
>>>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>>>> | | 39, 40, 41, 42, \ | |
>>>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>>>> | | 53, 54, 55, 56, } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>>>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>>>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>>>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>>>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>>>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>>>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>>>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>>>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>>
>>>> #* In addition following patches need to be pulled for u-boot:
>>>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>>>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>>>
>>>>
>>>> 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
>>>>
>>>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>>>> ---
>>>
>>> Though I have done initial level of testing on AM335x as mentioned above,
>>> But will it be possible for you to test and confirm if these set of patches
>>> solve regressions on your OMAP3 boards ?
>>
>> Those patches work fine on our custom AM335x board. So:
>>
>> Tested-by: Stefan Roese <sr@denx.de>
>>
>> Thanks,
>> Stefan
>>
>
>Sorry for this long delay, I'm doing this in my free time and
>sometimes it's difficult to find the time. Those patches also worked
>on my OMAP3 boards so
>
>Tested-by: Enric Balletbo i Serra <eballetbo@gmail.com>
>
>Many thanks for your great work.
>
>Best Regards,
> Enric
>
This patch-set fixes pending regression on OMAP3 platforms and now it
has received Tested-by from:
- Stefan Roese <sr@denx.de> for AM335x platform
- Enric Balletbo i Serra <eballetbo@gmail.com> for OMAP3 platform
So request you to please queue this on priority.
Also, please add following as I missed this while preparing the patches.
Reported-by: Enric Balletbo i Serra <eballetbo@gmail.com>
Thank you.
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-15 4:01 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-15 4:01 UTC (permalink / raw)
To: u-boot
Hi Brian,
>From: Enric Balletbo Serra [mailto:eballetbo at gmail.com]
>>2014/1/6 Stefan Roese <sr@denx.de>:
...
>>>> As there were parallel set of patches running between u-boot and kernel.
>>>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>>>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>>>>
>>>> Hence this patch series fixes those regressions, and tests complete
>>>> NAND boot sequence for multiple ecc-schemes on AM335x-EVM board.
>>>> (following configurations are required for building u-boot driver which is
>>>> compatible to kernel ecclayout for selected ecc-scheme)
>>>>
>>>>
>>>> (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
>>>>
>>>> *Configurations used to build u-boot and kernel for end-to-end NAND boot*
>>>> +------------+--------------------------------------------+------------------+
>>>> | ecc-scheme | u-boot/SPL configs | kernel DTS |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | HAM1_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_HAM1_CODE_HW | "ham1" |
>>>> | (1-bit | #define CONFIG_SYS_NAND_ECCBYTES 3 | |
>>>> | Hamming | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>>> | using h/w) | { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>>> | | 10, 11, 12 } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | BCH8_SW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_BCH8_CODE_HW_DETECTION_SW | "bch8" |
>>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 13 |(without ELM node)|
>>>> | using s/w | #define CONFIG_BCH | |
>>>> |library for | #undef CONFIG_SPL_NAND_AM33XX_BCH | |
>>>> |for ECC | #define CONFIG_SPL_NAND_SIMPLE | |
>>>> | error | #define CONFIG_SYS_NAND_ECCPOS \ | |
>>>> |correction) | {2, 3, 4, 5, 6, 7, 8, 9, 10, \ | |
>>>> | | 11, 12, 13, 14, \ | |
>>>> | | 16, 17, 18, 19, 20, 21, 22, 23, 24, \ | |
>>>> | | 25, 26, 27, 28, \ | |
>>>> | | 30, 31, 32, 33, 34, 35, 36, 37, 38, \ | |
>>>> | | 39, 40, 41, 42, \ | |
>>>> | | 44, 45, 46, 47, 48, 49, 50, 51, 52, \ | |
>>>> | | 53, 54, 55, 56, } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>> | | | |
>>>> | BCH8_HW | #define CONFIG_NAND_OMAP_ECCSCHEME \ |ti,nand-ecc-opts= |
>>>> | | OMAP_ECC_BCH8_CODE_HW | "bch8" |
>>>> |(8-bit BCH | #define CONFIG_SYS_NAND_ECCBYTES 14 | |
>>>> | using ELM | #define CONFIG_SPL_NAND_AM33XX_BCH |(with ELM node) |
>>>> | h/w engine | #define CONFIG_SYS_NAND_ECCPOS \ |ti,elm-id=<&elm> |
>>>> |for ECC | {2, 3, 4, 5, 6, 7, 8, 9, \ | |
>>>> | error | 10, 11, 12, 13, 14, 15, 16, 17, \ | |
>>>> |correction) | 18, 19, 20, 21, 22, 23, 24, 25, \ | |
>>>> | | 26, 27, 28, 29, 30, 31, 32, 33, \ | |
>>>> | | 34, 35, 36, 37, 38, 39, 40, 41, \ | |
>>>> | | 42, 43, 44, 45, 46, 47, 48, 49, \ | |
>>>> | | 50, 51, 52, 53, 54, 55, 56, 57, } | |
>>>> | | (for NAND page-size=2048) | |
>>>> | | #define CONFIG_SYS_NAND_ECCSIZE 512 | |
>>>> | | | |
>>>> +------------+--------------------------------------------+------------------+
>>>>
>>>> #* In addition following patches need to be pulled for u-boot:
>>>> http://lists.denx.de/pipermail/u-boot/2013-December/168506.html
>>>> http://lists.denx.de/pipermail/u-boot/2013-December/169021.html
>>>>
>>>>
>>>> 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
>>>>
>>>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>>>> ---
>>>
>>> Though I have done initial level of testing on AM335x as mentioned above,
>>> But will it be possible for you to test and confirm if these set of patches
>>> solve regressions on your OMAP3 boards ?
>>
>> Those patches work fine on our custom AM335x board. So:
>>
>> Tested-by: Stefan Roese <sr@denx.de>
>>
>> Thanks,
>> Stefan
>>
>
>Sorry for this long delay, I'm doing this in my free time and
>sometimes it's difficult to find the time. Those patches also worked
>on my OMAP3 boards so
>
>Tested-by: Enric Balletbo i Serra <eballetbo@gmail.com>
>
>Many thanks for your great work.
>
>Best Regards,
> Enric
>
This patch-set fixes pending regression on OMAP3 platforms and now it
has received Tested-by from:
- Stefan Roese <sr@denx.de> for AM335x platform
- Enric Balletbo i Serra <eballetbo@gmail.com> for OMAP3 platform
So request you to please queue this on priority.
Also, please add following as I missed this while preparing the patches.
Reported-by: Enric Balletbo i Serra <eballetbo@gmail.com>
Thank you.
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2013-12-13 9:12 ` Pekon Gupta
(?)
@ 2014-01-25 8:55 ` Brian Norris
-1 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:55 UTC (permalink / raw)
To: Pekon Gupta
Cc: Artem Bityutskiy, Scott Wood, Tom Rini, u-boot, linux-mtd,
linux-omap, Enric Balletbo Serra, Stefan Roese,
Javier Martinez Canillas, Thomas Petazzoni, Igor Grinberg,
Michael Trimarchi, r.meier, r-woodruff2, Nikita Kiryanov,
jp.francois, ivan.djelic, Ezequiel Garcia, avinashphilipk, balbi
Hi Pekon,
Sorry, I'm revisiting your patch series a bit late. There are a few
factors that contributed to this, though.
1. This patch series talks extensively about U-Boot. U-Boot is not my
interest, nor should it be the focus of kernel (driver) development.
Any work done here should be framed in the kernel driver context. [1]
2. You have previously CC'd me on U-Boot patches. Or at least, I think
so. More about this in the next point...
3. The U-Boot source code structure is rather similar to pieces of Linux
subsystems. So without additional effort, it is hard to discern
whether a patch is intended for Linux MTD or U-Boot MTD.
4. You cross-posted to both the U-Boot and Linux-MTD mailing lists.
So the sum of all this is that I ignored these patches at first, as they
weren't clearly intended for me.
On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
> As there were parallel set of patches running between u-boot and kernel.
I don't know what patches you're talking about.
> hence, some patch-sets caused regression for OMAP3x platforms when booting
"Hence" is not the right word. The previous sentence doesn't imply that
there were regressions.
> using u-boot specifically for ecc-schemes (like BCH4_SW).
Huh? What does "specifically for ecc-schemes" mean? You mean that only
some schemes had regressions? Can you be more specific? What are these
regressions?
The rest of this cover letter (and most of the patch descriptions)
describes the configurations and testing (which is all very good, don't
get me wrong), with a lot of focus on things I don't follow (namely,
U-Boot), but you omit many of the important details, like
* What is the regression? I honestly don't see a description of the
actual regression. (I can read the code to intuit that, but what's
the point of your pages of description if it doesn't mention this?)
Please give some logical description of the problem
* What are the effects of the regression? ECC bytes in the wrong place,
so you see correction failures/corruption? JFFS2 failures? (The
"oobfree" changes, for instance, should only affect something like
JFFS2.)
* What Linux commit caused the regression?
* Should this patch set be marked for -stable? And for what kernel
release? (the previous question would help answer this)
> Hence this patch series fixes those regressions, and tests complete
[...] snip
I'm preparing a 3.14 pull request soon, and since you seem committed to
fixing and properly testing a known regression here, I'd like to see
this go in. But given the late timing and the unanswered questions, I
think it's unlikely to go in -rc1. Perhaps I can send a later pull
request if we can get it together properly.
So I'd like to first of all get a few answers to my questions, and then
I think we might want to refresh this patch series with a little better
commit messages and perhaps a separate documentation file (not
necessarily in the same patch series, as the fix is more urgent).
Brian
[1] For instance, rather than saying "the ECC layout doesn't match
U-Boot", you should probably describe what the intended ECC layout
should look like and show that Linux does not match it. Perhaps it's
time for some better documentation, like Ezequiel stashed under
Documentation/mtd/nand/ for pxa3xx. You could also take some cues
from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-25 8:55 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:55 UTC (permalink / raw)
To: Pekon Gupta
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
u-boot, avinashphilipk, Artem Bityutskiy, jp.francois, r.meier,
balbi, Tom Rini, Enric Balletbo Serra, linux-mtd, Igor Grinberg,
Ezequiel Garcia, Scott Wood, Javier Martinez Canillas,
linux-omap, ivan.djelic, Nikita Kiryanov
Hi Pekon,
Sorry, I'm revisiting your patch series a bit late. There are a few
factors that contributed to this, though.
1. This patch series talks extensively about U-Boot. U-Boot is not my
interest, nor should it be the focus of kernel (driver) development.
Any work done here should be framed in the kernel driver context. [1]
2. You have previously CC'd me on U-Boot patches. Or at least, I think
so. More about this in the next point...
3. The U-Boot source code structure is rather similar to pieces of Linux
subsystems. So without additional effort, it is hard to discern
whether a patch is intended for Linux MTD or U-Boot MTD.
4. You cross-posted to both the U-Boot and Linux-MTD mailing lists.
So the sum of all this is that I ignored these patches at first, as they
weren't clearly intended for me.
On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
> As there were parallel set of patches running between u-boot and kernel.
I don't know what patches you're talking about.
> hence, some patch-sets caused regression for OMAP3x platforms when booting
"Hence" is not the right word. The previous sentence doesn't imply that
there were regressions.
> using u-boot specifically for ecc-schemes (like BCH4_SW).
Huh? What does "specifically for ecc-schemes" mean? You mean that only
some schemes had regressions? Can you be more specific? What are these
regressions?
The rest of this cover letter (and most of the patch descriptions)
describes the configurations and testing (which is all very good, don't
get me wrong), with a lot of focus on things I don't follow (namely,
U-Boot), but you omit many of the important details, like
* What is the regression? I honestly don't see a description of the
actual regression. (I can read the code to intuit that, but what's
the point of your pages of description if it doesn't mention this?)
Please give some logical description of the problem
* What are the effects of the regression? ECC bytes in the wrong place,
so you see correction failures/corruption? JFFS2 failures? (The
"oobfree" changes, for instance, should only affect something like
JFFS2.)
* What Linux commit caused the regression?
* Should this patch set be marked for -stable? And for what kernel
release? (the previous question would help answer this)
> Hence this patch series fixes those regressions, and tests complete
[...] snip
I'm preparing a 3.14 pull request soon, and since you seem committed to
fixing and properly testing a known regression here, I'd like to see
this go in. But given the late timing and the unanswered questions, I
think it's unlikely to go in -rc1. Perhaps I can send a later pull
request if we can get it together properly.
So I'd like to first of all get a few answers to my questions, and then
I think we might want to refresh this patch series with a little better
commit messages and perhaps a separate documentation file (not
necessarily in the same patch series, as the fix is more urgent).
Brian
[1] For instance, rather than saying "the ECC layout doesn't match
U-Boot", you should probably describe what the intended ECC layout
should look like and show that Linux does not match it. Perhaps it's
time for some better documentation, like Ezequiel stashed under
Documentation/mtd/nand/ for pxa3xx. You could also take some cues
from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-25 8:55 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:55 UTC (permalink / raw)
To: u-boot
Hi Pekon,
Sorry, I'm revisiting your patch series a bit late. There are a few
factors that contributed to this, though.
1. This patch series talks extensively about U-Boot. U-Boot is not my
interest, nor should it be the focus of kernel (driver) development.
Any work done here should be framed in the kernel driver context. [1]
2. You have previously CC'd me on U-Boot patches. Or at least, I think
so. More about this in the next point...
3. The U-Boot source code structure is rather similar to pieces of Linux
subsystems. So without additional effort, it is hard to discern
whether a patch is intended for Linux MTD or U-Boot MTD.
4. You cross-posted to both the U-Boot and Linux-MTD mailing lists.
So the sum of all this is that I ignored these patches at first, as they
weren't clearly intended for me.
On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
> As there were parallel set of patches running between u-boot and kernel.
I don't know what patches you're talking about.
> hence, some patch-sets caused regression for OMAP3x platforms when booting
"Hence" is not the right word. The previous sentence doesn't imply that
there were regressions.
> using u-boot specifically for ecc-schemes (like BCH4_SW).
Huh? What does "specifically for ecc-schemes" mean? You mean that only
some schemes had regressions? Can you be more specific? What are these
regressions?
The rest of this cover letter (and most of the patch descriptions)
describes the configurations and testing (which is all very good, don't
get me wrong), with a lot of focus on things I don't follow (namely,
U-Boot), but you omit many of the important details, like
* What is the regression? I honestly don't see a description of the
actual regression. (I can read the code to intuit that, but what's
the point of your pages of description if it doesn't mention this?)
Please give some logical description of the problem
* What are the effects of the regression? ECC bytes in the wrong place,
so you see correction failures/corruption? JFFS2 failures? (The
"oobfree" changes, for instance, should only affect something like
JFFS2.)
* What Linux commit caused the regression?
* Should this patch set be marked for -stable? And for what kernel
release? (the previous question would help answer this)
> Hence this patch series fixes those regressions, and tests complete
[...] snip
I'm preparing a 3.14 pull request soon, and since you seem committed to
fixing and properly testing a known regression here, I'd like to see
this go in. But given the late timing and the unanswered questions, I
think it's unlikely to go in -rc1. Perhaps I can send a later pull
request if we can get it together properly.
So I'd like to first of all get a few answers to my questions, and then
I think we might want to refresh this patch series with a little better
commit messages and perhaps a separate documentation file (not
necessarily in the same patch series, as the fix is more urgent).
Brian
[1] For instance, rather than saying "the ECC layout doesn't match
U-Boot", you should probably describe what the intended ECC layout
should look like and show that Linux does not match it. Perhaps it's
time for some better documentation, like Ezequiel stashed under
Documentation/mtd/nand/ for pxa3xx. You could also take some cues
from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
2013-12-13 9:12 ` Pekon Gupta
(?)
@ 2014-01-25 8:56 ` Brian Norris
-1 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:56 UTC (permalink / raw)
To: Pekon Gupta
Cc: Artem Bityutskiy, Scott Wood, Tom Rini, u-boot, linux-mtd,
linux-omap, Enric Balletbo Serra, Stefan Roese,
Javier Martinez Canillas, Thomas Petazzoni, Igor Grinberg,
Michael Trimarchi, r.meier, r-woodruff2, Nikita Kiryanov,
jp.francois, ivan.djelic, Ezequiel Garcia, avinashphilipk, balbi
On Fri, Dec 13, 2013 at 02:42:57PM +0530, Pekon Gupta wrote:
> This patch updates starting offset for free bytes in OOB which can be used by
> file-systems to store their metadata (like clean-marker in case of JFFS2).
This should be describing a regression fix, right? We don't just
arbitrarily change the "OOB free" layout; we need a reason. So please
describe it here.
(It seems like an off-by-one, or off-by-<N> error, where the oobfree
region was miscalculated.)
Possibly you can paste an example intended ecclayout as well as an
incorrect layout that was calculated before this fix.
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
> drivers/mtd/nand/omap2.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index f777250..bbdb5e8 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1835,8 +1835,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> else
> ecclayout->eccpos[0] = 1;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> break;
>
> case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> @@ -1854,8 +1852,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
> @@ -1890,8 +1886,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* 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");
> @@ -1920,8 +1914,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
> @@ -1963,8 +1955,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> break;
> #else
> pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
> @@ -1978,9 +1968,6 @@ 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);
> 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 */
> @@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
> err = -EINVAL;
> goto return_error;
> }
> + /* populate remaining ECC layout data */
> + ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
Hmm, this line seems a little odd. The ecclayout->eccpos[] array and the
value of ecclayout->eccbytes sould be related as follows:
let N = ecclayout->eccbytes
This means the eccpos[] array should have N entries, indexed 0 to N-1,
and eccpos[N] is out of bounds. But you are accessing eccpos[N] above
(i.e., eccpos[ecclayout->eccbytes]). Are you sure this is correct? It
seems like it should be:
ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
> + ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
> + ecclayout->eccbytes);
>
> /* second phase scan */
> if (nand_scan_tail(mtd)) {
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
@ 2014-01-25 8:56 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:56 UTC (permalink / raw)
To: Pekon Gupta
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
u-boot, avinashphilipk, Artem Bityutskiy, jp.francois, r.meier,
balbi, Tom Rini, Enric Balletbo Serra, linux-mtd, Igor Grinberg,
Ezequiel Garcia, Scott Wood, Javier Martinez Canillas,
linux-omap, ivan.djelic, Nikita Kiryanov
On Fri, Dec 13, 2013 at 02:42:57PM +0530, Pekon Gupta wrote:
> This patch updates starting offset for free bytes in OOB which can be used by
> file-systems to store their metadata (like clean-marker in case of JFFS2).
This should be describing a regression fix, right? We don't just
arbitrarily change the "OOB free" layout; we need a reason. So please
describe it here.
(It seems like an off-by-one, or off-by-<N> error, where the oobfree
region was miscalculated.)
Possibly you can paste an example intended ecclayout as well as an
incorrect layout that was calculated before this fix.
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
> drivers/mtd/nand/omap2.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index f777250..bbdb5e8 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1835,8 +1835,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> else
> ecclayout->eccpos[0] = 1;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> break;
>
> case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> @@ -1854,8 +1852,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
> @@ -1890,8 +1886,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* 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");
> @@ -1920,8 +1914,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
> @@ -1963,8 +1955,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> break;
> #else
> pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
> @@ -1978,9 +1968,6 @@ 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);
> 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 */
> @@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
> err = -EINVAL;
> goto return_error;
> }
> + /* populate remaining ECC layout data */
> + ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
Hmm, this line seems a little odd. The ecclayout->eccpos[] array and the
value of ecclayout->eccbytes sould be related as follows:
let N = ecclayout->eccbytes
This means the eccpos[] array should have N entries, indexed 0 to N-1,
and eccpos[N] is out of bounds. But you are accessing eccpos[N] above
(i.e., eccpos[ecclayout->eccbytes]). Are you sure this is correct? It
seems like it should be:
ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
> + ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
> + ecclayout->eccbytes);
>
> /* second phase scan */
> if (nand_scan_tail(mtd)) {
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
@ 2014-01-25 8:56 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:56 UTC (permalink / raw)
To: u-boot
On Fri, Dec 13, 2013 at 02:42:57PM +0530, Pekon Gupta wrote:
> This patch updates starting offset for free bytes in OOB which can be used by
> file-systems to store their metadata (like clean-marker in case of JFFS2).
This should be describing a regression fix, right? We don't just
arbitrarily change the "OOB free" layout; we need a reason. So please
describe it here.
(It seems like an off-by-one, or off-by-<N> error, where the oobfree
region was miscalculated.)
Possibly you can paste an example intended ecclayout as well as an
incorrect layout that was calculated before this fix.
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
> drivers/mtd/nand/omap2.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index f777250..bbdb5e8 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1835,8 +1835,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> else
> ecclayout->eccpos[0] = 1;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> break;
>
> case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
> @@ -1854,8 +1852,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
> @@ -1890,8 +1886,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* 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");
> @@ -1920,8 +1914,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
> @@ -1963,8 +1955,6 @@ static int omap_nand_probe(struct platform_device *pdev)
> (mtd->writesize /
> nand_chip->ecc.size);
> ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH;
> - ecclayout->oobfree->offset = ecclayout->eccpos[0] +
> - ecclayout->eccbytes;
> break;
> #else
> pr_err("nand: error: CONFIG_MTD_NAND_OMAP_BCH not enabled\n");
> @@ -1978,9 +1968,6 @@ 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);
> 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 */
> @@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
> err = -EINVAL;
> goto return_error;
> }
> + /* populate remaining ECC layout data */
> + ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
Hmm, this line seems a little odd. The ecclayout->eccpos[] array and the
value of ecclayout->eccbytes sould be related as follows:
let N = ecclayout->eccbytes
This means the eccpos[] array should have N entries, indexed 0 to N-1,
and eccpos[N] is out of bounds. But you are accessing eccpos[N] above
(i.e., eccpos[ecclayout->eccbytes]). Are you sure this is correct? It
seems like it should be:
ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
> + ecclayout->oobfree->length = mtd->oobsize - (BADBLOCK_MARKER_LENGTH +
> + ecclayout->eccbytes);
>
> /* second phase scan */
> if (nand_scan_tail(mtd)) {
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 2/2] mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
2013-12-13 9:12 ` Pekon Gupta
(?)
@ 2014-01-25 8:57 ` Brian Norris
-1 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:57 UTC (permalink / raw)
To: Pekon Gupta
Cc: Artem Bityutskiy, Scott Wood, Tom Rini, u-boot, linux-mtd,
linux-omap, Enric Balletbo Serra, Stefan Roese,
Javier Martinez Canillas, Thomas Petazzoni, Igor Grinberg,
Michael Trimarchi, r.meier, r-woodruff2, Nikita Kiryanov,
jp.francois, ivan.djelic, Ezequiel Garcia, avinashphilipk, balbi
On Fri, Dec 13, 2013 at 02:42:58PM +0530, Pekon Gupta wrote:
> @@ -1851,7 +1854,13 @@ 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++) {
> + if ((i % nand_chip->ecc.bytes) || (i == 0))
> + ecclayout->eccpos[i] = oob_index;
> + else
> + ecclayout->eccpos[i] = ++oob_index;
This if-else structure, with a combined assignment and increment kind of
obscures the logic of what you're really trying to do. Could this be
better as follows?
for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) {
ecclayout->eccpos[i] = oob_index;
if (((i + 1) % nand_chip->ecc.bytes) == 0)
oob_index++;
}
> + }
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 2/2] mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
@ 2014-01-25 8:57 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:57 UTC (permalink / raw)
To: Pekon Gupta
Cc: Thomas Petazzoni, Stefan Roese, Michael Trimarchi, r-woodruff2,
u-boot, avinashphilipk, Artem Bityutskiy, jp.francois, r.meier,
balbi, Tom Rini, Enric Balletbo Serra, linux-mtd, Igor Grinberg,
Ezequiel Garcia, Scott Wood, Javier Martinez Canillas,
linux-omap, ivan.djelic, Nikita Kiryanov
On Fri, Dec 13, 2013 at 02:42:58PM +0530, Pekon Gupta wrote:
> @@ -1851,7 +1854,13 @@ 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++) {
> + if ((i % nand_chip->ecc.bytes) || (i == 0))
> + ecclayout->eccpos[i] = oob_index;
> + else
> + ecclayout->eccpos[i] = ++oob_index;
This if-else structure, with a combined assignment and increment kind of
obscures the logic of what you're really trying to do. Could this be
better as follows?
for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) {
ecclayout->eccpos[i] = oob_index;
if (((i + 1) % nand_chip->ecc.bytes) == 0)
oob_index++;
}
> + }
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 2/2] mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver
@ 2014-01-25 8:57 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-25 8:57 UTC (permalink / raw)
To: u-boot
On Fri, Dec 13, 2013 at 02:42:58PM +0530, Pekon Gupta wrote:
> @@ -1851,7 +1854,13 @@ 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++) {
> + if ((i % nand_chip->ecc.bytes) || (i == 0))
> + ecclayout->eccpos[i] = oob_index;
> + else
> + ecclayout->eccpos[i] = ++oob_index;
This if-else structure, with a combined assignment and increment kind of
obscures the logic of what you're really trying to do. Could this be
better as follows?
for (i = 0; i < ecclayout->eccbytes; i++, oob_index++) {
ecclayout->eccpos[i] = oob_index;
if (((i + 1) % nand_chip->ecc.bytes) == 0)
oob_index++;
}
> + }
> /* software bch library is used for locating errors */
> nand_chip->ecc.priv = nand_bch_init(mtd,
> nand_chip->ecc.size,
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-25 8:55 ` Brian Norris
@ 2014-01-27 17:46 ` Gupta, Pekon
-1 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-27 17:46 UTC (permalink / raw)
To: Brian Norris
Cc: Thomas Petazzoni, Michael Trimarchi, Nikita Kiryanov, Woodruff,
Richard, u-boot, avinashphilipk, Artem Bityutskiy, jp.francois,
r.meier, Rini, Tom, Enric Balletbo Serra, linux-mtd,
Igor Grinberg, Ezequiel Garcia, Scott Wood,
Javier Martinez Canillas, Stefan Roese, ivan.djelic, Balbi,
Felipe
Hi Brian,
>From: Brian Norris
>
>1. This patch series talks extensively about U-Boot. U-Boot is not my
> interest, nor should it be the focus of kernel (driver) development.
> Any work done here should be framed in the kernel driver context. [1]
>
Apologies for cross-posting, I understand that you are already flooded by emails
from linux-mtd list. But my intention was to keep all users of OMAP3 informed,
as this regression was Reported-by: Enric Balletbo Serra <eballetbo@iseebcn.com>
while testing mainline u-boot & kernel on OMAP3 platform.
Thanks for you feedbacks.
I'll fix the commit messages with proper description of the regression,
And incorporate other comments when I re-send it. Also, I'll cut-down the
CC list, as u-boot mailman blocks emails with long CC list.
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-27 17:46 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-27 17:46 UTC (permalink / raw)
To: u-boot
Hi Brian,
>From: Brian Norris
>
>1. This patch series talks extensively about U-Boot. U-Boot is not my
> interest, nor should it be the focus of kernel (driver) development.
> Any work done here should be framed in the kernel driver context. [1]
>
Apologies for cross-posting, I understand that you are already flooded by emails
from linux-mtd list. But my intention was to keep all users of OMAP3 informed,
as this regression was Reported-by: Enric Balletbo Serra <eballetbo@iseebcn.com>
while testing mainline u-boot & kernel on OMAP3 platform.
Thanks for you feedbacks.
I'll fix the commit messages with proper description of the regression,
And incorporate other comments when I re-send it. Also, I'll cut-down the
CC list, as u-boot mailman blocks emails with long CC list.
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-27 17:46 ` [U-Boot] " Gupta, Pekon
@ 2014-01-27 19:07 ` Brian Norris
-1 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-27 19:07 UTC (permalink / raw)
To: Gupta, Pekon
Cc: Thomas Petazzoni, Michael Trimarchi, Nikita Kiryanov, Woodruff,
Richard, u-boot, avinashphilipk, Artem Bityutskiy, jp.francois,
r.meier, Rini, Tom, Enric Balletbo Serra, linux-mtd,
Igor Grinberg, Ezequiel Garcia, Scott Wood,
Javier Martinez Canillas, Stefan Roese, ivan.djelic, Balbi,
Felipe
On Mon, Jan 27, 2014 at 9:46 AM, Gupta, Pekon <pekon@ti.com> wrote:
>>From: Brian Norris
>>
>>1. This patch series talks extensively about U-Boot. U-Boot is not my
>> interest, nor should it be the focus of kernel (driver) development.
>> Any work done here should be framed in the kernel driver context. [1]
>>
> Apologies for cross-posting, I understand that you are already flooded by emails
> from linux-mtd list. But my intention was to keep all users of OMAP3 informed,
> as this regression was Reported-by: Enric Balletbo Serra <eballetbo@iseebcn.com>
> while testing mainline u-boot & kernel on OMAP3 platform.
This last line ("while testing mainline u-boot & kernel on OMAP3
platform") is part of what worries me and requires more explanation.
An upgrade to *either* U-Boot or kernel should not cause regressions
for already-supported platforms (if this is new platform support, then
that's different, and it's not exactly a "regression" in that case;
but I know some of the kernel features are new platform support).
> Thanks for you feedbacks.
> I'll fix the commit messages with proper description of the regression,
> And incorporate other comments when I re-send it.
Can you please respond to a few of the concerns before sending a new
patch set? I'd like to have the proper explanation and discussion up
front here, rather than burying it in your long patch descriptions
with test results. Then the end result can go into a proper commit
message, once we're all happy.
> Also, I'll cut-down the
> CC list, as u-boot mailman blocks emails with long CC list.
Yeah, I noticed that after I sent my replies... IMO, you can drop the
u-boot list if that helps.
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-27 19:07 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-01-27 19:07 UTC (permalink / raw)
To: u-boot
On Mon, Jan 27, 2014 at 9:46 AM, Gupta, Pekon <pekon@ti.com> wrote:
>>From: Brian Norris
>>
>>1. This patch series talks extensively about U-Boot. U-Boot is not my
>> interest, nor should it be the focus of kernel (driver) development.
>> Any work done here should be framed in the kernel driver context. [1]
>>
> Apologies for cross-posting, I understand that you are already flooded by emails
> from linux-mtd list. But my intention was to keep all users of OMAP3 informed,
> as this regression was Reported-by: Enric Balletbo Serra <eballetbo@iseebcn.com>
> while testing mainline u-boot & kernel on OMAP3 platform.
This last line ("while testing mainline u-boot & kernel on OMAP3
platform") is part of what worries me and requires more explanation.
An upgrade to *either* U-Boot or kernel should not cause regressions
for already-supported platforms (if this is new platform support, then
that's different, and it's not exactly a "regression" in that case;
but I know some of the kernel features are new platform support).
> Thanks for you feedbacks.
> I'll fix the commit messages with proper description of the regression,
> And incorporate other comments when I re-send it.
Can you please respond to a few of the concerns before sending a new
patch set? I'd like to have the proper explanation and discussion up
front here, rather than burying it in your long patch descriptions
with test results. Then the end result can go into a proper commit
message, once we're all happy.
> Also, I'll cut-down the
> CC list, as u-boot mailman blocks emails with long CC list.
Yeah, I noticed that after I sent my replies... IMO, you can drop the
u-boot list if that helps.
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-25 8:55 ` Brian Norris
(?)
@ 2014-01-28 7:42 ` Gupta, Pekon
-1 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-28 7:42 UTC (permalink / raw)
To: Brian Norris
Cc: Artem Bityutskiy, Balbi, Felipe, u-boot, Enric Balletbo Serra,
linux-mtd, linux-omap
Hi Brian,
>From: Brian Norris
>
>Hi Pekon,
>
>Sorry, I'm revisiting your patch series a bit late. There are a few
>factors that contributed to this, though.
>
>1. This patch series talks extensively about U-Boot. U-Boot is not my
> interest, nor should it be the focus of kernel (driver) development.
> Any work done here should be framed in the kernel driver context. [1]
>
>2. You have previously CC'd me on U-Boot patches. Or at least, I think
> so. More about this in the next point...
>
>3. The U-Boot source code structure is rather similar to pieces of Linux
> subsystems. So without additional effort, it is hard to discern
> whether a patch is intended for Linux MTD or U-Boot MTD.
>
>4. You cross-posted to both the U-Boot and Linux-MTD mailing lists.
>
>So the sum of all this is that I ignored these patches at first, as they
>weren't clearly intended for me.
>
>On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
>> As there were parallel set of patches running between u-boot and kernel.
>
>I don't know what patches you're talking about.
>
Following patch-sets touched ecc.layout while cleaning up OMAP NAND driver.
u-boot: http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
kernel: http://lists.infradead.org/pipermail/linux-mtd/2013-October/049414.html
>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>
>"Hence" is not the right word. The previous sentence doesn't imply that
>there were regressions.
>
>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>
>Huh? What does "specifically for ecc-schemes" mean? You mean that only
>some schemes had regressions? Can you be more specific? What are these
>regressions?
>
It was reported by Enric Balletbo Serra <eballetbo@iseebcn.com> [2] that
when using 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' ecc-scheme
there was a mismatch in ECC layout between mainline u-boot and kernel.
This caused boot failure when kernel tried to mount UBIFS with image
flashed from u-boot.
This was missed while testing earlier patch-sets because only OMAP3 boards
use above ecc-scheme as OMAP3 SoC do not have ELM hardware engine.
All the later platforms use 'OMAP_ECC_BCH8_CODE_HW' ecc-scheme.
Both ecc-scheme are based on same algorithm of BCH8 ECC code except that:
(1) 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' uses /lib/bch.c software
library for ecc-correction.
(2) 'OMAP_ECC_BCH8_CODE_HW' uses ELM hardware engine for ecc-correction.
This regression affects following ecc-schemes, as ecc.layout logic is just
copy-paste of each-other.
- 'OMAP_ECC_BCH4_CODE_HW_DETECTION_SW' and
- 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW'
>The rest of this cover letter (and most of the patch descriptions)
>describes the configurations and testing (which is all very good, don't
>get me wrong), with a lot of focus on things I don't follow (namely,
>U-Boot), but you omit many of the important details, like
>
> * What is the regression? I honestly don't see a description of the
> actual regression. (I can read the code to intuit that, but what's
> the point of your pages of description if it doesn't mention this?)
> Please give some logical description of the problem
>
[Patch 1/2] of this series actually simplification of 'ecclayout->oobfree->offset'
It does not fix the regression, but it's important to clean it, before any new
feature additions.
[Patch 2/2] Actually fixes the regression for affected ecc-schemes. But
"this patch also touches other ecc-schemes as the fix required
refactoring common code, into ecc-scheme specific code."
> * What are the effects of the regression? ECC bytes in the wrong place,
> so you see correction failures/corruption? JFFS2 failures? (The
> "oobfree" changes, for instance, should only affect something like
> JFFS2.)
>
As mentioned above [PATCH 1/2] which touches 'ecclayout->oobfree->offset'
is a clean-up not affecting regression, but it should be taken in before
any further feature or ecc-scheme changes.
Now, what is your preference?
(a) Take [PATCH 1/2] ecc.layout clean-up separately.
(b) Take ecc.layout as part of this series, but may be with different patch title.
> * What Linux commit caused the regression?
> * Should this patch set be marked for -stable? And for what kernel
> release? (the previous question would help answer this)
>
This regression was caused in
commit b491da7233d5dc1a24d46ca1ad0209900329c5d0
mtd: nand: omap: clean-up ecc layout for BCH ecc schemes
So, this should be applicable from 3.13.x+
>> Hence this patch series fixes those regressions, and tests complete
>[...] snip
>
>I'm preparing a 3.14 pull request soon, and since you seem committed to
>fixing and properly testing a known regression here, I'd like to see
>this go in. But given the late timing and the unanswered questions, I
>think it's unlikely to go in -rc1. Perhaps I can send a later pull
>request if we can get it together properly.
>
Yes, if the above details are sufficient, then I'll:
(1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
(2) Add above description and details of regression into commit log of [PATCH 2/2]
(3) Include your other comments on individual patches.
>So I'd like to first of all get a few answers to my questions, and then
>I think we might want to refresh this patch series with a little better
>commit messages and perhaps a separate documentation file (not
>necessarily in the same patch series, as the fix is more urgent).
>
Yes, I plan to get some documentation in kernel docs. But for now
I do have a 'un-polished' page for OMAP NAND driver on TI wiki [3].
Please let me know if you would like something similar for kernel docs.
I'm happy that you are scrutinizing each patch and their commit logs wisely.
Thanks much for that. This helps me and probably everyone else to tighten-up
so that only good quality patch-sets go in. This eventually will make the
sub-system and the drivers more stable for long-term.
Also, if you don't mind, I'll still continue to cross-post this particular
Patch-series on both u-boot and kernel mailing lists just to maintain
continuity, as I know there are lot of OMAP3 users which are following
only one of the two lists.. (but I'm cutting down on the CC list).
I'll avoid cross posting in future.
>Brian
>
> [1] For instance, rather than saying "the ECC layout doesn't match
> U-Boot", you should probably describe what the intended ECC layout
> should look like and show that Linux does not match it. Perhaps it's
> time for some better documentation, like Ezequiel stashed under
> Documentation/mtd/nand/ for pxa3xx. You could also take some cues
> from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
[2] http://lists.denx.de/pipermail/u-boot/2013-December/168440.html
[3] https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide (under construction)
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-28 7:42 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-28 7:42 UTC (permalink / raw)
To: Brian Norris
Cc: Artem Bityutskiy, Balbi, Felipe, u-boot, Enric Balletbo Serra,
linux-mtd, linux-omap
Hi Brian,
>From: Brian Norris
>
>Hi Pekon,
>
>Sorry, I'm revisiting your patch series a bit late. There are a few
>factors that contributed to this, though.
>
>1. This patch series talks extensively about U-Boot. U-Boot is not my
> interest, nor should it be the focus of kernel (driver) development.
> Any work done here should be framed in the kernel driver context. [1]
>
>2. You have previously CC'd me on U-Boot patches. Or at least, I think
> so. More about this in the next point...
>
>3. The U-Boot source code structure is rather similar to pieces of Linux
> subsystems. So without additional effort, it is hard to discern
> whether a patch is intended for Linux MTD or U-Boot MTD.
>
>4. You cross-posted to both the U-Boot and Linux-MTD mailing lists.
>
>So the sum of all this is that I ignored these patches at first, as they
>weren't clearly intended for me.
>
>On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
>> As there were parallel set of patches running between u-boot and kernel.
>
>I don't know what patches you're talking about.
>
Following patch-sets touched ecc.layout while cleaning up OMAP NAND driver.
u-boot: http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
kernel: http://lists.infradead.org/pipermail/linux-mtd/2013-October/049414.html
>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>
>"Hence" is not the right word. The previous sentence doesn't imply that
>there were regressions.
>
>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>
>Huh? What does "specifically for ecc-schemes" mean? You mean that only
>some schemes had regressions? Can you be more specific? What are these
>regressions?
>
It was reported by Enric Balletbo Serra <eballetbo@iseebcn.com> [2] that
when using 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' ecc-scheme
there was a mismatch in ECC layout between mainline u-boot and kernel.
This caused boot failure when kernel tried to mount UBIFS with image
flashed from u-boot.
This was missed while testing earlier patch-sets because only OMAP3 boards
use above ecc-scheme as OMAP3 SoC do not have ELM hardware engine.
All the later platforms use 'OMAP_ECC_BCH8_CODE_HW' ecc-scheme.
Both ecc-scheme are based on same algorithm of BCH8 ECC code except that:
(1) 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' uses /lib/bch.c software
library for ecc-correction.
(2) 'OMAP_ECC_BCH8_CODE_HW' uses ELM hardware engine for ecc-correction.
This regression affects following ecc-schemes, as ecc.layout logic is just
copy-paste of each-other.
- 'OMAP_ECC_BCH4_CODE_HW_DETECTION_SW' and
- 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW'
>The rest of this cover letter (and most of the patch descriptions)
>describes the configurations and testing (which is all very good, don't
>get me wrong), with a lot of focus on things I don't follow (namely,
>U-Boot), but you omit many of the important details, like
>
> * What is the regression? I honestly don't see a description of the
> actual regression. (I can read the code to intuit that, but what's
> the point of your pages of description if it doesn't mention this?)
> Please give some logical description of the problem
>
[Patch 1/2] of this series actually simplification of 'ecclayout->oobfree->offset'
It does not fix the regression, but it's important to clean it, before any new
feature additions.
[Patch 2/2] Actually fixes the regression for affected ecc-schemes. But
"this patch also touches other ecc-schemes as the fix required
refactoring common code, into ecc-scheme specific code."
> * What are the effects of the regression? ECC bytes in the wrong place,
> so you see correction failures/corruption? JFFS2 failures? (The
> "oobfree" changes, for instance, should only affect something like
> JFFS2.)
>
As mentioned above [PATCH 1/2] which touches 'ecclayout->oobfree->offset'
is a clean-up not affecting regression, but it should be taken in before
any further feature or ecc-scheme changes.
Now, what is your preference?
(a) Take [PATCH 1/2] ecc.layout clean-up separately.
(b) Take ecc.layout as part of this series, but may be with different patch title.
> * What Linux commit caused the regression?
> * Should this patch set be marked for -stable? And for what kernel
> release? (the previous question would help answer this)
>
This regression was caused in
commit b491da7233d5dc1a24d46ca1ad0209900329c5d0
mtd: nand: omap: clean-up ecc layout for BCH ecc schemes
So, this should be applicable from 3.13.x+
>> Hence this patch series fixes those regressions, and tests complete
>[...] snip
>
>I'm preparing a 3.14 pull request soon, and since you seem committed to
>fixing and properly testing a known regression here, I'd like to see
>this go in. But given the late timing and the unanswered questions, I
>think it's unlikely to go in -rc1. Perhaps I can send a later pull
>request if we can get it together properly.
>
Yes, if the above details are sufficient, then I'll:
(1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
(2) Add above description and details of regression into commit log of [PATCH 2/2]
(3) Include your other comments on individual patches.
>So I'd like to first of all get a few answers to my questions, and then
>I think we might want to refresh this patch series with a little better
>commit messages and perhaps a separate documentation file (not
>necessarily in the same patch series, as the fix is more urgent).
>
Yes, I plan to get some documentation in kernel docs. But for now
I do have a 'un-polished' page for OMAP NAND driver on TI wiki [3].
Please let me know if you would like something similar for kernel docs.
I'm happy that you are scrutinizing each patch and their commit logs wisely.
Thanks much for that. This helps me and probably everyone else to tighten-up
so that only good quality patch-sets go in. This eventually will make the
sub-system and the drivers more stable for long-term.
Also, if you don't mind, I'll still continue to cross-post this particular
Patch-series on both u-boot and kernel mailing lists just to maintain
continuity, as I know there are lot of OMAP3 users which are following
only one of the two lists.. (but I'm cutting down on the CC list).
I'll avoid cross posting in future.
>Brian
>
> [1] For instance, rather than saying "the ECC layout doesn't match
> U-Boot", you should probably describe what the intended ECC layout
> should look like and show that Linux does not match it. Perhaps it's
> time for some better documentation, like Ezequiel stashed under
> Documentation/mtd/nand/ for pxa3xx. You could also take some cues
> from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
[2] http://lists.denx.de/pipermail/u-boot/2013-December/168440.html
[3] https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide (under construction)
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-01-28 7:42 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-01-28 7:42 UTC (permalink / raw)
To: u-boot
Hi Brian,
>From: Brian Norris
>
>Hi Pekon,
>
>Sorry, I'm revisiting your patch series a bit late. There are a few
>factors that contributed to this, though.
>
>1. This patch series talks extensively about U-Boot. U-Boot is not my
> interest, nor should it be the focus of kernel (driver) development.
> Any work done here should be framed in the kernel driver context. [1]
>
>2. You have previously CC'd me on U-Boot patches. Or at least, I think
> so. More about this in the next point...
>
>3. The U-Boot source code structure is rather similar to pieces of Linux
> subsystems. So without additional effort, it is hard to discern
> whether a patch is intended for Linux MTD or U-Boot MTD.
>
>4. You cross-posted to both the U-Boot and Linux-MTD mailing lists.
>
>So the sum of all this is that I ignored these patches at first, as they
>weren't clearly intended for me.
>
>On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
>> As there were parallel set of patches running between u-boot and kernel.
>
>I don't know what patches you're talking about.
>
Following patch-sets touched ecc.layout while cleaning up OMAP NAND driver.
u-boot: http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
kernel: http://lists.infradead.org/pipermail/linux-mtd/2013-October/049414.html
>> hence, some patch-sets caused regression for OMAP3x platforms when booting
>
>"Hence" is not the right word. The previous sentence doesn't imply that
>there were regressions.
>
>> using u-boot specifically for ecc-schemes (like BCH4_SW).
>
>Huh? What does "specifically for ecc-schemes" mean? You mean that only
>some schemes had regressions? Can you be more specific? What are these
>regressions?
>
It was reported by Enric Balletbo Serra <eballetbo@iseebcn.com> [2] that
when using 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' ecc-scheme
there was a mismatch in ECC layout between mainline u-boot and kernel.
This caused boot failure when kernel tried to mount UBIFS with image
flashed from u-boot.
This was missed while testing earlier patch-sets because only OMAP3 boards
use above ecc-scheme as OMAP3 SoC do not have ELM hardware engine.
All the later platforms use 'OMAP_ECC_BCH8_CODE_HW' ecc-scheme.
Both ecc-scheme are based on same algorithm of BCH8 ECC code except that:
(1) 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' uses /lib/bch.c software
library for ecc-correction.
(2) 'OMAP_ECC_BCH8_CODE_HW' uses ELM hardware engine for ecc-correction.
This regression affects following ecc-schemes, as ecc.layout logic is just
copy-paste of each-other.
- 'OMAP_ECC_BCH4_CODE_HW_DETECTION_SW' and
- 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW'
>The rest of this cover letter (and most of the patch descriptions)
>describes the configurations and testing (which is all very good, don't
>get me wrong), with a lot of focus on things I don't follow (namely,
>U-Boot), but you omit many of the important details, like
>
> * What is the regression? I honestly don't see a description of the
> actual regression. (I can read the code to intuit that, but what's
> the point of your pages of description if it doesn't mention this?)
> Please give some logical description of the problem
>
[Patch 1/2] of this series actually simplification of 'ecclayout->oobfree->offset'
It does not fix the regression, but it's important to clean it, before any new
feature additions.
[Patch 2/2] Actually fixes the regression for affected ecc-schemes. But
"this patch also touches other ecc-schemes as the fix required
refactoring common code, into ecc-scheme specific code."
> * What are the effects of the regression? ECC bytes in the wrong place,
> so you see correction failures/corruption? JFFS2 failures? (The
> "oobfree" changes, for instance, should only affect something like
> JFFS2.)
>
As mentioned above [PATCH 1/2] which touches 'ecclayout->oobfree->offset'
is a clean-up not affecting regression, but it should be taken in before
any further feature or ecc-scheme changes.
Now, what is your preference?
(a) Take [PATCH 1/2] ecc.layout clean-up separately.
(b) Take ecc.layout as part of this series, but may be with different patch title.
> * What Linux commit caused the regression?
> * Should this patch set be marked for -stable? And for what kernel
> release? (the previous question would help answer this)
>
This regression was caused in
commit b491da7233d5dc1a24d46ca1ad0209900329c5d0
mtd: nand: omap: clean-up ecc layout for BCH ecc schemes
So, this should be applicable from 3.13.x+
>> Hence this patch series fixes those regressions, and tests complete
>[...] snip
>
>I'm preparing a 3.14 pull request soon, and since you seem committed to
>fixing and properly testing a known regression here, I'd like to see
>this go in. But given the late timing and the unanswered questions, I
>think it's unlikely to go in -rc1. Perhaps I can send a later pull
>request if we can get it together properly.
>
Yes, if the above details are sufficient, then I'll:
(1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
(2) Add above description and details of regression into commit log of [PATCH 2/2]
(3) Include your other comments on individual patches.
>So I'd like to first of all get a few answers to my questions, and then
>I think we might want to refresh this patch series with a little better
>commit messages and perhaps a separate documentation file (not
>necessarily in the same patch series, as the fix is more urgent).
>
Yes, I plan to get some documentation in kernel docs. But for now
I do have a 'un-polished' page for OMAP NAND driver on TI wiki [3].
Please let me know if you would like something similar for kernel docs.
I'm happy that you are scrutinizing each patch and their commit logs wisely.
Thanks much for that. This helps me and probably everyone else to tighten-up
so that only good quality patch-sets go in. This eventually will make the
sub-system and the drivers more stable for long-term.
Also, if you don't mind, I'll still continue to cross-post this particular
Patch-series on both u-boot and kernel mailing lists just to maintain
continuity, as I know there are lot of OMAP3 users which are following
only one of the two lists.. (but I'm cutting down on the CC list).
I'll avoid cross posting in future.
>Brian
>
> [1] For instance, rather than saying "the ECC layout doesn't match
> U-Boot", you should probably describe what the intended ECC layout
> should look like and show that Linux does not match it. Perhaps it's
> time for some better documentation, like Ezequiel stashed under
> Documentation/mtd/nand/ for pxa3xx. You could also take some cues
> from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
[2] http://lists.denx.de/pipermail/u-boot/2013-December/168440.html
[3] https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide (under construction)
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-28 7:42 ` Gupta, Pekon
(?)
@ 2014-02-04 11:30 ` Gupta, Pekon
-1 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-02-04 11:30 UTC (permalink / raw)
To: 'Brian Norris'
Cc: Artem Bityutskiy, Balbi, Felipe, u-boot, Enric Balletbo Serra,
linux-mtd, linux-omap
Hi Brian,
>From: Gupta, Pekon
>>I'm preparing a 3.14 pull request soon, and since you seem committed to
>>fixing and properly testing a known regression here, I'd like to see
>>this go in. But given the late timing and the unanswered questions, I
>>think it's unlikely to go in -rc1. Perhaps I can send a later pull
>>request if we can get it together properly.
>>
>Yes, if the above details are sufficient, then I'll:
>(1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
>(2) Add above description and details of regression into commit log of [PATCH 2/2]
>(3) Include your other comments on individual patches.
>
>
>>So I'd like to first of all get a few answers to my questions, and then
>>I think we might want to refresh this patch series with a little better
>>commit messages and perhaps a separate documentation file (not
>>necessarily in the same patch series, as the fix is more urgent).
>>
Do my previous responses look complete to you for re-submission ?
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-02-04 11:30 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-02-04 11:30 UTC (permalink / raw)
To: 'Brian Norris'
Cc: Artem Bityutskiy, Balbi, Felipe, u-boot, Enric Balletbo Serra,
linux-mtd, linux-omap
Hi Brian,
>From: Gupta, Pekon
>>I'm preparing a 3.14 pull request soon, and since you seem committed to
>>fixing and properly testing a known regression here, I'd like to see
>>this go in. But given the late timing and the unanswered questions, I
>>think it's unlikely to go in -rc1. Perhaps I can send a later pull
>>request if we can get it together properly.
>>
>Yes, if the above details are sufficient, then I'll:
>(1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
>(2) Add above description and details of regression into commit log of [PATCH 2/2]
>(3) Include your other comments on individual patches.
>
>
>>So I'd like to first of all get a few answers to my questions, and then
>>I think we might want to refresh this patch series with a little better
>>commit messages and perhaps a separate documentation file (not
>>necessarily in the same patch series, as the fix is more urgent).
>>
Do my previous responses look complete to you for re-submission ?
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-02-04 11:30 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-02-04 11:30 UTC (permalink / raw)
To: u-boot
Hi Brian,
>From: Gupta, Pekon
>>I'm preparing a 3.14 pull request soon, and since you seem committed to
>>fixing and properly testing a known regression here, I'd like to see
>>this go in. But given the late timing and the unanswered questions, I
>>think it's unlikely to go in -rc1. Perhaps I can send a later pull
>>request if we can get it together properly.
>>
>Yes, if the above details are sufficient, then I'll:
>(1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
>(2) Add above description and details of regression into commit log of [PATCH 2/2]
>(3) Include your other comments on individual patches.
>
>
>>So I'd like to first of all get a few answers to my questions, and then
>>I think we might want to refresh this patch series with a little better
>>commit messages and perhaps a separate documentation file (not
>>necessarily in the same patch series, as the fix is more urgent).
>>
Do my previous responses look complete to you for re-submission ?
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
2014-01-28 7:42 ` Gupta, Pekon
(?)
@ 2014-02-11 23:40 ` Brian Norris
-1 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-02-11 23:40 UTC (permalink / raw)
To: Gupta, Pekon
Cc: Artem Bityutskiy, u-boot, linux-mtd, linux-omap,
Enric Balletbo Serra, Balbi, Felipe
Hi Pekon,
On Tue, Jan 28, 2014 at 07:42:09AM +0000, Pekon Gupta wrote:
> >From: Brian Norris
> >
> >On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
> >> As there were parallel set of patches running between u-boot and kernel.
> >
> >I don't know what patches you're talking about.
> >
> Following patch-sets touched ecc.layout while cleaning up OMAP NAND driver.
> u-boot: http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
> kernel: http://lists.infradead.org/pipermail/linux-mtd/2013-October/049414.html
>
> >> hence, some patch-sets caused regression for OMAP3x platforms when booting
> >
> >"Hence" is not the right word. The previous sentence doesn't imply that
> >there were regressions.
> >
> >> using u-boot specifically for ecc-schemes (like BCH4_SW).
> >
> >Huh? What does "specifically for ecc-schemes" mean? You mean that only
> >some schemes had regressions? Can you be more specific? What are these
> >regressions?
> >
> It was reported by Enric Balletbo Serra <eballetbo@iseebcn.com> [2] that
> when using 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' ecc-scheme
> there was a mismatch in ECC layout between mainline u-boot and kernel.
> This caused boot failure when kernel tried to mount UBIFS with image
> flashed from u-boot.
>
> This was missed while testing earlier patch-sets because only OMAP3 boards
> use above ecc-scheme as OMAP3 SoC do not have ELM hardware engine.
> All the later platforms use 'OMAP_ECC_BCH8_CODE_HW' ecc-scheme.
> Both ecc-scheme are based on same algorithm of BCH8 ECC code except that:
> (1) 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' uses /lib/bch.c software
> library for ecc-correction.
> (2) 'OMAP_ECC_BCH8_CODE_HW' uses ELM hardware engine for ecc-correction.
>
> This regression affects following ecc-schemes, as ecc.layout logic is just
> copy-paste of each-other.
> - 'OMAP_ECC_BCH4_CODE_HW_DETECTION_SW' and
> - 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW'
OK
> >The rest of this cover letter (and most of the patch descriptions)
> >describes the configurations and testing (which is all very good, don't
> >get me wrong), with a lot of focus on things I don't follow (namely,
> >U-Boot), but you omit many of the important details, like
> >
> > * What is the regression? I honestly don't see a description of the
> > actual regression. (I can read the code to intuit that, but what's
> > the point of your pages of description if it doesn't mention this?)
> > Please give some logical description of the problem
> >
> [Patch 1/2] of this series actually simplification of 'ecclayout->oobfree->offset'
> It does not fix the regression, but it's important to clean it, before any new
> feature additions.
By my reading, it is actually wrong, and therefore not just a clean up.
Can you address my comment there about your indexing into the eccpos[]
array?
Did you actually check (e.g., print out) the resulting ecclayout before
and after your cleanup to make sure it's exactly the same? I'm looking
at the oobfree[] values, which looked wrong to me.
> [Patch 2/2] Actually fixes the regression for affected ecc-schemes. But
> "this patch also touches other ecc-schemes as the fix required
> refactoring common code, into ecc-scheme specific code."
>
>
> > * What are the effects of the regression? ECC bytes in the wrong place,
> > so you see correction failures/corruption? JFFS2 failures? (The
> > "oobfree" changes, for instance, should only affect something like
> > JFFS2.)
> >
> As mentioned above [PATCH 1/2] which touches 'ecclayout->oobfree->offset'
> is a clean-up not affecting regression, but it should be taken in before
> any further feature or ecc-scheme changes.
That's fine; a fixup patch can come just before a bugfix, if that helps
simplify the bufix.
> Now, what is your preference?
> (a) Take [PATCH 1/2] ecc.layout clean-up separately.
> (b) Take ecc.layout as part of this series, but may be with different patch title.
I think we'll keep these patches together, but they both need to have
better descriptions to tell what's actually going on.
> > * What Linux commit caused the regression?
> > * Should this patch set be marked for -stable? And for what kernel
> > release? (the previous question would help answer this)
> >
> This regression was caused in
> commit b491da7233d5dc1a24d46ca1ad0209900329c5d0
> mtd: nand: omap: clean-up ecc layout for BCH ecc schemes
> So, this should be applicable from 3.13.x+
>
>
> >> Hence this patch series fixes those regressions, and tests complete
> >[...] snip
> >
> >I'm preparing a 3.14 pull request soon, and since you seem committed to
> >fixing and properly testing a known regression here, I'd like to see
> >this go in. But given the late timing and the unanswered questions, I
> >think it's unlikely to go in -rc1. Perhaps I can send a later pull
> >request if we can get it together properly.
> >
> Yes, if the above details are sufficient, then I'll:
> (1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
> (2) Add above description and details of regression into commit log of [PATCH 2/2]
> (3) Include your other comments on individual patches.
That's a good plan. But please, before sending another version, can you
at least address my comment on patch 1 about the eccpos[] indexing you
are using? It doesn't look right to me.
Also, can you please print out your full layout (eccpos, eccbytes,
oobavail, and oobfree) and check it?
> >So I'd like to first of all get a few answers to my questions, and then
> >I think we might want to refresh this patch series with a little better
> >commit messages and perhaps a separate documentation file (not
> >necessarily in the same patch series, as the fix is more urgent).
> >
> Yes, I plan to get some documentation in kernel docs. But for now
> I do have a 'un-polished' page for OMAP NAND driver on TI wiki [3].
> Please let me know if you would like something similar for kernel docs.
Thanks for the link. Some portion of that doc is probably useful to
include in Linux eventually.
> I'm happy that you are scrutinizing each patch and their commit logs wisely.
> Thanks much for that. This helps me and probably everyone else to tighten-up
> so that only good quality patch-sets go in. This eventually will make the
> sub-system and the drivers more stable for long-term.
>
> Also, if you don't mind, I'll still continue to cross-post this particular
> Patch-series on both u-boot and kernel mailing lists just to maintain
> continuity, as I know there are lot of OMAP3 users which are following
> only one of the two lists.. (but I'm cutting down on the CC list).
> I'll avoid cross posting in future.
Cross-posting is OK with me if it's made a little more clear who is
targeted. The restrictive U-Boot mailing list policy (which rejects
non-member / too-large-CC-list email) diminishes the usefulness of
cross-posting too.
But most of all, using "u-boot" in the subject of the cover letter email
does not clearly convey that this is a Linux patch!
> >Brian
> >
> > [1] For instance, rather than saying "the ECC layout doesn't match
> > U-Boot", you should probably describe what the intended ECC layout
> > should look like and show that Linux does not match it. Perhaps it's
> > time for some better documentation, like Ezequiel stashed under
> > Documentation/mtd/nand/ for pxa3xx. You could also take some cues
> > from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
>
>
> [2] http://lists.denx.de/pipermail/u-boot/2013-December/168440.html
> [3] https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide (under construction)
I look forward to you addressing my last concerns and sending a good v2.
Thanks!
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-02-11 23:40 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-02-11 23:40 UTC (permalink / raw)
To: Gupta, Pekon
Cc: Artem Bityutskiy, Balbi, Felipe, u-boot, Enric Balletbo Serra,
linux-mtd, linux-omap
Hi Pekon,
On Tue, Jan 28, 2014 at 07:42:09AM +0000, Pekon Gupta wrote:
> >From: Brian Norris
> >
> >On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
> >> As there were parallel set of patches running between u-boot and kernel.
> >
> >I don't know what patches you're talking about.
> >
> Following patch-sets touched ecc.layout while cleaning up OMAP NAND driver.
> u-boot: http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
> kernel: http://lists.infradead.org/pipermail/linux-mtd/2013-October/049414.html
>
> >> hence, some patch-sets caused regression for OMAP3x platforms when booting
> >
> >"Hence" is not the right word. The previous sentence doesn't imply that
> >there were regressions.
> >
> >> using u-boot specifically for ecc-schemes (like BCH4_SW).
> >
> >Huh? What does "specifically for ecc-schemes" mean? You mean that only
> >some schemes had regressions? Can you be more specific? What are these
> >regressions?
> >
> It was reported by Enric Balletbo Serra <eballetbo@iseebcn.com> [2] that
> when using 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' ecc-scheme
> there was a mismatch in ECC layout between mainline u-boot and kernel.
> This caused boot failure when kernel tried to mount UBIFS with image
> flashed from u-boot.
>
> This was missed while testing earlier patch-sets because only OMAP3 boards
> use above ecc-scheme as OMAP3 SoC do not have ELM hardware engine.
> All the later platforms use 'OMAP_ECC_BCH8_CODE_HW' ecc-scheme.
> Both ecc-scheme are based on same algorithm of BCH8 ECC code except that:
> (1) 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' uses /lib/bch.c software
> library for ecc-correction.
> (2) 'OMAP_ECC_BCH8_CODE_HW' uses ELM hardware engine for ecc-correction.
>
> This regression affects following ecc-schemes, as ecc.layout logic is just
> copy-paste of each-other.
> - 'OMAP_ECC_BCH4_CODE_HW_DETECTION_SW' and
> - 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW'
OK
> >The rest of this cover letter (and most of the patch descriptions)
> >describes the configurations and testing (which is all very good, don't
> >get me wrong), with a lot of focus on things I don't follow (namely,
> >U-Boot), but you omit many of the important details, like
> >
> > * What is the regression? I honestly don't see a description of the
> > actual regression. (I can read the code to intuit that, but what's
> > the point of your pages of description if it doesn't mention this?)
> > Please give some logical description of the problem
> >
> [Patch 1/2] of this series actually simplification of 'ecclayout->oobfree->offset'
> It does not fix the regression, but it's important to clean it, before any new
> feature additions.
By my reading, it is actually wrong, and therefore not just a clean up.
Can you address my comment there about your indexing into the eccpos[]
array?
Did you actually check (e.g., print out) the resulting ecclayout before
and after your cleanup to make sure it's exactly the same? I'm looking
at the oobfree[] values, which looked wrong to me.
> [Patch 2/2] Actually fixes the regression for affected ecc-schemes. But
> "this patch also touches other ecc-schemes as the fix required
> refactoring common code, into ecc-scheme specific code."
>
>
> > * What are the effects of the regression? ECC bytes in the wrong place,
> > so you see correction failures/corruption? JFFS2 failures? (The
> > "oobfree" changes, for instance, should only affect something like
> > JFFS2.)
> >
> As mentioned above [PATCH 1/2] which touches 'ecclayout->oobfree->offset'
> is a clean-up not affecting regression, but it should be taken in before
> any further feature or ecc-scheme changes.
That's fine; a fixup patch can come just before a bugfix, if that helps
simplify the bufix.
> Now, what is your preference?
> (a) Take [PATCH 1/2] ecc.layout clean-up separately.
> (b) Take ecc.layout as part of this series, but may be with different patch title.
I think we'll keep these patches together, but they both need to have
better descriptions to tell what's actually going on.
> > * What Linux commit caused the regression?
> > * Should this patch set be marked for -stable? And for what kernel
> > release? (the previous question would help answer this)
> >
> This regression was caused in
> commit b491da7233d5dc1a24d46ca1ad0209900329c5d0
> mtd: nand: omap: clean-up ecc layout for BCH ecc schemes
> So, this should be applicable from 3.13.x+
>
>
> >> Hence this patch series fixes those regressions, and tests complete
> >[...] snip
> >
> >I'm preparing a 3.14 pull request soon, and since you seem committed to
> >fixing and properly testing a known regression here, I'd like to see
> >this go in. But given the late timing and the unanswered questions, I
> >think it's unlikely to go in -rc1. Perhaps I can send a later pull
> >request if we can get it together properly.
> >
> Yes, if the above details are sufficient, then I'll:
> (1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
> (2) Add above description and details of regression into commit log of [PATCH 2/2]
> (3) Include your other comments on individual patches.
That's a good plan. But please, before sending another version, can you
at least address my comment on patch 1 about the eccpos[] indexing you
are using? It doesn't look right to me.
Also, can you please print out your full layout (eccpos, eccbytes,
oobavail, and oobfree) and check it?
> >So I'd like to first of all get a few answers to my questions, and then
> >I think we might want to refresh this patch series with a little better
> >commit messages and perhaps a separate documentation file (not
> >necessarily in the same patch series, as the fix is more urgent).
> >
> Yes, I plan to get some documentation in kernel docs. But for now
> I do have a 'un-polished' page for OMAP NAND driver on TI wiki [3].
> Please let me know if you would like something similar for kernel docs.
Thanks for the link. Some portion of that doc is probably useful to
include in Linux eventually.
> I'm happy that you are scrutinizing each patch and their commit logs wisely.
> Thanks much for that. This helps me and probably everyone else to tighten-up
> so that only good quality patch-sets go in. This eventually will make the
> sub-system and the drivers more stable for long-term.
>
> Also, if you don't mind, I'll still continue to cross-post this particular
> Patch-series on both u-boot and kernel mailing lists just to maintain
> continuity, as I know there are lot of OMAP3 users which are following
> only one of the two lists.. (but I'm cutting down on the CC list).
> I'll avoid cross posting in future.
Cross-posting is OK with me if it's made a little more clear who is
targeted. The restrictive U-Boot mailing list policy (which rejects
non-member / too-large-CC-list email) diminishes the usefulness of
cross-posting too.
But most of all, using "u-boot" in the subject of the cover letter email
does not clearly convey that this is a Linux patch!
> >Brian
> >
> > [1] For instance, rather than saying "the ECC layout doesn't match
> > U-Boot", you should probably describe what the intended ECC layout
> > should look like and show that Linux does not match it. Perhaps it's
> > time for some better documentation, like Ezequiel stashed under
> > Documentation/mtd/nand/ for pxa3xx. You could also take some cues
> > from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
>
>
> [2] http://lists.denx.de/pipermail/u-boot/2013-December/168440.html
> [3] https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide (under construction)
I look forward to you addressing my last concerns and sending a good v2.
Thanks!
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot
@ 2014-02-11 23:40 ` Brian Norris
0 siblings, 0 replies; 45+ messages in thread
From: Brian Norris @ 2014-02-11 23:40 UTC (permalink / raw)
To: u-boot
Hi Pekon,
On Tue, Jan 28, 2014 at 07:42:09AM +0000, Pekon Gupta wrote:
> >From: Brian Norris
> >
> >On Fri, Dec 13, 2013 at 02:42:56PM +0530, Pekon Gupta wrote:
> >> As there were parallel set of patches running between u-boot and kernel.
> >
> >I don't know what patches you're talking about.
> >
> Following patch-sets touched ecc.layout while cleaning up OMAP NAND driver.
> u-boot: http://lists.denx.de/pipermail/u-boot/2013-November/167551.html
> kernel: http://lists.infradead.org/pipermail/linux-mtd/2013-October/049414.html
>
> >> hence, some patch-sets caused regression for OMAP3x platforms when booting
> >
> >"Hence" is not the right word. The previous sentence doesn't imply that
> >there were regressions.
> >
> >> using u-boot specifically for ecc-schemes (like BCH4_SW).
> >
> >Huh? What does "specifically for ecc-schemes" mean? You mean that only
> >some schemes had regressions? Can you be more specific? What are these
> >regressions?
> >
> It was reported by Enric Balletbo Serra <eballetbo@iseebcn.com> [2] that
> when using 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' ecc-scheme
> there was a mismatch in ECC layout between mainline u-boot and kernel.
> This caused boot failure when kernel tried to mount UBIFS with image
> flashed from u-boot.
>
> This was missed while testing earlier patch-sets because only OMAP3 boards
> use above ecc-scheme as OMAP3 SoC do not have ELM hardware engine.
> All the later platforms use 'OMAP_ECC_BCH8_CODE_HW' ecc-scheme.
> Both ecc-scheme are based on same algorithm of BCH8 ECC code except that:
> (1) 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW' uses /lib/bch.c software
> library for ecc-correction.
> (2) 'OMAP_ECC_BCH8_CODE_HW' uses ELM hardware engine for ecc-correction.
>
> This regression affects following ecc-schemes, as ecc.layout logic is just
> copy-paste of each-other.
> - 'OMAP_ECC_BCH4_CODE_HW_DETECTION_SW' and
> - 'OMAP_ECC_BCH8_CODE_HW_DETECTION_SW'
OK
> >The rest of this cover letter (and most of the patch descriptions)
> >describes the configurations and testing (which is all very good, don't
> >get me wrong), with a lot of focus on things I don't follow (namely,
> >U-Boot), but you omit many of the important details, like
> >
> > * What is the regression? I honestly don't see a description of the
> > actual regression. (I can read the code to intuit that, but what's
> > the point of your pages of description if it doesn't mention this?)
> > Please give some logical description of the problem
> >
> [Patch 1/2] of this series actually simplification of 'ecclayout->oobfree->offset'
> It does not fix the regression, but it's important to clean it, before any new
> feature additions.
By my reading, it is actually wrong, and therefore not just a clean up.
Can you address my comment there about your indexing into the eccpos[]
array?
Did you actually check (e.g., print out) the resulting ecclayout before
and after your cleanup to make sure it's exactly the same? I'm looking
at the oobfree[] values, which looked wrong to me.
> [Patch 2/2] Actually fixes the regression for affected ecc-schemes. But
> "this patch also touches other ecc-schemes as the fix required
> refactoring common code, into ecc-scheme specific code."
>
>
> > * What are the effects of the regression? ECC bytes in the wrong place,
> > so you see correction failures/corruption? JFFS2 failures? (The
> > "oobfree" changes, for instance, should only affect something like
> > JFFS2.)
> >
> As mentioned above [PATCH 1/2] which touches 'ecclayout->oobfree->offset'
> is a clean-up not affecting regression, but it should be taken in before
> any further feature or ecc-scheme changes.
That's fine; a fixup patch can come just before a bugfix, if that helps
simplify the bufix.
> Now, what is your preference?
> (a) Take [PATCH 1/2] ecc.layout clean-up separately.
> (b) Take ecc.layout as part of this series, but may be with different patch title.
I think we'll keep these patches together, but they both need to have
better descriptions to tell what's actually going on.
> > * What Linux commit caused the regression?
> > * Should this patch set be marked for -stable? And for what kernel
> > release? (the previous question would help answer this)
> >
> This regression was caused in
> commit b491da7233d5dc1a24d46ca1ad0209900329c5d0
> mtd: nand: omap: clean-up ecc layout for BCH ecc schemes
> So, this should be applicable from 3.13.x+
>
>
> >> Hence this patch series fixes those regressions, and tests complete
> >[...] snip
> >
> >I'm preparing a 3.14 pull request soon, and since you seem committed to
> >fixing and properly testing a known regression here, I'd like to see
> >this go in. But given the late timing and the unanswered questions, I
> >think it's unlikely to go in -rc1. Perhaps I can send a later pull
> >request if we can get it together properly.
> >
> Yes, if the above details are sufficient, then I'll:
> (1) re-word [PATCH 1/2] commit log & title to mention that it's a clean-up
> (2) Add above description and details of regression into commit log of [PATCH 2/2]
> (3) Include your other comments on individual patches.
That's a good plan. But please, before sending another version, can you
at least address my comment on patch 1 about the eccpos[] indexing you
are using? It doesn't look right to me.
Also, can you please print out your full layout (eccpos, eccbytes,
oobavail, and oobfree) and check it?
> >So I'd like to first of all get a few answers to my questions, and then
> >I think we might want to refresh this patch series with a little better
> >commit messages and perhaps a separate documentation file (not
> >necessarily in the same patch series, as the fix is more urgent).
> >
> Yes, I plan to get some documentation in kernel docs. But for now
> I do have a 'un-polished' page for OMAP NAND driver on TI wiki [3].
> Please let me know if you would like something similar for kernel docs.
Thanks for the link. Some portion of that doc is probably useful to
include in Linux eventually.
> I'm happy that you are scrutinizing each patch and their commit logs wisely.
> Thanks much for that. This helps me and probably everyone else to tighten-up
> so that only good quality patch-sets go in. This eventually will make the
> sub-system and the drivers more stable for long-term.
>
> Also, if you don't mind, I'll still continue to cross-post this particular
> Patch-series on both u-boot and kernel mailing lists just to maintain
> continuity, as I know there are lot of OMAP3 users which are following
> only one of the two lists.. (but I'm cutting down on the CC list).
> I'll avoid cross posting in future.
Cross-posting is OK with me if it's made a little more clear who is
targeted. The restrictive U-Boot mailing list policy (which rejects
non-member / too-large-CC-list email) diminishes the usefulness of
cross-posting too.
But most of all, using "u-boot" in the subject of the cover letter email
does not clearly convey that this is a Linux patch!
> >Brian
> >
> > [1] For instance, rather than saying "the ECC layout doesn't match
> > U-Boot", you should probably describe what the intended ECC layout
> > should look like and show that Linux does not match it. Perhaps it's
> > time for some better documentation, like Ezequiel stashed under
> > Documentation/mtd/nand/ for pxa3xx. You could also take some cues
> > from Huang's comments on ECC layouts, etc., in gpmi-nand.c.
>
>
> [2] http://lists.denx.de/pipermail/u-boot/2013-December/168440.html
> [3] https://processors.wiki.ti.com/index.php/Linux_Core_NAND_User%27s_Guide (under construction)
I look forward to you addressing my last concerns and sending a good v2.
Thanks!
Brian
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
2014-01-25 8:56 ` Brian Norris
(?)
@ 2014-02-12 11:33 ` Gupta, Pekon
-1 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-02-12 11:33 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, u-boot, Artem Bityutskiy, Balbi, Felipe, Rini, Tom,
linux-mtd, Ezequiel Garcia, Scott Wood, linux-omap
Hi Brian,
>From: Brian Norris [mailto:computersforpeace@gmail.com]
>>On Fri, Dec 13, 2013 at 02:42:57PM +0530, Pekon Gupta wrote:
>> This patch updates starting offset for free bytes in OOB which can be used by
>> file-systems to store their metadata (like clean-marker in case of JFFS2).
>
>This should be describing a regression fix, right? We don't just
>arbitrarily change the "OOB free" layout; we need a reason. So please
>describe it here.
>
>(It seems like an off-by-one, or off-by-<N> error, where the oobfree
>region was miscalculated.)
>
>Possibly you can paste an example intended ecclayout as well as an
>incorrect layout that was calculated before this fix.
>
>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>> ---
>> drivers/mtd/nand/omap2.c | 17 ++++-------------
>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
[...]
>>
>> - /* 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 */
>> @@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
>> err = -EINVAL;
>> goto return_error;
>> }
>> + /* populate remaining ECC layout data */
>> + ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
>
>Hmm, this line seems a little odd. The ecclayout->eccpos[] array and the
>value of ecclayout->eccbytes sould be related as follows:
>
> let N = ecclayout->eccbytes
>
> This means the eccpos[] array should have N entries, indexed 0 to N-1,
> and eccpos[N] is out of bounds. But you are accessing eccpos[N] above
> (i.e., eccpos[ecclayout->eccbytes]). Are you sure this is correct? It
> seems like it should be:
>
> ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
>
Thanks for this catch. Yes, you are correct. It's a typo.
This wasn't caught as I had tested everything on UBIFS which does not use 'oobfree'.
Also, as ecclayout->eccpos is defined as large static array, so this dint caused problems either.
#define MTD_MAX_ECCPOS_ENTRIES_LARGE 640
struct nand_ecclayout {
__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
But, I think mtd_tests.nand_oobtest would have caught this. I'll include this change in next version.
<stripping down the CC list to avoid getting moderated by u-boot mailman>
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
@ 2014-02-12 11:33 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-02-12 11:33 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, u-boot, Artem Bityutskiy, Balbi, Felipe, Rini, Tom,
linux-mtd, Ezequiel Garcia, Scott Wood, linux-omap
Hi Brian,
>From: Brian Norris [mailto:computersforpeace@gmail.com]
>>On Fri, Dec 13, 2013 at 02:42:57PM +0530, Pekon Gupta wrote:
>> This patch updates starting offset for free bytes in OOB which can be used by
>> file-systems to store their metadata (like clean-marker in case of JFFS2).
>
>This should be describing a regression fix, right? We don't just
>arbitrarily change the "OOB free" layout; we need a reason. So please
>describe it here.
>
>(It seems like an off-by-one, or off-by-<N> error, where the oobfree
>region was miscalculated.)
>
>Possibly you can paste an example intended ecclayout as well as an
>incorrect layout that was calculated before this fix.
>
>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>> ---
>> drivers/mtd/nand/omap2.c | 17 ++++-------------
>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
[...]
>>
>> - /* 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 */
>> @@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
>> err = -EINVAL;
>> goto return_error;
>> }
>> + /* populate remaining ECC layout data */
>> + ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
>
>Hmm, this line seems a little odd. The ecclayout->eccpos[] array and the
>value of ecclayout->eccbytes sould be related as follows:
>
> let N = ecclayout->eccbytes
>
> This means the eccpos[] array should have N entries, indexed 0 to N-1,
> and eccpos[N] is out of bounds. But you are accessing eccpos[N] above
> (i.e., eccpos[ecclayout->eccbytes]). Are you sure this is correct? It
> seems like it should be:
>
> ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
>
Thanks for this catch. Yes, you are correct. It's a typo.
This wasn't caught as I had tested everything on UBIFS which does not use 'oobfree'.
Also, as ecclayout->eccpos is defined as large static array, so this dint caused problems either.
#define MTD_MAX_ECCPOS_ENTRIES_LARGE 640
struct nand_ecclayout {
__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
But, I think mtd_tests.nand_oobtest would have caught this. I'll include this change in next version.
<stripping down the CC list to avoid getting moderated by u-boot mailman>
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [U-Boot] [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset
@ 2014-02-12 11:33 ` Gupta, Pekon
0 siblings, 0 replies; 45+ messages in thread
From: Gupta, Pekon @ 2014-02-12 11:33 UTC (permalink / raw)
To: u-boot
Hi Brian,
>From: Brian Norris [mailto:computersforpeace at gmail.com]
>>On Fri, Dec 13, 2013 at 02:42:57PM +0530, Pekon Gupta wrote:
>> This patch updates starting offset for free bytes in OOB which can be used by
>> file-systems to store their metadata (like clean-marker in case of JFFS2).
>
>This should be describing a regression fix, right? We don't just
>arbitrarily change the "OOB free" layout; we need a reason. So please
>describe it here.
>
>(It seems like an off-by-one, or off-by-<N> error, where the oobfree
>region was miscalculated.)
>
>Possibly you can paste an example intended ecclayout as well as an
>incorrect layout that was calculated before this fix.
>
>> Signed-off-by: Pekon Gupta <pekon@ti.com>
>> ---
>> drivers/mtd/nand/omap2.c | 17 ++++-------------
>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
[...]
>>
>> - /* 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 */
>> @@ -1990,6 +1977,10 @@ static int omap_nand_probe(struct platform_device *pdev)
>> err = -EINVAL;
>> goto return_error;
>> }
>> + /* populate remaining ECC layout data */
>> + ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes] + 1;
>
>Hmm, this line seems a little odd. The ecclayout->eccpos[] array and the
>value of ecclayout->eccbytes sould be related as follows:
>
> let N = ecclayout->eccbytes
>
> This means the eccpos[] array should have N entries, indexed 0 to N-1,
> and eccpos[N] is out of bounds. But you are accessing eccpos[N] above
> (i.e., eccpos[ecclayout->eccbytes]). Are you sure this is correct? It
> seems like it should be:
>
> ecclayout->oobfree->offset = ecclayout->eccpos[ecclayout->eccbytes - 1] + 1;
>
Thanks for this catch. Yes, you are correct. It's a typo.
This wasn't caught as I had tested everything on UBIFS which does not use 'oobfree'.
Also, as ecclayout->eccpos is defined as large static array, so this dint caused problems either.
#define MTD_MAX_ECCPOS_ENTRIES_LARGE 640
struct nand_ecclayout {
__u32 eccpos[MTD_MAX_ECCPOS_ENTRIES_LARGE];
But, I think mtd_tests.nand_oobtest would have caught this. I'll include this change in next version.
<stripping down the CC list to avoid getting moderated by u-boot mailman>
with regards, pekon
^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2014-02-12 11:34 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-13 9:12 [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot Pekon Gupta
2013-12-13 9:12 ` [U-Boot] " Pekon Gupta
2013-12-13 9:12 ` Pekon Gupta
2013-12-13 9:12 ` [PATCH v1 1/2] mtd: nand: omap: fix ecclayout->oobfree->offset Pekon Gupta
2013-12-13 9:12 ` [U-Boot] " Pekon Gupta
2013-12-13 9:12 ` Pekon Gupta
2014-01-25 8:56 ` Brian Norris
2014-01-25 8:56 ` [U-Boot] " Brian Norris
2014-01-25 8:56 ` Brian Norris
2014-02-12 11:33 ` Gupta, Pekon
2014-02-12 11:33 ` [U-Boot] " Gupta, Pekon
2014-02-12 11:33 ` Gupta, Pekon
2013-12-13 9:12 ` [PATCH v1 2/2] mtd: nand: omap: fix ecclayout to be in sync with u-boot NAND driver Pekon Gupta
2013-12-13 9:12 ` [U-Boot] " Pekon Gupta
2013-12-13 9:12 ` Pekon Gupta
2014-01-25 8:57 ` Brian Norris
2014-01-25 8:57 ` [U-Boot] " Brian Norris
2014-01-25 8:57 ` Brian Norris
2014-01-06 7:40 ` [PATCH v1 0/2] mtd: nand: omap: booting from NAND using u-boot Gupta, Pekon
2014-01-06 7:40 ` [U-Boot] " Gupta, Pekon
2014-01-06 7:40 ` Gupta, Pekon
2014-01-06 11:48 ` Stefan Roese
2014-01-06 11:48 ` [U-Boot] " Stefan Roese
2014-01-06 11:48 ` Stefan Roese
2014-01-14 14:27 ` Enric Balletbo Serra
2014-01-14 14:27 ` Enric Balletbo Serra
2014-01-15 4:01 ` Gupta, Pekon
2014-01-15 4:01 ` [U-Boot] " Gupta, Pekon
2014-01-15 4:01 ` Gupta, Pekon
2014-01-25 8:55 ` Brian Norris
2014-01-25 8:55 ` [U-Boot] " Brian Norris
2014-01-25 8:55 ` Brian Norris
2014-01-27 17:46 ` Gupta, Pekon
2014-01-27 17:46 ` [U-Boot] " Gupta, Pekon
2014-01-27 19:07 ` Brian Norris
2014-01-27 19:07 ` [U-Boot] " Brian Norris
2014-01-28 7:42 ` Gupta, Pekon
2014-01-28 7:42 ` [U-Boot] " Gupta, Pekon
2014-01-28 7:42 ` Gupta, Pekon
2014-02-04 11:30 ` Gupta, Pekon
2014-02-04 11:30 ` [U-Boot] " Gupta, Pekon
2014-02-04 11:30 ` Gupta, Pekon
2014-02-11 23:40 ` Brian Norris
2014-02-11 23:40 ` [U-Boot] " Brian Norris
2014-02-11 23:40 ` Brian Norris
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.