MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

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 this driver, the user needs to pass in the correction capability and the sector size.

This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2, YAFFS2, UBIFS and mtd-utils.

Signed-off-by Hong Xu <[email protected]>

Signed-off-by Josh Wu <[email protected]>

---
Hi,

Since Hong Xu will not work on pmecc part. so I send out this patch base Hong Xu's latest work.

Changes since v4,
fix typo and checkpatch warnings.
fix according to JC's suggestion. replace cpu_is_xxx() with DT modify dt binding atmel nand document to add pmecc support. tested in sam9263 without break hw ecc.
add ecc.strength.

 .../devicetree/bindings/mtd/atmel-nand.txt         |    5 +
 drivers/mtd/nand/atmel_nand.c                      |  955 ++++++++++++++++++--
 drivers/mtd/nand/atmel_nand_ecc.h                  |  123 ++-
 3 files changed, 1004 insertions(+), 79 deletions(-)

Documentation/devicetree/bindings/mtd/atmel-nand.txt +0 -0

index a200695..8936175 100644

Documentation/devicetree/bindings/mtd/atmel-nand.txt +3 -0

+- atmel,sector-size : sector size for ECC computation. Supported values are:
+  512, 1024.
 - nand-bus-width : 8 or 16 bus width if not present 8
 - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
 
