[PATCH 00/22] media: imx: Create media links in bound notifiers

[PATCH 00/22] media: imx: Create media links in bound notifiers

[Steve Longerbeam]

This series moves media link creation into the notifier bound callbacks
in the subdevices required by imx.

But to support the media graph on imx, a few improvements first needed
to be made to media core.

The default behavior of media_entity_get_fwnode_pad() fails when
the entity has multiple sink and/or source pads. Wrt imx, there are
three such entities: the imx6/7 csi-2 receivers, and the video mux.
All of those however have a 1:1 mapping from port numbers to pad
indeces.

So instead of implementing .get_fwnode_pad op for those entities,
or any other entities now or in the future that have multiple
source/sink pads, it is better to modify the default behavior
of media_entity_get_fwnode_pad() to first determine if the port
number at the provided endpoint firmware node corresponds to a
valid pad for the provided entity. That way the default behavior
will work for such entities that implement 1:1 mappings, without
requiring they implement the .get_fwnode_pad op. In other words,
the old behavior of media_entity_get_fwnode_pad() required entities
implement .get_fwnode_pad if they have multiple sink or source pads.
The new behavior requires entities implement .get_fwnode_pad _only_
if they have multiple sink or source pads, _and_ do not have 1:1
port:pad mappings.

Also, as part of the above work, it was discovered that all of the
current callers of media_entity_get_fwnode_pad() are not using that
function correctly. In all but one case the driver passes the firmware
node of the subdevice itself to the function, not one of its endpoint
nodes as the function expects. In more detail:

  - Cadence MIPI-CSI2 Receiver: Calls media_entity_get_fwnode_pad()
    in the subdev bound notifier callback, but passes sd->fwnode to
    the function. This is usually the fwnode of the subdevice itself,
    not one of its endpoint nodes. This only works now because there
    are no implementations of the .get_fwnode_pad op. This will break
    as soon as the bound subdevice implements the .get_fwnode_pad op.

  - ST Micro MIPID02 CSI-2: Calls media_entity_get_fwnode_pad() in the
    subdev bound notifier callback. Same issue as cadence driver, passes
    sd->fwnode.

  - Allwinner V3s CSI: Calls media_entity_get_fwnode_pad() in the
    completion notifier callback. Same issue as cadence driver, passes
    sd->fwnode.

  - Renesas R-Car MIPI CSI-2 Receiver: Calls media_entity_get_fwnode_pad()
    in the subdev bound notifier callback. In this case the driver passes
    the async subdev descriptor match fwnode. Again for most subdevices,
    this is the fwnode of the subdevice itself, not one of its endpoint
    nodes. However on the Salvator-X platform, the bound subdevice happens
    to be the ADV748x CSI-2 transmitter, which does indeed place the
    endpoint node in the asd match fwnode. But the problem is that the
    driver is now making assumptions about what subdevices it is binding
    to.

To resolve the above issues, this series adds a new function
media_create_fwnode_pad_links(), which creates the media links from/to
all pads on a remote entity, to/from a single specific local entity pad,
using the remote's endpoint fwnodes. It's API makes it convenient to call
from subdev bound notifier callbacks.

Another function media_create_fwnode_links() is added that will create
links from/to all pads on a remote entity, to/from all pads on a local
entity. It's API also makes it convenient to call from subdev bound
notifier callbacks, and can be called for entities that link to remote
entities via multiple pads (for example the video-mux which has multiple
sink pads that link to multiple bound subdevices).

This series has been tested on i.MX6 SabreSD, SabreLite, and SabreAuto
platforms, and the Renesas Salvator-X platform.

The series needs testing on i.MX7, Cadence, ST Micro MIPID02, and
Allwinner V3s platforms. Testing only needs to verify that the media
graph is unchanged, no stream testing is required. If the media graph
is broken, it means the subdevice(s) that bind to the drivers listed
above need to implement the .get_fwnode_pad op.


Steve Longerbeam (22):
  media: entity: Pass entity to get_fwnode_pad operation
  media: entity: Modify default behavior of media_entity_get_fwnode_pad
  media: entity: Convert media_entity_get_fwnode_pad() args to const
  media: Move v4l2_fwnode_parse_link from v4l2 to driver base
  media: entity: Add functions to convert fwnode endpoints to media
    links
  media: adv748x: csi2: Implement get_fwnode_pad
  media: rcar-csi2: Fix fwnode media link creation
  media: cadence: csi2rx: Fix fwnode media link creation
  media: sun6i: Fix fwnode media link creation
  media: st-mipid02: Fix fwnode media link creation
  media: v4l2-fwnode: Pass notifier to
    v4l2_async_register_fwnode_subdev()
  media: video-mux: Create media links in bound notifier
  media: imx: mipi csi-2: Create media links in bound notifier
  media: imx7-mipi-csis: Create media links in bound notifier
  media: imx7-media-csi: Create media links in bound notifier
  media: imx: csi: Implement get_fwnode_pad
  media: imx: csi: Embed notifier in struct csi_priv
  media: imx: csi: Create media links in bound notifier
  media: imx: csi: Lookup endpoint fwnode with
    media_entity_get_fwnode_pad
  media: imx: Add is_ipu_internal_subdev()
  media: imx: Use media_create_fwnode_links for external links
  media: imx: TODO: Remove issues regarding media link creation

 drivers/base/property.c                       |  63 ++++++
 drivers/media/i2c/adv748x/adv748x-csi2.c      |  21 ++
 drivers/media/i2c/st-mipid02.c                |  20 +-
 drivers/media/mc/mc-entity.c                  | 209 +++++++++++++++++-
 drivers/media/platform/cadence/cdns-csi2rx.c  |  27 +--
 drivers/media/platform/rcar-vin/rcar-csi2.c   |  23 +-
 .../platform/sunxi/sun6i-csi/sun6i_csi.c      |  36 +--
 drivers/media/platform/video-mux.c            |  30 ++-
 drivers/media/platform/xilinx/xilinx-vipp.c   |  69 +++---
 drivers/media/v4l2-core/v4l2-fwnode.c         |  50 +----
 drivers/staging/media/imx/TODO                |  29 ---
 drivers/staging/media/imx/imx-media-csi.c     |  92 +++++---
 .../staging/media/imx/imx-media-dev-common.c  |  49 ++--
 drivers/staging/media/imx/imx-media-of.c      | 114 ----------
 drivers/staging/media/imx/imx-media-utils.c   |  33 +++
 drivers/staging/media/imx/imx-media.h         |  13 +-
 drivers/staging/media/imx/imx6-mipi-csi2.c    |  29 ++-
 drivers/staging/media/imx/imx7-media-csi.c    |  54 +++--
 drivers/staging/media/imx/imx7-mipi-csis.c    |  30 ++-
 include/linux/fwnode.h                        |  14 ++
 include/linux/property.h                      |   5 +
 include/media/media-entity.h                  |  99 ++++++++-
 include/media/v4l2-fwnode.h                   |  56 +----
 23 files changed, 721 insertions(+), 444 deletions(-)

-- 
2.17.1

[PATCH 01/22] media: entity: Pass entity to get_fwnode_pad operation

[Steve Longerbeam]

Add a missing pointer to the entity in the media_entity operation
get_fwnode_pad.

Fixes: ae45cd5efc120 ("[media] media: entity: Add get_fwnode_pad entity
operation")

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/mc/mc-entity.c | 2 +-
 include/media/media-entity.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 7c429ce98bae..c333320f790a 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -386,7 +386,7 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
 	if (ret)
 		return ret;
 
-	ret = entity->ops->get_fwnode_pad(&endpoint);
+	ret = entity->ops->get_fwnode_pad(entity, &endpoint);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 8cb2c504a05c..cde80ad029b7 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -212,7 +212,8 @@ struct media_pad {
  *    mutex held.
  */
 struct media_entity_operations {
-	int (*get_fwnode_pad)(struct fwnode_endpoint *endpoint);
+	int (*get_fwnode_pad)(struct media_entity *entity,
+			      struct fwnode_endpoint *endpoint);
 	int (*link_setup)(struct media_entity *entity,
 			  const struct media_pad *local,
 			  const struct media_pad *remote, u32 flags);
-- 
2.17.1

[PATCH 02/22] media: entity: Modify default behavior of media_entity_get_fwnode_pad

[Steve Longerbeam]

Modify the default behavior of media_entity_get_fwnode_pad() (when the
entity does not provide the get_fwnode_pad op) to first assume the entity
implements a 1:1 mapping between fwnode port number and media pad index.

If the 1:1 mapping is not valid, e.g. the port number falls outside the
entity's pad index range, or the pad at that port number doesn't match the
given direction_flags, fall-back to the previous behavior that searches
the entity for the first pad that matches the given direction_flags.

The previous default behavior can choose the wrong pad for entities with
multiple sink or source pads. With this change the function will choose
the correct pad index if the entity implements a 1:1 port to pad mapping
at that port.

Add some comments to the @get_fwnode_pad operation to make it more clear
under what conditions entities must implement the operation.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++-----
 include/media/media-entity.h | 21 +++++++++++++++------
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index c333320f790a..47a39d9383d8 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
 				unsigned long direction_flags)
 {
 	struct fwnode_endpoint endpoint;
-	unsigned int i;
 	int ret;
 
+	ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
+	if (ret)
+		return ret;
+
 	if (!entity->ops || !entity->ops->get_fwnode_pad) {
+		unsigned int i;
+
+		/*
+		 * for the default case, first try a 1:1 mapping between
+		 * fwnode port number and pad index.
+		 */
+		ret = endpoint.port;
+		if (ret < entity->num_pads &&
+		    (entity->pads[ret].flags & direction_flags))
+			return ret;
+
+		/*
+		 * if that didn't work search the entity for the first
+		 * pad that matches the @direction_flags.
+		 */
 		for (i = 0; i < entity->num_pads; i++) {
 			if (entity->pads[i].flags & direction_flags)
 				return i;
 		}
 
+		/* else fail */
 		return -ENXIO;
 	}
 
-	ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
-	if (ret)
-		return ret;
-
 	ret = entity->ops->get_fwnode_pad(entity, &endpoint);
 	if (ret < 0)
 		return ret;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index cde80ad029b7..ed00adb4313b 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -199,6 +199,12 @@ struct media_pad {
  * @get_fwnode_pad:	Return the pad number based on a fwnode endpoint or
  *			a negative value on error. This operation can be used
  *			to map a fwnode to a media pad number. Optional.
+ *			Entities do not need to implement this operation
+ *			unless two conditions are met:
+ *			- the entity has more than one sink and/or source
+ *			  pad, _and_
+ *			- the entity does not implement a 1:1 mapping of
+ *			  fwnode port numbers to pad indexes.
  * @link_setup:		Notify the entity of link changes. The operation can
  *			return an error, in which case link setup will be
  *			cancelled. Optional.
@@ -858,21 +864,24 @@ struct media_link *media_entity_find_link(struct media_pad *source,
 struct media_pad *media_entity_remote_pad(const struct media_pad *pad);
 
 /**
- * media_entity_get_fwnode_pad - Get pad number from fwnode
+ * media_entity_get_fwnode_pad - Get pad number from an endpoint fwnode
  *
  * @entity: The entity
- * @fwnode: Pointer to the fwnode_handle which should be used to find the pad
+ * @fwnode: Pointer to the endpoint fwnode_handle which should be used to
+ *          find the pad
  * @direction_flags: Expected direction of the pad, as defined in
  *		     :ref:`include/uapi/linux/media.h <media_header>`
  *		     (seek for ``MEDIA_PAD_FL_*``)
  *
  * This function can be used to resolve the media pad number from
- * a fwnode. This is useful for devices which use more complex
- * mappings of media pads.
+ * an endpoint fwnode. This is useful for devices which use more
+ * complex mappings of media pads.
  *
  * If the entity does not implement the get_fwnode_pad() operation
- * then this function searches the entity for the first pad that
- * matches the @direction_flags.
+ * then this function first assumes the entity implements a 1:1 mapping
+ * between fwnode port number and media pad index. If the 1:1 mapping
+ * is not valid, then the function searches the entity for the first pad
+ * that matches the @direction_flags.
  *
  * Return: returns the pad number on success or a negative error code.
  */
-- 
2.17.1

[PATCH 03/22] media: entity: Convert media_entity_get_fwnode_pad() args to const

[Steve Longerbeam]

The function media_entity_get_fwnode_pad() can be passed the
const local_fwnode member from a struct fwnode_endpoint, so
the fwnode argument should be a const pointer. Change the
direction_flags argument to const in the process.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/mc/mc-entity.c | 4 ++--
 include/media/media-entity.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 47a39d9383d8..e9e090244fd4 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -366,8 +366,8 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph)
 EXPORT_SYMBOL_GPL(media_graph_walk_next);
 
 int media_entity_get_fwnode_pad(struct media_entity *entity,
-				struct fwnode_handle *fwnode,
-				unsigned long direction_flags)
+				const struct fwnode_handle *fwnode,
+				const unsigned long direction_flags)
 {
 	struct fwnode_endpoint endpoint;
 	int ret;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index ed00adb4313b..de7fc3676b5a 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -886,8 +886,8 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad);
  * Return: returns the pad number on success or a negative error code.
  */
 int media_entity_get_fwnode_pad(struct media_entity *entity,
-				struct fwnode_handle *fwnode,
-				unsigned long direction_flags);
+				const struct fwnode_handle *fwnode,
+				const unsigned long direction_flags);
 
 /**
  * media_graph_walk_init - Allocate resources used by graph walk.
-- 
2.17.1

[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

[PATCH 05/22] media: entity: Add functions to convert fwnode endpoints to media links

[Steve Longerbeam]

Adds two functions:

media_create_fwnode_pad_links(), which converts fwnode endpoints that
connect a local pad to all pads on a remote entity into media links.

media_create_fwnode_links(), which converts fwnode endpoints that
connect all pads from a local entity to all pads on a remote entity into
media links.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/mc/mc-entity.c | 178 +++++++++++++++++++++++++++++++++++
 include/media/media-entity.h |  71 ++++++++++++++
 2 files changed, 249 insertions(+)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index e9e090244fd4..45bbd6c91019 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -787,6 +787,184 @@ int media_create_pad_links(const struct media_device *mdev,
 }
 EXPORT_SYMBOL_GPL(media_create_pad_links);
 
+static int __media_create_fwnode_pad_link(struct media_pad *local_pad,
+					struct media_entity *remote,
+					const struct fwnode_handle *remote_ep,
+					const u32 flags)
+{
+	struct media_entity *local = local_pad->entity;
+	unsigned long local_dir = local_pad->flags;
+	unsigned long remote_dir = (local_dir & MEDIA_PAD_FL_SOURCE) ?
+		MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
+	struct media_entity *src, *sink;
+	int src_pad, sink_pad;
+	int remote_pad;
+	int ret;
+
+	remote_pad = media_entity_get_fwnode_pad(remote, remote_ep,
+						 remote_dir);
+	if (remote_pad < 0)
+		return 0;
+
+	if (local_dir & MEDIA_PAD_FL_SOURCE) {
+		src = local;
+		src_pad = local_pad->index;
+		sink = remote;
+		sink_pad = remote_pad;
+	} else {
+		src = remote;
+		src_pad = remote_pad;
+		sink = local;
+		sink_pad = local_pad->index;
+	}
+
+	/* make sure link doesn't already exist */
+	if (media_entity_find_link(&src->pads[src_pad],
+				   &sink->pads[sink_pad]))
+		return 0;
+
+	ret = media_create_pad_link(src, src_pad, sink, sink_pad, flags);
+	if (ret) {
+		dev_dbg(sink->graph_obj.mdev->dev,
+			"%s:%d -> %s:%d failed with %d\n",
+			src->name, src_pad, sink->name, sink_pad,
+			ret);
+		return ret;
+	}
+
+	dev_dbg(sink->graph_obj.mdev->dev, "%s:%d -> %s:%d\n",
+		src->name, src_pad, sink->name, sink_pad);
+
+	return 0;
+}
+
+int __media_create_fwnode_pad_links(struct media_pad *local_pad,
+				    const struct fwnode_handle *local_fwnode,
+				    struct media_entity *remote,
+				    const struct fwnode_handle *remote_fwnode,
+				    const u32 link_flags)
+{
+	struct fwnode_handle *endpoint;
+
+	fwnode_graph_for_each_endpoint(remote_fwnode, endpoint) {
+		struct fwnode_link link;
+		int ret;
+
+		ret = fwnode_graph_parse_link(endpoint, &link);
+		if (ret)
+			continue;
+
+		/*
+		 * if this endpoint does not link to the local fwnode
+		 * device, ignore it and continue.
+		 */
+		if (link.remote_port_parent != local_fwnode) {
+			fwnode_graph_put_link(&link);
+			continue;
+		}
+
+		ret = __media_create_fwnode_pad_link(local_pad,
+						     remote, endpoint,
+						     link_flags);
+
+		fwnode_graph_put_link(&link);
+
+		if (ret) {
+			fwnode_handle_put(endpoint);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__media_create_fwnode_pad_links);
+
+int media_create_fwnode_pad_links(struct media_pad *local_pad,
+				  const struct fwnode_handle *local_fwnode,
+				  struct media_entity *remote,
+				  const struct fwnode_handle *remote_fwnode,
+				  const u32 link_flags)
+{
+	struct media_device *mdev = local_pad->entity->graph_obj.mdev;
+	int ret;
+
+	mutex_lock(&mdev->graph_mutex);
+	ret = __media_create_fwnode_pad_links(local_pad, local_fwnode,
+					      remote, remote_fwnode,
+					      link_flags);
+	mutex_unlock(&mdev->graph_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(media_create_fwnode_pad_links);
+
+int __media_create_fwnode_links(struct media_entity *local,
+				const struct fwnode_handle *local_fwnode,
+				struct media_entity *remote,
+				const struct fwnode_handle *remote_fwnode,
+				const u32 link_flags)
+{
+	struct fwnode_handle *endpoint;
+
+	fwnode_graph_for_each_endpoint(local_fwnode, endpoint) {
+		struct fwnode_link link;
+		int local_pad;
+		int ret;
+
+		local_pad = media_entity_get_fwnode_pad(local, endpoint,
+							MEDIA_PAD_FL_SINK |
+							MEDIA_PAD_FL_SOURCE);
+		if (local_pad < 0)
+			continue;
+
+		ret = fwnode_graph_parse_link(endpoint, &link);
+		if (ret)
+			continue;
+
+		/*
+		 * if this endpoint does not link to the remote fwnode
+		 * device, ignore it and continue.
+		 */
+		if (link.remote_port_parent != remote_fwnode) {
+			fwnode_graph_put_link(&link);
+			continue;
+		}
+
+		ret = __media_create_fwnode_pad_link(&local->pads[local_pad],
+						     remote,
+						     link.remote.local_fwnode,
+						     link_flags);
+
+		fwnode_graph_put_link(&link);
+
+		if (ret) {
+			fwnode_handle_put(endpoint);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__media_create_fwnode_links);
+
+int media_create_fwnode_links(struct media_entity *local,
+			      const struct fwnode_handle *local_fwnode,
+			      struct media_entity *remote,
+			      const struct fwnode_handle *remote_fwnode,
+			      const u32 link_flags)
+{
+	struct media_device *mdev = local->graph_obj.mdev;
+	int ret;
+
+	mutex_lock(&mdev->graph_mutex);
+	ret = __media_create_fwnode_links(local, local_fwnode,
+					  remote, remote_fwnode, link_flags);
+	mutex_unlock(&mdev->graph_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(media_create_fwnode_links);
+
 void __media_entity_remove_links(struct media_entity *entity)
 {
 	struct media_link *link, *tmp;
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index de7fc3676b5a..781ac2df6460 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -772,6 +772,77 @@ int media_create_pad_links(const struct media_device *mdev,
 			   u32 flags,
 			   const bool allow_both_undefined);
 
+/**
+ * media_create_fwnode_links() - create links formed by the fwnode
+ *			     endpoints between a known local pad and
+ *			     a remote entity.
+ *
+ * @local_pad: Pointer to &media_pad of the local media pad.
+ * @local_fwnode: Pointer to the local device's firmware node.
+ * @remote: Pointer to &media_entity of the remote device.
+ * @remote_fwnode: Pointer to the remote device's firmware node.
+ * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
+ *
+ * .. note::
+ *
+ *    Before calling this function, media_entity_pads_init() and
+ *    media_device_register_entity() should be called previously for
+ *    both entities to be linked.
+ *
+ *    Locked (via the mdev graph_mutex) and unlocked versions of this
+ *    function are provided. If this function is called from v4l2-async
+ *    notifier bound handlers, the locked version should be used to
+ *    prevent races with other subdevices loading and binding to their
+ *    notifiers in parallel. The unlocked version can for example be
+ *    called from v4l2-async notifier complete handlers, after all
+ *    subdevices have loaded and bound.
+ */
+int __media_create_fwnode_pad_links(struct media_pad *local_pad,
+				    const struct fwnode_handle *local_fwnode,
+				    struct media_entity *remote,
+				    const struct fwnode_handle *remote_fwnode,
+				    const u32 link_flags);
+int media_create_fwnode_pad_links(struct media_pad *local_pad,
+				  const struct fwnode_handle *local_fwnode,
+				  struct media_entity *remote,
+				  const struct fwnode_handle *remote_fwnode,
+				  const u32 link_flags);
+
+/**
+ * media_create_fwnode_links() - create links formed by the fwnode
+ *			     endpoints between two entities.
+ *
+ * @local: Pointer to &media_entity of the local device.
+ * @local_fwnode: Pointer to the local device's firmware node.
+ * @remote: Pointer to &media_entity of the remote device.
+ * @remote_fwnode: Pointer to the remote device's firmware node.
+ * @link_flags: Link flags, as defined in include/uapi/linux/media.h.
+ *
+ * .. note::
+ *
+ *    Before calling this function, media_entity_pads_init() and
+ *    media_device_register_entity() should be called previously for
+ *    both entities to be linked.
+ *
+ *    Locked (via the mdev graph_mutex) and unlocked versions of this
+ *    function are provided. If this function is called from v4l2-async
+ *    notifier bound handlers, the locked version should be used to
+ *    prevent races with other subdevices loading and binding to their
+ *    notifiers in parallel. The unlocked version can for example be
+ *    called from v4l2-async notifier complete handlers, after all
+ *    subdevices have loaded and bound.
+ */
+int __media_create_fwnode_links(struct media_entity *local,
+				const struct fwnode_handle *local_fwnode,
+				struct media_entity *remote,
+				const struct fwnode_handle *remote_fwnode,
+				const u32 link_flags);
+int media_create_fwnode_links(struct media_entity *local,
+			      const struct fwnode_handle *local_fwnode,
+			      struct media_entity *remote,
+			      const struct fwnode_handle *remote_fwnode,
+			      const u32 link_flags);
+
 void __media_entity_remove_links(struct media_entity *entity);
 
 /**
-- 
2.17.1

[PATCH 06/22] media: adv748x: csi2: Implement get_fwnode_pad

[Steve Longerbeam]

If the given endpoint fwnode passed to the .get_fwnode_pad() op is
the adv748x-csi2 TXA/TXB source endpoint, return the associated media
pad index ADV748X_CSI2_SOURCE. The adv748x-csi2 has no other media pads
that are associated with fwnode endpoints.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/i2c/adv748x/adv748x-csi2.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
index 2091cda50935..810085a1f2f0 100644
--- a/drivers/media/i2c/adv748x/adv748x-csi2.c
+++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
@@ -228,6 +228,24 @@ static const struct v4l2_subdev_ops adv748x_csi2_ops = {
 	.pad = &adv748x_csi2_pad_ops,
 };
 
+/* -----------------------------------------------------------------------------
+ * media_entity_operations
+ */
+
+static int adv748x_csi2_get_fwnode_pad(struct media_entity *entity,
+				       struct fwnode_endpoint *endpoint)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
+
+	return endpoint->local_fwnode == tx->sd.fwnode ?
+		ADV748X_CSI2_SOURCE : -ENXIO;
+}
+
+static const struct media_entity_operations adv748x_csi2_entity_ops = {
+	.get_fwnode_pad = adv748x_csi2_get_fwnode_pad,
+};
+
 /* -----------------------------------------------------------------------------
  * Subdev module and controls
  */
@@ -295,6 +313,9 @@ int adv748x_csi2_init(struct adv748x_state *state, struct adv748x_csi2 *tx)
 	/* Register internal ops for incremental subdev registration */
 	tx->sd.internal_ops = &adv748x_csi2_internal_ops;
 
+	/* Register media_entity ops */
+	tx->sd.entity.ops = &adv748x_csi2_entity_ops;
+
 	tx->pads[ADV748X_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
 	tx->pads[ADV748X_CSI2_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
 
-- 
2.17.1

[PATCH 07/22] media: rcar-csi2: Fix fwnode media link creation

[Steve Longerbeam]

rcsi2_notify_bound() passes the bound subdev's match fwnode to
media_entity_get_fwnode_pad() to determine the subdev's source
pad for creating the media link to it. When the bound subdev is
the adv748x-csi2 transmitter, this is in fact correctly the endpoint
fwnode. For other subdevices this likely will not be the case, the
asd match fwnode is usually not an endpoint fwnode but rather the
port parent fwnode. So rcar-csi2 will fail to get the correct source
pad for bound subdev's other than the adv748x.

To fix and make rcar-csi2 connect more generally to other subdevices,
replace the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().

Fixes: 769afd212b160 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver")

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 23 +++++++++------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index c14af1b929df..13242cbad8d3 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -738,23 +738,20 @@ static int rcsi2_notify_bound(struct v4l2_async_notifier *notifier,
 			      struct v4l2_async_subdev *asd)
 {
 	struct rcar_csi2 *priv = notifier_to_csi2(notifier);
-	int pad;
+	int ret;
 
-	pad = media_entity_get_fwnode_pad(&subdev->entity, asd->match.fwnode,
-					  MEDIA_PAD_FL_SOURCE);
-	if (pad < 0) {
-		dev_err(priv->dev, "Failed to find pad for %s\n", subdev->name);
-		return pad;
-	}
+	ret = media_create_fwnode_pad_links(&priv->subdev.entity.pads[0],
+					    dev_fwnode(priv->dev),
+					    &subdev->entity,
+					    dev_fwnode(subdev->dev),
+					    MEDIA_LNK_FL_ENABLED |
+					    MEDIA_LNK_FL_IMMUTABLE);
+	if (ret)
+		return ret;
 
 	priv->remote = subdev;
 
-	dev_dbg(priv->dev, "Bound %s pad: %d\n", subdev->name, pad);
-
-	return media_create_pad_link(&subdev->entity, pad,
-				     &priv->subdev.entity, 0,
-				     MEDIA_LNK_FL_ENABLED |
-				     MEDIA_LNK_FL_IMMUTABLE);
+	return 0;
 }
 
 static void rcsi2_notify_unbind(struct v4l2_async_notifier *notifier,
-- 
2.17.1

[PATCH 08/22] media: cadence: csi2rx: Fix fwnode media link creation

[Steve Longerbeam]

csi2rx_async_bound() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.

Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().

Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 27 ++++++++------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 31ace114eda1..d478800a3859 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -83,7 +83,6 @@ struct csi2rx_priv {
 	/* Remote source */
 	struct v4l2_async_subdev	asd;
 	struct v4l2_subdev		*source_subdev;
-	int				source_pad;
 };
 
 static inline
@@ -251,26 +250,20 @@ static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
 {
 	struct v4l2_subdev *subdev = notifier->sd;
 	struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
+	int ret;
 
-	csi2rx->source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
-							 s_subdev->fwnode,
-							 MEDIA_PAD_FL_SOURCE);
-	if (csi2rx->source_pad < 0) {
-		dev_err(csi2rx->dev, "Couldn't find output pad for subdev %s\n",
-			s_subdev->name);
-		return csi2rx->source_pad;
-	}
+	ret = media_create_fwnode_pad_links(&csi2rx->subdev.entity.pads[0],
+					    dev_fwnode(csi2rx->dev),
+					    &s_subdev->entity,
+					    dev_fwnode(s_subdev->dev),
+					    MEDIA_LNK_FL_ENABLED |
+					    MEDIA_LNK_FL_IMMUTABLE);
+	if (ret)
+		return ret;
 
 	csi2rx->source_subdev = s_subdev;
 
-	dev_dbg(csi2rx->dev, "Bound %s pad: %d\n", s_subdev->name,
-		csi2rx->source_pad);
-
-	return media_create_pad_link(&csi2rx->source_subdev->entity,
-				     csi2rx->source_pad,
-				     &csi2rx->subdev.entity, 0,
-				     MEDIA_LNK_FL_ENABLED |
-				     MEDIA_LNK_FL_IMMUTABLE);
+	return 0;
 }
 
 static const struct v4l2_async_notifier_operations csi2rx_notifier_ops = {
-- 
2.17.1

[PATCH 09/22] media: sun6i: Fix fwnode media link creation

[Steve Longerbeam]

sun6i_csi_link_entity() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.

Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().

Fixes: 5cc7522d89655 ("media: sun6i: Add support for Allwinner CSI V3s")

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 .../platform/sunxi/sun6i-csi/sun6i_csi.c      | 36 +++----------------
 1 file changed, 5 insertions(+), 31 deletions(-)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index 6e0e894154f4..9cf726b5c25a 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -630,37 +630,11 @@ static int sun6i_csi_link_entity(struct sun6i_csi *csi,
 				 struct media_entity *entity,
 				 struct fwnode_handle *fwnode)
 {
-	struct media_entity *sink;
-	struct media_pad *sink_pad;
-	int src_pad_index;
-	int ret;
-
-	ret = media_entity_get_fwnode_pad(entity, fwnode, MEDIA_PAD_FL_SOURCE);
-	if (ret < 0) {
-		dev_err(csi->dev, "%s: no source pad in external entity %s\n",
-			__func__, entity->name);
-		return -EINVAL;
-	}
-
-	src_pad_index = ret;
-
-	sink = &csi->video.vdev.entity;
-	sink_pad = &csi->video.pad;
-
-	dev_dbg(csi->dev, "creating %s:%u -> %s:%u link\n",
-		entity->name, src_pad_index, sink->name, sink_pad->index);
-	ret = media_create_pad_link(entity, src_pad_index, sink,
-				    sink_pad->index,
-				    MEDIA_LNK_FL_ENABLED |
-				    MEDIA_LNK_FL_IMMUTABLE);
-	if (ret < 0) {
-		dev_err(csi->dev, "failed to create %s:%u -> %s:%u link\n",
-			entity->name, src_pad_index,
-			sink->name, sink_pad->index);
-		return ret;
-	}
-
-	return 0;
+	return media_create_fwnode_pad_links(&csi->video.pad,
+					     dev_fwnode(csi->dev),
+					     entity, fwnode,
+					     MEDIA_LNK_FL_ENABLED |
+					     MEDIA_LNK_FL_IMMUTABLE);
 }
 
 static int sun6i_subdev_notify_complete(struct v4l2_async_notifier *notifier)
-- 
2.17.1

[PATCH 10/22] media: st-mipid02: Fix fwnode media link creation

[Steve Longerbeam]

mipid02_async_bound() passes the bound subdev's sd->fwnode to
media_entity_get_fwnode_pad(). This is likely not an endpoint
fwnode as required by media_entity_get_fwnode_pad(), for most
subdevices it is the port parent of endpoint fwnode(s). This has only
worked before because no entities have implemented the .get_fwnode_pad()
op yet, and the default behavior of media_entity_get_fwnode_pad()
was to ignore the passed fwnode and return the first pad that matches
the given direction flags.

Fix this by replacing the calls to media_entity_get_fwnode_pad() and
media_create_pad_link() with a call to media_create_fwnode_pad_links().

Fixes: 642bb5e88fed8 ("media: st-mipid02: MIPID02 CSI-2 to PARALLEL
bridge driver")

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/i2c/st-mipid02.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index 81285b8d5cfb..b26d5550874f 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -798,24 +798,16 @@ static int mipid02_async_bound(struct v4l2_async_notifier *notifier,
 {
 	struct mipid02_dev *bridge = to_mipid02_dev(notifier->sd);
 	struct i2c_client *client = bridge->i2c_client;
-	int source_pad;
 	int ret;
 
 	dev_dbg(&client->dev, "sensor_async_bound call %p", s_subdev);
 
-	source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
-						 s_subdev->fwnode,
-						 MEDIA_PAD_FL_SOURCE);
-	if (source_pad < 0) {
-		dev_err(&client->dev, "Couldn't find output pad for subdev %s\n",
-			s_subdev->name);
-		return source_pad;
-	}
-
-	ret = media_create_pad_link(&s_subdev->entity, source_pad,
-				    &bridge->sd.entity, 0,
-				    MEDIA_LNK_FL_ENABLED |
-				    MEDIA_LNK_FL_IMMUTABLE);
+	ret = media_create_fwnode_pad_links(&bridge->sd.entity.pads[0],
+					    dev_fwnode(&client->dev),
+					    &s_subdev->entity,
+					    dev_fwnode(s_subdev->dev),
+					    MEDIA_LNK_FL_ENABLED |
+					    MEDIA_LNK_FL_IMMUTABLE);
 	if (ret) {
 		dev_err(&client->dev, "Couldn't create media link %d", ret);
 		return ret;
-- 
2.17.1

[PATCH 11/22] media: v4l2-fwnode: Pass notifier to v4l2_async_register_fwnode_subdev()

[Steve Longerbeam]

Instead of allocating a notifier in v4l2_async_register_fwnode_subdev(),
have the caller provide one. This allows the caller to implement
notifier ops (bind, unbind).

The caller is now responsible for first initializing its notifier with a
call to v4l2_async_notifier_init().

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/platform/video-mux.c         |  6 +++++-
 drivers/media/v4l2-core/v4l2-fwnode.c      | 11 +----------
 drivers/staging/media/imx/imx6-mipi-csi2.c |  5 ++++-
 drivers/staging/media/imx/imx7-media-csi.c |  5 ++++-
 drivers/staging/media/imx/imx7-mipi-csis.c |  5 ++++-
 include/media/v4l2-fwnode.h                | 12 ++++++++----
 6 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
index ddd0e338f9e4..ca1cef783646 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -21,6 +21,7 @@
 
 struct video_mux {
 	struct v4l2_subdev subdev;
+	struct v4l2_async_notifier notifier;
 	struct media_pad *pads;
 	struct v4l2_mbus_framefmt *format_mbus;
 	struct mux_control *mux;
@@ -354,8 +355,11 @@ static int video_mux_async_register(struct video_mux *vmux,
 	for (i = 0; i < num_input_pads; i++)
 		ports[i] = i;
 
+	v4l2_async_notifier_init(&vmux->notifier);
+
 	ret = v4l2_async_register_fwnode_subdev(
-		&vmux->subdev, sizeof(struct v4l2_async_subdev),
+		&vmux->subdev, &vmux->notifier,
+		sizeof(struct v4l2_async_subdev),
 		ports, num_input_pads, video_mux_parse_endpoint);
 
 	kfree(ports);
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 5d4ce4aa3fdc..8f5b3fdea061 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -1124,12 +1124,12 @@ int v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)
 EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
 
 int v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
+				      struct v4l2_async_notifier *notifier,
 				      size_t asd_struct_size,
 				      unsigned int *ports,
 				      unsigned int num_ports,
 				      parse_endpoint_func parse_endpoint)
 {
-	struct v4l2_async_notifier *notifier;
 	struct device *dev = sd->dev;
 	struct fwnode_handle *fwnode;
 	int ret;
@@ -1141,12 +1141,6 @@ int v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
 	if (!fwnode_device_is_available(fwnode))
 		return -ENODEV;
 
-	notifier = kzalloc(sizeof(*notifier), GFP_KERNEL);
-	if (!notifier)
-		return -ENOMEM;
-
-	v4l2_async_notifier_init(notifier);
-
 	if (!ports) {
 		ret = v4l2_async_notifier_parse_fwnode_endpoints(dev, notifier,
 								 asd_struct_size,
@@ -1171,15 +1165,12 @@ int v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
 	if (ret < 0)
 		goto out_unregister;
 
-	sd->subdev_notifier = notifier;
-
 	return 0;
 
 out_unregister:
 	v4l2_async_notifier_unregister(notifier);
 out_cleanup:
 	v4l2_async_notifier_cleanup(notifier);
-	kfree(notifier);
 
 	return ret;
 }
diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index f29e28df36ed..408ee2765e77 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -35,6 +35,7 @@
 struct csi2_dev {
 	struct device          *dev;
 	struct v4l2_subdev      sd;
+	struct v4l2_async_notifier notifier;
 	struct media_pad       pad[CSI2_NUM_PADS];
 	struct clk             *dphy_clk;
 	struct clk             *pllref_clk;
@@ -643,8 +644,10 @@ static int csi2_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, &csi2->sd);
 
+	v4l2_async_notifier_init(&csi2->notifier);
+
 	ret = v4l2_async_register_fwnode_subdev(
-		&csi2->sd, sizeof(struct v4l2_async_subdev),
+		&csi2->sd, &csi2->notifier, sizeof(struct v4l2_async_subdev),
 		&sink_port, 1, csi2_parse_endpoint);
 	if (ret)
 		goto dphy_off;
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 500b4c08d967..a1c96c52a606 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -155,6 +155,7 @@
 struct imx7_csi {
 	struct device *dev;
 	struct v4l2_subdev sd;
+	struct v4l2_async_notifier notifier;
 	struct imx_media_video_dev *vdev;
 	struct imx_media_dev *imxmd;
 	struct media_pad pad[IMX7_CSI_PADS_NUM];
@@ -1250,7 +1251,9 @@ static int imx7_csi_probe(struct platform_device *pdev)
 	v4l2_ctrl_handler_init(&csi->ctrl_hdlr, 0);
 	csi->sd.ctrl_handler = &csi->ctrl_hdlr;
 
-	ret = v4l2_async_register_fwnode_subdev(&csi->sd,
+	v4l2_async_notifier_init(&csi->notifier);
+
+	ret = v4l2_async_register_fwnode_subdev(&csi->sd, &csi->notifier,
 						sizeof(struct v4l2_async_subdev),
 						NULL, 0,
 						imx7_csi_parse_endpoint);
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index d1cdf011c8f1..f71d9183cad2 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -223,6 +223,7 @@ struct csi_state {
 	struct device *dev;
 	struct media_pad pads[CSIS_PADS_NUM];
 	struct v4l2_subdev mipi_sd;
+	struct v4l2_async_notifier notifier;
 	struct v4l2_subdev *src_sd;
 
 	u8 index;
@@ -892,7 +893,9 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
 
 	v4l2_set_subdevdata(mipi_sd, &pdev->dev);
 
-	ret = v4l2_async_register_fwnode_subdev(mipi_sd,
+	v4l2_async_notifier_init(&state->notifier);
+
+	ret = v4l2_async_register_fwnode_subdev(mipi_sd, &state->notifier,
 						sizeof(struct v4l2_async_subdev),
 						&sink_port, 1,
 						mipi_csis_parse_endpoint);
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index f81f8bf34526..27a7b78149c2 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -331,6 +331,7 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
  *					and parses fwnode endpoints
  *
  * @sd: pointer to struct &v4l2_subdev
+ * @notifier: the sub-device's notifier.
  * @asd_struct_size: size of the driver's async sub-device struct, including
  *		     sizeof(struct v4l2_async_subdev). The &struct
  *		     v4l2_async_subdev shall be the first member of
@@ -343,13 +344,15 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
  *		    endpoint. Optional.
  *
  * This function is just like v4l2_async_register_subdev() with the
- * exception that calling it will also allocate a notifier for the
- * sub-device, parse the sub-device's firmware node endpoints using
- * v4l2_async_notifier_parse_fwnode_endpoints() or
+ * exception that calling it will also parse the sub-device's firmware
+ * node endpoints using v4l2_async_notifier_parse_fwnode_endpoints() or
  * v4l2_async_notifier_parse_fwnode_endpoints_by_port(), and
- * registers the sub-device notifier. The sub-device is similarly
+ * registers the sub-device's notifier. The sub-device is similarly
  * unregistered by calling v4l2_async_unregister_subdev().
  *
+ * The caller must first initialize the notifier with a call to
+ * v4l2_async_notifier_init().
+ *
  * While registered, the subdev module is marked as in-use.
  *
  * An error is returned if the module is no longer loaded on any attempts
@@ -357,6 +360,7 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
  */
 int
 v4l2_async_register_fwnode_subdev(struct v4l2_subdev *sd,
+				  struct v4l2_async_notifier *notifier,
 				  size_t asd_struct_size,
 				  unsigned int *ports,
 				  unsigned int num_ports,
-- 
2.17.1

[PATCH 12/22] media: video-mux: Create media links in bound notifier

[Steve Longerbeam]

Implement a notifier bound op to register media links from the remote
sub-device's source pads to all of the video-mux sink pads.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/media/platform/video-mux.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
index ca1cef783646..de5b0698fc1d 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -36,6 +36,12 @@ static const struct v4l2_mbus_framefmt video_mux_format_mbus_default = {
 	.field = V4L2_FIELD_NONE,
 };
 
+static inline struct video_mux *
+notifier_to_video_mux(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct video_mux, notifier);
+}
+
 static inline struct video_mux *v4l2_subdev_to_video_mux(struct v4l2_subdev *sd)
 {
 	return container_of(sd, struct video_mux, subdev);
@@ -343,6 +349,22 @@ static int video_mux_parse_endpoint(struct device *dev,
 	return fwnode_device_is_available(asd->match.fwnode) ? 0 : -ENOTCONN;
 }
 
+static int video_mux_notify_bound(struct v4l2_async_notifier *notifier,
+				  struct v4l2_subdev *sd,
+				  struct v4l2_async_subdev *asd)
+{
+	struct video_mux *vmux = notifier_to_video_mux(notifier);
+
+	return media_create_fwnode_links(&vmux->subdev.entity,
+					 dev_fwnode(vmux->subdev.dev),
+					 &sd->entity,
+					 dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations video_mux_notify_ops = {
+	.bound = video_mux_notify_bound,
+};
+
 static int video_mux_async_register(struct video_mux *vmux,
 				    unsigned int num_input_pads)
 {
@@ -357,6 +379,8 @@ static int video_mux_async_register(struct video_mux *vmux,
 
 	v4l2_async_notifier_init(&vmux->notifier);
 
+	vmux->notifier.ops = &video_mux_notify_ops;
+
 	ret = v4l2_async_register_fwnode_subdev(
 		&vmux->subdev, &vmux->notifier,
 		sizeof(struct v4l2_async_subdev),
-- 
2.17.1

[PATCH 13/22] media: imx: mipi csi-2: Create media links in bound notifier

[Steve Longerbeam]

Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the mipi csi-2 receiver sink pad.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 408ee2765e77..5b2258156781 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -91,6 +91,11 @@ static inline struct csi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
 	return container_of(sdev, struct csi2_dev, sd);
 }
 
+static inline struct csi2_dev *notifier_to_dev(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct csi2_dev, notifier);
+}
+
 /*
  * The required sequence of MIPI CSI-2 startup as specified in the i.MX6
  * reference manual is as follows:
@@ -573,6 +578,23 @@ static int csi2_parse_endpoint(struct device *dev,
 	return 0;
 }
 
+static int csi2_notify_bound(struct v4l2_async_notifier *notifier,
+			     struct v4l2_subdev *sd,
+			     struct v4l2_async_subdev *asd)
+{
+	struct csi2_dev *csi2 = notifier_to_dev(notifier);
+	struct media_pad *sink = &csi2->sd.entity.pads[CSI2_SINK_PAD];
+
+	return media_create_fwnode_pad_links(sink,
+					     dev_fwnode(csi2->dev),
+					     &sd->entity,
+					     dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations csi2_notify_ops = {
+	.bound = csi2_notify_bound,
+};
+
 static int csi2_probe(struct platform_device *pdev)
 {
 	unsigned int sink_port = 0;
@@ -646,6 +668,8 @@ static int csi2_probe(struct platform_device *pdev)
 
 	v4l2_async_notifier_init(&csi2->notifier);
 
+	csi2->notifier.ops = &csi2_notify_ops;
+
 	ret = v4l2_async_register_fwnode_subdev(
 		&csi2->sd, &csi2->notifier, sizeof(struct v4l2_async_subdev),
 		&sink_port, 1, csi2_parse_endpoint);
-- 
2.17.1

[PATCH 14/22] media: imx7-mipi-csis: Create media links in bound notifier

[Steve Longerbeam]

Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the mipi csi-2 receiver sink pad.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx7-mipi-csis.c | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index f71d9183cad2..e03b2317a1ac 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -259,6 +259,12 @@ struct csi_state {
 	bool sink_linked;
 };
 
+static inline struct csi_state *
+notifier_to_csis_state(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct csi_state, notifier);
+}
+
 struct csis_pix_format {
 	unsigned int pix_width_alignment;
 	u32 code;
@@ -863,6 +869,23 @@ static int mipi_csis_parse_endpoint(struct device *dev,
 	return 0;
 }
 
+static int mipi_csis_notify_bound(struct v4l2_async_notifier *notifier,
+				  struct v4l2_subdev *sd,
+				  struct v4l2_async_subdev *asd)
+{
+	struct csi_state *state = notifier_to_csis_state(notifier);
+	struct media_pad *sink = &state->mipi_sd.entity.pads[CSIS_PAD_SINK];
+
+	return media_create_fwnode_pad_links(sink,
+					     dev_fwnode(state->mipi_sd.dev),
+					     &sd->entity,
+					     dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations mipi_csis_notify_ops = {
+	.bound = mipi_csis_notify_bound,
+};
+
 static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
 				 struct platform_device *pdev,
 				 const struct v4l2_subdev_ops *ops)
@@ -895,6 +918,8 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
 
 	v4l2_async_notifier_init(&state->notifier);
 
+	state->notifier.ops = &mipi_csis_notify_ops;
+
 	ret = v4l2_async_register_fwnode_subdev(mipi_sd, &state->notifier,
 						sizeof(struct v4l2_async_subdev),
 						&sink_port, 1,
-- 
2.17.1

Re: [PATCH 14/22] media: imx7-mipi-csis: Create media links in bound notifier

[Rui Miguel Silva]

Hi Steve,
Just to let you know that this series no longer apply clean after
your other series:
media: imx: Fix subdev unregister/register issues

And since you will need to send a new one, some notes...

On Tue 06 Aug 2019 at 00:34, Steve Longerbeam wrote:
> Implement a notifier bound op to register media links from the remote
> sub-device's source pad(s) to the mipi csi-2 receiver sink pad.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
> index f71d9183cad2..e03b2317a1ac 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -259,6 +259,12 @@ struct csi_state {
>  	bool sink_linked;
>  };
>
> +static inline struct csi_state *
> +notifier_to_csis_state(struct v4l2_async_notifier *n)
>

instead of adding this between structs declaration can you move it
down near the similar converter:

static struct csi_state *mipi_sd_to_csis_state(struct v4l2_subdev *sdev)

and remove the inline since the compiler should do this and add
namespacing function name like the other functions, something like:

static struct csi_state * mipi_notifier_to_csis_state(struct v4l2_async_notifier *n)

Just to coherency.

Other than this, looks good to me.

Cheers,
   Rui

> +{
> +	return container_of(n, struct csi_state, notifier);
> +}
> +
>  struct csis_pix_format {
>  	unsigned int pix_width_alignment;
>  	u32 code;
> @@ -863,6 +869,23 @@ static int mipi_csis_parse_endpoint(struct device *dev,
>  	return 0;
>  }
>
> +static int mipi_csis_notify_bound(struct v4l2_async_notifier *notifier,
> +				  struct v4l2_subdev *sd,
> +				  struct v4l2_async_subdev *asd)
> +{
> +	struct csi_state *state = notifier_to_csis_state(notifier);
> +	struct media_pad *sink = &state->mipi_sd.entity.pads[CSIS_PAD_SINK];
> +
> +	return media_create_fwnode_pad_links(sink,
> +					     dev_fwnode(state->mipi_sd.dev),
> +					     &sd->entity,
> +					     dev_fwnode(sd->dev), 0);
> +}
> +
> +static const struct v4l2_async_notifier_operations mipi_csis_notify_ops = {
> +	.bound = mipi_csis_notify_bound,
> +};
> +
>  static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
>  				 struct platform_device *pdev,
>  				 const struct v4l2_subdev_ops *ops)
> @@ -895,6 +918,8 @@ static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
>
>  	v4l2_async_notifier_init(&state->notifier);
>
> +	state->notifier.ops = &mipi_csis_notify_ops;
> +
>  	ret = v4l2_async_register_fwnode_subdev(mipi_sd, &state->notifier,
>  						sizeof(struct v4l2_async_subdev),
>  						&sink_port, 1,

[PATCH 15/22] media: imx7-media-csi: Create media links in bound notifier

[Steve Longerbeam]

Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the CSI sink pad.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx7-media-csi.c | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index a1c96c52a606..f71ac485f780 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -196,6 +196,11 @@ struct imx7_csi {
 	struct completion last_eof_completion;
 };
 
+static inline struct imx7_csi *notifier_to_dev(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct imx7_csi, notifier);
+}
+
 static u32 imx7_csi_reg_read(struct imx7_csi *csi, unsigned int offset)
 {
 	return readl(csi->regbase + offset);
@@ -1173,6 +1178,23 @@ static int imx7_csi_parse_endpoint(struct device *dev,
 	return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
 }
 
+static int imx7_csi_notify_bound(struct v4l2_async_notifier *notifier,
+				 struct v4l2_subdev *sd,
+				 struct v4l2_async_subdev *asd)
+{
+	struct imx7_csi *csi = notifier_to_dev(notifier);
+	struct media_pad *sink = &csi->sd.entity.pads[IMX7_CSI_PAD_SINK];
+
+	return media_create_fwnode_pad_links(sink,
+					     dev_fwnode(csi->sd.dev),
+					     &sd->entity,
+					     dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations imx7_csi_notify_ops = {
+	.bound = imx7_csi_notify_bound,
+};
+
 static int imx7_csi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1253,6 +1275,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
 
 	v4l2_async_notifier_init(&csi->notifier);
 
+	csi->notifier.ops = &imx7_csi_notify_ops;
+
 	ret = v4l2_async_register_fwnode_subdev(&csi->sd, &csi->notifier,
 						sizeof(struct v4l2_async_subdev),
 						NULL, 0,
-- 
2.17.1

Re: [PATCH 15/22] media: imx7-media-csi: Create media links in bound notifier

[Rui Miguel Silva]

Hi Steve,
On Tue 06 Aug 2019 at 00:34, Steve Longerbeam wrote:
> Implement a notifier bound op to register media links from the remote
> sub-device's source pad(s) to the CSI sink pad.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>  drivers/staging/media/imx/imx7-media-csi.c | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index a1c96c52a606..f71ac485f780 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -196,6 +196,11 @@ struct imx7_csi {
>  	struct completion last_eof_completion;
>  };
>
> +static inline struct imx7_csi *notifier_to_dev(struct v4l2_async_notifier *n)
>

As the other one add the namespace for the function name:
imx7_csi_notifier_to_dev

other than this, looks good to me.

Cheers,
  Rui
> +{
> +	return container_of(n, struct imx7_csi, notifier);
> +}
> +
>  static u32 imx7_csi_reg_read(struct imx7_csi *csi, unsigned int offset)
>  {
>  	return readl(csi->regbase + offset);
> @@ -1173,6 +1178,23 @@ static int imx7_csi_parse_endpoint(struct device *dev,
>  	return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
>  }
>
> +static int imx7_csi_notify_bound(struct v4l2_async_notifier *notifier,
> +				 struct v4l2_subdev *sd,
> +				 struct v4l2_async_subdev *asd)
> +{
> +	struct imx7_csi *csi = notifier_to_dev(notifier);
> +	struct media_pad *sink = &csi->sd.entity.pads[IMX7_CSI_PAD_SINK];
> +
> +	return media_create_fwnode_pad_links(sink,
> +					     dev_fwnode(csi->sd.dev),
> +					     &sd->entity,
> +					     dev_fwnode(sd->dev), 0);
> +}
> +
> +static const struct v4l2_async_notifier_operations imx7_csi_notify_ops = {
> +	.bound = imx7_csi_notify_bound,
> +};
> +
>  static int imx7_csi_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1253,6 +1275,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
>
>  	v4l2_async_notifier_init(&csi->notifier);
>
> +	csi->notifier.ops = &imx7_csi_notify_ops;
> +
>  	ret = v4l2_async_register_fwnode_subdev(&csi->sd, &csi->notifier,
>  						sizeof(struct v4l2_async_subdev),
>  						NULL, 0,

Re: [PATCH 15/22] media: imx7-media-csi: Create media links in bound notifier

[Steve Longerbeam]

Hi Rui,

On 9/2/19 2:38 AM, Rui Miguel Silva wrote:
> Hi Steve,
> On Tue 06 Aug 2019 at 00:34, Steve Longerbeam wrote:
>> Implement a notifier bound op to register media links from the remote
>> sub-device's source pad(s) to the CSI sink pad.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>>   drivers/staging/media/imx/imx7-media-csi.c | 24 ++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
>> index a1c96c52a606..f71ac485f780 100644
>> --- a/drivers/staging/media/imx/imx7-media-csi.c
>> +++ b/drivers/staging/media/imx/imx7-media-csi.c
>> @@ -196,6 +196,11 @@ struct imx7_csi {
>>   	struct completion last_eof_completion;
>>   };
>>
>> +static inline struct imx7_csi *notifier_to_dev(struct v4l2_async_notifier *n)
>>
> As the other one add the namespace for the function name:
> imx7_csi_notifier_to_dev
>
> other than this, looks good to me.

Thanks for the review. I will make those changes in next rev.

Steve

>
>
>> +{
>> +	return container_of(n, struct imx7_csi, notifier);
>> +}
>> +
>>   static u32 imx7_csi_reg_read(struct imx7_csi *csi, unsigned int offset)
>>   {
>>   	return readl(csi->regbase + offset);
>> @@ -1173,6 +1178,23 @@ static int imx7_csi_parse_endpoint(struct device *dev,
>>   	return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
>>   }
>>
>> +static int imx7_csi_notify_bound(struct v4l2_async_notifier *notifier,
>> +				 struct v4l2_subdev *sd,
>> +				 struct v4l2_async_subdev *asd)
>> +{
>> +	struct imx7_csi *csi = notifier_to_dev(notifier);
>> +	struct media_pad *sink = &csi->sd.entity.pads[IMX7_CSI_PAD_SINK];
>> +
>> +	return media_create_fwnode_pad_links(sink,
>> +					     dev_fwnode(csi->sd.dev),
>> +					     &sd->entity,
>> +					     dev_fwnode(sd->dev), 0);
>> +}
>> +
>> +static const struct v4l2_async_notifier_operations imx7_csi_notify_ops = {
>> +	.bound = imx7_csi_notify_bound,
>> +};
>> +
>>   static int imx7_csi_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>> @@ -1253,6 +1275,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
>>
>>   	v4l2_async_notifier_init(&csi->notifier);
>>
>> +	csi->notifier.ops = &imx7_csi_notify_ops;
>> +
>>   	ret = v4l2_async_register_fwnode_subdev(&csi->sd, &csi->notifier,
>>   						sizeof(struct v4l2_async_subdev),
>>   						NULL, 0,

[PATCH 16/22] media: imx: csi: Implement get_fwnode_pad

[Steve Longerbeam]

The CSI does not have a 1:1 relationship between fwnode port numbers and
pad indexes. In fact the CSI fwnode device is itself a port which is the
sink, containing only a single fwnode endpoint. Implement media_entity
operation get_fwnode_pad to first verify the given endpoint is the CSI's
sink endpoint, and if so return the CSI sink pad index.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media-csi.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 367e39f5b382..e193f66fa230 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1824,9 +1824,32 @@ static void csi_unregistered(struct v4l2_subdev *sd)
 		ipu_csi_put(priv->csi);
 }
 
+/*
+ * The CSI has only one fwnode endpoint, at the sink pad. Verify the
+ * endpoint belongs to us, and return CSI_SINK_PAD.
+ */
+static int csi_get_fwnode_pad(struct media_entity *entity,
+			      struct fwnode_endpoint *endpoint)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct csi_priv *priv = v4l2_get_subdevdata(sd);
+	struct fwnode_handle *csi_port = dev_fwnode(priv->dev);
+	struct fwnode_handle *csi_ep;
+	int ret;
+
+	csi_ep = fwnode_get_next_child_node(csi_port, NULL);
+
+	ret = endpoint->local_fwnode == csi_ep ? CSI_SINK_PAD : -ENXIO;
+
+	fwnode_handle_put(csi_ep);
+
+	return ret;
+}
+
 static const struct media_entity_operations csi_entity_ops = {
 	.link_setup = csi_link_setup,
 	.link_validate = v4l2_subdev_link_validate,
+	.get_fwnode_pad = csi_get_fwnode_pad,
 };
 
 static const struct v4l2_subdev_core_ops csi_core_ops = {
-- 
2.17.1

[PATCH 17/22] media: imx: csi: Embed notifier in struct csi_priv

[Steve Longerbeam]

Embed the notifier in 'struct csi_priv', instead of dynamically allocating
it, to make it possible to retrieve csi_priv in a notifier callback op.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media-csi.c | 25 +++++++++--------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index e193f66fa230..b39d79e63ac2 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -58,6 +58,8 @@ struct csi_priv {
 	struct ipu_soc *ipu;
 	struct v4l2_subdev sd;
 	struct media_pad pad[CSI_NUM_PADS];
+	struct v4l2_async_notifier notifier;
+
 	/* the video device at IDMAC output pad */
 	struct imx_media_video_dev *vdev;
 	struct imx_media_fim *fim;
@@ -1895,31 +1897,28 @@ static int imx_csi_parse_endpoint(struct device *dev,
 
 static int imx_csi_async_register(struct csi_priv *priv)
 {
-	struct v4l2_async_notifier *notifier;
 	struct fwnode_handle *fwnode;
 	unsigned int port;
 	int ret;
 
-	notifier = kzalloc(sizeof(*notifier), GFP_KERNEL);
-	if (!notifier)
-		return -ENOMEM;
-
-	v4l2_async_notifier_init(notifier);
+	v4l2_async_notifier_init(&priv->notifier);
 
 	fwnode = dev_fwnode(priv->dev);
 
 	/* get this CSI's port id */
 	ret = fwnode_property_read_u32(fwnode, "reg", &port);
 	if (ret < 0)
-		goto out_free;
+		return ret;
 
 	ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
-		priv->dev->parent, notifier, sizeof(struct v4l2_async_subdev),
+		priv->dev->parent, &priv->notifier,
+		sizeof(struct v4l2_async_subdev),
 		port, imx_csi_parse_endpoint);
 	if (ret < 0)
 		goto out_cleanup;
 
-	ret = v4l2_async_subdev_notifier_register(&priv->sd, notifier);
+	ret = v4l2_async_subdev_notifier_register(&priv->sd,
+						  &priv->notifier);
 	if (ret < 0)
 		goto out_cleanup;
 
@@ -1927,16 +1926,12 @@ static int imx_csi_async_register(struct csi_priv *priv)
 	if (ret < 0)
 		goto out_unregister;
 
-	priv->sd.subdev_notifier = notifier;
-
 	return 0;
 
 out_unregister:
-	v4l2_async_notifier_unregister(notifier);
+	v4l2_async_notifier_unregister(&priv->notifier);
 out_cleanup:
-	v4l2_async_notifier_cleanup(notifier);
-out_free:
-	kfree(notifier);
+	v4l2_async_notifier_cleanup(&priv->notifier);
 
 	return ret;
 }
-- 
2.17.1

[PATCH 18/22] media: imx: csi: Create media links in bound notifier

[Steve Longerbeam]

Implement a notifier bound op to register media links from the remote
sub-device's source pad(s) to the CSI sink pad.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media-csi.c | 24 +++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index b39d79e63ac2..37b10cbf3c1a 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -120,6 +120,11 @@ static inline struct csi_priv *sd_to_dev(struct v4l2_subdev *sdev)
 	return container_of(sdev, struct csi_priv, sd);
 }
 
+static inline struct csi_priv *notifier_to_dev(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct csi_priv, notifier);
+}
+
 static inline bool is_parallel_bus(struct v4l2_fwnode_endpoint *ep)
 {
 	return ep->bus_type != V4L2_MBUS_CSI2_DPHY;
@@ -1895,6 +1900,23 @@ static int imx_csi_parse_endpoint(struct device *dev,
 	return fwnode_device_is_available(asd->match.fwnode) ? 0 : -ENOTCONN;
 }
 
+static int imx_csi_notify_bound(struct v4l2_async_notifier *notifier,
+				struct v4l2_subdev *sd,
+				struct v4l2_async_subdev *asd)
+{
+	struct csi_priv *priv = notifier_to_dev(notifier);
+	struct media_pad *sink = &priv->sd.entity.pads[CSI_SINK_PAD];
+
+	return media_create_fwnode_pad_links(sink,
+					     dev_fwnode(priv->dev->parent),
+					     &sd->entity,
+					     dev_fwnode(sd->dev), 0);
+}
+
+static const struct v4l2_async_notifier_operations csi_notify_ops = {
+	.bound = imx_csi_notify_bound,
+};
+
 static int imx_csi_async_register(struct csi_priv *priv)
 {
 	struct fwnode_handle *fwnode;
@@ -1903,6 +1925,8 @@ static int imx_csi_async_register(struct csi_priv *priv)
 
 	v4l2_async_notifier_init(&priv->notifier);
 
+	priv->notifier.ops = &csi_notify_ops;
+
 	fwnode = dev_fwnode(priv->dev);
 
 	/* get this CSI's port id */
-- 
2.17.1

[PATCH 19/22] media: imx: csi: Lookup endpoint fwnode with media_entity_get_fwnode_pad

[Steve Longerbeam]

Fix the 1:1 port-id:pad-index assumption for the upstream subdevice
connected to a CSI via one of the subdevice's pads, by searching the
upstream subdevice's endpoints for one that maps to the pad's index,
using media_entity_get_fwnode_pad(). This is carried out by a new
reverse mapping function imx_media_get_pad_fwnode().

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media-csi.c   | 22 ++++----------
 drivers/staging/media/imx/imx-media-utils.c | 33 +++++++++++++++++++++
 drivers/staging/media/imx/imx-media.h       |  1 +
 drivers/staging/media/imx/imx7-media-csi.c  | 25 +++++-----------
 4 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 37b10cbf3c1a..38e70082ff2d 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -164,7 +164,7 @@ static inline bool requires_passthrough(struct v4l2_fwnode_endpoint *ep,
 static int csi_get_upstream_endpoint(struct csi_priv *priv,
 				     struct v4l2_fwnode_endpoint *ep)
 {
-	struct device_node *endpoint, *port;
+	struct fwnode_handle *endpoint;
 	struct media_entity *src;
 	struct v4l2_subdev *sd;
 	struct media_pad *pad;
@@ -203,23 +203,13 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
 	if (!pad)
 		return -ENODEV;
 
-	sd = media_entity_to_v4l2_subdev(pad->entity);
+	endpoint = imx_media_get_pad_fwnode(pad);
+	if (IS_ERR(endpoint))
+		return PTR_ERR(endpoint);
 
-	/*
-	 * NOTE: this assumes an OF-graph port id is the same as a
-	 * media pad index.
-	 */
-	port = of_graph_get_port_by_id(sd->dev->of_node, pad->index);
-	if (!port)
-		return -ENODEV;
-
-	endpoint = of_get_next_child(port, NULL);
-	of_node_put(port);
-	if (!endpoint)
-		return -ENODEV;
+	v4l2_fwnode_endpoint_parse(endpoint, ep);
 
-	v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint), ep);
-	of_node_put(endpoint);
+	fwnode_handle_put(endpoint);
 
 	return 0;
 }
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 4cc6a7462ae2..2e4f27c214c4 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -908,6 +908,39 @@ imx_media_pipeline_video_device(struct media_entity *start_entity,
 }
 EXPORT_SYMBOL_GPL(imx_media_pipeline_video_device);
 
+/*
+ * Find a fwnode endpoint that maps to the given subdevice's pad.
+ * If there are multiple endpoints that map to the pad, only the
+ * first endpoint encountered is returned.
+ *
+ * On success the refcount of the returned fwnode endpoint is
+ * incremented.
+ */
+struct fwnode_handle *imx_media_get_pad_fwnode(struct media_pad *pad)
+{
+	struct fwnode_handle *endpoint;
+	struct v4l2_subdev *sd;
+
+	if (!is_media_entity_v4l2_subdev(pad->entity))
+		return ERR_PTR(-ENODEV);
+
+	sd = media_entity_to_v4l2_subdev(pad->entity);
+
+	fwnode_graph_for_each_endpoint(dev_fwnode(sd->dev), endpoint) {
+		int pad_idx = media_entity_get_fwnode_pad(&sd->entity,
+							  endpoint,
+							  pad->flags);
+		if (pad_idx < 0)
+			continue;
+
+		if (pad_idx == pad->index)
+			return endpoint;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(imx_media_get_pad_fwnode);
+
 /*
  * Turn current pipeline streaming on/off starting from entity.
  */
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 4d124a86b358..d75b3c80101a 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -195,6 +195,7 @@ imx_media_pipeline_subdev(struct media_entity *start_entity, u32 grp_id,
 struct video_device *
 imx_media_pipeline_video_device(struct media_entity *start_entity,
 				enum v4l2_buf_type buftype, bool upstream);
+struct fwnode_handle *imx_media_get_pad_fwnode(struct media_pad *pad);
 
 struct imx_media_dma_buf {
 	void          *virt;
diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index f71ac485f780..50826e637cb5 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -438,9 +438,8 @@ static int imx7_csi_get_upstream_endpoint(struct imx7_csi *csi,
 					  struct v4l2_fwnode_endpoint *ep,
 					  bool skip_mux)
 {
-	struct device_node *endpoint, *port;
+	struct fwnode_handle *endpoint;
 	struct media_entity *src;
-	struct v4l2_subdev *sd;
 	struct media_pad *pad;
 
 	if (!csi->src_sd)
@@ -462,29 +461,19 @@ static int imx7_csi_get_upstream_endpoint(struct imx7_csi *csi,
 	if (!pad)
 		return -ENODEV;
 
-	sd = media_entity_to_v4l2_subdev(pad->entity);
-
 	/* To get bus type we may need to skip video mux */
 	if (skip_mux && src->function == MEDIA_ENT_F_VID_MUX) {
-		src = &sd->entity;
+		src = pad->entity;
 		goto skip_video_mux;
 	}
 
-	/*
-	 * NOTE: this assumes an OF-graph port id is the same as a
-	 * media pad index.
-	 */
-	port = of_graph_get_port_by_id(sd->dev->of_node, pad->index);
-	if (!port)
-		return -ENODEV;
+	endpoint = imx_media_get_pad_fwnode(pad);
+	if (IS_ERR(endpoint))
+		return PTR_ERR(endpoint);
 
-	endpoint = of_get_next_child(port, NULL);
-	of_node_put(port);
-	if (!endpoint)
-		return -ENODEV;
+	v4l2_fwnode_endpoint_parse(endpoint, ep);
 
-	v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint), ep);
-	of_node_put(endpoint);
+	fwnode_handle_put(endpoint);
 
 	return 0;
 }
-- 
2.17.1

[PATCH 20/22] media: imx: Add is_ipu_internal_subdev()

[Steve Longerbeam]

Add a convenience inline function that returns true if the given
subdevice is one of the i.MX5/6 IPU-internal subdevices.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index d75b3c80101a..c42a2cafb8fb 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -282,4 +282,12 @@ void imx_media_capture_device_error(struct imx_media_video_dev *vdev);
 #define IMX_MEDIA_GRP_ID_IPU_IC_PRPENC BIT(14)
 #define IMX_MEDIA_GRP_ID_IPU_IC_PRPVF  BIT(15)
 
+static inline bool is_ipu_internal_subdev(struct v4l2_subdev *sd)
+{
+	return sd->grp_id == IMX_MEDIA_GRP_ID_IPU_VDIC ||
+		sd->grp_id == IMX_MEDIA_GRP_ID_IPU_IC_PRP ||
+		sd->grp_id == IMX_MEDIA_GRP_ID_IPU_IC_PRPENC ||
+		sd->grp_id == IMX_MEDIA_GRP_ID_IPU_IC_PRPVF;
+}
+
 #endif
-- 
2.17.1

[PATCH 21/22] media: imx: Use media_create_fwnode_links for external links

[Steve Longerbeam]

The entities external to the i.MX6 IPU and i.MX7 now create the links
to their fwnode-endpoint connected entities in their notifier bound
callbacks.

This should mean that there is no longer a need for the imx-media driver
to create these external fwnode-endpoint based links. But the v4l2-async
framework does not allow multiple subdevice notifiers to contain a
duplicate subdevice in their asd_list. Only the first subdev notifier that
discovers and adds that one subdevice to its asd_list will receive a bound
callback for it. Other subdevices that also have firmware endpoint
connections to this duplicate subdevice will not have it in their asd_list,
and thus will never receive a bound callback for it.

Until there is a solution to that problem, imx_media_create_links() is
still needed to create media links for subdevices that never received a
bound callback.

But media_create_fwnode_links() can now be used to do that work, so
imx_media_create_of_links() and imx_media_create_csi_of_links() are
removed.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 .../staging/media/imx/imx-media-dev-common.c  |  49 ++++----
 drivers/staging/media/imx/imx-media-of.c      | 113 ------------------
 drivers/staging/media/imx/imx-media.h         |   4 -
 3 files changed, 27 insertions(+), 139 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-dev-common.c b/drivers/staging/media/imx/imx-media-dev-common.c
index 66b505f7e8df..4dbe983af030 100644
--- a/drivers/staging/media/imx/imx-media-dev-common.c
+++ b/drivers/staging/media/imx/imx-media-dev-common.c
@@ -36,31 +36,36 @@ static int imx_media_subdev_bound(struct v4l2_async_notifier *notifier,
 static int imx_media_create_links(struct v4l2_async_notifier *notifier)
 {
 	struct imx_media_dev *imxmd = notifier2dev(notifier);
-	struct v4l2_subdev *sd;
+	struct fwnode_handle *sda_fwnode, *sdb_fwnode;
+	struct v4l2_subdev *sda, *sdb;
+
+	list_for_each_entry(sda, &imxmd->v4l2_dev.subdevs, list) {
+		/* links have already been created for the IPU subdevs */
+		if (is_ipu_internal_subdev(sda))
+			continue;
+
+		sda_fwnode = (sda->grp_id & IMX_MEDIA_GRP_ID_IPU_CSI) ?
+			dev_fwnode(sda->dev->parent) :
+			dev_fwnode(sda->dev);
+
+		list_for_each_entry(sdb, &imxmd->v4l2_dev.subdevs, list) {
+			if (sda == sdb)
+				continue;
+			if (is_ipu_internal_subdev(sdb))
+				continue;
+
+			sdb_fwnode = (sdb->grp_id & IMX_MEDIA_GRP_ID_IPU_CSI) ?
+				dev_fwnode(sdb->dev->parent) :
+				dev_fwnode(sdb->dev);
 
-	list_for_each_entry(sd, &imxmd->v4l2_dev.subdevs, list) {
-		switch (sd->grp_id) {
-		case IMX_MEDIA_GRP_ID_IPU_VDIC:
-		case IMX_MEDIA_GRP_ID_IPU_IC_PRP:
-		case IMX_MEDIA_GRP_ID_IPU_IC_PRPENC:
-		case IMX_MEDIA_GRP_ID_IPU_IC_PRPVF:
-			/*
-			 * links have already been created for the
-			 * sync-registered subdevs.
-			 */
-			break;
-		case IMX_MEDIA_GRP_ID_IPU_CSI0:
-		case IMX_MEDIA_GRP_ID_IPU_CSI1:
-		case IMX_MEDIA_GRP_ID_CSI:
-			imx_media_create_csi_of_links(imxmd, sd);
-			break;
-		default:
 			/*
-			 * if this subdev has fwnode links, create media
-			 * links for them.
+			 * if there are fwnode endpoint connections between
+			 * these subdevs, create media links for them.
 			 */
-			imx_media_create_of_links(imxmd, sd);
-			break;
+			__media_create_fwnode_links(&sda->entity,
+						    sda_fwnode,
+						    &sdb->entity,
+						    sdb_fwnode, 0);
 		}
 	}
 
diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
index 736c954a8ff5..82e13e972e23 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -74,116 +74,3 @@ int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(imx_media_add_of_subdevs);
-
-/*
- * Create a single media link to/from sd using a fwnode link.
- *
- * NOTE: this function assumes an OF port node is equivalent to
- * a media pad (port id equal to media pad index), and that an
- * OF endpoint node is equivalent to a media link.
- */
-static int create_of_link(struct imx_media_dev *imxmd,
-			  struct v4l2_subdev *sd,
-			  struct fwnode_link *link)
-{
-	struct v4l2_subdev *remote, *src, *sink;
-	int src_pad, sink_pad;
-
-	if (link->local.port >= sd->entity.num_pads)
-		return -EINVAL;
-
-	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) {
-		src = remote;
-		src_pad = link->remote.port;
-		sink = sd;
-		sink_pad = link->local.port;
-	} else {
-		src = sd;
-		src_pad = link->local.port;
-		sink = remote;
-		sink_pad = link->remote.port;
-	}
-
-	/* make sure link doesn't already exist before creating */
-	if (media_entity_find_link(&src->entity.pads[src_pad],
-				   &sink->entity.pads[sink_pad]))
-		return 0;
-
-	v4l2_info(sd->v4l2_dev, "%s:%d -> %s:%d\n",
-		  src->name, src_pad, sink->name, sink_pad);
-
-	return media_create_pad_link(&src->entity, src_pad,
-				     &sink->entity, sink_pad, 0);
-}
-
-/*
- * Create media links to/from sd using its device-tree endpoints.
- */
-int imx_media_create_of_links(struct imx_media_dev *imxmd,
-			      struct v4l2_subdev *sd)
-{
-	struct fwnode_handle *endpoint;
-	struct fwnode_link link;
-	int ret;
-
-	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);
-		fwnode_graph_put_link(&link);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(imx_media_create_of_links);
-
-/*
- * Create media links to the given CSI subdevice's sink pads,
- * using its device-tree endpoints.
- */
-int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
-				  struct v4l2_subdev *csi)
-{
-	struct fwnode_handle *csi_np = dev_fwnode(csi->dev);
-	struct fwnode_handle *csi_ep;
-
-	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_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 = 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_port_parent = fwnode;
-
-		ret = create_of_link(imxmd, csi, &link);
-		fwnode_handle_put(link.remote_port_parent);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(imx_media_create_csi_of_links);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index c42a2cafb8fb..7bdeecd29248 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -238,10 +238,6 @@ void imx_media_unregister_ipu_internal_subdevs(struct imx_media_dev *imxmd);
 /* imx-media-of.c */
 int imx_media_add_of_subdevs(struct imx_media_dev *dev,
 			     struct device_node *np);
-int imx_media_create_of_links(struct imx_media_dev *imxmd,
-			      struct v4l2_subdev *sd);
-int imx_media_create_csi_of_links(struct imx_media_dev *imxmd,
-				  struct v4l2_subdev *csi);
 int imx_media_of_add_csi(struct imx_media_dev *imxmd,
 			 struct device_node *csi_np);
 
-- 
2.17.1

[PATCH 22/22] media: imx: TODO: Remove issues regarding media link creation

[Steve Longerbeam]

Remove the TODO items regarding media link creation, these issues are
resolved after exporting media link creation to individual entity bound
callbacks and the use of media_create_fwnode_links(),
media_create_fwnode_pad_links(), and media_entity_get_fwnode_pad().

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/TODO | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/drivers/staging/media/imx/TODO b/drivers/staging/media/imx/TODO
index 6f29b5ca5324..a371cdedcdb0 100644
--- a/drivers/staging/media/imx/TODO
+++ b/drivers/staging/media/imx/TODO
@@ -17,35 +17,6 @@
   decided whether this feature is useful enough to make it generally
   available by exporting to v4l2-core.
 
-- After all async subdevices have been bound, v4l2_fwnode_parse_link()
-  is used to form the media links between the devices discovered in
-  the OF graph.
-
-  While this approach allows support for arbitrary OF graphs, there
-  are some assumptions for this to work:
-
-  1. If a port owned by a device in the graph has endpoint nodes, the
-     port is treated as a media pad.
-
-     This presents problems for devices that don't make this port = pad
-     assumption. Examples are SMIAPP compatible cameras which define only
-     a single output port node, but which define multiple pads owned
-     by multiple subdevices (pixel-array, binner, scaler). Or video
-     decoders (entity function MEDIA_ENT_F_ATV_DECODER), which also define
-     only a single output port node, but define multiple pads for video,
-     VBI, and audio out.
-
-     A workaround at present is to set the port reg properties to
-     correspond to the media pad index that the port represents. A
-     possible long-term solution is to implement a subdev API that
-     maps a port id to a media pad index.
-
-  2. Every endpoint of a port owned by a device in the graph is treated
-     as a media link.
-
-     Which means a port must not contain mixed-use endpoints, they
-     must all refer to media links between V4L2 subdevices.
-
 - i.MX7: all of the above, since it uses the imx media core
 
 - i.MX7: use Frame Interval Monitor
-- 
2.17.1

Re: [PATCH 00/22] media: imx: Create media links in bound notifiers

[Steve Longerbeam]

Hi Sakari, Hans, etc.

Can you take a look at this series at some point? I think there's some 
good stuff in here.

Thanks,
Steve


On 8/5/19 4:34 PM, Steve Longerbeam wrote:
> This series moves media link creation into the notifier bound callbacks
> in the subdevices required by imx.
>
> But to support the media graph on imx, a few improvements first needed
> to be made to media core.
>
> The default behavior of media_entity_get_fwnode_pad() fails when
> the entity has multiple sink and/or source pads. Wrt imx, there are
> three such entities: the imx6/7 csi-2 receivers, and the video mux.
> All of those however have a 1:1 mapping from port numbers to pad
> indeces.
>
> So instead of implementing .get_fwnode_pad op for those entities,
> or any other entities now or in the future that have multiple
> source/sink pads, it is better to modify the default behavior
> of media_entity_get_fwnode_pad() to first determine if the port
> number at the provided endpoint firmware node corresponds to a
> valid pad for the provided entity. That way the default behavior
> will work for such entities that implement 1:1 mappings, without
> requiring they implement the .get_fwnode_pad op. In other words,
> the old behavior of media_entity_get_fwnode_pad() required entities
> implement .get_fwnode_pad if they have multiple sink or source pads.
> The new behavior requires entities implement .get_fwnode_pad _only_
> if they have multiple sink or source pads, _and_ do not have 1:1
> port:pad mappings.
>
> Also, as part of the above work, it was discovered that all of the
> current callers of media_entity_get_fwnode_pad() are not using that
> function correctly. In all but one case the driver passes the firmware
> node of the subdevice itself to the function, not one of its endpoint
> nodes as the function expects. In more detail:
>
>    - Cadence MIPI-CSI2 Receiver: Calls media_entity_get_fwnode_pad()
>      in the subdev bound notifier callback, but passes sd->fwnode to
>      the function. This is usually the fwnode of the subdevice itself,
>      not one of its endpoint nodes. This only works now because there
>      are no implementations of the .get_fwnode_pad op. This will break
>      as soon as the bound subdevice implements the .get_fwnode_pad op.
>
>    - ST Micro MIPID02 CSI-2: Calls media_entity_get_fwnode_pad() in the
>      subdev bound notifier callback. Same issue as cadence driver, passes
>      sd->fwnode.
>
>    - Allwinner V3s CSI: Calls media_entity_get_fwnode_pad() in the
>      completion notifier callback. Same issue as cadence driver, passes
>      sd->fwnode.
>
>    - Renesas R-Car MIPI CSI-2 Receiver: Calls media_entity_get_fwnode_pad()
>      in the subdev bound notifier callback. In this case the driver passes
>      the async subdev descriptor match fwnode. Again for most subdevices,
>      this is the fwnode of the subdevice itself, not one of its endpoint
>      nodes. However on the Salvator-X platform, the bound subdevice happens
>      to be the ADV748x CSI-2 transmitter, which does indeed place the
>      endpoint node in the asd match fwnode. But the problem is that the
>      driver is now making assumptions about what subdevices it is binding
>      to.
>
> To resolve the above issues, this series adds a new function
> media_create_fwnode_pad_links(), which creates the media links from/to
> all pads on a remote entity, to/from a single specific local entity pad,
> using the remote's endpoint fwnodes. It's API makes it convenient to call
> from subdev bound notifier callbacks.
>
> Another function media_create_fwnode_links() is added that will create
> links from/to all pads on a remote entity, to/from all pads on a local
> entity. It's API also makes it convenient to call from subdev bound
> notifier callbacks, and can be called for entities that link to remote
> entities via multiple pads (for example the video-mux which has multiple
> sink pads that link to multiple bound subdevices).
>
> This series has been tested on i.MX6 SabreSD, SabreLite, and SabreAuto
> platforms, and the Renesas Salvator-X platform.
>
> The series needs testing on i.MX7, Cadence, ST Micro MIPID02, and
> Allwinner V3s platforms. Testing only needs to verify that the media
> graph is unchanged, no stream testing is required. If the media graph
> is broken, it means the subdevice(s) that bind to the drivers listed
> above need to implement the .get_fwnode_pad op.
>
>
> Steve Longerbeam (22):
>    media: entity: Pass entity to get_fwnode_pad operation
>    media: entity: Modify default behavior of media_entity_get_fwnode_pad
>    media: entity: Convert media_entity_get_fwnode_pad() args to const
>    media: Move v4l2_fwnode_parse_link from v4l2 to driver base
>    media: entity: Add functions to convert fwnode endpoints to media
>      links
>    media: adv748x: csi2: Implement get_fwnode_pad
>    media: rcar-csi2: Fix fwnode media link creation
>    media: cadence: csi2rx: Fix fwnode media link creation
>    media: sun6i: Fix fwnode media link creation
>    media: st-mipid02: Fix fwnode media link creation
>    media: v4l2-fwnode: Pass notifier to
>      v4l2_async_register_fwnode_subdev()
>    media: video-mux: Create media links in bound notifier
>    media: imx: mipi csi-2: Create media links in bound notifier
>    media: imx7-mipi-csis: Create media links in bound notifier
>    media: imx7-media-csi: Create media links in bound notifier
>    media: imx: csi: Implement get_fwnode_pad
>    media: imx: csi: Embed notifier in struct csi_priv
>    media: imx: csi: Create media links in bound notifier
>    media: imx: csi: Lookup endpoint fwnode with
>      media_entity_get_fwnode_pad
>    media: imx: Add is_ipu_internal_subdev()
>    media: imx: Use media_create_fwnode_links for external links
>    media: imx: TODO: Remove issues regarding media link creation
>
>   drivers/base/property.c                       |  63 ++++++
>   drivers/media/i2c/adv748x/adv748x-csi2.c      |  21 ++
>   drivers/media/i2c/st-mipid02.c                |  20 +-
>   drivers/media/mc/mc-entity.c                  | 209 +++++++++++++++++-
>   drivers/media/platform/cadence/cdns-csi2rx.c  |  27 +--
>   drivers/media/platform/rcar-vin/rcar-csi2.c   |  23 +-
>   .../platform/sunxi/sun6i-csi/sun6i_csi.c      |  36 +--
>   drivers/media/platform/video-mux.c            |  30 ++-
>   drivers/media/platform/xilinx/xilinx-vipp.c   |  69 +++---
>   drivers/media/v4l2-core/v4l2-fwnode.c         |  50 +----
>   drivers/staging/media/imx/TODO                |  29 ---
>   drivers/staging/media/imx/imx-media-csi.c     |  92 +++++---
>   .../staging/media/imx/imx-media-dev-common.c  |  49 ++--
>   drivers/staging/media/imx/imx-media-of.c      | 114 ----------
>   drivers/staging/media/imx/imx-media-utils.c   |  33 +++
>   drivers/staging/media/imx/imx-media.h         |  13 +-
>   drivers/staging/media/imx/imx6-mipi-csi2.c    |  29 ++-
>   drivers/staging/media/imx/imx7-media-csi.c    |  54 +++--
>   drivers/staging/media/imx/imx7-mipi-csis.c    |  30 ++-
>   include/linux/fwnode.h                        |  14 ++
>   include/linux/property.h                      |   5 +
>   include/media/media-entity.h                  |  99 ++++++++-
>   include/media/v4l2-fwnode.h                   |  56 +----
>   23 files changed, 721 insertions(+), 444 deletions(-)
>