[PATCH 0/2] dma: imx-sdma: add generic DMA device tree bindings support

[PATCH 0/2] dma: imx-sdma: add generic DMA device tree bindings support

[Shawn Guo]

The series adds add generic DMA device tree bindings support for
imx-sdma.  The legacy DMA requesting support is still there and will
be removed after all clients get converted to generic DMA DT helper.

Shawn Guo (2):
  dma: imx: change enum definitions to macros
  dma: imx-sdma: move to generic device tree bindings

 .../devicetree/bindings/dma/fsl-imx-sdma.txt       |   22 ++++++++++
 drivers/dma/imx-sdma.c                             |   44 +++++++++++++++++++-
 include/dt-bindings/dma/imx.h                      |   29 +++++++++++++
 include/linux/platform_data/dma-imx.h              |   37 +---------------
 4 files changed, 95 insertions(+), 37 deletions(-)
 create mode 100644 include/dt-bindings/dma/imx.h

-- 
1.7.9.5

[PATCH 1/2] dma: imx: change enum definitions to macros

[Shawn Guo]

Change enum sdma_peripheral_type and imx_dma_prio to macro definitions
for easing the migration to generic DMA device tree bindings later.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/dma/imx-sdma.c                |    4 +--
 include/linux/platform_data/dma-imx.h |   56 +++++++++++++++------------------
 2 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 092867b..e57a0c6 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -249,7 +249,7 @@ struct sdma_channel {
 	struct sdma_engine		*sdma;
 	unsigned int			channel;
 	enum dma_transfer_direction		direction;
-	enum sdma_peripheral_type	peripheral_type;
+	int				peripheral_type;
 	unsigned int			event_id0;
 	unsigned int			event_id1;
 	enum dma_slave_buswidth		word_size;
@@ -580,7 +580,7 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
  * sets the pc of SDMA script according to the peripheral type
  */
 static void sdma_get_pc(struct sdma_channel *sdmac,
-		enum sdma_peripheral_type peripheral_type)
+		int peripheral_type)
 {
 	struct sdma_engine *sdma = sdmac->sdma;
 	int per_2_emi = 0, emi_2_per = 0;
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index f6d30cc..8a8a684 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -16,40 +16,36 @@
 /*
  * This enumerates peripheral types. Used for SDMA.
  */
-enum sdma_peripheral_type {
-	IMX_DMATYPE_SSI,	/* MCU domain SSI */
-	IMX_DMATYPE_SSI_SP,	/* Shared SSI */
-	IMX_DMATYPE_MMC,	/* MMC */
-	IMX_DMATYPE_SDHC,	/* SDHC */
-	IMX_DMATYPE_UART,	/* MCU domain UART */
-	IMX_DMATYPE_UART_SP,	/* Shared UART */
-	IMX_DMATYPE_FIRI,	/* FIRI */
-	IMX_DMATYPE_CSPI,	/* MCU domain CSPI */
-	IMX_DMATYPE_CSPI_SP,	/* Shared CSPI */
-	IMX_DMATYPE_SIM,	/* SIM */
-	IMX_DMATYPE_ATA,	/* ATA */
-	IMX_DMATYPE_CCM,	/* CCM */
-	IMX_DMATYPE_EXT,	/* External peripheral */
-	IMX_DMATYPE_MSHC,	/* Memory Stick Host Controller */
-	IMX_DMATYPE_MSHC_SP,	/* Shared Memory Stick Host Controller */
-	IMX_DMATYPE_DSP,	/* DSP */
-	IMX_DMATYPE_MEMORY,	/* Memory */
-	IMX_DMATYPE_FIFO_MEMORY,/* FIFO type Memory */
-	IMX_DMATYPE_SPDIF,	/* SPDIF */
-	IMX_DMATYPE_IPU_MEMORY,	/* IPU Memory */
-	IMX_DMATYPE_ASRC,	/* ASRC */
-	IMX_DMATYPE_ESAI,	/* ESAI */
-};
+#define IMX_DMATYPE_SSI		0  /* MCU domain SSI */
+#define IMX_DMATYPE_SSI_SP	1  /* Shared SSI */
+#define IMX_DMATYPE_MMC		2  /* MMC */
+#define IMX_DMATYPE_SDHC	3  /* SDHC */
+#define IMX_DMATYPE_UART	4  /* MCU domain UART */
+#define IMX_DMATYPE_UART_SP	5  /* Shared UART */
+#define IMX_DMATYPE_FIRI	6  /* FIRI */
+#define IMX_DMATYPE_CSPI	7  /* MCU domain CSPI */
+#define IMX_DMATYPE_CSPI_SP	8  /* Shared CSPI */
+#define IMX_DMATYPE_SIM		9  /* SIM */
+#define IMX_DMATYPE_ATA		10 /* ATA */
+#define IMX_DMATYPE_CCM		11 /* CCM */
+#define IMX_DMATYPE_EXT		12 /* External peripheral */
+#define IMX_DMATYPE_MSHC	13 /* Memory Stick Host Controller */
+#define IMX_DMATYPE_MSHC_SP	14 /* Shared Memory Stick Host Controller */
+#define IMX_DMATYPE_DSP		15 /* DSP */
+#define IMX_DMATYPE_MEMORY	16 /* Memory */
+#define IMX_DMATYPE_FIFO_MEMORY	17 /* FIFO type Memory */
+#define IMX_DMATYPE_SPDIF	18 /* SPDIF */
+#define IMX_DMATYPE_IPU_MEMORY	19 /* IPU Memory */
+#define IMX_DMATYPE_ASRC	20 /* ASRC */
+#define IMX_DMATYPE_ESAI	21 /* ESAI */
 
-enum imx_dma_prio {
-	DMA_PRIO_HIGH = 0,
-	DMA_PRIO_MEDIUM = 1,
-	DMA_PRIO_LOW = 2
-};
+#define DMA_PRIO_HIGH		0
+#define DMA_PRIO_MEDIUM		1
+#define DMA_PRIO_LOW		2
 
 struct imx_dma_data {
 	int dma_request; /* DMA request line */
-	enum sdma_peripheral_type peripheral_type;
+	int peripheral_type;
 	int priority;
 };
 
-- 
1.7.9.5

[PATCH 2/2] dma: imx-sdma: move to generic device tree bindings

[Shawn Guo]

Update imx-sdma driver to adopt generic DMA device tree bindings.  It
calls of_dma_controller_register() with imx-sdma specific of_dma_xlate
to get the generic DMA device tree helper support.  The #dma-cells for
imx-sdma must be 3, which includes request ID, peripheral type and
priority.

The definitions of peripheral type and priority get moved into
include/dt-bindings/dma/imx.h, so that the macros can also be used in
dts files.

The existing way of requesting channel, clients directly call
dma_request_channel(), still work there, and will be removed after
all imx-sdma clients get converted to generic DMA device tree helper.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 .../devicetree/bindings/dma/fsl-imx-sdma.txt       |   22 +++++++++++
 drivers/dma/imx-sdma.c                             |   40 ++++++++++++++++++++
 include/dt-bindings/dma/imx.h                      |   29 ++++++++++++++
 include/linux/platform_data/dma-imx.h              |   31 +--------------
 4 files changed, 92 insertions(+), 30 deletions(-)
 create mode 100644 include/dt-bindings/dma/imx.h

diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index d1e3f44..053e20e 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -4,6 +4,11 @@ Required properties:
 - compatible : Should be "fsl,<chip>-sdma"
 - reg : Should contain SDMA registers location and length
 - interrupts : Should contain SDMA interrupt
+- #dma-cells : Must be <3>.
+  The first cell specifies the DMA request/event ID.  The second and
+  third cell specifies the peripheral type and priority of DMA transfer
+  respectively.  Refer to include/dt-bindings/dma/imx.h for available
+  peripheral types and priorities.
 - fsl,sdma-ram-script-name : Should contain the full path of SDMA RAM
   scripts firmware
 
@@ -13,5 +18,22 @@ sdma at 83fb0000 {
 	compatible = "fsl,imx51-sdma", "fsl,imx35-sdma";
 	reg = <0x83fb0000 0x4000>;
 	interrupts = <6>;
+	#dma-cells = <3>;
 	fsl,sdma-ram-script-name = "sdma-imx51.bin";
 };
+
+DMA clients connected to the i.MX SDMA controller must use the format
+described in the dma.txt file.
+
+Examples:
+
+ssi2: ssi at 70014000 {
+	compatible = "fsl,imx51-ssi", "fsl,imx21-ssi";
+	reg = <0x70014000 0x4000>;
+	interrupts = <30>;
+	clocks = <&clks 49>;
+	dmas = <&sdma 24 DMA_PRIO_HIGH IMX_DMATYPE_SSI_SP>,
+	       <&sdma 25 DMA_PRIO_HIGH IMX_DMATYPE_SSI_SP>;
+	dma-names = "rx", "tx";
+	fsl,fifo-depth = <15>;
+};
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index e57a0c6..f1324da 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -36,6 +36,7 @@
 #include <linux/dmaengine.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_dma.h>
 
 #include <asm/irq.h>
 #include <linux/platform_data/dma-imx-sdma.h>
@@ -1296,6 +1297,35 @@ err_dma_alloc:
 	return ret;
 }
 
+static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
+{
+	struct imx_dma_data *data = fn_param;
+
+	if (!imx_dma_is_general_purpose(chan))
+		return false;
+
+	chan->private = data;
+
+	return true;
+}
+
+struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
+			    struct of_dma *ofdma)
+{
+	struct sdma_engine *sdma = ofdma->of_dma_data;
+	dma_cap_mask_t mask = sdma->dma_device.cap_mask;
+	struct imx_dma_data data;
+
+	if (dma_spec->args_count != 3)
+		return NULL;
+
+	data.dma_request = dma_spec->args[0];
+	data.peripheral_type = dma_spec->args[1];
+	data.priority = dma_spec->args[2];
+
+	return dma_request_channel(mask, sdma_filter_fn, &data);
+}
+
 static int __init sdma_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id =
@@ -1443,10 +1473,20 @@ static int __init sdma_probe(struct platform_device *pdev)
 		goto err_init;
 	}
 
+	if (np) {
+		ret = of_dma_controller_register(np, sdma_xlate, sdma);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to register controller\n");
+			goto err_register;
+		}
+	}
+
 	dev_info(sdma->dev, "initialized\n");
 
 	return 0;
 
+err_register:
+	dma_async_device_unregister(&sdma->dma_device);
 err_init:
 	kfree(sdma->script_addrs);
 err_alloc:
diff --git a/include/dt-bindings/dma/imx.h b/include/dt-bindings/dma/imx.h
new file mode 100644
index 0000000..d5de68c
--- /dev/null
+++ b/include/dt-bindings/dma/imx.h
@@ -0,0 +1,29 @@
+/*
+ * This enumerates peripheral types. Used for SDMA.
+ */
+#define IMX_DMATYPE_SSI		0  /* MCU domain SSI */
+#define IMX_DMATYPE_SSI_SP	1  /* Shared SSI */
+#define IMX_DMATYPE_MMC		2  /* MMC */
+#define IMX_DMATYPE_SDHC	3  /* SDHC */
+#define IMX_DMATYPE_UART	4  /* MCU domain UART */
+#define IMX_DMATYPE_UART_SP	5  /* Shared UART */
+#define IMX_DMATYPE_FIRI	6  /* FIRI */
+#define IMX_DMATYPE_CSPI	7  /* MCU domain CSPI */
+#define IMX_DMATYPE_CSPI_SP	8  /* Shared CSPI */
+#define IMX_DMATYPE_SIM		9  /* SIM */
+#define IMX_DMATYPE_ATA		10 /* ATA */
+#define IMX_DMATYPE_CCM		11 /* CCM */
+#define IMX_DMATYPE_EXT		12 /* External peripheral */
+#define IMX_DMATYPE_MSHC	13 /* Memory Stick Host Controller */
+#define IMX_DMATYPE_MSHC_SP	14 /* Shared Memory Stick Host Controller */
+#define IMX_DMATYPE_DSP		15 /* DSP */
+#define IMX_DMATYPE_MEMORY	16 /* Memory */
+#define IMX_DMATYPE_FIFO_MEMORY	17 /* FIFO type Memory */
+#define IMX_DMATYPE_SPDIF	18 /* SPDIF */
+#define IMX_DMATYPE_IPU_MEMORY	19 /* IPU Memory */
+#define IMX_DMATYPE_ASRC	20 /* ASRC */
+#define IMX_DMATYPE_ESAI	21 /* ESAI */
+
+#define DMA_PRIO_HIGH		0
+#define DMA_PRIO_MEDIUM		1
+#define DMA_PRIO_LOW		2
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index 8a8a684..33f7eda 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -12,36 +12,7 @@
 #include <linux/scatterlist.h>
 #include <linux/device.h>
 #include <linux/dmaengine.h>
