[PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

I am working on this IP for Socionext SoCs.

I was hit by several issues, and noticed various
clean-up candidates.

 - Fix and clean-up Kconfig
 - Fix various card detection problems
 - Move Renesas private data out of TMIO core
 - Allow to perform platform-specific settings before MMC host starts
 - Fix weird IRQ handling

I am getting more and more patches for TMIO.
I put all in a single series to clarify the patch order.

1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.


Masahiro Yamada (22):
  mmc: renesas_sdhi: consolidate DMAC CONFIG options
  mmc: renesas_sdhi: remove wrong depends on to enable compile test
  mmc: renesas_sdhi: remove eprobe jump label
  mmc: tmio: set tmio_mmc_host to driver data
  mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
  mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
  mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
  mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
  mmc: tmio: use mmc_can_gpio_cd() instead of checking
    TMIO_MMC_USE_GPIO_CD
  mmc: tmio: support IP-builtin card detection logic
  mmc: renesas_sdhi: remove always false condition
  mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
  mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
    renesas_sdhi.h
  mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
  mmc: tmio: change bus_shift to unsigned int
  mmc: tmio: fix never-detected card insertion bug
  mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
  mmc: tmio: remove useless TMIO_MASK_CMD handling in
    tmio_mmc_host_probe()
  mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
  mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
  mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
  mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument

 drivers/mmc/host/Kconfig                      |   5 +-
 drivers/mmc/host/Makefile                     |   8 +-
 drivers/mmc/host/renesas_sdhi.h               |  22 ++++
 drivers/mmc/host/renesas_sdhi_core.c          |  49 ++++-----
 drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  35 +++---
 drivers/mmc/host/tmio_mmc.c                   |  23 ++--
 drivers/mmc/host/tmio_mmc.h                   |  23 +---
 drivers/mmc/host/tmio_mmc_core.c              | 149 +++++++++++++-------------
 9 files changed, 170 insertions(+), 158 deletions(-)

-- 
2.7.4

[PATCH v2 01/22] mmc: renesas_sdhi: consolidate DMAC CONFIG options

[Masahiro Yamada]

The description in the Makefile is odd.  Fix the CONFIG selection
in a cleaner way.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes in v2:
  - Fix a typo consilidate -> consolidate

 drivers/mmc/host/Kconfig  | 4 ++--
 drivers/mmc/host/Makefile | 8 ++------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 567028c..1932188 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -598,8 +598,6 @@ config MMC_SDHI
 	depends on SUPERH || ARM || ARM64
 	depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
 	select MMC_TMIO_CORE
-	select MMC_SDHI_SYS_DMAC if (SUPERH || ARM)
-	select MMC_SDHI_INTERNAL_DMAC if ARM64
 	help
 	  This provides support for the SDHI SD/SDIO controller found in
 	  Renesas SuperH, ARM and ARM64 based SoCs
@@ -607,6 +605,7 @@ config MMC_SDHI
 config MMC_SDHI_SYS_DMAC
 	tristate "DMA for SDHI SD/SDIO controllers using SYS-DMAC"
 	depends on MMC_SDHI
+	default MMC_SDHI if (SUPERH || ARM)
 	help
 	  This provides DMA support for SDHI SD/SDIO controllers
 	  using SYS-DMAC via DMA Engine. This supports the controllers
@@ -616,6 +615,7 @@ config MMC_SDHI_INTERNAL_DMAC
 	tristate "DMA for SDHI SD/SDIO controllers using on-chip bus mastering"
 	depends on ARM64 || COMPILE_TEST
 	depends on MMC_SDHI
+	default MMC_SDHI if ARM64
 	help
 	  This provides DMA support for SDHI SD/SDIO controllers
 	  using on-chip bus mastering. This supports the controllers
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index a43cf0d..559bd9e 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -39,12 +39,8 @@ obj-$(CONFIG_MMC_SDRICOH_CS)	+= sdricoh_cs.o
 obj-$(CONFIG_MMC_TMIO)		+= tmio_mmc.o
 obj-$(CONFIG_MMC_TMIO_CORE)	+= tmio_mmc_core.o
 obj-$(CONFIG_MMC_SDHI)		+= renesas_sdhi_core.o
-ifeq ($(subst m,y,$(CONFIG_MMC_SDHI_SYS_DMAC)),y)
-obj-$(CONFIG_MMC_SDHI)		+= renesas_sdhi_sys_dmac.o
-endif
-ifeq ($(subst m,y,$(CONFIG_MMC_SDHI_INTERNAL_DMAC)),y)
-obj-$(CONFIG_MMC_SDHI)		+= renesas_sdhi_internal_dmac.o
-endif
+obj-$(CONFIG_MMC_SDHI_SYS_DMAC)		+= renesas_sdhi_sys_dmac.o
+obj-$(CONFIG_MMC_SDHI_INTERNAL_DMAC)	+= renesas_sdhi_internal_dmac.o
 obj-$(CONFIG_MMC_CB710)		+= cb710-mmc.o
 obj-$(CONFIG_MMC_VIA_SDMMC)	+= via-sdmmc.o
 obj-$(CONFIG_SDH_BFIN)		+= bfin_sdh.o
-- 
2.7.4

[PATCH v2 02/22] mmc: renesas_sdhi: remove wrong depends on to enable compile test

[Masahiro Yamada]

ARCH_RENESAS is a stronger condition than (ARM || ARM64).
If ARCH_RENESAS is enabled, (ARM || ARM64) is met as well.

What is worse, the first depends on line prevents COMPILE_TEST from
enabling this driver.  It should be removed.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes in v2:
  - Fix a typo:  renesas_sdhc -> renesas_sdhi

 drivers/mmc/host/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1932188..ac25354 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -595,7 +595,6 @@ config MMC_TMIO
 
 config MMC_SDHI
 	tristate "Renesas SDHI SD/SDIO controller support"
-	depends on SUPERH || ARM || ARM64
 	depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
 	select MMC_TMIO_CORE
 	help
-- 
2.7.4

Re: [PATCH v2 02/22] mmc: renesas_sdhi: remove wrong depends on to enable compile test

[Geert Uytterhoeven]

On Fri, Nov 24, 2017 at 5:24 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> ARCH_RENESAS is a stronger condition than (ARM || ARM64).
> If ARCH_RENESAS is enabled, (ARM || ARM64) is met as well.
>
> What is worse, the first depends on line prevents COMPILE_TEST from
> enabling this driver.  It should be removed.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 02/22] mmc: renesas_sdhi: remove wrong depends on to enable compile test

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:37AM +0900, Masahiro Yamada wrote:
> ARCH_RENESAS is a stronger condition than (ARM || ARM64).
> If ARCH_RENESAS is enabled, (ARM || ARM64) is met as well.
> 
> What is worse, the first depends on line prevents COMPILE_TEST from
> enabling this driver.  It should be removed.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Because COMPILE_TEST now works, buildbot spotted something:

https://www.spinics.net/lists/linux-i2c/msg32556.html

Likely, we should include linux/io.h in the header?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 02/22] mmc: renesas_sdhi: remove wrong depends on to enable compile test

[Masahiro Yamada]

2017-12-05 0:25 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
> On Sat, Nov 25, 2017 at 01:24:37AM +0900, Masahiro Yamada wrote:
>> ARCH_RENESAS is a stronger condition than (ARM || ARM64).
>> If ARCH_RENESAS is enabled, (ARM || ARM64) is met as well.
>>
>> What is worse, the first depends on line prevents COMPILE_TEST from
>> enabling this driver.  It should be removed.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Because COMPILE_TEST now works, buildbot spotted something:
>
> https://www.spinics.net/lists/linux-i2c/msg32556.html
>
> Likely, we should include linux/io.h in the header?
>

Yes.  Can you insert a patch before this one?


-- 
Best Regards
Masahiro Yamada

[PATCH v2 03/22] mmc: renesas_sdhi: remove eprobe jump label

[Masahiro Yamada]

"goto eprobe" does nothing.  Return directly.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Fix a typo : renesas_sdhc -> renesas_sdhi
  - Restore the "ret" printout for devm_clk_get()

 drivers/mmc/host/renesas_sdhi_core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index fcf7235..0590ae0 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -497,7 +497,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (IS_ERR(priv->clk)) {
 		ret = PTR_ERR(priv->clk);
 		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
-		goto eprobe;
+		return ret;
 	}
 
 	/*
@@ -524,10 +524,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	}
 
 	host = tmio_mmc_host_alloc(pdev);
-	if (!host) {
-		ret = -ENOMEM;
-		goto eprobe;
-	}
+	if (!host)
+		return -ENOMEM;
 
 	if (of_data) {
 		mmc_data->flags |= of_data->tmio_flags;
@@ -652,7 +650,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	tmio_mmc_host_remove(host);
 efree:
 	tmio_mmc_host_free(host);
-eprobe:
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(renesas_sdhi_probe);
-- 
2.7.4

Re: [PATCH v2 03/22] mmc: renesas_sdhi: remove eprobe jump label

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:38AM +0900, Masahiro Yamada wrote:
> "goto eprobe" does nothing.  Return directly.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[PATCH v2 04/22] mmc: tmio: set tmio_mmc_host to driver data

[Masahiro Yamada]

The remove, suspend, resume hooks need to get tmio_mmc_host.  It is
tedious to call mmc_priv() to convert mmc_host to tmio_mmc_host.
We can directly set tmio_mmc_host to driver data.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes in v2: None

 drivers/mmc/host/renesas_sdhi_core.c |  3 +--
 drivers/mmc/host/tmio_mmc.c          | 12 ++++--------
 drivers/mmc/host/tmio_mmc_core.c     |  8 +++-----
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 0590ae0..9baf4d1 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -657,8 +657,7 @@ EXPORT_SYMBOL_GPL(renesas_sdhi_probe);
 
 int renesas_sdhi_remove(struct platform_device *pdev)
 {
-	struct mmc_host *mmc = platform_get_drvdata(pdev);
-	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct tmio_mmc_host *host = platform_get_drvdata(pdev);
 
 	tmio_mmc_host_remove(host);
 
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index 64b7e9f..ccfbc15 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -128,15 +128,11 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 static int tmio_mmc_remove(struct platform_device *pdev)
 {
 	const struct mfd_cell *cell = mfd_get_cell(pdev);
-	struct mmc_host *mmc = platform_get_drvdata(pdev);
+	struct tmio_mmc_host *host = platform_get_drvdata(pdev);
 
-	if (mmc) {
-		struct tmio_mmc_host *host = mmc_priv(mmc);
-
-		tmio_mmc_host_remove(host);
-		if (cell->disable)
-			cell->disable(pdev);
-	}
+	tmio_mmc_host_remove(host);
+	if (cell->disable)
+		cell->disable(pdev);
 
 	return 0;
 }
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 583bf32..12df338 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1193,7 +1193,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 		return ret;
 
 	_host->pdata = pdata;
-	platform_set_drvdata(pdev, mmc);
+	platform_set_drvdata(pdev, _host);
 
 	_host->set_pwr = pdata->set_pwr;
 	_host->set_clk_div = pdata->set_clk_div;
@@ -1351,8 +1351,7 @@ EXPORT_SYMBOL_GPL(tmio_mmc_host_remove);
 #ifdef CONFIG_PM
 int tmio_mmc_host_runtime_suspend(struct device *dev)
 {
-	struct mmc_host *mmc = dev_get_drvdata(dev);
-	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
 	tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);
 
@@ -1372,8 +1371,7 @@ static bool tmio_mmc_can_retune(struct tmio_mmc_host *host)
 
 int tmio_mmc_host_runtime_resume(struct device *dev)
 {
-	struct mmc_host *mmc = dev_get_drvdata(dev);
-	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
 	tmio_mmc_reset(host);
 	tmio_mmc_clk_enable(host);
-- 
2.7.4

[PATCH v2 05/22] mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()

[Masahiro Yamada]

The TMIO core misses to call request_mem_region().
devm_ioremap_resource() takes care of it and makes the code cleaner.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes in v2: None

 drivers/mmc/host/tmio_mmc_core.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 12df338..d3b7af9 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1185,8 +1185,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 		_host->write16_hook = NULL;
 
 	res_ctl = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res_ctl)
-		return -EINVAL;
+	_host->ctl = devm_ioremap_resource(&pdev->dev, res_ctl);
+	if (IS_ERR(_host->ctl))
+		return PTR_ERR(_host->ctl);
 
 	ret = mmc_of_parse(mmc);
 	if (ret < 0)
@@ -1202,11 +1203,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (ret < 0)
 		return ret;
 
-	_host->ctl = devm_ioremap(&pdev->dev,
-				  res_ctl->start, resource_size(res_ctl));
-	if (!_host->ctl)
-		return -ENOMEM;
-
 	tmio_mmc_ops.card_busy = _host->card_busy;
 	tmio_mmc_ops.start_signal_voltage_switch =
 		_host->start_signal_voltage_switch;
-- 
2.7.4

[PATCH v2 06/22] mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data

[Masahiro Yamada]

Currently, tmio_mmc_ops is static data and tmio_mmc_host_probe()
updates some hooks in the static data.  This is a problem when
two or more instances call tmio_mmc_host_probe() and each of them
requests to use its own card_busy/start_signal_voltage_switch.

We can borrow a solution from sdhci_alloc_host().  Copy the whole
ops structure to host->mmc_host_ops, then override the hooks in
malloc'ed data.  Constify tmio_mmc_ops since it is now a template
ops used by default.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes in v2:
  - Rename "mmc_host_ops" to "ops" per Wolfram Sang

 drivers/mmc/host/tmio_mmc.h      | 1 +
 drivers/mmc/host/tmio_mmc_core.c | 9 +++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 3e6ff89..4fdb51b 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -134,6 +134,7 @@ struct tmio_mmc_host {
 	struct mmc_request      *mrq;
 	struct mmc_data         *data;
 	struct mmc_host         *mmc;
+	struct mmc_host_ops     ops;
 
 	/* Callbacks for clock / power control */
 	void (*set_pwr)(struct platform_device *host, int state);
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index d3b7af9..af9ed6a 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1098,7 +1098,7 @@ static int tmio_multi_io_quirk(struct mmc_card *card,
 	return blk_size;
 }
 
