Skip to contentSkip to conversation

Add PMECC support for at91 nand flash driver

PATCH Superseded Josh Wu 4 participants 14 messages May 15, 2012 – May 18, 2012

Subsystems (5)

v7 of 13 Superseded Latest is v13 →

lore
Share & tools
lore

Josh Wu

author
#0 lore raw

v7 cover Add PMECC support for at91 nand flash driver

Changes since v6,
split into 3 patches.
remove of_flat_dt_is_compatible() function. use additional dt parameter "has-pmecc". refine the error handling code.
refine original atmel_nand_init_params() function.

Changes since v5,
add has_pmecc field to replace cpu_has_pmecc() function. Use compatible check in when proble. simplify the pmecc_get_ecc_bytes() function.

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.

Josh Wu (3):
MTD: at91: extract hw ecc initialization to one function
MTD: add dt parameters for PMECC
MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

 .../devicetree/bindings/mtd/atmel-nand.txt         |    7 +
 drivers/mtd/nand/atmel_nand.c                      |  885 ++++++++++++++++++--
 drivers/mtd/nand/atmel_nand_ecc.h                  |  123 ++-
 3 files changed, 946 insertions(+), 69 deletions(-)
Signature · 1 line
1.7.10
author
#1 lore raw

PATCH v7 1/3 MTD: at91: extract hw ecc initialization to one function

---
 drivers/mtd/nand/atmel_nand.c     |  147 ++++++++++++++++++++-----------------
 drivers/mtd/nand/atmel_nand_ecc.h |    8 +-
 2 files changed, 85 insertions(+), 70 deletions(-)
drivers/mtd/nand/atmel_nand.c +78 -69
index 2165576..52ca295 100644
@@ -42,20 +42,15 @@
 
 #include <mach/cpu.h>
 
+/* Hardware ECC registers */
+#include "atmel_nand_ecc.h"
+
 static int use_dma = 1;
 module_param(use_dma, int, 0);
 
 static int on_flash_bbt = 0;
 module_param(on_flash_bbt, int, 0);
 
-/* Register access macros */
-#define ecc_readl(add, reg)				\
-	__raw_readl(add + ATMEL_ECC_##reg)
-#define ecc_writel(add, reg, value)			\
-	__raw_writel((value), add + ATMEL_ECC_##reg)
-
-#include "atmel_nand_ecc.h"	/* Hardware ECC registers */
-
 /* oob layout for large page size
  * bad block info is on bytes 0 and 1
  * the bytes have to be consecutives to avoid
@@ -523,6 +518,75 @@ static int __devinit atmel_of_init_port(struct atmel_nand_host *host,
 }
 #endif
 
+static int __init atmel_hw_nand_init_params(struct platform_device *pdev,
+					 struct atmel_nand_host *host)
+{
+	struct resource		*regs;
+	struct mtd_info		*mtd;
+	struct nand_chip	*nand_chip;
+
+	nand_chip = &host->nand_chip;
+	mtd = &host->mtd;
+
+	nand_chip->ecc.mode = NAND_ECC_SOFT;
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!regs) {
+		dev_err(host->dev, "Can't get I/O resource regs\n" \
+			"Rolling back on software ECC\n");
+	} else {
+		host->ecc = ioremap(regs->start, resource_size(regs));
+		if (host->ecc == NULL) {
+			printk(KERN_ERR "atmel_nand: ioremap failed\n");
+			return -EIO;
+		}
+
+		nand_chip->ecc.mode = NAND_ECC_HW;
+		nand_chip->ecc.calculate = atmel_nand_calculate;
+		nand_chip->ecc.correct = atmel_nand_correct;
+		nand_chip->ecc.hwctl = atmel_nand_hwctl;
+		nand_chip->ecc.read_page = atmel_nand_read_page;
+		nand_chip->ecc.bytes = 4;
+		nand_chip->ecc.strength = 1;
+
+		/* 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 512:
+			nand_chip->ecc.layout = &atmel_oobinfo_small;
+			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_528);
+			break;
+		case 1024:
+			nand_chip->ecc.layout = &atmel_oobinfo_large;
+			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_1056);
+			break;
+		case 2048:
+			nand_chip->ecc.layout = &atmel_oobinfo_large;
+			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_2112);
+			break;
+		case 4096:
+			nand_chip->ecc.layout = &atmel_oobinfo_large;
+			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_4224);
+			break;
+		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.postpad = 0;
+			nand_chip->ecc.prepad = 0;
+			nand_chip->ecc.bytes = 0;
+			break;
+		}
+	}
+
+	return 0;
+}
+
 /*
  * Probe for the NAND device.
  */
@@ -531,7 +595,6 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	struct atmel_nand_host *host;
 	struct mtd_info *mtd;
 	struct nand_chip *nand_chip;
-	struct resource *regs;
 	struct resource *mem;
 	struct mtd_part_parser_data ppdata = {};
 	int res;
@@ -583,29 +646,6 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 		nand_chip->dev_ready = atmel_nand_device_ready;
 
 	nand_chip->ecc.mode = host->board.ecc_mode;
-
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!regs && nand_chip->ecc.mode == NAND_ECC_HW) {
-		printk(KERN_ERR "atmel_nand: can't get I/O resource "
-				"regs\nFalling back on software ECC\n");
-		nand_chip->ecc.mode = NAND_ECC_SOFT;
-	}
-
-	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		host->ecc = ioremap(regs->start, resource_size(regs));
-		if (host->ecc == NULL) {
-			printk(KERN_ERR "atmel_nand: ioremap failed\n");
-			res = -EIO;
-			goto err_ecc_ioremap;
-		}
-		nand_chip->ecc.calculate = atmel_nand_calculate;
-		nand_chip->ecc.correct = atmel_nand_correct;
-		nand_chip->ecc.hwctl = atmel_nand_hwctl;
-		nand_chip->ecc.read_page = atmel_nand_read_page;
-		nand_chip->ecc.bytes = 4;
-		nand_chip->ecc.strength = 1;
-	}
-
 	nand_chip->chip_delay = 20;		/* 20us command delay time */
 
 	if (host->board.bus_width_16)	/* 16-bit bus width */
