Unified fwnode endpoint parser, async sub-device notifier support, N9 flash DTS

Remove V4L2 async re-probing support. The re-probing support has been there to support cases where the sub-devices require resources provided by the main driver's hardware to function, such as clocks.

Reprobing has allowed unbinding and again binding the main driver without explicitly unbinding the sub-device drivers. This is certainly not a common need, and the responsibility will be the user's going forward.

An alternative could have been to introduce notifier specific locks. Considering the complexity of the re-probing and that it isn't really a solution to a problem but a workaround, remove re-probing instead.

If there is a need to support the clock provider unregister/register cycle while keeping the clock references in the consumers in the future, this should be implemented in the clock framework instead, not in V4L2.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

Reviewed-by Sebastian Reichel <[email protected]>

---
 drivers/media/v4l2-core/v4l2-async.c | 54 +-----------------------------------
 1 file changed, 1 insertion(+), 53 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +1 -53

index d741a8e0fdac..60a1a50b9537 100644
@@ -198,78 +198,26 @@ EXPORT_SYMBOL(v4l2_async_notifier_register);
 void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
 {
 	struct v4l2_subdev *sd, *tmp;
-	unsigned int notif_n_subdev = notifier->num_subdevs;
-	unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
-	struct device **dev;
-	int i = 0;
 
 	if (!notifier->v4l2_dev)
 		return;
 
-	dev = kvmalloc_array(n_subdev, sizeof(*dev), GFP_KERNEL);
-	if (!dev) {
-		dev_err(notifier->v4l2_dev->dev,
-			"Failed to allocate device cache!\n");
-	}
-
 	mutex_lock(&list_lock);
 
 	list_del(&notifier->list);
 
 	list_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {
-		struct device *d;
-
-		d = get_device(sd->dev);
-
 		v4l2_async_cleanup(sd);
 
-		/* If we handled USB devices, we'd have to lock the parent too */
-		device_release_driver(d);
-
 		if (notifier->unbind)
 			notifier->unbind(notifier, sd, sd->asd);
 
-		/*
-		 * Store device at the device cache, in order to call
-		 * put_device() on the final step
-		 */
-		if (dev)
-			dev[i++] = d;
-		else
-			put_device(d);
+		list_move(&sd->async_list, &subdev_list);
 	}
 
 	mutex_unlock(&list_lock);
 
-	/*
-	 * Call device_attach() to reprobe devices
-	 *
-	 * NOTE: If dev allocation fails, i is 0, and the whole loop won't be
-	 * executed.
-	 */
-	while (i--) {
-		struct device *d = dev[i];
-
-		if (d && device_attach(d) < 0) {
-			const char *name = "(none)";
-			int lock = device_trylock(d);
-
-			if (lock && d->driver)
-				name = d->driver->name;
-			dev_err(d, "Failed to re-probe to %s\n", name);
-			if (lock)
-				device_unlock(d);
-		}
-		put_device(d);
-	}
-	kvfree(dev);
-
 	notifier->v4l2_dev = NULL;
-
-	/*
-	 * Don't care about the waiting list, it is initialised and populated
-	 * upon notifier registration.
-	 */
 }
 EXPORT_SYMBOL(v4l2_async_notifier_unregister);
 

Signature · 1 line

2.11.0

v4l2_async_cleanup() is called when the async sub-device is unbound from the media device. As the pointer is set by the driver registering the async sub-device, leave the pointer as set by the driver.

Signed-off-by Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/[email protected]>

Reviewed-by Sebastian Reichel <sebastian.reichel-ZGY8ohtN/[email protected]>

Acked-by Hans Verkuil <[email protected]>

---
 drivers/media/v4l2-core/v4l2-async.c | 1 -
 1 file changed, 1 deletion(-)

drivers/media/v4l2-core/v4l2-async.c +0 -1

index 60a1a50b9537..21c748bf3a7b 100644
@@ -134,7 +134,6 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
 	/* Subdevice driver will reprobe and put the subdev back onto the list */
 	list_del_init(&sd->async_list);
 	sd->asd = NULL;
-	sd->dev = NULL;
 }
 
 int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,

Signature · 1 line

2.11.0

The notifier complete callback may return an error. This error code was simply returned to the caller but never handled properly.

Move calling the complete callback function to the caller from v4l2_async_test_notify and undo the work that was done either in async sub-device or async notifier registration.

Reported-by Russell King <[email protected]>

Signed-off-by Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/[email protected]>

---
 drivers/media/v4l2-core/v4l2-async.c | 78 +++++++++++++++++++++++++++---------
 1 file changed, 60 insertions(+), 18 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +60 -18

index ca281438a0ae..4924481451ca 100644
@@ -122,9 +122,6 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
 	/* Move from the global subdevice list to notifier's done */
 	list_move(&sd->async_list, &notifier->done);
 
-	if (list_empty(&notifier->waiting) && notifier->complete)
-		return notifier->complete(notifier);
-
 	return 0;
 }
 

@@ -136,11 +133,27 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
 	sd->asd = NULL;
 }
 
+static void v4l2_async_notifier_unbind_all_subdevs(
+	struct v4l2_async_notifier *notifier)
+{
+	struct v4l2_subdev *sd, *tmp;
+
+	list_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {
+		if (notifier->unbind)
+			notifier->unbind(notifier, sd, sd->asd);
+
+		v4l2_async_cleanup(sd);
+
+		list_move(&sd->async_list, &subdev_list);
+	}
+}
+
 int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
 				 struct v4l2_async_notifier *notifier)
 {
 	struct v4l2_subdev *sd, *tmp;
 	struct v4l2_async_subdev *asd;
+	int ret;
 	int i;
 
 	if (!v4l2_dev || !notifier->num_subdevs ||

@@ -185,19 +198,30 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
 		}
 	}
 
+	if (list_empty(&notifier->waiting) && notifier->complete) {
+		ret = notifier->complete(notifier);
+		if (ret)
+			goto err_complete;
+	}
+
 	/* Keep also completed notifiers on the list */
 	list_add(&notifier->list, &notifier_list);
 
 	mutex_unlock(&list_lock);
 
 	return 0;
