[PATCH 0/9]: mmc: tmio header cleanup

[PATCH 0/9]: mmc: tmio header cleanup

[Kuninori Morimoto]

Hi Ulf, Chris, and Simon

These are tmio.h header cleanup patches.
1) - 7) are main cleanups,
8) - 9) are sh_mobile_sdhi specific cleanups.

By these patches,
tmio driver specific data/callbacks are implemented under tmio_mmc_host,
platform    specific data/callbacks are implemented under tmio_mmc_data.

I guess this is Ulf's wanted (?)
Please let me know if it is not enough.
I will re-send my previous sh_mobile_sdhi DMA cleanups patches
if these are accepted.

Simon, 8) - 9) are shmobile specific patches.
Can you please give your Ack for these patches ?

Kuninori Morimoto (9):
      1) mmc: tmio: add tmio_mmc_host_alloc/free()
      2) mmc: tmio: tmio_mmc_host has .dma
      3) mmc: tmio: tmio_mmc_host has .write16_hook
      4) mmc: tmio: tmio_mmc_host has .clk_enable
      5) mmc: tmio: tmio_mmc_host has .clk_disable
      6) mmc: tmio: tmio_mmc_host has .multi_io_quirk
      7) mmc: tmio: tmio_mmc_host has .bus_shift
      8) mmc: sh_mobile_sdhi: remove .init/.cleanup
      9) mmc: sh_mobile_sdhi: remove sh_mobile_sdhi_info

 arch/arm/mach-shmobile/board-ape6evm.c         |   12 ++---
 arch/arm/mach-shmobile/board-armadillo800eva.c |   12 ++---
 arch/arm/mach-shmobile/board-bockw.c           |   10 ++--
 arch/arm/mach-shmobile/board-kzm9g.c           |   12 ++---
 arch/arm/mach-shmobile/board-lager.c           |   16 +++---
 arch/arm/mach-shmobile/board-mackerel.c        |   14 +++---
 arch/arm/mach-shmobile/board-marzen.c          |    6 +--
 arch/sh/boards/board-sh7757lcr.c               |    5 +-
 arch/sh/boards/mach-ap325rxa/setup.c           |    9 ++--
 arch/sh/boards/mach-ecovec24/setup.c           |   12 ++---
 arch/sh/boards/mach-kfr2r09/setup.c            |    6 +--
 arch/sh/boards/mach-migor/setup.c              |    5 +-
 arch/sh/boards/mach-se/7724/setup.c            |    9 ++--
 drivers/mmc/host/sh_mobile_sdhi.c              |   64 ++++++++----------------
 drivers/mmc/host/tmio_mmc.c                    |   15 ++++--
 drivers/mmc/host/tmio_mmc.h                    |   41 +++++++++++----
 drivers/mmc/host/tmio_mmc_dma.c                |   26 +++++-----
 drivers/mmc/host/tmio_mmc_pio.c                |   58 ++++++++++++---------
 include/linux/mfd/tmio.h                       |   23 +--------
 include/linux/mmc/sh_mobile_sdhi.h             |   25 ---------
 20 files changed, 176 insertions(+), 204 deletions(-)



Best regards
---
Kuninori Morimoto

[PATCH 1/9] mmc: tmio: add tmio_mmc_host_alloc/free()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current tmio_mmc driver is using tmio_mmc_data for driver/platform
specific data/callback, and it is needed for tmio_mmc_host_probe()
function. Because of this style, include/linux/mfd/tmio.h header has
tmio driver/framework specific data which is not needed from platform.

This patch adds new tmio_mmc_host_alloc/free() as cleanup preparation.
tmio driver specific data/callback will be implemented in tmio_mmc_host,
and platform specific data/callback will be implemented in tmio_mmc_data
in this cleanup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   12 ++++++++---
 drivers/mmc/host/tmio_mmc.c       |   10 +++++++--
 drivers/mmc/host/tmio_mmc.h       |    5 +++--
 drivers/mmc/host/tmio_mmc_pio.c   |   42 ++++++++++++++++++++++++-------------
 include/linux/mfd/tmio.h          |    1 -
 5 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 00c8ebd..b2cec5b 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -113,7 +113,7 @@ static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
 		udelay(1);
 
 	if (!timeout) {
-		dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
+		dev_warn(&host->pdev->dev, "timeout waiting for SD bus idle\n");
 		return -EBUSY;
 	}
 
@@ -207,6 +207,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eclkget;
 	}
 
+	host = tmio_mmc_host_alloc(pdev);
+	if (!host)
+		goto eprobe;
+
 	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
@@ -274,9 +278,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
 	mmc_data->bus_shift = resource_size(res) >> 9;
 
-	ret = tmio_mmc_host_probe(&host, pdev, mmc_data);
+	ret = tmio_mmc_host_probe(host, mmc_data);
 	if (ret < 0)
-		goto eprobe;
+		goto efree;
 
 	/*
 	 * FIXME:
@@ -351,6 +355,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 eirq:
 	tmio_mmc_host_remove(host);
+efree:
+	tmio_mmc_host_free(host);
 eprobe:
 eclkget:
 	if (p && p->cleanup)
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index 2616fdf..f47ae08 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -92,10 +92,14 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 	pdata->bus_shift = resource_size(res) >> 10;
 	pdata->flags |= TMIO_MMC_HAVE_HIGH_REG;
 
-	ret = tmio_mmc_host_probe(&host, pdev, pdata);
-	if (ret)
+	host = tmio_mmc_host_alloc(pdev);
+	if (!host)
 		goto cell_disable;
 
+	ret = tmio_mmc_host_probe(host, pdata);
+	if (ret)
+		goto host_free;
+
 	ret = request_irq(irq, tmio_mmc_irq, IRQF_TRIGGER_FALLING,
 				dev_name(&pdev->dev), host);
 	if (ret)
@@ -108,6 +112,8 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 
 host_remove:
 	tmio_mmc_host_remove(host);
+host_free:
+	tmio_mmc_host_free(host);
 cell_disable:
 	if (cell->disable)
 		cell->disable(pdev);
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index a34ecbe..3d02a3c1 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -85,8 +85,9 @@ struct tmio_mmc_host {
 	bool			sdio_irq_enabled;
 };
 
-int tmio_mmc_host_probe(struct tmio_mmc_host **host,
-			struct platform_device *pdev,
+struct tmio_mmc_host* tmio_mmc_host_alloc(struct platform_device *pdev);
+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);
 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_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 250bf8c..580d00b 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1054,12 +1054,35 @@ static void tmio_mmc_of_parse(struct platform_device *pdev,
 		pdata->flags |= TMIO_MMC_WRPROTECT_DISABLE;
 }
 
-int tmio_mmc_host_probe(struct tmio_mmc_host **host,
-				  struct platform_device *pdev,
-				  struct tmio_mmc_data *pdata)
+struct tmio_mmc_host*
+tmio_mmc_host_alloc(struct platform_device *pdev)
 {
-	struct tmio_mmc_host *_host;
+	struct tmio_mmc_host *host;
 	struct mmc_host *mmc;
+
+	mmc = mmc_alloc_host(sizeof(struct tmio_mmc_host), &pdev->dev);
+	if (!mmc)
+		return NULL;
+
+	host = mmc_priv(mmc);
+	host->mmc = mmc;
+	host->pdev = pdev;
+
+	return host;
+}
+
+void tmio_mmc_host_free(struct tmio_mmc_host *host)
+{
+	mmc_free_host(host->mmc);
+
+	host->mmc = NULL;
+}
+
+int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
+			struct tmio_mmc_data *pdata)
+{
+	struct platform_device *pdev = _host->pdev;
+	struct mmc_host *mmc = _host->mmc;
 	struct resource *res_ctl;
 	int ret;
 	u32 irq_mask = TMIO_MASK_CMD;
@@ -1073,19 +1096,11 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	if (!res_ctl)
 		return -EINVAL;
 
-	mmc = mmc_alloc_host(sizeof(struct tmio_mmc_host), &pdev->dev);
-	if (!mmc)
-		return -ENOMEM;
-
 	ret = mmc_of_parse(mmc);
 	if (ret < 0)
 		goto host_free;
 
-	pdata->dev = &pdev->dev;
-	_host = mmc_priv(mmc);
 	_host->pdata = pdata;
-	_host->mmc = mmc;
-	_host->pdev = pdev;
 	platform_set_drvdata(pdev, mmc);
 
 	_host->set_pwr = pdata->set_pwr;
@@ -1192,8 +1207,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 		mmc_gpiod_request_cd_irq(mmc);
 	}
 
-	*host = _host;
-
 	return 0;
 
 host_free:
@@ -1222,7 +1235,6 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 	pm_runtime_disable(&pdev->dev);
 
 	iounmap(host->ctl);
-	mmc_free_host(mmc);
 }
 EXPORT_SYMBOL(tmio_mmc_host_remove);
 
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 5738817..c7d9af0 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -135,7 +135,6 @@ struct tmio_mmc_data {
 	unsigned long			bus_shift;
 	u32				ocr_mask;	/* available voltages */
 	struct tmio_mmc_dma		*dma;
-	struct device			*dev;
 	unsigned int			cd_gpio;
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
-- 
1.7.9.5

Re: [PATCH 1/9] mmc: tmio: add tmio_mmc_host_alloc/free()

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, Jan 5, 2015 at 8:02 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -1054,12 +1054,35 @@ static void tmio_mmc_of_parse(struct platform_device *pdev,
>                 pdata->flags |= TMIO_MMC_WRPROTECT_DISABLE;
>  }
>
> -int tmio_mmc_host_probe(struct tmio_mmc_host **host,
> -                                 struct platform_device *pdev,
> -                                 struct tmio_mmc_data *pdata)
> +struct tmio_mmc_host*
> +tmio_mmc_host_alloc(struct platform_device *pdev)

I think it would be better to match the naming in the mmc core layer, i.e.
tmio_mmc_alloc_host().

>  {
> -       struct tmio_mmc_host *_host;
> +       struct tmio_mmc_host *host;
>         struct mmc_host *mmc;
> +
> +       mmc = mmc_alloc_host(sizeof(struct tmio_mmc_host), &pdev->dev);
> +       if (!mmc)
> +               return NULL;
> +
> +       host = mmc_priv(mmc);
> +       host->mmc = mmc;
> +       host->pdev = pdev;
> +
> +       return host;
> +}
> +
> +void tmio_mmc_host_free(struct tmio_mmc_host *host)

tmio_mmc_free_host

> +{
> +       mmc_free_host(host->mmc);
> +
> +       host->mmc = NULL;
> +}

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 1/9] mmc: tmio: add tmio_mmc_host_alloc/free()

[Kuninori Morimoto]

Hi Geert

> > -int tmio_mmc_host_probe(struct tmio_mmc_host **host,
> > -                                 struct platform_device *pdev,
> > -                                 struct tmio_mmc_data *pdata)
> > +struct tmio_mmc_host*
> > +tmio_mmc_host_alloc(struct platform_device *pdev)
> 
> I think it would be better to match the naming in the mmc core layer, i.e.
> tmio_mmc_alloc_host().
> 
> >  {
> > -       struct tmio_mmc_host *_host;
> > +       struct tmio_mmc_host *host;
> >         struct mmc_host *mmc;
> > +
> > +       mmc = mmc_alloc_host(sizeof(struct tmio_mmc_host), &pdev->dev);
> > +       if (!mmc)
> > +               return NULL;
> > +
> > +       host = mmc_priv(mmc);
> > +       host->mmc = mmc;
> > +       host->pdev = pdev;
> > +
> > +       return host;
> > +}
> > +
> > +void tmio_mmc_host_free(struct tmio_mmc_host *host)
> 
> tmio_mmc_free_host

I don't care so match, but these are based on tmio_mmc_host_xxx naming rule
which is used in tmio_mmc driver.

Best regards
---
Kuninori Morimoto

[PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current .dma is implemented under tmio_mmc_data.
It goes to tmio_mmc_host by this patch.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    4 ++--
 drivers/mmc/host/tmio_mmc.h       |   11 +++++++++++
 drivers/mmc/host/tmio_mmc_dma.c   |   24 +++++++++++-------------
 include/linux/mfd/tmio.h          |   11 -----------
 4 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index b2cec5b..1e13a1f 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -211,6 +211,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	if (!host)
 		goto eprobe;
 
+	host->dma		= dma_priv;
+
 	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
@@ -239,8 +241,6 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	dma_priv->alignment_shift = 1; /* 2-byte alignment */
 	dma_priv->filter = shdma_chan_filter;
 
-	mmc_data->dma = dma_priv;
-
 	/*
 	 * All SDHI blocks support 2-byte and larger block sizes in 4-bit
 	 * bus width mode.
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 3d02a3c1..4c5eda8 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -40,6 +40,16 @@
 
 struct tmio_mmc_data;
 
+struct tmio_mmc_dma {
+	void *chan_priv_tx;
+	void *chan_priv_rx;
+	int slave_id_tx;
+	int slave_id_rx;
+	int alignment_shift;
+	dma_addr_t dma_rx_offset;
+	bool (*filter)(struct dma_chan *chan, void *arg);
+};
+
 struct tmio_mmc_host {
 	void __iomem *ctl;
 	struct mmc_command      *cmd;
@@ -59,6 +69,7 @@ struct tmio_mmc_host {
 
 	struct platform_device *pdev;
 	struct tmio_mmc_data *pdata;
+	struct tmio_mmc_dma	*dma;
 
 	/* DMA support */
 	bool			force_pio;
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index 7d07738..6c214d6 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -49,11 +49,10 @@ static void tmio_mmc_start_dma_rx(struct tmio_mmc_host *host)
 	struct scatterlist *sg = host->sg_ptr, *sg_tmp;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_rx;
-	struct tmio_mmc_data *pdata = host->pdata;
 	dma_cookie_t cookie;
 	int ret, i;
 	bool aligned = true, multiple = true;
-	unsigned int align = (1 << pdata->dma->alignment_shift) - 1;
+	unsigned int align = (1 << host->dma->alignment_shift) - 1;
 
 	for_each_sg(sg, sg_tmp, host->sg_len, i) {
 		if (sg_tmp->offset & align)
@@ -126,11 +125,10 @@ static void tmio_mmc_start_dma_tx(struct tmio_mmc_host *host)
 	struct scatterlist *sg = host->sg_ptr, *sg_tmp;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_tx;
-	struct tmio_mmc_data *pdata = host->pdata;
 	dma_cookie_t cookie;
 	int ret, i;
 	bool aligned = true, multiple = true;
-	unsigned int align = (1 << pdata->dma->alignment_shift) - 1;
+	unsigned int align = (1 << host->dma->alignment_shift) - 1;
 
 	for_each_sg(sg, sg_tmp, host->sg_len, i) {
 		if (sg_tmp->offset & align)
@@ -262,8 +260,8 @@ out:
 void tmio_mmc_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 (!pdata->dma || (!host->pdev->dev.of_node &&
-		(!pdata->dma->chan_priv_tx || !pdata->dma->chan_priv_rx)))
+	if (!host->dma || (!host->pdev->dev.of_node &&
+		(!host->dma->chan_priv_tx || !host->dma->chan_priv_rx)))
 		return;
 
 	if (!host->chan_tx && !host->chan_rx) {
@@ -280,7 +278,7 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 		dma_cap_set(DMA_SLAVE, mask);
 
 		host->chan_tx = dma_request_slave_channel_compat(mask,
-					pdata->dma->filter, pdata->dma->chan_priv_tx,
+					host->dma->filter, host->dma->chan_priv_tx,
 					&host->pdev->dev, "tx");
 		dev_dbg(&host->pdev->dev, "%s: TX: got channel %p\n", __func__,
 			host->chan_tx);
@@ -288,8 +286,8 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 		if (!host->chan_tx)
 			return;
 
-		if (pdata->dma->chan_priv_tx)
-			cfg.slave_id = pdata->dma->slave_id_tx;
+		if (host->dma->chan_priv_tx)
+			cfg.slave_id = host->dma->slave_id_tx;
 		cfg.direction = DMA_MEM_TO_DEV;
 		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->pdata->bus_shift);
 		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -299,7 +297,7 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 			goto ecfgtx;
 
 		host->chan_rx = dma_request_slave_channel_compat(mask,
-					pdata->dma->filter, pdata->dma->chan_priv_rx,
+					host->dma->filter, host->dma->chan_priv_rx,
 					&host->pdev->dev, "rx");
 		dev_dbg(&host->pdev->dev, "%s: RX: got channel %p\n", __func__,
 			host->chan_rx);
@@ -307,10 +305,10 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 		if (!host->chan_rx)
 			goto ereqrx;
 
-		if (pdata->dma->chan_priv_rx)
-			cfg.slave_id = pdata->dma->slave_id_rx;
+		if (host->dma->chan_priv_rx)
+			cfg.slave_id = host->dma->slave_id_rx;
 		cfg.direction = DMA_DEV_TO_MEM;
-		cfg.src_addr = cfg.dst_addr + pdata->dma->dma_rx_offset;
+		cfg.src_addr = cfg.dst_addr + host->dma->dma_rx_offset;
 		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 		cfg.dst_addr = 0;
 		ret = dmaengine_slave_config(host->chan_rx, &cfg);
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index c7d9af0..8d708c7 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -112,16 +112,6 @@ void tmio_core_mmc_clk_div(void __iomem *cnf, int shift, int state);
 
 struct dma_chan;
 
-struct tmio_mmc_dma {
-	void *chan_priv_tx;
-	void *chan_priv_rx;
-	int slave_id_tx;
-	int slave_id_rx;
-	int alignment_shift;
-	dma_addr_t dma_rx_offset;
-	bool (*filter)(struct dma_chan *chan, void *arg);
-};
-
 struct tmio_mmc_host;
 
 /*
@@ -134,7 +124,6 @@ struct tmio_mmc_data {
 	unsigned long			flags;
 	unsigned long			bus_shift;
 	u32				ocr_mask;	/* available voltages */
-	struct tmio_mmc_dma		*dma;
 	unsigned int			cd_gpio;
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
-- 
1.7.9.5

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Arnd Bergmann]

On Monday 05 January 2015 07:02:41 Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current .dma is implemented under tmio_mmc_data.
> It goes to tmio_mmc_host by this patch.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

The patch looks good, just like the rest of the series, but there is
one aspect that I think would make sense to clean up at the same time,
since you are already touching all those lines:

> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 3d02a3c1..4c5eda8 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -40,6 +40,16 @@
>  
>  struct tmio_mmc_data;
>  
> +struct tmio_mmc_dma {
> +	void *chan_priv_tx;
> +	void *chan_priv_rx;
> +	int slave_id_tx;
> +	int slave_id_rx;
> +	int alignment_shift;
> +	dma_addr_t dma_rx_offset;
> +	bool (*filter)(struct dma_chan *chan, void *arg);
> +};

The slave_id/chan_priv values are now passed three times into the
driver, and one should really be enough. I'd suggest removing the
integer fields from both tmio_mmc_dma and tmio_mmc_data (added in
patch 9), and instead pass it as a void* argument only to tmio_mmc_data.

The alignment_shift and dma_rx_offset values seem to always be
the same for all users (at least the remaining ones, possibly there
were others originally), so you could hardcode those in tmio_mmc_dma.c
and remove the tmio_mmc_dma structure entirely.

The filter pointer should always be passed in the same structure
as the 'void *chan_priv' argument that is used when calling it, so
that would go into tmio_mmc_data as well if you add the data there.

Alternatively you could keep the tmio_mmc_dma structure and fill
it from the platform, but then only have the filter and void* arguments
in it.

For some obscure reason, the 'pdata->dma->slave_id_?x' fields currently
get passed into cfg.slave_id, which is something we really want to
get rid of so we can remove the slave_id field from dma_slave_config:
The slave ID is generally considered specific to the channel allocation,
not the configuration, and not all dmaengine drivers can express it
as a single integer, so the concept is obsolete. When you remove
the slave_id_?x fields here, you should also be able to just remove
the cfg.slave_id assignment without any replacement, unless there is
a bug in the dmaengine driver that should be fixed instead.

	Arnd

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd

Thank you for your review

> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index 3d02a3c1..4c5eda8 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -40,6 +40,16 @@
> >  
> >  struct tmio_mmc_data;
> >  
> > +struct tmio_mmc_dma {
> > +	void *chan_priv_tx;
> > +	void *chan_priv_rx;
> > +	int slave_id_tx;
> > +	int slave_id_rx;
> > +	int alignment_shift;
> > +	dma_addr_t dma_rx_offset;
> > +	bool (*filter)(struct dma_chan *chan, void *arg);
> > +};
> 
> The slave_id/chan_priv values are now passed three times into the
> driver, and one should really be enough. I'd suggest removing the
> integer fields from both tmio_mmc_dma and tmio_mmc_data (added in
> patch 9), and instead pass it as a void* argument only to tmio_mmc_data.

Hmm. I guess this priv_?x and slave_id are based on filter ?
sh_mobile_sdhi case, and, if it is probed via non-DT,
it will use shdma_chan_filter, and then, it doesn't need both priv_?x and slave_id.
And, if it is probed via DT, it will use other filter,
and yes, it also doesn't need these parameter.
So, from sh_mobile_sdhi point of view, yes, we can do your idea.
But, if other driver want to use it with other filter, we need both ?
(sh_mobile_sdhi is the only dmaengine user at this point, so, there is no problem though...)

> The alignment_shift and dma_rx_offset values seem to always be
> the same for all users (at least the remaining ones, possibly there
> were others originally), so you could hardcode those in tmio_mmc_dma.c
> and remove the tmio_mmc_dma structure entirely.

Unfortunately, alignment_shift and dma_rx_offset value are based on SoC.
we can't hardcode these.

> For some obscure reason, the 'pdata->dma->slave_id_?x' fields currently
> get passed into cfg.slave_id, which is something we really want to
> get rid of so we can remove the slave_id field from dma_slave_config:
> The slave ID is generally considered specific to the channel allocation,
> not the configuration, and not all dmaengine drivers can express it
> as a single integer, so the concept is obsolete. When you remove
> the slave_id_?x fields here, you should also be able to just remove
> the cfg.slave_id assignment without any replacement, unless there is
> a bug in the dmaengine driver that should be fixed instead.

I guess maybe there is no problem about this,
but, actually I don't want do it, because of compatibility.
There are many combination for DMAEngine setting.
In sh-mobile-sdhi case, we are using this driver via non-DT / DT cases,
and different DMAEngine (sh-dma / rcar-dma).
But, I can't test all patterns today. So, I would like to keep it as-is
if possible.

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Arnd Bergmann]

On Monday 05 January 2015 09:35:16 Kuninori Morimoto wrote:
> 
> Hi Arnd
> 
> Thank you for your review
> 
> > > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > > index 3d02a3c1..4c5eda8 100644
> > > --- a/drivers/mmc/host/tmio_mmc.h
> > > +++ b/drivers/mmc/host/tmio_mmc.h
> > > @@ -40,6 +40,16 @@
> > >  
> > >  struct tmio_mmc_data;
> > >  
> > > +struct tmio_mmc_dma {
> > > +	void *chan_priv_tx;
> > > +	void *chan_priv_rx;
> > > +	int slave_id_tx;
> > > +	int slave_id_rx;
> > > +	int alignment_shift;
> > > +	dma_addr_t dma_rx_offset;
> > > +	bool (*filter)(struct dma_chan *chan, void *arg);
> > > +};
> > 
> > The slave_id/chan_priv values are now passed three times into the
> > driver, and one should really be enough. I'd suggest removing the
> > integer fields from both tmio_mmc_dma and tmio_mmc_data (added in
> > patch 9), and instead pass it as a void* argument only to tmio_mmc_data.
> 
> Hmm. I guess this priv_?x and slave_id are based on filter ?

priv_?x needs to be in a format that matches the filter function,
passing slave_id is basically always wrong, but dmaengine drivers
generally just ignore it.

> sh_mobile_sdhi case, and, if it is probed via non-DT,
> it will use shdma_chan_filter, and then, it doesn't need both priv_?x and slave_id.
> And, if it is probed via DT, it will use other filter,
> and yes, it also doesn't need these parameter.

Ok.

> So, from sh_mobile_sdhi point of view, yes, we can do your idea.
> But, if other driver want to use it with other filter, we need both ?
> (sh_mobile_sdhi is the only dmaengine user at this point, so, there
> is no problem though...)

No other driver besides shmobile should use the slave_id, and a lot
of them cannot even use it because it is not in a format that makes
sense to a driver. Passing just chan_priv is the only way to do
a platform-independent driver with our API, so if we ever wanted to
use DMA on another SoC with this driver, we have to make that change
anyway.

> > The alignment_shift and dma_rx_offset values seem to always be
> > the same for all users (at least the remaining ones, possibly there
> > were others originally), so you could hardcode those in tmio_mmc_dma.c
> > and remove the tmio_mmc_dma structure entirely.
> 
> Unfortunately, alignment_shift and dma_rx_offset value are based on SoC.
> we can't hardcode these.

Which SoCs use a different value here? Both of these look like
implementation details of the tmio_mmc, not of the integration
into the SoC, so they could just be keyed off the device identification.

> > For some obscure reason, the 'pdata->dma->slave_id_?x' fields currently
> > get passed into cfg.slave_id, which is something we really want to
> > get rid of so we can remove the slave_id field from dma_slave_config:
> > The slave ID is generally considered specific to the channel allocation,
> > not the configuration, and not all dmaengine drivers can express it
> > as a single integer, so the concept is obsolete. When you remove
> > the slave_id_?x fields here, you should also be able to just remove
> > the cfg.slave_id assignment without any replacement, unless there is
> > a bug in the dmaengine driver that should be fixed instead.
> 
> I guess maybe there is no problem about this,
> but, actually I don't want do it, because of compatibility.
> There are many combination for DMAEngine setting.
> In sh-mobile-sdhi case, we are using this driver via non-DT / DT cases,
> and different DMAEngine (sh-dma / rcar-dma).
> But, I can't test all patterns today. So, I would like to keep it as-is
> if possible.

Maybe we can hide the slave_id field in dma_slave_config within
'#if defined(CONFIG_ARCH_SH_MOBILE) && defined(CONFIG_ATAGS)' then?

I would really like to prevent other people from copying the mistake.
Apparently it has already happened on MIPS jz4740, but that one seems
easy enough to fix. Tegra used to use slave_id as well, but it's been
converted to DT-only a while ago and that is just dead code for them.

I've had a closer look at the existing code now and came up with a
patch that should work for all out-of-tree drivers you may be worried
about.

	Arnd
---
dmaengine: shdma: use normal interface for passing slave id

The shmobile platform is one of only two users of the slave_id field
in dma_slave_config, which is incompatible with the way that the
dmaengine API normally works.

I've had a closer look at the existing code now and found that all
slave drivers that pass a slave_id in dma_slave_config for SH do that
right after passing the same ID into shdma_chan_filter, so we can just
rely on that. However, the various shdma drivers currently do not
remember the slave ID that was passed into the filter function when
used in non-DT mode and only check the value to find a matching channel,
unlike all other drivers.

There might still be drivers that are not part of the kernel that rely
on setting the slave_id to some other value, so to be on the safe side,
this adds another 'real_slave_id' field to shdma_chan that remembers
the ID and uses it when a driver passes a zero slave_id in dma_slave_config,
like most drivers do.

Eventually, the real_slave_id and slave_id fields should just get merged
into one field, but that requires other changes.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
----
 drivers/dma/sh/shdma-base.c     | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------------------
 drivers/mmc/host/sh_mmcif.c     |  4 +---
 drivers/mmc/host/tmio_mmc_dma.c |  4 ----
 drivers/mtd/nand/sh_flctl.c     |  2 --
 drivers/spi/spi-rspi.c          |  1 -
 drivers/spi/spi-sh-msiof.c      |  1 -
 include/linux/shdma-base.h      |  1 +
 7 files changed, 52 insertions(+), 31 deletions(-)

Note: this also changes all affected drivers to no longer pass the slave_id
field in dma_slave_config, but those changes can also be done later, as
long as the main patch gets merged first.

diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
index 3a2adb131d46..cb3e7e3b5027 100644
--- a/drivers/dma/sh/shdma-base.c
+++ b/drivers/dma/sh/shdma-base.c
@@ -171,8 +171,7 @@ static struct shdma_desc *shdma_get_desc(struct shdma_chan *schan)
 	return NULL;
 }
 
