netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] mtd: Simplify erase handling
@ 2018-02-12 21:03 Boris Brezillon
  2018-02-12 21:03 ` [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase() Boris Brezillon
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Boris Brezillon @ 2018-02-12 21:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Artem Bityutskiy, Miquel Raynal, Michael Ellerman, linuxppc-dev,
	Joern Engel, Greg Kroah-Hartman, Kyungmin Park, netdev,
	Paul Mackerras, Benjamin Herrenschmidt, Edward Cree,
	Robert Jarzmik

Hello,

This series aims at simplifying erase handling both in MTD drivers and
MTD users code.

Historically, the erase operation has been designed to be asynchronous,
which, in theory, is a good thing since erasing a block usually takes
longer that reading/writing to a flash. In practice, all drivers are
implementing ->_erase() in a synchronous manner. Moreover, both drivers
and users are inconsistently updating/checking the erase_info fields.

In order to simplify things, let's assume ->_erase() is and will always
be synchronous. This also make error code checking more consistent and
allows us to get rid of a few hundred lines of code.

Regards,

Boris

Boris Brezillon (5):
  mtd: Initialize ->fail_addr early in mtd_erase()
  mtd: Get rid of unused fields in struct erase_info
  mtd: Stop assuming mtd_erase() is asynchronous
  mtd: Unconditionally update ->fail_addr and ->addr in part_erase()
  mtd: Stop updating erase_info->state and calling mtd_erase_callback()

 drivers/mtd/chips/cfi_cmdset_0001.c      | 16 +-----
 drivers/mtd/chips/cfi_cmdset_0002.c      | 26 ++-------
 drivers/mtd/chips/cfi_cmdset_0020.c      |  3 --
 drivers/mtd/chips/map_ram.c              |  2 -
 drivers/mtd/devices/bcm47xxsflash.c      | 12 +----
 drivers/mtd/devices/block2mtd.c          |  7 +--
 drivers/mtd/devices/docg3.c              | 16 +-----
 drivers/mtd/devices/lart.c               |  4 --
 drivers/mtd/devices/mtd_dataflash.c      |  4 --
 drivers/mtd/devices/mtdram.c             |  2 -
 drivers/mtd/devices/phram.c              |  2 -
 drivers/mtd/devices/pmc551.c             |  2 -
 drivers/mtd/devices/powernv_flash.c      | 11 +---
 drivers/mtd/devices/slram.c              |  2 -
 drivers/mtd/devices/spear_smi.c          |  3 --
 drivers/mtd/devices/sst25l.c             |  3 --
 drivers/mtd/devices/st_spi_fsm.c         |  4 --
 drivers/mtd/ftl.c                        | 52 +++---------------
 drivers/mtd/inftlmount.c                 |  8 ++-
 drivers/mtd/lpddr/lpddr2_nvm.c           | 10 +---
 drivers/mtd/lpddr/lpddr_cmds.c           |  2 -
 drivers/mtd/mtdblock.c                   | 21 --------
 drivers/mtd/mtdchar.c                    | 34 +-----------
 drivers/mtd/mtdconcat.c                  | 48 +----------------
 drivers/mtd/mtdcore.c                    | 17 +++---
 drivers/mtd/mtdoops.c                    | 20 -------
 drivers/mtd/mtdpart.c                    | 23 ++------
 drivers/mtd/mtdswap.c                    | 34 ------------
 drivers/mtd/nand/nand_base.c             | 16 ++----
 drivers/mtd/nand/nand_bbt.c              |  1 -
 drivers/mtd/nftlmount.c                  |  5 +-
 drivers/mtd/onenand/onenand_base.c       | 17 ------
 drivers/mtd/rfd_ftl.c                    | 93 ++++++++++----------------------
 drivers/mtd/sm_ftl.c                     | 19 -------
 drivers/mtd/sm_ftl.h                     |  4 --
 drivers/mtd/spi-nor/spi-nor.c            |  3 --
 drivers/mtd/tests/mtd_test.c             |  5 --
 drivers/mtd/tests/speedtest.c            |  7 ---
 drivers/mtd/ubi/gluebi.c                 |  3 --
 drivers/mtd/ubi/io.c                     | 36 -------------
 drivers/net/ethernet/sfc/falcon/mtd.c    | 11 +---
 drivers/net/ethernet/sfc/mtd.c           | 11 +---
 drivers/staging/goldfish/goldfish_nand.c |  3 --
 fs/jffs2/erase.c                         | 37 ++-----------
 include/linux/mtd/mtd.h                  | 19 +------
 45 files changed, 79 insertions(+), 599 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()
  2018-02-12 21:03 [PATCH 0/5] mtd: Simplify erase handling Boris Brezillon
@ 2018-02-12 21:03 ` Boris Brezillon
  2018-02-12 21:47   ` Richard Weinberger
  2018-03-18 21:22   ` Boris Brezillon
  2018-02-12 21:03 ` [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info Boris Brezillon
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Boris Brezillon @ 2018-02-12 21:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Artem Bityutskiy, Miquel Raynal, Michael Ellerman, linuxppc-dev,
	Joern Engel, Greg Kroah-Hartman, Kyungmin Park, netdev,
	Paul Mackerras, Benjamin Herrenschmidt, Edward Cree,
	Robert Jarzmik

mtd_erase() can return an error before ->fail_addr is initialized to
MTD_FAIL_ADDR_UNKNOWN. Move this initialization at the very beginning
of the function.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/mtdcore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index a1c94526fb88..c87859ff338b 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -953,6 +953,8 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
  */
 int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
+	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
+
 	if (!mtd->erasesize || !mtd->_erase)
 		return -ENOTSUPP;
 
@@ -961,7 +963,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
 
-	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 	if (!instr->len) {
 		instr->state = MTD_ERASE_DONE;
 		mtd_erase_callback(instr);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info
  2018-02-12 21:03 [PATCH 0/5] mtd: Simplify erase handling Boris Brezillon
  2018-02-12 21:03 ` [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase() Boris Brezillon
@ 2018-02-12 21:03 ` Boris Brezillon
  2018-02-12 21:47   ` Richard Weinberger
  2018-02-12 21:03 ` [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous Boris Brezillon
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Boris Brezillon @ 2018-02-12 21:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Artem Bityutskiy, Miquel Raynal, Michael Ellerman, linuxppc-dev,
	Joern Engel, Greg Kroah-Hartman, Kyungmin Park, netdev,
	Paul Mackerras, Benjamin Herrenschmidt, Edward Cree,
	Robert Jarzmik

Some fields are not used by MTD drivers, users or core code. Moreover,
those fields are not documented, so get rid of them to avoid any
confusion.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 include/linux/mtd/mtd.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 205ededccc60..2a407dc9beaa 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -48,14 +48,9 @@ struct erase_info {
 	uint64_t addr;
 	uint64_t len;
 	uint64_t fail_addr;
-	u_long time;
-	u_long retries;
-	unsigned dev;
-	unsigned cell;
 	void (*callback) (struct erase_info *self);
 	u_long priv;
 	u_char state;
-	struct erase_info *next;
 };
 
 struct mtd_erase_region_info {
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous
  2018-02-12 21:03 [PATCH 0/5] mtd: Simplify erase handling Boris Brezillon
  2018-02-12 21:03 ` [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase() Boris Brezillon
  2018-02-12 21:03 ` [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info Boris Brezillon
@ 2018-02-12 21:03 ` Boris Brezillon
  2018-02-12 21:58   ` Richard Weinberger
  2018-02-12 21:03 ` [PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase() Boris Brezillon
  2018-02-12 21:03 ` [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback() Boris Brezillon
  4 siblings, 1 reply; 16+ messages in thread
From: Boris Brezillon @ 2018-02-12 21:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Artem Bityutskiy, Miquel Raynal, Michael Ellerman, linuxppc-dev,
	Joern Engel, Greg Kroah-Hartman, Kyungmin Park, netdev,
	Paul Mackerras, Benjamin Herrenschmidt, Edward Cree,
	Robert Jarzmik

None of the mtd->_erase() implementations work in an asynchronous manner,
so let's simplify MTD users that call mtd_erase(). All they need to do
is check the value returned by mtd_erase() and assume that != 0 means
failure.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/devices/bcm47xxsflash.c |  3 --
 drivers/mtd/ftl.c                   | 51 ++++----------------
 drivers/mtd/inftlmount.c            |  5 +-
 drivers/mtd/mtdblock.c              | 20 --------
 drivers/mtd/mtdchar.c               | 33 +------------
 drivers/mtd/mtdconcat.c             | 48 ++-----------------
 drivers/mtd/mtdcore.c               |  8 ++--
 drivers/mtd/mtdoops.c               | 19 --------
 drivers/mtd/mtdpart.c               |  2 -
 drivers/mtd/mtdswap.c               | 32 -------------
 drivers/mtd/nftlmount.c             |  4 +-
 drivers/mtd/rfd_ftl.c               | 92 +++++++++++--------------------------
 drivers/mtd/sm_ftl.c                | 18 --------
 drivers/mtd/sm_ftl.h                |  4 --
 drivers/mtd/tests/mtd_test.c        |  4 --
 drivers/mtd/tests/speedtest.c       |  6 ---
 drivers/mtd/ubi/io.c                | 35 --------------
 fs/jffs2/erase.c                    | 36 ++-------------
 include/linux/mtd/mtd.h             |  2 -
 19 files changed, 52 insertions(+), 370 deletions(-)

diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
index e2bd81817df4..6b84947cfbea 100644
--- a/drivers/mtd/devices/bcm47xxsflash.c
+++ b/drivers/mtd/devices/bcm47xxsflash.c
@@ -95,9 +95,6 @@ static int bcm47xxsflash_erase(struct mtd_info *mtd, struct erase_info *erase)
 	else
 		erase->state = MTD_ERASE_DONE;
 
-	if (erase->callback)
-		erase->callback(erase);
-
 	return err;
 }
 
diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index 664d206a4cbe..fcf9907e7987 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -140,12 +140,6 @@ typedef struct partition_t {
 #define XFER_PREPARED	0x03
 #define XFER_FAILED	0x04
 
-/*====================================================================*/
-
-
-static void ftl_erase_callback(struct erase_info *done);
-
-
 /*======================================================================
 
     Scan_header() checks to see if a memory region contains an FTL
@@ -349,17 +343,19 @@ static int erase_xfer(partition_t *part,
             return -ENOMEM;
 
     erase->mtd = part->mbd.mtd;
-    erase->callback = ftl_erase_callback;
     erase->addr = xfer->Offset;
     erase->len = 1 << part->header.EraseUnitSize;
-    erase->priv = (u_long)part;
 
     ret = mtd_erase(part->mbd.mtd, erase);
+    if (!ret) {
+	xfer->state = XFER_ERASED;
+	xfer->EraseCount++;
+    } else {
+	xfer->state = XFER_FAILED;
+	pr_notice("ftl_cs: erase failed: err = %d\n", ret);
+    }
 
-    if (!ret)
-	    xfer->EraseCount++;
-    else
-	    kfree(erase);
+    kfree(erase);
 
     return ret;
 } /* erase_xfer */
@@ -371,37 +367,6 @@ static int erase_xfer(partition_t *part,
 
 ======================================================================*/
 
-static void ftl_erase_callback(struct erase_info *erase)
-{
-    partition_t *part;
-    struct xfer_info_t *xfer;
-    int i;
-
-    /* Look up the transfer unit */
-    part = (partition_t *)(erase->priv);
-
-    for (i = 0; i < part->header.NumTransferUnits; i++)
-	if (part->XferInfo[i].Offset == erase->addr) break;
-
-    if (i == part->header.NumTransferUnits) {
-	printk(KERN_NOTICE "ftl_cs: internal error: "
-	       "erase lookup failed!\n");
-	return;
-    }
-
-    xfer = &part->XferInfo[i];
-    if (erase->state == MTD_ERASE_DONE)
-	xfer->state = XFER_ERASED;
-    else {
-	xfer->state = XFER_FAILED;
-	printk(KERN_NOTICE "ftl_cs: erase failed: state = %d\n",
-	       erase->state);
-    }
-
-    kfree(erase);
-
-} /* ftl_erase_callback */
-
 static int prepare_xfer(partition_t *part, int i)
 {
     erase_unit_header_t header;
diff --git a/drivers/mtd/inftlmount.c b/drivers/mtd/inftlmount.c
index 8d6bb189ea8e..0f47be4834d8 100644
--- a/drivers/mtd/inftlmount.c
+++ b/drivers/mtd/inftlmount.c
@@ -393,9 +393,10 @@ int INFTL_formatblock(struct INFTLrecord *inftl, int block)
 	   mark only the failed block in the bbt. */
 	for (physblock = 0; physblock < inftl->EraseSize;
 	     physblock += instr->len, instr->addr += instr->len) {
-		mtd_erase(inftl->mbd.mtd, instr);
+		int ret;
 
-		if (instr->state == MTD_ERASE_FAILED) {
+		ret = mtd_erase(inftl->mbd.mtd, instr);
+		if (ret) {
 			printk(KERN_WARNING "INFTL: error while formatting block %d\n",
 				block);
 			goto fail;
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index bb4c14f83c75..7b2b7f651181 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -55,48 +55,28 @@ struct mtdblk_dev {
  * being written to until a different sector is required.
  */
 
-static void erase_callback(struct erase_info *done)
-{
-	wait_queue_head_t *wait_q = (wait_queue_head_t *)done->priv;
-	wake_up(wait_q);
-}
-
 static int erase_write (struct mtd_info *mtd, unsigned long pos,
 			int len, const char *buf)
 {
 	struct erase_info erase;
-	DECLARE_WAITQUEUE(wait, current);
-	wait_queue_head_t wait_q;
 	size_t retlen;
 	int ret;
 
 	/*
 	 * First, let's erase the flash block.
 	 */
-
-	init_waitqueue_head(&wait_q);
 	erase.mtd = mtd;
-	erase.callback = erase_callback;
 	erase.addr = pos;
 	erase.len = len;
-	erase.priv = (u_long)&wait_q;
-
-	set_current_state(TASK_INTERRUPTIBLE);
-	add_wait_queue(&wait_q, &wait);
 
 	ret = mtd_erase(mtd, &erase);
 	if (ret) {
-		set_current_state(TASK_RUNNING);
-		remove_wait_queue(&wait_q, &wait);
 		printk (KERN_WARNING "mtdblock: erase of region [0x%lx, 0x%x] "
 				     "on \"%s\" failed\n",
 			pos, len, mtd->name);
 		return ret;
 	}
 
-	schedule();  /* Wait for erase to finish. */
-	remove_wait_queue(&wait_q, &wait);
-
 	/*
 	 * Next, write the data to flash.
 	 */
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index de8c902059b8..2beb22dd6bbb 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -324,10 +324,6 @@ static ssize_t mtdchar_write(struct file *file, const char __user *buf, size_t c
     IOCTL calls for getting device parameters.
 
 ======================================================================*/
-static void mtdchar_erase_callback (struct erase_info *instr)
-{
-	wake_up((wait_queue_head_t *)instr->priv);
-}
 
 static int otp_select_filemode(struct mtd_file_info *mfi, int mode)
 {
@@ -709,11 +705,6 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 		if (!erase)
 			ret = -ENOMEM;
 		else {
-			wait_queue_head_t waitq;
-			DECLARE_WAITQUEUE(wait, current);
-
-			init_waitqueue_head(&waitq);
-
 			if (cmd == MEMERASE64) {
 				struct erase_info_user64 einfo64;
 
@@ -736,30 +727,8 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 				erase->len = einfo32.length;
 			}
 			erase->mtd = mtd;
-			erase->callback = mtdchar_erase_callback;
-			erase->priv = (unsigned long)&waitq;
-
-			/*
-			  FIXME: Allow INTERRUPTIBLE. Which means
-			  not having the wait_queue head on the stack.
-
-			  If the wq_head is on the stack, and we
-			  leave because we got interrupted, then the
-			  wq_head is no longer there when the
-			  callback routine tries to wake us up.
-			*/
+
 			ret = mtd_erase(mtd, erase);
-			if (!ret) {
-				set_current_state(TASK_UNINTERRUPTIBLE);
-				add_wait_queue(&waitq, &wait);
-				if (erase->state != MTD_ERASE_DONE &&
-				    erase->state != MTD_ERASE_FAILED)
-					schedule();
-				remove_wait_queue(&waitq, &wait);
-				set_current_state(TASK_RUNNING);
-
-				ret = (erase->state == MTD_ERASE_FAILED)?-EIO:0;
-			}
 			kfree(erase);
 		}
 		break;
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index 60bf53df5454..caa09bf6e572 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -333,45 +333,6 @@ concat_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops *ops)
 	return -EINVAL;
 }
 
-static void concat_erase_callback(struct erase_info *instr)
-{
-	wake_up((wait_queue_head_t *) instr->priv);
-}
-
-static int concat_dev_erase(struct mtd_info *mtd, struct erase_info *erase)
-{
-	int err;
-	wait_queue_head_t waitq;
-	DECLARE_WAITQUEUE(wait, current);
-
-	/*
-	 * This code was stol^H^H^H^Hinspired by mtdchar.c
-	 */
-	init_waitqueue_head(&waitq);
-
-	erase->mtd = mtd;
-	erase->callback = concat_erase_callback;
-	erase->priv = (unsigned long) &waitq;
-
-	/*
-	 * FIXME: Allow INTERRUPTIBLE. Which means
-	 * not having the wait_queue head on the stack.
-	 */
-	err = mtd_erase(mtd, erase);
-	if (!err) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		add_wait_queue(&waitq, &wait);
-		if (erase->state != MTD_ERASE_DONE
-		    && erase->state != MTD_ERASE_FAILED)
-			schedule();
-		remove_wait_queue(&waitq, &wait);
-		set_current_state(TASK_RUNNING);
-
-		err = (erase->state == MTD_ERASE_FAILED) ? -EIO : 0;
-	}
-	return err;
-}
-
 static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
 	struct mtd_concat *concat = CONCAT(mtd);
@@ -466,7 +427,8 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
 			erase->len = length;
 
 		length -= erase->len;
-		if ((err = concat_dev_erase(subdev, erase))) {
+		erase->mtd = subdev;
+		if ((err = mtd_erase(subdev, erase))) {
 			/* sanity check: should never happen since
 			 * block alignment has been checked above */
 			BUG_ON(err == -EINVAL);
@@ -487,12 +449,8 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
 	}
 	instr->state = erase->state;
 	kfree(erase);
-	if (err)
-		return err;
 
-	if (instr->callback)
-		instr->callback(instr);
-	return 0;
+	return err;
 }
 
 static int concat_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c87859ff338b..f92ad02959eb 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -945,11 +945,9 @@ void __put_mtd_device(struct mtd_info *mtd)
 EXPORT_SYMBOL_GPL(__put_mtd_device);
 
 /*
- * Erase is an asynchronous operation.  Device drivers are supposed
- * to call instr->callback() whenever the operation completes, even
- * if it completes with a failure.
- * Callers are supposed to pass a callback function and wait for it
- * to be called before writing to the block.
+ * Erase is an synchronous operation. Device drivers are epected to return a
+ * negative error code if the operation failed and update instr->fail_addr
+ * to point the portion that was not properly erased.
  */
 int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 97bb8f6304d4..028ded59297b 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -84,12 +84,6 @@ static int page_is_used(struct mtdoops_context *cxt, int page)
 	return test_bit(page, cxt->oops_page_used);
 }
 
-static void mtdoops_erase_callback(struct erase_info *done)
-{
-	wait_queue_head_t *wait_q = (wait_queue_head_t *)done->priv;
-	wake_up(wait_q);
-}
-
 static int mtdoops_erase_block(struct mtdoops_context *cxt, int offset)
 {
 	struct mtd_info *mtd = cxt->mtd;
@@ -97,34 +91,21 @@ static int mtdoops_erase_block(struct mtdoops_context *cxt, int offset)
 	u32 start_page = start_page_offset / record_size;
 	u32 erase_pages = mtd->erasesize / record_size;
 	struct erase_info erase;
-	DECLARE_WAITQUEUE(wait, current);
-	wait_queue_head_t wait_q;
 	int ret;
 	int page;
 
-	init_waitqueue_head(&wait_q);
 	erase.mtd = mtd;
-	erase.callback = mtdoops_erase_callback;
 	erase.addr = offset;
 	erase.len = mtd->erasesize;
-	erase.priv = (u_long)&wait_q;
-
-	set_current_state(TASK_INTERRUPTIBLE);
-	add_wait_queue(&wait_q, &wait);
 
 	ret = mtd_erase(mtd, &erase);
 	if (ret) {
-		set_current_state(TASK_RUNNING);
-		remove_wait_queue(&wait_q, &wait);
 		printk(KERN_WARNING "mtdoops: erase of region [0x%llx, 0x%llx] on \"%s\" failed\n",
 		       (unsigned long long)erase.addr,
 		       (unsigned long long)erase.len, mtddev);
 		return ret;
 	}
 
-	schedule();  /* Wait for erase to finish. */
-	remove_wait_queue(&wait_q, &wait);
-
 	/* Mark pages as unused */
 	for (page = start_page; page < start_page + erase_pages; page++)
 		mark_page_unused(cxt, page);
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 76cd21d1171b..ae1206633d9d 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -222,8 +222,6 @@ void mtd_erase_callback(struct erase_info *instr)
 			instr->fail_addr -= part->offset;
 		instr->addr -= part->offset;
 	}
-	if (instr->callback)
-		instr->callback(instr);
 }
 EXPORT_SYMBOL_GPL(mtd_erase_callback);
 
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index 7eb0e1f4f980..d390324d102e 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -536,18 +536,10 @@ static void mtdswap_store_eb(struct mtdswap_dev *d, struct swap_eb *eb)
 		mtdswap_rb_add(d, eb, MTDSWAP_HIFRAG);
 }
 
-
-static void mtdswap_erase_callback(struct erase_info *done)
-{
-	wait_queue_head_t *wait_q = (wait_queue_head_t *)done->priv;
-	wake_up(wait_q);
-}
-
 static int mtdswap_erase_block(struct mtdswap_dev *d, struct swap_eb *eb)
 {
 	struct mtd_info *mtd = d->mtd;
 	struct erase_info erase;
-	wait_queue_head_t wq;
 	unsigned int retries = 0;
 	int ret;
 
@@ -556,14 +548,11 @@ static int mtdswap_erase_block(struct mtdswap_dev *d, struct swap_eb *eb)
 		d->max_erase_count = eb->erase_count;
 
 retry:
-	init_waitqueue_head(&wq);
 	memset(&erase, 0, sizeof(struct erase_info));
 
 	erase.mtd	= mtd;
-	erase.callback	= mtdswap_erase_callback;
 	erase.addr	= mtdswap_eb_offset(d, eb);
 	erase.len	= mtd->erasesize;
-	erase.priv	= (u_long)&wq;
 
 	ret = mtd_erase(mtd, &erase);
 	if (ret) {
@@ -582,27 +571,6 @@ static int mtdswap_erase_block(struct mtdswap_dev *d, struct swap_eb *eb)
 		return -EIO;
 	}
 
-	ret = wait_event_interruptible(wq, erase.state == MTD_ERASE_DONE ||
-					   erase.state == MTD_ERASE_FAILED);
-	if (ret) {
-		dev_err(d->dev, "Interrupted erase block %#llx erasure on %s\n",
-			erase.addr, mtd->name);
-		return -EINTR;
-	}
-
-	if (erase.state == MTD_ERASE_FAILED) {
-		if (retries++ < MTDSWAP_ERASE_RETRIES) {
-			dev_warn(d->dev,
-				"erase of erase block %#llx on %s failed",
-				erase.addr, mtd->name);
-			yield();
-			goto retry;
-		}
-
-		mtdswap_handle_badblock(d, eb);
-		return -EIO;
-	}
-
 	return 0;
 }
 
diff --git a/drivers/mtd/nftlmount.c b/drivers/mtd/nftlmount.c
index 184c8fbfe465..07e122449759 100644
--- a/drivers/mtd/nftlmount.c
+++ b/drivers/mtd/nftlmount.c
@@ -331,9 +331,7 @@ int NFTL_formatblock(struct NFTLrecord *nftl, int block)
 	instr->mtd = nftl->mbd.mtd;
 	instr->addr = block * nftl->EraseSize;
 	instr->len = nftl->EraseSize;
-	mtd_erase(mtd, instr);
-
-	if (instr->state == MTD_ERASE_FAILED) {
+	if (mtd_erase(mtd, instr)) {
 		printk("Error while formatting block %d\n", block);
 		goto fail;
 	}
diff --git a/drivers/mtd/rfd_ftl.c b/drivers/mtd/rfd_ftl.c
index d1cbf26db2c0..4e0b55cd08e2 100644
--- a/drivers/mtd/rfd_ftl.c
+++ b/drivers/mtd/rfd_ftl.c
@@ -266,91 +266,55 @@ static int rfd_ftl_readsect(struct mtd_blktrans_dev *dev, u_long sector, char *b
 	return 0;
 }
 
-static void erase_callback(struct erase_info *erase)
-{
-	struct partition *part;
-	u16 magic;
-	int i, rc;
-	size_t retlen;
-
-	part = (struct partition*)erase->priv;
-
-	i = (u32)erase->addr / part->block_size;
-	if (i >= part->total_blocks || part->blocks[i].offset != erase->addr ||
-	    erase->addr > UINT_MAX) {
-		printk(KERN_ERR PREFIX "erase callback for unknown offset %llx "
-				"on '%s'\n", (unsigned long long)erase->addr, part->mbd.mtd->name);
-		return;
-	}
-
-	if (erase->state != MTD_ERASE_DONE) {
-		printk(KERN_WARNING PREFIX "erase failed at 0x%llx on '%s', "
-				"state %d\n", (unsigned long long)erase->addr,
-				part->mbd.mtd->name, erase->state);
-
-		part->blocks[i].state = BLOCK_FAILED;
-		part->blocks[i].free_sectors = 0;
-		part->blocks[i].used_sectors = 0;
-
-		kfree(erase);
-
-		return;
-	}
-
-	magic = cpu_to_le16(RFD_MAGIC);
-
-	part->blocks[i].state = BLOCK_ERASED;
-	part->blocks[i].free_sectors = part->data_sectors_per_block;
-	part->blocks[i].used_sectors = 0;
-	part->blocks[i].erases++;
-
-	rc = mtd_write(part->mbd.mtd, part->blocks[i].offset, sizeof(magic),
-		       &retlen, (u_char *)&magic);
-
-	if (!rc && retlen != sizeof(magic))
-		rc = -EIO;
-
-	if (rc) {
-		printk(KERN_ERR PREFIX "'%s': unable to write RFD "
-				"header at 0x%lx\n",
-				part->mbd.mtd->name,
-				part->blocks[i].offset);
-		part->blocks[i].state = BLOCK_FAILED;
-	}
-	else
-		part->blocks[i].state = BLOCK_OK;
-
-	kfree(erase);
-}
-
 static int erase_block(struct partition *part, int block)
 {
 	struct erase_info *erase;
-	int rc = -ENOMEM;
+	int rc;
 
 	erase = kmalloc(sizeof(struct erase_info), GFP_KERNEL);
 	if (!erase)
-		goto err;
+		return -ENOMEM;
 
 	erase->mtd = part->mbd.mtd;
-	erase->callback = erase_callback;
 	erase->addr = part->blocks[block].offset;
 	erase->len = part->block_size;
-	erase->priv = (u_long)part;
 
 	part->blocks[block].state = BLOCK_ERASING;
 	part->blocks[block].free_sectors = 0;
 
 	rc = mtd_erase(part->mbd.mtd, erase);
-
 	if (rc) {
 		printk(KERN_ERR PREFIX "erase of region %llx,%llx on '%s' "
 				"failed\n", (unsigned long long)erase->addr,
 				(unsigned long long)erase->len, part->mbd.mtd->name);
-		kfree(erase);
+		part->blocks[block].state = BLOCK_FAILED;
+		part->blocks[block].free_sectors = 0;
+		part->blocks[block].used_sectors = 0;
+	} else {
+		u16 magic = cpu_to_le16(RFD_MAGIC);
+		size_t retlen;
+
+		part->blocks[block].state = BLOCK_ERASED;
+		part->blocks[block].free_sectors = part->data_sectors_per_block;
+		part->blocks[block].used_sectors = 0;
+		part->blocks[block].erases++;
+
+		rc = mtd_write(part->mbd.mtd, part->blocks[block].offset,
+			       sizeof(magic), &retlen, (u_char *)&magic);
+		if (!rc && retlen != sizeof(magic))
+			rc = -EIO;
+
+		if (rc) {
+			pr_err(PREFIX "'%s': unable to write RFD header at 0x%lx\n",
+			       part->mbd.mtd->name, part->blocks[block].offset);
+			part->blocks[block].state = BLOCK_FAILED;
+		} else {
+			part->blocks[block].state = BLOCK_OK;
+		}
 	}
 
-err:
+	kfree(erase);
+
 	return rc;
 }
 
diff --git a/drivers/mtd/sm_ftl.c b/drivers/mtd/sm_ftl.c
index 4237c7cebf02..c11156f9d96f 100644
--- a/drivers/mtd/sm_ftl.c
+++ b/drivers/mtd/sm_ftl.c
@@ -461,10 +461,8 @@ static int sm_erase_block(struct sm_ftl *ftl, int zone_num, uint16_t block,
 	struct erase_info erase;
 
 	erase.mtd = mtd;
-	erase.callback = sm_erase_callback;
 	erase.addr = sm_mkoffset(ftl, zone_num, block, 0);
 	erase.len = ftl->block_size;
-	erase.priv = (u_long)ftl;
 
 	if (ftl->unstable)
 		return -EIO;
@@ -482,15 +480,6 @@ static int sm_erase_block(struct sm_ftl *ftl, int zone_num, uint16_t block,
 		goto error;
 	}
 
-	if (erase.state == MTD_ERASE_PENDING)
-		wait_for_completion(&ftl->erase_completion);
-
-	if (erase.state != MTD_ERASE_DONE) {
-		sm_printk("erase of block %d in zone %d failed after wait",
-			block, zone_num);
-		goto error;
-	}
-
 	if (put_free)
 		kfifo_in(&zone->free_sectors,
 			(const unsigned char *)&block, sizeof(block));
@@ -501,12 +490,6 @@ static int sm_erase_block(struct sm_ftl *ftl, int zone_num, uint16_t block,
 	return -EIO;
 }
 
-static void sm_erase_callback(struct erase_info *self)
-{
-	struct sm_ftl *ftl = (struct sm_ftl *)self->priv;
-	complete(&ftl->erase_completion);
-}
-
 /* Thoroughly test that block is valid. */
 static int sm_check_block(struct sm_ftl *ftl, int zone, int block)
 {
@@ -1141,7 +1124,6 @@ static void sm_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
 	mutex_init(&ftl->mutex);
 	timer_setup(&ftl->timer, sm_cache_flush_timer, 0);
 	INIT_WORK(&ftl->flush_work, sm_cache_flush_work);
-	init_completion(&ftl->erase_completion);
 
 	/* Read media information */
 	if (sm_get_media_info(ftl, mtd)) {
diff --git a/drivers/mtd/sm_ftl.h b/drivers/mtd/sm_ftl.h
index 43bb7300785b..0a46d75cdc6a 100644
--- a/drivers/mtd/sm_ftl.h
+++ b/drivers/mtd/sm_ftl.h
@@ -53,9 +53,6 @@ struct sm_ftl {
 	struct work_struct flush_work;
 	struct timer_list timer;
 
-	/* Async erase stuff */
-	struct completion erase_completion;
-
 	/* Geometry stuff */
 	int heads;
 	int sectors;
@@ -86,7 +83,6 @@ struct chs_entry {
 		printk(KERN_DEBUG "sm_ftl" ": " format "\n", ## __VA_ARGS__)
 
 
-static void sm_erase_callback(struct erase_info *self);
 static int sm_erase_block(struct sm_ftl *ftl, int zone_num, uint16_t block,
 								int put_free);
 static void sm_mark_block_bad(struct sm_ftl *ftl, int zone_num, int block);
diff --git a/drivers/mtd/tests/mtd_test.c b/drivers/mtd/tests/mtd_test.c
index 3d0b8b5c1a53..0ac625e8f798 100644
--- a/drivers/mtd/tests/mtd_test.c
+++ b/drivers/mtd/tests/mtd_test.c
@@ -24,10 +24,6 @@ int mtdtest_erase_eraseblock(struct mtd_info *mtd, unsigned int ebnum)
 		return err;
 	}
 
-	if (ei.state == MTD_ERASE_FAILED) {
-		pr_info("some erase error occurred at EB %d\n", ebnum);
-		return -EIO;
-	}
 	return 0;
 }
 
diff --git a/drivers/mtd/tests/speedtest.c b/drivers/mtd/tests/speedtest.c
index 0b89418a0888..f8e5dc11f943 100644
--- a/drivers/mtd/tests/speedtest.c
+++ b/drivers/mtd/tests/speedtest.c
@@ -70,12 +70,6 @@ static int multiblock_erase(int ebnum, int blocks)
 		return err;
 	}
 
-	if (ei.state == MTD_ERASE_FAILED) {
-		pr_err("some erase error occurred at EB %d,"
-		       "blocks %d\n", ebnum, blocks);
-		return -EIO;
-	}
-
 	return 0;
 }
 
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 8290432017ce..8843d26837b2 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -308,18 +308,6 @@ int ubi_io_write(struct ubi_device *ubi, const void *buf, int pnum, int offset,
 	return err;
 }
 
-/**
- * erase_callback - MTD erasure call-back.
- * @ei: MTD erase information object.
- *
- * Note, even though MTD erase interface is asynchronous, all the current
- * implementations are synchronous anyway.
- */
-static void erase_callback(struct erase_info *ei)
-{
-	wake_up_interruptible((wait_queue_head_t *)ei->priv);
-}
-
 /**
  * do_sync_erase - synchronously erase a physical eraseblock.
  * @ubi: UBI device description object
@@ -333,7 +321,6 @@ static int do_sync_erase(struct ubi_device *ubi, int pnum)
 {
 	int err, retries = 0;
 	struct erase_info ei;
-	wait_queue_head_t wq;
 
 	dbg_io("erase PEB %d", pnum);
 	ubi_assert(pnum >= 0 && pnum < ubi->peb_count);
@@ -344,14 +331,11 @@ static int do_sync_erase(struct ubi_device *ubi, int pnum)
 	}
 
 retry:
-	init_waitqueue_head(&wq);
 	memset(&ei, 0, sizeof(struct erase_info));
 
 	ei.mtd      = ubi->mtd;
 	ei.addr     = (loff_t)pnum * ubi->peb_size;
 	ei.len      = ubi->peb_size;
-	ei.callback = erase_callback;
-	ei.priv     = (unsigned long)&wq;
 
 	err = mtd_erase(ubi->mtd, &ei);
 	if (err) {
@@ -366,25 +350,6 @@ static int do_sync_erase(struct ubi_device *ubi, int pnum)
 		return err;
 	}
 
-	err = wait_event_interruptible(wq, ei.state == MTD_ERASE_DONE ||
-					   ei.state == MTD_ERASE_FAILED);
-	if (err) {
-		ubi_err(ubi, "interrupted PEB %d erasure", pnum);
-		return -EINTR;
-	}
-
-	if (ei.state == MTD_ERASE_FAILED) {
-		if (retries++ < UBI_IO_RETRIES) {
-			ubi_warn(ubi, "error while erasing PEB %d, retry",
-				 pnum);
-			yield();
-			goto retry;
-		}
-		ubi_err(ubi, "cannot erase PEB %d", pnum);
-		dump_stack();
-		return -EIO;
-	}
-
 	err = ubi_self_check_all_ff(ubi, pnum, 0, ubi->peb_size);
 	if (err)
 		return err;
diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index 4a6cf289be24..09bb6c00b869 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -21,14 +21,6 @@
 #include <linux/pagemap.h>
 #include "nodelist.h"
 
-struct erase_priv_struct {
-	struct jffs2_eraseblock *jeb;
-	struct jffs2_sb_info *c;
-};
-
-#ifndef __ECOS
-static void jffs2_erase_callback(struct erase_info *);
-#endif
 static void jffs2_erase_failed(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb, uint32_t bad_offset);
 static void jffs2_erase_succeeded(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb);
 static void jffs2_mark_erased_block(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb);
@@ -51,7 +43,7 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
 	jffs2_dbg(1, "%s(): erase block %#08x (range %#08x-%#08x)\n",
 		  __func__,
 		  jeb->offset, jeb->offset, jeb->offset + c->sector_size);
-	instr = kmalloc(sizeof(struct erase_info) + sizeof(struct erase_priv_struct), GFP_KERNEL);
+	instr = kmalloc(sizeof(struct erase_info), GFP_KERNEL);
 	if (!instr) {
 		pr_warn("kmalloc for struct erase_info in jffs2_erase_block failed. Refiling block for later\n");
 		mutex_lock(&c->erase_free_sem);
@@ -70,15 +62,13 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
 	instr->mtd = c->mtd;
 	instr->addr = jeb->offset;
 	instr->len = c->sector_size;
-	instr->callback = jffs2_erase_callback;
-	instr->priv = (unsigned long)(&instr[1]);
-
-	((struct erase_priv_struct *)instr->priv)->jeb = jeb;
-	((struct erase_priv_struct *)instr->priv)->c = c;
 
 	ret = mtd_erase(c->mtd, instr);
-	if (!ret)
+	if (!ret) {
+		jffs2_erase_succeeded(c, jeb);
+		kfree(instr);
 		return;
+	}
 
 	bad_offset = instr->fail_addr;
 	kfree(instr);
@@ -214,22 +204,6 @@ static void jffs2_erase_failed(struct jffs2_sb_info *c, struct jffs2_eraseblock
 	wake_up(&c->erase_wait);
 }
 
-#ifndef __ECOS
-static void jffs2_erase_callback(struct erase_info *instr)
-{
-	struct erase_priv_struct *priv = (void *)instr->priv;
-
-	if(instr->state != MTD_ERASE_DONE) {
-		pr_warn("Erase at 0x%08llx finished, but state != MTD_ERASE_DONE. State is 0x%x instead.\n",
-			(unsigned long long)instr->addr, instr->state);
-		jffs2_erase_failed(priv->c, priv->jeb, instr->fail_addr);
-	} else {
-		jffs2_erase_succeeded(priv->c, priv->jeb);
-	}
-	kfree(instr);
-}
-#endif /* !__ECOS */
-
 /* Hmmm. Maybe we should accept the extra space it takes and make
    this a standard doubly-linked list? */
 static inline void jffs2_remove_node_refs_from_ino_list(struct jffs2_sb_info *c,
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 2a407dc9beaa..5018437d7999 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -48,8 +48,6 @@ struct erase_info {
 	uint64_t addr;
 	uint64_t len;
 	uint64_t fail_addr;
-	void (*callback) (struct erase_info *self);
-	u_long priv;
 	u_char state;
 };
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase()
  2018-02-12 21:03 [PATCH 0/5] mtd: Simplify erase handling Boris Brezillon
                   ` (2 preceding siblings ...)
  2018-02-12 21:03 ` [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous Boris Brezillon
@ 2018-02-12 21:03 ` Boris Brezillon
  2018-02-12 22:05   ` Richard Weinberger
  2018-02-12 21:03 ` [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback() Boris Brezillon
  4 siblings, 1 reply; 16+ messages in thread
From: Boris Brezillon @ 2018-02-12 21:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: Joern Engel, Robert Jarzmik, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Kyungmin Park,
	Artem Bityutskiy, Solarflare linux maintainers, Edward Cree,
	Bert Kenward, Greg Kroah-Hartman, linuxppc-dev, netdev, devel,
	Miquel Raynal

->fail_addr and ->addr can be updated no matter the result of
parent->_erase(), we just need to remove the code doing the same thing
in mtd_erase_callback() to avoid adjusting those fields twice.

Note that this can be done because all MTD users have been converted to
not pass an erase_info->callback() and are thus only taking the
->addr_fail and ->addr fields into account after part_erase() has
returned.

While we're at it, get rid of the erase_info->mtd field which was only
needed to let mtd_erase_callback() get the partition device back.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/ftl.c             |  1 -
 drivers/mtd/inftlmount.c      |  3 ---
 drivers/mtd/mtdblock.c        |  1 -
 drivers/mtd/mtdchar.c         |  1 -
 drivers/mtd/mtdconcat.c       |  1 -
 drivers/mtd/mtdoops.c         |  1 -
 drivers/mtd/mtdpart.c         | 16 ++++------------
 drivers/mtd/mtdswap.c         |  2 --
 drivers/mtd/nand/nand_base.c  |  1 -
 drivers/mtd/nand/nand_bbt.c   |  1 -
 drivers/mtd/nftlmount.c       |  1 -
 drivers/mtd/rfd_ftl.c         |  1 -
 drivers/mtd/sm_ftl.c          |  1 -
 drivers/mtd/tests/mtd_test.c  |  1 -
 drivers/mtd/tests/speedtest.c |  1 -
 drivers/mtd/ubi/io.c          |  1 -
 fs/jffs2/erase.c              |  1 -
 include/linux/mtd/mtd.h       |  3 ++-
 18 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index fcf9907e7987..0a6adfaec7b5 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -342,7 +342,6 @@ static int erase_xfer(partition_t *part,
     if (!erase)
             return -ENOMEM;
 
-    erase->mtd = part->mbd.mtd;
     erase->addr = xfer->Offset;
     erase->len = 1 << part->header.EraseUnitSize;
 
diff --git a/drivers/mtd/inftlmount.c b/drivers/mtd/inftlmount.c
index 0f47be4834d8..aab4f68bd36f 100644
--- a/drivers/mtd/inftlmount.c
+++ b/drivers/mtd/inftlmount.c
@@ -208,8 +208,6 @@ static int find_boot_record(struct INFTLrecord *inftl)
 			if (ip->Reserved0 != ip->firstUnit) {
 				struct erase_info *instr = &inftl->instr;
 
-				instr->mtd = inftl->mbd.mtd;
-
 				/*
 				 * 	Most likely this is using the
 				 * 	undocumented qiuck mount feature.
@@ -385,7 +383,6 @@ int INFTL_formatblock(struct INFTLrecord *inftl, int block)
 	   _first_? */
 
 	/* Use async erase interface, test return code */
-	instr->mtd = inftl->mbd.mtd;
 	instr->addr = block * inftl->EraseSize;
 	instr->len = inftl->mbd.mtd->erasesize;
 	/* Erase one physical eraseblock at a time, even though the NAND api
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index 7b2b7f651181..a5b1933c0490 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -65,7 +65,6 @@ static int erase_write (struct mtd_info *mtd, unsigned long pos,
 	/*
 	 * First, let's erase the flash block.
 	 */
-	erase.mtd = mtd;
 	erase.addr = pos;
 	erase.len = len;
 
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 2beb22dd6bbb..c06b33f80e75 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -726,7 +726,6 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 				erase->addr = einfo32.start;
 				erase->len = einfo32.length;
 			}
-			erase->mtd = mtd;
 
 			ret = mtd_erase(mtd, erase);
 			kfree(erase);
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index caa09bf6e572..93c47e56d9d8 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -427,7 +427,6 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
 			erase->len = length;
 
 		length -= erase->len;
-		erase->mtd = subdev;
 		if ((err = mtd_erase(subdev, erase))) {
 			/* sanity check: should never happen since
 			 * block alignment has been checked above */
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 028ded59297b..9f25111fd559 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -94,7 +94,6 @@ static int mtdoops_erase_block(struct mtdoops_context *cxt, int offset)
 	int ret;
 	int page;
 
-	erase.mtd = mtd;
 	erase.addr = offset;
 	erase.len = mtd->erasesize;
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index ae1206633d9d..1c07a6f0dfe5 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -205,23 +205,15 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 	instr->addr += part->offset;
 	ret = part->parent->_erase(part->parent, instr);
-	if (ret) {
-		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
-			instr->fail_addr -= part->offset;
-		instr->addr -= part->offset;
-	}
+	if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
+		instr->fail_addr -= part->offset;
+	instr->addr -= part->offset;
+
 	return ret;
 }
 
 void mtd_erase_callback(struct erase_info *instr)
 {
-	if (instr->mtd->_erase == part_erase) {
-		struct mtd_part *part = mtd_to_part(instr->mtd);
-
-		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
-			instr->fail_addr -= part->offset;
-		instr->addr -= part->offset;
-	}
 }
 EXPORT_SYMBOL_GPL(mtd_erase_callback);
 
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index d390324d102e..7161f8a17f62 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -549,8 +549,6 @@ static int mtdswap_erase_block(struct mtdswap_dev *d, struct swap_eb *eb)
 
 retry:
 	memset(&erase, 0, sizeof(struct erase_info));
-
-	erase.mtd	= mtd;
 	erase.addr	= mtdswap_eb_offset(d, eb);
 	erase.len	= mtd->erasesize;
 
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e70ca16a5118..16c8bc06975d 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -527,7 +527,6 @@ static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
 
 		/* Attempt erase before marking OOB */
 		memset(&einfo, 0, sizeof(einfo));
-		einfo.mtd = mtd;
 		einfo.addr = ofs;
 		einfo.len = 1ULL << chip->phys_erase_shift;
 		nand_erase_nand(mtd, &einfo, 0);
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 36092850be2c..d9f4ceff2568 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -852,7 +852,6 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 		}
 
 		memset(&einfo, 0, sizeof(einfo));
-		einfo.mtd = mtd;
 		einfo.addr = to;
 		einfo.len = 1 << this->bbt_erase_shift;
 		res = nand_erase_nand(mtd, &einfo, 1);
diff --git a/drivers/mtd/nftlmount.c b/drivers/mtd/nftlmount.c
index 07e122449759..d8f6dba01c87 100644
--- a/drivers/mtd/nftlmount.c
+++ b/drivers/mtd/nftlmount.c
@@ -328,7 +328,6 @@ int NFTL_formatblock(struct NFTLrecord *nftl, int block)
 	memset(instr, 0, sizeof(struct erase_info));
 
 	/* XXX: use async erase interface, XXX: test return code */
-	instr->mtd = nftl->mbd.mtd;
 	instr->addr = block * nftl->EraseSize;
 	instr->len = nftl->EraseSize;
 	if (mtd_erase(mtd, instr)) {
diff --git a/drivers/mtd/rfd_ftl.c b/drivers/mtd/rfd_ftl.c
index 4e0b55cd08e2..df27f24ce0fa 100644
--- a/drivers/mtd/rfd_ftl.c
+++ b/drivers/mtd/rfd_ftl.c
@@ -275,7 +275,6 @@ static int erase_block(struct partition *part, int block)
 	if (!erase)
 		return -ENOMEM;
 
-	erase->mtd = part->mbd.mtd;
 	erase->addr = part->blocks[block].offset;
 	erase->len = part->block_size;
 
diff --git a/drivers/mtd/sm_ftl.c b/drivers/mtd/sm_ftl.c
index c11156f9d96f..72740ede9f05 100644
--- a/drivers/mtd/sm_ftl.c
+++ b/drivers/mtd/sm_ftl.c
@@ -460,7 +460,6 @@ static int sm_erase_block(struct sm_ftl *ftl, int zone_num, uint16_t block,
 	struct mtd_info *mtd = ftl->trans->mtd;
 	struct erase_info erase;
 
-	erase.mtd = mtd;
 	erase.addr = sm_mkoffset(ftl, zone_num, block, 0);
 	erase.len = ftl->block_size;
 
diff --git a/drivers/mtd/tests/mtd_test.c b/drivers/mtd/tests/mtd_test.c
index 0ac625e8f798..c84250beffdc 100644
--- a/drivers/mtd/tests/mtd_test.c
+++ b/drivers/mtd/tests/mtd_test.c
@@ -14,7 +14,6 @@ int mtdtest_erase_eraseblock(struct mtd_info *mtd, unsigned int ebnum)
 	loff_t addr = (loff_t)ebnum * mtd->erasesize;
 
 	memset(&ei, 0, sizeof(struct erase_info));
-	ei.mtd  = mtd;
 	ei.addr = addr;
 	ei.len  = mtd->erasesize;
 
diff --git a/drivers/mtd/tests/speedtest.c b/drivers/mtd/tests/speedtest.c
index f8e5dc11f943..20edb3b49c77 100644
--- a/drivers/mtd/tests/speedtest.c
+++ b/drivers/mtd/tests/speedtest.c
@@ -59,7 +59,6 @@ static int multiblock_erase(int ebnum, int blocks)
 	loff_t addr = (loff_t)ebnum * mtd->erasesize;
 
 	memset(&ei, 0, sizeof(struct erase_info));
-	ei.mtd  = mtd;
 	ei.addr = addr;
 	ei.len  = mtd->erasesize * blocks;
 
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 8843d26837b2..0e3a76a9e2f8 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -333,7 +333,6 @@ static int do_sync_erase(struct ubi_device *ubi, int pnum)
 retry:
 	memset(&ei, 0, sizeof(struct erase_info));
 
-	ei.mtd      = ubi->mtd;
 	ei.addr     = (loff_t)pnum * ubi->peb_size;
 	ei.len      = ubi->peb_size;
 
diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index 09bb6c00b869..83b8f06b4a64 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -59,7 +59,6 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
 
 	memset(instr, 0, sizeof(*instr));
 
-	instr->mtd = c->mtd;
 	instr->addr = jeb->offset;
 	instr->len = c->sector_size;
 
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 5018437d7999..4cbb7f555244 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -38,13 +38,14 @@
 
 #define MTD_FAIL_ADDR_UNKNOWN -1LL
 
+struct mtd_info;
+
 /*
  * If the erase fails, fail_addr might indicate exactly which block failed. If
  * fail_addr = MTD_FAIL_ADDR_UNKNOWN, the failure was not at the device level
  * or was not specific to any particular block.
  */
 struct erase_info {
-	struct mtd_info *mtd;
 	uint64_t addr;
 	uint64_t len;
 	uint64_t fail_addr;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()
  2018-02-12 21:03 [PATCH 0/5] mtd: Simplify erase handling Boris Brezillon
                   ` (3 preceding siblings ...)
  2018-02-12 21:03 ` [PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase() Boris Brezillon
@ 2018-02-12 21:03 ` Boris Brezillon
  2018-02-12 22:17   ` Richard Weinberger
                     ` (2 more replies)
  4 siblings, 3 replies; 16+ messages in thread
From: Boris Brezillon @ 2018-02-12 21:03 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Artem Bityutskiy, Miquel Raynal, Michael Ellerman, linuxppc-dev,
	Joern Engel, Greg Kroah-Hartman, Kyungmin Park, netdev,
	Paul Mackerras, Benjamin Herrenschmidt, Edward Cree,
	Robert Jarzmik

MTD users are no longer checking erase_info->state to determine if the
erase operation failed or succeeded. Moreover, mtd_erase_callback() is
now a NOP.

We can safely get rid of all mtd_erase_callback() calls and all
erase_info->state assignments. While at it, get rid of the
erase_info->state field, all MTD_ERASE_XXX definitions and the
mtd_erase_callback() function.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
 drivers/mtd/chips/cfi_cmdset_0001.c      | 16 ++--------------
 drivers/mtd/chips/cfi_cmdset_0002.c      | 26 +++-----------------------
 drivers/mtd/chips/cfi_cmdset_0020.c      |  3 ---
 drivers/mtd/chips/map_ram.c              |  2 --
 drivers/mtd/devices/bcm47xxsflash.c      |  9 +--------
 drivers/mtd/devices/block2mtd.c          |  7 +------
 drivers/mtd/devices/docg3.c              | 16 ++--------------
 drivers/mtd/devices/lart.c               |  4 ----
 drivers/mtd/devices/mtd_dataflash.c      |  4 ----
 drivers/mtd/devices/mtdram.c             |  2 --
 drivers/mtd/devices/phram.c              |  2 --
 drivers/mtd/devices/pmc551.c             |  2 --
 drivers/mtd/devices/powernv_flash.c      | 11 ++---------
 drivers/mtd/devices/slram.c              |  2 --
 drivers/mtd/devices/spear_smi.c          |  3 ---
 drivers/mtd/devices/sst25l.c             |  3 ---
 drivers/mtd/devices/st_spi_fsm.c         |  4 ----
 drivers/mtd/lpddr/lpddr2_nvm.c           | 10 ++--------
 drivers/mtd/lpddr/lpddr_cmds.c           |  2 --
 drivers/mtd/mtdconcat.c                  |  1 -
 drivers/mtd/mtdcore.c                    |  6 ++----
 drivers/mtd/mtdpart.c                    |  5 -----
 drivers/mtd/nand/nand_base.c             | 15 +++------------
 drivers/mtd/onenand/onenand_base.c       | 17 -----------------
 drivers/mtd/spi-nor/spi-nor.c            |  3 ---
 drivers/mtd/ubi/gluebi.c                 |  3 ---
 drivers/net/ethernet/sfc/falcon/mtd.c    | 11 +----------
 drivers/net/ethernet/sfc/mtd.c           | 11 +----------
 drivers/staging/goldfish/goldfish_nand.c |  3 ---
 include/linux/mtd/mtd.h                  |  9 ---------
 30 files changed, 20 insertions(+), 192 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 5e1b68cbcd0a..d4c07b85f18e 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -1993,20 +1993,8 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 
 static int cfi_intelext_erase_varsize(struct mtd_info *mtd, struct erase_info *instr)
 {
-	unsigned long ofs, len;
-	int ret;
-
-	ofs = instr->addr;
-	len = instr->len;
-
-	ret = cfi_varsize_frob(mtd, do_erase_oneblock, ofs, len, NULL);
-	if (ret)
-		return ret;
-
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
-	return 0;
+	return cfi_varsize_frob(mtd, do_erase_oneblock, instr->addr,
+				instr->len, NULL);
 }
 
 static void cfi_intelext_sync (struct mtd_info *mtd)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 56aa6b75213d..668e2cbc155b 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -2415,20 +2415,8 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 
 static int cfi_amdstd_erase_varsize(struct mtd_info *mtd, struct erase_info *instr)
 {
-	unsigned long ofs, len;
-	int ret;
-
-	ofs = instr->addr;
-	len = instr->len;
-
-	ret = cfi_varsize_frob(mtd, do_erase_oneblock, ofs, len, NULL);
-	if (ret)
-		return ret;
-
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
-	return 0;
+	return cfi_varsize_frob(mtd, do_erase_oneblock, instr->addr,
+				instr->len, NULL);
 }
 
 
@@ -2436,7 +2424,6 @@ static int cfi_amdstd_erase_chip(struct mtd_info *mtd, struct erase_info *instr)
 {
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
-	int ret = 0;
 
 	if (instr->addr != 0)
 		return -EINVAL;
@@ -2444,14 +2431,7 @@ static int cfi_amdstd_erase_chip(struct mtd_info *mtd, struct erase_info *instr)
 	if (instr->len != mtd->size)
 		return -EINVAL;
 
-	ret = do_erase_chip(map, &cfi->chips[0]);
-	if (ret)
-		return ret;
-
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
-	return 0;
+	return do_erase_chip(map, &cfi->chips[0]);
 }
 
 static int do_atmel_lock(struct map_info *map, struct flchip *chip,
diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c b/drivers/mtd/chips/cfi_cmdset_0020.c
index 7d342965f392..7b7658a05036 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -965,9 +965,6 @@ static int cfi_staa_erase_varsize(struct mtd_info *mtd,
 		}
 	}
 
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
 	return 0;
 }
 
diff --git a/drivers/mtd/chips/map_ram.c b/drivers/mtd/chips/map_ram.c
index 1cd0fff0e940..c37fce926864 100644
--- a/drivers/mtd/chips/map_ram.c
+++ b/drivers/mtd/chips/map_ram.c
@@ -131,8 +131,6 @@ static int mapram_erase (struct mtd_info *mtd, struct erase_info *instr)
 	allff = map_word_ff(map);
 	for (i=0; i<instr->len; i += map_bankwidth(map))
 		map_write(map, allff, instr->addr + i);
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 	return 0;
 }
 
diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
index 6b84947cfbea..9baa81b8780c 100644
--- a/drivers/mtd/devices/bcm47xxsflash.c
+++ b/drivers/mtd/devices/bcm47xxsflash.c
@@ -68,7 +68,6 @@ static int bcm47xxsflash_poll(struct bcm47xxsflash *b47s, int timeout)
 static int bcm47xxsflash_erase(struct mtd_info *mtd, struct erase_info *erase)
 {
 	struct bcm47xxsflash *b47s = mtd->priv;
-	int err;
 
 	switch (b47s->type) {
 	case BCM47XXSFLASH_TYPE_ST:
@@ -89,13 +88,7 @@ static int bcm47xxsflash_erase(struct mtd_info *mtd, struct erase_info *erase)
 		break;
 	}
 
-	err = bcm47xxsflash_poll(b47s, HZ);
-	if (err)
-		erase->state = MTD_ERASE_FAILED;
-	else
-		erase->state = MTD_ERASE_DONE;
-
-	return err;
+	return bcm47xxsflash_poll(b47s, HZ);
 }
 
 static int bcm47xxsflash_read(struct mtd_info *mtd, loff_t from, size_t len,
diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 62fd6905c648..0e741017f189 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -88,17 +88,12 @@ static int block2mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 	size_t len = instr->len;
 	int err;
 
-	instr->state = MTD_ERASING;
 	mutex_lock(&dev->write_mutex);
 	err = _block2mtd_erase(dev, from, len);
 	mutex_unlock(&dev->write_mutex);
-	if (err) {
+	if (err)
 		pr_err("erase failed err = %d\n", err);
-		instr->state = MTD_ERASE_FAILED;
-	} else
-		instr->state = MTD_ERASE_DONE;
 
-	mtd_erase_callback(instr);
 	return err;
 }
 
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index a85af236b44d..c594fe5eac08 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -1191,39 +1191,27 @@ 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;
+	int block0, block1, page, ret = 0, ofs = 0;
 
 	doc_dbg("doc_erase(from=%lld, len=%lld\n", info->addr, info->len);
 
-	info->state = MTD_ERASE_PENDING;
 	calc_block_sector(info->addr + info->len, &block0, &block1, &page,
 			  &ofs, docg3->reliable);
-	ret = -EINVAL;
 	if (info->addr + info->len > mtd->size || page || ofs)
-		goto reset_err;
+		return -EINVAL;
 
-	ret = 0;
 	calc_block_sector(info->addr, &block0, &block1, &page, &ofs,
 			  docg3->reliable);
 	mutex_lock(&docg3->cascade->lock);
 	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;
 		ret = doc_erase_block(docg3, block0, block1);
 		block0 += 2;
 		block1 += 2;
 	}
 	mutex_unlock(&docg3->cascade->lock);
 
-	if (ret)
-		goto reset_err;
-
-	info->state = MTD_ERASE_DONE;
-	return 0;
-
-reset_err:
-	info->state = MTD_ERASE_FAILED;
 	return ret;
 }
 
diff --git a/drivers/mtd/devices/lart.c b/drivers/mtd/devices/lart.c
index 555b94406e0b..3d6c8ffd351f 100644
--- a/drivers/mtd/devices/lart.c
+++ b/drivers/mtd/devices/lart.c
@@ -415,7 +415,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
 	 {
 		if (!erase_block (addr))
 		  {
-			 instr->state = MTD_ERASE_FAILED;
 			 return (-EIO);
 		  }
 
@@ -425,9 +424,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
 		if (addr == mtd->eraseregions[i].offset + (mtd->eraseregions[i].erasesize * mtd->eraseregions[i].numblocks)) i++;
 	 }
 
-   instr->state = MTD_ERASE_DONE;
-   mtd_erase_callback(instr);
-
    return (0);
 }
 
diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
index 5dc8bd042cc5..aaaeaae01e1d 100644
--- a/drivers/mtd/devices/mtd_dataflash.c
+++ b/drivers/mtd/devices/mtd_dataflash.c
@@ -220,10 +220,6 @@ static int dataflash_erase(struct mtd_info *mtd, struct erase_info *instr)
 	}
 	mutex_unlock(&priv->lock);
 
-	/* Inform MTD subsystem that erase is complete */
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
 	return 0;
 }
 
diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 0bf4aeaf0cb8..efef43c6684b 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -60,8 +60,6 @@ static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
 	if (check_offs_len(mtd, instr->addr, instr->len))
 		return -EINVAL;
 	memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 	return 0;
 }
 
diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
index 7287696a21f9..a963c88d392d 100644
--- a/drivers/mtd/devices/phram.c
+++ b/drivers/mtd/devices/phram.c
@@ -44,8 +44,6 @@ static int phram_erase(struct mtd_info *mtd, struct erase_info *instr)
 	 * I don't feel at all ashamed. This kind of thing is possible anyway
 	 * with flash, but unlikely.
 	 */
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 	return 0;
 }
 
diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
index cadea0620cd0..5d842cbca3de 100644
--- a/drivers/mtd/devices/pmc551.c
+++ b/drivers/mtd/devices/pmc551.c
@@ -184,12 +184,10 @@ static int pmc551_erase(struct mtd_info *mtd, struct erase_info *instr)
 	}
 
       out:
-	instr->state = MTD_ERASE_DONE;
 #ifdef CONFIG_MTD_PMC551_DEBUG
 	printk(KERN_DEBUG "pmc551_erase() done\n");
 #endif
 
-	mtd_erase_callback(instr);
 	return 0;
 }
 
diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c
index 26f9feaa5d17..5f383630c16f 100644
--- a/drivers/mtd/devices/powernv_flash.c
+++ b/drivers/mtd/devices/powernv_flash.c
@@ -175,19 +175,12 @@ static int powernv_flash_erase(struct mtd_info *mtd, struct erase_info *erase)
 {
 	int rc;
 
-	erase->state = MTD_ERASING;
-
 	/* todo: register our own notifier to do a true async implementation */
 	rc =  powernv_flash_async_op(mtd, FLASH_OP_ERASE, erase->addr,
 			erase->len, NULL, NULL);
-
-	if (rc) {
+	if (rc)
 		erase->fail_addr = erase->addr;
-		erase->state = MTD_ERASE_FAILED;
-	} else {
-		erase->state = MTD_ERASE_DONE;
-	}
-	mtd_erase_callback(erase);
+
 	return rc;
 }
 
diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
index 0ec85f316d24..2f05e1801047 100644
--- a/drivers/mtd/devices/slram.c
+++ b/drivers/mtd/devices/slram.c
@@ -88,8 +88,6 @@ static int slram_erase(struct mtd_info *mtd, struct erase_info *instr)
 	 * I don't feel at all ashamed. This kind of thing is possible anyway
 	 * with flash, but unlikely.
 	 */
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 	return(0);
 }
 
diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c
index ddf478976013..986f81d2f93e 100644
--- a/drivers/mtd/devices/spear_smi.c
+++ b/drivers/mtd/devices/spear_smi.c
@@ -518,7 +518,6 @@ static int spear_mtd_erase(struct mtd_info *mtd, struct erase_info *e_info)
 		/* preparing the command for flash */
 		ret = spear_smi_erase_sector(dev, bank, command, 4);
 		if (ret) {
-			e_info->state = MTD_ERASE_FAILED;
 			mutex_unlock(&flash->lock);
 			return ret;
 		}
@@ -527,8 +526,6 @@ static int spear_mtd_erase(struct mtd_info *mtd, struct erase_info *e_info)
 	}
 
 	mutex_unlock(&flash->lock);
-	e_info->state = MTD_ERASE_DONE;
-	mtd_erase_callback(e_info);
 
 	return 0;
 }
diff --git a/drivers/mtd/devices/sst25l.c b/drivers/mtd/devices/sst25l.c
index 5b84d71efb36..1897f33fe3e7 100644
--- a/drivers/mtd/devices/sst25l.c
+++ b/drivers/mtd/devices/sst25l.c
@@ -195,7 +195,6 @@ static int sst25l_erase(struct mtd_info *mtd, struct erase_info *instr)
 		err = sst25l_erase_sector(flash, addr);
 		if (err) {
 			mutex_unlock(&flash->lock);
-			instr->state = MTD_ERASE_FAILED;
 			dev_err(&flash->spi->dev, "Erase failed\n");
 			return err;
 		}
@@ -205,8 +204,6 @@ static int sst25l_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 	mutex_unlock(&flash->lock);
 
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 	return 0;
 }
 
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 7bc29d725200..b0a8ad3c2c7b 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -1825,13 +1825,9 @@ static int stfsm_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 	mutex_unlock(&fsm->lock);
 
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
 	return 0;
 
 out1:
-	instr->state = MTD_ERASE_FAILED;
 	mutex_unlock(&fsm->lock);
 
 	return ret;
diff --git a/drivers/mtd/lpddr/lpddr2_nvm.c b/drivers/mtd/lpddr/lpddr2_nvm.c
index 2342277c9bcb..5d73db2a496d 100644
--- a/drivers/mtd/lpddr/lpddr2_nvm.c
+++ b/drivers/mtd/lpddr/lpddr2_nvm.c
@@ -380,14 +380,8 @@ static int lpddr2_nvm_write(struct mtd_info *mtd, loff_t start_add,
  */
 static int lpddr2_nvm_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
-	int ret = lpddr2_nvm_do_block_op(mtd, instr->addr, instr->len,
-		LPDDR2_NVM_ERASE);
-	if (!ret) {
-		instr->state = MTD_ERASE_DONE;
-		mtd_erase_callback(instr);
-	}
-
-	return ret;
+	return lpddr2_nvm_do_block_op(mtd, instr->addr, instr->len,
+				      LPDDR2_NVM_ERASE);
 }
 
 /*
diff --git a/drivers/mtd/lpddr/lpddr_cmds.c b/drivers/mtd/lpddr/lpddr_cmds.c
index 018c75faadb3..5c5ba3c7c79d 100644
--- a/drivers/mtd/lpddr/lpddr_cmds.c
+++ b/drivers/mtd/lpddr/lpddr_cmds.c
@@ -693,8 +693,6 @@ static int lpddr_erase(struct mtd_info *mtd, struct erase_info *instr)
 		ofs += size;
 		len -= size;
 	}
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 
 	return 0;
 }
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index 93c47e56d9d8..6b86d1a73cf2 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -446,7 +446,6 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
 		erase->addr = 0;
 		offset += subdev->size;
 	}
-	instr->state = erase->state;
 	kfree(erase);
 
 	return err;
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index f92ad02959eb..f25d65ea7149 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -961,11 +961,9 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
 
-	if (!instr->len) {
-		instr->state = MTD_ERASE_DONE;
-		mtd_erase_callback(instr);
+	if (!instr->len)
 		return 0;
-	}
+
 	ledtrig_mtd_activity();
 	return mtd->_erase(mtd, instr);
 }
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 1c07a6f0dfe5..85fea8ea3423 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -212,11 +212,6 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
 	return ret;
 }
 
-void mtd_erase_callback(struct erase_info *instr)
-{
-}
-EXPORT_SYMBOL_GPL(mtd_erase_callback);
-
 static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
 	struct mtd_part *part = mtd_to_part(mtd);
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 16c8bc06975d..13dd67106859 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4604,22 +4604,20 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 	if (nand_check_wp(mtd)) {
 		pr_debug("%s: device is write protected!\n",
 				__func__);
-		instr->state = MTD_ERASE_FAILED;
+		ret = -EIO;
 		goto erase_exit;
 	}
 
 	/* Loop through the pages */
 	len = instr->len;
 
-	instr->state = MTD_ERASING;
-
 	while (len) {
 		/* Check if we have a bad block, we do not erase bad blocks! */
 		if (nand_block_checkbad(mtd, ((loff_t) page) <<
 					chip->page_shift, allowbbt)) {
 			pr_warn("%s: attempt to erase a bad block at page 0x%08x\n",
 				    __func__, page);
-			instr->state = MTD_ERASE_FAILED;
+			ret = -EIO;
 			goto erase_exit;
 		}
 
@@ -4637,7 +4635,7 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 		if (status) {
 			pr_debug("%s: failed erase, page 0x%08x\n",
 					__func__, page);
-			instr->state = MTD_ERASE_FAILED;
+			ret = -EIO;
 			instr->fail_addr =
 				((loff_t)page << chip->page_shift);
 			goto erase_exit;
@@ -4654,20 +4652,13 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 			chip->select_chip(mtd, chipnr);
 		}
 	}
-	instr->state = MTD_ERASE_DONE;
 
 erase_exit:
 
-	ret = instr->state == MTD_ERASE_DONE ? 0 : -EIO;
-
 	/* Deselect and wake up anyone waiting on the device */
 	chip->select_chip(mtd, -1);
 	nand_release_device(mtd);
 
-	/* Do call back function */
-	if (!ret)
-		mtd_erase_callback(instr);
-
 	/* Return more or less happy */
 	return ret;
 }
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 979f4031f23c..8d19b78777b5 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -2143,7 +2143,6 @@ static int onenand_multiblock_erase_verify(struct mtd_info *mtd,
 		if (ret) {
 			printk(KERN_ERR "%s: Failed verify, block %d\n",
 			       __func__, onenand_block(this, addr));
-			instr->state = MTD_ERASE_FAILED;
 			instr->fail_addr = addr;
 			return -1;
 		}
@@ -2172,8 +2171,6 @@ static int onenand_multiblock_erase(struct mtd_info *mtd,
 	int ret = 0;
 	int bdry_block = 0;
 
-	instr->state = MTD_ERASING;
-
 	if (ONENAND_IS_DDP(this)) {
 		loff_t bdry_addr = this->chipsize >> 1;
 		if (addr < bdry_addr && (addr + len) > bdry_addr)
@@ -2187,7 +2184,6 @@ static int onenand_multiblock_erase(struct mtd_info *mtd,
 			printk(KERN_WARNING "%s: attempt to erase a bad block "
 			       "at addr 0x%012llx\n",
 			       __func__, (unsigned long long) addr);
-			instr->state = MTD_ERASE_FAILED;
 			return -EIO;
 		}
 		len -= block_size;
@@ -2227,7 +2223,6 @@ static int onenand_multiblock_erase(struct mtd_info *mtd,
 				printk(KERN_ERR "%s: Failed multiblock erase, "
 				       "block %d\n", __func__,
 				       onenand_block(this, addr));
-				instr->state = MTD_ERASE_FAILED;
 				instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 				return -EIO;
 			}
@@ -2247,7 +2242,6 @@ static int onenand_multiblock_erase(struct mtd_info *mtd,
 		if (ret) {
 			printk(KERN_ERR "%s: Failed erase, block %d\n",
 			       __func__, onenand_block(this, addr));
-			instr->state = MTD_ERASE_FAILED;
 			instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 			return -EIO;
 		}
@@ -2259,7 +2253,6 @@ static int onenand_multiblock_erase(struct mtd_info *mtd,
 		/* verify */
 		verify_instr.len = eb_count * block_size;
 		if (onenand_multiblock_erase_verify(mtd, &verify_instr)) {
-			instr->state = verify_instr.state;
 			instr->fail_addr = verify_instr.fail_addr;
 			return -EIO;
 		}
@@ -2294,8 +2287,6 @@ static int onenand_block_by_block_erase(struct mtd_info *mtd,
 		region_end = region->offset + region->erasesize * region->numblocks;
 	}
 
-	instr->state = MTD_ERASING;
-
 	/* Loop through the blocks */
 	while (len) {
 		cond_resched();
@@ -2305,7 +2296,6 @@ static int onenand_block_by_block_erase(struct mtd_info *mtd,
 			printk(KERN_WARNING "%s: attempt to erase a bad block "
 					"at addr 0x%012llx\n",
 					__func__, (unsigned long long) addr);
-			instr->state = MTD_ERASE_FAILED;
 			return -EIO;
 		}
 
@@ -2318,7 +2308,6 @@ static int onenand_block_by_block_erase(struct mtd_info *mtd,
 		if (ret) {
 			printk(KERN_ERR "%s: Failed erase, block %d\n",
 				__func__, onenand_block(this, addr));
-			instr->state = MTD_ERASE_FAILED;
 			instr->fail_addr = addr;
 			return -EIO;
 		}
@@ -2407,12 +2396,6 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 	/* Deselect and wake up anyone waiting on the device */
 	onenand_release_device(mtd);
 
-	/* Do call back function */
-	if (!ret) {
-		instr->state = MTD_ERASE_DONE;
-		mtd_erase_callback(instr);
-	}
-
 	return ret;
 }
 
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d445a4d3b770..5bfa36e95f35 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -560,9 +560,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 erase_err:
 	spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_ERASE);
 
-	instr->state = ret ? MTD_ERASE_FAILED : MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
 	return ret;
 }
 
diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c
index 1cb287ec32ad..6b655a53113b 100644
--- a/drivers/mtd/ubi/gluebi.c
+++ b/drivers/mtd/ubi/gluebi.c
@@ -272,12 +272,9 @@ static int gluebi_erase(struct mtd_info *mtd, struct erase_info *instr)
 	if (err)
 		goto out_err;
 
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 	return 0;
 
 out_err:
-	instr->state = MTD_ERASE_FAILED;
 	instr->fail_addr = (long long)lnum * mtd->erasesize;
 	return err;
 }
diff --git a/drivers/net/ethernet/sfc/falcon/mtd.c b/drivers/net/ethernet/sfc/falcon/mtd.c
index cde593cb1052..2d67e4621a3d 100644
--- a/drivers/net/ethernet/sfc/falcon/mtd.c
+++ b/drivers/net/ethernet/sfc/falcon/mtd.c
@@ -24,17 +24,8 @@
 static int ef4_mtd_erase(struct mtd_info *mtd, struct erase_info *erase)
 {
 	struct ef4_nic *efx = mtd->priv;
-	int rc;
 
-	rc = efx->type->mtd_erase(mtd, erase->addr, erase->len);
-	if (rc == 0) {
-		erase->state = MTD_ERASE_DONE;
-	} else {
-		erase->state = MTD_ERASE_FAILED;
-		erase->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
-	}
-	mtd_erase_callback(erase);
-	return rc;
+	return efx->type->mtd_erase(mtd, erase->addr, erase->len);
 }
 
 static void ef4_mtd_sync(struct mtd_info *mtd)
diff --git a/drivers/net/ethernet/sfc/mtd.c b/drivers/net/ethernet/sfc/mtd.c
index a77a8bd2dd70..4ac30b6e5dab 100644
--- a/drivers/net/ethernet/sfc/mtd.c
+++ b/drivers/net/ethernet/sfc/mtd.c
@@ -24,17 +24,8 @@
 static int efx_mtd_erase(struct mtd_info *mtd, struct erase_info *erase)
 {
 	struct efx_nic *efx = mtd->priv;
-	int rc;
 
-	rc = efx->type->mtd_erase(mtd, erase->addr, erase->len);
-	if (rc == 0) {
-		erase->state = MTD_ERASE_DONE;
-	} else {
-		erase->state = MTD_ERASE_FAILED;
-		erase->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
-	}
-	mtd_erase_callback(erase);
-	return rc;
+	return efx->type->mtd_erase(mtd, erase->addr, erase->len);
 }
 
 static void efx_mtd_sync(struct mtd_info *mtd)
diff --git a/drivers/staging/goldfish/goldfish_nand.c b/drivers/staging/goldfish/goldfish_nand.c
index 52cc1363993e..f5e002ecba75 100644
--- a/drivers/staging/goldfish/goldfish_nand.c
+++ b/drivers/staging/goldfish/goldfish_nand.c
@@ -119,9 +119,6 @@ static int goldfish_nand_erase(struct mtd_info *mtd, struct erase_info *instr)
 		return -EIO;
 	}
 
-	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
-
 	return 0;
 
 invalid_arg:
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 4cbb7f555244..a86c4fa93115 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -30,12 +30,6 @@
 
 #include <asm/div64.h>
 
-#define MTD_ERASE_PENDING	0x01
-#define MTD_ERASING		0x02
-#define MTD_ERASE_SUSPEND	0x04
-#define MTD_ERASE_DONE		0x08
-#define MTD_ERASE_FAILED	0x10
-
 #define MTD_FAIL_ADDR_UNKNOWN -1LL
 
 struct mtd_info;
@@ -49,7 +43,6 @@ struct erase_info {
 	uint64_t addr;
 	uint64_t len;
 	uint64_t fail_addr;
-	u_char state;
 };
 
 struct mtd_erase_region_info {
@@ -589,8 +582,6 @@ extern void register_mtd_user (struct mtd_notifier *new);
 extern int unregister_mtd_user (struct mtd_notifier *old);
 void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size);
 
-void mtd_erase_callback(struct erase_info *instr);
-
 static inline int mtd_is_bitflip(int err) {
 	return err == -EUCLEAN;
 }
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info
  2018-02-12 21:03 ` [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info Boris Brezillon
@ 2018-02-12 21:47   ` Richard Weinberger
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2018-02-12 21:47 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen,
	linux-mtd, Joern Engel, Robert Jarzmik, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Kyungmin Park,
	Artem Bityutskiy, Solarflare linux maintainers, Edward Cree,
	Bert Kenward, Greg Kroah-Hartman, linuxppc-dev, netdev, devel,
	Miqu

Am Montag, 12. Februar 2018, 22:03:08 CET schrieb Boris Brezillon:
> Some fields are not used by MTD drivers, users or core code. Moreover,
> those fields are not documented, so get rid of them to avoid any
> confusion.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  include/linux/mtd/mtd.h | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 205ededccc60..2a407dc9beaa 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -48,14 +48,9 @@ struct erase_info {
>  	uint64_t addr;
>  	uint64_t len;
>  	uint64_t fail_addr;
> -	u_long time;
> -	u_long retries;
> -	unsigned dev;
> -	unsigned cell;
>  	void (*callback) (struct erase_info *self);
>  	u_long priv;
>  	u_char state;
> -	struct erase_info *next;
>  };
> 
>  struct mtd_erase_region_info {

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()
  2018-02-12 21:03 ` [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase() Boris Brezillon
@ 2018-02-12 21:47   ` Richard Weinberger
  2018-03-18 21:22   ` Boris Brezillon
  1 sibling, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2018-02-12 21:47 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen,
	linux-mtd, Joern Engel, Robert Jarzmik, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Kyungmin Park,
	Artem Bityutskiy, Solarflare linux maintainers, Edward Cree,
	Bert Kenward, Greg Kroah-Hartman, linuxppc-dev, netdev, devel,
	Miqu

Am Montag, 12. Februar 2018, 22:03:07 CET schrieb Boris Brezillon:
> mtd_erase() can return an error before ->fail_addr is initialized to
> MTD_FAIL_ADDR_UNKNOWN. Move this initialization at the very beginning
> of the function.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  drivers/mtd/mtdcore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index a1c94526fb88..c87859ff338b 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -953,6 +953,8 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
>   */
>  int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  {
> +	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> +
>  	if (!mtd->erasesize || !mtd->_erase)
>  		return -ENOTSUPP;
> 
> @@ -961,7 +963,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info
> *instr) if (!(mtd->flags & MTD_WRITEABLE))
>  		return -EROFS;
> 
> -	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>  	if (!instr->len) {
>  		instr->state = MTD_ERASE_DONE;
>  		mtd_erase_callback(instr);

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous
  2018-02-12 21:03 ` [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous Boris Brezillon
@ 2018-02-12 21:58   ` Richard Weinberger
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2018-02-12 21:58 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen,
	linux-mtd, Joern Engel, Robert Jarzmik, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Kyungmin Park,
	Artem Bityutskiy, Solarflare linux maintainers, Edward Cree,
	Bert Kenward, Greg Kroah-Hartman, linuxppc-dev, netdev, devel,
	Miqu

Am Montag, 12. Februar 2018, 22:03:09 CET schrieb Boris Brezillon:
> None of the mtd->_erase() implementations work in an asynchronous manner,
> so let's simplify MTD users that call mtd_erase(). All they need to do
> is check the value returned by mtd_erase() and assume that != 0 means
> failure.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase()
  2018-02-12 21:03 ` [PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase() Boris Brezillon
@ 2018-02-12 22:05   ` Richard Weinberger
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2018-02-12 22:05 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Benjamin Herrenschmidt, Artem Bityutskiy, Miquel Raynal,
	Robert Jarzmik, linuxppc-dev, Joern Engel, Greg Kroah-Hartman,
	Marek Vasut, Kyungmin Park, netdev, linux-mtd, Michael Ellerman,
	Cyrille Pitchen, Edward Cree, Paul Mackerras, Brian Norris,
	David Woodhouse

Am Montag, 12. Februar 2018, 22:03:10 CET schrieb Boris Brezillon:
> ->fail_addr and ->addr can be updated no matter the result of
> parent->_erase(), we just need to remove the code doing the same thing
> in mtd_erase_callback() to avoid adjusting those fields twice.
> 
> Note that this can be done because all MTD users have been converted to
> not pass an erase_info->callback() and are thus only taking the
> ->addr_fail and ->addr fields into account after part_erase() has
> returned.
> 
> While we're at it, get rid of the erase_info->mtd field which was only
> needed to let mtd_erase_callback() get the partition device back.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()
  2018-02-12 21:03 ` [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback() Boris Brezillon
@ 2018-02-12 22:17   ` Richard Weinberger
  2018-02-13  7:42   ` Miquel Raynal
  2018-02-13 12:09   ` Bert Kenward
  2 siblings, 0 replies; 16+ messages in thread
From: Richard Weinberger @ 2018-02-12 22:17 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Benjamin Herrenschmidt, Artem Bityutskiy, Miquel Raynal,
	Robert Jarzmik, linuxppc-dev, Joern Engel, Greg Kroah-Hartman,
	Marek Vasut, Kyungmin Park, netdev, linux-mtd, Michael Ellerman,
	Cyrille Pitchen, Edward Cree, Paul Mackerras, Brian Norris,
	David Woodhouse

Am Montag, 12. Februar 2018, 22:03:11 CET schrieb Boris Brezillon:
> MTD users are no longer checking erase_info->state to determine if the
> erase operation failed or succeeded. Moreover, mtd_erase_callback() is
> now a NOP.
> 
> We can safely get rid of all mtd_erase_callback() calls and all
> erase_info->state assignments. While at it, get rid of the
> erase_info->state field, all MTD_ERASE_XXX definitions and the
> mtd_erase_callback() function.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()
  2018-02-12 21:03 ` [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback() Boris Brezillon
  2018-02-12 22:17   ` Richard Weinberger
@ 2018-02-13  7:42   ` Miquel Raynal
  2018-02-13  8:17     ` Boris Brezillon
  2018-02-13 12:09   ` Bert Kenward
  2 siblings, 1 reply; 16+ messages in thread
From: Miquel Raynal @ 2018-02-13  7:42 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Benjamin Herrenschmidt, Artem Bityutskiy, Richard Weinberger,
	Robert Jarzmik, linuxppc-dev, Joern Engel, Greg Kroah-Hartman,
	Marek Vasut, Kyungmin Park, netdev, linux-mtd, Michael Ellerman,
	Cyrille Pitchen, Edward Cree, Paul Mackerras, Brian Norris,
	David Woodhouse

Hi Boris,

Just a few comments about the form.

Otherwise:
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


> diff --git a/drivers/mtd/devices/lart.c b/drivers/mtd/devices/lart.c
> index 555b94406e0b..3d6c8ffd351f 100644
> --- a/drivers/mtd/devices/lart.c
> +++ b/drivers/mtd/devices/lart.c
> @@ -415,7 +415,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
>  	 {
>  		if (!erase_block (addr))
>  		  {
> -			 instr->state = MTD_ERASE_FAILED;
>  			 return (-EIO);
>  		  }

You can also safely remove these '{' '}'

>  
> @@ -425,9 +424,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
>  		if (addr == mtd->eraseregions[i].offset + (mtd->eraseregions[i].erasesize * mtd->eraseregions[i].numblocks)) i++;
>  	 }
>  
> -   instr->state = MTD_ERASE_DONE;
> -   mtd_erase_callback(instr);
> -
>     return (0);
>  }
>  
> diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
> index 5dc8bd042cc5..aaaeaae01e1d 100644
> --- a/drivers/mtd/devices/mtd_dataflash.c
> +++ b/drivers/mtd/devices/mtd_dataflash.c
> @@ -220,10 +220,6 @@ static int dataflash_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	}
>  	mutex_unlock(&priv->lock);
>  
> -	/* Inform MTD subsystem that erase is complete */
> -	instr->state = MTD_ERASE_DONE;
> -	mtd_erase_callback(instr);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> index 0bf4aeaf0cb8..efef43c6684b 100644
> --- a/drivers/mtd/devices/mtdram.c
> +++ b/drivers/mtd/devices/mtdram.c
> @@ -60,8 +60,6 @@ static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	if (check_offs_len(mtd, instr->addr, instr->len))
>  		return -EINVAL;
>  	memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
> -	instr->state = MTD_ERASE_DONE;
> -	mtd_erase_callback(instr);

Space ?

>  	return 0;
>  }
>  
> diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> index 7287696a21f9..a963c88d392d 100644
> --- a/drivers/mtd/devices/phram.c
> +++ b/drivers/mtd/devices/phram.c
> @@ -44,8 +44,6 @@ static int phram_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	 * I don't feel at all ashamed. This kind of thing is possible anyway
>  	 * with flash, but unlikely.
>  	 */

Not sure this comment is still relevant? Maybe you could remove it
or at least change it? 

> -	instr->state = MTD_ERASE_DONE;
> -	mtd_erase_callback(instr);

Space ?

>  	return 0;
>  }
>  
> diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
> index cadea0620cd0..5d842cbca3de 100644
> --- a/drivers/mtd/devices/pmc551.c
> +++ b/drivers/mtd/devices/pmc551.c
> @@ -184,12 +184,10 @@ static int pmc551_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	}
>  
>        out:
> -	instr->state = MTD_ERASE_DONE;
>  #ifdef CONFIG_MTD_PMC551_DEBUG
>  	printk(KERN_DEBUG "pmc551_erase() done\n");
>  #endif
>  
> -	mtd_erase_callback(instr);
>  	return 0;
>  }
>  
> diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c
> index 26f9feaa5d17..5f383630c16f 100644
> --- a/drivers/mtd/devices/powernv_flash.c
> +++ b/drivers/mtd/devices/powernv_flash.c
> @@ -175,19 +175,12 @@ static int powernv_flash_erase(struct mtd_info *mtd, struct erase_info *erase)
>  {
>  	int rc;
>  
> -	erase->state = MTD_ERASING;
> -
>  	/* todo: register our own notifier to do a true async implementation */
>  	rc =  powernv_flash_async_op(mtd, FLASH_OP_ERASE, erase->addr,
>  			erase->len, NULL, NULL);

Are you sure this is still needed? Maybe this should go away in your
first patch?

> -
> -	if (rc) {
> +	if (rc)
>  		erase->fail_addr = erase->addr;
> -		erase->state = MTD_ERASE_FAILED;
> -	} else {
> -		erase->state = MTD_ERASE_DONE;
> -	}
> -	mtd_erase_callback(erase);
> +
>  	return rc;
>  }
>  
> diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
> index 0ec85f316d24..2f05e1801047 100644
> --- a/drivers/mtd/devices/slram.c
> +++ b/drivers/mtd/devices/slram.c
> @@ -88,8 +88,6 @@ static int slram_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	 * I don't feel at all ashamed. This kind of thing is possible anyway
>  	 * with flash, but unlikely.
>  	 */

Same with this comment.

> -	instr->state = MTD_ERASE_DONE;
> -	mtd_erase_callback(instr);

Space ?

>  	return(0);
>  }
>  




-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()
  2018-02-13  7:42   ` Miquel Raynal
@ 2018-02-13  8:17     ` Boris Brezillon
  2018-02-13  8:33       ` Miquel Raynal
  0 siblings, 1 reply; 16+ messages in thread
From: Boris Brezillon @ 2018-02-13  8:17 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
	Cyrille Pitchen, linux-mtd, Joern Engel, Robert Jarzmik,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Kyungmin Park, Artem Bityutskiy, Solarflare linux maintainers,
	Edward Cree, Bert Kenward, Greg Kroah-Hartman, linuxppc-dev,
	netdev

On Tue, 13 Feb 2018 08:42:46 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Just a few comments about the form.
> 
> Otherwise:
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> 
> > diff --git a/drivers/mtd/devices/lart.c b/drivers/mtd/devices/lart.c
> > index 555b94406e0b..3d6c8ffd351f 100644
> > --- a/drivers/mtd/devices/lart.c
> > +++ b/drivers/mtd/devices/lart.c
> > @@ -415,7 +415,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
> >  	 {
> >  		if (!erase_block (addr))
> >  		  {
> > -			 instr->state = MTD_ERASE_FAILED;
> >  			 return (-EIO);
> >  		  }  
> 
> You can also safely remove these '{' '}'

Well, this patch is not about fixing coding style issues, otherwise I'd
have a lot more work on this driver :-)

> 
> >  
> > @@ -425,9 +424,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
> >  		if (addr == mtd->eraseregions[i].offset + (mtd->eraseregions[i].erasesize * mtd->eraseregions[i].numblocks)) i++;
> >  	 }
> >  
> > -   instr->state = MTD_ERASE_DONE;
> > -   mtd_erase_callback(instr);
> > -
> >     return (0);
> >  }
> >  
> > diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
> > index 5dc8bd042cc5..aaaeaae01e1d 100644
> > --- a/drivers/mtd/devices/mtd_dataflash.c
> > +++ b/drivers/mtd/devices/mtd_dataflash.c
> > @@ -220,10 +220,6 @@ static int dataflash_erase(struct mtd_info *mtd, struct erase_info *instr)
> >  	}
> >  	mutex_unlock(&priv->lock);
> >  
> > -	/* Inform MTD subsystem that erase is complete */
> > -	instr->state = MTD_ERASE_DONE;
> > -	mtd_erase_callback(instr);
> > -
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> > index 0bf4aeaf0cb8..efef43c6684b 100644
> > --- a/drivers/mtd/devices/mtdram.c
> > +++ b/drivers/mtd/devices/mtdram.c
> > @@ -60,8 +60,6 @@ static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
> >  	if (check_offs_len(mtd, instr->addr, instr->len))
> >  		return -EINVAL;
> >  	memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
> > -	instr->state = MTD_ERASE_DONE;
> > -	mtd_erase_callback(instr);  
> 
> Space ?

I could add a blank line, but again, I'm just following the coding style
in place in this file :-).

> 
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> > index 7287696a21f9..a963c88d392d 100644
> > --- a/drivers/mtd/devices/phram.c
> > +++ b/drivers/mtd/devices/phram.c
> > @@ -44,8 +44,6 @@ static int phram_erase(struct mtd_info *mtd, struct erase_info *instr)
> >  	 * I don't feel at all ashamed. This kind of thing is possible anyway
> >  	 * with flash, but unlikely.
> >  	 */  
> 
> Not sure this comment is still relevant? Maybe you could remove it
> or at least change it? 
> 
> > -	instr->state = MTD_ERASE_DONE;
> > -	mtd_erase_callback(instr);  
> 
> Space ?
> 
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
> > index cadea0620cd0..5d842cbca3de 100644
> > --- a/drivers/mtd/devices/pmc551.c
> > +++ b/drivers/mtd/devices/pmc551.c
> > @@ -184,12 +184,10 @@ static int pmc551_erase(struct mtd_info *mtd, struct erase_info *instr)
> >  	}
> >  
> >        out:
> > -	instr->state = MTD_ERASE_DONE;
> >  #ifdef CONFIG_MTD_PMC551_DEBUG
> >  	printk(KERN_DEBUG "pmc551_erase() done\n");
> >  #endif
> >  
> > -	mtd_erase_callback(instr);
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c
> > index 26f9feaa5d17..5f383630c16f 100644
> > --- a/drivers/mtd/devices/powernv_flash.c
> > +++ b/drivers/mtd/devices/powernv_flash.c
> > @@ -175,19 +175,12 @@ static int powernv_flash_erase(struct mtd_info *mtd, struct erase_info *erase)
> >  {
> >  	int rc;
> >  
> > -	erase->state = MTD_ERASING;
> > -
> >  	/* todo: register our own notifier to do a true async implementation */
> >  	rc =  powernv_flash_async_op(mtd, FLASH_OP_ERASE, erase->addr,
> >  			erase->len, NULL, NULL);  
> 
> Are you sure this is still needed? Maybe this should go away in your
> first patch?

Hm, indeed. This comment should be dropped.

> 
> > -
> > -	if (rc) {
> > +	if (rc)
> >  		erase->fail_addr = erase->addr;
> > -		erase->state = MTD_ERASE_FAILED;
> > -	} else {
> > -		erase->state = MTD_ERASE_DONE;
> > -	}
> > -	mtd_erase_callback(erase);
> > +
> >  	return rc;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
> > index 0ec85f316d24..2f05e1801047 100644
> > --- a/drivers/mtd/devices/slram.c
> > +++ b/drivers/mtd/devices/slram.c
> > @@ -88,8 +88,6 @@ static int slram_erase(struct mtd_info *mtd, struct erase_info *instr)
> >  	 * I don't feel at all ashamed. This kind of thing is possible anyway
> >  	 * with flash, but unlikely.
> >  	 */  
> 
> Same with this comment.

Actually, I'm not sure I understand that comment, but I guess it's
talking about races between read/write and erase paths, so nothing
related to the changed I'm doing here.

> 
> > -	instr->state = MTD_ERASE_DONE;
> > -	mtd_erase_callback(instr);  
> 
> Space ?
> 
> >  	return(0);
> >  }
> >    
> 
> 
> 
> 



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()
  2018-02-13  8:17     ` Boris Brezillon
@ 2018-02-13  8:33       ` Miquel Raynal
  0 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2018-02-13  8:33 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Benjamin Herrenschmidt, Artem Bityutskiy, Richard Weinberger,
	Robert Jarzmik, linuxppc-dev, Joern Engel, Greg Kroah-Hartman,
	Marek Vasut, Kyungmin Park, netdev, linux-mtd, Michael Ellerman,
	Cyrille Pitchen, Edward Cree, Paul Mackerras, Brian Norris,
	David Woodhouse

Hi Boris,

On Tue, 13 Feb 2018 09:17:14 +0100, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> On Tue, 13 Feb 2018 08:42:46 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Just a few comments about the form.
> > 
> > Otherwise:
> > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > 
> >   
> > > diff --git a/drivers/mtd/devices/lart.c b/drivers/mtd/devices/lart.c
> > > index 555b94406e0b..3d6c8ffd351f 100644
> > > --- a/drivers/mtd/devices/lart.c
> > > +++ b/drivers/mtd/devices/lart.c
> > > @@ -415,7 +415,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
> > >  	 {
> > >  		if (!erase_block (addr))
> > >  		  {
> > > -			 instr->state = MTD_ERASE_FAILED;
> > >  			 return (-EIO);
> > >  		  }    
> > 
> > You can also safely remove these '{' '}'  
> 
> Well, this patch is not about fixing coding style issues, otherwise I'd
> have a lot more work on this driver :-)

Sure, I was not referring to the weird style but just that you switch
from two to one line in the block, thus the braces are not needed
anymore.

> 
> >   
> > >  
> > > @@ -425,9 +424,6 @@ static int flash_erase (struct mtd_info *mtd,struct erase_info *instr)
> > >  		if (addr == mtd->eraseregions[i].offset + (mtd->eraseregions[i].erasesize * mtd->eraseregions[i].numblocks)) i++;
> > >  	 }
> > >  
> > > -   instr->state = MTD_ERASE_DONE;
> > > -   mtd_erase_callback(instr);
> > > -
> > >     return (0);
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
> > > index 5dc8bd042cc5..aaaeaae01e1d 100644
> > > --- a/drivers/mtd/devices/mtd_dataflash.c
> > > +++ b/drivers/mtd/devices/mtd_dataflash.c
> > > @@ -220,10 +220,6 @@ static int dataflash_erase(struct mtd_info *mtd, struct erase_info *instr)
> > >  	}
> > >  	mutex_unlock(&priv->lock);
> > >  
> > > -	/* Inform MTD subsystem that erase is complete */
> > > -	instr->state = MTD_ERASE_DONE;
> > > -	mtd_erase_callback(instr);
> > > -
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> > > index 0bf4aeaf0cb8..efef43c6684b 100644
> > > --- a/drivers/mtd/devices/mtdram.c
> > > +++ b/drivers/mtd/devices/mtdram.c
> > > @@ -60,8 +60,6 @@ static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
> > >  	if (check_offs_len(mtd, instr->addr, instr->len))
> > >  		return -EINVAL;
> > >  	memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
> > > -	instr->state = MTD_ERASE_DONE;
> > > -	mtd_erase_callback(instr);    
> > 
> > Space ?  
> 
> I could add a blank line, but again, I'm just following the coding style
> in place in this file :-).

Ok.

> 
> >   
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
> > > index 7287696a21f9..a963c88d392d 100644
> > > --- a/drivers/mtd/devices/phram.c
> > > +++ b/drivers/mtd/devices/phram.c
> > > @@ -44,8 +44,6 @@ static int phram_erase(struct mtd_info *mtd, struct erase_info *instr)
> > >  	 * I don't feel at all ashamed. This kind of thing is possible anyway
> > >  	 * with flash, but unlikely.
> > >  	 */    
> > 
> > Not sure this comment is still relevant? Maybe you could remove it
> > or at least change it? 
> >   
> > > -	instr->state = MTD_ERASE_DONE;
> > > -	mtd_erase_callback(instr);    
> > 
> > Space ?
> >   
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
> > > index cadea0620cd0..5d842cbca3de 100644
> > > --- a/drivers/mtd/devices/pmc551.c
> > > +++ b/drivers/mtd/devices/pmc551.c
> > > @@ -184,12 +184,10 @@ static int pmc551_erase(struct mtd_info *mtd, struct erase_info *instr)
> > >  	}
> > >  
> > >        out:
> > > -	instr->state = MTD_ERASE_DONE;
> > >  #ifdef CONFIG_MTD_PMC551_DEBUG
> > >  	printk(KERN_DEBUG "pmc551_erase() done\n");
> > >  #endif
> > >  
> > > -	mtd_erase_callback(instr);
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c
> > > index 26f9feaa5d17..5f383630c16f 100644
> > > --- a/drivers/mtd/devices/powernv_flash.c
> > > +++ b/drivers/mtd/devices/powernv_flash.c
> > > @@ -175,19 +175,12 @@ static int powernv_flash_erase(struct mtd_info *mtd, struct erase_info *erase)
> > >  {
> > >  	int rc;
> > >  
> > > -	erase->state = MTD_ERASING;
> > > -
> > >  	/* todo: register our own notifier to do a true async implementation */
> > >  	rc =  powernv_flash_async_op(mtd, FLASH_OP_ERASE, erase->addr,
> > >  			erase->len, NULL, NULL);    
> > 
> > Are you sure this is still needed? Maybe this should go away in your
> > first patch?  
> 
> Hm, indeed. This comment should be dropped.
> 
> >   
> > > -
> > > -	if (rc) {
> > > +	if (rc)
> > >  		erase->fail_addr = erase->addr;
> > > -		erase->state = MTD_ERASE_FAILED;
> > > -	} else {
> > > -		erase->state = MTD_ERASE_DONE;
> > > -	}
> > > -	mtd_erase_callback(erase);
> > > +
> > >  	return rc;
> > >  }
> > >  
> > > diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
> > > index 0ec85f316d24..2f05e1801047 100644
> > > --- a/drivers/mtd/devices/slram.c
> > > +++ b/drivers/mtd/devices/slram.c
> > > @@ -88,8 +88,6 @@ static int slram_erase(struct mtd_info *mtd, struct erase_info *instr)
> > >  	 * I don't feel at all ashamed. This kind of thing is possible anyway
> > >  	 * with flash, but unlikely.
> > >  	 */    
> > 
> > Same with this comment.  
> 
> Actually, I'm not sure I understand that comment, but I guess it's
> talking about races between read/write and erase paths, so nothing
> related to the changed I'm doing here.

I thought that races could only happen because of the asynchronous
manner how erases were handled, but if you feel this is still relevant,
I trust you :)

> 
> >   
> > > -	instr->state = MTD_ERASE_DONE;
> > > -	mtd_erase_callback(instr);    
> > 
> > Space ?
> >   
> > >  	return(0);
> > >  }
> > >      
> > 
> > 
> > 
> >   
> 
> 
> 



