Subject: [PATCH 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 <[email protected]>
---
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@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@40030000 {
compatible = "xlnx,axi-vdma-mm2s-channel";
interrupts = < 0 54 4 >;
--
2.1.2


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

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

Signed-off-by: Kedareswara rao Appana <[email protected]>
---
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

2016-04-20 07:59:36

by Shubhrajyoti Datta

[permalink] [raw]
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
<[email protected]> wrote:
> This patch updates the binding doc with clock description
> for vdma.
>
> Signed-off-by: Kedareswara rao Appana <[email protected]>
> ---
> 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@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@40030000 {
> compatible = "xlnx,axi-vdma-mm2s-channel";
> interrupts = < 0 54 4 >;
> --
> 2.1.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

Hi Shubhrajyoti,

> -----Original Message-----
> From: Shubhrajyoti Datta [mailto:[email protected]]
> Sent: Wednesday, April 20, 2016 1:30 PM
> To: Appana Durga Kedareswara Rao <[email protected]>
> Cc: Rob Herring <[email protected]>; Pawel Moll <[email protected]>;
> Mark Rutland <[email protected]>; Ian Campbell
> <[email protected]>; Kumar Gala <[email protected]>; Michal
> Simek <[email protected]>; Soren Brinkmann <[email protected]>;
> [email protected]; [email protected]; Appana Durga Kedareswara
> Rao <[email protected]>; Moritz Fischer <[email protected]>;
> Laurent Pinchart <[email protected]>;
> [email protected]; Anirudha Sarangi <[email protected]>; Punnaiah
> Choudary Kalluri <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]
> 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
> <[email protected]> wrote:
> > This patch updates the binding doc with clock description for vdma.
> >
> > Signed-off-by: Kedareswara rao Appana <[email protected]>
> > ---
> > 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@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@40030000 {
> > compatible = "xlnx,axi-vdma-mm2s-channel";
> > interrupts = < 0 54 4 >;
> > --
> > 2.1.2
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2016-04-20 11:15:42

by Lars-Peter Clausen

[permalink] [raw]
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.

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

Hi Lars,

> -----Original Message-----
> From: Lars-Peter Clausen [mailto:[email protected]]
> Sent: Wednesday, April 20, 2016 4:45 PM
> To: Appana Durga Kedareswara Rao <[email protected]>; Shubhrajyoti Datta
> <[email protected]>
> Cc: Rob Herring <[email protected]>; Pawel Moll <[email protected]>;
> Mark Rutland <[email protected]>; Ian Campbell
> <[email protected]>; Kumar Gala <[email protected]>; Michal
> Simek <[email protected]>; Soren Brinkmann <[email protected]>;
> [email protected]; [email protected]; Moritz Fischer
> <[email protected]>; Laurent Pinchart
> <[email protected]>; [email protected]; Anirudha
> Sarangi <[email protected]>; Punnaiah Choudary Kalluri
> <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]
> 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.

2016-04-20 18:39:07

by Moritz Fischer

[permalink] [raw]
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
<[email protected]> 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

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

Hi Moritz,

Thanks for the review...

> -----Original Message-----
> From: Moritz Fischer [mailto:[email protected]]
> Sent: Thursday, April 21, 2016 12:09 AM
> To: Appana Durga Kedareswara Rao <[email protected]>
> Cc: Rob Herring <[email protected]>; [email protected]; Mark Rutland
> <[email protected]>; Ian Campbell <[email protected]>;
> Kumar Gala <[email protected]>; Michal Simek <[email protected]>;
> Soren Brinkmann <[email protected]>; Vinod Koul <[email protected]>;
> Dan Williams <[email protected]>; Appana Durga Kedareswara Rao
> <[email protected]>; Laurent Pinchart
> <[email protected]>; Luis de Bethencourt
> <[email protected]>; Anirudha Sarangi <[email protected]>; Punnaiah
> Choudary Kalluri <[email protected]>; Devicetree List
> <[email protected]>; linux-arm-kernel <linux-arm-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; [email protected]
> 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
> <[email protected]> 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.