@@ -657,40 +697,9 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	}
 
 	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		/* 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 512:
-			nand_chip->ecc.layout = &atmel_oobinfo_small;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_528);
-			break;
-		case 1024:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_1056);
-			break;
-		case 2048:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_2112);
-			break;
-		case 4096:
-			nand_chip->ecc.layout = &atmel_oobinfo_large;
-			ecc_writel(host->ecc, MR, ATMEL_ECC_PAGESIZE_4224);
-			break;
-		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.postpad = 0;
-			nand_chip->ecc.prepad = 0;
-			nand_chip->ecc.bytes = 0;
-			break;
-		}
+		res = atmel_hw_nand_init_params(pdev, host);
+		if (res != 0)
+			goto err_hw_ecc;
 	}
 
 	/* second phase scan */
@@ -707,15 +716,15 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 		return res;
 
 err_scan_tail:
+	if (host->ecc)
+		iounmap(host->ecc);
+err_hw_ecc:
 err_scan_ident:
 err_no_card:
 	atmel_nand_disable(host);
 	platform_set_drvdata(pdev, NULL);
 	if (host->dma_chan)
 		dma_release_channel(host->dma_chan);
-	if (host->ecc)
-		iounmap(host->ecc);
-err_ecc_ioremap:
 	iounmap(host->io_base);
 err_nand_ioremap:
 	kfree(host);
drivers/mtd/nand/atmel_nand_ecc.h +7 -1
index 578c776..b437567 100644
@@ -3,7 +3,7 @@
  * Based on AT91SAM9260 datasheet revision B.
  *
  * Copyright (C) 2007 Andrew Victor
- * Copyright (C) 2007 Atmel Corporation.
+ * Copyright (C) 2007 - 2012 Atmel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -36,4 +36,10 @@
 #define ATMEL_ECC_NPR		0x10			/* NParity register */
 #define		ATMEL_ECC_NPARITY	(0xffff << 0)		/* NParity */
 
