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

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

[Kedareswara rao Appana]

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

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
index fcc2b65..e1c9019 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
@@ -21,6 +21,10 @@ 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 "axi_clk", "tx_clk", "txs_clk" (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 +64,8 @@ axi_vdma_0: axivdma at 40030000 {
 	xlnx,num-fstores = <0x8>;
 	xlnx,flush-fsync = <0x1>;
 	xlnx,addrwidth = <0x20>;
+	clocks = <&clk 0>, <&clk 1>, <&clk 2>;
+	clock-names = "axi_clk", "tx_clk", "txs_clk";
 	dma-channel at 40030000 {
 		compatible = "xlnx,axi-vdma-mm2s-channel";
 		interrupts = < 0 54 4 >;
-- 
2.1.2

[PATCH 2/2] dmaengine: vdma: Add clock support

[Kedareswara rao Appana]

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

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
 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

[PATCH 2/2] dmaengine: vdma: Add clock support

[Moritz Fischer]

Hi,

thanks for looking into this.

On Wed, Apr 20, 2016 at 12:20 AM, Kedareswara rao Appana
<appana.durga.rao@xilinx.com> wrote:

> +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;

What happens to the ones you already turned on, if say the third fails?

Cheers,

Moritz

[PATCH 2/2] dmaengine: vdma: Add clock support

[Appana Durga Kedareswara Rao]

Hi Moritz,

        Thanks for the review...

> -----Original Message-----
> From: Moritz Fischer [mailto:moritz.fischer at ettus.com]
> Sent: Thursday, April 21, 2016 12:09 AM
> To: Appana Durga Kedareswara Rao <appanad@xilinx.com>
> Cc: Rob Herring <robh+dt@kernel.org>; pawel.moll at arm.com; Mark Rutland
> <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Michal Simek <michals@xilinx.com>;
> Soren Brinkmann <sorenb@xilinx.com>; Vinod Koul <vinod.koul@intel.com>;
> Dan Williams <dan.j.williams@intel.com>; Appana Durga Kedareswara Rao
> <appanad@xilinx.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; Luis de Bethencourt
> <luis@debethencourt.com>; Anirudha Sarangi <anirudh@xilinx.com>; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>; Devicetree List
> <devicetree@vger.kernel.org>; linux-arm-kernel <linux-arm-
> kernel at lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel at vger.kernel.org>; dmaengine at vger.kernel.org
> Subject: Re: [PATCH 2/2] dmaengine: vdma: Add clock support
>
> Hi,
>
> thanks for looking into this.
>
> On Wed, Apr 20, 2016 at 12:20 AM, Kedareswara rao Appana
> <appana.durga.rao@xilinx.com> wrote:
>
> > +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;
>
> What happens to the ones you already turned on, if say the third fails?

Will fix in the next version of the patch...

Regards,
Kedar.
>
> Cheers,
>
> Moritz


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

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

[Shubhrajyoti Datta]

On Wed, Apr 20, 2016 at 12:49 PM, Kedareswara rao Appana
<appana.durga.rao@xilinx.com> wrote:
> This patch updates the binding doc with clock description
> for vdma.
>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
>  Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> index fcc2b65..e1c9019 100644
> --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> @@ -21,6 +21,10 @@ 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 "axi_clk", "tx_clk", "txs_clk" (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 +64,8 @@ axi_vdma_0: axivdma at 40030000 {
>         xlnx,num-fstores = <0x8>;
>         xlnx,flush-fsync = <0x1>;
>         xlnx,addrwidth = <0x20>;
> +       clocks = <&clk 0>, <&clk 1>, <&clk 2>;
> +       clock-names = "axi_clk", "tx_clk", "txs_clk";

the module has
s_axi_lite_aclk Clock I AXI VDMA AXI4-Lite interface clock
 m_axi_mm2s_aclk Clock I AXI VDMA MM2S clock
 m_axi_s2mm_aclk Clock I AXI VDMA S2MM clock
 m_axis_mm2s_aclk Clock I AXI VDMA MM2S AXIS clock
 s_axis_s2mm_aclk Clock I AXI VDMA S2MM AXIS clock

I think a partial support is not wrong.
however  we should keep the names same as the TRM.


>         dma-channel at 40030000 {
>                 compatible = "xlnx,axi-vdma-mm2s-channel";
>                 interrupts = < 0 54 4 >;
> --
> 2.1.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Appana Durga Kedareswara Rao]

Hi Shubhrajyoti,

> -----Original Message-----
> From: Shubhrajyoti Datta [mailto:shubhrajyoti.datta at gmail.com]
> Sent: Wednesday, April 20, 2016 1:30 PM
> To: Appana Durga Kedareswara Rao <appanad@xilinx.com>
> Cc: Rob Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>;
> Mark Rutland <mark.rutland@arm.com>; Ian Campbell
> <ijc+devicetree@hellion.org.uk>; Kumar Gala <galak@codeaurora.org>; Michal
> Simek <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
> vinod.koul at intel.com; dan.j.williams at intel.com; Appana Durga Kedareswara
> Rao <appanad@xilinx.com>; Moritz Fischer <moritz.fischer@ettus.com>;
> Laurent Pinchart <laurent.pinchart@ideasonboard.com>;
> luis at debethencourt.com; Anirudha Sarangi <anirudh@xilinx.com>; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>; dmaengine at vger.kernel.org;
> devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH 1/2] Documentation: DT: vdma: Add clock support for vdma
> 
> On Wed, Apr 20, 2016 at 12:49 PM, Kedareswara rao Appana
> <appana.durga.rao@xilinx.com> wrote:
> > This patch updates the binding doc with clock description for vdma.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> > ---
> >  Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 6
> > ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> > b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> > index fcc2b65..e1c9019 100644
> > --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> > +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> > @@ -21,6 +21,10 @@ 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 "axi_clk", "tx_clk", "txs_clk" (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 +64,8 @@ axi_vdma_0: axivdma at 40030000 {
> >         xlnx,num-fstores = <0x8>;
> >         xlnx,flush-fsync = <0x1>;
> >         xlnx,addrwidth = <0x20>;
> > +       clocks = <&clk 0>, <&clk 1>, <&clk 2>;
> > +       clock-names = "axi_clk", "tx_clk", "txs_clk";
> 
> the module has
> s_axi_lite_aclk Clock I AXI VDMA AXI4-Lite interface clock  m_axi_mm2s_aclk
> Clock I AXI VDMA MM2S clock  m_axi_s2mm_aclk Clock I AXI VDMA S2MM clock
> m_axis_mm2s_aclk Clock I AXI VDMA MM2S AXIS clock  s_axis_s2mm_aclk
> Clock I AXI VDMA S2MM AXIS clock
> 
> I think a partial support is not wrong.

It is not partial support the driver is supporting all the clocks available in the IP (please refer the patch 2 in the series).
In the example I just putted clock names for TX path I mean when the IP is configured only with one channel.

Will update the clock-names example with the all the supported clocks in the next version.

> however  we should keep the names same as the TRM.

Why?? Why can't we use simple convenient names that refers to equivalent clocks of h/w 
I mean I used axi_clk instead of s_axi_lite_aclk,
Used tx_clk instead of m_axi_mm2s_aclk,
Used txs_clk instead of m_axis_mm2s_aclk.

If it mandatory to use names as the TRM please let me know will update the same v2...

Regards,
Kedar.

> 
> 
> >         dma-channel at 40030000 {
> >                 compatible = "xlnx,axi-vdma-mm2s-channel";
> >                 interrupts = < 0 54 4 >;
> > --
> > 2.1.2
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lars-Peter Clausen]

On 04/20/2016 10:39 AM, Appana Durga Kedareswara Rao wrote:
[...]
>> however  we should keep the names same as the TRM.
> 
> Why?? Why can't we use simple convenient names that refers to equivalent clocks of h/w 

Because it is confusing. If you use the same name it's very straight forward
to understand which clock is being referred to. That's not the case if you
come up with random new names.

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

[Appana Durga Kedareswara Rao]

Hi Lars,

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars at metafoo.de]
> Sent: Wednesday, April 20, 2016 4:45 PM
> To: Appana Durga Kedareswara Rao <appanad@xilinx.com>; Shubhrajyoti Datta
> <shubhrajyoti.datta@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>;
> Mark Rutland <mark.rutland@arm.com>; Ian Campbell
> <ijc+devicetree@hellion.org.uk>; Kumar Gala <galak@codeaurora.org>; Michal
> Simek <michals@xilinx.com>; Soren Brinkmann <sorenb@xilinx.com>;
> vinod.koul at intel.com; dan.j.williams at intel.com; Moritz Fischer
> <moritz.fischer@ettus.com>; Laurent Pinchart
> <laurent.pinchart@ideasonboard.com>; luis at debethencourt.com; Anirudha
> Sarangi <anirudh@xilinx.com>; Punnaiah Choudary Kalluri
> <punnaia@xilinx.com>; dmaengine at vger.kernel.org;
> devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH 1/2] Documentation: DT: vdma: Add clock support for vdma
> 
> On 04/20/2016 10:39 AM, Appana Durga Kedareswara Rao wrote:
> [...]
> >> however  we should keep the names same as the TRM.
> >
> > Why?? Why can't we use simple convenient names that refers to
> > equivalent clocks of h/w
> 
> Because it is confusing. If you use the same name it's very straight forward to
> understand which clock is being referred to. That's not the case if you come up
> with random new names.

Ok will fix in the v2...

Regards,
Kedar.