Dear Josh Wu,

On 16.08.2012 07:05, Josh Wu wrote:
> The Programmable Multibit ECC (PMECC) controller is a programmable binary
> BCH(Bose, Chaudhuri and Hocquenghem) encoder and decoder. This controller
> can be used to support both SLC and MLC NAND Flash devices. It supports to
> generate ECC to correct 2, 4, 8, 12 or 24 bits of error per sector of data.
> 
> To use PMECC in this driver, the user needs to set the PMECC correction
> capability, the sector size and ROM lookup table offsets in board config file.
> 
> This driver is ported from Linux kernel atmel_nand PMECC patch. The main 
> difference
> is in this version it uses registers structure access hardware instead of 
> using macros.
> It is tested in 9x5 serial boards.
> 
> Signed-off-by: Josh Wu <josh...@atmel.com>
> ---
> Changes since v1:
>   Change 'ecc' array's type from u32 to u8 in structure pmecc_regs (u32 
> ecc[11] -> u8 ecc[44]). That will make PMECC write correctly.
>   enable 4k-page nand flash pmecc support.
>   fix coding style errors and warnings.
>   changed lookup table variable name which sounds correct.
> 
>  drivers/mtd/nand/atmel_nand.c     |  654 
> ++++++++++++++++++++++++++++++++++++-
>  drivers/mtd/nand/atmel_nand_ecc.h |  111 +++++++
>  2 files changed, 763 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index 9dc003e..784370c 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -5,6 +5,9 @@
>   *
>   * (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
>   *
> + * Add Programmable Multibit ECC support for various AT91 SoC
> + *     (C) Copyright 2012 ATMEL, Hong Xu
> + *
>   * See file CREDITS for list of people who contributed to this
>   * project.
>   *
> @@ -41,6 +44,648 @@
>  
>  #include "atmel_nand_ecc.h"  /* Hardware ECC registers */
>  
> +static int chip_nr;
> +
> +#ifdef CONFIG_ATMEL_NAND_HW_PMECC
> +
> +struct atmel_nand_host {
> +     struct pmecc_regs __iomem *pmecc;
> +     struct pmecc_errloc_regs __iomem *pmerrloc;
> +     void __iomem            *pmecc_rom_base;
> +
> +     u8              pmecc_corr_cap;
> +     u16             pmecc_sector_size;
> +     u32             pmecc_index_table_offset;
> +
> +     int             pmecc_bytes_per_sector;
> +     int             pmecc_sector_number;
> +     int             pmecc_degree;   /* Degree of remainders */
> +     int             pmecc_cw_len;   /* Length of codeword */
> +
> +     /* lookup table for alpha_to and index_of */
> +     void __iomem    *pmecc_alpha_to;
> +     void __iomem    *pmecc_index_of;
> +
> +     /* data for pmecc computation */
> +     int16_t pmecc_smu[(CONFIG_PMECC_CAP + 2) * (2 * CONFIG_PMECC_CAP + 1)];
> +     int16_t pmecc_partial_syn[2 * CONFIG_PMECC_CAP + 1];
> +     int16_t pmecc_si[2 * CONFIG_PMECC_CAP + 1];
> +     int16_t pmecc_lmu[CONFIG_PMECC_CAP + 1]; /* polynomal order */
> +     int     pmecc_mu[CONFIG_PMECC_CAP + 1];
> +     int     pmecc_dmu[CONFIG_PMECC_CAP + 1];
> +     int     pmecc_delta[CONFIG_PMECC_CAP + 1];

can you please add some README entry describing these new config parameters?
Namely CONFIG_ATMEL_NAND_HW_PMECC, CONFIG_PMECC_CAP,
CONFIG_PMECC_SECTOR_SIZE (can't this be derived from some already
available NAND information?) and CONFIG_PMECC_INDEX_TABLE_OFFSET.

> +};
> +
> +static struct atmel_nand_host pmecc_host;
> +static struct nand_ecclayout atmel_pmecc_oobinfo;
> +
> +/*
> + * Return number of ecc bytes per sector according to sector size and
> + * correction capability
> + *
> + * Following table shows what at91 PMECC supported:
> + * Correction Capability     Sector_512_bytes        Sector_1024_bytes
> + * =====================     ================        =================
> + *                2-bits                 4-bytes                  4-bytes
> + *                4-bits                 7-bytes                  7-bytes
> + *                8-bits                13-bytes                 14-bytes
> + *               12-bits                20-bytes                 21-bytes
> + *               24-bits                39-bytes                 42-bytes
> + */
> +static int pmecc_get_ecc_bytes(int cap, int sector_size)
> +{
> +     int m = 12 + sector_size / 512;
> +     return (m * cap + 7) / 8;
> +}
> +
> +static void pmecc_config_ecc_layout(struct nand_ecclayout *layout,
> +     int oobsize, int ecc_len)
> +{
> +     int i;
> +
> +     layout->eccbytes = ecc_len;
> +
> +     /* ECC will occupy the last ecc_len bytes continuously */
> +     for (i = 0; i < ecc_len; i++)
> +             layout->eccpos[i] = oobsize - ecc_len + i;
> +
> +     layout->oobfree[0].offset = 2;
> +     layout->oobfree[0].length =
> +             oobsize - ecc_len - layout->oobfree[0].offset;
> +}
> +
> +static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
> +{
> +     int table_size;
> +
> +     table_size = host->pmecc_sector_size == 512 ?
> +             PMECC_INDEX_TABLE_SIZE_512 : PMECC_INDEX_TABLE_SIZE_1024;
> +
> +     /* the ALPHA lookup table is right behind the INDEX lookup table. */
> +     return host->pmecc_rom_base + host->pmecc_index_table_offset +
> +                     table_size * sizeof(int16_t);
> +}
> +
> +static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
> +{
> +     struct nand_chip *nand_chip = mtd->priv;
> +     struct atmel_nand_host *host = nand_chip->priv;
> +     int i;
> +     uint32_t value;
> +
> +     /* Fill odd syndromes */
> +     for (i = 0; i < host->pmecc_corr_cap; i++) {
> +             value = readl(&host->pmecc->rem_port[sector].rem[i / 2]);
> +             if (i & 1)
> +                     value >>= 16;
> +             value &= 0xffff;
> +             host->pmecc_partial_syn[(2 * i) + 1] = (int16_t)value;
> +     }
> +}
> +
> +static void pmecc_substitute(struct mtd_info *mtd)
> +{
> +     struct nand_chip *nand_chip = mtd->priv;
> +     struct atmel_nand_host *host = nand_chip->priv;
> +     int16_t __iomem *alpha_to = host->pmecc_alpha_to;
> +     int16_t __iomem *index_of = host->pmecc_index_of;
> +     int16_t *partial_syn = host->pmecc_partial_syn;
> +     const int cap = host->pmecc_corr_cap;
> +     int16_t *si;
> +     int i, j;
> +
> +     /* si[] is a table that holds the current syndrome value,
> +      * an element of that table belongs to the field
> +      */
> +     si = host->pmecc_si;
> +
> +     memset(&si[1], 0, sizeof(int16_t) * (2 * cap - 1));
> +
> +     /* Computation 2t syndromes based on S(x) */
> +     /* Odd syndromes */
> +     for (i = 1; i < 2 * cap; i += 2) {
> +             for (j = 0; j < host->pmecc_degree; j++) {
> +                     if (partial_syn[i] & ((unsigned short)0x1 << j))
> +                             si[i] = readw(alpha_to + i * j) ^ si[i];
> +             }
> +     }
> +     /* Even syndrome = (Odd syndrome) ** 2 */
> +     for (i = 2, j = 1; j <= cap; i = ++j << 1) {
> +             if (si[j] == 0) {
> +                     si[i] = 0;
> +             } else {
> +                     int16_t tmp;
> +
> +                     tmp = readw(index_of + si[j]);
> +                     tmp = (tmp * 2) % host->pmecc_cw_len;
> +                     si[i] = readw(alpha_to + tmp);
> +             }
> +     }
> +
> +     return;
> +}
> +
> +static void pmecc_get_sigma(struct mtd_info *mtd)
> +{
> +     struct nand_chip *nand_chip = mtd->priv;
> +     struct atmel_nand_host *host = nand_chip->priv;
> +
> +     int16_t *lmu = host->pmecc_lmu;
> +     int16_t *si = host->pmecc_si;
> +     int *mu = host->pmecc_mu;
> +     int *dmu = host->pmecc_dmu;     /* Discrepancy */
> +     int *delta = host->pmecc_delta; /* Delta order */
> +     int cw_len = host->pmecc_cw_len;
> +     const int16_t cap = host->pmecc_corr_cap;
> +     const int num = 2 * cap + 1;
> +     int16_t __iomem *index_of = host->pmecc_index_of;
> +     int16_t __iomem *alpha_to = host->pmecc_alpha_to;
> +     int i, j, k;
> +     uint32_t dmu_0_count, tmp;
> +     int16_t *smu = host->pmecc_smu;
> +
> +     /* index of largest delta */
> +     int ro;
> +     int largest;
> +     int diff;
> +
> +     dmu_0_count = 0;
> +
> +     /* First Row */
> +
> +     /* Mu */
> +     mu[0] = -1;
> +
> +     memset(smu, 0, sizeof(int16_t) * num);
> +     smu[0] = 1;
> +
> +     /* discrepancy set to 1 */
> +     dmu[0] = 1;
> +     /* polynom order set to 0 */
> +     lmu[0] = 0;
> +     delta[0] = (mu[0] * 2 - lmu[0]) >> 1;
---------------------^
isn't mu[0] -1 here
> +
> +     /* Second Row */
> +
> +     /* Mu */
> +     mu[1] = 0;
> +     /* Sigma(x) set to 1 */
> +     memset(&smu[num], 0, sizeof(int16_t) * num);
> +     smu[num] = 1;
> +
> +     /* discrepancy set to S1 */
> +     dmu[1] = si[1];
> +
> +     /* polynom order set to 0 */
> +     lmu[1] = 0;
> +
> +     delta[1] = (mu[1] * 2 - lmu[1]) >> 1;
----------------------^
isn't mu[1] 0 here?

I see, this algorithm is just copied from the processors datasheet.

> +
> +     /* Init the Sigma(x) last row */
> +     memset(&smu[(cap + 1) * num], 0, sizeof(int16_t) * num);

Isn't the whole smu zeroed here at this stage? why not just call
memset(smu, 0, sizeof(int16) * ARRAY_SIZE(smu))?

> +
> +     for (i = 1; i <= cap; i++) {
> +             mu[i + 1] = i << 1;
> +             /* Begin Computing Sigma (Mu+1) and L(mu) */
> +             /* check if discrepancy is set to 0 */
> +             if (dmu[i] == 0) {
> +                     dmu_0_count++;
> +
> +                     tmp = ((cap - (lmu[i] >> 1) - 1) / 2);
> +                     if ((cap - (lmu[i] >> 1) - 1) & 0x1)
> +                             tmp += 2;
> +                     else
> +                             tmp += 1;
> +
> +                     if (dmu_0_count == tmp) {
> +                             for (j = 0; j <= (lmu[i] >> 1) + 1; j++)
> +                                     smu[(cap + 1) * num + j] =
> +                                                     smu[i * num + j];
> +
> +                             lmu[cap + 1] = lmu[i];
> +                             return;
> +                     }
> +
> +                     /* copy polynom */
> +                     for (j = 0; j <= lmu[i] >> 1; j++)
> +                             smu[(i + 1) * num + j] = smu[i * num + j];
> +
> +                     /* copy previous polynom order to the next */
> +                     lmu[i + 1] = lmu[i];

Horrible to read, this was written first by an electrical engineer
(insider joke)? Can you please add a comment at top of pmecc_get_sigma
and add a warning about extremely complicated to read algorithm ;) But
it must be right cause it is written down in the datasheet ...

> +             } else {
> +                     ro = 0;
> +                     largest = -1;
> +                     /* find largest delta with dmu != 0 */
> +                     for (j = 0; j < i; j++) {
> +                             if ((dmu[j]) && (delta[j] > largest)) {
> +                                     largest = delta[j];
> +                                     ro = j;
> +                             }
> +                     }
> +
> +                     /* compute difference */
> +                     diff = (mu[i] - mu[ro]);
> +
> +                     /* Compute degree of the new smu polynomial */
> +                     if ((lmu[i] >> 1) > ((lmu[ro] >> 1) + diff))
> +                             lmu[i + 1] = lmu[i];
> +                     else
> +                             lmu[i + 1] = ((lmu[ro] >> 1) + diff) * 2;
> +
> +                     /* Init smu[i+1] with 0 */
> +                     for (k = 0; k < num; k++)
> +                             smu[(i + 1) * num + k] = 0;
> +
> +                     /* Compute smu[i+1] */
> +                     for (k = 0; k <= lmu[ro] >> 1; k++) {
> +                             int16_t a, b, c;
> +
> +                             if (!(smu[ro * num + k] && dmu[i]))
> +                                     continue;
> +                             a = readw(index_of + dmu[i]);
> +                             b = readw(index_of + dmu[ro]);
> +                             c = readw(index_of + smu[ro * num + k]);
> +                             tmp = a + (cw_len - b) + c;
> +                             a = readw(alpha_to + tmp % cw_len);
> +                             smu[(i + 1) * num + (k + diff)] = a;
> +                     }
> +
> +                     for (k = 0; k <= lmu[i] >> 1; k++)
> +                             smu[(i + 1) * num + k] ^= smu[i * num + k];
> +             }
> +
> +             /* End Computing Sigma (Mu+1) and L(mu) */
> +             /* In either case compute delta */
> +             delta[i + 1] = (mu[i + 1] * 2 - lmu[i + 1]) >> 1;
> +
> +             /* Do not compute discrepancy for the last iteration */
> +             if (i >= cap)
> +                     continue;
> +
> +             for (k = 0; k <= (lmu[i + 1] >> 1); k++) {
> +                     tmp = 2 * (i - 1);
> +                     if (k == 0) {
> +                             dmu[i + 1] = si[tmp + 3];
> +                     } else if (smu[(i + 1) * num + k] && si[tmp + 3 - k]) {
> +                             int16_t a, b, c;
> +                             a = readw(index_of +
> +                                             smu[(i + 1) * num + k]);
> +                             b = si[2 * (i - 1) + 3 - k];
> +                             c = readw(index_of + b);
> +                             tmp = a + c;
> +                             tmp %= cw_len;
> +                             dmu[i + 1] = readw(alpha_to + tmp) ^
> +                                     dmu[i + 1];
> +                     }
> +             }
> +     }
> +
> +     return;
> +}
> +
> +static int pmecc_err_location(struct mtd_info *mtd)
> +{
> +     struct nand_chip *nand_chip = mtd->priv;
> +     struct atmel_nand_host *host = nand_chip->priv;
> +     const int cap = host->pmecc_corr_cap;
> +     const int num = 2 * cap + 1;
> +     int sector_size = host->pmecc_sector_size;
> +     int err_nbr = 0;        /* number of error */
> +     int roots_nbr;          /* number of roots */
> +     int i;
> +     uint32_t val;
> +     int16_t *smu = host->pmecc_smu;
> +
> +     writel(PMERRLOC_DISABLE, &host->pmerrloc->eldis);
> +
> +     for (i = 0; i <= host->pmecc_lmu[cap + 1] >> 1; i++) {
> +             writel(smu[(cap + 1) * num + i], &host->pmerrloc->sigma[i]);
> +             err_nbr++;
> +     }
> +
> +     val = PMERRLOC_ELCFG_NUM_ERRORS(err_nbr - 1);
> +     if (sector_size == 1024)
> +             val |= PMERRLOC_ELCFG_SECTOR_1024;
> +
> +     writel(val, &host->pmerrloc->elcfg);
> +     writel(sector_size * 8 + host->pmecc_degree * cap,
> +                     &host->pmerrloc->elen);
> +
> +     while (!(readl(&host->pmerrloc->elisr) & PMERRLOC_CALC_DONE))
> +             udelay(10);

Can you please add some timeout here. Maybe a WATCHDOG_RESET is also
required?

> +
> +     roots_nbr = (readl(&host->pmerrloc->elisr) & PMERRLOC_ERR_NUM_MASK)
> +                     >> 8;
> +     /* Number of roots == degree of smu hence <= cap */
> +     if (roots_nbr == host->pmecc_lmu[cap + 1] >> 1)
> +             return err_nbr - 1;
> +
> +     /* Number of roots does not match the degree of smu
> +      * unable to correct error */
> +     return -1;
> +}
> +
> +static void pmecc_correct_data(struct mtd_info *mtd, uint8_t *buf, uint8_t 
> *ecc,
> +             int sector_num, int extra_bytes, int err_nbr)
> +{
> +     struct nand_chip *nand_chip = mtd->priv;
> +     struct atmel_nand_host *host = nand_chip->priv;
> +     int i = 0;
> +     int byte_pos, bit_pos, sector_size, pos;
> +     uint32_t tmp;
> +     uint8_t err_byte;
> +
> +     sector_size = host->pmecc_sector_size;
> +
> +     while (err_nbr) {
> +             tmp = readl(&host->pmerrloc->el[i]) - 1;
> +             byte_pos = tmp / 8;
> +             bit_pos  = tmp % 8;
> +
> +             if (byte_pos >= (sector_size + extra_bytes))
> +                     BUG();  /* should never happen */
> +
> +             if (byte_pos < sector_size) {
> +                     err_byte = *(buf + byte_pos);
> +                     *(buf + byte_pos) ^= (1 << bit_pos);
> +
> +                     pos = sector_num * host->pmecc_sector_size + byte_pos;
> +                     printk(KERN_INFO "Bit flip in data area, byte_pos: %d, 
> bit_pos: %d, 0x%02x -> 0x%02x\n",
> +                             pos, bit_pos, err_byte, *(buf + byte_pos));
> +             } else {
> +                     /* Bit flip in OOB area */
> +                     tmp = sector_num * host->pmecc_bytes_per_sector
> +                                     + (byte_pos - sector_size);
> +                     err_byte = ecc[tmp];
> +                     ecc[tmp] ^= (1 << bit_pos);
> +
> +                     pos = tmp + nand_chip->ecc.layout->eccpos[0];
> +                     printk(KERN_INFO "Bit flip in OOB, oob_byte_pos: %d, 
> bit_pos: %d, 0x%02x -> 0x%02x\n",
> +                             pos, bit_pos, err_byte, ecc[tmp]);
> +             }
> +
> +             i++;
> +             err_nbr--;
> +     }
> +
> +     return;
> +}
> +
> +static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t 
> *buf,
> +     u8 *ecc)
> +{
> +     struct nand_chip *nand_chip = mtd->priv;
> +     struct atmel_nand_host *host = nand_chip->priv;
> +     int i, err_nbr, eccbytes;
> +     uint8_t *buf_pos;
> +
> +     eccbytes = nand_chip->ecc.bytes;
> +     for (i = 0; i < eccbytes; i++)
> +             if (ecc[i] != 0xff)
> +                     goto normal_check;
> +     /* Erased page, return OK */
> +     return 0;
> +
> +normal_check:
> +     for (i = 0; i < host->pmecc_sector_number; i++) {
> +             err_nbr = 0;
> +             if (pmecc_stat & 0x1) {
> +                     buf_pos = buf + i * host->pmecc_sector_size;
> +
> +                     pmecc_gen_syndrome(mtd, i);
> +                     pmecc_substitute(mtd);
> +                     pmecc_get_sigma(mtd);
> +
> +                     err_nbr = pmecc_err_location(mtd);
> +                     if (err_nbr == -1) {
> +                             printk(KERN_ERR "PMECC: Too many errors\n");
> +                             mtd->ecc_stats.failed++;
> +                             return -EIO;
> +                     } else {
> +                             pmecc_correct_data(mtd, buf_pos, ecc, i,
> +                                     host->pmecc_bytes_per_sector, err_nbr);
> +                             mtd->ecc_stats.corrected += err_nbr;
> +                     }
> +             }
> +             pmecc_stat >>= 1;
> +     }
> +
> +     return 0;
> +}
> +
> +static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
> +     struct nand_chip *chip, uint8_t *buf, int page)
> +{
> +     struct atmel_nand_host *host = chip->priv;
> +     int eccsize = chip->ecc.size;
> +     uint8_t *oob = chip->oob_poi;
> +     uint32_t *eccpos = chip->ecc.layout->eccpos;
> +     uint32_t stat;
> +
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_RST);
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_DISABLE);
> +     pmecc_writel(host->pmecc, cfg, ((pmecc_readl(host->pmecc, cfg))
> +             & ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
> +
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_ENABLE);
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_DATA);
> +
> +     chip->read_buf(mtd, buf, eccsize);
> +     chip->read_buf(mtd, oob, mtd->oobsize);
> +
> +     while ((pmecc_readl(host->pmecc, sr) & PMECC_SR_BUSY))
> +             udelay(1);
> +
> +     stat = pmecc_readl(host->pmecc, isr);
> +     if (stat != 0)
> +             if (pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]) != 0)
> +                     return -EIO;
> +
> +     return 0;
> +}
> +
> +static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
> +             struct nand_chip *chip, const uint8_t *buf)
> +{
> +     struct atmel_nand_host *host = chip->priv;
> +     uint32_t *eccpos = chip->ecc.layout->eccpos;
> +     int i, j;
> +
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_RST);
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_DISABLE);
> +
> +     pmecc_writel(host->pmecc, cfg, (pmecc_readl(host->pmecc, cfg) |
> +             PMECC_CFG_WRITE_OP) & ~PMECC_CFG_AUTO_ENABLE);
> +
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_ENABLE);
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_DATA);
> +
> +     chip->write_buf(mtd, (u8 *)buf, mtd->writesize);
> +
> +     while ((pmecc_readl(host->pmecc, sr) & PMECC_SR_BUSY))
> +             udelay(1);
> +
> +     for (i = 0; i < host->pmecc_sector_number; i++) {
> +             for (j = 0; j < host->pmecc_bytes_per_sector; j++) {
> +                     int pos;
> +
> +                     pos = i * host->pmecc_bytes_per_sector + j;
> +                     chip->oob_poi[eccpos[pos]] =
> +                             readb(&host->pmecc->ecc_port[i].ecc[j]);
> +             }
> +     }
> +     chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +}
> +
> +static void atmel_pmecc_core_init(struct mtd_info *mtd)
> +{
> +     struct nand_chip *nand_chip = mtd->priv;
> +     struct atmel_nand_host *host = nand_chip->priv;
> +     uint32_t val = 0;
> +     struct nand_ecclayout *ecc_layout;
> +
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_RST);
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_DISABLE);
> +
> +     switch (host->pmecc_corr_cap) {
> +     case 2:
> +             val = PMECC_CFG_BCH_ERR2;
> +             break;
> +     case 4:
> +             val = PMECC_CFG_BCH_ERR4;
> +             break;
> +     case 8:
> +             val = PMECC_CFG_BCH_ERR8;
> +             break;
> +     case 12:
> +             val = PMECC_CFG_BCH_ERR12;
> +             break;
> +     case 24:
> +             val = PMECC_CFG_BCH_ERR24;
> +             break;
> +     }
> +
> +     if (host->pmecc_sector_size == 512)
> +             val |= PMECC_CFG_SECTOR512;
> +     else if (host->pmecc_sector_size == 1024)
> +             val |= PMECC_CFG_SECTOR1024;
> +
> +     switch (host->pmecc_sector_number) {
> +     case 1:
> +             val |= PMECC_CFG_PAGE_1SECTOR;
> +             break;
> +     case 2:
> +             val |= PMECC_CFG_PAGE_2SECTORS;
> +             break;
> +     case 4:
> +             val |= PMECC_CFG_PAGE_4SECTORS;
> +             break;
> +     case 8:
> +             val |= PMECC_CFG_PAGE_8SECTORS;
> +             break;
> +     }
> +
> +     val |= (PMECC_CFG_READ_OP | PMECC_CFG_SPARE_DISABLE
> +             | PMECC_CFG_AUTO_DISABLE);
> +     pmecc_writel(host->pmecc, cfg, val);
> +
> +     ecc_layout = nand_chip->ecc.layout;
> +     pmecc_writel(host->pmecc, sarea, mtd->oobsize - 1);
> +     pmecc_writel(host->pmecc, saddr, ecc_layout->eccpos[0]);
> +     pmecc_writel(host->pmecc, eaddr,
> +                     ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
> +     /* See datasheet about PMECC Clock Control Register */
> +     pmecc_writel(host->pmecc, clk, PMECC_CLK_133MHZ);
> +     pmecc_writel(host->pmecc, idr, 0xff);
> +     pmecc_writel(host->pmecc, ctrl, PMECC_CTRL_ENABLE);
> +}
> +
> +static int atmel_pmecc_nand_init_params(struct nand_chip *nand)
> +{
> +     struct mtd_info *mtd;
> +     struct atmel_nand_host *host;
> +     int cap, sector_size;
> +
> +     mtd = &nand_info[chip_nr++];

NAK

> +     mtd->priv = nand;
> +     host = nand->priv = &pmecc_host;
> +
> +     /* Detect NAND chips */
> +     if (nand_scan_ident(mtd, 1, NULL)) {

please change that, read later on.

> +             printk(KERN_WARNING "NAND Flash not found !\n");
> +             return -ENXIO;
> +     }
> +
> +     nand->ecc.mode = NAND_ECC_HW;
> +     nand->ecc.calculate = NULL;
> +     nand->ecc.correct = NULL;
> +     nand->ecc.hwctl = NULL;
> +
> +     cap = host->pmecc_corr_cap = CONFIG_PMECC_CAP;
> +     sector_size = host->pmecc_sector_size = CONFIG_PMECC_SECTOR_SIZE;

Can't we use some information from the nand chip here? Do we need to
hard code this paramater?

> +     host->pmecc_index_table_offset = CONFIG_PMECC_INDEX_TABLE_OFFSET;
> +
> +     printk(KERN_INFO "Initialize PMECC params, cap: %d, sector: %d\n",
> +              cap, sector_size);
> +
> +     host->pmecc = (struct pmecc_regs __iomem *) ATMEL_BASE_PMECC;
> +     host->pmerrloc = (struct pmecc_errloc_regs __iomem *)
> +                     ATMEL_BASE_PMERRLOC;
> +     host->pmecc_rom_base = (void __iomem *) ATMEL_BASE_ROM;
> +
> +     /* ECC is calculated for the whole page (1 step) */
> +     nand->ecc.size = mtd->writesize;
> +
> +     /* set ECC page size and oob layout */
> +     switch (mtd->writesize) {
> +     case 2048:
> +     case 4096:
> +             host->pmecc_degree = PMECC_GF_DIMENSION_13;
> +             host->pmecc_cw_len = (1 << host->pmecc_degree) - 1;
> +             host->pmecc_sector_number = mtd->writesize / sector_size;
> +             host->pmecc_bytes_per_sector = pmecc_get_ecc_bytes(
> +                     cap, sector_size);
> +             host->pmecc_alpha_to = pmecc_get_alpha_to(host);
> +             host->pmecc_index_of = host->pmecc_rom_base +
> +                     host->pmecc_index_table_offset;
> +
> +             nand->ecc.steps = 1;
> +             nand->ecc.bytes = host->pmecc_bytes_per_sector *
> +                                    host->pmecc_sector_number;
> +             if (nand->ecc.bytes > mtd->oobsize - 2) {
> +                     printk(KERN_ERR "No room for ECC bytes\n");
> +                     return -EINVAL;
> +             }
> +             pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
> +                                     mtd->oobsize,
> +                                     nand->ecc.bytes);
> +             nand->ecc.layout = &atmel_pmecc_oobinfo;
> +             break;
> +     case 512:
> +     case 1024:
> +             /* TODO */
> +             printk(KERN_ERR "Unsupported page size for PMECC, use Software 
> ECC\n");
> +     default:
> +             /* page size not handled by HW ECC */
> +             /* switching back to soft ECC */
> +             nand->ecc.mode = NAND_ECC_SOFT;
> +             nand->ecc.read_page = NULL;
> +             nand->ecc.postpad = 0;
> +             nand->ecc.prepad = 0;
> +             nand->ecc.bytes = 0;
> +             return 0;
> +     }
> +
> +     nand->ecc.read_page = atmel_nand_pmecc_read_page;
> +     nand->ecc.write_page = atmel_nand_pmecc_write_page;
> +
> +     atmel_pmecc_core_init(mtd);
> +
> +     return 0;
> +}
> +
> +#else
> +
>  /* oob layout for large page size
>   * bad block info is on bytes 0 and 1
>   * the bytes have to be consecutives to avoid
> @@ -234,7 +879,6 @@ static void atmel_nand_hwctl(struct mtd_info *mtd, int 
> mode)
>  
>  int atmel_hw_nand_init_param(struct nand_chip *nand)
>  {
> -     static int chip_nr = 0;
>       struct mtd_info *mtd;
>  
>       nand->ecc.mode = NAND_ECC_HW;
> @@ -293,7 +937,9 @@ int atmel_hw_nand_init_param(struct nand_chip *nand)
>       return 0;
>  }
>  
> -#endif
> +#endif /* CONFIG_ATMEL_NAND_HW_PMECC */
> +
> +#endif /* CONFIG_ATMEL_NAND_HWECC */
>  
>  static void at91_nand_hwcontrol(struct mtd_info *mtd,
>                                        int cmd, unsigned int ctrl)
> @@ -343,8 +989,12 @@ int board_nand_init(struct nand_chip *nand)
>       nand->chip_delay = 20;
>  
>  #ifdef CONFIG_ATMEL_NAND_HWECC
> +#ifdef CONFIG_ATMEL_NAND_HW_PMECC
> +     res = atmel_pmecc_nand_init_params(nand);
> +#else
>       res = atmel_hw_nand_init_param(nand);
>  #endif
> +#endif
>  
>       return res;
>  }
> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h 
> b/drivers/mtd/nand/atmel_nand_ecc.h
> index 1ee7f99..8f06b14 100644
> --- a/drivers/mtd/nand/atmel_nand_ecc.h
> +++ b/drivers/mtd/nand/atmel_nand_ecc.h
> @@ -33,4 +33,115 @@
>  #define ATMEL_ECC_NPR                0x10                    /* NParity 
> register */
>  #define              ATMEL_ECC_NPARITY       (0xffff << 0)           /* 
> NParity */
>  
> +/* Register access macros for PMECC */
> +#define pmecc_readl(addr, reg) \
> +     readl(&addr->reg)
> +
> +#define pmecc_writel(addr, reg, value) \
> +     writel((value), &addr->reg)
> +
> +/* PMECC Register Definitions */
> +#define PMECC_MAX_SECTOR_NUM                 8
> +struct pmecc_regs {
> +     u32 cfg;                /* 0x00 PMECC Configuration Register */
> +     u32 sarea;              /* 0x04 PMECC Spare Area Size Register */
> +     u32 saddr;              /* 0x08 PMECC Start Address Register */
> +     u32 eaddr;              /* 0x0C PMECC End Address Register */
> +     u32 clk;                /* 0x10 PMECC Clock Control Register */
> +     u32 ctrl;               /* 0x14 PMECC Control Register */
> +     u32 sr;                 /* 0x18 PMECC Status Register */
> +     u32 ier;                /* 0x1C PMECC Interrupt Enable Register */
> +     u32 idr;                /* 0x20 PMECC Interrupt Disable Register */
> +     u32 imr;                /* 0x24 PMECC Interrupt Mask Register */
> +     u32 isr;                /* 0x28 PMECC Interrupt Status Register */
> +     u32 reserved0[5];       /* 0x2C-0x3C Reserved */
> +
> +     /* 0x40 + sector_num * (0x40), Redundancy Registers */
> +     struct {
> +             u8 ecc[44];     /* PMECC Generated Redundancy Byte Per Sector */
> +             u32 reserved1[5];
> +     } ecc_port[PMECC_MAX_SECTOR_NUM];
> +
> +     /* 0x240 + sector_num * (0x40) Remainder Registers */
> +     struct {
> +             u32 rem[12];
> +             u32 reserved2[4];
> +     } rem_port[PMECC_MAX_SECTOR_NUM];
> +     u32 reserved3[16];      /* 0x440-0x47C Reserved */
> +};
> +
> +/* For PMECC Configuration Register */
> +#define              PMECC_CFG_BCH_ERR2              (0 << 0)
> +#define              PMECC_CFG_BCH_ERR4              (1 << 0)
> +#define              PMECC_CFG_BCH_ERR8              (2 << 0)
> +#define              PMECC_CFG_BCH_ERR12             (3 << 0)
> +#define              PMECC_CFG_BCH_ERR24             (4 << 0)
> +
> +#define              PMECC_CFG_SECTOR512             (0 << 4)
> +#define              PMECC_CFG_SECTOR1024            (1 << 4)
> +
> +#define              PMECC_CFG_PAGE_1SECTOR          (0 << 8)
> +#define              PMECC_CFG_PAGE_2SECTORS         (1 << 8)
> +#define              PMECC_CFG_PAGE_4SECTORS         (2 << 8)
> +#define              PMECC_CFG_PAGE_8SECTORS         (3 << 8)
> +
> +#define              PMECC_CFG_READ_OP               (0 << 12)
> +#define              PMECC_CFG_WRITE_OP              (1 << 12)
> +
> +#define              PMECC_CFG_SPARE_ENABLE          (1 << 16)
> +#define              PMECC_CFG_SPARE_DISABLE         (0 << 16)
> +
> +#define              PMECC_CFG_AUTO_ENABLE           (1 << 20)
> +#define              PMECC_CFG_AUTO_DISABLE          (0 << 20)
> +
> +/* For PMECC Clock Control Register */
> +#define              PMECC_CLK_133MHZ                (2 << 0)
> +
> +/* For PMECC Control Register */
> +#define              PMECC_CTRL_RST                  (1 << 0)
> +#define              PMECC_CTRL_DATA                 (1 << 1)
> +#define              PMECC_CTRL_USER                 (1 << 2)
> +#define              PMECC_CTRL_ENABLE               (1 << 4)
> +#define              PMECC_CTRL_DISABLE              (1 << 5)
> +
> +/* For PMECC Status Register */
> +#define              PMECC_SR_BUSY                   (1 << 0)
> +#define              PMECC_SR_ENABLE                 (1 << 4)
> +
> +/* PMERRLOC Register Definitions */
> +struct pmecc_errloc_regs {
> +     u32 elcfg;      /* 0x00 Error Location Configuration Register */
> +     u32 elprim;     /* 0x04 Error Location Primitive Register */
> +     u32 elen;       /* 0x08 Error Location Enable Register */
> +     u32 eldis;      /* 0x0C Error Location Disable Register */
> +     u32 elsr;       /* 0x10 Error Location Status Register */
> +     u32 elier;      /* 0x14 Error Location Interrupt Enable Register */
> +     u32 elidr;      /* 0x08 Error Location Interrupt Disable Register */
> +     u32 elimr;      /* 0x0C Error Location Interrupt Mask Register */
> +     u32 elisr;      /* 0x20 Error Location Interrupt Status Register */
> +     u32 reserved0;  /* 0x24 Reserved */
> +     u32 sigma[25];  /* 0x28-0x88 Error Location Sigma Registers */
> +     u32 el[24];     /* 0x8C-0xE8 Error Location Registers */
> +     u32 reserved1[5];       /* 0xEC-0xFC Reserved */
> +};
> +
> +/* For Error Location Configuration Register */
> +#define              PMERRLOC_ELCFG_SECTOR_512       (0 << 0)
> +#define              PMERRLOC_ELCFG_SECTOR_1024      (1 << 0)
> +#define              PMERRLOC_ELCFG_NUM_ERRORS(n)    ((n) << 16)
> +
> +/* For Error Location Disable Register */
> +#define              PMERRLOC_DISABLE                (1 << 0)
> +
> +/* For Error Location Interrupt Status Register */
> +#define              PMERRLOC_ERR_NUM_MASK           (0x1f << 8)
> +#define              PMERRLOC_CALC_DONE              (1 << 0)
> +
> +/* Galois field dimension */
> +#define PMECC_GF_DIMENSION_13                        13
> +#define PMECC_GF_DIMENSION_14                        14
> +
> +#define PMECC_INDEX_TABLE_SIZE_512           0x2000
> +#define PMECC_INDEX_TABLE_SIZE_1024          0x4000
> +
>  #endif
> 

Not really nice code but it seems the kernel guys accepted it the same
way. So I will _not_ NAK it. But please add at least a README entry
describing the new config parameters, think about the sector size (ore
explain me why you need another config) and please change the driver
initialization to the SELF_INIT as mentioned in another mail. You use
again the nand_scan_indent() inside your xx_nand_init_params().
Please rewrite the board_nand_init() as shown in doc/README.nand, maybe
copy the stub from mtd/nand.c's nand_init() and nand_init_chip(). Call
in your own atmel_nand_init_chip() nand_scan_ident() and after that
setup the parameters for eighter ECC or PMECC. Always hand over the
struct nand_chip and do not work with the nand_chip array in the
xx_init_param() functions.

Best regards

Andreas Bießmann
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to