+
+err_complete:
+	v4l2_async_notifier_unbind_all_subdevs(notifier);
+
+	mutex_unlock(&list_lock);
+
+	return ret;
 }
 EXPORT_SYMBOL(v4l2_async_notifier_register);
 
 void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
 {
-	struct v4l2_subdev *sd, *tmp;
-
 	if (!notifier->v4l2_dev)
 		return;
 

@@ -205,14 +229,7 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
 
 	list_del(&notifier->list);
 
-	list_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {
-		if (notifier->unbind)
-			notifier->unbind(notifier, sd, sd->asd);
-
-		v4l2_async_cleanup(sd);
-
-		list_move(&sd->async_list, &subdev_list);
-	}
+	v4l2_async_notifier_unbind_all_subdevs(notifier);
 
 	mutex_unlock(&list_lock);
 

@@ -223,6 +240,7 @@ EXPORT_SYMBOL(v4l2_async_notifier_unregister);
 int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 {
 	struct v4l2_async_notifier *notifier;
+	int ret;
 
 	/*
 	 * No reference taken. The reference is held by the device

@@ -238,19 +256,43 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 
 	list_for_each_entry(notifier, &notifier_list, list) {
 		struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, sd);
-		if (asd) {
-			int ret = v4l2_async_test_notify(notifier, sd, asd);
-			mutex_unlock(&list_lock);
-			return ret;
-		}
+		int ret;
+
+		if (!asd)
+			continue;
+
+		ret = v4l2_async_test_notify(notifier, sd, asd);
+		if (ret)
+			goto err_unlock;
+
+		if (!list_empty(&notifier->waiting) || !notifier->complete)
+			goto out_unlock;
+
+		ret = notifier->complete(notifier);
+		if (ret)
+			goto err_cleanup;
+
+		goto out_unlock;
 	}
 
 	/* None matched, wait for hot-plugging */
 	list_add(&sd->async_list, &subdev_list);
 
+out_unlock:
 	mutex_unlock(&list_lock);
 
 	return 0;
+
+err_cleanup:
+	if (notifier->unbind)
+		notifier->unbind(notifier, sd, sd->asd);
+
+	v4l2_async_cleanup(sd);
+
+err_unlock:
+	mutex_unlock(&list_lock);
+
+	return ret;
 }
 EXPORT_SYMBOL(v4l2_async_register_subdev);
 

Signature · 1 line

2.11.0

The notifier complete callback may return an error. This error code was simply returned to the caller but never handled properly.

Move calling the complete callback function to the caller from v4l2_async_test_notify and undo the work that was done either in async sub-device or async notifier registration.

Reported-by Russell King <[email protected]>

Signed-off-by Sakari Ailus <[email protected]>

---

 drivers/media/v4l2-core/v4l2-async.c | 78 +++++++++++++++++++++++++++---------
 1 file changed, 60 insertions(+), 18 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +60 -18

index ca281438a0ae..4924481451ca 100644
@@ -122,9 +122,6 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
 	/* Move from the global subdevice list to notifier's done */
 	list_move(&sd->async_list, &notifier->done);
 
-	if (list_empty(&notifier->waiting) && notifier->complete)
-		return notifier->complete(notifier);
-
 	return 0;
 }
 

@@ -136,11 +133,27 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd)
 	sd->asd = NULL;
 }
 
+static void v4l2_async_notifier_unbind_all_subdevs(
+	struct v4l2_async_notifier *notifier)
+{
+	struct v4l2_subdev *sd, *tmp;
+
+	list_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {
+		if (notifier->unbind)
+			notifier->unbind(notifier, sd, sd->asd);
+
+		v4l2_async_cleanup(sd);
+
+		list_move(&sd->async_list, &subdev_list);
+	}
+}
+
 int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
 				 struct v4l2_async_notifier *notifier)
 {
 	struct v4l2_subdev *sd, *tmp;
 	struct v4l2_async_subdev *asd;
+	int ret;
 	int i;
 
 	if (!v4l2_dev || !notifier->num_subdevs ||

@@ -185,19 +198,30 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
 		}
 	}
 
+	if (list_empty(&notifier->waiting) && notifier->complete) {
+		ret = notifier->complete(notifier);
+		if (ret)
+			goto err_complete;
+	}
+
 	/* Keep also completed notifiers on the list */
 	list_add(&notifier->list, &notifier_list);
 
 	mutex_unlock(&list_lock);
 
 	return 0;
+
+err_complete:
+	v4l2_async_notifier_unbind_all_subdevs(notifier);
+
+	mutex_unlock(&list_lock);
+
+	return ret;
 }
 EXPORT_SYMBOL(v4l2_async_notifier_register);
 
 void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
 {
-	struct v4l2_subdev *sd, *tmp;
-
 	if (!notifier->v4l2_dev)
 		return;
 

@@ -205,14 +229,7 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
 
 	list_del(&notifier->list);
 
-	list_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {
-		if (notifier->unbind)
-			notifier->unbind(notifier, sd, sd->asd);
-
-		v4l2_async_cleanup(sd);
-
-		list_move(&sd->async_list, &subdev_list);
-	}
+	v4l2_async_notifier_unbind_all_subdevs(notifier);
 
 	mutex_unlock(&list_lock);
 

@@ -223,6 +240,7 @@ EXPORT_SYMBOL(v4l2_async_notifier_unregister);
 int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 {
 	struct v4l2_async_notifier *notifier;
+	int ret;
 
 	/*
 	 * No reference taken. The reference is held by the device

@@ -238,19 +256,43 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 
 	list_for_each_entry(notifier, &notifier_list, list) {
 		struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, sd);
-		if (asd) {
-			int ret = v4l2_async_test_notify(notifier, sd, asd);
-			mutex_unlock(&list_lock);
-			return ret;
-		}
+		int ret;
+
+		if (!asd)
+			continue;
+
+		ret = v4l2_async_test_notify(notifier, sd, asd);
+		if (ret)
+			goto err_unlock;
+
+		if (!list_empty(&notifier->waiting) || !notifier->complete)
+			goto out_unlock;
+
+		ret = notifier->complete(notifier);
+		if (ret)
+			goto err_cleanup;
+
+		goto out_unlock;
 	}
 
 	/* None matched, wait for hot-plugging */
 	list_add(&sd->async_list, &subdev_list);
 
+out_unlock:
 	mutex_unlock(&list_lock);
 
 	return 0;
+
+err_cleanup:
+	if (notifier->unbind)
+		notifier->unbind(notifier, sd, sd->asd);
+
+	v4l2_async_cleanup(sd);
+
+err_unlock:
+	mutex_unlock(&list_lock);
+
+	return ret;
 }
 EXPORT_SYMBOL(v4l2_async_register_subdev);
 

Signature · 1 line

2.11.0

Rename internal functions to make the names of the functions better describe what they do.

Old name New name
v4l2_async_test_notify v4l2_async_match_notify
v4l2_async_belongs v4l2_async_find_match

Signed-off-by Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/[email protected]>

Acked-by Hans Verkuil <[email protected]>

Acked-by Pavel Machek <[email protected]>

Reviewed-by Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/[email protected]>

Reviewed-by Sebastian Reichel <sebastian.reichel-ZGY8ohtN/[email protected]>

---
 drivers/media/v4l2-core/v4l2-async.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +10 -9

index cde2cf2ab4b0..8b84fea50c2a 100644
@@ -60,8 +60,8 @@ static LIST_HEAD(subdev_list);
 static LIST_HEAD(notifier_list);
 static DEFINE_MUTEX(list_lock);
 
-static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *notifier,
-						    struct v4l2_subdev *sd)
+static struct v4l2_async_subdev *v4l2_async_find_match(
+	struct v4l2_async_notifier *notifier, struct v4l2_subdev *sd)
 {
 	bool (*match)(struct v4l2_subdev *, struct v4l2_async_subdev *);
 	struct v4l2_async_subdev *asd;

@@ -95,9 +95,9 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *
 	return NULL;
 }
 