-static int shdma_setup_slave(struct shdma_chan *schan, int slave_id,
-			     dma_addr_t slave_addr)
+static int shdma_setup_slave(struct shdma_chan *schan, dma_addr_t slave_addr)
 {
 	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
 	const struct shdma_ops *ops = sdev->ops;
@@ -183,25 +182,23 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id,
 		ret = ops->set_slave(schan, match, slave_addr, true);
 		if (ret < 0)
 			return ret;
-
-		slave_id = schan->slave_id;
 	} else {
-		match = slave_id;
+		match = schan->real_slave_id;
 	}
 
-	if (slave_id < 0 || slave_id >= slave_num)
+	if (schan->real_slave_id < 0 || schan->real_slave_id >= slave_num)
 		return -EINVAL;
 
-	if (test_and_set_bit(slave_id, shdma_slave_used))
+	if (test_and_set_bit(schan->real_slave_id, shdma_slave_used))
 		return -EBUSY;
 
 	ret = ops->set_slave(schan, match, slave_addr, false);
 	if (ret < 0) {
-		clear_bit(slave_id, shdma_slave_used);
+		clear_bit(schan->real_slave_id, shdma_slave_used);
 		return ret;
 	}
 
-	schan->slave_id = slave_id;
+	schan->slave_id = schan->real_slave_id;
 
 	return 0;
 }
@@ -221,10 +218,12 @@ static int shdma_alloc_chan_resources(struct dma_chan *chan)
 	 */
 	if (slave) {
 		/* Legacy mode: .private is set in filter */
-		ret = shdma_setup_slave(schan, slave->slave_id, 0);
+		schan->real_slave_id = slave->slave_id;
+		ret = shdma_setup_slave(schan, 0);
 		if (ret < 0)
 			goto esetslave;
 	} else {
+		/* Normal mode: real_slave_id was set by filter */
 		schan->slave_id = -EINVAL;
 	}
 
@@ -258,11 +257,14 @@ esetslave:
 
 /*
  * This is the standard shdma filter function to be used as a replacement to the
- * "old" method, using the .private pointer. If for some reason you allocate a
- * channel without slave data, use something like ERR_PTR(-EINVAL) as a filter
+ * "old" method, using the .private pointer.
+ * You always have to pass a valid slave id as the argument, old drivers that
+ * pass ERR_PTR(-EINVAL) as a filter parameter and set it up in dma_slave_config
+ * need to be updated so we can remove the slave_id field from dma_slave_config.
  * parameter. If this filter is used, the slave driver, after calling
  * dma_request_channel(), will also have to call dmaengine_slave_config() with
- * .slave_id, .direction, and either .src_addr or .dst_addr set.
+ * .direction, and either .src_addr or .dst_addr set.
+ *
  * NOTE: this filter doesn't support multiple DMAC drivers with the DMA_SLAVE
  * capability! If this becomes a requirement, hardware glue drivers, using this
  * services would have to provide their own filters, which first would check
@@ -276,7 +278,7 @@ bool shdma_chan_filter(struct dma_chan *chan, void *arg)
 {
 	struct shdma_chan *schan;
 	struct shdma_dev *sdev;
-	int match = (long)arg;
+	int slave_id = (long)arg;
 	int ret;
 
 	/* Only support channels handled by this driver. */
@@ -284,16 +286,35 @@ bool shdma_chan_filter(struct dma_chan *chan, void *arg)
 	    shdma_alloc_chan_resources)
 		return false;
 
-	if (match < 0)
+	schan = to_shdma_chan(chan);
+	sdev = to_shdma_dev(chan->device);
+
+	/*
+	 * For DT, the schan->slave_id field is generated by the
+	 * set_slave function from the slave ID that is passed in
+	 * from xlate. For the non-DT case, the slave ID is
+	 * directly passed into the filter function by the driver
+	 */
+	if (schan->dev->of_node) {
+		ret = sdev->ops->set_slave(schan, slave_id, 0, true);
+		if (ret < 0)
+			return false;
+
+		schan->real_slave_id = schan->slave_id;
+		return true;
+	}
+
+	if (slave_id < 0) {
 		/* No slave requested - arbitrary channel */
+		dev_warn(sdev->dma_dev.dev, "invalid slave ID passed to dma_request_slave\n");
 		return true;
+	}
 
-	schan = to_shdma_chan(chan);
-	if (!schan->dev->of_node && match >= slave_num)
+	if (slave_id >= slave_num)
 		return false;
 
-	sdev = to_shdma_dev(schan->dma_chan.device);
-	ret = sdev->ops->set_slave(schan, match, 0, true);
+	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
+	schan->real_slave_id = slave_id;
 	if (ret < 0)
 		return false;
 
@@ -452,6 +473,8 @@ static void shdma_free_chan_resources(struct dma_chan *chan)
 		chan->private = NULL;
 	}
 
+	schan->real_slave_id = 0;
+
 	spin_lock_irq(&schan->chan_lock);
 
 	list_splice_init(&schan->ld_free, &list);
@@ -767,7 +790,14 @@ static int shdma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 		 * channel, while using it...
 		 */
 		config = (struct dma_slave_config *)arg;
-		ret = shdma_setup_slave(schan, config->slave_id,
+
+		/* overriding the slave_id through dma_slave_config is deprecated,
+		 * but possibly some out-of-tree drivers still do it. */
+		if (WARN_ON_ONCE(config->slave_id &&
+		    config->slave_id != schan->real_slave_id))
+			schan->real_slave_id = config->slave_id;
+
+		ret = shdma_setup_slave(schan,
 					config->direction = DMA_DEV_TO_MEM ?
 					config->src_addr : config->dst_addr);
 		if (ret < 0)
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 7d9d6a321521..df3a537f5a83 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -388,7 +388,7 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 {
 	struct dma_slave_config cfg = { 0, };
 	struct dma_chan *chan;
-	unsigned int slave_id;
+	void *slave_data;
 	struct resource *res;
 	dma_cap_mask_t mask;
 	int ret;
@@ -414,8 +414,6 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 
 	res = platform_get_resource(host->pd, IORESOURCE_MEM, 0);
 
-	/* In the OF case the driver will get the slave ID from the DT */
-	cfg.slave_id = slave_id;
 	cfg.direction = direction;
 
 	if (direction = DMA_DEV_TO_MEM) {
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index 7d077388b9eb..2ba47ab689ff 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -288,8 +288,6 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 		if (!host->chan_tx)
 			return;
 
-		if (pdata->dma->chan_priv_tx)
-			cfg.slave_id = pdata->dma->slave_id_tx;
 		cfg.direction = DMA_MEM_TO_DEV;
 		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->pdata->bus_shift);
 		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -307,8 +305,6 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 		if (!host->chan_rx)
 			goto ereqrx;
 
-		if (pdata->dma->chan_priv_rx)
-			cfg.slave_id = pdata->dma->slave_id_rx;
 		cfg.direction = DMA_DEV_TO_MEM;
 		cfg.src_addr = cfg.dst_addr + pdata->dma->dma_rx_offset;
 		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index a21c378f096a..c3ce81c1a716 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -159,7 +159,6 @@ static void flctl_setup_dma(struct sh_flctl *flctl)
 		return;
 
 	memset(&cfg, 0, sizeof(cfg));
-	cfg.slave_id = pdata->slave_id_fifo0_tx;
 	cfg.direction = DMA_MEM_TO_DEV;
 	cfg.dst_addr = (dma_addr_t)FLDTFIFO(flctl);
 	cfg.src_addr = 0;
@@ -175,7 +174,6 @@ static void flctl_setup_dma(struct sh_flctl *flctl)
 	if (!flctl->chan_fifo0_rx)
 		goto err;
 
-	cfg.slave_id = pdata->slave_id_fifo0_rx;
 	cfg.direction = DMA_DEV_TO_MEM;
 	cfg.dst_addr = 0;
 	cfg.src_addr = (dma_addr_t)FLDTFIFO(flctl);
diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 2071f788c6fb..6cbcdc04c947 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -918,7 +918,6 @@ static struct dma_chan *rspi_request_dma_chan(struct device *dev,
 	}
 
 	memset(&cfg, 0, sizeof(cfg));
-	cfg.slave_id = id;
 	cfg.direction = dir;
 	if (dir = DMA_MEM_TO_DEV) {
 		cfg.dst_addr = port_addr;
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 239be7cbe5a8..786ac9ec11fc 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -984,7 +984,6 @@ static struct dma_chan *sh_msiof_request_dma_chan(struct device *dev,
 	}
 
 	memset(&cfg, 0, sizeof(cfg));
-	cfg.slave_id = id;
 	cfg.direction = dir;
 	if (dir = DMA_MEM_TO_DEV) {
 		cfg.dst_addr = port_addr;
diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index abdf1f229dc3..dd0ba502ccb3 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -69,6 +69,7 @@ struct shdma_chan {
 	int id;				/* Raw id of this channel */
 	int irq;			/* Channel IRQ */
 	int slave_id;			/* Client ID for slave DMA */
+	int real_slave_id;		/* argument passed to filter function */
 	int hw_req;			/* DMA request line for slave DMA - same
 					 * as MID/RID, used with DT */
 	enum shdma_pm_state pm_state;

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd, Ulf

Thank you for your feedback

> > > The slave_id/chan_priv values are now passed three times into the
> > > driver, and one should really be enough. I'd suggest removing the
> > > integer fields from both tmio_mmc_dma and tmio_mmc_data (added in
> > > patch 9), and instead pass it as a void* argument only to tmio_mmc_data.
> > 
> > Hmm. I guess this priv_?x and slave_id are based on filter ?
> 
> priv_?x needs to be in a format that matches the filter function,
> passing slave_id is basically always wrong, but dmaengine drivers
> generally just ignore it.
(snip)
> Maybe we can hide the slave_id field in dma_slave_config within
> '#if defined(CONFIG_ARCH_SH_MOBILE) && defined(CONFIG_ATAGS)' then?
> 
> I would really like to prevent other people from copying the mistake.
> Apparently it has already happened on MIPS jz4740, but that one seems
> easy enough to fix. Tegra used to use slave_id as well, but it's been
> converted to DT-only a while ago and that is just dead code for them.
> 
> I've had a closer look at the existing code now and came up with a
> patch that should work for all out-of-tree drivers you may be worried
> about.

I don't want to use #ifdef in driver :P
OK, I understand your opinion. I can try fix this DMAEngine issue
(without #ifdef :)

But, I want "step by step" for this cleanup.
So, can you please accept about current "header cleanup" patch-set as 1st step ?
Then, I want to try "dmaengine cleanup" patch-set as 2nd step if possible.
I guess [8/9] and [9/9] are not good for "header cleanup" (?) I don't know.
Arnd, Ulf, what is your opinion ?

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Arnd Bergmann]

On Tuesday 06 January 2015 00:20:59 Kuninori Morimoto wrote:
> 
> Hi Arnd, Ulf
> 
> Thank you for your feedback
> 
> > > > The slave_id/chan_priv values are now passed three times into the
> > > > driver, and one should really be enough. I'd suggest removing the
> > > > integer fields from both tmio_mmc_dma and tmio_mmc_data (added in
> > > > patch 9), and instead pass it as a void* argument only to tmio_mmc_data.
> > > 
> > > Hmm. I guess this priv_?x and slave_id are based on filter ?
> > 
> > priv_?x needs to be in a format that matches the filter function,
> > passing slave_id is basically always wrong, but dmaengine drivers
> > generally just ignore it.
> (snip)
> > Maybe we can hide the slave_id field in dma_slave_config within
> > '#if defined(CONFIG_ARCH_SH_MOBILE) && defined(CONFIG_ATAGS)' then?
> > 
> > I would really like to prevent other people from copying the mistake.
> > Apparently it has already happened on MIPS jz4740, but that one seems
> > easy enough to fix. Tegra used to use slave_id as well, but it's been
> > converted to DT-only a while ago and that is just dead code for them.
> > 
> > I've had a closer look at the existing code now and came up with a
> > patch that should work for all out-of-tree drivers you may be worried
> > about.
> 
> I don't want to use #ifdef in driver :P
> OK, I understand your opinion. I can try fix this DMAEngine issue
> (without #ifdef :)

Ok, thanks. I have also posted a patch now for the jz4740 platform
that is the other user of slave_id.

> But, I want "step by step" for this cleanup.
> So, can you please accept about current "header cleanup" patch-set as 1st step ?

Fair enough.

> Then, I want to try "dmaengine cleanup" patch-set as 2nd step if possible.
> I guess [8/9] and [9/9] are not good for "header cleanup" (?) I don't know.
> Arnd, Ulf, what is your opinion ?

Patch 8 looks great to me.

Patch 9 as I mentioned is a bit strange because it duplicates the some
of the data between tmio_mmc_data and tmio_mmc_dma, which are both
visible to the tmio_mmc_dma.c file. Simply folding tmio_mmc_dma into
tmio_mmc_data seems like the simplest solution to that.

	Arnd

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd, Ulf

Thank you for your feedback

> > Then, I want to try "dmaengine cleanup" patch-set as 2nd step if possible.
> > I guess [8/9] and [9/9] are not good for "header cleanup" (?) I don't know.
> > Arnd, Ulf, what is your opinion ?
> 
> Patch 8 looks great to me.
> 
> Patch 9 as I mentioned is a bit strange because it duplicates the some
> of the data between tmio_mmc_data and tmio_mmc_dma, which are both
> visible to the tmio_mmc_dma.c file. Simply folding tmio_mmc_dma into
> tmio_mmc_data seems like the simplest solution to that.

OK, I see. I will fixup 9/9.

Ulf
Can you please care about 1/9 - 8/9 as 1st step ?
(I got review from Geert about 8/9, so, 1/9 - 7/9 ?)

I will re-try about 9/9 (or 8/9, 9/9) part.
And, then, I will try dmaengine cleanup.

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd again

> > > The alignment_shift and dma_rx_offset values seem to always be
> > > the same for all users (at least the remaining ones, possibly there
> > > were others originally), so you could hardcode those in tmio_mmc_dma.c
> > > and remove the tmio_mmc_dma structure entirely.
> > 
> > Unfortunately, alignment_shift and dma_rx_offset value are based on SoC.
> > we can't hardcode these.
> 
> Which SoCs use a different value here? Both of these look like
> implementation details of the tmio_mmc, not of the integration
> into the SoC, so they could just be keyed off the device identification.

About .alignment_shift, it is not implemented today, but our new SoC
want to use different value (= .alignment_shift = 5).
About .dma_rx_offset, please check this
  ${LINUX}/drivers/mmc/host/sh_mobile_sdhi.c :: of_rcar_gen1_compatible
  ${LINUX}/drivers/mmc/host/sh_mobile_sdhi.c :: of_rcar_gen2_compatible
or
384b2cbd56a02efb16358ed7c0c039e4afca5ed0
(mmc: tmio: care about DMA tx/rx addr offset)

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Arnd Bergmann]

On Tuesday 06 January 2015 02:38:53 Kuninori Morimoto wrote:
> Hi Arnd again
> 
> > > > The alignment_shift and dma_rx_offset values seem to always be
> > > > the same for all users (at least the remaining ones, possibly there
> > > > were others originally), so you could hardcode those in tmio_mmc_dma.c
> > > > and remove the tmio_mmc_dma structure entirely.
> > > 
> > > Unfortunately, alignment_shift and dma_rx_offset value are based on SoC.
> > > we can't hardcode these.
> > 
> > Which SoCs use a different value here? Both of these look like
> > implementation details of the tmio_mmc, not of the integration
> > into the SoC, so they could just be keyed off the device identification.
> 
> About .alignment_shift, it is not implemented today, but our new SoC
> want to use different value (= .alignment_shift = 5).

Ok, I see.

> About .dma_rx_offset, please check this
>   ${LINUX}/drivers/mmc/host/sh_mobile_sdhi.c :: of_rcar_gen1_compatible
>   ${LINUX}/drivers/mmc/host/sh_mobile_sdhi.c :: of_rcar_gen2_compatible
> or
> 384b2cbd56a02efb16358ed7c0c039e4afca5ed0
> (mmc: tmio: care about DMA tx/rx addr offset)

Right. How about moving these two into tmio_mmc_data then along with
the other members of tmio_mmc_dma?

	Arnd

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd, Ulf

> > > > > The alignment_shift and dma_rx_offset values seem to always be
> > > > > the same for all users (at least the remaining ones, possibly there
> > > > > were others originally), so you could hardcode those in tmio_mmc_dma.c
> > > > > and remove the tmio_mmc_dma structure entirely.
> > > > 
> > > > Unfortunately, alignment_shift and dma_rx_offset value are based on SoC.
> > > > we can't hardcode these.
> > > 
> > > Which SoCs use a different value here? Both of these look like
> > > implementation details of the tmio_mmc, not of the integration
> > > into the SoC, so they could just be keyed off the device identification.
> > 
> > About .alignment_shift, it is not implemented today, but our new SoC
> > want to use different value (= .alignment_shift = 5).
> 
> Ok, I see.
> 
> > About .dma_rx_offset, please check this
> >   ${LINUX}/drivers/mmc/host/sh_mobile_sdhi.c :: of_rcar_gen1_compatible
> >   ${LINUX}/drivers/mmc/host/sh_mobile_sdhi.c :: of_rcar_gen2_compatible
> > or
> > 384b2cbd56a02efb16358ed7c0c039e4afca5ed0
> > (mmc: tmio: care about DMA tx/rx addr offset)
> 
> Right. How about moving these two into tmio_mmc_data then along with
> the other members of tmio_mmc_dma?

OK, I can do it.

Ulf, can I send above as additional patch ?
Or do you want v2 patch-set ?

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd

Thank you for your DMAEngine fixup patch.
I tried this patch, and have comment.

> dmaengine: shdma: use normal interface for passing slave id
> 
> The shmobile platform is one of only two users of the slave_id field
> in dma_slave_config, which is incompatible with the way that the
> dmaengine API normally works.
> 
> I've had a closer look at the existing code now and found that all
> slave drivers that pass a slave_id in dma_slave_config for SH do that
> right after passing the same ID into shdma_chan_filter, so we can just
> rely on that. However, the various shdma drivers currently do not
> remember the slave ID that was passed into the filter function when
> used in non-DT mode and only check the value to find a matching channel,
> unlike all other drivers.
> 
> There might still be drivers that are not part of the kernel that rely
> on setting the slave_id to some other value, so to be on the safe side,
> this adds another 'real_slave_id' field to shdma_chan that remembers
> the ID and uses it when a driver passes a zero slave_id in dma_slave_config,
> like most drivers do.
> 
> Eventually, the real_slave_id and slave_id fields should just get merged
> into one field, but that requires other changes.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ----
>  drivers/dma/sh/shdma-base.c     | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------------------
>  drivers/mmc/host/sh_mmcif.c     |  4 +---
>  drivers/mmc/host/tmio_mmc_dma.c |  4 ----
>  drivers/mtd/nand/sh_flctl.c     |  2 --
>  drivers/spi/spi-rspi.c          |  1 -
>  drivers/spi/spi-sh-msiof.c      |  1 -
>  include/linux/shdma-base.h      |  1 +
>  7 files changed, 52 insertions(+), 31 deletions(-)
(snip)
> @@ -284,16 +286,35 @@ bool shdma_chan_filter(struct dma_chan *chan, void *arg)
>  	    shdma_alloc_chan_resources)
>  		return false;
>  
> -	if (match < 0)
> +	schan = to_shdma_chan(chan);
> +	sdev = to_shdma_dev(chan->device);
> +
> +	/*
> +	 * For DT, the schan->slave_id field is generated by the
> +	 * set_slave function from the slave ID that is passed in
> +	 * from xlate. For the non-DT case, the slave ID is
> +	 * directly passed into the filter function by the driver
> +	 */
> +	if (schan->dev->of_node) {
> +		ret = sdev->ops->set_slave(schan, slave_id, 0, true);
> +		if (ret < 0)
> +			return false;
> +
> +		schan->real_slave_id = schan->slave_id;
> +		return true;
> +	}
> +
> +	if (slave_id < 0) {
>  		/* No slave requested - arbitrary channel */
> +		dev_warn(sdev->dma_dev.dev, "invalid slave ID passed to dma_request_slave\n");
>  		return true;
> +	}
>  
> -	schan = to_shdma_chan(chan);
> -	if (!schan->dev->of_node && match >= slave_num)
> +	if (slave_id >= slave_num)
>  		return false;
>  
> -	sdev = to_shdma_dev(schan->dma_chan.device);
> -	ret = sdev->ops->set_slave(schan, match, 0, true);
> +	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
> +	schan->real_slave_id = slave_id;
>  	if (ret < 0)
>  		return false;

Here, your patch is

	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
	schan->real_slave_id = slave_id;
	if (ret < 0)

But, it doesn't work. Maybe, you want this

	schan->real_slave_id = slave_id;
	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
	if (ret < 0)

> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 7d9d6a321521..df3a537f5a83 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -388,7 +388,7 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
>  {
>  	struct dma_slave_config cfg = { 0, };
>  	struct dma_chan *chan;
> -	unsigned int slave_id;
> +	void *slave_data;
>  	struct resource *res;
>  	dma_cap_mask_t mask;
>  	int ret;
> @@ -414,8 +414,6 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
>  
>  	res = platform_get_resource(host->pd, IORESOURCE_MEM, 0);
>  
> -	/* In the OF case the driver will get the slave ID from the DT */
> -	cfg.slave_id = slave_id;
>  	cfg.direction = direction;
>  
>  	if (direction = DMA_DEV_TO_MEM) {

I got error here


/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c: In function ‘sh_mmcif_request_dma_one’:
/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:400:3: error: ‘slave_id’ undeclared (first use in this function)
   slave_id = direction = DMA_MEM_TO_DEV
   ^
/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:400:3: note: each undeclared identifier is reported only once for each function it appears in
/opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:391:8: warning: unused variable ‘slave_data’ [-Wunused-variable]
  void *slave_data;
        ^

Maybe you are missing this

        if (pdata)
-               slave_id = direction = DMA_MEM_TO_DEV
-                        ? pdata->slave_id_tx : pdata->slave_id_rx;
+               slave_data = direction = DMA_MEM_TO_DEV
+                       ? (void *)pdata->slave_id_tx : (void *)pdata->slave_id_rx;
        else
-               slave_id = 0;
+               slave_data = 0;
 
        chan = dma_request_slave_channel_compat(mask, shdma_chan_filter,
-                               (void *)(unsigned long)slave_id, &host->pd->dev,
+                               slave_data, &host->pd->dev,
                                direction = DMA_MEM_TO_DEV ? "tx" : "rx");
 
        dev_dbg(&host->pd->dev, "%s: %s: got channel %p\n", __func__,


sh_mobile_sdhi DMA works if I fixuped above

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Arnd Bergmann]

On Wednesday 07 January 2015 02:28:43 Kuninori Morimoto wrote:
> 
> Hi Arnd
> 
> Thank you for your DMAEngine fixup patch.
> I tried this patch, and have comment.

Thanks for looking at the changes

> >  
> > -	sdev = to_shdma_dev(schan->dma_chan.device);
> > -	ret = sdev->ops->set_slave(schan, match, 0, true);
> > +	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
> > +	schan->real_slave_id = slave_id;
> >  	if (ret < 0)
> >  		return false;
> 
> Here, your patch is
> 
> 	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
> 	schan->real_slave_id = slave_id;
> 	if (ret < 0)
> 
> But, it doesn't work. Maybe, you want this
> 
> 	schan->real_slave_id = slave_id;
> 	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
> 	if (ret < 0)

Right, I broke it in a last-minute change. Originally I had

 	schan->real_slave_id = slave_id;
 	ret = sdev->ops->set_slave(schan, schan->real_slave_id, 0, true);
 	if (ret < 0) {
		schan->real_slave_id = 0;
		return ret;
	}

and then meant to shorten it to

 	ret = sdev->ops->set_slave(schan, slave_id, 0, true);
 	if (ret < 0)
		return ret;
 	schan->real_slave_id = slave_id;

Either of those should be fine, but what I wrote was instead was
completely wrong.

> > diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> > index 7d9d6a321521..df3a537f5a83 100644
> > --- a/drivers/mmc/host/sh_mmcif.c
> > +++ b/drivers/mmc/host/sh_mmcif.c
> > @@ -388,7 +388,7 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
> >  {
> >  	struct dma_slave_config cfg = { 0, };
> >  	struct dma_chan *chan;
> > -	unsigned int slave_id;
> > +	void *slave_data;
> >  	struct resource *res;
> >  	dma_cap_mask_t mask;
> >  	int ret;
> > @@ -414,8 +414,6 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
> >  
> >  	res = platform_get_resource(host->pd, IORESOURCE_MEM, 0);
> >  
> > -	/* In the OF case the driver will get the slave ID from the DT */
> > -	cfg.slave_id = slave_id;
> >  	cfg.direction = direction;
> >  
> >  	if (direction = DMA_DEV_TO_MEM) {
> 
> I got error here
> 
> 
> /opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c: In function ‘sh_mmcif_request_dma_one’:
> /opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:400:3: error: ‘slave_id’ undeclared (first use in this function)
>    slave_id = direction = DMA_MEM_TO_DEV
>    ^
> /opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:400:3: note: each undeclared identifier is reported only once for each function it appears in
> /opt/home/morimoto/linux/drivers/mmc/host/sh_mmcif.c:391:8: warning: unused variable ‘slave_data’ [-Wunused-variable]
>   void *slave_data;
>         ^
> 
> Maybe you are missing this
> 
>         if (pdata)
> -               slave_id = direction = DMA_MEM_TO_DEV
> -                        ? pdata->slave_id_tx : pdata->slave_id_rx;
> +               slave_data = direction = DMA_MEM_TO_DEV
> +                       ? (void *)pdata->slave_id_tx : (void *)pdata->slave_id_rx;
>         else
> -               slave_id = 0;
> +               slave_data = 0;
>  
>         chan = dma_request_slave_channel_compat(mask, shdma_chan_filter,
> -                               (void *)(unsigned long)slave_id, &host->pd->dev,
> +                               slave_data, &host->pd->dev,
>                                 direction = DMA_MEM_TO_DEV ? "tx" : "rx");
>  
>         dev_dbg(&host->pd->dev, "%s: %s: got channel %p\n", __func__,
> 
> 
> sh_mobile_sdhi DMA works if I fixuped above

Either that or undo the change to the type. I originally planned to change the
sh_mmcif_plat_data to use a void* type already, but then didn't do that because
it conflicts with your other patch, and I failed to revert my earlier change
correctly.

	Arnd

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd

> dmaengine: shdma: use normal interface for passing slave id
> 
> The shmobile platform is one of only two users of the slave_id field
> in dma_slave_config, which is incompatible with the way that the
> dmaengine API normally works.
> 
> I've had a closer look at the existing code now and found that all
> slave drivers that pass a slave_id in dma_slave_config for SH do that
> right after passing the same ID into shdma_chan_filter, so we can just
> rely on that. However, the various shdma drivers currently do not
> remember the slave ID that was passed into the filter function when
> used in non-DT mode and only check the value to find a matching channel,
> unlike all other drivers.
> 
> There might still be drivers that are not part of the kernel that rely
> on setting the slave_id to some other value, so to be on the safe side,
> this adds another 'real_slave_id' field to shdma_chan that remembers
> the ID and uses it when a driver passes a zero slave_id in dma_slave_config,
> like most drivers do.
> 
> Eventually, the real_slave_id and slave_id fields should just get merged
> into one field, but that requires other changes.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ----
>  drivers/dma/sh/shdma-base.c     | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------------------
>  drivers/mmc/host/sh_mmcif.c     |  4 +---
>  drivers/mmc/host/tmio_mmc_dma.c |  4 ----
>  drivers/mtd/nand/sh_flctl.c     |  2 --
>  drivers/spi/spi-rspi.c          |  1 -
>  drivers/spi/spi-sh-msiof.c      |  1 -
>  include/linux/shdma-base.h      |  1 +
>  7 files changed, 52 insertions(+), 31 deletions(-)
(snip)
> -	/* In the OF case the driver will get the slave ID from the DT */
> -	cfg.slave_id = slave_id;
>  	cfg.direction = direction;

My other drivers are using slave_id
 linux/sound/soc/sh/fsi.c
 linux/sound/soc/sh/rcar/core.c

I guess it should remove cfg.slave_id = xxx ?
Can you include these in this patch ?
or I can do it as other patch

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Arnd Bergmann]

On Wednesday 07 January 2015 03:01:22 Kuninori Morimoto wrote:
> Hi Arnd
> 
> > dmaengine: shdma: use normal interface for passing slave id
> > 
> > The shmobile platform is one of only two users of the slave_id field
> > in dma_slave_config, which is incompatible with the way that the
> > dmaengine API normally works.
> > 
> > I've had a closer look at the existing code now and found that all
> > slave drivers that pass a slave_id in dma_slave_config for SH do that
> > right after passing the same ID into shdma_chan_filter, so we can just
> > rely on that. However, the various shdma drivers currently do not
> > remember the slave ID that was passed into the filter function when
> > used in non-DT mode and only check the value to find a matching channel,
> > unlike all other drivers.
> > 
> > There might still be drivers that are not part of the kernel that rely
> > on setting the slave_id to some other value, so to be on the safe side,
> > this adds another 'real_slave_id' field to shdma_chan that remembers
> > the ID and uses it when a driver passes a zero slave_id in dma_slave_config,
> > like most drivers do.
> > 
> > Eventually, the real_slave_id and slave_id fields should just get merged
> > into one field, but that requires other changes.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ----
> >  drivers/dma/sh/shdma-base.c     | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------------------
> >  drivers/mmc/host/sh_mmcif.c     |  4 +---
> >  drivers/mmc/host/tmio_mmc_dma.c |  4 ----
> >  drivers/mtd/nand/sh_flctl.c     |  2 --
> >  drivers/spi/spi-rspi.c          |  1 -
> >  drivers/spi/spi-sh-msiof.c      |  1 -
> >  include/linux/shdma-base.h      |  1 +
> >  7 files changed, 52 insertions(+), 31 deletions(-)
> (snip)
> > -     /* In the OF case the driver will get the slave ID from the DT */
> > -     cfg.slave_id = slave_id;
> >       cfg.direction = direction;
> 
> My other drivers are using slave_id
>  linux/sound/soc/sh/fsi.c
>  linux/sound/soc/sh/rcar/core.c

My mistake, I did a 'git grep' the linux/drivers but missed linux/sound/soc.
 
> I guess it should remove cfg.slave_id = xxx ?

Correct. I checked both drivers, and  they behave just like the ones
I included in my patch.

> Can you include these in this patch ?
> or I can do it as other patch

I was actually hoping that you could pick up my patch and send it as a
follow-up to your series once you have a working version. As I mentioned,
the driver changes to remove the slave_id assignment can be done either
together with the base patch or as a follow-up.

	Arnd

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd

> > > dmaengine: shdma: use normal interface for passing slave id
> > > 
> > > The shmobile platform is one of only two users of the slave_id field
> > > in dma_slave_config, which is incompatible with the way that the
> > > dmaengine API normally works.
> > > 
> > > I've had a closer look at the existing code now and found that all
> > > slave drivers that pass a slave_id in dma_slave_config for SH do that
> > > right after passing the same ID into shdma_chan_filter, so we can just
> > > rely on that. However, the various shdma drivers currently do not
> > > remember the slave ID that was passed into the filter function when
> > > used in non-DT mode and only check the value to find a matching channel,
> > > unlike all other drivers.
> > > 
> > > There might still be drivers that are not part of the kernel that rely
> > > on setting the slave_id to some other value, so to be on the safe side,
> > > this adds another 'real_slave_id' field to shdma_chan that remembers
> > > the ID and uses it when a driver passes a zero slave_id in dma_slave_config,
> > > like most drivers do.
> > > 
> > > Eventually, the real_slave_id and slave_id fields should just get merged
> > > into one field, but that requires other changes.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ----
(snip)
> I was actually hoping that you could pick up my patch and send it as a
> follow-up to your series once you have a working version. As I mentioned,
> the driver changes to remove the slave_id assignment can be done either
> together with the base patch or as a follow-up.

OK, I can do it if you want :)

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd, Ulf

> > > > dmaengine: shdma: use normal interface for passing slave id
> > > > 
> > > > The shmobile platform is one of only two users of the slave_id field
> > > > in dma_slave_config, which is incompatible with the way that the
> > > > dmaengine API normally works.
> > > > 
> > > > I've had a closer look at the existing code now and found that all
> > > > slave drivers that pass a slave_id in dma_slave_config for SH do that
> > > > right after passing the same ID into shdma_chan_filter, so we can just
> > > > rely on that. However, the various shdma drivers currently do not
> > > > remember the slave ID that was passed into the filter function when
> > > > used in non-DT mode and only check the value to find a matching channel,
> > > > unlike all other drivers.
> > > > 
> > > > There might still be drivers that are not part of the kernel that rely
> > > > on setting the slave_id to some other value, so to be on the safe side,
> > > > this adds another 'real_slave_id' field to shdma_chan that remembers
> > > > the ID and uses it when a driver passes a zero slave_id in dma_slave_config,
> > > > like most drivers do.
> > > > 
> > > > Eventually, the real_slave_id and slave_id fields should just get merged
> > > > into one field, but that requires other changes.
> > > > 
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > ----
> (snip)
> Either that or undo the change to the type. I originally planned to change the
> sh_mmcif_plat_data to use a void* type already, but then didn't do that because
> it conflicts with your other patch, and I failed to revert my earlier change
> correctly.

Hmm... indeed Arnd's patch and my patch-set conflicts.
I have these patch / patch-set
 1) header cleanup for tmio
 2) slave_id cleanup for shdma
 3) add DMA feature for sh_mobile_sdhi

1 ) and 2) conflicts here. one idea is like this
 1) header cleanup for tmio
 2) add DMA feature for sh_mobile_sdhi
 3) slave_id cleanup for shdma

