[U-Boot] [PATCH v3 5/5] mtd: nand: mxs_nand: add minimal ECC support

[Stefan Agner]

On 27.04.2018 09:31, Stefano Babic wrote:
> Hi Stefan,
> 
> On 11/04/2018 18:04, Stefan Agner wrote:
>> From: Stefan Agner <stefan.agner at toradex.com>
>>
>> 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 <stefan.agner at toradex.com>
>> ---
>>
>> 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.

--
Stefan