-static struct mmc_host_ops tmio_mmc_ops = {
+static const struct mmc_host_ops tmio_mmc_ops = {
 	.request	= tmio_mmc_request,
 	.set_ios	= tmio_mmc_set_ios,
 	.get_ro         = tmio_mmc_get_ro,
@@ -1158,6 +1158,8 @@ tmio_mmc_host_alloc(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
 	host->pdev = pdev;
+	host->ops = tmio_mmc_ops;
+	mmc->ops = &host->ops;
 
 	return host;
 }
@@ -1203,10 +1205,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (ret < 0)
 		return ret;
 
-	tmio_mmc_ops.card_busy = _host->card_busy;
-	tmio_mmc_ops.start_signal_voltage_switch =
+	_host->ops.card_busy = _host->card_busy;
+	_host->ops.start_signal_voltage_switch =
 		_host->start_signal_voltage_switch;
-	mmc->ops = &tmio_mmc_ops;
 
 	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->caps2 |= pdata->capabilities2;
-- 
2.7.4

[PATCH v2 07/22] mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly

[Masahiro Yamada]

Drivers can set any mmc_host_ops hooks between tmio_mmc_host_alloc()
and tmio_mmc_host_probe().  Remove duplicated hooks in tmio_mmc_host.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Changes in v2:
  - Rename "mmc_host_ops" to "ops" per Wolfram Sang

 drivers/mmc/host/renesas_sdhi_core.c | 4 ++--
 drivers/mmc/host/tmio_mmc.h          | 3 ---
 drivers/mmc/host/tmio_mmc_core.c     | 4 ----
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 9baf4d1..267e2e0 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -549,8 +549,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	/* SDR speeds are only available on Gen2+ */
 	if (mmc_data->flags & TMIO_MMC_MIN_RCAR2) {
 		/* card_busy caused issues on r8a73a4 (pre-Gen2) CD-less SDHI */
-		host->card_busy	= renesas_sdhi_card_busy;
-		host->start_signal_voltage_switch =
+		host->ops.card_busy = renesas_sdhi_card_busy;
+		host->ops.start_signal_voltage_switch =
 			renesas_sdhi_start_signal_voltage_switch;
 	}
 
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 4fdb51b..37409c5 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -186,9 +186,6 @@ struct tmio_mmc_host {
 	void (*clk_disable)(struct tmio_mmc_host *host);
 	int (*multi_io_quirk)(struct mmc_card *card,
 			      unsigned int direction, int blk_size);
-	int (*card_busy)(struct mmc_host *mmc);
-	int (*start_signal_voltage_switch)(struct mmc_host *mmc,
-					   struct mmc_ios *ios);
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 	void (*hw_reset)(struct tmio_mmc_host *host);
 	void (*prepare_tuning)(struct tmio_mmc_host *host, unsigned long tap);
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index af9ed6a..46f77c3 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1205,10 +1205,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (ret < 0)
 		return ret;
 
-	_host->ops.card_busy = _host->card_busy;
-	_host->ops.start_signal_voltage_switch =
-		_host->start_signal_voltage_switch;
-
 	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->caps2 |= pdata->capabilities2;
 	mmc->max_segs = pdata->max_segs ? : 32;
-- 
2.7.4

[PATCH v2 08/22] mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()

[Masahiro Yamada]

Drivers do not need to call mmc_gpiod_request_cd_irq() explicitly
because mmc_start_host() calls it.  To make it work, cd_gpio must
be set before mmc_add_host().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/tmio_mmc_core.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 46f77c3..efffb04 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1205,6 +1205,12 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (ret < 0)
 		return ret;
 
+	if (pdata->flags & TMIO_MMC_USE_GPIO_CD) {
+		ret = mmc_gpio_request_cd(mmc, pdata->cd_gpio, 0);
+		if (ret)
+			return ret;
+	}
+
 	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->caps2 |= pdata->capabilities2;
 	mmc->max_segs = pdata->max_segs ? : 32;
@@ -1300,14 +1306,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 
 	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
 
-	if (pdata->flags & TMIO_MMC_USE_GPIO_CD) {
-		ret = mmc_gpio_request_cd(mmc, pdata->cd_gpio, 0);
-		if (ret)
-			goto remove_host;
-
-		mmc_gpiod_request_cd_irq(mmc);
-	}
-
 	return 0;
 
 remove_host:
-- 
2.7.4

Re: [PATCH v2 08/22] mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:43AM +0900, Masahiro Yamada wrote:
> Drivers do not need to call mmc_gpiod_request_cd_irq() explicitly
> because mmc_start_host() calls it.  To make it work, cd_gpio must
> be set before mmc_add_host().
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[PATCH v2 09/22] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Masahiro Yamada]

To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is set
by a legacy board (arch/sh/boards/mach-ecovec24).

For DT platforms, the "cd-gpios" property is a legitimate way for that
in case the IP-builtin card detection can not be used for some reason.
mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
the "cd-gpios" property is specified.

To cater to both cases, mmc_can_gpio_cd() is a correct way to check
which card detection logic is used.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/tmio_mmc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index efffb04..610f26f 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1232,7 +1232,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	}
 	mmc->max_seg_size = mmc->max_req_size;
 
-	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
+	_host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
 				  mmc->caps & MMC_CAP_NEEDS_POLL ||
 				  !mmc_card_is_removable(mmc));
 
-- 
2.7.4

Re: [PATCH v2 09/22] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:44AM +0900, Masahiro Yamada wrote:
> To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is set
> by a legacy board (arch/sh/boards/mach-ecovec24).
> 
> For DT platforms, the "cd-gpios" property is a legitimate way for that
> in case the IP-builtin card detection can not be used for some reason.
> mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
> the "cd-gpios" property is specified.
> 
> To cater to both cases, mmc_can_gpio_cd() is a correct way to check
> which card detection logic is used.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

My gut feeling is that your patch is correct, but I need to have another
look at this native_hotplug code with a fresh brain and take your other
patches into account as well then, too.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 09/22] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:44AM +0900, Masahiro Yamada wrote:
> To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is set
> by a legacy board (arch/sh/boards/mach-ecovec24).
> 
> For DT platforms, the "cd-gpios" property is a legitimate way for that
> in case the IP-builtin card detection can not be used for some reason.
> mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
> the "cd-gpios" property is specified.
> 
> To cater to both cases, mmc_can_gpio_cd() is a correct way to check
> which card detection logic is used.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

This patch is correct, yet needed some time for testing because it
inverts the results for R-Car SoCs. Again, it is correct that it inverts
it because those SoCs have GPIOs defined in their devicetrees, so they
shouldn't be using native hotplug. Still, this meant checking that no
regression gets introduced. Also, for R-Car Gen 2 & 3 native hotplug
seems to work fine, so I was trying to find out why we use GPIOs here. I
wasn't successful up to now, but since GPIOs work well, too, and seem to
react a bit faster even, I am fine with the patch being merged.

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

> ---
> 
> Changes in v2: None
> 
>  drivers/mmc/host/tmio_mmc_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index efffb04..610f26f 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1232,7 +1232,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>  	}
>  	mmc->max_seg_size = mmc->max_req_size;
>  
> -	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
> +	_host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
>  				  mmc->caps & MMC_CAP_NEEDS_POLL ||
>  				  !mmc_card_is_removable(mmc));
>  
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 09/22] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Masahiro Yamada]

Hi Ulf,


2018-01-02 21:56 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
> On Sat, Nov 25, 2017 at 01:24:44AM +0900, Masahiro Yamada wrote:
>> To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is set
>> by a legacy board (arch/sh/boards/mach-ecovec24).
>>
>> For DT platforms, the "cd-gpios" property is a legitimate way for that
>> in case the IP-builtin card detection can not be used for some reason.
>> mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
>> the "cd-gpios" property is specified.
>>
>> To cater to both cases, mmc_can_gpio_cd() is a correct way to check
>> which card detection logic is used.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>
> This patch is correct, yet needed some time for testing because it
> inverts the results for R-Car SoCs. Again, it is correct that it inverts
> it because those SoCs have GPIOs defined in their devicetrees, so they
> shouldn't be using native hotplug. Still, this meant checking that no
> regression gets introduced. Also, for R-Car Gen 2 & 3 native hotplug
> seems to work fine, so I was trying to find out why we use GPIOs here. I
> wasn't successful up to now, but since GPIOs work well, too, and seem to
> react a bit faster even, I am fine with the patch being merged.
>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
>> ---
>>
>> Changes in v2: None
>>
>>  drivers/mmc/host/tmio_mmc_core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
>> index efffb04..610f26f 100644
>> --- a/drivers/mmc/host/tmio_mmc_core.c
>> +++ b/drivers/mmc/host/tmio_mmc_core.c
>> @@ -1232,7 +1232,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>>       }
>>       mmc->max_seg_size = mmc->max_req_size;
>>
>> -     _host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
>> +     _host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
>>                                 mmc->caps & MMC_CAP_NEEDS_POLL ||
>>                                 !mmc_card_is_removable(mmc));
>>
>> --
>> 2.7.4
>>

Wolfram issued Reviewed-by.

Could you pick up this patch for -next?

-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 09/22] mmc: tmio: use mmc_can_gpio_cd() instead of checking TMIO_MMC_USE_GPIO_CD

[Ulf Hansson]

On 24 November 2017 at 17:24, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> To use a GPIO line for card detection, TMIO_MMC_USE_GPIO_CD is set
> by a legacy board (arch/sh/boards/mach-ecovec24).
>
> For DT platforms, the "cd-gpios" property is a legitimate way for that
> in case the IP-builtin card detection can not be used for some reason.
> mmc_of_parse() calls mmc_gpiod_request_cd() to set up ctx->cd_gpio if
> the "cd-gpios" property is specified.
>
> To cater to both cases, mmc_can_gpio_cd() is a correct way to check
> which card detection logic is used.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Thanks, applied for next!

For the rest of the series, please re-post those changes.

Kind regards
Uffe

> ---
>
> Changes in v2: None
>
>  drivers/mmc/host/tmio_mmc_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index efffb04..610f26f 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1232,7 +1232,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>         }
>         mmc->max_seg_size = mmc->max_req_size;
>
> -       _host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
> +       _host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
>                                   mmc->caps & MMC_CAP_NEEDS_POLL ||
>                                   !mmc_card_is_removable(mmc));
>
> --
> 2.7.4
>

[PATCH v2 10/22] mmc: tmio: support IP-builtin card detection logic

[Masahiro Yamada]

A card detect GPIO is set up only for platforms with "cd-gpios"
DT property or TMIO_MMC_USE_GPIO_CD flag.  However, the driver
core always uses mmc_gpio_get_cd, which just fails with -ENOSYS
if ctx->cd_gpio is unset.

The bit 5 of the status register provides the current signal level
of the CD line.  Allow to use it if the GPIO is unused.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 drivers/mmc/host/tmio_mmc_core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 610f26f..b51bb06 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1087,6 +1087,19 @@ static int tmio_mmc_get_ro(struct mmc_host *mmc)
 	return ret;
 }
 
+static int tmio_mmc_get_cd(struct mmc_host *mmc)
+{
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+	int ret;
+
+	ret = mmc_gpio_get_cd(mmc);
+	if (ret >= 0)
+		return ret;
+
+	return !!(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) &
+							TMIO_STAT_SIGSTATE);
+}
+
 static int tmio_multi_io_quirk(struct mmc_card *card,
 			       unsigned int direction, int blk_size)
 {
@@ -1102,7 +1115,7 @@ static const struct mmc_host_ops tmio_mmc_ops = {
 	.request	= tmio_mmc_request,
 	.set_ios	= tmio_mmc_set_ios,
 	.get_ro         = tmio_mmc_get_ro,
-	.get_cd		= mmc_gpio_get_cd,
+	.get_cd		= tmio_mmc_get_cd,
 	.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
 	.multi_io_quirk	= tmio_multi_io_quirk,
 	.hw_reset	= tmio_mmc_hw_reset,
-- 
2.7.4

Re: [PATCH v2 10/22] mmc: tmio: support IP-builtin card detection logic

[Wolfram Sang]

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

> +static int tmio_mmc_get_cd(struct mmc_host *mmc)
> +{
> +	struct tmio_mmc_host *host = mmc_priv(mmc);
> +	int ret;
> +
> +	ret = mmc_gpio_get_cd(mmc);
> +	if (ret >= 0)
> +		return ret;
> +
> +	return !!(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) &
> +							TMIO_STAT_SIGSTATE);
> +}

I wonder if we shouldn't do something like:

	if (mmc_can_gpio_cd())
		return mmc_gpio_get_cd()
	else
		return !!(sd_ctrl_read16_and_16_as_32...)

If we have a GPIO CD defined, I think we want the value of
mmc_gpio_get_cd() in all cases. It makes clearer that this is an
'either-or' case and not a fallback mechanism.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 10/22] mmc: tmio: support IP-builtin card detection logic

[Masahiro Yamada]

2017-12-05 0:13 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
>> +static int tmio_mmc_get_cd(struct mmc_host *mmc)
>> +{
>> +     struct tmio_mmc_host *host = mmc_priv(mmc);
>> +     int ret;
>> +
>> +     ret = mmc_gpio_get_cd(mmc);
>> +     if (ret >= 0)
>> +             return ret;
>> +
>> +     return !!(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) &
>> +                                                     TMIO_STAT_SIGSTATE);
>> +}


I just followed tmio_mmc_get_ro() implementation.

If we do not care the symmetry between _get_ro() and _get_cd() hooks,
yes, your suggestion makes sense.



> I wonder if we shouldn't do something like:
>
>         if (mmc_can_gpio_cd())
>                 return mmc_gpio_get_cd()
>         else
>                 return !!(sd_ctrl_read16_and_16_as_32...)
>
> If we have a GPIO CD defined, I think we want the value of
> mmc_gpio_get_cd() in all cases. It makes clearer that this is an
> 'either-or' case and not a fallback mechanism.
>


Another possibility would select this in _probe().
We would need to evaluate mmc_can_gpio_cd() only once when probing.

I will follow your suggestion, though.
Either way is fine with me.



static int tmio_mmc_get_cd(struct mmc_host *mmc)
{
       struct tmio_mmc_host *host = mmc_priv(mmc);

       return !!(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) &
                                                TMIO_STAT_SIGSTATE);
}

static const struct mmc_host_ops tmio_mmc_ops = {
      ...
      .get_cd      = tmio_mmc_get_cd,
      ...
};

int tmio_mmc_host_probe( ... )
{
      ....

      /* replace get_cd hook when we use GPIO for card detection */
      if (mmc_can_gpio_cd(mmc))
                _host->ops.get_cd = mmc_gpio_get_cd;


}





-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 10/22] mmc: tmio: support IP-builtin card detection logic

[Wolfram Sang]

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


> Another possibility would select this in _probe().
> We would need to evaluate mmc_can_gpio_cd() only once when probing.

I like this, too.

Maybe like this to be more obvious:

	if (mmc_can_gpio_cd(mmc))
		 _host->ops.get_cd = mmc_gpio_get_cd;
	else
		 _host->ops.get_cd = tmio...

?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 10/22] mmc: tmio: support IP-builtin card detection logic

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:45AM +0900, Masahiro Yamada wrote:
> A card detect GPIO is set up only for platforms with "cd-gpios"
> DT property or TMIO_MMC_USE_GPIO_CD flag.  However, the driver
> core always uses mmc_gpio_get_cd, which just fails with -ENOSYS
> if ctx->cd_gpio is unset.
> 
> The bit 5 of the status register provides the current signal level
> of the CD line.  Allow to use it if the GPIO is unused.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

As mentioned before (sorry, I lost this thread :( ), I like the
refactoring to select in probe() which function to call depending on
GPIO usage or not. If you like, we can do the same for read_only, too.

Thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 11/22] mmc: renesas_sdhi: remove always false condition

[Masahiro Yamada]

renesas_sdhi_probe() always sets host->dma as follows:

        host->dma               = dma_priv;

