[PATCH v2 1/2] Documentation: DT: vdma: Add clock support for vdma

This patch updates the binding doc with clock description
for vdma.

Signed-off-by: Kedareswara rao Appana <redacted>
---
Changes for v2:
--> Listed down all the clocks supported by the h/w
    as suggested by the Datta.
--> Used IP clock names instead of shortcut clock names.

 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
index fcc2b65..afe9eb7 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt

@@ -21,6 +21,11 @@ Required properties:
 - dma-channel child node: Should have at least one channel and can have up to
 	two channels per device. This node specifies the properties of each
 	DMA channel (see child node properties below).
+- clocks: Input clock specifier. Refer to common clock bindings.
+- clock-names: List of input clocks "s_axi_lite_aclk", "m_axi_mm2s_aclk"
+	       "m_axi_s2mm_aclk", "m_axis_mm2s_aclk", "s_axis_s2mm_aclk"
+	       (list of input cloks may vary based on the ip configuration.
+		see clock bindings for more info).
 
 Required properties for VDMA:
 - xlnx,num-fstores: Should be the number of framebuffers as configured in h/w.

@@ -60,6 +65,9 @@ axi_vdma_0: axivdma at 40030000 {
 	xlnx,num-fstores = <0x8>;
 	xlnx,flush-fsync = <0x1>;
 	xlnx,addrwidth = <0x20>;
+	clocks = <&clk 0>, <&clk 1>, <&clk 2>, <&clk 3>, <&clk 4>;
+	clock-names = "s_axi_lite_aclk", "m_axi_mm2s_aclk", "m_axi_s2mm_aclk",
+		      "m_axis_mm2s_aclk", "s_axis_s2mm_aclk";
 	dma-channel at 40030000 {
 		compatible = "xlnx,axi-vdma-mm2s-channel";
 		interrupts = < 0 54 4 >;

-- 
2.1.2

Added basic clock support. The clocks are requested at probe
and released at remove.

Signed-off-by: Kedareswara rao Appana <redacted>
---
Changes for v2:
--> None.

 drivers/dma/xilinx/xilinx_vdma.c | 56 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_vdma.c b/drivers/dma/xilinx/xilinx_vdma.c
index 70caea6..d526029 100644
--- a/drivers/dma/xilinx/xilinx_vdma.c
+++ b/drivers/dma/xilinx/xilinx_vdma.c

@@ -44,6 +44,7 @@
 #include <linux/of_platform.h>
 #include <linux/of_irq.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 
 #include "../dmaengine.h"
 

@@ -352,6 +353,8 @@ struct xilinx_dma_chan {
  * @flush_on_fsync: Flush on frame sync
  * @ext_addr: Indicates 64 bit addressing is supported by dma device
  * @dmatype: DMA ip type
+ * @clks:	pointer to array of clocks
+ * @numclks:	number of clocks available
  */
 struct xilinx_dma_device {
 	void __iomem *regs;

@@ -362,6 +365,8 @@ struct xilinx_dma_device {
 	u32 flush_on_fsync;
 	bool ext_addr;
 	enum xdma_ip_type dmatype;
+	struct clk **clks;
+	int numclks;
 };
 
 /* Macros */

@@ -1731,6 +1736,26 @@ int xilinx_vdma_channel_set_config(struct dma_chan *dchan,
 }
 EXPORT_SYMBOL(xilinx_vdma_channel_set_config);
 
+static int xdma_clk_init(struct xilinx_dma_device *xdev, bool enable)
+{
+	int i = 0, ret;
+
+	for (i = 0; i < xdev->numclks; i++) {
+		if (enable) {
+			ret = clk_prepare_enable(xdev->clks[i]);
+			if (ret) {
+				dev_err(xdev->dev,
+					"failed to enable the axidma clock\n");
+				return ret;
+			}
+		} else {
+			clk_disable_unprepare(xdev->clks[i]);
+		}
+	}
+
+	return 0;
+}
+
 /* -----------------------------------------------------------------------------
  * Probe and remove
  */

@@ -1919,6 +1944,7 @@ static int xilinx_dma_probe(struct platform_device *pdev)
 	struct resource *io;
 	u32 num_frames, addr_width;
 	int i, err;
+	const char *str;
 
 	/* Allocate and initialize the DMA engine structure */
 	xdev = devm_kzalloc(&pdev->dev, sizeof(*xdev), GFP_KERNEL);

@@ -1965,6 +1991,32 @@ static int xilinx_dma_probe(struct platform_device *pdev)
 	/* Set the dma mask bits */
 	dma_set_mask(xdev->dev, DMA_BIT_MASK(addr_width));
 
+	xdev->numclks = of_property_count_strings(pdev->dev.of_node,
+						  "clock-names");
+	if (xdev->numclks > 0) {
+		xdev->clks = devm_kmalloc_array(&pdev->dev, xdev->numclks,
+						sizeof(struct clk *),
+						GFP_KERNEL);
+		if (!xdev->clks)
+			return -ENOMEM;
+
+		for (i = 0; i < xdev->numclks; i++) {
+			of_property_read_string_index(pdev->dev.of_node,
+						      "clock-names", i, &str);
+			xdev->clks[i] = devm_clk_get(xdev->dev, str);
+			if (IS_ERR(xdev->clks[i])) {
+				if (PTR_ERR(xdev->clks[i]) == -ENOENT)
+					xdev->clks[i] = NULL;
+				else
+					return PTR_ERR(xdev->clks[i]);
+			}
+		}
+
+		err = xdma_clk_init(xdev, true);
+		if (err)
+			return err;
+	}
+
 	/* Initialize the DMA engine */
 	xdev->common.dev = &pdev->dev;
 

@@ -2025,6 +2077,8 @@ static int xilinx_dma_probe(struct platform_device *pdev)
 	return 0;
 
 error:
+	if (xdev->numclks > 0)
+		xdma_clk_init(xdev, false);
 	for (i = 0; i < XILINX_DMA_MAX_CHANS_PER_DEVICE; i++)
 		if (xdev->chan[i])
 			xilinx_dma_chan_remove(xdev->chan[i]);

@@ -2050,6 +2104,8 @@ static int xilinx_dma_remove(struct platform_device *pdev)
 	for (i = 0; i < XILINX_DMA_MAX_CHANS_PER_DEVICE; i++)
 		if (xdev->chan[i])
 			xilinx_dma_chan_remove(xdev->chan[i]);
+	if (xdev->numclks > 0)
+		xdma_clk_init(xdev, false);
 
 	return 0;
 }

-- 
2.1.2

On Wed, Apr 20, 2016 at 5:13 PM, Kedareswara rao Appana
[off-list ref] wrote:

Added basic clock support. The clocks are requested at probe
and released at remove.

Signed-off-by: Kedareswara rao Appana <redacted>

Reviewed-by: Shubhrajyoti Datta  <redacted>

On Wed, 2016-04-20 at 17:13:19 +0530, Kedareswara rao Appana wrote:

I guess this works, but the relation to the binding is a little loose,
IMHO. Instead of using the clock names from the binding this is just
using whatever is provided in DT and enabling it. Also, all the clocks
here are handled as optional feature, while binding - and I guess
reality too - have them as mandatory.
It would be nicer if the driver specifically asks for the clocks it
needs and return an error if a mandatory clock is missing.

	S?ren

Hi Soren,

-----Original Message-----
From: S?ren Brinkmann [mailto:soren.brinkmann at xilinx.com]
Sent: Wednesday, April 20, 2016 8:06 PM
To: Appana Durga Kedareswara Rao <redacted>
Cc: robh+dt at kernel.org; pawel.moll at arm.com; mark.rutland at arm.com;
ijc+devicetree at hellion.org.uk; galak at codeaurora.org; Michal Simek
[off-list ref]; vinod.koul at intel.com; dan.j.williams at intel.com;
Appana Durga Kedareswara Rao [off-list ref];
moritz.fischer at ettus.com; laurent.pinchart at ideasonboard.com;
luis at debethencourt.com; Anirudha Sarangi [off-list ref]; Punnaiah
Choudary Kalluri [off-list ref]; Shubhrajyoti Datta
[off-list ref]; devicetree at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
dmaengine at vger.kernel.org
Subject: Re: [PATCH v2 2/2] dmaengine: vdma: Add clock support

On Wed, 2016-04-20 at 17:13:19 +0530, Kedareswara rao Appana wrote:

quoted

Added basic clock support. The clocks are requested at probe and
released at remove.

Signed-off-by: Kedareswara rao Appana <redacted>
---
Changes for v2:
--> None.

 drivers/dma/xilinx/xilinx_vdma.c | 56
++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_vdma.c

b/drivers/dma/xilinx/xilinx_vdma.c
index 70caea6..d526029 100644

--- a/drivers/dma/xilinx/xilinx_vdma.c
+++ b/drivers/dma/xilinx/xilinx_vdma.c

@@ -44,6 +44,7 @@
 #include <linux/of_platform.h>
 #include <linux/of_irq.h>
 #include <linux/slab.h>
+#include <linux/clk.h>

 #include "../dmaengine.h"

@@ -352,6 +353,8 @@ struct xilinx_dma_chan {
  * @flush_on_fsync: Flush on frame sync
  * @ext_addr: Indicates 64 bit addressing is supported by dma device
  * @dmatype: DMA ip type
+ * @clks:	pointer to array of clocks
+ * @numclks:	number of clocks available
  */
 struct xilinx_dma_device {
 	void __iomem *regs;

@@ -362,6 +365,8 @@ struct xilinx_dma_device {
 	u32 flush_on_fsync;
 	bool ext_addr;
 	enum xdma_ip_type dmatype;
+	struct clk **clks;
+	int numclks;
 };

 /* Macros */

@@ -1731,6 +1736,26 @@ int xilinx_vdma_channel_set_config(struct

dma_chan *dchan,  }  EXPORT_SYMBOL(xilinx_vdma_channel_set_config);

+static int xdma_clk_init(struct xilinx_dma_device *xdev, bool enable)
+{
+	int i = 0, ret;
+
+	for (i = 0; i < xdev->numclks; i++) {
+		if (enable) {
+			ret = clk_prepare_enable(xdev->clks[i]);
+			if (ret) {
+				dev_err(xdev->dev,
+					"failed to enable the axidma clock\n");
+				return ret;
+			}
+		} else {
+			clk_disable_unprepare(xdev->clks[i]);
+		}
+	}
+
+	return 0;
+}
+
 /* -----------------------------------------------------------------------------
  * Probe and remove
  */

@@ -1919,6 +1944,7 @@ static int xilinx_dma_probe(struct platform_device

*pdev)

quoted

 	struct resource *io;
 	u32 num_frames, addr_width;
 	int i, err;
+	const char *str;

 	/* Allocate and initialize the DMA engine structure */
 	xdev = devm_kzalloc(&pdev->dev, sizeof(*xdev), GFP_KERNEL); @@
-1965,6 +1991,32 @@ static int xilinx_dma_probe(struct platform_device

*pdev)

quoted

 	/* Set the dma mask bits */
 	dma_set_mask(xdev->dev, DMA_BIT_MASK(addr_width));

+	xdev->numclks = of_property_count_strings(pdev->dev.of_node,
+						  "clock-names");
+	if (xdev->numclks > 0) {
+		xdev->clks = devm_kmalloc_array(&pdev->dev, xdev->numclks,
+						sizeof(struct clk *),
+						GFP_KERNEL);
+		if (!xdev->clks)
+			return -ENOMEM;
+
+		for (i = 0; i < xdev->numclks; i++) {
+			of_property_read_string_index(pdev->dev.of_node,
+						      "clock-names", i, &str);
+			xdev->clks[i] = devm_clk_get(xdev->dev, str);
+			if (IS_ERR(xdev->clks[i])) {
+				if (PTR_ERR(xdev->clks[i]) == -ENOENT)
+					xdev->clks[i] = NULL;
+				else
+					return PTR_ERR(xdev->clks[i]);
+			}
+		}
+
+		err = xdma_clk_init(xdev, true);
+		if (err)
+			return err;
+	}

I guess this works, but the relation to the binding is a little loose, IMHO. Instead
of using the clock names from the binding this is just using whatever is provided
in DT and enabling it. Also, all the clocks here are handled as optional feature,
while binding - and I guess reality too - have them as mandatory.
It would be nicer if the driver specifically asks for the clocks it needs and return
an error if a mandatory clock is missing.

Here DMA channels are configurable I mean if user select only mm2s channel then we will have
Only 3 clocks. If user selects both mm2s and s2mm channels we will have 5 clocks.
That?s why reading the number of clocks from the clock-names property.

And also the AXI DMA/CDMA Ip support also getting added to this driver for those IP's also the clocks are configurable
For AXI DMA it is either 3 or 4 clocks and for AXI CDMA it is 2 clocks.

That's why I thought it is the proper solution.

If you have any better idea please let me know will try in that way...

Regards,
Kedar.

	S?ren

On Wed, 2016-04-20 at 07:55:27 -0700, Appana Durga Kedareswara Rao wrote:

Hi Soren,

quoted

-----Original Message-----
From: S?ren Brinkmann [mailto:soren.brinkmann at xilinx.com]
Sent: Wednesday, April 20, 2016 8:06 PM
To: Appana Durga Kedareswara Rao <redacted>
Cc: robh+dt at kernel.org; pawel.moll at arm.com; mark.rutland at arm.com;
ijc+devicetree at hellion.org.uk; galak at codeaurora.org; Michal Simek
[off-list ref]; vinod.koul at intel.com; dan.j.williams at intel.com;
Appana Durga Kedareswara Rao [off-list ref];
moritz.fischer at ettus.com; laurent.pinchart at ideasonboard.com;
luis at debethencourt.com; Anirudha Sarangi [off-list ref]; Punnaiah
Choudary Kalluri [off-list ref]; Shubhrajyoti Datta
[off-list ref]; devicetree at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
dmaengine at vger.kernel.org
Subject: Re: [PATCH v2 2/2] dmaengine: vdma: Add clock support

On Wed, 2016-04-20 at 17:13:19 +0530, Kedareswara rao Appana wrote:

quoted

Added basic clock support. The clocks are requested at probe and
released at remove.

Signed-off-by: Kedareswara rao Appana <redacted>
---
Changes for v2:
--> None.

 drivers/dma/xilinx/xilinx_vdma.c | 56
++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_vdma.c

b/drivers/dma/xilinx/xilinx_vdma.c
index 70caea6..d526029 100644

--- a/drivers/dma/xilinx/xilinx_vdma.c
+++ b/drivers/dma/xilinx/xilinx_vdma.c

@@ -44,6 +44,7 @@
 #include <linux/of_platform.h>
 #include <linux/of_irq.h>
 #include <linux/slab.h>
+#include <linux/clk.h>

 #include "../dmaengine.h"

@@ -352,6 +353,8 @@ struct xilinx_dma_chan {
  * @flush_on_fsync: Flush on frame sync
  * @ext_addr: Indicates 64 bit addressing is supported by dma device
  * @dmatype: DMA ip type
+ * @clks:	pointer to array of clocks
+ * @numclks:	number of clocks available
  */
 struct xilinx_dma_device {
 	void __iomem *regs;

@@ -362,6 +365,8 @@ struct xilinx_dma_device {
 	u32 flush_on_fsync;
 	bool ext_addr;
 	enum xdma_ip_type dmatype;
+	struct clk **clks;
+	int numclks;
 };

 /* Macros */

@@ -1731,6 +1736,26 @@ int xilinx_vdma_channel_set_config(struct

dma_chan *dchan,  }  EXPORT_SYMBOL(xilinx_vdma_channel_set_config);

+static int xdma_clk_init(struct xilinx_dma_device *xdev, bool enable)
+{
+	int i = 0, ret;
+
+	for (i = 0; i < xdev->numclks; i++) {
+		if (enable) {
+			ret = clk_prepare_enable(xdev->clks[i]);
+			if (ret) {
+				dev_err(xdev->dev,
+					"failed to enable the axidma clock\n");
+				return ret;
+			}
+		} else {
+			clk_disable_unprepare(xdev->clks[i]);
+		}
+	}
+
+	return 0;
+}
+
 /* -----------------------------------------------------------------------------
  * Probe and remove
  */

@@ -1919,6 +1944,7 @@ static int xilinx_dma_probe(struct platform_device

*pdev)

quoted

 	struct resource *io;
 	u32 num_frames, addr_width;
 	int i, err;
+	const char *str;

 	/* Allocate and initialize the DMA engine structure */
 	xdev = devm_kzalloc(&pdev->dev, sizeof(*xdev), GFP_KERNEL); @@
-1965,6 +1991,32 @@ static int xilinx_dma_probe(struct platform_device

*pdev)

quoted

 	/* Set the dma mask bits */
 	dma_set_mask(xdev->dev, DMA_BIT_MASK(addr_width));

+	xdev->numclks = of_property_count_strings(pdev->dev.of_node,
+						  "clock-names");
+	if (xdev->numclks > 0) {
+		xdev->clks = devm_kmalloc_array(&pdev->dev, xdev->numclks,
+						sizeof(struct clk *),
+						GFP_KERNEL);
+		if (!xdev->clks)
+			return -ENOMEM;
+
+		for (i = 0; i < xdev->numclks; i++) {
+			of_property_read_string_index(pdev->dev.of_node,
+						      "clock-names", i, &str);
+			xdev->clks[i] = devm_clk_get(xdev->dev, str);
+			if (IS_ERR(xdev->clks[i])) {
+				if (PTR_ERR(xdev->clks[i]) == -ENOENT)
+					xdev->clks[i] = NULL;
+				else
+					return PTR_ERR(xdev->clks[i]);
+			}
+		}
+
+		err = xdma_clk_init(xdev, true);
+		if (err)
+			return err;
+	}

I guess this works, but the relation to the binding is a little loose, IMHO. Instead
of using the clock names from the binding this is just using whatever is provided
in DT and enabling it. Also, all the clocks here are handled as optional feature,
while binding - and I guess reality too - have them as mandatory.
It would be nicer if the driver specifically asks for the clocks it needs and return
an error if a mandatory clock is missing.

Here DMA channels are configurable I mean if user select only mm2s channel then we will have
Only 3 clocks. If user selects both mm2s and s2mm channels we will have 5 clocks.
That?s why reading the number of clocks from the clock-names property.

And also the AXI DMA/CDMA Ip support also getting added to this driver for those IP's also the clocks are configurable
For AXI DMA it is either 3 or 4 clocks and for AXI CDMA it is 2 clocks.

That's why I thought it is the proper solution.

If you have any better idea please let me know will try in that way...

I guess the driver know all these things: whether it controls vdma or
cdma, what interfaces it has and how many channels? Based on that, I
guess it should be possible to derive what clocks are required for
correct operation.

	S?ren

On Wed, 2016-04-20 at 17:13:18 +0530, Kedareswara rao Appana wrote:

s/cloks/clocks

Acked-by: S?ren Brinkmann <redacted>

	S?ren

Hi Soren,	

-----Original Message-----
From: S?ren Brinkmann [mailto:soren.brinkmann at xilinx.com]
Sent: Wednesday, April 20, 2016 7:58 PM
To: Appana Durga Kedareswara Rao <redacted>
Cc: robh+dt at kernel.org; pawel.moll at arm.com; mark.rutland at arm.com;
ijc+devicetree at hellion.org.uk; galak at codeaurora.org; Michal Simek
[off-list ref]; vinod.koul at intel.com; dan.j.williams at intel.com;
Appana Durga Kedareswara Rao [off-list ref];
moritz.fischer at ettus.com; laurent.pinchart at ideasonboard.com;
luis at debethencourt.com; Anirudha Sarangi [off-list ref]; Punnaiah
Choudary Kalluri [off-list ref]; Shubhrajyoti Datta
[off-list ref]; devicetree at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
dmaengine at vger.kernel.org
Subject: Re: [PATCH v2 1/2] Documentation: DT: vdma: Add clock support for
vdma

On Wed, 2016-04-20 at 17:13:18 +0530, Kedareswara rao Appana wrote:

quoted

This patch updates the binding doc with clock description for vdma.

Signed-off-by: Kedareswara rao Appana <redacted>
---
Changes for v2:
--> Listed down all the clocks supported by the h/w
    as suggested by the Datta.
--> Used IP clock names instead of shortcut clock names.

 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 8
++++++++
 1 file changed, 8 insertions(+)

diff --git
a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
index fcc2b65..afe9eb7 100644

--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt

@@ -21,6 +21,11 @@ Required properties:
 - dma-channel child node: Should have at least one channel and can have up

to

quoted

 	two channels per device. This node specifies the properties of each
 	DMA channel (see child node properties below).
+- clocks: Input clock specifier. Refer to common clock bindings.
+- clock-names: List of input clocks "s_axi_lite_aclk", "m_axi_mm2s_aclk"
+	       "m_axi_s2mm_aclk", "m_axis_mm2s_aclk", "s_axis_s2mm_aclk"
+	       (list of input cloks may vary based on the ip configuration.

s/cloks/clocks

Ok will fix in next version...

Acked-by: S?ren Brinkmann <redacted>

Thanks for the ack...

Regards,
Kedar.

	S?ren

On Wed, Apr 20, 2016 at 05:13:18PM +0530, Kedareswara rao Appana wrote:

s/cloks/clocks/

+		see clock bindings for more info).

This does not make sense. The common clock binding is going to tell me 
more about how the clocks vary?

You need to define here how the clocks can vary.

Hi Rob,

	Thanks for the review...

-----Original Message-----
From: Rob Herring [mailto:robh at kernel.org]
Sent: Saturday, April 23, 2016 1:07 AM
To: Appana Durga Kedareswara Rao <redacted>
Cc: pawel.moll at arm.com; mark.rutland at arm.com;
ijc+devicetree at hellion.org.uk; galak at codeaurora.org; Michal Simek
[off-list ref]; Soren Brinkmann [off-list ref];
vinod.koul at intel.com; dan.j.williams at intel.com; Appana Durga Kedareswara
Rao [off-list ref]; moritz.fischer at ettus.com;
laurent.pinchart at ideasonboard.com; luis at debethencourt.com; Anirudha
Sarangi [off-list ref]; Punnaiah Choudary Kalluri
[off-list ref]; Shubhrajyoti Datta [off-list ref];
devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
kernel at vger.kernel.org; dmaengine at vger.kernel.org
Subject: Re: [PATCH v2 1/2] Documentation: DT: vdma: Add clock support for
vdma

On Wed, Apr 20, 2016 at 05:13:18PM +0530, Kedareswara rao Appana wrote:

quoted

This patch updates the binding doc with clock description for vdma.

Signed-off-by: Kedareswara rao Appana <redacted>
---
Changes for v2:
--> Listed down all the clocks supported by the h/w
    as suggested by the Datta.
--> Used IP clock names instead of shortcut clock names.

 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 8
++++++++
 1 file changed, 8 insertions(+)

diff --git
a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
index fcc2b65..afe9eb7 100644

--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt

@@ -21,6 +21,11 @@ Required properties:
 - dma-channel child node: Should have at least one channel and can have up

to

quoted

 	two channels per device. This node specifies the properties of each
 	DMA channel (see child node properties below).
+- clocks: Input clock specifier. Refer to common clock bindings.
+- clock-names: List of input clocks "s_axi_lite_aclk", "m_axi_mm2s_aclk"
+	       "m_axi_s2mm_aclk", "m_axis_mm2s_aclk", "s_axis_s2mm_aclk"
+	       (list of input cloks may vary based on the ip configuration.

s/cloks/clocks/

quoted

+		see clock bindings for more info).

This does not make sense. The common clock binding is going to tell me more
about how the clocks vary?

I have fixed these comments in the other version (v4) you acked that patch...

Regards,
Kedar.

You need to define here how the clocks can vary.