diff mbox

[U-Boot,1/3] atmel_nand: if don't have gf table in rom code we will build it runtime

Message ID 1410854244-30424-2-git-send-email-voice.shen@atmel.com
State Changes Requested, archived
Delegated to: Andreas Bießmann
Headers show

Commit Message

Bo Shen Sept. 16, 2014, 7:57 a.m. UTC
From: Josh Wu <josh.wu@atmel.com>

Add a macro NO_GALOIS_TABLE_IN_ROM. If it is defined we will build a
runtime pmecc galois table.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
Signed-off-by: Bo Shen <voice.shen@atmel.com>
---

 drivers/mtd/nand/atmel_nand.c | 127 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 126 insertions(+), 1 deletion(-)

Comments

Andreas Bießmann Oct. 24, 2014, 10:22 p.m. UTC | #1
Hi Bo,

before diving into that code I have a question. What will be the boot
mode for devices without integrated PMECC table? Can the first stage
loader be read with PMECC mode?
The question arises while thinking about this patch. We have generally
two options here:
 a) pre-generated PMECC table included in u-boot
 b) runtime generated PMECC table in RAM (this solution)

While a) will bloat the u-boot binary b) will require some RAM to hold
the table. If I understood correctly we will malloc at most 128 KiB for
the PMECC table ... Ok option a) is not an option ;)
Here my question arises, how can the ROM loader allocate 128 KiB? Do
these devices have so many SRAM? Or am I wrong?

On 16.09.14 09:57, Bo Shen wrote:
> From: Josh Wu <josh.wu@atmel.com>
> 
> Add a macro NO_GALOIS_TABLE_IN_ROM. If it is defined we will build a
> runtime pmecc galois table.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> ---
> 
>  drivers/mtd/nand/atmel_nand.c | 127 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 126 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index e73834d..7cf7b39 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -761,6 +761,108 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
>  }
>  #endif
>  
> +#if defined(NO_GALOIS_TABLE_IN_ROM)
> +static uint16_t *pmecc_galois_table;
> +static int pmecc_build_galois_table(unsigned int mm,
> +		int16_t *index_of, int16_t *alpha_to)
> +{
> +	unsigned int i, mask, nn;
> +	unsigned int p[15];
> +
> +	nn = (1 << mm) - 1;
> +	/* set default value */
> +	for (i = 1; i < mm; i++)
> +		p[i] = 0;

memset(p[1], 0, mm-1); // or even the whole field?

> +
> +	/* 1 + X^mm */
> +	p[0]  = 1;
> +	p[mm] = 1;
> +
> +	/* others  */
> +	switch (mm) {
> +	case 3:
> +	case 4:
> +	case 6:
> +	case 15:
> +		p[1] = 1;
> +		break;
> +	case 5:
> +	case 11:
> +		p[2] = 1;
> +		break;
> +	case 7:
> +	case 10:
> +		p[3] = 1;
> +		break;
> +	case 8:
> +		p[2] = 1;
> +		p[3] = 1;
> +		p[4] = 1;
> +		break;
> +	case 9:
> +		p[4] = 1;
> +		break;
> +	case 12:
> +		p[1] = 1;
> +		p[4] = 1;
> +		p[6] = 1;
> +		break;
> +	case 13:
> +		p[1] = 1;
> +		p[3] = 1;
> +		p[4] = 1;
> +		break;
> +	case 14:
> +		p[1] = 1;
> +		p[6] = 1;
> +		p[10] = 1;
> +		break;
> +	default:
> +		/* Error */
> +		return -EINVAL;
> +	}
> +
> +	/* Build alpha ^ mm it will help to generate the field (primitiv) */
> +	alpha_to[mm] = 0;
> +	for (i = 0; i < mm; i++)
> +		if (p[i])
> +			alpha_to[mm] |= 1 << i;

In fact p[] is a bit-field which is then copied to alpha_to[mm], why not
use alpha_to[mm] in the switch-case and eliminate p[]?

> +
> +	/*
> +	 * Then build elements from 0 to mm - 1. As degree is less than mm
> +	 * so it is just a logical shift.
> +	 */
> +	mask = 1;
> +	for (i = 0; i < mm; i++) {
> +		alpha_to[i] = mask;
> +		index_of[alpha_to[i]] = i;
> +		mask <<= 1;
> +	}
> +
> +	index_of[alpha_to[mm]] = mm;
> +
> +	/* use a mask to select the MSB bit of the LFSR */
> +	mask >>= 1;
> +
> +	/* then finish the building */
> +	for (i = mm + 1; i <= nn; i++) {
> +		/* check if the msb bit of the lfsr is set */
> +		if (alpha_to[i - 1] & mask)
> +			alpha_to[i] = alpha_to[mm] ^
> +				((alpha_to[i - 1] ^ mask) << 1);
> +		else
> +			alpha_to[i] = alpha_to[i - 1] << 1;
> +
> +		index_of[alpha_to[i]] = i % nn;
> +	}
> +
> +	/* index of 0 is undefined in a multiplicative field */
> +	index_of[0] = -1;
> +
> +	return 0;

The whole function is so longish and complicated ... I don't really like
it. Maybe eliminating p[] makes it better?

> +}
> +#endif
> +
>  static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>  		struct mtd_info *mtd)
>  {
> @@ -808,11 +910,18 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>  	sector_size = host->pmecc_sector_size;
>  
>  	/* TODO: need check whether cap & sector_size is validate */
> -
> +#if defined(NO_GALOIS_TABLE_IN_ROM)
> +	/*
> +	 * As pmecc_rom_base is the begin of the gallois field table, So the
> +	 * index offset just set as 0.
> +	 */
> +	host->pmecc_index_table_offset = 0;
> +#else
>  	if (host->pmecc_sector_size == 512)
>  		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_512;
>  	else
>  		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_1024;
> +#endif
>  
>  	MTDDEBUG(MTD_DEBUG_LEVEL1,
>  		"Initialize PMECC params, cap: %d, sector: %d\n",
> @@ -821,7 +930,23 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>  	host->pmecc = (struct pmecc_regs __iomem *) ATMEL_BASE_PMECC;
>  	host->pmerrloc = (struct pmecc_errloc_regs __iomem *)
>  			ATMEL_BASE_PMERRLOC;
> +#if defined(NO_GALOIS_TABLE_IN_ROM)
> +	/* Set pmecc_rom_base as the begin of gf table */
> +	int size = host->pmecc_sector_size == 512 ?
> +		PMECC_INDEX_TABLE_SIZE_512 :
> +		PMECC_INDEX_TABLE_SIZE_1024;
> +	pmecc_galois_table = malloc(2 * size * sizeof(uint16_t));

I fear this will not work with current SPL implementation ...

> +	if (!pmecc_galois_table)
> +		return -ENOMEM;
> +
> +	host->pmecc_rom_base = pmecc_galois_table;
> +	pmecc_build_galois_table((sector_size == 512) ? PMECC_GF_DIMENSION_13 :
> +			PMECC_GF_DIMENSION_14,
> +			host->pmecc_rom_base,
> +			host->pmecc_rom_base + (size * sizeof(int16_t)));
> +#else
>  	host->pmecc_rom_base = (void __iomem *) ATMEL_BASE_ROM;
> +#endif
>  
>  	/* ECC is calculated for the whole page (1 step) */
>  	nand->ecc.size = mtd->writesize;
> 

Best regards

Andreas Bießmann
Bo Shen Oct. 27, 2014, 1:47 a.m. UTC | #2
Hi Andreas,

On 10/25/2014 06:22 AM, Andreas Bießmann wrote:
> Hi Bo,
>
> before diving into that code I have a question. What will be the boot
> mode for devices without integrated PMECC table? Can the first stage
> loader be read with PMECC mode?
> The question arises while thinking about this patch. We have generally
> two options here:
>   a) pre-generated PMECC table included in u-boot
>   b) runtime generated PMECC table in RAM (this solution)
>
> While a) will bloat the u-boot binary b) will require some RAM to hold
> the table. If I understood correctly we will malloc at most 128 KiB for
> the PMECC table ... Ok option a) is not an option ;)
> Here my question arises, how can the ROM loader allocate 128 KiB? Do
> these devices have so many SRAM? Or am I wrong?

   On SAMA5D4 SoC, the PMECC table is only seen by the ROM loader. So, 
