[PATCH 00/13] DocG3 fixes and write support

[PATCH 00/13] DocG3 fixes and write support

[Robert Jarzmik]

This patchset is aimed at :
 - fixing 4 glitches in the driver
 - add several new functions

The added functions are :
 - add block erase capability
 - add write page capability
 - add powerdown and powerup
 - add multiple floor support
 - add ECC support

The ECC part should especially be reviewed by Ivan and Mike,
as this is their work (patch "add ECC correction code"). 

Happy review.

--
Robert

Robert Jarzmik (13):
  mtd/docg3: fix debug log verbosity
  mtd/docg3: fix tracing of IO in writeb
  mtd/docg3: fix protection areas reading
  mtd/docg3: fix BCH registers
  mtd/docg3: add multiple floor support
  mtd/docg3: add OOB layout to mtdinfo
  mtd/docg3: add registers for erasing and writing
  mtd/docg3: add OOB buffer to device structure
  mtd/docg3: add write functions
  mtd/docg3: add erase functions
  mtd/docg3: map erase and write functions
  mtd/docg3: add ECC correction code
  mtd/docg3: add suspend and resume

 drivers/mtd/devices/Kconfig |    1 +
 drivers/mtd/devices/docg3.c |  933 +++++++++++++++++++++++++++++++++++++------
 drivers/mtd/devices/docg3.h |   42 ++-
 3 files changed, 860 insertions(+), 116 deletions(-)

-- 
1.7.5.4

[PATCH 01/13] mtd/docg3: fix debug log verbosity

[Robert Jarzmik]

Change the NOP debug log verbosity to very verbose to
unburden log analysis.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index bdcf5df..6f5233d 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -143,7 +143,7 @@ static void doc_delay(struct docg3 *docg3, int nbNOPs)
 {
 	int i;
 
-	doc_dbg("NOP x %d\n", nbNOPs);
+	doc_vdbg("NOP x %d\n", nbNOPs);
 	for (i = 0; i < nbNOPs; i++)
 		doc_writeb(docg3, 0, DOC_NOP);
 }
-- 
1.7.5.4

[PATCH 02/13] mtd/docg3: fix tracing of IO in writeb

[Robert Jarzmik]

Writeb was incorrectly traced as a 16 bits write, instead of
a 8 bits write. Fix it by tracing the correct width.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 6f5233d..8942a75 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -82,7 +82,7 @@ static inline u16 doc_readw(struct docg3 *docg3, u16 reg)
 static inline void doc_writeb(struct docg3 *docg3, u8 val, u16 reg)
 {
 	writeb(val, docg3->base + reg);
-	trace_docg3_io(1, 16, reg, val);
+	trace_docg3_io(1, 8, reg, val);
 }
 
 static inline void doc_writew(struct docg3 *docg3, u16 val, u16 reg)
-- 
1.7.5.4

[PATCH 03/13] mtd/docg3: fix protection areas reading

[Robert Jarzmik]

The protection areas boundaries were on 16bit registers, not
8bit. This is consistent with block numbers, which can
extend up to 4096 on bigger chips (and is 2048 on the
docg3).

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 8942a75..5834e6e 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -852,13 +852,15 @@ static int dbg_protection_show(struct seq_file *s, void *p)
 {
 	struct docg3 *docg3 = (struct docg3 *)s->private;
 	int pos = 0;
-	int protect = doc_register_readb(docg3, DOC_PROTECTION);
-	int dps0 = doc_register_readb(docg3, DOC_DPS0_STATUS);
-	int dps0_low = doc_register_readb(docg3, DOC_DPS0_ADDRLOW);
-	int dps0_high = doc_register_readb(docg3, DOC_DPS0_ADDRHIGH);
-	int dps1 = doc_register_readb(docg3, DOC_DPS1_STATUS);
-	int dps1_low = doc_register_readb(docg3, DOC_DPS1_ADDRLOW);
-	int dps1_high = doc_register_readb(docg3, DOC_DPS1_ADDRHIGH);
+	int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high;
+
+	protect = doc_register_readb(docg3, DOC_PROTECTION);
+	dps0 = doc_register_readb(docg3, DOC_DPS0_STATUS);
+	dps0_low = doc_register_readw(docg3, DOC_DPS0_ADDRLOW);
+	dps0_high = doc_register_readw(docg3, DOC_DPS0_ADDRHIGH);
+	dps1 = doc_register_readb(docg3, DOC_DPS1_STATUS);
+	dps1_low = doc_register_readw(docg3, DOC_DPS1_ADDRLOW);
+	dps1_high = doc_register_readw(docg3, DOC_DPS1_ADDRHIGH);
 
 	pos += seq_printf(s, "Protection = 0x%02x (",
 			 protect);
-- 
1.7.5.4

[PATCH 04/13] mtd/docg3: fix BCH registers

[Robert Jarzmik]

BCH registers are contiguous, not on every byte. Fix the
register definitions.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index 0d407be..62af5aa 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -105,7 +105,7 @@
 #define DOC_ECCCONF1			0x1042
 #define DOC_ECCPRESET			0x1044
 #define DOC_HAMMINGPARITY		0x1046
-#define DOC_BCH_SYNDROM(idx)		(0x1048 + (idx << 1))
+#define DOC_BCH_SYNDROM(idx)		(0x1048 + (idx << 0))
 
 #define DOC_PROTECTION			0x1056
 #define DOC_DPS0_ADDRLOW		0x1060
-- 
1.7.5.4

[PATCH 05/13] mtd/docg3: add multiple floor support

[Robert Jarzmik]

Add support for multiple floors, ie. cascaded docg3
chips. There might be 4 docg3 chips cascaded, sharing the
same address space, and providing up to 4 times the storage
capacity of a unique chip.

Each floor will be seen as an independant mtd device.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |  181 +++++++++++++++++++++++++++++-------------
 drivers/mtd/devices/docg3.h |    1 +
 2 files changed, 126 insertions(+), 56 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 5834e6e..709d886 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -952,7 +952,8 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
 
 	switch (chip_id) {
 	case DOC_CHIPID_G3:
-		mtd->name = "DiskOnChip G3";
+		mtd->name = kasprintf(GFP_KERNEL, "DiskOnChip G3 floor %d",
+				      docg3->device_id);
 		docg3->max_block = 2047;
 		break;
 	}
@@ -979,22 +980,24 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
 }
 
 /**
- * doc_probe - Probe the IO space for a DiskOnChip G3 chip
- * @pdev: platform device
+ * doc_probe_device - Check if a device is available
+ * @base: the io space where the device is probed
+ * @floor: the floor of the probed device
+ * @dev: the device
  *
- * Probes for a G3 chip at the specified IO space in the platform data
- * ressources.
+ * Checks whether a device at the specified IO range, and floor is available.
  *
- * Returns 0 on success, -ENOMEM, -ENXIO on error
+ * Returns a mtd_info struct if there is a device, ENODEV if none found, ENOMEM
+ * if a memory allocation failed. If floor 0 is checked, a reset of the ASIC is
+ * launched.
  */
-static int __init docg3_probe(struct platform_device *pdev)
+static struct mtd_info *doc_probe_device(void __iomem *base, int floor,
+					 struct device *dev)
 {
-	struct device *dev = &pdev->dev;
-	struct docg3 *docg3;
-	struct mtd_info *mtd;
-	struct resource *ress;
 	int ret, bbt_nbpages;
 	u16 chip_id, chip_id_inv;
+	struct docg3 *docg3;
+	struct mtd_info *mtd;
 
 	ret = -ENOMEM;
 	docg3 = kzalloc(sizeof(struct docg3), GFP_KERNEL);
@@ -1004,69 +1007,132 @@ static int __init docg3_probe(struct platform_device *pdev)
 	if (!mtd)
 		goto nomem2;
 	mtd->priv = docg3;
+	bbt_nbpages = DIV_ROUND_UP(docg3->max_block + 1,
+				   8 * DOC_LAYOUT_PAGE_SIZE);
+	docg3->bbt = kzalloc(bbt_nbpages * DOC_LAYOUT_PAGE_SIZE, GFP_KERNEL);
+	if (!docg3->bbt)
+		goto nomem3;
 
-	ret = -ENXIO;
-	ress = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!ress) {
-		dev_err(dev, "No I/O memory resource defined\n");
-		goto noress;
-	}
-	docg3->base = ioremap(ress->start, DOC_IOSPACE_SIZE);
-
-	docg3->dev = &pdev->dev;
-	docg3->device_id = 0;
+	docg3->dev = dev;
+	docg3->device_id = floor;
+	docg3->base = base;
 	doc_set_device_id(docg3, docg3->device_id);
-	doc_set_asic_mode(docg3, DOC_ASICMODE_RESET);
+	if (!floor)
+		doc_set_asic_mode(docg3, DOC_ASICMODE_RESET);
 	doc_set_asic_mode(docg3, DOC_ASICMODE_NORMAL);
 
 	chip_id = doc_register_readw(docg3, DOC_CHIPID);
 	chip_id_inv = doc_register_readw(docg3, DOC_CHIPID_INV);
 
-	ret = -ENODEV;
+	ret = 0;
 	if (chip_id != (u16)(~chip_id_inv)) {
-		doc_info("No device found at IO addr %p\n",
-			 (void *)ress->start);
-		goto nochipfound;
+		goto nomem3;
 	}
 
 	switch (chip_id) {
 	case DOC_CHIPID_G3:
-		doc_info("Found a G3 DiskOnChip at addr %p\n",
-			 (void *)ress->start);
+		doc_info("Found a G3 DiskOnChip at addr %p, floor %d\n",
+			 base, floor);
 		break;
 	default:
 		doc_err("Chip id %04x is not a DiskOnChip G3 chip\n", chip_id);
-		goto nochipfound;
+		goto nomem3;
 	}
 
 	doc_set_driver_info(chip_id, mtd);
-	platform_set_drvdata(pdev, mtd);
 
-	ret = -ENOMEM;
-	bbt_nbpages = DIV_ROUND_UP(docg3->max_block + 1,
-				   8 * DOC_LAYOUT_PAGE_SIZE);
-	docg3->bbt = kzalloc(bbt_nbpages * DOC_LAYOUT_PAGE_SIZE, GFP_KERNEL);
-	if (!docg3->bbt)
-		goto nochipfound;
 	doc_reload_bbt(docg3);
+	return mtd;
 
-	ret = mtd_device_parse_register(mtd, part_probes,
-					NULL, NULL, 0);
-	if (ret)
-		goto register_error;
-
-	doc_dbg_register(docg3);
-	return 0;
-
-register_error:
-	kfree(docg3->bbt);
-nochipfound:
-	iounmap(docg3->base);
-noress:
+nomem3:
 	kfree(mtd);
 nomem2:
 	kfree(docg3);
 nomem1:
+	return ERR_PTR(ret);
+}
+
+/**
+ * doc_release_device - Release a docg3 floor
+ * @mtd: the device
+ */
+static void doc_release_device(struct mtd_info *mtd)
+{
+	struct docg3 *docg3 = mtd->priv;
+
+	mtd_device_unregister(mtd);
+	kfree(docg3->bbt);
+	kfree(docg3);
+	kfree(mtd->name);
+	kfree(mtd);
+}
+
+/**
+ * doc_probe - Probe the IO space for a DiskOnChip G3 chip
+ * @pdev: platform device
+ *
+ * Probes for a G3 chip at the specified IO space in the platform data
+ * ressources. The floor 0 must be available.
+ *
+ * Returns 0 on success, -ENOMEM, -ENXIO on error
+ */
+static int __init docg3_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mtd_info *mtd;
+	struct resource *ress;
+	void __iomem *base;
+	int ret, floor, found = 0;
+	struct mtd_info **docg3_floors;
+
+	ret = -ENXIO;
+	ress = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!ress) {
+		dev_err(dev, "No I/O memory resource defined\n");
+		goto noress;
+	}
+	base = ioremap(ress->start, DOC_IOSPACE_SIZE);
+
+	ret = -ENOMEM;
+	docg3_floors = kzalloc(sizeof(*docg3_floors) * DOC_MAX_NBFLOORS,
+			       GFP_KERNEL);
+	if (!docg3_floors)
+		goto nomem;
+
+	ret = 0;
+	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++) {
+		mtd = doc_probe_device(base, floor, dev);
+		if (floor == 0 && !mtd)
+			goto notfound;
+		if (!IS_ERR_OR_NULL(mtd))
+			ret = mtd_device_parse_register(mtd, part_probes,
+							NULL, NULL, 0);
+		else
+			ret = PTR_ERR(mtd);
+		docg3_floors[floor] = mtd;
+		if (ret)
+			goto err_probe;
+		if (mtd)
+			found++;
+	}
+
+	if (!found)
+		goto notfound;
+
+	platform_set_drvdata(pdev, docg3_floors);
+	doc_dbg_register(docg3_floors[0]->priv);
+	return 0;
+
+notfound:
+	ret = -ENODEV;
+	dev_info(dev, "No supported DiskOnChip found\n");
+err_probe:
+	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++)
+		if (docg3_floors[floor])
+			doc_release_device(docg3_floors[floor]);
+nomem:
+	iounmap(base);
+noress:
 	return ret;
 }
 
@@ -1078,15 +1144,18 @@ nomem1:
  */
 static int __exit docg3_release(struct platform_device *pdev)
 {
-	struct mtd_info *mtd = platform_get_drvdata(pdev);
-	struct docg3 *docg3 = mtd->priv;
+	struct mtd_info **docg3_floors = platform_get_drvdata(pdev);
+	struct docg3 *docg3 = docg3_floors[0]->priv;
+	void __iomem *base = docg3->base;
+	int floor;
 
 	doc_dbg_unregister(docg3);
-	mtd_device_unregister(mtd);
-	iounmap(docg3->base);
-	kfree(docg3->bbt);
-	kfree(docg3);
-	kfree(mtd);
+	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++)
+		if (docg3_floors[floor])
+			doc_release_device(docg3_floors[floor]);
+
+	kfree(docg3_floors);
+	iounmap(base);
 	return 0;
 }
 
diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index 62af5aa..75df3a1 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -80,6 +80,7 @@
 
 #define DOC_CHIPID_G3			0x200
 #define DOC_ERASE_MARK			0xaa
+#define DOC_MAX_NBFLOORS		4
 /*
  * Flash registers
  */
-- 
1.7.5.4

[PATCH 06/13] mtd/docg3: add OOB layout to mtdinfo

[Robert Jarzmik]

Add OOB layout description for docg3, so that userspace can
use this information to setup the data for write_oob().

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 709d886..26fc135 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -63,6 +63,18 @@
  *
  */
 
+/**
+ * struct docg3_oobinfo - DiskOnChip G3 OOB layout
+ * @eccbytes: 8 bytes are used (1 for Hamming ECC, 7 for BCH ECC)
+ * @eccpos: ecc positions (byte 7 is Hamming ECC, byte 8-14 are BCH ECC)
+ * @oobfree: free pageinfo bytes (byte 0 until byte 6, byte 15
+ */
+static struct nand_ecclayout docg3_oobinfo = {
+	.eccbytes = 8,
+	.eccpos = {7, 8, 9, 10, 11, 12, 13, 14},
+	.oobfree = {{0, 7}, {15, 1} }
+};
+
 static inline u8 doc_readb(struct docg3 *docg3, u16 reg)
 {
 	u8 val = readb(docg3->base + reg);
@@ -977,6 +989,7 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
 	mtd->write_oob = NULL;
 	mtd->sync = NULL;
 	mtd->block_isbad = doc_block_isbad;
+	mtd->ecclayout = &docg3_oobinfo;
 }
 
 /**
-- 
1.7.5.4

[PATCH 07/13] mtd/docg3: add registers for erasing and writing

[Robert Jarzmik]

Add the required registers and commands to erase and write
flash pages / blocks.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.h |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index 75df3a1..e9967ab 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -130,6 +130,8 @@
 #define DOC_SEQ_SET_PLANE1		0x0e
 #define DOC_SEQ_SET_PLANE2		0x10
 #define DOC_SEQ_PAGE_SETUP		0x1d
+#define DOC_SEQ_ERASE			0x27
+#define DOC_SEQ_PLANES_STATUS		0x31
 
 /*
  * Flash commands
@@ -144,7 +146,10 @@
 #define DOC_CMD_PROG_BLOCK_ADDR		0x60
 #define DOC_CMD_PROG_CYCLE1		0x80
 #define DOC_CMD_PROG_CYCLE2		0x10
+#define DOC_CMD_PROG_CYCLE3		0x11
 #define DOC_CMD_ERASECYCLE2		0xd0
+#define DOC_CMD_READ_STATUS		0x70
+#define DOC_CMD_PLANES_STATUS		0x71
 
 #define DOC_CMD_RELIABLE_MODE		0x22
 #define DOC_CMD_FAST_MODE		0xa2
@@ -186,7 +191,7 @@
  */
 #define DOC_ECCCONF1_BCH_SYNDROM_ERR	0x80
 #define DOC_ECCCONF1_UNKOWN1		0x40
-#define DOC_ECCCONF1_UNKOWN2		0x20
+#define DOC_ECCCONF1_PAGE_IS_WRITTEN	0x20
 #define DOC_ECCCONF1_UNKOWN3		0x10
 #define DOC_ECCCONF1_HAMMING_BITS_MASK	0x0f
 
@@ -224,6 +229,13 @@
 #define DOC_READADDR_ONE_BYTE		0x4000
 #define DOC_READADDR_ADDR_MASK		0x1fff
 
