[PATCH v3] mtd: gpmi: fix the bch setting backward compatible issue

Fabio Estevam festevam at gmail.com
Thu Mar 24 22:43:29 CET 2022


On Thu, Mar 24, 2022 at 6:39 PM Han Xu <han.xu at nxp.com> wrote:
>
> Previous u-boot code changed the default bch setting behavior and caused
> backward compatible issue. This fix choose the legacy bch geometry back
> again as the default option. If the minimum ecc strength that NAND chips
> required need to be chosen, it can be enabled by either adding DT flag
> "fsl,use-minimum-ecc" or CONFIG_NAND_MXS_USE_MINIMUM_ECC in configs. The
> unused flag "fsl,legacy-bch-geometry" get removed.
>
> Fixes: 51cdf83eea (mtd: gpmi: provide the option to use legacy bch geometry)
> Fixes: 616f03daba (mtd: gpmi: change the BCH layout setting for large
> oob NAND)

Like I said previously, the recommended Fixes tag should contain
12-digit hash followed by ("text"),
so it should be:

Fixes: 51cdf83eea45 ("mtd: gpmi: provide the option to use legacy bch geometry")
Fixes: 616f03dabacb ("mtd: gpmi: change the BCH layout setting for
large oob NAND")

Please don't split the fixes tag in multiples lines.

> Tested-by: Tim Harvey <tharvey at gateworks.com>
> Tested-by: Sean Nyekjaer <sean at geanix.com>
> Signed-off-by: Han Xu <han.xu at nxp.com>
>
> ---
> Changes in v2:
>  - change the commit log about backward compatible issue is in u-boot
>  - removed the unused flag
>  - add fixes and test tag
>
> Changes in v3:
>  - fix the fixes syntax
>  - fix the change log syntax
>  - add more test tag
> ---
>  drivers/mtd/nand/raw/mxs_nand.c    | 71 ++++++++++++++++++++++++++----
>  drivers/mtd/nand/raw/mxs_nand_dt.c |  2 -
>  include/mxs_nand.h                 |  2 -
>  3 files changed, 62 insertions(+), 13 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);
>
> diff --git a/drivers/mtd/nand/raw/mxs_nand_dt.c b/drivers/mtd/nand/raw/mxs_nand_dt.c
> index 878796d555..b9833a646f 100644
> --- a/drivers/mtd/nand/raw/mxs_nand_dt.c
> +++ b/drivers/mtd/nand/raw/mxs_nand_dt.c
> @@ -92,8 +92,6 @@ static int mxs_nand_dt_probe(struct udevice *dev)
>
>         info->use_minimum_ecc = dev_read_bool(dev, "fsl,use-minimum-ecc");
>
> -       info->legacy_bch_geometry = dev_read_bool(dev, "fsl,legacy-bch-geometry");
> -
>         if (IS_ENABLED(CONFIG_CLK) && IS_ENABLED(CONFIG_IMX8)) {
>                 /* Assigned clock already set clock */
>                 struct clk gpmi_clk;
> diff --git a/include/mxs_nand.h b/include/mxs_nand.h
> index 66c909318d..741dc8734e 100644
> --- a/include/mxs_nand.h
> +++ b/include/mxs_nand.h
> @@ -44,8 +44,6 @@ struct mxs_nand_info {
>         struct udevice *dev;
>         unsigned int    max_ecc_strength_supported;
>         bool            use_minimum_ecc;
> -       /* legacy bch geometry flag */
> -       bool            legacy_bch_geometry;
>         int             cur_chip;
>
>         uint32_t        cmd_queue_len;
> --
> 2.17.1
>


More information about the U-Boot mailing list