[PATCH 0/4] Fixes for docg3 to enable ubifs over docg3

[PATCH 0/4] Fixes for docg3 to enable ubifs over docg3

[Robert Jarzmik]

This set of patches is the feedback from ubifs usage over the docg3.
This set is split into 2 subsets:
 - patches 1 and 2 are fixes
 - patches 3 and 4 are an evolution to provided concurrent access protection

Cheers.

--
Robert

Robert Jarzmik (4):
  drivers/mtd: docg3 fix inbound calculations
  drivers/mtd: docg3 increase write/erase timeout
  drivers/mtd: docg3 refactor cascade floors structure
  drivers/mtd: docg3 add protection against concurrency

 drivers/mtd/devices/docg3.c |  169 ++++++++++++++++++++++++++-----------------
 drivers/mtd/devices/docg3.h |   20 +++++-
 2 files changed, 119 insertions(+), 70 deletions(-)

-- 
1.7.5.4

[PATCH 1/4] drivers/mtd: docg3 fix inbound calculations

[Robert Jarzmik]

The last erase block was not accessible, as the out of bound
check was incorrectly rejecting the last block.
The read/write/erase offset checks were forbidding the usage of the
last block, because of the calculation which was considering the
byte after the last instead of the last byte.

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

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index db9a4b7..09d039d 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -872,11 +872,8 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
 	if (ooblen % DOC_LAYOUT_OOB_SIZE)
 		return -EINVAL;
 
-	ret = -EINVAL;
-	calc_block_sector(from + len, &block0, &block1, &page, &ofs,
-			  docg3->reliable);
-	if (block1 > docg3->max_block)
-		goto err;
+	if (from + len > mtd->size)
+		return -EINVAL;
 
 	ops->oobretlen = 0;
 	ops->retlen = 0;
@@ -1206,7 +1203,7 @@ static int doc_erase(struct mtd_info *mtd, struct erase_info *info)
 	calc_block_sector(info->addr + info->len, &block0, &block1, &page,
 			  &ofs, docg3->reliable);
 	ret = -EINVAL;
-	if (block1 > docg3->max_block || page || ofs)
+	if (info->addr + info->len > mtd->size || page || ofs)
 		goto reset_err;
 
 	ret = 0;
@@ -1442,12 +1439,8 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
 	if (len && ooblen &&
 	    (len / DOC_LAYOUT_PAGE_SIZE) != (ooblen / oobdelta))
 		return -EINVAL;
-
-	ret = -EINVAL;
-	calc_block_sector(ofs + len, &block0, &block1, &page, &pofs,
-			  docg3->reliable);
-	if (block1 > docg3->max_block)
-		goto err;
+	if (ofs + len > mtd->size)
+		return -EINVAL;
 
 	ops->oobretlen = 0;
 	ops->retlen = 0;
-- 
1.7.5.4

[PATCH 2/4] drivers/mtd: docg3 increase write/erase timeout

[Robert Jarzmik]

After several tries with ubifs, it appears empirically that constructor
provided figures for erase/write timeouts are underestimated. A timeout
of 100ms seems to work with a 5 years worn chip, and no timeouts occur
anymore.

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

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 09d039d..f33c130 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -1115,10 +1115,10 @@ static int doc_get_op_status(struct docg3 *docg3)
  */
 static int doc_write_erase_wait_status(struct docg3 *docg3)
 {
-	int status, ret = 0;
+	int i, status, ret = 0;
 
-	if (!doc_is_ready(docg3))
-		usleep_range(3000, 3000);
+	for (i = 0; !doc_is_ready(docg3) && i < 5; i++)
+		msleep(20);
 	if (!doc_is_ready(docg3)) {
 		doc_dbg("Timeout reached and the chip is still not ready\n");
 		ret = -EAGAIN;
-- 
1.7.5.4

[PATCH 3/4] drivers/mtd: docg3 refactor cascade floors structure

[Robert Jarzmik]

Group floors into a common cascade structure. This will provide a common
structure to store common data to all cascaded docg3 chips, like IO
addressing, locking protection.

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

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index f33c130..03c9c99 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -80,14 +80,9 @@ static struct nand_ecclayout docg3_oobinfo = {
 	.oobavail = 8,
 };
 
-/**
- * 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);
+	u8 val = readb(docg3->cascade->base + reg);
 
 	trace_docg3_io(0, 8, reg, (int)val);
 	return val;
@@ -95,7 +90,7 @@ static inline u8 doc_readb(struct docg3 *docg3, u16 reg)
 
 static inline u16 doc_readw(struct docg3 *docg3, u16 reg)
 {
-	u16 val = readw(docg3->base + reg);
+	u16 val = readw(docg3->cascade->base + reg);
 
 	trace_docg3_io(0, 16, reg, (int)val);
 	return val;
@@ -103,13 +98,13 @@ 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);
+	writeb(val, docg3->cascade->base + reg);
 	trace_docg3_io(1, 8, reg, val);
 }
 
 static inline void doc_writew(struct docg3 *docg3, u16 val, u16 reg)
 {
-	writew(val, docg3->base + reg);
+	writew(val, docg3->cascade->base + reg);
 	trace_docg3_io(1, 16, reg, val);
 }
 
@@ -643,7 +638,8 @@ static int doc_ecc_bch_fix_data(struct docg3 *docg3, void *buf, u8 *hwecc)
 
 	for (i = 0; i < DOC_ECC_BCH_SIZE; i++)
 		ecc[i] = bitrev8(hwecc[i]);
-	numerrs = decode_bch(docg3_bch, NULL, DOC_ECC_BCH_COVERED_BYTES,
+	numerrs = decode_bch(docg3->cascade->bch, NULL,
+			     DOC_ECC_BCH_COVERED_BYTES,
 			     NULL, ecc, NULL, errorpos);
 	BUG_ON(numerrs == -EINVAL);
 	if (numerrs < 0)
@@ -1598,13 +1594,13 @@ static struct device_attribute doc_sys_attrs[DOC_MAX_NBFLOORS][4] = {
 };
 
 static int doc_register_sysfs(struct platform_device *pdev,
-			      struct mtd_info **floors)
+			      struct docg3_cascade *cascade)
 {
 	int ret = 0, floor, i = 0;
 	struct device *dev = &pdev->dev;
 
-	for (floor = 0; !ret && floor < DOC_MAX_NBFLOORS && floors[floor];
-	     floor++)
+	for (floor = 0; !ret && floor < DOC_MAX_NBFLOORS &&
+		     cascade->floors[floor]; floor++)
 		for (i = 0; !ret && i < 4; i++)
 			ret = device_create_file(dev, &doc_sys_attrs[floor][i]);
 	if (!ret)
@@ -1618,12 +1614,12 @@ static int doc_register_sysfs(struct platform_device *pdev,
 }
 
 static void doc_unregister_sysfs(struct platform_device *pdev,
-				 struct mtd_info **floors)
+				 struct docg3_cascade *cascade)
 {
 	struct device *dev = &pdev->dev;
 	int floor, i;
 
-	for (floor = 0; floor < DOC_MAX_NBFLOORS && floors[floor];
+	for (floor = 0; floor < DOC_MAX_NBFLOORS && cascade->floors[floor];
 	     floor++)
 		for (i = 0; i < 4; i++)
 			device_remove_file(dev, &doc_sys_attrs[floor][i]);
@@ -1832,6 +1828,7 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
  * @base: the io space where the device is probed
  * @floor: the floor of the probed device
  * @dev: the device
+ * @cascade: the cascade of chips this devices will belong to
  *
  * Checks whether a device at the specified IO range, and floor is available.
  *
@@ -1840,7 +1837,7 @@ static void __init doc_set_driver_info(int chip_id, struct mtd_info *mtd)
  * launched.
  */
 static struct mtd_info * __init
-doc_probe_device(void __iomem *base, int floor, struct device *dev)
+doc_probe_device(struct docg3_cascade *cascade, int floor, struct device *dev)
 {
 	int ret, bbt_nbpages;
 	u16 chip_id, chip_id_inv;
@@ -1863,7 +1860,7 @@ doc_probe_device(void __iomem *base, int floor, struct device *dev)
 
 	docg3->dev = dev;
 	docg3->device_id = floor;
-	docg3->base = base;
+	docg3->cascade = cascade;
 	doc_set_device_id(docg3, docg3->device_id);
 	if (!floor)
 		doc_set_asic_mode(docg3, DOC_ASICMODE_RESET);
@@ -1880,7 +1877,7 @@ doc_probe_device(void __iomem *base, int floor, struct device *dev)
 	switch (chip_id) {
 	case DOC_CHIPID_G3:
 		doc_info("Found a G3 DiskOnChip at addr %p, floor %d\n",
-			 base, floor);
+			 docg3->cascade->base, floor);
 		break;
 	default:
 		doc_err("Chip id %04x is not a DiskOnChip G3 chip\n", chip_id);
@@ -1925,10 +1922,12 @@ static void doc_release_device(struct mtd_info *mtd)
 static int docg3_resume(struct platform_device *pdev)
 {
 	int i;
+	struct docg3_cascade *cascade;
 	struct mtd_info **docg3_floors, *mtd;
 	struct docg3 *docg3;
 
-	docg3_floors = platform_get_drvdata(pdev);
+	cascade = platform_get_drvdata(pdev);
+	docg3_floors = cascade->floors;
 	mtd = docg3_floors[0];
 	docg3 = mtd->priv;
 
@@ -1950,11 +1949,13 @@ static int docg3_resume(struct platform_device *pdev)
 static int docg3_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	int floor, i;
+	struct docg3_cascade *cascade;
 	struct mtd_info **docg3_floors, *mtd;
 	struct docg3 *docg3;
 	u8 ctrl, pwr_down;
 
-	docg3_floors = platform_get_drvdata(pdev);
+	cascade = platform_get_drvdata(pdev);
+	docg3_floors = cascade->floors;
 	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++) {
 		mtd = docg3_floors[floor];
 		if (!mtd)
@@ -2004,7 +2005,7 @@ static int __init docg3_probe(struct platform_device *pdev)
 	struct resource *ress;
 	void __iomem *base;
 	int ret, floor, found = 0;
-	struct mtd_info **docg3_floors;
+	struct docg3_cascade *cascade;
 
 	ret = -ENXIO;
 	ress = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2015,17 +2016,18 @@ static int __init docg3_probe(struct platform_device *pdev)
 	base = ioremap(ress->start, DOC_IOSPACE_SIZE);
 
 	ret = -ENOMEM;
-	docg3_floors = kzalloc(sizeof(*docg3_floors) * DOC_MAX_NBFLOORS,
-			       GFP_KERNEL);
-	if (!docg3_floors)
+	cascade = kzalloc(sizeof(*cascade) * DOC_MAX_NBFLOORS,
+			  GFP_KERNEL);
+	if (!cascade)
 		goto nomem1;
-	docg3_bch = init_bch(DOC_ECC_BCH_M, DOC_ECC_BCH_T,
+	cascade->base = base;
+	cascade->bch = init_bch(DOC_ECC_BCH_M, DOC_ECC_BCH_T,
 			     DOC_ECC_BCH_PRIMPOLY);
-	if (!docg3_bch)
+	if (!cascade->bch)
 		goto nomem2;
 
 	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++) {
-		mtd = doc_probe_device(base, floor, dev);
+		mtd = doc_probe_device(cascade, floor, dev);
 		if (IS_ERR(mtd)) {
 			ret = PTR_ERR(mtd);
 			goto err_probe;
@@ -2036,7 +2038,7 @@ static int __init docg3_probe(struct platform_device *pdev)
 			else
 				continue;
 		}
-		docg3_floors[floor] = mtd;
+		cascade->floors[floor] = mtd;
 		ret = mtd_device_parse_register(mtd, part_probes, NULL, NULL,
 						0);
 		if (ret)
@@ -2044,26 +2046,26 @@ static int __init docg3_probe(struct platform_device *pdev)
 		found++;
 	}
 
-	ret = doc_register_sysfs(pdev, docg3_floors);
+	ret = doc_register_sysfs(pdev, cascade);
 	if (ret)
 		goto err_probe;
 	if (!found)
 		goto notfound;
 
-	platform_set_drvdata(pdev, docg3_floors);
-	doc_dbg_register(docg3_floors[0]->priv);
+	platform_set_drvdata(pdev, cascade);
+	doc_dbg_register(cascade->floors[0]->priv);
 	return 0;
 
 notfound:
 	ret = -ENODEV;
 	dev_info(dev, "No supported DiskOnChip found\n");
 err_probe:
-	free_bch(docg3_bch);
+	kfree(cascade->bch);
 	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++)
-		if (docg3_floors[floor])
-			doc_release_device(docg3_floors[floor]);
+		if (cascade->floors[floor])
+			doc_release_device(cascade->floors[floor]);
 nomem2:
-	kfree(docg3_floors);
+	kfree(cascade);
 nomem1:
 	iounmap(base);
 noress:
@@ -2078,19 +2080,19 @@ noress:
  */
 static int __exit docg3_release(struct platform_device *pdev)
 {
-	struct mtd_info **docg3_floors = platform_get_drvdata(pdev);
-	struct docg3 *docg3 = docg3_floors[0]->priv;
-	void __iomem *base = docg3->base;
+	struct docg3_cascade *cascade = platform_get_drvdata(pdev);
+	struct docg3 *docg3 = cascade->floors[0]->priv;
+	void __iomem *base = cascade->base;
 	int floor;
 
-	doc_unregister_sysfs(pdev, docg3_floors);
+	doc_unregister_sysfs(pdev, cascade);
 	doc_dbg_unregister(docg3);
 	for (floor = 0; floor < DOC_MAX_NBFLOORS; floor++)
-		if (docg3_floors[floor])
-			doc_release_device(docg3_floors[floor]);
+		if (cascade->floors[floor])
+			doc_release_device(cascade->floors[floor]);
 
-	kfree(docg3_floors);
-	free_bch(docg3_bch);
+	free_bch(docg3->cascade->bch);
+	kfree(cascade);
 	iounmap(base);
 	return 0;
 }
diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index db0da43..642e606 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -22,6 +22,8 @@
 #ifndef _MTD_DOCG3_H
 #define _MTD_DOCG3_H
 
+#include <linux/mtd/mtd.h>
+
 /*
  * Flash memory areas :
  *   - 0x0000 .. 0x07ff : IPL
@@ -267,9 +269,21 @@
 #define DOC_LAYOUT_DPS_KEY_LENGTH	8
 
 /**
+ * struct docg3_cascade - Cascade of 1 to 4 docg3 chips
+ * @floors: floors (ie. one physical docg3 chip is one floor)
+ * @base: IO space to access all chips in the cascade
+ * @bch: the BCH correcting control structure
+ */
+struct docg3_cascade {
+	struct mtd_info *floors[DOC_MAX_NBFLOORS];
+	void __iomem *base;
+	struct bch_control *bch;
+};
+
+/**
  * struct docg3 - DiskOnChip driver private data
  * @dev: the device currently under control
- * @base: mapped IO space
+ * @cascade: the cascade this device belongs to
  * @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
 
@@ -287,7 +301,7 @@
  */
 struct docg3 {
 	struct device *dev;
-	void __iomem *base;
+	struct docg3_cascade *cascade;
 	unsigned int device_id:4;
 	unsigned int if_cfg:1;
 	unsigned int reliable:2;
-- 
1.7.5.4

[PATCH 4/4] drivers/mtd: docg3 add protection against concurrency

[Robert Jarzmik]

As docg3 is intolerant against reentrancy, especially
because of its weird register access (ie. a register read is
performed by a first register write), each access to the
docg3 IO space must be locked.

Lock the IO space with a mutex, shared by all chips on the
same cascade, as they all share the same IO space.

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

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 03c9c99..6703b6c 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -80,6 +80,16 @@ static struct nand_ecclayout docg3_oobinfo = {
 	.oobavail = 8,
 };
 
+static void cascade_lock(struct docg3 *docg3)
+{
+	mutex_lock(&docg3->cascade->lock);
+}
+
+static void cascade_unlock(struct docg3 *docg3)
+{
+	mutex_unlock(&docg3->cascade->lock);
+}
+
 static inline u8 doc_readb(struct docg3 *docg3, u16 reg)
 {
 	u8 val = readb(docg3->cascade->base + reg);
@@ -875,6 +885,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
 	ops->retlen = 0;
 	ret = 0;
 	skip = from % DOC_LAYOUT_PAGE_SIZE;
+	cascade_lock(docg3);
 	while (!ret && (len > 0 || ooblen > 0)) {
 		calc_block_sector(from - skip, &block0, &block1, &page, &ofs,
 			docg3->reliable);
@@ -882,7 +893,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
 		nboob = min_t(size_t, ooblen, (size_t)DOC_LAYOUT_OOB_SIZE);
 		ret = doc_read_page_prepare(docg3, block0, block1, page, ofs);
 		if (ret < 0)
-			goto err;
+			goto out;
 		ret = doc_read_page_ecc_init(docg3, DOC_ECC_BCH_TOTAL_BYTES);
 		if (ret < 0)
 			goto err_in_read;
@@ -949,11 +960,12 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
 		skip = 0;
 	}
 
+out:
+	cascade_unlock(docg3);
 	return ret;
 err_in_read:
 	doc_read_page_finish(docg3);
-err:
-	return ret;
+	goto out;
 }
 
 /**
@@ -1193,7 +1205,6 @@ static int doc_erase(struct mtd_info *mtd, struct erase_info *info)
 	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,
@@ -1205,6 +1216,8 @@ static int doc_erase(struct mtd_info *mtd, struct erase_info *info)
 	ret = 0;
 	calc_block_sector(info->addr, &block0, &block1, &page, &ofs,
 			  docg3->reliable);
+	cascade_lock(docg3);
+	doc_set_device_id(docg3, docg3->device_id);
 	doc_set_reliable_mode(docg3);
 	for (len = info->len; !ret && len > 0; len -= mtd->erasesize) {
 		info->state = MTD_ERASING;
@@ -1212,6 +1225,7 @@ static int doc_erase(struct mtd_info *mtd, struct erase_info *info)
 		block0 += 2;
 		block1 += 2;
 	}
+	cascade_unlock(docg3);
 
 	if (ret)
 		goto reset_err;
@@ -1398,7 +1412,7 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
 			 struct mtd_oob_ops *ops)
 {
 	struct docg3 *docg3 = mtd->priv;
-	int block0, block1, page, ret, pofs = 0, autoecc, oobdelta;
+	int ret, autoecc, oobdelta;
 	u8 *oobbuf = ops->oobbuf;
 	u8 *buf = ops->datbuf;
 	size_t len, ooblen;
@@ -1450,6 +1464,7 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
 	if (autoecc < 0)
 		return autoecc;
 
+	cascade_lock(docg3);
 	while (!ret && len > 0) {
 		memset(oob, 0, sizeof(oob));
 		if (ofs == docg3->oob_write_ofs)
@@ -1470,8 +1485,9 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs,
 		}
 		ops->retlen += DOC_LAYOUT_PAGE_SIZE;
 	}
-err:
+
 	doc_set_device_id(docg3, 0);
+	cascade_unlock(docg3);
 	return ret;
 }
 
@@ -1528,9 +1544,11 @@ static ssize_t dps0_is_key_locked(struct device *dev,
 	struct docg3 *docg3 = sysfs_dev2docg3(dev, attr);
 	int dps0;
 
+	cascade_lock(docg3);
 	doc_set_device_id(docg3, docg3->device_id);
 	dps0 = doc_register_readb(docg3, DOC_DPS0_STATUS);
 	doc_set_device_id(docg3, 0);
+	cascade_unlock(docg3);
 
 	return sprintf(buf, "%d\n", !(dps0 & DOC_DPS_KEY_OK));
 }
@@ -1541,9 +1559,11 @@ static ssize_t dps1_is_key_locked(struct device *dev,
 	struct docg3 *docg3 = sysfs_dev2docg3(dev, attr);
 	int dps1;
 
+	cascade_lock(docg3);
 	doc_set_device_id(docg3, docg3->device_id);
 	dps1 = doc_register_readb(docg3, DOC_DPS1_STATUS);
 	doc_set_device_id(docg3, 0);
+	cascade_unlock(docg3);
 
 	return sprintf(buf, "%d\n", !(dps1 & DOC_DPS_KEY_OK));
 }
@@ -1558,10 +1578,12 @@ static ssize_t dps0_insert_key(struct device *dev,
 	if (count != DOC_LAYOUT_DPS_KEY_LENGTH)
 		return -EINVAL;
 
+	cascade_lock(docg3);
 	doc_set_device_id(docg3, docg3->device_id);
 	for (i = 0; i < DOC_LAYOUT_DPS_KEY_LENGTH; i++)
 		doc_writeb(docg3, buf[i], DOC_DPS0_KEY);
 	doc_set_device_id(docg3, 0);
+	cascade_unlock(docg3);
 	return count;
 }
 
@@ -1575,10 +1597,12 @@ static ssize_t dps1_insert_key(struct device *dev,
 	if (count != DOC_LAYOUT_DPS_KEY_LENGTH)
 		return -EINVAL;
 
+	cascade_lock(docg3);
 	doc_set_device_id(docg3, docg3->device_id);
 	for (i = 0; i < DOC_LAYOUT_DPS_KEY_LENGTH; i++)
 		doc_writeb(docg3, buf[i], DOC_DPS1_KEY);
 	doc_set_device_id(docg3, 0);
+	cascade_unlock(docg3);
 	return count;
 }
 
@@ -1633,7 +1657,11 @@ static int dbg_flashctrl_show(struct seq_file *s, void *p)
 	struct docg3 *docg3 = (struct docg3 *)s->private;
 
 	int pos = 0;
-	u8 fctrl = doc_register_readb(docg3, DOC_FLASHCONTROL);
+	u8 fctrl;
+
+	cascade_lock(docg3);
+	fctrl = doc_register_readb(docg3, DOC_FLASHCONTROL);
+	cascade_unlock(docg3);
 
 	pos += seq_printf(s,
 		 "FlashControl : 0x%02x (%s,CE# %s,%s,%s,flash %s)\n",
@@ -1651,9 +1679,12 @@ static int dbg_asicmode_show(struct seq_file *s, void *p)
 {
 	struct docg3 *docg3 = (struct docg3 *)s->private;
 
-	int pos = 0;
-	int pctrl = doc_register_readb(docg3, DOC_ASICMODE);
-	int mode = pctrl & 0x03;
+	int pos = 0, pctrl, mode;
+
+	cascade_lock(docg3);
+	pctrl = doc_register_readb(docg3, DOC_ASICMODE);
+	mode = pctrl & 0x03;
+	cascade_unlock(docg3);
 
 	pos += seq_printf(s,
 			 "%04x : RAM_WE=%d,RSTIN_RESET=%d,BDETCT_RESET=%d,WRITE_ENABLE=%d,POWERDOWN=%d,MODE=%d%d (",
@@ -1685,7 +1716,11 @@ static int dbg_device_id_show(struct seq_file *s, void *p)
 {
 	struct docg3 *docg3 = (struct docg3 *)s->private;
 	int pos = 0;
-	int id = doc_register_readb(docg3, DOC_DEVICESELECT);
+	int id;
+
+	cascade_lock(docg3);
+	id = doc_register_readb(docg3, DOC_DEVICESELECT);
+	cascade_unlock(docg3);
 
 	pos += seq_printf(s, "DeviceId = %d\n", id);
 	return pos;
@@ -1698,6 +1733,7 @@ static int dbg_protection_show(struct seq_file *s, void *p)
 	int pos = 0;
 	int protect, dps0, dps0_low, dps0_high, dps1, dps1_low, dps1_high;
 
+	cascade_lock(docg3);
 	protect = doc_register_readb(docg3, DOC_PROTECTION);
 	dps0 = doc_register_readb(docg3, DOC_DPS0_STATUS);
 	dps0_low = doc_register_readw(docg3, DOC_DPS0_ADDRLOW);
@@ -1705,6 +1741,7 @@ static int dbg_protection_show(struct seq_file *s, void *p)
 	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);
+	cascade_unlock(docg3);
 
 	pos += seq_printf(s, "Protection = 0x%02x (",
 			 protect);
@@ -2021,6 +2058,7 @@ static int __init docg3_probe(struct platform_device *pdev)
 	if (!cascade)
 		goto nomem1;
 	cascade->base = base;
+	mutex_init(&cascade->lock);
 	cascade->bch = init_bch(DOC_ECC_BCH_M, DOC_ECC_BCH_T,
 			     DOC_ECC_BCH_PRIMPOLY);
 	if (!cascade->bch)
diff --git a/drivers/mtd/devices/docg3.h b/drivers/mtd/devices/docg3.h
index 642e606..19fb93f 100644
--- a/drivers/mtd/devices/docg3.h
+++ b/drivers/mtd/devices/docg3.h
@@ -273,11 +273,13 @@
  * @floors: floors (ie. one physical docg3 chip is one floor)
  * @base: IO space to access all chips in the cascade
  * @bch: the BCH correcting control structure
+ * @lock: lock to protect docg3 IO space from concurrent accesses
  */
 struct docg3_cascade {
 	struct mtd_info *floors[DOC_MAX_NBFLOORS];
 	void __iomem *base;
 	struct bch_control *bch;
+	struct mutex lock;
 };
 
 /**
-- 
1.7.5.4

Re: [PATCH 4/4] drivers/mtd: docg3 add protection against concurrency

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

On Mon, 2012-03-19 at 23:32 +0100, Robert Jarzmik wrote:
> +static void cascade_lock(struct docg3 *docg3)
> +{
> +	mutex_lock(&docg3->cascade->lock);
> +}
> +
> +static void cascade_unlock(struct docg3 *docg3)
> +{
> +	mutex_unlock(&docg3->cascade->lock);
> +}

Please, do not introduce that trivial wrappers - they are useless and
only hurt readability. Open-coded mutex_lock()/mutex_unlock() is more
readable.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] drivers/mtd: docg3 add protection against concurrency

[Robert Jarzmik]

Artem Bityutskiy <dedekind1@gmail.com> writes:

> On Mon, 2012-03-19 at 23:32 +0100, Robert Jarzmik wrote:
>> +static void cascade_lock(struct docg3 *docg3)
>> +{
>> +	mutex_lock(&docg3->cascade->lock);
>> +}
>> +
>> +static void cascade_unlock(struct docg3 *docg3)
>> +{
>> +	mutex_unlock(&docg3->cascade->lock);
>> +}
>
> Please, do not introduce that trivial wrappers - they are useless and
> only hurt readability. Open-coded mutex_lock()/mutex_unlock() is more
> readable.
OK for V2.

I'll await comments for 3 or 4 days before releasing V2.

Cheers.

-- 
Robert

Re: [PATCH 4/4] drivers/mtd: docg3 add protection against concurrency

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

On Wed, 2012-03-21 at 21:24 +0100, Robert Jarzmik wrote:
> I'll await comments for 3 or 4 days before releasing V2.

On the surface they look ok to me. If you are quick you _may_ be able to
have this merged to 3.4, but send v2 patches today then.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] drivers/mtd: docg3 add protection against concurrency

[Robert Jarzmik]

Artem Bityutskiy <dedekind1@gmail.com> writes:

> On Wed, 2012-03-21 at 21:24 +0100, Robert Jarzmik wrote:
>> I'll await comments for 3 or 4 days before releasing V2.
>
> On the surface they look ok to me. If you are quick you _may_ be able to
> have this merged to 3.4, but send v2 patches today then.
Ok, understood, and pushed out.

Cheers.

-- 
Robert