-static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
-				  struct v4l2_subdev *sd,
-				  struct v4l2_async_subdev *asd)
+static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *sd,
+				   struct v4l2_async_subdev *asd)
 {
 	int ret;
 

@@ -187,11 +187,11 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
 	list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
 		int ret;
 
-		asd = v4l2_async_belongs(notifier, sd);
+		asd = v4l2_async_find_match(notifier, sd);
 		if (!asd)
 			continue;
 
-		ret = v4l2_async_test_notify(notifier, sd, asd);
+		ret = v4l2_async_match_notify(notifier, sd, asd);
 		if (ret < 0) {
 			mutex_unlock(&list_lock);
 			return ret;

@@ -255,13 +255,14 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 	INIT_LIST_HEAD(&sd->async_list);
 
 	list_for_each_entry(notifier, &notifier_list, list) {
-		struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, sd);
+		struct v4l2_async_subdev *asd = v4l2_async_find_match(notifier,
+								      sd);
 		int ret;
 
 		if (!asd)
 			continue;
 
-		ret = v4l2_async_test_notify(notifier, sd, asd);
+		ret = v4l2_async_match_notify(notifier, sd, asd);
 		if (ret)
 			goto err_unlock;
 

Signature · 1 line

2.11.0

Add a convenience function for parsing firmware for information on related devices using v4l2_async_notifier_parse_fwnode_sensor_common() registering the notifier and finally the async sub-device itself.

This should be useful for sensor drivers that do not have device specific requirements related to firmware information parsing or the async framework.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

---
 drivers/media/v4l2-core/v4l2-async.c  | 19 ++++++++++++----
 drivers/media/v4l2-core/v4l2-fwnode.c | 41 +++++++++++++++++++++++++++++++++++
 include/media/v4l2-async.h            | 22 +++++++++++++++++++
 include/media/v4l2-subdev.h           |  3 +++
 4 files changed, 81 insertions(+), 4 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +15 -4

index b4e88eef195f..e81a72b8d46e 100644
@@ -474,19 +474,25 @@ int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
 }
 EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);
 
-void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+static void __v4l2_async_notifier_unregister(
+	struct v4l2_async_notifier *notifier)
 {
-	if (!notifier->v4l2_dev && !notifier->sd)
+	if (!notifier || (!notifier->v4l2_dev && !notifier->sd))
 		return;
 
-	mutex_lock(&list_lock);
-
 	v4l2_async_notifier_unbind_all_subdevs(notifier);
 
 	notifier->sd = NULL;
 	notifier->v4l2_dev = NULL;
 
 	list_del(&notifier->list);
+}
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+	mutex_lock(&list_lock);
+
+	__v4l2_async_notifier_unregister(notifier);
 
 	mutex_unlock(&list_lock);
 }

