Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: linux-media@vger.kernel.org
Cc: hverkuil@xs4all.nl, laurent.pinchart@ideasonboard.com,
	jacopo@jmondi.org, niklas.soderlund@ragnatech.se
Subject: [PATCH v2 3/9] v4l2-async: Get fwnode reference when putting it to the notifier's list
Date: Thu,  6 Jun 2019 16:02:19 +0300
Message-ID: <20190606130225.10751-4-sakari.ailus@linux.intel.com> (raw)
In-Reply-To: <20190606130225.10751-1-sakari.ailus@linux.intel.com>

The v4l2_async_notifier_add_fwnode_subdev() did not take a reference of
the added fwnode, relying on the caller to handle that instead, in essence
putting the fwnode to be added if there was an error.

As the reference is eventually released during the notifier cleanup, this
is not intuitive nor logical. Improve this by always getting a reference
when the function succeeds, and the caller releasing the reference when it
does not *itself* need it anymore.

Luckily, perhaps, there were just a handful of callers using the function.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/platform/am437x/am437x-vpfe.c   |  5 ++---
 drivers/media/platform/davinci/vpif_capture.c | 16 ++++++++--------
 drivers/media/platform/qcom/camss/camss.c     |  2 +-
 drivers/media/platform/xilinx/xilinx-vipp.c   |  2 +-
 drivers/media/v4l2-core/v4l2-async.c          |  3 ++-
 drivers/media/v4l2-core/v4l2-fwnode.c         | 23 ++++++-----------------
 include/media/v4l2-async.h                    |  5 +++--
 7 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index db263c0ce48e..ccdbd9227955 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2505,10 +2505,9 @@ vpfe_get_pdata(struct vpfe_device *vpfe)
 		pdata->asd[i] = v4l2_async_notifier_add_fwnode_subdev(
 			&vpfe->notifier, of_fwnode_handle(rem),
 			sizeof(struct v4l2_async_subdev));
-		if (IS_ERR(pdata->asd[i])) {
-			of_node_put(rem);
+		of_node_put(rem);
+		if (IS_ERR(pdata->asd[i]))
 			goto cleanup;
-		}
 	}
 
 	of_node_put(endpoint);
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 8fdea45ae090..2a0c3f3fb443 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1512,6 +1512,7 @@ static struct vpif_capture_config *
 vpif_capture_get_pdata(struct platform_device *pdev)
 {
 	struct device_node *endpoint = NULL;
+	struct device_node *rem = NULL, *rem_ep = NULL;
 	struct vpif_capture_config *pdata;
 	struct vpif_subdev_info *sdinfo;
 	struct vpif_capture_chan_config *chan;
@@ -1542,7 +1543,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, *rem_ep;
 		unsigned int flags;
 		int err;
 
@@ -1565,7 +1565,6 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 			of_node_put(rem_ep);
 			goto done;
 		}
-		of_node_put(rem_ep);
 
 		sdinfo = &pdata->subdev_info[i];
 		chan = &pdata->chan_config[i];
@@ -1573,10 +1572,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 					    VPIF_CAPTURE_NUM_CHANNELS,
 					    sizeof(*chan->inputs),
 					    GFP_KERNEL);
-		if (!chan->inputs) {
-			of_node_put(rem);
+		if (!chan->inputs)
 			goto err_cleanup;
-		}
 
 		chan->input_count++;
 		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
@@ -1588,6 +1585,7 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 		if (err) {
 			dev_err(&pdev->dev, "Could not parse the endpoint\n");
 			of_node_put(rem);
+			of_node_put(rem_ep);
 			goto done;
 		}
 
@@ -1608,11 +1606,11 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 		pdata->asd[i] = v4l2_async_notifier_add_fwnode_subdev(
 			&vpif_obj.notifier, of_fwnode_handle(rem_ep),
 			sizeof(struct v4l2_async_subdev));
-		if (IS_ERR(pdata->asd[i])) {
-			of_node_put(rem);
+		if (IS_ERR(pdata->asd[i]))
 			goto err_cleanup;
-		}
 
+		of_node_put(rem);
+		of_node_put(rem_ep);
 	}
 
 done:
@@ -1624,6 +1622,8 @@ vpif_capture_get_pdata(struct platform_device *pdev)
 	return pdata;
 
 err_cleanup:
+	of_node_put(rem);
+	of_node_put(rem_ep);
 	of_node_put(endpoint);
 	v4l2_async_notifier_cleanup(&vpif_obj.notifier);
 
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
index a979f210f441..4ab4a47d34f3 100644
--- a/drivers/media/platform/qcom/camss/camss.c
+++ b/drivers/media/platform/qcom/camss/camss.c
@@ -486,9 +486,9 @@ static int camss_of_parse_ports(struct camss *camss)
 
 		asd = v4l2_async_notifier_add_fwnode_subdev(
 			&camss->notifier, remote, sizeof(*csd));