1) and 2) can be controled by Ulf with no-conflict.
if these are merged correctly, I can send 3) to DMAEngine ML.
Then, I can point the Ulf's branch as base branch.

Arnd, Ulf what do you think ?

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Arnd Bergmann]

On Thursday 08 January 2015 07:30:36 Kuninori Morimoto wrote:
> 
> Hmm... indeed Arnd's patch and my patch-set conflicts.
> I have these patch / patch-set
>  1) header cleanup for tmio
>  2) slave_id cleanup for shdma
>  3) add DMA feature for sh_mobile_sdhi
> 
> 1 ) and 2) conflicts here. one idea is like this
>  1) header cleanup for tmio
>  2) add DMA feature for sh_mobile_sdhi
>  3) slave_id cleanup for shdma
> 
> 1) and 2) can be controled by Ulf with no-conflict.
> if these are merged correctly, I can send 3) to DMAEngine ML.
> Then, I can point the Ulf's branch as base branch.
> 
> Arnd, Ulf what do you think ?
> 

Sounds good. You could also leave out the sh_mobile_sdhi part from
3) patch to avoid the conflict, and add a comment in that place
as part of 2), to say that the slave_id assignment can be removed
once the other parts are done. That way, we know where we're at
if we want to remove slave_id from dma_slave_config and it's still
part of the sdhi driver.

	Arnd

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Kuninori Morimoto]