@@ -596,6 +602,11 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
 {
 	mutex_lock(&list_lock);
 
+	__v4l2_async_notifier_unregister(sd->subdev_notifier);
+	v4l2_async_notifier_cleanup(sd->subdev_notifier);
+	kfree(sd->subdev_notifier);
+	sd->subdev_notifier = NULL;
+
 	if (sd->asd) {
 		struct v4l2_async_notifier *notifier = sd->notifier;
 

drivers/media/v4l2-core/v4l2-fwnode.c +41 -0

index 1234bd1a2f49..82af608fd626 100644
@@ -29,6 +29,7 @@
 
 #include <media/v4l2-async.h>
 #include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
 
 enum v4l2_fwnode_bus_type {
 	V4L2_FWNODE_BUS_TYPE_GUESS = 0,

@@ -900,6 +901,46 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(
 }
 EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_sensor_common);
 
+int v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)
+{
+	struct v4l2_async_notifier *notifier;
+	int ret;
+
+	if (WARN_ON(!sd->dev))
+		return -ENODEV;
+
+	notifier = kzalloc(sizeof(*notifier), GFP_KERNEL);
+	if (!notifier)
+		return -ENOMEM;
+
+	ret = v4l2_async_notifier_parse_fwnode_sensor_common(sd->dev,
+							     notifier);
+	if (ret < 0)
+		goto out_cleanup;
+
+	ret = v4l2_async_subdev_notifier_register(sd, notifier);
+	if (ret < 0)
+		goto out_cleanup;
+
+	ret = v4l2_async_register_subdev(sd);
+	if (ret < 0)
+		goto out_unregister;
+
+	sd->subdev_notifier = notifier;
+
+	return 0;
+
+out_unregister:
+	v4l2_async_notifier_unregister(notifier);
+
+out_cleanup:
+	v4l2_async_notifier_cleanup(notifier);
+	kfree(notifier);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Sakari Ailus <[email protected]>");
 MODULE_AUTHOR("Sylwester Nawrocki <[email protected]>");

include/media/v4l2-async.h +22 -0

index 8d8cfc3f3100..6152434cbe82 100644
@@ -174,6 +174,28 @@ void v4l2_async_notifier_cleanup(struct v4l2_async_notifier *notifier);
 int v4l2_async_register_subdev(struct v4l2_subdev *sd);
 
 /**
+ * v4l2_async_register_subdev_sensor_common - registers a sensor sub-device to
+ *					      the asynchronous sub-device
+ *					      framework and parse set up common
+ *					      sensor related devices
+ *
+ * @sd: pointer to struct &v4l2_subdev
+ *
+ * This function is just like v4l2_async_register_subdev() with the exception
+ * that calling it will also parse firmware interfaces for remote references
+ * using v4l2_async_notifier_parse_fwnode_sensor_common() and registers the
+ * async sub-devices. The sub-device is similarly unregistered by calling
+ * v4l2_async_unregister_subdev().
+ *
+ * While registered, the subdev module is marked as in-use.
+ *
+ * An error is returned if the module is no longer loaded on any attempts
+ * to register it.
+ */
+int __must_check v4l2_async_register_subdev_sensor_common(
+	struct v4l2_subdev *sd);
+
+/**
  * v4l2_async_unregister_subdev - unregisters a sub-device to the asynchronous
  * 	subdevice framework
  *

include/media/v4l2-subdev.h +3 -0

index e83872078376..ec399c770301 100644
@@ -793,6 +793,8 @@ struct v4l2_subdev_platform_data {
  *	list.
  * @asd: Pointer to respective &struct v4l2_async_subdev.
  * @notifier: Pointer to the managing notifier.
+ * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
+ *		     device using v4l2_device_register_sensor_subdev().
  * @pdata: common part of subdevice platform data
  *
  * Each instance of a subdev driver should create this struct, either

@@ -823,6 +825,7 @@ struct v4l2_subdev {
 	struct list_head async_list;
 	struct v4l2_async_subdev *asd;
 	struct v4l2_async_notifier *notifier;
+	struct v4l2_async_notifier *subdev_notifier;
 	struct v4l2_subdev_platform_data *pdata;
 };
 

Signature · 1 line

2.11.0

Add a convenience function for parsing firmware for information on related devices using v4l2_async_notifier_parse_fwnode_sensor_common() registering the notifier and finally the async sub-device itself.

This should be useful for sensor drivers that do not have device specific requirements related to firmware information parsing or the async framework.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

---

 drivers/media/v4l2-core/v4l2-async.c  | 19 ++++++++++++----
 drivers/media/v4l2-core/v4l2-fwnode.c | 41 +++++++++++++++++++++++++++++++++++
 include/media/v4l2-async.h            | 22 +++++++++++++++++++
 include/media/v4l2-subdev.h           |  3 +++
 4 files changed, 81 insertions(+), 4 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +15 -4

index b4e88eef195f..e81a72b8d46e 100644
@@ -474,19 +474,25 @@ int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
 }
 EXPORT_SYMBOL(v4l2_async_subdev_notifier_register);
 
-void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+static void __v4l2_async_notifier_unregister(
+	struct v4l2_async_notifier *notifier)
 {
-	if (!notifier->v4l2_dev && !notifier->sd)
+	if (!notifier || (!notifier->v4l2_dev && !notifier->sd))
 		return;
 
-	mutex_lock(&list_lock);
-
 	v4l2_async_notifier_unbind_all_subdevs(notifier);
 
 	notifier->sd = NULL;
 	notifier->v4l2_dev = NULL;
 
 	list_del(&notifier->list);
+}
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+	mutex_lock(&list_lock);
+
+	__v4l2_async_notifier_unregister(notifier);
 
 	mutex_unlock(&list_lock);
 }

@@ -596,6 +602,11 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
 {
 	mutex_lock(&list_lock);
 
+	__v4l2_async_notifier_unregister(sd->subdev_notifier);
+	v4l2_async_notifier_cleanup(sd->subdev_notifier);
+	kfree(sd->subdev_notifier);
+	sd->subdev_notifier = NULL;
+
 	if (sd->asd) {
 		struct v4l2_async_notifier *notifier = sd->notifier;
 

drivers/media/v4l2-core/v4l2-fwnode.c +41 -0

index 1234bd1a2f49..82af608fd626 100644
@@ -29,6 +29,7 @@
 
 #include <media/v4l2-async.h>
 #include <media/v4l2-fwnode.h>
+#include <media/v4l2-subdev.h>
 
 enum v4l2_fwnode_bus_type {
 	V4L2_FWNODE_BUS_TYPE_GUESS = 0,

@@ -900,6 +901,46 @@ int v4l2_async_notifier_parse_fwnode_sensor_common(
 }
 EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_sensor_common);
 
+int v4l2_async_register_subdev_sensor_common(struct v4l2_subdev *sd)
+{
+	struct v4l2_async_notifier *notifier;
+	int ret;
+
+	if (WARN_ON(!sd->dev))
+		return -ENODEV;
+
+	notifier = kzalloc(sizeof(*notifier), GFP_KERNEL);
+	if (!notifier)
+		return -ENOMEM;
+
+	ret = v4l2_async_notifier_parse_fwnode_sensor_common(sd->dev,
+							     notifier);
+	if (ret < 0)
+		goto out_cleanup;
+
+	ret = v4l2_async_subdev_notifier_register(sd, notifier);
+	if (ret < 0)
+		goto out_cleanup;
+
+	ret = v4l2_async_register_subdev(sd);
+	if (ret < 0)
+		goto out_unregister;
+
+	sd->subdev_notifier = notifier;
+
+	return 0;
+
+out_unregister:
+	v4l2_async_notifier_unregister(notifier);
+
+out_cleanup:
+	v4l2_async_notifier_cleanup(notifier);
+	kfree(notifier);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor_common);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Sakari Ailus <[email protected]>");
 MODULE_AUTHOR("Sylwester Nawrocki <[email protected]>");

include/media/v4l2-async.h +22 -0

index 8d8cfc3f3100..6152434cbe82 100644
@@ -174,6 +174,28 @@ void v4l2_async_notifier_cleanup(struct v4l2_async_notifier *notifier);
 int v4l2_async_register_subdev(struct v4l2_subdev *sd);
 
 /**
+ * v4l2_async_register_subdev_sensor_common - registers a sensor sub-device to
+ *					      the asynchronous sub-device
+ *					      framework and parse set up common
+ *					      sensor related devices
+ *
+ * @sd: pointer to struct &v4l2_subdev
+ *
+ * This function is just like v4l2_async_register_subdev() with the exception
+ * that calling it will also parse firmware interfaces for remote references
+ * using v4l2_async_notifier_parse_fwnode_sensor_common() and registers the
+ * async sub-devices. The sub-device is similarly unregistered by calling
+ * v4l2_async_unregister_subdev().
+ *
+ * While registered, the subdev module is marked as in-use.
+ *
+ * An error is returned if the module is no longer loaded on any attempts
+ * to register it.
+ */
+int __must_check v4l2_async_register_subdev_sensor_common(
+	struct v4l2_subdev *sd);
+
+/**
  * v4l2_async_unregister_subdev - unregisters a sub-device to the asynchronous
  * 	subdevice framework
  *

include/media/v4l2-subdev.h +3 -0

index e83872078376..ec399c770301 100644
@@ -793,6 +793,8 @@ struct v4l2_subdev_platform_data {
  *	list.
  * @asd: Pointer to respective &struct v4l2_async_subdev.
  * @notifier: Pointer to the managing notifier.
+ * @subdev_notifier: A sub-device notifier implicitly registered for the sub-
+ *		     device using v4l2_device_register_sensor_subdev().
  * @pdata: common part of subdevice platform data
  *
  * Each instance of a subdev driver should create this struct, either

@@ -823,6 +825,7 @@ struct v4l2_subdev {
 	struct list_head async_list;
 	struct v4l2_async_subdev *asd;
 	struct v4l2_async_notifier *notifier;
+	struct v4l2_async_notifier *subdev_notifier;
 	struct v4l2_subdev_platform_data *pdata;
 };
 

Signature · 1 line

2.11.0

Document optional lens-focus and flash-leds properties for the smiapp driver.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Acked-by Pavel Machek <[email protected]>

---
 Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
 1 file changed, 2 insertions(+)

Document optional lens-focus and flash-leds properties for the smiapp driver.

Signed-off-by Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/[email protected]>

Acked-by Hans Verkuil <[email protected]>

Acked-by Pavel Machek <[email protected]>

---
 Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
 1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/media/i2c/nokia,smia.txt +2 -0

index 855e1faf73e2..33f10a94c381 100644
@@ -27,6 +27,8 @@ Optional properties
 - nokia,nvm-size: The size of the NVM, in bytes. If the size is not given,
   the NVM contents will not be read.
 - reset-gpios: XSHUTDOWN GPIO
+- flash-leds: See ../video-interfaces.txt
+- lens-focus: See ../video-interfaces.txt
 
 
 Endpoint node mandatory properties

Signature · 1 line

2.11.0

Parse async sub-devices related to the sensor by switching the async sub-device registration function.

These types devices aren't directly related to the sensor, but are nevertheless handled by the smiapp driver due to the relationship of these component to the main part of the camera module --- the sensor.

This does not yet address providing the user space with information on how to associate the sensor or lens devices but the kernel now has the necessary information to do that.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Acked-by Pavel Machek <[email protected]>

---

 drivers/media/i2c/smiapp/smiapp-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/media/i2c/smiapp/smiapp-core.c +1 -1

index fbd851be51d2..a87c50373813 100644
@@ -3118,7 +3118,7 @@ static int smiapp_probe(struct i2c_client *client,
 	if (rval < 0)
 		goto out_media_entity_cleanup;
 
-	rval = v4l2_async_register_subdev(&sensor->src->sd);
+	rval = v4l2_async_register_subdev_sensor_common(&sensor->src->sd);
 	if (rval < 0)
 		goto out_media_entity_cleanup;
 

Signature · 1 line

2.11.0

Parse async sub-devices related to the sensor by switching the async sub-device registration function.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

---

 drivers/media/i2c/ov5670.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/media/i2c/ov5670.c +1 -1

index a65469f88e36..9f9196568eb8 100644
@@ -2529,7 +2529,7 @@ static int ov5670_probe(struct i2c_client *client)
 	}
 
 	/* Async register for subdev */
-	ret = v4l2_async_register_subdev(&ov5670->sd);
+	ret = v4l2_async_register_subdev_sensor_common(&ov5670->sd);
 	if (ret < 0) {
 		err_msg = "v4l2_async_register_subdev() error";
 		goto error_entity_cleanup;

Signature · 1 line

2.11.0

The check whether an async sub-device is bound to a notifier was performed without list_lock held, making it possible for another process to unbind the async sub-device before the sub-device unregistration function proceeds to take the lock.

Fix this by first acquiring the lock and then proceeding with the check.

Signed-off-by Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/[email protected]>

Reviewed-by Sebastian Reichel <sebastian.reichel-ZGY8ohtN/[email protected]>

Acked-by Hans Verkuil <[email protected]>

---
 drivers/media/v4l2-core/v4l2-async.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +7 -11

index 4924481451ca..cde2cf2ab4b0 100644
@@ -298,20 +298,16 @@ EXPORT_SYMBOL(v4l2_async_register_subdev);
 
 void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
 {
-	struct v4l2_async_notifier *notifier = sd->notifier;
-
-	if (!sd->asd) {
-		if (!list_empty(&sd->async_list))
-			v4l2_async_cleanup(sd);
-		return;
-	}
-
 	mutex_lock(&list_lock);
 
-	list_add(&sd->asd->list, &notifier->waiting);
+	if (sd->asd) {
+		struct v4l2_async_notifier *notifier = sd->notifier;
 
-	if (notifier->unbind)
-		notifier->unbind(notifier, sd, sd->asd);
+		list_add(&sd->asd->list, &notifier->waiting);
+
+		if (notifier->unbind)
+			notifier->unbind(notifier, sd, sd->asd);
+	}
 
 	v4l2_async_cleanup(sd);
 

Signature · 1 line

2.11.0

Add two functions for parsing devices graph endpoints:
v4l2_async_notifier_parse_fwnode_endpoints and v4l2_async_notifier_parse_fwnode_endpoints_by_port. The former iterates over all endpoints whereas the latter only iterates over the endpoints in a given port.

The former is mostly useful for existing drivers that currently implement the iteration over all the endpoints themselves whereas the latter is especially intended for devices with both sinks and sources: async sub-devices for external devices connected to the device's sources will have already been set up, or the external sub-devices are part of the master device.

Depends-on: ("device property: preserve usecount for node passed to of_fwnode_graph_get_port_parent()")

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

---
 drivers/media/v4l2-core/v4l2-async.c  |  31 ++++++
 drivers/media/v4l2-core/v4l2-fwnode.c | 196 ++++++++++++++++++++++++++++++++++
 include/media/v4l2-async.h            |  24 ++++-
 include/media/v4l2-fwnode.h           | 118 ++++++++++++++++++++
 4 files changed, 367 insertions(+), 2 deletions(-)

drivers/media/v4l2-core/v4l2-async.c +31 -0

index 8b84fea50c2a..46aebfc75e43 100644
@@ -22,6 +22,7 @@
 
 #include <media/v4l2-async.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
 static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)

@@ -237,6 +238,36 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
 }
 EXPORT_SYMBOL(v4l2_async_notifier_unregister);
 
+void v4l2_async_notifier_cleanup(struct v4l2_async_notifier *notifier)
+{
+	unsigned int i;
+
+	if (!notifier->max_subdevs)
+		return;
+
+	for (i = 0; i < notifier->num_subdevs; i++) {
+		struct v4l2_async_subdev *asd = notifier->subdevs[i];
+
+		switch (asd->match_type) {
+		case V4L2_ASYNC_MATCH_FWNODE:
+			fwnode_handle_put(asd->match.fwnode.fwnode);
+			break;
+		default:
+			WARN_ON_ONCE(true);
+			break;
+		}
+
+		kfree(asd);
+	}
+
+	notifier->max_subdevs = 0;
+	notifier->num_subdevs = 0;
+
+	kvfree(notifier->subdevs);
+	notifier->subdevs = NULL;
+}
+EXPORT_SYMBOL_GPL(v4l2_async_notifier_cleanup);
+
 int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 {
 	struct v4l2_async_notifier *notifier;

drivers/media/v4l2-core/v4l2-fwnode.c +196 -0

index 40b2fbfe8865..df0695b7bbcc 100644
@@ -19,6 +19,7 @@
  */
 #include <linux/acpi.h>
 #include <linux/kernel.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/property.h>

@@ -26,6 +27,7 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
+#include <media/v4l2-async.h>
 #include <media/v4l2-fwnode.h>
 
 enum v4l2_fwnode_bus_type {

@@ -388,6 +390,200 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link)
 }
 EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
 
+static int v4l2_async_notifier_realloc(struct v4l2_async_notifier *notifier,
+				       unsigned int max_subdevs)
+{
+	struct v4l2_async_subdev **subdevs;
+
+	if (max_subdevs <= notifier->max_subdevs)
+		return 0;
+
+	subdevs = kvmalloc_array(
+		max_subdevs, sizeof(*notifier->subdevs),
+		GFP_KERNEL | __GFP_ZERO);
+	if (!subdevs)
+		return -ENOMEM;
+
+	if (notifier->subdevs) {
+		memcpy(subdevs, notifier->subdevs,
+		       sizeof(*subdevs) * notifier->num_subdevs);
+
+		kvfree(notifier->subdevs);
+	}
+
+	notifier->subdevs = subdevs;
+	notifier->max_subdevs = max_subdevs;
+
+	return 0;
+}
+
+static int v4l2_async_notifier_fwnode_parse_endpoint(
+	struct device *dev, struct v4l2_async_notifier *notifier,
+	struct fwnode_handle *endpoint, unsigned int asd_struct_size,
+	int (*parse_endpoint)(struct device *dev,
+			    struct v4l2_fwnode_endpoint *vep,
+			    struct v4l2_async_subdev *asd))
+{
+	struct v4l2_async_subdev *asd;
+	struct v4l2_fwnode_endpoint *vep;
+	int ret = 0;
+
+	asd = kzalloc(asd_struct_size, GFP_KERNEL);
+	if (!asd)
+		return -ENOMEM;
+
+	asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
+	asd->match.fwnode.fwnode =
+		fwnode_graph_get_remote_port_parent(endpoint);
+	if (!asd->match.fwnode.fwnode) {
+		dev_warn(dev, "bad remote port parent\n");
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
+	if (IS_ERR(vep)) {
+		ret = PTR_ERR(vep);
+		dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
+			 ret);
+		goto out_err;
+	}
+
+	ret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;
+	if (ret == -ENOTCONN)
+		dev_dbg(dev, "ignoring port@%u/endpoint@%u\n", vep->base.port,
+			vep->base.id);
+	else if (ret < 0)
+		dev_warn(dev,
+			 "driver could not parse port@%u/endpoint@%u (%d)\n",
+			 vep->base.port, vep->base.id, ret);
+	v4l2_fwnode_endpoint_free(vep);
+	if (ret < 0)
+		goto out_err;
+
+	notifier->subdevs[notifier->num_subdevs] = asd;
+	notifier->num_subdevs++;
+
+	return 0;
+
+out_err:
+	fwnode_handle_put(asd->match.fwnode.fwnode);
+	kfree(asd);
+
+	return ret == -ENOTCONN ? 0 : ret;
+}
+
+static int __v4l2_async_notifier_parse_fwnode_endpoints(
+	struct device *dev, struct v4l2_async_notifier *notifier,
+	size_t asd_struct_size, unsigned int port, bool has_port,
+	int (*parse_endpoint)(struct device *dev,
+			    struct v4l2_fwnode_endpoint *vep,
+			    struct v4l2_async_subdev *asd))
+{
+	struct fwnode_handle *fwnode;
+	unsigned int max_subdevs = notifier->max_subdevs;
+	int ret;
+
+	if (WARN_ON(asd_struct_size < sizeof(struct v4l2_async_subdev)))
+		return -EINVAL;
+
+	for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
+				     dev_fwnode(dev), fwnode)); ) {
+		struct fwnode_handle *dev_fwnode;
+		bool is_available;
+
+		dev_fwnode = fwnode_graph_get_port_parent(fwnode);
+		is_available = fwnode_device_is_available(dev_fwnode);
+		fwnode_handle_put(dev_fwnode);
+		if (!is_available)
+			continue;
+
+		if (has_port) {
+			struct fwnode_endpoint ep;
+
+			ret = fwnode_graph_parse_endpoint(fwnode, &ep);
+			if (ret) {
+				fwnode_handle_put(fwnode);
+				return ret;
+			}
+
+			if (ep.port != port)
+				continue;
+		}
+		max_subdevs++;
+	}
+
+	/* No subdevs to add? Return here. */
+	if (max_subdevs == notifier->max_subdevs)
+		return 0;
+
+	ret = v4l2_async_notifier_realloc(notifier, max_subdevs);
+	if (ret)
+		return ret;
+
+	for (fwnode = NULL; (fwnode = fwnode_graph_get_next_endpoint(
+				     dev_fwnode(dev), fwnode)); ) {
+		struct fwnode_handle *dev_fwnode;
+		bool is_available;
+
+		dev_fwnode = fwnode_graph_get_port_parent(fwnode);
+		is_available = fwnode_device_is_available(dev_fwnode);
+		fwnode_handle_put(dev_fwnode);
+
+		if (!fwnode_device_is_available(dev_fwnode))
+			continue;
+
+		if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (has_port) {
+			struct fwnode_endpoint ep;
+
+			ret = fwnode_graph_parse_endpoint(fwnode, &ep);
+			if (ret)
+				break;
+
+			if (ep.port != port)
+				continue;
+		}
+
+		ret = v4l2_async_notifier_fwnode_parse_endpoint(
+			dev, notifier, fwnode, asd_struct_size, parse_endpoint);
+		if (ret < 0)
+			break;
+	}
+
+	fwnode_handle_put(fwnode);
+
+	return ret;
+}
+
+int v4l2_async_notifier_parse_fwnode_endpoints(
+	struct device *dev, struct v4l2_async_notifier *notifier,
+	size_t asd_struct_size,
+	int (*parse_endpoint)(struct device *dev,
+			    struct v4l2_fwnode_endpoint *vep,
+			    struct v4l2_async_subdev *asd))
+{
+	return __v4l2_async_notifier_parse_fwnode_endpoints(
+		dev, notifier, asd_struct_size, 0, false, parse_endpoint);
+}
+EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints);
+
+int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
+	struct device *dev, struct v4l2_async_notifier *notifier,
+	size_t asd_struct_size, unsigned int port,
+	int (*parse_endpoint)(struct device *dev,
+			    struct v4l2_fwnode_endpoint *vep,
+			    struct v4l2_async_subdev *asd))
+{
+	return __v4l2_async_notifier_parse_fwnode_endpoints(
+		dev, notifier, asd_struct_size, port, true, parse_endpoint);
+}
+EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Sakari Ailus <[email protected]>");
 MODULE_AUTHOR("Sylwester Nawrocki <[email protected]>");

