| Message ID | 1394011242-16783-7-git-send-email-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 5 Mar 2014 10:20:40 +0100, Philipp Zabel <p.zabel@pengutronix.de> wrote: > 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 <p.zabel@pengutronix.de> Ergh... I think this is too loosely defined. The caller really should be explicit about which behaviour it needs. I'll listen to arguments though if you can make a strong argument. g. > --- > Changes since v5: > - Unrolled for-loop in of_graph_get_remote_port_parent > --- > drivers/of/base.c | 42 +++++++++++++++++++++++++++++------------- > 1 file changed, 29 insertions(+), 13 deletions(-) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 715144af..ffd0217 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -2003,7 +2003,8 @@ int of_graph_parse_endpoint(const struct device_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); > + if (port_node && !of_node_cmp(port_node->name, "port")) > + of_property_read_u32(port_node, "reg", &endpoint->port); > of_property_read_u32(node, "reg", &endpoint->id); > > of_node_put(port_node); > @@ -2034,8 +2035,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; > @@ -2046,8 +2052,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) > @@ -2062,6 +2066,10 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, > if (WARN_ONCE(!port, "%s(): endpoint %s has no parent node\n", > __func__, prev->full_name)) > return NULL; > + if (port == parent) { > + of_node_put(port); > + return NULL; > + } > > /* Avoid dropping prev node refcount to 0. */ > of_node_get(prev); > @@ -2097,18 +2105,21 @@ 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); > + if (!np) > + return NULL; > > - /* 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; > + np = of_get_next_parent(np); > + if (!np || of_node_cmp(np->name, "port") != 0) > + return np; > + > + np = of_get_next_parent(np); > + if (!np || of_node_cmp(np->name, "ports") != 0) > + return np; > + > + return of_get_next_parent(np); > } > EXPORT_SYMBOL(of_graph_get_remote_port_parent); > > @@ -2127,6 +2138,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.9.0.rc3 > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 07, 2014 at 06:38:02PM +0000, Grant Likely wrote: > On Wed, 5 Mar 2014 10:20:40 +0100, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > 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 <p.zabel@pengutronix.de> > > Ergh... I think this is too loosely defined. The caller really should be > explicit about which behaviour it needs. I'll listen to arguments > though if you can make a strong argument. I have dropped this patch and the corresponding documentation patch for now. This simplification is a separate issue from the move and there is no consensus yet. Basically the main issue with the port { endpoint { remote-endpoint=... } } binding is that it is very verbose if you just need a single link. Instead of removing the port node, we could also remove the endpoint node and have the remote-endpoint property direcly in the port node. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/of/base.c b/drivers/of/base.c index 715144af..ffd0217 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -2003,7 +2003,8 @@ int of_graph_parse_endpoint(const struct device_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); + if (port_node && !of_node_cmp(port_node->name, "port")) + of_property_read_u32(port_node, "reg", &endpoint->port); of_property_read_u32(node, "reg", &endpoint->id); of_node_put(port_node); @@ -2034,8 +2035,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; @@ -2046,8 +2052,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) @@ -2062,6 +2066,10 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, if (WARN_ONCE(!port, "%s(): endpoint %s has no parent node\n", __func__, prev->full_name)) return NULL; + if (port == parent) { + of_node_put(port); + return NULL; + } /* Avoid dropping prev node refcount to 0. */ of_node_get(prev); @@ -2097,18 +2105,21 @@ 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); + if (!np) + return NULL; - /* 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; + np = of_get_next_parent(np); + if (!np || of_node_cmp(np->name, "port") != 0) + return np; + + np = of_get_next_parent(np); + if (!np || of_node_cmp(np->name, "ports") != 0) + return np; + + return of_get_next_parent(np); } EXPORT_SYMBOL(of_graph_get_remote_port_parent); @@ -2127,6 +2138,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);
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 <p.zabel@pengutronix.de> --- Changes since v5: - Unrolled for-loop in of_graph_get_remote_port_parent --- drivers/of/base.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)