-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()
  2018-02-12 21:03 ` [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback() Boris Brezillon
  2018-02-12 22:17   ` Richard Weinberger
  2018-02-13  7:42   ` Miquel Raynal
@ 2018-02-13 12:09   ` Bert Kenward
  2 siblings, 0 replies; 16+ messages in thread
From: Bert Kenward @ 2018-02-13 12:09 UTC (permalink / raw)
  To: Boris Brezillon, David Woodhouse, Brian Norris, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: Joern Engel, Robert Jarzmik, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Kyungmin Park,
	Artem Bityutskiy, Solarflare linux maintainers, Edward Cree,
	Greg Kroah-Hartman, linuxppc-dev, netdev, devel, Miquel Raynal

On 12/02/18 21:03, Boris Brezillon wrote:
> MTD users are no longer checking erase_info->state to determine if the
> erase operation failed or succeeded. Moreover, mtd_erase_callback() is
> now a NOP.
> 
> We can safely get rid of all mtd_erase_callback() calls and all
> erase_info->state assignments. While at it, get rid of the
> erase_info->state field, all MTD_ERASE_XXX definitions and the
> mtd_erase_callback() function.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

For sfc parts:

Acked-by: Bert Kenward <bkenward@solarflare.com>


Thanks,

Bert.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()
  2018-02-12 21:03 ` [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase() Boris Brezillon
  2018-02-12 21:47   ` Richard Weinberger
@ 2018-03-18 21:22   ` Boris Brezillon
  1 sibling, 0 replies; 16+ messages in thread
From: Boris Brezillon @ 2018-03-18 21:22 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: devel, Bert Kenward, Solarflare linux maintainers,
	Artem Bityutskiy, Miquel Raynal, Michael Ellerman, linuxppc-dev,
	Joern Engel, Greg Kroah-Hartman, Kyungmin Park, netdev,
	Paul Mackerras, Benjamin Herrenschmidt, Edward Cree,
	Robert Jarzmik

On Mon, 12 Feb 2018 22:03:07 +0100
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> mtd_erase() can return an error before ->fail_addr is initialized to
> MTD_FAIL_ADDR_UNKNOWN. Move this initialization at the very beginning
> of the function.

Applied the patchset after addressing Miquel's comments.

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  drivers/mtd/mtdcore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index a1c94526fb88..c87859ff338b 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -953,6 +953,8 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
>   */
>  int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  {
> +	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> +
>  	if (!mtd->erasesize || !mtd->_erase)
>  		return -ENOTSUPP;
>  
> @@ -961,7 +963,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	if (!(mtd->flags & MTD_WRITEABLE))
>  		return -EROFS;
>  
> -	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>  	if (!instr->len) {
>  		instr->state = MTD_ERASE_DONE;
>  		mtd_erase_callback(instr);



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-03-18 21:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-12 21:03 [PATCH 0/5] mtd: Simplify erase handling Boris Brezillon
2018-02-12 21:03 ` [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase() Boris Brezillon
2018-02-12 21:47   ` Richard Weinberger
2018-03-18 21:22   ` Boris Brezillon
2018-02-12 21:03 ` [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info Boris Brezillon
2018-02-12 21:47   ` Richard Weinberger
2018-02-12 21:03 ` [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous Boris Brezillon
2018-02-12 21:58   ` Richard Weinberger
2018-02-12 21:03 ` [PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase() Boris Brezillon
2018-02-12 22:05   ` Richard Weinberger
2018-02-12 21:03 ` [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback() Boris Brezillon
2018-02-12 22:17   ` Richard Weinberger
2018-02-13  7:42   ` Miquel Raynal
2018-02-13  8:17     ` Boris Brezillon
2018-02-13  8:33       ` Miquel Raynal
2018-02-13 12:09   ` Bert Kenward

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).