[U-Boot] [PATCH v2] mtd: resync with Linux-3.7.1

[Scott Wood]

On 01/18/2013 06:00:28 AM, Sergey Lapin wrote:
> diff --git a/drivers/mtd/nand/bfin_nand.c  
> b/drivers/mtd/nand/bfin_nand.c
> index c7ddbb2..7e755e8 100644
> --- a/drivers/mtd/nand/bfin_nand.c
> +++ b/drivers/mtd/nand/bfin_nand.c
> @@ -374,9 +374,11 @@ int board_nand_init(struct nand_chip *chip)
>  		if (!NAND_IS_512()) {
>  			chip->ecc.bytes = 3;
>  			chip->ecc.size = 256;
> +			chip->ecc.strength = 1;
>  		} else {
>  			chip->ecc.bytes = 6;
>  			chip->ecc.size = 512;
> +			chip->ecc.strength = 2;
>  		}

Hmm, I'm not sure that ecc.strength should be set to 2 here, if it's  
really two separate 256-byte regions each of which can correct 1 bit.   
2 bit correction can only be done under certain circumstances (i.e. the  
errors are not in the same half).

Linux does the same thing you've done, but that was also a "fix all the  
drivers" patch, so I'd like the input of someone familiar with the  
hardware.  CCing Mike Frysinger.  Maybe the driver should be reporting  
an ECC block size of 256 bytes?  Or at least internally return the  
number of bitflips from the half that had more bitflips.

>  		chip->ecc.mode = NAND_ECC_HW;
>  		chip->ecc.calculate = bfin_nfc_calculate_ecc;
> diff --git a/drivers/mtd/nand/davinci_nand.c  
> b/drivers/mtd/nand/davinci_nand.c
> index 04b24f3..9777878 100644
> --- a/drivers/mtd/nand/davinci_nand.c
> +++ b/drivers/mtd/nand/davinci_nand.c
> @@ -613,6 +613,7 @@ void davinci_nand_init(struct nand_chip *nand)
>  	nand->ecc.mode = NAND_ECC_HW;
>  	nand->ecc.size = 512;
>  	nand->ecc.bytes = 3;
> +	nand->ecc.strength = 4;
>  	nand->ecc.calculate = nand_davinci_calculate_ecc;
>  	nand->ecc.correct  = nand_davinci_correct_data;
>  	nand->ecc.hwctl  = nand_davinci_enable_hwecc;

This is in the 1-bit ECC #ifdef, so ecc.strength should be 1, and later  
in CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST in should be set to 4.

> diff --git a/drivers/mtd/nand/diskonchip.c  
> b/drivers/mtd/nand/diskonchip.c
> index 104d97f..4cd741e 100644
> --- a/drivers/mtd/nand/diskonchip.c
> +++ b/drivers/mtd/nand/diskonchip.c
> @@ -1658,6 +1658,7 @@ static int __init doc_probe(unsigned long  
> physadr)
>  	nand->ecc.mode		= NAND_ECC_HW_SYNDROME;
>  	nand->ecc.size		= 512;
>  	nand->ecc.bytes		= 6;
> +	nand->ecc.strength	= 2;
>  	nand->bbt_options	= NAND_BBT_USE_FLASH;
> 
>  	doc->physadr		= physadr;
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c  
> b/drivers/mtd/nand/fsl_elbc_nand.c
> index 65edbf5..c3f350d 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -781,6 +781,12 @@ static int fsl_elbc_chip_init(int devnum, u8  
> *addr)
>  		nand->ecc.size = 512;
>  		nand->ecc.bytes = 3;
>  		nand->ecc.steps = 1;
> +		chip->ecc.strength = 1;
> +		/*
> +		 * FIXME: can hardware ecc correct 4 bitflips if page  
> size is
> +		 * 2k?  Then does hardware report number of corrections  
> for this
> +		 * case?  If so, ecc_stats reporting needs to be fixed  
> as well.
> +		 */

It can correct 1 bitflip per 512 bytes.  This is a similar situation to  
blackfin (as far as I can tell), except that we report the actual  
hardware ECC block size, so ecc.strength = 1 is clearly correct.

As for ecc_stats, read_page() is supposed to return the number of  
bitflips from the ECC block with the most bitflips (see the generic  
read_page implementations).  This change was made to fsl_elbc_nand.c in  
Linux.  Note that all of the conversions of read_page() functions to  
simply return zero seem to be incorrect, and should be returning  
max_bitflips.

> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c  
> b/drivers/mtd/nand/fsl_ifc_nand.c
> index 5d47b40..823a2eb 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -871,6 +871,7 @@ int board_nand_init(struct nand_chip *nand)
>  	/* Hardware generates ECC per 512 Bytes */
>  	nand->ecc.size = 512;
>  	nand->ecc.bytes = 8;
> +	nand->ecc.strength = 1;
> 
>  	switch (csor & CSOR_NAND_PGS_MASK) {
>  	case CSOR_NAND_PGS_512:

IFC has ECC strength of 4 or 8 depending on ECC mode.  It also needs  
max_bitflips handling in read_page().

> diff --git a/drivers/mtd/nand/jz4740_nand.c  
> b/drivers/mtd/nand/jz4740_nand.c
> index 3ec34f3..3f64dc3 100644
> --- a/drivers/mtd/nand/jz4740_nand.c
> +++ b/drivers/mtd/nand/jz4740_nand.c
> @@ -253,6 +253,11 @@ int board_nand_init(struct nand_chip *nand)
>  	nand->ecc.mode		= NAND_ECC_HW_OOB_FIRST;
>  	nand->ecc.size		= CONFIG_SYS_NAND_ECCSIZE;
>  	nand->ecc.bytes		= CONFIG_SYS_NAND_ECCBYTES;
> +	nand->ecc.strength	= 2;
> +	/*
> +	 * FIXME: ecc_strength value of 2 bits per 512 bytes of data is  
> a
> +	 * conservative guess, given 9 ecc bytes and reed-solomon alg.
> +	 */
>  	nand->ecc.layout	= &qi_lb60_ecclayout_2gb;
>  	nand->chip_delay	= 50;
>  	nand->options		= NAND_USE_FLASH_BBT;

The Linux driver sets ecc.strength to 4, though that may have been  
after Linux 3.7.

> diff --git a/drivers/mtd/nand/tegra_nand.c  
> b/drivers/mtd/nand/tegra_nand.c
> index 854fd21..6afbec6 100644
> --- a/drivers/mtd/nand/tegra_nand.c
> +++ b/drivers/mtd/nand/tegra_nand.c
> @@ -1014,6 +1014,7 @@ int tegra_nand_init(struct nand_chip *nand, int  
> devnum)
>  	nand->ecc.write_page_raw = nand_write_page_raw;
>  	nand->ecc.read_oob = nand_read_oob;
>  	nand->ecc.write_oob = nand_write_oob;
> +	nand->ecc.strength = 1;
>  	nand->select_chip = nand_select_chip;
>  	nand->dev_ready  = nand_dev_ready;
>  	nand->priv = &nand_ctrl;

Jim, does ecc.strength = 1 look correct for Tegra?  It has more ECC  
bytes than I'd expect for 1-bit correction (does "4 symbol correct ECC"  
mean "4-bit correctable ECC"?).

-Scott