+/*
+ * Status of erase and write operation
+ */
+#define DOC_PLANES_STATUS_FAIL		0x01
+#define DOC_PLANES_STATUS_PLANE0_KO	0x02
+#define DOC_PLANES_STATUS_PLANE1_KO	0x04
+
 /**
  * struct docg3 - DiskOnChip driver private data
  * @dev: the device currently under control
-- 
1.7.5.4

[PATCH 08/13] mtd/docg3: add OOB buffer to device structure

[Robert Jarzmik]

Add OOB buffer area to store the OOB data until the actual
page is written, so that it can be completed by hardware ECC
generator.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index e9967ab..397e461 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -243,6 +243,11 @@
  * @device_id: number of the cascaded DoCG3 device (0, 1, 2 or 3)
  * @if_cfg: if true, reads are on 16bits, else reads are on 8bits
  * @bbt: bad block table cache
+ * @oob_write_ofs: offset of the MTD where this OOB should belong (ie. in next
+ *                 page_write)
+ * @oob_autoecc: if 1, use only bytes 0-7, 15, and fill the others with HW ECC
+ *               if 0, use all the 16 bytes.
+ * @oob_write_buf: prepared OOB for next page_write
  * @debugfs_root: debugfs root node
  */
 struct docg3 {
@@ -252,6 +257,9 @@ struct docg3 {
 	unsigned int if_cfg:1;
 	int max_block;
 	u8 *bbt;
+	loff_t oob_write_ofs;
+	int oob_autoecc;
+	u8 oob_write_buf[DOC_LAYOUT_OOB_SIZE];
 	struct dentry *debugfs_root;
 };
 
-- 
1.7.5.4

[PATCH 09/13] mtd/docg3: add write functions

[Robert Jarzmik]

Add write capability to the docg3 driver. The writes are
possible on a single page (512 bytes + 16 bytes), even if
that page is split on 2 physical pages on 2 blocks (each on
one plane).

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |  434 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 421 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 26fc135..92522e0 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -248,6 +248,40 @@ static void doc_read_data_area(struct docg3 *docg3, void *buf, int len,
 }
 
 /**
+ * doc_write_data_area - Write data into data area
+ * @docg3: the device
+ * @buf: the buffer to get input bytes from
+ * @len: the length to write
+ *
+ * Writes bytes into flash data. Handles the single byte / even bytes writes.
+ */
+static void doc_write_data_area(struct docg3 *docg3, const void *buf, int len)
+{
+	int i, cdr, len4;
+	u16 *src16;
+	u8 *src8;
+
+	doc_dbg("doc_write_data_area(buf=%p, len=%d)\n", buf, len);
+	cdr = len & 0x3;
+	len4 = len - cdr;
+
+	doc_writew(docg3, DOC_IOSPACE_DATA, DOC_READADDRESS);
+	src16 = (u16 *)buf;
+	for (i = 0; i < len4; i += 2) {
+		doc_writew(docg3, *src16, DOC_IOSPACE_DATA);
+		src16++;
+	}
+
+	src8 = (u8 *)src16;
+	for (i = 0; i < cdr; i++) {
+		doc_writew(docg3, DOC_IOSPACE_DATA | DOC_READADDR_ONE_BYTE,
+			   DOC_READADDRESS);
+		doc_writeb(docg3, *src8, DOC_IOSPACE_DATA);
+		src8++;
+	}
+}
+
+/**
  * doc_set_data_mode - Sets the flash to reliable data mode
  * @docg3: the device
  *
@@ -337,6 +371,37 @@ static int doc_set_extra_page_mode(struct docg3 *docg3)
 }
 
 /**
+ * doc_setup_addr_sector - Setup blocks/page/ofs address for one plane
+ * @docg3: the device
+ * @sector: the sector
+ */
+static void doc_setup_addr_sector(struct docg3 *docg3, int sector)
+{
+	doc_delay(docg3, 1);
+	doc_flash_address(docg3, sector & 0xff);
+	doc_flash_address(docg3, (sector >> 8) & 0xff);
+	doc_flash_address(docg3, (sector >> 16) & 0xff);
+	doc_delay(docg3, 1);
+}
+
+/**
+ * doc_setup_writeaddr_sector - Setup blocks/page/ofs address for one plane
+ * @docg3: the device
+ * @sector: the sector
+ * @ofs: the offset in the page, between 0 and (512 + 16 + 512)
+ */
+static void doc_setup_writeaddr_sector(struct docg3 *docg3, int sector, int ofs)
+{
+	ofs = ofs >> 2;
+	doc_delay(docg3, 1);
+	doc_flash_address(docg3, ofs & 0xff);
+	doc_flash_address(docg3, sector & 0xff);
+	doc_flash_address(docg3, (sector >> 8) & 0xff);
+	doc_flash_address(docg3, (sector >> 16) & 0xff);
+	doc_delay(docg3, 1);
+}
+
+/**
  * doc_seek - Set both flash planes to the specified block, page for reading
  * @docg3: the device
  * @block0: the first plane block index
@@ -372,27 +437,73 @@ static int doc_read_seek(struct docg3 *docg3, int block0, int block1, int page,
 	if (ret)
 		goto out;
 
-	sector = (block0 << DOC_ADDR_BLOCK_SHIFT) + (page & DOC_ADDR_PAGE_MASK);
 	doc_flash_sequence(docg3, DOC_SEQ_READ);
+	sector = (block0 << DOC_ADDR_BLOCK_SHIFT) + (page & DOC_ADDR_PAGE_MASK);
 	doc_flash_command(docg3, DOC_CMD_PROG_BLOCK_ADDR);
-	doc_delay(docg3, 1);
-	doc_flash_address(docg3, sector & 0xff);
-	doc_flash_address(docg3, (sector >> 8) & 0xff);
-	doc_flash_address(docg3, (sector >> 16) & 0xff);
-	doc_delay(docg3, 1);
+	doc_setup_addr_sector(docg3, sector);
 
 	sector = (block1 << DOC_ADDR_BLOCK_SHIFT) + (page & DOC_ADDR_PAGE_MASK);
 	doc_flash_command(docg3, DOC_CMD_PROG_BLOCK_ADDR);
+	doc_setup_addr_sector(docg3, sector);
 	doc_delay(docg3, 1);
-	doc_flash_address(docg3, sector & 0xff);
-	doc_flash_address(docg3, (sector >> 8) & 0xff);
-	doc_flash_address(docg3, (sector >> 16) & 0xff);
+
+out:
+	return ret;
+}
+
+/**
+ * doc_write_seek - Set both flash planes to the specified block, page for writing
+ * @docg3: the device
+ * @block0: the first plane block index
+ * @block1: the second plane block index
+ * @page: the page index within the block
+ * @ofs: offset in page to write
+ *
+ * Programs the flash even and odd planes to the specific block and page.
+ * Alternatively, programs the flash to the wear area of the specified page.
+ */
+static int doc_write_seek(struct docg3 *docg3, int block0, int block1, int page,
+			 int ofs)
+{
+	int ret = 0, sector;
+
+	doc_dbg("doc_write_seek(blocks=(%d,%d), page=%d, ofs=%d)\n",
+		block0, block1, page, ofs);
+
+	doc_set_reliable_mode(docg3);
+
+	if (ofs < 2 * DOC_LAYOUT_PAGE_SIZE) {
+		doc_flash_sequence(docg3, DOC_SEQ_SET_PLANE1);
+		doc_flash_command(docg3, DOC_CMD_READ_PLANE1);
+		doc_delay(docg3, 2);
+	} else {
+		doc_flash_sequence(docg3, DOC_SEQ_SET_PLANE2);
+		doc_flash_command(docg3, DOC_CMD_READ_PLANE2);
+		doc_delay(docg3, 2);
+	}
+
+	doc_flash_sequence(docg3, DOC_SEQ_PAGE_SETUP);
+	doc_flash_command(docg3, DOC_CMD_PROG_CYCLE1);
+
+	sector = (block0 << DOC_ADDR_BLOCK_SHIFT) + (page & DOC_ADDR_PAGE_MASK);
+	doc_setup_writeaddr_sector(docg3, sector, ofs);
+
+	doc_flash_command(docg3, DOC_CMD_PROG_CYCLE3);
 	doc_delay(docg3, 2);
+	ret = doc_wait_ready(docg3);
+	if (ret)
+		goto out;
+
+	doc_flash_command(docg3, DOC_CMD_PROG_CYCLE1);
+	sector = (block1 << DOC_ADDR_BLOCK_SHIFT) + (page & DOC_ADDR_PAGE_MASK);
+	doc_setup_writeaddr_sector(docg3, sector, ofs);
+	doc_delay(docg3, 1);
 
 out:
 	return ret;
 }
 
+
 /**
  * doc_read_page_ecc_init - Initialize hardware ECC engine
  * @docg3: the device
@@ -415,6 +526,58 @@ static int doc_read_page_ecc_init(struct docg3 *docg3, int len)
 }
 
 /**
+ * doc_write_page_ecc_init - Initialize hardware BCH ECC engine
+ * @docg3: the device
+ * @len: the number of bytes covered by the ECC (BCH covered)
+ *
+ * The function does initialize the hardware ECC engine to compute the Hamming
+ * ECC (on 1 byte) and the BCH Syndroms (on 7 bytes).
+ *
+ * Return 0 if succeeded, -EIO on error
+ */
+static int doc_write_page_ecc_init(struct docg3 *docg3, int len)
+{
+	doc_writew(docg3, !DOC_ECCCONF0_READ_MODE
+		   | DOC_ECCCONF0_BCH_ENABLE | DOC_ECCCONF0_HAMMING_ENABLE
+		   | (len & DOC_ECCCONF0_DATA_BYTES_MASK),
+		   DOC_ECCCONF0);
+	doc_delay(docg3, 4);
+	doc_register_readb(docg3, DOC_FLASHCONTROL);
+	return doc_wait_ready(docg3);
+}
+
+/**
+ * doc_ecc_disable - Disable Hamming and BCH ECC hardware calculator
+ * @docg3: the device
+ *
+ * Disables the hardware ECC generator and checker, for unchecked reads (as when
+ * reading OOB only or write status byte).
+ */
+static void doc_ecc_disable(struct docg3 *docg3)
+{
+	doc_writew(docg3, DOC_ECCCONF0_READ_MODE, DOC_ECCCONF0);
+	doc_delay(docg3, 4);
+}
+
+/**
+ * doc_hamming_ecc_init - Initialize hardware Hamming ECC engine
+ * @docg3: the device
+ * @nb_bytes: the number of bytes covered by the ECC (Hamming covered)
+ *
+ * This function programs the ECC hardware to compute the hamming code on the
+ * last provided N bytes to the hardware generator.
+ */
+static void doc_hamming_ecc_init(struct docg3 *docg3, int nb_bytes)
+{
+	u8 ecc_conf1;
+
+	ecc_conf1 = doc_register_readb(docg3, DOC_ECCCONF1);
+	ecc_conf1 &= ~DOC_ECCCONF1_HAMMING_BITS_MASK;
+	ecc_conf1 |= (nb_bytes & DOC_ECCCONF1_HAMMING_BITS_MASK);
+	doc_writeb(docg3, ecc_conf1, DOC_ECCCONF1);
+}
+
+/**
  * doc_read_page_prepare - Prepares reading data from a flash page
  * @docg3: the device
  * @block0: the first plane block index on flash memory
@@ -500,11 +663,25 @@ static int doc_read_page_getbytes(struct docg3 *docg3, int len, u_char *buf,
 }
 
 /**
+ * doc_write_page_putbytes - Writes bytes into a prepared page
+ * @docg3: the device
+ * @len: the number of bytes to be written
+ * @buf: the buffer of input bytes
+ *
+ */
+static void doc_write_page_putbytes(struct docg3 *docg3, int len,
+				    const u_char *buf)
+{
+	doc_write_data_area(docg3, buf, len);
+	doc_delay(docg3, 2);
+}
+
+/**
  * doc_get_hw_bch_syndroms - Get hardware calculated BCH syndroms
  * @docg3: the device
  * @syns:  the array of 7 integers where the syndroms will be stored
  */
-static void doc_get_hw_bch_syndroms(struct docg3 *docg3, int *syns)
+static void doc_get_hw_bch_syndroms(struct docg3 *docg3, u8 *syns)
 {
 	int i;
 
@@ -513,6 +690,16 @@ static void doc_get_hw_bch_syndroms(struct docg3 *docg3, int *syns)
 }
 
 /**
+ * doc_page_finish - Ends reading/writing of a flash page
+ * @docg3: the device
+ */
+static void doc_page_finish(struct docg3 *docg3)
+{
+	doc_writeb(docg3, 0, DOC_DATAEND);
+	doc_delay(docg3, 2);
+}
+
+/**
  * doc_read_page_finish - Ends reading of a flash page
  * @docg3: the device
  *
@@ -522,8 +709,7 @@ static void doc_get_hw_bch_syndroms(struct docg3 *docg3, int *syns)
  */
 static void doc_read_page_finish(struct docg3 *docg3)
 {
-	doc_writeb(docg3, 0, DOC_DATAEND);
-	doc_delay(docg3, 2);
+	doc_page_finish(docg3);
 	doc_set_device_id(docg3, 0);
 }
 
@@ -572,7 +758,7 @@ static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
 {
 	struct docg3 *docg3 = mtd->priv;
 	int block0, block1, page, readlen, ret, ofs = 0;
-	int syn[DOC_ECC_BCH_SIZE], eccconf1;
+	u8 syn[DOC_ECC_BCH_SIZE], eccconf1;
 	u8 oob[DOC_LAYOUT_OOB_SIZE];
 
 	ret = -EINVAL;
@@ -793,6 +979,227 @@ static int doc_get_erase_count(struct docg3 *docg3, loff_t from)
 	return max(plane1_erase_count, plane2_erase_count);
 }
 
+/**
+ * doc_get_op_status - get erase/write operation status
+ * @docg3: the device
+ *
+ * Queries the status from the chip, and returns it
+ *
+ * Returns the status (bits DOC_PLANES_STATUS_*)
+ */
+static int doc_get_op_status(struct docg3 *docg3)
+{
+	u8 status;
+
+	doc_flash_sequence(docg3, DOC_SEQ_PLANES_STATUS);
+	doc_flash_command(docg3, DOC_CMD_PLANES_STATUS);
+	doc_delay(docg3, 5);
+
+	doc_ecc_disable(docg3);
+	doc_read_data_area(docg3, &status, 1, 1);
+	doc_page_finish(docg3);
+	return status;
+}
+
+/**
+ * doc_write_erase_wait_status - wait for write or erase completion
+ * @docg3: the device
+ *
+ * Wait for the chip to be ready again after erase or write operation, and check
+ * erase/write status.
+ *
+ * Returns 0 if erase successfull, -EIO if erase/write issue, -ETIMEOUT if
+ * timeout
+ */
+static int doc_write_erase_wait_status(struct docg3 *docg3)
+{
+	int status, ret = 0;
+
+	if (!doc_is_ready(docg3))
+		usleep_range(3000, 3000);
+	if (!doc_is_ready(docg3)) {
+		doc_dbg("Timeout reached and the chip is still not ready\n");
+		ret = -EAGAIN;
+		goto out;
+	}
+
+	status = doc_get_op_status(docg3);
+	if (status & DOC_PLANES_STATUS_FAIL) {
+		doc_dbg("Erase/Write failed on (a) plane(s), status = %x\n",
+			status);
+		ret = -EIO;
+	}
+
+out:
+	doc_writeb(docg3, 0, DOC_DATAEND);
+	doc_delay(docg3, 2);
+	return ret;
+}
+
+/**
+ * doc_write_page - Write a single page to the chip
+ * @docg3: the device
+ * @to: the offset from first block and first page, in bytes, aligned on page
+ *      size
+ * @buf: buffer to get bytes from
+ * @oob: buffer to get out of band bytes from (can be NULL if no OOB should be
+ *       written)
+ * @autoecc: if 0, all 16 bytes from OOB are taken, regardless of HW Hamming or
+ *           BCH computations. If 1, only bytes 0-7 and byte 15 are taken,
+ *           remaining ones are filled with hardware Hamming and BCH
+ *           computations. Its value is not meaningfull is oob == NULL.
+ *
+ * Write one full page (ie. 1 page split on two planes), of 512 bytes, with the
+ * OOB data. The OOB ECC is automatically computed by the hardware Hamming and
+ * BCH generator if autoecc is not null.
+ *
+ * Returns 0 if write successful, -EIO if write error, -EAGAIN if timeout
+ */
+static int doc_write_page(struct docg3 *docg3, loff_t to, const u_char *buf,
+			  const u_char *oob, int autoecc)
+{
+	int block0, block1, page, ret, ofs = 0;
+	u8 syn[DOC_ECC_BCH_SIZE], hamming;
+
+	doc_dbg("doc_write_page(to=%lld)\n", to);
+	calc_block_sector(to, &block0, &block1, &page, &ofs);
+
+	doc_set_device_id(docg3, docg3->device_id);
+	ret = doc_reset_seq(docg3);
+	if (ret)
+		goto err;
+
+	/* Program the flash address block and page */
+	ret = doc_write_seek(docg3, block0, block1, page, ofs);
+	if (ret)
+		goto err;
+
+	doc_write_page_ecc_init(docg3, DOC_ECC_BCH_COVERED_BYTES);
+	doc_delay(docg3, 2);
+	doc_write_page_putbytes(docg3, DOC_LAYOUT_PAGE_SIZE, buf);
+
+	if (oob && autoecc) {
+		doc_write_page_putbytes(docg3, DOC_LAYOUT_OOB_PAGEINFO_SZ, oob);
+		doc_delay(docg3, 2);
+		oob += DOC_LAYOUT_OOB_UNUSED_OFS;
+
+		hamming = doc_register_readb(docg3, DOC_HAMMINGPARITY);
+		doc_delay(docg3, 2);
+		doc_write_page_putbytes(docg3, DOC_LAYOUT_OOB_HAMMING_SZ,
+					&hamming);
+		doc_delay(docg3, 2);
+
+		doc_get_hw_bch_syndroms(docg3, syn);
+		doc_write_page_putbytes(docg3, DOC_LAYOUT_OOB_BCH_SZ, syn);
+		doc_delay(docg3, 2);
+
+		doc_write_page_putbytes(docg3, DOC_LAYOUT_OOB_UNUSED_SZ, oob);
+	}
+	if (oob && !autoecc)
+		doc_write_page_putbytes(docg3, DOC_LAYOUT_OOB_SIZE, oob);
+
+	doc_delay(docg3, 2);
+	doc_page_finish(docg3);
+	doc_delay(docg3, 2);
+	doc_flash_command(docg3, DOC_CMD_PROG_CYCLE2);
+	doc_delay(docg3, 2);
+
+	/*
+	 * The wait status will perform another doc_page_finish() call, but that
+	 * seems to please the docg3, so leave it.
+	 */
+	ret = doc_write_erase_wait_status(docg3);
+	return ret;
+err:
+	doc_read_page_finish(docg3);
+	return ret;
+}
+
+/**
+ * doc_write - Write a buffer to the chip
+ * @mtd: the device
+ * @to: the offset from first block and first page, in bytes, aligned on page
+ *      size
+ * @len: the number of bytes to write (must be a full page size, ie. 512)
+ * @retlen: the number of bytes actually written (0 or 512)
+ * @buf: the buffer to get bytes from
+ *
+ * Writes data to the chip. As there is need to have an OOB at hand, and only
+ * one OOB data is in the docg3 private data, only one page can be written at
+ * once.
+ *
+ * Returns 0 if write successful, -EIO if write error
+ */
+static int doc_write(struct mtd_info *mtd, loff_t to, size_t len,
+		     size_t *retlen, const u_char *buf)
+{
+	struct docg3 *docg3 = mtd->priv;
+	int block0, block1, page, ofs, ret = 0;
+
+	doc_dbg("doc_write(to=%lld, len=%zu)\n", to, len);
+	calc_block_sector(to, &block0, &block1, &page, &ofs);
+	if (block1 > docg3->max_block || len != mtd->writesize ||
+	    to + len > mtd->size || to != docg3->oob_write_ofs)
+		return -EINVAL;
+
+	*retlen = 0;
+	while (!ret && len > 0) {
+		ret = doc_write_page(docg3, to, buf, docg3->oob_write_buf,
+				     docg3->oob_autoecc);
+		to += DOC_LAYOUT_PAGE_SIZE;
+		len -= DOC_LAYOUT_PAGE_SIZE;
+		buf += DOC_LAYOUT_PAGE_SIZE;
+		*retlen += DOC_LAYOUT_PAGE_SIZE;
+	}
+
+	doc_set_device_id(docg3, 0);
+	return ret;
+}
+
+/**
+ * doc_write_oob - Write out of band bytes to flash
+ * @mtd: the device
+ * @ofs: the offset from first block and first page, in bytes, aligned on page
+ *       size
+ * @ops: the mtd oob structure
+ *
+ * Write OOB data into a temporary buffer, for the subsequent write page. The
+ * provided OOB should be 16 bytes long. If a data buffer is provided as well,
+ * issue the page write.
+ *
+ * Returns 0 is successfull, EINVAL if length is not 14 bytes
+ */
+static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
+			 struct mtd_oob_ops *ops)
+{
+	struct docg3 *docg3 = mtd->priv;
+
+	doc_dbg("doc_write_oob(from=%lld, buf=%p, len=%zu, mode=%d, "
+		"oob=%p, ooblen=%zu)\n", ofs, ops->datbuf, ops->len, ops->mode,
+		ops->oobbuf, ops->ooblen);
+	if (ops->oobbuf) {
+		if (ops->ooblen != DOC_LAYOUT_OOB_SIZE)
+			return -EINVAL;
+		switch (ops->mode) {
+		case MTD_OPS_PLACE_OOB:
+		case MTD_OPS_RAW:
+			docg3->oob_autoecc = 1;
+			docg3->oob_write_ofs = ofs;
+			memcpy(docg3->oob_write_buf, ops->oobbuf,
+			       DOC_LAYOUT_OOB_SIZE);
+			return 0;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	ops->retlen = 0;
+	if (ops->datbuf)
+		return doc_write(mtd, ofs, ops->len, &ops->retlen, ops->datbuf);
+	else
+		return 0;
+}
+
 /*
  * Debug sysfs entries
  */
@@ -1054,6 +1461,7 @@ static struct mtd_info *doc_probe_device(void __iomem *base, int floor,
 
 	doc_set_driver_info(chip_id, mtd);
 
+	doc_hamming_ecc_init(docg3, DOC_LAYOUT_OOB_PAGEINFO_SZ);
 	doc_reload_bbt(docg3);
 	return mtd;
 
-- 
1.7.5.4

[PATCH 10/13] mtd/docg3: add erase functions

[Robert Jarzmik]

Add erase capability to the docg3 driver. The erase block is
made of 2 physical blocks, as both share all 64 pages. That
makes an erase block of at least 64 kBytes.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |   90 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 92522e0..6e323f3 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -1037,6 +1037,96 @@ out:
 }
 
 /**
+ * doc_erase_block - Erase a couple of blocks
+ * @docg3: the device
+ * @block0: the first block to erase (leftmost plane)
+ * @block1: the second block to erase (rightmost plane)
+ *
+ * Erase both blocks, and return operation status
+ *
+ * Returns 0 if erase successful, -EIO if erase issue, -ETIMEOUT if chip not
+ * ready for too long
+ */
+static int doc_erase_block(struct docg3 *docg3, int block0, int block1)
+{
+	int ret, sector;
+
+	doc_dbg("doc_erase_block(blocks=(%d,%d))\n", block0, block1);
+	ret = doc_reset_seq(docg3);
+	if (ret)
+		return -EIO;
+
+	doc_set_reliable_mode(docg3);
+	doc_flash_sequence(docg3, DOC_SEQ_ERASE);
+
+	sector = block0 << DOC_ADDR_BLOCK_SHIFT;
+	doc_flash_command(docg3, DOC_CMD_PROG_BLOCK_ADDR);
+	doc_setup_addr_sector(docg3, sector);
+	sector = block1 << DOC_ADDR_BLOCK_SHIFT;
+	doc_flash_command(docg3, DOC_CMD_PROG_BLOCK_ADDR);
+	doc_setup_addr_sector(docg3, sector);
+	doc_delay(docg3, 1);
+
+	doc_flash_command(docg3, DOC_CMD_ERASECYCLE2);
+	doc_delay(docg3, 2);
+
+	if (is_prot_seq_error(docg3)) {
+		doc_err("Erase blocks %d,%d error\n", block0, block1);
+		return -EIO;
+	}
+
+	return doc_write_erase_wait_status(docg3);
+}
+
+/**
+ * doc_erase - Erase a portion of the chip
+ * @mtd: the device
+ * @info: the erase info
+ *
+ * Erase a bunch of contiguous blocks, by pairs, as a "mtd" page of 1024 is
+ * split into 2 pages of 512 bytes on 2 contiguous blocks.
+ *
+ * Returns 0 if erase successful, -EINVAL if adressing error, -EIO if erase
+ * issue
+ */
+static int doc_erase(struct mtd_info *mtd, struct erase_info *info)
+{
+	struct docg3 *docg3 = mtd->priv;
+	uint64_t len;
+	int block0, block1, page, ret, ofs = 0;
+
+	doc_dbg("doc_erase(from=%lld, len=%lld\n", info->addr, info->len);
+	doc_set_device_id(docg3, docg3->device_id);
+
+	info->state = MTD_ERASE_PENDING;
+	calc_block_sector(info->addr + info->len,
+			  &block0, &block1, &page, &ofs);
+	ret = -EINVAL;
+	if (block1 > docg3->max_block || page || ofs)
+		goto reset_err;
+
+	ret = 0;
+	calc_block_sector(info->addr, &block0, &block1, &page, &ofs);
+	doc_set_reliable_mode(docg3);
+	for (len = info->len; !ret && len > 0; len -= mtd->erasesize) {
+		info->state = MTD_ERASING;
+		ret = doc_erase_block(docg3, block0, block1);
+		block0 += 2;
+		block1 += 2;
+	}
+
+	if (ret)
+		goto reset_err;
+
+	info->state = MTD_ERASE_DONE;
+	return 0;
+
+reset_err:
+	info->state = MTD_ERASE_FAILED;
+	return ret;
+}
+
+/**
  * doc_write_page - Write a single page to the chip
  * @docg3: the device
  * @to: the offset from first block and first page, in bytes, aligned on page
-- 
1.7.5.4

[PATCH 11/13] mtd/docg3: map erase and write functions

[Robert Jarzmik]

Map the developped write and erase functions into the mtd
structure.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 6e323f3..9983594 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -41,8 +41,6 @@
  *
  * As no specification is available from M-Systems/Sandisk, this drivers lacks
  * several functions available on the chip, as :
- *  - block erase
- *  - page write
  *  - IPL write
  *  - ECC fixing (lack of BCH algorith understanding)
  *  - powerdown / powerup
@@ -1467,23 +1465,19 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
 		break;
 	}
 	mtd->type = MTD_NANDFLASH;
-	/*
-	 * Once write methods are added, the correct flags will be set.
-	 * mtd->flags = MTD_CAP_NANDFLASH;
-	 */
-	mtd->flags = MTD_CAP_ROM;
+	mtd->flags = MTD_CAP_NANDFLASH;
 	mtd->size = (docg3->max_block + 1) * DOC_LAYOUT_BLOCK_SIZE;
 	mtd->erasesize = DOC_LAYOUT_BLOCK_SIZE * DOC_LAYOUT_NBPLANES;
 	mtd->writesize = DOC_LAYOUT_PAGE_SIZE;
 	mtd->oobsize = DOC_LAYOUT_OOB_SIZE;
 	mtd->owner = THIS_MODULE;
-	mtd->erase = NULL;
+	mtd->erase = doc_erase;
 	mtd->point = NULL;
 	mtd->unpoint = NULL;
 	mtd->read = doc_read;
-	mtd->write = NULL;
+	mtd->write = doc_write;
 	mtd->read_oob = doc_read_oob;
-	mtd->write_oob = NULL;
+	mtd->write_oob = doc_write_oob;
 	mtd->sync = NULL;
 	mtd->block_isbad = doc_block_isbad;
 	mtd->ecclayout = &docg3_oobinfo;
-- 
1.7.5.4

[PATCH 12/13] mtd/docg3: add ECC correction code

[Robert Jarzmik]

Credit for discovering the BCH algorith parameters, and bit
reversing algorithm is to be give to Mike Dunn and Ivan
Djelic.

The BCH correction code relied upon the BCH library, where
all data and ECC is bit-reversed. The BCH library works
correctly when each input byte is bit-reversed, and
accordingly ECC output is also bit-reversed.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/Kconfig |    1 +
 drivers/mtd/devices/docg3.c |  118 ++++++++++++++++++++++++++++++++-----------
 drivers/mtd/devices/docg3.h |   11 ++++-
 3 files changed, 99 insertions(+), 31 deletions(-)

diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig
index 6d91a1f..ae138c3 100644
--- a/drivers/mtd/devices/Kconfig
+++ b/drivers/mtd/devices/Kconfig
@@ -251,6 +251,7 @@ config MTD_DOC2001PLUS
 
 config MTD_DOCG3
 	tristate "M-Systems Disk-On-Chip G3"
+	select BCH
 	---help---
 	  This provides an MTD device driver for the M-Systems DiskOnChip
 	  G3 devices.
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 9983594..3c4f7ed 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -29,6 +29,9 @@
 #include <linux/delay.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/bitmap.h>
+#include <linux/bitrev.h>
+#include <linux/bch.h>
 
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
@@ -42,7 +45,6 @@
  * As no specification is available from M-Systems/Sandisk, this drivers lacks
  * several functions available on the chip, as :
  *  - IPL write
- *  - ECC fixing (lack of BCH algorith understanding)
  *  - powerdown / powerup
  *
  * The bus data width (8bits versus 16bits) is not handled (if_cfg flag), and
@@ -51,8 +53,7 @@
  * DocG3 relies on 2 ECC algorithms, which are handled in hardware :
  *  - a 1 byte Hamming code stored in the OOB for each page
  *  - a 7 bytes BCH code stored in the OOB for each page
- * The BCH part is only used for check purpose, no correction is available as
- * some information is missing. What is known is that :
+ * The BCH ECC is :
  *  - BCH is in GF(2^14)
  *  - BCH is over data of 520 bytes (512 page + 7 page_info bytes
  *                                   + 1 hamming byte)
@@ -73,6 +74,11 @@ static struct nand_ecclayout docg3_oobinfo = {
 	.oobfree = {{0, 7}, {15, 1} }
 };
 
+/**
+ * struct docg3_bch - BCH engine
+ */
+static struct bch_control *docg3_bch;
+
 static inline u8 doc_readb(struct docg3 *docg3, u16 reg)
 {
 	u8 val = readb(docg3->base + reg);
@@ -576,6 +582,43 @@ static void doc_hamming_ecc_init(struct docg3 *docg3, int nb_bytes)
 }
 
 /**
+ * doc_correct_data - Fix if need be read data from flash
+ * @docg3: the device
+ * @buf: the buffer of read data (512 + 7 + 1 bytes)
+ * @calc_ecc: the hardware calculated ECC
+ *
+ * Checks if the received data matches the ECC, and if an error is detected,
+ * tries to fix the bit flips (at most 4) in the buffer buf.  As the docg3
+ * understands the (data, ecc, syndroms) in an inverted order in comparison to
+ * the BCH library, the function reverses the order of bits (ie. bit7 and bit0,
+ * bit6 and bit 1, ...) for all ECC data.
+ *
+ * Returns number of fixed bits (0, 1, 2, 3, 4) or -EBADMSG if too many bit
+ * errors were detected and cannot be fixed.
+ */
+static int doc_ecc_bch_fix_data(struct docg3 *docg3, void *buf, u8 *calc_ecc)
+{
+	u8 ecc[DOC_ECC_BCH_T + 1];
+	int errorpos[DOC_ECC_BCH_T + 1], i, numerrs;
+
+	for (i = 0; i < DOC_ECC_BCH_SIZE; i++)
+		ecc[i] = bitrev8(calc_ecc[i]);
+	numerrs = decode_bch(docg3_bch, NULL, DOC_ECC_BCH_COVERED_BYTES,
+			     NULL, ecc, NULL, errorpos);
+	if (numerrs < 0)
+		return numerrs;
+
+	for (i = 0; i < numerrs; i++)
+		errorpos[i] = (errorpos[i] & ~7) | (7 - (errorpos[i] & 7));
+	for (i = 0; i < numerrs; i++)
+		change_bit(errorpos[i], buf);
+
+	doc_dbg("doc_ecc_bch_fix_data: flipped %d bits\n", numerrs);
+	return numerrs;
+}
+
+
+/**
  * doc_read_page_prepare - Prepares reading data from a flash page
  * @docg3: the device
  * @block0: the first plane block index on flash memory
@@ -756,7 +799,7 @@ static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
 {
 	struct docg3 *docg3 = mtd->priv;
 	int block0, block1, page, readlen, ret, ofs = 0;
-	u8 syn[DOC_ECC_BCH_SIZE], eccconf1;
+	u8 calc_ecc[DOC_ECC_BCH_SIZE], eccconf1;
 	u8 oob[DOC_LAYOUT_OOB_SIZE];
 
 	ret = -EINVAL;
@@ -777,7 +820,7 @@ static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
 		ret = doc_read_page_prepare(docg3, block0, block1, page, ofs);
 		if (ret < 0)
 			goto err;
-		ret = doc_read_page_ecc_init(docg3, DOC_ECC_BCH_COVERED_BYTES);
+		ret = doc_read_page_ecc_init(docg3, DOC_ECC_BCH_TOTAL_BYTES);
 		if (ret < 0)
 			goto err_in_read;
 		ret = doc_read_page_getbytes(docg3, readlen, buf, 1);
@@ -788,24 +831,11 @@ static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
 		if (ret < DOC_LAYOUT_OOB_SIZE)
 			goto err_in_read;
 
-		*retlen += readlen;
-		buf += readlen;
-		len -= readlen;
-
-		ofs ^= DOC_LAYOUT_PAGE_OOB_SIZE;
-		if (ofs == 0)
-			page += 2;
-		if (page > DOC_ADDR_PAGE_MASK) {
-			page = 0;
-			block0 += 2;
-			block1 += 2;
-		}
-
 		/*
 		 * There should be a BCH bitstream fixing algorithm here ...
 		 * By now, a page read failure is triggered by BCH error
 		 */
-		doc_get_hw_bch_syndroms(docg3, syn);
+		doc_get_hw_bch_syndroms(docg3, calc_ecc);
 		eccconf1 = doc_register_readb(docg3, DOC_ECCCONF1);
 
 		doc_dbg("OOB - INFO: %02x:%02x:%02x:%02x:%02x:%02x:%02x\n",
@@ -817,20 +847,41 @@ static int doc_read(struct mtd_info *mtd, loff_t from, size_t len,
 			 oob[13], oob[14]);
 		doc_dbg("OOB - UNUSED: %02x\n", oob[15]);
 		doc_dbg("ECC checks: ECCConf1=%x\n", eccconf1);
-		doc_dbg("ECC BCH syndrom: %02x:%02x:%02x:%02x:%02x:%02x:%02x\n",
-			syn[0], syn[1], syn[2], syn[3], syn[4], syn[5], syn[6]);
+		doc_dbg("ECC CALC_ECC: %02x:%02x:%02x:%02x:%02x:%02x:%02x\n",
+			calc_ecc[0], calc_ecc[1], calc_ecc[2], calc_ecc[3],
+			calc_ecc[4], calc_ecc[5], calc_ecc[6]);
 
 		ret = -EBADMSG;
-		if (block0 >= DOC_LAYOUT_BLOCK_FIRST_DATA) {
-			if (eccconf1 & DOC_ECCCONF1_BCH_SYNDROM_ERR)
-				goto err_in_read;
-			if (is_prot_seq_error(docg3))
-				goto err_in_read;
+		if (is_prot_seq_error(docg3))
+			goto err_in_read;
+		ret = 0;
+		if ((block0 >= DOC_LAYOUT_BLOCK_FIRST_DATA) &&
+		    (eccconf1 & DOC_ECCCONF1_BCH_SYNDROM_ERR)) {
+			ret = doc_ecc_bch_fix_data(docg3, buf, calc_ecc);
+			if (ret < 0)
+				mtd->ecc_stats.failed++;
+			if (ret > 0) {
+				mtd->ecc_stats.corrected += ret;
+				ret = 0;
+			}
 		}
+
 		doc_read_page_finish(docg3);
+		*retlen += readlen;
+		buf += readlen;
+		len -= readlen;
+
+		ofs ^= DOC_LAYOUT_PAGE_OOB_SIZE;
+		if (ofs == 0)
+			page += 2;
+		if (page > DOC_ADDR_PAGE_MASK) {
+			page = 0;
+			block0 += 2;
+			block1 += 2;
+		}
 	}
 
-	return 0;
+	return ret;
 err_in_read:
 	doc_read_page_finish(docg3);
 err:
@@ -1162,7 +1213,7 @@ static int doc_write_page(struct docg3 *docg3, loff_t to, const u_char *buf,
 	if (ret)
 		goto err;
 
-	doc_write_page_ecc_init(docg3, DOC_ECC_BCH_COVERED_BYTES);
+	doc_write_page_ecc_init(docg3, DOC_ECC_BCH_TOTAL_BYTES);
 	doc_delay(docg3, 2);
 	doc_write_page_putbytes(docg3, DOC_LAYOUT_PAGE_SIZE, buf);
 
@@ -1602,7 +1653,11 @@ static int __init docg3_probe(struct platform_device *pdev)
 	docg3_floors = kzalloc(sizeof(*docg3_floors) * DOC_MAX_NBFLOORS,
 			       GFP_KERNEL);
 	if (!docg3_floors)
-		goto nomem;
+		goto nomem1;
+	docg3_bch = init_bch(DOC_ECC_BCH_M, DOC_ECC_BCH_T,
+			     DOC_ECC_BCH_PRIMPOLY);
+	if (!docg3_bch)
+		goto nomem2;
 
 	ret = 0;
 	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++) {
@@ -1635,7 +1690,9 @@ err_probe:
 	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++)
 		if (docg3_floors[floor])
 			doc_release_device(docg3_floors[floor]);
-nomem:
+nomem2:
+	kfree(docg3_floors);
+nomem1:
 	iounmap(base);
 noress:
 	return ret;
@@ -1660,6 +1717,7 @@ static int __exit docg3_release(struct platform_device *pdev)
 			doc_release_device(docg3_floors[floor]);
 
 	kfree(docg3_floors);
+	kfree(docg3_bch);
 	iounmap(base);
 	return 0;
 }
diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index 397e461..33db727 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -51,10 +51,19 @@
 #define DOC_LAYOUT_WEAR_OFFSET		(DOC_LAYOUT_PAGE_OOB_SIZE * 2)
 #define DOC_LAYOUT_BLOCK_SIZE					\
 	(DOC_LAYOUT_PAGES_PER_BLOCK * DOC_LAYOUT_PAGE_SIZE)
+
+/*
+ * ECC related constants
+ */
+#define DOC_ECC_BCH_M			14
+#define DOC_ECC_BCH_T			4
+#define DOC_ECC_BCH_PRIMPOLY		0x4443
 #define DOC_ECC_BCH_SIZE		7
 #define DOC_ECC_BCH_COVERED_BYTES				\
 	(DOC_LAYOUT_PAGE_SIZE + DOC_LAYOUT_OOB_PAGEINFO_SZ +	\
-	 DOC_LAYOUT_OOB_HAMMING_SZ + DOC_LAYOUT_OOB_BCH_SZ)
+	 DOC_LAYOUT_OOB_HAMMING_SZ)
+#define DOC_ECC_BCH_TOTAL_BYTES					\
+	(DOC_ECC_BCH_COVERED_BYTES + DOC_LAYOUT_OOB_BCH_SZ)
 
 /*
  * Blocks distribution
-- 
1.7.5.4

Re: [PATCH 12/13] mtd/docg3: add ECC correction code

[Ivan Djelic]

On Fri, Oct 28, 2011 at 06:51:31PM +0100, Robert Jarzmik wrote:
>  
> +/**
> + * struct docg3_bch - BCH engine
> + */
> +static struct bch_control *docg3_bch;

Why not putting this into your struct docg3, instead of adding a global var ?

> +static int doc_ecc_bch_fix_data(struct docg3 *docg3, void *buf, u8 *calc_ecc)
> +{
> +	u8 ecc[DOC_ECC_BCH_T + 1];

Should be 'u8 ecc[DOC_ECC_BCH_SIZE];'

> +	int errorpos[DOC_ECC_BCH_T + 1], i, numerrs;

Using 'errorpos[DOC_ECC_BCH_T]' is fine, no need for '+ 1'.

(...)

> +
> +	for (i = 0; i < DOC_ECC_BCH_SIZE; i++)
> +		ecc[i] = bitrev8(calc_ecc[i]);
> +	numerrs = decode_bch(docg3_bch, NULL, DOC_ECC_BCH_COVERED_BYTES,
> +			     NULL, ecc, NULL, errorpos);
> +	if (numerrs < 0)
> +		return numerrs;

Maybe do something like 'printk(KERN_ERR "ecc unrecoverable error\n");' when
numerrs < 0 ?

(...)

> +	for (i = 0; i < numerrs; i++)
> +		change_bit(errorpos[i], buf);

'buf' holds the buffer of read data (512 + 7 + 1 bytes); hence you should
change the above code into something like:

	for (i = 0; i < numerrs; i++)
		if (errorpos[i] < DOC_ECC_BCH_COVERED_BYTES*8)
			/* error is located in data, correct it */
			change_bit(errorpos[i], buf);
	        /* else error in ecc bytes, no data change needed */

otherwise 'change_bit' will be out of bounds when a bitflip occurs in ecc
bytes. Note that we still need to report bitflips in that case, to let upper
layers scrub them.

(...)
> +	docg3_bch = init_bch(DOC_ECC_BCH_M, DOC_ECC_BCH_T,
> +			     DOC_ECC_BCH_PRIMPOLY);
> +	if (!docg3_bch)
> +		goto nomem2;

Just a side note: if you need to get maximum performance from the BCH library,
you can set fixed values for M and T in your Kconfig, with something like:

 config MTD_DOCG3
        tristate "M-Systems Disk-On-Chip G3"
	select BCH
        ---help---
          This provides an MTD device driver for the M-Systems DiskOnChip
          G3 devices.

if MTD_DOCG3
config BCH_CONST_M
	default 14
config BCH_CONST_T
	default 4
endif


The only drawback is that it won't work if you select your DOCG3 driver and, at
the same time, other MTD drivers that also use fixed, but different parameters.

(...)
> @@ -1660,6 +1717,7 @@ static int __exit docg3_release(struct platform_device *pdev)
>  			doc_release_device(docg3_floors[floor]);
>  
>  	kfree(docg3_floors);
> +	kfree(docg3_bch);

This should be 'free_bch(docg3_bch)'.

Otherwise it looks OK to me; did you test BCH correction by simulating
corruptions (of 1-4 bits, and also 5 bits to trigger failures) in nand data ?

Best Regards,
--
Ivan

Re: [PATCH 12/13] mtd/docg3: add ECC correction code

[Ivan Djelic]

On Sat, Oct 29, 2011 at 10:52:48AM +0200, Ivan Djelic wrote:
> Just a side note: if you need to get maximum performance from the BCH library,
> you can set fixed values for M and T in your Kconfig, with something like:
> 
>  config MTD_DOCG3
>         tristate "M-Systems Disk-On-Chip G3"
> 	select BCH

Oops, just noticed that you would also need to add this line here:
	select BCH_CONST_PARAMS

>         ---help---
>           This provides an MTD device driver for the M-Systems DiskOnChip
>           G3 devices.
> 
> if MTD_DOCG3
> config BCH_CONST_M
> 	default 14
> config BCH_CONST_T
> 	default 4
> endif
> 
> 
> The only drawback is that it won't work if you select your DOCG3 driver and, at
> the same time, other MTD drivers that also use fixed, but different parameters.

Re: [PATCH 12/13] mtd/docg3: add ECC correction code

[Robert Jarzmik]

Ivan Djelic <ivan.djelic@parrot.com> writes:

> On Fri, Oct 28, 2011 at 06:51:31PM +0100, Robert Jarzmik wrote:
>>  
>> +/**
>> + * struct docg3_bch - BCH engine
>> + */
>> +static struct bch_control *docg3_bch;
>
> Why not putting this into your struct docg3, instead of adding a global var ?
Because I have multiple floors (ie. 4 floors for example), which are split into
4 different devices. If I put that in docg3 structures (ie. the 4 allocated
structures, each for one floor), I'd either have to :
 - allocate 4 different bch "engines"
 - or count docg3 releases and release the bch at the last kfree(docg3), which
 makes me have another global variable.


>
>> +static int doc_ecc_bch_fix_data(struct docg3 *docg3, void *buf, u8 *calc_ecc)
>> +{
>> +	u8 ecc[DOC_ECC_BCH_T + 1];
>
> Should be 'u8 ecc[DOC_ECC_BCH_SIZE];'
OK, I'll amend it.

>
>> +	int errorpos[DOC_ECC_BCH_T + 1], i, numerrs;
>
> Using 'errorpos[DOC_ECC_BCH_T]' is fine, no need for '+ 1'.
OK, got it.

>> +
>> +	for (i = 0; i < DOC_ECC_BCH_SIZE; i++)
>> +		ecc[i] = bitrev8(calc_ecc[i]);
>> +	numerrs = decode_bch(docg3_bch, NULL, DOC_ECC_BCH_COVERED_BYTES,
>> +			     NULL, ecc, NULL, errorpos);
>> +	if (numerrs < 0)
>> +		return numerrs;
>
> Maybe do something like 'printk(KERN_ERR "ecc unrecoverable error\n");' when
> numerrs < 0 ?
That will be too verbose. As there are special partitions where the ECC is not
used that way (ie. SAFTL partitions I don't understand well yet), the ECC is
always wrong there.
Moreover, the error is reported as EBADMSG later (in doc_read), making the
information available to userland.

>
> (...)
>
>> +	for (i = 0; i < numerrs; i++)
>> +		change_bit(errorpos[i], buf);
>
> 'buf' holds the buffer of read data (512 + 7 + 1 bytes); hence you should
> change the above code into something like:
>
> 	for (i = 0; i < numerrs; i++)
> 		if (errorpos[i] < DOC_ECC_BCH_COVERED_BYTES*8)
> 			/* error is located in data, correct it */
> 			change_bit(errorpos[i], buf);
> 	        /* else error in ecc bytes, no data change needed */
>
> otherwise 'change_bit' will be out of bounds when a bitflip occurs in ecc
> bytes. Note that we still need to report bitflips in that case, to let upper
> layers scrub them.
Ah OK, I wasn't aware of that. I'll amend the code, thanks.

>
> (...)
>> +	docg3_bch = init_bch(DOC_ECC_BCH_M, DOC_ECC_BCH_T,
>> +			     DOC_ECC_BCH_PRIMPOLY);
>> +	if (!docg3_bch)
>> +		goto nomem2;
>
> Just a side note: if you need to get maximum performance from the BCH library,
> you can set fixed values for M and T in your Kconfig, with something like:
>
>  config MTD_DOCG3
>         tristate "M-Systems Disk-On-Chip G3"
> 	select BCH
>         ---help---
>           This provides an MTD device driver for the M-Systems DiskOnChip
>           G3 devices.
>
> if MTD_DOCG3
> config BCH_CONST_M
> 	default 14
> config BCH_CONST_T
> 	default 4
> endif
>
>
> The only drawback is that it won't work if you select your DOCG3 driver and, at
> the same time, other MTD drivers that also use fixed, but different
> parameters.
Oh, that seems nice. As I'm working with a smartphone, there is only one mtd
inside, so the speed-up will be nice.

>
> (...)
>> @@ -1660,6 +1717,7 @@ static int __exit docg3_release(struct platform_device *pdev)
>>  			doc_release_device(docg3_floors[floor]);
>>  
>>  	kfree(docg3_floors);
>> +	kfree(docg3_bch);
>
> This should be 'free_bch(docg3_bch)'.
Right.

>
> Otherwise it looks OK to me; did you test BCH correction by simulating
> corruptions (of 1-4 bits, and also 5 bits to trigger failures) in nand data ?
I did test the algorithm, yes.
To be more precise, I tested your last BCH encoding algorithm (emulate_docg4_hw)
which gives exactly the same ECC.

I then flipped 2 bits (buf[0] ^= 0x01 and buf[1] ^= 0x02), and got correct
errorpos (ie. 0 and 10 IIRC).

The thing I didn't check is the decode_bch() call with the hardware calculated
ECC. I tried on my PC:
  decode_bch(bch, data, 520, ecc, NULL, NULL, errorpos);
  => this *does* work
while in the kernel I did:
  decode_bch(docg3_bch, NULL, DOC_ECC_BCH_COVERED_BYTES,
			     NULL, ecc, NULL, errorpos);
  => this might work...

What I'm a bit afraid of is my poor understanding of the hardware ECC engine. I
know that the write part is correct (ie. ECC calculation), but I'm a bit
confused by the read part.

What wories me is that the hardware ECC got back while reading (ie. what I
called calc_ecc) is always 00:00:00:00:00:00:00 when I read data (because I
don't have bitflips on my flash). This looks to me more a "syndrom" than a
"calc_ecc".

To be sure, I could write a page of 512 bytes + 16 bytes, where the BCH would be
forced (and incorrect), to check what the hardware generator gives me back. I'd
like you to help me, ie:
 - tell me what to write to the first 512 bytes (only 0, all 0 but one byte to
 1, other ...)
 - I think I'll write 8 bytes to 0x01 for the first 8 OOB bytes (Hamming false
 but I won't care)
 - tell me what to write to the 7 BCH ECC

That way, I could provide you the result and you could tell me if
doc_ecc_bch_fix_data() is correct or not. Would you agree to spend some time on
that ?

Cheers.

--
Robert

Re: [PATCH 12/13] mtd/docg3: add ECC correction code

[Ivan Djelic]

On Sat, Oct 29, 2011 at 05:37:35PM +0100, Robert Jarzmik wrote:
> >> +static struct bch_control *docg3_bch;
> >
> > Why not putting this into your struct docg3, instead of adding a global var ?
> Because I have multiple floors (ie. 4 floors for example), which are split into
> 4 different devices. If I put that in docg3 structures (ie. the 4 allocated
> structures, each for one floor), I'd either have to :
>  - allocate 4 different bch "engines"
>  - or count docg3 releases and release the bch at the last kfree(docg3), which
>  makes me have another global variable.

OK, got it; using a struct to hold all your common vars (docg3_floors,
docg3_bch, ...) and hook that to your platform data instead of docg3_floors
would still be a bit cleaner I think, but no big deal.

> What I'm a bit afraid of is my poor understanding of the hardware ECC engine. I
> know that the write part is correct (ie. ECC calculation), but I'm a bit
> confused by the read part.
> 
> What wories me is that the hardware ECC got back while reading (ie. what I
> called calc_ecc) is always 00:00:00:00:00:00:00 when I read data (because I
> don't have bitflips on my flash). This looks to me more a "syndrom" than a
> "calc_ecc".

OK, I'll try to clarify that. The hardware ECC engine divides a huge polynomial
(520*8 = 4160 bits) by a generator polynomial and computes a 56-bit remainder.
So this remainder (let's call it R) depends only on 520 input data bytes.

- during a write operation: input data is what you write to the controller,
you get R from the ecc engine and this is what you write to oob[8..14].

- during a read operation: the ecc engine computes R on 520 input bytes read
from flash (this is calc_ecc), and also reads oob[8..14] (this is recv_ecc,
previously programmed during the write operation).
Then the ecc engine computes calc_ecc^recv_ecc, and this is what you get from
the ecc registers. And as long as there is no bitflip, its all 00s (because
calc_ecc=recv_ecc).

> To be sure, I could write a page of 512 bytes + 16 bytes, where the BCH would be
> forced (and incorrect), to check what the hardware generator gives me back. I'd
> like you to help me, ie:
>  - tell me what to write to the first 512 bytes (only 0, all 0 but one byte to
>  1, other ...)
>  - I think I'll write 8 bytes to 0x01 for the first 8 OOB bytes (Hamming false
>  but I won't care)
>  - tell me what to write to the 7 BCH ECC

OK, this is really simple:

1. Prepare a buffer of 520 bytes of data, containing pseudo-random bytes or
any pattern you like. Let's call this buffer 'ref_buf'.

2. Program 'ref_buf' to a nand page; you will write ecc bytes to oob during
that operation; let's call those ecc bytes 'ref_ecc'.

3. Now, you are ready to perform corruption tests:

 3.1 Make a copy of 'ref_buf' in which you flip 1, 2, 3 or 4 bits selected
     at random.

 3.2 Program this corrupt buffer, _but_ write 'ref_ecc' to oob instead of hw
     generated ecc bytes.

 3.3 Read page back: you should get exactly 'ref_buf', and the errorpos[]
     array of corrected bits should match your flip bits.

After step 3.2, your flash is exactly in the same state as if it had produced
the bitflips itself.

Repeat steps 3.1 to 3.3 on a large enough set of random vectors to convince
yourself that your code works (be careful not to wear out your device,
though :-). You should also try a few 5-bit corruptions and see failures, just
to verify that your corruptions have some effect.

In theory, testing the BCH algorithm like you did should be enough, but real
hardware tests are helpful to verify that the entire system behaves as
expected.

Hope that helps,
BR,
--
Ivan

[PATCH 13/13] mtd/docg3: add suspend and resume

[Robert Jarzmik]

Add functions to powerdown and powerup from suspend, in
order to save power.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/mtd/devices/docg3.c |   75 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/mtd/devices/docg3.h |    6 +++
 2 files changed, 80 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 3c4f7ed..9260034 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -45,7 +45,6 @@
  * As no specification is available from M-Systems/Sandisk, this drivers lacks
  * several functions available on the chip, as :
  *  - IPL write
- *  - powerdown / powerup
  *
  * The bus data width (8bits versus 16bits) is not handled (if_cfg flag), and
  * the driver assumes a 16bits data bus.
@@ -1624,6 +1623,78 @@ static void doc_release_device(struct mtd_info *mtd)
 }
 
 /**
+ * docg3_resume - Awakens docg3 floor
+ * @pdev: platfrom device
+ *
+ * Returns 0 (always successfull)
+ */
+static int docg3_resume(struct platform_device *pdev)
+{
+	int i;
+	struct mtd_info **docg3_floors, *mtd;
+	struct docg3 *docg3;
+
+	docg3_floors = platform_get_drvdata(pdev);
+	mtd = docg3_floors[0];
+	docg3 = mtd->priv;
+
+	doc_dbg("docg3_resume()\n");
+	for (i = 0; i < 12; i++)
+		doc_readb(docg3, DOC_IOSPACE_IPL);
+	return 0;
+}
+
+/**
+ * docg3_suspend - Put in low power mode the docg3 floor
+ * @pdev: platform device
+ * @state: power state
+ *
+ * Shuts off most of docg3 circuitery to lower power consumption.
+ *
+ * Returns 0 if suspend succeeded, -EIO if chip refused suspend
+ */
+static int docg3_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	int floor, i;
+	struct mtd_info **docg3_floors, *mtd;
+	struct docg3 *docg3;
+	u8 ctrl, pwr_down;
+
+	docg3_floors = platform_get_drvdata(pdev);
+	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++) {
+		mtd = docg3_floors[floor];
+		if (!mtd)
+			continue;
+		docg3 = mtd->priv;
+
+		doc_writeb(docg3, floor, DOC_DEVICESELECT);
+		ctrl = doc_register_readb(docg3, DOC_FLASHCONTROL);
+		ctrl &= ~DOC_CTRL_VIOLATION & ~DOC_CTRL_CE;
+		doc_writeb(docg3, ctrl, DOC_FLASHCONTROL);
+
+		for (i = 0; i < 10; i++) {
+			usleep_range(3000, 4000);
+			pwr_down = doc_register_readb(docg3, DOC_POWERMODE);
+			if (pwr_down & DOC_POWERDOWN_READY)
+				break;
+		}
+		if (pwr_down & DOC_POWERDOWN_READY) {
+			doc_dbg("docg3_suspend(): floor %d powerdown ok\n",
+				floor);
+		} else {
+			doc_err("docg3_suspend(): floor %d powerdown failed\n",
+				floor);
+			return -EIO;
+		}
+	}
+
+	mtd = docg3_floors[0];
+	docg3 = mtd->priv;
+	doc_set_asic_mode(docg3, DOC_ASICMODE_POWERDOWN);
+	return 0;
+}
+
+/**
  * doc_probe - Probe the IO space for a DiskOnChip G3 chip
  * @pdev: platform device
  *
@@ -1727,6 +1798,8 @@ static struct platform_driver g3_driver = {
 		.name	= "docg3",
 		.owner	= THIS_MODULE,
 	},
+	.suspend	= docg3_suspend,
+	.resume		= docg3_resume,
 	.remove		= __exit_p(docg3_release),
 };
 
diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index 33db727..cd70b18 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -127,6 +127,7 @@
 
 #define DOC_ASICMODECONFIRM		0x1072
 #define DOC_CHIPID_INV			0x1074
+#define DOC_POWERMODE			0x107c
 
 /*
  * Flash sequences
@@ -239,6 +240,11 @@
 #define DOC_READADDR_ADDR_MASK		0x1fff
 
 /*
+ * Flash register : DOC_POWERMODE
+ */
+#define DOC_POWERDOWN_READY		0x80
+
+/*
  * Status of erase and write operation
  */
 #define DOC_PLANES_STATUS_FAIL		0x01
-- 
1.7.5.4

Re: [PATCH 00/13] DocG3 fixes and write support

[Marek Vasut]

> This patchset is aimed at :
>  - fixing 4 glitches in the driver
>  - add several new functions
> 
> The added functions are :
>  - add block erase capability
>  - add write page capability
>  - add powerdown and powerup
>  - add multiple floor support
>  - add ECC support
> 
> The ECC part should especially be reviewed by Ivan and Mike,
> as this is their work (patch "add ECC correction code").
> 
> Happy review.
> 
> --
> Robert
> 
> Robert Jarzmik (13):
>   mtd/docg3: fix debug log verbosity
>   mtd/docg3: fix tracing of IO in writeb
>   mtd/docg3: fix protection areas reading
>   mtd/docg3: fix BCH registers
>   mtd/docg3: add multiple floor support
>   mtd/docg3: add OOB layout to mtdinfo
>   mtd/docg3: add registers for erasing and writing
>   mtd/docg3: add OOB buffer to device structure
>   mtd/docg3: add write functions
>   mtd/docg3: add erase functions
>   mtd/docg3: map erase and write functions
>   mtd/docg3: add ECC correction code
>   mtd/docg3: add suspend and resume
> 
>  drivers/mtd/devices/Kconfig |    1 +
>  drivers/mtd/devices/docg3.c |  933
> +++++++++++++++++++++++++++++++++++++------ drivers/mtd/devices/docg3.h | 
>  42 ++-
>  3 files changed, 860 insertions(+), 116 deletions(-)

Hi,

can you sum up the situation about the another (docg4) driver for us not 
watching this too tightly? Was there any improvement/progress/... ?

Cheers

Re: [PATCH 00/13] DocG3 fixes and write support

[Robert Jarzmik]

Marek Vasut <marek.vasut@gmail.com> writes:

> Hi,
>
> can you sum up the situation about the another (docg4) driver for us not 
> watching this too tightly? Was there any improvement/progress/... ?
The review is underway, my comments should be dealt with, and a V2 of the patch
should be sent. It's not in my hands.

The global view we have with Mike is that chips are different, and each one
deserves its own driver. Several registers are common, and the docg3.h could
become common.

Cheers.

--
Robert

Re: [PATCH 00/13] DocG3 fixes and write support

[Mike Dunn]

Hi guys,


On 10/30/2011 02:04 AM, Robert Jarzmik wrote:
> Marek Vasut <marek.vasut@gmail.com> writes:
>
>> Hi,
>>
>> can you sum up the situation about the another (docg4) driver for us not 
>> watching this too tightly? Was there any improvement/progress/... ?
> The review is underway, my comments should be dealt with, and a V2 of the patch
> should be sent. It's not in my hands.


I've been busy working on this, and should post a patch within the next few
days.  Progress has been very good.  I still want to run the mtd tests in the
kernel before posting the patch (except the torture test - I'm not ready to
sacrifice my development Treo), but it continues to pass the nandtest userspace
utility (part of mtd-utils) flawlessly, and a ubifs still appears to be working
well.  The code is now in a much cleaner state.


> The global view we have with Mike is that chips are different, and each one
> deserves its own driver. Several registers are common, and the docg3.h could
> become common.
>

Actually, I'm open on this.  My opinion is that they should share a header file
for sure, because the register set largely overlaps.  To that end, my upcoming
patch will adopt Robert's register #defines, with just a few additional ones
that are G4 specific.

On the broader question of combined or separate drivers... a combined driver is
certainly doable.  Each device would have to use its own set of low-level
functions, but I think there's merit in combining them because it would provide
a consistent interface with the mtd nand infrastructure code, which is rather
messy.  But before that discussion can take place, the more fundamental question
of whether or not the drivers should use the nand interface needs to be
resolved.  I used the nand interface when I started on the G4 driver, because it
*is* a nand device, and the legacy diskonchip.c driver was updated not long ago
from a stand-alone driver to the nand interface.  I'm not knowledgeable enough
of the mtd subsystem to argue for or against the nand interface, but the end
result (once I invested the time slogging through nand_base.c) is fairly clean,
with just a couple minor hacks to get around the fact that it doesnt have a
"standard" nand controller.

Hopefully some mtd experts can share an opinion on this.

Thanks,
Mike

Re: [PATCH 00/13] DocG3 fixes and write support

[Marek Vasut]

> Hi guys,
> 
> On 10/30/2011 02:04 AM, Robert Jarzmik wrote:
> > Marek Vasut <marek.vasut@gmail.com> writes:
> >> Hi,
> >> 
> >> can you sum up the situation about the another (docg4) driver for us not
> >> watching this too tightly? Was there any improvement/progress/... ?
> > 
> > The review is underway, my comments should be dealt with, and a V2 of the
> > patch should be sent. It's not in my hands.
> 
> I've been busy working on this, and should post a patch within the next few
> days.  Progress has been very good.  I still want to run the mtd tests in
> the kernel before posting the patch (except the torture test - I'm not
> ready to sacrifice my development Treo), but it continues to pass the
> nandtest userspace utility (part of mtd-utils) flawlessly, and a ubifs
> still appears to be working well.  The code is now in a much cleaner
> state.

CCing Tomas Cech, we might be able to get you a spare device if really 
necessary.
> 
> > The global view we have with Mike is that chips are different, and each
> > one deserves its own driver. Several registers are common, and the
> > docg3.h could become common.
> 
> Actually, I'm open on this.  My opinion is that they should share a header
> file for sure, because the register set largely overlaps.  To that end, my
> upcoming patch will adopt Robert's register #defines, with just a few
> additional ones that are G4 specific.
> 
> On the broader question of combined or separate drivers... a combined
> driver is certainly doable.  Each device would have to use its own set of
> low-level functions, but I think there's merit in combining them because
> it would provide a consistent interface with the mtd nand infrastructure
> code, which is rather messy.  But before that discussion can take place,
> the more fundamental question of whether or not the drivers should use the
> nand interface needs to be resolved.  I used the nand interface when I
> started on the G4 driver, because it *is* a nand device, and the legacy
> diskonchip.c driver was updated not long ago from a stand-alone driver to
> the nand interface.  I'm not knowledgeable enough of the mtd subsystem to
> argue for or against the nand interface, but the end result (once I
> invested the time slogging through nand_base.c) is fairly clean, with just
> a couple minor hacks to get around the fact that it doesnt have a
> "standard" nand controller.
> 
> Hopefully some mtd experts can share an opinion on this.
> 
> Thanks,
> Mike