All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/RFC 0/6] DMA: shdma: add Device Tree support
@ 2013-04-10 22:19 ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

This patch series adds Device Tree support to the shdma dmaengine driver 
and illustraits its use with the kzm9g-reference board's MMCIF interface.

Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Guennadi Liakhovetski (6):
  DMA: shdma: (cosmetic) don't re-calculate a pointer
  DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
  DMA: shdma: add DT support
  ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
  mmc: sh_mmcif: add support for Device Tree DMA bindings
  ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT

 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 ++
 arch/arm/boot/dts/sh73a0.dtsi                |   36 ++++++++++++++++++
 arch/arm/mach-shmobile/setup-sh73a0.c        |    4 ++
 drivers/dma/sh/shdma-base.c                  |   51 +++++++++++++++++++++++---
 drivers/dma/sh/shdma.c                       |   46 ++++++++++++++++++++----
 drivers/mmc/host/sh_mmcif.c                  |   29 +++++++++------
 include/linux/sh_dma.h                       |    2 -
 include/linux/shdma-base.h                   |    6 +++
 8 files changed, 151 insertions(+), 26 deletions(-)

-- 
1.7.2.5

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

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

* [PATCH/RFC 0/6] DMA: shdma: add Device Tree support
@ 2013-04-10 22:19 ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

This patch series adds Device Tree support to the shdma dmaengine driver 
and illustraits its use with the kzm9g-reference board's MMCIF interface.

Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Guennadi Liakhovetski (6):
  DMA: shdma: (cosmetic) don't re-calculate a pointer
  DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
  DMA: shdma: add DT support
  ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
  mmc: sh_mmcif: add support for Device Tree DMA bindings
  ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT

 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 ++
 arch/arm/boot/dts/sh73a0.dtsi                |   36 ++++++++++++++++++
 arch/arm/mach-shmobile/setup-sh73a0.c        |    4 ++
 drivers/dma/sh/shdma-base.c                  |   51 +++++++++++++++++++++++---
 drivers/dma/sh/shdma.c                       |   46 ++++++++++++++++++++----
 drivers/mmc/host/sh_mmcif.c                  |   29 +++++++++------
 include/linux/sh_dma.h                       |    2 -
 include/linux/shdma-base.h                   |    6 +++
 8 files changed, 151 insertions(+), 26 deletions(-)

-- 
1.7.2.5

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

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

* [PATCH 1/6] DMA: shdma: (cosmetic) don't re-calculate a pointer
  2013-04-10 22:19 ` Guennadi Liakhovetski
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

Use an existing pointer instead of retrieving it again.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 drivers/dma/sh/shdma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index b70709b..a5a1887 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -729,7 +729,7 @@ static int sh_dmae_probe(struct platform_device *pdev)
 		goto eshdma;
 
 	/* platform data */
-	shdev->pdata = pdev->dev.platform_data;
+	shdev->pdata = pdata;
 
 	if (pdata->chcr_offset)
 		shdev->chcr_offset = pdata->chcr_offset;
-- 
1.7.2.5


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

* [PATCH 1/6] DMA: shdma: (cosmetic) don't re-calculate a pointer
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

Use an existing pointer instead of retrieving it again.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 drivers/dma/sh/shdma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index b70709b..a5a1887 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -729,7 +729,7 @@ static int sh_dmae_probe(struct platform_device *pdev)
 		goto eshdma;
 
 	/* platform data */
-	shdev->pdata = pdev->dev.platform_data;
+	shdev->pdata = pdata;
 
 	if (pdata->chcr_offset)
 		shdev->chcr_offset = pdata->chcr_offset;
-- 
1.7.2.5


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

* [PATCH 2/6] DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
  2013-04-10 22:19 ` Guennadi Liakhovetski
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

