diff mbox

[v6,2/8] Documentation: of: Document graph bindings

Message ID 1394443690.7380.10.camel@paszta.hi.pengutronix.de
State Superseded, archived
Headers show

Commit Message

Philipp Zabel March 10, 2014, 9:28 a.m. UTC
Hi Grant,

Am Freitag, den 07.03.2014, 18:27 +0000 schrieb Grant Likely:
> On Wed,  5 Mar 2014 10:20:36 +0100, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > The device tree graph bindings as used by V4L2 and documented in
> > Documentation/device-tree/bindings/media/video-interfaces.txt contain
> > generic parts that are not media specific but could be useful for any
> > subsystem with data flow between multiple devices. This document
> > describes the generic bindings.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> See my comments on the previous version. My concerns are the handling of
> the optional 'ports' node and the usage of reverse links.

would this change address your concern about the reverse links? As the
preexisting video-interfaces.txt bindings mandate the reverse links, I
worry about introducing a second, subtly different binding. It should be
noted somewhere in video-interfaces.txt that the reverse links are
deprecated for the but still supported by the code for backwards
compatibility.


regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Laurent Pinchart March 10, 2014, 11:37 a.m. UTC | #1
Hi Philipp,

On Monday 10 March 2014 10:28:10 Philipp Zabel wrote:
> Hi Grant,
> 
> Am Freitag, den 07.03.2014, 18:27 +0000 schrieb Grant Likely:
> > On Wed,  5 Mar 2014 10:20:36 +0100, Philipp Zabel wrote:
> > > The device tree graph bindings as used by V4L2 and documented in
> > > Documentation/device-tree/bindings/media/video-interfaces.txt contain
> > > generic parts that are not media specific but could be useful for any
> > > subsystem with data flow between multiple devices. This document
> > > describes the generic bindings.
> > > 
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > 
> > See my comments on the previous version. My concerns are the handling of
> > the optional 'ports' node and the usage of reverse links.
> 
> would this change address your concern about the reverse links? As the
> preexisting video-interfaces.txt bindings mandate the reverse links, I
> worry about introducing a second, subtly different binding. It should be
> noted somewhere in video-interfaces.txt that the reverse links are
> deprecated for the but still supported by the code for backwards
> compatibility.

I'm very much against removing the reverse links. Without them the graph will 
become much more complex to parse. You can try to convince me, but for now I'm 
afraid it's a NACK.

> diff --git a/Documentation/devicetree/bindings/graph.txt
> b/Documentation/devicetree/bindings/graph.txt index 1a69c07..eb6cae5 100644
> --- a/Documentation/devicetree/bindings/graph.txt
> +++ b/Documentation/devicetree/bindings/graph.txt
> @@ -87,12 +87,13 @@ device {
>  Links between endpoints
>  -----------------------
> 
> -Each endpoint should contain a 'remote-endpoint' phandle property that
> points -to the corresponding endpoint in the port of the remote device. In
> turn, the -remote endpoint should contain a 'remote-endpoint' property. If
> it has one, -it must not point to another than the local endpoint. Two
> endpoints with their -'remote-endpoint' phandles pointing at each other
> form a link between the -containing ports.
> +Two endpoint nodes form a link between the two ports they are contained in
> +if one contains a 'remote-endpoint' phandle property, pointing to the other
> +endpoint. The endpoint pointed to should not contain a 'remote-endpoint'
> +property itself. Which direction the phandle should point in depends on
> the +device type. In general, links should be pointing outwards from
> central +devices that provide DMA memory interfaces, such as display
> controller, +video capture interface, or serial digital audio interface
> cores.
> 
>  device-1 {
>          port {
> @@ -104,8 +105,8 @@ device-1 {
> 
>  device-2 {
>          port {
> -                device_2_input: endpoint {
> -                        remote-endpoint = <&device_1_output>;
> +                device_2_input: endpoint { };
> +                       /* no remote-endpoint, this endpoint is pointed at
> */ };
>          };
>  };
Philipp Zabel March 10, 2014, 1:57 p.m. UTC | #2
Am Montag, den 10.03.2014, 12:37 +0100 schrieb Laurent Pinchart:
> Hi Philipp,
> 
> On Monday 10 March 2014 10:28:10 Philipp Zabel wrote:
> > Hi Grant,
> > 
> > Am Freitag, den 07.03.2014, 18:27 +0000 schrieb Grant Likely:
> > > On Wed,  5 Mar 2014 10:20:36 +0100, Philipp Zabel wrote:
> > > > The device tree graph bindings as used by V4L2 and documented in
> > > > Documentation/device-tree/bindings/media/video-interfaces.txt contain
> > > > generic parts that are not media specific but could be useful for any
> > > > subsystem with data flow between multiple devices. This document
> > > > describes the generic bindings.
> > > > 
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > 
> > > See my comments on the previous version. My concerns are the handling of
> > > the optional 'ports' node and the usage of reverse links.
> > 
> > would this change address your concern about the reverse links? As the
> > preexisting video-interfaces.txt bindings mandate the reverse links, I
> > worry about introducing a second, subtly different binding. It should be
> > noted somewhere in video-interfaces.txt that the reverse links are
> > deprecated for the but still supported by the code for backwards
> > compatibility.
> 
> I'm very much against removing the reverse links. Without them the graph will 
> become much more complex to parse. You can try to convince me, but for now I'm 
> afraid it's a NACK.

For the record, I'd prefer to keep them, too. Besides the parsing
complexity, it just feels more natural to take both ends and connect
them together. If phandles are only permitted to point in one direction,
there's always the additional question which direction is the right one.

Assume, for example, the following setup of two SoC digital audio
interfaces connected to an audio codec and a bluetooth chip,
respectively. The audio codec has a second audio interface that is
connected directly to the bluetooth chip for headset operation:

,-------.     ,--------.
| dai1 [0]---[0] codec |
`-------'  ,-[1]       |
           |  `--------'
           |  ,-----.
,-------.  `-[1] bt |
| dai2 [0]---[0]    |
`-------'     `-----ยด

How to decide which direction the codec:1 <-->  bt:1 link should point
in?

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/graph.txt b/Documentation/devicetree/bindings/graph.txt
index 1a69c07..eb6cae5 100644
--- a/Documentation/devicetree/bindings/graph.txt
+++ b/Documentation/devicetree/bindings/graph.txt
@@ -87,12 +87,13 @@  device {
 Links between endpoints
 -----------------------
 
-Each endpoint should contain a 'remote-endpoint' phandle property that points
-to the corresponding endpoint in the port of the remote device. In turn, the
-remote endpoint should contain a 'remote-endpoint' property. If it has one,
-it must not point to another than the local endpoint. Two endpoints with their
-'remote-endpoint' phandles pointing at each other form a link between the
-containing ports.
+Two endpoint nodes form a link between the two ports they are contained in
+if one contains a 'remote-endpoint' phandle property, pointing to the other
+endpoint. The endpoint pointed to should not contain a 'remote-endpoint'
+property itself. Which direction the phandle should point in depends on the
+device type. In general, links should be pointing outwards from central
+devices that provide DMA memory interfaces, such as display controller,
+video capture interface, or serial digital audio interface cores.
 
 device-1 {
         port {
@@ -104,8 +105,8 @@  device-1 {
 
 device-2 {
         port {
-                device_2_input: endpoint {
-                        remote-endpoint = <&device_1_output>;
+                device_2_input: endpoint { };
+                       /* no remote-endpoint, this endpoint is pointed at */
                 };
         };
 };