for atbootstrap, we also need to generate the PMECC table and put it in 
DDR2 SDRAM.
   We can use the PMECC generate by atbootstrap, however we must to 
maintain the address where to put the PMECC table, it will easily cause 
issues, so we try to regenerate the PMECC table also in u-boot.

> On 16.09.14 09:57, Bo Shen wrote:
>> From: Josh Wu <josh.wu@atmel.com>
>>
>> Add a macro NO_GALOIS_TABLE_IN_ROM. If it is defined we will build a
>> runtime pmecc galois table.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>> ---
>>
>>   drivers/mtd/nand/atmel_nand.c | 127 +++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 126 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
>> index e73834d..7cf7b39 100644
>> --- a/drivers/mtd/nand/atmel_nand.c
>> +++ b/drivers/mtd/nand/atmel_nand.c
>> @@ -761,6 +761,108 @@ static int pmecc_choose_ecc(struct atmel_nand_host *host,
>>   }
>>   #endif
>>
>> +#if defined(NO_GALOIS_TABLE_IN_ROM)
>> +static uint16_t *pmecc_galois_table;
>> +static int pmecc_build_galois_table(unsigned int mm,
>> +		int16_t *index_of, int16_t *alpha_to)
>> +{
>> +	unsigned int i, mask, nn;
>> +	unsigned int p[15];
>> +
>> +	nn = (1 << mm) - 1;
>> +	/* set default value */
>> +	for (i = 1; i < mm; i++)
>> +		p[i] = 0;
>
> memset(p[1], 0, mm-1); // or even the whole field?

OK. I will change it.

>> +
>> +	/* 1 + X^mm */
>> +	p[0]  = 1;
>> +	p[mm] = 1;
>> +
>> +	/* others  */
>> +	switch (mm) {
>> +	case 3:
>> +	case 4:
>> +	case 6:
>> +	case 15:
>> +		p[1] = 1;
>> +		break;
>> +	case 5:
>> +	case 11:
>> +		p[2] = 1;
>> +		break;
>> +	case 7:
>> +	case 10:
>> +		p[3] = 1;
>> +		break;
>> +	case 8:
>> +		p[2] = 1;
>> +		p[3] = 1;
>> +		p[4] = 1;
>> +		break;
>> +	case 9:
>> +		p[4] = 1;
>> +		break;
>> +	case 12:
>> +		p[1] = 1;
>> +		p[4] = 1;
>> +		p[6] = 1;
>> +		break;
>> +	case 13:
>> +		p[1] = 1;
>> +		p[3] = 1;
>> +		p[4] = 1;
>> +		break;
>> +	case 14:
>> +		p[1] = 1;
>> +		p[6] = 1;
>> +		p[10] = 1;
>> +		break;
>> +	default:
>> +		/* Error */
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Build alpha ^ mm it will help to generate the field (primitiv) */
>> +	alpha_to[mm] = 0;
>> +	for (i = 0; i < mm; i++)
>> +		if (p[i])
>> +			alpha_to[mm] |= 1 << i;
>
> In fact p[] is a bit-field which is then copied to alpha_to[mm], why not
> use alpha_to[mm] in the switch-case and eliminate p[]?

I will consider that.

>> +
>> +	/*
>> +	 * Then build elements from 0 to mm - 1. As degree is less than mm
>> +	 * so it is just a logical shift.
>> +	 */
>> +	mask = 1;
>> +	for (i = 0; i < mm; i++) {
>> +		alpha_to[i] = mask;
>> +		index_of[alpha_to[i]] = i;
>> +		mask <<= 1;
>> +	}
>> +
>> +	index_of[alpha_to[mm]] = mm;
>> +
>> +	/* use a mask to select the MSB bit of the LFSR */
>> +	mask >>= 1;
>> +
>> +	/* then finish the building */
>> +	for (i = mm + 1; i <= nn; i++) {
>> +		/* check if the msb bit of the lfsr is set */
>> +		if (alpha_to[i - 1] & mask)
>> +			alpha_to[i] = alpha_to[mm] ^
>> +				((alpha_to[i - 1] ^ mask) << 1);
>> +		else
>> +			alpha_to[i] = alpha_to[i - 1] << 1;
>> +
>> +		index_of[alpha_to[i]] = i % nn;
>> +	}
>> +
>> +	/* index of 0 is undefined in a multiplicative field */
>> +	index_of[0] = -1;
>> +
>> +	return 0;
>
> The whole function is so longish and complicated ... I don't really like
> it. Maybe eliminating p[] makes it better?

I will try to check the table would be usable in BCH library code.

>> +}
>> +#endif
>> +
>>   static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>>   		struct mtd_info *mtd)
>>   {
>> @@ -808,11 +910,18 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>>   	sector_size = host->pmecc_sector_size;
>>
>>   	/* TODO: need check whether cap & sector_size is validate */
>> -
>> +#if defined(NO_GALOIS_TABLE_IN_ROM)
>> +	/*
>> +	 * As pmecc_rom_base is the begin of the gallois field table, So the
>> +	 * index offset just set as 0.
>> +	 */
>> +	host->pmecc_index_table_offset = 0;
>> +#else
>>   	if (host->pmecc_sector_size == 512)
>>   		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_512;
>>   	else
>>   		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_1024;
>> +#endif
>>
>>   	MTDDEBUG(MTD_DEBUG_LEVEL1,
>>   		"Initialize PMECC params, cap: %d, sector: %d\n",
>> @@ -821,7 +930,23 @@ static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
>>   	host->pmecc = (struct pmecc_regs __iomem *) ATMEL_BASE_PMECC;
>>   	host->pmerrloc = (struct pmecc_errloc_regs __iomem *)
>>   			ATMEL_BASE_PMERRLOC;
>> +#if defined(NO_GALOIS_TABLE_IN_ROM)
>> +	/* Set pmecc_rom_base as the begin of gf table */
>> +	int size = host->pmecc_sector_size == 512 ?
>> +		PMECC_INDEX_TABLE_SIZE_512 :
>> +		PMECC_INDEX_TABLE_SIZE_1024;
>> +	pmecc_galois_table = malloc(2 * size * sizeof(uint16_t));
>
> I fear this will not work with current SPL implementation ...

I test with sama5d3xek board, it is OK. No problem found.

>> +	if (!pmecc_galois_table)
>> +		return -ENOMEM;
>> +
>> +	host->pmecc_rom_base = pmecc_galois_table;
>> +	pmecc_build_galois_table((sector_size == 512) ? PMECC_GF_DIMENSION_13 :
>> +			PMECC_GF_DIMENSION_14,
>> +			host->pmecc_rom_base,
>> +			host->pmecc_rom_base + (size * sizeof(int16_t)));
>> +#else
>>   	host->pmecc_rom_base = (void __iomem *) ATMEL_BASE_ROM;
>> +#endif
>>
>>   	/* ECC is calculated for the whole page (1 step) */
>>   	nand->ecc.size = mtd->writesize;
>>
>
> Best regards
>
> Andreas Bießmann
>

Best Regards,
Bo Shen
Josh Wu Oct. 27, 2014, 3:31 a.m. UTC | #3
Hi, Andreas, Bo

On 10/27/2014 9:47 AM, Bo Shen wrote:
> Hi Andreas,
>
> On 10/25/2014 06:22 AM, Andreas Bießmann wrote:
>> Hi Bo,
>>
>> before diving into that code I have a question. What will be the boot
>> mode for devices without integrated PMECC table? Can the first stage
>> loader be read with PMECC mode?
>> The question arises while thinking about this patch. We have generally
>> two options here:
>>   a) pre-generated PMECC table included in u-boot
>>   b) runtime generated PMECC table in RAM (this solution)
>>
>> While a) will bloat the u-boot binary b) will require some RAM to hold
>> the table. If I understood correctly we will malloc at most 128 KiB for
>> the PMECC table ... Ok option a) is not an option ;)
>> Here my question arises, how can the ROM loader allocate 128 KiB? Do
>> these devices have so many SRAM? Or am I wrong?