shdma_chan_filter() is a function, provided by the shdma-base.c module,
move its declaration to the appropriate header.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 include/linux/sh_dma.h     |    2 --
 include/linux/shdma-base.h |    1 +
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
index b64d6be..4e83f3e 100644
--- a/include/linux/sh_dma.h
+++ b/include/linux/sh_dma.h
@@ -99,6 +99,4 @@ struct sh_dmae_pdata {
 #define CHCR_TE	0x00000002
 #define CHCR_IE	0x00000004
 
-bool shdma_chan_filter(struct dma_chan *chan, void *arg);
-
 #endif
diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index a3728bf..9a93897 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -122,5 +122,6 @@ void shdma_chan_remove(struct shdma_chan *schan);
 int shdma_init(struct device *dev, struct shdma_dev *sdev,
 		    int chan_num);
 void shdma_cleanup(struct shdma_dev *sdev);
+bool shdma_chan_filter(struct dma_chan *chan, void *arg);
 
 #endif
-- 
1.7.2.5


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

* [PATCH 2/6] DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

shdma_chan_filter() is a function, provided by the shdma-base.c module,
move its declaration to the appropriate header.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 include/linux/sh_dma.h     |    2 --
 include/linux/shdma-base.h |    1 +
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
index b64d6be..4e83f3e 100644
--- a/include/linux/sh_dma.h
+++ b/include/linux/sh_dma.h
@@ -99,6 +99,4 @@ struct sh_dmae_pdata {
 #define CHCR_TE	0x00000002
 #define CHCR_IE	0x00000004
 
-bool shdma_chan_filter(struct dma_chan *chan, void *arg);
-
 #endif
diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index a3728bf..9a93897 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -122,5 +122,6 @@ void shdma_chan_remove(struct shdma_chan *schan);
 int shdma_init(struct device *dev, struct shdma_dev *sdev,
 		    int chan_num);
 void shdma_cleanup(struct shdma_dev *sdev);
+bool shdma_chan_filter(struct dma_chan *chan, void *arg);
 
 #endif
-- 
1.7.2.5


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

* [PATCH/RFC 3/6] DMA: shdma: add DT support
  2013-04-10 22:19 ` Guennadi Liakhovetski
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

This patch adds Device Tree support to the shdma driver. No special DT
properties are used, only standard DMA DT bindings are implemented. Since
shdma controllers reside on SoCs, their configuration is SoC-specific and
shall be passed to the driver from the SoC platform data, using the
auxdata procedure.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---

Note concerning the filtering parameter. Without DT slave channels are 
configured, based on a purely virtual slave ID, passed by DMA clients 
to the shdma dmaengine driver. In the DT case we cannot use such 
virtual IDs, we use a hardware MID/RID (DMA request ID) value. This is 
confusing, ideally, all existing users should be converted to also use 
MID/RID. This patch doesn't do that, it leaves existing users 
unaffected and only uses MID/RID for DT.

 drivers/dma/sh/shdma-base.c |   51 +++++++++++++++++++++++++++++++++++++-----
 drivers/dma/sh/shdma.c      |   44 ++++++++++++++++++++++++++++++++-----
 include/linux/shdma-base.h  |    5 ++++
 3 files changed, 88 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
index 4acb85a..4fd8293 100644
--- a/drivers/dma/sh/shdma-base.c
+++ b/drivers/dma/sh/shdma-base.c
@@ -14,6 +14,8 @@
  */
 
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
 #include <linux/shdma-base.h>
 #include <linux/dmaengine.h>
 #include <linux/init.h>
@@ -175,7 +177,18 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
 {
 	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
 	const struct shdma_ops *ops = sdev->ops;
-	int ret;
+	int ret, match;
+
+	if (schan->dev->of_node) {
+		match = schan->hw_req;
+		ret = ops->set_slave(schan, match, true);
+		if (ret < 0)
+			return ret;
+
+		slave_id = schan->slave_id;
+	} else {
+		match = slave_id;
+	}
 
 	if (slave_id < 0 || slave_id >= slave_num)
 		return -EINVAL;
@@ -183,7 +196,7 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
 	if (test_and_set_bit(slave_id, shdma_slave_used))
 		return -EBUSY;
 
-	ret = ops->set_slave(schan, slave_id, false);
+	ret = ops->set_slave(schan, match, false);
 	if (ret < 0) {
 		clear_bit(slave_id, shdma_slave_used);
 		return ret;
@@ -206,23 +219,26 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
  * services would have to provide their own filters, which first would check
  * the device driver, similar to how other DMAC drivers, e.g., sa11x0-dma.c, do
  * this, and only then, in case of a match, call this common filter.
+ * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
+ * In that case the MID-RID value is used for slave channel filtering and is
+ * passed to this function in the "arg" parameter.
  */
 bool shdma_chan_filter(struct dma_chan *chan, void *arg)
 {
 	struct shdma_chan *schan = to_shdma_chan(chan);
 	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
 	const struct shdma_ops *ops = sdev->ops;
-	int slave_id = (int)arg;
+	int match = (int)arg;
 	int ret;
 
-	if (slave_id < 0)
+	if (match < 0)
 		/* No slave requested - arbitrary channel */
 		return true;
 
-	if (slave_id >= slave_num)
+	if (!schan->dev->of_node && match >= slave_num)
 		return false;
 
-	ret = ops->set_slave(schan, slave_id, true);
+	ret = ops->set_slave(schan, match, true);
 	if (ret < 0)
 		return false;
 
@@ -867,6 +883,29 @@ void shdma_chan_remove(struct shdma_chan *schan)
 }
 EXPORT_SYMBOL(shdma_chan_remove);
 
+struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
+				 struct of_dma *ofdma)
+{
+	struct shdma_dev *sdev = ofdma->of_dma_data;
+	u32 id = dma_spec->args[0];
+	dma_cap_mask_t mask;
+	struct dma_chan *chan;
+
+	if (dma_spec->args_count != 1 || !sdev)
+		return NULL;
+
+	dma_cap_zero(mask);
+	/* Only slave DMA channels can be allocated via DT */
+	dma_cap_set(DMA_SLAVE, mask);
+
+	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
+	if (chan)
+		to_shdma_chan(chan)->hw_req = id;
+
+	return chan;
+}
+EXPORT_SYMBOL(shdma_xlate);
+
 int shdma_init(struct device *dev, struct shdma_dev *sdev,
 		    int chan_num)
 {
diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index a5a1887..db497d6 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -20,6 +20,8 @@
 
 #include <linux/init.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/dmaengine.h>
@@ -301,20 +303,32 @@ static void sh_dmae_setup_xfer(struct shdma_chan *schan,
 	}
 }
 
+/*
+ * Find a slave channel configuration from the contoller list by either a slave
+ * ID in the non-DT case, or by a MID/RID value in the DT case
+ */
 static const struct sh_dmae_slave_config *dmae_find_slave(
-	struct sh_dmae_chan *sh_chan, int slave_id)
+	struct sh_dmae_chan *sh_chan, int match)
 {
 	struct sh_dmae_device *shdev = to_sh_dev(sh_chan);
 	struct sh_dmae_pdata *pdata = shdev->pdata;
 	const struct sh_dmae_slave_config *cfg;
 	int i;
 
-	if (slave_id >= SH_DMA_SLAVE_NUMBER)
-		return NULL;
+	if (!sh_chan->shdma_chan.dev->of_node) {
+		if (match >= SH_DMA_SLAVE_NUMBER)
+			return NULL;
 
-	for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
-		if (cfg->slave_id = slave_id)
-			return cfg;
+		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
+			if (cfg->slave_id = match)
+				return cfg;
+	} else {
+		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
+			if (cfg->mid_rid = match) {
+				sh_chan->shdma_chan.slave_id = cfg->slave_id;
+				return cfg;
+			}
+	}
 
 	return NULL;
 }
@@ -841,6 +855,14 @@ static int sh_dmae_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto edmadevreg;
 
+	if (pdev->dev.of_node) {
+		int of_err = of_dma_controller_register(pdev->dev.of_node,
+							shdma_xlate, shdev);
+		if (of_err < 0 && of_err != -ENODEV)
+			dev_err(&pdev->dev,
+				"could not register of_dma_controller\n");
+	}
+
 	return err;
 
 edmadevreg:
@@ -889,6 +911,9 @@ static int sh_dmae_remove(struct platform_device *pdev)
 
 	dma_async_device_unregister(dma_dev);
 
+	if (pdev->dev.of_node)
+		of_dma_controller_free(pdev->dev.of_node);
+
 	if (errirq > 0)
 		free_irq(errirq, shdev);
 
@@ -920,11 +945,18 @@ static int sh_dmae_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id sh_dmae_of_match[] = {
+	{ .compatible = "renesas,shdma", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sh_dmae_of_match);
+
 static struct platform_driver sh_dmae_driver = {
 	.driver 	= {
 		.owner	= THIS_MODULE,
 		.pm	= &sh_dmae_pm,
 		.name	= SH_DMAE_DRV_NAME,
+		.of_match_table = sh_dmae_of_match,
 	},
 	.remove		= sh_dmae_remove,
 	.shutdown	= sh_dmae_shutdown,
diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index 9a93897..0815a0e 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -19,6 +19,7 @@
 #include <linux/dmaengine.h>
 #include <linux/interrupt.h>
 #include <linux/list.h>
+#include <linux/of_dma.h>
 #include <linux/types.h>
 
 /**
@@ -68,6 +69,8 @@ struct shdma_chan {
 	int id;				/* Raw id of this channel */
 	int irq;			/* Channel IRQ */
 	int slave_id;			/* Client ID for slave DMA */
+	int hw_req;			/* DMA request line for slave DMA - same
+					 * as MID/RID, used with DT */
 	enum shdma_pm_state pm_state;
 };
 
@@ -123,5 +126,7 @@ int shdma_init(struct device *dev, struct shdma_dev *sdev,
 		    int chan_num);
 void shdma_cleanup(struct shdma_dev *sdev);
 bool shdma_chan_filter(struct dma_chan *chan, void *arg);
+struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
+			     struct of_dma *ofdma);
 
 #endif
-- 
1.7.2.5


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

* [PATCH/RFC 3/6] DMA: shdma: add DT support
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

This patch adds Device Tree support to the shdma driver. No special DT
properties are used, only standard DMA DT bindings are implemented. Since
shdma controllers reside on SoCs, their configuration is SoC-specific and
shall be passed to the driver from the SoC platform data, using the
auxdata procedure.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---

Note concerning the filtering parameter. Without DT slave channels are 
configured, based on a purely virtual slave ID, passed by DMA clients 
to the shdma dmaengine driver. In the DT case we cannot use such 
virtual IDs, we use a hardware MID/RID (DMA request ID) value. This is 
confusing, ideally, all existing users should be converted to also use 
MID/RID. This patch doesn't do that, it leaves existing users 
unaffected and only uses MID/RID for DT.

 drivers/dma/sh/shdma-base.c |   51 +++++++++++++++++++++++++++++++++++++-----
 drivers/dma/sh/shdma.c      |   44 ++++++++++++++++++++++++++++++++-----
 include/linux/shdma-base.h  |    5 ++++
 3 files changed, 88 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
index 4acb85a..4fd8293 100644
--- a/drivers/dma/sh/shdma-base.c
+++ b/drivers/dma/sh/shdma-base.c
@@ -14,6 +14,8 @@
  */
 
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
 #include <linux/shdma-base.h>
 #include <linux/dmaengine.h>
 #include <linux/init.h>
@@ -175,7 +177,18 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
 {
 	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
 	const struct shdma_ops *ops = sdev->ops;
-	int ret;
+	int ret, match;
+
+	if (schan->dev->of_node) {
+		match = schan->hw_req;
+		ret = ops->set_slave(schan, match, true);
+		if (ret < 0)
+			return ret;
+
+		slave_id = schan->slave_id;
+	} else {
+		match = slave_id;
+	}
 
 	if (slave_id < 0 || slave_id >= slave_num)
 		return -EINVAL;
@@ -183,7 +196,7 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
 	if (test_and_set_bit(slave_id, shdma_slave_used))
 		return -EBUSY;
 
-	ret = ops->set_slave(schan, slave_id, false);
+	ret = ops->set_slave(schan, match, false);
 	if (ret < 0) {
 		clear_bit(slave_id, shdma_slave_used);
 		return ret;
@@ -206,23 +219,26 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
  * services would have to provide their own filters, which first would check
  * the device driver, similar to how other DMAC drivers, e.g., sa11x0-dma.c, do
  * this, and only then, in case of a match, call this common filter.
+ * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
+ * In that case the MID-RID value is used for slave channel filtering and is
+ * passed to this function in the "arg" parameter.
  */
 bool shdma_chan_filter(struct dma_chan *chan, void *arg)
 {
 	struct shdma_chan *schan = to_shdma_chan(chan);
 	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
 	const struct shdma_ops *ops = sdev->ops;
-	int slave_id = (int)arg;
+	int match = (int)arg;
 	int ret;
 
-	if (slave_id < 0)
+	if (match < 0)
 		/* No slave requested - arbitrary channel */
 		return true;
 
-	if (slave_id >= slave_num)
+	if (!schan->dev->of_node && match >= slave_num)
 		return false;
 
-	ret = ops->set_slave(schan, slave_id, true);
+	ret = ops->set_slave(schan, match, true);
 	if (ret < 0)
 		return false;
 
@@ -867,6 +883,29 @@ void shdma_chan_remove(struct shdma_chan *schan)
 }
 EXPORT_SYMBOL(shdma_chan_remove);
 
+struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
+				 struct of_dma *ofdma)
+{
+	struct shdma_dev *sdev = ofdma->of_dma_data;
+	u32 id = dma_spec->args[0];
+	dma_cap_mask_t mask;
+	struct dma_chan *chan;
+
+	if (dma_spec->args_count != 1 || !sdev)
+		return NULL;
+
+	dma_cap_zero(mask);
+	/* Only slave DMA channels can be allocated via DT */
+	dma_cap_set(DMA_SLAVE, mask);
+
+	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
+	if (chan)
+		to_shdma_chan(chan)->hw_req = id;
+
+	return chan;
+}
+EXPORT_SYMBOL(shdma_xlate);
+
 int shdma_init(struct device *dev, struct shdma_dev *sdev,
 		    int chan_num)
 {
diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index a5a1887..db497d6 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -20,6 +20,8 @@
 
 #include <linux/init.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_dma.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/dmaengine.h>
@@ -301,20 +303,32 @@ static void sh_dmae_setup_xfer(struct shdma_chan *schan,
 	}
 }
 
+/*
+ * Find a slave channel configuration from the contoller list by either a slave
+ * ID in the non-DT case, or by a MID/RID value in the DT case
+ */
 static const struct sh_dmae_slave_config *dmae_find_slave(
-	struct sh_dmae_chan *sh_chan, int slave_id)
+	struct sh_dmae_chan *sh_chan, int match)
 {
 	struct sh_dmae_device *shdev = to_sh_dev(sh_chan);
 	struct sh_dmae_pdata *pdata = shdev->pdata;
 	const struct sh_dmae_slave_config *cfg;
 	int i;
 
-	if (slave_id >= SH_DMA_SLAVE_NUMBER)
-		return NULL;
+	if (!sh_chan->shdma_chan.dev->of_node) {
+		if (match >= SH_DMA_SLAVE_NUMBER)
+			return NULL;
 
-	for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
-		if (cfg->slave_id == slave_id)
-			return cfg;
+		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
+			if (cfg->slave_id == match)
+				return cfg;
+	} else {
+		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
+			if (cfg->mid_rid == match) {
+				sh_chan->shdma_chan.slave_id = cfg->slave_id;
+				return cfg;
+			}
+	}
 
 	return NULL;
 }
@@ -841,6 +855,14 @@ static int sh_dmae_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto edmadevreg;
 
+	if (pdev->dev.of_node) {
+		int of_err = of_dma_controller_register(pdev->dev.of_node,
+							shdma_xlate, shdev);
+		if (of_err < 0 && of_err != -ENODEV)
+			dev_err(&pdev->dev,
+				"could not register of_dma_controller\n");
+	}
+
 	return err;
 
 edmadevreg:
@@ -889,6 +911,9 @@ static int sh_dmae_remove(struct platform_device *pdev)
 
 	dma_async_device_unregister(dma_dev);
 
+	if (pdev->dev.of_node)
+		of_dma_controller_free(pdev->dev.of_node);
+
 	if (errirq > 0)
 		free_irq(errirq, shdev);
 
@@ -920,11 +945,18 @@ static int sh_dmae_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id sh_dmae_of_match[] = {
+	{ .compatible = "renesas,shdma", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sh_dmae_of_match);
+
 static struct platform_driver sh_dmae_driver = {
 	.driver 	= {
 		.owner	= THIS_MODULE,
 		.pm	= &sh_dmae_pm,
 		.name	= SH_DMAE_DRV_NAME,
+		.of_match_table = sh_dmae_of_match,
 	},
 	.remove		= sh_dmae_remove,
 	.shutdown	= sh_dmae_shutdown,
diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index 9a93897..0815a0e 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -19,6 +19,7 @@
 #include <linux/dmaengine.h>
 #include <linux/interrupt.h>
 #include <linux/list.h>
+#include <linux/of_dma.h>
 #include <linux/types.h>
 
 /**
@@ -68,6 +69,8 @@ struct shdma_chan {
 	int id;				/* Raw id of this channel */
 	int irq;			/* Channel IRQ */
 	int slave_id;			/* Client ID for slave DMA */
+	int hw_req;			/* DMA request line for slave DMA - same
+					 * as MID/RID, used with DT */
 	enum shdma_pm_state pm_state;
 };
 
@@ -123,5 +126,7 @@ int shdma_init(struct device *dev, struct shdma_dev *sdev,
 		    int chan_num);
 void shdma_cleanup(struct shdma_dev *sdev);
 bool shdma_chan_filter(struct dma_chan *chan, void *arg);
+struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
+			     struct of_dma *ofdma);
 
 #endif
-- 
1.7.2.5


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

* [PATCH/RFC 4/6] ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
  2013-04-10 22:19 ` Guennadi Liakhovetski
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

Add a Device Tree node for the DMA0 controller on sh73a0 and
auxdata to supply platform data to the driver. To enable the
DMA0 controller it also has to be taken out of reset.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 arch/arm/boot/dts/sh73a0.dtsi         |   36 +++++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/setup-sh73a0.c |    4 +++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0.dtsi b/arch/arm/boot/dts/sh73a0.dtsi
index ec40bf7..edcf4f7 100644
--- a/arch/arm/boot/dts/sh73a0.dtsi
+++ b/arch/arm/boot/dts/sh73a0.dtsi
@@ -119,6 +119,42 @@
 			      0 32 0x4>;
 	};
 
+	dma0: shdma@fe000020 {
+		compatible = "renesas,shdma";
+		reg = <0xfe000020 0x89e0>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 129 4
+				0 109 4
+				0 110 4
+				0 111 4
+				0 112 4
+				0 113 4
+				0 114 4
+				0 115 4
+				0 116 4
+				0 117 4
+				0 118 4
+				0 119 4
+				0 120 4
+				0 121 4
+				0 122 4
+				0 123 4
+				0 124 4
+				0 125 4
+				0 126 4
+				0 127 4
+				0 128 4>;
+		interrupt-names = "error",
+				"ch0", "ch1", "ch2", "ch3",
+				"ch4", "ch5", "ch6", "ch7",
+				"ch8", "ch9", "ch10", "ch11",
+				"ch12", "ch13", "ch14", "ch15",
+				"ch16", "ch17", "ch18", "ch19";
+		#dma-cells = <1>;
+		dma-channels = <20>;
+		dma-requests = <256>;
+	};
+
 	i2c0: i2c@0xe6820000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/arch/arm/mach-shmobile/setup-sh73a0.c b/arch/arm/mach-shmobile/setup-sh73a0.c
index b8aa795..980dcf3 100644
--- a/arch/arm/mach-shmobile/setup-sh73a0.c
+++ b/arch/arm/mach-shmobile/setup-sh73a0.c
@@ -998,6 +998,7 @@ void __init sh73a0_init_delay(void)
 }
 
 static const struct of_dev_auxdata sh73a0_auxdata_lookup[] __initconst = {
+	OF_DEV_AUXDATA("renesas,shdma", 0xfe000020, "sh-dma-engine.0", &sh73a0_dmae_platform_data),
 	{},
 };
 
