[PATCH 4/5] v4l2: async: Postpone subdev_notifier registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Currently, subdevice notifiers are tested against all available
subdevices as soon as they get registered. It often happens anyway
that the subdevice they are connected to is not yet initialized, as
it usually gets registered later in drivers' code. This makes debug
of v4l2_async particularly painful, as identifying a notifier with
an unitialized subdevice is tricky as they don't have a valid
'struct device *' or 'struct fwnode_handle *' to be identified with.

In order to make sure that the notifier's subdevices is initialized
when the notifier is tesed against available subdevices post-pone the
actual notifier registration at subdevice registration time.

It is worth noting that post-poning registration of a subdevice notifier
does not impact on the completion of the notifiers chain, as even if a
subdev notifier completes as soon as it gets registered, the complete()
call chain cannot be upscaled as long as the subdevice the notifiers
belongs to is not registered.

Also, it is now safe to access a notifier 'struct device *' as we're now
sure it is properly initialized when the notifier is actually
registered.

Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
---
 drivers/media/v4l2-core/v4l2-async.c | 65 +++++++++++++++++++++++-------------
 include/media/v4l2-async.h           |  2 ++
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 0a1bf1d..c13a781 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -25,6 +25,13 @@
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>

+static struct device *v4l2_async_notifier_dev(
+					struct v4l2_async_notifier *notifier)
+{
+	return notifier->v4l2_dev ? notifier->v4l2_dev->dev :
+				    notifier->sd->dev;
+}
+
 static int v4l2_async_notifier_call_bound(struct v4l2_async_notifier *n,
 					  struct v4l2_subdev *subdev,
 					  struct v4l2_async_subdev *asd)
@@ -124,19 +131,6 @@ static struct v4l2_async_subdev *v4l2_async_find_match(
 	return NULL;
 }

-/* Find the sub-device notifier registered by a sub-device driver. */
-static struct v4l2_async_notifier *v4l2_async_find_subdev_notifier(
-	struct v4l2_subdev *sd)
-{
-	struct v4l2_async_notifier *n;
-
-	list_for_each_entry(n, &notifier_list, list)
-		if (n->sd == sd)
-			return n;
-
-	return NULL;
-}
-
 /* Get v4l2_device related to the notifier if one can be found. */
 static struct v4l2_device *v4l2_async_notifier_find_v4l2_dev(
 	struct v4l2_async_notifier *notifier)
@@ -160,7 +154,7 @@ static bool v4l2_async_notifier_can_complete(

 	list_for_each_entry(sd, &notifier->done, async_list) {
 		struct v4l2_async_notifier *subdev_notifier =
-			v4l2_async_find_subdev_notifier(sd);
+							sd->subdev_notifier;

 		if (subdev_notifier &&
 		    !v4l2_async_notifier_can_complete(subdev_notifier))
@@ -228,7 +222,7 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
 	/*
 	 * See if the sub-device has a notifier. If not, return here.
 	 */
-	subdev_notifier = v4l2_async_find_subdev_notifier(sd);
+	subdev_notifier = sd->subdev_notifier;
 	if (!subdev_notifier || subdev_notifier->parent)
 		return 0;

@@ -294,7 +288,7 @@ static void v4l2_async_notifier_unbind_all_subdevs(

 	list_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {
 		struct v4l2_async_notifier *subdev_notifier =
-			v4l2_async_find_subdev_notifier(sd);
+							sd->subdev_notifier;

 		if (subdev_notifier)
 			v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
@@ -371,8 +365,7 @@ static bool v4l2_async_notifier_fwnode_has_async_subdev(

 static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
 {
-	struct device *dev =
-		notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL;
+	struct device *dev = v4l2_async_notifier_dev(notifier);
 	struct v4l2_async_subdev *asd;
 	int ret;
 	int i;
@@ -383,6 +376,8 @@ static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
 	INIT_LIST_HEAD(&notifier->waiting);
 	INIT_LIST_HEAD(&notifier->done);

+	notifier->owner = dev_fwnode(dev);
+
 	mutex_lock(&list_lock);

 	for (i = 0; i < notifier->num_subdevs; i++) {
@@ -421,6 +416,7 @@ static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)

 	/* Keep also completed notifiers on the list */
 	list_add(&notifier->list, &notifier_list);
+	notifier->registered = true;

 	mutex_unlock(&list_lock);

@@ -447,7 +443,7 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
 		return -EINVAL;

 	notifier->v4l2_dev = v4l2_dev;
-	notifier->owner = dev_fwnode(v4l2_dev->dev);
+	notifier->registered = false;

 	ret = __v4l2_async_notifier_register(notifier);
 	if (ret)
@@ -466,7 +462,11 @@ int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
 		return -EINVAL;

 	notifier->sd = sd;
-	notifier->owner = dev_fwnode(sd->dev);
+	sd->subdev_notifier = notifier;
+	notifier->registered = false;
+
+	if (!sd->dev || !sd->fwnode)
+		return 0;

 	ret = __v4l2_async_notifier_register(notifier);
 	if (ret)
@@ -482,12 +482,15 @@ static void __v4l2_async_notifier_unregister(
 	if (!notifier || (!notifier->v4l2_dev && !notifier->sd))
 		return;

-	v4l2_async_notifier_unbind_all_subdevs(notifier);
+	if (notifier->registered) {
+		v4l2_async_notifier_unbind_all_subdevs(notifier);
+		list_del(&notifier->list);
+	}

 	notifier->sd = NULL;
 	notifier->v4l2_dev = NULL;
-
-	list_del(&notifier->list);
+	notifier->owner = NULL;
+	notifier->registered = false;
 }

 void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
@@ -548,6 +551,20 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 			sd->fwnode = dev_fwnode(sd->dev);
 	}

+	/*
+	 * If the subdevice has an unregisterd notifier, it's now time
+	 * to register it.
+	 */
+	subdev_notifier = sd->subdev_notifier;
+	if (subdev_notifier && !subdev_notifier->registered) {
+		ret = __v4l2_async_notifier_register(subdev_notifier);
+		if (ret) {
+			sd->fwnode = NULL;
+			subdev_notifier->owner = NULL;
+			return ret;
+		}
+	}
+
 	mutex_lock(&list_lock);

 	INIT_LIST_HEAD(&sd->async_list);
@@ -589,7 +606,7 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 	 * Complete failed. Unbind the sub-devices bound through registering
 	 * this async sub-device.
 	 */
-	subdev_notifier = v4l2_async_find_subdev_notifier(sd);
+	subdev_notifier = sd->subdev_notifier;
 	if (subdev_notifier)
 		v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);

diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index a15c01d..6ab04ad 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -110,6 +110,7 @@ struct v4l2_async_notifier_operations {
  * @waiting:	list of struct v4l2_async_subdev, waiting for their drivers
  * @done:	list of struct v4l2_subdev, already probed
  * @list:	member in a global list of notifiers
+ * @registered: notifier registered complete flag
  */
 struct v4l2_async_notifier {
 	const struct v4l2_async_notifier_operations *ops;
@@ -123,6 +124,7 @@ struct v4l2_async_notifier {
 	struct list_head waiting;
 	struct list_head done;
 	struct list_head list;
+	bool registered;
 };

 /**
--
2.7.4
[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux