[U-Boot] [PATCH 0/4] atmel_nand: misc update in atmel nandflash driver

[U-Boot] [PATCH 0/4] atmel_nand: misc update in atmel nandflash driver

[Josh Wu]

Since the nand driver is synced with v4.1 kernel, here is a update also
for atmel_nand driver. which includes:
  1. fix format and using definition not magic number.
  2. use ecc_{strength, step}_ds instead of our own function.
  3. add delay time to support MF29F32G08CBADA.


Josh Wu (4):
  atmel_nand: use nand ecc_{strength,step}_ds instead of our own
    function
  atmel_nand: add '\n' in the end of error message for better display
  atmel_nand: increase more delay to support MT29F32G08CBADA
  atmel_nand: use the definition: PMECC_OOB_RESERVED_BYTES instead magic
    number

 drivers/mtd/nand/atmel_nand.c | 52 +++++++++----------------------------------
 1 file changed, 11 insertions(+), 41 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH 1/4] atmel_nand: use nand ecc_{strength, step}_ds instead of our own function

[Josh Wu]

Since ecc_{strength,step}_ds is introduced in nand_chip structure for
minimum ecc requirements. So we can use them directly and remove our
own get_onfi_ecc_param function.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---

 drivers/mtd/nand/atmel_nand.c | 44 +++++++------------------------------------
 1 file changed, 7 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 0d4f327..e179f74 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -677,34 +677,6 @@ static void atmel_pmecc_core_init(struct mtd_info *mtd)
 
 #ifdef CONFIG_SYS_NAND_ONFI_DETECTION
 /*
- * get_onfi_ecc_param - Get ECC requirement from ONFI parameters
- * @ecc_bits: store the ONFI ECC correct bits capbility
- * @sector_size: in how many bytes that ONFI require to correct @ecc_bits
- *
- * Returns -1 if ONFI parameters is not supported. In this case @ecc_bits,
- * @sector_size are initialize to 0.
- * Return 0 if success to get the ECC requirement.
- */
-static int get_onfi_ecc_param(struct nand_chip *chip,
-		int *ecc_bits, int *sector_size)
-{
-	*ecc_bits = *sector_size = 0;
-
-	if (chip->onfi_params.ecc_bits == 0xff)
-		/* TODO: the sector_size and ecc_bits need to be find in
-		 * extended ecc parameter, currently we don't support it.
-		 */
-		return -1;
-
-	*ecc_bits = chip->onfi_params.ecc_bits;
-
-	/* The default sector size (ecc codeword size) is 512 */
-	*sector_size = 512;
-
-	return 0;
-}
-
-/*
  * pmecc_choose_ecc - Get ecc requirement from ONFI parameters. If
  *                    pmecc_corr_cap or pmecc_sector_size is 0, then set it as
  *                    ONFI ECC parameters.
@@ -724,17 +696,15 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
 	/* Get ECC requirement from ONFI parameters */
 	*cap = *sector_size = 0;
 	if (chip->onfi_version) {
-		if (!get_onfi_ecc_param(chip, cap, sector_size)) {
-			MTDDEBUG(MTD_DEBUG_LEVEL1, "ONFI params, minimum required ECC: %d bits in %d bytes\n",
-				*cap, *sector_size);
-		} else {
-			dev_info(host->dev, "NAND chip ECC reqirement is in Extended ONFI parameter, we don't support yet.\n");
-		}
-	} else {
-		dev_info(host->dev, "NAND chip is not ONFI compliant, assume ecc_bits is 2 in 512 bytes");
+		*cap = chip->ecc_strength_ds;
+		*sector_size = chip->ecc_step_ds;
+		MTDDEBUG(MTD_DEBUG_LEVEL1, "ONFI params, minimum required ECC: %d bits in %d bytes\n",
+			 *cap, *sector_size);
 	}
+
 	if (*cap == 0 && *sector_size == 0) {
-		/* Non-ONFI compliant or use extended ONFI parameters */
+		/* Non-ONFI compliant */
+		dev_info(host->dev, "NAND chip is not ONFI compliant, assume ecc_bits is 2 in 512 bytes\n");
 		*cap = 2;
 		*sector_size = 512;
 	}
-- 
1.9.1

[U-Boot] [PATCH 1/4] atmel_nand: use nand ecc_{strength, step}_ds instead of our own function

[Andreas Bießmann]

On 24.11.15 09:26, Josh Wu wrote:
> Since ecc_{strength,step}_ds is introduced in nand_chip structure for
> minimum ecc requirements. So we can use them directly and remove our
> own get_onfi_ecc_param function.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>

Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>

