[PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Steve Longerbeam]

There is nothing v4l2-specific about v4l2_fwnode_{parse|put}_link().
Make these functions more generally available by moving them to driver
base, with the appropriate name changes to the functions and struct.

In the process embed a 'struct fwnode_endpoint' in 'struct fwnode_link'
for both sides of the link, and make use of fwnode_graph_parse_endpoint()
to fully parse both endpoints. Rename members local_node and
remote_node to more descriptive local_port_parent and
remote_port_parent.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/base/property.c                     | 63 +++++++++++++++++++
 drivers/media/platform/xilinx/xilinx-vipp.c | 69 +++++++++++----------
 drivers/media/v4l2-core/v4l2-fwnode.c       | 39 ------------
 drivers/staging/media/imx/imx-media-of.c    | 49 +++++++--------
 include/linux/fwnode.h                      | 14 +++++
 include/linux/property.h                    |  5 ++
 include/media/v4l2-fwnode.h                 | 44 -------------
 7 files changed, 141 insertions(+), 142 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 81bd01ed4042..dd82cd150d84 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1100,6 +1100,69 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
 
+/**
+ * fwnode_graph_parse_link() - parse a link between two endpoints
+ * @local_endpoint: the endpoint's fwnode at the local end of the link
+ * @link: pointer to the fwnode link data structure
+ *
+ * Fill the link structure with the parsed local and remote endpoint info
+ * and the local and remote port parent nodes.
+ *
+ * A reference is taken to both the local and remote port parent nodes,
+ * the caller must use fwnode_graph_put_link() to drop the references
+ * when done with the link.
+ *
+ * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode
+ * can't be found.
+ */
+int fwnode_graph_parse_link(struct fwnode_handle *local_endpoint,
+			    struct fwnode_link *link)
+{
+	struct fwnode_handle *remote_endpoint;
+	int ret;
+
+	memset(link, 0, sizeof(*link));
+
+	ret = fwnode_graph_parse_endpoint(local_endpoint, &link->local);
+	if (ret < 0)
+		return ret;
+
+	remote_endpoint = fwnode_graph_get_remote_endpoint(local_endpoint);
+	if (!remote_endpoint)
+		return -ENOLINK;
+
+	ret = fwnode_graph_parse_endpoint(remote_endpoint, &link->remote);
+	if (ret < 0) {
+		fwnode_handle_put(remote_endpoint);
+		return ret;
+	}
+
+	link->local_port_parent =
+		fwnode_graph_get_port_parent(local_endpoint);
+	link->remote_port_parent =
+		fwnode_graph_get_port_parent(remote_endpoint);
+
+	fwnode_handle_put(remote_endpoint);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_parse_link);
+
+/**
+ * fwnode_graph_put_link() - drop references to port parent nodes in a link
+ * @link: pointer to the fwnode link data structure
+ *
+ * Drop references to the local and remote port parent nodes in the link.
+ * This function must be called on every link parsed with
+ * fwnode_graph_parse_link().
+ */
+void fwnode_graph_put_link(struct fwnode_link *link)
+{
+	fwnode_handle_put(link->local_port_parent);
+	fwnode_handle_put(link->remote_port_parent);
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_put_link);
+
 const void *device_get_match_data(struct device *dev)
 {
 	return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev);
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index cc2856efea59..9c0dfc694478 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -74,7 +74,7 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
 	struct media_pad *local_pad;
 	struct media_pad *remote_pad;
 	struct xvip_graph_entity *ent;
-	struct v4l2_fwnode_link link;
+	struct fwnode_link link;
 	struct fwnode_handle *ep = NULL;
 	int ret = 0;
 
@@ -89,7 +89,7 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
 
 		dev_dbg(xdev->dev, "processing endpoint %p\n", ep);
 
