All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-05-31 13:56 ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

Since eec95ee2261 "mmc: sdhi/tmio: switch to using
dmaengine_slave_config()", we are getting a link error with
a number of shmobile configurations that turn on the sdhi
driver but the not the SH_DMAE_BASE driver.

The reason is that the driver now incorrectly refers to
a global filter function that should be referenced only
by platform code:

drivers/built-in.o: In function `sh_mobile_sdhi_probe':
drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'

The fix is to move the pointer to the filter function out
of the driver, similar to how we do it for most other
platforms. This makes the driver almost independent
of the underlying dma engine, besides the fact that
it tries to pass the same number both into the filter
function and into slave config, which only works on
some DMA engine drivers.

Since the bug is only present on the mmc tree, I think
the fix should be applied there as well.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++++
 arch/arm/mach-shmobile/board-ap4evb.c          |  7 +++++++
 arch/arm/mach-shmobile/board-armadillo800eva.c |  7 +++++++
 arch/arm/mach-shmobile/board-kzm9g.c           |  7 +++++++
 arch/arm/mach-shmobile/board-mackerel.c        | 10 ++++++++++
 drivers/mmc/host/sh_mobile_sdhi.c              |  9 ++++++++-
 include/linux/mmc/sh_mobile_sdhi.h             |  1 +
 7 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index c754071..ad01651 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -42,6 +42,7 @@
 #include <linux/mmc/sh_mobile_sdhi.h>
 #include <linux/mfd/tmio.h>
 #include <linux/sh_clk.h>
+#include <linux/sh_dma.h>
 #include <linux/irqchip/arm-gic.h>
 #include <video/sh_mobile_lcdc.h>
 #include <video/sh_mipi_dsi.h>
@@ -403,8 +404,11 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[]  
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
 	.tmio_ocr_mask	= MMC_VDD_27_28 | MMC_VDD_28_29,
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 297bf5e..c900e24 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -38,6 +38,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
+#include <linux/sh_dma.h>
 #include <linux/sh_intc.h>
 #include <linux/sh_clk.h>
 #include <linux/gpio.h>
@@ -355,8 +356,11 @@ static struct platform_device sh_mmcif_device = {
 
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_caps	= MMC_CAP_SDIO_IRQ,
 };
 
@@ -393,8 +397,11 @@ static struct platform_device sdhi0_device = {
 
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_ocr_mask	= MMC_VDD_165_195,
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE,
 	.tmio_caps	= MMC_CAP_NEEDS_POLL | MMC_CAP_SDIO_IRQ,
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 44a6215..e95e5dc 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/gpio-regulator.h>
 #include <linux/regulator/machine.h>
+#include <linux/sh_dma.h>
 #include <linux/sh_eth.h>
 #include <linux/videodev2.h>
 #include <linux/usb/renesas_usbhs.h>
@@ -688,8 +689,11 @@ static struct platform_device vcc_sdhi1 = {
  */
 #define IRQ31	irq_pin(31)
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_caps	= 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,
@@ -730,8 +734,11 @@ static struct platform_device sdhi0_device = {
 
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_caps	= 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,
diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
index 165483c..0434b36 100644
--- a/arch/arm/mach-shmobile/board-kzm9g.c
+++ b/arch/arm/mach-shmobile/board-kzm9g.c
@@ -37,6 +37,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
+#include <linux/sh_dma.h>
 #include <linux/usb/r8a66597.h>
 #include <linux/usb/renesas_usbhs.h>
 #include <linux/videodev2.h>
@@ -442,8 +443,11 @@ static struct platform_device vcc_sdhi2 = {
 
 /* SDHI */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_POWER_OFF_CARD,
@@ -484,8 +488,11 @@ static struct platform_device sdhi0_device = {
 
 /* Micro SD */
 static struct sh_mobile_sdhi_info sdhi2_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
 			  TMIO_MMC_USE_GPIO_CD |
 			  TMIO_MMC_WRPROTECT_DISABLE,
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 85f51a8..49755ff 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -45,6 +45,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
+#include <linux/sh_dma.h>
 #include <linux/sh_intc.h>
 #include <linux/tca6416_keypad.h>
 #include <linux/usb/renesas_usbhs.h>
@@ -971,8 +972,11 @@ static struct platform_device nand_flash_device = {
 
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 	.cd_gpio	= 172,
@@ -1010,8 +1014,11 @@ static struct platform_device sdhi0_device = {
 
 /* GPIO 41 can trigger IRQ8, but it is used by USBHS1, we have to poll */
 static struct sh_mobile_sdhi_info sdhi1_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_NEEDS_POLL,
@@ -1053,8 +1060,11 @@ static struct platform_device sdhi1_device = {
  * connected to GPIO SCIFB_SCK of SH7372 (GPIO 162).
  */
 static struct sh_mobile_sdhi_info sdhi2_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_NEEDS_POLL,
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index cc4c872..efe3386 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -193,13 +193,20 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 			 */
 			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
 			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
+			/*
+			 * This is a layering violation: the slave driver
+			 * should not be aware that the chan_priv_* is the
+			 * slave id.
+			 * We should not really need to set the slave id
+			 * here anyway. -arnd
+			 */
 			dma_priv->slave_id_tx = p->dma_slave_tx;
 			dma_priv->slave_id_rx = p->dma_slave_rx;
+			dma_priv->filter = p->dma_filter;
 		}
 	}
 
 	dma_priv->alignment_shift = 1; /* 2-byte alignment */
-	dma_priv->filter = shdma_chan_filter;
 
 	mmc_data->dma = dma_priv;
 
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index b76bcf0..342f07b 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -20,6 +20,7 @@ struct sh_mobile_sdhi_ops {
 struct sh_mobile_sdhi_info {
 	int dma_slave_tx;
 	int dma_slave_rx;
+	dma_filter_fn dma_filter;
 	unsigned long tmio_flags;
 	unsigned long tmio_caps;
 	unsigned long tmio_caps2;
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-05-31 13:56 ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

Since eec95ee2261 "mmc: sdhi/tmio: switch to using
dmaengine_slave_config()", we are getting a link error with
a number of shmobile configurations that turn on the sdhi
driver but the not the SH_DMAE_BASE driver.

The reason is that the driver now incorrectly refers to
a global filter function that should be referenced only
by platform code:

drivers/built-in.o: In function `sh_mobile_sdhi_probe':
drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'

The fix is to move the pointer to the filter function out
of the driver, similar to how we do it for most other
platforms. This makes the driver almost independent
of the underlying dma engine, besides the fact that
it tries to pass the same number both into the filter
function and into slave config, which only works on
some DMA engine drivers.

Since the bug is only present on the mmc tree, I think
the fix should be applied there as well.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++++
 arch/arm/mach-shmobile/board-ap4evb.c          |  7 +++++++
 arch/arm/mach-shmobile/board-armadillo800eva.c |  7 +++++++
 arch/arm/mach-shmobile/board-kzm9g.c           |  7 +++++++
 arch/arm/mach-shmobile/board-mackerel.c        | 10 ++++++++++
 drivers/mmc/host/sh_mobile_sdhi.c              |  9 ++++++++-
 include/linux/mmc/sh_mobile_sdhi.h             |  1 +
 7 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index c754071..ad01651 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -42,6 +42,7 @@
 #include <linux/mmc/sh_mobile_sdhi.h>
 #include <linux/mfd/tmio.h>
 #include <linux/sh_clk.h>
+#include <linux/sh_dma.h>
 #include <linux/irqchip/arm-gic.h>
 #include <video/sh_mobile_lcdc.h>
 #include <video/sh_mipi_dsi.h>
@@ -403,8 +404,11 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[] =
 
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
 	.tmio_ocr_mask	= MMC_VDD_27_28 | MMC_VDD_28_29,
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 297bf5e..c900e24 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -38,6 +38,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
+#include <linux/sh_dma.h>
 #include <linux/sh_intc.h>
 #include <linux/sh_clk.h>
 #include <linux/gpio.h>
@@ -355,8 +356,11 @@ static struct platform_device sh_mmcif_device = {
 
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_caps	= MMC_CAP_SDIO_IRQ,
 };
 
