[PATCH] mtd: gpmi: fix the gpmi bch setting unalignment issue
Han Xu
han.xu at nxp.com
Wed Mar 23 22:16:23 CET 2022
On 22/03/22 12:34PM, Tim Harvey wrote:
> On Mon, Mar 21, 2022 at 1:37 PM Han Xu <han.xu at nxp.com> wrote:
> >
> > The code change fixed the kernel gpmi bch setting not aligned with
> > u-boot settings issue. It still uses the legacy bch geometry as the
> > default option. If users need to choose the minimum ecc strength that
> > NAND chips required, it can be enabled by either adding DT flag
> > "fsl,use-minimum-ecc" or CONFIG_NAND_MXS_USE_MINIMUM_ECC in configs.
> >
> > Fixes: 51cdf83eea mtd: gpmi: provide the option to use legacy bch geometry
> > Signed-off-by: Han Xu <han.xu at nxp.com>
> > ---
> > drivers/mtd/nand/raw/mxs_nand.c | 71 ++++++++++++++++++++++++++++-----
> > 1 file changed, 62 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/mxs_nand.c b/drivers/mtd/nand/raw/mxs_nand.c
> > index 748056a43e..ee5d7fde9c 100644
> > --- a/drivers/mtd/nand/raw/mxs_nand.c
> > +++ b/drivers/mtd/nand/raw/mxs_nand.c
> > @@ -195,6 +195,7 @@ static inline int mxs_nand_legacy_calc_ecc_layout(struct bch_geometry *geo,
> > struct nand_chip *chip = mtd_to_nand(mtd);
> > struct mxs_nand_info *nand_info = nand_get_controller_data(chip);
> > unsigned int block_mark_bit_offset;
> > + int corr, ds_corr;
> >
> > /* The default for the length of Galois Field. */
> > geo->gf_len = 13;
> > @@ -225,6 +226,17 @@ static inline int mxs_nand_legacy_calc_ecc_layout(struct bch_geometry *geo,
> > geo->ecc_strength = min(round_down(geo->ecc_strength, 2),
> > nand_info->max_ecc_strength_supported);
> >
> > + /* check ecc strength, same as nand_ecc_is_strong_enough() did*/
> > + if (chip->ecc_step_ds) {
> > + corr = mtd->writesize * geo->ecc_strength /
> > + geo->ecc_chunkn_size;
> > + ds_corr = mtd->writesize * chip->ecc_strength_ds /
> > + chip->ecc_step_ds;
> > + if (corr < ds_corr ||
> > + geo->ecc_strength < chip->ecc_strength_ds)
> > + return -EINVAL;
> > + }
> > +
> > block_mark_bit_offset = mtd->writesize * 8 -
> > (geo->ecc_strength * geo->gf_len * (geo->ecc_chunk_count - 1)
> > + MXS_NAND_METADATA_SIZE * 8);
> > @@ -1111,6 +1123,7 @@ static int mxs_nand_set_geometry(struct mtd_info *mtd, struct bch_geometry *geo)
> > struct nand_chip *chip = mtd_to_nand(mtd);
> > struct nand_chip *nand = mtd_to_nand(mtd);
> > struct mxs_nand_info *nand_info = nand_get_controller_data(nand);
> > + int err;
> >
> > if (chip->ecc_strength_ds > nand_info->max_ecc_strength_supported) {
> > printf("unsupported NAND chip, minimum ecc required %d\n"
> > @@ -1118,19 +1131,57 @@ static int mxs_nand_set_geometry(struct mtd_info *mtd, struct bch_geometry *geo)
> > return -EINVAL;
> > }
> >
> > - if ((!(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0) &&
> > - mtd->oobsize < 1024) || nand_info->legacy_bch_geometry) {
> > - dev_warn(mtd->dev, "use legacy bch geometry\n");
> > - return mxs_nand_legacy_calc_ecc_layout(geo, mtd);
> > + /* use the legacy bch setting by default */
> > + if ((!nand_info->use_minimum_ecc && mtd->oobsize < 1024) ||
> > + !(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0)) {
> > + dev_dbg(mtd->dev, "use legacy bch geometry\n");
> > + err = mxs_nand_legacy_calc_ecc_layout(geo, mtd);
> > + if (!err)
> > + return 0;
> > }
> >
> > - if (mtd->oobsize > 1024 || chip->ecc_step_ds < mtd->oobsize)
> > - return mxs_nand_calc_ecc_for_large_oob(geo, mtd);
> > + /* for large oob nand */
> > + if (mtd->oobsize > 1024) {
> > + dev_dbg(mtd->dev, "use large oob bch geometry\n");
> > + err = mxs_nand_calc_ecc_for_large_oob(geo, mtd);
> > + if (!err)
> > + return 0;
> > + }
> >
> > - return mxs_nand_calc_ecc_layout_by_info(geo, mtd,
> > - chip->ecc_strength_ds, chip->ecc_step_ds);
> > + /* otherwise use the minimum ecc nand chips required */
> > + dev_dbg(mtd->dev, "use minimum ecc bch geometry\n");
> > + err = mxs_nand_calc_ecc_layout_by_info(geo, mtd, chip->ecc_strength_ds,
> > + chip->ecc_step_ds);
> >
> > - return 0;
> > + if (err)
> > + dev_err(mtd->dev, "none of the bch geometry setting works\n");
> > +
> > + return err;
> > +}
> > +
> > +void mxs_nand_dump_geo(struct mtd_info *mtd)
> > +{
> > + struct nand_chip *nand = mtd_to_nand(mtd);
> > + struct mxs_nand_info *nand_info = nand_get_controller_data(nand);
> > + struct bch_geometry *geo = &nand_info->bch_geometry;
> > +
> > + dev_dbg(mtd->dev, "BCH Geometry :\n"
> > + "GF Length\t\t: %u\n"
> > + "ECC Strength\t\t: %u\n"
> > + "ECC for Meta\t\t: %u\n"
> > + "ECC Chunk0 Size\t\t: %u\n"
> > + "ECC Chunkn Size\t\t: %u\n"
> > + "ECC Chunk Count\t\t: %u\n"
> > + "Block Mark Byte Offset\t: %u\n"
> > + "Block Mark Bit Offset\t: %u\n",
> > + geo->gf_len,
> > + geo->ecc_strength,
> > + geo->ecc_for_meta,
> > + geo->ecc_chunk0_size,
> > + geo->ecc_chunkn_size,
> > + geo->ecc_chunk_count,
> > + geo->block_mark_byte_offset,
> > + geo->block_mark_bit_offset);
> > }
> >
> > /*
> > @@ -1159,6 +1210,8 @@ int mxs_nand_setup_ecc(struct mtd_info *mtd)
> > if (ret)
> > return ret;
> >
> > + mxs_nand_dump_geo(mtd);
> > +
> > /* Configure BCH and set NFC geometry */
> > mxs_reset_block(&bch_regs->hw_bch_ctrl_reg);
> >
> > --
> > 2.17.1
> >
> >
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-mtd%2F&data=04%7C01%7Chan.xu%40nxp.com%7Cdd88699d0a5a420924f608da0c3b0c34%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637835744992699148%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=cVwFQPYbDPPcZ5fRRxhD8rBsNcDTEkm1h6mWMQ8wzFo%3D&reserved=0
>
> Han,
>
> I have boards (gwventana_nand_defconfig / boards/gateworks/gw_ventana)
> that support several different NAND chips:
> 2GiB Micron Technology MT29F16G08ADACAH4 (4k page size, 256k erase
> size, 224 oob size)
> 2GiB Cypress S34ML16G202BHI000 (2k page size, 128k erase size, 128B oob size)
> 1GiB Micron MT29F8G08ABACAH4 (4k page size, 156k erase size, 224 oob size)
> 256MiB Micron MT29F2G08ABAEAH4 (2k page size, 128k erase size, 64B oob size)
>
> commit 616f03dabacb ("mtd: gpmi: change the BCH layout setting for
> large oob NAND") did break NAND for these boards by forcing a change
> to this 'modern' geometry that never even existed in the mainline
> kernel
> commit 51cdf83eea ("mtd: gpmi: provide the option to use legacy bch
> geometry") does not help my boards as like Frieder, I don't have
> access in DT at this time to set the flag (if I alter code to force
> legacy mode it fixes it)
>
> This patch does resolve the issue on my boards by using the legacy
> layout by default. I would say that this fixes an issue that pre-dates
> commit ("51cdf83eea mtd: gpmi: provide the option to use legacy bch
> geometry") so I would add:
> Fixes: 616f03dabacb ("mtd: gpmi: change the BCH layout setting for
> large oob NAND")
>
> I agree with Sean's comments that the commit log should not indicate
> anything about the kernel as there was no change done in the mainline
> kernel and this was simply a breakage of U-Boot and that a follow-up
> patch is needed to remove the unused 'fsl,legacy-bch-geometry'.
>
> Tested-by: Tim Harvey <tharvey at gateworks.com>
Thanks, will send patch v2 with changes.
>
> Best regards,
>
> Tim
More information about the U-Boot
mailing list