Hi Arnd, Ulf

> > Hmm... indeed Arnd's patch and my patch-set conflicts.
> > I have these patch / patch-set
> >  1) header cleanup for tmio
> >  2) slave_id cleanup for shdma
> >  3) add DMA feature for sh_mobile_sdhi
> > 
> > 1 ) and 2) conflicts here. one idea is like this
> >  1) header cleanup for tmio
> >  2) add DMA feature for sh_mobile_sdhi
> >  3) slave_id cleanup for shdma
> > 
> > 1) and 2) can be controled by Ulf with no-conflict.
> > if these are merged correctly, I can send 3) to DMAEngine ML.
> > Then, I can point the Ulf's branch as base branch.
> > 
> > Arnd, Ulf what do you think ?
> > 
> 
> Sounds good. You could also leave out the sh_mobile_sdhi part from
> 3) patch to avoid the conflict, and add a comment in that place
> as part of 2), to say that the slave_id assignment can be removed
> once the other parts are done. That way, we know where we're at
> if we want to remove slave_id from dma_slave_config and it's still
> part of the sdhi driver.

Thank you.
I wait Ulf's opinion

Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

[Ulf Hansson]

On 9 January 2015 at 10:44, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
> Hi Arnd, Ulf
>
>> > Hmm... indeed Arnd's patch and my patch-set conflicts.
>> > I have these patch / patch-set
>> >  1) header cleanup for tmio
>> >  2) slave_id cleanup for shdma
>> >  3) add DMA feature for sh_mobile_sdhi
>> >
>> > 1 ) and 2) conflicts here. one idea is like this
>> >  1) header cleanup for tmio
>> >  2) add DMA feature for sh_mobile_sdhi
>> >  3) slave_id cleanup for shdma
>> >
>> > 1) and 2) can be controled by Ulf with no-conflict.
>> > if these are merged correctly, I can send 3) to DMAEngine ML.
>> > Then, I can point the Ulf's branch as base branch.
>> >
>> > Arnd, Ulf what do you think ?
>> >
>>
>> Sounds good. You could also leave out the sh_mobile_sdhi part from
>> 3) patch to avoid the conflict, and add a comment in that place
>> as part of 2), to say that the slave_id assignment can be removed
>> once the other parts are done. That way, we know where we're at
>> if we want to remove slave_id from dma_slave_config and it's still
>> part of the sdhi driver.
>
> Thank you.
> I wait Ulf's opinion
>

I am happy to share an immutable branch of needed. Please post a v2
with patches for me to review/pick up for mmc.

Kind regards
Uffe

[PATCH 3/9] mmc: tmio: tmio_mmc_host has .write16_hook

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current .write16_hook is implemented under tmio_mmc_data.
It goes to tmio_mmc_host by this patch.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    2 +-
 drivers/mmc/host/tmio_mmc.h       |    4 +++-
 drivers/mmc/host/tmio_mmc_pio.c   |    2 +-
 include/linux/mfd/tmio.h          |    3 ---
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 1e13a1f..6ffd981 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -212,11 +212,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		goto eprobe;
 
 	host->dma		= dma_priv;
+	host->write16_hook	= sh_mobile_sdhi_write16_hook;
 
 	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
-	mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
 	mmc_data->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk;
 	if (p) {
 		mmc_data->flags = p->tmio_flags;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 4c5eda8..2fb87ea 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -94,6 +94,8 @@ struct tmio_mmc_host {
 	struct mutex		ios_lock;	/* protect set_ios() context */
 	bool			native_hotplug;
 	bool			sdio_irq_enabled;
+
+	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 };
 
 struct tmio_mmc_host* tmio_mmc_host_alloc(struct platform_device *pdev);
@@ -183,7 +185,7 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val
 	/* If there is a hook and it returns non-zero then there
 	 * is an error and the write should be skipped
 	 */
-	if (host->pdata->write16_hook && host->pdata->write16_hook(host, addr))
+	if (host->write16_hook && host->write16_hook(host, addr))
 		return;
 	writew(val, host->ctl + (addr << host->pdata->bus_shift));
 }
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 580d00b..8bc0c77 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1090,7 +1090,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
 	tmio_mmc_of_parse(pdev, pdata);
 
 	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
-		pdata->write16_hook = NULL;
+		_host->write16_hook = NULL;
 
 	res_ctl = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res_ctl)
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 8d708c7..0872ca1 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -112,8 +112,6 @@ void tmio_core_mmc_clk_div(void __iomem *cnf, int shift, int state);
 
 struct dma_chan;
 
-struct tmio_mmc_host;
-
 /*
  * data for the MMC controller
  */
@@ -127,7 +125,6 @@ struct tmio_mmc_data {
 	unsigned int			cd_gpio;
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
-	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 	/* clock management callbacks */
 	int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
 	void (*clk_disable)(struct platform_device *pdev);
-- 
1.7.9.5

[PATCH 4/9] mmc: tmio: tmio_mmc_host has .clk_enable

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current .clk_enable is implemented under tmio_mmc_data.
It goes to tmio_mmc_host by this patch.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    2 +-
 drivers/mmc/host/tmio_mmc.h       |    1 +
 drivers/mmc/host/tmio_mmc_pio.c   |    5 ++---
 include/linux/mfd/tmio.h          |    1 -
 4 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 6ffd981..178ae0f 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -213,8 +213,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	host->dma		= dma_priv;
 	host->write16_hook	= sh_mobile_sdhi_write16_hook;
+	host->clk_enable	= sh_mobile_sdhi_clk_enable;
 
-	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
 	mmc_data->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 2fb87ea..0634a94 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -96,6 +96,7 @@ struct tmio_mmc_host {
 	bool			sdio_irq_enabled;
 
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
+	int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
 };
 
 struct tmio_mmc_host* tmio_mmc_host_alloc(struct platform_device *pdev);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 8bc0c77..511ce96 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -835,13 +835,12 @@ fail:
 static int tmio_mmc_clk_update(struct tmio_mmc_host *host)
 {
 	struct mmc_host *mmc = host->mmc;
-	struct tmio_mmc_data *pdata = host->pdata;
 	int ret;
 
-	if (!pdata->clk_enable)
+	if (!host->clk_enable)
 		return -ENOTSUPP;
 
-	ret = pdata->clk_enable(host->pdev, &mmc->f_max);
+	ret = host->clk_enable(host->pdev, &mmc->f_max);
 	if (!ret)
 		mmc->f_min = mmc->f_max / 512;
 
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 0872ca1..472587a 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -126,7 +126,6 @@ struct tmio_mmc_data {
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
 	/* clock management callbacks */
-	int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
 	void (*clk_disable)(struct platform_device *pdev);
 	int (*multi_io_quirk)(struct mmc_card *card,
 			      unsigned int direction, int blk_size);
-- 
1.7.9.5

[PATCH 5/9] mmc: tmio: tmio_mmc_host has .clk_disable

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current .clk_disable is implemented under tmio_mmc_data.
It goes to tmio_mmc_host by this patch.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    2 +-
 drivers/mmc/host/tmio_mmc.h       |    1 +
 drivers/mmc/host/tmio_mmc_pio.c   |    4 ++--
 include/linux/mfd/tmio.h          |    3 +--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 178ae0f..66951cc 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -214,8 +214,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	host->dma		= dma_priv;
 	host->write16_hook	= sh_mobile_sdhi_write16_hook;
 	host->clk_enable	= sh_mobile_sdhi_clk_enable;
+	host->clk_disable	= sh_mobile_sdhi_clk_disable;
 
-	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
 	mmc_data->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk;
 	if (p) {
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 0634a94..cdce015 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -97,6 +97,7 @@ struct tmio_mmc_host {
 
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 	int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
+	void (*clk_disable)(struct platform_device *pdev);
 };
 
 struct tmio_mmc_host* tmio_mmc_host_alloc(struct platform_device *pdev);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 511ce96..84123d3 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1248,8 +1248,8 @@ int tmio_mmc_host_runtime_suspend(struct device *dev)
 	if (host->clk_cache)
 		tmio_mmc_clk_stop(host);
 
-	if (host->pdata->clk_disable)
-		host->pdata->clk_disable(host->pdev);
+	if (host->clk_disable)
+		host->clk_disable(host->pdev);
 
 	return 0;
 }
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 472587a..a3f78da 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -125,8 +125,7 @@ struct tmio_mmc_data {
 	unsigned int			cd_gpio;
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
-	/* clock management callbacks */
-	void (*clk_disable)(struct platform_device *pdev);
+
 	int (*multi_io_quirk)(struct mmc_card *card,
 			      unsigned int direction, int blk_size);
 };
-- 
1.7.9.5

[PATCH 6/9] mmc: tmio: tmio_mmc_host has .multi_io_quirk

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current .multi_io_quirk is implemented under tmio_mmc_data.
It goes to tmio_mmc_host by this patch.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    2 +-
 drivers/mmc/host/tmio_mmc.h       |    2 ++
 drivers/mmc/host/tmio_mmc_pio.c   |    5 ++---
 include/linux/mfd/tmio.h          |    3 ---
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 66951cc..79206a6 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -215,9 +215,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	host->write16_hook	= sh_mobile_sdhi_write16_hook;
 	host->clk_enable	= sh_mobile_sdhi_clk_enable;
 	host->clk_disable	= sh_mobile_sdhi_clk_disable;
+	host->multi_io_quirk	= sh_mobile_sdhi_multi_io_quirk;
 
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
-	mmc_data->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk;
 	if (p) {
 		mmc_data->flags = p->tmio_flags;
 		mmc_data->ocr_mask = p->tmio_ocr_mask;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index cdce015..c669801 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -98,6 +98,8 @@ struct tmio_mmc_host {
 	int (*write16_hook)(struct tmio_mmc_host *host, int addr);
 	int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
 	void (*clk_disable)(struct platform_device *pdev);
+	int (*multi_io_quirk)(struct mmc_card *card,
+			      unsigned int direction, int blk_size);
 };
 
 struct tmio_mmc_host* tmio_mmc_host_alloc(struct platform_device *pdev);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 84123d3..c683384 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1004,10 +1004,9 @@ static int tmio_multi_io_quirk(struct mmc_card *card,
 			       unsigned int direction, int blk_size)
 {
 	struct tmio_mmc_host *host = mmc_priv(card->host);
-	struct tmio_mmc_data *pdata = host->pdata;
 
-	if (pdata->multi_io_quirk)
-		return pdata->multi_io_quirk(card, direction, blk_size);
+	if (host->multi_io_quirk)
+		return host->multi_io_quirk(card, direction, blk_size);
 
 	return blk_size;
 }
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index a3f78da..3edaa17 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -125,9 +125,6 @@ struct tmio_mmc_data {
 	unsigned int			cd_gpio;
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
-
-	int (*multi_io_quirk)(struct mmc_card *card,
-			      unsigned int direction, int blk_size);
 };
 
 /*
-- 
1.7.9.5

[PATCH 7/9] mmc: tmio: tmio_mmc_host has .bus_shift

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current .bus_shift is implemented under tmio_mmc_data.
It goes to tmio_mmc_host by this patch.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    5 ++---
 drivers/mmc/host/tmio_mmc.c       |    5 +++--
 drivers/mmc/host/tmio_mmc.h       |   17 +++++++++--------
 drivers/mmc/host/tmio_mmc_dma.c   |    2 +-
 include/linux/mfd/tmio.h          |    1 -
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 79206a6..f8268b7 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -216,6 +216,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	host->clk_enable	= sh_mobile_sdhi_clk_enable;
 	host->clk_disable	= sh_mobile_sdhi_clk_disable;
 	host->multi_io_quirk	= sh_mobile_sdhi_multi_io_quirk;
+	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
+	host->bus_shift		= resource_size(res) >> 9;
 
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
 	if (p) {
@@ -275,9 +277,6 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		dma_priv->dma_rx_offset = of_data->dma_rx_offset;
 	}
 
-	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
-	mmc_data->bus_shift = resource_size(res) >> 9;
-
 	ret = tmio_mmc_host_probe(host, mmc_data);
 	if (ret < 0)
 		goto efree;
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index f47ae08..9e8723d 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -88,14 +88,15 @@ static int tmio_mmc_probe(struct platform_device *pdev)
 	if (!res)
 		return -EINVAL;
 
-	/* SD control register space size is 0x200, 0x400 for bus_shift=1 */
-	pdata->bus_shift = resource_size(res) >> 10;
 	pdata->flags |= TMIO_MMC_HAVE_HIGH_REG;
 
 	host = tmio_mmc_host_alloc(pdev);
 	if (!host)
 		goto cell_disable;
 
+	/* 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);
 	if (ret)
 		goto host_free;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index c669801..fee7785 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -66,6 +66,7 @@ struct tmio_mmc_host {
 	struct scatterlist      *sg_orig;
 	unsigned int            sg_len;
 	unsigned int            sg_off;
+	unsigned long		bus_shift;
 
 	struct platform_device *pdev;
 	struct tmio_mmc_data *pdata;
@@ -169,19 +170,19 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
 
 static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->pdata->bus_shift));
+	return readw(host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
-	readsw(host->ctl + (addr << host->pdata->bus_shift), buf, count);
+	readsw(host->ctl + (addr << host->bus_shift), buf, count);
 }
 
 static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->pdata->bus_shift)) |
-	       readw(host->ctl + ((addr + 2) << host->pdata->bus_shift)) << 16;
+	return readw(host->ctl + (addr << host->bus_shift)) |
+	       readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
 }
 
 static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
@@ -191,19 +192,19 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val
 	 */
 	if (host->write16_hook && host->write16_hook(host, addr))
 		return;
-	writew(val, host->ctl + (addr << host->pdata->bus_shift));
+	writew(val, host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
-	writesw(host->ctl + (addr << host->pdata->bus_shift), buf, count);
+	writesw(host->ctl + (addr << host->bus_shift), buf, count);
 }
 
 static inline void sd_ctrl_write32(struct tmio_mmc_host *host, int addr, u32 val)
 {
-	writew(val, host->ctl + (addr << host->pdata->bus_shift));
-	writew(val >> 16, host->ctl + ((addr + 2) << host->pdata->bus_shift));
+	writew(val, host->ctl + (addr << host->bus_shift));
+	writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
 }
 
 
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index 6c214d6..ee0131e 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -289,7 +289,7 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
 		if (host->dma->chan_priv_tx)
 			cfg.slave_id = host->dma->slave_id_tx;
 		cfg.direction = DMA_MEM_TO_DEV;
-		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->pdata->bus_shift);
+		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift);
 		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 		cfg.src_addr = 0;
 		ret = dmaengine_slave_config(host->chan_tx, &cfg);
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 3edaa17..7a5c279 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -120,7 +120,6 @@ struct tmio_mmc_data {
 	unsigned long			capabilities;
 	unsigned long			capabilities2;
 	unsigned long			flags;
-	unsigned long			bus_shift;
 	u32				ocr_mask;	/* available voltages */
 	unsigned int			cd_gpio;
 	void (*set_pwr)(struct platform_device *host, int state);
-- 
1.7.9.5

[PATCH 8/9] mmc: sh_mobile_sdhi: remove .init/.cleanup

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

No one is using .init/.cleanup callback function.
Let's remove these.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c  |   26 +-------------------------
 include/linux/mmc/sh_mobile_sdhi.h |   15 ---------------
 2 files changed, 1 insertion(+), 40 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index f8268b7..eae229f 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -156,15 +156,6 @@ static int sh_mobile_sdhi_multi_io_quirk(struct mmc_card *card,
 	return blk_size;
 }
 
-static void sh_mobile_sdhi_cd_wakeup(const struct platform_device *pdev)
-{
-	mmc_detect_change(platform_get_drvdata(pdev), msecs_to_jiffies(100));
-}
-
-static const struct sh_mobile_sdhi_ops sdhi_ops = {
-	.cd_wakeup = sh_mobile_sdhi_cd_wakeup,
-};
-
 static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id @@ -192,19 +183,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	mmc_data = &priv->mmc_data;
 	dma_priv = &priv->dma_priv;
 
-	if (p) {
-		if (p->init) {
-			ret = p->init(pdev, &sdhi_ops);
-			if (ret)
-				return ret;
-		}
-	}
-
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		ret = PTR_ERR(priv->clk);
 		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
-		goto eclkget;
+		goto eprobe;
 	}
 
 	host = tmio_mmc_host_alloc(pdev);
@@ -357,9 +340,6 @@ eirq:
 efree:
 	tmio_mmc_host_free(host);
 eprobe:
-eclkget:
-	if (p && p->cleanup)
-		p->cleanup(pdev);
 	return ret;
 }
 
@@ -367,13 +347,9 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc = platform_get_drvdata(pdev);
 	struct tmio_mmc_host *host = mmc_priv(mmc);
-	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 
 	tmio_mmc_host_remove(host);
 
-	if (p && p->cleanup)
-		p->cleanup(pdev);
-
 	return 0;
 }
 
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index 68927ae..da77e5e20 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -3,20 +3,10 @@
 
 #include <linux/types.h>
 
