Hi,
this version of the OF graph helper move series addresses a few of
Grant's and Tomi's comments.
Changes since v4:
- Moved graph helpers into drivers/of/base.c
- Fixed endpoint parsing patch to update users
- Improved documentation, emphasizing features that differentiate
the graph bindings from simple phandle graphs. Made it clear that
this is not necessarily specific to data connections
- Added cleanups to of_graph_get_next_endpoint routine
- Added simplified binding for single port devices
regards
Philipp
Philipp Zabel (7):
[media] of: move graph helpers from drivers/media/v4l2-core to
drivers/of
Documentation: of: Document graph bindings
of: Warn if of_graph_get_next_endpoint is called with the root node
of: Reduce indentation in of_graph_get_next_endpoint
[media] of: move common endpoint parsing to drivers/of
of: Implement simplified graph binding for single port devices
of: Document simplified graph binding for single port devices
Documentation/devicetree/bindings/graph.txt | 137 +++++++++++++++++++++
drivers/media/i2c/adv7343.c | 4 +-
drivers/media/i2c/mt9p031.c | 4 +-
drivers/media/i2c/s5k5baf.c | 3 +-
drivers/media/i2c/tvp514x.c | 3 +-
drivers/media/i2c/tvp7002.c | 3 +-
drivers/media/platform/exynos4-is/fimc-is.c | 6 +-
drivers/media/platform/exynos4-is/media-dev.c | 13 +-
drivers/media/platform/exynos4-is/mipi-csis.c | 5 +-
drivers/media/v4l2-core/v4l2-of.c | 133 +--------------------
drivers/of/base.c | 165 ++++++++++++++++++++++++++
include/linux/of_graph.h | 66 +++++++++++
include/media/v4l2-of.h | 33 +-----
13 files changed, 397 insertions(+), 178 deletions(-)
create mode 100644 Documentation/devicetree/bindings/graph.txt
create mode 100644 include/linux/of_graph.h
--
1.8.5.3
If of_graph_get_next_endpoint is given a parentless node instead of an
endpoint node, it is clearly a bug.
Signed-off-by: Philipp Zabel <[email protected]>
---
drivers/of/base.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index b2f223f..6e650cf 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2028,8 +2028,8 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
of_node_put(node);
} else {
port = of_get_parent(prev);
- if (!port)
- /* Hm, has someone given us the root node ?... */
+ if (WARN_ONCE(!port, "%s(): endpoint has no parent node\n",
+ __func__))
return NULL;
/* Avoid dropping prev node refcount to 0. */
--
1.8.5.3
For simple devices with only one port, it can be made implicit.
The endpoint node can be a direct child of the device node.
Signed-off-by: Philipp Zabel <[email protected]>
---
Documentation/devicetree/bindings/graph.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/graph.txt b/Documentation/devicetree/bindings/graph.txt
index 554865b..6dfaff8 100644
--- a/Documentation/devicetree/bindings/graph.txt
+++ b/Documentation/devicetree/bindings/graph.txt
@@ -84,6 +84,14 @@ device {
};
};
+For devices with only a single port and a single endpoint, this can be further
+simplified by making the port implicit, and adding the endpoint node as a direct
+child of the device node.
+
+device {
+ endpoint { ... };
+};
+
Links between endpoints
-----------------------
--
1.8.5.3
This patch adds a new struct of_endpoint which is then embedded in struct
v4l2_of_endpoint and contains the endpoint properties that are not V4L2
(or even media) specific: the port number, endpoint id, local device tree
node and remote endpoint phandle. of_graph_parse_endpoint parses those
properties and is used by v4l2_of_parse_endpoint, which just adds the
V4L2 MBUS information to the containing v4l2_of_endpoint structure.
Signed-off-by: Philipp Zabel <[email protected]>
---
Changes since v4:
- Fixed users of struct v4l2_of_endpoint
- Removed left-over #include <media/of_graph.h> from v4l2-of.h
---
drivers/media/platform/exynos4-is/media-dev.c | 10 ++++-----
drivers/media/platform/exynos4-is/mipi-csis.c | 2 +-
drivers/media/v4l2-core/v4l2-of.c | 16 +++-----------
drivers/of/base.c | 31 +++++++++++++++++++++++++++
include/linux/of_graph.h | 20 +++++++++++++++++
include/media/v4l2-of.h | 8 ++-----
6 files changed, 62 insertions(+), 25 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index d0f82da..04d6ecd 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -469,10 +469,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
return 0;
v4l2_of_parse_endpoint(ep, &endpoint);
- if (WARN_ON(endpoint.port == 0) || index >= FIMC_MAX_SENSORS)
+ if (WARN_ON(endpoint.base.port == 0) || index >= FIMC_MAX_SENSORS)
return -EINVAL;
- pd->mux_id = (endpoint.port - 1) & 0x1;
+ pd->mux_id = (endpoint.base.port - 1) & 0x1;
rem = of_graph_get_remote_port_parent(ep);
of_node_put(ep);
@@ -494,13 +494,13 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
return -EINVAL;
}
- if (fimc_input_is_parallel(endpoint.port)) {
+ if (fimc_input_is_parallel(endpoint.base.port)) {
if (endpoint.bus_type == V4L2_MBUS_PARALLEL)
pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_601;
else
pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_656;
pd->flags = endpoint.bus.parallel.flags;
- } else if (fimc_input_is_mipi_csi(endpoint.port)) {
+ } else if (fimc_input_is_mipi_csi(endpoint.base.port)) {
/*
* MIPI CSI-2: only input mux selection and
* the sensor's clock frequency is needed.
@@ -508,7 +508,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
pd->sensor_bus_type = FIMC_BUS_TYPE_MIPI_CSI2;
} else {
v4l2_err(&fmd->v4l2_dev, "Wrong port id (%u) at node %s\n",
- endpoint.port, rem->full_name);
+ endpoint.base.port, rem->full_name);
}
/*
* For FIMC-IS handled sensors, that are placed under i2c-isp device
diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index fd1ae65..3678ba5 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -772,7 +772,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
/* Get port node and validate MIPI-CSI channel id. */
v4l2_of_parse_endpoint(node, &endpoint);
- state->index = endpoint.port - FIMC_INPUT_MIPI_CSI2_0;
+ state->index = endpoint.base.port - FIMC_INPUT_MIPI_CSI2_0;
if (state->index < 0 || state->index >= CSIS_MAX_ENTITIES)
return -ENXIO;
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index f919db3..b4ed9a9 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -127,17 +127,9 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
int v4l2_of_parse_endpoint(const struct device_node *node,
struct v4l2_of_endpoint *endpoint)
{
- struct device_node *port_node = of_get_parent(node);
-
- memset(endpoint, 0, offsetof(struct v4l2_of_endpoint, head));
-
- endpoint->local_node = node;
- /*
- * It doesn't matter whether the two calls below succeed.
- * If they don't then the default value 0 is used.
- */
- of_property_read_u32(port_node, "reg", &endpoint->port);
- of_property_read_u32(node, "reg", &endpoint->id);
+ of_graph_parse_endpoint(node, &endpoint->base);
+ endpoint->bus_type = 0;
+ memset(&endpoint->bus, 0, sizeof(endpoint->bus));
v4l2_of_parse_csi_bus(node, endpoint);
/*
@@ -147,8 +139,6 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
if (endpoint->bus.mipi_csi2.flags == 0)
v4l2_of_parse_parallel_bus(node, endpoint);
- of_node_put(port_node);
-
return 0;
}
EXPORT_SYMBOL(v4l2_of_parse_endpoint);
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8ecca7a..ba3cfca 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1985,6 +1985,37 @@ struct device_node *of_find_next_cache_node(const struct device_node *np)
}
/**
+ * of_graph_parse_endpoint() - parse common endpoint node properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to the OF endpoint data structure
+ *
+ * All properties are optional. If none are found, we don't set any flags.
+ * This means the port has a static configuration and no properties have
+ * to be specified explicitly.
+ * The caller should hold a reference to @node.
+ */
+int of_graph_parse_endpoint(const struct device_node *node,
+ struct of_endpoint *endpoint)
+{
+ struct device_node *port_node = of_get_parent(node);
+
+ memset(endpoint, 0, sizeof(*endpoint));
+
+ endpoint->local_node = node;
+ /*
+ * It doesn't matter whether the two calls below succeed.
+ * If they don't then the default value 0 is used.
+ */
+ of_property_read_u32(port_node, "reg", &endpoint->port);
+ of_property_read_u32(node, "reg", &endpoint->id);
+
+ of_node_put(port_node);
+
+ return 0;
+}
+EXPORT_SYMBOL(of_graph_parse_endpoint);
+
+/**
* of_graph_get_next_endpoint() - get next endpoint node
* @parent: pointer to the parent device node
* @prev: previous endpoint node, or NULL to get first
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index 3bbeb60..2b233db 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -14,7 +14,21 @@
#ifndef __LINUX_OF_GRAPH_H
#define __LINUX_OF_GRAPH_H
+/**
+ * struct of_endpoint - the OF graph endpoint data structure
+ * @port: identifier (value of reg property) of a port this endpoint belongs to
+ * @id: identifier (value of reg property) of this endpoint
+ * @local_node: pointer to device_node of this endpoint
+ */
+struct of_endpoint {
+ unsigned int port;
+ unsigned int id;
+ const struct device_node *local_node;
+};
+
#ifdef CONFIG_OF
+int of_graph_parse_endpoint(const struct device_node *node,
+ struct of_endpoint *endpoint);
struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
struct device_node *previous);
struct device_node *of_graph_get_remote_port_parent(
@@ -22,6 +36,12 @@ struct device_node *of_graph_get_remote_port_parent(
struct device_node *of_graph_get_remote_port(const struct device_node *node);
#else
+static inline int of_graph_parse_endpoint(const struct device_node *node,
+ struct of_endpoint *endpoint);
+{
+ return -ENOSYS;
+}
+
static inline struct device_node *of_graph_get_next_endpoint(
const struct device_node *parent,
struct device_node *previous)
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 3a49735..70fa7b7 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h
@@ -51,17 +51,13 @@ struct v4l2_of_bus_parallel {
/**
* struct v4l2_of_endpoint - the endpoint data structure
- * @port: identifier (value of reg property) of a port this endpoint belongs to
- * @id: identifier (value of reg property) of this endpoint
- * @local_node: pointer to device_node of this endpoint
+ * @base: struct of_endpoint containing port, id, and local of_node
* @bus_type: bus type
* @bus: bus configuration data structure
* @head: list head for this structure
*/
struct v4l2_of_endpoint {
- unsigned int port;
- unsigned int id;
- const struct device_node *local_node;
+ struct of_endpoint base;
enum v4l2_mbus_type bus_type;
union {
struct v4l2_of_bus_parallel parallel;
--
1.8.5.3
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/v4l2-of.c into drivers/of/base.c.
This allows to reuse the same parser code from outside the V4L2
framework, most importantly from display drivers.
The functions v4l2_of_get_next_endpoint, v4l2_of_get_remote_port,
and v4l2_of_get_remote_port_parent are moved. They are renamed to
of_graph_get_next_endpoint, of_graph_get_remote_port, and
of_graph_get_remote_port_parent, respectively.
Since there are not that many current users yet, switch all of
them to the new functions right away.
Signed-off-by: Philipp Zabel <[email protected]>
---
Changes since v4:
- Moved into drivers/of/base.c instead of creating of_graph.c
---
drivers/media/i2c/adv7343.c | 4 +-
drivers/media/i2c/mt9p031.c | 4 +-
drivers/media/i2c/s5k5baf.c | 3 +-
drivers/media/i2c/tvp514x.c | 3 +-
drivers/media/i2c/tvp7002.c | 3 +-
drivers/media/platform/exynos4-is/fimc-is.c | 6 +-
drivers/media/platform/exynos4-is/media-dev.c | 3 +-
drivers/media/platform/exynos4-is/mipi-csis.c | 3 +-
drivers/media/v4l2-core/v4l2-of.c | 117 -------------------------
drivers/of/base.c | 118 ++++++++++++++++++++++++++
include/linux/of_graph.h | 46 ++++++++++
include/media/v4l2-of.h | 25 +-----
12 files changed, 182 insertions(+), 153 deletions(-)
create mode 100644 include/linux/of_graph.h
diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c
index d4e15a6..9d38f7b 100644
--- a/drivers/media/i2c/adv7343.c
+++ b/drivers/media/i2c/adv7343.c
@@ -26,12 +26,12 @@
#include <linux/videodev2.h>
#include <linux/uaccess.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <media/adv7343.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ctrls.h>
-#include <media/v4l2-of.h>
#include "adv7343_regs.h"
@@ -410,7 +410,7 @@ adv7343_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!np)
return NULL;
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index e5ddf47..192c4aa 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -21,6 +21,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/of_graph.h>
#include <linux/pm.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -29,7 +30,6 @@
#include <media/mt9p031.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
-#include <media/v4l2-of.h>
#include <media/v4l2-subdev.h>
#include "aptina-pll.h"
@@ -943,7 +943,7 @@ mt9p031_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!np)
return NULL;
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 77e10e0..2d768ef 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -21,6 +21,7 @@
#include <linux/media.h>
#include <linux/module.h>
#include <linux/of_gpio.h>
+#include <linux/of_graph.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -1855,7 +1856,7 @@ static int s5k5baf_parse_device_node(struct s5k5baf *state, struct device *dev)
if (ret < 0)
return ret;
- node_ep = v4l2_of_get_next_endpoint(node, NULL);
+ node_ep = of_graph_get_next_endpoint(node, NULL);
if (!node_ep) {
dev_err(dev, "no endpoint defined at node %s\n",
node->full_name);
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 83d85df..ca00117 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -36,6 +36,7 @@
#include <linux/module.h>
#include <linux/v4l2-mediabus.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
@@ -1068,7 +1069,7 @@ tvp514x_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!endpoint)
return NULL;
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 912e1cc..c4e1e2c 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -30,6 +30,7 @@
#include <linux/videodev2.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/v4l2-dv-timings.h>
#include <media/tvp7002.h>
#include <media/v4l2-async.h>
@@ -957,7 +958,7 @@ tvp7002_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!endpoint)
return NULL;
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index 13a4228..9bdfa45 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -24,13 +24,13 @@
#include <linux/i2c.h>
#include <linux/of_irq.h>
#include <linux/of_address.h>
+#include <linux/of_graph.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/videodev2.h>
-#include <media/v4l2-of.h>
#include <media/videobuf2-dma-contig.h>
#include "media-dev.h"
@@ -167,10 +167,10 @@ static int fimc_is_parse_sensor_config(struct fimc_is_sensor *sensor,
u32 tmp = 0;
int ret;
- np = v4l2_of_get_next_endpoint(np, NULL);
+ np = of_graph_get_next_endpoint(np, NULL);
if (!np)
return -ENXIO;
- np = v4l2_of_get_remote_port(np);
+ np = of_graph_get_remote_port(np);
if (!np)
return -ENXIO;
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index c1bce17..d0f82da 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -20,6 +20,7 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/of_device.h>
+#include <linux/of_graph.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/types.h>
@@ -473,7 +474,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
pd->mux_id = (endpoint.port - 1) & 0x1;
- rem = v4l2_of_get_remote_port_parent(ep);
+ rem = of_graph_get_remote_port_parent(ep);
of_node_put(ep);
if (rem == NULL) {
v4l2_info(&fmd->v4l2_dev, "Remote device at %s not found\n",
diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index f3c3591..fd1ae65 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -20,6 +20,7 @@
#include <linux/memory.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/phy/phy.h>
#include <linux/platform_data/mipi-csis.h>
#include <linux/platform_device.h>
@@ -762,7 +763,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
&state->max_num_lanes))
return -EINVAL;
- node = v4l2_of_get_next_endpoint(node, NULL);
+ node = of_graph_get_next_endpoint(node, NULL);
if (!node) {
dev_err(&pdev->dev, "No port node at %s\n",
pdev->dev.of_node->full_name);
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index 42e3e8a..f919db3 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -152,120 +152,3 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
return 0;
}
EXPORT_SYMBOL(v4l2_of_parse_endpoint);
-
-/**
- * v4l2_of_get_next_endpoint() - get next endpoint node
- * @parent: pointer to the parent device node
- * @prev: previous endpoint node, or NULL to get first
- *
- * Return: An 'endpoint' node pointer with refcount incremented. Refcount
- * of the passed @prev node is not decremented, the caller have to use
- * of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
- struct device_node *prev)
-{
- struct device_node *endpoint;
- struct device_node *port = NULL;
-
- if (!parent)
- return NULL;
-
- if (!prev) {
- struct device_node *node;
- /*
- * It's the first call, we have to find a port subnode
- * within this node or within an optional 'ports' node.
- */
- node = of_get_child_by_name(parent, "ports");
- if (node)
- parent = node;
-
- port = of_get_child_by_name(parent, "port");
-
- if (port) {
- /* Found a port, get an endpoint. */
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- } else {
- endpoint = NULL;
- }
-
- if (!endpoint)
- pr_err("%s(): no endpoint nodes specified for %s\n",
- __func__, parent->full_name);
- of_node_put(node);
- } else {
- port = of_get_parent(prev);
- if (!port)
- /* Hm, has someone given us the root node ?... */
- return NULL;
-
- /* Avoid dropping prev node refcount to 0. */
- of_node_get(prev);
- endpoint = of_get_next_child(port, prev);
- if (endpoint) {
- of_node_put(port);
- return endpoint;
- }
-
- /* No more endpoints under this port, try the next one. */
- do {
- port = of_get_next_child(parent, port);
- if (!port)
- return NULL;
- } while (of_node_cmp(port->name, "port"));
-
- /* Pick up the first endpoint in this port. */
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- }
-
- return endpoint;
-}
-EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
-
-/**
- * v4l2_of_get_remote_port_parent() - get remote port's parent node
- * @node: pointer to a local endpoint device_node
- *
- * Return: Remote device node associated with remote endpoint node linked
- * to @node. Use of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node)
-{
- struct device_node *np;
- unsigned int depth;
-
- /* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
-
- /* Walk 3 levels up only if there is 'ports' node. */
- for (depth = 3; depth && np; depth--) {
- np = of_get_next_parent(np);
- if (depth == 2 && of_node_cmp(np->name, "ports"))
- break;
- }
- return np;
-}
-EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
-
-/**
- * v4l2_of_get_remote_port() - get remote port node
- * @node: pointer to a local endpoint device_node
- *
- * Return: Remote port node associated with remote endpoint node linked
- * to @node. Use of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_remote_port(const struct device_node *node)
-{
- struct device_node *np;
-
- /* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
- if (!np)
- return NULL;
- return of_get_next_parent(np);
-}
-EXPORT_SYMBOL(v4l2_of_get_remote_port);
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 89e888a..b2f223f 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -21,6 +21,7 @@
#include <linux/cpu.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/proc_fs.h>
@@ -1982,3 +1983,120 @@ struct device_node *of_find_next_cache_node(const struct device_node *np)
return NULL;
}
+
+/**
+ * of_graph_get_next_endpoint() - get next endpoint node
+ * @parent: pointer to the parent device node
+ * @prev: previous endpoint node, or NULL to get first
+ *
+ * Return: An 'endpoint' node pointer with refcount incremented. Refcount
+ * of the passed @prev node is not decremented, the caller have to use
+ * of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
+ struct device_node *prev)
+{
+ struct device_node *endpoint;
+ struct device_node *port = NULL;
+
+ if (!parent)
+ return NULL;
+
+ if (!prev) {
+ struct device_node *node;
+ /*
+ * It's the first call, we have to find a port subnode
+ * within this node or within an optional 'ports' node.
+ */
+ node = of_get_child_by_name(parent, "ports");
+ if (node)
+ parent = node;
+
+ port = of_get_child_by_name(parent, "port");
+
+ if (port) {
+ /* Found a port, get an endpoint. */
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+ } else {
+ endpoint = NULL;
+ }
+
+ if (!endpoint)
+ pr_err("%s(): no endpoint nodes specified for %s\n",
+ __func__, parent->full_name);
+ of_node_put(node);
+ } else {
+ port = of_get_parent(prev);
+ if (!port)
+ /* Hm, has someone given us the root node ?... */
+ return NULL;
+
+ /* Avoid dropping prev node refcount to 0. */
+ of_node_get(prev);
+ endpoint = of_get_next_child(port, prev);
+ if (endpoint) {
+ of_node_put(port);
+ return endpoint;
+ }
+
+ /* No more endpoints under this port, try the next one. */
+ do {
+ port = of_get_next_child(parent, port);
+ if (!port)
+ return NULL;
+ } while (of_node_cmp(port->name, "port"));
+
+ /* Pick up the first endpoint in this port. */
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+ }
+
+ return endpoint;
+}
+EXPORT_SYMBOL(of_graph_get_next_endpoint);
+
+/**
+ * of_graph_get_remote_port_parent() - get remote port's parent node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote device node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node)
+{
+ struct device_node *np;
+ unsigned int depth;
+
+ /* Get remote endpoint node. */
+ np = of_parse_phandle(node, "remote-endpoint", 0);
+
+ /* Walk 3 levels up only if there is 'ports' node. */
+ for (depth = 3; depth && np; depth--) {
+ np = of_get_next_parent(np);
+ if (depth == 2 && of_node_cmp(np->name, "ports"))
+ break;
+ }
+ return np;
+}
+EXPORT_SYMBOL(of_graph_get_remote_port_parent);
+
+/**
+ * of_graph_get_remote_port() - get remote port node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote port node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_port(const struct device_node *node)
+{
+ struct device_node *np;
+
+ /* Get remote endpoint node. */
+ np = of_parse_phandle(node, "remote-endpoint", 0);
+ if (!np)
+ return NULL;
+ return of_get_next_parent(np);
+}
+EXPORT_SYMBOL(of_graph_get_remote_port);
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
new file mode 100644
index 0000000..3bbeb60
--- /dev/null
+++ b/include/linux/of_graph.h
@@ -0,0 +1,46 @@
+/*
+ * OF graph binding parsing helpers
+ *
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <[email protected]>
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#ifndef __LINUX_OF_GRAPH_H
+#define __LINUX_OF_GRAPH_H
+
+#ifdef CONFIG_OF
+struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
+ struct device_node *previous);
+struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node);
+struct device_node *of_graph_get_remote_port(const struct device_node *node);
+#else
+
+static inline struct device_node *of_graph_get_next_endpoint(
+ const struct device_node *parent,
+ struct device_node *previous)
+{
+ return NULL;
+}
+
+static inline struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
+static inline struct device_node *of_graph_get_remote_port(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
+#endif /* CONFIG_OF */
+
+#endif /* __LINUX_OF_GRAPH_H */
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 541cea4..3a49735 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,6 @@ static inline int v4l2_of_parse_endpoint(const struct device_node *node,
return -ENOSYS;
}
-static inline struct device_node *v4l2_of_get_next_endpoint(
- const struct device_node *parent,
- struct device_node *previous)
-{
- return NULL;
-}
-
-static inline struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node)
-{
- return NULL;
-}
-
-static inline struct device_node *v4l2_of_get_remote_port(
- const struct device_node *node)
-{
- return NULL;
-}
-
#endif /* CONFIG_OF */
#endif /* _V4L2_OF_H */
--
1.8.5.3
For simple devices with only one port, it can be made implicit.
The endpoint node can be a direct child of the device node.
Signed-off-by: Philipp Zabel <[email protected]>
---
drivers/of/base.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ba3cfca..7d9c62b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2037,8 +2037,13 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
struct device_node *node;
/*
* It's the first call, we have to find a port subnode
- * within this node or within an optional 'ports' node.
+ * within this node or within an optional 'ports' node,
+ * or at least a single endpoint.
*/
+ endpoint = of_get_child_by_name(parent, "endpoint");
+ if (endpoint)
+ return endpoint;
+
node = of_get_child_by_name(parent, "ports");
if (node)
parent = node;
@@ -2049,8 +2054,6 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
/* Found a port, get an endpoint. */
endpoint = of_get_next_child(port, NULL);
of_node_put(port);
- } else {
- endpoint = NULL;
}
if (!endpoint)
@@ -2065,6 +2068,10 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
if (WARN_ONCE(!port, "%s(): endpoint has no parent node\n",
__func__))
return NULL;
+ if (port == parent) {
+ of_node_put(port);
+ return NULL;
+ }
/* Avoid dropping prev node refcount to 0. */
of_node_get(prev);
@@ -2105,9 +2112,11 @@ struct device_node *of_graph_get_remote_port_parent(
/* Get remote endpoint node. */
np = of_parse_phandle(node, "remote-endpoint", 0);
- /* Walk 3 levels up only if there is 'ports' node. */
+ /* Walk 3 levels up only if there is 'ports' node */
for (depth = 3; depth && np; depth--) {
np = of_get_next_parent(np);
+ if (depth == 3 && of_node_cmp(np->name, "port"))
+ break;
if (depth == 2 && of_node_cmp(np->name, "ports"))
break;
}
@@ -2130,6 +2139,11 @@ struct device_node *of_graph_get_remote_port(const struct device_node *node)
np = of_parse_phandle(node, "remote-endpoint", 0);
if (!np)
return NULL;
- return of_get_next_parent(np);
+ np = of_get_next_parent(np);
+ if (of_node_cmp(np->name, "port")) {
+ of_node_put(np);
+ return NULL;
+ }
+ return np;
}
EXPORT_SYMBOL(of_graph_get_remote_port);
--
1.8.5.3
A 'return endpoint;' at the end of the (!prev) case allows to
reduce the indentation level of the (prev) case.
Signed-off-by: Philipp Zabel <[email protected]>
---
drivers/of/base.c | 42 ++++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 6e650cf..8ecca7a 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2026,32 +2026,34 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
pr_err("%s(): no endpoint nodes specified for %s\n",
__func__, parent->full_name);
of_node_put(node);
- } else {
- port = of_get_parent(prev);
- if (WARN_ONCE(!port, "%s(): endpoint has no parent node\n",
- __func__))
- return NULL;
- /* Avoid dropping prev node refcount to 0. */
- of_node_get(prev);
- endpoint = of_get_next_child(port, prev);
- if (endpoint) {
- of_node_put(port);
- return endpoint;
- }
+ return endpoint;
+ }
- /* No more endpoints under this port, try the next one. */
- do {
- port = of_get_next_child(parent, port);
- if (!port)
- return NULL;
- } while (of_node_cmp(port->name, "port"));
+ port = of_get_parent(prev);
+ if (WARN_ONCE(!port, "%s(): endpoint has no parent node\n",
+ __func__))
+ return NULL;
- /* Pick up the first endpoint in this port. */
- endpoint = of_get_next_child(port, NULL);
+ /* Avoid dropping prev node refcount to 0. */
+ of_node_get(prev);
+ endpoint = of_get_next_child(port, prev);
+ if (endpoint) {
of_node_put(port);
+ return endpoint;
}
+ /* No more endpoints under this port, try the next one. */
+ do {
+ port = of_get_next_child(parent, port);
+ if (!port)
+ return NULL;
+ } while (of_node_cmp(port->name, "port"));
+
+ /* Pick up the first endpoint in this port. */
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+
return endpoint;
}
EXPORT_SYMBOL(of_graph_get_next_endpoint);
--
1.8.5.3
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
describe the generic bindings.
Signed-off-by: Philipp Zabel <[email protected]>
---
Changes since v4:
- Differentiate from graphs made by simple phandle links
- Do not mention data flow except in video-interfaces example
-
---
Documentation/devicetree/bindings/graph.txt | 129 ++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
create mode 100644 Documentation/devicetree/bindings/graph.txt
diff --git a/Documentation/devicetree/bindings/graph.txt b/Documentation/devicetree/bindings/graph.txt
new file mode 100644
index 0000000..554865b
--- /dev/null
+++ b/Documentation/devicetree/bindings/graph.txt
@@ -0,0 +1,129 @@
+Common bindings for device graphs
+
+General concept
+---------------
+
+The hierarchical organisation of the device tree is well suited to describe
+control flow to devices, but there can be more complex connections between
+devices that work together to form a logical compound device, following an
+arbitrarily complex graph.
+There already is a simple directed graph between devices tree nodes using
+phandle properties pointing to other nodes to describe connections that
+can not be inferred from device tree parent-child relationships. The device
+tree graph bindings described herein abstract more complex devices that can
+have multiple specifiable ports, each of which can be linked to one or more
+ports of other devices.
+
+These common bindings do not contain any information about the direction of
+type of the connections, they just map their existence. Specific properties
+may be described by specialized bindings depending on the type of connection.
+
+To see how this binding applies to video pipelines, for example, see
+Documentation/device-tree/bindings/media/video-interfaces.txt.
+Here the ports describe data interfaces, and the links between them are
+the connecting data buses. A single port with multiple connections can
+correspond to multiple devices being connected to the same physical bus.
+
+Organisation of ports and endpoints
+-----------------------------------
+
+Ports are described by child 'port' nodes contained in the device node.
+Each port node contains an 'endpoint' subnode for each remote device port
+connected to this port. If a single port is connected to more than one
+remote device, an 'endpoint' child node must be provided for each link.
+If more than one port is present in a device node or there is more than one
+endpoint at a port, or a port node needs to be associated with a selected
+hardware interface, a common scheme using '#address-cells', '#size-cells'
+and 'reg' properties is used number the nodes.
+
+device {
+ ...
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ endpoint@0 {
+ reg = <0>;
+ ...
+ };
+ endpoint@1 {
+ reg = <1>;
+ ...
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ endpoint { ... };
+ };
+};
+
+All 'port' nodes can be grouped under an optional 'ports' node, which
+allows to specify #address-cells, #size-cells properties for the 'port'
+nodes independently from any other child device nodes a device might
+have.
+
+device {
+ ...
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ ...
+ endpoint@0 { ... };
+ endpoint@1 { ... };
+ };
+
+ port@1 { ... };
+ };
+};
+
+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.
+
+device_1 {
+ port {
+ device_1_output: endpoint {
+ remote-endpoint = <&device_2_input>;
+ };
+ };
+};
+
+device_1 {
+ port {
+ device_2_input: endpoint {
+ remote-endpoint = <&device_1_output>;
+ };
+ };
+};
+
+
+Required properties
+-------------------
+
+If there is more than one 'port' or more than one 'endpoint' node or 'reg'
+property is present in port and/or endpoint nodes the following properties
+are required in a relevant parent node:
+
+ - #address-cells : number of cells required to define port/endpoint
+ identifier, should be 1.
+ - #size-cells : should be zero.
+
+Optional endpoint properties
+----------------------------
+
+- remote-endpoint: phandle to an 'endpoint' subnode of a remote device node.
+
--
1.8.5.3
Hi Philipp,
Just couple minor comments...
On 02/27/2014 06:35 PM, 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
> describe the generic bindings.
s/describe/describes/
> Signed-off-by: Philipp Zabel<[email protected]>
> ---
> Changes since v4:
> - Differentiate from graphs made by simple phandle links
> - Do not mention data flow except in video-interfaces example
> -
> ---
> Documentation/devicetree/bindings/graph.txt | 129 ++++++++++++++++++++++++++++
> 1 file changed, 129 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/graph.txt
>
> diff --git a/Documentation/devicetree/bindings/graph.txt b/Documentation/devicetree/bindings/graph.txt
> new file mode 100644
> index 0000000..554865b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/graph.txt
> @@ -0,0 +1,129 @@
> +Common bindings for device graphs
> +
> +General concept
> +---------------
> +
> +The hierarchical organisation of the device tree is well suited to describe
> +control flow to devices, but there can be more complex connections between
> +devices that work together to form a logical compound device, following an
> +arbitrarily complex graph.
> +There already is a simple directed graph between devices tree nodes using
> +phandle properties pointing to other nodes to describe connections that
> +can not be inferred from device tree parent-child relationships. The device
> +tree graph bindings described herein abstract more complex devices that can
> +have multiple specifiable ports, each of which can be linked to one or more
> +ports of other devices.
> +
> +These common bindings do not contain any information about the direction of
s/of/or/ ?
> +type of the connections, they just map their existence. Specific properties
> +may be described by specialized bindings depending on the type of connection.
> +
> +To see how this binding applies to video pipelines, for example, see
> +Documentation/device-tree/bindings/media/video-interfaces.txt.
> +Here the ports describe data interfaces, and the links between them are
> +the connecting data buses. A single port with multiple connections can
> +correspond to multiple devices being connected to the same physical bus.
> +
> +Organisation of ports and endpoints
> +-----------------------------------
> +
> +Ports are described by child 'port' nodes contained in the device node.
> +Each port node contains an 'endpoint' subnode for each remote device port
> +connected to this port. If a single port is connected to more than one
> +remote device, an 'endpoint' child node must be provided for each link.
> +If more than one port is present in a device node or there is more than one
> +endpoint at a port, or a port node needs to be associated with a selected
> +hardware interface, a common scheme using '#address-cells', '#size-cells'
> +and 'reg' properties is used number the nodes.
> +
> +device {
> + ...
> + #address-cells =<1>;
> + #size-cells =<0>;
> +
> + port@0 {
> + #address-cells =<1>;
> + #size-cells =<0>;
> + reg =<0>;
> +
> + endpoint@0 {
> + reg =<0>;
> + ...
> + };
> + endpoint@1 {
> + reg =<1>;
> + ...
> + };
> + };
> +
> + port@1 {
> + reg =<1>;
> +
> + endpoint { ... };
> + };
> +};
> +
> +All 'port' nodes can be grouped under an optional 'ports' node, which
> +allows to specify #address-cells, #size-cells properties for the 'port'
> +nodes independently from any other child device nodes a device might
> +have.
> +
> +device {
> + ...
> + ports {
> + #address-cells =<1>;
> + #size-cells =<0>;
> +
> + port@0 {
> + ...
> + endpoint@0 { ... };
> + endpoint@1 { ... };
> + };
> +
> + port@1 { ... };
> + };
> +};
> +
> +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.
> +
> +device_1 {
> + port {
> + device_1_output: endpoint {
> + remote-endpoint =<&device_2_input>;
> + };
> + };
> +};
> +
> +device_1 {
s/device_1/device_2/
But it might be better to use dashes instead of underscores
for the node names.
> + port {
> + device_2_input: endpoint {
> + remote-endpoint =<&device_1_output>;
> + };
> + };
> +};
> +
> +
> +Required properties
> +-------------------
> +
> +If there is more than one 'port' or more than one 'endpoint' node or 'reg'
> +property is present in port and/or endpoint nodes the following properties
> +are required in a relevant parent node:
> +
> + - #address-cells : number of cells required to define port/endpoint
> + identifier, should be 1.
> + - #size-cells : should be zero.
> +
> +Optional endpoint properties
> +----------------------------
> +
> +- remote-endpoint: phandle to an 'endpoint' subnode of a remote device node.
> +
--
Regards,
Sylwester
On 02/27/2014 06:35 PM, Philipp Zabel wrote:
> If of_graph_get_next_endpoint is given a parentless node instead of an
> endpoint node, it is clearly a bug.
>
> Signed-off-by: Philipp Zabel<[email protected]>
> ---
> drivers/of/base.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index b2f223f..6e650cf 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2028,8 +2028,8 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> of_node_put(node);
> } else {
> port = of_get_parent(prev);
> - if (!port)
> - /* Hm, has someone given us the root node ?... */
> + if (WARN_ONCE(!port, "%s(): endpoint has no parent node\n",
> + __func__))
Perhaps we can add more information to this error log, e.g.
if (WARN_ONCE(!port, "%s(): endpoint %s has no parent node\n",
__func__, prev->full_name))
?
> return NULL;
>
> /* Avoid dropping prev node refcount to 0. */
On 02/27/2014 06:35 PM, Philipp Zabel wrote:
> This patch adds a new struct of_endpoint which is then embedded in struct
> v4l2_of_endpoint and contains the endpoint properties that are not V4L2
> (or even media) specific: the port number, endpoint id, local device tree
> node and remote endpoint phandle. of_graph_parse_endpoint parses those
> properties and is used by v4l2_of_parse_endpoint, which just adds the
> V4L2 MBUS information to the containing v4l2_of_endpoint structure.
>
> Signed-off-by: Philipp Zabel<[email protected]>
> ---
> Changes since v4:
> - Fixed users of struct v4l2_of_endpoint
> - Removed left-over #include<media/of_graph.h> from v4l2-of.h
> ---
> drivers/media/platform/exynos4-is/media-dev.c | 10 ++++-----
> drivers/media/platform/exynos4-is/mipi-csis.c | 2 +-
> drivers/media/v4l2-core/v4l2-of.c | 16 +++-----------
> drivers/of/base.c | 31 +++++++++++++++++++++++++++
> include/linux/of_graph.h | 20 +++++++++++++++++
> include/media/v4l2-of.h | 8 ++-----
> 6 files changed, 62 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index d0f82da..04d6ecd 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -469,10 +469,10 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> return 0;
>
> v4l2_of_parse_endpoint(ep,&endpoint);
> - if (WARN_ON(endpoint.port == 0) || index>= FIMC_MAX_SENSORS)
> + if (WARN_ON(endpoint.base.port == 0) || index>= FIMC_MAX_SENSORS)
> return -EINVAL;
>
> - pd->mux_id = (endpoint.port - 1)& 0x1;
> + pd->mux_id = (endpoint.base.port - 1)& 0x1;
>
> rem = of_graph_get_remote_port_parent(ep);
> of_node_put(ep);
> @@ -494,13 +494,13 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> return -EINVAL;
> }
>
> - if (fimc_input_is_parallel(endpoint.port)) {
> + if (fimc_input_is_parallel(endpoint.base.port)) {
> if (endpoint.bus_type == V4L2_MBUS_PARALLEL)
> pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_601;
> else
> pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_656;
> pd->flags = endpoint.bus.parallel.flags;
> - } else if (fimc_input_is_mipi_csi(endpoint.port)) {
> + } else if (fimc_input_is_mipi_csi(endpoint.base.port)) {
> /*
> * MIPI CSI-2: only input mux selection and
> * the sensor's clock frequency is needed.
> @@ -508,7 +508,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
> pd->sensor_bus_type = FIMC_BUS_TYPE_MIPI_CSI2;
> } else {
> v4l2_err(&fmd->v4l2_dev, "Wrong port id (%u) at node %s\n",
> - endpoint.port, rem->full_name);
> + endpoint.base.port, rem->full_name);
> }
> /*
> * For FIMC-IS handled sensors, that are placed under i2c-isp device
> diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
> index fd1ae65..3678ba5 100644
> --- a/drivers/media/platform/exynos4-is/mipi-csis.c
> +++ b/drivers/media/platform/exynos4-is/mipi-csis.c
> @@ -772,7 +772,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
> /* Get port node and validate MIPI-CSI channel id. */
> v4l2_of_parse_endpoint(node,&endpoint);
>
> - state->index = endpoint.port - FIMC_INPUT_MIPI_CSI2_0;
> + state->index = endpoint.base.port - FIMC_INPUT_MIPI_CSI2_0;
> if (state->index< 0 || state->index>= CSIS_MAX_ENTITIES)
> return -ENXIO;
>
> diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
> index f919db3..b4ed9a9 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -127,17 +127,9 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
> int v4l2_of_parse_endpoint(const struct device_node *node,
> struct v4l2_of_endpoint *endpoint)
> {
> - struct device_node *port_node = of_get_parent(node);
> -
> - memset(endpoint, 0, offsetof(struct v4l2_of_endpoint, head));
> -
> - endpoint->local_node = node;
> - /*
> - * It doesn't matter whether the two calls below succeed.
> - * If they don't then the default value 0 is used.
> - */
> - of_property_read_u32(port_node, "reg",&endpoint->port);
> - of_property_read_u32(node, "reg",&endpoint->id);
> + of_graph_parse_endpoint(node,&endpoint->base);
> + endpoint->bus_type = 0;
> + memset(&endpoint->bus, 0, sizeof(endpoint->bus));
>
> v4l2_of_parse_csi_bus(node, endpoint);
> /*
> @@ -147,8 +139,6 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
> if (endpoint->bus.mipi_csi2.flags == 0)
> v4l2_of_parse_parallel_bus(node, endpoint);
>
> - of_node_put(port_node);
> -
> return 0;
> }
> EXPORT_SYMBOL(v4l2_of_parse_endpoint);
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 8ecca7a..ba3cfca 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1985,6 +1985,37 @@ struct device_node *of_find_next_cache_node(const struct device_node *np)
> }
>
> /**
> + * of_graph_parse_endpoint() - parse common endpoint node properties
> + * @node: pointer to endpoint device_node
> + * @endpoint: pointer to the OF endpoint data structure
> + *
> + * All properties are optional. If none are found, we don't set any flags.
> + * This means the port has a static configuration and no properties have
> + * to be specified explicitly.
I don't think these two sentences are needed, it's all described in the
DT binding documentation. And struct of_endpoint doesn't contain any
"flags" field.
> + * The caller should hold a reference to @node.
> + */
> +int of_graph_parse_endpoint(const struct device_node *node,
> + struct of_endpoint *endpoint)
> +{
> + struct device_node *port_node = of_get_parent(node);
> +
> + memset(endpoint, 0, sizeof(*endpoint));
> +
> + endpoint->local_node = node;
> + /*
> + * It doesn't matter whether the two calls below succeed.
> + * If they don't then the default value 0 is used.
> + */
> + of_property_read_u32(port_node, "reg",&endpoint->port);
> + of_property_read_u32(node, "reg",&endpoint->id);
> +
> + of_node_put(port_node);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(of_graph_parse_endpoint);
> +
> +/**
> * of_graph_get_next_endpoint() - get next endpoint node
> * @parent: pointer to the parent device node
> * @prev: previous endpoint node, or NULL to get first
> diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
> index 3bbeb60..2b233db 100644
> --- a/include/linux/of_graph.h
> +++ b/include/linux/of_graph.h
> @@ -14,7 +14,21 @@
> #ifndef __LINUX_OF_GRAPH_H
> #define __LINUX_OF_GRAPH_H
>
> +/**
> + * struct of_endpoint - the OF graph endpoint data structure
> + * @port: identifier (value of reg property) of a port this endpoint belongs to
> + * @id: identifier (value of reg property) of this endpoint
> + * @local_node: pointer to device_node of this endpoint
> + */
> +struct of_endpoint {
> + unsigned int port;
> + unsigned int id;
> + const struct device_node *local_node;
> +};
> +
> #ifdef CONFIG_OF
> +int of_graph_parse_endpoint(const struct device_node *node,
> + struct of_endpoint *endpoint);
> struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
> struct device_node *previous);
> struct device_node *of_graph_get_remote_port_parent(
> @@ -22,6 +36,12 @@ struct device_node *of_graph_get_remote_port_parent(
> struct device_node *of_graph_get_remote_port(const struct device_node *node);
> #else
>
> +static inline int of_graph_parse_endpoint(const struct device_node *node,
> + struct of_endpoint *endpoint);
> +{
> + return -ENOSYS;
> +}
> +
> static inline struct device_node *of_graph_get_next_endpoint(
> const struct device_node *parent,
> struct device_node *previous)
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> index 3a49735..70fa7b7 100644
> --- a/include/media/v4l2-of.h
> +++ b/include/media/v4l2-of.h
> @@ -51,17 +51,13 @@ struct v4l2_of_bus_parallel {
>
> /**
> * struct v4l2_of_endpoint - the endpoint data structure
> - * @port: identifier (value of reg property) of a port this endpoint belongs to
> - * @id: identifier (value of reg property) of this endpoint
> - * @local_node: pointer to device_node of this endpoint
> + * @base: struct of_endpoint containing port, id, and local of_node
> * @bus_type: bus type
> * @bus: bus configuration data structure
> * @head: list head for this structure
> */
> struct v4l2_of_endpoint {
> - unsigned int port;
> - unsigned int id;
> - const struct device_node *local_node;
> + struct of_endpoint base;
> enum v4l2_mbus_type bus_type;
> union {
> struct v4l2_of_bus_parallel parallel;
Otherwise looks good to me.