@@ -1005,6 +1006,9 @@ void __init sh73a0_add_standard_devices_dt(void)
 {
 	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", .id = -1, };
 
+	/* Clear software reset bit on SY-DMAC module */
+	__raw_writel(__raw_readl(SRCR2) & ~(1 << 18), SRCR2);
+
 	platform_add_devices(sh73a0_devices_dt,
 			     ARRAY_SIZE(sh73a0_devices_dt));
 	of_platform_populate(NULL, of_default_bus_match_table,
-- 
1.7.2.5


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

* [PATCH/RFC 4/6] ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

Add a Device Tree node for the DMA0 controller on sh73a0 and
auxdata to supply platform data to the driver. To enable the
DMA0 controller it also has to be taken out of reset.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 arch/arm/boot/dts/sh73a0.dtsi         |   36 +++++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/setup-sh73a0.c |    4 +++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0.dtsi b/arch/arm/boot/dts/sh73a0.dtsi
index ec40bf7..edcf4f7 100644
--- a/arch/arm/boot/dts/sh73a0.dtsi
+++ b/arch/arm/boot/dts/sh73a0.dtsi
@@ -119,6 +119,42 @@
 			      0 32 0x4>;
 	};
 
+	dma0: shdma@fe000020 {
+		compatible = "renesas,shdma";
+		reg = <0xfe000020 0x89e0>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 129 4
+				0 109 4
+				0 110 4
+				0 111 4
+				0 112 4
+				0 113 4
+				0 114 4
+				0 115 4
+				0 116 4
+				0 117 4
+				0 118 4
+				0 119 4
+				0 120 4
+				0 121 4
+				0 122 4
+				0 123 4
+				0 124 4
+				0 125 4
+				0 126 4
+				0 127 4
+				0 128 4>;
+		interrupt-names = "error",
+				"ch0", "ch1", "ch2", "ch3",
+				"ch4", "ch5", "ch6", "ch7",
+				"ch8", "ch9", "ch10", "ch11",
+				"ch12", "ch13", "ch14", "ch15",
+				"ch16", "ch17", "ch18", "ch19";
+		#dma-cells = <1>;
+		dma-channels = <20>;
+		dma-requests = <256>;
+	};
+
 	i2c0: i2c@0xe6820000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/arch/arm/mach-shmobile/setup-sh73a0.c b/arch/arm/mach-shmobile/setup-sh73a0.c
index b8aa795..980dcf3 100644
--- a/arch/arm/mach-shmobile/setup-sh73a0.c
+++ b/arch/arm/mach-shmobile/setup-sh73a0.c
@@ -998,6 +998,7 @@ void __init sh73a0_init_delay(void)
 }
 
 static const struct of_dev_auxdata sh73a0_auxdata_lookup[] __initconst = {
+	OF_DEV_AUXDATA("renesas,shdma", 0xfe000020, "sh-dma-engine.0", &sh73a0_dmae_platform_data),
 	{},
 };
 
