From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Agner Date: Thu, 21 Jun 2018 13:46:10 +0200 Subject: [U-Boot] [PATCH v3 5/5] mtd: nand: mxs_nand: add minimal ECC support In-Reply-To: <0ab21b8ec4ee2d0711643711ddac0873@agner.ch> References: <20180411160452.2087-1-stefan@agner.ch> <20180411160452.2087-6-stefan@agner.ch> <91682986f9f635d650ebb41c86b0da09@agner.ch> <0ab21b8ec4ee2d0711643711ddac0873@agner.ch> Message-ID: <814aea3bd0e80aff7422f001b268fee3@agner.ch> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Tom, Stefano, Scott Wood seems not to be very active this days in the U-Boot community. This patchsets seem all to be blocked due to that: https://patchwork.ozlabs.org/cover/897263/ https://patchwork.ozlabs.org/cover/901995/ https://patchwork.ozlabs.org/patch/922907/ Those patches have been around quite a while and I tested them well. Any change to get that still in? Best regards, Stefan On 30.05.2018 16:51, Stefan Agner wrote: > Hi Stefano, > > On 30.04.2018 10:08, Stefan Agner wrote: >> On 27.04.2018 09:31, Stefano Babic wrote: >>> Hi Stefan, >>> >>> On 11/04/2018 18:04, Stefan Agner wrote: >>>> From: Stefan Agner >>>> >>>> Add support for minimum ECC strength supported by the NAND chip. >>>> This aligns with the behavior when using the fsl,use-minimum-ecc >>>> device tree property in Linux. >>>> >>>> Signed-off-by: Stefan Agner >>>> --- >>>> >>>> Changes in v3: None >>>> Changes in v2: None >>>> >>>> drivers/mtd/nand/Kconfig | 8 +++++ >>>> drivers/mtd/nand/mxs_nand.c | 71 +++++++++++++++++++++++++++++-------- >>>> 2 files changed, 65 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig >>>> index 4db259fcb2..c039b9cc60 100644 >>>> --- a/drivers/mtd/nand/Kconfig >>>> +++ b/drivers/mtd/nand/Kconfig >>>> @@ -152,6 +152,14 @@ config NAND_MXS >>>> This enables NAND driver for the NAND flash controller on the >>>> MXS processors. >>>> >>>> +if NAND_MXS >>>> + >>>> +config NAND_MXS_USE_MINIMUM_ECC >>>> + bool "Use minimum ECC strength supported by the controller" >>>> + default false >>>> + >>>> +endif >>>> + >>>> config NAND_ZYNQ >>>> bool "Support for Zynq Nand controller" >>>> select SYS_NAND_SELF_INIT >>>> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c >>>> index 2696b543ef..8305bf2302 100644 >>>> --- a/drivers/mtd/nand/mxs_nand.c >>>> +++ b/drivers/mtd/nand/mxs_nand.c >>>> @@ -211,11 +211,52 @@ static inline int mxs_nand_calc_mark_offset(struct bch_geometry *geo, >>>> return 0; >>>> } >>>> >>>> +static inline unsigned int mxs_nand_max_ecc_strength_supported(void) >>>> +{ >>>> + /* Refer to Chapter 17 for i.MX6DQ, Chapter 18 for i.MX6SX */ >>>> + if (is_mx6sx() || is_mx7()) >>>> + return 62; >>>> + else >>>> + return 40; >>>> +} >>>> + >>>> +static inline int mxs_nand_calc_ecc_layout_by_info(struct bch_geometry *geo, >>>> + struct mtd_info *mtd) >>>> +{ >>>> + struct nand_chip *chip = mtd_to_nand(mtd); >>>> + >>>> + if (!(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0)) >>>> + return -ENOTSUPP; >>>> + >>>> + switch (chip->ecc_step_ds) { >>>> + case SZ_512: >>>> + geo->gf_len = 13; >>>> + break; >>>> + case SZ_1K: >>>> + geo->gf_len = 14; >>>> + break; >>>> + default: >>>> + return -EINVAL; >>>> + } >>>> + >>>> + geo->ecc_chunk_size = chip->ecc_step_ds; >>>> + geo->ecc_strength = round_up(chip->ecc_strength_ds, 2); >>>> + >>>> + /* Keep the C >= O */ >>>> + if (geo->ecc_chunk_size < mtd->oobsize) >>>> + return -EINVAL; >>>> + >>>> + if (geo->ecc_strength > mxs_nand_max_ecc_strength_supported()) >>>> + return -EINVAL; >>>> + >>>> + geo->ecc_chunk_count = mtd->writesize / geo->ecc_chunk_size; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static inline int mxs_nand_calc_ecc_layout(struct bch_geometry *geo, >>>> struct mtd_info *mtd) >>>> { >>>> - unsigned int max_ecc_strength_supported; >>>> - >>>> /* The default for the length of Galois Field. */ >>>> geo->gf_len = 13; >>>> >>>> @@ -235,12 +276,6 @@ static inline int mxs_nand_calc_ecc_layout(struct bch_geometry *geo, >>>> >>>> geo->ecc_chunk_count = mtd->writesize / geo->ecc_chunk_size; >>>> >>>> - /* Refer to Chapter 17 for i.MX6DQ, Chapter 18 for i.MX6SX */ >>>> - if (is_mx6sx() || is_mx7()) >>>> - max_ecc_strength_supported = 62; >>>> - else >>>> - max_ecc_strength_supported = 40; >>>> - >>>> /* >>>> * Determine the ECC layout with the formula: >>>> * ECC bits per chunk = (total page spare data bits) / >>>> @@ -252,10 +287,8 @@ static inline int mxs_nand_calc_ecc_layout(struct bch_geometry *geo, >>>> geo->ecc_strength = ((mtd->oobsize - MXS_NAND_METADATA_SIZE) * 8) >>>> / (geo->gf_len * geo->ecc_chunk_count); >>>> >>>> - geo->ecc_strength = min(round_down(geo->ecc_strength, 2), max_ecc_strength_supported); >>>> - >>>> - if (mxs_nand_calc_mark_offset(geo, mtd->writesize) < 0) >>>> - return -EINVAL; >>>> + geo->ecc_strength = min(round_down(geo->ecc_strength, 2), >>>> + mxs_nand_max_ecc_strength_supported()); >>>> >>>> return 0; >>>> } >>>> @@ -1006,9 +1039,19 @@ static int mxs_nand_setup_ecc(struct mtd_info *mtd) >>>> struct bch_geometry *geo = &nand_info->bch_geometry; >>>> struct mxs_bch_regs *bch_regs = (struct mxs_bch_regs *)MXS_BCH_BASE; >>>> uint32_t tmp; >>>> + int ret = -ENOTSUPP; >>>> >>>> - if (mxs_nand_calc_ecc_layout(geo, mtd)) >>>> - return -EINVAL; >>>> +#ifdef CONFIG_NAND_MXS_USE_MINIMUM_ECC >>>> + ret = mxs_nand_calc_ecc_layout_by_info(geo, mtd); >>>> +#endif >>>> + >>>> + if (ret == -ENOTSUPP) >>>> + ret = mxs_nand_calc_ecc_layout(geo, mtd); >>>> + >>>> + if (ret) >>>> + return ret; >>>> + >>>> + mxs_nand_calc_mark_offset(geo, mtd->writesize); >>>> >>>> /* Configure BCH and set NFC geometry */ >>>> mxs_reset_block(&bch_regs->hw_bch_ctrl_reg); >>>> >>> >>> This is not completely clear to me - is this series replaced by : >>> >>> http://patchwork.ozlabs.org/patch/902010/ >> >> No, the other patch set is on-top of this work. >> >> With this patch set I already introduce minimum ECC, but by just using a >> config symbol. The other patchset allows to enable the same using the >> device tree property. >> >> I guess the config symbol is still useful for non-dt platforms or SPLs. > > Is there anything stopping these two patchset from getting merged? > > -- > Stefan > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot