[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