LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: KVM compile error
From: Christian Zigotzky @ 2018-02-12 20:57 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, kvm-ppc@vger.kernel.org
In-Reply-To: <84E200BC-4202-4BAD-85C2-C8ED6E464FCC@xenosoft.de>

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

Hello Michael,

I compiled the RC1 of kernel 4.16 today. Unfortunately the issue with KVM still exists.
I get the error 'label out defined but not used' (see error messages below).

Link to the rc1 kernel config without KVM: http://www.xenosoft.de/cyrus-4.16-rc1.config

Link to the git kernel config with KVM: http://www.xenosoft.de/cyrus-4.16-alpha10.config

Thanks,
Christian

Sent from my iPhone

> On 12. Feb 2018, at 13:04, Christian Zigotzky <chzigotzky@xenosoft.de> wrote:
> 
> It‘s only an info. I tried to compile the latest git version yesterday and I got this error. I will try to compile the RC1 today and test if this error still exists.
> 
> Cheers,
> Christian
> 
> Sent from my iPhone
> 
>> On 12. Feb 2018, at 12:08, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> 
>> Christian Zigotzky <chzigotzky@xenosoft.de> writes:
>> 
>>> Just for info: KVM doesn’t compile currently.
>>> 
>>> Error messages:
>>> 
>>> CC      arch/powerpc/kvm/powerpc.o
>>> arch/powerpc/kvm/powerpc.c: In function 'kvm_arch_vcpu_ioctl_run':
>>> arch/powerpc/kvm/powerpc.c:1611:1: error: label 'out' defined but not used [-Werror=unused-label]
>>> out:
>>> ^
>>> cc1: all warnings being treated as errors
>> 
>> I don't see this, which compiler/config/commit is that?
>> 
>> cheers
> 

[-- Attachment #2: Type: text/html, Size: 3564 bytes --]

^ permalink raw reply

* [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info
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
In-Reply-To: <20180212210311.23244-1-boris.brezillon@bootlin.com>

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

* [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()
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
In-Reply-To: <20180212210311.23244-1-boris.brezillon@bootlin.com>

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

* [PATCH 0/5] mtd: Simplify erase handling
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

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

* [PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase()
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
In-Reply-To: <20180212210311.23244-1-boris.brezillon@bootlin.com>

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

* [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous
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
In-Reply-To: <20180212210311.23244-1-boris.brezillon@bootlin.com>

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

* [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()
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
In-Reply-To: <20180212210311.23244-1-boris.brezillon@bootlin.com>

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

* pata-macio WARNING at dmam_alloc_coherent+0xec/0x110
From: Meelis Roos @ 2018-02-12 20:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linuxppc-dev, iommu, Linux Kernel list

I tested 4.16-rc1 on my PowerMac G4 and got the following warning from 
macio pata driver. Since pata-macio has no recent changes, dma-mapping.h 
changes seem to be related.

[    0.228408] MacIO PCI driver attached to Keylargo chipset
[    1.283931] pata-macio 0.0001f000:ata-4: Activating pata-macio chipset KeyLargo ATA-4, Apple bus ID 2
[    1.284398] WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 dmam_alloc_coherent+0xec/0x110
[    1.284689] Modules linked in:
[    1.284797] CPU: 0 PID: 1 Comm: swapper Not tainted 4.16.0-rc1 #60
[    1.284991] NIP:  c03259ec LR: c0325948 CTR: 00000000
[    1.285150] REGS: ef047c10 TRAP: 0700   Not tainted  (4.16.0-rc1)
[    1.285337] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 24fff228  XER: 20000000
[    1.285559] 
               GPR00: c0325948 ef047cc0 ef048000 ef1321b0 00000000 00000000 ef1321bc 00000000 
               GPR08: 00000000 00000000 c04f1bd0 00000000 22fff884 00000000 c0004c80 00000000 
               GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 c0660000 c05f0960 
               GPR24: 00000007 c063d7a8 00000000 ef1e59ac 00001020 ef1321b0 ef135c18 014000c0 
[    1.303085] NIP [c03259ec] dmam_alloc_coherent+0xec/0x110
[    1.308751] LR [c0325948] dmam_alloc_coherent+0x48/0x110
[    1.314511] Call Trace:
[    1.320187] [ef047cc0] [c0325948] dmam_alloc_coherent+0x48/0x110 (unreliable)
[    1.326133] [ef047ce0] [c0370a90] pata_macio_port_start+0x44/0xb8
[    1.332110] [ef047d00] [c0355ed4] ata_host_start.part.5+0x138/0x254
[    1.338100] [ef047d30] [c035c1e8] ata_host_activate+0x84/0x1a0
[    1.344007] [ef047d50] [c0371214] pata_macio_common_init+0x3b0/0x608
[    1.349890] [ef047db0] [c0336f9c] macio_device_probe+0x60/0x120
[    1.355761] [ef047dd0] [c031868c] driver_probe_device+0x25c/0x35c
[    1.361576] [ef047e00] [c031887c] __driver_attach+0xf0/0xf4
[    1.367320] [ef047e20] [c0316340] bus_for_each_dev+0x80/0xc0
[    1.373051] [ef047e50] [c031782c] bus_add_driver+0x144/0x258
[    1.378805] [ef047e70] [c03190dc] driver_register+0x8c/0x140
[    1.384580] [ef047e80] [c060ce14] pata_macio_init+0x5c/0x8c
[    1.390303] [ef047ea0] [c0004aa0] do_one_initcall+0x48/0x18c
[    1.396000] [ef047f00] [c05f1214] kernel_init_freeable+0x12c/0x1ec
[    1.401615] [ef047f30] [c0004c98] kernel_init+0x18/0x128
[    1.407208] [ef047f40] [c00122e4] ret_from_kernel_thread+0x5c/0x64
[    1.412829] Instruction dump:
[    1.418409] 939d0000 4bff6329 80010024 7fe3fb78 8361000c 83810010 7c0803a6 83a10014
[    1.424201] 83c10018 83e1001c 38210020 4e800020 <0fe00000> 4bffff84 7fa3eb78 3be00000 
[    1.430020] ---[ end trace 89c0f4a91a110769 ]---


-- 
Meelis Roos (mroos@linux.ee)

^ permalink raw reply

* Re: [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()
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, Miquel Raynal
In-Reply-To: <20180212210311.23244-2-boris.brezillon@bootlin.com>

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

* Re: [PATCH 2/5] mtd: Get rid of unused fields in struct erase_info
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, Miquel Raynal
In-Reply-To: <20180212210311.23244-3-boris.brezillon@bootlin.com>

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

* Re: [PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous
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, Miquel Raynal
In-Reply-To: <20180212210311.23244-4-boris.brezillon@bootlin.com>

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

* [PATCH v6 00/17] ASoC: fsl_ssi: Clean up - program flow level
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel

[ The v6 just rebased the series and fixed the comments in probe().
  There is no need to re-test it except the uncovered tests listed
  at the end of this cover letter.
  
  Timur, these patches have been in the list for quite a long time
  and there has been no review for nearly a month since last ver.
  Would you please give a review and send an ack? Thanks a lot. ]

==Change log==
v6
 * Added Tested-by and Reviewed-by from Maciej.
 * Fixed one line of comments in the probe() suggested by Maciej.
 * Rebased all patches since there's a change of fsl_ssi merged
   recently.
v5
 * Reworked the series by taking suggestions from Maciej for AC97
   + Fixed SSI lockup issue by changing cleanup sequence in PATCH-13
   + Moved fsl_ssi_hw_clean() after unregistering the CODEC device
     in PATCH-13
   + Set NULL as the parent of CODEC platform device to fix a NULL
     pointer dereference bug in PATCH-16
 * Updated comments of three variables/pointers in struct fsl_ssi
   to describe them more accurately in PATCH-16
v4
 * Reworked the series by taking suggestions from Maciej
   + Added TXBIT0 bit back to play safe in PATCH-14
   + Made bool synchronous exclusive with AC97 mode in PATCH-16
v3
 * Reworked the series by taking suggestions from Maciej
   + Added PATCH-01 to make RX and TX more clearly defined
   + Replaced "bool dir" with "int dir" in PATCH-04
   + Replaced "!dir" with "int adir" in PATCH-05
   + Put CBM_CFS behind the baudclk check to keep the same
     program flow in PATCH-14
   + Removed all cpu_dai_drv changes in PATCH-15
v2
 * Reworked the series by taking suggestions from Maciej
   + Added PATCH-01 to keep all ssi->i2s_net updated
   + Replaced bool tx with bool dir in PATCH-03 and PATCH-06
   + Moved all initial register configurations from dai probe() to
     platform probe() so as to let AC97 CODEC successfully probe.
 * Added Tested-by from Caleb for TDM test cases.

==Background==
The fsl_ssi driver was designed for PPC originally and then it has
been updated to support different modes for i.MX Series, including
SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
contributors for different use cases in different coding styles.

Additionally, in order to fix/work-around hardware bugs and design
flaws, the driver made a lot of compromise so now its program flow
looks very complicated and it's getting hard to maintain or update.

So I am going to clean up the driver on both coding style level and
program flow level.

==Introduction==
This series of patches is the second set to clean up fsl_ssi driver
in the program flow level. Any patch here may impact a fundamental
test case like playback or record.

==Verification==
This series of patches require to be fully tested. I have done such
tests on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
 - Playback via I2S Master and Slave mode
 - Record via I2S Master and Slave mode
 - Simultaneous playback and record via I2S Master and Slave mode
 - Background playback with foreground record (starting at different
   time) via I2S Master and Slave mode
 - Background record with foreground playback (starting at different
   time) via I2S Master and Slave mode
 * All tests above by hacking offline_config to true in imx51.

Caleb has tested all versions with TDM lookback tests on i.MX6.

Maciej has tested v5 with AC97 tests on i.MX6.

Example of uncovered tests: PowerPC and FIQ.

Nicolin Chen (17):
  ASoC: fsl_ssi: Redefine RX and TX macros
  ASoC: fsl_ssi: Keep ssi->i2s_net updated
  ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
  ASoC: fsl_ssi: Maintain a mask of active streams
  ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
  ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
  ASoC: fsl_ssi: Clean up helper functions of trigger()
  ASoC: fsl_ssi: Add DAIFMT define for AC97
  ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
  ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
  ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
  ASoC: fsl_ssi: Move one-time configurations to probe()
  ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
  ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
  ASoC: fsl_ssi: Add bool synchronous to mark synchronous mode
  ASoC: fsl_ssi: Move DT related code to a separate probe()
  ASoC: fsl_ssi: Use ssi->streams instead of reading register

 sound/soc/fsl/fsl_ssi.c | 756 +++++++++++++++++++++++++-----------------------
 sound/soc/fsl/fsl_ssi.h |   3 -
 2 files changed, 395 insertions(+), 364 deletions(-)

-- 
2.1.4

^ permalink raw reply

* [PATCH v6 01/17] ASoC: fsl_ssi: Redefine RX and TX macros
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

The RX and TX macros were defined implicitly and there was
a potential risk if someone changes their values.

Since they were defined to index the array ssi->regvals[2],
this patch moves these two macros to fsl_ssi.c, closer to
its owner ssi->regvals. And it also puts some comments here
to limit their value within [0, 1].

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 4 ++++
 sound/soc/fsl/fsl_ssi.h | 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 24fb672..3c8dd60 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -56,6 +56,10 @@
 #include "fsl_ssi.h"
 #include "imx-pcm.h"
 
+/* Define RX and TX to index ssi->regvals array; Can be 0 or 1 only */
+#define RX 0
+#define TX 1
+
 /**
  * FSLSSI_I2S_FORMATS: audio formats supported by the SSI
  *
diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h
index de2fdc5..18f8dd5 100644
--- a/sound/soc/fsl/fsl_ssi.h
+++ b/sound/soc/fsl/fsl_ssi.h
@@ -12,9 +12,6 @@
 #ifndef _MPC8610_I2S_H
 #define _MPC8610_I2S_H
 
-#define RX 0
-#define TX 1
-
 /* -- SSI Register Map -- */
 
 /* SSI Transmit Data Register 0 */
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 02/17] ASoC: fsl_ssi: Keep ssi->i2s_net updated
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

The hw_params() overwrites i2s_net settings for special cases like
mono-channel support, however, it doesn't update ssi->i2s_net as
set_dai_fmt() does.

This patch removes the local i2s_net variable and directly updates
ssi->i2s_net in the hw_params() so that the driver can simply look
up the ssi->i2s_net instead of reading the register.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 3c8dd60..d4f1f0d 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -838,16 +838,16 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	if (!fsl_ssi_is_ac97(ssi)) {
-		u8 i2s_net;
 		/* Normal + Network mode to send 16-bit data in 32-bit frames */
 		if (fsl_ssi_is_i2s_cbm_cfs(ssi) && sample_size == 16)
-			i2s_net = SSI_SCR_I2S_MODE_NORMAL | SSI_SCR_NET;
-		else
-			i2s_net = ssi->i2s_net;
+			ssi->i2s_net = SSI_SCR_I2S_MODE_NORMAL | SSI_SCR_NET;
+
+		/* Use Normal mode to send mono data at 1st slot of 2 slots */
+		if (channels == 1)
+			ssi->i2s_net = SSI_SCR_I2S_MODE_NORMAL;
 
 		regmap_update_bits(regs, REG_SSI_SCR,
-				   SSI_SCR_I2S_NET_MASK,
-				   channels == 1 ? 0 : i2s_net);
+				   SSI_SCR_I2S_NET_MASK, ssi->i2s_net);
 	}
 
 	/* In synchronous mode, the SSI uses STCCR for capture */
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 03/17] ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

This patch replaces the register read with ssi->i2s_net for
simplification. It also removes masking SSIEN from scr value
since it's handled later by regmap_update_bits() to set this
scr value back.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index d4f1f0d..14046c3 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1055,9 +1055,7 @@ static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask,
 	}
 
 	/* The slot number should be >= 2 if using Network mode or I2S mode */
-	regmap_read(regs, REG_SSI_SCR, &val);
-	val &= SSI_SCR_I2S_MODE_MASK | SSI_SCR_NET;
-	if (val && slots < 2) {
+	if (ssi->i2s_net && slots < 2) {
 		dev_err(dai->dev, "slot number should be >= 2 in I2S or NET\n");
 		return -EINVAL;
 	}
@@ -1067,9 +1065,8 @@ static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask,
 	regmap_update_bits(regs, REG_SSI_SRCCR,
 			   SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(slots));
 
-	/* Save SSIEN bit of the SCR register */
+	/* Save the SCR register value */
 	regmap_read(regs, REG_SSI_SCR, &val);
-	val &= SSI_SCR_SSIEN;
 	/* Temporarily enable SSI to allow SxMSKs to be configurable */
 	regmap_update_bits(regs, REG_SSI_SCR, SSI_SCR_SSIEN, SSI_SCR_SSIEN);
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 04/17] ASoC: fsl_ssi: Maintain a mask of active streams
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

Checking TE and RE bits in SCR register doesn't work for AC97 mode
which enables SSIEN, TE and RE in the fsl_ssi_setup_ac97() that's
called during probe().

So when running into the trigger(), it will always get the result
of both TE and RE being enabled already, even if actually there is
no active stream.

This patch fixes this issue by adding a variable to log the active
streams manually.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 14046c3..b277a56 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -205,6 +205,7 @@ struct fsl_ssi_soc_data {
  * @cpu_dai_drv: CPU DAI driver for this device
  *
  * @dai_fmt: DAI configuration this device is currently used with
+ * @streams: Mask of current active streams: BIT(TX) and BIT(RX)
  * @i2s_net: I2S and Network mode configurations of SCR register
  * @use_dma: DMA is used or FIQ with stream filter
  * @use_dual_fifo: DMA with support for dual FIFO mode
@@ -249,6 +250,7 @@ struct fsl_ssi {
 	struct snd_soc_dai_driver cpu_dai_drv;
 
 	unsigned int dai_fmt;
+	u8 streams;
 	u8 i2s_net;
 	bool use_dma;
 	bool use_dual_fifo;
@@ -444,15 +446,14 @@ static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
 static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 			   struct fsl_ssi_regvals *vals)
 {
+	int dir = (&ssi->regvals[TX] == vals) ? TX : RX;
 	struct regmap *regs = ssi->regs;
 	struct fsl_ssi_regvals *avals;
 	int nr_active_streams;
-	u32 scr;
 	int keep_active;
 
-	regmap_read(regs, REG_SSI_SCR, &scr);
-
-	nr_active_streams = !!(scr & SSI_SCR_TE) + !!(scr & SSI_SCR_RE);
+	nr_active_streams = !!(ssi->streams & BIT(TX)) +
+			    !!(ssi->streams & BIT(RX));
 
 	if (nr_active_streams - 1 > 0)
 		keep_active = 1;
@@ -474,6 +475,9 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 					      keep_active);
 		/* Safely disable SCR register for the stream */
 		regmap_update_bits(regs, REG_SSI_SCR, scr, 0);
+
+		/* Log the disabled stream to the mask */
+		ssi->streams &= ~BIT(dir);
 	}
 
 	/*
@@ -549,6 +553,9 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 		}
 		/* Enable all remaining bits */
 		regmap_update_bits(regs, REG_SSI_SCR, vals->scr, vals->scr);
+
+		/* Log the enabled stream to the mask */
+		ssi->streams |= BIT(dir);
 	}
 }
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 05/17] ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

The define of fsl_ssi_disable_val is not so clear as it mixes two
steps of calculations together. And those parameter names are also
a bit long to read.

Since it just tries to exclude the shared bits from the regvals of
current stream while the opposite stream is active, it's better to
use something like ssi_excl_shared_bits.

This patch also bisects fsl_ssi_disable_val into two macros of two
corresponding steps and then shortens its parameter names. It also
updates callers in the fsl_ssi_config() accordingly.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 55 +++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index b277a56..0d8c800 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -421,24 +421,24 @@ static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
 }
 
 /**
- * Calculate the bits that have to be disabled for the current stream that is
- * getting disabled. This keeps the bits enabled that are necessary for the
- * second stream to work if 'stream_active' is true.
+ * Exclude bits that are used by the opposite stream
  *
- * Detailed calculation:
- * These are the values that need to be active after disabling. For non-active
- * second stream, this is 0:
- *	vals_stream * !!stream_active
+ * When both streams are active, disabling some bits for the current stream
+ * might break the other stream if these bits are used by it.
  *
- * The following computes the overall differences between the setup for the
- * to-disable stream and the active stream, a simple XOR:
- *	vals_disable ^ (vals_stream * !!(stream_active))
+ * @vals : regvals of the current stream
+ * @avals: regvals of the opposite stream
+ * @aactive: active state of the opposite stream
  *
- * The full expression adds a mask on all values we care about
+ *  1) XOR vals and avals to get the differences if the other stream is active;
+ *     Otherwise, return current vals if the other stream is not active
+ *  2) AND the result of 1) with the current vals
  */
-#define fsl_ssi_disable_val(vals_disable, vals_stream, stream_active) \
-	((vals_disable) & \
-	 ((vals_disable) ^ ((vals_stream) * (u32)!!(stream_active))))
+#define _ssi_xor_shared_bits(vals, avals, aactive) \
+	((vals) ^ ((avals) * (aactive)))
+
+#define ssi_excl_shared_bits(vals, avals, aactive) \
+	((vals) & _ssi_xor_shared_bits(vals, avals, aactive))
 
 /**
  * Enable or disable SSI configuration.
@@ -446,19 +446,14 @@ static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
 static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 			   struct fsl_ssi_regvals *vals)
 {
+	int adir = (&ssi->regvals[TX] == vals) ? RX : TX;
 	int dir = (&ssi->regvals[TX] == vals) ? TX : RX;
 	struct regmap *regs = ssi->regs;
 	struct fsl_ssi_regvals *avals;
-	int nr_active_streams;
-	int keep_active;
-
-	nr_active_streams = !!(ssi->streams & BIT(TX)) +
-			    !!(ssi->streams & BIT(RX));
+	bool aactive;
 
-	if (nr_active_streams - 1 > 0)
-		keep_active = 1;
-	else
-		keep_active = 0;
+	/* Check if the opposite stream is active */
+	aactive = ssi->streams & BIT(adir);
 
 	/* Get the opposite direction to keep its values untouched */
 	if (&ssi->regvals[RX] == vals)
@@ -471,8 +466,7 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 		 * To keep the other stream safe, exclude shared bits between
 		 * both streams, and get safe bits to disable current stream
 		 */
-		u32 scr = fsl_ssi_disable_val(vals->scr, avals->scr,
-					      keep_active);
+		u32 scr = ssi_excl_shared_bits(vals->scr, avals->scr, aactive);
 		/* Safely disable SCR register for the stream */
 		regmap_update_bits(regs, REG_SSI_SCR, scr, 0);
 
@@ -487,7 +481,7 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 	 * 2) Disable all remaining bits of both streams when last stream ends
 	 */
 	if (ssi->soc->offline_config) {
-		if ((enable && !nr_active_streams) || (!enable && !keep_active))
+		if ((enable && !ssi->streams) || (!enable && !aactive))
 			fsl_ssi_rxtx_config(ssi, enable);
 
 		goto config_done;
@@ -509,12 +503,9 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 		 * To keep the other stream safe, exclude shared bits between
 		 * both streams, and get safe bits to disable current stream
 		 */
-		sier = fsl_ssi_disable_val(vals->sier, avals->sier,
-					   keep_active);
-		srcr = fsl_ssi_disable_val(vals->srcr, avals->srcr,
-					   keep_active);
-		stcr = fsl_ssi_disable_val(vals->stcr, avals->stcr,
-					   keep_active);
+		sier = ssi_excl_shared_bits(vals->sier, avals->sier, aactive);
+		srcr = ssi_excl_shared_bits(vals->srcr, avals->srcr, aactive);
+		stcr = ssi_excl_shared_bits(vals->stcr, avals->stcr, aactive);
 
 		/* Safely disable other control registers for the stream */
 		regmap_update_bits(regs, REG_SSI_SRCR, srcr, 0);
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 06/17] ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

The FIFO clear helper function is just one line of code now.
So it could be cleaned up by removing it and calling regmap
directly.

Meanwhile, FIFO clear could be applied to all use cases, not
confined to AC97. So this patch also moves FIFO clear in the
trigger() to fsl_ssi_config() and removes the AC97 check.

Note that SOR register is safe from offline_config HW limit.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 0d8c800..d276b78 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -410,17 +410,6 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool enable)
 }
 
 /**
- * Clear remaining data in the FIFO to avoid dirty data or channel slipping
- */
-static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
-{
-	bool tx = !is_rx;
-
-	regmap_update_bits(ssi->regs, REG_SSI_SOR,
-			   SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
-}
-
-/**
  * Exclude bits that are used by the opposite stream
  *
  * When both streams are active, disabling some bits for the current stream
@@ -446,10 +435,11 @@ static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
 static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 			   struct fsl_ssi_regvals *vals)
 {
-	int adir = (&ssi->regvals[TX] == vals) ? RX : TX;
-	int dir = (&ssi->regvals[TX] == vals) ? TX : RX;
+	bool tx = &ssi->regvals[TX] == vals;
 	struct regmap *regs = ssi->regs;
 	struct fsl_ssi_regvals *avals;
+	int adir = tx ? RX : TX;
+	int dir = tx ? TX : RX;
 	bool aactive;
 
 	/* Check if the opposite stream is active */
@@ -489,7 +479,9 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 
 	/* Online configure single direction while SSI is running */
 	if (enable) {
-		fsl_ssi_fifo_clear(ssi, vals->scr & SSI_SCR_RE);
+		/* Clear FIFO to prevent dirty data or channel slipping */
+		regmap_update_bits(ssi->regs, REG_SSI_SOR,
+				   SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
 
 		regmap_update_bits(regs, REG_SSI_SRCR, vals->srcr, vals->srcr);
 		regmap_update_bits(regs, REG_SSI_STCR, vals->stcr, vals->stcr);
@@ -511,6 +503,10 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 		regmap_update_bits(regs, REG_SSI_SRCR, srcr, 0);
 		regmap_update_bits(regs, REG_SSI_STCR, stcr, 0);
 		regmap_update_bits(regs, REG_SSI_SIER, sier, 0);
+
+		/* Clear FIFO to prevent dirty data or channel slipping */
+		regmap_update_bits(ssi->regs, REG_SSI_SOR,
+				   SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
 	}
 
 config_done:
@@ -1091,7 +1087,6 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(rtd->cpu_dai);
-	struct regmap *regs = ssi->regs;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -1116,14 +1111,6 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 		return -EINVAL;
 	}
 
-	/* Clear corresponding FIFO */
-	if (fsl_ssi_is_ac97(ssi)) {
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			regmap_write(regs, REG_SSI_SOR, SSI_SOR_TX_CLR);
-		else
-			regmap_write(regs, REG_SSI_SOR, SSI_SOR_RX_CLR);
-	}
-
 	return 0;
 }
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 07/17] ASoC: fsl_ssi: Clean up helper functions of trigger()
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

The trigger() calls fsl_ssi_tx_config() and fsl_ssi_rx_config(),
and both of them jump to fsl_ssi_config(). And fsl_ssi_config()
later calls another fsl_ssi_rxtx_config().

However, the whole routine, especially fsl_ssi_config() function,
is too complicated because of the folowing reasons:
1) It has to handle the concern of the opposite stream.
2) It has to handle cases of offline configurations support.
3) It has to handle enable and disable operations while they're
   mostly different.

Since the enable and disable routines have more differences than
TX and RX rountines, this patch simplifies these helper functions
with the following changes:
- Changing to two helper functions of enable and disable instead
  of TX and RX.
- Removing fsl_ssi_rxtx_config() by separately integrating it to
  two newly introduced enable & disable functions.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 256 +++++++++++++++++++++++-------------------------
 1 file changed, 122 insertions(+), 134 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index d276b78..9f024a9 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -382,31 +382,83 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
 }
 
 /**
- * Enable or disable all rx/tx config flags at once
+ * Set SCR, SIER, STCR and SRCR registers with cached values in regvals
+ *
+ * Notes:
+ * 1) For offline_config SoCs, enable all necessary bits of both streams
+ *    when 1st stream starts, even if the opposite stream will not start
+ * 2) It also clears FIFO before setting regvals; SOR is safe to set online
  */
-static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool enable)
+static void fsl_ssi_config_enable(struct fsl_ssi *ssi, bool tx)
 {
-	struct regmap *regs = ssi->regs;
 	struct fsl_ssi_regvals *vals = ssi->regvals;
+	int dir = tx ? TX : RX;
+	u32 sier, srcr, stcr;
 
-	if (enable) {
-		regmap_update_bits(regs, REG_SSI_SIER,
-				   vals[RX].sier | vals[TX].sier,
-				   vals[RX].sier | vals[TX].sier);
-		regmap_update_bits(regs, REG_SSI_SRCR,
-				   vals[RX].srcr | vals[TX].srcr,
-				   vals[RX].srcr | vals[TX].srcr);
-		regmap_update_bits(regs, REG_SSI_STCR,
-				   vals[RX].stcr | vals[TX].stcr,
-				   vals[RX].stcr | vals[TX].stcr);
+	/* Clear dirty data in the FIFO; It also prevents channel slipping */
+	regmap_update_bits(ssi->regs, REG_SSI_SOR,
+			   SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
+
+	/*
+	 * On offline_config SoCs, SxCR and SIER are already configured when
+	 * the previous stream started. So skip all SxCR and SIER settings
+	 * to prevent online reconfigurations, then jump to set SCR directly
+	 */
+	if (ssi->soc->offline_config && ssi->streams)
+		goto enable_scr;
+
+	if (ssi->soc->offline_config) {
+		/*
+		 * Online reconfiguration not supported, so enable all bits for
+		 * both streams at once to avoid necessity of reconfigurations
+		 */
+		srcr = vals[RX].srcr | vals[TX].srcr;
+		stcr = vals[RX].stcr | vals[TX].stcr;
+		sier = vals[RX].sier | vals[TX].sier;
 	} else {
-		regmap_update_bits(regs, REG_SSI_SRCR,
-				   vals[RX].srcr | vals[TX].srcr, 0);
-		regmap_update_bits(regs, REG_SSI_STCR,
-				   vals[RX].stcr | vals[TX].stcr, 0);
-		regmap_update_bits(regs, REG_SSI_SIER,
-				   vals[RX].sier | vals[TX].sier, 0);
+		/* Otherwise, only set bits for the current stream */
+		srcr = vals[dir].srcr;
+		stcr = vals[dir].stcr;
+		sier = vals[dir].sier;
 	}
+
+	/* Configure SRCR, STCR and SIER at once */
+	regmap_update_bits(ssi->regs, REG_SSI_SRCR, srcr, srcr);
+	regmap_update_bits(ssi->regs, REG_SSI_STCR, stcr, stcr);
+	regmap_update_bits(ssi->regs, REG_SSI_SIER, sier, sier);
+
+enable_scr:
+	/*
+	 * Start DMA before setting TE to avoid FIFO underrun
+	 * which may cause a channel slip or a channel swap
+	 *
+	 * TODO: FIQ cases might also need this upon testing
+	 */
+	if (ssi->use_dma && tx) {
+		int try = 100;
+		u32 sfcsr;
+
+		/* Enable SSI first to send TX DMA request */
+		regmap_update_bits(ssi->regs, REG_SSI_SCR,
+				   SSI_SCR_SSIEN, SSI_SCR_SSIEN);
+
+		/* Busy wait until TX FIFO not empty -- DMA working */
+		do {
+			regmap_read(ssi->regs, REG_SSI_SFCSR, &sfcsr);
+			if (SSI_SFCSR_TFCNT0(sfcsr))
+				break;
+		} while (--try);
+
+		/* FIFO still empty -- something might be wrong */
+		if (!SSI_SFCSR_TFCNT0(sfcsr))
+			dev_warn(ssi->dev, "Timeout waiting TX FIFO filling\n");
+	}
+	/* Enable all remaining bits in SCR */
+	regmap_update_bits(ssi->regs, REG_SSI_SCR,
+			   vals[dir].scr, vals[dir].scr);
+
+	/* Log the enabled stream to the mask */
+	ssi->streams |= BIT(dir);
 }
 
 /**
@@ -430,14 +482,17 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool enable)
 	((vals) & _ssi_xor_shared_bits(vals, avals, aactive))
 
 /**
- * Enable or disable SSI configuration.
+ * Unset SCR, SIER, STCR and SRCR registers with cached values in regvals
+ *
+ * Notes:
+ * 1) For offline_config SoCs, to avoid online reconfigurations, disable all
+ *    bits of both streams at once when the last stream is abort to end
+ * 2) It also clears FIFO after unsetting regvals; SOR is safe to set online
  */
-static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
-			   struct fsl_ssi_regvals *vals)
+static void fsl_ssi_config_disable(struct fsl_ssi *ssi, bool tx)
 {
-	bool tx = &ssi->regvals[TX] == vals;
-	struct regmap *regs = ssi->regs;
-	struct fsl_ssi_regvals *avals;
+	struct fsl_ssi_regvals *vals, *avals;
+	u32 sier, srcr, stcr, scr;
 	int adir = tx ? RX : TX;
 	int dir = tx ? TX : RX;
 	bool aactive;
@@ -445,52 +500,36 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 	/* Check if the opposite stream is active */
 	aactive = ssi->streams & BIT(adir);
 
-	/* Get the opposite direction to keep its values untouched */
-	if (&ssi->regvals[RX] == vals)
-		avals = &ssi->regvals[TX];
-	else
-		avals = &ssi->regvals[RX];
+	vals = &ssi->regvals[dir];
 
-	if (!enable) {
-		/*
-		 * To keep the other stream safe, exclude shared bits between
-		 * both streams, and get safe bits to disable current stream
-		 */
-		u32 scr = ssi_excl_shared_bits(vals->scr, avals->scr, aactive);
-		/* Safely disable SCR register for the stream */
-		regmap_update_bits(regs, REG_SSI_SCR, scr, 0);
-
-		/* Log the disabled stream to the mask */
-		ssi->streams &= ~BIT(dir);
-	}
+	/* Get regvals of the opposite stream to keep opposite stream safe */
+	avals = &ssi->regvals[adir];
 
 	/*
-	 * For cases where online configuration is not supported,
-	 * 1) Enable all necessary bits of both streams when 1st stream starts
-	 *    even if the opposite stream will not start
-	 * 2) Disable all remaining bits of both streams when last stream ends
+	 * To keep the other stream safe, exclude shared bits between
+	 * both streams, and get safe bits to disable current stream
 	 */
-	if (ssi->soc->offline_config) {
-		if ((enable && !ssi->streams) || (!enable && !aactive))
-			fsl_ssi_rxtx_config(ssi, enable);
+	scr = ssi_excl_shared_bits(vals->scr, avals->scr, aactive);
 
-		goto config_done;
-	}
+	/* Disable safe bits of SCR register for the current stream */
+	regmap_update_bits(ssi->regs, REG_SSI_SCR, scr, 0);
 
-	/* Online configure single direction while SSI is running */
-	if (enable) {
-		/* Clear FIFO to prevent dirty data or channel slipping */
-		regmap_update_bits(ssi->regs, REG_SSI_SOR,
-				   SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
+	/* Log the disabled stream to the mask */
+	ssi->streams &= ~BIT(dir);
 
-		regmap_update_bits(regs, REG_SSI_SRCR, vals->srcr, vals->srcr);
-		regmap_update_bits(regs, REG_SSI_STCR, vals->stcr, vals->stcr);
-		regmap_update_bits(regs, REG_SSI_SIER, vals->sier, vals->sier);
-	} else {
-		u32 sier;
-		u32 srcr;
-		u32 stcr;
+	/*
+	 * On offline_config SoCs, if the other stream is active, skip
+	 * SxCR and SIER settings to prevent online reconfigurations
+	 */
+	if (ssi->soc->offline_config && aactive)
+		goto fifo_clear;
 
+	if (ssi->soc->offline_config) {
+		/* Now there is only current stream active, disable all bits */
+		srcr = vals->srcr | avals->srcr;
+		stcr = vals->stcr | avals->stcr;
+		sier = vals->sier | avals->sier;
+	} else {
 		/*
 		 * To keep the other stream safe, exclude shared bits between
 		 * both streams, and get safe bits to disable current stream
@@ -498,57 +537,17 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
 		sier = ssi_excl_shared_bits(vals->sier, avals->sier, aactive);
 		srcr = ssi_excl_shared_bits(vals->srcr, avals->srcr, aactive);
 		stcr = ssi_excl_shared_bits(vals->stcr, avals->stcr, aactive);
-
-		/* Safely disable other control registers for the stream */
-		regmap_update_bits(regs, REG_SSI_SRCR, srcr, 0);
-		regmap_update_bits(regs, REG_SSI_STCR, stcr, 0);
-		regmap_update_bits(regs, REG_SSI_SIER, sier, 0);
-
-		/* Clear FIFO to prevent dirty data or channel slipping */
-		regmap_update_bits(ssi->regs, REG_SSI_SOR,
-				   SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
 	}
 
-config_done:
-	/* Enabling of subunits is done after configuration */
-	if (enable) {
-		/*
-		 * Start DMA before setting TE to avoid FIFO underrun
-		 * which may cause a channel slip or a channel swap
-		 *
-		 * TODO: FIQ cases might also need this upon testing
-		 */
-		if (ssi->use_dma && (vals->scr & SSI_SCR_TE)) {
-			int i;
-			int max_loop = 100;
-
-			/* Enable SSI first to send TX DMA request */
-			regmap_update_bits(regs, REG_SSI_SCR,
-					   SSI_SCR_SSIEN, SSI_SCR_SSIEN);
-
-			/* Busy wait until TX FIFO not empty -- DMA working */
-			for (i = 0; i < max_loop; i++) {
-				u32 sfcsr;
-				regmap_read(regs, REG_SSI_SFCSR, &sfcsr);
-				if (SSI_SFCSR_TFCNT0(sfcsr))
-					break;
-			}
-			if (i == max_loop) {
-				dev_err(ssi->dev,
-					"Timeout waiting TX FIFO filling\n");
-			}
-		}
-		/* Enable all remaining bits */
-		regmap_update_bits(regs, REG_SSI_SCR, vals->scr, vals->scr);
-
-		/* Log the enabled stream to the mask */
-		ssi->streams |= BIT(dir);
-	}
-}
+	/* Clear configurations of SRCR, STCR and SIER at once */
+	regmap_update_bits(ssi->regs, REG_SSI_SRCR, srcr, 0);
+	regmap_update_bits(ssi->regs, REG_SSI_STCR, stcr, 0);
+	regmap_update_bits(ssi->regs, REG_SSI_SIER, sier, 0);
 
-static void fsl_ssi_rx_config(struct fsl_ssi *ssi, bool enable)
-{
-	fsl_ssi_config(ssi, enable, &ssi->regvals[RX]);
+fifo_clear:
+	/* Clear remaining data in the FIFO */
+	regmap_update_bits(ssi->regs, REG_SSI_SOR,
+			   SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
 }
 
 static void fsl_ssi_tx_ac97_saccst_setup(struct fsl_ssi *ssi)
@@ -564,21 +563,6 @@ static void fsl_ssi_tx_ac97_saccst_setup(struct fsl_ssi *ssi)
 	}
 }
 
-static void fsl_ssi_tx_config(struct fsl_ssi *ssi, bool enable)
-{
-	/*
-	 * SACCST might be modified via AC Link by a CODEC if it sends
-	 * extra bits in their SLOTREQ requests, which'll accidentally
-	 * send valid data to slots other than normal playback slots.
-	 *
-	 * To be safe, configure SACCST right before TX starts.
-	 */
-	if (enable && fsl_ssi_is_ac97(ssi))
-		fsl_ssi_tx_ac97_saccst_setup(ssi);
-
-	fsl_ssi_config(ssi, enable, &ssi->regvals[TX]);
-}
-
 /**
  * Cache critical bits of SIER, SRCR, STCR and SCR to later set them safely
  */
@@ -1087,24 +1071,28 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			fsl_ssi_tx_config(ssi, true);
-		else
-			fsl_ssi_rx_config(ssi, true);
+		/*
+		 * SACCST might be modified via AC Link by a CODEC if it sends
+		 * extra bits in their SLOTREQ requests, which'll accidentally
+		 * send valid data to slots other than normal playback slots.
+		 *
+		 * To be safe, configure SACCST right before TX starts.
+		 */
+		if (tx && fsl_ssi_is_ac97(ssi))
+			fsl_ssi_tx_ac97_saccst_setup(ssi);
+		fsl_ssi_config_enable(ssi, tx);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			fsl_ssi_tx_config(ssi, false);
-		else
-			fsl_ssi_rx_config(ssi, false);
+		fsl_ssi_config_disable(ssi, tx);
 		break;
 
 	default:
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 08/17] ASoC: fsl_ssi: Add DAIFMT define for AC97
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

The _fsl_ssi_set_dai_fmt() bypasses an undefined format for AC97
mode. However, it's not really necessary if AC97 has its complete
format defined.

So this patch adds a DAIFMT macro of complete format including a
clock direction and polarity.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 9f024a9..fc5768d 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -90,6 +90,16 @@
 	 SNDRV_PCM_FMTBIT_S24_LE)
 #endif
 
+/*
+ * In AC97 mode, TXDIR bit is forced to 0 and TFDIR bit is forced to 1:
+ *  - SSI inputs external bit clock and outputs frame sync clock -- CBM_CFS
+ *  - Also have NB_NF to mark these two clocks will not be inverted
+ */
+#define FSLSSI_AC97_DAIFMT \
+	(SND_SOC_DAIFMT_AC97 | \
+	 SND_SOC_DAIFMT_CBM_CFS | \
+	 SND_SOC_DAIFMT_NB_NF)
+
 #define FSLSSI_SIER_DBG_RX_FLAGS \
 	(SSI_SIER_RFF0_EN | \
 	 SSI_SIER_RLS_EN | \
@@ -964,8 +974,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 		scr &= ~SSI_SCR_SYS_CLK_EN;
 		break;
 	default:
-		if (!fsl_ssi_is_ac97(ssi))
-			return -EINVAL;
+		return -EINVAL;
 	}
 
 	stcr |= strcr;
@@ -1372,7 +1381,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	sprop = of_get_property(np, "fsl,mode", NULL);
 	if (sprop) {
 		if (!strcmp(sprop, "ac97-slave"))
-			ssi->dai_fmt = SND_SOC_DAIFMT_AC97;
+			ssi->dai_fmt = FSLSSI_AC97_DAIFMT;
 	}
 
 	/* Select DMA or FIQ */
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 09/17] ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