-		ret = v4l2_fwnode_parse_link(ep, &link);
+		ret = fwnode_graph_parse_link(ep, &link);
 		if (ret < 0) {
 			dev_err(xdev->dev, "failed to parse link for %p\n",
 				ep);
@@ -99,54 +99,55 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
 		/* Skip sink ports, they will be processed from the other end of
 		 * the link.
 		 */
-		if (link.local_port >= local->num_pads) {
+		if (link.local.port >= local->num_pads) {
 			dev_err(xdev->dev, "invalid port number %u for %p\n",
-				link.local_port, link.local_node);
-			v4l2_fwnode_put_link(&link);
+				link.local.port, link.local_port_parent);
+			fwnode_graph_put_link(&link);
 			ret = -EINVAL;
 			break;
 		}
 
-		local_pad = &local->pads[link.local_port];
+		local_pad = &local->pads[link.local.port];
 
 		if (local_pad->flags & MEDIA_PAD_FL_SINK) {
 			dev_dbg(xdev->dev, "skipping sink port %p:%u\n",
-				link.local_node, link.local_port);
-			v4l2_fwnode_put_link(&link);
+				link.local_port_parent, link.local.port);
+			fwnode_graph_put_link(&link);
 			continue;
 		}
 
 		/* Skip DMA engines, they will be processed separately. */
-		if (link.remote_node == of_fwnode_handle(xdev->dev->of_node)) {
+		if (link.remote_port_parent ==
+		    of_fwnode_handle(xdev->dev->of_node)) {
 			dev_dbg(xdev->dev, "skipping DMA port %p:%u\n",
-				link.local_node, link.local_port);
-			v4l2_fwnode_put_link(&link);
+				link.local_port_parent, link.local.port);
+			fwnode_graph_put_link(&link);
 			continue;
 		}
 
 		/* Find the remote entity. */
-		ent = xvip_graph_find_entity(xdev, link.remote_node);
+		ent = xvip_graph_find_entity(xdev, link.remote_port_parent);
 		if (ent == NULL) {
 			dev_err(xdev->dev, "no entity found for %p\n",
-				link.remote_node);
-			v4l2_fwnode_put_link(&link);
+				link.remote_port_parent);
+			fwnode_graph_put_link(&link);
 			ret = -ENODEV;
 			break;
 		}
 
 		remote = ent->entity;
 
-		if (link.remote_port >= remote->num_pads) {
+		if (link.remote.port >= remote->num_pads) {
 			dev_err(xdev->dev, "invalid port number %u on %p\n",
-				link.remote_port, link.remote_node);
-			v4l2_fwnode_put_link(&link);
+				link.remote.port, link.remote_port_parent);
+			fwnode_graph_put_link(&link);
 			ret = -EINVAL;
 			break;
 		}
 
-		remote_pad = &remote->pads[link.remote_port];
+		remote_pad = &remote->pads[link.remote.port];
 
-		v4l2_fwnode_put_link(&link);
+		fwnode_graph_put_link(&link);
 
 		/* Create the media link. */
 		dev_dbg(xdev->dev, "creating %s:%u -> %s:%u link\n",
@@ -191,7 +192,7 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
 	struct media_pad *source_pad;
 	struct media_pad *sink_pad;
 	struct xvip_graph_entity *ent;
-	struct v4l2_fwnode_link link;
+	struct fwnode_link link;
 	struct device_node *ep = NULL;
 	struct xvip_dma *dma;
 	int ret = 0;
@@ -206,7 +207,7 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
 
 		dev_dbg(xdev->dev, "processing endpoint %pOF\n", ep);
 
-		ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), &link);
+		ret = fwnode_graph_parse_link(of_fwnode_handle(ep), &link);
 		if (ret < 0) {
 			dev_err(xdev->dev, "failed to parse link for %pOF\n",
 				ep);
@@ -214,11 +215,11 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
 		}
 
 		/* Find the DMA engine. */
-		dma = xvip_graph_find_dma(xdev, link.local_port);
+		dma = xvip_graph_find_dma(xdev, link.local.port);
 		if (dma == NULL) {
 			dev_err(xdev->dev, "no DMA engine found for port %u\n",
-				link.local_port);
-			v4l2_fwnode_put_link(&link);
+				link.local.port);
+			fwnode_graph_put_link(&link);
 			ret = -EINVAL;
 			break;
 		}
@@ -227,20 +228,20 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
 			dma->video.name);
 
 		/* Find the remote entity. */
-		ent = xvip_graph_find_entity(xdev, link.remote_node);
+		ent = xvip_graph_find_entity(xdev, link.remote_port_parent);
 		if (ent == NULL) {
 			dev_err(xdev->dev, "no entity found for %pOF\n",
-				to_of_node(link.remote_node));
-			v4l2_fwnode_put_link(&link);
+				to_of_node(link.remote_port_parent));
+			fwnode_graph_put_link(&link);
 			ret = -ENODEV;
 			break;
 		}
 
-		if (link.remote_port >= ent->entity->num_pads) {
+		if (link.remote.port >= ent->entity->num_pads) {
 			dev_err(xdev->dev, "invalid port number %u on %pOF\n",
-				link.remote_port,
-				to_of_node(link.remote_node));
-			v4l2_fwnode_put_link(&link);
+				link.remote.port,
+				to_of_node(link.remote_port_parent));
+			fwnode_graph_put_link(&link);
 			ret = -EINVAL;
 			break;
 		}
@@ -249,15 +250,15 @@ static int xvip_graph_build_dma(struct xvip_composite_device *xdev)
 			source = &dma->video.entity;
 			source_pad = &dma->pad;
 			sink = ent->entity;
-			sink_pad = &sink->pads[link.remote_port];
+			sink_pad = &sink->pads[link.remote.port];
 		} else {
 			source = ent->entity;
-			source_pad = &source->pads[link.remote_port];
+			source_pad = &source->pads[link.remote.port];
 			sink = &dma->video.entity;
 			sink_pad = &dma->pad;
 		}
 
-		v4l2_fwnode_put_link(&link);
+		fwnode_graph_put_link(&link);
 
 		/* Create the media link. */
 		dev_dbg(xdev->dev, "creating %s:%u -> %s:%u link\n",
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 3bd1888787eb..5d4ce4aa3fdc 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -556,45 +556,6 @@ int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL_GPL(v4l2_fwnode_endpoint_alloc_parse);
 
-int v4l2_fwnode_parse_link(struct fwnode_handle *__fwnode,
-			   struct v4l2_fwnode_link *link)
-{
-	const char *port_prop = is_of_node(__fwnode) ? "reg" : "port";
-	struct fwnode_handle *fwnode;
-
-	memset(link, 0, sizeof(*link));
-
-	fwnode = fwnode_get_parent(__fwnode);
-	fwnode_property_read_u32(fwnode, port_prop, &link->local_port);
-	fwnode = fwnode_get_next_parent(fwnode);
-	if (is_of_node(fwnode) && of_node_name_eq(to_of_node(fwnode), "ports"))
-		fwnode = fwnode_get_next_parent(fwnode);
-	link->local_node = fwnode;
-
-	fwnode = fwnode_graph_get_remote_endpoint(__fwnode);
-	if (!fwnode) {
-		fwnode_handle_put(fwnode);
-		return -ENOLINK;
-	}
-
-	fwnode = fwnode_get_parent(fwnode);
-	fwnode_property_read_u32(fwnode, port_prop, &link->remote_port);
-	fwnode = fwnode_get_next_parent(fwnode);
-	if (is_of_node(fwnode) && of_node_name_eq(to_of_node(fwnode), "ports"))
-		fwnode = fwnode_get_next_parent(fwnode);
-	link->remote_node = fwnode;
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(v4l2_fwnode_parse_link);
-
-void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)
-{
-	fwnode_handle_put(link->local_node);
-	fwnode_handle_put(link->remote_node);
-}
-EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
-
 static int
 v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
 					  struct v4l2_async_notifier *notifier,
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index 2d3efd2a6dde..736c954a8ff5 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -84,28 +84,29 @@ EXPORT_SYMBOL_GPL(imx_media_add_of_subdevs);
  */
 static int create_of_link(struct imx_media_dev *imxmd,
 			  struct v4l2_subdev *sd,
-			  struct v4l2_fwnode_link *link)
+			  struct fwnode_link *link)
 {
 	struct v4l2_subdev *remote, *src, *sink;
 	int src_pad, sink_pad;
 
-	if (link->local_port >= sd->entity.num_pads)
+	if (link->local.port >= sd->entity.num_pads)
 		return -EINVAL;
 
-	remote = imx_media_find_subdev_by_fwnode(imxmd, link->remote_node);
+	remote = imx_media_find_subdev_by_fwnode(imxmd,
+						 link->remote_port_parent);
 	if (!remote)
 		return 0;
 
-	if (sd->entity.pads[link->local_port].flags & MEDIA_PAD_FL_SINK) {
+	if (sd->entity.pads[link->local.port].flags & MEDIA_PAD_FL_SINK) {
 		src = remote;
-		src_pad = link->remote_port;
+		src_pad = link->remote.port;
 		sink = sd;
-		sink_pad = link->local_port;
+		sink_pad = link->local.port;
 	} else {
 		src = sd;
-		src_pad = link->local_port;
+		src_pad = link->local.port;
 		sink = remote;
-		sink_pad = link->remote_port;
+		sink_pad = link->remote.port;
 	}
 
 	/* make sure link doesn't already exist before creating */
@@ -126,17 +127,17 @@ static int create_of_link(struct imx_media_dev *imxmd,
 int imx_media_create_of_links(struct imx_media_dev *imxmd,
 			      struct v4l2_subdev *sd)
 {
-	struct v4l2_fwnode_link link;
-	struct device_node *ep;
+	struct fwnode_handle *endpoint;
+	struct fwnode_link link;
 	int ret;
 
-	for_each_endpoint_of_node(sd->dev->of_node, ep) {
-		ret = v4l2_fwnode_parse_link(of_fwnode_handle(ep), &link);
+	fwnode_graph_for_each_endpoint(dev_fwnode(sd->dev), endpoint) {
+		ret = fwnode_graph_parse_link(endpoint, &link);
 		if (ret)
 			continue;
 
 		ret = create_of_link(imxmd, sd, &link);
-		v4l2_fwnode_put_link(&link);
+		fwnode_graph_put_link(&link);
 		if (ret)
 			return ret;
 	}
@@ -152,35 +153,33 @@ EXPORT_SYMBOL_GPL(imx_media_create_of_links);
 int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
 				  struct v4l2_subdev *csi)
 {
-	struct device_node *csi_np = csi->dev->of_node;
-	struct device_node *ep;
+	struct fwnode_handle *csi_np = dev_fwnode(csi->dev);
+	struct fwnode_handle *csi_ep;
 
-	for_each_child_of_node(csi_np, ep) {
-		struct fwnode_handle *fwnode, *csi_ep;
-		struct v4l2_fwnode_link link;
+	fwnode_for_each_child_node(csi_np, csi_ep) {
+		struct fwnode_handle *fwnode;
+		struct fwnode_link link;
 		int ret;
 
 		memset(&link, 0, sizeof(link));
 
-		link.local_node = of_fwnode_handle(csi_np);
-		link.local_port = CSI_SINK_PAD;
-
-		csi_ep = of_fwnode_handle(ep);
+		link.local_port_parent = csi_np;
+		link.local.port = CSI_SINK_PAD;
 
 		fwnode = fwnode_graph_get_remote_endpoint(csi_ep);
 		if (!fwnode)
 			continue;
 
 		fwnode = fwnode_get_parent(fwnode);
-		fwnode_property_read_u32(fwnode, "reg", &link.remote_port);
+		fwnode_property_read_u32(fwnode, "reg", &link.remote.port);
 		fwnode = fwnode_get_next_parent(fwnode);
 		if (is_of_node(fwnode) &&
 		    of_node_name_eq(to_of_node(fwnode), "ports"))
 			fwnode = fwnode_get_next_parent(fwnode);
-		link.remote_node = fwnode;
+		link.remote_port_parent = fwnode;
 
 		ret = create_of_link(imxmd, csi, &link);
-		fwnode_handle_put(link.remote_node);
+		fwnode_handle_put(link.remote_port_parent);
 		if (ret)
 			return ret;
 	}
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index a11c8c56c78b..c2063ae2affe 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -31,6 +31,20 @@ struct fwnode_endpoint {
 	const struct fwnode_handle *local_fwnode;
 };
 
+/**
+ * struct fwnode_link - a link between two fwnode graph endpoints
+ * @local: parsed local endpoint of the link
+ * @local_port_parent: port parent fwnode of local endpoint
+ * @remote: parsed remote endpoint of the link
+ * @remote_port_parent: port parent fwnode of the remote endpoint
+ */
+struct fwnode_link {
+	struct fwnode_endpoint local;
+	struct fwnode_handle *local_port_parent;
+	struct fwnode_endpoint remote;
+	struct fwnode_handle *remote_port_parent;
+};
+
 #define NR_FWNODE_REFERENCE_ARGS	8
 
 /**
diff --git a/include/linux/property.h b/include/linux/property.h
index 5a910ad79591..3923906fc314 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -374,6 +374,11 @@ fwnode_graph_get_endpoint_by_id(const struct fwnode_handle *fwnode,
 int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 				struct fwnode_endpoint *endpoint);
 
+int fwnode_graph_parse_link(struct fwnode_handle *fwnode,
+			    struct fwnode_link *link);
+
+void fwnode_graph_put_link(struct fwnode_link *link);
+
 /* -------------------------------------------------------------------------- */
 /* Software fwnode support - when HW description is incomplete or missing */
 
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index f6a7bcd13197..f81f8bf34526 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -109,20 +109,6 @@ struct v4l2_fwnode_endpoint {
 	unsigned int nr_of_link_frequencies;
 };
 
-/**
- * struct v4l2_fwnode_link - a link between two endpoints
- * @local_node: pointer to device_node of this endpoint
- * @local_port: identifier of the port this endpoint belongs to
- * @remote_node: pointer to device_node of the remote endpoint
- * @remote_port: identifier of the port the remote endpoint belongs to
- */
-struct v4l2_fwnode_link {
-	struct fwnode_handle *local_node;
-	unsigned int local_port;
-	struct fwnode_handle *remote_node;
-	unsigned int remote_port;
-};
-
 /**
  * v4l2_fwnode_endpoint_parse() - parse all fwnode node properties
  * @fwnode: pointer to the endpoint's fwnode handle
@@ -203,36 +189,6 @@ void v4l2_fwnode_endpoint_free(struct v4l2_fwnode_endpoint *vep);
 int v4l2_fwnode_endpoint_alloc_parse(struct fwnode_handle *fwnode,
 				     struct v4l2_fwnode_endpoint *vep);
 
-/**
- * v4l2_fwnode_parse_link() - parse a link between two endpoints
- * @fwnode: pointer to the endpoint's fwnode at the local end of the link
- * @link: pointer to the V4L2 fwnode link data structure
- *
- * Fill the link structure with the local and remote nodes and port numbers.
- * The local_node and remote_node fields are set to point to the local and
- * remote port's parent nodes respectively (the port parent node being the
- * parent node of the port node if that node isn't a 'ports' node, or the
- * grand-parent node of the port node otherwise).
- *
- * A reference is taken to both the local and remote nodes, the caller must use
- * v4l2_fwnode_put_link() to drop the references when done with the
- * link.
- *
- * Return: 0 on success, or -ENOLINK if the remote endpoint fwnode can't be
- * found.
- */
-int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,
-			   struct v4l2_fwnode_link *link);
-
-/**
- * v4l2_fwnode_put_link() - drop references to nodes in a link
- * @link: pointer to the V4L2 fwnode link data structure
- *
- * Drop references to the local and remote nodes in the link. This function
- * must be called on every link parsed with v4l2_fwnode_parse_link().
- */
-void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
-
 /**
  * typedef parse_endpoint_func - Driver's callback function to be called on
  *	each V4L2 fwnode endpoint.
-- 
2.17.1

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Andy Shevchenko]

On Tue, Aug 6, 2019 at 2:37 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> There is nothing v4l2-specific about v4l2_fwnode_{parse|put}_link().
> Make these functions more generally available by moving them to driver
> base, with the appropriate name changes to the functions and struct.
>
> In the process embed a 'struct fwnode_endpoint' in 'struct fwnode_link'
> for both sides of the link, and make use of fwnode_graph_parse_endpoint()
> to fully parse both endpoints. Rename members local_node and
> remote_node to more descriptive local_port_parent and
> remote_port_parent.
>

May I ask if it's going to be used outside of v4l2?
Any user in mind?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Steve Longerbeam]

Hi Andy,

On 8/5/19 11:41 PM, Andy Shevchenko wrote:
> On Tue, Aug 6, 2019 at 2:37 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> There is nothing v4l2-specific about v4l2_fwnode_{parse|put}_link().
>> Make these functions more generally available by moving them to driver
>> base, with the appropriate name changes to the functions and struct.
>>
>> In the process embed a 'struct fwnode_endpoint' in 'struct fwnode_link'
>> for both sides of the link, and make use of fwnode_graph_parse_endpoint()
>> to fully parse both endpoints. Rename members local_node and
>> remote_node to more descriptive local_port_parent and
>> remote_port_parent.
>>
> May I ask if it's going to be used outside of v4l2?

It could be, I can see fwnode_graph_parse_link() being useful in DRM, 
for example. But at this, only media core and v4l2 are making use of it.

This patch was created so that fwnode links can be parsed in a new media 
core function that forms media links from fwnode links.

The full patchset doesn't seem to be up yet, but see [1] for the cover 
letter.

Steve

[1] https://www.mail-archive.com/linux-media@vger.kernel.org/msg148910.html

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Russell King - ARM Linux admin]

On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
> The full patchset doesn't seem to be up yet, but see [1] for the cover
> letter.

Was the entire series copied to the mailing lists, or just selected
patches?  I only saw 4, 9, 11 and 13-22 via lakml.

In the absence of the other patches, will this solve imx-media binding
the internal subdevs of sensor devices to the CSI2 interface?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Steve Longerbeam]

On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
> On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
>> The full patchset doesn't seem to be up yet, but see [1] for the cover
>> letter.
> Was the entire series copied to the mailing lists, or just selected
> patches?  I only saw 4, 9, 11 and 13-22 via lakml.

The whole series was posted to the linux-media ML, see [1]. At the time, 
none of the linux-media ML archives had the whole series.

> In the absence of the other patches, will this solve imx-media binding
> the internal subdevs of sensor devices to the CSI2 interface?

"internal subdevs of sensor devices" ?? That doesn't make any sense.

Sensors are external to the SoC, there are no "internal" sensor devices.

Not sure what you mean by "binding" either in this context, but external 
sensors can connect via fwnode endpoint, and later translated to media 
link, to the receiver CSI-2 sink.

Steve

[1] https://www.spinics.net/lists/linux-media/msg155160.html

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Russell King - ARM Linux admin]

On Wed, Aug 14, 2019 at 12:04:41PM -0700, Steve Longerbeam wrote:
> 
> 
> On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
> > On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
> > > The full patchset doesn't seem to be up yet, but see [1] for the cover
> > > letter.
> > Was the entire series copied to the mailing lists, or just selected
> > patches?  I only saw 4, 9, 11 and 13-22 via lakml.
> 
> The whole series was posted to the linux-media ML, see [1]. At the time,
> none of the linux-media ML archives had the whole series.
> 
> > In the absence of the other patches, will this solve imx-media binding
> > the internal subdevs of sensor devices to the CSI2 interface?
> 
> "internal subdevs of sensor devices" ?? That doesn't make any sense.

Sorry, but it makes complete sense when you consider that sensor
devices may have more than one subdev, but there should be only one
that is the "output" to whatever the camera is attached to.  The
other subdevs are internal to the sensor.

subdevs are not purely the remit of SoC drivers.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Steve Longerbeam]

On 8/14/19 3:04 PM, Russell King - ARM Linux admin wrote:
> On Wed, Aug 14, 2019 at 12:04:41PM -0700, Steve Longerbeam wrote:
>>
>> On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
>>> On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
>>>> The full patchset doesn't seem to be up yet, but see [1] for the cover
>>>> letter.
>>> Was the entire series copied to the mailing lists, or just selected
>>> patches?  I only saw 4, 9, 11 and 13-22 via lakml.
>> The whole series was posted to the linux-media ML, see [1]. At the time,
>> none of the linux-media ML archives had the whole series.
>>
>>> In the absence of the other patches, will this solve imx-media binding
>>> the internal subdevs of sensor devices to the CSI2 interface?
>> "internal subdevs of sensor devices" ?? That doesn't make any sense.
> Sorry, but it makes complete sense when you consider that sensor
> devices may have more than one subdev, but there should be only one
> that is the "output" to whatever the camera is attached to.  The
> other subdevs are internal to the sensor.

Ah, thanks for the clarification. Yes, by "internal subdevs" I 
understand what you mean now. The adv748x and smiapp are examples.

>
> subdevs are not purely the remit of SoC drivers.

So there is no binding of internal subdevs to the receiver CSI-2. The 
receiver CSI-2 subdev will create media links to the subdev that has an 
externally exposed fwnode endpoint that connects with the CSI-2 sink pad.

Steve

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Russell King - ARM Linux admin]

On Wed, Aug 14, 2019 at 04:00:30PM -0700, Steve Longerbeam wrote:
> 
> 
> On 8/14/19 3:04 PM, Russell King - ARM Linux admin wrote:
> > On Wed, Aug 14, 2019 at 12:04:41PM -0700, Steve Longerbeam wrote:
> > > 
> > > On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
> > > > On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
> > > > > The full patchset doesn't seem to be up yet, but see [1] for the cover
> > > > > letter.
> > > > Was the entire series copied to the mailing lists, or just selected
> > > > patches?  I only saw 4, 9, 11 and 13-22 via lakml.
> > > The whole series was posted to the linux-media ML, see [1]. At the time,
> > > none of the linux-media ML archives had the whole series.
> > > 
> > > > In the absence of the other patches, will this solve imx-media binding
> > > > the internal subdevs of sensor devices to the CSI2 interface?
> > > "internal subdevs of sensor devices" ?? That doesn't make any sense.
> > Sorry, but it makes complete sense when you consider that sensor
> > devices may have more than one subdev, but there should be only one
> > that is the "output" to whatever the camera is attached to.  The
> > other subdevs are internal to the sensor.
> 
> Ah, thanks for the clarification. Yes, by "internal subdevs" I understand
> what you mean now. The adv748x and smiapp are examples.
> 
> > 
> > subdevs are not purely the remit of SoC drivers.
> 
> So there is no binding of internal subdevs to the receiver CSI-2. The
> receiver CSI-2 subdev will create media links to the subdev that has an
> externally exposed fwnode endpoint that connects with the CSI-2 sink pad.

Maybe - with 5.2, I get:

- entity 15: imx6-mipi-csi2 (5 pads, 6 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev2
        pad0: Sink
...
                <- "imx219 0-0010":0 []
                <- "imx219 pixel 0-0010":0 []

Adding some debug in gives:

[   11.963362] imx-media: imx_media_create_of_links() for imx6-mipi-csi2
[   11.963396] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
[   11.963422] imx-media: create_of_link(): /soc/ipu@2400000
[   11.963450] imx-media: create_of_link(): /soc/ipu@2800000
[   11.963478] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu2_csi1_mux
[   11.963489] imx-media: imx6-mipi-csi2:4 -> ipu2_csi1_mux:0
[   11.963522] imx-media: create_of_link(): /soc/aips-bus@2100000/i2c@21a0000/camera@10
[   11.963533] imx-media: imx219 0-0010:0 -> imx6-mipi-csi2:0
[   11.963549] imx-media: imx_media_create_of_links() for imx219 pixel 0-0010
[   11.963577] imx-media: create_of_link(): /soc/aips-bus@2100000/mipi@21dc000
[   11.963587] imx-media: imx219 pixel 0-0010:0 -> imx6-mipi-csi2:0
[   11.963602] imx-media: imx_media_create_of_links() for imx219 0-0010

Note that it's not created by imx6-mipi-csi2, but by imx-media delving
around in the imx219 subdevs.

From what I can see, smiapp does the same thing that I do in imx219 -
sets the subdev->dev member to point at the struct device, which then
means that v4l2_device_register_subdev() will associate the same fwnode
with both "imx219 pixel 0-0010" and "imx219 0-0010".

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 04/22] media: Move v4l2_fwnode_parse_link from v4l2 to driver base

[Steve Longerbeam]

On 8/14/19 4:15 PM, Russell King - ARM Linux admin wrote:
> On Wed, Aug 14, 2019 at 04:00:30PM -0700, Steve Longerbeam wrote:
>>
>> On 8/14/19 3:04 PM, Russell King - ARM Linux admin wrote:
>>> On Wed, Aug 14, 2019 at 12:04:41PM -0700, Steve Longerbeam wrote:
>>>> On 8/14/19 3:30 AM, Russell King - ARM Linux admin wrote:
>>>>> On Tue, Aug 06, 2019 at 09:53:41AM -0700, Steve Longerbeam wrote:
>>>>>> The full patchset doesn't seem to be up yet, but see [1] for the cover
>>>>>> letter.
>>>>> Was the entire series copied to the mailing lists, or just selected
>>>>> patches?  I only saw 4, 9, 11 and 13-22 via lakml.
>>>> The whole series was posted to the linux-media ML, see [1]. At the time,
>>>> none of the linux-media ML archives had the whole series.
>>>>
>>>>> In the absence of the other patches, will this solve imx-media binding
>>>>> the internal subdevs of sensor devices to the CSI2 interface?
>>>> "internal subdevs of sensor devices" ?? That doesn't make any sense.
>>> Sorry, but it makes complete sense when you consider that sensor
>>> devices may have more than one subdev, but there should be only one
>>> that is the "output" to whatever the camera is attached to.  The
>>> other subdevs are internal to the sensor.
>> Ah, thanks for the clarification. Yes, by "internal subdevs" I understand
>> what you mean now. The adv748x and smiapp are examples.
>>
>>> subdevs are not purely the remit of SoC drivers.
>> So there is no binding of internal subdevs to the receiver CSI-2. The
>> receiver CSI-2 subdev will create media links to the subdev that has an
>> externally exposed fwnode endpoint that connects with the CSI-2 sink pad.
> Maybe - with 5.2, I get:
>
> - entity 15: imx6-mipi-csi2 (5 pads, 6 links)
>               type V4L2 subdev subtype Unknown flags 0
>               device node name /dev/v4l-subdev2
>          pad0: Sink
> ...
>                  <- "imx219 0-0010":0 []
>                  <- "imx219 pixel 0-0010":0 []
>
> Adding some debug in gives:
>
> [   11.963362] imx-media: imx_media_create_of_links() for imx6-mipi-csi2
> [   11.963396] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> [   11.963422] imx-media: create_of_link(): /soc/ipu@2400000
> [   11.963450] imx-media: create_of_link(): /soc/ipu@2800000
> [   11.963478] imx-media: create_of_link(): /soc/aips-bus@2000000/iomuxc-gpr@20e0000/ipu2_csi1_mux
> [   11.963489] imx-media: imx6-mipi-csi2:4 -> ipu2_csi1_mux:0
> [   11.963522] imx-media: create_of_link(): /soc/aips-bus@2100000/i2c@21a0000/camera@10
> [   11.963533] imx-media: imx219 0-0010:0 -> imx6-mipi-csi2:0
> [   11.963549] imx-media: imx_media_create_of_links() for imx219 pixel 0-0010
> [   11.963577] imx-media: create_of_link(): /soc/aips-bus@2100000/mipi@21dc000
> [   11.963587] imx-media: imx219 pixel 0-0010:0 -> imx6-mipi-csi2:0
> [   11.963602] imx-media: imx_media_create_of_links() for imx219 0-0010
>
> Note that it's not created by imx6-mipi-csi2, but by imx-media delving
> around in the imx219 subdevs.
>
>  From what I can see, smiapp does the same thing that I do in imx219 -
> sets the subdev->dev member to point at the struct device, which then
> means that v4l2_device_register_subdev() will associate the same fwnode
> with both "imx219 pixel 0-0010" and "imx219 0-0010".

Ok, understood.

I realize imx_media_create_of_link() is a bit intrusive, and that's one 
of the things I'm trying to get rid of in this patchset. Unfortunately 
it's there for a reason which is described in patch 0021. But to explain 
here, the imx6-mipi-csi2 receiver outputs its four virtual channels on 
four separate source pads, and those connect to four different 
subdevices (video mux's and CSI's), and the problem is that only the 
first subdev that adds imx6-mipi-csi2 to its notifier asd list will get 
a notifier bind() callback (where links can be created to 
imx6-mipi-csi2) -- the other subdevs don't contain it in their asd lists 
so they never create the links to imx6-mipi-csi2. So until the 
requirement in v4l2-async that no notifiers can contain the same asd in 
its list is relaxed, this function will have to remain, but I can make 
it less intrusive (only create the missing links from imx6-mipi-csi2). 
I'm not able to find a cleaner workaround at the moment.

Steve