Re: [PATCH v16 26/32] v4l: fwnode: Add a convenience function for registering sensors

[Sebastian Reichel]

Hi,

On Thu, Oct 26, 2017 at 10:53:36AM +0300, Sakari Ailus wrote:
> 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 
> Acked-by: Hans Verkuil 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  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(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index b4e88eef195f..e81a72b8d46e 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -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(_lock);
> -
>   v4l2_async_notifier_unbind_all_subdevs(notifier);
>  
>   notifier->sd = NULL;
>   notifier->v4l2_dev = NULL;
>  
>   list_del(>list);
> +}
> +
> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> +{
> + mutex_lock(_lock);
> +
> + __v4l2_async_notifier_unregister(notifier);
>  
>   mutex_unlock(_lock);
>  }
> @@ -596,6 +602,11 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
>  {
>   mutex_lock(_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;
>  
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 1234bd1a2f49..82af608fd626 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -29,6 +29,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  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 ");
>  MODULE_AUTHOR("Sylwester Nawrocki ");
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 8d8cfc3f3100..6152434cbe82 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -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 _subdev
> + *
> + * This function is just like 

Re: [PATCH v16 26/32] v4l: fwnode: Add a convenience function for registering sensors

[Niklas Söderlund]

On 2017-10-26 10:53:36 +0300, Sakari Ailus wrote:
> 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 
> Acked-by: Hans Verkuil 

Reviewed-by: Niklas Söderlund 

> ---
>  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(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index b4e88eef195f..e81a72b8d46e 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -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(_lock);
> -
>   v4l2_async_notifier_unbind_all_subdevs(notifier);
>  
>   notifier->sd = NULL;
>   notifier->v4l2_dev = NULL;
>  
>   list_del(>list);
> +}
> +
> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> +{
> + mutex_lock(_lock);
> +
> + __v4l2_async_notifier_unregister(notifier);
>  
>   mutex_unlock(_lock);
>  }
> @@ -596,6 +602,11 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
>  {
>   mutex_lock(_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;
>  
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 1234bd1a2f49..82af608fd626 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -29,6 +29,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  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 ");
>  MODULE_AUTHOR("Sylwester Nawrocki ");
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 8d8cfc3f3100..6152434cbe82 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -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 _subdev
> + *
> + * This function is just like v4l2_async_register_subdev() with the exception
> + 

[PATCH v16 26/32] v4l: fwnode: Add a convenience function for registering sensors

[Sakari Ailus]

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 
Acked-by: Hans Verkuil 
---
 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(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index b4e88eef195f..e81a72b8d46e 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -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(_lock);
-
v4l2_async_notifier_unbind_all_subdevs(notifier);
 
notifier->sd = NULL;
notifier->v4l2_dev = NULL;
 
list_del(>list);
+}
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+   mutex_lock(_lock);
+
+   __v4l2_async_notifier_unregister(notifier);
 
mutex_unlock(_lock);
 }
@@ -596,6 +602,11 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
 {
mutex_lock(_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;
 
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
b/drivers/media/v4l2-core/v4l2-fwnode.c
index 1234bd1a2f49..82af608fd626 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -29,6 +29,7 @@
 
 #include 
 #include 
+#include 
 
 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 ");
 MODULE_AUTHOR("Sylwester Nawrocki ");
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 8d8cfc3f3100..6152434cbe82 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -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 _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