include/media/v4l2-async.h +22 -2

index c69d8c8a66d0..329aeebd1a80 100644
@@ -18,7 +18,6 @@ struct device;
 struct device_node;
 struct v4l2_device;
 struct v4l2_subdev;
-struct v4l2_async_notifier;
 
 /* A random max subdevice number, used to allocate an array on stack */
 #define V4L2_MAX_SUBDEVS 128U

@@ -50,6 +49,10 @@ enum v4l2_async_match_type {
  * @match:	union of per-bus type matching data sets
  * @list:	used to link struct v4l2_async_subdev objects, waiting to be
  *		probed, to a notifier->waiting list
+ *
+ * When this struct is used as a member in a driver specific struct,
+ * the driver specific struct shall contain the &struct
+ * v4l2_async_subdev as its first member.
  */
 struct v4l2_async_subdev {
 	enum v4l2_async_match_type match_type;

@@ -78,7 +81,8 @@ struct v4l2_async_subdev {
 /**
  * struct v4l2_async_notifier - v4l2_device notifier data
  *
- * @num_subdevs: number of subdevices
+ * @num_subdevs: number of subdevices used in the subdevs array
+ * @max_subdevs: number of subdevices allocated in the subdevs array
  * @subdevs:	array of pointers to subdevice descriptors
  * @v4l2_dev:	pointer to struct v4l2_device
  * @waiting:	list of struct v4l2_async_subdev, waiting for their drivers

@@ -90,6 +94,7 @@ struct v4l2_async_subdev {
  */
 struct v4l2_async_notifier {
 	unsigned int num_subdevs;
+	unsigned int max_subdevs;
 	struct v4l2_async_subdev **subdevs;
 	struct v4l2_device *v4l2_dev;
 	struct list_head waiting;

@@ -121,6 +126,21 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
 void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
 
 /**
+ * v4l2_async_notifier_cleanup - clean up notifier resources
+ * @notifier: the notifier the resources of which are to be cleaned up
+ *
+ * Release memory resources related to a notifier, including the async
+ * sub-devices allocated for the purposes of the notifier but not the notifier
+ * itself. The user is responsible for calling this function to clean up the
+ * notifier after calling @v4l2_async_notifier_parse_fwnode_endpoints.
+ *
+ * There is no harm from calling v4l2_async_notifier_cleanup in other
+ * cases as long as its memory has been zeroed after it has been
+ * allocated.
+ */
+void v4l2_async_notifier_cleanup(struct v4l2_async_notifier *notifier);
+
+/**
  * v4l2_async_register_subdev - registers a sub-device to the asynchronous
  * 	subdevice framework
  *

include/media/v4l2-fwnode.h +118 -0

index 7adec9851d9e..ac605af9b877 100644
@@ -25,6 +25,8 @@
 #include <media/v4l2-mediabus.h>
 
 struct fwnode_handle;
+struct v4l2_async_notifier;
+struct v4l2_async_subdev;
 
 #define V4L2_FWNODE_CSI2_MAX_DATA_LANES	4
 

@@ -122,4 +124,120 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,
 			   struct v4l2_fwnode_link *link);
 void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
 
+/**
+ * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints in a
+ *						device node
+ * @dev: the device the endpoints of which are to be parsed
+ * @notifier: notifier for @dev
+ * @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.
+ * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
+ *		    endpoint. Optional.
+ *		    Return: %0 on success
+ *			    %-ENOTCONN if the endpoint is to be skipped but this
+ *				       should not be considered as an error
+ *			    %-EINVAL if the endpoint configuration is invalid
+ *
+ * Parse the fwnode endpoints of the @dev device and populate the async sub-
+ * devices array of the notifier. The @parse_endpoint callback function is
+ * called for each endpoint with the corresponding async sub-device pointer to
+ * let the caller initialize the driver-specific part of the async sub-device
+ * structure.
+ *
+ * The notifier memory shall be zeroed before this function is called on the
+ * notifier.
+ *
+ * This function may not be called on a registered notifier and may be called on
+ * a notifier only once.
+ *
+ * Do not change the notifier's subdevs array, take references to the subdevs
+ * array itself or change the notifier's num_subdevs field. This is because this
+ * function allocates and reallocates the subdevs array based on parsing
+ * endpoints.
+ *
+ * The &struct v4l2_fwnode_endpoint passed to the callback function
+ * @parse_endpoint is released once the function is finished. If there is a need
+ * to retain that configuration, the user needs to allocate memory for it.
+ *
+ * Any notifier populated using this function must be released with a call to
+ * v4l2_async_notifier_cleanup() after it has been unregistered and the async
+ * sub-devices are no longer in use, even if the function returned an error.
+ *
+ * Return: %0 on success, including when no async sub-devices are found
+ *	   %-ENOMEM if memory allocation failed
+ *	   %-EINVAL if graph or endpoint parsing failed
+ *	   Other error codes as returned by @parse_endpoint
+ */
+int v4l2_async_notifier_parse_fwnode_endpoints(
+	struct device *dev, struct v4l2_async_notifier *notifier,
+	size_t asd_struct_size,
+	int (*parse_endpoint)(struct device *dev,
+			      struct v4l2_fwnode_endpoint *vep,
+			      struct v4l2_async_subdev *asd));
+
+/**
+ * v4l2_async_notifier_parse_fwnode_endpoints_by_port - Parse V4L2 fwnode
+ *							endpoints of a port in a
+ *							device node
+ * @dev: the device the endpoints of which are to be parsed
+ * @notifier: notifier for @dev
+ * @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.
+ * @port: port number where endpoints are to be parsed
+ * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
+ *		    endpoint. Optional.
+ *		    Return: %0 on success
+ *			    %-ENOTCONN if the endpoint is to be skipped but this
+ *				       should not be considered as an error
+ *			    %-EINVAL if the endpoint configuration is invalid
+ *
+ * This function is just like v4l2_async_notifier_parse_fwnode_endpoints() with
+ * the exception that it only parses endpoints in a given port. This is useful
+ * on devices that have both sinks and sources: the async sub-devices connected
+ * to sources have already been configured by another driver (on capture
+ * devices). In this case the driver must know which ports to parse.
+ *
+ * Parse the fwnode endpoints of the @dev device on a given @port and populate
+ * the async sub-devices array of the notifier. The @parse_endpoint callback
+ * function is called for each endpoint with the corresponding async sub-device
+ * pointer to let the caller initialize the driver-specific part of the async
+ * sub-device structure.
+ *
+ * The notifier memory shall be zeroed before this function is called on the
+ * notifier the first time.
+ *
+ * This function may not be called on a registered notifier and may be called on
+ * a notifier only once per port.
+ *
+ * Do not change the notifier's subdevs array, take references to the subdevs
+ * array itself or change the notifier's num_subdevs field. This is because this
+ * function allocates and reallocates the subdevs array based on parsing
+ * endpoints.
+ *
+ * The &struct v4l2_fwnode_endpoint passed to the callback function
+ * @parse_endpoint is released once the function is finished. If there is a need
+ * to retain that configuration, the user needs to allocate memory for it.
+ *
+ * Any notifier populated using this function must be released with a call to
+ * v4l2_async_notifier_cleanup() after it has been unregistered and the async
+ * sub-devices are no longer in use, even if the function returned an error.
+ *
+ * Return: %0 on success, including when no async sub-devices are found
+ *	   %-ENOMEM if memory allocation failed
+ *	   %-EINVAL if graph or endpoint parsing failed
+ *	   Other error codes as returned by @parse_endpoint
+ */
+int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
+	struct device *dev, struct v4l2_async_notifier *notifier,
+	size_t asd_struct_size, unsigned int port,
+	int (*parse_endpoint)(struct device *dev,
+			      struct v4l2_fwnode_endpoint *vep,
+			      struct v4l2_async_subdev *asd));
+
 #endif /* _V4L2_FWNODE_H */

Signature · 1 line

2.11.0

Instead of using a custom driver implementation, use v4l2_async_notifier_parse_fwnode_endpoints() to parse the fwnode endpoints of the device.

Signed-off-by Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/[email protected]>

Acked-by Hans Verkuil <[email protected]>

Reviewed-by Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/[email protected]>

---

 drivers/media/platform/omap3isp/isp.c | 121 +++++++++++-----------------------
 drivers/media/platform/omap3isp/isp.h |   5 +-
 2 files changed, 40 insertions(+), 86 deletions(-)

drivers/media/platform/omap3isp/isp.c +39 -82

index 1a428fe9f070..97a5206b6ddc 100644
@@ -2001,6 +2001,7 @@ static int isp_remove(struct platform_device *pdev)
 	__omap3isp_put(isp, false);
 
 	media_entity_enum_cleanup(&isp->crashed);
+	v4l2_async_notifier_cleanup(&isp->notifier);
 
 	return 0;
 }

@@ -2011,44 +2012,41 @@ enum isp_of_phy {
 	ISP_OF_PHY_CSIPHY2,
 };
 
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
-			    struct isp_async_subdev *isd)
+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;
-	struct v4l2_fwnode_endpoint vep;
-	unsigned int i;
-	int ret;
 	bool csi1 = false;
-
-	ret = v4l2_fwnode_endpoint_parse(fwnode, &vep);
-	if (ret)
-		return ret;
+	unsigned int i;
 
 	dev_dbg(dev, "parsing endpoint %pOF, interface %u\n",
-		to_of_node(fwnode), vep.base.port);
+		to_of_node(vep->base.local_fwnode), vep->base.port);
 
-	switch (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;
+			vep->bus.parallel.data_shift;
 		buscfg->bus.parallel.clk_pol =
-			!!(vep.bus.parallel.flags
+			!!(vep->bus.parallel.flags
 			   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
 		buscfg->bus.parallel.hs_pol =
-			!!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+			!!(vep->bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
 		buscfg->bus.parallel.vs_pol =
-			!!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+			!!(vep->bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
 		buscfg->bus.parallel.fld_pol =
-			!!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+			!!(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;
+			!!(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) {
+		switch (vep->bus_type) {
 		case V4L2_MBUS_CCP2:
 		case V4L2_MBUS_CSI1:
 			dev_dbg(dev, "CSI-1/CCP-2 configuration\n");

@@ -2060,11 +2058,11 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
 			break;
 		default:
 			dev_err(dev, "unsupported bus type %u\n",
-				vep.bus_type);
+				vep->bus_type);
 			return -EINVAL;
 		}
 
-		switch (vep.base.port) {
+		switch (vep->base.port) {
 		case ISP_OF_PHY_CSIPHY1:
 			if (csi1)
 				buscfg->interface = ISP_INTERFACE_CCP2B_PHY1;

@@ -2080,47 +2078,47 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
 		}
 		if (csi1) {
 			buscfg->bus.ccp2.lanecfg.clk.pos =
-				vep.bus.mipi_csi1.clock_lane;
+				vep->bus.mipi_csi1.clock_lane;
 			buscfg->bus.ccp2.lanecfg.clk.pol =
-				vep.bus.mipi_csi1.lane_polarity[0];
+				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;
+				vep->bus.mipi_csi1.data_lane;
 			buscfg->bus.ccp2.lanecfg.data[0].pol =
-				vep.bus.mipi_csi1.lane_polarity[1];
+				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;
+				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;
+				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;
+				vep->bus.mipi_csi2.clock_lane;
 			buscfg->bus.csi2.lanecfg.clk.pol =
-				vep.bus.mipi_csi2.lane_polarities[0];
+				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;
+				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];
+					vep->bus.mipi_csi2.data_lanes[i];
 				buscfg->bus.csi2.lanecfg.data[i].pol =
-					vep.bus.mipi_csi2.lane_polarities[i + 1];
+					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,

@@ -2137,57 +2135,13 @@ static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
 
 	default:
 		dev_warn(dev, "%pOF: invalid interface %u\n",
-			 to_of_node(fwnode), vep.base.port);
+			 to_of_node(vep->base.local_fwnode), vep->base.port);
 		return -EINVAL;
 	}
 
 	return 0;
 }
 
-static int isp_fwnodes_parse(struct device *dev,
-			     struct v4l2_async_notifier *notifier)
-{
-	struct fwnode_handle *fwnode = NULL;
-
-	notifier->subdevs = devm_kcalloc(
-		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
-	if (!notifier->subdevs)
-		return -ENOMEM;
-
-	while (notifier->num_subdevs < ISP_MAX_SUBDEVS &&
-	       (fwnode = fwnode_graph_get_next_endpoint(
-			of_fwnode_handle(dev->of_node), fwnode))) {
-		struct isp_async_subdev *isd;
-
-		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
-		if (!isd)
-			goto error;
-
-		if (isp_fwnode_parse(dev, fwnode, isd)) {
-			devm_kfree(dev, isd);
-			continue;
-		}
-
-		notifier->subdevs[notifier->num_subdevs] = &isd->asd;
-
-		isd->asd.match.fwnode.fwnode =
-			fwnode_graph_get_remote_port_parent(fwnode);
-		if (!isd->asd.match.fwnode.fwnode) {
-			dev_warn(dev, "bad remote port parent\n");
-			goto error;
-		}
-
-		isd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
-		notifier->num_subdevs++;
-	}
-
-	return notifier->num_subdevs;
-
-error:
-	fwnode_handle_put(fwnode);
-	return -EINVAL;
-}
-
 static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
 {
 	struct isp_device *isp = container_of(async, struct isp_device,

@@ -2256,15 +2210,17 @@ static int isp_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = isp_fwnodes_parse(&pdev->dev, &isp->notifier);
-	if (ret < 0)
-		return ret;
-
 	isp->autoidle = autoidle;
 
 	mutex_init(&isp->isp_mutex);
 	spin_lock_init(&isp->stat_lock);
 
+	ret = v4l2_async_notifier_parse_fwnode_endpoints(
+		&pdev->dev, &isp->notifier, sizeof(struct isp_async_subdev),
+		isp_fwnode_parse);
+	if (ret < 0)
+		goto error;
+
 	isp->dev = &pdev->dev;
 	isp->ref_count = 0;
 

@@ -2406,6 +2362,7 @@ static int isp_probe(struct platform_device *pdev)
 	isp_xclk_cleanup(isp);
 	__omap3isp_put(isp, false);
 error:
+	v4l2_async_notifier_cleanup(&isp->notifier);
 	mutex_destroy(&isp->isp_mutex);
 
 	return ret;

drivers/media/platform/omap3isp/isp.h +1 -4

index e528df6efc09..8b9043db94b3 100644
@@ -220,14 +220,11 @@ struct isp_device {
 
 	unsigned int sbl_resources;
 	unsigned int subclk_resources;
-
-#define ISP_MAX_SUBDEVS		8
-	struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
 };
 
 struct isp_async_subdev {
-	struct isp_bus_cfg bus;
 	struct v4l2_async_subdev asd;
+	struct isp_bus_cfg bus;
 };
 
 #define v4l2_subdev_to_bus_cfg(sd) \

Signature · 1 line

2.11.0

We only want to link sub-devices that were bound to the async notifier the isp driver registered but there may be other sub-devices in the v4l2_device as well. Check for the correct async notifier.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Acked-by Pavel Machek <[email protected]>

---

 drivers/media/platform/omap3isp/isp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/media/platform/omap3isp/isp.c +1 -1

index 97a5206b6ddc..4afd7ba4fad6 100644
@@ -2155,7 +2155,7 @@ static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
 		return ret;
 
 	list_for_each_entry(sd, &v4l2_dev->subdevs, list) {
-		if (!sd->asd)
+		if (sd->notifier != &isp->notifier)
 			continue;
 
 		ret = isp_link_entity(isp, &sd->entity,

Signature · 1 line

2.11.0

If no source pads are found in an entity, print the name of the entity.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Acked-by Pavel Machek <[email protected]>

---

 drivers/media/platform/omap3isp/isp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/media/platform/omap3isp/isp.c +2 -2

index 4afd7ba4fad6..35687c9707e0 100644
@@ -1669,8 +1669,8 @@ static int isp_link_entity(
 			break;
 	}
 	if (i == entity->num_pads) {
-		dev_err(isp->dev, "%s: no source pad in external entity\n",
-			__func__);
+		dev_err(isp->dev, "%s: no source pad in external entity %s\n",
+			__func__, entity->name);
 		return -EINVAL;
 	}
 

Signature · 1 line

2.11.0