This patch cleans fsl_ssi_setup_regvals() by following changes:
1) Moving DBG bits to the first lines.
2) Setting SSIE, RE/TE as default and cleaning it for AC97

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index fc5768d..156f5132 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -580,18 +580,16 @@ static void fsl_ssi_setup_regvals(struct fsl_ssi *ssi)
 {
 	struct fsl_ssi_regvals *vals = ssi->regvals;
 
-	vals[RX].sier = SSI_SIER_RFF0_EN;
+	vals[RX].sier = SSI_SIER_RFF0_EN | FSLSSI_SIER_DBG_RX_FLAGS;
 	vals[RX].srcr = SSI_SRCR_RFEN0;
-	vals[RX].scr = 0;
-	vals[TX].sier = SSI_SIER_TFE0_EN;
+	vals[RX].scr = SSI_SCR_SSIEN | SSI_SCR_RE;
+	vals[TX].sier = SSI_SIER_TFE0_EN | FSLSSI_SIER_DBG_TX_FLAGS;
 	vals[TX].stcr = SSI_STCR_TFEN0;
-	vals[TX].scr = 0;
+	vals[TX].scr = SSI_SCR_SSIEN | SSI_SCR_TE;
 
 	/* AC97 has already enabled SSIEN, RE and TE, so ignore them */
-	if (!fsl_ssi_is_ac97(ssi)) {
-		vals[RX].scr = SSI_SCR_SSIEN | SSI_SCR_RE;
-		vals[TX].scr = SSI_SCR_SSIEN | SSI_SCR_TE;
-	}
+	if (fsl_ssi_is_ac97(ssi))
+		vals[RX].scr = vals[TX].scr = 0;
 
 	if (ssi->use_dma) {
 		vals[RX].sier |= SSI_SIER_RDMAE;
@@ -600,9 +598,6 @@ static void fsl_ssi_setup_regvals(struct fsl_ssi *ssi)
 		vals[RX].sier |= SSI_SIER_RIE;
 		vals[TX].sier |= SSI_SIER_TIE;
 	}
-
-	vals[RX].sier |= FSLSSI_SIER_DBG_RX_FLAGS;
-	vals[TX].sier |= FSLSSI_SIER_DBG_TX_FLAGS;
 }
 
 static void fsl_ssi_setup_ac97(struct fsl_ssi *ssi)
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 10/17] ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

