From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josh Wu Date: Mon, 27 Oct 2014 11:31:52 +0800 Subject: [U-Boot] [PATCH 1/3] atmel_nand: if don't have gf table in rom code we will build it runtime In-Reply-To: <544DA441.6010501@atmel.com> References: <1410854244-30424-1-git-send-email-voice.shen@atmel.com> <1410854244-30424-2-git-send-email-voice.shen@atmel.com> <544AD113.1070104@googlemail.com> <544DA441.6010501@atmel.com> Message-ID: <544DBCA8.4040608@atmel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>> >>> 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 >>> Signed-off-by: Bo Shen >>> --- >>> >>> 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