
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