[PATCH v3 2/4] mtd: rawnand: gpmi: Add strict ecc strength check

[Han Xu]

On 22/04/05 09:28AM, Miquel Raynal wrote:
> Hi Han,
> 
> han.xu at nxp.com wrote on Mon,  4 Apr 2022 14:54:25 -0500:
> 
> > Add strict ecc strength check in gpmi_check_ecc() function, which is
> > same as nand_ecc_is_strong_enough() did. It will check both correct bits
> > and correct bits per byte to ensure it meets chip required ecc strength.
> > 
> > Signed-off-by: Han Xu <han.xu at nxp.com>
> > 
> > ---
> > Changes since v2:
> >  - split the ecc check to a single patch
> > ---
> > ---
> >  drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 24 ++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > index 4144d5937103..9a37f8cc663e 100644
> > --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > @@ -238,9 +238,14 @@ static void gpmi_dump_info(struct gpmi_nand_data *this)
> >  		geo->block_mark_bit_offset);
> >  }
> >  
> > -static inline bool gpmi_check_ecc(struct gpmi_nand_data *this)
> > +static bool gpmi_check_ecc(struct gpmi_nand_data *this)
> 
> This change should be in a separate commit.
> 
> >  {
> > +	struct nand_chip *chip = &this->nand;
> > +	struct mtd_info *mtd = nand_to_mtd(&this->nand);
> >  	struct bch_geometry *geo = &this->bch_geometry;
> > +	const struct nand_ecc_props *requirements =
> > +		nanddev_get_ecc_requirements(&chip->base);
> > +	int corr, ds_corr;
> >  
> >  	/* Do the sanity check. */
> >  	if (GPMI_IS_MXS(this)) {
> > @@ -248,7 +253,22 @@ static inline bool gpmi_check_ecc(struct gpmi_nand_data *this)
> >  		if (geo->gf_len == 14)
> >  			return false;
> >  	}
> > -	return geo->ecc_strength <= this->devdata->bch_max_ecc_strength;
> > +
> > +	if (geo->ecc_strength > this->devdata->bch_max_ecc_strength)
> > +		return false;
> > +
> > +	/* check ecc strength, same as nand_ecc_is_strong_enough() did */
> 
> Why not using nand_ecc_is_strong_enough() in this case?

Hi Miquèl,

1. current nand_ecc_is_strong_enough() compared required ecc with conf ecc, but
I only see conf was initialized in some spi-nand and sw ecc drivers. Not sure if
it's a issue or someting missed in the gpmi driver? On my side,
nand_ecc_is_strong_enough() just returned.

2. I remembered that nand_ecc_is_strong_enough() used to compare required ecc
with nand_ecc_ctrl ecc strength, but even in this case, I don't know if it's a
good idea just directly call this function. Usually the driver looking for a
proper bch setting and get the ecc strength/step, at last set the nand_ecc_ctrl
. So at this moment, it's not ready to use nand_ecc_is_strong_enough().

> 
> > +	if (requirements->step_size) {
> > +		corr = mtd->writesize * geo->ecc_strength /
> > +		       geo->ecc_chunk_size;
> > +		ds_corr = mtd->writesize * requirements->strength /
> > +			  requirements->step_size;
> > +		if (corr < ds_corr ||
> > +		    geo->ecc_strength < requirements->strength)
> > +			return false;
> > +	}
> > +
> > +	return true;
> >  }
> >  
> >  /*
> 
> 
> Thanks,
> Miquèl