It'd be safer to enable both FIFOs for TX or RX at the same time.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 156f5132..00dfdc7 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -591,6 +591,11 @@ static void fsl_ssi_setup_regvals(struct fsl_ssi *ssi)
 	if (fsl_ssi_is_ac97(ssi))
 		vals[RX].scr = vals[TX].scr = 0;
 
+	if (ssi->use_dual_fifo) {
+		vals[RX].srcr |= SSI_SRCR_RFEN1;
+		vals[TX].stcr |= SSI_STCR_TFEN1;
+	}
+
 	if (ssi->use_dma) {
 		vals[RX].sier |= SSI_SIER_RDMAE;
 		vals[TX].sier |= SSI_SIER_TDMAE;
@@ -991,14 +996,9 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 		     SSI_SFCSR_TFWM0(wm) | SSI_SFCSR_RFWM0(wm) |
 		     SSI_SFCSR_TFWM1(wm) | SSI_SFCSR_RFWM1(wm));
 
-	if (ssi->use_dual_fifo) {
-		regmap_update_bits(regs, REG_SSI_SRCR,
-				   SSI_SRCR_RFEN1, SSI_SRCR_RFEN1);
-		regmap_update_bits(regs, REG_SSI_STCR,
-				   SSI_STCR_TFEN1, SSI_STCR_TFEN1);
+	if (ssi->use_dual_fifo)
 		regmap_update_bits(regs, REG_SSI_SCR,
 				   SSI_SCR_TCH_EN, SSI_SCR_TCH_EN);
-	}
 
 	if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) == SND_SOC_DAIFMT_AC97)
 		fsl_ssi_setup_ac97(ssi);
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 11/17] ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

Since there is a helper function, use it to help readability.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 00dfdc7..7e15b30 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1110,10 +1110,9 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
 {
 	struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
 
-	if (ssi->soc->imx && ssi->use_dma) {
-		dai->playback_dma_data = &ssi->dma_params_tx;
-		dai->capture_dma_data = &ssi->dma_params_rx;
-	}
+	if (ssi->soc->imx && ssi->use_dma)
+		snd_soc_dai_init_dma_data(dai, &ssi->dma_params_tx,
+					  &ssi->dma_params_rx);
 
 	return 0;
 }
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 12/17] ASoC: fsl_ssi: Move one-time configurations to probe()
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

The probe() could handle some one-time configurations since
they will not be changed once being configured.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 7e15b30..36f3d51 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -865,7 +865,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 {
 	struct regmap *regs = ssi->regs;
 	u32 strcr = 0, stcr, srcr, scr, mask;
-	u8 wm;
 
 	ssi->dai_fmt = fmt;
 
@@ -874,8 +873,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 		return -EINVAL;
 	}
 
