Hi Rob,

Am Dienstag, den 11.02.2014, 07:56 -0600 schrieb Rob Herring:

On Tue, Feb 11, 2014 at 5:45 AM, Philipp Zabel [off-list ref] wrote:

quoted

From: Philipp Zabel <redacted>

This patch moves the parsing helpers used to parse connected graphs
in the device tree, like the video interface bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt, from
drivers/media/v4l2-core to drivers/of.

This is the opposite direction things have been moving...

I understand subsystem specific functionality is moving from drivers/of
into the subsystems. In this case three subsystems all could benefit
from the same set of parsing functions, so it is not clear to me where
if not drivers/of would be the correct place for this code.

quoted

This allows to reuse the same parser code from outside the V4L2 framework,
most importantly from display drivers. There have been patches that duplicate
the code (and I am going to send one of my own), such as
http://lists.freedesktop.org/archives/dri-devel/2013-August/043308.html
and others that parse the same binding in a different way:
https://www.mail-archive.com/linux-omap@vger.kernel.org/msg100761.html

I think that all common video interface parsing helpers should be moved to a
single place, outside of the specific subsystems, so that it can be reused
by all drivers.

Perhaps that should be done rather than moving to drivers/of now and
then again to somewhere else.

quoted

I moved v4l2_of_get_next_endpoint, v4l2_of_get_remote_port,
and v4l2_of_get_remote_port_parent. They are renamed to
of_graph_get_next_endpoint, of_graph_get_remote_port, and
of_graph_get_remote_port_parent, respectively.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/Kconfig             |   1 +
 drivers/media/v4l2-core/v4l2-of.c | 117 ---------------------------------
 drivers/of/Kconfig                |   3 +
 drivers/of/Makefile               |   1 +
 drivers/of/of_graph.c             | 133 ++++++++++++++++++++++++++++++++++++++
 include/linux/of_graph.h          |  23 +++++++
 include/media/v4l2-of.h           |  16 ++---
 7 files changed, 167 insertions(+), 127 deletions(-)
 create mode 100644 drivers/of/of_graph.c
 create mode 100644 include/linux/of_graph.h

[snip]

quoted

diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 541cea4..404a493 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h

@@ -17,6 +17,7 @@
 #include <linux/list.h>
 #include <linux/types.h>
 #include <linux/errno.h>
+#include <linux/of_graph.h>

 #include <media/v4l2-mediabus.h>

@@ -72,11 +73,6 @@ struct v4l2_of_endpoint {
 #ifdef CONFIG_OF
 int v4l2_of_parse_endpoint(const struct device_node *node,
                           struct v4l2_of_endpoint *endpoint);
-struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
-                                       struct device_node *previous);
-struct device_node *v4l2_of_get_remote_port_parent(
-                                       const struct device_node *node);
-struct device_node *v4l2_of_get_remote_port(const struct device_node *node);
 #else /* CONFIG_OF */

 static inline int v4l2_of_parse_endpoint(const struct device_node *node,

@@ -85,25 +81,25 @@ static inline int v4l2_of_parse_endpoint(const struct device_node *node,
        return -ENOSYS;
 }

+#endif /* CONFIG_OF */
+
 static inline struct device_node *v4l2_of_get_next_endpoint(
                                        const struct device_node *parent,
                                        struct device_node *previous)
 {
-       return NULL;
+       return of_graph_get_next_endpoint(parent, previous);

Won't this break for !OF?

It will. The of_graph_* functions should get their own stubs for that
case.

regards
Philipp