RE: [PATCH v7 4/7] mtd: nand: gpmi: may use minimum required ecc for 744 oobsize NAND

[Bean Huo 霍斌斌 (beanhuo)]

> By default NAND driver will choose the highest ecc strength that oob could
> contain, in this case, for some 8K+744 NAND flash, the ecc strength will be up
> to 52bit, which beyonds the i.MX6QDL BCH capability (40bit).


For normal working environment, if hardware BCH ECC cannot meet NAND ecc requirement,
We can set a minimum required ecc strength, and file system refresh/scrub can control bitflips under 
NAND ecc strength. But during reflow solder, it is very possible that NAND bitflips may increase over 
your hardware BCH capability, I don't know how this controller handle this?
For example, NAND require 80bit ECC, but hardware BCH ECC can only hold 40bit.
After reflow, there are some blocks that bitflips over 40bit.


> This patch allows the NAND driver try to use minimum required ecc strength if
> it failed to use the highest ecc, even without explicitly claiming
> "fsl,use-minimum-ecc" in dts.
> 
> Signed-off-by: Han Xu <b45815@xxxxxxxxxxxxx>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 0961b93..d8f2403 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -136,7 +136,7 @@ static inline bool gpmi_check_ecc(struct
> gpmi_nand_data *this)
>   *
>   * We may have available oob space in this case.
>   */
> -static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> +static int set_geometry_by_ecc_info(struct gpmi_nand_data *this)
>  {
>  	struct bch_geometry *geo = &this->bch_geometry;
>  	struct mtd_info *mtd = &this->mtd;
> @@ -145,7 +145,7 @@ static bool set_geometry_by_ecc_info(struct
> gpmi_nand_data *this)
>  	unsigned int block_mark_bit_offset;
> 
>  	if (!(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0))
> -		return false;
> +		return -EINVAL;
> 
>  	switch (chip->ecc_step_ds) {
>  	case SZ_512:
> @@ -158,19 +158,19 @@ static bool set_geometry_by_ecc_info(struct
> gpmi_nand_data *this)
>  		dev_err(this->dev,
>  			"unsupported nand chip. ecc bits : %d, ecc size : %d\n",
>  			chip->ecc_strength_ds, chip->ecc_step_ds);
> -		return false;
> +		return -EINVAL;
>  	}
>  	geo->ecc_chunk_size = chip->ecc_step_ds;
>  	geo->ecc_strength = round_up(chip->ecc_strength_ds, 2);
>  	if (!gpmi_check_ecc(this))
> -		return false;
> +		return -EINVAL;
> 
>  	/* Keep the C >= O */
>  	if (geo->ecc_chunk_size < mtd->oobsize) {
>  		dev_err(this->dev,
>  			"unsupported nand chip. ecc size: %d, oob size : %d\n",
>  			chip->ecc_step_ds, mtd->oobsize);
> -		return false;
> +		return -EINVAL;
>  	}
> 
>  	/* The default value, see comment in the legacy_set_geometry(). */ @@
> -242,7 +242,7 @@ static bool set_geometry_by_ecc_info(struct
> gpmi_nand_data *this)
>  				+ ALIGN(geo->ecc_chunk_count, 4);
> 
>  	if (!this->swap_block_mark)
> -		return true;
> +		return 0;
> 
>  	/* For bit swap. */
>  	block_mark_bit_offset = mtd->writesize * 8 - @@ -251,7 +251,7 @@
> static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> 
>  	geo->block_mark_byte_offset = block_mark_bit_offset / 8;
>  	geo->block_mark_bit_offset  = block_mark_bit_offset % 8;
> -	return true;
> +	return 0;
>  }
> 
>  static int legacy_set_geometry(struct gpmi_nand_data *this) @@ -285,7
> +285,8 @@ static int legacy_set_geometry(struct gpmi_nand_data *this)
>  	geo->ecc_strength = get_ecc_strength(this);
>  	if (!gpmi_check_ecc(this)) {
>  		dev_err(this->dev,
> -			"required ecc strength of the NAND chip: %d is not supported by
> the GPMI controller (%d)\n",
> +			"ecc strength: %d cannot be supported by the controller (%d)\n"
> +			"try to use minimum ecc strength that NAND chip required\n",
>  			geo->ecc_strength,
>  			this->devdata->bch_max_ecc_strength);
>  		return -EINVAL;
> @@ -366,10 +367,11 @@ static int legacy_set_geometry(struct
> gpmi_nand_data *this)
> 
>  int common_nfc_set_geometry(struct gpmi_nand_data *this)  {
> -	if (of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc")
> -		&& set_geometry_by_ecc_info(this))
> -		return 0;
> -	return legacy_set_geometry(this);
> +	if ((of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc"))
> +				|| legacy_set_geometry(this))
> +		return set_geometry_by_ecc_info(this);
> +
> +	return 0;
>  }
> 
>  struct dma_chan *get_dma_chan(struct gpmi_nand_data *this)
> --
> 1.9.1