@@ -393,8 +397,11 @@ static struct platform_device sdhi0_device = {
 
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_ocr_mask	= MMC_VDD_165_195,
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE,
 	.tmio_caps	= MMC_CAP_NEEDS_POLL | MMC_CAP_SDIO_IRQ,
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 44a6215..e95e5dc 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/gpio-regulator.h>
 #include <linux/regulator/machine.h>
+#include <linux/sh_dma.h>
 #include <linux/sh_eth.h>
 #include <linux/videodev2.h>
 #include <linux/usb/renesas_usbhs.h>
@@ -688,8 +689,11 @@ static struct platform_device vcc_sdhi1 = {
  */
 #define IRQ31	irq_pin(31)
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_caps	= 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,
@@ -730,8 +734,11 @@ static struct platform_device sdhi0_device = {
 
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_caps	= 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,
diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
index 165483c..0434b36 100644
--- a/arch/arm/mach-shmobile/board-kzm9g.c
+++ b/arch/arm/mach-shmobile/board-kzm9g.c
@@ -37,6 +37,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
+#include <linux/sh_dma.h>
 #include <linux/usb/r8a66597.h>
 #include <linux/usb/renesas_usbhs.h>
 #include <linux/videodev2.h>
@@ -442,8 +443,11 @@ static struct platform_device vcc_sdhi2 = {
 
 /* SDHI */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_POWER_OFF_CARD,
@@ -484,8 +488,11 @@ static struct platform_device sdhi0_device = {
 
 /* Micro SD */
 static struct sh_mobile_sdhi_info sdhi2_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
 			  TMIO_MMC_USE_GPIO_CD |
 			  TMIO_MMC_WRPROTECT_DISABLE,
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 85f51a8..49755ff 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -45,6 +45,7 @@
 #include <linux/regulator/fixed.h>
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
+#include <linux/sh_dma.h>
 #include <linux/sh_intc.h>
 #include <linux/tca6416_keypad.h>
 #include <linux/usb/renesas_usbhs.h>
@@ -971,8 +972,11 @@ static struct platform_device nand_flash_device = {
 
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 	.cd_gpio	= 172,
@@ -1010,8 +1014,11 @@ static struct platform_device sdhi0_device = {
 
 /* GPIO 41 can trigger IRQ8, but it is used by USBHS1, we have to poll */
 static struct sh_mobile_sdhi_info sdhi1_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_NEEDS_POLL,
@@ -1053,8 +1060,11 @@ static struct platform_device sdhi1_device = {
  * connected to GPIO SCIFB_SCK of SH7372 (GPIO 162).
  */
 static struct sh_mobile_sdhi_info sdhi2_info = {
+#ifdef CONFIG_SH_DMAE_BASE
 	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
 	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_filter	= shdma_chan_filter,
+#endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
 			  MMC_CAP_NEEDS_POLL,
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index cc4c872..efe3386 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -193,13 +193,20 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 			 */
 			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
 			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
+			/*
+			 * This is a layering violation: the slave driver
+			 * should not be aware that the chan_priv_* is the
+			 * slave id.
+			 * We should not really need to set the slave id
+			 * here anyway. -arnd
+			 */
 			dma_priv->slave_id_tx = p->dma_slave_tx;
 			dma_priv->slave_id_rx = p->dma_slave_rx;
+			dma_priv->filter = p->dma_filter;
 		}
 	}
 
 	dma_priv->alignment_shift = 1; /* 2-byte alignment */
-	dma_priv->filter = shdma_chan_filter;
 
 	mmc_data->dma = dma_priv;
 
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index b76bcf0..342f07b 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -20,6 +20,7 @@ struct sh_mobile_sdhi_ops {
 struct sh_mobile_sdhi_info {
 	int dma_slave_tx;
 	int dma_slave_rx;
+	dma_filter_fn dma_filter;
 	unsigned long tmio_flags;
 	unsigned long tmio_caps;
 	unsigned long tmio_caps2;
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-05-31 13:56 ` Arnd Bergmann
@ 2013-05-31 13:56   ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

The MMC driver should not need to care what the dma engine
is that it is using, so it must not rely on the argument
to the filter function to be a 'slave_id' value.

Passing the slave id in dmaengine_slave_config is not
portable, and does not work with DT-enabled systems,
so this turns the filter argument into a void pointer
that gets set by the platform code and gets passed
to the dmaengine code as an opaque value.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++--
 arch/arm/mach-shmobile/board-ap4evb.c          |  8 ++++----
 arch/arm/mach-shmobile/board-armadillo800eva.c |  8 ++++----
 arch/arm/mach-shmobile/board-kzm9g.c           |  8 ++++----
 arch/arm/mach-shmobile/board-mackerel.c        | 12 ++++++------
 drivers/mmc/host/sh_mobile_sdhi.c              | 20 +++-----------------
 drivers/mmc/host/tmio_mmc_dma.c                |  6 ++----
 include/linux/mfd/tmio.h                       |  2 --
 include/linux/mmc/sh_mobile_sdhi.h             |  5 +++--
 9 files changed, 28 insertions(+), 45 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ad01651..cb467fb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -405,8 +405,8 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[]  /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index c900e24..d7ccb2e 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -357,8 +357,8 @@ static struct platform_device sh_mmcif_device = {
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_caps	= MMC_CAP_SDIO_IRQ,
@@ -398,8 +398,8 @@ static struct platform_device sdhi0_device = {
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI1_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI1_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_ocr_mask	= MMC_VDD_165_195,
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index e95e5dc..fcea414 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -690,8 +690,8 @@ static struct platform_device vcc_sdhi1 = {
 #define IRQ31	irq_pin(31)
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
@@ -735,8 +735,8 @@ static struct platform_device sdhi0_device = {
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI1_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI1_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
index 0434b36..3f3092f 100644
--- a/arch/arm/mach-shmobile/board-kzm9g.c
+++ b/arch/arm/mach-shmobile/board-kzm9g.c
@@ -444,8 +444,8 @@ static struct platform_device vcc_sdhi2 = {
 /* SDHI */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
@@ -489,8 +489,8 @@ static struct platform_device sdhi0_device = {
 /* Micro SD */
 static struct sh_mobile_sdhi_info sdhi2_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI2_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI2_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 49755ff..4522b76 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -973,8 +973,8 @@ static struct platform_device nand_flash_device = {
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
@@ -1015,8 +1015,8 @@ static struct platform_device sdhi0_device = {
 /* GPIO 41 can trigger IRQ8, but it is used by USBHS1, we have to poll */
 static struct sh_mobile_sdhi_info sdhi1_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI1_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI1_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
@@ -1061,8 +1061,8 @@ static struct platform_device sdhi1_device = {
  */
 static struct sh_mobile_sdhi_info sdhi2_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI2_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI2_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index efe3386..c4a0aab 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -185,23 +185,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		if (p->get_cd)
 			mmc_data->get_cd = sh_mobile_sdhi_get_cd;
 
-		if (p->dma_slave_tx > 0 && p->dma_slave_rx > 0) {
-			/*
-			 * Yes, we have to provide slave IDs twice to TMIO:
-			 * once as a filter parameter and once for channel
-			 * configuration as an explicit slave ID
-			 */
-			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
-			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
-			/*
-			 * This is a layering violation: the slave driver
-			 * should not be aware that the chan_priv_* is the
-			 * slave id.
-			 * We should not really need to set the slave id
-			 * here anyway. -arnd
-			 */
-			dma_priv->slave_id_tx = p->dma_slave_tx;
-			dma_priv->slave_id_rx = p->dma_slave_rx;
+		if (p->dma_slave_tx && p->dma_slave_rx) {
+			dma_priv->chan_priv_tx = p->dma_slave_tx;
+			dma_priv->chan_priv_rx = p->dma_slave_rx;
 			dma_priv->filter = p->dma_filter;
 		}
 	}
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index 47bdb8f..caad28e 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -290,8 +290,7 @@ 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.slave_id = 0; /* already set */
 		cfg.direction = DMA_MEM_TO_DEV;
 		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift);
 		cfg.src_addr = 0;
@@ -308,8 +307,7 @@ 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.slave_id = 0;
 		cfg.direction = DMA_DEV_TO_MEM;
 		cfg.src_addr = cfg.dst_addr;
 		cfg.dst_addr = 0;
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index ce35113..0990d8a 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -86,8 +86,6 @@ 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;
 	bool (*filter)(struct dma_chan *chan, void *arg);
 };
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index 342f07b..66a7d1c 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -2,6 +2,7 @@
 #define LINUX_MMC_SH_MOBILE_SDHI_H
 
 #include <linux/types.h>
+#include <linux/dmaengine.h>
 
 struct platform_device;
 
@@ -18,8 +19,8 @@ struct sh_mobile_sdhi_ops {
 };
 
 struct sh_mobile_sdhi_info {
-	int dma_slave_tx;
-	int dma_slave_rx;
+	void *dma_slave_tx;
+	void *dma_slave_rx;
 	dma_filter_fn dma_filter;
 	unsigned long tmio_flags;
 	unsigned long tmio_caps;
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-05-31 13:56   ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

The MMC driver should not need to care what the dma engine
is that it is using, so it must not rely on the argument
to the filter function to be a 'slave_id' value.

Passing the slave id in dmaengine_slave_config is not
portable, and does not work with DT-enabled systems,
so this turns the filter argument into a void pointer
that gets set by the platform code and gets passed
to the dmaengine code as an opaque value.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++--
 arch/arm/mach-shmobile/board-ap4evb.c          |  8 ++++----
 arch/arm/mach-shmobile/board-armadillo800eva.c |  8 ++++----
 arch/arm/mach-shmobile/board-kzm9g.c           |  8 ++++----
 arch/arm/mach-shmobile/board-mackerel.c        | 12 ++++++------
 drivers/mmc/host/sh_mobile_sdhi.c              | 20 +++-----------------
 drivers/mmc/host/tmio_mmc_dma.c                |  6 ++----
 include/linux/mfd/tmio.h                       |  2 --
 include/linux/mmc/sh_mobile_sdhi.h             |  5 +++--
 9 files changed, 28 insertions(+), 45 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index ad01651..cb467fb 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -405,8 +405,8 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[] =
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index c900e24..d7ccb2e 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -357,8 +357,8 @@ static struct platform_device sh_mmcif_device = {
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_caps	= MMC_CAP_SDIO_IRQ,
@@ -398,8 +398,8 @@ static struct platform_device sdhi0_device = {
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI1_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI1_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_ocr_mask	= MMC_VDD_165_195,
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index e95e5dc..fcea414 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -690,8 +690,8 @@ static struct platform_device vcc_sdhi1 = {
 #define IRQ31	irq_pin(31)
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
@@ -735,8 +735,8 @@ static struct platform_device sdhi0_device = {
 /* SDHI1 */
 static struct sh_mobile_sdhi_info sdhi1_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI1_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI1_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
index 0434b36..3f3092f 100644
--- a/arch/arm/mach-shmobile/board-kzm9g.c
+++ b/arch/arm/mach-shmobile/board-kzm9g.c
@@ -444,8 +444,8 @@ static struct platform_device vcc_sdhi2 = {
 /* SDHI */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
@@ -489,8 +489,8 @@ static struct platform_device sdhi0_device = {
 /* Micro SD */
 static struct sh_mobile_sdhi_info sdhi2_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI2_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI2_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 49755ff..4522b76 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -973,8 +973,8 @@ static struct platform_device nand_flash_device = {
 /* SDHI0 */
 static struct sh_mobile_sdhi_info sdhi0_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI0_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI0_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
@@ -1015,8 +1015,8 @@ static struct platform_device sdhi0_device = {
 /* GPIO 41 can trigger IRQ8, but it is used by USBHS1, we have to poll */
 static struct sh_mobile_sdhi_info sdhi1_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI1_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI1_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
@@ -1061,8 +1061,8 @@ static struct platform_device sdhi1_device = {
  */
 static struct sh_mobile_sdhi_info sdhi2_info = {
 #ifdef CONFIG_SH_DMAE_BASE
-	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
-	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
+	.dma_slave_tx	= (void *)SHDMA_SLAVE_SDHI2_TX,
+	.dma_slave_rx	= (void *)SHDMA_SLAVE_SDHI2_RX,
 	.dma_filter	= shdma_chan_filter,
 #endif
 	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index efe3386..c4a0aab 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -185,23 +185,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		if (p->get_cd)
 			mmc_data->get_cd = sh_mobile_sdhi_get_cd;
 
-		if (p->dma_slave_tx > 0 && p->dma_slave_rx > 0) {
-			/*
-			 * Yes, we have to provide slave IDs twice to TMIO:
-			 * once as a filter parameter and once for channel
-			 * configuration as an explicit slave ID
-			 */
-			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
-			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
-			/*
-			 * This is a layering violation: the slave driver
-			 * should not be aware that the chan_priv_* is the
-			 * slave id.
-			 * We should not really need to set the slave id
-			 * here anyway. -arnd
-			 */
-			dma_priv->slave_id_tx = p->dma_slave_tx;
-			dma_priv->slave_id_rx = p->dma_slave_rx;
+		if (p->dma_slave_tx && p->dma_slave_rx) {
+			dma_priv->chan_priv_tx = p->dma_slave_tx;
+			dma_priv->chan_priv_rx = p->dma_slave_rx;
 			dma_priv->filter = p->dma_filter;
 		}
 	}
diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
index 47bdb8f..caad28e 100644
--- a/drivers/mmc/host/tmio_mmc_dma.c
+++ b/drivers/mmc/host/tmio_mmc_dma.c
@@ -290,8 +290,7 @@ 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.slave_id = 0; /* already set */
 		cfg.direction = DMA_MEM_TO_DEV;
 		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift);
 		cfg.src_addr = 0;
@@ -308,8 +307,7 @@ 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.slave_id = 0;
 		cfg.direction = DMA_DEV_TO_MEM;
 		cfg.src_addr = cfg.dst_addr;
 		cfg.dst_addr = 0;
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index ce35113..0990d8a 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -86,8 +86,6 @@ 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;
 	bool (*filter)(struct dma_chan *chan, void *arg);
 };
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index 342f07b..66a7d1c 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -2,6 +2,7 @@
 #define LINUX_MMC_SH_MOBILE_SDHI_H
 
 #include <linux/types.h>
+#include <linux/dmaengine.h>
 
 struct platform_device;
 
@@ -18,8 +19,8 @@ struct sh_mobile_sdhi_ops {
 };
 
 struct sh_mobile_sdhi_info {
-	int dma_slave_tx;
-	int dma_slave_rx;
+	void *dma_slave_tx;
+	void *dma_slave_rx;
 	dma_filter_fn dma_filter;
 	unsigned long tmio_flags;
 	unsigned long tmio_caps;
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-05-31 13:56   ` Arnd Bergmann
@ 2013-05-31 13:58     ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 13:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 May 2013 15:56:45 Arnd Bergmann wrote:
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -2,6 +2,7 @@
>  #define LINUX_MMC_SH_MOBILE_SDHI_H
>  
>  #include <linux/types.h>
> +#include <linux/dmaengine.h>
>  
>  struct platform_device;
>  

This hunk should have been in the first patch. I'll
wait for comments before re-sending. Don't apply as-is.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-05-31 13:58     ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 13:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 May 2013 15:56:45 Arnd Bergmann wrote:
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -2,6 +2,7 @@
>  #define LINUX_MMC_SH_MOBILE_SDHI_H
>  
>  #include <linux/types.h>
> +#include <linux/dmaengine.h>
>  
>  struct platform_device;
>  

This hunk should have been in the first patch. I'll
wait for comments before re-sending. Don't apply as-is.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-05-31 13:56 ` Arnd Bergmann
@ 2013-05-31 14:52   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-05-31 14:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

Thanks for the patches. How about this fix:

http://thread.gmane.org/gmane.linux.kernel.mmc/20619

I think it's a better fix. SHDMA users cannot use other DMA drivers, so, I 
don't think we need to pass the filter via the platform data.

Thanks
Guennadi

On Fri, 31 May 2013, Arnd Bergmann wrote:

> Since eec95ee2261 "mmc: sdhi/tmio: switch to using
> dmaengine_slave_config()", we are getting a link error with
> a number of shmobile configurations that turn on the sdhi
> driver but the not the SH_DMAE_BASE driver.
> 
> The reason is that the driver now incorrectly refers to
> a global filter function that should be referenced only
> by platform code:
> 
> drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> 
> The fix is to move the pointer to the filter function out
> of the driver, similar to how we do it for most other
> platforms. This makes the driver almost independent
> of the underlying dma engine, besides the fact that
> it tries to pass the same number both into the filter
> function and into slave config, which only works on
> some DMA engine drivers.
> 
> Since the bug is only present on the mmc tree, I think
> the fix should be applied there as well.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++++
>  arch/arm/mach-shmobile/board-ap4evb.c          |  7 +++++++
>  arch/arm/mach-shmobile/board-armadillo800eva.c |  7 +++++++
>  arch/arm/mach-shmobile/board-kzm9g.c           |  7 +++++++
>  arch/arm/mach-shmobile/board-mackerel.c        | 10 ++++++++++
>  drivers/mmc/host/sh_mobile_sdhi.c              |  9 ++++++++-
>  include/linux/mmc/sh_mobile_sdhi.h             |  1 +
>  7 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
> index c754071..ad01651 100644
> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -42,6 +42,7 @@
>  #include <linux/mmc/sh_mobile_sdhi.h>
>  #include <linux/mfd/tmio.h>
>  #include <linux/sh_clk.h>
> +#include <linux/sh_dma.h>
>  #include <linux/irqchip/arm-gic.h>
>  #include <video/sh_mobile_lcdc.h>
>  #include <video/sh_mipi_dsi.h>
> @@ -403,8 +404,11 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[] >  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
>  	.tmio_ocr_mask	= MMC_VDD_27_28 | MMC_VDD_28_29,
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index 297bf5e..c900e24 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -38,6 +38,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/smsc911x.h>
> +#include <linux/sh_dma.h>
>  #include <linux/sh_intc.h>
>  #include <linux/sh_clk.h>
>  #include <linux/gpio.h>
> @@ -355,8 +356,11 @@ static struct platform_device sh_mmcif_device = {
>  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_caps	= MMC_CAP_SDIO_IRQ,
>  };
>  
> @@ -393,8 +397,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* SDHI1 */
>  static struct sh_mobile_sdhi_info sdhi1_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_ocr_mask	= MMC_VDD_165_195,
>  	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE,
>  	.tmio_caps	= MMC_CAP_NEEDS_POLL | MMC_CAP_SDIO_IRQ,
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 44a6215..e95e5dc 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -34,6 +34,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/gpio-regulator.h>
>  #include <linux/regulator/machine.h>
> +#include <linux/sh_dma.h>
>  #include <linux/sh_eth.h>
>  #include <linux/videodev2.h>
>  #include <linux/usb/renesas_usbhs.h>
> @@ -688,8 +689,11 @@ static struct platform_device vcc_sdhi1 = {
>   */
>  #define IRQ31	irq_pin(31)
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_caps	= 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,
> @@ -730,8 +734,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* SDHI1 */
>  static struct sh_mobile_sdhi_info sdhi1_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_caps	= 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,
> diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
> index 165483c..0434b36 100644
> --- a/arch/arm/mach-shmobile/board-kzm9g.c
> +++ b/arch/arm/mach-shmobile/board-kzm9g.c
> @@ -37,6 +37,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/smsc911x.h>
> +#include <linux/sh_dma.h>
>  #include <linux/usb/r8a66597.h>
>  #include <linux/usb/renesas_usbhs.h>
>  #include <linux/videodev2.h>
> @@ -442,8 +443,11 @@ static struct platform_device vcc_sdhi2 = {
>  
>  /* SDHI */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
>  			  MMC_CAP_POWER_OFF_CARD,
> @@ -484,8 +488,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* Micro SD */
>  static struct sh_mobile_sdhi_info sdhi2_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
>  			  TMIO_MMC_USE_GPIO_CD |
>  			  TMIO_MMC_WRPROTECT_DISABLE,
> diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> index 85f51a8..49755ff 100644
> --- a/arch/arm/mach-shmobile/board-mackerel.c
> +++ b/arch/arm/mach-shmobile/board-mackerel.c
> @@ -45,6 +45,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/smsc911x.h>
> +#include <linux/sh_dma.h>
>  #include <linux/sh_intc.h>
>  #include <linux/tca6416_keypad.h>
>  #include <linux/usb/renesas_usbhs.h>
> @@ -971,8 +972,11 @@ static struct platform_device nand_flash_device = {
>  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
>  	.cd_gpio	= 172,
> @@ -1010,8 +1014,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* GPIO 41 can trigger IRQ8, but it is used by USBHS1, we have to poll */
>  static struct sh_mobile_sdhi_info sdhi1_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
>  			  MMC_CAP_NEEDS_POLL,
> @@ -1053,8 +1060,11 @@ static struct platform_device sdhi1_device = {
>   * connected to GPIO SCIFB_SCK of SH7372 (GPIO 162).
>   */
>  static struct sh_mobile_sdhi_info sdhi2_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
>  			  MMC_CAP_NEEDS_POLL,
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index cc4c872..efe3386 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -193,13 +193,20 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  			 */
>  			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
>  			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
> +			/*
> +			 * This is a layering violation: the slave driver
> +			 * should not be aware that the chan_priv_* is the
> +			 * slave id.
> +			 * We should not really need to set the slave id
> +			 * here anyway. -arnd
> +			 */
>  			dma_priv->slave_id_tx = p->dma_slave_tx;
>  			dma_priv->slave_id_rx = p->dma_slave_rx;
> +			dma_priv->filter = p->dma_filter;
>  		}
>  	}
>  
>  	dma_priv->alignment_shift = 1; /* 2-byte alignment */
> -	dma_priv->filter = shdma_chan_filter;
>  
>  	mmc_data->dma = dma_priv;
>  
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index b76bcf0..342f07b 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -20,6 +20,7 @@ struct sh_mobile_sdhi_ops {
>  struct sh_mobile_sdhi_info {
>  	int dma_slave_tx;
>  	int dma_slave_rx;
> +	dma_filter_fn dma_filter;
>  	unsigned long tmio_flags;
>  	unsigned long tmio_caps;
>  	unsigned long tmio_caps2;
> -- 
> 1.8.1.2
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-05-31 14:52   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-05-31 14:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

Thanks for the patches. How about this fix:

http://thread.gmane.org/gmane.linux.kernel.mmc/20619

I think it's a better fix. SHDMA users cannot use other DMA drivers, so, I 
don't think we need to pass the filter via the platform data.

Thanks
Guennadi

On Fri, 31 May 2013, Arnd Bergmann wrote:

> Since eec95ee2261 "mmc: sdhi/tmio: switch to using
> dmaengine_slave_config()", we are getting a link error with
> a number of shmobile configurations that turn on the sdhi
> driver but the not the SH_DMAE_BASE driver.
> 
> The reason is that the driver now incorrectly refers to
> a global filter function that should be referenced only
> by platform code:
> 
> drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> 
> The fix is to move the pointer to the filter function out
> of the driver, similar to how we do it for most other
> platforms. This makes the driver almost independent
> of the underlying dma engine, besides the fact that
> it tries to pass the same number both into the filter
> function and into slave config, which only works on
> some DMA engine drivers.
> 
> Since the bug is only present on the mmc tree, I think
> the fix should be applied there as well.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++++
>  arch/arm/mach-shmobile/board-ap4evb.c          |  7 +++++++
>  arch/arm/mach-shmobile/board-armadillo800eva.c |  7 +++++++
>  arch/arm/mach-shmobile/board-kzm9g.c           |  7 +++++++
>  arch/arm/mach-shmobile/board-mackerel.c        | 10 ++++++++++
>  drivers/mmc/host/sh_mobile_sdhi.c              |  9 ++++++++-
>  include/linux/mmc/sh_mobile_sdhi.h             |  1 +
>  7 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
> index c754071..ad01651 100644
> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -42,6 +42,7 @@
>  #include <linux/mmc/sh_mobile_sdhi.h>
>  #include <linux/mfd/tmio.h>
>  #include <linux/sh_clk.h>
> +#include <linux/sh_dma.h>
>  #include <linux/irqchip/arm-gic.h>
>  #include <video/sh_mobile_lcdc.h>
>  #include <video/sh_mipi_dsi.h>
> @@ -403,8 +404,11 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[] =
>  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
>  	.tmio_ocr_mask	= MMC_VDD_27_28 | MMC_VDD_28_29,
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index 297bf5e..c900e24 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -38,6 +38,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/smsc911x.h>
> +#include <linux/sh_dma.h>
>  #include <linux/sh_intc.h>
>  #include <linux/sh_clk.h>
>  #include <linux/gpio.h>
> @@ -355,8 +356,11 @@ static struct platform_device sh_mmcif_device = {
>  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_caps	= MMC_CAP_SDIO_IRQ,
>  };
>  
> @@ -393,8 +397,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* SDHI1 */
>  static struct sh_mobile_sdhi_info sdhi1_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_ocr_mask	= MMC_VDD_165_195,
>  	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE,
>  	.tmio_caps	= MMC_CAP_NEEDS_POLL | MMC_CAP_SDIO_IRQ,
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 44a6215..e95e5dc 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -34,6 +34,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/gpio-regulator.h>
>  #include <linux/regulator/machine.h>
> +#include <linux/sh_dma.h>
>  #include <linux/sh_eth.h>
>  #include <linux/videodev2.h>
>  #include <linux/usb/renesas_usbhs.h>
> @@ -688,8 +689,11 @@ static struct platform_device vcc_sdhi1 = {
>   */
>  #define IRQ31	irq_pin(31)
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_caps	= 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,
> @@ -730,8 +734,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* SDHI1 */
>  static struct sh_mobile_sdhi_info sdhi1_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_caps	= 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,
> diff --git a/arch/arm/mach-shmobile/board-kzm9g.c b/arch/arm/mach-shmobile/board-kzm9g.c
> index 165483c..0434b36 100644
> --- a/arch/arm/mach-shmobile/board-kzm9g.c
> +++ b/arch/arm/mach-shmobile/board-kzm9g.c
> @@ -37,6 +37,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/smsc911x.h>
> +#include <linux/sh_dma.h>
>  #include <linux/usb/r8a66597.h>
>  #include <linux/usb/renesas_usbhs.h>
>  #include <linux/videodev2.h>
> @@ -442,8 +443,11 @@ static struct platform_device vcc_sdhi2 = {
>  
>  /* SDHI */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
>  			  MMC_CAP_POWER_OFF_CARD,
> @@ -484,8 +488,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* Micro SD */
>  static struct sh_mobile_sdhi_info sdhi2_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT |
>  			  TMIO_MMC_USE_GPIO_CD |
>  			  TMIO_MMC_WRPROTECT_DISABLE,
> diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> index 85f51a8..49755ff 100644
> --- a/arch/arm/mach-shmobile/board-mackerel.c
> +++ b/arch/arm/mach-shmobile/board-mackerel.c
> @@ -45,6 +45,7 @@
>  #include <linux/regulator/fixed.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/smsc911x.h>
> +#include <linux/sh_dma.h>
>  #include <linux/sh_intc.h>
>  #include <linux/tca6416_keypad.h>
>  #include <linux/usb/renesas_usbhs.h>
> @@ -971,8 +972,11 @@ static struct platform_device nand_flash_device = {
>  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
>  	.cd_gpio	= 172,
> @@ -1010,8 +1014,11 @@ static struct platform_device sdhi0_device = {
>  
>  /* GPIO 41 can trigger IRQ8, but it is used by USBHS1, we have to poll */
>  static struct sh_mobile_sdhi_info sdhi1_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI1_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI1_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
>  			  MMC_CAP_NEEDS_POLL,
> @@ -1053,8 +1060,11 @@ static struct platform_device sdhi1_device = {
>   * connected to GPIO SCIFB_SCK of SH7372 (GPIO 162).
>   */
>  static struct sh_mobile_sdhi_info sdhi2_info = {
> +#ifdef CONFIG_SH_DMAE_BASE
>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI2_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI2_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_WRPROTECT_DISABLE | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
>  			  MMC_CAP_NEEDS_POLL,
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index cc4c872..efe3386 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -193,13 +193,20 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  			 */
>  			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
>  			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
> +			/*
> +			 * This is a layering violation: the slave driver
> +			 * should not be aware that the chan_priv_* is the
> +			 * slave id.
> +			 * We should not really need to set the slave id
> +			 * here anyway. -arnd
> +			 */
>  			dma_priv->slave_id_tx = p->dma_slave_tx;
>  			dma_priv->slave_id_rx = p->dma_slave_rx;
> +			dma_priv->filter = p->dma_filter;
>  		}
>  	}
>  
>  	dma_priv->alignment_shift = 1; /* 2-byte alignment */
> -	dma_priv->filter = shdma_chan_filter;
>  
>  	mmc_data->dma = dma_priv;
>  
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index b76bcf0..342f07b 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -20,6 +20,7 @@ struct sh_mobile_sdhi_ops {
>  struct sh_mobile_sdhi_info {
>  	int dma_slave_tx;
>  	int dma_slave_rx;
> +	dma_filter_fn dma_filter;
>  	unsigned long tmio_flags;
>  	unsigned long tmio_caps;
>  	unsigned long tmio_caps2;
> -- 
> 1.8.1.2
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-05-31 14:52   ` Guennadi Liakhovetski
@ 2013-05-31 15:11     ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 15:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:
> 
> Thanks for the patches. How about this fix:
> 
> http://thread.gmane.org/gmane.linux.kernel.mmc/20619
> 
> I think it's a better fix. SHDMA users cannot use other DMA drivers, so, I 
> don't think we need to pass the filter via the platform data.

I think it's more a matter of using the API correctly. The dmaengine
API is an abstraction to separate the slave driver from the master
through well-defined calls. If you make additional assumptions
in the slave driver about the master, that is a layering violation.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-05-31 15:11     ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 15:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:
> 
> Thanks for the patches. How about this fix:
> 
> http://thread.gmane.org/gmane.linux.kernel.mmc/20619
> 
> I think it's a better fix. SHDMA users cannot use other DMA drivers, so, I 
> don't think we need to pass the filter via the platform data.

I think it's more a matter of using the API correctly. The dmaengine
API is an abstraction to separate the slave driver from the master
through well-defined calls. If you make additional assumptions
in the slave driver about the master, that is a layering violation.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-05-31 15:11     ` Arnd Bergmann
@ 2013-05-31 15:30       ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-05-31 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 31 May 2013, Arnd Bergmann wrote:

> On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:
> > 
> > Thanks for the patches. How about this fix:
> > 
> > http://thread.gmane.org/gmane.linux.kernel.mmc/20619
> > 
> > I think it's a better fix. SHDMA users cannot use other DMA drivers, so, I 
> > don't think we need to pass the filter via the platform data.
> 
> I think it's more a matter of using the API correctly. The dmaengine
> API is an abstraction to separate the slave driver from the master
> through well-defined calls. If you make additional assumptions
> in the slave driver about the master, that is a layering violation.

I think it is a common practice, see e.g.

drivers/mmc/host/omap_hsmmc.c
drivers/mmc/host/davinci_mmc.c

And what do you do for DT-based platforms?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-05-31 15:30       ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-05-31 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 31 May 2013, Arnd Bergmann wrote:

> On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:
> > 
> > Thanks for the patches. How about this fix:
> > 
> > http://thread.gmane.org/gmane.linux.kernel.mmc/20619
> > 
> > I think it's a better fix. SHDMA users cannot use other DMA drivers, so, I 
> > don't think we need to pass the filter via the platform data.
> 
> I think it's more a matter of using the API correctly. The dmaengine
> API is an abstraction to separate the slave driver from the master
> through well-defined calls. If you make additional assumptions
> in the slave driver about the master, that is a layering violation.

I think it is a common practice, see e.g.

drivers/mmc/host/omap_hsmmc.c
drivers/mmc/host/davinci_mmc.c

And what do you do for DT-based platforms?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-05-31 15:30       ` Guennadi Liakhovetski
@ 2013-05-31 16:02         ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 16:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 May 2013 17:30:01 Guennadi Liakhovetski wrote:
> On Fri, 31 May 2013, Arnd Bergmann wrote:
> > On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:

> > I think it's more a matter of using the API correctly. The dmaengine
> > API is an abstraction to separate the slave driver from the master
> > through well-defined calls. If you make additional assumptions
> > in the slave driver about the master, that is a layering violation.
> 
> I think it is a common practice, see e.g.
> 
> drivers/mmc/host/omap_hsmmc.c
> drivers/mmc/host/davinci_mmc.c

Yes, those should be fixed as well.

> And what do you do for DT-based platforms?

Once the driver is DT-enabled, we no longer have to pass
a filter function at all because the slave id and all settings
will get pulled out of the DT using the dma engine's xlate()
callback.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-05-31 16:02         ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-05-31 16:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 May 2013 17:30:01 Guennadi Liakhovetski wrote:
> On Fri, 31 May 2013, Arnd Bergmann wrote:
> > On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:

> > I think it's more a matter of using the API correctly. The dmaengine
> > API is an abstraction to separate the slave driver from the master
> > through well-defined calls. If you make additional assumptions
> > in the slave driver about the master, that is a layering violation.
> 
> I think it is a common practice, see e.g.
> 
> drivers/mmc/host/omap_hsmmc.c
> drivers/mmc/host/davinci_mmc.c

Yes, those should be fixed as well.

> And what do you do for DT-based platforms?

Once the driver is DT-enabled, we no longer have to pass
a filter function at all because the slave id and all settings
will get pulled out of the DT using the dma engine's xlate()
callback.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-05-31 16:02         ` Arnd Bergmann
@ 2013-06-05  8:28           ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-05  8:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

On Fri, 31 May 2013, Arnd Bergmann wrote:

> On Friday 31 May 2013 17:30:01 Guennadi Liakhovetski wrote:
> > On Fri, 31 May 2013, Arnd Bergmann wrote:
> > > On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:
> 
> > > I think it's more a matter of using the API correctly. The dmaengine
> > > API is an abstraction to separate the slave driver from the master
> > > through well-defined calls. If you make additional assumptions
> > > in the slave driver about the master, that is a layering violation.
> > 
> > I think it is a common practice, see e.g.
> > 
> > drivers/mmc/host/omap_hsmmc.c
> > drivers/mmc/host/davinci_mmc.c
> 
> Yes, those should be fixed as well.

Then we'll have to fix quite a few of those - I only looked under 
drivers/mmc for now.

But isn't that a separate issue? The problem we have to address now is 
broken compilation, for which, I think, my patch is the simplest solution. 
DMA slave drivers being DMAC implementation agnostic is good, no doubt 
about that, even though many of them will ever only use one DMAC type, but 
isn't that a separate issue? Fixing it would require patching 3 locations: 
a header to add a callback field, arches to add filters and drivers to 
actually call them instead of hard-coded functions. In the worst case 
those changes would go via 3 different git-trees, so, might take 3 kernel 
releases... Ok, at least two if we puch the header change together with 
arch updates via the same tree with suitable acks, still, that's too long, 
IMHO.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-06-05  8:28           ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-05  8:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

On Fri, 31 May 2013, Arnd Bergmann wrote:

> On Friday 31 May 2013 17:30:01 Guennadi Liakhovetski wrote:
> > On Fri, 31 May 2013, Arnd Bergmann wrote:
> > > On Friday 31 May 2013 16:52:13 Guennadi Liakhovetski wrote:
> 
> > > I think it's more a matter of using the API correctly. The dmaengine
> > > API is an abstraction to separate the slave driver from the master
> > > through well-defined calls. If you make additional assumptions
> > > in the slave driver about the master, that is a layering violation.
> > 
> > I think it is a common practice, see e.g.
> > 
> > drivers/mmc/host/omap_hsmmc.c
> > drivers/mmc/host/davinci_mmc.c
> 
> Yes, those should be fixed as well.

Then we'll have to fix quite a few of those - I only looked under 
drivers/mmc for now.

But isn't that a separate issue? The problem we have to address now is 
broken compilation, for which, I think, my patch is the simplest solution. 
DMA slave drivers being DMAC implementation agnostic is good, no doubt 
about that, even though many of them will ever only use one DMAC type, but 
isn't that a separate issue? Fixing it would require patching 3 locations: 
a header to add a callback field, arches to add filters and drivers to 
actually call them instead of hard-coded functions. In the worst case 
those changes would go via 3 different git-trees, so, might take 3 kernel 
releases... Ok, at least two if we puch the header change together with 
arch updates via the same tree with suitable acks, still, that's too long, 
IMHO.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-05-31 13:56   ` Arnd Bergmann
@ 2013-06-07 10:22     ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 10:22 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

Unrelated to the original trigger, that prompted you to look at this, I 
don't think this would work:

On Fri, 31 May 2013, Arnd Bergmann wrote:

> The MMC driver should not need to care what the dma engine
> is that it is using, so it must not rely on the argument
> to the filter function to be a 'slave_id' value.
> 
> Passing the slave id in dmaengine_slave_config is not
> portable, and does not work with DT-enabled systems,
> so this turns the filter argument into a void pointer
> that gets set by the platform code and gets passed
> to the dmaengine code as an opaque value.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++--
>  arch/arm/mach-shmobile/board-ap4evb.c          |  8 ++++----
>  arch/arm/mach-shmobile/board-armadillo800eva.c |  8 ++++----
>  arch/arm/mach-shmobile/board-kzm9g.c           |  8 ++++----
>  arch/arm/mach-shmobile/board-mackerel.c        | 12 ++++++------
>  drivers/mmc/host/sh_mobile_sdhi.c              | 20 +++-----------------
>  drivers/mmc/host/tmio_mmc_dma.c                |  6 ++----
>  include/linux/mfd/tmio.h                       |  2 --
>  include/linux/mmc/sh_mobile_sdhi.h             |  5 +++--
>  9 files changed, 28 insertions(+), 45 deletions(-)

Aren't you forgetting about arch/sh?

> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index efe3386..c4a0aab 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -185,23 +185,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  		if (p->get_cd)
>  			mmc_data->get_cd = sh_mobile_sdhi_get_cd;
>  
> -		if (p->dma_slave_tx > 0 && p->dma_slave_rx > 0) {
> -			/*
> -			 * Yes, we have to provide slave IDs twice to TMIO:
> -			 * once as a filter parameter and once for channel
> -			 * configuration as an explicit slave ID
> -			 */
> -			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
> -			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
> -			/*
> -			 * This is a layering violation: the slave driver
> -			 * should not be aware that the chan_priv_* is the
> -			 * slave id.
> -			 * We should not really need to set the slave id
> -			 * here anyway. -arnd
> -			 */
> -			dma_priv->slave_id_tx = p->dma_slave_tx;
> -			dma_priv->slave_id_rx = p->dma_slave_rx;
> +		if (p->dma_slave_tx && p->dma_slave_rx) {
> +			dma_priv->chan_priv_tx = p->dma_slave_tx;
> +			dma_priv->chan_priv_rx = p->dma_slave_rx;
>  			dma_priv->filter = p->dma_filter;
>  		}
>  	}
> diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> index 47bdb8f..caad28e 100644
> --- a/drivers/mmc/host/tmio_mmc_dma.c
> +++ b/drivers/mmc/host/tmio_mmc_dma.c
> @@ -290,8 +290,7 @@ 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.slave_id = 0; /* already set */
>  		cfg.direction = DMA_MEM_TO_DEV;
>  		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift);
>  		cfg.src_addr = 0;
> @@ -308,8 +307,7 @@ 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.slave_id = 0;

slave_id is needed for the DMAC driver to configure slave DMA. And this 
_is_ the current mainstreem method (in the non-DT case) to configure slave 
DMA, AFAIK. In the non-DT case the filter function is used to verify, 
whether this slave can be served with this channel, and 
dmaengine_slave_config() is used to actually configure the channel for 
slave DMA.

>  		cfg.direction = DMA_DEV_TO_MEM;
>  		cfg.src_addr = cfg.dst_addr;
>  		cfg.dst_addr = 0;
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index ce35113..0990d8a 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -86,8 +86,6 @@ 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;
>  	bool (*filter)(struct dma_chan *chan, void *arg);
>  };
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index 342f07b..66a7d1c 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -2,6 +2,7 @@
>  #define LINUX_MMC_SH_MOBILE_SDHI_H
>  
>  #include <linux/types.h>
> +#include <linux/dmaengine.h>
>  
>  struct platform_device;
>  
> @@ -18,8 +19,8 @@ struct sh_mobile_sdhi_ops {
>  };
>  
>  struct sh_mobile_sdhi_info {
> -	int dma_slave_tx;
> -	int dma_slave_rx;
> +	void *dma_slave_tx;
> +	void *dma_slave_rx;
>  	dma_filter_fn dma_filter;
>  	unsigned long tmio_flags;
>  	unsigned long tmio_caps;
> -- 
> 1.8.1.2
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-07 10:22     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 10:22 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

Unrelated to the original trigger, that prompted you to look at this, I 
don't think this would work:

On Fri, 31 May 2013, Arnd Bergmann wrote:

> The MMC driver should not need to care what the dma engine
> is that it is using, so it must not rely on the argument
> to the filter function to be a 'slave_id' value.
> 
> Passing the slave id in dmaengine_slave_config is not
> portable, and does not work with DT-enabled systems,
> so this turns the filter argument into a void pointer
> that gets set by the platform code and gets passed
> to the dmaengine code as an opaque value.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++--
>  arch/arm/mach-shmobile/board-ap4evb.c          |  8 ++++----
>  arch/arm/mach-shmobile/board-armadillo800eva.c |  8 ++++----
>  arch/arm/mach-shmobile/board-kzm9g.c           |  8 ++++----
>  arch/arm/mach-shmobile/board-mackerel.c        | 12 ++++++------
>  drivers/mmc/host/sh_mobile_sdhi.c              | 20 +++-----------------
>  drivers/mmc/host/tmio_mmc_dma.c                |  6 ++----
>  include/linux/mfd/tmio.h                       |  2 --
>  include/linux/mmc/sh_mobile_sdhi.h             |  5 +++--
>  9 files changed, 28 insertions(+), 45 deletions(-)

Aren't you forgetting about arch/sh?

> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index efe3386..c4a0aab 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -185,23 +185,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
>  		if (p->get_cd)
>  			mmc_data->get_cd = sh_mobile_sdhi_get_cd;
>  
> -		if (p->dma_slave_tx > 0 && p->dma_slave_rx > 0) {
> -			/*
> -			 * Yes, we have to provide slave IDs twice to TMIO:
> -			 * once as a filter parameter and once for channel
> -			 * configuration as an explicit slave ID
> -			 */
> -			dma_priv->chan_priv_tx = (void *)p->dma_slave_tx;
> -			dma_priv->chan_priv_rx = (void *)p->dma_slave_rx;
> -			/*
> -			 * This is a layering violation: the slave driver
> -			 * should not be aware that the chan_priv_* is the
> -			 * slave id.
> -			 * We should not really need to set the slave id
> -			 * here anyway. -arnd
> -			 */
> -			dma_priv->slave_id_tx = p->dma_slave_tx;
> -			dma_priv->slave_id_rx = p->dma_slave_rx;
> +		if (p->dma_slave_tx && p->dma_slave_rx) {
> +			dma_priv->chan_priv_tx = p->dma_slave_tx;
> +			dma_priv->chan_priv_rx = p->dma_slave_rx;
>  			dma_priv->filter = p->dma_filter;
>  		}
>  	}
> diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> index 47bdb8f..caad28e 100644
> --- a/drivers/mmc/host/tmio_mmc_dma.c
> +++ b/drivers/mmc/host/tmio_mmc_dma.c
> @@ -290,8 +290,7 @@ 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.slave_id = 0; /* already set */
>  		cfg.direction = DMA_MEM_TO_DEV;
>  		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift);
>  		cfg.src_addr = 0;
> @@ -308,8 +307,7 @@ 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.slave_id = 0;

slave_id is needed for the DMAC driver to configure slave DMA. And this 
_is_ the current mainstreem method (in the non-DT case) to configure slave 
DMA, AFAIK. In the non-DT case the filter function is used to verify, 
whether this slave can be served with this channel, and 
dmaengine_slave_config() is used to actually configure the channel for 
slave DMA.

>  		cfg.direction = DMA_DEV_TO_MEM;
>  		cfg.src_addr = cfg.dst_addr;
>  		cfg.dst_addr = 0;
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index ce35113..0990d8a 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -86,8 +86,6 @@ 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;
>  	bool (*filter)(struct dma_chan *chan, void *arg);
>  };
> diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
> index 342f07b..66a7d1c 100644
> --- a/include/linux/mmc/sh_mobile_sdhi.h
> +++ b/include/linux/mmc/sh_mobile_sdhi.h
> @@ -2,6 +2,7 @@
>  #define LINUX_MMC_SH_MOBILE_SDHI_H
>  
>  #include <linux/types.h>
> +#include <linux/dmaengine.h>
>  
>  struct platform_device;
>  
> @@ -18,8 +19,8 @@ struct sh_mobile_sdhi_ops {
>  };
>  
>  struct sh_mobile_sdhi_info {
> -	int dma_slave_tx;
> -	int dma_slave_rx;
> +	void *dma_slave_tx;
> +	void *dma_slave_rx;
>  	dma_filter_fn dma_filter;
>  	unsigned long tmio_flags;
>  	unsigned long tmio_caps;
> -- 
> 1.8.1.2
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-05-31 13:56 ` Arnd Bergmann
@ 2013-06-07 10:25   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

On Fri, 31 May 2013, Arnd Bergmann wrote:

> Since eec95ee2261 "mmc: sdhi/tmio: switch to using
> dmaengine_slave_config()", we are getting a link error with
> a number of shmobile configurations that turn on the sdhi
> driver but the not the SH_DMAE_BASE driver.
> 
> The reason is that the driver now incorrectly refers to
> a global filter function that should be referenced only
> by platform code:
> 
> drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> 
> The fix is to move the pointer to the filter function out
> of the driver, similar to how we do it for most other
> platforms. This makes the driver almost independent
> of the underlying dma engine, besides the fact that
> it tries to pass the same number both into the filter
> function and into slave config, which only works on
> some DMA engine drivers.
> 
> Since the bug is only present on the mmc tree, I think
> the fix should be applied there as well.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++++
>  arch/arm/mach-shmobile/board-ap4evb.c          |  7 +++++++
>  arch/arm/mach-shmobile/board-armadillo800eva.c |  7 +++++++
>  arch/arm/mach-shmobile/board-kzm9g.c           |  7 +++++++
>  arch/arm/mach-shmobile/board-mackerel.c        | 10 ++++++++++
>  drivers/mmc/host/sh_mobile_sdhi.c              |  9 ++++++++-
>  include/linux/mmc/sh_mobile_sdhi.h             |  1 +
>  7 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
> index c754071..ad01651 100644
> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -42,6 +42,7 @@
>  #include <linux/mmc/sh_mobile_sdhi.h>
>  #include <linux/mfd/tmio.h>
>  #include <linux/sh_clk.h>
> +#include <linux/sh_dma.h>
>  #include <linux/irqchip/arm-gic.h>
>  #include <video/sh_mobile_lcdc.h>
>  #include <video/sh_mipi_dsi.h>
> @@ -403,8 +404,11 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[] >  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE

Right, that's the problem, I think. Don't think we want these #ifdefs in 
all shdma users - under arch/arm and arch/sh. That's why I think we need 
my fix in the first place to fix the compile breakage. After that we can 
think about improving DMA slave driver decoupling from specific DMAC 
drivers - if at all needed.

>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
>  	.tmio_ocr_mask	= MMC_VDD_27_28 | MMC_VDD_28_29,

[snip]

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-06-07 10:25   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

On Fri, 31 May 2013, Arnd Bergmann wrote:

> Since eec95ee2261 "mmc: sdhi/tmio: switch to using
> dmaengine_slave_config()", we are getting a link error with
> a number of shmobile configurations that turn on the sdhi
> driver but the not the SH_DMAE_BASE driver.
> 
> The reason is that the driver now incorrectly refers to
> a global filter function that should be referenced only
> by platform code:
> 
> drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> drivers/mmc/host/sh_mobile_sdhi.c:202: undefined reference to `shdma_chan_filter'
> 
> The fix is to move the pointer to the filter function out
> of the driver, similar to how we do it for most other
> platforms. This makes the driver almost independent
> of the underlying dma engine, besides the fact that
> it tries to pass the same number both into the filter
> function and into slave config, which only works on
> some DMA engine drivers.
> 
> Since the bug is only present on the mmc tree, I think
> the fix should be applied there as well.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++++
>  arch/arm/mach-shmobile/board-ap4evb.c          |  7 +++++++
>  arch/arm/mach-shmobile/board-armadillo800eva.c |  7 +++++++
>  arch/arm/mach-shmobile/board-kzm9g.c           |  7 +++++++
>  arch/arm/mach-shmobile/board-mackerel.c        | 10 ++++++++++
>  drivers/mmc/host/sh_mobile_sdhi.c              |  9 ++++++++-
>  include/linux/mmc/sh_mobile_sdhi.h             |  1 +
>  7 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
> index c754071..ad01651 100644
> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -42,6 +42,7 @@
>  #include <linux/mmc/sh_mobile_sdhi.h>
>  #include <linux/mfd/tmio.h>
>  #include <linux/sh_clk.h>
> +#include <linux/sh_dma.h>
>  #include <linux/irqchip/arm-gic.h>
>  #include <video/sh_mobile_lcdc.h>
>  #include <video/sh_mipi_dsi.h>
> @@ -403,8 +404,11 @@ static struct regulator_consumer_supply fixed2v8_power_consumers[] =
>  
>  /* SDHI0 */
>  static struct sh_mobile_sdhi_info sdhi0_info = {
> +#ifdef CONFIG_SH_DMAE_BASE

Right, that's the problem, I think. Don't think we want these #ifdefs in 
all shdma users - under arch/arm and arch/sh. That's why I think we need 
my fix in the first place to fix the compile breakage. After that we can 
think about improving DMA slave driver decoupling from specific DMAC 
drivers - if at all needed.

>  	.dma_slave_tx	= SHDMA_SLAVE_SDHI0_TX,
>  	.dma_slave_rx	= SHDMA_SLAVE_SDHI0_RX,
> +	.dma_filter	= shdma_chan_filter,
> +#endif
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_USE_GPIO_CD,
>  	.tmio_caps	= MMC_CAP_SD_HIGHSPEED,
>  	.tmio_ocr_mask	= MMC_VDD_27_28 | MMC_VDD_28_29,

[snip]

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-06-07 10:25   ` Guennadi Liakhovetski
@ 2013-06-07 12:52     ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 12:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013 12:25:11 Guennadi Liakhovetski wrote:
> >  /* SDHI0 */
> >  static struct sh_mobile_sdhi_info sdhi0_info = {
> > +#ifdef CONFIG_SH_DMAE_BASE
> 
> Right, that's the problem, I think. Don't think we want these #ifdefs in 
> all shdma users - under arch/arm and arch/sh. That's why I think we need 
> my fix in the first place to fix the compile breakage. After that we can 
> think about improving DMA slave driver decoupling from specific DMAC 
> drivers - if at all needed.
> 
> >       .dma_slave_tx   = SHDMA_SLAVE_SDHI0_TX,
> >       .dma_slave_rx   = SHDMA_SLAVE_SDHI0_RX,
> > +     .dma_filter     = shdma_chan_filter,
> > +#endif

Well, the problem is that you check the value of dma_slave_rx/tx in order
to find out whether you should do DMA in the driver. If the filter
function is NULL but the values are positive, I think the driver won't
actually fall back to PIO mode but just fail.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-06-07 12:52     ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 12:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013 12:25:11 Guennadi Liakhovetski wrote:
> >  /* SDHI0 */
> >  static struct sh_mobile_sdhi_info sdhi0_info = {
> > +#ifdef CONFIG_SH_DMAE_BASE
> 
> Right, that's the problem, I think. Don't think we want these #ifdefs in 
> all shdma users - under arch/arm and arch/sh. That's why I think we need 
> my fix in the first place to fix the compile breakage. After that we can 
> think about improving DMA slave driver decoupling from specific DMAC 
> drivers - if at all needed.
> 
> >       .dma_slave_tx   = SHDMA_SLAVE_SDHI0_TX,
> >       .dma_slave_rx   = SHDMA_SLAVE_SDHI0_RX,
> > +     .dma_filter     = shdma_chan_filter,
> > +#endif

Well, the problem is that you check the value of dma_slave_rx/tx in order
to find out whether you should do DMA in the driver. If the filter
function is NULL but the values are positive, I think the driver won't
actually fall back to PIO mode but just fail.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-06-07 10:22     ` Guennadi Liakhovetski
@ 2013-06-07 12:59       ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 12:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013 12:22:15 Guennadi Liakhovetski wrote:

> >  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++--
> >  arch/arm/mach-shmobile/board-ap4evb.c          |  8 ++++----
> >  arch/arm/mach-shmobile/board-armadillo800eva.c |  8 ++++----
> >  arch/arm/mach-shmobile/board-kzm9g.c           |  8 ++++----
> >  arch/arm/mach-shmobile/board-mackerel.c        | 12 ++++++------
> >  drivers/mmc/host/sh_mobile_sdhi.c              | 20 +++-----------------
> >  drivers/mmc/host/tmio_mmc_dma.c                |  6 ++----
> >  include/linux/mfd/tmio.h                       |  2 --
> >  include/linux/mmc/sh_mobile_sdhi.h             |  5 +++--
> >  9 files changed, 28 insertions(+), 45 deletions(-)
> 
> Aren't you forgetting about arch/sh?

Right, sorry about that. I was assuming that sh_mobile_sdhi is only used on
ARM, which was clearly incorrect.

> > diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> > index 47bdb8f..caad28e 100644
> > --- a/drivers/mmc/host/tmio_mmc_dma.c
> > +++ b/drivers/mmc/host/tmio_mmc_dma.c
> > @@ -290,8 +290,7 @@ 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.slave_id = 0; /* already set */
> >  		cfg.direction = DMA_MEM_TO_DEV;
> >  		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift);
> >  		cfg.src_addr = 0;
> > @@ -308,8 +307,7 @@ 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.slave_id = 0;
> 
> slave_id is needed for the DMAC driver to configure slave DMA. And this 
> _is_ the current mainstreem method (in the non-DT case) to configure slave 
> DMA, AFAIK. In the non-DT case the filter function is used to verify, 
> whether this slave can be served with this channel, and 
> dmaengine_slave_config() is used to actually configure the channel for 
> slave DMA.

I think that is a flaw in the dmaengine driver. I don't actually know how
any dma-engine driver could use dmaengine_slave_config to pick the request
line, since that is something the driver should not know at the time
it calls dmaengine_slave_config().

It's probably best to change the shdma driver to just ignore the slave_id
value in slave_config, like most other drivers to. I don't see how that
could possibly work when used from a portable slave driver.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-07 12:59       ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 12:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013 12:22:15 Guennadi Liakhovetski wrote:

> >  arch/arm/mach-shmobile/board-ag5evm.c          |  4 ++--
> >  arch/arm/mach-shmobile/board-ap4evb.c          |  8 ++++----
> >  arch/arm/mach-shmobile/board-armadillo800eva.c |  8 ++++----
> >  arch/arm/mach-shmobile/board-kzm9g.c           |  8 ++++----
> >  arch/arm/mach-shmobile/board-mackerel.c        | 12 ++++++------
> >  drivers/mmc/host/sh_mobile_sdhi.c              | 20 +++-----------------
> >  drivers/mmc/host/tmio_mmc_dma.c                |  6 ++----
> >  include/linux/mfd/tmio.h                       |  2 --
> >  include/linux/mmc/sh_mobile_sdhi.h             |  5 +++--
> >  9 files changed, 28 insertions(+), 45 deletions(-)
> 
> Aren't you forgetting about arch/sh?

Right, sorry about that. I was assuming that sh_mobile_sdhi is only used on
ARM, which was clearly incorrect.

> > diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> > index 47bdb8f..caad28e 100644
> > --- a/drivers/mmc/host/tmio_mmc_dma.c
> > +++ b/drivers/mmc/host/tmio_mmc_dma.c
> > @@ -290,8 +290,7 @@ 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.slave_id = 0; /* already set */
> >  		cfg.direction = DMA_MEM_TO_DEV;
> >  		cfg.dst_addr = res->start + (CTL_SD_DATA_PORT << host->bus_shift);
> >  		cfg.src_addr = 0;
> > @@ -308,8 +307,7 @@ 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.slave_id = 0;
> 
> slave_id is needed for the DMAC driver to configure slave DMA. And this 
> _is_ the current mainstreem method (in the non-DT case) to configure slave 
> DMA, AFAIK. In the non-DT case the filter function is used to verify, 
> whether this slave can be served with this channel, and 
> dmaengine_slave_config() is used to actually configure the channel for 
> slave DMA.

I think that is a flaw in the dmaengine driver. I don't actually know how
any dma-engine driver could use dmaengine_slave_config to pick the request
line, since that is something the driver should not know at the time
it calls dmaengine_slave_config().

It's probably best to change the shdma driver to just ignore the slave_id
value in slave_config, like most other drivers to. I don't see how that
could possibly work when used from a portable slave driver.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
  2013-06-07 12:52     ` Arnd Bergmann
@ 2013-06-07 13:01       ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

> On Friday 07 June 2013 12:25:11 Guennadi Liakhovetski wrote:
> > >  /* SDHI0 */
> > >  static struct sh_mobile_sdhi_info sdhi0_info = {
> > > +#ifdef CONFIG_SH_DMAE_BASE
> > 
> > Right, that's the problem, I think. Don't think we want these #ifdefs in 
> > all shdma users - under arch/arm and arch/sh. That's why I think we need 
> > my fix in the first place to fix the compile breakage. After that we can 
> > think about improving DMA slave driver decoupling from specific DMAC 
> > drivers - if at all needed.
> > 
> > >       .dma_slave_tx   = SHDMA_SLAVE_SDHI0_TX,
> > >       .dma_slave_rx   = SHDMA_SLAVE_SDHI0_RX,
> > > +     .dma_filter     = shdma_chan_filter,
> > > +#endif
> 
> Well, the problem is that you check the value of dma_slave_rx/tx in order
> to find out whether you should do DMA in the driver. If the filter
> function is NULL but the values are positive, I think the driver won't
> actually fall back to PIO mode but just fail.

So far the tmio_mmc_request_dma() function has only one caller - SDHI. 
Even worse - tmio_mmc_dma.o is only built for SDHI. With SDHI the filter 
function cannot be NULL, it is hard-coded. When we change that, then we'll 
take care of the problem. So far we just have to fix the breakage.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code
@ 2013-06-07 13:01       ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

> On Friday 07 June 2013 12:25:11 Guennadi Liakhovetski wrote:
> > >  /* SDHI0 */
> > >  static struct sh_mobile_sdhi_info sdhi0_info = {
> > > +#ifdef CONFIG_SH_DMAE_BASE
> > 
> > Right, that's the problem, I think. Don't think we want these #ifdefs in 
> > all shdma users - under arch/arm and arch/sh. That's why I think we need 
> > my fix in the first place to fix the compile breakage. After that we can 
> > think about improving DMA slave driver decoupling from specific DMAC 
> > drivers - if at all needed.
> > 
> > >       .dma_slave_tx   = SHDMA_SLAVE_SDHI0_TX,
> > >       .dma_slave_rx   = SHDMA_SLAVE_SDHI0_RX,
> > > +     .dma_filter     = shdma_chan_filter,
> > > +#endif
> 
> Well, the problem is that you check the value of dma_slave_rx/tx in order
> to find out whether you should do DMA in the driver. If the filter
> function is NULL but the values are positive, I think the driver won't
> actually fall back to PIO mode but just fail.

So far the tmio_mmc_request_dma() function has only one caller - SDHI. 
Even worse - tmio_mmc_dma.o is only built for SDHI. With SDHI the filter 
function cannot be NULL, it is hard-coded. When we change that, then we'll 
take care of the problem. So far we just have to fix the breakage.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-06-07 12:59       ` Arnd Bergmann
@ 2013-06-07 13:12         ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 13:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

[snip]

> I think that is a flaw in the dmaengine driver. I don't actually know how
> any dma-engine driver could use dmaengine_slave_config to pick the request
> line, since that is something the driver should not know at the time
> it calls dmaengine_slave_config().
> 
> It's probably best to change the shdma driver to just ignore the slave_id
> value in slave_config, like most other drivers to.

This is how shdma driver used to work. We used to pass the slave ID (DMA 
request line ID) to shdma from clients from the filter. This was 
considered bad and we were asked to switch to using 
dmaengine_slave_config(), which is the current preferred API, IIUC.

> I don't see how that
> could possibly work when used from a portable slave driver.

A DMA slave ID (DMA request line) is considered to be a portable 
parameter. I.e. on every platform your DMA slave has to request a DMA 
request line to the DMAC and that line can be coded with an int. That's 
the current assumption, I think.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-07 13:12         ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 13:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

[snip]

> I think that is a flaw in the dmaengine driver. I don't actually know how
> any dma-engine driver could use dmaengine_slave_config to pick the request
> line, since that is something the driver should not know at the time
> it calls dmaengine_slave_config().
> 
> It's probably best to change the shdma driver to just ignore the slave_id
> value in slave_config, like most other drivers to.

This is how shdma driver used to work. We used to pass the slave ID (DMA 
request line ID) to shdma from clients from the filter. This was 
considered bad and we were asked to switch to using 
dmaengine_slave_config(), which is the current preferred API, IIUC.

> I don't see how that
> could possibly work when used from a portable slave driver.

A DMA slave ID (DMA request line) is considered to be a portable 
parameter. I.e. on every platform your DMA slave has to request a DMA 
request line to the DMAC and that line can be coded with an int. That's 
the current assumption, I think.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-06-07 13:12         ` Guennadi Liakhovetski
@ 2013-06-07 14:53           ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 14:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> > I think that is a flaw in the dmaengine driver. I don't actually know how
> > any dma-engine driver could use dmaengine_slave_config to pick the request
> > line, since that is something the driver should not know at the time
> > it calls dmaengine_slave_config().
> > 
> > It's probably best to change the shdma driver to just ignore the slave_id
> > value in slave_config, like most other drivers to.
> 
> This is how shdma driver used to work. We used to pass the slave ID (DMA 
> request line ID) to shdma from clients from the filter. This was 
> considered bad and we were asked to switch to using 
> dmaengine_slave_config(), which is the current preferred API, IIUC.

I think you misunderstood the requirement. All other configuration,
i.e. slave port address, burst size, bus width, really should be
configured using dmaengine_slave_config, since this is data that
is known by the /driver/, independent of the DMA engine in use and
unknown to the platform. The request line number is something that
the driver cannot know, since it is a property of the DMA engine.

> > I don't see how that
> > could possibly work when used from a portable slave driver.
> 
> A DMA slave ID (DMA request line) is considered to be a portable 
> parameter. I.e. on every platform your DMA slave has to request a DMA 
> request line to the DMAC and that line can be coded with an int. That's 
> the current assumption, I think.

In a lot of cases you actually need more than one number here, e.g.
to configure which master port of the DMA engine is being used.
That's why the dmaengine DT binding needs a variable #dma-cells
property that is specific to the dma engine being used.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-07 14:53           ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 14:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> > I think that is a flaw in the dmaengine driver. I don't actually know how
> > any dma-engine driver could use dmaengine_slave_config to pick the request
> > line, since that is something the driver should not know at the time
> > it calls dmaengine_slave_config().
> > 
> > It's probably best to change the shdma driver to just ignore the slave_id
> > value in slave_config, like most other drivers to.
> 
> This is how shdma driver used to work. We used to pass the slave ID (DMA 
> request line ID) to shdma from clients from the filter. This was 
> considered bad and we were asked to switch to using 
> dmaengine_slave_config(), which is the current preferred API, IIUC.

I think you misunderstood the requirement. All other configuration,
i.e. slave port address, burst size, bus width, really should be
configured using dmaengine_slave_config, since this is data that
is known by the /driver/, independent of the DMA engine in use and
unknown to the platform. The request line number is something that
the driver cannot know, since it is a property of the DMA engine.

> > I don't see how that
> > could possibly work when used from a portable slave driver.
> 
> A DMA slave ID (DMA request line) is considered to be a portable 
> parameter. I.e. on every platform your DMA slave has to request a DMA 
> request line to the DMAC and that line can be coded with an int. That's 
> the current assumption, I think.

In a lot of cases you actually need more than one number here, e.g.
to configure which master port of the DMA engine is being used.
That's why the dmaengine DT binding needs a variable #dma-cells
property that is specific to the dma engine being used.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-06-07 14:53           ` Arnd Bergmann
@ 2013-06-07 15:32             ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 15:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

> On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> > > I think that is a flaw in the dmaengine driver. I don't actually know how
> > > any dma-engine driver could use dmaengine_slave_config to pick the request
> > > line, since that is something the driver should not know at the time
> > > it calls dmaengine_slave_config().
> > > 
> > > It's probably best to change the shdma driver to just ignore the slave_id
> > > value in slave_config, like most other drivers to.
> > 
> > This is how shdma driver used to work. We used to pass the slave ID (DMA 
> > request line ID) to shdma from clients from the filter. This was 
> > considered bad and we were asked to switch to using 
> > dmaengine_slave_config(), which is the current preferred API, IIUC.
> 
> I think you misunderstood the requirement. All other configuration,
> i.e. slave port address, burst size, bus width, really should be
> configured using dmaengine_slave_config, since this is data that
> is known by the /driver/, independent of the DMA engine in use and
> unknown to the platform. The request line number is something that
> the driver cannot know, since it is a property of the DMA engine.

Isn't it a board property? In case of external devices, or an SoC property 
in case of integrated DMA slave and controller? Let me try to explain. The 
DMAC has to configure the controller to "link" a specific DMA channel to a 
slave request line. E.g. to link an internal channel #0 to SDHI0 tx. To do 
this it has to write specific "magic" values in certain DMAC registers. 
Those values aren't known to the DMAC driver, they are a property of the 
SoC. Let's say, the SoC has 3 DMAC instances, each of them can serve any 
of compatible DMA slaves, including SDHI0, on each of their 6 channels. To 
configure a channel on a DMAC instance for SDHI0 tx you have to write to 
that channel's config register a certain value, that cannot be calculated. 

Those values are only known to the SoC code, so, they are passed as 
platform parameters to the DMAC driver. Now, when SDHI0 asks DMAC to set 
up a DMA channel for its tx, the DMAC driver has to find that value in its 
platform data. In most cases you could use the client address (e.g. FIFO 
register) and direction to find that value. But, I think, that might not 
always be enough. So, we use unique enum values to find those values in 
DMAC platform data. The SDHI driver gets that enum value in its platform 
data, passes it to the DMAC driver as a slave ID, and the DMAC driver uses 
it to find the value(s), it needs to set up its DMA channel. Do you see a 
better way to do the same?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-07 15:32             ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-07 15:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

> On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> > > I think that is a flaw in the dmaengine driver. I don't actually know how
> > > any dma-engine driver could use dmaengine_slave_config to pick the request
> > > line, since that is something the driver should not know at the time
> > > it calls dmaengine_slave_config().
> > > 
> > > It's probably best to change the shdma driver to just ignore the slave_id
> > > value in slave_config, like most other drivers to.
> > 
> > This is how shdma driver used to work. We used to pass the slave ID (DMA 
> > request line ID) to shdma from clients from the filter. This was 
> > considered bad and we were asked to switch to using 
> > dmaengine_slave_config(), which is the current preferred API, IIUC.
> 
> I think you misunderstood the requirement. All other configuration,
> i.e. slave port address, burst size, bus width, really should be
> configured using dmaengine_slave_config, since this is data that
> is known by the /driver/, independent of the DMA engine in use and
> unknown to the platform. The request line number is something that
> the driver cannot know, since it is a property of the DMA engine.

Isn't it a board property? In case of external devices, or an SoC property 
in case of integrated DMA slave and controller? Let me try to explain. The 
DMAC has to configure the controller to "link" a specific DMA channel to a 
slave request line. E.g. to link an internal channel #0 to SDHI0 tx. To do 
this it has to write specific "magic" values in certain DMAC registers. 
Those values aren't known to the DMAC driver, they are a property of the 
SoC. Let's say, the SoC has 3 DMAC instances, each of them can serve any 
of compatible DMA slaves, including SDHI0, on each of their 6 channels. To 
configure a channel on a DMAC instance for SDHI0 tx you have to write to 
that channel's config register a certain value, that cannot be calculated. 

Those values are only known to the SoC code, so, they are passed as 
platform parameters to the DMAC driver. Now, when SDHI0 asks DMAC to set 
up a DMA channel for its tx, the DMAC driver has to find that value in its 
platform data. In most cases you could use the client address (e.g. FIFO 
register) and direction to find that value. But, I think, that might not 
always be enough. So, we use unique enum values to find those values in 
DMAC platform data. The SDHI driver gets that enum value in its platform 
data, passes it to the DMAC driver as a slave ID, and the DMAC driver uses 
it to find the value(s), it needs to set up its DMA channel. Do you see a 
better way to do the same?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-06-07 15:32             ` Guennadi Liakhovetski
@ 2013-06-07 16:06               ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 16:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> Isn't it a board property? In case of external devices, or an SoC property 
> in case of integrated DMA slave and controller? 

Correct.

> Let me try to explain. The 
> DMAC has to configure the controller to "link" a specific DMA channel to a 
> slave request line. E.g. to link an internal channel #0 to SDHI0 tx. To do 
> this it has to write specific "magic" values in certain DMAC registers. 
> Those values aren't known to the DMAC driver, they are a property of the 
> SoC. Let's say, the SoC has 3 DMAC instances, each of them can serve any 
> of compatible DMA slaves, including SDHI0, on each of their 6 channels. To 
> configure a channel on a DMAC instance for SDHI0 tx you have to write to 
> that channel's config register a certain value, that cannot be calculated. 

Right. Or multiple values.

> Those values are only known to the SoC code, so, they are passed as 
> platform parameters to the DMAC driver. Now, when SDHI0 asks DMAC to set 
> up a DMA channel for its tx, the DMAC driver has to find that value in its 
> platform data. In most cases you could use the client address (e.g. FIFO 
> register) and direction to find that value. But, I think, that might not 
> always be enough. So, we use unique enum values to find those values in 
> DMAC platform data. The SDHI driver gets that enum value in its platform 
> data, passes it to the DMAC driver as a slave ID, and the DMAC driver uses 
> it to find the value(s), it needs to set up its DMA channel. Do you see a 
> better way to do the same?

Ah, so you have multiple values, too, and you just abstract them by using
an enum to index an array of platform_data in the dma engine.

This obviously works as well, and lets you get away with a single 32-bit
enum to use as the slave-id. However, I think slave drivers should not
be written with the assumption that all dma-engine drivers do this.
In particular, you seem to assume that the argument to the filter function
is not a pointer but this enum.

It also gets more complicated with the DT binding, since that should
reflect the hardware settings, i.e. not an arbitrarily defined enum
but the data that you have in your array. To keep the DT and platform_data
cases similar, I would suggest you actually remove the array listing
the per-slave data, and move that data instead as an opaque pointer into
the platform data. You can take a look at drivers/mmc/host/mmci.c for
an example of a driver that does this and that is portable across
multiple DMA engine drivers.

Essentially we pass the dma_filter function and dma_param struct pointers
to dma_request_channel directly from the platform_data, without trying
to interpret them. In case of stedma40, the dma_param contains a complex
data structure, while for others it may contain a single integer.

If you do the same, you would essentially pass the mid_rid and chcr values
of your sh_dmae_slave_config as in the pointer that gets passed from
the platform_data down to the filter function, get rid of the slave_id
number and pass the addr value through slave_config.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-07 16:06               ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-07 16:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> Isn't it a board property? In case of external devices, or an SoC property 
> in case of integrated DMA slave and controller? 

Correct.

> Let me try to explain. The 
> DMAC has to configure the controller to "link" a specific DMA channel to a 
> slave request line. E.g. to link an internal channel #0 to SDHI0 tx. To do 
> this it has to write specific "magic" values in certain DMAC registers. 
> Those values aren't known to the DMAC driver, they are a property of the 
> SoC. Let's say, the SoC has 3 DMAC instances, each of them can serve any 
> of compatible DMA slaves, including SDHI0, on each of their 6 channels. To 
> configure a channel on a DMAC instance for SDHI0 tx you have to write to 
> that channel's config register a certain value, that cannot be calculated. 

Right. Or multiple values.

> Those values are only known to the SoC code, so, they are passed as 
> platform parameters to the DMAC driver. Now, when SDHI0 asks DMAC to set 
> up a DMA channel for its tx, the DMAC driver has to find that value in its 
> platform data. In most cases you could use the client address (e.g. FIFO 
> register) and direction to find that value. But, I think, that might not 
> always be enough. So, we use unique enum values to find those values in 
> DMAC platform data. The SDHI driver gets that enum value in its platform 
> data, passes it to the DMAC driver as a slave ID, and the DMAC driver uses 
> it to find the value(s), it needs to set up its DMA channel. Do you see a 
> better way to do the same?

Ah, so you have multiple values, too, and you just abstract them by using
an enum to index an array of platform_data in the dma engine.

This obviously works as well, and lets you get away with a single 32-bit
enum to use as the slave-id. However, I think slave drivers should not
be written with the assumption that all dma-engine drivers do this.
In particular, you seem to assume that the argument to the filter function
is not a pointer but this enum.

It also gets more complicated with the DT binding, since that should
reflect the hardware settings, i.e. not an arbitrarily defined enum
but the data that you have in your array. To keep the DT and platform_data
cases similar, I would suggest you actually remove the array listing
the per-slave data, and move that data instead as an opaque pointer into
the platform data. You can take a look at drivers/mmc/host/mmci.c for
an example of a driver that does this and that is portable across
multiple DMA engine drivers.

Essentially we pass the dma_filter function and dma_param struct pointers
to dma_request_channel directly from the platform_data, without trying
to interpret them. In case of stedma40, the dma_param contains a complex
data structure, while for others it may contain a single integer.

If you do the same, you would essentially pass the mid_rid and chcr values
of your sh_dmae_slave_config as in the pointer that gets passed from
the platform_data down to the filter function, get rid of the slave_id
number and pass the addr value through slave_config.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-06-07 16:06               ` Arnd Bergmann
@ 2013-06-19 19:51                 ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-19 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Vinod

Sorry, originally the 2 patches from this thread only touched files under 
mmc and ARM, so, you were not CCed, but eventually this turned into a 
dmaengine API discussion, so, I'm adding you to CC and asking about your 
opinion.

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

> On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> > Isn't it a board property? In case of external devices, or an SoC property 
> > in case of integrated DMA slave and controller? 
> 
> Correct.
> 
> > Let me try to explain. The 
> > DMAC has to configure the controller to "link" a specific DMA channel to a 
> > slave request line. E.g. to link an internal channel #0 to SDHI0 tx. To do 
> > this it has to write specific "magic" values in certain DMAC registers. 
> > Those values aren't known to the DMAC driver, they are a property of the 
> > SoC. Let's say, the SoC has 3 DMAC instances, each of them can serve any 
> > of compatible DMA slaves, including SDHI0, on each of their 6 channels. To 
> > configure a channel on a DMAC instance for SDHI0 tx you have to write to 
> > that channel's config register a certain value, that cannot be calculated. 
> 
> Right. Or multiple values.
> 
> > Those values are only known to the SoC code, so, they are passed as 
> > platform parameters to the DMAC driver. Now, when SDHI0 asks DMAC to set 
> > up a DMA channel for its tx, the DMAC driver has to find that value in its 
> > platform data. In most cases you could use the client address (e.g. FIFO 
> > register) and direction to find that value. But, I think, that might not 
> > always be enough. So, we use unique enum values to find those values in 
> > DMAC platform data. The SDHI driver gets that enum value in its platform 
> > data, passes it to the DMAC driver as a slave ID, and the DMAC driver uses 
> > it to find the value(s), it needs to set up its DMA channel. Do you see a 
> > better way to do the same?
> 
> Ah, so you have multiple values, too, and you just abstract them by using
> an enum to index an array of platform_data in the dma engine.
> 
> This obviously works as well, and lets you get away with a single 32-bit
> enum to use as the slave-id. However, I think slave drivers should not
> be written with the assumption that all dma-engine drivers do this.
> In particular, you seem to assume that the argument to the filter function
> is not a pointer but this enum.
> 
> It also gets more complicated with the DT binding, since that should
> reflect the hardware settings, i.e. not an arbitrarily defined enum
> but the data that you have in your array. To keep the DT and platform_data
> cases similar, I would suggest you actually remove the array listing
> the per-slave data, and move that data instead as an opaque pointer into
> the platform data. You can take a look at drivers/mmc/host/mmci.c for
> an example of a driver that does this and that is portable across
> multiple DMA engine drivers.
> 
> Essentially we pass the dma_filter function and dma_param struct pointers
> to dma_request_channel directly from the platform_data, without trying
> to interpret them. In case of stedma40, the dma_param contains a complex
> data structure, while for others it may contain a single integer.

And what happens then? How do those parameters get used by the stedma40 
driver to configure the hardware? I see, in its filter function the 
stedma40 driver is storing the configuration in the driver private channel 
struct for later use. SHDMA used to do the same (ok, we used the .private 
pointer for that, but essentially it was the same), but we were told to 
stop doing that. The filter function should only verify, whether a channel 
is suitable, and not set any configuration. That's what the 
dmaengine_slave_config() function is for, including the .slave_id 
parameter in struct dma_slave_config.

> If you do the same, you would essentially pass the mid_rid and chcr values
> of your sh_dmae_slave_config as in the pointer that gets passed from
> the platform_data down to the filter function, get rid of the slave_id
> number and pass the addr value through slave_config.

As explained in an earlier email, my opinion is, that my simple header 
patch is a corrent fix for the currently present build failure. As for 
this discussion, what is your opinion on this? Is the new use of the 
.slave_id field in struct dma_slave_config correct in the driver and its 
users or not? 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-19 19:51                 ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-19 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Vinod

Sorry, originally the 2 patches from this thread only touched files under 
mmc and ARM, so, you were not CCed, but eventually this turned into a 
dmaengine API discussion, so, I'm adding you to CC and asking about your 
opinion.

On Fri, 7 Jun 2013, Arnd Bergmann wrote:

> On Friday 07 June 2013, Guennadi Liakhovetski wrote:
> > Isn't it a board property? In case of external devices, or an SoC property 
> > in case of integrated DMA slave and controller? 
> 
> Correct.
> 
> > Let me try to explain. The 
> > DMAC has to configure the controller to "link" a specific DMA channel to a 
> > slave request line. E.g. to link an internal channel #0 to SDHI0 tx. To do 
> > this it has to write specific "magic" values in certain DMAC registers. 
> > Those values aren't known to the DMAC driver, they are a property of the 
> > SoC. Let's say, the SoC has 3 DMAC instances, each of them can serve any 
> > of compatible DMA slaves, including SDHI0, on each of their 6 channels. To 
> > configure a channel on a DMAC instance for SDHI0 tx you have to write to 
> > that channel's config register a certain value, that cannot be calculated. 
> 
> Right. Or multiple values.
> 
> > Those values are only known to the SoC code, so, they are passed as 
> > platform parameters to the DMAC driver. Now, when SDHI0 asks DMAC to set 
> > up a DMA channel for its tx, the DMAC driver has to find that value in its 
> > platform data. In most cases you could use the client address (e.g. FIFO 
> > register) and direction to find that value. But, I think, that might not 
> > always be enough. So, we use unique enum values to find those values in 
> > DMAC platform data. The SDHI driver gets that enum value in its platform 
> > data, passes it to the DMAC driver as a slave ID, and the DMAC driver uses 
> > it to find the value(s), it needs to set up its DMA channel. Do you see a 
> > better way to do the same?
> 
> Ah, so you have multiple values, too, and you just abstract them by using
> an enum to index an array of platform_data in the dma engine.
> 
> This obviously works as well, and lets you get away with a single 32-bit
> enum to use as the slave-id. However, I think slave drivers should not
> be written with the assumption that all dma-engine drivers do this.
> In particular, you seem to assume that the argument to the filter function
> is not a pointer but this enum.
> 
> It also gets more complicated with the DT binding, since that should
> reflect the hardware settings, i.e. not an arbitrarily defined enum
> but the data that you have in your array. To keep the DT and platform_data
> cases similar, I would suggest you actually remove the array listing
> the per-slave data, and move that data instead as an opaque pointer into
> the platform data. You can take a look at drivers/mmc/host/mmci.c for
> an example of a driver that does this and that is portable across
> multiple DMA engine drivers.
> 
> Essentially we pass the dma_filter function and dma_param struct pointers
> to dma_request_channel directly from the platform_data, without trying
> to interpret them. In case of stedma40, the dma_param contains a complex
> data structure, while for others it may contain a single integer.

And what happens then? How do those parameters get used by the stedma40 
driver to configure the hardware? I see, in its filter function the 
stedma40 driver is storing the configuration in the driver private channel 
struct for later use. SHDMA used to do the same (ok, we used the .private 
pointer for that, but essentially it was the same), but we were told to 
stop doing that. The filter function should only verify, whether a channel 
is suitable, and not set any configuration. That's what the 
dmaengine_slave_config() function is for, including the .slave_id 
parameter in struct dma_slave_config.

> If you do the same, you would essentially pass the mid_rid and chcr values
> of your sh_dmae_slave_config as in the pointer that gets passed from
> the platform_data down to the filter function, get rid of the slave_id
> number and pass the addr value through slave_config.

As explained in an earlier email, my opinion is, that my simple header 
patch is a corrent fix for the currently present build failure. As for 
this discussion, what is your opinion on this? Is the new use of the 
.slave_id field in struct dma_slave_config correct in the driver and its 
users or not? 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
  2013-06-19 19:51                 ` Guennadi Liakhovetski
@ 2013-06-19 21:51                   ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-19 21:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 19 June 2013, Guennadi Liakhovetski wrote:
> > Ah, so you have multiple values, too, and you just abstract them by using
> > an enum to index an array of platform_data in the dma engine.
> > 
> > This obviously works as well, and lets you get away with a single 32-bit
> > enum to use as the slave-id. However, I think slave drivers should not
> > be written with the assumption that all dma-engine drivers do this.
> > In particular, you seem to assume that the argument to the filter function
> > is not a pointer but this enum.
> > 
> > It also gets more complicated with the DT binding, since that should
> > reflect the hardware settings, i.e. not an arbitrarily defined enum
> > but the data that you have in your array. To keep the DT and platform_data
> > cases similar, I would suggest you actually remove the array listing
> > the per-slave data, and move that data instead as an opaque pointer into
> > the platform data. You can take a look at drivers/mmc/host/mmci.c for
> > an example of a driver that does this and that is portable across
> > multiple DMA engine drivers.
> > 
> > Essentially we pass the dma_filter function and dma_param struct pointers
> > to dma_request_channel directly from the platform_data, without trying
> > to interpret them. In case of stedma40, the dma_param contains a complex
> > data structure, while for others it may contain a single integer.
> 
> And what happens then? How do those parameters get used by the stedma40 
> driver to configure the hardware? I see, in its filter function the 
> stedma40 driver is storing the configuration in the driver private channel 
> struct for later use. SHDMA used to do the same (ok, we used the .private 
> pointer for that, but essentially it was the same), but we were told to 
> stop doing that. The filter function should only verify, whether a channel 
> is suitable, and not set any configuration. That's what the 
> dmaengine_slave_config() function is for, including the .slave_id 
> parameter in struct dma_slave_config.

The problem with this is that on a lot of dma engines you have one channel
per slave (in particular with virt-dma.c), so the slave-id has to be
part of the filter data. Since the slave driver cannot know what kind
of DMA engine it is connected to, it has to assume that the slave-id
is inherent to the channel an cannot change.
Also, the DT binding is built around the idea that you identify the
channel or request line with the specifier in whichever form the
dma engine requires, and there is no general way for the slave driver
to find out a slave id. Setting it with dma_slave_config is inherently
nonportable, and we should stop doing that.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies
@ 2013-06-19 21:51                   ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-19 21:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 19 June 2013, Guennadi Liakhovetski wrote:
> > Ah, so you have multiple values, too, and you just abstract them by using
> > an enum to index an array of platform_data in the dma engine.
> > 
> > This obviously works as well, and lets you get away with a single 32-bit
> > enum to use as the slave-id. However, I think slave drivers should not
> > be written with the assumption that all dma-engine drivers do this.
> > In particular, you seem to assume that the argument to the filter function
> > is not a pointer but this enum.
> > 
> > It also gets more complicated with the DT binding, since that should
> > reflect the hardware settings, i.e. not an arbitrarily defined enum
> > but the data that you have in your array. To keep the DT and platform_data
> > cases similar, I would suggest you actually remove the array listing
> > the per-slave data, and move that data instead as an opaque pointer into
> > the platform data. You can take a look at drivers/mmc/host/mmci.c for
> > an example of a driver that does this and that is portable across
> > multiple DMA engine drivers.
> > 
> > Essentially we pass the dma_filter function and dma_param struct pointers
> > to dma_request_channel directly from the platform_data, without trying
> > to interpret them. In case of stedma40, the dma_param contains a complex
> > data structure, while for others it may contain a single integer.
> 
> And what happens then? How do those parameters get used by the stedma40 
> driver to configure the hardware? I see, in its filter function the 
> stedma40 driver is storing the configuration in the driver private channel 
> struct for later use. SHDMA used to do the same (ok, we used the .private 
> pointer for that, but essentially it was the same), but we were told to 
> stop doing that. The filter function should only verify, whether a channel 
> is suitable, and not set any configuration. That's what the 
> dmaengine_slave_config() function is for, including the .slave_id 
> parameter in struct dma_slave_config.

The problem with this is that on a lot of dma engines you have one channel
per slave (in particular with virt-dma.c), so the slave-id has to be
part of the filter data. Since the slave driver cannot know what kind
of DMA engine it is connected to, it has to assume that the slave-id
is inherent to the channel an cannot change.
Also, the DT binding is built around the idea that you identify the
channel or request line with the specifier in whichever form the
dma engine requires, and there is no general way for the slave driver
to find out a slave id. Setting it with dma_slave_config is inherently
nonportable, and we should stop doing that.

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* DMA channels (was Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies)
  2013-06-19 21:51                   ` Arnd Bergmann
@ 2013-06-26 10:10                     ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-26 10:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

I think it would be good to come to a certain conclusion regarding the 
current dmaengine channel selection and configuration API and its possibly 
desired amendments.

On Wed, 19 Jun 2013, Arnd Bergmann wrote:

[snip]

> The problem with this is that on a lot of dma engines you have one channel
> per slave (in particular with virt-dma.c), so the slave-id has to be
> part of the filter data. Since the slave driver cannot know what kind
> of DMA engine it is connected to, it has to assume that the slave-id
> is inherent to the channel an cannot change.
> Also, the DT binding is built around the idea that you identify the
> channel or request line with the specifier in whichever form the
> dma engine requires, and there is no general way for the slave driver
> to find out a slave id. Setting it with dma_slave_config is inherently
> nonportable, and we should stop doing that.

Don't you think, that this situation, where without DT a filter function 
has to be used, which has to be provided by the platform and is just a 
kind of a platform callback; and with DT channels are selected and 
configured by a reference to suitable DMAC instances and a hardware- 
specific data set is suboptimal? Wouldn't it be better to unify it?

If yes, the unification would go in the direction of DT compatibility, 
i.e. dropping filter functions and just directly referencing required 
DMACs and using DMAC-specific configuration? Wouldn't a channel requesting 
API, similar to that for requesting clocks, pinmux settings be a better 
option, than the current filtering procedure? Something like

	dma_request_slave_channel(dev, "tx", "dmac0", config);

in a simple case of just one suitable DMAC, or a NULL if DMACs should be 
able to figure out themselves, whether they can serve this slave, or 
something like "dmamux0" if there is a multiplexer, for which a driver has 
to be available, and in which case "config" would be that multiplexer 
driver's configuration?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* DMA channels (was Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies)
@ 2013-06-26 10:10                     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 44+ messages in thread
From: Guennadi Liakhovetski @ 2013-06-26 10:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd

I think it would be good to come to a certain conclusion regarding the 
current dmaengine channel selection and configuration API and its possibly 
desired amendments.

On Wed, 19 Jun 2013, Arnd Bergmann wrote:

[snip]

> The problem with this is that on a lot of dma engines you have one channel
> per slave (in particular with virt-dma.c), so the slave-id has to be
> part of the filter data. Since the slave driver cannot know what kind
> of DMA engine it is connected to, it has to assume that the slave-id
> is inherent to the channel an cannot change.
> Also, the DT binding is built around the idea that you identify the
> channel or request line with the specifier in whichever form the
> dma engine requires, and there is no general way for the slave driver
> to find out a slave id. Setting it with dma_slave_config is inherently
> nonportable, and we should stop doing that.

Don't you think, that this situation, where without DT a filter function 
has to be used, which has to be provided by the platform and is just a 
kind of a platform callback; and with DT channels are selected and 
configured by a reference to suitable DMAC instances and a hardware- 
specific data set is suboptimal? Wouldn't it be better to unify it?

If yes, the unification would go in the direction of DT compatibility, 
i.e. dropping filter functions and just directly referencing required 
DMACs and using DMAC-specific configuration? Wouldn't a channel requesting 
API, similar to that for requesting clocks, pinmux settings be a better 
option, than the current filtering procedure? Something like

	dma_request_slave_channel(dev, "tx", "dmac0", config);

in a simple case of just one suitable DMAC, or a NULL if DMACs should be 
able to figure out themselves, whether they can serve this slave, or 
something like "dmamux0" if there is a multiplexer, for which a driver has 
to be available, and in which case "config" would be that multiplexer 
driver's configuration?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: DMA channels (was Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies)
  2013-06-26 10:10                     ` Guennadi Liakhovetski
@ 2013-06-26 14:35                       ` Arnd Bergmann
  -1 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-26 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 26 June 2013, Guennadi Liakhovetski wrote:
> > The problem with this is that on a lot of dma engines you have one channel
> > per slave (in particular with virt-dma.c), so the slave-id has to be
> > part of the filter data. Since the slave driver cannot know what kind
> > of DMA engine it is connected to, it has to assume that the slave-id
> > is inherent to the channel an cannot change.
> > Also, the DT binding is built around the idea that you identify the
> > channel or request line with the specifier in whichever form the
> > dma engine requires, and there is no general way for the slave driver
> > to find out a slave id. Setting it with dma_slave_config is inherently
> > nonportable, and we should stop doing that.
> 
> Don't you think, that this situation, where without DT a filter function 
> has to be used, which has to be provided by the platform and is just a 
> kind of a platform callback; and with DT channels are selected and 
> configured by a reference to suitable DMAC instances and a hardware- 
> specific data set is suboptimal? Wouldn't it be better to unify it?
> 
> If yes, the unification would go in the direction of DT compatibility, 
> i.e. dropping filter functions and just directly referencing required 
> DMACs and using DMAC-specific configuration?

With the introduction of dma_request_slave_channel() we tried to
only change the cases going forward with DT, SFI and ACPI but
without changing the interface for the existing drivers, which
are highly inconsistent at the moment (filter function being defined
in the slave driver /or/ the platform /or/ the dmaengine driver).

> Wouldn't a channel requesting 
> API, similar to that for requesting clocks, pinmux settings be a better 
> option, than the current filtering procedure? Something like
> 
>         dma_request_slave_channel(dev, "tx", "dmac0", config);
> 
> in a simple case of just one suitable DMAC, or a NULL if DMACs should be 
> able to figure out themselves, whether they can serve this slave, or 
> something like "dmamux0" if there is a multiplexer, for which a driver has 
> to be available, and in which case "config" would be that multiplexer 
> driver's configuration?

I think what you suggest here is very similar to the existing
dma_request_slave_channel_compat() function, except that you
pass a string ("dmac0") instead of the filter function pointer,
and that string presumably also has to come from the platform,
since there is no other way for the driver to know it, right?

One idea that Vinod suggested was that the platform could register
a table of DMA configurations with the dmaengine core to do the
lookup by device and string. That would actually help unify the
API to the current dma_request_slave_channel(dev, name) form
for all the possible cases (DT, ACPI, SFI, platform_data).

In essence, the platform would have a table like clk_lookup
but also containing the config:

struct dma_lookup {
	struct list_head node;
	const char *dev_id;	/* the slave device */
	const char *con_id;	/* request line of the slave, e.g. "rx" */
	const char *dmadev_id;	/* master device name */
	void *slave_id;		/* data passed to the master */
};

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* DMA channels (was Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies)
@ 2013-06-26 14:35                       ` Arnd Bergmann
  0 siblings, 0 replies; 44+ messages in thread
From: Arnd Bergmann @ 2013-06-26 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 26 June 2013, Guennadi Liakhovetski wrote:
> > The problem with this is that on a lot of dma engines you have one channel
> > per slave (in particular with virt-dma.c), so the slave-id has to be
> > part of the filter data. Since the slave driver cannot know what kind
> > of DMA engine it is connected to, it has to assume that the slave-id
> > is inherent to the channel an cannot change.
> > Also, the DT binding is built around the idea that you identify the
> > channel or request line with the specifier in whichever form the
> > dma engine requires, and there is no general way for the slave driver
> > to find out a slave id. Setting it with dma_slave_config is inherently
> > nonportable, and we should stop doing that.
> 
> Don't you think, that this situation, where without DT a filter function 
> has to be used, which has to be provided by the platform and is just a 
> kind of a platform callback; and with DT channels are selected and 
> configured by a reference to suitable DMAC instances and a hardware- 
> specific data set is suboptimal? Wouldn't it be better to unify it?
> 
> If yes, the unification would go in the direction of DT compatibility, 
> i.e. dropping filter functions and just directly referencing required 
> DMACs and using DMAC-specific configuration?

With the introduction of dma_request_slave_channel() we tried to
only change the cases going forward with DT, SFI and ACPI but
without changing the interface for the existing drivers, which
are highly inconsistent at the moment (filter function being defined
in the slave driver /or/ the platform /or/ the dmaengine driver).

> Wouldn't a channel requesting 
> API, similar to that for requesting clocks, pinmux settings be a better 
> option, than the current filtering procedure? Something like
> 
>         dma_request_slave_channel(dev, "tx", "dmac0", config);
> 
> in a simple case of just one suitable DMAC, or a NULL if DMACs should be 
> able to figure out themselves, whether they can serve this slave, or 
> something like "dmamux0" if there is a multiplexer, for which a driver has 
> to be available, and in which case "config" would be that multiplexer 
> driver's configuration?

I think what you suggest here is very similar to the existing
dma_request_slave_channel_compat() function, except that you
pass a string ("dmac0") instead of the filter function pointer,
and that string presumably also has to come from the platform,
since there is no other way for the driver to know it, right?

One idea that Vinod suggested was that the platform could register
a table of DMA configurations with the dmaengine core to do the
lookup by device and string. That would actually help unify the
API to the current dma_request_slave_channel(dev, name) form
for all the possible cases (DT, ACPI, SFI, platform_data).

In essence, the platform would have a table like clk_lookup
but also containing the config:

struct dma_lookup {
	struct list_head node;
	const char *dev_id;	/* the slave device */
	const char *con_id;	/* request line of the slave, e.g. "rx" */
	const char *dmadev_id;	/* master device name */
	void *slave_id;		/* data passed to the master */
};

	Arnd

^ permalink raw reply	[flat|nested] 44+ messages in thread
* DMA channels (was Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies)
  2013-06-26 14:35                       ` Arnd Bergmann
@ 2013-06-26 16:00                         ` Vinod Koul
  -1 siblings, 0 replies; 44+ messages in thread
From: Vinod Koul @ 2013-06-26 15:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jun 26, 2013 at 04:35:45PM +0200, Arnd Bergmann wrote:
> On Wednesday 26 June 2013, Guennadi Liakhovetski wrote:
> > > The problem with this is that on a lot of dma engines you have one channel
> > > per slave (in particular with virt-dma.c), so the slave-id has to be
> > > part of the filter data. Since the slave driver cannot know what kind
> > > of DMA engine it is connected to, it has to assume that the slave-id
> > > is inherent to the channel an cannot change.
> > > Also, the DT binding is built around the idea that you identify the
> > > channel or request line with the specifier in whichever form the
> > > dma engine requires, and there is no general way for the slave driver
> > > to find out a slave id. Setting it with dma_slave_config is inherently
> > > nonportable, and we should stop doing that.
> > 
> > Don't you think, that this situation, where without DT a filter function 
> > has to be used, which has to be provided by the platform and is just a 
> > kind of a platform callback; and with DT channels are selected and 
> > configured by a reference to suitable DMAC instances and a hardware- 
> > specific data set is suboptimal? Wouldn't it be better to unify it?
> > 
> > If yes, the unification would go in the direction of DT compatibility, 
> > i.e. dropping filter functions and just directly referencing required 
> > DMACs and using DMAC-specific configuration?
> 
> With the introduction of dma_request_slave_channel() we tried to
> only change the cases going forward with DT, SFI and ACPI but
> without changing the interface for the existing drivers, which
> are highly inconsistent at the moment (filter function being defined
> in the slave driver /or/ the platform /or/ the dmaengine driver).
> 
> > Wouldn't a channel requesting 
> > API, similar to that for requesting clocks, pinmux settings be a better 
> > option, than the current filtering procedure? Something like
> > 
> >         dma_request_slave_channel(dev, "tx", "dmac0", config);
> > 
> > in a simple case of just one suitable DMAC, or a NULL if DMACs should be 
> > able to figure out themselves, whether they can serve this slave, or 
> > something like "dmamux0" if there is a multiplexer, for which a driver has 
> > to be available, and in which case "config" would be that multiplexer 
> > driver's configuration?
> 
> I think what you suggest here is very similar to the existing
> dma_request_slave_channel_compat() function, except that you
> pass a string ("dmac0") instead of the filter function pointer,
> and that string presumably also has to come from the platform,
> since there is no other way for the driver to know it, right?
> 
> One idea that Vinod suggested was that the platform could register
> a table of DMA configurations with the dmaengine core to do the
> lookup by device and string. That would actually help unify the
> API to the current dma_request_slave_channel(dev, name) form
> for all the possible cases (DT, ACPI, SFI, platform_data).
> 
> In essence, the platform would have a table like clk_lookup
> but also containing the config:
> 
> struct dma_lookup {
> 	struct list_head node;
> 	const char *dev_id;	/* the slave device */
> 	const char *con_id;	/* request line of the slave, e.g. "rx" */
> 	const char *dmadev_id;	/* master device name */
> 	void *slave_id;		/* data passed to the master */
> };
Let me describes the scenarios we want to solve:
1. dma controllers with SW mux:
	In this case the dma controller has SW mux to connect to periphrals.
The mux is programmed to talk to a slave controller. SO we can grab any channel
from that controller, we just need to pass the mux value.
Ideally, in this scenario virtual channels should be described by driver (not
one per physical channel though) and driver uses mux values to transfer to
periphrals

2. dma controllers with hardwired channels:
	In this case you need a channel from a controller A and also channel X,
as it is hard wired to periphral. This is quite common in bunch of drivers.
In this case we just have to look for this channel.

3. MIX
	Yes, I believe we have controllers where they have SW mux as well as
hard wired.

In case of 1 and 3a(sw), we just need controller info
In 2 and3b we need both info

SO the idea was that DT, ACPI pdata etc just tells me that how the assignments
should be done and at runtime dmanegine just does that.

This is what I had in mind (more on same lines as above)
struct dmaengine_slave_map {
	char *dma;	/* dma controller */
	char *client;	/* client */
	int ch;		/* specfic channel id, NULL for SW muxes */
	int slave_id;	/* request line */
	struct dmaengine_slave_map_entries *entry;
};
https://lkml.org/lkml/2012/9/14/139

This way we just ask for a channel and dmaengine know what to do.

No filter functions, no multiple request methods etc.

--
~Vinod

^ permalink raw reply	[flat|nested] 44+ messages in thread
* Re: DMA channels (was Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies)
@ 2013-06-26 16:00                         ` Vinod Koul
  0 siblings, 0 replies; 44+ messages in thread
From: Vinod Koul @ 2013-06-26 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jun 26, 2013 at 04:35:45PM +0200, Arnd Bergmann wrote:
> On Wednesday 26 June 2013, Guennadi Liakhovetski wrote:
> > > The problem with this is that on a lot of dma engines you have one channel
> > > per slave (in particular with virt-dma.c), so the slave-id has to be
> > > part of the filter data. Since the slave driver cannot know what kind
> > > of DMA engine it is connected to, it has to assume that the slave-id
> > > is inherent to the channel an cannot change.
> > > Also, the DT binding is built around the idea that you identify the
> > > channel or request line with the specifier in whichever form the
> > > dma engine requires, and there is no general way for the slave driver
> > > to find out a slave id. Setting it with dma_slave_config is inherently
> > > nonportable, and we should stop doing that.
> > 
> > Don't you think, that this situation, where without DT a filter function 
> > has to be used, which has to be provided by the platform and is just a 
> > kind of a platform callback; and with DT channels are selected and 
> > configured by a reference to suitable DMAC instances and a hardware- 
> > specific data set is suboptimal? Wouldn't it be better to unify it?
> > 
> > If yes, the unification would go in the direction of DT compatibility, 
> > i.e. dropping filter functions and just directly referencing required 
> > DMACs and using DMAC-specific configuration?
> 
> With the introduction of dma_request_slave_channel() we tried to
> only change the cases going forward with DT, SFI and ACPI but
> without changing the interface for the existing drivers, which
> are highly inconsistent at the moment (filter function being defined
> in the slave driver /or/ the platform /or/ the dmaengine driver).
> 
> > Wouldn't a channel requesting 
> > API, similar to that for requesting clocks, pinmux settings be a better 
> > option, than the current filtering procedure? Something like
> > 
> >         dma_request_slave_channel(dev, "tx", "dmac0", config);
> > 
> > in a simple case of just one suitable DMAC, or a NULL if DMACs should be 
> > able to figure out themselves, whether they can serve this slave, or 
> > something like "dmamux0" if there is a multiplexer, for which a driver has 
> > to be available, and in which case "config" would be that multiplexer 
> > driver's configuration?
> 
> I think what you suggest here is very similar to the existing
> dma_request_slave_channel_compat() function, except that you
> pass a string ("dmac0") instead of the filter function pointer,
> and that string presumably also has to come from the platform,
> since there is no other way for the driver to know it, right?
> 
> One idea that Vinod suggested was that the platform could register
> a table of DMA configurations with the dmaengine core to do the
> lookup by device and string. That would actually help unify the
> API to the current dma_request_slave_channel(dev, name) form
> for all the possible cases (DT, ACPI, SFI, platform_data).
> 
> In essence, the platform would have a table like clk_lookup
> but also containing the config:
> 
> struct dma_lookup {
> 	struct list_head node;
> 	const char *dev_id;	/* the slave device */
> 	const char *con_id;	/* request line of the slave, e.g. "rx" */
> 	const char *dmadev_id;	/* master device name */
> 	void *slave_id;		/* data passed to the master */
> };
Let me describes the scenarios we want to solve:
1. dma controllers with SW mux:
	In this case the dma controller has SW mux to connect to periphrals.
The mux is programmed to talk to a slave controller. SO we can grab any channel
from that controller, we just need to pass the mux value.
Ideally, in this scenario virtual channels should be described by driver (not
one per physical channel though) and driver uses mux values to transfer to
periphrals

2. dma controllers with hardwired channels:
	In this case you need a channel from a controller A and also channel X,
as it is hard wired to periphral. This is quite common in bunch of drivers.
In this case we just have to look for this channel.

3. MIX
	Yes, I believe we have controllers where they have SW mux as well as
hard wired.

In case of 1 and 3a(sw), we just need controller info
In 2 and3b we need both info

SO the idea was that DT, ACPI pdata etc just tells me that how the assignments
should be done and at runtime dmanegine just does that.

This is what I had in mind (more on same lines as above)
struct dmaengine_slave_map {
	char *dma;	/* dma controller */
	char *client;	/* client */
	int ch;		/* specfic channel id, NULL for SW muxes */
	int slave_id;	/* request line */
	struct dmaengine_slave_map_entries *entry;
};
https://lkml.org/lkml/2012/9/14/139

This way we just ask for a channel and dmaengine know what to do.

No filter functions, no multiple request methods etc.

--
~Vinod

^ permalink raw reply	[flat|nested] 44+ messages in thread
end of thread, other threads:[~2013-06-26 16:00 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-31 13:56 [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code Arnd Bergmann
2013-05-31 13:56 ` Arnd Bergmann
2013-05-31 13:56 ` [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies Arnd Bergmann
2013-05-31 13:56   ` Arnd Bergmann
2013-05-31 13:58   ` Arnd Bergmann
2013-05-31 13:58     ` Arnd Bergmann
2013-06-07 10:22   ` Guennadi Liakhovetski
2013-06-07 10:22     ` Guennadi Liakhovetski
2013-06-07 12:59     ` Arnd Bergmann
2013-06-07 12:59       ` Arnd Bergmann
2013-06-07 13:12       ` Guennadi Liakhovetski
2013-06-07 13:12         ` Guennadi Liakhovetski
2013-06-07 14:53         ` Arnd Bergmann
2013-06-07 14:53           ` Arnd Bergmann
2013-06-07 15:32           ` Guennadi Liakhovetski
2013-06-07 15:32             ` Guennadi Liakhovetski
2013-06-07 16:06             ` Arnd Bergmann
2013-06-07 16:06               ` Arnd Bergmann
2013-06-19 19:51               ` Guennadi Liakhovetski
2013-06-19 19:51                 ` Guennadi Liakhovetski
2013-06-19 21:51                 ` Arnd Bergmann
2013-06-19 21:51                   ` Arnd Bergmann
2013-06-26 10:10                   ` DMA channels (was Re: [PATCH 2/2] ARM: shmobile: sdhi: remove DMA hardware dependencies) Guennadi Liakhovetski
2013-06-26 10:10                     ` Guennadi Liakhovetski
2013-06-26 14:35                     ` Arnd Bergmann
2013-06-26 14:35                       ` Arnd Bergmann
2013-06-26 15:48                       ` Vinod Koul
2013-06-26 16:00                         ` Vinod Koul
2013-05-31 14:52 ` [PATCH 1/2] ARM: shmobile: sdhi: pass DMA filter from platform code Guennadi Liakhovetski
2013-05-31 14:52   ` Guennadi Liakhovetski
2013-05-31 15:11   ` Arnd Bergmann
2013-05-31 15:11     ` Arnd Bergmann
2013-05-31 15:30     ` Guennadi Liakhovetski
2013-05-31 15:30       ` Guennadi Liakhovetski
2013-05-31 16:02       ` Arnd Bergmann
2013-05-31 16:02         ` Arnd Bergmann
2013-06-05  8:28         ` Guennadi Liakhovetski
2013-06-05  8:28           ` Guennadi Liakhovetski
2013-06-07 10:25 ` Guennadi Liakhovetski
2013-06-07 10:25   ` Guennadi Liakhovetski
2013-06-07 12:52   ` Arnd Bergmann
2013-06-07 12:52     ` Arnd Bergmann
2013-06-07 13:01     ` Guennadi Liakhovetski
2013-06-07 13:01       ` Guennadi Liakhovetski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.