@@ -1005,6 +1006,9 @@ void __init sh73a0_add_standard_devices_dt(void)
 {
 	struct platform_device_info devinfo = { .name = "cpufreq-cpu0", .id = -1, };
 
+	/* Clear software reset bit on SY-DMAC module */
+	__raw_writel(__raw_readl(SRCR2) & ~(1 << 18), SRCR2);
+
 	platform_add_devices(sh73a0_devices_dt,
 			     ARRAY_SIZE(sh73a0_devices_dt));
 	of_platform_populate(NULL, of_default_bus_match_table,
-- 
1.7.2.5


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

* [PATCH/RFC 5/6] mmc: sh_mmcif: add support for Device Tree DMA bindings
  2013-04-10 22:19 ` Guennadi Liakhovetski
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

To use DMA in the Device Tree case the driver has to be modified
to use suitable API to obtain DMA channels.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 drivers/mmc/host/sh_mmcif.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index ba76a53..446d948 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -386,25 +386,30 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 
 	host->dma_active = false;
 
-	if (!pdata)
-		return;
-
-	if (pdata->slave_id_tx <= 0 || pdata->slave_id_rx <= 0)
-		return;
+	if (pdata) {
+		if (pdata->slave_id_tx <= 0 || pdata->slave_id_rx <= 0)
+			return;
+	} else {
+		if (!host->pd->dev.of_node)
+			return;
+	}
 
 	/* We can only either use DMA for both Tx and Rx or not use it at all */
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 
-	host->chan_tx = dma_request_channel(mask, shdma_chan_filter,
-					    (void *)pdata->slave_id_tx);
+	host->chan_tx = dma_request_slave_channel_compat(mask, shdma_chan_filter,
+				pdata ? (void *)pdata->slave_id_tx : NULL,
+				&host->pd->dev, "tx");
 	dev_dbg(&host->pd->dev, "%s: TX: got channel %p\n", __func__,
 		host->chan_tx);
 
 	if (!host->chan_tx)
 		return;
 
-	cfg.slave_id = pdata->slave_id_tx;
+	/* In the OF case the driver will get the slave ID from the DT */
+	if (pdata)
+		cfg.slave_id = pdata->slave_id_tx;
 	cfg.direction = DMA_MEM_TO_DEV;
 	cfg.dst_addr = res->start + MMCIF_CE_DATA;
 	cfg.src_addr = 0;
@@ -412,15 +417,17 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 	if (ret < 0)
 		goto ecfgtx;
 
-	host->chan_rx = dma_request_channel(mask, shdma_chan_filter,
-					    (void *)pdata->slave_id_rx);
+	host->chan_rx = dma_request_slave_channel_compat(mask, shdma_chan_filter,
+				pdata ? (void *)pdata->slave_id_rx : NULL,
+				&host->pd->dev, "rx");
 	dev_dbg(&host->pd->dev, "%s: RX: got channel %p\n", __func__,
 		host->chan_rx);
 
 	if (!host->chan_rx)
 		goto erqrx;
 
-	cfg.slave_id = pdata->slave_id_rx;
+	if (pdata)
+		cfg.slave_id = pdata->slave_id_rx;
 	cfg.direction = DMA_DEV_TO_MEM;
 	cfg.dst_addr = 0;
 	cfg.src_addr = res->start + MMCIF_CE_DATA;
-- 
1.7.2.5


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

* [PATCH/RFC 5/6] mmc: sh_mmcif: add support for Device Tree DMA bindings
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

To use DMA in the Device Tree case the driver has to be modified
to use suitable API to obtain DMA channels.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 drivers/mmc/host/sh_mmcif.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index ba76a53..446d948 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -386,25 +386,30 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 
 	host->dma_active = false;
 
-	if (!pdata)
-		return;
-
-	if (pdata->slave_id_tx <= 0 || pdata->slave_id_rx <= 0)
-		return;
+	if (pdata) {
+		if (pdata->slave_id_tx <= 0 || pdata->slave_id_rx <= 0)
+			return;
+	} else {
+		if (!host->pd->dev.of_node)
+			return;
+	}
 
 	/* We can only either use DMA for both Tx and Rx or not use it at all */
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 
-	host->chan_tx = dma_request_channel(mask, shdma_chan_filter,
-					    (void *)pdata->slave_id_tx);
+	host->chan_tx = dma_request_slave_channel_compat(mask, shdma_chan_filter,
+				pdata ? (void *)pdata->slave_id_tx : NULL,
+				&host->pd->dev, "tx");
 	dev_dbg(&host->pd->dev, "%s: TX: got channel %p\n", __func__,
 		host->chan_tx);
 
 	if (!host->chan_tx)
 		return;
 
-	cfg.slave_id = pdata->slave_id_tx;
+	/* In the OF case the driver will get the slave ID from the DT */
+	if (pdata)
+		cfg.slave_id = pdata->slave_id_tx;
 	cfg.direction = DMA_MEM_TO_DEV;
 	cfg.dst_addr = res->start + MMCIF_CE_DATA;
 	cfg.src_addr = 0;
@@ -412,15 +417,17 @@ static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 	if (ret < 0)
 		goto ecfgtx;
 
-	host->chan_rx = dma_request_channel(mask, shdma_chan_filter,
-					    (void *)pdata->slave_id_rx);
+	host->chan_rx = dma_request_slave_channel_compat(mask, shdma_chan_filter,
+				pdata ? (void *)pdata->slave_id_rx : NULL,
+				&host->pd->dev, "rx");
 	dev_dbg(&host->pd->dev, "%s: RX: got channel %p\n", __func__,
 		host->chan_rx);
 
 	if (!host->chan_rx)
 		goto erqrx;
 
-	cfg.slave_id = pdata->slave_id_rx;
+	if (pdata)
+		cfg.slave_id = pdata->slave_id_rx;
 	cfg.direction = DMA_DEV_TO_MEM;
 	cfg.dst_addr = 0;
 	cfg.src_addr = res->start + MMCIF_CE_DATA;
-- 
1.7.2.5


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

* [PATCH/RFC 6/6] ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
  2013-04-10 22:19 ` Guennadi Liakhovetski
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

The MMCIF driver can use DMA for data transfer, add suitable
Device Tree bindings.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
index ed9dd45..e1d0d5d 100644
--- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
+++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
@@ -156,6 +156,9 @@
 	bus-width = <8>;
 	vmmc-supply = <&reg_1p8v>;
 	status = "okay";
+	dmas = <&dma0 0xd1
+		&dma0 0xd2>;
+	dma-names = "tx", "rx";
 };
 
 &sdhi0 {
-- 
1.7.2.5


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

* [PATCH/RFC 6/6] ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
@ 2013-04-10 22:19   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-10 22:19 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

The MMCIF driver can use DMA for data transfer, add suitable
Device Tree bindings.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---
 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
index ed9dd45..e1d0d5d 100644
--- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
+++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
@@ -156,6 +156,9 @@
 	bus-width = <8>;
 	vmmc-supply = <&reg_1p8v>;
 	status = "okay";
+	dmas = <&dma0 0xd1
+		&dma0 0xd2>;
+	dma-names = "tx", "rx";
 };
 
 &sdhi0 {
-- 
1.7.2.5


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

* Re: [PATCH/RFC 6/6] ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
  2013-04-10 22:19   ` Guennadi Liakhovetski
@ 2013-04-11  3:00     ` Simon Horman
  -1 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-04-11  3:00 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:19:49AM +0200, Guennadi Liakhovetski wrote:
> The MMCIF driver can use DMA for data transfer, add suitable
> Device Tree bindings.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Hi Guennadi, this seems reasonable to me.

I guess the best thing is for you to repost it once the pre-requisites
are in place.

> ---
>  arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> index ed9dd45..e1d0d5d 100644
> --- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> +++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> @@ -156,6 +156,9 @@
>  	bus-width = <8>;
>  	vmmc-supply = <&reg_1p8v>;
>  	status = "okay";
> +	dmas = <&dma0 0xd1
> +		&dma0 0xd2>;
> +	dma-names = "tx", "rx";
>  };
>  
>  &sdhi0 {
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH/RFC 6/6] ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
@ 2013-04-11  3:00     ` Simon Horman
  0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-04-11  3:00 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:19:49AM +0200, Guennadi Liakhovetski wrote:
> The MMCIF driver can use DMA for data transfer, add suitable
> Device Tree bindings.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Hi Guennadi, this seems reasonable to me.

I guess the best thing is for you to repost it once the pre-requisites
are in place.

> ---
>  arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> index ed9dd45..e1d0d5d 100644
> --- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> +++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
> @@ -156,6 +156,9 @@
>  	bus-width = <8>;
>  	vmmc-supply = <&reg_1p8v>;
>  	status = "okay";
> +	dmas = <&dma0 0xd1
> +		&dma0 0xd2>;
> +	dma-names = "tx", "rx";
>  };
>  
>  &sdhi0 {
> -- 
> 1.7.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH/RFC 4/6] ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
  2013-04-10 22:19   ` Guennadi Liakhovetski
@ 2013-04-11  3:01     ` Simon Horman
  -1 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-04-11  3:01 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:19:47AM +0200, Guennadi Liakhovetski wrote:
> Add a Device Tree node for the DMA0 controller on sh73a0 and
> auxdata to supply platform data to the driver. To enable the
> DMA0 controller it also has to be taken out of reset.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>


Hi Guennadi, this seems reasonable to me.

I guess the best thing is for you to repost it once the pre-requisites
are in place.

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

* Re: [PATCH/RFC 4/6] ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
@ 2013-04-11  3:01     ` Simon Horman
  0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-04-11  3:01 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:19:47AM +0200, Guennadi Liakhovetski wrote:
> Add a Device Tree node for the DMA0 controller on sh73a0 and
> auxdata to supply platform data to the driver. To enable the
> DMA0 controller it also has to be taken out of reset.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>


Hi Guennadi, this seems reasonable to me.

I guess the best thing is for you to repost it once the pre-requisites
are in place.

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

* Re: [PATCH/RFC 4/6] ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
  2013-04-11  3:01     ` Simon Horman
@ 2013-04-11  3:02       ` Simon Horman
  -1 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-04-11  3:02 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:01:27PM +0900, Simon Horman wrote:
> On Thu, Apr 11, 2013 at 12:19:47AM +0200, Guennadi Liakhovetski wrote:
> > Add a Device Tree node for the DMA0 controller on sh73a0 and
> > auxdata to supply platform data to the driver. To enable the
> > DMA0 controller it also has to be taken out of reset.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> 
> 
> Hi Guennadi, this seems reasonable to me.
> 
> I guess the best thing is for you to repost it once the pre-requisites
> are in place.

Also, Magnus, could you review this?

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

* Re: [PATCH/RFC 4/6] ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
@ 2013-04-11  3:02       ` Simon Horman
  0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-04-11  3:02 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:01:27PM +0900, Simon Horman wrote:
> On Thu, Apr 11, 2013 at 12:19:47AM +0200, Guennadi Liakhovetski wrote:
> > Add a Device Tree node for the DMA0 controller on sh73a0 and
> > auxdata to supply platform data to the driver. To enable the
> > DMA0 controller it also has to be taken out of reset.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> 
> 
> Hi Guennadi, this seems reasonable to me.
> 
> I guess the best thing is for you to repost it once the pre-requisites
> are in place.

Also, Magnus, could you review this?

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
  2013-04-10 22:19   ` Guennadi Liakhovetski
@ 2013-04-15 16:54     ` Vinod Koul
  -1 siblings, 0 replies; 32+ messages in thread
From: Vinod Koul @ 2013-04-15 16:42 UTC (permalink / raw)
  To: Guennadi Liakhovetski, Arnd Bergmann
  Cc: linux-sh, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:19:46AM +0200, Guennadi Liakhovetski wrote:
> This patch adds Device Tree support to the shdma driver. No special DT
> properties are used, only standard DMA DT bindings are implemented. Since
> shdma controllers reside on SoCs, their configuration is SoC-specific and
> shall be passed to the driver from the SoC platform data, using the
> auxdata procedure.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
Need Arnd to review :)
> 
> Note concerning the filtering parameter. Without DT slave channels are 
> configured, based on a purely virtual slave ID, passed by DMA clients 
> to the shdma dmaengine driver. In the DT case we cannot use such 
> virtual IDs, we use a hardware MID/RID (DMA request ID) value. This is 
> confusing, ideally, all existing users should be converted to also use 
> MID/RID. This patch doesn't do that, it leaves existing users 
> unaffected and only uses MID/RID for DT.
> 
>  drivers/dma/sh/shdma-base.c |   51 +++++++++++++++++++++++++++++++++++++-----
>  drivers/dma/sh/shdma.c      |   44 ++++++++++++++++++++++++++++++++-----
>  include/linux/shdma-base.h  |    5 ++++
>  3 files changed, 88 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
> index 4acb85a..4fd8293 100644
> --- a/drivers/dma/sh/shdma-base.c
> +++ b/drivers/dma/sh/shdma-base.c
> @@ -14,6 +14,8 @@
>   */
>  
>  #include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_dma.h>
>  #include <linux/shdma-base.h>
>  #include <linux/dmaengine.h>
>  #include <linux/init.h>
> @@ -175,7 +177,18 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
>  {
>  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
>  	const struct shdma_ops *ops = sdev->ops;
> -	int ret;
> +	int ret, match;
> +
> +	if (schan->dev->of_node) {
> +		match = schan->hw_req;
> +		ret = ops->set_slave(schan, match, true);
> +		if (ret < 0)
> +			return ret;
> +
> +		slave_id = schan->slave_id;
> +	} else {
> +		match = slave_id;
> +	}
>  
>  	if (slave_id < 0 || slave_id >= slave_num)
>  		return -EINVAL;
> @@ -183,7 +196,7 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
>  	if (test_and_set_bit(slave_id, shdma_slave_used))
>  		return -EBUSY;
>  
> -	ret = ops->set_slave(schan, slave_id, false);
> +	ret = ops->set_slave(schan, match, false);
>  	if (ret < 0) {
>  		clear_bit(slave_id, shdma_slave_used);
>  		return ret;
> @@ -206,23 +219,26 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
>   * services would have to provide their own filters, which first would check
>   * the device driver, similar to how other DMAC drivers, e.g., sa11x0-dma.c, do
>   * this, and only then, in case of a match, call this common filter.
> + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> + * In that case the MID-RID value is used for slave channel filtering and is
> + * passed to this function in the "arg" parameter.
>   */
>  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
>  {
>  	struct shdma_chan *schan = to_shdma_chan(chan);
>  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
>  	const struct shdma_ops *ops = sdev->ops;
> -	int slave_id = (int)arg;
> +	int match = (int)arg;
>  	int ret;
>  
> -	if (slave_id < 0)
> +	if (match < 0)
>  		/* No slave requested - arbitrary channel */
>  		return true;
>  
> -	if (slave_id >= slave_num)
> +	if (!schan->dev->of_node && match >= slave_num)
>  		return false;
>  
> -	ret = ops->set_slave(schan, slave_id, true);
> +	ret = ops->set_slave(schan, match, true);
>  	if (ret < 0)
>  		return false;
>  
> @@ -867,6 +883,29 @@ void shdma_chan_remove(struct shdma_chan *schan)
>  }
>  EXPORT_SYMBOL(shdma_chan_remove);
>  
> +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> +				 struct of_dma *ofdma)
> +{
> +	struct shdma_dev *sdev = ofdma->of_dma_data;
> +	u32 id = dma_spec->args[0];
> +	dma_cap_mask_t mask;
> +	struct dma_chan *chan;
> +
> +	if (dma_spec->args_count != 1 || !sdev)
> +		return NULL;
> +
> +	dma_cap_zero(mask);
> +	/* Only slave DMA channels can be allocated via DT */
> +	dma_cap_set(DMA_SLAVE, mask);
> +
> +	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
> +	if (chan)
> +		to_shdma_chan(chan)->hw_req = id;
> +
> +	return chan;
> +}
> +EXPORT_SYMBOL(shdma_xlate);
> +
>  int shdma_init(struct device *dev, struct shdma_dev *sdev,
>  		    int chan_num)
>  {
> diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
> index a5a1887..db497d6 100644
> --- a/drivers/dma/sh/shdma.c
> +++ b/drivers/dma/sh/shdma.c
> @@ -20,6 +20,8 @@
>  
>  #include <linux/init.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_dma.h>
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/dmaengine.h>
> @@ -301,20 +303,32 @@ static void sh_dmae_setup_xfer(struct shdma_chan *schan,
>  	}
>  }
>  
> +/*
> + * Find a slave channel configuration from the contoller list by either a slave
> + * ID in the non-DT case, or by a MID/RID value in the DT case
> + */
>  static const struct sh_dmae_slave_config *dmae_find_slave(
> -	struct sh_dmae_chan *sh_chan, int slave_id)
> +	struct sh_dmae_chan *sh_chan, int match)
>  {
>  	struct sh_dmae_device *shdev = to_sh_dev(sh_chan);
>  	struct sh_dmae_pdata *pdata = shdev->pdata;
>  	const struct sh_dmae_slave_config *cfg;
>  	int i;
>  
> -	if (slave_id >= SH_DMA_SLAVE_NUMBER)
> -		return NULL;
> +	if (!sh_chan->shdma_chan.dev->of_node) {
> +		if (match >= SH_DMA_SLAVE_NUMBER)
> +			return NULL;
>  
> -	for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
> -		if (cfg->slave_id == slave_id)
> -			return cfg;
> +		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
> +			if (cfg->slave_id == match)
> +				return cfg;
> +	} else {
> +		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
> +			if (cfg->mid_rid == match) {
> +				sh_chan->shdma_chan.slave_id = cfg->slave_id;
> +				return cfg;
> +			}
> +	}
>  
>  	return NULL;
>  }
> @@ -841,6 +855,14 @@ static int sh_dmae_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto edmadevreg;
>  
> +	if (pdev->dev.of_node) {
> +		int of_err = of_dma_controller_register(pdev->dev.of_node,
> +							shdma_xlate, shdev);
> +		if (of_err < 0 && of_err != -ENODEV)
> +			dev_err(&pdev->dev,
> +				"could not register of_dma_controller\n");
> +	}
> +
>  	return err;
>  
>  edmadevreg:
> @@ -889,6 +911,9 @@ static int sh_dmae_remove(struct platform_device *pdev)
>  
>  	dma_async_device_unregister(dma_dev);
>  
> +	if (pdev->dev.of_node)
> +		of_dma_controller_free(pdev->dev.of_node);
> +
>  	if (errirq > 0)
>  		free_irq(errirq, shdev);
>  
> @@ -920,11 +945,18 @@ static int sh_dmae_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id sh_dmae_of_match[] = {
> +	{ .compatible = "renesas,shdma", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sh_dmae_of_match);
> +
>  static struct platform_driver sh_dmae_driver = {
>  	.driver 	= {
>  		.owner	= THIS_MODULE,
>  		.pm	= &sh_dmae_pm,
>  		.name	= SH_DMAE_DRV_NAME,
> +		.of_match_table = sh_dmae_of_match,
>  	},
>  	.remove		= sh_dmae_remove,
>  	.shutdown	= sh_dmae_shutdown,
> diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
> index 9a93897..0815a0e 100644
> --- a/include/linux/shdma-base.h
> +++ b/include/linux/shdma-base.h
> @@ -19,6 +19,7 @@
>  #include <linux/dmaengine.h>
>  #include <linux/interrupt.h>
>  #include <linux/list.h>
> +#include <linux/of_dma.h>
>  #include <linux/types.h>
>  
>  /**
> @@ -68,6 +69,8 @@ struct shdma_chan {
>  	int id;				/* Raw id of this channel */
>  	int irq;			/* Channel IRQ */
>  	int slave_id;			/* Client ID for slave DMA */
> +	int hw_req;			/* DMA request line for slave DMA - same
> +					 * as MID/RID, used with DT */
>  	enum shdma_pm_state pm_state;
>  };
>  
> @@ -123,5 +126,7 @@ int shdma_init(struct device *dev, struct shdma_dev *sdev,
>  		    int chan_num);
>  void shdma_cleanup(struct shdma_dev *sdev);
>  bool shdma_chan_filter(struct dma_chan *chan, void *arg);
> +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> +			     struct of_dma *ofdma);
>  
>  #endif
> -- 
> 1.7.2.5
> 

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
@ 2013-04-15 16:54     ` Vinod Koul
  0 siblings, 0 replies; 32+ messages in thread
From: Vinod Koul @ 2013-04-15 16:54 UTC (permalink / raw)
  To: Guennadi Liakhovetski, Arnd Bergmann
  Cc: linux-sh, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Thu, Apr 11, 2013 at 12:19:46AM +0200, Guennadi Liakhovetski wrote:
> This patch adds Device Tree support to the shdma driver. No special DT
> properties are used, only standard DMA DT bindings are implemented. Since
> shdma controllers reside on SoCs, their configuration is SoC-specific and
> shall be passed to the driver from the SoC platform data, using the
> auxdata procedure.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
Need Arnd to review :)
> 
> Note concerning the filtering parameter. Without DT slave channels are 
> configured, based on a purely virtual slave ID, passed by DMA clients 
> to the shdma dmaengine driver. In the DT case we cannot use such 
> virtual IDs, we use a hardware MID/RID (DMA request ID) value. This is 
> confusing, ideally, all existing users should be converted to also use 
> MID/RID. This patch doesn't do that, it leaves existing users 
> unaffected and only uses MID/RID for DT.
> 
>  drivers/dma/sh/shdma-base.c |   51 +++++++++++++++++++++++++++++++++++++-----
>  drivers/dma/sh/shdma.c      |   44 ++++++++++++++++++++++++++++++++-----
>  include/linux/shdma-base.h  |    5 ++++
>  3 files changed, 88 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
> index 4acb85a..4fd8293 100644
> --- a/drivers/dma/sh/shdma-base.c
> +++ b/drivers/dma/sh/shdma-base.c
> @@ -14,6 +14,8 @@
>   */
>  
>  #include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_dma.h>
>  #include <linux/shdma-base.h>
>  #include <linux/dmaengine.h>
>  #include <linux/init.h>
> @@ -175,7 +177,18 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
>  {
>  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
>  	const struct shdma_ops *ops = sdev->ops;
> -	int ret;
> +	int ret, match;
> +
> +	if (schan->dev->of_node) {
> +		match = schan->hw_req;
> +		ret = ops->set_slave(schan, match, true);
> +		if (ret < 0)
> +			return ret;
> +
> +		slave_id = schan->slave_id;
> +	} else {
> +		match = slave_id;
> +	}
>  
>  	if (slave_id < 0 || slave_id >= slave_num)
>  		return -EINVAL;
> @@ -183,7 +196,7 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
>  	if (test_and_set_bit(slave_id, shdma_slave_used))
>  		return -EBUSY;
>  
> -	ret = ops->set_slave(schan, slave_id, false);
> +	ret = ops->set_slave(schan, match, false);
>  	if (ret < 0) {
>  		clear_bit(slave_id, shdma_slave_used);
>  		return ret;
> @@ -206,23 +219,26 @@ static int shdma_setup_slave(struct shdma_chan *schan, int slave_id)
>   * services would have to provide their own filters, which first would check
>   * the device driver, similar to how other DMAC drivers, e.g., sa11x0-dma.c, do
>   * this, and only then, in case of a match, call this common filter.
> + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> + * In that case the MID-RID value is used for slave channel filtering and is
> + * passed to this function in the "arg" parameter.
>   */
>  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
>  {
>  	struct shdma_chan *schan = to_shdma_chan(chan);
>  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
>  	const struct shdma_ops *ops = sdev->ops;
> -	int slave_id = (int)arg;
> +	int match = (int)arg;
>  	int ret;
>  
> -	if (slave_id < 0)
> +	if (match < 0)
>  		/* No slave requested - arbitrary channel */
>  		return true;
>  
> -	if (slave_id >= slave_num)
> +	if (!schan->dev->of_node && match >= slave_num)
>  		return false;
>  
> -	ret = ops->set_slave(schan, slave_id, true);
> +	ret = ops->set_slave(schan, match, true);
>  	if (ret < 0)
>  		return false;
>  
> @@ -867,6 +883,29 @@ void shdma_chan_remove(struct shdma_chan *schan)
>  }
>  EXPORT_SYMBOL(shdma_chan_remove);
>  
> +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> +				 struct of_dma *ofdma)
> +{
> +	struct shdma_dev *sdev = ofdma->of_dma_data;
> +	u32 id = dma_spec->args[0];
> +	dma_cap_mask_t mask;
> +	struct dma_chan *chan;
> +
> +	if (dma_spec->args_count != 1 || !sdev)
> +		return NULL;
> +
> +	dma_cap_zero(mask);
> +	/* Only slave DMA channels can be allocated via DT */
> +	dma_cap_set(DMA_SLAVE, mask);
> +
> +	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
> +	if (chan)
> +		to_shdma_chan(chan)->hw_req = id;
> +
> +	return chan;
> +}
> +EXPORT_SYMBOL(shdma_xlate);
> +
>  int shdma_init(struct device *dev, struct shdma_dev *sdev,
>  		    int chan_num)
>  {
> diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
> index a5a1887..db497d6 100644
> --- a/drivers/dma/sh/shdma.c
> +++ b/drivers/dma/sh/shdma.c
> @@ -20,6 +20,8 @@
>  
>  #include <linux/init.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_dma.h>
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/dmaengine.h>
> @@ -301,20 +303,32 @@ static void sh_dmae_setup_xfer(struct shdma_chan *schan,
>  	}
>  }
>  
> +/*
> + * Find a slave channel configuration from the contoller list by either a slave
> + * ID in the non-DT case, or by a MID/RID value in the DT case
> + */
>  static const struct sh_dmae_slave_config *dmae_find_slave(
> -	struct sh_dmae_chan *sh_chan, int slave_id)
> +	struct sh_dmae_chan *sh_chan, int match)
>  {
>  	struct sh_dmae_device *shdev = to_sh_dev(sh_chan);
>  	struct sh_dmae_pdata *pdata = shdev->pdata;
>  	const struct sh_dmae_slave_config *cfg;
>  	int i;
>  
> -	if (slave_id >= SH_DMA_SLAVE_NUMBER)
> -		return NULL;
> +	if (!sh_chan->shdma_chan.dev->of_node) {
> +		if (match >= SH_DMA_SLAVE_NUMBER)
> +			return NULL;
>  
> -	for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
> -		if (cfg->slave_id = slave_id)
> -			return cfg;
> +		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
> +			if (cfg->slave_id = match)
> +				return cfg;
> +	} else {
> +		for (i = 0, cfg = pdata->slave; i < pdata->slave_num; i++, cfg++)
> +			if (cfg->mid_rid = match) {
> +				sh_chan->shdma_chan.slave_id = cfg->slave_id;
> +				return cfg;
> +			}
> +	}
>  
>  	return NULL;
>  }
> @@ -841,6 +855,14 @@ static int sh_dmae_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto edmadevreg;
>  
> +	if (pdev->dev.of_node) {
> +		int of_err = of_dma_controller_register(pdev->dev.of_node,
> +							shdma_xlate, shdev);
> +		if (of_err < 0 && of_err != -ENODEV)
> +			dev_err(&pdev->dev,
> +				"could not register of_dma_controller\n");
> +	}
> +
>  	return err;
>  
>  edmadevreg:
> @@ -889,6 +911,9 @@ static int sh_dmae_remove(struct platform_device *pdev)
>  
>  	dma_async_device_unregister(dma_dev);
>  
> +	if (pdev->dev.of_node)
> +		of_dma_controller_free(pdev->dev.of_node);
> +
>  	if (errirq > 0)
>  		free_irq(errirq, shdev);
>  
> @@ -920,11 +945,18 @@ static int sh_dmae_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id sh_dmae_of_match[] = {
> +	{ .compatible = "renesas,shdma", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sh_dmae_of_match);
> +
>  static struct platform_driver sh_dmae_driver = {
>  	.driver 	= {
>  		.owner	= THIS_MODULE,
>  		.pm	= &sh_dmae_pm,
>  		.name	= SH_DMAE_DRV_NAME,
> +		.of_match_table = sh_dmae_of_match,
>  	},
>  	.remove		= sh_dmae_remove,
>  	.shutdown	= sh_dmae_shutdown,
> diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
> index 9a93897..0815a0e 100644
> --- a/include/linux/shdma-base.h
> +++ b/include/linux/shdma-base.h
> @@ -19,6 +19,7 @@
>  #include <linux/dmaengine.h>
>  #include <linux/interrupt.h>
>  #include <linux/list.h>
> +#include <linux/of_dma.h>
>  #include <linux/types.h>
>  
>  /**
> @@ -68,6 +69,8 @@ struct shdma_chan {
>  	int id;				/* Raw id of this channel */
>  	int irq;			/* Channel IRQ */
>  	int slave_id;			/* Client ID for slave DMA */
> +	int hw_req;			/* DMA request line for slave DMA - same
> +					 * as MID/RID, used with DT */
>  	enum shdma_pm_state pm_state;
>  };
>  
> @@ -123,5 +126,7 @@ int shdma_init(struct device *dev, struct shdma_dev *sdev,
>  		    int chan_num);
>  void shdma_cleanup(struct shdma_dev *sdev);
>  bool shdma_chan_filter(struct dma_chan *chan, void *arg);
> +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> +			     struct of_dma *ofdma);
>  
>  #endif
> -- 
> 1.7.2.5
> 

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
  2013-04-15 16:54     ` Vinod Koul
@ 2013-04-15 21:18       ` Arnd Bergmann
  -1 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2013-04-15 21:18 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Guennadi Liakhovetski, linux-sh, Magnus Damm, linux-kernel,
	Guennadi Liakhovetski

