Re: [PATCH v16.1 18/32] v4l: async: Allow binding notifiers to sub-devices

2017-10-29 Thread Sebastian Reichel 
Hi,

On Fri, Oct 27, 2017 at 11:27:09AM +0300, Sakari Ailus wrote:
> Registering a notifier has required the knowledge of struct v4l2_device
> for the reason that sub-devices generally are registered to the
> v4l2_device (as well as the media device, also available through
> v4l2_device).
> 
> This information is not available for sub-device drivers at probe time.
> 
> What this patch does is that it allows registering notifiers without
> having v4l2_device around. Instead the sub-device pointer is stored in the
> notifier. Once the sub-device of the driver that registered the notifier
> is registered, the notifier will gain the knowledge of the v4l2_device,
> and the binding of async sub-devices from the sub-device driver's notifier
> may proceed.
> 
> The complete callback of the root notifier will be called only when the
> v4l2_device is available and no notifier has pending sub-devices to bind.
> No complete callbacks are supported for sub-device notifiers.
> 
> Signed-off-by: Sakari Ailus 
> Acked-by: Hans Verkuil 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/media/v4l2-core/v4l2-async.c | 212 
> ---
>  include/media/v4l2-async.h   |  19 +++-
>  2 files changed, 189 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 6265717769d2..ed539c4fd5dc 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -124,11 +124,87 @@ 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, _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)
> +{
> + while (notifier->parent)
> + notifier = notifier->parent;
> +
> + return notifier->v4l2_dev;
> +}
> +
> +/*
> + * Return true if all child sub-device notifiers are complete, false 
> otherwise.
> + */
> +static bool v4l2_async_notifier_can_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd;
> +
> + if (!list_empty(>waiting))
> + return false;
> +
> + list_for_each_entry(sd, >done, async_list) {
> + struct v4l2_async_notifier *subdev_notifier =
> + v4l2_async_find_subdev_notifier(sd);
> +
> + if (subdev_notifier &&
> + !v4l2_async_notifier_can_complete(subdev_notifier))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/*
> + * Complete the master notifier if possible. This is done when all async
> + * sub-devices have been bound; v4l2_device is also available then.
> + */
> +static int v4l2_async_notifier_try_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + /* Quick check whether there are still more sub-devices here. */
> + if (!list_empty(>waiting))
> + return 0;
> +
> + /* Check the entire notifier tree; find the root notifier first. */
> + while (notifier->parent)
> + notifier = notifier->parent;
> +
> + /* This is root if it has v4l2_dev. */
> + if (!notifier->v4l2_dev)
> + return 0;
> +
> + /* Is everything ready? */
> + if (!v4l2_async_notifier_can_complete(notifier))
> + return 0;
> +
> + return v4l2_async_notifier_call_complete(notifier);
> +}
> +
> +static int v4l2_async_notifier_try_all_subdevs(
> + struct v4l2_async_notifier *notifier);
> +
>  static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
>  struct v4l2_device *v4l2_dev,
>  struct v4l2_subdev *sd,
>  struct v4l2_async_subdev *asd)
>  {
> + struct v4l2_async_notifier *subdev_notifier;
>   int ret;
>  
>   ret = v4l2_device_register_subdev(v4l2_dev, sd);
> @@ -149,17 +225,36 @@ static int v4l2_async_match_notify(struct 
> v4l2_async_notifier *notifier,
>   /* Move from the global subdevice list to notifier's done */
>   list_move(>async_list, >done);
>  
> - return 0;
> + /*
> +  * See if the sub-device has a notifier. If not, return here.
> +  */
> + subdev_notifier = v4l2_async_find_subdev_notifier(sd);
> + if (!subdev_notifier || subdev_notifier->parent)
> + return 0;
> +
> + /*
> +  * Proceed with checking for the sub-device notifier's async
> +  *

Re: [PATCH v16.1 18/32] v4l: async: Allow binding notifiers to sub-devices

2017-10-27 Thread Niklas Söderlund 
On 2017-10-27 11:27:09 +0300, Sakari Ailus wrote:
> Registering a notifier has required the knowledge of struct v4l2_device
> for the reason that sub-devices generally are registered to the
> v4l2_device (as well as the media device, also available through
> v4l2_device).
> 
> This information is not available for sub-device drivers at probe time.
> 
> What this patch does is that it allows registering notifiers without
> having v4l2_device around. Instead the sub-device pointer is stored in the
> notifier. Once the sub-device of the driver that registered the notifier
> is registered, the notifier will gain the knowledge of the v4l2_device,
> and the binding of async sub-devices from the sub-device driver's notifier
> may proceed.
> 
> The complete callback of the root notifier will be called only when the
> v4l2_device is available and no notifier has pending sub-devices to bind.
> No complete callbacks are supported for sub-device notifiers.
> 
> Signed-off-by: Sakari Ailus 
> Acked-by: Hans Verkuil 

Acked-by: Niklas Söderlund 

> ---
>  drivers/media/v4l2-core/v4l2-async.c | 212 
> ---
>  include/media/v4l2-async.h   |  19 +++-
>  2 files changed, 189 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 6265717769d2..ed539c4fd5dc 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -124,11 +124,87 @@ 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, _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)
> +{
> + while (notifier->parent)
> + notifier = notifier->parent;
> +
> + return notifier->v4l2_dev;
> +}
> +
> +/*
> + * Return true if all child sub-device notifiers are complete, false 
> otherwise.
> + */
> +static bool v4l2_async_notifier_can_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd;
> +
> + if (!list_empty(>waiting))
> + return false;
> +
> + list_for_each_entry(sd, >done, async_list) {
> + struct v4l2_async_notifier *subdev_notifier =
> + v4l2_async_find_subdev_notifier(sd);
> +
> + if (subdev_notifier &&
> + !v4l2_async_notifier_can_complete(subdev_notifier))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/*
> + * Complete the master notifier if possible. This is done when all async
> + * sub-devices have been bound; v4l2_device is also available then.
> + */
> +static int v4l2_async_notifier_try_complete(
> + struct v4l2_async_notifier *notifier)
> +{
> + /* Quick check whether there are still more sub-devices here. */
> + if (!list_empty(>waiting))
> + return 0;
> +
> + /* Check the entire notifier tree; find the root notifier first. */
> + while (notifier->parent)
> + notifier = notifier->parent;
> +
> + /* This is root if it has v4l2_dev. */
> + if (!notifier->v4l2_dev)
> + return 0;
> +
> + /* Is everything ready? */
> + if (!v4l2_async_notifier_can_complete(notifier))
> + return 0;
> +
> + return v4l2_async_notifier_call_complete(notifier);
> +}
> +
> +static int v4l2_async_notifier_try_all_subdevs(
> + struct v4l2_async_notifier *notifier);
> +
>  static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
>  struct v4l2_device *v4l2_dev,
>  struct v4l2_subdev *sd,
>  struct v4l2_async_subdev *asd)
>  {
> + struct v4l2_async_notifier *subdev_notifier;
>   int ret;
>  
>   ret = v4l2_device_register_subdev(v4l2_dev, sd);
> @@ -149,17 +225,36 @@ static int v4l2_async_match_notify(struct 
> v4l2_async_notifier *notifier,
>   /* Move from the global subdevice list to notifier's done */
>   list_move(>async_list, >done);
>  
> - return 0;
> + /*
> +  * See if the sub-device has a notifier. If not, return here.
> +  */
> + subdev_notifier = v4l2_async_find_subdev_notifier(sd);
> + if (!subdev_notifier || subdev_notifier->parent)
> + return 0;
> +
> + /*
> +  * Proceed with checking for the sub-device notifier's async
> +  * sub-devices, and return the result.

[PATCH v16.1 18/32] v4l: async: Allow binding notifiers to sub-devices

2017-10-27 Thread Sakari Ailus 
Registering a notifier has required the knowledge of struct v4l2_device
for the reason that sub-devices generally are registered to the
v4l2_device (as well as the media device, also available through
v4l2_device).

This information is not available for sub-device drivers at probe time.

What this patch does is that it allows registering notifiers without
having v4l2_device around. Instead the sub-device pointer is stored in the
notifier. Once the sub-device of the driver that registered the notifier
is registered, the notifier will gain the knowledge of the v4l2_device,
and the binding of async sub-devices from the sub-device driver's notifier
may proceed.

The complete callback of the root notifier will be called only when the
v4l2_device is available and no notifier has pending sub-devices to bind.
No complete callbacks are supported for sub-device notifiers.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-async.c | 212 ---
 include/media/v4l2-async.h   |  19 +++-
 2 files changed, 189 insertions(+), 42 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 6265717769d2..ed539c4fd5dc 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -124,11 +124,87 @@ 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, _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)
+{
+   while (notifier->parent)
+   notifier = notifier->parent;
+
+   return notifier->v4l2_dev;
+}
+
+/*
+ * Return true if all child sub-device notifiers are complete, false otherwise.
+ */
+static bool v4l2_async_notifier_can_complete(
+   struct v4l2_async_notifier *notifier)
+{
+   struct v4l2_subdev *sd;
+
+   if (!list_empty(>waiting))
+   return false;
+
+   list_for_each_entry(sd, >done, async_list) {
+   struct v4l2_async_notifier *subdev_notifier =
+   v4l2_async_find_subdev_notifier(sd);
+
+   if (subdev_notifier &&
+   !v4l2_async_notifier_can_complete(subdev_notifier))
+   return false;
+   }
+
+   return true;
+}
+
+/*
+ * Complete the master notifier if possible. This is done when all async
+ * sub-devices have been bound; v4l2_device is also available then.
+ */
+static int v4l2_async_notifier_try_complete(
+   struct v4l2_async_notifier *notifier)
+{
+   /* Quick check whether there are still more sub-devices here. */
+   if (!list_empty(>waiting))
+   return 0;
+
+   /* Check the entire notifier tree; find the root notifier first. */
+   while (notifier->parent)
+   notifier = notifier->parent;
+
+   /* This is root if it has v4l2_dev. */
+   if (!notifier->v4l2_dev)
+   return 0;
+
+   /* Is everything ready? */
+   if (!v4l2_async_notifier_can_complete(notifier))
+   return 0;
+
+   return v4l2_async_notifier_call_complete(notifier);
+}
+
+static int v4l2_async_notifier_try_all_subdevs(
+   struct v4l2_async_notifier *notifier);
+
 static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
   struct v4l2_device *v4l2_dev,
   struct v4l2_subdev *sd,
   struct v4l2_async_subdev *asd)
 {
+   struct v4l2_async_notifier *subdev_notifier;
int ret;
 
ret = v4l2_device_register_subdev(v4l2_dev, sd);
@@ -149,17 +225,36 @@ static int v4l2_async_match_notify(struct 
v4l2_async_notifier *notifier,
/* Move from the global subdevice list to notifier's done */
list_move(>async_list, >done);
 
-   return 0;
+   /*
+* See if the sub-device has a notifier. If not, return here.
+*/
+   subdev_notifier = v4l2_async_find_subdev_notifier(sd);
+   if (!subdev_notifier || subdev_notifier->parent)
+   return 0;
+
+   /*
+* Proceed with checking for the sub-device notifier's async
+* sub-devices, and return the result. The error will be handled by the
+* caller.
+*/
+   subdev_notifier->parent = notifier;
+
+   return v4l2_async_notifier_try_all_subdevs(subdev_notifier);
 }
 
 /* Test all async sub-devices in a notifier for a match. */
 static int