-
-/*
- * This enumerates peripheral types. Used for SDMA.
- */
-#define IMX_DMATYPE_SSI		0  /* MCU domain SSI */
-#define IMX_DMATYPE_SSI_SP	1  /* Shared SSI */
-#define IMX_DMATYPE_MMC		2  /* MMC */
-#define IMX_DMATYPE_SDHC	3  /* SDHC */
-#define IMX_DMATYPE_UART	4  /* MCU domain UART */
-#define IMX_DMATYPE_UART_SP	5  /* Shared UART */
-#define IMX_DMATYPE_FIRI	6  /* FIRI */
-#define IMX_DMATYPE_CSPI	7  /* MCU domain CSPI */
-#define IMX_DMATYPE_CSPI_SP	8  /* Shared CSPI */
-#define IMX_DMATYPE_SIM		9  /* SIM */
-#define IMX_DMATYPE_ATA		10 /* ATA */
-#define IMX_DMATYPE_CCM		11 /* CCM */
-#define IMX_DMATYPE_EXT		12 /* External peripheral */
-#define IMX_DMATYPE_MSHC	13 /* Memory Stick Host Controller */
-#define IMX_DMATYPE_MSHC_SP	14 /* Shared Memory Stick Host Controller */
-#define IMX_DMATYPE_DSP		15 /* DSP */
-#define IMX_DMATYPE_MEMORY	16 /* Memory */
-#define IMX_DMATYPE_FIFO_MEMORY	17 /* FIFO type Memory */
-#define IMX_DMATYPE_SPDIF	18 /* SPDIF */
-#define IMX_DMATYPE_IPU_MEMORY	19 /* IPU Memory */
-#define IMX_DMATYPE_ASRC	20 /* ASRC */
-#define IMX_DMATYPE_ESAI	21 /* ESAI */
-
-#define DMA_PRIO_HIGH		0
-#define DMA_PRIO_MEDIUM		1
-#define DMA_PRIO_LOW		2
+#include <dt-bindings/dma/imx.h>
 
 struct imx_dma_data {
 	int dma_request; /* DMA request line */
-- 
1.7.9.5

[PATCH 2/2] dma: imx-sdma: move to generic device tree bindings

[Fabio Estevam]

On Wed, Apr 24, 2013 at 11:04 AM, Shawn Guo <shawn.guo@linaro.org> wrote:

> +ssi2: ssi at 70014000 {
> +       compatible = "fsl,imx51-ssi", "fsl,imx21-ssi";
> +       reg = <0x70014000 0x4000>;
> +       interrupts = <30>;
> +       clocks = <&clks 49>;
> +       dmas = <&sdma 24 DMA_PRIO_HIGH IMX_DMATYPE_SSI_SP>,
> +              <&sdma 25 DMA_PRIO_HIGH IMX_DMATYPE_SSI_SP>;

The document says:
"third cell specifies the peripheral type and priority of DMA transfer
respectively"

,but what I see in this example is the opposite:

priority of DMA and then peripheral type.

[PATCH 2/2] dma: imx-sdma: move to generic device tree bindings

[Arnd Bergmann]

On Wednesday 24 April 2013, Shawn Guo wrote:
> +/*
> + * This enumerates peripheral types. Used for SDMA.
> + */
> +#define IMX_DMATYPE_SSI                0  /* MCU domain SSI */
> +#define IMX_DMATYPE_SSI_SP     1  /* Shared SSI */
> +#define IMX_DMATYPE_MMC                2  /* MMC */
> +#define IMX_DMATYPE_SDHC       3  /* SDHC */
> +#define IMX_DMATYPE_UART       4  /* MCU domain UART */
> +#define IMX_DMATYPE_UART_SP    5  /* Shared UART */
> +#define IMX_DMATYPE_FIRI       6  /* FIRI */
> +#define IMX_DMATYPE_CSPI       7  /* MCU domain CSPI */
> +#define IMX_DMATYPE_CSPI_SP    8  /* Shared CSPI */
> +#define IMX_DMATYPE_SIM                9  /* SIM */
> +#define IMX_DMATYPE_ATA                10 /* ATA */
> +#define IMX_DMATYPE_CCM                11 /* CCM */
> +#define IMX_DMATYPE_EXT                12 /* External peripheral */
> +#define IMX_DMATYPE_MSHC       13 /* Memory Stick Host Controller */
> +#define IMX_DMATYPE_MSHC_SP    14 /* Shared Memory Stick Host Controller */
> +#define IMX_DMATYPE_DSP                15 /* DSP */
> +#define IMX_DMATYPE_MEMORY     16 /* Memory */
> +#define IMX_DMATYPE_FIFO_MEMORY        17 /* FIFO type Memory */
> +#define IMX_DMATYPE_SPDIF      18 /* SPDIF */
> +#define IMX_DMATYPE_IPU_MEMORY 19 /* IPU Memory */
> +#define IMX_DMATYPE_ASRC       20 /* ASRC */
> +#define IMX_DMATYPE_ESAI       21 /* ESAI */

These macros don't seem helpful to me, since they are used in only one
place each. Why not put the literal numbers into the dts file directly?

	Arnd

[PATCH 2/2] dma: imx-sdma: move to generic device tree bindings

[Shawn Guo]

On Wed, Apr 24, 2013 at 11:23:04AM -0300, Fabio Estevam wrote:
> On Wed, Apr 24, 2013 at 11:04 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> 
> > +ssi2: ssi at 70014000 {
> > +       compatible = "fsl,imx51-ssi", "fsl,imx21-ssi";
> > +       reg = <0x70014000 0x4000>;
> > +       interrupts = <30>;
> > +       clocks = <&clks 49>;
> > +       dmas = <&sdma 24 DMA_PRIO_HIGH IMX_DMATYPE_SSI_SP>,
> > +              <&sdma 25 DMA_PRIO_HIGH IMX_DMATYPE_SSI_SP>;
> 
> The document says:
> "third cell specifies the peripheral type and priority of DMA transfer
> respectively"
> 
> ,but what I see in this example is the opposite:
> 
> priority of DMA and then peripheral type.

Ah, yes.  Will fix it.  Thanks.

Shawn

[PATCH 2/2] dma: imx-sdma: move to generic device tree bindings

[Shawn Guo]

On Wed, Apr 24, 2013 at 04:26:22PM +0200, Arnd Bergmann wrote:
> On Wednesday 24 April 2013, Shawn Guo wrote:
> > +/*
> > + * This enumerates peripheral types. Used for SDMA.
> > + */
> > +#define IMX_DMATYPE_SSI                0  /* MCU domain SSI */
> > +#define IMX_DMATYPE_SSI_SP     1  /* Shared SSI */
> > +#define IMX_DMATYPE_MMC                2  /* MMC */
> > +#define IMX_DMATYPE_SDHC       3  /* SDHC */
> > +#define IMX_DMATYPE_UART       4  /* MCU domain UART */
> > +#define IMX_DMATYPE_UART_SP    5  /* Shared UART */
> > +#define IMX_DMATYPE_FIRI       6  /* FIRI */
> > +#define IMX_DMATYPE_CSPI       7  /* MCU domain CSPI */
> > +#define IMX_DMATYPE_CSPI_SP    8  /* Shared CSPI */
> > +#define IMX_DMATYPE_SIM                9  /* SIM */
> > +#define IMX_DMATYPE_ATA                10 /* ATA */
> > +#define IMX_DMATYPE_CCM                11 /* CCM */
> > +#define IMX_DMATYPE_EXT                12 /* External peripheral */
> > +#define IMX_DMATYPE_MSHC       13 /* Memory Stick Host Controller */
> > +#define IMX_DMATYPE_MSHC_SP    14 /* Shared Memory Stick Host Controller */
> > +#define IMX_DMATYPE_DSP                15 /* DSP */
> > +#define IMX_DMATYPE_MEMORY     16 /* Memory */
> > +#define IMX_DMATYPE_FIFO_MEMORY        17 /* FIFO type Memory */
> > +#define IMX_DMATYPE_SPDIF      18 /* SPDIF */
> > +#define IMX_DMATYPE_IPU_MEMORY 19 /* IPU Memory */
> > +#define IMX_DMATYPE_ASRC       20 /* ASRC */
> > +#define IMX_DMATYPE_ESAI       21 /* ESAI */
> 
> These macros don't seem helpful to me, since they are used in only one
> place each. Why not put the literal numbers into the dts file directly?

Using macros will help us to:

1) Improve the readability of "dmas" property in dts
2) Keep the values stay in sync between kernel and DT

Shawn

[PATCH 2/2] dma: imx-sdma: move to generic device tree bindings

[Arnd Bergmann]

On Wednesday 24 April 2013, Shawn Guo wrote:
> Using macros will help us to:
> 
> 1) Improve the readability of "dmas" property in dts
> 2) Keep the values stay in sync between kernel and DT
> 