+/* Register Access Macros */
+#define ecc_readl(add, reg)				\
+	__raw_readl(add + ATMEL_ECC_##reg)
+#define ecc_writel(add, reg, value)			\
+	__raw_writel((value), add + ATMEL_ECC_##reg)
+
 #endif
Signature · 1 line
1.7.10
author
#2 lore raw

PATCH v7 2/3 MTD: add dt parameters for PMECC

In reply to Josh Wu
---
 .../devicetree/bindings/mtd/atmel-nand.txt         |    7 +++++++
 drivers/mtd/nand/atmel_nand.c                      |   20 ++++++++++++++++++++
 2 files changed, 27 insertions(+)
Documentation/devicetree/bindings/mtd/atmel-nand.txt +6 -0
index a200695..895d72b 100644
@@ -16,6 +16,12 @@ 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,has-pmecc : boolean to enable Programmable Multibit ECC hardware.
+  Only supported by at91sam9x5 or later sam9 product.
+- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
+  Controller. Supported values are: 2, 4, 8, 12, 24.
+- 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
 
drivers/mtd/nand/atmel_nand.c +20 -0
index 52ca295..937b080 100644
@@ -88,6 +88,10 @@ struct atmel_nand_host {
 
 	struct completion	comp;
 	struct dma_chan		*dma_chan;
+
+	bool			has_pmecc;
+	u8			correction_cap;
+	u16			sector_size;
 };
 
 static int cpu_has_dma(void)
@@ -508,6 +512,22 @@ static int __devinit atmel_of_init_port(struct atmel_nand_host *host,
 	board->enable_pin = of_get_gpio(np, 1);
 	board->det_pin = of_get_gpio(np, 2);
 
+	host->has_pmecc = of_property_read_bool(np, "atmel,has-pmecc");
+
+	if ((board->ecc_mode == NAND_ECC_HW) && host->has_pmecc) {
+		if (of_property_read_u32(np, "atmel,pmecc-cap", &val) != 0) {
+			dev_err(host->dev, "Cannot decide PMECC Capability\n");
+			return -EINVAL;
+		}
+		host->correction_cap = (u8)val;
+
+		if (of_property_read_u32(np, "atmel,sector-size", &val) != 0) {
+			dev_err(host->dev, "Cannot decide PMECC Sector Size\n");
+			return -EINVAL;
+		}
+		host->sector_size = (u16)val;
+	}
+
 	return 0;
 }
 #else
Signature · 1 line
1.7.10
author
#3 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

In reply to 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.

---
 drivers/mtd/nand/atmel_nand.c     |  722 ++++++++++++++++++++++++++++++++++++-
 drivers/mtd/nand/atmel_nand_ecc.h |  115 ++++++
 2 files changed, 836 insertions(+), 1 deletion(-)
drivers/mtd/nand/atmel_nand.c +721 -1
index 937b080..16f3247 100644
@@ -15,6 +15,8 @@
  *     		(u-boot-1.1.5/board/atmel/at91sam9263ek/nand.c)
  *     (C) Copyright 2006 ATMEL Rousset, Lacressonniere Nicolas
  *
+ *  Add Programmable Multibit ECC support for various AT91 SoC
+ *     (C) Copyright 2012 ATMEL, Hong Xu
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -92,8 +94,35 @@ struct atmel_nand_host {
 	bool			has_pmecc;
 	u8			correction_cap;
 	u16			sector_size;
+
+	void __iomem		*pmerrloc_base;
+	void __iomem		*rom_base;
+
+	int			cap;	/* Correcting capability */
+	int			ecc_bytes_per_sector;
+	int			degree;	/*Degree of remainders,GF(2**degree)*/
+	int			cw_len;	/*Length of codeword*/
+	int			sector_number;
+
+	/* lookup table for alpha_to and index_of */
+	void __iomem		*alpha_to;
+	void __iomem		*index_of;
+
+	int16_t			partial_syn[2 * AT_NB_ERROR_MAX + 1];
+	int16_t			si[2 * AT_NB_ERROR_MAX + 1];
+
+	/* Sigma table */
+	int16_t			smu[AT_NB_ERROR_MAX + 2][2 * AT_NB_ERROR_MAX + 1];
+	/* polynomal order */
+	int16_t			lmu[AT_NB_ERROR_MAX + 1];
+
+	int			mu[AT_NB_ERROR_MAX + 1];
+	int			dmu[AT_NB_ERROR_MAX + 1];
+	int			delta[AT_NB_ERROR_MAX + 1];
 };
 
+static struct nand_ecclayout atmel_pmecc_oobinfo;
+
 static int cpu_has_dma(void)
 {
 	return cpu_is_at91sam9rl() || cpu_is_at91sam9g45();
@@ -287,6 +316,676 @@ 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
+ *
+ * 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 = 0;
+	layout->oobfree[0].length = oobsize - ecc_len;
+}
+
+static void __iomem *pmecc_get_alpha_to(struct atmel_nand_host *host)
+{
+	void __iomem *p;
+
+	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;
+	default:
+		BUG();
+	}
+
+	return p;
+}
+
+static void __iomem *pmecc_get_index_of(struct atmel_nand_host *host)
+{
+	void __iomem *p;
+
+	switch (host->sector_size) {
+	case 512:
+		p = host->rom_base + AT_PMECC_LOOKUP_TABLE_OFFSET_512;
+		break;
+	case 1024:
+		p = host->rom_base + AT_PMECC_LOOKUP_TABLE_OFFSET_1024;
+		break;
+	default:
+		BUG();
+	}
+
+	return p;
+}
+
+static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
+{
+	int			i;
+	uint32_t		value;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	/* Fill odd syndromes */
+	for (i = 0; i < host->cap; i++) {
+		value = pmecc_readl_rem(host->ecc, sector, i / 2);
+		value = (i & 1) ? (value & 0xffff0000) >> 16 : value & 0xffff;
+		host->partial_syn[(2 * i) + 1] = (int16_t)value;
+	}
+}
+
+static void pmecc_substitute(struct mtd_info *mtd)
+{
+	int16_t			*si;
+	int			i, j;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	int16_t __iomem		*alpha_to = host->alpha_to;
+	int16_t __iomem		*index_of = host->index_of;
+	int16_t			*partial_syn = host->partial_syn;
+
+	/* si[] is a table that holds the current syndrome value,
+	 * an element of that table belongs to the field
+	 */
+	si = host->si;
+
+	for (i = 1; i < 2 * AT_NB_ERROR_MAX; i++)
+		si[i] = 0;
+
+	/* Computation 2t syndromes based on S(x) */
+	/* Odd syndromes */
+	for (i = 1; i < 2 * host->cap; i += 2) {
+		si[i] = 0;
+		for (j = 0; j < host->degree; j++) {
+			if (partial_syn[i] & ((unsigned short)0x1 << j))
+				si[i] = readw_relaxed(alpha_to + i * j) ^ si[i];
+		}
+	}
+	/* Even syndrome = (Odd syndrome) ** 2 */
+	for (i = 2; i <= 2 * host->cap; i += 2) {
+		j = i / 2;
+		if (si[j] == 0)
+			si[i] = 0;
+		else {
+			int16_t tmp;
+			tmp = readw_relaxed(index_of + si[j]);
+			tmp = (tmp * 2) % host->cw_len;
+			si[i] = readw_relaxed(alpha_to + tmp);
+		}
+	}
+
+	return;
+}
+
+static void pmecc_get_sigma(struct mtd_info *mtd)
+{
+	int			i, j, k;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	uint32_t		dmu_0_count, tmp;
+	int16_t			*lmu = host->lmu;
+	int16_t			*si = host->si;
+	int			*mu = host->mu;
+	int			*dmu = host->dmu; /*Discrepancy*/
+	int			*delta = host->delta; /*Delta order*/
+	int16_t			cap = host->cap;
+	int16_t __iomem		*index_of = host->index_of;
+	int16_t __iomem		*alpha_to = host->alpha_to;
+
+	/* index of largest delta */
+	int ro;
+	int largest;
+	int diff;
+
+	dmu_0_count = 0;
+
+	/* First Row */
+
+	/* Mu */
+	host->mu[0] = -1;
+
+	memset(&host->smu[0][0], 0,
+		sizeof(int16_t) * (2 * AT_NB_ERROR_MAX + 1));
+	host->smu[0][0] = 1;
+
+	/* discrepancy set to 1 */
+	dmu[0] = 1;
+	/* polynom order set to 0 */
+	lmu[0] = 0;
+	delta[0]  = (host->mu[0] * 2 - lmu[0]) >> 1;
+
+	/* Second Row */
+
+	/* Mu */
+	host->mu[1]  = 0;
+	/* Sigma(x) set to 1 */
+	memset(&host->smu[1][0], 0,
+		sizeof(int16_t) * (2 * AT_NB_ERROR_MAX + 1));
+	host->smu[1][0] = 1;
+
+	/* discrepancy set to S1 */
+	dmu[1] = si[1];
+
+	/* polynom order set to 0 */
+	lmu[1] = 0;
+
+	delta[1]  = (host->mu[1] * 2 - lmu[1]) >> 1;
+
+	/* Init the Sigma(x) last row */
+	memset(&host->smu[cap + 1][0], 0,
+		sizeof(int16_t) * (2 * AT_NB_ERROR_MAX + 1));
+
+	for (i = 1; i <= cap; i++) {
+		host->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++)
+					host->smu[cap + 1][j] = host->smu[i][j];
+				lmu[cap + 1] = lmu[i];
+				return;
+			}
+
+			/* copy polynom */
+			for (j = 0; j <= lmu[i] >> 1; j++)
+				host->smu[i + 1][j] = host->smu[i][j];
+
+			/* copy previous polynom order to the next */
+			lmu[i + 1] = lmu[i];
+		} 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 < (2 * AT_NB_ERROR_MAX + 1); k++)
+				host->smu[i+1][k] = 0;
+
+			/* Compute smu[i+1] */
+			for (k = 0; k <= lmu[ro] >> 1; k++) {
+				int16_t a, b, c;
+
+				if (!(host->smu[ro][k] && dmu[i]))
+					continue;
+				a = readw_relaxed(index_of + dmu[i]);
+				b = readw_relaxed(index_of + dmu[ro]);
+				c = readw_relaxed(index_of + host->smu[ro][k]);
+				tmp = a + (host->cw_len - b) + c;
+				a = readw_relaxed(alpha_to + tmp % host->cw_len);
+				host->smu[i + 1][k + diff] = a;
+			}
+
+			for (k = 0; k <= lmu[i] >> 1; k++)
+				host->smu[i + 1][k] ^= host->smu[i][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 (host->smu[i+1][k] && si[tmp + 3 - k]) {
+				int16_t a, b, c;
+				a = readw_relaxed(index_of + host->smu[i + 1][k]);
+				b = si[2 * (i - 1) + 3 - k];
+				c = readw_relaxed(index_of + b);
+				tmp = a + c;
+				tmp %= host->cw_len;
+				dmu[i + 1] = readw_relaxed(alpha_to + tmp) ^
+					dmu[i + 1];
+			}
+		}
+	}
+
+	return;
+}
+
+static int pmecc_err_location(struct mtd_info *mtd)
+{
+	int			i;
+	int			err_nbr;	/* number of error */
+	int			roots_nbr;	/* number of roots */
+	int			sector_size;
+	uint32_t		val;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	err_nbr = 0;
+	sector_size = host->sector_size;
+
+	pmerrloc_writel(host->pmerrloc_base, ELDIS, PMERRLOC_DISABLE);
+
+	for (i = 0; i <= host->lmu[host->cap + 1] >> 1; i++) {
+		pmerrloc_writel_sigma(host->pmerrloc_base, i,
+				      host->smu[host->cap + 1][i]);
+		err_nbr++;
+	}
+
+	val = (err_nbr - 1) << 16;
+	if (sector_size == 1024)
+		val |= 1;
+
+	pmerrloc_writel(host->pmerrloc_base, ELCFG, val);
+	pmerrloc_writel(host->pmerrloc_base, ELEN,
+			sector_size * 8 + host->degree * host->cap);
+
+	while (!(pmerrloc_readl(host->pmerrloc_base, ELISR)
+		 & PMERRLOC_CALC_DONE))
+		cpu_relax();
+
+	roots_nbr = (pmerrloc_readl(host->pmerrloc_base, ELISR)
+		& PMERRLOC_ERR_NUM_MASK) >> 8;
+	/* Number of roots == degree of smu hence <= cap */
+	if (roots_nbr == host->lmu[host->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,
+		int extra_bytes, int err_nbr)
+{
+	int			i = 0;
+	int			byte_pos, bit_pos;
+	int			sector_size, ecc_size;
+	uint32_t		tmp;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+
+	sector_size = host->sector_size;
+	ecc_size = nand_chip->ecc.bytes;
+
+	while (err_nbr) {
+		tmp = pmerrloc_readl_el(host->pmerrloc_base, i) - 1;
+		byte_pos = tmp / 8;
+		bit_pos  = tmp % 8;
+		dev_info(host->dev, "PMECC correction, byte_pos: %d" \
+			 " bit_pos: %d\n", byte_pos, bit_pos);
+
+		if (byte_pos < (sector_size + extra_bytes)) {
+			tmp = sector_size + pmecc_readl(host->ecc, SADDR);
+			if (byte_pos < tmp)
+				*(buf + byte_pos) ^= (1 << bit_pos);
+			else
+				*(buf + byte_pos + ecc_size) ^= (1 << bit_pos);
+		}
+
+		i++;
+		err_nbr--;
+	}
+
+	return;
+}
+
+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;
+	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->sector_number; i++) {
+		err_nbr = 0;
+		if (pmecc_stat & 0x1) {
+			buf_pos = buf + i * host->sector_size;
+
+			pmecc_gen_syndrome(mtd, i);
+			pmecc_substitute(mtd);
+			pmecc_get_sigma(mtd);
+
+			err_nbr = pmecc_err_location(mtd);
+			if (err_nbr == -1) {
+				dev_err(host->dev, "PMECC: Too many errors\n");
+				mtd->ecc_stats.failed++;
+				return -EIO;
+			} else {
+				pmecc_correct_data(mtd, buf_pos, 0, 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, 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();
+
+	stat = pmecc_readl(host->ecc, 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)
+{
+	int			i, j;
+	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->write_buf(mtd, (u8 *)buf, mtd->writesize);
+
+	while ((pmecc_readl(host->ecc, SR) & PMECC_SR_BUSY))
+		cpu_relax();
+
+	for (i = 0; i < host->sector_number; i++) {
+		for (j = 0; j < host->ecc_bytes_per_sector; j++) {
+			int pos;
+
+			pos = i * host->ecc_bytes_per_sector + j;
+			chip->oob_poi[eccpos[pos]] =
+				pmecc_readb_ecc(host->ecc, i, j);
+		}
+	}
+	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	return;
+}
+
+static void atmel_pmecc_core_init(struct mtd_info *mtd)
+{
+	uint32_t		val = 0;
+	struct nand_chip	*nand_chip = mtd->priv;
+	struct atmel_nand_host	*host = nand_chip->priv;
+	struct nand_ecclayout	*ecc_layout;
+
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+
+	switch (host->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->sector_size == 512)
+		val |= PMECC_CFG_SECTOR512;
+	else if (host->sector_size == 1024)
+		val |= PMECC_CFG_SECTOR1024;
+
+	switch (host->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->ecc, CFG, val);
+
+	ecc_layout = nand_chip->ecc.layout;
+	pmecc_writel(host->ecc, SAREA, mtd->oobsize - 1);
+	pmecc_writel(host->ecc, SADDR, ecc_layout->eccpos[0]);
+	pmecc_writel(host->ecc, EADDR,
+			ecc_layout->eccpos[ecc_layout->eccbytes - 1]);
+	/* See datasheet about PMECC Clock Control Register */
+	pmecc_writel(host->ecc, CLK, 2);
+	pmecc_writel(host->ecc, IDR, 0xff);
+	pmecc_writel(host->ecc, CTRL, PMECC_CTRL_ENABLE);
+}
+
+static int __init atmel_pmecc_nand_init_params(struct platform_device *pdev,
+					 struct atmel_nand_host *host)
+{
+	int			cap, sector_size, err_no;
+	struct mtd_info		*mtd;
+	struct nand_chip	*nand_chip;
+	struct resource		*regs;
+	struct resource		*regs_pmerr, *regs_rom;
+
+	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
@@ -717,7 +1416,11 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 	}
 
 	if (nand_chip->ecc.mode == NAND_ECC_HW) {
-		res = atmel_hw_nand_init_params(pdev, host);
+		if (host->has_pmecc)
+			res = atmel_pmecc_nand_init_params(pdev, host);
+		else
+			res = atmel_hw_nand_init_params(pdev, host);
+
 		if (res != 0)
 			goto err_hw_ecc;
 	}
@@ -738,6 +1441,12 @@ static int __init atmel_nand_probe(struct platform_device *pdev)
 err_scan_tail:
 	if (host->ecc)
 		iounmap(host->ecc);
+	if (host->has_pmecc) {
+		if (host->pmerrloc_base)
+			iounmap(host->pmerrloc_base);
+		if (host->rom_base)
+			iounmap(host->rom_base);
+	}
 err_hw_ecc:
 err_scan_ident:
 err_no_card:
@@ -763,6 +1472,17 @@ static int __exit atmel_nand_remove(struct platform_device *pdev)
 
 	atmel_nand_disable(host);
 
+	if (host->has_pmecc) {
+		pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE);
+		if (host->pmerrloc_base) {
+			pmerrloc_writel(host->pmerrloc_base, ELDIS,
+				PMERRLOC_DISABLE);
+			iounmap(host->pmerrloc_base);
+		}
+		if (host->rom_base)
+			iounmap(host->rom_base);
+	}
+
 	if (host->ecc)
 		iounmap(host->ecc);
 
drivers/mtd/nand/atmel_nand_ecc.h +115 -0

119 lines not on this page · raw message

Signature · 1 line
1.7.10
#4 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

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

Quoted 3 lines · Josh Wu
drivers/mtd/nand/atmel_nand.c +3 -0
+	while (!(pmerrloc_readl(host->pmerrloc_base, ELISR)
+		 & PMERRLOC_CALC_DONE))
+		cpu_relax();

How about some kind of time-out in something goes wrong?

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

Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+		dev_warn(host->dev, "Only 2048 page size is currently " \
+			"supported for PMECC, rolling back to Software ECC\n");

Why do you use backslashes?

Signature · 9 lines
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120516/4f2f1bd8/attachment-0001.sig>
#5 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

Hi,

Quoted 7 lines · Artem Bityutskiy

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

Quoted 3 lines · Josh Wu
drivers/mtd/nand/atmel_nand.c +3 -0
+	while (!(pmerrloc_readl(host->pmerrloc_base, ELISR)
+		 & PMERRLOC_CALC_DONE))
+		cpu_relax();

How about some kind of time-out in something goes wrong?

Quoted 5 lines

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

Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+		dev_warn(host->dev, "Only 2048 page size is currently " \
+			"supported for PMECC, rolling back to Software ECC\n");

Why do you use backslashes?

message text should not be split across lines anyway to ease grepping for the message.

Lothar Wa?mann

Signature · 9 lines
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
#6 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

Quoted 7 lines · Lothar Wa?mann
Quoted 5 lines

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

Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+		dev_warn(host->dev, "Only 2048 page size is currently " \
+			"supported for PMECC, rolling back to Software ECC\n");

Why do you use backslashes?

message text should not be split across lines anyway to ease grepping for the message.

I think the consensus is that it is personal decision of the author. I am not trying to enforce this and I also split messages. In this case I was only wondering why that backslash is used? I never saw this before in the linux kernel.

Signature · 9 lines
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120516/ffb6af6a/attachment.sig>
author
#7 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

Hi, Artem

Quoted 5 lines · Artem Bityutskiy
Quoted 4 lines · Lothar Wa?mann
Quoted 4 lines

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

Quoted 2 lines
drivers/mtd/nand/atmel_nand.c +2 -0
+		dev_warn(host->dev, "Only 2048 page size is currently " \
+			"supported for PMECC, rolling back to Software ECC\n");

Why do you use backslashes?

Quoted 7 lines
Quoted 2 lines

message text should not be split across lines anyway to ease grepping for the message.

I think the consensus is that it is personal decision of the author. I am not trying to enforce this and I also split messages. In this case I was only wondering why that backslash is used? I never saw this before in the linux kernel.

oh, I add the backslash only because if without it, there should be a warning about message is splitting into two lines.

Now I think I will just keep the text message in one line. That will avoid any warnings. thanks.

Best Regards,
Josh Wu

#8 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

Quoted 10 lines · Josh Wu
Quoted 7 lines
Quoted 2 lines

message text should not be split across lines anyway to ease grepping for the message.

I think the consensus is that it is personal decision of the author. I am not trying to enforce this and I also split messages. In this case I was only wondering why that backslash is used? I never saw this before in the linux kernel.

oh, I add the backslash only because if without it, there should be a warning about message is splitting into two lines.

Backslash is not needed in C. The preprocessor removes it anyway. We do not put them.

Quoted 2 lines

Now I think I will just keep the text message in one line. That will avoid any warnings. thanks.

You can just remove the backslashes. I think your lines splitting is fine otherwise.

Signature · 9 lines
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120517/c59c579b/attachment.sig>
#9 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

Quoted 12 lines · Artem Bityutskiy
Quoted 10 lines · Josh Wu
Quoted 7 lines
Quoted 2 lines

message text should not be split across lines anyway to ease grepping for the message.

I think the consensus is that it is personal decision of the author. I am not trying to enforce this and I also split messages. In this case I was only wondering why that backslash is used? I never saw this before in the linux kernel.

oh, I add the backslash only because if without it, there should be a warning about message is splitting into two lines.

Quoted 8 lines

Backslash is not needed in C. The preprocessor removes it anyway. We do not put them.

Quoted 2 lines

Now I think I will just keep the text message in one line. That will avoid any warnings. thanks.

You can just remove the backslashes. I think your lines splitting is fine otherwise.

It's also preferred _not_ to wrap error messages as that makes them harder to grep for. Linus has mentioned this several times on LKML and has corrected error messages that he encounters that he can't find via grep.

#10 lore raw

PATCH v7 3/3 MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller

Quoted 7 lines · Russell King - ARM Linux
Quoted 2 lines

You can just remove the backslashes. I think your lines splitting is fine otherwise.

It's also preferred _not_ to wrap error messages as that makes them harder to grep for. Linus has mentioned this several times on LKML and has corrected error messages that he encounters that he can't find via grep.

OK, thanks for the info.

One could argue that it probably only makes sense to avoid splitting up to the first %d or another printk placeholder. Beyond the placeholder grepping is not more difficult.

Well, anyway, I would not like to force people to split or not split strings. I'd also say that it is more important to follow the style of the driver/subsystem and be consistent.

Signature · 9 lines
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120518/3d47ab6c/attachment.sig>
#11 lore raw

v7 cover Add PMECC support for at91 nand flash driver

In reply to Josh Wu
Quoted 10 lines · Josh Wu

Changes since v6,
split into 3 patches.
remove of_flat_dt_is_compatible() function. use additional dt parameter "has-pmecc". refine the error handling code.
refine original atmel_nand_init_params() function.

Changes since v5,
add has_pmecc field to replace cpu_has_pmecc() function. Use compatible check in when proble. simplify the pmecc_get_ecc_bytes() function.

Quoted 6 lines

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.

Hi,

Aiaiai identified the following issues with your patch-set - could you please take a look?

--------------------------------------------------------------------------------

Successfully built configuration "arm-at91cap9_defconfig,arm,arm-unknown-linux-gnueabi-", results:

after_patching.log renamed from before_patching.log +0 -0
@@ @@
+drivers/mtd/nand/atmel_nand.c: In function 'atmel_pmecc_nand_init_params':
+drivers/mtd/nand/atmel_nand.c:910:29: warning: assignment from incompatible pointer type [enabled by default]
+drivers/mtd/nand/atmel_nand.c:910:50: warning: incorrect type in assignment (different argument counts) [sparse]
+drivers/mtd/nand/atmel_nand.c:910:50:    expected int ( *read_page )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:910:50:    got int ( static [toplevel] *<noident> )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:30: warning: assignment from incompatible pointer type [enabled by default]
+drivers/mtd/nand/atmel_nand.c:912:51: warning: incorrect type in assignment (different argument counts) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:51:    expected void ( *write_page )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:51:    got void ( static [toplevel] *<noident> )( ... ) [sparse]

--------------------------------------------------------------------------------

checkpatch.pl has some complaints:

--------------------------------------------------------------------------------

checkpatch.pl results for patch "[PATCH 3/3] MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller"

WARNING:LONG_LINE: line over 80 characters
#60: FILE: drivers/mtd/nand/atmel_nand.c:115:
+ int16_t smu[AT_NB_ERROR_MAX + 2][2 * AT_NB_ERROR_MAX + 1];

WARNING:LONG_LINE: line over 80 characters
#326: FILE: drivers/mtd/nand/atmel_nand.c:567:
+ a = readw_relaxed(alpha_to + tmp % host->cw_len);

WARNING:LONG_LINE: line over 80 characters
#348: FILE: drivers/mtd/nand/atmel_nand.c:589:
+ a = readw_relaxed(index_of + host->smu[i + 1][k]);

total: 0 errors, 3 warnings, 879 lines checked

--------------------------------------------------------------------------------

checkpatch.pl results for the entire squashed patch-set

WARNING:LONG_LINE: line over 80 characters
#376: FILE: drivers/mtd/nand/atmel_nand.c:115:
+ int16_t smu[AT_NB_ERROR_MAX + 2][2 * AT_NB_ERROR_MAX + 1];

WARNING:LONG_LINE: line over 80 characters
#642: FILE: drivers/mtd/nand/atmel_nand.c:567:
+ a = readw_relaxed(alpha_to + tmp % host->cw_len);

WARNING:LONG_LINE: line over 80 characters
#664: FILE: drivers/mtd/nand/atmel_nand.c:589:
+ a = readw_relaxed(index_of + host->smu[i + 1][k]);

total: 0 errors, 3 warnings, 1136 lines checked

--------------------------------------------------------------------------------

Signature · 9 lines
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120516/34eaf287/attachment.sig>
author
#12 lore raw

v7 cover Add PMECC support for at91 nand flash driver

Hi, Artem

Quoted 17 lines · Artem Bityutskiy
Quoted 10 lines · Josh Wu

Changes since v6,
split into 3 patches.
remove of_flat_dt_is_compatible() function. use additional dt parameter "has-pmecc". refine the error handling code.
refine original atmel_nand_init_params() function.

Changes since v5,
add has_pmecc field to replace cpu_has_pmecc() function. Use compatible check in when proble. simplify the pmecc_get_ecc_bytes() function.

Quoted 6 lines

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.

Quoted 4 lines

Hi,

Aiaiai identified the following issues with your patch-set - could you please take a look?

sure.

Quoted 9 lines

--------------------------------------------------------------------------------

Successfully built configuration "arm-at91cap9_defconfig,arm,arm-unknown-linux-gnueabi-", results:

after_patching.log renamed from before_patching.log +0 -0

@@ @@

after_patching.log renamed from before_patching.log +1 -0
+drivers/mtd/nand/atmel_nand.c: In function 'atmel_pmecc_nand_init_params':

after_patching.log

Quoted 10 lines
after_patching.log renamed from before_patching.log +8 -1
+drivers/mtd/nand/atmel_nand.c:910:29: warning: assignment from incompatible pointer type [enabled by default]
+drivers/mtd/nand/atmel_nand.c:910:50: warning: incorrect type in assignment (different argument counts) [sparse]
+drivers/mtd/nand/atmel_nand.c:910:50:    expected int ( *read_page )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:910:50:    got int ( static [toplevel] *<noident>  )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:30: warning: assignment from incompatible pointer type [enabled by default]
+drivers/mtd/nand/atmel_nand.c:912:51: warning: incorrect type in assignment (different argument counts) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:51:    expected void ( *write_page )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:51:    got void ( static [toplevel] *<noident>  )( ... ) [sparse]
 
--------------------------------------------------------------------------------

Since I used Code Sourcery tools-chain (2010q1-202) to compile the code and I didn't get any warning.

So I am not sure what's caused above warnings. From the warning message, I don't know the reason exactly.

I just checked the PMECC read/write functions definition in my patch

+static int atmel_nand_pmecc_read_page(struct mtd_info *mtd,
+ struct nand_chip *chip, uint8_t *buf, int32_t page)

+static void atmel_nand_pmecc_write_page(struct mtd_info *mtd,
+ struct nand_chip *chip, const uint8_t *buf)

compare with the original HW read function (no write function)

static int atmel_nand_read_page(struct mtd_info *mtd,
struct nand_chip *chip, uint8_t *buf, int page)

The only difference is int32_t, and I don't think the warning is caused by this int32_t.

Could you have any ideas to fix such warning? That should be very helpful for me.

Quoted 21 lines

checkpatch.pl has some complaints:

--------------------------------------------------------------------------------

checkpatch.pl results for patch "[PATCH 3/3] MTD: atmel_nand: Update driver to support Programmable Multibit ECC controller"

WARNING:LONG_LINE: line over 80 characters
#60: FILE: drivers/mtd/nand/atmel_nand.c:115:
+ int16_t smu[AT_NB_ERROR_MAX + 2][2 * AT_NB_ERROR_MAX + 1];

WARNING:LONG_LINE: line over 80 characters
#326: FILE: drivers/mtd/nand/atmel_nand.c:567:
+ a = readw_relaxed(alpha_to + tmp % host->cw_len);

WARNING:LONG_LINE: line over 80 characters
#348: FILE: drivers/mtd/nand/atmel_nand.c:589:
+ a = readw_relaxed(index_of + host->smu[i + 1][k]);

total: 0 errors, 3 warnings, 879 lines checked

Quoted 7 lines

--------------------------------------------------------------------------------

checkpatch.pl results for the entire squashed patch-set

WARNING:LONG_LINE: line over 80 characters
#376: FILE: drivers/mtd/nand/atmel_nand.c:115:
+ int16_t smu[AT_NB_ERROR_MAX + 2][2 * AT_NB_ERROR_MAX + 1];

I'll fix it.

Quoted 10 lines

WARNING:LONG_LINE: line over 80 characters
#642: FILE: drivers/mtd/nand/atmel_nand.c:567:
+ a = readw_relaxed(alpha_to + tmp % host->cw_len);

WARNING:LONG_LINE: line over 80 characters
#664: FILE: drivers/mtd/nand/atmel_nand.c:589:
+ a = readw_relaxed(index_of + host->smu[i + 1][k]);

total: 0 errors, 3 warnings, 1136 lines checked

For these two warnings, I would like to keep it for more readable. but if you would like to fix the warning, then I can fix them too by split into two lines.

Quoted 2 lines

--------------------------------------------------------------------------------

Thanks
Best Regards,
Josh Wu

#13 lore raw

v7 cover Add PMECC support for at91 nand flash driver

Quoted 17 lines · Josh Wu
Quoted 17 lines

--------------------------------------------------------------------------------

Successfully built configuration "arm-at91cap9_defconfig,arm,arm-unknown-linux-gnueabi-", results:

after_patching.log renamed from before_patching.log +0 -0

@@ @@

after_patching.log renamed from before_patching.log +9 -0
+drivers/mtd/nand/atmel_nand.c: In function 'atmel_pmecc_nand_init_params':
+drivers/mtd/nand/atmel_nand.c:910:29: warning: assignment from incompatible pointer type [enabled by default]
+drivers/mtd/nand/atmel_nand.c:910:50: warning: incorrect type in assignment (different argument counts) [sparse]
+drivers/mtd/nand/atmel_nand.c:910:50:    expected int ( *read_page )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:910:50:    got int ( static [toplevel] *<noident>  )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:30: warning: assignment from incompatible pointer type [enabled by default]
+drivers/mtd/nand/atmel_nand.c:912:51: warning: incorrect type in assignment (different argument counts) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:51:    expected void ( *write_page )( ... ) [sparse]
+drivers/mtd/nand/atmel_nand.c:912:51:    got void ( static [toplevel] *<noident>  )( ... ) [sparse]
Quoted 7 lines
Quoted 1 line

--------------------------------------------------------------------------------

Since I used Code Sourcery tools-chain (2010q1-202) to compile the code and I didn't get any warning.

So I am not sure what's caused above warnings. From the warning message, I don't know the reason exactly.

The warnings come from sparse, not gcc. And they are introduced by this patch - aiaiai informs only about new warnings, otherwise it would be useless.

Quoted 8 lines
Quoted 8 lines

WARNING:LONG_LINE: line over 80 characters
#642: FILE: drivers/mtd/nand/atmel_nand.c:567:
+ a = readw_relaxed(alpha_to + tmp % host->cw_len);

WARNING:LONG_LINE: line over 80 characters
#664: FILE: drivers/mtd/nand/atmel_nand.c:589:
+ a = readw_relaxed(index_of + host->smu[i + 1][k]);

Quoted 5 lines
Quoted 1 line

total: 0 errors, 3 warnings, 1136 lines checked

For these two warnings, I would like to keep it for more readable. but if you would like to fix the warning, then I can fix them too by split into two lines.

Fine with me.

Signature · 9 lines
Best Regards,
Artem Bityutskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120517/ab3233ab/attachment.sig>