On Monday 15 April 2013, Vinod Koul wrote:
> On Thu, Apr 11, 2013 at 12:19:46AM +0200, Guennadi Liakhovetski wrote:
> > This patch adds Device Tree support to the shdma driver. No special DT
> > properties are used, only standard DMA DT bindings are implemented. Since
> > shdma controllers reside on SoCs, their configuration is SoC-specific and
> > shall be passed to the driver from the SoC platform data, using the
> > auxdata procedure.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> Need Arnd to review :)

This patch is missing the DT binding document, which is necessary for a proper
review, and as a documentation for anyone trying to write device tree source
files.

In particular, it is not clear what mid/rid refer to here and whether they should
be represented as one or two cells. The driver here uses one cell, which is probably
ok, but I'd like to understand that anyway.

> > + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> > + * In that case the MID-RID value is used for slave channel filtering and is
> > + * passed to this function in the "arg" parameter.
> >   */
> >  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> >  {
> >  	struct shdma_chan *schan = to_shdma_chan(chan);
> >  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
> >  	const struct shdma_ops *ops = sdev->ops;
> > -	int slave_id = (int)arg;
> > +	int match = (int)arg;
> >  	int ret;
> >  
> > -	if (slave_id < 0)
> > +	if (match < 0)
> >  		/* No slave requested - arbitrary channel */
> >  		return true;
> >  
> > -	if (slave_id >= slave_num)
> > +	if (!schan->dev->of_node && match >= slave_num)
> >  		return false;
> >  
> > -	ret = ops->set_slave(schan, slave_id, true);
> > +	ret = ops->set_slave(schan, match, true);
> >  	if (ret < 0)
> >  		return false;

The filter function should really ensure that the dma_chan.device matches
the device passed as the argument before accessing any members of
the outer structures. This seems to be a preexisting bug.

> > +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> > +				 struct of_dma *ofdma)
> > +{
> > +	struct shdma_dev *sdev = ofdma->of_dma_data;
> > +	u32 id = dma_spec->args[0];
> > +	dma_cap_mask_t mask;
> > +	struct dma_chan *chan;
> > +
> > +	if (dma_spec->args_count != 1 || !sdev)
> > +		return NULL;
> > +
> > +	dma_cap_zero(mask);
> > +	/* Only slave DMA channels can be allocated via DT */
> > +	dma_cap_set(DMA_SLAVE, mask);
> > +
> > +	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
> > +	if (chan)
> > +		to_shdma_chan(chan)->hw_req = id;
> > +
> > +	return chan;
> > +}
> > +EXPORT_SYMBOL(shdma_xlate);

