Re: cio2 ipu3 module to automatically connect sensors via swnodes

[Dan Scally <djrscally@gmail.com>]

Hello Heikki

On 09/09/2020 14:40, Heikki Krogerus wrote:
> I'm almost certain that my graph patch is broken. My tests did not
> cover nearly as much that is needed to properly test the patch. I
> think at least the refcount handling is totally broken in
> software_node_graph_get_next_endpoint(), so that function at least
> needs to be rewritten.
>
> Unfortunately I do not have time to work on that patch right now.
>
> thanks,

I managed to get to the bottom of the remaining issue (which was the 
cause of the problem I originally posted here about - that's now all 
resolved).  In addition to the refcount handling problems, 
software_node_graph_get_next_endpoint() was occasionally passing an 
invalid combination of parameters to software_node_get_next_child(); 
sometimes it would pass an existing endpoint as old in combination with 
a port which was not the endpoint's parent. Applying this on top of your 
patch seems to stop both of those errors:

---
  drivers/base/swnode.c | 13 +++++++++++++
  1 file changed, 13 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 5cf9f1eef96f..80255e0b7739 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -563,6 +563,7 @@ software_node_graph_get_next_endpoint(const struct fwnode_handle *fwnode,
  {
  	struct swnode *swnode = to_swnode(fwnode);
  	struct fwnode_handle *old = endpoint;
+	struct fwnode_handle *parent_of_old;
  	struct fwnode_handle *parent;
  	struct fwnode_handle *port;
  
@@ -581,10 +582,22 @@ software_node_graph_get_next_endpoint(const struct fwnode_handle *fwnode,
  	}
  
  	for (; port; port = swnode_graph_find_next_port(parent, port)) {
+
+		if (old) {
+			parent_of_old = software_node_get_parent(old);
+
+			if (parent_of_old != port)
+				old = NULL;
+
+			fwnode_handle_put(parent_of_old);
+		}
+
  		endpoint = software_node_get_next_child(port, old);
  		fwnode_handle_put(old);
  		if (endpoint)
  			break;
+		else
+			fwnode_handle_put(port);
  	}
  
  	fwnode_handle_put(port);
-- 
2.25.1

Following that change, everything seems to be working ok. The module linking sensors to the cio2 infrastructure can correctly link multiple sensors now, and the reference count issues that prevented that module from unloading are resolved too.

Getting those patches and the bridge module upstream would be a good step to getting working cameras on ACPI platforms using ipu3. I'd like to take that on if you haven't the time; would you be ok with me applying my changes on top of your original patch, and submitting the combined result as a v2? And then I'll tackle any changes that come back - might take me a little while but I should be able to manage it.

Thanks
Dan