Re: [PATCH v6 6/8] of: Implement simplified graph binding for single port devices

[Philipp Zabel]

On Fri, Mar 07, 2014 at 06:38:02PM +, Grant Likely wrote:
 On Wed,  5 Mar 2014 10:20:40 +0100, Philipp Zabel p.za...@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.za...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 6/8] of: Implement simplified graph binding for single port devices

[Grant Likely]

On Wed,  5 Mar 2014 10:20:40 +0100, Philipp Zabel p.za...@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.za...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html