2013-04-30 07:11:32

by Guennadi Liakhovetski

[permalink] [raw]
Subject: [PATCH/RFC 0/4] dmaengine: add DT support for DMA multiplexers

Previously an issue has been discussed, arising on sh-/r-mobile ARM-based
systems. There we typically have multiple DMA controller instances with
exactly equal or very similar capabilities. Each of them can serve the same
slaves, using the same slave identifiers (request line IDs). With the
present DMA DT implementation _each_ such DMA slave would have to reference
_each_ of those DMA controllers in its DMA bindings, e.g.

mmc0: mmc@10000000 {
...
dmas = <&dma0 0x10
&dma1 0x10
&dma2 0x10
&dma3 0x10
&dma0 0x11
&dma1 0x11
&dma2 0x11
&dma3 0x11>;
dma-names = "tx", "tx", "tx", "tx",
"rx", "rx", "rx", "rx";
};

Which certainly isn't pretty. To avoid such redundancy it has been proposed
to implement a DMA multiplexer DT node. That way slaves would just
reference the multiplexer and one of DMA controller instances in it would
be picked up automatically to provide DMA channels to slaves. Patches 1-3
in this series propose such an implementation. Patch 4 is just a minor
clean up, can be applied independently.

Cc: Guennadi Liakhovetski <[email protected]>

Guennadi Liakhovetski (4):
OF: add a new phandle parsing function for grouped nodes
dmaengine: add support for DMA multiplexer DT nodes
ARM: shmobile: move r8a7740 DMA controller DT node under a "dma-mux"
node
OF: modify function stubs to match proper function declarations.

Documentation/devicetree/bindings/dma/dma.txt | 44 ++++++++++++++++++++
.../boot/dts/r8a7740-armadillo800eva-reference.dts | 12 +++---
arch/arm/boot/dts/r8a7740.dtsi | 43 +++++++++++--------
drivers/dma/of-dma.c | 39 +++++++++++++----
drivers/of/base.c | 28 +++++++++++-
include/linux/of.h | 20 ++++++++-
6 files changed, 147 insertions(+), 39 deletions(-)

--
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/


2013-04-30 07:11:33

by Guennadi Liakhovetski

[permalink] [raw]
Subject: [PATCH 4/4] OF: modify function stubs to match proper function declarations.

of_parse_phandle_with_args() and of_count_phandle_with_args() functions
are declared with their first parameter as const. However, their
respective stubs, used when CONFIG_OF isn't defined, don't have the "const"
modifier. This patch adds it to fix the mismatch.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
include/linux/of.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index 30ae71f..d9dbf73 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -466,7 +466,7 @@ static inline struct device_node *of_parse_phandle(const struct device_node *np,
return NULL;
}

-static inline int of_parse_phandle_with_args(struct device_node *np,
+static inline int of_parse_phandle_with_args(const struct device_node *np,
const char *list_name,
const char *cells_name,
int index,
@@ -485,7 +485,7 @@ static inline int of_parse_phandle_with_child_args(const struct device_node *np,
return -ENOSYS;
}

-static inline int of_count_phandle_with_args(struct device_node *np,
+static inline int of_count_phandle_with_args(const struct device_node *np,
const char *list_name,
const char *cells_name)
{
--
1.7.2.5

2013-04-30 07:12:14

by Guennadi Liakhovetski

[permalink] [raw]
Subject: [PATCH 1/4] OF: add a new phandle parsing function for grouped nodes

Sometimes it is useful to group similar DT nodes under a common parent,
when a different node can reference the group, meaning, that any subnode
will do the job. An example of such a group is a DMA multiplexer, when a
DMA slave can be served by any DMA controller from the group. This patch
slightly extends an internal __of_parse_phandle_with_args() function to
accept one more optional parameter and exports a new API function
of_parse_phandle_with_child_args(), which allows the caller to provide a
compatibility string for a group. In this case the function returns the
first matching node. Once a matching node is found, standard DT iterators
can be used to look for further matches.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
drivers/of/base.c | 28 +++++++++++++++++++++++++---
include/linux/of.h | 16 ++++++++++++++++
2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 321d3ef..d114515 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1066,11 +1066,16 @@ EXPORT_SYMBOL(of_parse_phandle);
* @cells_name: property name that specifies phandles' arguments count
* @index: index of a phandle to parse out
* @out_args: optional pointer to output arguments structure (will be filled)
+ * @parent: optional parent-compatibility string
*
* This function is useful to parse lists of phandles and their arguments.
* Returns 0 on success and fills out_args, on error returns appropriate
* errno value.
*
+ * If @parent is specified and the DT node, pointed by the currently iterated
+ * phandle is compatible with it, it is assumed, that the phandle is a parent of
+ * DT nodes with equal cell counts. In that case the first child is taken.
+ *
* Caller is responsible to call of_node_put() on the returned out_args->node
* pointer.
*
@@ -1094,7 +1099,8 @@ EXPORT_SYMBOL(of_parse_phandle);
static int __of_parse_phandle_with_args(const struct device_node *np,
const char *list_name,
const char *cells_name, int index,
- struct of_phandle_args *out_args)
+ struct of_phandle_args *out_args,
+ const char *parent)
{
const __be32 *list, *list_end;
int rc = 0, size, cur_index = 0;
@@ -1129,6 +1135,12 @@ static int __of_parse_phandle_with_args(const struct device_node *np,
np->full_name);
goto err;
}
+ if (parent && of_device_is_compatible(node, parent)) {
+ struct device_node *child =
+ of_get_next_available_child(node, NULL);
+ of_node_put(node);
+ node = child;
+ }
if (of_property_read_u32(node, cells_name, &count)) {
pr_err("%s: could not get %s for %s\n",
np->full_name, cells_name,
@@ -1199,10 +1211,20 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
{
if (index < 0)
return -EINVAL;
- return __of_parse_phandle_with_args(np, list_name, cells_name, index, out_args);
+ return __of_parse_phandle_with_args(np, list_name, cells_name, index, out_args, NULL);
}
EXPORT_SYMBOL(of_parse_phandle_with_args);

+int of_parse_phandle_with_child_args(const struct device_node *np, const char *list_name,
+ const char *cells_name, int index,
+ struct of_phandle_args *out_args, const char *parent)
+{
+ if (index < 0)
+ return -EINVAL;
+ return __of_parse_phandle_with_args(np, list_name, cells_name, index, out_args, parent);
+}
+EXPORT_SYMBOL(of_parse_phandle_with_child_args);
+
/**
* of_count_phandle_with_args() - Find the number of phandles references in a property
* @np: pointer to a device tree node containing a list
@@ -1221,7 +1243,7 @@ EXPORT_SYMBOL(of_parse_phandle_with_args);
int of_count_phandle_with_args(const struct device_node *np, const char *list_name,
const char *cells_name)
{
- return __of_parse_phandle_with_args(np, list_name, cells_name, -1, NULL);
+ return __of_parse_phandle_with_args(np, list_name, cells_name, -1, NULL, NULL);
}
EXPORT_SYMBOL(of_count_phandle_with_args);

diff --git a/include/linux/of.h b/include/linux/of.h
index a0f1292..30ae71f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -277,6 +277,12 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
extern int of_parse_phandle_with_args(const struct device_node *np,
const char *list_name, const char *cells_name, int index,
struct of_phandle_args *out_args);
+extern int of_parse_phandle_with_child_args(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name,
+ int index,
+ struct of_phandle_args *out_args,
+ const char *parent);
extern int of_count_phandle_with_args(const struct device_node *np,
const char *list_name, const char *cells_name);

@@ -469,6 +475,16 @@ static inline int of_parse_phandle_with_args(struct device_node *np,
return -ENOSYS;
}

+static inline int of_parse_phandle_with_child_args(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name,
+ int index,
+ struct of_phandle_args *out_args,
+ const char *parent)
+{
+ return -ENOSYS;
+}
+
static inline int of_count_phandle_with_args(struct device_node *np,
const char *list_name,
const char *cells_name)
--
1.7.2.5

2013-04-30 07:11:30

by Guennadi Liakhovetski

[permalink] [raw]
Subject: [PATCH 3/4] ARM: shmobile: move r8a7740 DMA controller DT node under a "dma-mux" node

On r8a7740 multiple DMA controllers can serve the same slaves with the
same DMA slave IDs. To use this feature DMA controller DT nodes have
to be grouped under "dma-mux" multiplexer nodes and slaves have to be
modified to reference multiplexer nodes instead of individual DMA DT
nodes. Future patches shall add further DMA controller DT nodes under
the same DMA multiplexer DT node.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
---

Applies on top of my earlier "ARM: shmobile: r8a7740: add DT node for one
DMAC instance" patch still under review.

.../boot/dts/r8a7740-armadillo800eva-reference.dts | 12 +++---
arch/arm/boot/dts/r8a7740.dtsi | 43 +++++++++++--------
2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
index c201445..1983d49 100644
--- a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
+++ b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
@@ -47,8 +47,8 @@
vmmc-supply = <&reg_3p3v>;
bus-width = <8>;
non-removable;
- dmas = <&dma0 0xd1
- &dma0 0xd2>;
+ dmas = <&dmac 0xd1
+ &dmac 0xd2>;
dma-names = "tx", "rx";
status = "okay";
};
@@ -57,8 +57,8 @@
vmmc-supply = <&reg_3p3v>;
bus-width = <4>;
broken-cd;
- dmas = <&dma0 0xc1
- &dma0 0xc2>;
+ dmas = <&dmac 0xc1
+ &dmac 0xc2>;
dma-names = "tx", "rx";
status = "okay";
};
@@ -66,8 +66,8 @@
&sdhi1 {
vmmc-supply = <&reg_3p3v>;
bus-width = <4>;
- dmas = <&dma0 0xc9
- &dma0 0xca>;
+ dmas = <&dmac 0xc9
+ &dmac 0xca>;
dma-names = "tx", "rx";
status = "okay";
};
diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
index 054bc1a..965ca4c 100644
--- a/arch/arm/boot/dts/r8a7740.dtsi
+++ b/arch/arm/boot/dts/r8a7740.dtsi
@@ -112,24 +112,31 @@
0 149 0x4>;
};

- dma0: shdma@fe008020 {
- compatible = "renesas,shdma";
- reg = <0xfe008020 0x270
- 0xfe009000 0xc>;
- interrupt-parent = <&gic>;
- interrupts = <0 34 4
- 0 28 4
- 0 29 4
- 0 30 4
- 0 31 4
- 0 32 4
- 0 33 4>;
- interrupt-names = "error",
- "ch0", "ch1", "ch2", "ch3",
- "ch4", "ch5";
- #dma-cells = <1>;
- dma-channels = <6>;
- dma-requests = <256>;
+ dmac: dma-mux0 {
+ compatible = "simple-bus", "dma-mux";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ dma0: shdma@fe008020 {
+ compatible = "renesas,shdma";
+ reg = <0xfe008020 0x270>,
+ <0xfe009000 0xc>;
+ interrupt-parent = <&gic>;
+ interrupts = <0 34 4
+ 0 28 4
+ 0 29 4
+ 0 30 4
+ 0 31 4
+ 0 32 4
+ 0 33 4>;
+ interrupt-names = "error",
+ "ch0", "ch1", "ch2", "ch3",
+ "ch4", "ch5";
+ #dma-cells = <1>;
+ dma-channels = <6>;
+ dma-requests = <256>;
+ };
};

i2c0: i2c@fff20000 {
--
1.7.2.5

2013-04-30 07:11:29

by Guennadi Liakhovetski

[permalink] [raw]
Subject: [PATCH 2/4] dmaengine: add support for DMA multiplexer DT nodes

If a slave can use any of several DMA controllers on the system, multiple
DMA descriptors can be listed in its "dmas" DT property with the same
channel name and different DMA controller phandles. However, if multiple
such slaves can use any of a set of DMA controllers on the system, listing
them all in each slave's "dmas" property becomes counterproductive. This
patch adds support for a "dma-mux" DT node, whose sole purpose is to group
such DMA controller DT nodes. Slaves can then specify the group's phandle
in their "dmas" property. DMA controllers, belonging to the same group
must have the same #dma-cells number and use the same slave IDs.

Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
Documentation/devicetree/bindings/dma/dma.txt | 44 +++++++++++++++++++++++++
drivers/dma/of-dma.c | 39 ++++++++++++++++------
2 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/dma.txt b/Documentation/devicetree/bindings/dma/dma.txt
index 8f504e6..a861298 100644
--- a/Documentation/devicetree/bindings/dma/dma.txt
+++ b/Documentation/devicetree/bindings/dma/dma.txt
@@ -31,6 +31,50 @@ Example:
dma-requests = <127>;
};

+* DMA multiplexer
+
+Several DMA controllers with the same #dma-cells number and the same request
+line IDs can be grouped under a DMA multiplexer super-node, if slaves can use
+DMA channels on any of them.
+
+Required property:
+- compatible: Must include "dma-mux".
+
+Some standard optional properties can be helpful:
+
+Optional properties:
+- compatible: You will probably also want to include compatibility
+ with "simple-bus" to automatically create platform
+ devices from subnodes.
+- ranges: Map DMA controller memory areas in the parent address
+ space.
+- #address-cells: Number of address cells in case automatic propagation
+ with the help of "ranges" doesn't work.
+- #size-cells: Number of size cells.
+
+Example:
+
+ dmac: dma-mux {
+ compatible = "simple-bus", "dma-mux";
+ ranges;
+
+ dma0: dma@10000000 {
+ #dma-cells = <1>;
+ ...
+ };
+
+ dma1: dma@20000000 {
+ #dma-cells = <1>;
+ ...
+ };
+ };
+
+ mmc0: mmc@30000000 {
+ dmas = <&dmac 1
+ &dmac 2>;
+ dma-names = "tx", "rx";
+ ...
+ };

* DMA client

diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 69d04d2..bd7652b 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -183,8 +183,8 @@ static int of_dma_match_channel(struct device_node *np, char *name, int index,
if (strcmp(name, s))
return -ENODEV;

- if (of_parse_phandle_with_args(np, "dmas", "#dma-cells", index,
- dma_spec))
+ if (of_parse_phandle_with_child_args(np, "dmas", "#dma-cells", index,
+ dma_spec, "dma-mux"))
return -ENODEV;

return 0;
@@ -217,22 +217,41 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
}

for (i = 0; i < count; i++) {
+ struct device_node *dma = NULL, *parent;
+ bool is_mux;
+
if (of_dma_match_channel(np, name, i, &dma_spec))
continue;

- ofdma = of_dma_get_controller(&dma_spec);
+ parent = of_node_get(dma_spec.np->parent);
+ is_mux = of_device_is_compatible(parent, "dma-mux");

- if (!ofdma)
- continue;
+ do {
+ if (is_mux) {
+ dma = of_get_next_available_child(parent, dma);
+ if (!dma)
+ break;
+ dma_spec.np = dma;
+ }
+
+ ofdma = of_dma_get_controller(&dma_spec);

- chan = ofdma->of_dma_xlate(&dma_spec, ofdma);
+ if (!ofdma)
+ continue;

- of_dma_put_controller(ofdma);
+ chan = ofdma->of_dma_xlate(&dma_spec, ofdma);

- of_node_put(dma_spec.np);
+ of_dma_put_controller(ofdma);
+
+ of_node_put(dma_spec.np);
+
+ if (chan) {
+ of_node_put(parent);
+ return chan;
+ }
+ } while (dma);

- if (chan)
- return chan;
+ of_node_put(parent);
}

return NULL;
--
1.7.2.5

2013-04-30 11:16:53

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH/RFC 0/4] dmaengine: add DT support for DMA multiplexers

On Tue, Apr 30, 2013 at 09:11:19AM +0200, Guennadi Liakhovetski wrote:
> Previously an issue has been discussed, arising on sh-/r-mobile ARM-based
> systems. There we typically have multiple DMA controller instances with
> exactly equal or very similar capabilities. Each of them can serve the same
> slaves, using the same slave identifiers (request line IDs). With the
> present DMA DT implementation _each_ such DMA slave would have to reference
> _each_ of those DMA controllers in its DMA bindings, e.g.
But why... if that is the case then we havent define DT-bindings clearly enough

And we havent merged that yet, so why not fix that in first set itself

--
~Vinod
>
> mmc0: mmc@10000000 {
> ...
> dmas = <&dma0 0x10
> &dma1 0x10
> &dma2 0x10
> &dma3 0x10
> &dma0 0x11
> &dma1 0x11
> &dma2 0x11
> &dma3 0x11>;
> dma-names = "tx", "tx", "tx", "tx",
> "rx", "rx", "rx", "rx";
> };
>
> Which certainly isn't pretty. To avoid such redundancy it has been proposed
> to implement a DMA multiplexer DT node. That way slaves would just
> reference the multiplexer and one of DMA controller instances in it would
> be picked up automatically to provide DMA channels to slaves. Patches 1-3
> in this series propose such an implementation. Patch 4 is just a minor
> clean up, can be applied independently.
>
> Cc: Guennadi Liakhovetski <[email protected]>
>
> Guennadi Liakhovetski (4):
> OF: add a new phandle parsing function for grouped nodes
> dmaengine: add support for DMA multiplexer DT nodes
> ARM: shmobile: move r8a7740 DMA controller DT node under a "dma-mux"
> node
> OF: modify function stubs to match proper function declarations.
>
> Documentation/devicetree/bindings/dma/dma.txt | 44 ++++++++++++++++++++
> .../boot/dts/r8a7740-armadillo800eva-reference.dts | 12 +++---
> arch/arm/boot/dts/r8a7740.dtsi | 43 +++++++++++--------
> drivers/dma/of-dma.c | 39 +++++++++++++----
> drivers/of/base.c | 28 +++++++++++-
> include/linux/of.h | 20 ++++++++-
> 6 files changed, 147 insertions(+), 39 deletions(-)
>
> --
> 1.7.2.5
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

2013-04-30 12:17:29

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH/RFC 0/4] dmaengine: add DT support for DMA multiplexers

(added Russell to CC, sorry for not including initially)

Hi Vinod

On Tue, 30 Apr 2013, Vinod Koul wrote:

> On Tue, Apr 30, 2013 at 09:11:19AM +0200, Guennadi Liakhovetski wrote:
> > Previously an issue has been discussed, arising on sh-/r-mobile ARM-based
> > systems. There we typically have multiple DMA controller instances with
> > exactly equal or very similar capabilities. Each of them can serve the same
> > slaves, using the same slave identifiers (request line IDs). With the
> > present DMA DT implementation _each_ such DMA slave would have to reference
> > _each_ of those DMA controllers in its DMA bindings, e.g.
> But why... if that is the case then we havent define DT-bindings clearly enough

Sorry, what do you mean "why?" Why each slave has to reference each DMA
controller? We have discussed this A LOT before... My understanding is,
that we decided, that the sh-/r-mobile case of multiple equal DMA
controllers is an exception and that we don't want to punish everyone for
it. So, the design includes only explicit requesting of specific DMA
request lines on specific DMA controllers, no wild-cards. If a slave DMA
channel can be provided by several DMA controllers we decided to list them
all explicitly too. And for the sh-/r-mobile case a DMA-mux DT node has
been proposed. This is exactly what this patch series is implementing. Is
my understanding wrong?

> And we havent merged that yet, so why not fix that in first set itself

Sorry, don't understand. The series isn't merged yet, that's right. That's
why I explicitly mention this dependency here. But this isn't a fix. This
is a new feature. The first patch-series only touches a specific DMA
controller driver and relevant platforms. No core changes, so, it's not
that intrusive and can be applied quickly. Whereas this series affects the
core and might need a more careful consideration, discussion, etc.

Thanks
Guennadi

> --
> ~Vinod
> >
> > mmc0: mmc@10000000 {
> > ...
> > dmas = <&dma0 0x10
> > &dma1 0x10
> > &dma2 0x10
> > &dma3 0x10
> > &dma0 0x11
> > &dma1 0x11
> > &dma2 0x11
> > &dma3 0x11>;
> > dma-names = "tx", "tx", "tx", "tx",
> > "rx", "rx", "rx", "rx";
> > };
> >
> > Which certainly isn't pretty. To avoid such redundancy it has been proposed
> > to implement a DMA multiplexer DT node. That way slaves would just
> > reference the multiplexer and one of DMA controller instances in it would
> > be picked up automatically to provide DMA channels to slaves. Patches 1-3
> > in this series propose such an implementation. Patch 4 is just a minor
> > clean up, can be applied independently.
> >
> > Cc: Guennadi Liakhovetski <[email protected]>
> >
> > Guennadi Liakhovetski (4):
> > OF: add a new phandle parsing function for grouped nodes
> > dmaengine: add support for DMA multiplexer DT nodes
> > ARM: shmobile: move r8a7740 DMA controller DT node under a "dma-mux"
> > node
> > OF: modify function stubs to match proper function declarations.
> >
> > Documentation/devicetree/bindings/dma/dma.txt | 44 ++++++++++++++++++++
> > .../boot/dts/r8a7740-armadillo800eva-reference.dts | 12 +++---
> > arch/arm/boot/dts/r8a7740.dtsi | 43 +++++++++++--------
> > drivers/dma/of-dma.c | 39 +++++++++++++----
> > drivers/of/base.c | 28 +++++++++++-
> > include/linux/of.h | 20 ++++++++-
> > 6 files changed, 147 insertions(+), 39 deletions(-)
> >
> > --
> > 1.7.2.5
> >
> > Thanks
> > Guennadi
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
>

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

2013-05-02 17:02:30

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH/RFC 0/4] dmaengine: add DT support for DMA multiplexers

On Tue, Apr 30, 2013 at 02:17:13PM +0200, Guennadi Liakhovetski wrote:
> (added Russell to CC, sorry for not including initially)
>
> Hi Vinod
>
> On Tue, 30 Apr 2013, Vinod Koul wrote:
>
> > On Tue, Apr 30, 2013 at 09:11:19AM +0200, Guennadi Liakhovetski wrote:
> > > Previously an issue has been discussed, arising on sh-/r-mobile ARM-based
> > > systems. There we typically have multiple DMA controller instances with
> > > exactly equal or very similar capabilities. Each of them can serve the same
> > > slaves, using the same slave identifiers (request line IDs). With the
> > > present DMA DT implementation _each_ such DMA slave would have to reference
> > > _each_ of those DMA controllers in its DMA bindings, e.g.
> > But why... if that is the case then we havent define DT-bindings clearly enough
>
> Sorry, what do you mean "why?" Why each slave has to reference each DMA
> controller? We have discussed this A LOT before... My understanding is,
> that we decided, that the sh-/r-mobile case of multiple equal DMA
> controllers is an exception and that we don't want to punish everyone for
> it. So, the design includes only explicit requesting of specific DMA
> request lines on specific DMA controllers, no wild-cards. If a slave DMA
> channel can be provided by several DMA controllers we decided to list them
> all explicitly too. And for the sh-/r-mobile case a DMA-mux DT node has
> been proposed. This is exactly what this patch series is implementing. Is
> my understanding wrong?
>
> > And we havent merged that yet, so why not fix that in first set itself
>
> Sorry, don't understand. The series isn't merged yet, that's right. That's
> why I explicitly mention this dependency here. But this isn't a fix. This
> is a new feature. The first patch-series only touches a specific DMA
> controller driver and relevant platforms. No core changes, so, it's not
> that intrusive and can be applied quickly. Whereas this series affects the
> core and might need a more careful consideration, discussion, etc.
What i mean from above is if we were already defining the sh-DT binding then why
wasnt this taken care in the orignal definition?

It would make sense to have proper binding which works well for both of these
case, why a two shot approach?

--
~Vinod

2013-05-02 20:46:30

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH/RFC 0/4] dmaengine: add DT support for DMA multiplexers

On Thu, 2 May 2013, Vinod Koul wrote:

> On Tue, Apr 30, 2013 at 02:17:13PM +0200, Guennadi Liakhovetski wrote:
> > (added Russell to CC, sorry for not including initially)
> >
> > Hi Vinod
> >
> > On Tue, 30 Apr 2013, Vinod Koul wrote:
> >
> > > On Tue, Apr 30, 2013 at 09:11:19AM +0200, Guennadi Liakhovetski wrote:
> > > > Previously an issue has been discussed, arising on sh-/r-mobile ARM-based
> > > > systems. There we typically have multiple DMA controller instances with
> > > > exactly equal or very similar capabilities. Each of them can serve the same
> > > > slaves, using the same slave identifiers (request line IDs). With the
> > > > present DMA DT implementation _each_ such DMA slave would have to reference
> > > > _each_ of those DMA controllers in its DMA bindings, e.g.
> > > But why... if that is the case then we havent define DT-bindings clearly enough
> >
> > Sorry, what do you mean "why?" Why each slave has to reference each DMA
> > controller? We have discussed this A LOT before... My understanding is,
> > that we decided, that the sh-/r-mobile case of multiple equal DMA
> > controllers is an exception and that we don't want to punish everyone for
> > it. So, the design includes only explicit requesting of specific DMA
> > request lines on specific DMA controllers, no wild-cards. If a slave DMA
> > channel can be provided by several DMA controllers we decided to list them
> > all explicitly too. And for the sh-/r-mobile case a DMA-mux DT node has
> > been proposed. This is exactly what this patch series is implementing. Is
> > my understanding wrong?
> >
> > > And we havent merged that yet, so why not fix that in first set itself
> >
> > Sorry, don't understand. The series isn't merged yet, that's right. That's
> > why I explicitly mention this dependency here. But this isn't a fix. This
> > is a new feature. The first patch-series only touches a specific DMA
> > controller driver and relevant platforms. No core changes, so, it's not
> > that intrusive and can be applied quickly. Whereas this series affects the
> > core and might need a more careful consideration, discussion, etc.
> What i mean from above is if we were already defining the sh-DT binding then why
> wasnt this taken care in the orignal definition?
>
> It would make sense to have proper binding which works well for both of these
> case, why a two shot approach?

In the first patch-series we implement DT support in shdma according to
the existing API and add DT nodes to a platform. This is already a
functioning implementation. The second patch-set is a new feature. It adds
a new API - a DMA multiplexer and implementation examples. This way we can
clearly show how the new API shall be used - what _changes_ are required
to existing DMA DT implementations to support it. I think it makes sense
to do this in two steps.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

2013-06-06 06:55:04

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH 4/4] OF: modify function stubs to match proper function declarations.

Hi

On Tue, 30 Apr 2013, Guennadi Liakhovetski wrote:

> of_parse_phandle_with_args() and of_count_phandle_with_args() functions
> are declared with their first parameter as const. However, their
> respective stubs, used when CONFIG_OF isn't defined, don't have the "const"
> modifier. This patch adds it to fix the mismatch.
>
> Signed-off-by: Guennadi Liakhovetski <[email protected]>
> ---
> include/linux/of.h | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)

This patch is a trivial fix and doesn't actually depend on others in this
series, any reason it hasn't been applied yet?

Thanks
Guennadi

> diff --git a/include/linux/of.h b/include/linux/of.h
> index 30ae71f..d9dbf73 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -466,7 +466,7 @@ static inline struct device_node *of_parse_phandle(const struct device_node *np,
> return NULL;
> }
>
> -static inline int of_parse_phandle_with_args(struct device_node *np,
> +static inline int of_parse_phandle_with_args(const struct device_node *np,
> const char *list_name,
> const char *cells_name,
> int index,
> @@ -485,7 +485,7 @@ static inline int of_parse_phandle_with_child_args(const struct device_node *np,
> return -ENOSYS;
> }
>
> -static inline int of_count_phandle_with_args(struct device_node *np,
> +static inline int of_count_phandle_with_args(const struct device_node *np,
> const char *list_name,
> const char *cells_name)
> {
> --
> 1.7.2.5
>

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/