-struct platform_device;
-
 #define SH_MOBILE_SDHI_IRQ_CARD_DETECT	"card_detect"
 #define SH_MOBILE_SDHI_IRQ_SDCARD	"sdcard"
 #define SH_MOBILE_SDHI_IRQ_SDIO		"sdio"
 
-/**
- * struct sh_mobile_sdhi_ops - SDHI driver callbacks
- * @cd_wakeup:		trigger a card-detection run
- */
-struct sh_mobile_sdhi_ops {
-	void (*cd_wakeup)(const struct platform_device *pdev);
-};
-
 struct sh_mobile_sdhi_info {
 	int dma_slave_tx;
 	int dma_slave_rx;
@@ -25,11 +15,6 @@ struct sh_mobile_sdhi_info {
 	unsigned long tmio_caps2;
 	u32 tmio_ocr_mask;	/* available MMC voltages */
 	unsigned int cd_gpio;
-
-	/* callbacks for board specific setup code */
-	int (*init)(struct platform_device *pdev,
-		    const struct sh_mobile_sdhi_ops *ops);
-	void (*cleanup)(struct platform_device *pdev);
 };
 
 #endif /* LINUX_MMC_SH_MOBILE_SDHI_H */
-- 
1.7.9.5

Re: [PATCH 8/9] mmc: sh_mobile_sdhi: remove .init/.cleanup

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, Jan 5, 2015 at 8:03 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> No one is using .init/.cleanup callback function.
> Let's remove these.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c  |   26 +-------------------------
>  include/linux/mmc/sh_mobile_sdhi.h |   15 ---------------
>  2 files changed, 1 insertion(+), 40 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index f8268b7..eae229f 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -156,15 +156,6 @@ static int sh_mobile_sdhi_multi_io_quirk(struct mmc_card *card,
>         return blk_size;
>  }
>
> -static void sh_mobile_sdhi_cd_wakeup(const struct platform_device *pdev)
> -{
> -       mmc_detect_change(platform_get_drvdata(pdev), msecs_to_jiffies(100));
> -}
> -
> -static const struct sh_mobile_sdhi_ops sdhi_ops = {
> -       .cd_wakeup = sh_mobile_sdhi_cd_wakeup,
> -};
> -

Is this intentional?

While .cd_wakeup looks indeed unused, you forgot to mention its removal in
the patch description.

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 8/9] mmc: sh_mobile_sdhi: remove .init/.cleanup

[Kuninori Morimoto]

Hi Geert

Thank you for your review.

> > -static void sh_mobile_sdhi_cd_wakeup(const struct platform_device *pdev)
> > -{
> > -       mmc_detect_change(platform_get_drvdata(pdev), msecs_to_jiffies(100));
> > -}
> > -
> > -static const struct sh_mobile_sdhi_ops sdhi_ops = {
> > -       .cd_wakeup = sh_mobile_sdhi_cd_wakeup,
> > -};
> > -
> 
> Is this intentional?
> 
> While .cd_wakeup looks indeed unused, you forgot to mention its removal in
> the patch description.

Because, sdhi_ops (and .cd_wakeup) is used from .init
OK, I will explain about it in patch description in v2
Thanks

Best regards
---
Kuninori Morimoto

[PATCH 9/9] mmc: sh_mobile_sdhi: remove sh_mobile_sdhi_info

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current sh_mobile_sdhi's platform data is set via sh_mobile_sdhi_info
and it is just copied to tmio_mmc_data.
Now, tmio mmc platform data is specified via tmio_mmc_data.
This patch moves missing dma tx/rx ID from sh_mobile_sdhi_info to
tmio_mmc_data, and removes sh_mobile_sdhi_info.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/mach-shmobile/board-ape6evm.c         |   12 ++++++------
 arch/arm/mach-shmobile/board-armadillo800eva.c |   12 ++++++------
 arch/arm/mach-shmobile/board-bockw.c           |   10 +++++-----
 arch/arm/mach-shmobile/board-kzm9g.c           |   12 ++++++------
 arch/arm/mach-shmobile/board-lager.c           |   16 ++++++++--------
 arch/arm/mach-shmobile/board-mackerel.c        |   14 +++++++-------
 arch/arm/mach-shmobile/board-marzen.c          |    6 +++---
 arch/sh/boards/board-sh7757lcr.c               |    5 +++--
 arch/sh/boards/mach-ap325rxa/setup.c           |    9 +++++----
 arch/sh/boards/mach-ecovec24/setup.c           |   12 ++++++------
 arch/sh/boards/mach-kfr2r09/setup.c            |    6 +++---
 arch/sh/boards/mach-migor/setup.c              |    5 +++--
 arch/sh/boards/mach-se/7724/setup.c            |    9 +++++----
 drivers/mmc/host/sh_mobile_sdhi.c              |   11 ++++-------
 include/linux/mfd/tmio.h                       |    2 ++
 include/linux/mmc/sh_mobile_sdhi.h             |   10 ----------
 16 files changed, 72 insertions(+), 79 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ape6evm.c b/arch/arm/mach-shmobile/board-ape6evm.c
index 66f6781..1b92aeb 100644
--- a/arch/arm/mach-shmobile/board-ape6evm.c
+++ b/arch/arm/mach-shmobile/board-ape6evm.c
@@ -171,9 +171,9 @@ static const struct resource mmcif0_resources[] __initconst = {
 };
 
 /* SDHI0 */