And consequently, this function should pass the sdev into the filter function
along with the id.

With a binding document added, the patch seems ok. The check for the device should
probably be added to the filter function in any case.


	Arnd

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
@ 2013-04-15 21:18       ` Arnd Bergmann
  0 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2013-04-15 21:18 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Guennadi Liakhovetski, linux-sh, Magnus Damm, linux-kernel,
	Guennadi Liakhovetski

On Monday 15 April 2013, Vinod Koul wrote:
> On Thu, Apr 11, 2013 at 12:19:46AM +0200, Guennadi Liakhovetski wrote:
> > This patch adds Device Tree support to the shdma driver. No special DT
> > properties are used, only standard DMA DT bindings are implemented. Since
> > shdma controllers reside on SoCs, their configuration is SoC-specific and
> > shall be passed to the driver from the SoC platform data, using the
> > auxdata procedure.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> Need Arnd to review :)

This patch is missing the DT binding document, which is necessary for a proper
review, and as a documentation for anyone trying to write device tree source
files.

In particular, it is not clear what mid/rid refer to here and whether they should
be represented as one or two cells. The driver here uses one cell, which is probably
ok, but I'd like to understand that anyway.

> > + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> > + * In that case the MID-RID value is used for slave channel filtering and is
> > + * passed to this function in the "arg" parameter.
> >   */
> >  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> >  {
> >  	struct shdma_chan *schan = to_shdma_chan(chan);
> >  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
> >  	const struct shdma_ops *ops = sdev->ops;
> > -	int slave_id = (int)arg;
> > +	int match = (int)arg;
> >  	int ret;
> >  
> > -	if (slave_id < 0)
> > +	if (match < 0)
> >  		/* No slave requested - arbitrary channel */
> >  		return true;
> >  
> > -	if (slave_id >= slave_num)
> > +	if (!schan->dev->of_node && match >= slave_num)
> >  		return false;
> >  
> > -	ret = ops->set_slave(schan, slave_id, true);
> > +	ret = ops->set_slave(schan, match, true);
> >  	if (ret < 0)
> >  		return false;

The filter function should really ensure that the dma_chan.device matches
the device passed as the argument before accessing any members of
the outer structures. This seems to be a preexisting bug.

> > +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> > +				 struct of_dma *ofdma)
> > +{
> > +	struct shdma_dev *sdev = ofdma->of_dma_data;
> > +	u32 id = dma_spec->args[0];
> > +	dma_cap_mask_t mask;
> > +	struct dma_chan *chan;
> > +
> > +	if (dma_spec->args_count != 1 || !sdev)
> > +		return NULL;
> > +
> > +	dma_cap_zero(mask);
> > +	/* Only slave DMA channels can be allocated via DT */
> > +	dma_cap_set(DMA_SLAVE, mask);
> > +
> > +	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
> > +	if (chan)
> > +		to_shdma_chan(chan)->hw_req = id;
> > +
> > +	return chan;
> > +}
> > +EXPORT_SYMBOL(shdma_xlate);

And consequently, this function should pass the sdev into the filter function
along with the id.

With a binding document added, the patch seems ok. The check for the device should
probably be added to the filter function in any case.


	Arnd

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
  2013-04-15 21:18       ` Arnd Bergmann
@ 2013-04-15 21:34         ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-15 21:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Vinod Koul, linux-sh, Magnus Damm, linux-kernel, Guennadi Liakhovetski

Hi Arnd

Thanks for the review.

On Mon, 15 Apr 2013, Arnd Bergmann wrote:

> On Monday 15 April 2013, Vinod Koul wrote:
> > On Thu, Apr 11, 2013 at 12:19:46AM +0200, Guennadi Liakhovetski wrote:
> > > This patch adds Device Tree support to the shdma driver. No special DT
> > > properties are used, only standard DMA DT bindings are implemented. Since
> > > shdma controllers reside on SoCs, their configuration is SoC-specific and
> > > shall be passed to the driver from the SoC platform data, using the
> > > auxdata procedure.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > > ---
> > Need Arnd to review :)
> 
> This patch is missing the DT binding document, which is necessary for a proper
> review, and as a documentation for anyone trying to write device tree source
> files.

The documentation was left off consciously, because this driver isn't 
using any non-standard DMA DT bindings, only those, already described in 
Documentation/devicetree/bindings/dma/dma.txt.

> In particular, it is not clear what mid/rid refer to here and whether they should
> be represented as one or two cells. The driver here uses one cell, which is probably
> ok, but I'd like to understand that anyway.

Yes, mid/rid is a kind of a DMA request line number on these controllers, 
IIUC. It does have some internal hardware meaning, so, it's not a plane 
index, but for a high-level view it's just an 8-bit DMA request ID.

> > > + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> > > + * In that case the MID-RID value is used for slave channel filtering and is
> > > + * passed to this function in the "arg" parameter.
> > >   */
> > >  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> > >  {
> > >  	struct shdma_chan *schan = to_shdma_chan(chan);
> > >  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
> > >  	const struct shdma_ops *ops = sdev->ops;
> > > -	int slave_id = (int)arg;
> > > +	int match = (int)arg;
> > >  	int ret;
> > >  
> > > -	if (slave_id < 0)
> > > +	if (match < 0)
> > >  		/* No slave requested - arbitrary channel */
> > >  		return true;
> > >  
> > > -	if (slave_id >= slave_num)
> > > +	if (!schan->dev->of_node && match >= slave_num)
> > >  		return false;
> > >  
> > > -	ret = ops->set_slave(schan, slave_id, true);
> > > +	ret = ops->set_slave(schan, match, true);
> > >  	if (ret < 0)
> > >  		return false;
> 
> The filter function should really ensure that the dma_chan.device matches
> the device passed as the argument before accessing any members of
> the outer structures. This seems to be a preexisting bug.

Also this I wouldn't necessarily call a bug, but a pre-existing and known 
limitation :) So far no platform, using such DMA controllers, has DMA 
controllers, driven by different dmaengine drivers, so, no confusion is 
possible, since those request line IDs are unique across SoCs. If in the 
future need arises, this limitation can certainly be lifted.

> > > +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> > > +				 struct of_dma *ofdma)
> > > +{
> > > +	struct shdma_dev *sdev = ofdma->of_dma_data;
> > > +	u32 id = dma_spec->args[0];
> > > +	dma_cap_mask_t mask;
> > > +	struct dma_chan *chan;
> > > +
> > > +	if (dma_spec->args_count != 1 || !sdev)
> > > +		return NULL;
> > > +
> > > +	dma_cap_zero(mask);
> > > +	/* Only slave DMA channels can be allocated via DT */
> > > +	dma_cap_set(DMA_SLAVE, mask);
> > > +
> > > +	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
> > > +	if (chan)
> > > +		to_shdma_chan(chan)->hw_req = id;
> > > +
> > > +	return chan;
> > > +}
> > > +EXPORT_SYMBOL(shdma_xlate);
> 
> And consequently, this function should pass the sdev into the filter function
> along with the id.
> 
> With a binding document added, the patch seems ok. The check for the device should
> probably be added to the filter function in any case.
> 
> 	Arnd

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

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
@ 2013-04-15 21:34         ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-15 21:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Vinod Koul, linux-sh, Magnus Damm, linux-kernel, Guennadi Liakhovetski

Hi Arnd

Thanks for the review.

On Mon, 15 Apr 2013, Arnd Bergmann wrote:

