[U-Boot,1/2] mtd:mxs:nand calculate ecc strength dynamically

Calculate ecc strength according oobsize, but not hardcoded
which is not aligned with kernel driver

Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
Signed-off-by: Ye.Li <b37916@freescale.com>
---
 drivers/mtd/nand/mxs_nand.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

Hi Marek,

Since you are familiar with this driver, would you please help review 
this patch?

On 12/19/2014 12:39 PM, Peng Fan wrote:
> Calculate ecc strength according oobsize, but not hardcoded
> which is not aligned with kernel driver
>
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Ye.Li <b37916@freescale.com>
> ---
>   drivers/mtd/nand/mxs_nand.c | 22 ++++------------------
>   1 file changed, 4 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 7a064ab..a45fcf9 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -146,26 +146,12 @@ static uint32_t mxs_nand_aux_status_offset(void)
>   static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
>   						uint32_t page_oob_size)
>   {
> -	if (page_data_size == 2048) {
> -		if (page_oob_size == 64)
> -			return 8;
> +	int ecc_strength;
>   
> -		if (page_oob_size == 112)
> -			return 14;
> -	}
> -
> -	if (page_data_size == 4096) {
> -		if (page_oob_size == 128)
> -			return 8;
> -
> -		if (page_oob_size == 218)
> -			return 16;
> +	ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
> +			/ (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
>   
> -		if (page_oob_size == 224)
> -			return 16;
> -	}
> -
> -	return 0;
> +	return round_down(ecc_strength, 2);
>   }
>   
>   static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
Thanks,
Peng.

On Friday, December 19, 2014 at 05:39:12 AM, Peng Fan wrote:
> Calculate ecc strength according oobsize, but not hardcoded
> which is not aligned with kernel driver
> 
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Ye.Li <b37916@freescale.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

On Tuesday, January 20, 2015 at 07:35:26 AM, Peng Fan wrote:
> Hi Marek,
> 
> Since you are familiar with this driver, would you please help review
> this patch?

Hi!

I commented on both. Next time, please CC me and Stefano, since the patches
might slip just like this one did.

Thank you for this work !

Best regards,
Marek Vasut

Hi, Marek

On 1/20/2015 7:04 PM, Marek Vasut wrote:
> On Tuesday, January 20, 2015 at 07:35:26 AM, Peng Fan wrote:
>> Hi Marek,
>>
>> Since you are familiar with this driver, would you please help review
>> this patch?
> Hi!
>
> I commented on both. Next time, please CC me and Stefano, since the patches
> might slip just like this one did.
>
> Thank you for this work !
Sorry for this and thanks for this tip.
>
> Best regards,
> Marek Vasut
Best regards,
Peng.

On Fr, 2014-12-19 at 12:39 +0800, Peng Fan wrote:
> Calculate ecc strength according oobsize, but not hardcoded
> which is not aligned with kernel driver
> 
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Ye.Li <b37916@freescale.com>
> ---
>  drivers/mtd/nand/mxs_nand.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 7a064ab..a45fcf9 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -146,26 +146,12 @@ static uint32_t mxs_nand_aux_status_offset(void)
>  static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
>  						uint32_t page_oob_size)
>  {
> -	if (page_data_size == 2048) {
> -		if (page_oob_size == 64)
> -			return 8;
> +	int ecc_strength;
>  
> -		if (page_oob_size == 112)
> -			return 14;
> -	}
> -
> -	if (page_data_size == 4096) {
> -		if (page_oob_size == 128)
> -			return 8;
> -
> -		if (page_oob_size == 218)
> -			return 16;
> +	ecc_strength = ((page_oob_size - MXS_NAND_METADATA_SIZE) * 8)
> +			/ (13 * mxs_nand_ecc_chunk_cnt(page_data_size));
>  
> -		if (page_oob_size == 224)
> -			return 16;
> -	}
> -
> -	return 0;
> +	return round_down(ecc_strength, 2);
>  }
>  
>  static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,

Many thanks for the patch! But this patch affects mxsboot which is no
not aligned with the U-Boot mxs nand driver.

I was able to fix mxsboot, but I had difficulties with round_down, which
is a macro definition in linux/kernel.h. I've copied the macro
definition to mxsboot. I will submit the patch in a seperate mail.

I would like to see a comment or a macro for the magic number 13, which
is the value for the Galois Field, just for clarification

With fixing mxsboot, I was able to test the patch on a custom
i.MX28-based board assembled with a 1Gbit NAND flash (page size = 2048
bytes, oob size = 128 bytes).

U-Boot correctly reads the NAND info
=> nand info
        Device 0: nand0, sector size 128 KiB
          Page size      2048 b
          OOB size        128 b
          Erase size   131072 b

Before the patch linux failed to read from the UBI device with an ECC
error:
        UBI error: ubi_io_read: error -74 (ECC error)

This patch resolves the error. Linux can read the UBI device now. This
is kernel message:
        nand: device found, Manufacturer ID: 0x98, Chip ID: 0xf1
        [    1.327810] nand: Toshiba NAND 128MiB 3,3V 8-bit
        [    1.332482] nand: 128MiB, SLC, page size: 2048, OOB size: 128
        BCH Geometry :
        [    1.594658] GF length              : 13
        [    1.594658] ECC Strength           : 18
        [    1.594658] Page Size in Bytes     : 2176
        [    1.594658] Metadata Size in Bytes : 10
        [    1.594658] ECC Chunk Size in Bytes: 512
        [    1.594658] ECC Chunk Count        : 4
        [    1.594658] Payload Size in Bytes  : 2048
        [    1.594658] Auxiliary Size in Bytes: 16
        [    1.594658] Auxiliary Status Offset: 12
        [    1.594658] Block Mark Byte Offset : 1950
        [    1.594658] Block Mark Bit Offset  : 2