-static const struct sh_mobile_sdhi_info sdhi0_pdata __initconst = {
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
+static const struct tmio_mmc_data sdhi0_pdata __initconst = {
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 };
 
 static const struct resource sdhi0_resources[] __initconst = {
@@ -182,9 +182,9 @@ static const struct resource sdhi0_resources[] __initconst = {
 };
 
 /* SDHI1 */
-static const struct sh_mobile_sdhi_info sdhi1_pdata __initconst = {
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+static const struct tmio_mmc_data sdhi1_pdata __initconst = {
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_NEEDS_POLL,
 };
 
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 6d949f1..b0b0eb7 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -754,12 +754,12 @@ static struct platform_device vcc_sdhi1 = {
 };
 
 /* SDHI0 */
-static struct sh_mobile_sdhi_info sdhi0_info = {
+static struct tmio_mmc_data sdhi0_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_POWER_OFF_CARD,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
 	.cd_gpio	= 167,
 };
 
@@ -796,12 +796,12 @@ static struct platform_device sdhi0_device = {
 };
 
 /* SDHI1 */
-static struct sh_mobile_sdhi_info sdhi1_info = {
+static struct tmio_mmc_data sdhi1_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_POWER_OFF_CARD,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
 	/* Port72 cannot generate IRQs, will be used in polling mode. */
 	.cd_gpio	= 72,
 };
diff --git a/arch/arm/mach-shmobile/board-bockw.c b/arch/arm/mach-shmobile/board-bockw.c
index f27b5a8..a4ad559f 100644
--- a/arch/arm/mach-shmobile/board-bockw.c
+++ b/arch/arm/mach-shmobile/board-bockw.c
@@ -201,12 +201,12 @@ static struct rcar_phy_platform_data usb_phy_platform_data __initdata  
 
 /* SDHI */
-static struct sh_mobile_sdhi_info sdhi0_info __initdata = {
+static struct tmio_mmc_data sdhi0_info __initdata = {
 	.dma_slave_tx	= HPBDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= HPBDMA_SLAVE_SDHI0_RX,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
-	.tmio_ocr_mask	= MMC_VDD_165_195 | MMC_VDD_32_33 | MMC_VDD_33_34,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED,
+	.ocr_mask	= MMC_VDD_165_195 | MMC_VDD_32_33 | MMC_VDD_33_34,
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT,
 };
 
 static struct resource sdhi0_resources[] __initdata = {
@@ -683,7 +683,7 @@ static void __init bockw_init(void)
 		platform_device_register_resndata(
 			NULL, "sh_mobile_sdhi", 0,
 			sdhi0_resources, ARRAY_SIZE(sdhi0_resources),
-			&sdhi0_info, sizeof(struct sh_mobile_sdhi_info));
+			&sdhi0_info, sizeof(struct tmio_mmc_data));
 	}
 
 	/* for Audio */
diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
index 7c9b63b..e4e612d 100644
--- a/arch/arm/mach-shmobile/board-kzm9g.c
+++ b/arch/arm/mach-shmobile/board-kzm9g.c
@@ -442,11 +442,11 @@ static struct platform_device vcc_sdhi2 = {
 };
 
 /* SDHI */
-static struct sh_mobile_sdhi_info sdhi0_info = {
+static struct tmio_mmc_data sdhi0_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_POWER_OFF_CARD,
 };
 
@@ -484,13 +484,13 @@ static struct platform_device sdhi0_device = {
 };
 
 /* Micro SD */
-static struct sh_mobile_sdhi_info sdhi2_info = {
+static struct tmio_mmc_data sdhi2_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT |
 			  TMIO_MMC_USE_GPIO_CD |
 			  TMIO_MMC_WRPROTECT_DISABLE,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_POWER_OFF_CARD,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_POWER_OFF_CARD,
 	.cd_gpio	= 13,
 };
 
diff --git a/arch/arm/mach-shmobile/board-lager.c b/arch/arm/mach-shmobile/board-lager.c
index f8197eb..233161a 100644
--- a/arch/arm/mach-shmobile/board-lager.c
+++ b/arch/arm/mach-shmobile/board-lager.c
@@ -566,10 +566,10 @@ static void __init lager_add_rsnd_device(void)
 }
 
 /* SDHI0 */
-static struct sh_mobile_sdhi_info sdhi0_info __initdata = {
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+static struct tmio_mmc_data sdhi0_info __initdata = {
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_POWER_OFF_CARD,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT |
 			  TMIO_MMC_WRPROTECT_DISABLE,
 };
 
@@ -579,10 +579,10 @@ static struct resource sdhi0_resources[] __initdata = {
 };
 
 /* SDHI2 */
-static struct sh_mobile_sdhi_info sdhi2_info __initdata = {
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+static struct tmio_mmc_data sdhi2_info __initdata = {
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_POWER_OFF_CARD,
-	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
+	.flags		= TMIO_MMC_HAS_IDLE_WAIT |
 			  TMIO_MMC_WRPROTECT_DISABLE,
 };
 
@@ -777,10 +777,10 @@ static void __init lager_add_standard_devices(void)
 
 	platform_device_register_resndata(NULL, "sh_mobile_sdhi", 0,
 					  sdhi0_resources, ARRAY_SIZE(sdhi0_resources),
-					  &sdhi0_info, sizeof(struct sh_mobile_sdhi_info));
+					  &sdhi0_info, sizeof(struct tmio_mmc_data));
 	platform_device_register_resndata(NULL, "sh_mobile_sdhi", 2,
 					  sdhi2_resources, ARRAY_SIZE(sdhi2_resources),
-					  &sdhi2_info, sizeof(struct sh_mobile_sdhi_info));
+					  &sdhi2_info, sizeof(struct tmio_mmc_data));
 }
 
 /*
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index a1c1dfb..0f2dcb6 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -972,11 +972,11 @@ static struct platform_device nand_flash_device = {
 };
 
 /* SDHI0 */
-static struct sh_mobile_sdhi_info sdhi0_info = {
+static struct tmio_mmc_data sdhi0_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
+	.flags		= TMIO_MMC_USE_GPIO_CD,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 	.cd_gpio	= 172,
 };
 
@@ -1011,11 +1011,11 @@ static struct platform_device sdhi0_device = {
 /* SDHI1 */
 
 /* GPIO 41 can trigger IRQ8, but it is used by USBHS1, we have to poll */
-static struct sh_mobile_sdhi_info sdhi1_info = {
+static struct tmio_mmc_data sdhi1_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
-	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+	.flags		= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_NEEDS_POLL,
 	.cd_gpio	= 41,
 };
@@ -1054,7 +1054,7 @@ static struct platform_device sdhi1_device = {
  * The card detect pin of the top SD/MMC slot (CN23) is active low and is
  * connected to GPIO SCIFB_SCK of SH7372 (GPIO 162).
  */
-static struct sh_mobile_sdhi_info sdhi2_info = {
+static struct tmio_mmc_data sdhi2_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
diff --git a/arch/arm/mach-shmobile/board-marzen.c b/arch/arm/mach-shmobile/board-marzen.c
index 598f704..d6f07a3 100644
--- a/arch/arm/mach-shmobile/board-marzen.c
+++ b/arch/arm/mach-shmobile/board-marzen.c
@@ -122,11 +122,11 @@ static struct resource sdhi0_resources[] = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sdhi0_platform_data = {
+static struct tmio_mmc_data sdhi0_platform_data = {
 	.dma_slave_tx = HPBDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx = HPBDMA_SLAVE_SDHI0_RX,
-	.tmio_flags = TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_HAS_IDLE_WAIT,
-	.tmio_caps = MMC_CAP_SD_HIGHSPEED,
+	.flags        = TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_HAS_IDLE_WAIT,
+	.capabilities = MMC_CAP_SD_HIGHSPEED,
 };
 
 static struct platform_device sdhi0_device = {
diff --git a/arch/sh/boards/board-sh7757lcr.c b/arch/sh/boards/board-sh7757lcr.c
index 669df51..2211ff5 100644
--- a/arch/sh/boards/board-sh7757lcr.c
+++ b/arch/sh/boards/board-sh7757lcr.c
@@ -17,6 +17,7 @@
 #include <linux/spi/spi.h>
 #include <linux/spi/flash.h>
 #include <linux/io.h>
+#include <linux/mfd/tmio.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sh_mmcif.h>
 #include <linux/mmc/sh_mobile_sdhi.h>
@@ -243,10 +244,10 @@ static struct platform_device sh_mmcif_device = {
 };
 
 /* SDHI0 */
-static struct sh_mobile_sdhi_info sdhi_info = {
+static struct tmio_mmc_data sdhi_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI_RX,
-	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED,
 };
 
 static struct resource sdhi_resources[] = {
diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
index d4b01d4..cbd2a9f 100644
--- a/arch/sh/boards/mach-ap325rxa/setup.c
+++ b/arch/sh/boards/mach-ap325rxa/setup.c
@@ -18,6 +18,7 @@
 #include <linux/mmc/sh_mobile_sdhi.h>
 #include <linux/mtd/physmap.h>
 #include <linux/mtd/sh_flctl.h>
+#include <linux/mfd/tmio.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/regulator/fixed.h>
@@ -447,8 +448,8 @@ static struct resource sdhi0_cn3_resources[] = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sdhi0_cn3_data = {
-	.tmio_caps      = MMC_CAP_SDIO_IRQ,
+static struct tmio_mmc_data sdhi0_cn3_data = {
+	.capabilities	= MMC_CAP_SDIO_IRQ,
 };
 
 static struct platform_device sdhi0_cn3_device = {
@@ -474,8 +475,8 @@ static struct resource sdhi1_cn7_resources[] = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sdhi1_cn7_data = {
-	.tmio_caps      = MMC_CAP_SDIO_IRQ,
+static struct tmio_mmc_data sdhi1_cn7_data = {
+	.capabilities	= MMC_CAP_SDIO_IRQ,
 };
 
 static struct platform_device sdhi1_cn7_device = {
diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 0d30492..98cdd7e 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -601,12 +601,12 @@ static struct platform_device sdhi0_power = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sdhi0_info = {
+static struct tmio_mmc_data sdhi0_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_caps      = MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
+	.capabilities	= MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
 			  MMC_CAP_NEEDS_POLL,
-	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
+	.flags		= TMIO_MMC_USE_GPIO_CD,
 	.cd_gpio	= GPIO_PTY7,
 };
 
@@ -635,12 +635,12 @@ static struct platform_device sdhi0_device = {
 
 #if !defined(CONFIG_MMC_SH_MMCIF) && !defined(CONFIG_MMC_SH_MMCIF_MODULE)
 /* SDHI1 */
-static struct sh_mobile_sdhi_info sdhi1_info = {
+static struct tmio_mmc_data sdhi1_info = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
-	.tmio_caps      = MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
+	.capabilities	= MMC_CAP_SDIO_IRQ | MMC_CAP_POWER_OFF_CARD |
 			  MMC_CAP_NEEDS_POLL,
-	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
+	.flags		= TMIO_MMC_USE_GPIO_CD,
 	.cd_gpio	= GPIO_PTW7,
 };
 
diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
index 1df4398..00836ee 100644
--- a/arch/sh/boards/mach-kfr2r09/setup.c
+++ b/arch/sh/boards/mach-kfr2r09/setup.c
@@ -373,11 +373,11 @@ static struct resource kfr2r09_sh_sdhi0_resources[] = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sh7724_sdhi0_data = {
+static struct tmio_mmc_data sh7724_sdhi0_data = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE,
-	.tmio_caps      = MMC_CAP_SDIO_IRQ,
+	.flags		= TMIO_MMC_WRPROTECT_DISABLE,
+	.capabilities	= MMC_CAP_SDIO_IRQ,
 };
 
 static struct platform_device kfr2r09_sh_sdhi0_device = {
diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index 8b73194..ed1ba70 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -15,6 +15,7 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/sh_mobile_sdhi.h>
 #include <linux/mtd/physmap.h>
+#include <linux/mfd/tmio.h>
 #include <linux/mtd/nand.h>
 #include <linux/i2c.h>
 #include <linux/regulator/fixed.h>
@@ -408,10 +409,10 @@ static struct resource sdhi_cn9_resources[] = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sh7724_sdhi_data = {
+static struct tmio_mmc_data sh7724_sdhi_data = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_caps      = MMC_CAP_SDIO_IRQ,
+	.capabilities	= MMC_CAP_SDIO_IRQ,
 };
 
 static struct platform_device sdhi_cn9_device = {
diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
index 1162bc6..d68e40f 100644
--- a/arch/sh/boards/mach-se/7724/setup.c
+++ b/arch/sh/boards/mach-se/7724/setup.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sh_mobile_sdhi.h>
+#include <linux/mfd/tmio.h>
 #include <linux/mtd/physmap.h>
 #include <linux/delay.h>
 #include <linux/regulator/fixed.h>
@@ -468,10 +469,10 @@ static struct resource sdhi0_cn7_resources[] = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sh7724_sdhi0_data = {
+static struct tmio_mmc_data sh7724_sdhi0_data = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
-	.tmio_caps      = MMC_CAP_SDIO_IRQ,
+	.capabilities	= MMC_CAP_SDIO_IRQ,
 };
 
 static struct platform_device sdhi0_cn7_device = {
@@ -497,10 +498,10 @@ static struct resource sdhi1_cn8_resources[] = {
 	},
 };
 
-static struct sh_mobile_sdhi_info sh7724_sdhi1_data = {
+static struct tmio_mmc_data sh7724_sdhi1_data = {
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
-	.tmio_caps      = MMC_CAP_SDIO_IRQ,
+	.capabilities	= MMC_CAP_SDIO_IRQ,
 };
 
 static struct platform_device sdhi1_cn8_device = {
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index eae229f..1f70fca 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -162,7 +162,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		of_match_device(sh_mobile_sdhi_of_match, &pdev->dev);
 	struct sh_mobile_sdhi *priv;
 	struct tmio_mmc_data *mmc_data;
-	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
+	struct tmio_mmc_data *p = pdev->dev.platform_data;
 	struct tmio_mmc_host *host;
 	struct resource *res;
 	int irq, ret, i = 0;
@@ -202,13 +202,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	/* SD control register space size is 0x100, 0x200 for bus_shift=1 */
 	host->bus_shift		= resource_size(res) >> 9;
 
-	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
 	if (p) {
-		mmc_data->flags = p->tmio_flags;
-		mmc_data->ocr_mask = p->tmio_ocr_mask;
-		mmc_data->capabilities |= p->tmio_caps;
-		mmc_data->capabilities2 |= p->tmio_caps2;
-		mmc_data->cd_gpio = p->cd_gpio;
+		*mmc_data = *p;
 
 		if (p->dma_slave_tx > 0 && p->dma_slave_rx > 0) {
 			/*
@@ -226,6 +221,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	dma_priv->alignment_shift = 1; /* 2-byte alignment */
 	dma_priv->filter = shdma_chan_filter;
 
+	mmc_data->capabilities |= MMC_CAP_MMC_HIGHSPEED;
+
 	/*
 	 * All SDHI blocks support 2-byte and larger block sizes in 4-bit
 	 * bus width mode.
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 7a5c279..8505039 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -116,6 +116,8 @@ struct dma_chan;
  * data for the MMC controller
  */
 struct tmio_mmc_data {
+	int dma_slave_tx;
+	int dma_slave_rx;
 	unsigned int			hclk;
 	unsigned long			capabilities;
 	unsigned long			capabilities2;
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index da77e5e20..95d6f03 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -7,14 +7,4 @@
 #define SH_MOBILE_SDHI_IRQ_SDCARD	"sdcard"
 #define SH_MOBILE_SDHI_IRQ_SDIO		"sdio"
 
-struct sh_mobile_sdhi_info {
-	int dma_slave_tx;
-	int dma_slave_rx;
-	unsigned long tmio_flags;
-	unsigned long tmio_caps;
-	unsigned long tmio_caps2;
-	u32 tmio_ocr_mask;	/* available MMC voltages */
-	unsigned int cd_gpio;
-};
-
 #endif /* LINUX_MMC_SH_MOBILE_SDHI_H */
-- 
1.7.9.5