!host->dma is always false.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/renesas_sdhi_sys_dmac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 9ab1043..e210644 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -360,8 +360,8 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
 					      struct tmio_mmc_data *pdata)
 {
 	/* We can only either use DMA for both Tx and Rx or not use it at all */
-	if (!host->dma || (!host->pdev->dev.of_node &&
-			   (!pdata->chan_priv_tx || !pdata->chan_priv_rx)))
+	if (!host->pdev->dev.of_node &&
+	    (!pdata->chan_priv_tx || !pdata->chan_priv_rx))
 		return;
 
 	if (!host->chan_tx && !host->chan_rx) {
-- 
2.7.4

Re: [PATCH v2 11/22] mmc: renesas_sdhi: remove always false condition

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:46AM +0900, Masahiro Yamada wrote:
> renesas_sdhi_probe() always sets host->dma as follows:
> 
>         host->dma               = dma_priv;
> 
> !host->dma is always false.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[PATCH v2 12/22] mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h

[Masahiro Yamada]

struct tmio_mmc_dma looks like TMIO core data, but in fact, Renesas
private data.  Move it to renesas_sdhi.h (probably, it is better to
rename it to renesas_sdhi_dma, or squash it into struct renesas_sdhi).

I also moved struct renesas_sdhi and host_to_priv() to that header
because they are necessary to convert the tmio_mmc_host pointer into
the renesas_sdhi pointer.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/renesas_sdhi.h               | 19 +++++++++++++++++++
 drivers/mmc/host/renesas_sdhi_core.c          | 14 --------------
 drivers/mmc/host/renesas_sdhi_internal_dmac.c |  6 ++++--
 drivers/mmc/host/renesas_sdhi_sys_dmac.c      | 16 ++++++++++------
 drivers/mmc/host/tmio_mmc.h                   |  7 -------
 5 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index b9dfea5..9a507b3 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -35,6 +35,25 @@ struct renesas_sdhi_of_data {
 	unsigned short max_segs;
 };
 
+struct tmio_mmc_dma {
+	enum dma_slave_buswidth dma_buswidth;
+	bool (*filter)(struct dma_chan *chan, void *arg);
+	void (*enable)(struct tmio_mmc_host *host, bool enable);
+};
+
+struct renesas_sdhi {
+	struct clk *clk;
+	struct clk *clk_cd;
+	struct tmio_mmc_data mmc_data;
+	struct tmio_mmc_dma dma_priv;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pins_default, *pins_uhs;
+	void __iomem *scc_ctl;
+};
+
+#define host_to_priv(host) \
+	container_of((host)->pdata, struct renesas_sdhi, mmc_data)
+
 int renesas_sdhi_probe(struct platform_device *pdev,
 		       const struct tmio_mmc_dma_ops *dma_ops);
 int renesas_sdhi_remove(struct platform_device *pdev);
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 267e2e0..34fd194 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -46,19 +46,6 @@
 #define SDHI_VER_GEN3_SD	0xcc10
 #define SDHI_VER_GEN3_SDMMC	0xcd10
 
-#define host_to_priv(host) \
-	container_of((host)->pdata, struct renesas_sdhi, mmc_data)
-
-struct renesas_sdhi {
-	struct clk *clk;
-	struct clk *clk_cd;
-	struct tmio_mmc_data mmc_data;
-	struct tmio_mmc_dma dma_priv;
-	struct pinctrl *pinctrl;
-	struct pinctrl_state *pins_default, *pins_uhs;
-	void __iomem *scc_ctl;
-};
-
 static void renesas_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
 {
 	u32 val;
@@ -539,7 +526,6 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		host->bus_shift = of_data->bus_shift;
 	}
 
-	host->dma		= dma_priv;
 	host->write16_hook	= renesas_sdhi_write16_hook;
 	host->clk_enable	= renesas_sdhi_clk_enable;
 	host->clk_update	= renesas_sdhi_clk_update;
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 41cbe84..691a99f 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -103,6 +103,8 @@ renesas_sdhi_internal_dmac_dm_write(struct tmio_mmc_host *host,
 static void
 renesas_sdhi_internal_dmac_enable_dma(struct tmio_mmc_host *host, bool enable)
 {
+	struct renesas_sdhi *priv = host_to_priv(host);
+
 	if (!host->chan_tx || !host->chan_rx)
 		return;
 
@@ -110,8 +112,8 @@ renesas_sdhi_internal_dmac_enable_dma(struct tmio_mmc_host *host, bool enable)
 		renesas_sdhi_internal_dmac_dm_write(host, DM_CM_INFO1,
 						    INFO1_CLEAR);
 
-	if (host->dma->enable)
-		host->dma->enable(host, enable);
+	if (priv->dma_priv.enable)
+		priv->dma_priv.enable(host, enable);
 }
 
 static void
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index e210644..aeb3838 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -117,11 +117,13 @@ MODULE_DEVICE_TABLE(of, renesas_sdhi_sys_dmac_of_match);
 static void renesas_sdhi_sys_dmac_enable_dma(struct tmio_mmc_host *host,
 					     bool enable)
 {
+	struct renesas_sdhi *priv = host_to_priv(host);
+
 	if (!host->chan_tx || !host->chan_rx)
 		return;
 
-	if (host->dma->enable)
-		host->dma->enable(host, enable);
+	if (priv->dma_priv.enable)
+		priv->dma_priv.enable(host, enable);
 }
 
 static void renesas_sdhi_sys_dmac_abort_dma(struct tmio_mmc_host *host)
@@ -359,6 +361,8 @@ static void renesas_sdhi_sys_dmac_issue_tasklet_fn(unsigned long priv)
 static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
 					      struct tmio_mmc_data *pdata)
 {
+	struct renesas_sdhi *priv = host_to_priv(host);
+
 	/* We can only either use DMA for both Tx and Rx or not use it at all */
 	if (!host->pdev->dev.of_node &&
 	    (!pdata->chan_priv_tx || !pdata->chan_priv_rx))
@@ -378,7 +382,7 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
 		dma_cap_set(DMA_SLAVE, mask);
 
 		host->chan_tx = dma_request_slave_channel_compat(mask,
-					host->dma->filter, pdata->chan_priv_tx,
+					priv->dma_priv.filter, pdata->chan_priv_tx,
 					&host->pdev->dev, "tx");
 		dev_dbg(&host->pdev->dev, "%s: TX: got channel %p\n", __func__,
 			host->chan_tx);
@@ -389,7 +393,7 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
 		cfg.direction = DMA_MEM_TO_DEV;
 		cfg.dst_addr = res->start +
 			(CTL_SD_DATA_PORT << host->bus_shift);
-		cfg.dst_addr_width = host->dma->dma_buswidth;
+		cfg.dst_addr_width = priv->dma_priv.dma_buswidth;
 		if (!cfg.dst_addr_width)
 			cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 		cfg.src_addr = 0;
@@ -398,7 +402,7 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
 			goto ecfgtx;
 
 		host->chan_rx = dma_request_slave_channel_compat(mask,
-					host->dma->filter, pdata->chan_priv_rx,
+					priv->dma_priv.filter, pdata->chan_priv_rx,
 					&host->pdev->dev, "rx");
 		dev_dbg(&host->pdev->dev, "%s: RX: got channel %p\n", __func__,
 			host->chan_rx);
@@ -408,7 +412,7 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
 
 		cfg.direction = DMA_DEV_TO_MEM;
 		cfg.src_addr = cfg.dst_addr + host->pdata->dma_rx_offset;
-		cfg.src_addr_width = host->dma->dma_buswidth;
+		cfg.src_addr_width = priv->dma_priv.dma_buswidth;
 		if (!cfg.src_addr_width)
 			cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 		cfg.dst_addr = 0;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 37409c5..dc470d3 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -112,12 +112,6 @@
 struct tmio_mmc_data;
 struct tmio_mmc_host;
 
-struct tmio_mmc_dma {
-	enum dma_slave_buswidth dma_buswidth;
-	bool (*filter)(struct dma_chan *chan, void *arg);
-	void (*enable)(struct tmio_mmc_host *host, bool enable);
-};
-
 struct tmio_mmc_dma_ops {
 	void (*start)(struct tmio_mmc_host *host, struct mmc_data *data);
 	void (*enable)(struct tmio_mmc_host *host, bool enable);
@@ -149,7 +143,6 @@ struct tmio_mmc_host {
 
 	struct platform_device *pdev;
 	struct tmio_mmc_data *pdata;
-	struct tmio_mmc_dma	*dma;
 
 	/* DMA support */
 	bool			force_pio;
-- 
2.7.4

Re: [PATCH v2 12/22] mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:47AM +0900, Masahiro Yamada wrote:
> struct tmio_mmc_dma looks like TMIO core data, but in fact, Renesas
> private data.  Move it to renesas_sdhi.h (probably, it is better to
> rename it to renesas_sdhi_dma, or squash it into struct renesas_sdhi).
> 
> I also moved struct renesas_sdhi and host_to_priv() to that header
> because they are necessary to convert the tmio_mmc_host pointer into
> the renesas_sdhi pointer.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[PATCH v2 13/22] mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to renesas_sdhi.h

[Masahiro Yamada]

struct tmio_mmc_host has "dma_dataend" and "dma_complete", but in fact,
they are Renesas private data.  Move them to renesas_sdhi.h

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/renesas_sdhi.h               |  2 ++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c |  8 ++++++--
 drivers/mmc/host/renesas_sdhi_sys_dmac.c      | 15 ++++++++++-----
 drivers/mmc/host/tmio_mmc.h                   |  2 --
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index 9a507b3..3250dbe 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -39,6 +39,8 @@ struct tmio_mmc_dma {
 	enum dma_slave_buswidth dma_buswidth;
 	bool (*filter)(struct dma_chan *chan, void *arg);
 	void (*enable)(struct tmio_mmc_host *host, bool enable);
+	struct completion	dma_dataend;
+	struct tasklet_struct	dma_complete;
 };
 
 struct renesas_sdhi {
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 691a99f..02c2724 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -132,7 +132,9 @@ renesas_sdhi_internal_dmac_abort_dma(struct tmio_mmc_host *host) {
 
 static void
 renesas_sdhi_internal_dmac_dataend_dma(struct tmio_mmc_host *host) {
-	tasklet_schedule(&host->dma_complete);
+	struct renesas_sdhi *priv = host_to_priv(host);
+
+	tasklet_schedule(&priv->dma_priv.dma_complete);
 }
 
 static void
@@ -222,10 +224,12 @@ static void
 renesas_sdhi_internal_dmac_request_dma(struct tmio_mmc_host *host,
 				       struct tmio_mmc_data *pdata)
 {
+	struct renesas_sdhi *priv = host_to_priv(host);
+
 	/* Each value is set to non-zero to assume "enabling" each DMA */
 	host->chan_rx = host->chan_tx = (void *)0xdeadbeaf;
 
-	tasklet_init(&host->dma_complete,
+	tasklet_init(&priv->dma_priv.dma_complete,
 		     renesas_sdhi_internal_dmac_complete_tasklet_fn,
 		     (unsigned long)host);
 	tasklet_init(&host->dma_issue,
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index aeb3838..c8a74b2 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -140,12 +140,15 @@ static void renesas_sdhi_sys_dmac_abort_dma(struct tmio_mmc_host *host)
 
 static void renesas_sdhi_sys_dmac_dataend_dma(struct tmio_mmc_host *host)
 {
-	complete(&host->dma_dataend);
+	struct renesas_sdhi *priv = host_to_priv(host);
+
+	complete(&priv->dma_priv.dma_dataend);
 }
 
 static void renesas_sdhi_sys_dmac_dma_callback(void *arg)
 {
 	struct tmio_mmc_host *host = arg;
+	struct renesas_sdhi *priv = host_to_priv(host);
 
 	spin_lock_irq(&host->lock);
 
@@ -163,7 +166,7 @@ static void renesas_sdhi_sys_dmac_dma_callback(void *arg)
 
 	spin_unlock_irq(&host->lock);
 
-	wait_for_completion(&host->dma_dataend);
+	wait_for_completion(&priv->dma_priv.dma_dataend);
 
 	spin_lock_irq(&host->lock);
 	tmio_mmc_do_data_irq(host);
@@ -173,6 +176,7 @@ static void renesas_sdhi_sys_dmac_dma_callback(void *arg)
 
 static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
 {
+	struct renesas_sdhi *priv = host_to_priv(host);
 	struct scatterlist *sg = host->sg_ptr, *sg_tmp;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_rx;
@@ -216,7 +220,7 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
 					       DMA_CTRL_ACK);
 
 	if (desc) {
-		reinit_completion(&host->dma_dataend);
+		reinit_completion(&priv->dma_priv.dma_dataend);
 		desc->callback = renesas_sdhi_sys_dmac_dma_callback;
 		desc->callback_param = host;
 
@@ -247,6 +251,7 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
 
 static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
 {
+	struct renesas_sdhi *priv = host_to_priv(host);
 	struct scatterlist *sg = host->sg_ptr, *sg_tmp;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_tx;
@@ -295,7 +300,7 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
 					       DMA_CTRL_ACK);
 
 	if (desc) {
-		reinit_completion(&host->dma_dataend);
+		reinit_completion(&priv->dma_priv.dma_dataend);
 		desc->callback = renesas_sdhi_sys_dmac_dma_callback;
 		desc->callback_param = host;
 
@@ -424,7 +429,7 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
 		if (!host->bounce_buf)
 			goto ebouncebuf;
 
-		init_completion(&host->dma_dataend);
+		init_completion(&priv->dma_priv.dma_dataend);
 		tasklet_init(&host->dma_issue,
 			     renesas_sdhi_sys_dmac_issue_tasklet_fn,
 			     (unsigned long)host);
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index dc470d3..8b169a0 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -148,8 +148,6 @@ struct tmio_mmc_host {
 	bool			force_pio;
 	struct dma_chan		*chan_rx;
 	struct dma_chan		*chan_tx;
-	struct completion	dma_dataend;
-	struct tasklet_struct	dma_complete;
 	struct tasklet_struct	dma_issue;
 	struct scatterlist	bounce_sg;
 	u8			*bounce_buf;
-- 
2.7.4

Re: [PATCH v2 13/22] mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to renesas_sdhi.h

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:48AM +0900, Masahiro Yamada wrote:
> struct tmio_mmc_host has "dma_dataend" and "dma_complete", but in fact,
> they are Renesas private data.  Move them to renesas_sdhi.h
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[PATCH v2 14/22] mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h

[Masahiro Yamada]

struct tmio_mmc_host has "scc_tappos", but in fact, it is Renesas
private data.  Move it to renesas_sdhi.h

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/renesas_sdhi.h      | 1 +
 drivers/mmc/host/renesas_sdhi_core.c | 4 ++--
 drivers/mmc/host/tmio_mmc.h          | 1 -
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index 3250dbe..f13f798 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -51,6 +51,7 @@ struct renesas_sdhi {
 	struct pinctrl *pinctrl;
 	struct pinctrl_state *pins_default, *pins_uhs;
 	void __iomem *scc_ctl;
+	u32 scc_tappos;
 };
 
 #define host_to_priv(host) \
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 34fd194..e78dca7 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -267,7 +267,7 @@ static unsigned int renesas_sdhi_init_tuning(struct tmio_mmc_host *host)
 		       ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
 		       sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
 
-	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DT2FF, host->scc_tappos);
+	sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_DT2FF, priv->scc_tappos);
 
 	/* Read TAPNUM */
 	return (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL) >>
@@ -590,7 +590,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		for (i = 0; i < of_data->taps_num; i++) {
 			if (taps[i].clk_rate == 0 ||
 			    taps[i].clk_rate == host->mmc->f_max) {
-				host->scc_tappos = taps->tap;
+				priv->scc_tappos = taps->tap;
 				hit = true;
 				break;
 			}
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 8b169a0..9f083dc 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -166,7 +166,6 @@ struct tmio_mmc_host {
 	struct mutex		ios_lock;	/* protect set_ios() context */
 	bool			native_hotplug;
 	bool			sdio_irq_enabled;
-	u32			scc_tappos;
 
 	/* Mandatory callback */
 	int (*clk_enable)(struct tmio_mmc_host *host);
-- 
2.7.4

Re: [PATCH v2 14/22] mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:49AM +0900, Masahiro Yamada wrote:
> struct tmio_mmc_host has "scc_tappos", but in fact, it is Renesas
> private data.  Move it to renesas_sdhi.h
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[PATCH v2 15/22] mmc: tmio: change bus_shift to unsigned int

[Masahiro Yamada]

Sane values for bus_shift are:
   0 - for 16 bit bus
   1 - for 32 bit bus
   2 - for 64 bit bus

"unsigned long" is too much.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/tmio_mmc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 9f083dc..27894474 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -139,7 +139,7 @@ struct tmio_mmc_host {
 	struct scatterlist      *sg_orig;
 	unsigned int            sg_len;
 	unsigned int            sg_off;
-	unsigned long		bus_shift;
+	unsigned int		bus_shift;
 
 	struct platform_device *pdev;
 	struct tmio_mmc_data *pdata;
-- 
2.7.4

Re: [PATCH v2 15/22] mmc: tmio: change bus_shift to unsigned int

[Wolfram Sang]

On Sat, Nov 25, 2017 at 01:24:50AM +0900, Masahiro Yamada wrote:
> Sane values for bus_shift are:
>    0 - for 16 bit bus
>    1 - for 32 bit bus
>    2 - for 64 bit bus
> 
> "unsigned long" is too much.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[PATCH v2 16/22] mmc: tmio: fix never-detected card insertion bug

[Masahiro Yamada]

The TMIO mmc cannot detect the card insertion in native_hotplug mode
if the driver is probed without a card inserted.

The reason is obvious; all IRQs are disabled by tmio_mmc_host_probe(),
as follows:

  tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);

The IRQs are first enabled by tmio_mmc_start_command() as follows:

  if (!host->native_hotplug)
          irq_mask &= ~(TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
  tmio_mmc_enable_mmc_irqs(host, irq_mask);

If the driver is probed without a card, tmio_mmc_start_command() is
never called in the first place.  So, the card is never detected.

The card event IRQs must be enabled in probe/resume functions.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/tmio_mmc_core.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index b51bb06..7d169ed 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -350,8 +350,6 @@ static int tmio_mmc_start_command(struct tmio_mmc_host *host,
 			c |= TRANSFER_READ;
 	}
 
-	if (!host->native_hotplug)
-		irq_mask &= ~(TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
 	tmio_mmc_enable_mmc_irqs(host, irq_mask);
 
 	/* Fire off the command */
@@ -1292,11 +1290,13 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 		irq_mask |= TMIO_MASK_READOP;
 	if (!_host->chan_tx)
 		irq_mask |= TMIO_MASK_WRITEOP;
-	if (!_host->native_hotplug)
-		irq_mask &= ~(TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
 
 	_host->sdcard_irq_mask &= ~irq_mask;
 
+	if (_host->native_hotplug)
+		tmio_mmc_enable_mmc_irqs(_host,
+				TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
+
 	spin_lock_init(&_host->lock);
 	mutex_init(&_host->ios_lock);
 
@@ -1383,6 +1383,10 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 	if (host->clk_cache)
 		tmio_mmc_set_clock(host, host->clk_cache);
 
+	if (host->native_hotplug)
+		tmio_mmc_enable_mmc_irqs(host,
+				TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
+
 	tmio_mmc_enable_dma(host, true);
 
 	if (tmio_mmc_can_retune(host) && host->select_tuning(host))
-- 
2.7.4

Re: [PATCH v2 16/22] mmc: tmio: fix never-detected card insertion bug

[Wolfram Sang]

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

Yamada-san,

> The TMIO mmc cannot detect the card insertion in native_hotplug mode
> if the driver is probed without a card inserted.

Hmm, it works for me without your patch just fine. Iam currently
researching it...

Happy new year, by the way! :)

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 16/22] mmc: tmio: fix never-detected card insertion bug

[Wolfram Sang]

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


> > The TMIO mmc cannot detect the card insertion in native_hotplug mode
> > if the driver is probed without a card inserted.
> 
> Hmm, it works for me without your patch just fine. Iam currently
> researching it...

It really doesn't work for you?

mmc_add_host -> mmc_start_host -> _mmc_detect_change ->
mmc_schedule_delayed_work -> mmc_rescan -> mmc_start_request


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 16/22] mmc: tmio: fix never-detected card insertion bug

[Masahiro Yamada]

Hi Wolfram,


2018-01-03 2:13 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
>
>> > The TMIO mmc cannot detect the card insertion in native_hotplug mode
>> > if the driver is probed without a card inserted.
>>
>> Hmm, it works for me without your patch just fine. Iam currently
>> researching it...
>
> It really doesn't work for you?


Does not work.


> mmc_add_host -> mmc_start_host -> _mmc_detect_change ->
> mmc_schedule_delayed_work -> mmc_rescan -> mmc_start_request
>

Correct except "-> mmc_start_request".

If a card is not inserted, the power will go down. [1]
So, mmc_start_request will not happen.


BTW, did you understand my git-log?

I am talking about the card detection
by the IP-builtin circuit.

I am trying to support it by the following patch:
https://patchwork.kernel.org/patch/10074255/

TMIO MMC calls mmc_detect_change() [2],
but please note this _never_ detects the card event.
It just re-schedules mmc_rescan. [3]

To detect the card insertion/removal, .get_cd hook must be implemented,
but, it is fixed to mmc_gpio_get_cd. [4]

Unless you had already migrated your boards to built-in CD
based on my patch, GPIO is the only way to detect card events.

If so, you are testing unrelated things.

It is really hard to understand what is going on Renesas side,
but I imagined some possibilities:
 - Poll the GPIO for card events          -> out of interest of this patch
 - Use interrupt from GPIO irqchip        -> ditto
 - The media is non-removable like eMMC   -> ditto
 - GPIO is not set up                     -> mmc_gpio_get_cd() returns -ENOSYS


The last case is treated as "inserted", but the card removal will not
be detected.


[1] http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/mmc/core/core.c#L2818
[2] http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/mmc/host/tmio_mmc_core.c#L656
[3] http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/mmc/core/core.c#L1995
[4] http://elixir.free-electrons.com/linux/v4.15-rc6/source/drivers/mmc/host/tmio_mmc_core.c#L1105

-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 16/22] mmc: tmio: fix never-detected card insertion bug

[Wolfram Sang]

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


> I am talking about the card detection
> by the IP-builtin circuit.

Yes, I know. As I wrote in one of the previous patches when reviewing
it, I disabled GPIO CD and used the internal mechanism (for tests where
it is relevant). Like here, too.

>  - GPIO is not set up                     -> mmc_gpio_get_cd() returns -ENOSYS

Thanks! That pointed me to the right direction. I missed that patch
10/22 was still under discussion and not applied to mmc/next, so I had
to pick it manually.

I can confirm now that there is an issue and your patch fixes it for the
non-GPIO case. For the GPIO case, however, the TMIO_STAT_CARD_REMOVE |
TMIO_STAT_CARD_INSERT interrupts are enabled now, too. It didn't harm
when doing my tests, but we shouldn't do it, to be safe IMO.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 16/22] mmc: tmio: fix never-detected card insertion bug

[Masahiro Yamada]

2018-01-14 5:59 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
>
>> I am talking about the card detection
>> by the IP-builtin circuit.
>
> Yes, I know. As I wrote in one of the previous patches when reviewing
> it, I disabled GPIO CD and used the internal mechanism (for tests where
> it is relevant). Like here, too.
>
>>  - GPIO is not set up                     -> mmc_gpio_get_cd() returns -ENOSYS
>
> Thanks! That pointed me to the right direction. I missed that patch
> 10/22 was still under discussion and not applied to mmc/next, so I had
> to pick it manually.
>
> I can confirm now that there is an issue and your patch fixes it for the
> non-GPIO case. For the GPIO case, however, the TMIO_STAT_CARD_REMOVE |
> TMIO_STAT_CARD_INSERT interrupts are enabled now, too. It didn't harm
> when doing my tests, but we shouldn't do it, to be safe IMO.
>


Could you explain why?


    _host->native_hotplug = !(mmc_can_gpio_cd(mmc) ||
                              mmc->caps & MMC_CAP_NEEDS_POLL ||
                             !mmc_card_is_removable(mmc));

For the GPIO case, mmc_can_gpio_cd(mmc) is 1,
so _host_>native_hogplug is 0.


Then, my code does nothing, doesn't it?

+       if (_host->native_hotplug)
+               tmio_mmc_enable_mmc_irqs(_host,
+                               TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);




-- 
Best Regards
Masahiro Yamada

[PATCH v2 17/22] mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place

[Masahiro Yamada]

This driver was largely extended by Renesas, but actually used by
several SoC vendors.  The current code does not work for UniPhier
SoCs at least.

The DMA mode for UniPhier SoCs failed with the following error message:
  PIO IRQ in DMA mode!

For UniPhier SoCs, the TMIO_MASK_{READOP,WRITEOP} are asserted in the
DMA mode as well.

In fact, the code is very strange.

The TMIO_MASK_{READOP,WRITEOP} IRQs are set as follows:

    /* Unmask the IRQs we want to know about */
    if (!_host->chan_rx)
            irq_mask |= TMIO_MASK_READOP;
    if (!_host->chan_tx)
            irq_mask |= TMIO_MASK_WRITEOP;

At this point, _host->{chan_rx,chan_tx} are _always_ NULL because
tmio_mmc_request_dma() is called after this code.  Consequently,
TMIO_MASK_{READOP,WRITEOP} are set whether DMA is used or not.

tmio_mmc_cmd_irq() enables TMIO_MASK_{READOP,WRITEOP}, but never
disables them.  This does not take care of a case where ->force_pio
is set, but unset later.

After all, the correct place to handle those flags is just before
starting the data transfer.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/tmio_mmc_core.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 7d169ed..345e379 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -621,15 +621,19 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, unsigned int stat)
 	 */
 	if (host->data && (!cmd->error || cmd->error == -EILSEQ)) {
 		if (host->data->flags & MMC_DATA_READ) {
-			if (host->force_pio || !host->chan_rx)
+			if (host->force_pio || !host->chan_rx) {
 				tmio_mmc_enable_mmc_irqs(host, TMIO_MASK_READOP);
-			else
+			} else {
+				tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_READOP);
 				tasklet_schedule(&host->dma_issue);
+			}
 		} else {
-			if (host->force_pio || !host->chan_tx)
+			if (host->force_pio || !host->chan_tx) {
 				tmio_mmc_enable_mmc_irqs(host, TMIO_MASK_WRITEOP);
-			else
+			} else {
+				tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_WRITEOP);
 				tasklet_schedule(&host->dma_issue);
+			}
 		}
 	} else {
 		schedule_work(&host->done);
@@ -1285,12 +1289,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, CTL_IRQ_MASK);
 	tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
 
-	/* Unmask the IRQs we want to know about */
-	if (!_host->chan_rx)
-		irq_mask |= TMIO_MASK_READOP;
-	if (!_host->chan_tx)
-		irq_mask |= TMIO_MASK_WRITEOP;
-
 	_host->sdcard_irq_mask &= ~irq_mask;
 
 	if (_host->native_hotplug)
-- 
2.7.4

Re: [PATCH v2 17/22] mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:52AM +0900, Masahiro Yamada wrote:
> This driver was largely extended by Renesas, but actually used by
> several SoC vendors.  The current code does not work for UniPhier
> SoCs at least.

Do you insist on this paragraph? All people working on the code so far
tried hard to make it work on all devices they had access to. So far, I
can't recall one of the several SoC vendors contacting upstream to get
their support merged (until now, of course) or even to file bugs.
Because I can't guess unknown stuff, I'd prefer to skip this paragraph.

> The DMA mode for UniPhier SoCs failed with the following error message:
>   PIO IRQ in DMA mode!
> 
> For UniPhier SoCs, the TMIO_MASK_{READOP,WRITEOP} are asserted in the
> DMA mode as well.

Sure, that we need to fix! Note that both Renesas DMA drivers also
enable DATAEND and disable RXRDY | TXRQ on their own, too. So, probably
the same issue? And IIUC we can decide now to prepare the irqs like this
in tmio_core because all known DMA engines need it. Or we keep it that
DMA engines set up irqs themselves. More flexible but maybe
over-engineered?

> In fact, the code is very strange.

Yes.

> The TMIO_MASK_{READOP,WRITEOP} IRQs are set as follows:
> 
>     /* Unmask the IRQs we want to know about */
>     if (!_host->chan_rx)
>             irq_mask |= TMIO_MASK_READOP;
>     if (!_host->chan_tx)
>             irq_mask |= TMIO_MASK_WRITEOP;
> 
> At this point, _host->{chan_rx,chan_tx} are _always_ NULL because
> tmio_mmc_request_dma() is called after this code.  Consequently,
> TMIO_MASK_{READOP,WRITEOP} are set whether DMA is used or not.

Yes :( Bummer.

> tmio_mmc_cmd_irq() enables TMIO_MASK_{READOP,WRITEOP}, but never
> disables them.  This does not take care of a case where ->force_pio
> is set, but unset later.

force_pio gets disabled within the same request? I may be overlooking
something but I only see code paths where force_pio gets cleared and a
little later the request gets finished. Can you give me a pointer?

> After all, the correct place to handle those flags is just before
> starting the data transfer.

While I totally agree the code below can be improved for sure, I'd
prefer to keep the design pattern that irqs get disabled once they are
not actively used. Generally spoken, I think it makes sense to keep
regressions on old platforms low. Any chance this can be achieved?
Other than that, fixing/removing this irq_mask handling from probe() is
really good.

> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
> Changes in v2:
>   - Newly added
> 
>  drivers/mmc/host/tmio_mmc_core.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 7d169ed..345e379 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -621,15 +621,19 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, unsigned int stat)
>  	 */
>  	if (host->data && (!cmd->error || cmd->error == -EILSEQ)) {
>  		if (host->data->flags & MMC_DATA_READ) {
> -			if (host->force_pio || !host->chan_rx)
> +			if (host->force_pio || !host->chan_rx) {
>  				tmio_mmc_enable_mmc_irqs(host, TMIO_MASK_READOP);
> -			else
> +			} else {
> +				tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_READOP);
>  				tasklet_schedule(&host->dma_issue);
> +			}
>  		} else {
> -			if (host->force_pio || !host->chan_tx)
> +			if (host->force_pio || !host->chan_tx) {
>  				tmio_mmc_enable_mmc_irqs(host, TMIO_MASK_WRITEOP);
> -			else
> +			} else {
> +				tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_WRITEOP);
>  				tasklet_schedule(&host->dma_issue);
> +			}
>  		}
>  	} else {
>  		schedule_work(&host->done);
> @@ -1285,12 +1289,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
>  	_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, CTL_IRQ_MASK);
>  	tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
>  
> -	/* Unmask the IRQs we want to know about */
> -	if (!_host->chan_rx)
> -		irq_mask |= TMIO_MASK_READOP;
> -	if (!_host->chan_tx)
> -		irq_mask |= TMIO_MASK_WRITEOP;
> -
>  	_host->sdcard_irq_mask &= ~irq_mask;
>  
>  	if (_host->native_hotplug)
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 17/22] mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place

[Masahiro Yamada]

Hi Wolfram,


2018-01-16 17:01 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
> On Sat, Nov 25, 2017 at 01:24:52AM +0900, Masahiro Yamada wrote:
>> This driver was largely extended by Renesas, but actually used by
>> several SoC vendors.  The current code does not work for UniPhier
>> SoCs at least.
>
> Do you insist on this paragraph? All people working on the code so far
> tried hard to make it work on all devices they had access to. So far, I
> can't recall one of the several SoC vendors contacting upstream to get
> their support merged (until now, of course) or even to file bugs.
> Because I can't guess unknown stuff, I'd prefer to skip this paragraph.

I dropped this in v3.



>> The DMA mode for UniPhier SoCs failed with the following error message:
>>   PIO IRQ in DMA mode!
>>
>> For UniPhier SoCs, the TMIO_MASK_{READOP,WRITEOP} are asserted in the
>> DMA mode as well.
>
> Sure, that we need to fix! Note that both Renesas DMA drivers also
> enable DATAEND and disable RXRDY | TXRQ on their own, too. So, probably
> the same issue? And IIUC we can decide now to prepare the irqs like this
> in tmio_core because all known DMA engines need it. Or we keep it that
> DMA engines set up irqs themselves. More flexible but maybe
> over-engineered?

OK, I cleaned up RXRDY | TXRQ in renesas drivers, too.

It is probably possible to cleanup DATAEND,
but I did not touch it for now.

As you may know, the original IP from Panasonic
did not support internal DMAC.

Renesas extended the IP by itself for the internal DMAC.

Panasonic did so in a different way.  So, the DMA engine HW
is completely different, and the current core is not
flexible enough for UniPhier SoCs because
our DMA engines added own interrupt masks.

I will consider how to handle it later,
but I do not see a reason to do it in this series.


>> In fact, the code is very strange.
>
> Yes.
>
>> The TMIO_MASK_{READOP,WRITEOP} IRQs are set as follows:
>>
>>     /* Unmask the IRQs we want to know about */
>>     if (!_host->chan_rx)
>>             irq_mask |= TMIO_MASK_READOP;
>>     if (!_host->chan_tx)
>>             irq_mask |= TMIO_MASK_WRITEOP;
>>
>> At this point, _host->{chan_rx,chan_tx} are _always_ NULL because
>> tmio_mmc_request_dma() is called after this code.  Consequently,
>> TMIO_MASK_{READOP,WRITEOP} are set whether DMA is used or not.
>
> Yes :( Bummer.
>
>> tmio_mmc_cmd_irq() enables TMIO_MASK_{READOP,WRITEOP}, but never
>> disables them.  This does not take care of a case where ->force_pio
>> is set, but unset later.
>
> force_pio gets disabled within the same request? I may be overlooking
> something but I only see code paths where force_pio gets cleared and a
> little later the request gets finished. Can you give me a pointer?

I cleaned up force_pio, but in a separate patch
because it is just loosely related.


>> After all, the correct place to handle those flags is just before
>> starting the data transfer.
>
> While I totally agree the code below can be improved for sure, I'd
> prefer to keep the design pattern that irqs get disabled once they are
> not actively used. Generally spoken, I think it makes sense to keep
> regressions on old platforms low. Any chance this can be achieved?
> Other than that, fixing/removing this irq_mask handling from probe() is
> really good.
>

I could not understand this suggestion.
Please let me know if v3 is still a problem.



-- 
Best Regards
Masahiro Yamada

[PATCH v2 18/22] mmc: tmio: remove useless TMIO_MASK_CMD handling in tmio_mmc_host_probe()

[Masahiro Yamada]

TMIO_MASK_CMD is properly enabled in tmio_mmc_start_command().

We have no reason to set it up in tmio_mmc_host_probe().  (If we
really wanted to set it in the probe, we would have to do likewise
when resuming.)

Even worse, the following code is extremely confusing:

  _host->sdcard_irq_mask &= ~irq_mask;

The logic is opposite between "->sdcard_irq_mask" and "irq_mask".
The intention is not clear at a glance.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/tmio_mmc_core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 345e379..f5b11b8 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1194,7 +1194,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	struct mmc_host *mmc = _host->mmc;
 	struct resource *res_ctl;
 	int ret;
-	u32 irq_mask = TMIO_MASK_CMD;
 
 	tmio_mmc_of_parse(pdev, pdata);
 
@@ -1289,8 +1288,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, CTL_IRQ_MASK);
 	tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
 
-	_host->sdcard_irq_mask &= ~irq_mask;
-
 	if (_host->native_hotplug)
 		tmio_mmc_enable_mmc_irqs(_host,
 				TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT);
-- 
2.7.4

Re: [PATCH v2 18/22] mmc: tmio: remove useless TMIO_MASK_CMD handling in tmio_mmc_host_probe()

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:53AM +0900, Masahiro Yamada wrote:
> TMIO_MASK_CMD is properly enabled in tmio_mmc_start_command().
> 
> We have no reason to set it up in tmio_mmc_host_probe().  (If we
> really wanted to set it in the probe, we would have to do likewise
> when resuming.)
> 
> Even worse, the following code is extremely confusing:
> 
>   _host->sdcard_irq_mask &= ~irq_mask;
> 
> The logic is opposite between "->sdcard_irq_mask" and "irq_mask".
> The intention is not clear at a glance.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

In general, I like it a lot. It just depends on the previous patches
which are still subject to discussion.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 19/22] mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()

[Masahiro Yamada]

The register region is ioremap'ed in the tmio_mmc_host_probe(), i.e.
drivers cannot get access to the hardware before mmc_add_host().

Actually, renesas_sdhi_core.c reads out the CTL_VERSION register to
complete the platform-specific settings.  However, at this point,
the MMC host is already running.

Move the register ioremap to tmio_mmc_host_alloc() so that drivers
can perform platform-specific settings between tmio_mmc_host_alloc()
and tmio_mmc_host_probe().

I changed tmio_mmc_host_alloc() to return an error pointer to
propagate the return code from devm_ioremap_resource().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/renesas_sdhi_core.c |  4 ++--
 drivers/mmc/host/tmio_mmc.c          |  4 +++-
 drivers/mmc/host/tmio_mmc_core.c     | 16 +++++++++-------
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index e78dca7..446ff54 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -511,8 +511,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	}
 
 	host = tmio_mmc_host_alloc(pdev);
-	if (!host)
-		return -ENOMEM;
+	if (IS_ERR(host))
+		return PTR_ERR(host);
 
 	if (of_data) {
 		mmc_data->flags |= of_data->tmio_flags;
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index ccfbc15..d660816 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -93,8 +93,10 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 	pdata->flags |= TMIO_MMC_HAVE_HIGH_REG;
 
 	host = tmio_mmc_host_alloc(pdev);
-	if (!host)
+	if (IS_ERR(host)) {
+		ret = PTR_ERR(host);
 		goto cell_disable;
+	}
 
 	/* SD control register space size is 0x200, 0x400 for bus_shift=1 */
 	host->bus_shift = resource_size(res) >> 10;
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index f5b11b8..c392694 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1165,12 +1165,20 @@ tmio_mmc_host_alloc(struct platform_device *pdev)
 {
 	struct tmio_mmc_host *host;
 	struct mmc_host *mmc;
+	struct resource *res;
+	void __iomem *ctl;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ctl = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ctl))
+		return ERR_CAST(ctl);
 
 	mmc = mmc_alloc_host(sizeof(struct tmio_mmc_host), &pdev->dev);
 	if (!mmc)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	host = mmc_priv(mmc);
+	host->ctl = ctl;
 	host->mmc = mmc;
 	host->pdev = pdev;
 	host->ops = tmio_mmc_ops;
@@ -1192,7 +1200,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 {
 	struct platform_device *pdev = _host->pdev;
 	struct mmc_host *mmc = _host->mmc;
-	struct resource *res_ctl;
 	int ret;
 
 	tmio_mmc_of_parse(pdev, pdata);
@@ -1200,11 +1207,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
 		_host->write16_hook = NULL;
 
-	res_ctl = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	_host->ctl = devm_ioremap_resource(&pdev->dev, res_ctl);
-	if (IS_ERR(_host->ctl))
-		return PTR_ERR(_host->ctl);
-
 	ret = mmc_of_parse(mmc);
 	if (ret < 0)
 		return ret;
-- 
2.7.4

Re: [PATCH v2 19/22] mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:54AM +0900, Masahiro Yamada wrote:
> The register region is ioremap'ed in the tmio_mmc_host_probe(), i.e.
> drivers cannot get access to the hardware before mmc_add_host().
> 
> Actually, renesas_sdhi_core.c reads out the CTL_VERSION register to
> complete the platform-specific settings.  However, at this point,
> the MMC host is already running.
> 
> Move the register ioremap to tmio_mmc_host_alloc() so that drivers
> can perform platform-specific settings between tmio_mmc_host_alloc()
> and tmio_mmc_host_probe().
> 
> I changed tmio_mmc_host_alloc() to return an error pointer to
> propagate the return code from devm_ioremap_resource().
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 20/22] mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()

[Masahiro Yamada]

The clock is enabled in the tmio_mmc_host_probe().  It also prevents
drivers from performing platform-specific settings before mmc_add_host()
because the register access generally requires a clock.

Enable/disable the clock in drivers' probe/remove.  Also, I passed
tmio_mmc_data to tmio_mmc_host_alloc() because renesas_sdhi_clk_enable()
needs it to get the private data from tmio_mmc_host.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/renesas_sdhi_core.c | 13 ++++++++++---
 drivers/mmc/host/tmio_mmc.c          |  7 +++++--
 drivers/mmc/host/tmio_mmc.h          |  4 ++--
 drivers/mmc/host/tmio_mmc_core.c     | 33 +++++++++++++--------------------
 4 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 446ff54..a5749d7 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -510,7 +510,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 						"state_uhs");
 	}
 
-	host = tmio_mmc_host_alloc(pdev);
+	host = tmio_mmc_host_alloc(pdev, mmc_data);
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
@@ -570,10 +570,14 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	/* All SDHI have SDIO status bits which must be 1 */
 	mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS;
 
-	ret = tmio_mmc_host_probe(host, mmc_data, dma_ops);
-	if (ret < 0)
+	ret = renesas_sdhi_clk_enable(host);
+	if (ret)
 		goto efree;
 
+	ret = tmio_mmc_host_probe(host, dma_ops);
+	if (ret < 0)
+		goto edisclk;
+
 	/* One Gen2 SDHI incarnation does NOT have a CBSY bit */
 	if (sd_ctrl_read16(host, CTL_VERSION) == SDHI_VER_GEN2_SDR50)
 		mmc_data->flags &= ~TMIO_MMC_HAVE_CBSY;
@@ -634,6 +638,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
 eirq:
 	tmio_mmc_host_remove(host);
+edisclk:
+	renesas_sdhi_clk_disable(host);
 efree:
 	tmio_mmc_host_free(host);
 
@@ -646,6 +652,7 @@ int renesas_sdhi_remove(struct platform_device *pdev)
 	struct tmio_mmc_host *host = platform_get_drvdata(pdev);
 
 	tmio_mmc_host_remove(host);
+	renesas_sdhi_clk_disable(host);
 
 	return 0;
 }
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index d660816..11b87ce 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -92,7 +92,7 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 
 	pdata->flags |= TMIO_MMC_HAVE_HIGH_REG;
 
-	host = tmio_mmc_host_alloc(pdev);
+	host = tmio_mmc_host_alloc(pdev, pdata);
 	if (IS_ERR(host)) {
 		ret = PTR_ERR(host);
 		goto cell_disable;
@@ -101,7 +101,10 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 	/* SD control register space size is 0x200, 0x400 for bus_shift=1 */
 	host->bus_shift = resource_size(res) >> 10;
 
-	ret = tmio_mmc_host_probe(host, pdata, NULL);
+	host->mmc->f_max = pdata->hclk;
+	host->mmc->f_min = pdata->hclk / 512;
+
+	ret = tmio_mmc_host_probe(host, NULL);
 	if (ret)
 		goto host_free;
 
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 27894474..f46d282 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -195,10 +195,10 @@ struct tmio_mmc_host {
 	const struct tmio_mmc_dma_ops *dma_ops;
 };
 
-struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
+struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev,
+					  struct tmio_mmc_data *pdata);
 void tmio_mmc_host_free(struct tmio_mmc_host *host);
 int tmio_mmc_host_probe(struct tmio_mmc_host *host,
-			struct tmio_mmc_data *pdata,
 			const struct tmio_mmc_dma_ops *dma_ops);
 void tmio_mmc_host_remove(struct tmio_mmc_host *host);
 void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index c392694..b51486d 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1160,8 +1160,8 @@ static void tmio_mmc_of_parse(struct platform_device *pdev,
 		pdata->flags |= TMIO_MMC_WRPROTECT_DISABLE;
 }
 
-struct tmio_mmc_host*
-tmio_mmc_host_alloc(struct platform_device *pdev)
+struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev,
+					  struct tmio_mmc_data *pdata)
 {
 	struct tmio_mmc_host *host;
 	struct mmc_host *mmc;
@@ -1181,9 +1181,12 @@ tmio_mmc_host_alloc(struct platform_device *pdev)
 	host->ctl = ctl;
 	host->mmc = mmc;
 	host->pdev = pdev;
+	host->pdata = pdata;
 	host->ops = tmio_mmc_ops;
 	mmc->ops = &host->ops;
 
+	platform_set_drvdata(pdev, host);
+
 	return host;
 }
 EXPORT_SYMBOL_GPL(tmio_mmc_host_alloc);
@@ -1195,13 +1198,20 @@ void tmio_mmc_host_free(struct tmio_mmc_host *host)
 EXPORT_SYMBOL_GPL(tmio_mmc_host_free);
 
 int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
-			struct tmio_mmc_data *pdata,
 			const struct tmio_mmc_dma_ops *dma_ops)
 {
 	struct platform_device *pdev = _host->pdev;
+	struct tmio_mmc_data *pdata = _host->pdata;
 	struct mmc_host *mmc = _host->mmc;
 	int ret;
 
+	/*
+	 * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() from
+	 * looping forever...
+	 */
+	if (mmc->f_min == 0)
+		return -EINVAL;
+
 	tmio_mmc_of_parse(pdev, pdata);
 
 	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
@@ -1211,9 +1221,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (ret < 0)
 		return ret;
 
-	_host->pdata = pdata;
-	platform_set_drvdata(pdev, _host);
-
 	_host->set_pwr = pdata->set_pwr;
 	_host->set_clk_div = pdata->set_clk_div;
 
@@ -1261,18 +1268,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (pdata->flags & TMIO_MMC_MIN_RCAR2)
 		_host->native_hotplug = true;
 
-	if (tmio_mmc_clk_enable(_host) < 0) {
-		mmc->f_max = pdata->hclk;
-		mmc->f_min = mmc->f_max / 512;
-	}
-
-	/*
-	 * Check the sanity of mmc->f_min to prevent tmio_mmc_set_clock() from
-	 * looping forever...
-	 */
-	if (mmc->f_min == 0)
-		return -EINVAL;
-
 	/*
 	 * While using internal tmio hardware logic for card detection, we need
 	 * to ensure it stays powered for it to work.
@@ -1344,8 +1339,6 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-
-	tmio_mmc_clk_disable(host);
 }
 EXPORT_SYMBOL_GPL(tmio_mmc_host_remove);
 
-- 
2.7.4

Re: [PATCH v2 20/22] mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:55AM +0900, Masahiro Yamada wrote:
> The clock is enabled in the tmio_mmc_host_probe().  It also prevents
> drivers from performing platform-specific settings before mmc_add_host()
> because the register access generally requires a clock.
> 
> Enable/disable the clock in drivers' probe/remove.  Also, I passed
> tmio_mmc_data to tmio_mmc_host_alloc() because renesas_sdhi_clk_enable()
> needs it to get the private data from tmio_mmc_host.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 21/22] mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()

[Masahiro Yamada]

mmc_of_parse() parses various DT properties and sets capability flags
accordingly.  However, drivers have no chance to run platform init
code depending on such flags because mmc_of_parse() is called from
tmio_mmc_host_probe().

Move mmc_of_parse() to tmio_mmc_host_alloc() so that drivers can
handle capabilities before mmc_add_host().  Move tmio_mmc_of_parse()
likewise.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/tmio_mmc_core.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index b51486d..02e1438 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1167,6 +1167,7 @@ struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev,
 	struct mmc_host *mmc;
 	struct resource *res;
 	void __iomem *ctl;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	ctl = devm_ioremap_resource(&pdev->dev, res);
@@ -1185,9 +1186,21 @@ struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev,
 	host->ops = tmio_mmc_ops;
 	mmc->ops = &host->ops;
 
+	ret = mmc_of_parse(host->mmc);
+	if (ret) {
+		host = ERR_PTR(ret);
+		goto free;
+	}
+
+	tmio_mmc_of_parse(pdev, pdata);
+
 	platform_set_drvdata(pdev, host);
 
 	return host;
+free:
+	mmc_free_host(mmc);
+
+	return host;
 }
 EXPORT_SYMBOL_GPL(tmio_mmc_host_alloc);
 
@@ -1212,15 +1225,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	if (mmc->f_min == 0)
 		return -EINVAL;
 
-	tmio_mmc_of_parse(pdev, pdata);
-
 	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
 		_host->write16_hook = NULL;
 
-	ret = mmc_of_parse(mmc);
-	if (ret < 0)
-		return ret;
-
 	_host->set_pwr = pdata->set_pwr;
 	_host->set_clk_div = pdata->set_clk_div;
 
-- 
2.7.4

Re: [PATCH v2 21/22] mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:56AM +0900, Masahiro Yamada wrote:
> mmc_of_parse() parses various DT properties and sets capability flags
> accordingly.  However, drivers have no chance to run platform init
> code depending on such flags because mmc_of_parse() is called from
> tmio_mmc_host_probe().
> 
> Move mmc_of_parse() to tmio_mmc_host_alloc() so that drivers can
> handle capabilities before mmc_add_host().  Move tmio_mmc_of_parse()
> likewise.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 22/22] mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument

[Masahiro Yamada]

Drivers need to set up various struct members for tmio_mmc_host before
calling tmio_mmc_host_probe().  Do likewise for host->dma_ops instead
of passing it as a function argument.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2:
  - Newly added

 drivers/mmc/host/renesas_sdhi_core.c | 3 ++-
 drivers/mmc/host/tmio_mmc.c          | 2 +-
 drivers/mmc/host/tmio_mmc.h          | 3 +--
 drivers/mmc/host/tmio_mmc_core.c     | 4 +---
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index a5749d7..825378a 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -531,6 +531,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	host->clk_update	= renesas_sdhi_clk_update;
 	host->clk_disable	= renesas_sdhi_clk_disable;
 	host->multi_io_quirk	= renesas_sdhi_multi_io_quirk;
+	host->dma_ops		= dma_ops;
 
 	/* SDR speeds are only available on Gen2+ */
 	if (mmc_data->flags & TMIO_MMC_MIN_RCAR2) {
@@ -574,7 +575,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (ret)
 		goto efree;
 
-	ret = tmio_mmc_host_probe(host, dma_ops);
+	ret = tmio_mmc_host_probe(host);
 	if (ret < 0)
 		goto edisclk;
 
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index 11b87ce..43a2ea5 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -104,7 +104,7 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 	host->mmc->f_max = pdata->hclk;
 	host->mmc->f_min = pdata->hclk / 512;
 
-	ret = tmio_mmc_host_probe(host, NULL);
+	ret = tmio_mmc_host_probe(host);
 	if (ret)
 		goto host_free;
 
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index f46d282..7609434 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -198,8 +198,7 @@ struct tmio_mmc_host {
 struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev,
 					  struct tmio_mmc_data *pdata);
 void tmio_mmc_host_free(struct tmio_mmc_host *host);
-int tmio_mmc_host_probe(struct tmio_mmc_host *host,
-			const struct tmio_mmc_dma_ops *dma_ops);
+int tmio_mmc_host_probe(struct tmio_mmc_host *host);
 void tmio_mmc_host_remove(struct tmio_mmc_host *host);
 void tmio_mmc_do_data_irq(struct tmio_mmc_host *host);
 
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 02e1438..1d60a41 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1210,8 +1210,7 @@ void tmio_mmc_host_free(struct tmio_mmc_host *host)
 }
 EXPORT_SYMBOL_GPL(tmio_mmc_host_free);
 
-int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
-			const struct tmio_mmc_dma_ops *dma_ops)
+int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
 {
 	struct platform_device *pdev = _host->pdev;
 	struct tmio_mmc_data *pdata = _host->pdata;
@@ -1304,7 +1303,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	INIT_WORK(&_host->done, tmio_mmc_done_work);
 
 	/* See if we also get DMA */
-	_host->dma_ops = dma_ops;
 	tmio_mmc_request_dma(_host, pdata);
 
 	pm_runtime_set_active(&pdev->dev);
-- 
2.7.4

Re: [PATCH v2 22/22] mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument

[Wolfram Sang]

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

On Sat, Nov 25, 2017 at 01:24:57AM +0900, Masahiro Yamada wrote:
> Drivers need to set up various struct members for tmio_mmc_host before
> calling tmio_mmc_host_probe().  Do likewise for host->dma_ops instead
> of passing it as a function argument.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

I think I have done all the review by now. I'll do some more regression
testing tomorrow and then give Tested-by as well if everything works
fine (but it looks like it so far).


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

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


> I am getting more and more patches for TMIO.
> I put all in a single series to clarify the patch order.

Hmmm, that makes reviewing harder in my opinion.

I really liked the small serieses which could be improved, acked, and
applied. This big one will take much longer to review and resending will
cause quite some noise.

For clarifying patch order in small series, it should be enough to state
the dependencies in the cover letter. Super helpful is providing a
public git branch where ppl can pull from.

Also, can you state what kind of testing you did on what platforms? My
gut feeling says that the card detect changes might be easy to catch
regressions on old platforms, so we'd need to be extra cautious. I/we
can do testing on some Renesas platforms, too.

Other changes like moving stuff from tmio_core to renesas_sdhi is less
critical. Either it works or not, so that is easy to spot :)

Which might be another argument for multiple smaller patch series.

Nonetheless, I'll try to get this reviewed in the next days...

All the best,

   Wolfram


> 
> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
> 
> 
> Masahiro Yamada (22):
>   mmc: renesas_sdhi: consolidate DMAC CONFIG options
>   mmc: renesas_sdhi: remove wrong depends on to enable compile test
>   mmc: renesas_sdhi: remove eprobe jump label
>   mmc: tmio: set tmio_mmc_host to driver data
>   mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>   mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>   mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>   mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>   mmc: tmio: use mmc_can_gpio_cd() instead of checking
>     TMIO_MMC_USE_GPIO_CD
>   mmc: tmio: support IP-builtin card detection logic
>   mmc: renesas_sdhi: remove always false condition
>   mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>   mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>     renesas_sdhi.h
>   mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>   mmc: tmio: change bus_shift to unsigned int
>   mmc: tmio: fix never-detected card insertion bug
>   mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>   mmc: tmio: remove useless TMIO_MASK_CMD handling in
>     tmio_mmc_host_probe()
>   mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>   mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>   mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>   mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
> 
>  drivers/mmc/host/Kconfig                      |   5 +-
>  drivers/mmc/host/Makefile                     |   8 +-
>  drivers/mmc/host/renesas_sdhi.h               |  22 ++++
>  drivers/mmc/host/renesas_sdhi_core.c          |  49 ++++-----
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  35 +++---
>  drivers/mmc/host/tmio_mmc.c                   |  23 ++--
>  drivers/mmc/host/tmio_mmc.h                   |  23 +---
>  drivers/mmc/host/tmio_mmc_core.c              | 149 +++++++++++++-------------
>  9 files changed, 170 insertions(+), 158 deletions(-)
> 
> -- 
> 2.7.4
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

Hi Wolfram,


2017-11-28 2:13 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
>
>> I am getting more and more patches for TMIO.
>> I put all in a single series to clarify the patch order.
>
> Hmmm, that makes reviewing harder in my opinion.


I think this depends on people.

You are the maintainer of TMIO MMC, so
I will try my best to adjust to your opinion next time.



> I really liked the small serieses which could be improved, acked, and
> applied. This big one will take much longer to review and resending will
> cause quite some noise.
>
> For clarifying patch order in small series, it should be enough to state
> the dependencies in the cover letter. Super helpful is providing a
> public git branch where ppl can pull from.
>
> Also, can you state what kind of testing you did on what platforms?

I have no Renesas boards.

I tested this series on UniPhier boards.
(arch/arm/boot/dts/uniphier*.dts,  arch/arm64/boot/dts/socionext/uniphier*.dts)


> My
> gut feeling says that the card detect changes might be easy to catch
> regressions on old platforms, so we'd need to be extra cautious. I/we
> can do testing on some Renesas platforms, too.
>
> Other changes like moving stuff from tmio_core to renesas_sdhi is less
> critical. Either it works or not, so that is easy to spot :)
>
> Which might be another argument for multiple smaller patch series.
>
> Nonetheless, I'll try to get this reviewed in the next days...

Thanks!


> All the best,
>
>    Wolfram
>
>
-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 24 November 2017 at 17:24, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> I am working on this IP for Socionext SoCs.
>
> I was hit by several issues, and noticed various
> clean-up candidates.
>
>  - Fix and clean-up Kconfig
>  - Fix various card detection problems
>  - Move Renesas private data out of TMIO core
>  - Allow to perform platform-specific settings before MMC host starts
>  - Fix weird IRQ handling
>
> I am getting more and more patches for TMIO.
> I put all in a single series to clarify the patch order.
>
> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>
>
> Masahiro Yamada (22):
>   mmc: renesas_sdhi: consolidate DMAC CONFIG options
>   mmc: renesas_sdhi: remove wrong depends on to enable compile test
>   mmc: renesas_sdhi: remove eprobe jump label
>   mmc: tmio: set tmio_mmc_host to driver data
>   mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>   mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>   mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>   mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>   mmc: tmio: use mmc_can_gpio_cd() instead of checking
>     TMIO_MMC_USE_GPIO_CD
>   mmc: tmio: support IP-builtin card detection logic
>   mmc: renesas_sdhi: remove always false condition
>   mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>   mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>     renesas_sdhi.h
>   mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>   mmc: tmio: change bus_shift to unsigned int
>   mmc: tmio: fix never-detected card insertion bug
>   mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>   mmc: tmio: remove useless TMIO_MASK_CMD handling in
>     tmio_mmc_host_probe()
>   mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>   mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>   mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>   mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>
>  drivers/mmc/host/Kconfig                      |   5 +-
>  drivers/mmc/host/Makefile                     |   8 +-
>  drivers/mmc/host/renesas_sdhi.h               |  22 ++++
>  drivers/mmc/host/renesas_sdhi_core.c          |  49 ++++-----
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  35 +++---
>  drivers/mmc/host/tmio_mmc.c                   |  23 ++--
>  drivers/mmc/host/tmio_mmc.h                   |  23 +---
>  drivers/mmc/host/tmio_mmc_core.c              | 149 +++++++++++++-------------
>  9 files changed, 170 insertions(+), 158 deletions(-)
>
> --
> 2.7.4
>

To get this moving, I have applied patch 1->8 for next, thanks!

Kind regards
Uffe

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

2017-12-15 18:18 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 24 November 2017 at 17:24, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>>
>> I am working on this IP for Socionext SoCs.
>>
>> I was hit by several issues, and noticed various
>> clean-up candidates.
>>
>>  - Fix and clean-up Kconfig
>>  - Fix various card detection problems
>>  - Move Renesas private data out of TMIO core
>>  - Allow to perform platform-specific settings before MMC host starts
>>  - Fix weird IRQ handling
>>
>> I am getting more and more patches for TMIO.
>> I put all in a single series to clarify the patch order.
>>
>> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>>
>>
>> Masahiro Yamada (22):
>>   mmc: renesas_sdhi: consolidate DMAC CONFIG options
>>   mmc: renesas_sdhi: remove wrong depends on to enable compile test
>>   mmc: renesas_sdhi: remove eprobe jump label
>>   mmc: tmio: set tmio_mmc_host to driver data
>>   mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>>   mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>>   mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>>   mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>>   mmc: tmio: use mmc_can_gpio_cd() instead of checking
>>     TMIO_MMC_USE_GPIO_CD
>>   mmc: tmio: support IP-builtin card detection logic
>>   mmc: renesas_sdhi: remove always false condition
>>   mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>>   mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>>     renesas_sdhi.h
>>   mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>>   mmc: tmio: change bus_shift to unsigned int
>>   mmc: tmio: fix never-detected card insertion bug
>>   mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>>   mmc: tmio: remove useless TMIO_MASK_CMD handling in
>>     tmio_mmc_host_probe()
>>   mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>>   mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>>   mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>>   mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>>
>>  drivers/mmc/host/Kconfig                      |   5 +-
>>  drivers/mmc/host/Makefile                     |   8 +-
>>  drivers/mmc/host/renesas_sdhi.h               |  22 ++++
>>  drivers/mmc/host/renesas_sdhi_core.c          |  49 ++++-----
>>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
>>  drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  35 +++---
>>  drivers/mmc/host/tmio_mmc.c                   |  23 ++--
>>  drivers/mmc/host/tmio_mmc.h                   |  23 +---
>>  drivers/mmc/host/tmio_mmc_core.c              | 149 +++++++++++++-------------
>>  9 files changed, 170 insertions(+), 158 deletions(-)
>>
>> --
>> 2.7.4
>>
>
> To get this moving, I have applied patch 1->8 for next, thanks!
>
> Kind regards
> Uffe



After 2, COMPILE_TEST will work correctly.

Then, Wolfram mentioned we would need to include <linux/io.h> from tmio_mmc.h

https://patchwork.kernel.org/patch/10074333/


I was waiting for a patch from him.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

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


> After 2, COMPILE_TEST will work correctly.
> 
> Then, Wolfram mentioned we would need to include <linux/io.h> from tmio_mmc.h
> 
> https://patchwork.kernel.org/patch/10074333/
> 
> 
> I was waiting for a patch from him.

Yes, I am sorry. I am currently swamped with I2C work, not so much time
for SDHI. However, I did this patch now and pushed the branch out to see
if buildbot is happy.

Ulf, this patch then in deed should ideally be applied before 1-8 here.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 15 December 2017 at 12:12, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> After 2, COMPILE_TEST will work correctly.
>>
>> Then, Wolfram mentioned we would need to include <linux/io.h> from tmio_mmc.h
>>
>> https://patchwork.kernel.org/patch/10074333/
>>
>>
>> I was waiting for a patch from him.
>
> Yes, I am sorry. I am currently swamped with I2C work, not so much time
> for SDHI. However, I did this patch now and pushed the branch out to see
> if buildbot is happy.

Great!

>
> Ulf, this patch then in deed should ideally be applied before 1-8 here.

Okay, once you post it to linux-mmc I will pick it up, and put it in front.

Kind regards
Uffe

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

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


> > Ulf, this patch then in deed should ideally be applied before 1-8 here.
> 
> Okay, once you post it to linux-mmc I will pick it up, and put it in front.

Bad news, that patch didn't help. The problem is that sparc64 doesn't
include 'asm-generic/io.h' and also has no own 'readsw'. No surprise
that just adding this include will cause lots of redefines which are way
too much to handle as a side-task.

So, the best option I see here is to drop COMPILE_TEST for now, report
this to the sparc64 maintainers (I assume they know already), and see if
they can fix it.

Other thoughts?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 15 December 2017 at 17:30, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> > Ulf, this patch then in deed should ideally be applied before 1-8 here.
>>
>> Okay, once you post it to linux-mmc I will pick it up, and put it in front.
>
> Bad news, that patch didn't help. The problem is that sparc64 doesn't
> include 'asm-generic/io.h' and also has no own 'readsw'. No surprise
> that just adding this include will cause lots of redefines which are way
> too much to handle as a side-task.
>
> So, the best option I see here is to drop COMPILE_TEST for now, report
> this to the sparc64 maintainers (I assume they know already), and see if
> they can fix it.

Okay!

If some of you send a patch on top, I can fold it into the offending commit.

>
> Other thoughts?

Nope.

Kind regards
Uffe

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Wolfram Sang]

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