+		fwnode_handle_put(remote);
 		if (IS_ERR(asd)) {
 			ret = PTR_ERR(asd);
-			fwnode_handle_put(remote);
 			goto err_cleanup;
 		}
 
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c b/drivers/media/platform/xilinx/xilinx-vipp.c
index 41df417153bd..d788f4870a23 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -392,9 +392,9 @@ static int xvip_graph_parse_one(struct xvip_composite_device *xdev,
 		asd = v4l2_async_notifier_add_fwnode_subdev(
 			&xdev->notifier, remote,
 			sizeof(struct xvip_graph_entity));
+		fwnode_handle_put(remote);
 		if (IS_ERR(asd)) {
 			ret = PTR_ERR(asd);
-			fwnode_handle_put(remote);
 			goto err_notifier_cleanup;
 		}
 	}
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 304969ff3191..dc4e83b4f6ba 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -596,10 +596,11 @@ v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
 		return ERR_PTR(-ENOMEM);
 
 	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
-	asd->match.fwnode = fwnode;
+	asd->match.fwnode = fwnode_handle_get(fwnode);
 
 	ret = v4l2_async_notifier_add_subdev(notifier, asd);
 	if (ret) {
+		fwnode_handle_put(fwnode);
 		kfree(asd);
 		return ERR_PTR(ret);
 	}
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 810b6584b522..b48725824580 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -780,23 +780,17 @@ static int v4l2_fwnode_reference_parse(struct device *dev,
 		asd = v4l2_async_notifier_add_fwnode_subdev(notifier,
 							    args.fwnode,
 							    sizeof(*asd));
+		fwnode_handle_put(args.fwnode);
 		if (IS_ERR(asd)) {
-			ret = PTR_ERR(asd);
 			/* not an error if asd already exists */
-			if (ret == -EEXIST) {
-				fwnode_handle_put(args.fwnode);
+			if (PTR_ERR(asd) == -EEXIST)
 				continue;
-			}
 
-			goto error;
+			return PTR_ERR(asd);
 		}
 	}
 
 	return 0;
-
-error:
-	fwnode_handle_put(args.fwnode);
-	return ret;
 }
 
 /*
@@ -1085,22 +1079,17 @@ v4l2_fwnode_reference_parse_int_props(struct device *dev,
 
 		asd = v4l2_async_notifier_add_fwnode_subdev(notifier, fwnode,
 							    sizeof(*asd));
+		fwnode_handle_put(fwnode);
 		if (IS_ERR(asd)) {
 			/* not an error if asd already exists */
-			if (PTR_ERR(asd) == -EEXIST) {
-				fwnode_handle_put(fwnode);
+			if (PTR_ERR(asd) == -EEXIST)
 				continue;
-			}
 
-			goto error;
+			return PTR_ERR(asd);
 		}
 	}
 
 	return PTR_ERR(fwnode) == -ENOENT ? 0 : PTR_ERR(fwnode);
-
-error:
-	fwnode_handle_put(fwnode);
-	return PTR_ERR(fwnode);
 }
 
 int v4l2_async_notifier_parse_fwnode_sensor_common(struct device *dev,
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 1497bda66c3b..b9141ffa188a 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -175,8 +175,9 @@ int v4l2_async_notifier_add_subdev(struct v4l2_async_notifier *notifier,
  *		     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.
+ * Allocate a fwnode-matched asd of size asd_struct_size, and add it to the
+ * notifiers @asd_list. The function also gets a reference of the fwnode which
+ * is released later at notifier cleanup time.
  */
 struct v4l2_async_subdev *
 v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
-- 
2.11.0


  parent reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06 13:02 [PATCH v2 0/9] Rework V4L2 fwnode parsing; add defaults and avoid iteration Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 1/9] davinci-vpif: Don't dereference endpoint after putting it, fix refcounting Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 2/9] v4l2-async: Use endpoint node, not device node, for fwnode match Sakari Ailus
2019-06-14 15:45   ` Jacopo Mondi
2019-06-14 21:21   ` Niklas Söderlund
2019-06-21  9:19     ` Jacopo Mondi
2019-06-21 11:43       ` Niklas Söderlund
2019-06-28  8:29     ` Sakari Ailus
2019-06-06 13:02 ` Sakari Ailus [this message]
2019-06-14 16:01   ` [PATCH v2 3/9] v4l2-async: Get fwnode reference when putting it to the notifier's list Jacopo Mondi
2019-06-28 11:35     ` Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 4/9] v4l2-async: Add v4l2_async_notifier_add_fwnode_remote_subdev Sakari Ailus
2019-06-14 16:14   ` Jacopo Mondi
2019-06-28 11:47     ` Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 5/9] omap3isp: Rework OF endpoint parsing Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 6/9] v4l2-async: Safely clean up an uninitialised notifier Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 7/9] ipu3-cio2: Clean up notifier's subdev list if parsing endpoints fails Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 8/9] ipu3-cio2: Proceed with notifier init even if there are no subdevs Sakari Ailus
2019-06-06 13:02 ` [PATCH v2 9/9] ipu3-cio2: Parse information from firmware without using callbacks Sakari Ailus
2019-06-14 16:31   ` Jacopo Mondi
2019-06-28 11:57     ` Sakari Ailus

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190606130225.10751-4-sakari.ailus@linux.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jacopo@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=niklas.soderlund@ragnatech.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox