[PATCH] mtd: gpmi: fix the gpmi bch setting unalignment issue
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@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);
Hi Han,
On Mon, Mar 21, 2022 at 03:18:34PM -0500, Han Xu 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
AFAIK there is no merged changes in the kernel regarding this. The breaking change was done in u-boot.
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.
Cool, then remember to remove the non used "fsl,legacy-bch-geometry" option.
Fixes: 51cdf83eea mtd: gpmi: provide the option to use legacy bch geometry Signed-off-by: Han Xu han.xu@nxp.com
I will have time to test this on thursday...
/Sean
On Mon, Mar 21, 2022 at 1:37 PM Han Xu han.xu@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@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 http://lists.infradead.org/mailman/listinfo/linux-mtd/
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@gateworks.com
Best regards,
Tim
On 22/03/22 12:34PM, Tim Harvey wrote:
On Mon, Mar 21, 2022 at 1:37 PM Han Xu han.xu@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@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.infra...
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@gateworks.com
Thanks, will send patch v2 with changes.
Best regards,
Tim
participants (3)
-
Han Xu -
Sean Nyekjaer -
Tim Harvey