> > Other thoughts?
> 
> Nope.

I have :) Another solution seems to be to switch to ioread16_rep and
friends. Those are defined on sparc64. Compiles locally here, now sent
it to buildbot.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Masahiro Yamada]

Hi Ulf,


2017-12-15 18:18 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 24 November 2017 at 17:24, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>>
>> I am working on this IP for Socionext SoCs.
>>
>> I was hit by several issues, and noticed various
>> clean-up candidates.
>>
>>  - Fix and clean-up Kconfig
>>  - Fix various card detection problems
>>  - Move Renesas private data out of TMIO core
>>  - Allow to perform platform-specific settings before MMC host starts
>>  - Fix weird IRQ handling
>>
>> I am getting more and more patches for TMIO.
>> I put all in a single series to clarify the patch order.
>>
>> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>>
>>
>> Masahiro Yamada (22):
>>   mmc: renesas_sdhi: consolidate DMAC CONFIG options
>>   mmc: renesas_sdhi: remove wrong depends on to enable compile test
>>   mmc: renesas_sdhi: remove eprobe jump label
>>   mmc: tmio: set tmio_mmc_host to driver data
>>   mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>>   mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>>   mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>>   mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>>   mmc: tmio: use mmc_can_gpio_cd() instead of checking
>>     TMIO_MMC_USE_GPIO_CD
>>   mmc: tmio: support IP-builtin card detection logic
>>   mmc: renesas_sdhi: remove always false condition
>>   mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>>   mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>>     renesas_sdhi.h
>>   mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>>   mmc: tmio: change bus_shift to unsigned int
>>   mmc: tmio: fix never-detected card insertion bug
>>   mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>>   mmc: tmio: remove useless TMIO_MASK_CMD handling in
>>     tmio_mmc_host_probe()
>>   mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>>   mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>>   mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>>   mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>>
>>  drivers/mmc/host/Kconfig                      |   5 +-
>>  drivers/mmc/host/Makefile                     |   8 +-
>>  drivers/mmc/host/renesas_sdhi.h               |  22 ++++
>>  drivers/mmc/host/renesas_sdhi_core.c          |  49 ++++-----
>>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
>>  drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  35 +++---
>>  drivers/mmc/host/tmio_mmc.c                   |  23 ++--
>>  drivers/mmc/host/tmio_mmc.h                   |  23 +---
>>  drivers/mmc/host/tmio_mmc_core.c              | 149 +++++++++++++-------------
>>  9 files changed, 170 insertions(+), 158 deletions(-)
>>
>> --
>> 2.7.4
>>
>
> To get this moving, I have applied patch 1->8 for next, thanks!