-	fsl_ssi_setup_regvals(ssi);
-
 	regmap_read(regs, REG_SSI_SCR, &scr);
 	scr &= ~(SSI_SCR_SYN | SSI_SCR_I2S_MODE_MASK);
 	/* Synchronize frame sync clock for TE to avoid data slipping */
@@ -990,16 +987,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 	regmap_write(regs, REG_SSI_SRCR, srcr);
 	regmap_write(regs, REG_SSI_SCR, scr);
 
-	wm = ssi->fifo_watermark;
-
-	regmap_write(regs, REG_SSI_SFCSR,
-		     SSI_SFCSR_TFWM0(wm) | SSI_SFCSR_RFWM0(wm) |
-		     SSI_SFCSR_TFWM1(wm) | SSI_SFCSR_RFWM1(wm));
-
-	if (ssi->use_dual_fifo)
-		regmap_update_bits(regs, REG_SSI_SCR,
-				   SSI_SCR_TCH_EN, SSI_SCR_TCH_EN);
-
 	if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) == SND_SOC_DAIFMT_AC97)
 		fsl_ssi_setup_ac97(ssi);
 
@@ -1249,6 +1236,29 @@ static struct snd_ac97_bus_ops fsl_ssi_ac97_ops = {
 };
 
 /**
+ * Initialize SSI registers
+ */
+static int fsl_ssi_hw_init(struct fsl_ssi *ssi)
+{
+	u32 wm = ssi->fifo_watermark;
+
+	/* Initialize regvals */
+	fsl_ssi_setup_regvals(ssi);
+
+	/* Set watermarks */
+	regmap_write(ssi->regs, REG_SSI_SFCSR,
+		     SSI_SFCSR_TFWM0(wm) | SSI_SFCSR_RFWM0(wm) |
+		     SSI_SFCSR_TFWM1(wm) | SSI_SFCSR_RFWM1(wm));
+
+	/* Enable Dual FIFO mode */
+	if (ssi->use_dual_fifo)
+		regmap_update_bits(ssi->regs, REG_SSI_SCR,
+				   SSI_SCR_TCH_EN, SSI_SCR_TCH_EN);
+
+	return 0;
+}
+
+/**
  * Make every character in a string lower-case
  */
 static void make_lowercase(char *s)
@@ -1533,6 +1543,9 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	if (ssi->dai_fmt)
 		_fsl_ssi_set_dai_fmt(dev, ssi, ssi->dai_fmt);
 
+	/* Initially configures SSI registers */
+	fsl_ssi_hw_init(ssi);
+
 	if (fsl_ssi_is_ac97(ssi)) {
 		u32 ssi_idx;
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v6 13/17] ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
From: Nicolin Chen @ 2018-02-12 22:03 UTC (permalink / raw)
  To: broonie, timur
  Cc: caleb, mail, fabio.estevam, alsa-devel, linuxppc-dev,
	linux-kernel, lgirdwood, arnaud.mouiche, lukma, kernel
In-Reply-To: <1518473005-14090-1-git-send-email-nicoleotsuka@gmail.com>

AC97 configures most of registers earlier to start a communication
with CODECs in order to successfully initialize CODEC. Currently,
_fsl_ssi_set_dai_fmt() and fsl_ssi_setup_ac97() are called to get
all SSI registers properly set.

Since now the driver has a fsl_ssi_hw_init() to handle all register
initial settings, this patch moves those register settings of AC97
to the fsl_ssi_hw_init() as well.

Meanwhile it applies _fsl_ssi_set_dai_fmt() call to AC97 only since
other formats would be configured via normal set_dai_fmt() directly.

This patch also adds fsl_ssi_hw_clean() to cleanup control bits for
AC97 in the platform remote() function.

Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Tested-by: Caleb Crome <caleb@crome.org>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 sound/soc/fsl/fsl_ssi.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 36f3d51..dfb0da3 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -987,9 +987,6 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
 	regmap_write(regs, REG_SSI_SRCR, srcr);
 	regmap_write(regs, REG_SSI_SCR, scr);
 
-	if ((fmt & SND_SOC_DAIFMT_FORMAT_MASK) == SND_SOC_DAIFMT_AC97)
-		fsl_ssi_setup_ac97(ssi);
-
 	return 0;
 }
 
@@ -1255,10 +1252,34 @@ static int fsl_ssi_hw_init(struct fsl_ssi *ssi)
 		regmap_update_bits(ssi->regs, REG_SSI_SCR,
 				   SSI_SCR_TCH_EN, SSI_SCR_TCH_EN);
 
+	/* AC97 should start earlier to communicate with CODECs */
+	if (fsl_ssi_is_ac97(ssi)) {
+		_fsl_ssi_set_dai_fmt(ssi->dev, ssi, ssi->dai_fmt);
+		fsl_ssi_setup_ac97(ssi);
+	}
+
 	return 0;
 }
 
 /**
+ * Clear SSI registers
+ */
+static void fsl_ssi_hw_clean(struct fsl_ssi *ssi)
+{
+	/* Disable registers for AC97 */
+	if (fsl_ssi_is_ac97(ssi)) {
+		/* Disable TE and RE bits first */
+		regmap_update_bits(ssi->regs, REG_SSI_SCR,
+				   SSI_SCR_TE | SSI_SCR_RE, 0);
+		/* Disable AC97 mode */
+		regmap_write(ssi->regs, REG_SSI_SACNT, 0);
+		/* Unset WAIT bits */
+		regmap_write(ssi->regs, REG_SSI_SOR, 0);
+		/* Disable SSI -- software reset */
+		regmap_update_bits(ssi->regs, REG_SSI_SCR, SSI_SCR_SSIEN, 0);
+	}
+}
+/**
  * Make every character in a string lower-case
  */
 static void make_lowercase(char *s)
@@ -1540,9 +1561,6 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 	}
 
 done:
-	if (ssi->dai_fmt)
-		_fsl_ssi_set_dai_fmt(dev, ssi, ssi->dai_fmt);
-
 	/* Initially configures SSI registers */
 	fsl_ssi_hw_init(ssi);
 
@@ -1592,6 +1610,9 @@ static int fsl_ssi_remove(struct platform_device *pdev)
 	if (ssi->pdev)
 		platform_device_unregister(ssi->pdev);
 
+	/* Clean up SSI registers */
+	fsl_ssi_hw_clean(ssi);
+
 	if (ssi->soc->imx)
 		fsl_ssi_imx_clean(pdev, ssi);
 
-- 
2.1.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox