[PATCH v2 0/2] mtd: nand: omap_gpmc: Fix NAND for AM335x

Hi,
These patches fix NAND and ELM for AM335x and related legacy platforms that use HW BCH and ELM modules.
All CI tests pass: https://github.com/u-boot/u-boot/pull/453
Changelog:
v2: - added __maybe_unused to omap_calculate_ecc_bch. fixes CI tests - Added Tested-by Tags - Explained about omap_elm single sector support in commit log
cheers, -roger
Roger Quadros (2): mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x mtd: rawnand: omap_elm: Fix elm_init definition
drivers/mtd/nand/raw/omap_elm.c | 4 +- drivers/mtd/nand/raw/omap_elm.h | 6 -- drivers/mtd/nand/raw/omap_gpmc.c | 95 ++++++++++---------------------- 3 files changed, 31 insertions(+), 74 deletions(-)
base-commit: 2f0282922b2c458eea7f85c500a948a587437b63

AM335x uses a special driver "am335x_spl_bch.c" as SPL NAND loader. This driver expects 1 sector at a time ECC and doesn't work well with multi-sector ECC that was implemented in commit 04fcd2587321 ("mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction")
Additionally, the omap_elm driver does not support multi sector ECC and will need more work and tests to get multi sector working correctly on all platforms.
Switch back to 1 sector at a time read/ECC.
Fixes: 04fcd2587321 ("mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction") Signed-off-by: Roger Quadros rogerq@kernel.org Tested-by: Enrico Leto enrico.leto@siemens.com Tested-by: Heiko Schocher hs@denx.de --- drivers/mtd/nand/raw/omap_gpmc.c | 95 ++++++++++---------------------- 1 file changed, 29 insertions(+), 66 deletions(-)
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c index 1a5ed0de31..7345fd579b 100644 --- a/drivers/mtd/nand/raw/omap_gpmc.c +++ b/drivers/mtd/nand/raw/omap_gpmc.c @@ -293,7 +293,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, break; case OMAP_ECC_BCH8_CODE_HW: bch_type = 1; - nsectors = chip->ecc.steps; + nsectors = 1; if (mode == NAND_ECC_READ) { wr_mode = BCH_WRAPMODE_1; ecc_size0 = BCH8R_ECC_SIZE0; @@ -306,7 +306,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, break; case OMAP_ECC_BCH16_CODE_HW: bch_type = 0x2; - nsectors = chip->ecc.steps; + nsectors = 1; if (mode == NAND_ECC_READ) { wr_mode = 0x01; ecc_size0 = 52; /* ECC bits in nibbles per sector */ @@ -345,17 +345,16 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info *mtd, }
/** - * _omap_calculate_ecc_bch - Generate BCH ECC bytes for one sector + * omap_calculate_ecc_bch - Generate BCH ECC bytes for one sector * @mtd: MTD device structure * @dat: The pointer to data on which ecc is computed * @ecc_code: The ecc_code buffer - * @sector: The sector number (for a multi sector page) * * Support calculating of BCH4/8/16 ECC vectors for one sector * within a page. Sector number is in @sector. */ -static int _omap_calculate_ecc_bch(struct mtd_info *mtd, const u8 *dat, - u8 *ecc_code, int sector) +static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd, const u8 *dat, + u8 *ecc_code) { struct nand_chip *chip = mtd_to_nand(mtd); struct omap_nand_info *info = nand_get_controller_data(chip); @@ -368,7 +367,7 @@ static int _omap_calculate_ecc_bch(struct mtd_info *mtd, const u8 *dat, case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW: #endif case OMAP_ECC_BCH8_CODE_HW: - ptr = &gpmc_cfg->bch_result_0_3[sector].bch_result_x[3]; + ptr = &gpmc_cfg->bch_result_0_3[0].bch_result_x[3]; val = readl(ptr); ecc_code[i++] = (val >> 0) & 0xFF; ptr--; @@ -383,21 +382,21 @@ static int _omap_calculate_ecc_bch(struct mtd_info *mtd, const u8 *dat,
break; case OMAP_ECC_BCH16_CODE_HW: - val = readl(&gpmc_cfg->bch_result_4_6[sector].bch_result_x[2]); + val = readl(&gpmc_cfg->bch_result_4_6[0].bch_result_x[2]); ecc_code[i++] = (val >> 8) & 0xFF; ecc_code[i++] = (val >> 0) & 0xFF; - val = readl(&gpmc_cfg->bch_result_4_6[sector].bch_result_x[1]); + val = readl(&gpmc_cfg->bch_result_4_6[0].bch_result_x[1]); ecc_code[i++] = (val >> 24) & 0xFF; ecc_code[i++] = (val >> 16) & 0xFF; ecc_code[i++] = (val >> 8) & 0xFF; ecc_code[i++] = (val >> 0) & 0xFF; - val = readl(&gpmc_cfg->bch_result_4_6[sector].bch_result_x[0]); + val = readl(&gpmc_cfg->bch_result_4_6[0].bch_result_x[0]); ecc_code[i++] = (val >> 24) & 0xFF; ecc_code[i++] = (val >> 16) & 0xFF; ecc_code[i++] = (val >> 8) & 0xFF; ecc_code[i++] = (val >> 0) & 0xFF; for (j = 3; j >= 0; j--) { - val = readl(&gpmc_cfg->bch_result_0_3[sector].bch_result_x[j] + val = readl(&gpmc_cfg->bch_result_0_3[0].bch_result_x[j] ); ecc_code[i++] = (val >> 24) & 0xFF; ecc_code[i++] = (val >> 16) & 0xFF; @@ -431,22 +430,6 @@ static int _omap_calculate_ecc_bch(struct mtd_info *mtd, const u8 *dat, return 0; }
-/** - * omap_calculate_ecc_bch - ECC generator for 1 sector - * @mtd: MTD device structure - * @dat: The pointer to data on which ecc is computed - * @ecc_code: The ecc_code buffer - * - * Support calculating of BCH4/8/16 ECC vectors for one sector. This is used - * when SW based correction is required as ECC is required for one sector - * at a time. - */ -static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd, - const u_char *dat, u_char *ecc_calc) -{ - return _omap_calculate_ecc_bch(mtd, dat, ecc_calc, 0); -} - static inline void omap_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len) { struct nand_chip *chip = mtd_to_nand(mtd); @@ -572,34 +555,6 @@ static void omap_nand_read_prefetch(struct mtd_info *mtd, uint8_t *buf, int len)
#ifdef CONFIG_NAND_OMAP_ELM
-/** - * omap_calculate_ecc_bch_multi - Generate ECC for multiple sectors - * @mtd: MTD device structure - * @dat: The pointer to data on which ecc is computed - * @ecc_code: The ecc_code buffer - * - * Support calculating of BCH4/8/16 ecc vectors for the entire page in one go. - */ -static int omap_calculate_ecc_bch_multi(struct mtd_info *mtd, - const u_char *dat, u_char *ecc_calc) -{ - struct nand_chip *chip = mtd_to_nand(mtd); - int eccbytes = chip->ecc.bytes; - unsigned long nsectors; - int i, ret; - - nsectors = ((readl(&gpmc_cfg->ecc_config) >> 4) & 0x7) + 1; - for (i = 0; i < nsectors; i++) { - ret = _omap_calculate_ecc_bch(mtd, dat, ecc_calc, i); - if (ret) - return ret; - - ecc_calc += eccbytes; - } - - return 0; -} - /* * omap_reverse_list - re-orders list elements in reverse order [internal] * @list: pointer to start of list @@ -752,7 +707,6 @@ static int omap_read_page_bch(struct mtd_info *mtd, struct nand_chip *chip, { int i, eccsize = chip->ecc.size; int eccbytes = chip->ecc.bytes; - int ecctotal = chip->ecc.total; int eccsteps = chip->ecc.steps; uint8_t *p = buf; uint8_t *ecc_calc = chip->buffers->ecccalc; @@ -760,24 +714,30 @@ static int omap_read_page_bch(struct mtd_info *mtd, struct nand_chip *chip, uint32_t *eccpos = chip->ecc.layout->eccpos; uint8_t *oob = chip->oob_poi; uint32_t oob_pos; + u32 data_pos = 0;
/* oob area start */ oob_pos = (eccsize * eccsteps) + chip->ecc.layout->eccpos[0]; oob += chip->ecc.layout->eccpos[0];
- /* Enable ECC engine */ - chip->ecc.hwctl(mtd, NAND_ECC_READ); + for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize, + oob += eccbytes) { + /* Enable ECC engine */ + chip->ecc.hwctl(mtd, NAND_ECC_READ);
- /* read entire page */ - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, 0, -1); - chip->read_buf(mtd, buf, mtd->writesize); + /* read data */ + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_pos, -1); + chip->read_buf(mtd, p, eccsize);
- /* read all ecc bytes from oob area */ - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_pos, -1); - chip->read_buf(mtd, oob, ecctotal); + /* read respective ecc from oob area */ + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_pos, -1); + chip->read_buf(mtd, oob, eccbytes); + /* read syndrome */ + chip->ecc.calculate(mtd, p, &ecc_calc[i]);
- /* Calculate ecc bytes */ - omap_calculate_ecc_bch_multi(mtd, buf, ecc_calc); + data_pos += eccsize; + oob_pos += eccbytes; + }
for (i = 0; i < chip->ecc.total; i++) ecc_code[i] = chip->oob_poi[eccpos[i]]; @@ -945,6 +905,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, nand->ecc.hwctl = omap_enable_hwecc_bch; nand->ecc.correct = omap_correct_data_bch_sw; nand->ecc.calculate = omap_calculate_ecc_bch; + nand->ecc.steps = eccsteps; /* define ecc-layout */ ecclayout->eccbytes = nand->ecc.bytes * eccsteps; ecclayout->eccpos[0] = BADBLOCK_MARKER_LENGTH; @@ -987,6 +948,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, nand->ecc.correct = omap_correct_data_bch; nand->ecc.calculate = omap_calculate_ecc_bch; nand->ecc.read_page = omap_read_page_bch; + nand->ecc.steps = eccsteps; /* define ecc-layout */ ecclayout->eccbytes = nand->ecc.bytes * eccsteps; for (i = 0; i < ecclayout->eccbytes; i++) @@ -1020,6 +982,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, nand->ecc.correct = omap_correct_data_bch; nand->ecc.calculate = omap_calculate_ecc_bch; nand->ecc.read_page = omap_read_page_bch; + nand->ecc.steps = eccsteps; /* define ecc-layout */ ecclayout->eccbytes = nand->ecc.bytes * eccsteps; for (i = 0; i < ecclayout->eccbytes; i++)

The macro ELM_BASE is defined in mach/hardware.h and is not visible at the omap_elm.h header file. Avoid using it in omap_elm.h.
Reported-by: Hong Guan hguan@ti.com Fixes: 7363cf0581a3 ("mtd: rawnand: omap_elm: u-boot driver model support") Signed-off-by: Roger Quadros rogerq@kernel.org --- drivers/mtd/nand/raw/omap_elm.c | 4 ++-- drivers/mtd/nand/raw/omap_elm.h | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/raw/omap_elm.c b/drivers/mtd/nand/raw/omap_elm.c index 56a2c39e4f..015ec9bc2d 100644 --- a/drivers/mtd/nand/raw/omap_elm.c +++ b/drivers/mtd/nand/raw/omap_elm.c @@ -185,7 +185,6 @@ void elm_reset(void) ; }
-#ifdef ELM_BASE /** * elm_init - Initialize ELM module * @@ -194,10 +193,11 @@ void elm_reset(void) */ void elm_init(void) { +#ifdef ELM_BASE elm_cfg = (struct elm *)ELM_BASE; elm_reset(); -} #endif +}
#if CONFIG_IS_ENABLED(SYS_NAND_SELF_INIT)
diff --git a/drivers/mtd/nand/raw/omap_elm.h b/drivers/mtd/nand/raw/omap_elm.h index a7f7bacb15..f3db00d55d 100644 --- a/drivers/mtd/nand/raw/omap_elm.h +++ b/drivers/mtd/nand/raw/omap_elm.h @@ -74,12 +74,6 @@ int elm_check_error(u8 *syndrome, enum bch_level bch_type, u32 *error_count, u32 *error_locations); int elm_config(enum bch_level level); void elm_reset(void); -#ifdef ELM_BASE void elm_init(void); -#else -static inline void elm_init(void) -{ -} -#endif #endif /* __ASSEMBLY__ */ #endif /* __ASM_ARCH_ELM_H */

Hi,
On 11/12/2023 13:45, Roger Quadros wrote:
Hi,
These patches fix NAND and ELM for AM335x and related legacy platforms that use HW BCH and ELM modules.
All CI tests pass: https://github.com/u-boot/u-boot/pull/453
Changelog:
v2:
- added __maybe_unused to omap_calculate_ecc_bch. fixes CI tests
- Added Tested-by Tags
- Explained about omap_elm single sector support in commit log
cheers, -roger
Roger Quadros (2): mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x mtd: rawnand: omap_elm: Fix elm_init definition
drivers/mtd/nand/raw/omap_elm.c | 4 +- drivers/mtd/nand/raw/omap_elm.h | 6 -- drivers/mtd/nand/raw/omap_gpmc.c | 95 ++++++++++---------------------- 3 files changed, 31 insertions(+), 74 deletions(-)
base-commit: 2f0282922b2c458eea7f85c500a948a587437b63
If no objections can this be Acked and picked up please? Without these NAND is broken on some TI platforms.
Thanks!

Hi
Il gio 11 gen 2024, 14:06 Roger Quadros rogerq@kernel.org ha scritto:
Hi,
On 11/12/2023 13:45, Roger Quadros wrote:
Hi,
These patches fix NAND and ELM for AM335x and related legacy platforms that use HW BCH and ELM modules.
All CI tests pass: https://github.com/u-boot/u-boot/pull/453
Changelog:
v2:
- added __maybe_unused to omap_calculate_ecc_bch. fixes CI tests
- Added Tested-by Tags
- Explained about omap_elm single sector support in commit log
cheers, -roger
Roger Quadros (2): mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x mtd: rawnand: omap_elm: Fix elm_init definition
drivers/mtd/nand/raw/omap_elm.c | 4 +- drivers/mtd/nand/raw/omap_elm.h | 6 -- drivers/mtd/nand/raw/omap_gpmc.c | 95 ++++++++++---------------------- 3 files changed, 31 insertions(+), 74 deletions(-)
base-commit: 2f0282922b2c458eea7f85c500a948a587437b63
If no objections can this be Acked and picked up please? Without these NAND is broken on some TI platforms.
We will queue them and send over the weekend
Michael
Thanks!
-- cheers, -roger

Hi Roger And Michael,
On Thu, Jan 11, 2024 at 2:13 PM Michael Nazzareno Trimarchi michael@amarulasolutions.com wrote:
Hi
Il gio 11 gen 2024, 14:06 Roger Quadros rogerq@kernel.org ha scritto:
Hi,
On 11/12/2023 13:45, Roger Quadros wrote:
Hi,
These patches fix NAND and ELM for AM335x and related legacy platforms that use HW BCH and ELM modules.
All CI tests pass: https://github.com/u-boot/u-boot/pull/453
Changelog:
v2:
- added __maybe_unused to omap_calculate_ecc_bch. fixes CI tests
- Added Tested-by Tags
- Explained about omap_elm single sector support in commit log
cheers, -roger
Roger Quadros (2): mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x mtd: rawnand: omap_elm: Fix elm_init definition
drivers/mtd/nand/raw/omap_elm.c | 4 +- drivers/mtd/nand/raw/omap_elm.h | 6 -- drivers/mtd/nand/raw/omap_gpmc.c | 95 ++++++++++---------------------- 3 files changed, 31 insertions(+), 74 deletions(-)
base-commit: 2f0282922b2c458eea7f85c500a948a587437b63
If no objections can this be Acked and picked up please? Without these NAND is broken on some TI platforms.
We will queue them and send over the weekend
Michael
Thanks!
-- cheers, -roger
Applied to nand-next.
Thanks and regards, Dario
participants (3)
-
Dario Binacchi
-
Michael Nazzareno Trimarchi
-
Roger Quadros