[PATCH v2 0/3] dmaengine: mpc512x: add device tree binding document and DMA channel lookup

[PATCH v2 0/3] dmaengine: mpc512x: add device tree binding document and DMA channel lookup

[Alexander Popov]

This patch series introduces a device tree binding document for
the MPC512x DMA controller and adds device tree based DMA channel lookup
for it.

This version has improved device tree binding document.

Alexander Popov (3):
  dmaengine: mpc512x: add device tree binding document
  dmaengine: of: add common xlate function for matching by channel id
  dmaengine: mpc512x: register for device tree channel lookup

 .../devicetree/bindings/dma/mpc512x-dma.txt        | 31 +++++++++++++++++++
 arch/powerpc/boot/dts/mpc5121.dtsi                 |  1 +
 drivers/dma/mpc512x_dma.c                          | 13 +++++++-
 drivers/dma/of-dma.c                               | 35 ++++++++++++++++++++++
 include/linux/of_dma.h                             |  4 +++
 5 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt

-- 
1.8.4.2

[PATCH v2 1/3] dmaengine: mpc512x: add device tree binding document

[Alexander Popov]

Introduce a device tree binding document for the MPC512x DMA controller

Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
 .../devicetree/bindings/dma/mpc512x-dma.txt        | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt

diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
new file mode 100644
index 0000000..95e2ca0
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,31 @@
+* Freescale MPC512x and MPC8308 DMA Controller
+
+The DMA controller in Freescale MPC512x and MPC8308 SoCs can move
+blocks of memory contents between memory and peripherals or
+from memory to memory.
+
+Refer to "Generic DMA Controller and DMA request bindings" in
+the dma/dma.txt file for a more detailed description of binding.
+
+Required properties:
+- compatible: should be "fsl,mpc5121-dma" or "fsl,mpc8308-dma";
+- reg: should contain the DMA controller registers location and length;
+- interrupt for the DMA controller: syntax of interrupt client node
+	is described in interrupt-controller/interrupts.txt file.
+
+Optional properties:
+- #dma-cells: the length of the DMA specifier, must be <1>.
+	Each channel of this DMA controller has a peripheral request line,
+	the assignment is fixed in hardware. This one cell
+	in dmas property of a client device represents the channel number.
+
+Example:
+
+	dma0: dma@14000 {
+		compatible = "fsl,mpc5121-dma";
+		reg = <0x14000 0x1800>;
+		interrupts = <65 0x8>;
+		#dma-cells = <1>;
+	};
+
+DMA clients must use the format described in dma/dma.txt file.
-- 
1.8.4.2

Re: [PATCH v2 1/3] dmaengine: mpc512x: add device tree binding document

[Mark Rutland]

On Wed, Jun 18, 2014 at 11:48:10AM +0100, Alexander Popov wrote:
> Introduce a device tree binding document for the MPC512x DMA controller
> 
> Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
> ---
>  .../devicetree/bindings/dma/mpc512x-dma.txt        | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt
> 
> diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
> new file mode 100644
> index 0000000..95e2ca0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
> @@ -0,0 +1,31 @@
> +* Freescale MPC512x and MPC8308 DMA Controller
> +
> +The DMA controller in Freescale MPC512x and MPC8308 SoCs can move
> +blocks of memory contents between memory and peripherals or
> +from memory to memory.
> +
> +Refer to "Generic DMA Controller and DMA request bindings" in
> +the dma/dma.txt file for a more detailed description of binding.
> +
> +Required properties:
> +- compatible: should be "fsl,mpc5121-dma" or "fsl,mpc8308-dma";
> +- reg: should contain the DMA controller registers location and length;
> +- interrupt for the DMA controller: syntax of interrupt client node
> +	is described in interrupt-controller/interrupts.txt file.
> +
> +Optional properties:
> +- #dma-cells: the length of the DMA specifier, must be <1>.
> +	Each channel of this DMA controller has a peripheral request line,
> +	the assignment is fixed in hardware. This one cell
> +	in dmas property of a client device represents the channel number.

Surely this is required to be able to refer to DMA channels on the
device?

Mark.

Re: [PATCH v2 1/3] dmaengine: mpc512x: add device tree binding document

[Alexander Popov]

Hello Mark, thanks for your reply!

2014-06-18 17:37 GMT+04:00 Mark Rutland <mark.rutland@arm.com>:
> On Wed, Jun 18, 2014 at 11:48:10AM +0100, Alexander Popov wrote:
>> Introduce a device tree binding document for the MPC512x DMA controller
>>
>> Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
>> ---
>>  .../devicetree/bindings/dma/mpc512x-dma.txt        | 31 ++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt
>>
>> diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
>> new file mode 100644
>> index 0000000..95e2ca0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
>> @@ -0,0 +1,31 @@
>> +* Freescale MPC512x and MPC8308 DMA Controller
>> +
>> +The DMA controller in Freescale MPC512x and MPC8308 SoCs can move
>> +blocks of memory contents between memory and peripherals or
>> +from memory to memory.
>> +
>> +Refer to "Generic DMA Controller and DMA request bindings" in
>> +the dma/dma.txt file for a more detailed description of binding.
>> +
>> +Required properties:
>> +- compatible: should be "fsl,mpc5121-dma" or "fsl,mpc8308-dma";
>> +- reg: should contain the DMA controller registers location and length;
>> +- interrupt for the DMA controller: syntax of interrupt client node
>> +     is described in interrupt-controller/interrupts.txt file.
>> +
>> +Optional properties:
>> +- #dma-cells: the length of the DMA specifier, must be <1>.
>> +     Each channel of this DMA controller has a peripheral request line,
>> +     the assignment is fixed in hardware. This one cell
>> +     in dmas property of a client device represents the channel number.
>
> Surely this is required to be able to refer to DMA channels on the
> device?

Excuse me, I didn't understand your question.
Do you inquire about the reason of making #dma-cells an optional property?
It's optional because device tree based lookup support is made
optional (part 3/3).

Best regards,
Alexander

Re: [PATCH v2 1/3] dmaengine: mpc512x: add device tree binding document

[Alexander Popov]

2014-06-18 18:56 GMT+04:00 Alexander Popov <a13xp0p0v88@gmail.com>:
> 2014-06-18 17:37 GMT+04:00 Mark Rutland <mark.rutland@arm.com>:
>> On Wed, Jun 18, 2014 at 11:48:10AM +0100, Alexander Popov wrote:
>>> Introduce a device tree binding document for the MPC512x DMA controller
>>> +Optional properties:
>>> +- #dma-cells: the length of the DMA specifier, must be <1>.
>>> +     Each channel of this DMA controller has a peripheral request line,
>>> +     the assignment is fixed in hardware. This one cell
>>> +     in dmas property of a client device represents the channel number.
>>
>> Surely this is required to be able to refer to DMA channels on the
>> device?
>
> Excuse me, I didn't understand your question.
> Do you inquire about the reason of making #dma-cells an optional property?
> It's optional because device tree based lookup support is made
> optional (part 3/3).

Mark, did I answer your question?
Should I fix anything in this patch series?

Hope for the reply.
Alexander

Re: [PATCH v2 1/3] dmaengine: mpc512x: add device tree binding document

[Mark Rutland]

Hi Alexander,

Apologies for the late reply. DT-related email is somewhat a firehose
and unfortunately I lose track of things.

On Thu, Jun 19, 2014 at 02:59:19PM +0100, Alexander Popov wrote:
> 2014-06-18 18:56 GMT+04:00 Alexander Popov <a13xp0p0v88@gmail.com>:
> > 2014-06-18 17:37 GMT+04:00 Mark Rutland <mark.rutland@arm.com>:
> >> On Wed, Jun 18, 2014 at 11:48:10AM +0100, Alexander Popov wrote:
> >>> Introduce a device tree binding document for the MPC512x DMA controller
> >>> +Optional properties:
> >>> +- #dma-cells: the length of the DMA specifier, must be <1>.
> >>> +     Each channel of this DMA controller has a peripheral request line,
> >>> +     the assignment is fixed in hardware. This one cell
> >>> +     in dmas property of a client device represents the channel number.
> >>
> >> Surely this is required to be able to refer to DMA channels on the
> >> device?
> >
> > Excuse me, I didn't understand your question.
> > Do you inquire about the reason of making #dma-cells an optional property?
> > It's optional because device tree based lookup support is made
> > optional (part 3/3).
> 
> Mark, did I answer your question?
> Should I fix anything in this patch series?

I would move it under required properties even if we happen to not use
it in certain edge cases.

Moving forwards everything should be DT-driven, so it'll be necessary.

Thanks,
Mark.

[PATCH v2 2/3] dmaengine: of: add common xlate function for matching by channel id

[Alexander Popov]

This patch adds a new common OF dma xlate callback function which will match a
channel by it's id. The binding expects one integer argument which it will use to
lookup the channel by the id.

Unlike of_dma_simple_xlate this function is able to handle a system with
multiple DMA controllers. When registering the of dma provider with
of_dma_controller_register a pointer to the dma_device struct which is
associated with the dt node needs to passed as the data parameter.
New function will use this pointer to match only channels which belong to the
specified DMA controller.

Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
 drivers/dma/of-dma.c   | 35 +++++++++++++++++++++++++++++++++++
 include/linux/of_dma.h |  4 ++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index e8fe9dc..d5fbeaa 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -218,3 +218,38 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 			&dma_spec->args[0]);
 }
 EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
+
+/**
+ * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id
+ * @dma_spec:	pointer to DMA specifier as found in the device tree
+ * @of_dma:	pointer to DMA controller data
+ *
+ * This function can be used as the of xlate callback for DMA driver which wants
+ * to match the channel based on the channel id. When using this xlate function
+ * the #dma-cells propety of the DMA controller dt node needs to be set to 1.
+ * The data parameter of of_dma_controller_register must be a pointer to the
+ * dma_device struct the function should match upon.
+ *
+ * Returns pointer to appropriate dma channel on success or NULL on error.
+ */
+struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+					 struct of_dma *ofdma)
+{
+	struct dma_device *dev = ofdma->of_dma_data;
+	struct dma_chan *chan, *candidate = NULL;
+
+	if (!dev || dma_spec->args_count != 1)
+		return NULL;
+
+	list_for_each_entry(chan, &dev->channels, device_node)
+		if (chan->chan_id == dma_spec->args[0]) {
+			candidate = chan;
+			break;
+		}
+
+	if (!candidate)
+		return NULL;
+
+	return dma_get_slave_channel(candidate);
+}
+EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index ae36298..56bc026 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -41,6 +41,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
 						     const char *name);
 extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
 		struct of_dma *ofdma);
+extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+		struct of_dma *ofdma);
 #else
 static inline int of_dma_controller_register(struct device_node *np,
 		struct dma_chan *(*of_dma_xlate)
@@ -66,6 +68,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s
 	return NULL;
 }
 
+#define of_dma_xlate_by_chan_id NULL
+
 #endif
 
 #endif /* __LINUX_OF_DMA_H */
-- 
1.8.4.2

[PATCH v2 3/3] dmaengine: mpc512x: register for device tree channel lookup

[Alexander Popov]

Register the controller for device tree based lookup of DMA channels
(non-fatal for backwards compatibility with older device trees) and
provide the '#dma-cells' property in the shared mpc5121.dtsi file

Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
 arch/powerpc/boot/dts/mpc5121.dtsi |  1 +
 drivers/dma/mpc512x_dma.c          | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
index 2c0e155..7f9d14f 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -498,6 +498,7 @@
 			compatible = "fsl,mpc5121-dma";
 			reg = <0x14000 0x1800>;
 			interrupts = <65 0x8>;
+			#dma-cells = <1>;
 		};
 	};
 
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 2ad4373..881db2b 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -53,6 +53,7 @@
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
+#include <linux/of_dma.h>
 #include <linux/of_platform.h>
 
 #include <linux/random.h>
@@ -1036,7 +1037,15 @@ static int mpc_dma_probe(struct platform_device *op)
 	if (retval)
 		goto err_free2;
 
-	return retval;
+	/* Register with OF helpers for DMA lookups (nonfatal) */
+	if (dev->of_node) {
+		retval = of_dma_controller_register(dev->of_node,
+						of_dma_xlate_by_chan_id, mdma);
+		if (retval)
+			dev_warn(dev, "Could not register for OF lookup\n");
+	}
+
+	return 0;
 
 err_free2:
 	if (mdma->is_mpc8308)
@@ -1057,6 +1066,8 @@ static int mpc_dma_remove(struct platform_device *op)
 	struct device *dev = &op->dev;
 	struct mpc_dma *mdma = dev_get_drvdata(dev);
 
+	if (dev->of_node)
+		of_dma_controller_free(dev->of_node);
 	dma_async_device_unregister(&mdma->dma);
 	if (mdma->is_mpc8308) {
 		free_irq(mdma->irq2, mdma);
-- 
1.8.4.2