+
 Examples:
 nand0: nand at 40000000,0 {
 	compatible = "atmel,at91rm9200-nand";

Documentation/devicetree/bindings/mtd/atmel-nand.txt +127 -0

+
+	cap = host->correction_cap;
+	sector_size = host->sector_size;
+	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
+		 cap, sector_size);
+
+	/* Sanity check */
+	if ((sector_size != 512) && (sector_size != 1024)) {
+		dev_err(host->dev, "Unsupported PMECC sector size: %d; " \
+			"Valid sector size: 512 or 1024 bytes\n", sector_size);
+		return -EINVAL;
+	}
+	if ((cap != 2) && (cap != 4) && (cap != 8) && (cap != 12) &&
+	    (cap != 24)) {
+		dev_err(host->dev, "Unsupported PMECC correction capability," \
+			" Valid one is either 2, 4, 8, 12 or 24\n");
+		return -EINVAL;
+	}
+
+	nand_chip = &host->nand_chip;
+	mtd = &host->mtd;
+
+	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
+
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!regs) {
+		dev_warn(host->dev, "Can't get I/O resource regs, " \
+				"rolling back on software ECC\n");
+		return 0;
+	}
+
+	host->ecc = ioremap(regs->start, resource_size(regs));
+	if (host->ecc == NULL) {
+		dev_err(host->dev, "ioremap failed\n");
+		err_no = -EIO;
+		goto err_pmecc_ioremap;
+	}
+
+	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (regs_pmerr && regs_rom) {
+		host->pmerrloc_base = ioremap(regs_pmerr->start,
+			resource_size(regs_pmerr));
+		host->rom_base = ioremap(regs_rom->start,
+			resource_size(regs_rom));
+
+		if (host->pmerrloc_base && host->rom_base) {
+			nand_chip->ecc.mode = NAND_ECC_HW;
+			nand_chip->ecc.read_page =
+				atmel_nand_pmecc_read_page;
+			nand_chip->ecc.write_page =
+				atmel_nand_pmecc_write_page;
+		} else {
+			dev_err(host->dev, "Can not get I/O resource" \
+				" for HW PMECC controller!\n");
+			err_no = -EIO;
+			goto err_pmloc_ioremap;
+		}
+	}
+
+	/* ECC is calculated for the whole page (1 step) */
+	nand_chip->ecc.size = mtd->writesize;
+
+	/* set ECC page size and oob layout */
+	switch (mtd->writesize) {
+	case 2048:
+		host->degree = GF_DIMENSION_13;
+		host->cw_len = (1 << host->degree) - 1;
+		host->cap = cap;
+		host->sector_number = mtd->writesize / sector_size;
+		host->ecc_bytes_per_sector = pmecc_get_ecc_bytes(
+			host->cap, sector_size);
+		host->alpha_to = pmecc_get_alpha_to(host);
+		host->index_of = pmecc_get_index_of(host);
+
+		nand_chip->ecc.steps = 1;
+		nand_chip->ecc.strength = cap;
+		nand_chip->ecc.bytes = host->ecc_bytes_per_sector *
+				       host->sector_number;
+		if (nand_chip->ecc.bytes > mtd->oobsize - 2) {
+			dev_err(host->dev, "No room for ECC bytes\n");
+			err_no = -EINVAL;
+			goto err;
+		}
+		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
+					mtd->oobsize,
+					nand_chip->ecc.bytes);
+		nand_chip->ecc.layout = &atmel_pmecc_oobinfo;
+		break;
+	case 512:
+	case 1024:
+	case 4096:
+		/* TODO */
+		dev_warn(host->dev, "Only 2048 page size is currently " \
+			"supported for PMECC, rolling back to Software ECC\n");
+	default:
+		/* page size not handled by HW ECC */
+		/* switching back to soft ECC */
+		nand_chip->ecc.mode = NAND_ECC_SOFT;
+		nand_chip->ecc.calculate = NULL;
+		nand_chip->ecc.correct = NULL;
+		nand_chip->ecc.hwctl = NULL;
+		nand_chip->ecc.read_page = NULL;
+		nand_chip->ecc.write_page = NULL;
+		nand_chip->ecc.postpad = 0;
+		nand_chip->ecc.prepad = 0;
+		nand_chip->ecc.bytes = 0;
+		err_no = 0;
+		goto err;
+	}
+
+	atmel_pmecc_core_init(mtd);
+
+	return 0;
+
+err:
+err_pmloc_ioremap:
+	iounmap(host->ecc);
+	if (host->pmerrloc_base)
+		iounmap(host->pmerrloc_base);
+	if (host->rom_base)
+		iounmap(host->rom_base);
+err_pmecc_ioremap:
+	return err_no;
+}
+
+/*
  * Calculate HW ECC
  *
  * function called after a write

Quoted 35 lines · Josh Wu

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 this driver, the user needs to pass in the correction capability and the sector size.

This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2, YAFFS2, UBIFS and mtd-utils.

Signed-off-by Hong Xu<[email protected]>

Signed-off-by Josh Wu<[email protected]>

---
Hi,

Since Hong Xu will not work on pmecc part. so I send out this patch base Hong Xu's latest work.

Changes since v4,
fix typo and checkpatch warnings.
fix according to JC's suggestion. replace cpu_is_xxx() with DT modify dt binding atmel nand document to add pmecc support. tested in sam9263 without break hw ecc.
add ecc.strength.

  .../devicetree/bindings/mtd/atmel-nand.txt         |    5 +
  drivers/mtd/nand/atmel_nand.c                      |  955 ++++++++++++++++++--
  drivers/mtd/nand/atmel_nand_ecc.h                  |  123 ++-
  3 files changed, 1004 insertions(+), 79 deletions(-)

Documentation/devicetree/bindings/mtd/atmel-nand.txt +0 -0

index a200695..8936175 100644

Documentation/devicetree/bindings/mtd/atmel-nand.txt Optional properties: · line 20

Quoted 5 lines

Documentation/devicetree/bindings/mtd/atmel-nand.txt +2 -0

@@ -16,9 +16,14 @@ Optional properties:
  - nand-ecc-mode : String, operation mode of the NAND ecc mode, soft by default.
    Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
    "soft_bch".
+- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
+  Controller. Supported values are: 2, 4, 8, 12, 24.

you need to use it in the code instead of hardcoding it so

Quoted 9 lines

Documentation/devicetree/bindings/mtd/atmel-nand.txt +3 -0

+- atmel,sector-size : sector size for ECC computation. Supported values are:
+  512, 1024.
  - nand-bus-width : 8 or 16 bus width if not present 8
  - nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
 
+
  Examples:
  nand0: nand at 40000000,0 {
  	compatible = "atmel,at91rm9200-nand";

<snip>

Quoted 133 lines

Documentation/devicetree/bindings/mtd/atmel-nand.txt +127 -0

+
+	cap = host->correction_cap;
+	sector_size = host->sector_size;
+	dev_info(host->dev, "Initialize PMECC params, cap: %d, sector: %d\n",
+		 cap, sector_size);
+
+	/* Sanity check */
+	if ((sector_size != 512)&&  (sector_size != 1024)) {
+		dev_err(host->dev, "Unsupported PMECC sector size: %d; " \
+			"Valid sector size: 512 or 1024 bytes\n", sector_size);
+		return -EINVAL;
+	}
+	if ((cap != 2)&&  (cap != 4)&&  (cap != 8)&&  (cap != 12)&&
+	    (cap != 24)) {
+		dev_err(host->dev, "Unsupported PMECC correction capability," \
+			" Valid one is either 2, 4, 8, 12 or 24\n");
+		return -EINVAL;
+	}
+
+	nand_chip =&host->nand_chip;
+	mtd =&host->mtd;
+
+	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* By default */
+
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!regs) {
+		dev_warn(host->dev, "Can't get I/O resource regs, " \
+				"rolling back on software ECC\n");
+		return 0;
+	}
+
+	host->ecc = ioremap(regs->start, resource_size(regs));
+	if (host->ecc == NULL) {
+		dev_err(host->dev, "ioremap failed\n");
+		err_no = -EIO;
+		goto err_pmecc_ioremap;
+	}
+
+	regs_pmerr = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	regs_rom = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (regs_pmerr&&  regs_rom) {
+		host->pmerrloc_base = ioremap(regs_pmerr->start,
+			resource_size(regs_pmerr));
+		host->rom_base = ioremap(regs_rom->start,
+			resource_size(regs_rom));
+
+		if (host->pmerrloc_base&&  host->rom_base) {
+			nand_chip->ecc.mode = NAND_ECC_HW;
+			nand_chip->ecc.read_page =
+				atmel_nand_pmecc_read_page;
+			nand_chip->ecc.write_page =
+				atmel_nand_pmecc_write_page;
+		} else {
+			dev_err(host->dev, "Can not get I/O resource" \
+				" for HW PMECC controller!\n");
+			err_no = -EIO;
+			goto err_pmloc_ioremap;
+		}
+	}
+
+	/* ECC is calculated for the whole page (1 step) */
+	nand_chip->ecc.size = mtd->writesize;
+
+	/* set ECC page size and oob layout */
+	switch (mtd->writesize) {
+	case 2048:
+		host->degree = GF_DIMENSION_13;
+		host->cw_len = (1<<  host->degree) - 1;
+		host->cap = cap;
+		host->sector_number = mtd->writesize / sector_size;
+		host->ecc_bytes_per_sector = pmecc_get_ecc_bytes(
+			host->cap, sector_size);
+		host->alpha_to = pmecc_get_alpha_to(host);
+		host->index_of = pmecc_get_index_of(host);
+
+		nand_chip->ecc.steps = 1;
+		nand_chip->ecc.strength = cap;
+		nand_chip->ecc.bytes = host->ecc_bytes_per_sector *
+				       host->sector_number;
+		if (nand_chip->ecc.bytes>  mtd->oobsize - 2) {
+			dev_err(host->dev, "No room for ECC bytes\n");
+			err_no = -EINVAL;
+			goto err;
+		}
+		pmecc_config_ecc_layout(&atmel_pmecc_oobinfo,
+					mtd->oobsize,
+					nand_chip->ecc.bytes);
+		nand_chip->ecc.layout =&atmel_pmecc_oobinfo;
+		break;
+	case 512:
+	case 1024:
+	case 4096:
+		/* TODO */
+		dev_warn(host->dev, "Only 2048 page size is currently " \
+			"supported for PMECC, rolling back to Software ECC\n");
+	default:
+		/* page size not handled by HW ECC */
+		/* switching back to soft ECC */
+		nand_chip->ecc.mode = NAND_ECC_SOFT;
+		nand_chip->ecc.calculate = NULL;
+		nand_chip->ecc.correct = NULL;
+		nand_chip->ecc.hwctl = NULL;
+		nand_chip->ecc.read_page = NULL;
+		nand_chip->ecc.write_page = NULL;
+		nand_chip->ecc.postpad = 0;
+		nand_chip->ecc.prepad = 0;
+		nand_chip->ecc.bytes = 0;
+		err_no = 0;
+		goto err;
+	}
+
+	atmel_pmecc_core_init(mtd);
+
+	return 0;
+
+err:
+err_pmloc_ioremap:
+	iounmap(host->ecc);
+	if (host->pmerrloc_base)
+		iounmap(host->pmerrloc_base);
+	if (host->rom_base)
+		iounmap(host->rom_base);
+err_pmecc_ioremap:
+	return err_no;
+}
+
+/*
   * Calculate HW ECC
   *
   * function called after a write

@@ -474,6 +1212,15 @@ static void atmel_nand_hwctl(struct mtd_info *mtd, int mode)
  }
 

Documentation/devicetree/bindings/mtd/atmel-nand.txt static void atmel_nand_hwctl(struct mtd_info *mtd, int mode) · line 1221

Quoted 8 lines

Documentation/devicetree/bindings/mtd/atmel-nand.txt +7 -0

  #if defined(CONFIG_OF)
+static int cpu_has_pmecc(void)
+{
+	unsigned long dt_root;
+
+	dt_root = of_get_flat_dt_root();
+	return of_flat_dt_is_compatible(dt_root, "atmel,at91sam9n12") ||
+		of_flat_dt_is_compatible(dt_root, "atmel,at91sam9x5");

you need to detect it at probe time

and you need to put the first compatbile only no need to put a list

Best Regards,
J.

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 this driver, the user needs to pass in the correction capability and the sector size.

This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2, YAFFS2, UBIFS and mtd-utils.

+ * ===================== ================ =================

+ *                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

+ *
+ */

drivers/mtd/nand/atmel_nand.c +31 -0

+static const int ecc_bytes_table[5][2] = {
+       {4, 4}, {7, 7}, {13, 14}, {20, 21}, {39, 42}
 };
 
 static int cpu_has_dma(void)

@@ -288,6 +333,699 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 }
 
 /*
+ * Return number of ecc bytes per sector according to sector size and
+ * correction capability
+ */
+static int pmecc_get_ecc_bytes(int cap, int sector_size)
+{
+       int i, j;
+
+       switch (cap) {
+       case 2:
+               i = 0;
+               break;
+       case 4:
+               i = 1;
+               break;
+       case 8:
+               i = 2;
+               break;
+       case 12:
+               i = 3;
+               break;
+       case 24:
+               i = 4;
+               break;
+       default:
+               BUG();
+       }
+
+       switch (sector_size) {
+       case 512:

drivers/mtd/nand/atmel_nand.c +5 -0

+static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
+       u8 *ecc)
+{
+       int                     i, err_nbr;
+       uint8_t                 *buf_pos;

Quoted 11 lines · Josh Wu

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 this driver, the user needs to pass in the correction capability and the sector size.

This driver has been tested on AT91SAM9X5-EK and AT91SAM9N12-EK with JFFS2, YAFFS2, UBIFS and mtd-utils.

(...)

Hello Josh,
Few comments below,

Quoted 46 lines

+ * ===================== ================ =================

+ *                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

+ *
+ */

drivers/mtd/nand/atmel_nand.c +31 -0

+static const int ecc_bytes_table[5][2] = {
+       {4, 4}, {7, 7}, {13, 14}, {20, 21}, {39, 42}
  };
 
  static int cpu_has_dma(void)

@@ -288,6 +333,699 @@ static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
  }
 
  /*
+ * Return number of ecc bytes per sector according to sector size and
+ * correction capability
+ */
+static int pmecc_get_ecc_bytes(int cap, int sector_size)
+{
+       int i, j;
+
+       switch (cap) {
+       case 2:
+               i = 0;
+               break;
+       case 4:
+               i = 1;
+               break;
+       case 8:
+               i = 2;
+               break;
+       case 12:
+               i = 3;
+               break;
+       case 24:
+               i = 4;
+               break;
+       default:
+               BUG();
+       }
+
+       switch (sector_size) {
+       case 512:

drivers/mtd/nand/atmel_nand.c static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len) · line 374

Quoted 10 lines

drivers/mtd/nand/atmel_nand.c +10 -0

+               j = 0;
+               break;
+       case 1024:
+               j = 1;
+               break;
+       default:
+               BUG();
+       }
+
+       return ecc_bytes_table[i][j];

You could get rid of your table and simplify your function:

       int m;

       m = 12+sector_size/512;
       return (m*cap+7)/8;

Quoted 4 lines

drivers/mtd/nand/atmel_nand.c +3 -0

+static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
+{
+       void __iomem *p;

drivers/mtd/nand/atmel_nand.c static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len) · line 404

Quoted 10 lines

drivers/mtd/nand/atmel_nand.c +10 -0

+
+       switch (host->sector_size) {
+       case 512:
+               p = host->rom_base + AT_PMECC_LOOKUP_TABLE_OFFSET_512 +
+                       AT_PMECC_LOOKUP_TABLE_SIZE_512 * sizeof(int16_t);
+               break;
+       case 1024:
+               p = host->rom_base + AT_PMECC_LOOKUP_TABLE_OFFSET_1024 +
+                       AT_PMECC_LOOKUP_TABLE_SIZE_1024 * sizeof(int16_t);
+               break;

Just being curious here: it looks like your hardware provides hardcoded Galois field lookup tables. Do you access these through a cached mapping ? If not, maybe your code would be faster with ram-based tables ?

Quoted 11 lines

drivers/mtd/nand/atmel_nand.c +10 -0

+static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf,
+       u8 *ecc)
+{
+       int                     i, err_nbr;
+       uint8_t                 *buf_pos;
+       int                     eccbytes;
+       struct nand_chip        *nand_chip = mtd->priv;
+       struct atmel_nand_host  *host = nand_chip->priv;
+
+       eccbytes = nand_chip->ecc.bytes;

drivers/mtd/nand/atmel_nand.c static void atmel_write_buf(struct mtd_info *mtd, const u8 *buf, int len) · line 736

Quoted 5 lines

drivers/mtd/nand/atmel_nand.c +5 -0

+       for (i = 0; i<  eccbytes; i++)
+               if (ecc[i] != 0xff)
+                       goto normal_check;
+       /* Erased page, return OK */
+       return 0;

Here, you work around the fact that an erased page does not have a valid ecc. If an erased page has a bitflip in the ecc bytes region, then your code will jump to 'normal_check' and fail (because the page is erased).

On 4-bit and 8-bit NAND devices, bitflips are very common and the risk of such a failure is quite high, especially with a large number of ecc bytes.

There is a cleaner (and faster) workaround: just pretend your HW generates an extra ecc byte, always equal to 0. Assume you put this extra byte at the beginning of your ecc layout. Now, upon reading, you just need to do something like:

        eccbytes = nand_chip->ecc.bytes;
        /*
         * first ecc byte should be zero if page has been programmed,
         * but tolerate up to 4 bitflips in the same byte (very unlikely)
         */
        if (hweight_long(ecc[0])<= 4)
        	  goto normal_check;

        /* Erased page, return OK */
        return 0;

On an erased page, even with bitflips, it is _very_ unlikely that ecc[0] would have less than 4 bits set out of 8.

Regardless of the workaround, skipping ecc check on an erased page still has a drawback: UBIFS does not like to find uncorrected bitflips in erased pages.

drivers/mtd/nand/atmel_nand.c +22 -0

+static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
+               struct nand_chip *chip, uint8_t *buf, int32_t page)
+{
+       uint32_t                stat;
+       int                     eccsize = chip->ecc.size;
+       uint8_t                 *oob = chip->oob_poi;
+       struct atmel_nand_host  *host = chip->priv;
+       uint32_t                *eccpos = chip->ecc.layout->eccpos;
+
+       pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+       pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+       pmecc_writel(host->ecc, CFG, (pmecc_readl(host->ecc, CFG)
+&  ~PMECC_CFG_WRITE_OP) | PMECC_CFG_AUTO_ENABLE);
+
+       pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+       pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DATA);
+
+       chip->read_buf(mtd, buf, eccsize);
+       chip->read_buf(mtd, oob, mtd->oobsize);
+
+       while ((pmecc_readl(host->ecc, SR)&  PMECC_SR_BUSY))
+               cpu_relax();

Here it seems to me you feed the hardware ecc engine with the entire page (data+oob).
Is it possible to feed it with data+oob-eccbytes, i.e. leave out the ecc bytes from the computation ? I'm asking this because it probably would allow correcting bitflips in erased pages (if you ever care about this :).

BR,
--
Ivan