Could you apply 11->15 as well?
They were reviewed by Wolfram.

We can skip 9, 10.





-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 00/22] mmc: tmio: various fixes and cleanups

[Ulf Hansson]

On 19 December 2017 at 04:56, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Ulf,
>
>
> 2017-12-15 18:18 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 24 November 2017 at 17:24, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>>
>>> I am working on this IP for Socionext SoCs.
>>>
>>> I was hit by several issues, and noticed various
>>> clean-up candidates.
>>>
>>>  - Fix and clean-up Kconfig
>>>  - Fix various card detection problems
>>>  - Move Renesas private data out of TMIO core
>>>  - Allow to perform platform-specific settings before MMC host starts
>>>  - Fix weird IRQ handling
>>>
>>> I am getting more and more patches for TMIO.
>>> I put all in a single series to clarify the patch order.
>>>
>>> 1, 2, 4, 5, 6, 7 were already acked or reviewed by Wolfram Sang.
>>>
>>>
>>> Masahiro Yamada (22):
>>>   mmc: renesas_sdhi: consolidate DMAC CONFIG options
>>>   mmc: renesas_sdhi: remove wrong depends on to enable compile test
>>>   mmc: renesas_sdhi: remove eprobe jump label
>>>   mmc: tmio: set tmio_mmc_host to driver data
>>>   mmc: tmio: use devm_ioremap_resource() instead of devm_ioremap()
>>>   mmc: tmio: move mmc_host_ops to struct tmio_mmc_host from static data
>>>   mmc: tmio, renesas_sdhi: set mmc_host_ops hooks directly
>>>   mmc: tmio: move mmc_gpio_request_cd() before mmc_add_host()
>>>   mmc: tmio: use mmc_can_gpio_cd() instead of checking
>>>     TMIO_MMC_USE_GPIO_CD
>>>   mmc: tmio: support IP-builtin card detection logic
>>>   mmc: renesas_sdhi: remove always false condition
>>>   mmc: tmio,renesas_sdhi: move struct tmio_mmc_dma to renesas_sdhi.h
>>>   mmc: tmio,renesas_sdhi: move Renesas-specific DMA data to
>>>     renesas_sdhi.h
>>>   mmc: tmio,renesas_sdhi: move ssc_tappos to renesas_sdhi.h
>>>   mmc: tmio: change bus_shift to unsigned int
>>>   mmc: tmio: fix never-detected card insertion bug
>>>   mmc: tmio: move TMIO_MASK_{READOP,WRITEOP} handling to correct place
>>>   mmc: tmio: remove useless TMIO_MASK_CMD handling in
>>>     tmio_mmc_host_probe()
>>>   mmc: tmio: ioremap memory resource in tmio_mmc_host_alloc()
>>>   mmc: tmio: move clk_enable/disable out of tmio_mmc_host_probe()
>>>   mmc: tmio: move {tmio_}mmc_of_parse() to tmio_mmc_host_alloc()
>>>   mmc: tmio: remove dma_ops from tmio_mmc_host_probe() argument
>>>
>>>  drivers/mmc/host/Kconfig                      |   5 +-
>>>  drivers/mmc/host/Makefile                     |   8 +-
>>>  drivers/mmc/host/renesas_sdhi.h               |  22 ++++
>>>  drivers/mmc/host/renesas_sdhi_core.c          |  49 ++++-----
>>>  drivers/mmc/host/renesas_sdhi_internal_dmac.c |  14 ++-
>>>  drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  35 +++---
>>>  drivers/mmc/host/tmio_mmc.c                   |  23 ++--
>>>  drivers/mmc/host/tmio_mmc.h                   |  23 +---
>>>  drivers/mmc/host/tmio_mmc_core.c              | 149 +++++++++++++-------------
>>>  9 files changed, 170 insertions(+), 158 deletions(-)
>>>
>>> --
>>> 2.7.4
>>>
>>
>> To get this moving, I have applied patch 1->8 for next, thanks!
>
>
> Could you apply 11->15 as well?
> They were reviewed by Wolfram.

Applied for next!

>
> We can skip 9, 10.

Okay, thought there were a dependency.

Thanks and kind regards
Uffe