[RFC 0/8] Rework V4L2 fwnode parsing; add defaults and avoid iteration

[RFC 0/8] Rework V4L2 fwnode parsing; add defaults and avoid iteration

[Sakari Ailus]

Hi folks,

This patchset reworks V4L2 fwnode endpoint parsing. It enables the use of
endpoint configuration defaults that is available sensors and other
drivers that only use a single endpoint. Well, the functionality was
available already but no driver used it likely because of two reasons:
lack of any examples in a non-trivial problem area as well as lack of a
needed functionality to obtain through non-iterative means in the fwnode
graph API. Also the fwnode framework did not provide the most convenient
APIs to perform this for drivers.

Conversion from the iterative API is done for the omap3isp and ipu3-cio2
drivers. A downside here is that this adds code: what used to be done in
the framework in a one-size-fits-all fashion is now the responsibility of
the driver. The benefits (default settings and simplicity of the
implementation from driver's point of view) are not really achievable
without some of that.

Also baked in the set is matching devices with endpoints by endpoint node
rather than the device's node. This allows finding out more information
than just the device bound (i.e. the port --- or endpoint --- through
which it was bound). Compatibility support is provided for existing
drivers by setting the fwnode to be matched based on the available
endpoints. Drivers that use the async non-notifier API and have multiple
ports will likely need special care and this is right now missing from the
set, hence this is RFC for now.

The set depends on a few other patches. They all can be found here:

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=fwnode-linear>

Sakari Ailus (8):
  v4l2-async: Use endpoint node, not device node, for fwnode match
  v4l2-async: Add v4l2_async_notifier_add_fwnode_remote_subdev
  v4l2-fwnode: Use v4l2_async_notifier_add_fwnode_remote_subdev
  omap3isp: Rework OF endpoint parsing
  v4l2-async: Safely clean up an uninitialised notifier
  ipu3-cio2: Clean up notifier's subdev list if parsing endpoints fails
  ipu3-cio2: Proceed with notifier init even if there are no subdevs
  ipu3-cio2: Parse information from firmware without using callbacks

 drivers/media/pci/intel/ipu3/ipu3-cio2.c      |  97 ++++----
 drivers/media/platform/am437x/am437x-vpfe.c   |   2 +-
 drivers/media/platform/atmel/atmel-isc.c      |   2 +-
 drivers/media/platform/atmel/atmel-isi.c      |   2 +-
 drivers/media/platform/cadence/cdns-csi2rx.c  |   2 +-
 drivers/media/platform/davinci/vpif_capture.c |  14 +-
 drivers/media/platform/exynos4-is/media-dev.c |  14 +-
 drivers/media/platform/omap3isp/isp.c         | 334 +++++++++++++++-----------
 drivers/media/platform/pxa_camera.c           |   2 +-
 drivers/media/platform/qcom/camss/camss.c     |  10 +-
 drivers/media/platform/rcar_drif.c            |   3 +-
 drivers/media/platform/renesas-ceu.c          |   2 +-
 drivers/media/platform/stm32/stm32-dcmi.c     |   2 +-
 drivers/media/platform/ti-vpe/cal.c           |   2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c   |  13 +-
 drivers/media/v4l2-core/v4l2-async.c          |  33 ++-
 drivers/media/v4l2-core/v4l2-fwnode.c         |  12 +-
 drivers/staging/media/soc_camera/soc_camera.c |  14 +-
 include/media/v4l2-async.h                    |  24 ++
 19 files changed, 354 insertions(+), 230 deletions(-)

-- 
2.11.0

[RFC 1/8] v4l2-async: Use endpoint node, not device node, for fwnode match

[Sakari Ailus]

V4L2 async framework can use both device's fwnode and endpoints's fwnode
for matching the async sub-device with the sub-device. In order to proceed
moving towards endpoint matching assign the endpoint to the async
sub-device.

As most async sub-device drivers (and the related hardware) only supports
a single endpoint, use the first endpoint found. This works for all
current drivers --- we only ever supported a single async sub-device per
device to begin with.

For async devices that have no endpoints, continue to use the fwnode
related to the device. This includes e.g. lens devices.

Depends-on: ("pxa-camera: Match with device node, not the port node")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/am437x/am437x-vpfe.c   |  2 +-
 drivers/media/platform/atmel/atmel-isc.c      |  2 +-
 drivers/media/platform/atmel/atmel-isi.c      |  2 +-
 drivers/media/platform/cadence/cdns-csi2rx.c  |  2 +-
 drivers/media/platform/davinci/vpif_capture.c | 14 +-------------
 drivers/media/platform/exynos4-is/media-dev.c | 14 ++++++++++----
 drivers/media/platform/pxa_camera.c           |  2 +-
 drivers/media/platform/qcom/camss/camss.c     | 10 +++++-----
 drivers/media/platform/rcar_drif.c            |  3 ++-
 drivers/media/platform/renesas-ceu.c          |  2 +-
 drivers/media/platform/stm32/stm32-dcmi.c     |  2 +-
 drivers/media/platform/ti-vpe/cal.c           |  2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c   | 13 ++++++++++---
 drivers/media/v4l2-core/v4l2-async.c          |  8 ++++++--
 drivers/media/v4l2-core/v4l2-fwnode.c         |  2 +-
 drivers/staging/media/soc_camera/soc_camera.c | 14 ++++++++------
 16 files changed, 51 insertions(+), 43 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index 5c17624aaade..01c483a1c717 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2495,7 +2495,7 @@ vpfe_get_pdata(struct vpfe_device *vpfe)
 		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
 			sdinfo->vpfe_param.vdpol = 1;
 
-		rem = of_graph_get_remote_port_parent(endpoint);
+		rem = of_graph_get_remote_endpoint(endpoint);
 		if (!rem) {
 			dev_err(dev, "Remote device at %pOF not found\n",
 				endpoint);
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index 50178968b8a6..d51deda7accb 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -2041,7 +2041,7 @@ static int isc_parse_dt(struct device *dev, struct isc_device *isc)
 		if (!epn)
 			return 0;
 
-		rem = of_graph_get_remote_port_parent(epn);
+		rem = of_graph_get_remote_endpoint(epn);
 		if (!rem) {
 			dev_notice(dev, "Remote device at %pOF not found\n",
 				   epn);
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index 08b8d5583080..e4e74454e016 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1110,7 +1110,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
 	if (!ep)
 		return -EINVAL;
 
-	remote = of_graph_get_remote_port_parent(ep);
+	remote = of_graph_get_remote_endpoint(ep);
 	of_node_put(ep);
 	if (!remote)
 		return -EINVAL;
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 31ace114eda1..2da34b93e8f4 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -395,7 +395,7 @@ static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
 		return -EINVAL;
 	}
 
-	csi2rx->asd.match.fwnode = fwnode_graph_get_remote_port_parent(fwh);
+	csi2rx->asd.match.fwnode = fwnode_graph_get_remote_endpoint(fwh);
 	csi2rx->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
 	of_node_put(ep);
 
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 6216b7ac6875..bb4c9cb9f2ad 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1541,7 +1541,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 
 	for (i = 0; i < VPIF_CAPTURE_NUM_CHANNELS; i++) {
 		struct v4l2_fwnode_endpoint bus_cfg = { .bus_type = 0 };
-		struct device_node *rem;
 		unsigned int flags;
 		int err;
 
@@ -1550,14 +1549,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 		if (!endpoint)
 			break;
 
-		rem = of_graph_get_remote_port_parent(endpoint);
-		if (!rem) {
-			dev_dbg(&pdev->dev, "Remote device at %pOF not found\n",
-				endpoint);
-			of_node_put(endpoint);
-			goto done;
-		}
-
 		sdinfo = &pdata->subdev_info[i];
 		chan = &pdata->chan_config[i];
 		chan->inputs = devm_kcalloc(&pdev->dev,
@@ -1565,7 +1556,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 					    sizeof(*chan->inputs),
 					    GFP_KERNEL);
 		if (!chan->inputs) {
-			of_node_put(rem);
 			of_node_put(endpoint);
 			goto err_cleanup;
 		}
@@ -1577,10 +1567,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 
 		err = v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
 						 &bus_cfg);
-		of_node_put(endpoint);
 		if (err) {
 			dev_err(&pdev->dev, "Could not parse the endpoint\n");
-			of_node_put(rem);
 			goto done;
 		}
 
@@ -1599,7 +1587,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 		sdinfo->name = rem->full_name;
 
 		pdata->asd[i] = v4l2_async_notifier_add_fwnode_subdev(
-			&vpif_obj.notifier, of_fwnode_handle(rem),
+			&vpif_obj.notifier, of_fwnode_handle(endpoint),
 			sizeof(struct v4l2_async_subdev));
 		if (IS_ERR(pdata->asd[i])) {
 			of_node_put(rem);
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 463f2d84553e..2090a627b553 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -411,7 +411,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
 
 	pd->mux_id = (endpoint.base.port - 1) & 0x1;
 
-	rem = of_graph_get_remote_port_parent(ep);
+	rem = of_graph_get_remote_endpoint(ep);
 	of_node_put(ep);
 	if (rem == NULL) {
 		v4l2_info(&fmd->v4l2_dev, "Remote device at %pOF not found\n",
@@ -1372,11 +1372,17 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
 	int i;
 
 	/* Find platform data for this sensor subdev */
-	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
-		if (fmd->sensor[i].asd.match.fwnode ==
-		    of_fwnode_handle(subdev->dev->of_node))
+	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) {
+		struct fwnode_handle *fwnode =
+			fwnode_graph_get_remote_port_parent(fmd->sensor[i].asd.
+							    match.fwnode);
+
+		if (fwnode == of_fwnode_handle(subdev->dev->of_node))
 			si = &fmd->sensor[i];
 
+		fwnode_handle_put(fwnode);
+	}
+
 	if (si == NULL)
 		return -EINVAL;
 
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index a632f06d9fff..2a1395d4f74d 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2350,7 +2350,7 @@ static int pxa_camera_pdata_from_dt(struct device *dev,
 		pcdev->platform_flags |= PXA_CAMERA_PCLK_EN;
 
 	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
-	remote = of_graph_get_remote_port_parent(np);
+	remote = of_graph_get_remote_endpoint(np);
 	if (remote)
 		asd->match.fwnode = of_fwnode_handle(remote);
 	else
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 63da18773d24..a979f210f441 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -466,7 +466,7 @@ static int camss_of_parse_ports(struct camss *camss)
 {
 	struct device *dev = camss->dev;
 	struct device_node *node = NULL;
-	struct device_node *remote = NULL;
+	struct fwnode_handle *remote = NULL;
 	int ret, num_subdevs = 0;
 
 	for_each_endpoint_of_node(dev->of_node, node) {
@@ -476,7 +476,8 @@ static int camss_of_parse_ports(struct camss *camss)
 		if (!of_device_is_available(node))
 			continue;
 
-		remote = of_graph_get_remote_port_parent(node);
+		remote = fwnode_graph_get_remote_endpoint(
+			of_fwnode_handle(node));
 		if (!remote) {
 			dev_err(dev, "Cannot get remote parent\n");
 			ret = -EINVAL;
@@ -484,11 +485,10 @@ static int camss_of_parse_ports(struct camss *camss)
 		}
 
 		asd = v4l2_async_notifier_add_fwnode_subdev(
-			&camss->notifier, of_fwnode_handle(remote),
-			sizeof(*csd));
+			&camss->notifier, remote, sizeof(*csd));
 		if (IS_ERR(asd)) {
 			ret = PTR_ERR(asd);
-			of_node_put(remote);
+			fwnode_handle_put(remote);
 			goto err_cleanup;
 		}
 
diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
index c417ff8f6fe5..39da118f882a 100644
--- a/drivers/media/platform/rcar_drif.c
+++ b/drivers/media/platform/rcar_drif.c
@@ -1222,7 +1222,8 @@ static int rcar_drif_parse_subdevs(struct rcar_drif_sdr *sdr)
 	if (!ep)
 		return 0;
 
-	fwnode = fwnode_graph_get_remote_port_parent(ep);
+	notifier->subdevs[notifier->num_subdevs] = &sdr->ep.asd;
+	fwnode = fwnode_graph_get_remote_endpoint(ep);
 	if (!fwnode) {
 		dev_warn(sdr->dev, "bad remote port parent\n");
 		fwnode_handle_put(ep);
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
index 150196f7cf96..a9553bc928fa 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -1581,7 +1581,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
 		ceu_sd = &ceudev->subdevs[i];
 		INIT_LIST_HEAD(&ceu_sd->asd.list);
 
-		remote = of_graph_get_remote_port_parent(ep);
+		remote = of_graph_get_remote_endpoint(ep);
 		ceu_sd->mbus_flags = fw_ep.bus.parallel.flags;
 		ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
 		ceu_sd->asd.match.fwnode = of_fwnode_handle(remote);
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index 5fe5b38fa901..1dd0e8be95d9 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1576,7 +1576,7 @@ static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
 	if (!ep)
 		return -EINVAL;
 
-	remote = of_graph_get_remote_port_parent(ep);
+	remote = of_graph_get_remote_endpoint(ep);
 	of_node_put(ep);
 	if (!remote)
 		return -EINVAL;
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 8d075683e448..d7995e2f4c54 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1693,7 +1693,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)
 		goto cleanup_exit;
 	}
 
-	sensor_node = of_graph_get_remote_port_parent(ep_node);
+	sensor_node = of_graph_get_remote_endpoint(ep_node);
 	if (!sensor_node) {
 		ctx_dbg(3, ctx, "can't get remote parent\n");
 		goto cleanup_exit;
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index edce0402155d..41df417153bd 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include <media/v4l2-async.h>
@@ -81,6 +82,8 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
 	dev_dbg(xdev->dev, "creating links for entity %s\n", local->name);
 
 	while (1) {
+		struct fwnode_handle *fwnode;
+
 		/* Get the next endpoint and parse its link. */
 		ep = fwnode_graph_get_next_endpoint(entity->asd.match.fwnode,
 						    ep);
@@ -116,11 +119,13 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
 			continue;
 		}
 
+		fwnode = fwnode_graph_get_port_parent(link.remote_node);
+		fwnode_handle_put(fwnode);
+
 		/* Skip DMA engines, they will be processed separately. */
-		if (link.remote_node == of_fwnode_handle(xdev->dev->of_node)) {
+		if (fwnode == dev_fwnode(xdev->dev)) {
 			dev_dbg(xdev->dev, "skipping DMA port %p:%u\n",
 				link.local_node, link.local_port);
-			v4l2_fwnode_put_link(&link);
 			continue;
 		}
 
@@ -374,9 +379,11 @@ static int xvip_graph_parse_one(struct xvip_composite_device *xdev,
 		}
 
 		fwnode_handle_put(ep);
+		fwnode = fwnode_graph_get_port_parent(remote);
+		fwnode_handle_put(fwnode);
 
 		/* Skip entities that we have already processed. */
-		if (remote == of_fwnode_handle(xdev->dev->of_node) ||
+		if (fwnode == dev_fwnode(xdev->dev) ||
 		    xvip_graph_find_entity(xdev, remote)) {
 			fwnode_handle_put(remote);
 			continue;
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 15b0c44a76e7..4cb49d5f8c03 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -670,8 +670,12 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 	 * (struct v4l2_subdev.dev), and async sub-device does not
 	 * exist independently of the device at any point of time.
 	 */
-	if (!sd->fwnode && sd->dev)
-		sd->fwnode = dev_fwnode(sd->dev);
+	if (!sd->fwnode && sd->dev) {
+		sd->fwnode = fwnode_graph_get_next_endpoint(
+			dev_fwnode(sd->dev), NULL);
+		if (!sd->fwnode)
+			sd->fwnode = dev_fwnode(sd->dev);
+	}
 
 	mutex_lock(&list_lock);
 
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index ccefa55813ad..27827842dffd 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -617,7 +617,7 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
 
 	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
 	asd->match.fwnode =
-		fwnode_graph_get_remote_port_parent(endpoint);
+		fwnode_graph_get_remote_endpoint(endpoint);
 	if (!asd->match.fwnode) {
 		dev_dbg(dev, "no remote endpoint found\n");
 		ret = -ENOTCONN;
diff --git a/drivers/staging/media/soc_camera/soc_camera.c b/drivers/staging/media/soc_camera/soc_camera.c
index 1ab86a7499b9..1b8344a2ea41 100644
--- a/drivers/staging/media/soc_camera/soc_camera.c
+++ b/drivers/staging/media/soc_camera/soc_camera.c
@@ -1518,6 +1518,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
 	struct soc_camera_async_client *sasc;
 	struct soc_of_info *info;
 	struct i2c_client *client;
+	struct device_node *np;
 	char clk_name[V4L2_CLK_NAME_SIZE];
 	int ret;
 
@@ -1552,23 +1553,23 @@ static int soc_of_bind(struct soc_camera_host *ici,
 	v4l2_async_notifier_init(&sasc->notifier);
 
 	ret = v4l2_async_notifier_add_subdev(&sasc->notifier, info->subdev);
-	if (ret) {
-		of_node_put(remote);
+	if (ret)
 		goto eaddasd;
-	}
 
 	sasc->notifier.ops = &soc_camera_async_ops;
 
 	icd->sasc = sasc;
 	icd->parent = ici->v4l2_dev.dev;
+	np = of_graph_get_port_parent(remote);
+	of_node_put(remote);
 
-	client = of_find_i2c_device_by_node(remote);
+	client = of_find_i2c_device_by_node(np);
 
 	if (client)
 		v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
 				  client->adapter->nr, client->addr);
 	else
-		v4l2_clk_name_of(clk_name, sizeof(clk_name), remote);
+		v4l2_clk_name_of(clk_name, sizeof(clk_name), np);
 
 	icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, icd);
 	if (IS_ERR(icd->clk)) {
@@ -1591,6 +1592,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
 eallocpdev:
 	devm_kfree(ici->v4l2_dev.dev, info);
 	dev_err(ici->v4l2_dev.dev, "group probe failed: %d\n", ret);
+	of_node_put(np);
 
 	return ret;
 }
@@ -1607,7 +1609,7 @@ static void scan_of_host(struct soc_camera_host *ici)
 		if (!epn)
 			break;
 
-		rem = of_graph_get_remote_port_parent(epn);
+		rem = of_graph_get_remote_endpoint(epn);
 		if (!rem) {
 			dev_notice(dev, "no remote for %pOF\n", epn);
 			continue;
-- 
2.11.0

[RFC 2/8] v4l2-async: Add v4l2_async_notifier_add_fwnode_remote_subdev

[Sakari Ailus]

v4l2_async_notifier_add_fwnode_remote_subdev is a convenience function for
parsing information on V4L2 fwnode subdevs.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-async.c | 23 +++++++++++++++++++++++
 include/media/v4l2-async.h           | 24 ++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 4cb49d5f8c03..9c1937d6ce17 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -608,6 +608,29 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
 }
 EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_subdev);
 
+int
+v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
+					     struct fwnode_handle *endpoint,
+					     struct v4l2_async_subdev *asd)
+{
+	struct fwnode_handle *remote_ep;
+	int ret;
+
+	remote_ep = fwnode_graph_get_remote_endpoint(endpoint);
+	if (!remote_ep)
+		return -ENOTCONN;
+
+	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+	asd->match.fwnode = remote_ep;
+
+	ret = v4l2_async_notifier_add_subdev(notif, asd);
+	if (ret)
+		fwnode_handle_put(remote_ep);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_remote_subdev);
+
 struct v4l2_async_subdev *
 v4l2_async_notifier_add_i2c_subdev(struct v4l2_async_notifier *notifier,
 				   int adapter_id, unsigned short address,
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 1497bda66c3b..215e73eddfc3 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -184,6 +184,30 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
 				      unsigned int asd_struct_size);
 
 /**
+ * v4l2_async_notifier_add_fwnode_remote_subdev - Allocate and add a fwnode
+ *						  remote async subdev to the
+ *						  notifier's master asd_list.
+ *
+ * @notifier: pointer to &struct v4l2_async_notifier
+ * @endpoint: local endpoint the remote sub-device to be matched
+ * @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
+ *		     the driver's async sub-device struct, i.e. both
+ *		     begin at the same memory address.
+ *
+ * Allocate a fwnode-matched asd of size asd_struct_size, and add it
+ * to the notifiers @asd_list.
+ *
+ * This is just like @v4l2_async_notifier_add_fwnode_subdev, but with the
+ * exception that the fwnode refers to a local endpoint, not the remote one.
+ */
+int
+v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
+					     struct fwnode_handle *endpoint,
+					     struct v4l2_async_subdev *asd);
+
+/**
  * v4l2_async_notifier_add_i2c_subdev - Allocate and add an i2c async
  *				subdev to the notifier's master asd_list.
  *
-- 
2.11.0

[RFC 3/8] v4l2-fwnode: Use v4l2_async_notifier_add_fwnode_remote_subdev

[Sakari Ailus]

Shorten v4l2_async_notifier_fwnode_parse_endpoint by using
v4l2_async_notifier_add_fwnode_remote_subdev.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 27827842dffd..74af7065996a 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -615,15 +615,6 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
 	if (!asd)
 		return -ENOMEM;
 
-	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
-	asd->match.fwnode =
-		fwnode_graph_get_remote_endpoint(endpoint);
-	if (!asd->match.fwnode) {
-		dev_dbg(dev, "no remote endpoint found\n");
-		ret = -ENOTCONN;
-		goto out_err;
-	}
-
 	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &vep);
 	if (ret) {
 		dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
@@ -643,7 +634,8 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
 	if (ret < 0)
 		goto out_err;
 
-	ret = v4l2_async_notifier_add_subdev(notifier, asd);
+	ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, endpoint,
+							   asd);
 	if (ret < 0) {
 		/* not an error if asd already exists */
 		if (ret == -EEXIST)
-- 
2.11.0

[RFC 4/8] omap3isp: Rework OF endpoint parsing

[Sakari Ailus]

Rework OF endpoint parsing for the omap3isp driver. This does add some
lines of code. The benefits are still clear:

- the great complication related to callbacks in endpoint parsing is gone;
  instead endpoints are obtained port by port and

- endpoints may now have a default bus configuration which was not
  possible while using callbacks. This driver does not benefit from that
  feature, but as the omap3isp is one of the exemplary drivers, this works
  as an example for driver developers.

Depends-on: ("device property: Add fwnode_graph_get_endpoint_by_id")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/omap3isp/isp.c | 334 ++++++++++++++++++++--------------
 1 file changed, 200 insertions(+), 134 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index bd57174d81a7..ac23948f6ade 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2015,136 +2015,6 @@ enum isp_of_phy {
 	ISP_OF_PHY_CSIPHY2,
 };
 
-static int isp_fwnode_parse(struct device *dev,
-			    struct v4l2_fwnode_endpoint *vep,
-			    struct v4l2_async_subdev *asd)
-{
-	struct isp_async_subdev *isd =
-		container_of(asd, struct isp_async_subdev, asd);
-	struct isp_bus_cfg *buscfg = &isd->bus;
-	bool csi1 = false;
-	unsigned int i;
-
-	dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
-		to_of_node(vep->base.local_fwnode), vep->base.port);
-
-	switch (vep->base.port) {
-	case ISP_OF_PHY_PARALLEL:
-		buscfg->interface = ISP_INTERFACE_PARALLEL;
-		buscfg->bus.parallel.data_lane_shift =
-			vep->bus.parallel.data_shift;
-		buscfg->bus.parallel.clk_pol =
-			!!(vep->bus.parallel.flags
-			   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
-		buscfg->bus.parallel.hs_pol =
-			!!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
-		buscfg->bus.parallel.vs_pol =
-			!!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
-		buscfg->bus.parallel.fld_pol =
-			!!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
-		buscfg->bus.parallel.data_pol =
-			!!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
-		buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
-		break;
-
-	case ISP_OF_PHY_CSIPHY1:
-	case ISP_OF_PHY_CSIPHY2:
-		switch (vep->bus_type) {
-		case V4L2_MBUS_CCP2:
-		case V4L2_MBUS_CSI1:
-			dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
-			csi1 = true;
-			break;
-		case V4L2_MBUS_CSI2_DPHY:
-			dev_dbg(dev, "CSI-2 configuration\n");
-			csi1 = false;
-			break;
-		default:
-			dev_err(dev, "unsupported bus type %u\n",
-				vep->bus_type);
-			return -EINVAL;
-		}
-
-		switch (vep->base.port) {
-		case ISP_OF_PHY_CSIPHY1:
-			if (csi1)
-				buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
-			else
-				buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
-			break;
-		case ISP_OF_PHY_CSIPHY2:
-			if (csi1)
-				buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
-			else
-				buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
-			break;
-		}
-		if (csi1) {
-			buscfg->bus.ccp2.lanecfg.clk.pos =
-				vep->bus.mipi_csi1.clock_lane;
-			buscfg->bus.ccp2.lanecfg.clk.pol =
-				vep->bus.mipi_csi1.lane_polarity[0];
-			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
-				buscfg->bus.ccp2.lanecfg.clk.pol,
-				buscfg->bus.ccp2.lanecfg.clk.pos);
-
-			buscfg->bus.ccp2.lanecfg.data[0].pos =
-				vep->bus.mipi_csi1.data_lane;
-			buscfg->bus.ccp2.lanecfg.data[0].pol =
-				vep->bus.mipi_csi1.lane_polarity[1];
-
-			dev_dbg(dev, "data lane polarity %u, pos %u\n",
-				buscfg->bus.ccp2.lanecfg.data[0].pol,
-				buscfg->bus.ccp2.lanecfg.data[0].pos);
-
-			buscfg->bus.ccp2.strobe_clk_pol =
-				vep->bus.mipi_csi1.clock_inv;
-			buscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;
-			buscfg->bus.ccp2.ccp2_mode =
-				vep->bus_type == V4L2_MBUS_CCP2;
-			buscfg->bus.ccp2.vp_clk_pol = 1;
-
-			buscfg->bus.ccp2.crc = 1;
-		} else {
-			buscfg->bus.csi2.lanecfg.clk.pos =
-				vep->bus.mipi_csi2.clock_lane;
-			buscfg->bus.csi2.lanecfg.clk.pol =
-				vep->bus.mipi_csi2.lane_polarities[0];
-			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
-				buscfg->bus.csi2.lanecfg.clk.pol,
-				buscfg->bus.csi2.lanecfg.clk.pos);
-
-			buscfg->bus.csi2.num_data_lanes =
-				vep->bus.mipi_csi2.num_data_lanes;
-
-			for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
-				buscfg->bus.csi2.lanecfg.data[i].pos =
-					vep->bus.mipi_csi2.data_lanes[i];
-				buscfg->bus.csi2.lanecfg.data[i].pol =
-					vep->bus.mipi_csi2.lane_polarities[i + 1];
-				dev_dbg(dev,
-					"data lane %u polarity %u, pos %u\n", i,
-					buscfg->bus.csi2.lanecfg.data[i].pol,
-					buscfg->bus.csi2.lanecfg.data[i].pos);
-			}
-			/*
-			 * FIXME: now we assume the CRC is always there.
-			 * Implement a way to obtain this information from the
-			 * sensor. Frame descriptors, perhaps?
-			 */
-			buscfg->bus.csi2.crc = 1;
-		}
-		break;
-
-	default:
-		dev_warn(dev, "%pOF: invalid interface %u\n",
-			 to_of_node(vep->base.local_fwnode), vep->base.port);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
 {
 	struct isp_device *isp = container_of(async, struct isp_device,
@@ -2174,6 +2044,204 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
 	return media_device_register(&isp->media_dev);
 }
 
+static void isp_parse_of_parallel_endpoint(struct device *dev,
+					   struct v4l2_fwnode_endpoint *vep,
+					   struct isp_bus_cfg *buscfg)
+{
+	buscfg->interface = ISP_INTERFACE_PARALLEL;
+	buscfg->bus.parallel.data_lane_shift = vep->bus.parallel.data_shift;
+	buscfg->bus.parallel.clk_pol =
+		!!(vep->bus.parallel.flags & V4L2_MBUS_PCLK_SAMPLE_FALLING);
+	buscfg->bus.parallel.hs_pol =
+		!!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+	buscfg->bus.parallel.vs_pol =
+		!!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+	buscfg->bus.parallel.fld_pol =
+		!!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+	buscfg->bus.parallel.data_pol =
+		!!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+	buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
+}
+
+static void isp_parse_of_csi2_endpoint(struct device *dev,
+				       struct v4l2_fwnode_endpoint *vep,
+				       struct isp_bus_cfg *buscfg)
+{
+	unsigned int i;
+
+	buscfg->bus.csi2.lanecfg.clk.pos = vep->bus.mipi_csi2.clock_lane;
+	buscfg->bus.csi2.lanecfg.clk.pol =
+		vep->bus.mipi_csi2.lane_polarities[0];
+	dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+		buscfg->bus.csi2.lanecfg.clk.pol,
+		buscfg->bus.csi2.lanecfg.clk.pos);
+
+	buscfg->bus.csi2.num_data_lanes = vep->bus.mipi_csi2.num_data_lanes;
+
+	for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
+		buscfg->bus.csi2.lanecfg.data[i].pos =
+			vep->bus.mipi_csi2.data_lanes[i];
+		buscfg->bus.csi2.lanecfg.data[i].pol =
+			vep->bus.mipi_csi2.lane_polarities[i + 1];
+		dev_dbg(dev,
+			"data lane %u polarity %u, pos %u\n", i,
+			buscfg->bus.csi2.lanecfg.data[i].pol,
+			buscfg->bus.csi2.lanecfg.data[i].pos);
+	}
+	/*
+	 * FIXME: now we assume the CRC is always there. Implement a way to
+	 * obtain this information from the sensor. Frame descriptors, perhaps?
+	 */
+	buscfg->bus.csi2.crc = 1;
+}
+
+static void isp_parse_of_csi1_endpoint(struct device *dev,
+				       struct v4l2_fwnode_endpoint *vep,
+				       struct isp_bus_cfg *buscfg)
+{
+	buscfg->bus.ccp2.lanecfg.clk.pos = vep->bus.mipi_csi1.clock_lane;
+	buscfg->bus.ccp2.lanecfg.clk.pol = vep->bus.mipi_csi1.lane_polarity[0];
+	dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+		buscfg->bus.ccp2.lanecfg.clk.pol,
+	buscfg->bus.ccp2.lanecfg.clk.pos);
+
+	buscfg->bus.ccp2.lanecfg.data[0].pos = vep->bus.mipi_csi1.data_lane;
+	buscfg->bus.ccp2.lanecfg.data[0].pol =
+		vep->bus.mipi_csi1.lane_polarity[1];
+
+	dev_dbg(dev, "data lane polarity %u, pos %u\n",
+		buscfg->bus.ccp2.lanecfg.data[0].pol,
+		buscfg->bus.ccp2.lanecfg.data[0].pos);
+
+	buscfg->bus.ccp2.strobe_clk_pol = vep->bus.mipi_csi1.clock_inv;
+	buscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;
+	buscfg->bus.ccp2.ccp2_mode = vep->bus_type == V4L2_MBUS_CCP2;
+	buscfg->bus.ccp2.vp_clk_pol = 1;
+
+	buscfg->bus.ccp2.crc = 1;
+}
+
+static int isp_alloc_isd(struct isp_async_subdev **isd,
+			 struct isp_bus_cfg **buscfg)
+{
+	struct isp_async_subdev *__isd;
+
+	__isd = kzalloc(sizeof(*isd), GFP_KERNEL);
+	if (!__isd)
+		return -ENOMEM;
+
+	*isd = __isd;
+	*buscfg = &__isd->bus;
+
+	return 0;
+}
+
+static struct {
+	u32 phy;
+	u32 csi2_if;
+	u32 csi1_if;
+} isp_bus_interfaces[2] = {
+	{ ISP_OF_PHY_CSIPHY1,
+	  ISP_INTERFACE_CSI2C_PHY1, ISP_INTERFACE_CCP2B_PHY1 },
+	{ ISP_OF_PHY_CSIPHY2,
+	  ISP_INTERFACE_CSI2A_PHY2, ISP_INTERFACE_CCP2B_PHY2 },
+};
+
+static int isp_parse_of_endpoints(struct isp_device *isp)
+{
+	struct fwnode_handle *ep;
+	struct isp_async_subdev *isd;
+	struct isp_bus_cfg *buscfg;
+	unsigned int i;
+
+	ep = fwnode_graph_get_endpoint_by_id(
+		dev_fwnode(isp->dev), ISP_OF_PHY_PARALLEL, 0,
+		FWNODE_GRAPH_ENDPOINT_NEXT);
+
+	if (ep) {
+		struct v4l2_fwnode_endpoint vep = {
+			.bus_type = V4L2_MBUS_PARALLEL
+		};
+		int ret;
+
+		dev_dbg(isp->dev, "parsing parallel interface\n");
+
+		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+		if (!ret)
+			ret = isp_alloc_isd(&isd, &buscfg);
+
+		if (!ret) {
+			isp_parse_of_parallel_endpoint(isp->dev, &vep, buscfg);
+			ret = v4l2_async_notifier_add_fwnode_remote_subdev(
+				&isp->notifier, ep, &isd->asd);
+		}
+
+		if (ret) {
+			kfree(isd);
+			fwnode_handle_put(ep);
+		}
+	}
+
+	for (i = 0; i < ARRAY_SIZE(isp_bus_interfaces); i++) {
+		ep = fwnode_graph_get_endpoint_by_id(
+			dev_fwnode(isp->dev), isp_bus_interfaces[i].phy, 0,
+			FWNODE_GRAPH_ENDPOINT_NEXT);
+
+		if (ep) {
+			struct v4l2_fwnode_endpoint vep = {
+				.bus_type = V4L2_MBUS_CSI2_DPHY
+			};
+			int ret;
+
+			dev_dbg(isp->dev,
+				"parsing serial interface %u, node %pOF\n", i,
+				to_of_node(ep));
+
+			ret = isp_alloc_isd(&isd, &buscfg);
+			if (ret)
+				return ret;
+
+			ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+			if (!ret) {
+				buscfg->interface =
+					isp_bus_interfaces[i].csi2_if;
+				isp_parse_of_csi2_endpoint(isp->dev, &vep,
+							   buscfg);
+			} else if (ret == -ENXIO) {
+				vep = (struct v4l2_fwnode_endpoint)
+					{ .bus_type = V4L2_MBUS_CSI1 };
+				ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+
+				if (ret == -ENXIO) {
+					vep = (struct v4l2_fwnode_endpoint)
+						{ .bus_type = V4L2_MBUS_CCP2 };
+					ret = v4l2_fwnode_endpoint_parse(ep,
+									 &vep);
+				}
+				if (!ret) {
+					buscfg->interface =
+						isp_bus_interfaces[i].csi1_if;
+					isp_parse_of_csi1_endpoint(isp->dev,
+								   &vep,
+								   buscfg);
+				}
+			}
+
+			if (!ret)
+				ret = v4l2_async_notifier_add_fwnode_remote_subdev(
+					&isp->notifier, ep, &isd->asd);
+
+			if (ret) {
+				kfree(isd);
+				fwnode_handle_put(ep);
+				return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
 static const struct v4l2_async_notifier_operations isp_subdev_notifier_ops = {
 	.complete = isp_subdev_notifier_complete,
 };
@@ -2222,14 +2290,12 @@ static int isp_probe(struct platform_device *pdev)
 	mutex_init(&isp->isp_mutex);
 	spin_lock_init(&isp->stat_lock);
 	v4l2_async_notifier_init(&isp->notifier);
+	isp->dev = &pdev->dev;
 
-	ret = v4l2_async_notifier_parse_fwnode_endpoints(
-		&pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),
-		isp_fwnode_parse);
+	ret = isp_parse_of_endpoints(isp);
 	if (ret < 0)
 		goto error;
 
-	isp->dev = &pdev->dev;
 	isp->ref_count = 0;
 
 	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
-- 
2.11.0

[RFC 5/8] v4l2-async: Safely clean up an uninitialised notifier

[Sakari Ailus]

Make the V4L2 async framework a bit more robust by allowing to clean up an
uninitialised notifier. Otherwise the result would be a (close to) NULL
pointer dereference.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-async.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 9c1937d6ce17..b1a37a8be22a 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -537,7 +537,7 @@ static void __v4l2_async_notifier_cleanup(struct v4l2_async_notifier *notifier)
 {
 	struct v4l2_async_subdev *asd, *tmp;
 
-	if (!notifier)
+	if (!notifier || !notifier->asd_list.next)
 		return;
 
 	list_for_each_entry_safe(asd, tmp, &notifier->asd_list, asd_list) {
-- 
2.11.0

[RFC 6/8] ipu3-cio2: Clean up notifier's subdev list if parsing endpoints fails

[Sakari Ailus]

The notifier must be cleaned up whenever parsing endpoints fails. Do that
to avoid a memory leak in that case.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 617fb2e944dc..5972c215148a 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1504,7 +1504,7 @@ static int cio2_notifier_init(struct cio2_device *cio2)
 		sizeof(struct sensor_async_subdev),
 		cio2_fwnode_parse);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	if (list_empty(&cio2->notifier.asd_list))
 		return -ENODEV;	/* no endpoint */
@@ -1514,9 +1514,13 @@ static int cio2_notifier_init(struct cio2_device *cio2)
 	if (ret) {
 		dev_err(&cio2->pci_dev->dev,
 			"failed to register async notifier : %d\n", ret);
-		v4l2_async_notifier_cleanup(&cio2->notifier);
+		goto out;
 	}
 
+out:
+	if (ret)
+		v4l2_async_notifier_cleanup(&cio2->notifier);
+
 	return ret;
 }
 
-- 
2.11.0

[RFC 7/8] ipu3-cio2: Proceed with notifier init even if there are no subdevs

[Sakari Ailus]

The notifier may be registered even if there are no subdevs. Do that to
simplify the code.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 5972c215148a..5c48e19f458d 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1506,9 +1506,10 @@ static int cio2_notifier_init(struct cio2_device *cio2)
 	if (ret < 0)
 		goto out;
 
-	if (list_empty(&cio2->notifier.asd_list))
-		return -ENODEV;	/* no endpoint */
-
+	/*
+	 * Proceed even without sensors connected to allow the device to
+	 * suspend.
+	 */
 	cio2->notifier.ops = &cio2_async_ops;
 	ret = v4l2_async_notifier_register(&cio2->v4l2_dev, &cio2->notifier);
 	if (ret) {
@@ -1814,8 +1815,7 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
 
 	/* Register notifier for subdevices we care */
 	r = cio2_notifier_init(cio2);
-	/* Proceed without sensors connected to allow the device to suspend. */
-	if (r && r != -ENODEV)
+	if (r)
 		goto fail_cio2_queue_exit;
 
 	r = devm_request_irq(&pci_dev->dev, pci_dev->irq, cio2_irq,
-- 
2.11.0

[RFC 8/8] ipu3-cio2: Parse information from firmware without using callbacks

[Sakari Ailus]

Instead of using the convenience function
v4l2_async_notifier_parse_fwnode_endpoints(), parse the endpoints and set
up the async sub-devices without using callbacks. While this adds a little
bit of code, it makes parsing the endpoints quite a bit more simple and
gives more control to the driver over the process. The parsing assumes
D-PHY instead of letting the V4L2 fwnode framework guess it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 93 +++++++++++++++++---------------
 1 file changed, 50 insertions(+), 43 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index 5c48e19f458d..f39ebb84b831 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -1475,36 +1475,52 @@ static const struct v4l2_async_notifier_operations cio2_async_ops = {
 	.complete = cio2_notifier_complete,
 };
 
-static int cio2_fwnode_parse(struct device *dev,
-			     struct v4l2_fwnode_endpoint *vep,
-			     struct v4l2_async_subdev *asd)
+static int cio2_parse_firmware(struct cio2_device *cio2)
 {
-	struct sensor_async_subdev *s_asd =
-			container_of(asd, struct sensor_async_subdev, asd);
+	unsigned int i;
+	int ret;
 
-	if (vep->bus_type != V4L2_MBUS_CSI2_DPHY) {
-		dev_err(dev, "Only CSI2 bus type is currently supported\n");
-		return -EINVAL;
-	}
+	for (i = 0; i < CIO2_NUM_PORTS; i++) {
+		struct v4l2_fwnode_endpoint vep = {
+			.bus_type = V4L2_MBUS_CSI2_DPHY
+		};
+		struct sensor_async_subdev *s_asd = NULL;
+		struct fwnode_handle *ep;
 
-	s_asd->csi2.port = vep->base.port;
-	s_asd->csi2.lanes = vep->bus.mipi_csi2.num_data_lanes;
+		ep = fwnode_graph_get_endpoint_by_id(
+			dev_fwnode(&cio2->pci_dev->dev), i, 0,
+			FWNODE_GRAPH_ENDPOINT_NEXT);
 
-	return 0;
-}
+		if (!ep)
+			continue;
 
-static int cio2_notifier_init(struct cio2_device *cio2)
-{
-	int ret;
+		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+		if (ret)
+			goto err_parse;
 
-	v4l2_async_notifier_init(&cio2->notifier);
+		s_asd = kzalloc(sizeof(*s_asd), GFP_KERNEL);
+		if (!s_asd) {
+			ret = -ENOMEM;
+			goto err_parse;
+		}
 
-	ret = v4l2_async_notifier_parse_fwnode_endpoints(
-		&cio2->pci_dev->dev, &cio2->notifier,
-		sizeof(struct sensor_async_subdev),
-		cio2_fwnode_parse);
-	if (ret < 0)
-		goto out;
+		s_asd->csi2.port = vep.base.port;
+		s_asd->csi2.lanes = vep.bus.mipi_csi2.num_data_lanes;
+
+		ret = v4l2_async_notifier_add_fwnode_remote_subdev(
+			&cio2->notifier, ep, &s_asd->asd);
+		if (ret)
+			goto err_parse;
+
+		fwnode_handle_put(ep);
+
+		continue;
+
+err_parse:
+		fwnode_handle_put(ep);
+		kfree(s_asd);
+		return ret;
+	}
 
 	/*
 	 * Proceed even without sensors connected to allow the device to
@@ -1512,25 +1528,13 @@ static int cio2_notifier_init(struct cio2_device *cio2)
 	 */
 	cio2->notifier.ops = &cio2_async_ops;
 	ret = v4l2_async_notifier_register(&cio2->v4l2_dev, &cio2->notifier);
-	if (ret) {
+	if (ret)
 		dev_err(&cio2->pci_dev->dev,
 			"failed to register async notifier : %d\n", ret);
-		goto out;
-	}
-
-out:
-	if (ret)
-		v4l2_async_notifier_cleanup(&cio2->notifier);
 
 	return ret;
 }
 
-static void cio2_notifier_exit(struct cio2_device *cio2)
-{
-	v4l2_async_notifier_unregister(&cio2->notifier);
-	v4l2_async_notifier_cleanup(&cio2->notifier);
-}
-
 /**************** Queue initialization ****************/
 static const struct media_entity_operations cio2_media_ops = {
 	.link_validate = v4l2_subdev_link_validate,
@@ -1813,16 +1817,18 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
 	if (r)
 		goto fail_v4l2_device_unregister;
 
+	v4l2_async_notifier_init(&cio2->notifier);
+
 	/* Register notifier for subdevices we care */
-	r = cio2_notifier_init(cio2);
+	r = cio2_parse_firmware(cio2);
 	if (r)
-		goto fail_cio2_queue_exit;
+		goto fail_clean_notifier;
 
 	r = devm_request_irq(&pci_dev->dev, pci_dev->irq, cio2_irq,
 			     IRQF_SHARED, CIO2_NAME, cio2);
 	if (r) {
 		dev_err(&pci_dev->dev, "failed to request IRQ (%d)\n", r);
-		goto fail;
+		goto fail_clean_notifier;
 	}
 
 	pm_runtime_put_noidle(&pci_dev->dev);
@@ -1830,9 +1836,9 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
 
 	return 0;
 
-fail:
-	cio2_notifier_exit(cio2);
-fail_cio2_queue_exit:
+fail_clean_notifier:
+	v4l2_async_notifier_unregister(&cio2->notifier);
+	v4l2_async_notifier_cleanup(&cio2->notifier);
 	cio2_queues_exit(cio2);
 fail_v4l2_device_unregister:
 	v4l2_device_unregister(&cio2->v4l2_dev);
@@ -1851,7 +1857,8 @@ static void cio2_pci_remove(struct pci_dev *pci_dev)
 	struct cio2_device *cio2 = pci_get_drvdata(pci_dev);
 
 	media_device_unregister(&cio2->media_dev);
-	cio2_notifier_exit(cio2);
+	v4l2_async_notifier_unregister(&cio2->notifier);
+	v4l2_async_notifier_cleanup(&cio2->notifier);
 	cio2_queues_exit(cio2);
 	cio2_fbpt_exit_dummy(cio2);
 	v4l2_device_unregister(&cio2->v4l2_dev);
-- 
2.11.0

[RFC v1.1 2/8] v4l2-async: Add v4l2_async_notifier_add_fwnode_remote_subdev

[Sakari Ailus]

v4l2_async_notifier_add_fwnode_remote_subdev is a convenience function for
parsing information on V4L2 fwnode subdevs.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since RFC v1:

- Fix documentation for v4l2_async_notifier_add_fwnode_remote_subdev.

 drivers/media/v4l2-core/v4l2-async.c | 23 +++++++++++++++++++++++
 include/media/v4l2-async.h           | 23 +++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 4cb49d5f8c03..9c1937d6ce17 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -608,6 +608,29 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
 }
 EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_subdev);
 
+int
+v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
+					     struct fwnode_handle *endpoint,
+					     struct v4l2_async_subdev *asd)
+{
+	struct fwnode_handle *remote_ep;
+	int ret;
+
+	remote_ep = fwnode_graph_get_remote_endpoint(endpoint);
+	if (!remote_ep)
+		return -ENOTCONN;
+
+	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+	asd->match.fwnode = remote_ep;
+
+	ret = v4l2_async_notifier_add_subdev(notif, asd);
+	if (ret)
+		fwnode_handle_put(remote_ep);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_remote_subdev);
+
 struct v4l2_async_subdev *
 v4l2_async_notifier_add_i2c_subdev(struct v4l2_async_notifier *notifier,
 				   int adapter_id, unsigned short address,
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 1497bda66c3b..b0c50d125ee5 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -184,6 +184,29 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
 				      unsigned int asd_struct_size);
 
 /**
+ * v4l2_async_notifier_add_fwnode_remote_subdev - Allocate and add a fwnode
+ *						  remote async subdev to the
+ *						  notifier's master asd_list.
+ *
+ * @notif: pointer to &struct v4l2_async_notifier
+ * @endpoint: local endpoint the remote sub-device to be matched
+ * @asd: Async sub-device struct allocated by the caller. The &struct
+ *	 v4l2_async_subdev shall be the first member of the driver's async
+ *	 sub-device struct, i.e. both begin at the same memory address.
+ *
+ * Gets the remote endpoint of a given local endpoint, set it up for fwnode
+ * matching and add the async sub-device to the notifier's @asd_list.
+ *
+ * This is just like @v4l2_async_notifier_add_fwnode_subdev, but with the
+ * exception that the fwnode refers to a local endpoint, not the remote one, and
+ * the function relies on the caller to allocate the async sub-device struct.
+ */
+int
+v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
+					     struct fwnode_handle *endpoint,
+					     struct v4l2_async_subdev *asd);
+
+/**
  * v4l2_async_notifier_add_i2c_subdev - Allocate and add an i2c async
  *				subdev to the notifier's master asd_list.
  *
-- 
2.11.0

[RFC v1.1 1/8] v4l2-async: Use endpoint node, not device node, for fwnode match

[Sakari Ailus]

V4L2 async framework can use both device's fwnode and endpoints's fwnode
for matching the async sub-device with the sub-device. In order to proceed
moving towards endpoint matching assign the endpoint to the async
sub-device.

As most async sub-device drivers (and the related hardware) only supports
a single endpoint, use the first endpoint found. This works for all
current drivers --- we only ever supported a single async sub-device per
device to begin with.

For async devices that have no endpoints, continue to use the fwnode
related to the device. This includes e.g. lens devices.

Depends-on: ("pxa-camera: Match with device node, not the port node")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since RFC v1:

- Fix ti-vpe driver changes.

 drivers/media/platform/am437x/am437x-vpfe.c   |  2 +-
 drivers/media/platform/atmel/atmel-isc.c      |  2 +-
 drivers/media/platform/atmel/atmel-isi.c      |  2 +-
 drivers/media/platform/cadence/cdns-csi2rx.c  |  2 +-
 drivers/media/platform/davinci/vpif_capture.c | 15 +++------------
 drivers/media/platform/exynos4-is/media-dev.c | 14 ++++++++++----
 drivers/media/platform/pxa_camera.c           |  2 +-
 drivers/media/platform/qcom/camss/camss.c     | 10 +++++-----
 drivers/media/platform/rcar_drif.c            |  3 ++-
 drivers/media/platform/renesas-ceu.c          |  2 +-
 drivers/media/platform/stm32/stm32-dcmi.c     |  2 +-
 drivers/media/platform/ti-vpe/cal.c           |  2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c   | 13 ++++++++++---
 drivers/media/v4l2-core/v4l2-async.c          |  8 ++++++--
 drivers/media/v4l2-core/v4l2-fwnode.c         |  2 +-
 drivers/staging/media/soc_camera/soc_camera.c | 14 ++++++++------
 16 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index 5c17624aaade..01c483a1c717 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2495,7 +2495,7 @@ vpfe_get_pdata(struct vpfe_device *vpfe)
 		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
 			sdinfo->vpfe_param.vdpol = 1;
 
-		rem = of_graph_get_remote_port_parent(endpoint);
+		rem = of_graph_get_remote_endpoint(endpoint);
 		if (!rem) {
 			dev_err(dev, "Remote device at %pOF not found\n",
 				endpoint);
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index 50178968b8a6..d51deda7accb 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -2041,7 +2041,7 @@ static int isc_parse_dt(struct device *dev, struct isc_device *isc)
 		if (!epn)
 			return 0;
 
-		rem = of_graph_get_remote_port_parent(epn);
+		rem = of_graph_get_remote_endpoint(epn);
 		if (!rem) {
 			dev_notice(dev, "Remote device at %pOF not found\n",
 				   epn);
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index 08b8d5583080..e4e74454e016 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1110,7 +1110,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
 	if (!ep)
 		return -EINVAL;
 
-	remote = of_graph_get_remote_port_parent(ep);
+	remote = of_graph_get_remote_endpoint(ep);
 	of_node_put(ep);
 	if (!remote)
 		return -EINVAL;
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 31ace114eda1..2da34b93e8f4 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -395,7 +395,7 @@ static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
 		return -EINVAL;
 	}
 
-	csi2rx->asd.match.fwnode = fwnode_graph_get_remote_port_parent(fwh);
+	csi2rx->asd.match.fwnode = fwnode_graph_get_remote_endpoint(fwh);
 	csi2rx->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
 	of_node_put(ep);
 
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 6216b7ac6875..d3a747f3902c 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1550,14 +1550,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 		if (!endpoint)
 			break;
 
-		rem = of_graph_get_remote_port_parent(endpoint);
-		if (!rem) {
-			dev_dbg(&pdev->dev, "Remote device at %pOF not found\n",
-				endpoint);
-			of_node_put(endpoint);
-			goto done;
-		}
-
 		sdinfo = &pdata->subdev_info[i];
 		chan = &pdata->chan_config[i];
 		chan->inputs = devm_kcalloc(&pdev->dev,
@@ -1565,7 +1557,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 					    sizeof(*chan->inputs),
 					    GFP_KERNEL);
 		if (!chan->inputs) {
-			of_node_put(rem);
 			of_node_put(endpoint);
 			goto err_cleanup;
 		}
@@ -1577,10 +1568,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 
 		err = v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
 						 &bus_cfg);
-		of_node_put(endpoint);
 		if (err) {
 			dev_err(&pdev->dev, "Could not parse the endpoint\n");
-			of_node_put(rem);
 			goto done;
 		}
 
@@ -1595,11 +1584,13 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
 			chan->vpif_if.vd_pol = 1;
 
+		rem = of_graph_get_remote_port_parent(to_of_node(endpoint)));
 		dev_dbg(&pdev->dev, "Remote device %pOF found\n", rem);
 		sdinfo->name = rem->full_name;
+		of_node_put(rem);
 
 		pdata->asd[i] = v4l2_async_notifier_add_fwnode_subdev(
-			&vpif_obj.notifier, of_fwnode_handle(rem),
+			&vpif_obj.notifier, of_fwnode_handle(endpoint),
 			sizeof(struct v4l2_async_subdev));
 		if (IS_ERR(pdata->asd[i])) {
 			of_node_put(rem);
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 463f2d84553e..2090a627b553 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -411,7 +411,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
 
 	pd->mux_id = (endpoint.base.port - 1) & 0x1;
 
-	rem = of_graph_get_remote_port_parent(ep);
+	rem = of_graph_get_remote_endpoint(ep);
 	of_node_put(ep);
 	if (rem == NULL) {
 		v4l2_info(&fmd->v4l2_dev, "Remote device at %pOF not found\n",
@@ -1372,11 +1372,17 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
 	int i;
 
 	/* Find platform data for this sensor subdev */
-	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
-		if (fmd->sensor[i].asd.match.fwnode ==
-		    of_fwnode_handle(subdev->dev->of_node))
+	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) {
+		struct fwnode_handle *fwnode =
+			fwnode_graph_get_remote_port_parent(fmd->sensor[i].asd.
+							    match.fwnode);
+
+		if (fwnode == of_fwnode_handle(subdev->dev->of_node))
 			si = &fmd->sensor[i];
 
+		fwnode_handle_put(fwnode);
+	}
+
 	if (si == NULL)
 		return -EINVAL;
 
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index a632f06d9fff..2a1395d4f74d 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2350,7 +2350,7 @@ static int pxa_camera_pdata_from_dt(struct device *dev,
 		pcdev->platform_flags |= PXA_CAMERA_PCLK_EN;
 
 	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
-	remote = of_graph_get_remote_port_parent(np);
+	remote = of_graph_get_remote_endpoint(np);
 	if (remote)
 		asd->match.fwnode = of_fwnode_handle(remote);
 	else
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index 63da18773d24..a979f210f441 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -466,7 +466,7 @@ static int camss_of_parse_ports(struct camss *camss)
 {
 	struct device *dev = camss->dev;
 	struct device_node *node = NULL;
-	struct device_node *remote = NULL;
+	struct fwnode_handle *remote = NULL;
 	int ret, num_subdevs = 0;
 
 	for_each_endpoint_of_node(dev->of_node, node) {
@@ -476,7 +476,8 @@ static int camss_of_parse_ports(struct camss *camss)
 		if (!of_device_is_available(node))
 			continue;
 
-		remote = of_graph_get_remote_port_parent(node);
+		remote = fwnode_graph_get_remote_endpoint(
+			of_fwnode_handle(node));
 		if (!remote) {
 			dev_err(dev, "Cannot get remote parent\n");
 			ret = -EINVAL;
@@ -484,11 +485,10 @@ static int camss_of_parse_ports(struct camss *camss)
 		}
 
 		asd = v4l2_async_notifier_add_fwnode_subdev(
-			&camss->notifier, of_fwnode_handle(remote),
-			sizeof(*csd));
+			&camss->notifier, remote, sizeof(*csd));
 		if (IS_ERR(asd)) {
 			ret = PTR_ERR(asd);
-			of_node_put(remote);
+			fwnode_handle_put(remote);
 			goto err_cleanup;
 		}
 
diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
index c417ff8f6fe5..39da118f882a 100644
--- a/drivers/media/platform/rcar_drif.c
+++ b/drivers/media/platform/rcar_drif.c
@@ -1222,7 +1222,8 @@ static int rcar_drif_parse_subdevs(struct rcar_drif_sdr *sdr)
 	if (!ep)
 		return 0;
 
-	fwnode = fwnode_graph_get_remote_port_parent(ep);
+	notifier->subdevs[notifier->num_subdevs] = &sdr->ep.asd;
+	fwnode = fwnode_graph_get_remote_endpoint(ep);
 	if (!fwnode) {
 		dev_warn(sdr->dev, "bad remote port parent\n");
 		fwnode_handle_put(ep);
diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
index 150196f7cf96..a9553bc928fa 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -1581,7 +1581,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
 		ceu_sd = &ceudev->subdevs[i];
 		INIT_LIST_HEAD(&ceu_sd->asd.list);
 
-		remote = of_graph_get_remote_port_parent(ep);
+		remote = of_graph_get_remote_endpoint(ep);
 		ceu_sd->mbus_flags = fw_ep.bus.parallel.flags;
 		ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
 		ceu_sd->asd.match.fwnode = of_fwnode_handle(remote);
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index 5fe5b38fa901..1dd0e8be95d9 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1576,7 +1576,7 @@ static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
 	if (!ep)
 		return -EINVAL;
 
-	remote = of_graph_get_remote_port_parent(ep);
+	remote = of_graph_get_remote_endpoint(ep);
 	of_node_put(ep);
 	if (!remote)
 		return -EINVAL;
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 8d075683e448..d7995e2f4c54 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1693,7 +1693,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)
 		goto cleanup_exit;
 	}
 
-	sensor_node = of_graph_get_remote_port_parent(ep_node);
+	sensor_node = of_graph_get_remote_endpoint(ep_node);
 	if (!sensor_node) {
 		ctx_dbg(3, ctx, "can't get remote parent\n");
 		goto cleanup_exit;
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index edce0402155d..41df417153bd 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include <media/v4l2-async.h>
@@ -81,6 +82,8 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
 	dev_dbg(xdev->dev, "creating links for entity %s\n", local->name);
 
 	while (1) {
+		struct fwnode_handle *fwnode;
+
 		/* Get the next endpoint and parse its link. */
 		ep = fwnode_graph_get_next_endpoint(entity->asd.match.fwnode,
 						    ep);
@@ -116,11 +119,13 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
 			continue;
 		}
 
+		fwnode = fwnode_graph_get_port_parent(link.remote_node);
+		fwnode_handle_put(fwnode);
+
 		/* Skip DMA engines, they will be processed separately. */
-		if (link.remote_node == of_fwnode_handle(xdev->dev->of_node)) {
+		if (fwnode == dev_fwnode(xdev->dev)) {
 			dev_dbg(xdev->dev, "skipping DMA port %p:%u\n",
 				link.local_node, link.local_port);
-			v4l2_fwnode_put_link(&link);
 			continue;
 		}
 
@@ -374,9 +379,11 @@ static int xvip_graph_parse_one(struct xvip_composite_device *xdev,
 		}
 
 		fwnode_handle_put(ep);
+		fwnode = fwnode_graph_get_port_parent(remote);
+		fwnode_handle_put(fwnode);
 
 		/* Skip entities that we have already processed. */
-		if (remote == of_fwnode_handle(xdev->dev->of_node) ||
+		if (fwnode == dev_fwnode(xdev->dev) ||
 		    xvip_graph_find_entity(xdev, remote)) {
 			fwnode_handle_put(remote);
 			continue;
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 15b0c44a76e7..4cb49d5f8c03 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -670,8 +670,12 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 	 * (struct v4l2_subdev.dev), and async sub-device does not
 	 * exist independently of the device at any point of time.
 	 */
-	if (!sd->fwnode && sd->dev)
-		sd->fwnode = dev_fwnode(sd->dev);
+	if (!sd->fwnode && sd->dev) {
+		sd->fwnode = fwnode_graph_get_next_endpoint(
+			dev_fwnode(sd->dev), NULL);
+		if (!sd->fwnode)
+			sd->fwnode = dev_fwnode(sd->dev);
+	}
 
 	mutex_lock(&list_lock);
 
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index ccefa55813ad..27827842dffd 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -617,7 +617,7 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
 
 	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
 	asd->match.fwnode =
-		fwnode_graph_get_remote_port_parent(endpoint);
+		fwnode_graph_get_remote_endpoint(endpoint);
 	if (!asd->match.fwnode) {
 		dev_dbg(dev, "no remote endpoint found\n");
 		ret = -ENOTCONN;
diff --git a/drivers/staging/media/soc_camera/soc_camera.c b/drivers/staging/media/soc_camera/soc_camera.c
index 1ab86a7499b9..1b8344a2ea41 100644
--- a/drivers/staging/media/soc_camera/soc_camera.c
+++ b/drivers/staging/media/soc_camera/soc_camera.c
@@ -1518,6 +1518,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
 	struct soc_camera_async_client *sasc;
 	struct soc_of_info *info;
 	struct i2c_client *client;
+	struct device_node *np;
 	char clk_name[V4L2_CLK_NAME_SIZE];
 	int ret;
 
@@ -1552,23 +1553,23 @@ static int soc_of_bind(struct soc_camera_host *ici,
 	v4l2_async_notifier_init(&sasc->notifier);
 
 	ret = v4l2_async_notifier_add_subdev(&sasc->notifier, info->subdev);
-	if (ret) {
-		of_node_put(remote);
+	if (ret)
 		goto eaddasd;
-	}
 
 	sasc->notifier.ops = &soc_camera_async_ops;
 
 	icd->sasc = sasc;
 	icd->parent = ici->v4l2_dev.dev;
+	np = of_graph_get_port_parent(remote);
+	of_node_put(remote);
 
-	client = of_find_i2c_device_by_node(remote);
+	client = of_find_i2c_device_by_node(np);
 
 	if (client)
 		v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
 				  client->adapter->nr, client->addr);
 	else
-		v4l2_clk_name_of(clk_name, sizeof(clk_name), remote);
+		v4l2_clk_name_of(clk_name, sizeof(clk_name), np);
 
 	icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, icd);
 	if (IS_ERR(icd->clk)) {
@@ -1591,6 +1592,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
 eallocpdev:
 	devm_kfree(ici->v4l2_dev.dev, info);
 	dev_err(ici->v4l2_dev.dev, "group probe failed: %d\n", ret);
+	of_node_put(np);
 
 	return ret;
 }
@@ -1607,7 +1609,7 @@ static void scan_of_host(struct soc_camera_host *ici)
 		if (!epn)
 			break;
 
-		rem = of_graph_get_remote_port_parent(epn);
+		rem = of_graph_get_remote_endpoint(epn);
 		if (!rem) {
 			dev_notice(dev, "no remote for %pOF\n", epn);
 			continue;
-- 
2.11.0

Re: [RFC v1.1 1/8] v4l2-async: Use endpoint node, not device node, for fwnode match

[Niklas Söderlund]

Hi Sakari,

Thanks for your work.

On 2019-03-19 15:02:00 +0200, Sakari Ailus wrote:
[snip]
> diff --git a/drivers/media/platform/rcar_drif.c 
> b/drivers/media/platform/rcar_drif.c
> index c417ff8f6fe5..39da118f882a 100644
> --- a/drivers/media/platform/rcar_drif.c
> +++ b/drivers/media/platform/rcar_drif.c
> @@ -1222,7 +1222,8 @@ static int rcar_drif_parse_subdevs(struct rcar_drif_sdr *sdr)
>  	if (!ep)
>  		return 0;
>  
> -	fwnode = fwnode_graph_get_remote_port_parent(ep);
> +	notifier->subdevs[notifier->num_subdevs] = &sdr->ep.asd;
> +	fwnode = fwnode_graph_get_remote_endpoint(ep);
>  	if (!fwnode) {
>  		dev_warn(sdr->dev, "bad remote port parent\n");
>  		fwnode_handle_put(ep);

I tried the branch you referred to in your cover letter but this driver 
fails to compile for me. Looking at struct v4l2_async_notifier it has 
neither a 'subdevs' nor 'num_subdevs' members so I might be missing a 
dependency. However looking at the change it seems different from what 
you do for other drivers, is this a typo?

-- 
Regards,
Niklas Söderlund

Re: [RFC 3/8] v4l2-fwnode: Use v4l2_async_notifier_add_fwnode_remote_subdev

[Niklas Söderlund]

Hi Sakari,

Thanks for your work.

On 2019-03-18 21:16:48 +0200, Sakari Ailus wrote:
> Shorten v4l2_async_notifier_fwnode_parse_endpoint by using
> v4l2_async_notifier_add_fwnode_remote_subdev.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 27827842dffd..74af7065996a 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -615,15 +615,6 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
>  	if (!asd)
>  		return -ENOMEM;
>  
> -	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> -	asd->match.fwnode =
> -		fwnode_graph_get_remote_endpoint(endpoint);
> -	if (!asd->match.fwnode) {
> -		dev_dbg(dev, "no remote endpoint found\n");
> -		ret = -ENOTCONN;
> -		goto out_err;
> -	}
> -

This change breaks rcar-vin I'm afraid :-(

A bit further down in v4l2_async_notifier_fwnode_parse_endpoint() we 
have this section.

    ret = parse_endpoint ? parse_endpoint(dev, &vep, asd) : 0;
    if (ret == -ENOTCONN)
        dev_err(dev, "ignoring port@%u/endpoint@%u\n", vep.base.port,
            vep.base.id);
    ...
    if (ret < 0)
        goto out_err;


As rcar-vin uses the information in asd in its parse_endpoint() callback 
and fails if it can't interact with it. So nothing is registers with the 
notifier and not much happens after that. Is the rcar-vin driver wrong 
in assuming the asd should be populated when the callback is called?

>  	ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &vep);
>  	if (ret) {
>  		dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
> @@ -643,7 +634,8 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
>  	if (ret < 0)
>  		goto out_err;
>  
> -	ret = v4l2_async_notifier_add_subdev(notifier, asd);
> +	ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, endpoint,
> +							   asd);
>  	if (ret < 0) {
>  		/* not an error if asd already exists */
>  		if (ret == -EEXIST)
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

Re: [RFC 1/8] v4l2-async: Use endpoint node, not device node, for fwnode match

[Hans Verkuil]

Some review comments:

On 3/18/19 8:16 PM, Sakari Ailus wrote:
> V4L2 async framework can use both device's fwnode and endpoints's fwnode
> for matching the async sub-device with the sub-device. In order to proceed
> moving towards endpoint matching assign the endpoint to the async
> sub-device.
> 
> As most async sub-device drivers (and the related hardware) only supports
> a single endpoint, use the first endpoint found. This works for all
> current drivers --- we only ever supported a single async sub-device per
> device to begin with.
> 
> For async devices that have no endpoints, continue to use the fwnode
> related to the device. This includes e.g. lens devices.
> 
> Depends-on: ("pxa-camera: Match with device node, not the port node")
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/platform/am437x/am437x-vpfe.c   |  2 +-
>  drivers/media/platform/atmel/atmel-isc.c      |  2 +-
>  drivers/media/platform/atmel/atmel-isi.c      |  2 +-
>  drivers/media/platform/cadence/cdns-csi2rx.c  |  2 +-
>  drivers/media/platform/davinci/vpif_capture.c | 14 +-------------
>  drivers/media/platform/exynos4-is/media-dev.c | 14 ++++++++++----
>  drivers/media/platform/pxa_camera.c           |  2 +-
>  drivers/media/platform/qcom/camss/camss.c     | 10 +++++-----
>  drivers/media/platform/rcar_drif.c            |  3 ++-
>  drivers/media/platform/renesas-ceu.c          |  2 +-
>  drivers/media/platform/stm32/stm32-dcmi.c     |  2 +-
>  drivers/media/platform/ti-vpe/cal.c           |  2 +-
>  drivers/media/platform/xilinx/xilinx-vipp.c   | 13 ++++++++++---
>  drivers/media/v4l2-core/v4l2-async.c          |  8 ++++++--
>  drivers/media/v4l2-core/v4l2-fwnode.c         |  2 +-
>  drivers/staging/media/soc_camera/soc_camera.c | 14 ++++++++------
>  16 files changed, 51 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
> index 5c17624aaade..01c483a1c717 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2495,7 +2495,7 @@ vpfe_get_pdata(struct vpfe_device *vpfe)
>  		if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
>  			sdinfo->vpfe_param.vdpol = 1;
>  
> -		rem = of_graph_get_remote_port_parent(endpoint);
> +		rem = of_graph_get_remote_endpoint(endpoint);
>  		if (!rem) {
>  			dev_err(dev, "Remote device at %pOF not found\n",
>  				endpoint);
> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
> index 50178968b8a6..d51deda7accb 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -2041,7 +2041,7 @@ static int isc_parse_dt(struct device *dev, struct isc_device *isc)
>  		if (!epn)
>  			return 0;
>  
> -		rem = of_graph_get_remote_port_parent(epn);
> +		rem = of_graph_get_remote_endpoint(epn);
>  		if (!rem) {
>  			dev_notice(dev, "Remote device at %pOF not found\n",
>  				   epn);
> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
> index 08b8d5583080..e4e74454e016 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1110,7 +1110,7 @@ static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
>  	if (!ep)
>  		return -EINVAL;
>  
> -	remote = of_graph_get_remote_port_parent(ep);
> +	remote = of_graph_get_remote_endpoint(ep);
>  	of_node_put(ep);
>  	if (!remote)
>  		return -EINVAL;
> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
> index 31ace114eda1..2da34b93e8f4 100644
> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
> @@ -395,7 +395,7 @@ static int csi2rx_parse_dt(struct csi2rx_priv *csi2rx)
>  		return -EINVAL;
>  	}
>  
> -	csi2rx->asd.match.fwnode = fwnode_graph_get_remote_port_parent(fwh);
> +	csi2rx->asd.match.fwnode = fwnode_graph_get_remote_endpoint(fwh);
>  	csi2rx->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
>  	of_node_put(ep);
>  
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 6216b7ac6875..bb4c9cb9f2ad 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1541,7 +1541,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
>  
>  	for (i = 0; i < VPIF_CAPTURE_NUM_CHANNELS; i++) {
>  		struct v4l2_fwnode_endpoint bus_cfg = { .bus_type = 0 };
> -		struct device_node *rem;
>  		unsigned int flags;
>  		int err;
>  
> @@ -1550,14 +1549,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
>  		if (!endpoint)
>  			break;
>  
> -		rem = of_graph_get_remote_port_parent(endpoint);
> -		if (!rem) {
> -			dev_dbg(&pdev->dev, "Remote device at %pOF not found\n",
> -				endpoint);
> -			of_node_put(endpoint);
> -			goto done;
> -		}
> -
>  		sdinfo = &pdata->subdev_info[i];
>  		chan = &pdata->chan_config[i];
>  		chan->inputs = devm_kcalloc(&pdev->dev,
> @@ -1565,7 +1556,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
>  					    sizeof(*chan->inputs),
>  					    GFP_KERNEL);
>  		if (!chan->inputs) {
> -			of_node_put(rem);
>  			of_node_put(endpoint);
>  			goto err_cleanup;
>  		}
> @@ -1577,10 +1567,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
>  
>  		err = v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
>  						 &bus_cfg);
> -		of_node_put(endpoint);

I don't think you can delete this line, seems to be an accidental deletion.

>  		if (err) {
>  			dev_err(&pdev->dev, "Could not parse the endpoint\n");
> -			of_node_put(rem);
>  			goto done;
>  		}
>  
> @@ -1599,7 +1587,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
>  		sdinfo->name = rem->full_name;
>  
>  		pdata->asd[i] = v4l2_async_notifier_add_fwnode_subdev(
> -			&vpif_obj.notifier, of_fwnode_handle(rem),
> +			&vpif_obj.notifier, of_fwnode_handle(endpoint),
>  			sizeof(struct v4l2_async_subdev));
>  		if (IS_ERR(pdata->asd[i])) {
>  			of_node_put(rem);
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 463f2d84553e..2090a627b553 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -411,7 +411,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
>  
>  	pd->mux_id = (endpoint.base.port - 1) & 0x1;
>  
> -	rem = of_graph_get_remote_port_parent(ep);
> +	rem = of_graph_get_remote_endpoint(ep);
>  	of_node_put(ep);
>  	if (rem == NULL) {
>  		v4l2_info(&fmd->v4l2_dev, "Remote device at %pOF not found\n",
> @@ -1372,11 +1372,17 @@ static int subdev_notifier_bound(struct v4l2_async_notifier *notifier,
>  	int i;
>  
>  	/* Find platform data for this sensor subdev */
> -	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
> -		if (fmd->sensor[i].asd.match.fwnode ==
> -		    of_fwnode_handle(subdev->dev->of_node))
> +	for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) {
> +		struct fwnode_handle *fwnode =
> +			fwnode_graph_get_remote_port_parent(fmd->sensor[i].asd.
> +							    match.fwnode);
> +
> +		if (fwnode == of_fwnode_handle(subdev->dev->of_node))
>  			si = &fmd->sensor[i];
>  
> +		fwnode_handle_put(fwnode);
> +	}
> +
>  	if (si == NULL)
>  		return -EINVAL;
>  
> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> index a632f06d9fff..2a1395d4f74d 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -2350,7 +2350,7 @@ static int pxa_camera_pdata_from_dt(struct device *dev,
>  		pcdev->platform_flags |= PXA_CAMERA_PCLK_EN;
>  
>  	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> -	remote = of_graph_get_remote_port_parent(np);
> +	remote = of_graph_get_remote_endpoint(np);
>  	if (remote)
>  		asd->match.fwnode = of_fwnode_handle(remote);
>  	else
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 63da18773d24..a979f210f441 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -466,7 +466,7 @@ static int camss_of_parse_ports(struct camss *camss)
>  {
>  	struct device *dev = camss->dev;
>  	struct device_node *node = NULL;
> -	struct device_node *remote = NULL;
> +	struct fwnode_handle *remote = NULL;
>  	int ret, num_subdevs = 0;
>  
>  	for_each_endpoint_of_node(dev->of_node, node) {
> @@ -476,7 +476,8 @@ static int camss_of_parse_ports(struct camss *camss)
>  		if (!of_device_is_available(node))
>  			continue;
>  
> -		remote = of_graph_get_remote_port_parent(node);
> +		remote = fwnode_graph_get_remote_endpoint(
> +			of_fwnode_handle(node));
>  		if (!remote) {
>  			dev_err(dev, "Cannot get remote parent\n");
>  			ret = -EINVAL;
> @@ -484,11 +485,10 @@ static int camss_of_parse_ports(struct camss *camss)
>  		}
>  
>  		asd = v4l2_async_notifier_add_fwnode_subdev(
> -			&camss->notifier, of_fwnode_handle(remote),
> -			sizeof(*csd));
> +			&camss->notifier, remote, sizeof(*csd));
>  		if (IS_ERR(asd)) {
>  			ret = PTR_ERR(asd);
> -			of_node_put(remote);
> +			fwnode_handle_put(remote);
>  			goto err_cleanup;
>  		}
>  
> diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
> index c417ff8f6fe5..39da118f882a 100644
> --- a/drivers/media/platform/rcar_drif.c
> +++ b/drivers/media/platform/rcar_drif.c
> @@ -1222,7 +1222,8 @@ static int rcar_drif_parse_subdevs(struct rcar_drif_sdr *sdr)
>  	if (!ep)
>  		return 0;
>  
> -	fwnode = fwnode_graph_get_remote_port_parent(ep);
> +	notifier->subdevs[notifier->num_subdevs] = &sdr->ep.asd;
> +	fwnode = fwnode_graph_get_remote_endpoint(ep);
>  	if (!fwnode) {
>  		dev_warn(sdr->dev, "bad remote port parent\n");
>  		fwnode_handle_put(ep);
> diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c
> index 150196f7cf96..a9553bc928fa 100644
> --- a/drivers/media/platform/renesas-ceu.c
> +++ b/drivers/media/platform/renesas-ceu.c
> @@ -1581,7 +1581,7 @@ static int ceu_parse_dt(struct ceu_device *ceudev)
>  		ceu_sd = &ceudev->subdevs[i];
>  		INIT_LIST_HEAD(&ceu_sd->asd.list);
>  
> -		remote = of_graph_get_remote_port_parent(ep);
> +		remote = of_graph_get_remote_endpoint(ep);
>  		ceu_sd->mbus_flags = fw_ep.bus.parallel.flags;
>  		ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
>  		ceu_sd->asd.match.fwnode = of_fwnode_handle(remote);
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
> index 5fe5b38fa901..1dd0e8be95d9 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -1576,7 +1576,7 @@ static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
>  	if (!ep)
>  		return -EINVAL;
>  
> -	remote = of_graph_get_remote_port_parent(ep);
> +	remote = of_graph_get_remote_endpoint(ep);
>  	of_node_put(ep);
>  	if (!remote)
>  		return -EINVAL;
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 8d075683e448..d7995e2f4c54 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -1693,7 +1693,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)
>  		goto cleanup_exit;
>  	}
>  
> -	sensor_node = of_graph_get_remote_port_parent(ep_node);
> +	sensor_node = of_graph_get_remote_endpoint(ep_node);
>  	if (!sensor_node) {
>  		ctx_dbg(3, ctx, "can't get remote parent\n");
>  		goto cleanup_exit;
> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
> index edce0402155d..41df417153bd 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -14,6 +14,7 @@
>  #include <linux/of.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/slab.h>
>  
>  #include <media/v4l2-async.h>
> @@ -81,6 +82,8 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
>  	dev_dbg(xdev->dev, "creating links for entity %s\n", local->name);
>  
>  	while (1) {
> +		struct fwnode_handle *fwnode;
> +
>  		/* Get the next endpoint and parse its link. */
>  		ep = fwnode_graph_get_next_endpoint(entity->asd.match.fwnode,
>  						    ep);
> @@ -116,11 +119,13 @@ static int xvip_graph_build_one(struct xvip_composite_device *xdev,
>  			continue;
>  		}
>  
> +		fwnode = fwnode_graph_get_port_parent(link.remote_node);
> +		fwnode_handle_put(fwnode);
> +
>  		/* Skip DMA engines, they will be processed separately. */
> -		if (link.remote_node == of_fwnode_handle(xdev->dev->of_node)) {
> +		if (fwnode == dev_fwnode(xdev->dev)) {
>  			dev_dbg(xdev->dev, "skipping DMA port %p:%u\n",
>  				link.local_node, link.local_port);
> -			v4l2_fwnode_put_link(&link);
>  			continue;
>  		}
>  
> @@ -374,9 +379,11 @@ static int xvip_graph_parse_one(struct xvip_composite_device *xdev,
>  		}
>  
>  		fwnode_handle_put(ep);
> +		fwnode = fwnode_graph_get_port_parent(remote);
> +		fwnode_handle_put(fwnode);
>  
>  		/* Skip entities that we have already processed. */
> -		if (remote == of_fwnode_handle(xdev->dev->of_node) ||
> +		if (fwnode == dev_fwnode(xdev->dev) ||
>  		    xvip_graph_find_entity(xdev, remote)) {
>  			fwnode_handle_put(remote);
>  			continue;
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 15b0c44a76e7..4cb49d5f8c03 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -670,8 +670,12 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>  	 * (struct v4l2_subdev.dev), and async sub-device does not
>  	 * exist independently of the device at any point of time.
>  	 */
> -	if (!sd->fwnode && sd->dev)
> -		sd->fwnode = dev_fwnode(sd->dev);
> +	if (!sd->fwnode && sd->dev) {
> +		sd->fwnode = fwnode_graph_get_next_endpoint(
> +			dev_fwnode(sd->dev), NULL);

Doesn't this take a reference? As opposed to the assignment below?

Shouldn't you call 'fwnode_handle_put(sd->fwnode);'?

> +		if (!sd->fwnode)
> +			sd->fwnode = dev_fwnode(sd->dev);
> +	}
>  
>  	mutex_lock(&list_lock);
>  
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index ccefa55813ad..27827842dffd 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -617,7 +617,7 @@ v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev,
>  
>  	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
>  	asd->match.fwnode =
> -		fwnode_graph_get_remote_port_parent(endpoint);
> +		fwnode_graph_get_remote_endpoint(endpoint);
>  	if (!asd->match.fwnode) {
>  		dev_dbg(dev, "no remote endpoint found\n");
>  		ret = -ENOTCONN;
> diff --git a/drivers/staging/media/soc_camera/soc_camera.c b/drivers/staging/media/soc_camera/soc_camera.c
> index 1ab86a7499b9..1b8344a2ea41 100644
> --- a/drivers/staging/media/soc_camera/soc_camera.c
> +++ b/drivers/staging/media/soc_camera/soc_camera.c
> @@ -1518,6 +1518,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
>  	struct soc_camera_async_client *sasc;
>  	struct soc_of_info *info;
>  	struct i2c_client *client;
> +	struct device_node *np;
>  	char clk_name[V4L2_CLK_NAME_SIZE];
>  	int ret;
>  
> @@ -1552,23 +1553,23 @@ static int soc_of_bind(struct soc_camera_host *ici,
>  	v4l2_async_notifier_init(&sasc->notifier);
>  
>  	ret = v4l2_async_notifier_add_subdev(&sasc->notifier, info->subdev);
> -	if (ret) {
> -		of_node_put(remote);
> +	if (ret)
>  		goto eaddasd;
> -	}
>  
>  	sasc->notifier.ops = &soc_camera_async_ops;
>  
>  	icd->sasc = sasc;
>  	icd->parent = ici->v4l2_dev.dev;
> +	np = of_graph_get_port_parent(remote);
> +	of_node_put(remote);
>  
> -	client = of_find_i2c_device_by_node(remote);
> +	client = of_find_i2c_device_by_node(np);
>  
>  	if (client)
>  		v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
>  				  client->adapter->nr, client->addr);
>  	else
> -		v4l2_clk_name_of(clk_name, sizeof(clk_name), remote);
> +		v4l2_clk_name_of(clk_name, sizeof(clk_name), np);
>  
>  	icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, icd);
>  	if (IS_ERR(icd->clk)) {
> @@ -1591,6 +1592,7 @@ static int soc_of_bind(struct soc_camera_host *ici,
>  eallocpdev:
>  	devm_kfree(ici->v4l2_dev.dev, info);
>  	dev_err(ici->v4l2_dev.dev, "group probe failed: %d\n", ret);
> +	of_node_put(np);
>  
>  	return ret;
>  }
> @@ -1607,7 +1609,7 @@ static void scan_of_host(struct soc_camera_host *ici)
>  		if (!epn)
>  			break;
>  
> -		rem = of_graph_get_remote_port_parent(epn);
> +		rem = of_graph_get_remote_endpoint(epn);
>  		if (!rem) {
>  			dev_notice(dev, "no remote for %pOF\n", epn);
>  			continue;
> 

Regards,

	Hans

Re: [RFC 2/8] v4l2-async: Add v4l2_async_notifier_add_fwnode_remote_subdev

[Hans Verkuil]

On 3/18/19 8:16 PM, Sakari Ailus wrote:
> v4l2_async_notifier_add_fwnode_remote_subdev is a convenience function for
> parsing information on V4L2 fwnode subdevs.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 23 +++++++++++++++++++++++
>  include/media/v4l2-async.h           | 24 ++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 4cb49d5f8c03..9c1937d6ce17 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -608,6 +608,29 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_subdev);
>  
> +int
> +v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
> +					     struct fwnode_handle *endpoint,
> +					     struct v4l2_async_subdev *asd)
> +{
> +	struct fwnode_handle *remote_ep;
> +	int ret;
> +
> +	remote_ep = fwnode_graph_get_remote_endpoint(endpoint);
> +	if (!remote_ep)
> +		return -ENOTCONN;
> +
> +	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> +	asd->match.fwnode = remote_ep;
> +
> +	ret = v4l2_async_notifier_add_subdev(notif, asd);
> +	if (ret)
> +		fwnode_handle_put(remote_ep);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_remote_subdev);
> +
>  struct v4l2_async_subdev *
>  v4l2_async_notifier_add_i2c_subdev(struct v4l2_async_notifier *notifier,
>  				   int adapter_id, unsigned short address,
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 1497bda66c3b..215e73eddfc3 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -184,6 +184,30 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
>  				      unsigned int asd_struct_size);
>  
>  /**
> + * v4l2_async_notifier_add_fwnode_remote_subdev - Allocate and add a fwnode
> + *						  remote async subdev to the
> + *						  notifier's master asd_list.
> + *
> + * @notifier: pointer to &struct v4l2_async_notifier
> + * @endpoint: local endpoint the remote sub-device to be matched
> + * @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
> + *		     the driver's async sub-device struct, i.e. both
> + *		     begin at the same memory address.

Huh? This argument is @asd, not a struct size. Also it's @notif, not
@notifier. It seems that this documentation isn't in sync with the actual
function.

	Hans

> + *
> + * Allocate a fwnode-matched asd of size asd_struct_size, and add it
> + * to the notifiers @asd_list.
> + *
> + * This is just like @v4l2_async_notifier_add_fwnode_subdev, but with the
> + * exception that the fwnode refers to a local endpoint, not the remote one.
> + */
> +int
> +v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
> +					     struct fwnode_handle *endpoint,
> +					     struct v4l2_async_subdev *asd);
> +
> +/**
>   * v4l2_async_notifier_add_i2c_subdev - Allocate and add an i2c async
>   *				subdev to the notifier's master asd_list.
>   *
> 

Re: [RFC 4/8] omap3isp: Rework OF endpoint parsing

[Hans Verkuil]

On 3/18/19 8:16 PM, Sakari Ailus wrote:
> Rework OF endpoint parsing for the omap3isp driver. This does add some
> lines of code. The benefits are still clear:
> 
> - the great complication related to callbacks in endpoint parsing is gone;
>   instead endpoints are obtained port by port and
> 
> - endpoints may now have a default bus configuration which was not
>   possible while using callbacks. This driver does not benefit from that
>   feature, but as the omap3isp is one of the exemplary drivers, this works
>   as an example for driver developers.
> 
> Depends-on: ("device property: Add fwnode_graph_get_endpoint_by_id")
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/platform/omap3isp/isp.c | 334 ++++++++++++++++++++--------------
>  1 file changed, 200 insertions(+), 134 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
> index bd57174d81a7..ac23948f6ade 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2015,136 +2015,6 @@ enum isp_of_phy {
>  	ISP_OF_PHY_CSIPHY2,
>  };
>  
> -static int isp_fwnode_parse(struct device *dev,
> -			    struct v4l2_fwnode_endpoint *vep,
> -			    struct v4l2_async_subdev *asd)
> -{
> -	struct isp_async_subdev *isd =
> -		container_of(asd, struct isp_async_subdev, asd);
> -	struct isp_bus_cfg *buscfg = &isd->bus;
> -	bool csi1 = false;
> -	unsigned int i;
> -
> -	dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
> -		to_of_node(vep->base.local_fwnode), vep->base.port);
> -
> -	switch (vep->base.port) {
> -	case ISP_OF_PHY_PARALLEL:
> -		buscfg->interface = ISP_INTERFACE_PARALLEL;
> -		buscfg->bus.parallel.data_lane_shift =
> -			vep->bus.parallel.data_shift;
> -		buscfg->bus.parallel.clk_pol =
> -			!!(vep->bus.parallel.flags
> -			   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
> -		buscfg->bus.parallel.hs_pol =
> -			!!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> -		buscfg->bus.parallel.vs_pol =
> -			!!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> -		buscfg->bus.parallel.fld_pol =
> -			!!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> -		buscfg->bus.parallel.data_pol =
> -			!!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> -		buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
> -		break;
> -
> -	case ISP_OF_PHY_CSIPHY1:
> -	case ISP_OF_PHY_CSIPHY2:
> -		switch (vep->bus_type) {
> -		case V4L2_MBUS_CCP2:
> -		case V4L2_MBUS_CSI1:
> -			dev_dbg(dev, "CSI-1/CCP-2 configuration\n");
> -			csi1 = true;
> -			break;
> -		case V4L2_MBUS_CSI2_DPHY:
> -			dev_dbg(dev, "CSI-2 configuration\n");
> -			csi1 = false;
> -			break;
> -		default:
> -			dev_err(dev, "unsupported bus type %u\n",
> -				vep->bus_type);
> -			return -EINVAL;
> -		}
> -
> -		switch (vep->base.port) {
> -		case ISP_OF_PHY_CSIPHY1:
> -			if (csi1)
> -				buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;
> -			else
> -				buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> -			break;
> -		case ISP_OF_PHY_CSIPHY2:
> -			if (csi1)
> -				buscfg->interface = ISP_INTERFACE_CCP2B_PHY2;
> -			else
> -				buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> -			break;
> -		}
> -		if (csi1) {
> -			buscfg->bus.ccp2.lanecfg.clk.pos =
> -				vep->bus.mipi_csi1.clock_lane;
> -			buscfg->bus.ccp2.lanecfg.clk.pol =
> -				vep->bus.mipi_csi1.lane_polarity[0];
> -			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> -				buscfg->bus.ccp2.lanecfg.clk.pol,
> -				buscfg->bus.ccp2.lanecfg.clk.pos);
> -
> -			buscfg->bus.ccp2.lanecfg.data[0].pos =
> -				vep->bus.mipi_csi1.data_lane;
> -			buscfg->bus.ccp2.lanecfg.data[0].pol =
> -				vep->bus.mipi_csi1.lane_polarity[1];
> -
> -			dev_dbg(dev, "data lane polarity %u, pos %u\n",
> -				buscfg->bus.ccp2.lanecfg.data[0].pol,
> -				buscfg->bus.ccp2.lanecfg.data[0].pos);
> -
> -			buscfg->bus.ccp2.strobe_clk_pol =
> -				vep->bus.mipi_csi1.clock_inv;
> -			buscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;
> -			buscfg->bus.ccp2.ccp2_mode =
> -				vep->bus_type == V4L2_MBUS_CCP2;
> -			buscfg->bus.ccp2.vp_clk_pol = 1;
> -
> -			buscfg->bus.ccp2.crc = 1;
> -		} else {
> -			buscfg->bus.csi2.lanecfg.clk.pos =
> -				vep->bus.mipi_csi2.clock_lane;
> -			buscfg->bus.csi2.lanecfg.clk.pol =
> -				vep->bus.mipi_csi2.lane_polarities[0];
> -			dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> -				buscfg->bus.csi2.lanecfg.clk.pol,
> -				buscfg->bus.csi2.lanecfg.clk.pos);
> -
> -			buscfg->bus.csi2.num_data_lanes =
> -				vep->bus.mipi_csi2.num_data_lanes;
> -
> -			for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
> -				buscfg->bus.csi2.lanecfg.data[i].pos =
> -					vep->bus.mipi_csi2.data_lanes[i];
> -				buscfg->bus.csi2.lanecfg.data[i].pol =
> -					vep->bus.mipi_csi2.lane_polarities[i + 1];
> -				dev_dbg(dev,
> -					"data lane %u polarity %u, pos %u\n", i,
> -					buscfg->bus.csi2.lanecfg.data[i].pol,
> -					buscfg->bus.csi2.lanecfg.data[i].pos);
> -			}
> -			/*
> -			 * FIXME: now we assume the CRC is always there.
> -			 * Implement a way to obtain this information from the
> -			 * sensor. Frame descriptors, perhaps?
> -			 */
> -			buscfg->bus.csi2.crc = 1;
> -		}
> -		break;
> -
> -	default:
> -		dev_warn(dev, "%pOF: invalid interface %u\n",
> -			 to_of_node(vep->base.local_fwnode), vep->base.port);
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
>  static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
>  {
>  	struct isp_device *isp = container_of(async, struct isp_device,
> @@ -2174,6 +2044,204 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
>  	return media_device_register(&isp->media_dev);
>  }
>  
> +static void isp_parse_of_parallel_endpoint(struct device *dev,
> +					   struct v4l2_fwnode_endpoint *vep,
> +					   struct isp_bus_cfg *buscfg)
> +{
> +	buscfg->interface = ISP_INTERFACE_PARALLEL;
> +	buscfg->bus.parallel.data_lane_shift = vep->bus.parallel.data_shift;
> +	buscfg->bus.parallel.clk_pol =
> +		!!(vep->bus.parallel.flags & V4L2_MBUS_PCLK_SAMPLE_FALLING);
> +	buscfg->bus.parallel.hs_pol =
> +		!!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> +	buscfg->bus.parallel.vs_pol =
> +		!!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> +	buscfg->bus.parallel.fld_pol =
> +		!!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> +	buscfg->bus.parallel.data_pol =
> +		!!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> +	buscfg->bus.parallel.bt656 = vep->bus_type == V4L2_MBUS_BT656;
> +}
> +
> +static void isp_parse_of_csi2_endpoint(struct device *dev,
> +				       struct v4l2_fwnode_endpoint *vep,
> +				       struct isp_bus_cfg *buscfg)
> +{
> +	unsigned int i;
> +
> +	buscfg->bus.csi2.lanecfg.clk.pos = vep->bus.mipi_csi2.clock_lane;
> +	buscfg->bus.csi2.lanecfg.clk.pol =
> +		vep->bus.mipi_csi2.lane_polarities[0];
> +	dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> +		buscfg->bus.csi2.lanecfg.clk.pol,
> +		buscfg->bus.csi2.lanecfg.clk.pos);
> +
> +	buscfg->bus.csi2.num_data_lanes = vep->bus.mipi_csi2.num_data_lanes;
> +
> +	for (i = 0; i < buscfg->bus.csi2.num_data_lanes; i++) {
> +		buscfg->bus.csi2.lanecfg.data[i].pos =
> +			vep->bus.mipi_csi2.data_lanes[i];
> +		buscfg->bus.csi2.lanecfg.data[i].pol =
> +			vep->bus.mipi_csi2.lane_polarities[i + 1];
> +		dev_dbg(dev,
> +			"data lane %u polarity %u, pos %u\n", i,
> +			buscfg->bus.csi2.lanecfg.data[i].pol,
> +			buscfg->bus.csi2.lanecfg.data[i].pos);
> +	}
> +	/*
> +	 * FIXME: now we assume the CRC is always there. Implement a way to
> +	 * obtain this information from the sensor. Frame descriptors, perhaps?
> +	 */
> +	buscfg->bus.csi2.crc = 1;
> +}
> +
> +static void isp_parse_of_csi1_endpoint(struct device *dev,
> +				       struct v4l2_fwnode_endpoint *vep,
> +				       struct isp_bus_cfg *buscfg)
> +{
> +	buscfg->bus.ccp2.lanecfg.clk.pos = vep->bus.mipi_csi1.clock_lane;
> +	buscfg->bus.ccp2.lanecfg.clk.pol = vep->bus.mipi_csi1.lane_polarity[0];
> +	dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> +		buscfg->bus.ccp2.lanecfg.clk.pol,
> +	buscfg->bus.ccp2.lanecfg.clk.pos);
> +
> +	buscfg->bus.ccp2.lanecfg.data[0].pos = vep->bus.mipi_csi1.data_lane;
> +	buscfg->bus.ccp2.lanecfg.data[0].pol =
> +		vep->bus.mipi_csi1.lane_polarity[1];
> +
> +	dev_dbg(dev, "data lane polarity %u, pos %u\n",
> +		buscfg->bus.ccp2.lanecfg.data[0].pol,
> +		buscfg->bus.ccp2.lanecfg.data[0].pos);
> +
> +	buscfg->bus.ccp2.strobe_clk_pol = vep->bus.mipi_csi1.clock_inv;
> +	buscfg->bus.ccp2.phy_layer = vep->bus.mipi_csi1.strobe;
> +	buscfg->bus.ccp2.ccp2_mode = vep->bus_type == V4L2_MBUS_CCP2;
> +	buscfg->bus.ccp2.vp_clk_pol = 1;
> +
> +	buscfg->bus.ccp2.crc = 1;
> +}
> +
> +static int isp_alloc_isd(struct isp_async_subdev **isd,
> +			 struct isp_bus_cfg **buscfg)
> +{
> +	struct isp_async_subdev *__isd;
> +
> +	__isd = kzalloc(sizeof(*isd), GFP_KERNEL);
> +	if (!__isd)
> +		return -ENOMEM;
> +
> +	*isd = __isd;
> +	*buscfg = &__isd->bus;
> +
> +	return 0;
> +}
> +
> +static struct {
> +	u32 phy;
> +	u32 csi2_if;
> +	u32 csi1_if;
> +} isp_bus_interfaces[2] = {
> +	{ ISP_OF_PHY_CSIPHY1,
> +	  ISP_INTERFACE_CSI2C_PHY1, ISP_INTERFACE_CCP2B_PHY1 },
> +	{ ISP_OF_PHY_CSIPHY2,
> +	  ISP_INTERFACE_CSI2A_PHY2, ISP_INTERFACE_CCP2B_PHY2 },
> +};
> +
> +static int isp_parse_of_endpoints(struct isp_device *isp)
> +{
> +	struct fwnode_handle *ep;
> +	struct isp_async_subdev *isd;
> +	struct isp_bus_cfg *buscfg;
> +	unsigned int i;
> +
> +	ep = fwnode_graph_get_endpoint_by_id(
> +		dev_fwnode(isp->dev), ISP_OF_PHY_PARALLEL, 0,
> +		FWNODE_GRAPH_ENDPOINT_NEXT);
> +
> +	if (ep) {
> +		struct v4l2_fwnode_endpoint vep = {
> +			.bus_type = V4L2_MBUS_PARALLEL
> +		};
> +		int ret;
> +
> +		dev_dbg(isp->dev, "parsing parallel interface\n");
> +
> +		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> +		if (!ret)
> +			ret = isp_alloc_isd(&isd, &buscfg);
> +
> +		if (!ret) {
> +			isp_parse_of_parallel_endpoint(isp->dev, &vep, buscfg);
> +			ret = v4l2_async_notifier_add_fwnode_remote_subdev(
> +				&isp->notifier, ep, &isd->asd);
> +		}
> +
> +		if (ret) {
> +			kfree(isd);
> +			fwnode_handle_put(ep);
> +		}

If ret == 0, then who calls 'fwnode_handle_put(ep);'?

Am I missing something?

Regards,

	Hans

> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(isp_bus_interfaces); i++) {
> +		ep = fwnode_graph_get_endpoint_by_id(
> +			dev_fwnode(isp->dev), isp_bus_interfaces[i].phy, 0,
> +			FWNODE_GRAPH_ENDPOINT_NEXT);
> +
> +		if (ep) {
> +			struct v4l2_fwnode_endpoint vep = {
> +				.bus_type = V4L2_MBUS_CSI2_DPHY
> +			};
> +			int ret;
> +
> +			dev_dbg(isp->dev,
> +				"parsing serial interface %u, node %pOF\n", i,
> +				to_of_node(ep));
> +
> +			ret = isp_alloc_isd(&isd, &buscfg);
> +			if (ret)
> +				return ret;
> +
> +			ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> +			if (!ret) {
> +				buscfg->interface =
> +					isp_bus_interfaces[i].csi2_if;
> +				isp_parse_of_csi2_endpoint(isp->dev, &vep,
> +							   buscfg);
> +			} else if (ret == -ENXIO) {
> +				vep = (struct v4l2_fwnode_endpoint)
> +					{ .bus_type = V4L2_MBUS_CSI1 };
> +				ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> +
> +				if (ret == -ENXIO) {
> +					vep = (struct v4l2_fwnode_endpoint)
> +						{ .bus_type = V4L2_MBUS_CCP2 };
> +					ret = v4l2_fwnode_endpoint_parse(ep,
> +									 &vep);
> +				}
> +				if (!ret) {
> +					buscfg->interface =
> +						isp_bus_interfaces[i].csi1_if;
> +					isp_parse_of_csi1_endpoint(isp->dev,
> +								   &vep,
> +								   buscfg);
> +				}
> +			}
> +
> +			if (!ret)
> +				ret = v4l2_async_notifier_add_fwnode_remote_subdev(
> +					&isp->notifier, ep, &isd->asd);
> +
> +			if (ret) {
> +				kfree(isd);
> +				fwnode_handle_put(ep);
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct v4l2_async_notifier_operations isp_subdev_notifier_ops = {
>  	.complete = isp_subdev_notifier_complete,
>  };
> @@ -2222,14 +2290,12 @@ static int isp_probe(struct platform_device *pdev)
>  	mutex_init(&isp->isp_mutex);
>  	spin_lock_init(&isp->stat_lock);
>  	v4l2_async_notifier_init(&isp->notifier);
> +	isp->dev = &pdev->dev;
>  
> -	ret = v4l2_async_notifier_parse_fwnode_endpoints(
> -		&pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),
> -		isp_fwnode_parse);
> +	ret = isp_parse_of_endpoints(isp);
>  	if (ret < 0)
>  		goto error;
>  
> -	isp->dev = &pdev->dev;
>  	isp->ref_count = 0;
>  
>  	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
> 

Re: [RFC 1/8] v4l2-async: Use endpoint node, not device node, for fwnode match

[Sakari Ailus]

Hi Hans,

Thank you for the review.

On Thu, Apr 04, 2019 at 03:37:44PM +0200, Hans Verkuil wrote:
...
> > diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> > index 6216b7ac6875..bb4c9cb9f2ad 100644
> > --- a/drivers/media/platform/davinci/vpif_capture.c
> > +++ b/drivers/media/platform/davinci/vpif_capture.c
> > @@ -1541,7 +1541,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
> >  
> >  	for (i = 0; i < VPIF_CAPTURE_NUM_CHANNELS; i++) {
> >  		struct v4l2_fwnode_endpoint bus_cfg = { .bus_type = 0 };
> > -		struct device_node *rem;
> >  		unsigned int flags;
> >  		int err;
> >  
> > @@ -1550,14 +1549,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
> >  		if (!endpoint)
> >  			break;
> >  
> > -		rem = of_graph_get_remote_port_parent(endpoint);
> > -		if (!rem) {
> > -			dev_dbg(&pdev->dev, "Remote device at %pOF not found\n",
> > -				endpoint);
> > -			of_node_put(endpoint);
> > -			goto done;
> > -		}
> > -
> >  		sdinfo = &pdata->subdev_info[i];
> >  		chan = &pdata->chan_config[i];
> >  		chan->inputs = devm_kcalloc(&pdev->dev,
> > @@ -1565,7 +1556,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
> >  					    sizeof(*chan->inputs),
> >  					    GFP_KERNEL);
> >  		if (!chan->inputs) {
> > -			of_node_put(rem);
> >  			of_node_put(endpoint);
> >  			goto err_cleanup;
> >  		}
> > @@ -1577,10 +1567,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
> >  
> >  		err = v4l2_fwnode_endpoint_parse(of_fwnode_handle(endpoint),
> >  						 &bus_cfg);
> > -		of_node_put(endpoint);
> 
> I don't think you can delete this line, seems to be an accidental deletion.

Right. of_graph_get_next_endpoint() will put the endpoint node on the next
iteration, so if the loop is executed again, the endpoint node musn't be
put here. I think I'll properly address this in a separate patch (before
this one).

> 
> >  		if (err) {
> >  			dev_err(&pdev->dev, "Could not parse the endpoint\n");
> > -			of_node_put(rem);
> >  			goto done;
> >  		}
> >  
> > @@ -1599,7 +1587,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
> >  		sdinfo->name = rem->full_name;
> >  
> >  		pdata->asd[i] = v4l2_async_notifier_add_fwnode_subdev(
> > -			&vpif_obj.notifier, of_fwnode_handle(rem),
> > +			&vpif_obj.notifier, of_fwnode_handle(endpoint),
> >  			sizeof(struct v4l2_async_subdev));
> >  		if (IS_ERR(pdata->asd[i])) {
> >  			of_node_put(rem);

...

> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 15b0c44a76e7..4cb49d5f8c03 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -670,8 +670,12 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> >  	 * (struct v4l2_subdev.dev), and async sub-device does not
> >  	 * exist independently of the device at any point of time.
> >  	 */
> > -	if (!sd->fwnode && sd->dev)
> > -		sd->fwnode = dev_fwnode(sd->dev);
> > +	if (!sd->fwnode && sd->dev) {
> > +		sd->fwnode = fwnode_graph_get_next_endpoint(
> > +			dev_fwnode(sd->dev), NULL);
> 
> Doesn't this take a reference? As opposed to the assignment below?
> 
> Shouldn't you call 'fwnode_handle_put(sd->fwnode);'?

Yes. I'll fix this for the next version.

> 
> > +		if (!sd->fwnode)
> > +			sd->fwnode = dev_fwnode(sd->dev);
> > +	}
> >  
> >  	mutex_lock(&list_lock);
> >  

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RFC 2/8] v4l2-async: Add v4l2_async_notifier_add_fwnode_remote_subdev

[Sakari Ailus]

On Thu, Apr 04, 2019 at 03:39:41PM +0200, Hans Verkuil wrote:
> On 3/18/19 8:16 PM, Sakari Ailus wrote:
> > v4l2_async_notifier_add_fwnode_remote_subdev is a convenience function for
> > parsing information on V4L2 fwnode subdevs.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 23 +++++++++++++++++++++++
> >  include/media/v4l2-async.h           | 24 ++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 4cb49d5f8c03..9c1937d6ce17 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -608,6 +608,29 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_subdev);
> >  
> > +int
> > +v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
> > +					     struct fwnode_handle *endpoint,
> > +					     struct v4l2_async_subdev *asd)
> > +{
> > +	struct fwnode_handle *remote_ep;
> > +	int ret;
> > +
> > +	remote_ep = fwnode_graph_get_remote_endpoint(endpoint);
> > +	if (!remote_ep)
> > +		return -ENOTCONN;
> > +
> > +	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
> > +	asd->match.fwnode = remote_ep;
> > +
> > +	ret = v4l2_async_notifier_add_subdev(notif, asd);
> > +	if (ret)
> > +		fwnode_handle_put(remote_ep);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_fwnode_remote_subdev);
> > +
> >  struct v4l2_async_subdev *
> >  v4l2_async_notifier_add_i2c_subdev(struct v4l2_async_notifier *notifier,
> >  				   int adapter_id, unsigned short address,
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index 1497bda66c3b..215e73eddfc3 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -184,6 +184,30 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
> >  				      unsigned int asd_struct_size);
> >  
> >  /**
> > + * v4l2_async_notifier_add_fwnode_remote_subdev - Allocate and add a fwnode
> > + *						  remote async subdev to the
> > + *						  notifier's master asd_list.
> > + *
> > + * @notifier: pointer to &struct v4l2_async_notifier
> > + * @endpoint: local endpoint the remote sub-device to be matched
> > + * @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
> > + *		     the driver's async sub-device struct, i.e. both
> > + *		     begin at the same memory address.
> 
> Huh? This argument is @asd, not a struct size. Also it's @notif, not
> @notifier. It seems that this documentation isn't in sync with the actual
> function.

Yes, I'll address this in the next version.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RFC 4/8] omap3isp: Rework OF endpoint parsing

[Sakari Ailus]

Hi Hans,

Thank you for reviewing this.

On Thu, Apr 04, 2019 at 03:43:51PM +0200, Hans Verkuil wrote:
> On 3/18/19 8:16 PM, Sakari Ailus wrote:
...
> > +static int isp_parse_of_endpoints(struct isp_device *isp)
> > +{
> > +	struct fwnode_handle *ep;
> > +	struct isp_async_subdev *isd;
> > +	struct isp_bus_cfg *buscfg;
> > +	unsigned int i;
> > +
> > +	ep = fwnode_graph_get_endpoint_by_id(
> > +		dev_fwnode(isp->dev), ISP_OF_PHY_PARALLEL, 0,
> > +		FWNODE_GRAPH_ENDPOINT_NEXT);
> > +
> > +	if (ep) {
> > +		struct v4l2_fwnode_endpoint vep = {
> > +			.bus_type = V4L2_MBUS_PARALLEL
> > +		};
> > +		int ret;
> > +
> > +		dev_dbg(isp->dev, "parsing parallel interface\n");
> > +
> > +		ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> > +		if (!ret)
> > +			ret = isp_alloc_isd(&isd, &buscfg);
> > +
> > +		if (!ret) {
> > +			isp_parse_of_parallel_endpoint(isp->dev, &vep, buscfg);
> > +			ret = v4l2_async_notifier_add_fwnode_remote_subdev(
> > +				&isp->notifier, ep, &isd->asd);
> > +		}
> > +
> > +		if (ret) {
> > +			kfree(isd);
> > +			fwnode_handle_put(ep);
> > +		}
> 
> If ret == 0, then who calls 'fwnode_handle_put(ep);'?
> 
> Am I missing something?

Good question. The reference is released when the notifier is eventually
cleaned up. That's not very intuitive. I'll see if I could address that in
the next version.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com