Does your ROM loader means the ROM code?
As the ROM code has a pre-generated PMECC table included. ROM code and 
the pre-generated PMECC table are both in the embedded readonly ROM. So 
that's not in the SRAM.

>
>   On SAMA5D4 SoC, the PMECC table is only seen by the ROM loader. So, 
> for atbootstrap, we also need to generate the PMECC table and put it 
> in DDR2 SDRAM.
>   We can use the PMECC generate by atbootstrap, however we must to 
> maintain the address where to put the PMECC table, it will easily 
> cause issues, so we try to regenerate the PMECC table also in u-boot.
>
>> On 16.09.14 09:57, Bo Shen wrote:
>>> From: Josh Wu <josh.wu@atmel.com>
>>>
>>> Add a macro NO_GALOIS_TABLE_IN_ROM. If it is defined we will build a
>>> runtime pmecc galois table.
>>>
>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>> ---
>>>
>>>   drivers/mtd/nand/atmel_nand.c | 127 
>>> +++++++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 126 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/nand/atmel_nand.c 
>>> b/drivers/mtd/nand/atmel_nand.c
>>> index e73834d..7cf7b39 100644
>>> --- a/drivers/mtd/nand/atmel_nand.c
>>> +++ b/drivers/mtd/nand/atmel_nand.c
>>> @@ -761,6 +761,108 @@ static int pmecc_choose_ecc(struct 
>>> atmel_nand_host *host,
>>>   }
>>>   #endif
>>>
>>> +#if defined(NO_GALOIS_TABLE_IN_ROM)
>>> +static uint16_t *pmecc_galois_table;
>>> +static int pmecc_build_galois_table(unsigned int mm,
>>> +        int16_t *index_of, int16_t *alpha_to)
>>> +{
>>> +    unsigned int i, mask, nn;
>>> +    unsigned int p[15];
>>> +
>>> +    nn = (1 << mm) - 1;
>>> +    /* set default value */
>>> +    for (i = 1; i < mm; i++)
>>> +        p[i] = 0;
>>
>> memset(p[1], 0, mm-1); // or even the whole field?
>
> OK. I will change it.
>
>>> +
>>> +    /* 1 + X^mm */
>>> +    p[0]  = 1;
>>> +    p[mm] = 1;
>>> +
>>> +    /* others  */
>>> +    switch (mm) {
>>> +    case 3:
>>> +    case 4:
>>> +    case 6:
>>> +    case 15:
>>> +        p[1] = 1;
>>> +        break;
>>> +    case 5:
>>> +    case 11:
>>> +        p[2] = 1;
>>> +        break;
>>> +    case 7:
>>> +    case 10:
>>> +        p[3] = 1;
>>> +        break;
>>> +    case 8:
>>> +        p[2] = 1;
>>> +        p[3] = 1;
>>> +        p[4] = 1;
>>> +        break;
>>> +    case 9:
>>> +        p[4] = 1;
>>> +        break;
>>> +    case 12:
>>> +        p[1] = 1;
>>> +        p[4] = 1;
>>> +        p[6] = 1;
>>> +        break;
>>> +    case 13:
>>> +        p[1] = 1;
>>> +        p[3] = 1;
>>> +        p[4] = 1;
>>> +        break;
>>> +    case 14:
>>> +        p[1] = 1;
>>> +        p[6] = 1;
>>> +        p[10] = 1;
>>> +        break;
>>> +    default:
>>> +        /* Error */
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    /* Build alpha ^ mm it will help to generate the field 
>>> (primitiv) */
>>> +    alpha_to[mm] = 0;
>>> +    for (i = 0; i < mm; i++)
>>> +        if (p[i])
>>> +            alpha_to[mm] |= 1 << i;
>>
>> In fact p[] is a bit-field which is then copied to alpha_to[mm], why not
>> use alpha_to[mm] in the switch-case and eliminate p[]?
>
> I will consider that.
>
>>> +
>>> +    /*
>>> +     * Then build elements from 0 to mm - 1. As degree is less than mm
>>> +     * so it is just a logical shift.
>>> +     */
>>> +    mask = 1;
>>> +    for (i = 0; i < mm; i++) {
>>> +        alpha_to[i] = mask;
>>> +        index_of[alpha_to[i]] = i;
>>> +        mask <<= 1;
>>> +    }
>>> +
>>> +    index_of[alpha_to[mm]] = mm;
>>> +
>>> +    /* use a mask to select the MSB bit of the LFSR */
>>> +    mask >>= 1;
>>> +
>>> +    /* then finish the building */
>>> +    for (i = mm + 1; i <= nn; i++) {
>>> +        /* check if the msb bit of the lfsr is set */
>>> +        if (alpha_to[i - 1] & mask)
>>> +            alpha_to[i] = alpha_to[mm] ^
>>> +                ((alpha_to[i - 1] ^ mask) << 1);
>>> +        else
>>> +            alpha_to[i] = alpha_to[i - 1] << 1;
>>> +
>>> +        index_of[alpha_to[i]] = i % nn;
>>> +    }
>>> +
>>> +    /* index of 0 is undefined in a multiplicative field */
>>> +    index_of[0] = -1;
>>> +
>>> +    return 0;
>>
>> The whole function is so longish and complicated ... I don't really like
>> it. Maybe eliminating p[] makes it better?
>
> I will try to check the table would be usable in BCH library code.

I already submitted a patch to the kernel: 
http://patchwork.ozlabs.org/patch/398827/
In that patch, this function is rewritten according to BCH code.
So I think maybe after that patch is accepted by kernel, then, we can 
resend new version patch to u-boot.

Best Regards,
Josh Wu
Bo Shen Oct. 28, 2014, 8:05 a.m. UTC | #4
Hi Andreas,

On 10/25/2014 06:22 AM, Andreas Bießmann wrote:
>>   			ATMEL_BASE_PMERRLOC;
>> >+#if defined(NO_GALOIS_TABLE_IN_ROM)
>> >+	/* Set pmecc_rom_base as the begin of gf table */
>> >+	int size = host->pmecc_sector_size == 512 ?
>> >+		PMECC_INDEX_TABLE_SIZE_512 :
>> >+		PMECC_INDEX_TABLE_SIZE_1024;
>> >+	pmecc_galois_table = malloc(2 * size * sizeof(uint16_t));
> I fear this will not work with current SPL implementation ...

Yes, when I try to implement the SPL support for sama5d4ek board, I 
really meet this issue. I don't know what reason cause this issue. So, 
can you give me some hints about it? Thanks.

Best Regards,
Bo Shen
Bo Shen Oct. 28, 2014, 9:27 a.m. UTC | #5
Hi Andreas,

On 10/28/2014 04:05 PM, Bo Shen wrote:
> Hi Andreas,
>
> On 10/25/2014 06:22 AM, Andreas Bießmann wrote:
>>>               ATMEL_BASE_PMERRLOC;
>>> >+#if defined(NO_GALOIS_TABLE_IN_ROM)
>>> >+    /* Set pmecc_rom_base as the begin of gf table */
>>> >+    int size = host->pmecc_sector_size == 512 ?
>>> >+        PMECC_INDEX_TABLE_SIZE_512 :
>>> >+        PMECC_INDEX_TABLE_SIZE_1024;
>>> >+    pmecc_galois_table = malloc(2 * size * sizeof(uint16_t));
>> I fear this will not work with current SPL implementation ...
>
> Yes, when I try to implement the SPL support for sama5d4ek board, I
> really meet this issue. I don't know what reason cause this issue. So,
> can you give me some hints about it? Thanks.

I use the nand read.raw and nand write.raw command with wrong parameter.

It is OK for SPL with this kind of code on sama5d4ek.

Sorry for this noise.

> Best Regards,
> Bo Shen
>

Best Regards,
Bo Shen
Andreas Bießmann Oct. 28, 2014, 9:34 a.m. UTC | #6
Hi Bo,

On 28.10.14 10:27, Bo Shen wrote:
> Hi Andreas,
> 
> On 10/28/2014 04:05 PM, Bo Shen wrote:
>> Hi Andreas,
>>
>> On 10/25/2014 06:22 AM, Andreas Bießmann wrote:
>>>>               ATMEL_BASE_PMERRLOC;
>>>> >+#if defined(NO_GALOIS_TABLE_IN_ROM)
>>>> >+    /* Set pmecc_rom_base as the begin of gf table */
>>>> >+    int size = host->pmecc_sector_size == 512 ?
>>>> >+        PMECC_INDEX_TABLE_SIZE_512 :
>>>> >+        PMECC_INDEX_TABLE_SIZE_1024;
>>>> >+    pmecc_galois_table = malloc(2 * size * sizeof(uint16_t));
>>> I fear this will not work with current SPL implementation ...
>>
>> Yes, when I try to implement the SPL support for sama5d4ek board, I
>> really meet this issue. I don't know what reason cause this issue. So,
>> can you give me some hints about it? Thanks.
> 
> I use the nand read.raw and nand write.raw command with wrong parameter.
> 
> It is OK for SPL with this kind of code on sama5d4ek.

nice to hear, great!

Best regards

Andreas
diff mbox

Patch

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index e73834d..7cf7b39 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -761,6 +761,108 @@  static int pmecc_choose_ecc(struct atmel_nand_host *host,
 }
 #endif
 
+#if defined(NO_GALOIS_TABLE_IN_ROM)
+static uint16_t *pmecc_galois_table;
+static int pmecc_build_galois_table(unsigned int mm,
+		int16_t *index_of, int16_t *alpha_to)
+{
+	unsigned int i, mask, nn;
+	unsigned int p[15];
+
+	nn = (1 << mm) - 1;
+	/* set default value */
+	for (i = 1; i < mm; i++)
+		p[i] = 0;
+
+	/* 1 + X^mm */
+	p[0]  = 1;
+	p[mm] = 1;
+
+	/* others  */
+	switch (mm) {
+	case 3:
+	case 4:
+	case 6:
+	case 15:
+		p[1] = 1;
+		break;
+	case 5:
+	case 11:
+		p[2] = 1;
+		break;
+	case 7:
+	case 10:
+		p[3] = 1;
+		break;
+	case 8:
+		p[2] = 1;
+		p[3] = 1;
+		p[4] = 1;
+		break;
+	case 9:
+		p[4] = 1;
+		break;
+	case 12:
+		p[1] = 1;
+		p[4] = 1;
+		p[6] = 1;
+		break;
+	case 13:
+		p[1] = 1;
+		p[3] = 1;
+		p[4] = 1;
+		break;
+	case 14:
+		p[1] = 1;
+		p[6] = 1;
+		p[10] = 1;
+		break;
+	default:
+		/* Error */
+		return -EINVAL;
+	}
+
+	/* Build alpha ^ mm it will help to generate the field (primitiv) */
+	alpha_to[mm] = 0;
+	for (i = 0; i < mm; i++)
+		if (p[i])
+			alpha_to[mm] |= 1 << i;
+
+	/*
+	 * Then build elements from 0 to mm - 1. As degree is less than mm
+	 * so it is just a logical shift.
+	 */
+	mask = 1;
+	for (i = 0; i < mm; i++) {
+		alpha_to[i] = mask;
+		index_of[alpha_to[i]] = i;
+		mask <<= 1;
+	}
+
+	index_of[alpha_to[mm]] = mm;
+
+	/* use a mask to select the MSB bit of the LFSR */
+	mask >>= 1;
+
+	/* then finish the building */
+	for (i = mm + 1; i <= nn; i++) {
+		/* check if the msb bit of the lfsr is set */
+		if (alpha_to[i - 1] & mask)
+			alpha_to[i] = alpha_to[mm] ^
+				((alpha_to[i - 1] ^ mask) << 1);
+		else
+			alpha_to[i] = alpha_to[i - 1] << 1;
+
+		index_of[alpha_to[i]] = i % nn;
+	}
+
+	/* index of 0 is undefined in a multiplicative field */
+	index_of[0] = -1;
+
+	return 0;
+}
+#endif
+
 static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 		struct mtd_info *mtd)
 {
@@ -808,11 +910,18 @@  static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 	sector_size = host->pmecc_sector_size;
 
 	/* TODO: need check whether cap & sector_size is validate */
-
+#if defined(NO_GALOIS_TABLE_IN_ROM)
+	/*
+	 * As pmecc_rom_base is the begin of the gallois field table, So the
+	 * index offset just set as 0.
+	 */
+	host->pmecc_index_table_offset = 0;
+#else
 	if (host->pmecc_sector_size == 512)
 		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_512;
 	else
 		host->pmecc_index_table_offset = ATMEL_PMECC_INDEX_OFFSET_1024;
+#endif
 
 	MTDDEBUG(MTD_DEBUG_LEVEL1,
 		"Initialize PMECC params, cap: %d, sector: %d\n",
@@ -821,7 +930,23 @@  static int atmel_pmecc_nand_init_params(struct nand_chip *nand,
 	host->pmecc = (struct pmecc_regs __iomem *) ATMEL_BASE_PMECC;
 	host->pmerrloc = (struct pmecc_errloc_regs __iomem *)
 			ATMEL_BASE_PMERRLOC;
+#if defined(NO_GALOIS_TABLE_IN_ROM)
+	/* Set pmecc_rom_base as the begin of gf table */
+	int size = host->pmecc_sector_size == 512 ?
+		PMECC_INDEX_TABLE_SIZE_512 :
+		PMECC_INDEX_TABLE_SIZE_1024;
+	pmecc_galois_table = malloc(2 * size * sizeof(uint16_t));
+	if (!pmecc_galois_table)
+		return -ENOMEM;
+
+	host->pmecc_rom_base = pmecc_galois_table;
+	pmecc_build_galois_table((sector_size == 512) ? PMECC_GF_DIMENSION_13 :
+			PMECC_GF_DIMENSION_14,
+			host->pmecc_rom_base,
+			host->pmecc_rom_base + (size * sizeof(int16_t)));
+#else
 	host->pmecc_rom_base = (void __iomem *) ATMEL_BASE_ROM;
+#endif
 
 	/* ECC is calculated for the whole page (1 step) */
 	nand->ecc.size = mtd->writesize;