Hi dario On Mon, Nov 27, 2023 at 3:00 PM Leto, Enrico <enrico.l...@siemens.com> wrote: > > Hi, > > Works on my draco thuban AM335x based boards booting from NAND with ECC BCH8 > code. > > Tested-by: Enrico Leto <enrico.l...@siemens.com> > > Thanks > > > > -----Original Message----- > > From: Michael Nazzareno Trimarchi <mich...@amarulasolutions.com> > > Sent: Saturday, November 25, 2023 2:07 PM > > To: Roger Quadros <rog...@kernel.org> > > Cc: dario.binac...@amarulasolutions.com; Schocher, Heiko (EXT) (DENX > > Software Engineering GmbH) <h...@denx.de>; Leto, Enrico (SI BP R&D ZG FW > > CCP) <enrico.l...@siemens.com>; tr...@konsulko.com; prane...@ti.com; > > n...@ti.com; vigne...@ti.com; u-boot@lists.denx.de > > Subject: Re: [PATCH] mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x > > > > Hi Roger > > > > On Sat, Nov 25, 2023 at 12:16 PM Roger Quadros <rog...@kernel.org> > > wrote: > > > > > > 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") > > > > > > 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 <rog...@kernel.org> > > > --- > > > 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..2d2d2c2b6d 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 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++) > > > > > > base-commit: 9e53e45292ee2f1d9d2ccc59914b161bef9b10d7 > > > -- > > > 2.34.1 > > > > > > > Let's wait on some tested-by > >
I'm not a fan of this patch but we need to cover this regression Michael > > Michael -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com