> ---
> 
>  drivers/mtd/nand/atmel_nand.c | 44 +++++++------------------------------------
>  1 file changed, 7 insertions(+), 37 deletions(-)

[U-Boot] [PATCH 1/4] atmel_nand: use nand ecc_{strength, step}_ds instead of our own function

[Yang, Wenyou]

Hi Andreas,

Thank you for your review.

As Josh have left our company, I will follow up this patches.

> -----Original Message-----
> From: Andreas Bie?mann [mailto:andreas.devel at googlemail.com]
> Sent: 2016?1?25? 5:23
> To: Wu, Josh <Josh.wu@atmel.com>; Scott Wood <scottwood@freescale.com>;
> U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Yang, Wenyou <Wenyou.Yang@atmel.com>; Bo Shen
> <voice.shen@gmail.com>
> Subject: Re: [PATCH 1/4] atmel_nand: use nand ecc_{strength,step}_ds instead
> of our own function
> 
> On 24.11.15 09:26, Josh Wu wrote:
> > Since ecc_{strength,step}_ds is introduced in nand_chip structure for
> > minimum ecc requirements. So we can use them directly and remove our
> > own get_onfi_ecc_param function.
> >
> > Signed-off-by: Josh Wu <josh.wu@atmel.com>
> 
> Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>
> 
> > ---
> >
> >  drivers/mtd/nand/atmel_nand.c | 44
> > +++++++------------------------------------
> >  1 file changed, 7 insertions(+), 37 deletions(-)


Best Regards,
Wenyou Yang

[U-Boot] [PATCH 2/4] atmel_nand: add '\n' in the end of error message for better display

[Josh Wu]

Also align the open parenthesis.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---

 drivers/mtd/nand/atmel_nand.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index e179f74..5226acf 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -821,8 +821,8 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 	 * from ONFI.
 	 */
 	if (pmecc_choose_ecc(host, nand, &cap, &sector_size)) {
-		dev_err(host->dev, "The NAND flash's ECC requirement(ecc_bits: %d, sector_size: %d) are not support!",
-				cap, sector_size);
+		dev_err(host->dev, "The NAND flash's ECC requirement(ecc_bits: %d, sector_size: %d) are not support!\n",
+			cap, sector_size);
 		return -EINVAL;
 	}
 
-- 
1.9.1

[U-Boot] [PATCH 2/4] atmel_nand: add '\n' in the end of error message for better display

[Andreas Bießmann]

Hi Josh,

On 24.11.15 09:26, Josh Wu wrote:
> Also align the open parenthesis.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
> 
>  drivers/mtd/nand/atmel_nand.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index e179f74..5226acf 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -821,8 +821,8 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>  	 * from ONFI.
>  	 */
>  	if (pmecc_choose_ecc(host, nand, &cap, &sector_size)) {
> -		dev_err(host->dev, "The NAND flash's ECC requirement(ecc_bits: %d, sector_size: %d) are not support!",
> -				cap, sector_size);
> +		dev_err(host->dev, "The NAND flash's ECC requirement(ecc_bits: %d, sector_size: %d) are not support!\n",
> +			cap, sector_size);

Could you please rephrase this?

NAND flash's ECC requirement (ecc_bits: %d, sector_size: %d) is not
supported!

or since dev_err() should print pmecc in any case:

Required ECC %d bits in %d bytes not supported!

Andreas

>  		return -EINVAL;
>  	}
>  
> 

[U-Boot] [PATCH 3/4] atmel_nand: increase more delay to support MT29F32G08CBADA

[Josh Wu]

The tR is 100us in the datasheet section: Array Characteristics

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---

 drivers/mtd/nand/atmel_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 5226acf..266dfd9 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -1492,7 +1492,7 @@ int atmel_nand_chip_init(int devnum, ulong base_addr)
 #ifdef CONFIG_SYS_NAND_READY_PIN
 	nand->dev_ready = at91_nand_ready;
 #endif
-	nand->chip_delay = 75;
+	nand->chip_delay = 100;
 #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
 	nand->bbt_options |= NAND_BBT_USE_FLASH;
 #endif
-- 
1.9.1

[U-Boot] [PATCH 3/4] atmel_nand: increase more delay to support MT29F32G08CBADA

[Andreas Bießmann]

Hi Josh,

On 24.11.15 09:26, Josh Wu wrote:
> The tR is 100us in the datasheet section: Array Characteristics
> 

I've seen 50us for tR. Which datasheet do you have?

Andreas

> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
> 
>  drivers/mtd/nand/atmel_nand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 5226acf..266dfd9 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -1492,7 +1492,7 @@ int atmel_nand_chip_init(int devnum, ulong base_addr)
>  #ifdef CONFIG_SYS_NAND_READY_PIN
>  	nand->dev_ready = at91_nand_ready;
>  #endif
> -	nand->chip_delay = 75;
> +	nand->chip_delay = 100;
>  #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
>  	nand->bbt_options |= NAND_BBT_USE_FLASH;
>  #endif
> 

[U-Boot] [PATCH 3/4] atmel_nand: increase more delay to support MT29F32G08CBADA

[Yang, Wenyou]

Hi Andreas,

> -----Original Message-----
> From: Andreas Bie?mann [mailto:andreas.devel at googlemail.com]
> Sent: 2016?1?25? 5:42
> To: Wu, Josh <Josh.wu@atmel.com>; Scott Wood <scottwood@freescale.com>;
> U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Yang, Wenyou <Wenyou.Yang@atmel.com>; Bo Shen
> <voice.shen@gmail.com>
> Subject: Re: [PATCH 3/4] atmel_nand: increase more delay to support
> MT29F32G08CBADA
> 
> Hi Josh,
> 
> On 24.11.15 09:26, Josh Wu wrote:
> > The tR is 100us in the datasheet section: Array Characteristics
> >
> 
> I've seen 50us for tR. Which datasheet do you have?

The datasheet is downloaded from micron official website.

https://www.micron.com/parts/nand-flash/mass-storage/mt29f32g08cbadawp?pc=%7B80EFFAAD-26CB-4D06-84BC-0E3274B960A9%7D

This device requires the number of bits ECC correctability is 40 bits,

but the maximum error correction capability of PMECC IP of SAMA5D2 is 32 bits.

That is,  SAMA5D2 can't support this device.

So, I suggest to drop this patch.

> 
> Andreas
> 
> > Signed-off-by: Josh Wu <josh.wu@atmel.com>
> > ---
> >
> >  drivers/mtd/nand/atmel_nand.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/atmel_nand.c
> > b/drivers/mtd/nand/atmel_nand.c index 5226acf..266dfd9 100644
> > --- a/drivers/mtd/nand/atmel_nand.c
> > +++ b/drivers/mtd/nand/atmel_nand.c
> > @@ -1492,7 +1492,7 @@ int atmel_nand_chip_init(int devnum, ulong
> > base_addr)  #ifdef CONFIG_SYS_NAND_READY_PIN
> >  	nand->dev_ready = at91_nand_ready;
> >  #endif
> > -	nand->chip_delay = 75;
> > +	nand->chip_delay = 100;
> >  #ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
> >  	nand->bbt_options |= NAND_BBT_USE_FLASH;  #endif
> >

Best Regards,
Wenyou Yang

[U-Boot] [PATCH 4/4] atmel_nand: use the definition: PMECC_OOB_RESERVED_BYTES instead magic number

[Josh Wu]

As atmel_nand_ecc.h is sync with v4.1 kernel, which adds the
PMECC_OOB_RESERVED_BYTES. So use it in the driver.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---

 drivers/mtd/nand/atmel_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 266dfd9..2967eee 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -901,7 +901,7 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 			return -EINVAL;
 		}
 
-		if (nand->ecc.bytes > mtd->oobsize - 2) {
+		if (nand->ecc.bytes > mtd->oobsize - PMECC_OOB_RESERVED_BYTES) {
 			dev_err(host->dev, "No room for ECC bytes\n");
 			return -EINVAL;
 		}
-- 
1.9.1

[U-Boot] [PATCH 4/4] atmel_nand: use the definition: PMECC_OOB_RESERVED_BYTES instead magic number

[Andreas Bießmann]

On 24.11.15 09:26, Josh Wu wrote:
> As atmel_nand_ecc.h is sync with v4.1 kernel, which adds the
> PMECC_OOB_RESERVED_BYTES. So use it in the driver.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>

Reviewed-by: Andreas Bie?mann <andreas.devel@googlemail.com>

> ---
> 
>  drivers/mtd/nand/atmel_nand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 266dfd9..2967eee 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -901,7 +901,7 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>  			return -EINVAL;
>  		}
>  
> -		if (nand->ecc.bytes > mtd->oobsize - 2) {
> +		if (nand->ecc.bytes > mtd->oobsize - PMECC_OOB_RESERVED_BYTES) {
>  			dev_err(host->dev, "No room for ECC bytes\n");
>  			return -EINVAL;
>  		}
>