All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: linux-media@vger.kernel.org
Cc: hverkuil@xs4all.nl, niklas.soderlund@ragnatech.se,
	laurent.pinchart@ideasonboard.com
Subject: [PATCH 3/9] v4l2-async: Get fwnode reference when putting it to the notifier's list
Date: Fri,  5 Apr 2019 12:41:21 +0300	[thread overview]
Message-ID: <20190405094127.18975-5-sakari.ailus@linux.intel.com> (raw)
In-Reply-To: <20190405094127.18975-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         | 24 ++++++------------------
 include/media/v4l2-async.h                    |  5 +++--
 7 files changed, 23 insertions(+), 34 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 014b3f11e1ba..1bd6df477189 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;
 }
 
 /*
@@ -1086,23 +1080,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)) {
-			ret = PTR_ERR(asd);
 			/* not an error if asd already exists */
-			if (ret == -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 ret;
 }
 
 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	other threads:[~2019-04-05  9:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-05  9:41 [PATCH 0/9] Rework V4L2 fwnode parsing; add defaults and avoid iteration Sakari Ailus
2019-04-05  9:41 ` [PATCH 1/9] davinci-vpif: Don't dereference endpoint after putting it, fix refcounting Sakari Ailus
2019-04-05  9:41 ` [PATCH 1/9] davinci-vpif: Fix endpoint refcounting Sakari Ailus
2019-04-05 11:30   ` Ian Arkver
2019-04-05 11:34     ` Sakari Ailus
2019-04-05  9:41 ` [PATCH 2/9] v4l2-async: Use endpoint node, not device node, for fwnode match Sakari Ailus
2019-06-13  0:52   ` Niklas Söderlund
2019-04-05  9:41 ` Sakari Ailus [this message]
2019-04-05  9:41 ` [PATCH 4/9] v4l2-async: Add v4l2_async_notifier_add_fwnode_remote_subdev Sakari Ailus
2019-04-05  9:41 ` [PATCH 5/9] omap3isp: Rework OF endpoint parsing Sakari Ailus
2019-04-05  9:41 ` [PATCH 6/9] v4l2-async: Safely clean up an uninitialised notifier Sakari Ailus
2019-04-05  9:41 ` [PATCH 7/9] ipu3-cio2: Clean up notifier's subdev list if parsing endpoints fails Sakari Ailus
2019-04-05  9:41 ` [PATCH 8/9] ipu3-cio2: Proceed with notifier init even if there are no subdevs Sakari Ailus
2019-04-05  9:41 ` [PATCH 9/9] ipu3-cio2: Parse information from firmware without using callbacks Sakari Ailus
2019-04-05 11:16 ` [PATCH 0/9] Rework V4L2 fwnode parsing; add defaults and avoid iteration Sakari Ailus
2019-04-12 15:33 ` Jacopo Mondi
2019-04-12 20:27   ` Sakari Ailus

Reply instructions:

You may reply publicly 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=20190405094127.18975-5-sakari.ailus@linux.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=hverkuil@xs4all.nl \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.