> On Monday 15 April 2013, Vinod Koul wrote:
> > On Thu, Apr 11, 2013 at 12:19:46AM +0200, Guennadi Liakhovetski wrote:
> > > This patch adds Device Tree support to the shdma driver. No special DT
> > > properties are used, only standard DMA DT bindings are implemented. Since
> > > shdma controllers reside on SoCs, their configuration is SoC-specific and
> > > shall be passed to the driver from the SoC platform data, using the
> > > auxdata procedure.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > > ---
> > Need Arnd to review :)
> 
> This patch is missing the DT binding document, which is necessary for a proper
> review, and as a documentation for anyone trying to write device tree source
> files.

The documentation was left off consciously, because this driver isn't 
using any non-standard DMA DT bindings, only those, already described in 
Documentation/devicetree/bindings/dma/dma.txt.

> In particular, it is not clear what mid/rid refer to here and whether they should
> be represented as one or two cells. The driver here uses one cell, which is probably
> ok, but I'd like to understand that anyway.

Yes, mid/rid is a kind of a DMA request line number on these controllers, 
IIUC. It does have some internal hardware meaning, so, it's not a plane 
index, but for a high-level view it's just an 8-bit DMA request ID.

> > > + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> > > + * In that case the MID-RID value is used for slave channel filtering and is
> > > + * passed to this function in the "arg" parameter.
> > >   */
> > >  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> > >  {
> > >  	struct shdma_chan *schan = to_shdma_chan(chan);
> > >  	struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
> > >  	const struct shdma_ops *ops = sdev->ops;
> > > -	int slave_id = (int)arg;
> > > +	int match = (int)arg;
> > >  	int ret;
> > >  
> > > -	if (slave_id < 0)
> > > +	if (match < 0)
> > >  		/* No slave requested - arbitrary channel */
> > >  		return true;
> > >  
> > > -	if (slave_id >= slave_num)
> > > +	if (!schan->dev->of_node && match >= slave_num)
> > >  		return false;
> > >  
> > > -	ret = ops->set_slave(schan, slave_id, true);
> > > +	ret = ops->set_slave(schan, match, true);
> > >  	if (ret < 0)
> > >  		return false;
> 
> The filter function should really ensure that the dma_chan.device matches
> the device passed as the argument before accessing any members of
> the outer structures. This seems to be a preexisting bug.

Also this I wouldn't necessarily call a bug, but a pre-existing and known 
limitation :) So far no platform, using such DMA controllers, has DMA 
controllers, driven by different dmaengine drivers, so, no confusion is 
possible, since those request line IDs are unique across SoCs. If in the 
future need arises, this limitation can certainly be lifted.

> > > +struct dma_chan *shdma_xlate(struct of_phandle_args *dma_spec,
> > > +				 struct of_dma *ofdma)
> > > +{
> > > +	struct shdma_dev *sdev = ofdma->of_dma_data;
> > > +	u32 id = dma_spec->args[0];
> > > +	dma_cap_mask_t mask;
> > > +	struct dma_chan *chan;
> > > +
> > > +	if (dma_spec->args_count != 1 || !sdev)
> > > +		return NULL;
> > > +
> > > +	dma_cap_zero(mask);
> > > +	/* Only slave DMA channels can be allocated via DT */
> > > +	dma_cap_set(DMA_SLAVE, mask);
> > > +
> > > +	chan = dma_request_channel(mask, shdma_chan_filter, (void *)id);
> > > +	if (chan)
> > > +		to_shdma_chan(chan)->hw_req = id;
> > > +
> > > +	return chan;
> > > +}
> > > +EXPORT_SYMBOL(shdma_xlate);
> 
> And consequently, this function should pass the sdev into the filter function
> along with the id.
> 
> With a binding document added, the patch seems ok. The check for the device should
> probably be added to the filter function in any case.
> 
> 	Arnd

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

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
  2013-04-15 21:34         ` Guennadi Liakhovetski
@ 2013-04-16 12:45           ` Arnd Bergmann
  -1 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2013-04-16 12:45 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Vinod Koul, linux-sh, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Monday 15 April 2013 23:34:57 Guennadi Liakhovetski wrote:
> 
> > 
> > This patch is missing the DT binding document, which is necessary for a proper
> > review, and as a documentation for anyone trying to write device tree source
> > files.
> 
> The documentation was left off consciously, because this driver isn't 
> using any non-standard DMA DT bindings, only those, already described in 
> Documentation/devicetree/bindings/dma/dma.txt.
>
> > In particular, it is not clear what mid/rid refer to here and whether they should
> > be represented as one or two cells. The driver here uses one cell, which is probably
> > ok, but I'd like to understand that anyway.
> 
> Yes, mid/rid is a kind of a DMA request line number on these controllers, 
> IIUC. It does have some internal hardware meaning, so, it's not a plane 
> index, but for a high-level view it's just an 8-bit DMA request ID.

Ok, this needs to be in the binding then.

You have to at least describe the meaning of the dma descriptor, because
the generic binding only defines that the descriptor contains a phandle of the
dma engine device, followed by a hardware specific number of cells to identify
a channel. The description of the device itself should mention the valid
strings for the "compatible" property and the required value of #dma-cells (<1>).

> > > > + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> > > > + * In that case the MID-RID value is used for slave channel filtering and is
> > > > + * passed to this function in the "arg" parameter.
> > > >   */
> > > >  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> > > >  {
> > > >   struct shdma_chan *schan = to_shdma_chan(chan);
> > > >   struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
> > > >   const struct shdma_ops *ops = sdev->ops;
> > > > - int slave_id = (int)arg;
> > > > + int match = (int)arg;
> > > >   int ret;
> > > >  
> > > > - if (slave_id < 0)
> > > > + if (match < 0)
> > > >           /* No slave requested - arbitrary channel */
> > > >           return true;
> > > >  
> > > > - if (slave_id >= slave_num)
> > > > + if (!schan->dev->of_node && match >= slave_num)
> > > >           return false;
> > > >  
> > > > - ret = ops->set_slave(schan, slave_id, true);
> > > > + ret = ops->set_slave(schan, match, true);
> > > >   if (ret < 0)
> > > >           return false;
> > 
> > The filter function should really ensure that the dma_chan.device matches
> > the device passed as the argument before accessing any members of
> > the outer structures. This seems to be a preexisting bug.
> 
> Also this I wouldn't necessarily call a bug, but a pre-existing and known 
> limitation  So far no platform, using such DMA controllers, has DMA 
> controllers, driven by different dmaengine drivers, so, no confusion is 
> possible, since those request line IDs are unique across SoCs. If in the 
> future need arises, this limitation can certainly be lifted.

I think it's a bit dangerous to rely on this. There are PCI add-on cards
with general-purpose DMA engines on them, so all it takes to make this
a real bug is a system with a PCI slot.

	Arnd

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

* Re: [PATCH/RFC 3/6] DMA: shdma: add DT support
@ 2013-04-16 12:45           ` Arnd Bergmann
  0 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2013-04-16 12:45 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Vinod Koul, linux-sh, Magnus Damm, linux-kernel, Guennadi Liakhovetski

On Monday 15 April 2013 23:34:57 Guennadi Liakhovetski wrote:
> 
> > 
> > This patch is missing the DT binding document, which is necessary for a proper
> > review, and as a documentation for anyone trying to write device tree source
> > files.
> 
> The documentation was left off consciously, because this driver isn't 
> using any non-standard DMA DT bindings, only those, already described in 
> Documentation/devicetree/bindings/dma/dma.txt.
>
> > In particular, it is not clear what mid/rid refer to here and whether they should
> > be represented as one or two cells. The driver here uses one cell, which is probably
> > ok, but I'd like to understand that anyway.
> 
> Yes, mid/rid is a kind of a DMA request line number on these controllers, 
> IIUC. It does have some internal hardware meaning, so, it's not a plane 
> index, but for a high-level view it's just an 8-bit DMA request ID.

Ok, this needs to be in the binding then.

You have to at least describe the meaning of the dma descriptor, because
the generic binding only defines that the descriptor contains a phandle of the
dma engine device, followed by a hardware specific number of cells to identify
a channel. The description of the device itself should mention the valid
strings for the "compatible" property and the required value of #dma-cells (<1>).

> > > > + * NOTE 2: This filter function is also used in the DT case by shdma_xlate().
> > > > + * In that case the MID-RID value is used for slave channel filtering and is
> > > > + * passed to this function in the "arg" parameter.
> > > >   */
> > > >  bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> > > >  {
> > > >   struct shdma_chan *schan = to_shdma_chan(chan);
> > > >   struct shdma_dev *sdev = to_shdma_dev(schan->dma_chan.device);
> > > >   const struct shdma_ops *ops = sdev->ops;
> > > > - int slave_id = (int)arg;
> > > > + int match = (int)arg;
> > > >   int ret;
> > > >  
> > > > - if (slave_id < 0)
> > > > + if (match < 0)
> > > >           /* No slave requested - arbitrary channel */
> > > >           return true;
> > > >  
> > > > - if (slave_id >= slave_num)
> > > > + if (!schan->dev->of_node && match >= slave_num)
> > > >           return false;
> > > >  
> > > > - ret = ops->set_slave(schan, slave_id, true);
> > > > + ret = ops->set_slave(schan, match, true);
> > > >   if (ret < 0)
> > > >           return false;
> > 
> > The filter function should really ensure that the dma_chan.device matches
> > the device passed as the argument before accessing any members of
> > the outer structures. This seems to be a preexisting bug.
> 
> Also this I wouldn't necessarily call a bug, but a pre-existing and known 
> limitation  So far no platform, using such DMA controllers, has DMA 
> controllers, driven by different dmaengine drivers, so, no confusion is 
> possible, since those request line IDs are unique across SoCs. If in the 
> future need arises, this limitation can certainly be lifted.

I think it's a bit dangerous to rely on this. There are PCI add-on cards
with general-purpose DMA engines on them, so all it takes to make this
a real bug is a system with a PCI slot.

	Arnd

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

* Re: [PATCH/RFC 0/6] DMA: shdma: add Device Tree support
  2013-04-10 22:19 ` Guennadi Liakhovetski
@ 2013-05-02  7:47   ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-05-02  7:47 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Simon Horman, Chris Ball

Hi Chris, Simon, Vinod (in alphabetical order :-) )

On Thu, 11 Apr 2013, Guennadi Liakhovetski wrote:

> This patch series adds Device Tree support to the shdma dmaengine driver 
> and illustraits its use with the kzm9g-reference board's MMCIF interface.

Formally patches in this series belong to your 3 trees: 1-3 and 7 
(documentation, sent later separately) to DMA, 4 and 6 to ARM/shmobile, 5 
to MMC. However, they can actually be safely applied in any order, as long 
as per-tree patches are kept together. Even if the DMA portion is applied 
last, the MMC driver will find DMA DT bindings in the DT, will request 
channels, but no DMA channels will be found, then the driver will fall 
back to PIO.

So, it's up to you to decide, whether to pull these patches separately via 
respective trees, or have some of them acked and push all of them via one 
tree.

Thanks
Guennadi

> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> 
> Guennadi Liakhovetski (6):
>   DMA: shdma: (cosmetic) don't re-calculate a pointer
>   DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
>   DMA: shdma: add DT support
>   ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
>   mmc: sh_mmcif: add support for Device Tree DMA bindings
>   ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
> 
>  arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 ++
>  arch/arm/boot/dts/sh73a0.dtsi                |   36 ++++++++++++++++++
>  arch/arm/mach-shmobile/setup-sh73a0.c        |    4 ++
>  drivers/dma/sh/shdma-base.c                  |   51 +++++++++++++++++++++++---
>  drivers/dma/sh/shdma.c                       |   46 ++++++++++++++++++++----
>  drivers/mmc/host/sh_mmcif.c                  |   29 +++++++++------
>  include/linux/sh_dma.h                       |    2 -
>  include/linux/shdma-base.h                   |    6 +++
>  8 files changed, 151 insertions(+), 26 deletions(-)
> 
> -- 
> 1.7.2.5
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

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

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

* Re: [PATCH/RFC 0/6] DMA: shdma: add Device Tree support
@ 2013-05-02  7:47   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 32+ messages in thread
From: Guennadi Liakhovetski @ 2013-05-02  7:47 UTC (permalink / raw)
  To: linux-sh; +Cc: Vinod Koul, Magnus Damm, linux-kernel, Simon Horman, Chris Ball

Hi Chris, Simon, Vinod (in alphabetical order :-) )

On Thu, 11 Apr 2013, Guennadi Liakhovetski wrote:

> This patch series adds Device Tree support to the shdma dmaengine driver 
> and illustraits its use with the kzm9g-reference board's MMCIF interface.

Formally patches in this series belong to your 3 trees: 1-3 and 7 
(documentation, sent later separately) to DMA, 4 and 6 to ARM/shmobile, 5 
to MMC. However, they can actually be safely applied in any order, as long 
as per-tree patches are kept together. Even if the DMA portion is applied 
last, the MMC driver will find DMA DT bindings in the DT, will request 
channels, but no DMA channels will be found, then the driver will fall 
back to PIO.

So, it's up to you to decide, whether to pull these patches separately via 
respective trees, or have some of them acked and push all of them via one 
tree.

Thanks
Guennadi

> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> 
> Guennadi Liakhovetski (6):
>   DMA: shdma: (cosmetic) don't re-calculate a pointer
>   DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
>   DMA: shdma: add DT support
>   ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
>   mmc: sh_mmcif: add support for Device Tree DMA bindings
>   ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
> 
>  arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 ++
>  arch/arm/boot/dts/sh73a0.dtsi                |   36 ++++++++++++++++++
>  arch/arm/mach-shmobile/setup-sh73a0.c        |    4 ++
>  drivers/dma/sh/shdma-base.c                  |   51 +++++++++++++++++++++++---
>  drivers/dma/sh/shdma.c                       |   46 ++++++++++++++++++++----
>  drivers/mmc/host/sh_mmcif.c                  |   29 +++++++++------
>  include/linux/sh_dma.h                       |    2 -
>  include/linux/shdma-base.h                   |    6 +++
>  8 files changed, 151 insertions(+), 26 deletions(-)
> 
> -- 
> 1.7.2.5
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

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

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

* Re: [PATCH/RFC 0/6] DMA: shdma: add Device Tree support
  2013-05-02  7:47   ` Guennadi Liakhovetski
@ 2013-05-08  0:54     ` Simon Horman
  -1 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-05-08  0:54 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Chris Ball

On Thu, May 02, 2013 at 09:47:52AM +0200, Guennadi Liakhovetski wrote:
> Hi Chris, Simon, Vinod (in alphabetical order :-) )
> 
> On Thu, 11 Apr 2013, Guennadi Liakhovetski wrote:
> 
> > This patch series adds Device Tree support to the shdma dmaengine driver 
> > and illustraits its use with the kzm9g-reference board's MMCIF interface.
> 
> Formally patches in this series belong to your 3 trees: 1-3 and 7 
> (documentation, sent later separately) to DMA, 4 and 6 to ARM/shmobile, 5 
> to MMC. However, they can actually be safely applied in any order, as long 
> as per-tree patches are kept together. Even if the DMA portion is applied 
> last, the MMC driver will find DMA DT bindings in the DT, will request 
> channels, but no DMA channels will be found, then the driver will fall 
> back to PIO.
> 
> So, it's up to you to decide, whether to pull these patches separately via 
> respective trees, or have some of them acked and push all of them via one 
> tree.

Hi Guennadi, Hi all,

I apologise for not replying earlier.

I am happy to take these changes through my renesas tree and
request they be pulled into the arm-soc tree provided that there
are acks from Vinod and Chris on the relevant patches.

I am also happy for either Chris or Vinod to take the patches.

To that end, for the two smhobile patches below:

Acked-by: Simon Horman <horms+renesas@verge.net.au>

> Thanks
> Guennadi
> 
> > Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > 
> > Guennadi Liakhovetski (6):
> >   DMA: shdma: (cosmetic) don't re-calculate a pointer
> >   DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
> >   DMA: shdma: add DT support
> >   ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
> >   mmc: sh_mmcif: add support for Device Tree DMA bindings
> >   ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
> > 
> >  arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 ++
> >  arch/arm/boot/dts/sh73a0.dtsi                |   36 ++++++++++++++++++
> >  arch/arm/mach-shmobile/setup-sh73a0.c        |    4 ++
> >  drivers/dma/sh/shdma-base.c                  |   51 +++++++++++++++++++++++---
> >  drivers/dma/sh/shdma.c                       |   46 ++++++++++++++++++++----
> >  drivers/mmc/host/sh_mmcif.c                  |   29 +++++++++------
> >  include/linux/sh_dma.h                       |    2 -
> >  include/linux/shdma-base.h                   |    6 +++
> >  8 files changed, 151 insertions(+), 26 deletions(-)
> > 
> > -- 
> > 1.7.2.5
> > 
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > 
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

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

* Re: [PATCH/RFC 0/6] DMA: shdma: add Device Tree support
@ 2013-05-08  0:54     ` Simon Horman
  0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2013-05-08  0:54 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: linux-sh, Vinod Koul, Magnus Damm, linux-kernel, Chris Ball

On Thu, May 02, 2013 at 09:47:52AM +0200, Guennadi Liakhovetski wrote:
> Hi Chris, Simon, Vinod (in alphabetical order :-) )
> 
> On Thu, 11 Apr 2013, Guennadi Liakhovetski wrote:
> 
> > This patch series adds Device Tree support to the shdma dmaengine driver 
> > and illustraits its use with the kzm9g-reference board's MMCIF interface.
> 
> Formally patches in this series belong to your 3 trees: 1-3 and 7 
> (documentation, sent later separately) to DMA, 4 and 6 to ARM/shmobile, 5 
> to MMC. However, they can actually be safely applied in any order, as long 
> as per-tree patches are kept together. Even if the DMA portion is applied 
> last, the MMC driver will find DMA DT bindings in the DT, will request 
> channels, but no DMA channels will be found, then the driver will fall 
> back to PIO.
> 
> So, it's up to you to decide, whether to pull these patches separately via 
> respective trees, or have some of them acked and push all of them via one 
> tree.

Hi Guennadi, Hi all,

I apologise for not replying earlier.

I am happy to take these changes through my renesas tree and
request they be pulled into the arm-soc tree provided that there
are acks from Vinod and Chris on the relevant patches.

I am also happy for either Chris or Vinod to take the patches.

To that end, for the two smhobile patches below:

Acked-by: Simon Horman <horms+renesas@verge.net.au>

> Thanks
> Guennadi
> 
> > Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > 
> > Guennadi Liakhovetski (6):
> >   DMA: shdma: (cosmetic) don't re-calculate a pointer
> >   DMA: shdma: shdma_chan_filter() has to be in shdma-base.h
> >   DMA: shdma: add DT support
> >   ARM: shmobile: sh73a0: add support for the DMA0 controller in DT
> >   mmc: sh_mmcif: add support for Device Tree DMA bindings
> >   ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT
> > 
> >  arch/arm/boot/dts/sh73a0-kzm9g-reference.dts |    3 ++
> >  arch/arm/boot/dts/sh73a0.dtsi                |   36 ++++++++++++++++++
> >  arch/arm/mach-shmobile/setup-sh73a0.c        |    4 ++
> >  drivers/dma/sh/shdma-base.c                  |   51 +++++++++++++++++++++++---
> >  drivers/dma/sh/shdma.c                       |   46 ++++++++++++++++++++----
> >  drivers/mmc/host/sh_mmcif.c                  |   29 +++++++++------
> >  include/linux/sh_dma.h                       |    2 -
> >  include/linux/shdma-base.h                   |    6 +++
> >  8 files changed, 151 insertions(+), 26 deletions(-)
> > 
> > -- 
> > 1.7.2.5
> > 
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > 
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

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

end of thread, other threads:[~2013-05-08  4:00 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-10 22:19 [PATCH/RFC 0/6] DMA: shdma: add Device Tree support Guennadi Liakhovetski
2013-04-10 22:19 ` Guennadi Liakhovetski
2013-04-10 22:19 ` [PATCH 1/6] DMA: shdma: (cosmetic) don't re-calculate a pointer Guennadi Liakhovetski
2013-04-10 22:19   ` Guennadi Liakhovetski
2013-04-10 22:19 ` [PATCH 2/6] DMA: shdma: shdma_chan_filter() has to be in shdma-base.h Guennadi Liakhovetski
2013-04-10 22:19   ` Guennadi Liakhovetski
2013-04-10 22:19 ` [PATCH/RFC 3/6] DMA: shdma: add DT support Guennadi Liakhovetski
2013-04-10 22:19   ` Guennadi Liakhovetski
2013-04-15 16:42   ` Vinod Koul
2013-04-15 16:54     ` Vinod Koul
2013-04-15 21:18     ` Arnd Bergmann
2013-04-15 21:18       ` Arnd Bergmann
2013-04-15 21:34       ` Guennadi Liakhovetski
2013-04-15 21:34         ` Guennadi Liakhovetski
2013-04-16 12:45         ` Arnd Bergmann
2013-04-16 12:45           ` Arnd Bergmann
2013-04-10 22:19 ` [PATCH/RFC 4/6] ARM: shmobile: sh73a0: add support for the DMA0 controller in DT Guennadi Liakhovetski
2013-04-10 22:19   ` Guennadi Liakhovetski
2013-04-11  3:01   ` Simon Horman
2013-04-11  3:01     ` Simon Horman
2013-04-11  3:02     ` Simon Horman
2013-04-11  3:02       ` Simon Horman
2013-04-10 22:19 ` [PATCH/RFC 5/6] mmc: sh_mmcif: add support for Device Tree DMA bindings Guennadi Liakhovetski
2013-04-10 22:19   ` Guennadi Liakhovetski
2013-04-10 22:19 ` [PATCH/RFC 6/6] ARM: shmobile: kzm9g-reference: add DMA channels to the MMCIF DT Guennadi Liakhovetski
2013-04-10 22:19   ` Guennadi Liakhovetski
2013-04-11  3:00   ` Simon Horman
2013-04-11  3:00     ` Simon Horman
2013-05-02  7:47 ` [PATCH/RFC 0/6] DMA: shdma: add Device Tree support Guennadi Liakhovetski
2013-05-02  7:47   ` Guennadi Liakhovetski
2013-05-08  0:54   ` Simon Horman
2013-05-08  0:54     ` Simon Horman

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.