I can understand the second part, but I strongly disagree on the first one.
Using symbolic constants in the DT will make it harder to understand because
you now have to look in multiple places to see what you actually put in there.

It makes some sense for the priorities, since they get reused for each
dma specifier.

	Arnd

[PATCH 2/2] dma: imx-sdma: move to generic device tree bindings

[Shawn Guo]

On Wed, Apr 24, 2013 at 04:52:48PM +0200, Arnd Bergmann wrote:
> On Wednesday 24 April 2013, Shawn Guo wrote:
> > Using macros will help us to:
> > 
> > 1) Improve the readability of "dmas" property in dts
> > 2) Keep the values stay in sync between kernel and DT
> > 
> 
> I can understand the second part, but I strongly disagree on the first one.
> Using symbolic constants in the DT will make it harder to understand because
> you now have to look in multiple places to see what you actually put in there.

So for example below, I did not use macro for the first cell which is
the DMA request ID, because it's unique for each peripheral.  But
IMX_DMATYPE_SSI_SP will be applied on all 3 SSI controllers on imx51
for 6 times (rx and tx).

	dmas = <&sdma 24 IMX_DMATYPE_SSI_SP DMA_PRIO_HIGH>,
	       <&sdma 25 IMX_DMATYPE_SSI_SP DMA_PRIO_HIGH>;
	dma-names = "rx", "tx";

Shawn

> It makes some sense for the priorities, since they get reused for each
> dma specifier.