From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans Verkuil Subject: Re: [PATCH v8 14/21] v4l: async: Allow binding notifiers to sub-devices Date: Wed, 6 Sep 2017 10:46:31 +0200 Message-ID: <910b3d01-a8a4-2363-4b24-ed0edd1e1f4d@xs4all.nl> References: <20170905130553.1332-1-sakari.ailus@linux.intel.com> <20170905130553.1332-15-sakari.ailus@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170905130553.1332-15-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> Content-Language: en-US Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Sakari Ailus , linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: niklas.soderlund-1zkq55x86MTxsAP9Fp7wbw@public.gmane.org, robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, pavel-+ZI9xUNit7I@public.gmane.org, sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org List-Id: devicetree@vger.kernel.org On 09/05/2017 03:05 PM, 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 to the to -> in > 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 master notifier's complete callback is only called when all sub-device > notifiers are completed. > > Signed-off-by: Sakari Ailus > --- > drivers/media/v4l2-core/v4l2-async.c | 209 ++++++++++++++++++++++++++++++----- > include/media/v4l2-async.h | 16 ++- > 2 files changed, 194 insertions(+), 31 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index 79f216723a3f..620b2cd29fc3 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n) > return n->ops->complete(n); > } > > +static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, > + struct v4l2_subdev *sd, > + struct v4l2_async_subdev *asd); > + > static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) > { > #if IS_ENABLED(CONFIG_I2C) > @@ -129,14 +133,119 @@ static struct v4l2_async_subdev *v4l2_async_find_match( > return NULL; > } > > +/* Get the sub-device notifier registered by a sub-device driver. */ > +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier( > + struct v4l2_subdev *sd) > +{ > + struct v4l2_async_notifier *n; > + > + list_for_each_entry(n, ¬ifier_list, list) > + if (n->sd == sd) > + return n; > + > + return NULL; > +} > + > +/* Return true if all sub-device notifiers are complete, false otherwise. */ > +static bool v4l2_async_subdev_notifiers_complete( > + struct v4l2_async_notifier *notifier) > +{ > + struct v4l2_subdev *sd; > + > + if (!list_empty(¬ifier->waiting)) > + return false; > + > + list_for_each_entry(sd, ¬ifier->done, async_list) { > + struct v4l2_async_notifier *subdev_notifier = > + v4l2_async_get_subdev_notifier(sd); > + > + if (!subdev_notifier) > + continue; > + > + if (!v4l2_async_subdev_notifiers_complete(subdev_notifier)) > + return false; > + } > + > + return true; > +} > + > +/* Get v4l2_device related to the notifier if one can be found. */ > +static struct v4l2_device *v4l2_async_notifier_get_v4l2_dev( > + struct v4l2_async_notifier *notifier) > +{ > + while (notifier->master) > + notifier = notifier->master; > + > + return notifier->v4l2_dev; > +} > + > +/* Test all async sub-devices in a notifier for a match. */ > +static int v4l2_async_notifier_try_all_subdevs( > + struct v4l2_async_notifier *notifier) > +{ > + struct v4l2_subdev *sd, *tmp; > + > + if (!v4l2_async_notifier_get_v4l2_dev(notifier)) > + return 0; > + > + list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { > + struct v4l2_async_subdev *asd; > + int ret; > + > + asd = v4l2_async_find_match(notifier, sd); > + if (!asd) > + continue; > + > + ret = v4l2_async_match_notify(notifier, sd, asd); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +/* Try completing a notifier. */ > +static int v4l2_async_notifier_try_complete( > + struct v4l2_async_notifier *notifier) > +{ > + do { > + int ret; > + > + /* Any local async sub-devices left? */ > + if (!list_empty(¬ifier->waiting)) > + return 0; > + > + /* > + * Any sub-device notifiers waiting for async subdevs > + * to be bound? > + */ > + if (!v4l2_async_subdev_notifiers_complete(notifier)) > + return 0; > + > + /* Proceed completing the notifier */ > + ret = v4l2_async_notifier_call_complete(notifier); > + if (ret < 0) > + return ret; > + > + /* > + * Obtain notifier's master. If there is one, repeat > + * the process, otherwise we're done here. > + */ > + } while ((notifier = notifier->master)); I'd change this to: notifier = notifier->master; } while (notifier); > + > + return 0; > +} > + > static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, > struct v4l2_subdev *sd, > struct v4l2_async_subdev *asd) > { > + struct v4l2_async_notifier *subdev_notifier; > int ret; > > - ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd); > - if (ret < 0) > + ret = v4l2_device_register_subdev( > + v4l2_async_notifier_get_v4l2_dev(notifier), sd); > + if (ret) > return ret; > > ret = v4l2_async_notifier_call_bound(notifier, sd, asd); > @@ -153,10 +262,20 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, > /* Move from the global subdevice list to notifier's done */ > list_move(&sd->async_list, ¬ifier->done); > > - if (list_empty(¬ifier->waiting)) > - return v4l2_async_notifier_call_complete(notifier); > + /* > + * See if the sub-device has a notifier. If it does, proceed > + * with checking for its async sub-devices. > + */ > + subdev_notifier = v4l2_async_get_subdev_notifier(sd); > + if (subdev_notifier && !subdev_notifier->master) { > + subdev_notifier->master = notifier; > + ret = v4l2_async_notifier_try_all_subdevs(subdev_notifier); > + if (ret) > + return ret; > + } > > - return 0; > + /* Try completing the notifier and its master(s). */ > + return v4l2_async_notifier_try_complete(notifier); > } > > static void v4l2_async_cleanup(struct v4l2_subdev *sd) > @@ -168,18 +287,17 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd) > sd->dev = NULL; > } > > -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > - struct v4l2_async_notifier *notifier) > +static int __v4l2_async_notifier_register(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 || > + if (!notifier->v4l2_dev == !notifier->sd || !notifier->num_subdevs || With the changes suggested below this can be changed to: if (!notifier->num_subdevs || However, I have a question about that: why would it be wrong to call this with no subdevs in the list? It's perfectly valid to have no subdevs at all. There may be a fixed incoming video stream that is not controlled by a subdev. We have a case like that in fact. > notifier->num_subdevs > V4L2_MAX_SUBDEVS) > return -EINVAL; > > - notifier->v4l2_dev = v4l2_dev; > + INIT_LIST_HEAD(¬ifier->list); > INIT_LIST_HEAD(¬ifier->waiting); > INIT_LIST_HEAD(¬ifier->done); > > @@ -203,18 +321,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > > mutex_lock(&list_lock); > > - list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { > - int ret; > - > - asd = v4l2_async_find_match(notifier, sd); > - if (!asd) > - continue; > - > - ret = v4l2_async_match_notify(notifier, sd, asd); > - if (ret < 0) { > - mutex_unlock(&list_lock); > - return ret; > - } > + ret = v4l2_async_notifier_try_all_subdevs(notifier); > + if (ret) { > + mutex_unlock(&list_lock); > + return ret; > } > > /* Keep also completed notifiers on the list */ > @@ -224,28 +334,67 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > > return 0; > } > + > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > + struct v4l2_async_notifier *notifier) > +{ > + if (!v4l2_dev) I'd change this to: if (!v4l2_dev || notifier->sd) > + return -EINVAL; > + > + notifier->v4l2_dev = v4l2_dev; > + > + return __v4l2_async_notifier_register(notifier); > +} > EXPORT_SYMBOL(v4l2_async_notifier_register); > > -void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd, > + struct v4l2_async_notifier *notifier) > { > - struct v4l2_subdev *sd, *tmp; > + if (!sd) and this to: if (!sd || notifier->v4l2_dev) > + return -EINVAL; > > - if (!notifier->v4l2_dev) > - return; > + notifier->sd = sd; > > - mutex_lock(&list_lock); > + return __v4l2_async_notifier_register(notifier); > +} > +EXPORT_SYMBOL(v4l2_async_subdev_notifier_register); > > - list_del(¬ifier->list); > +/* Unbind all sub-devices in the notifier tree. */ > +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, ¬ifier->done, async_list) { > + struct v4l2_async_notifier *subdev_notifier = > + v4l2_async_get_subdev_notifier(sd); > + > + if (subdev_notifier) > + v4l2_async_notifier_unbind_all_subdevs(subdev_notifier); > + > v4l2_async_cleanup(sd); > > v4l2_async_notifier_call_unbind(notifier, sd, sd->asd); > + > + list_del(&sd->async_list); > + list_add(&sd->async_list, &subdev_list); > } > > - mutex_unlock(&list_lock); > + notifier->master = NULL; > +} > + > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > +{ > + if (!notifier->v4l2_dev && !notifier->sd) > + return; > > - notifier->v4l2_dev = NULL; > + mutex_lock(&list_lock); > + > + v4l2_async_notifier_unbind_all_subdevs(notifier); > + > + list_del(¬ifier->list); > + > + mutex_unlock(&list_lock); > } > EXPORT_SYMBOL(v4l2_async_notifier_unregister); > > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h > index 3bc8a7c0d83f..12739be44bd1 100644 > --- a/include/media/v4l2-async.h > +++ b/include/media/v4l2-async.h > @@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations { > * @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 > + * @v4l2_dev: v4l2_device of the master, for subdev notifiers NULL > + * @sd: sub-device that registered the notifier, NULL otherwise > + * @master: master notifier carrying @v4l2_dev I think this description is out of date. It is really the parent notifier, right? Should 'master' be renamed to 'parent'? Same problem with the description of @v4l2_dev: it's the v4l2_device of the root/top-level notifier. > * @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 > @@ -113,6 +115,8 @@ struct v4l2_async_notifier { > unsigned int max_subdevs; > struct v4l2_async_subdev **subdevs; > struct v4l2_device *v4l2_dev; > + struct v4l2_subdev *sd; > + struct v4l2_async_notifier *master; > struct list_head waiting; > struct list_head done; > struct list_head list; > @@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > struct v4l2_async_notifier *notifier); > > /** > + * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous > + * notifier for a sub-device > + * > + * @sd: pointer to &struct v4l2_subdev > + * @notifier: pointer to &struct v4l2_async_notifier > + */ > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd, > + struct v4l2_async_notifier *notifier); > + > +/** > * v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier > * > * @notifier: pointer to &struct v4l2_async_notifier > This v8 is much better and is getting close. Thanks! Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from lb3-smtp-cloud7.xs4all.net ([194.109.24.31]:35542 "EHLO lb3-smtp-cloud7.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750832AbdIFIqg (ORCPT ); Wed, 6 Sep 2017 04:46:36 -0400 Subject: Re: [PATCH v8 14/21] v4l: async: Allow binding notifiers to sub-devices To: Sakari Ailus , linux-media@vger.kernel.org Cc: niklas.soderlund@ragnatech.se, robh@kernel.org, laurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org References: <20170905130553.1332-1-sakari.ailus@linux.intel.com> <20170905130553.1332-15-sakari.ailus@linux.intel.com> From: Hans Verkuil Message-ID: <910b3d01-a8a4-2363-4b24-ed0edd1e1f4d@xs4all.nl> Date: Wed, 6 Sep 2017 10:46:31 +0200 MIME-Version: 1.0 In-Reply-To: <20170905130553.1332-15-sakari.ailus@linux.intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: On 09/05/2017 03:05 PM, 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 to the to -> in > 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 master notifier's complete callback is only called when all sub-device > notifiers are completed. > > Signed-off-by: Sakari Ailus > --- > drivers/media/v4l2-core/v4l2-async.c | 209 ++++++++++++++++++++++++++++++----- > include/media/v4l2-async.h | 16 ++- > 2 files changed, 194 insertions(+), 31 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index 79f216723a3f..620b2cd29fc3 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct v4l2_async_notifier *n) > return n->ops->complete(n); > } > > +static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, > + struct v4l2_subdev *sd, > + struct v4l2_async_subdev *asd); > + > static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) > { > #if IS_ENABLED(CONFIG_I2C) > @@ -129,14 +133,119 @@ static struct v4l2_async_subdev *v4l2_async_find_match( > return NULL; > } > > +/* Get the sub-device notifier registered by a sub-device driver. */ > +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier( > + struct v4l2_subdev *sd) > +{ > + struct v4l2_async_notifier *n; > + > + list_for_each_entry(n, ¬ifier_list, list) > + if (n->sd == sd) > + return n; > + > + return NULL; > +} > + > +/* Return true if all sub-device notifiers are complete, false otherwise. */ > +static bool v4l2_async_subdev_notifiers_complete( > + struct v4l2_async_notifier *notifier) > +{ > + struct v4l2_subdev *sd; > + > + if (!list_empty(¬ifier->waiting)) > + return false; > + > + list_for_each_entry(sd, ¬ifier->done, async_list) { > + struct v4l2_async_notifier *subdev_notifier = > + v4l2_async_get_subdev_notifier(sd); > + > + if (!subdev_notifier) > + continue; > + > + if (!v4l2_async_subdev_notifiers_complete(subdev_notifier)) > + return false; > + } > + > + return true; > +} > + > +/* Get v4l2_device related to the notifier if one can be found. */ > +static struct v4l2_device *v4l2_async_notifier_get_v4l2_dev( > + struct v4l2_async_notifier *notifier) > +{ > + while (notifier->master) > + notifier = notifier->master; > + > + return notifier->v4l2_dev; > +} > + > +/* Test all async sub-devices in a notifier for a match. */ > +static int v4l2_async_notifier_try_all_subdevs( > + struct v4l2_async_notifier *notifier) > +{ > + struct v4l2_subdev *sd, *tmp; > + > + if (!v4l2_async_notifier_get_v4l2_dev(notifier)) > + return 0; > + > + list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { > + struct v4l2_async_subdev *asd; > + int ret; > + > + asd = v4l2_async_find_match(notifier, sd); > + if (!asd) > + continue; > + > + ret = v4l2_async_match_notify(notifier, sd, asd); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +/* Try completing a notifier. */ > +static int v4l2_async_notifier_try_complete( > + struct v4l2_async_notifier *notifier) > +{ > + do { > + int ret; > + > + /* Any local async sub-devices left? */ > + if (!list_empty(¬ifier->waiting)) > + return 0; > + > + /* > + * Any sub-device notifiers waiting for async subdevs > + * to be bound? > + */ > + if (!v4l2_async_subdev_notifiers_complete(notifier)) > + return 0; > + > + /* Proceed completing the notifier */ > + ret = v4l2_async_notifier_call_complete(notifier); > + if (ret < 0) > + return ret; > + > + /* > + * Obtain notifier's master. If there is one, repeat > + * the process, otherwise we're done here. > + */ > + } while ((notifier = notifier->master)); I'd change this to: notifier = notifier->master; } while (notifier); > + > + return 0; > +} > + > static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, > struct v4l2_subdev *sd, > struct v4l2_async_subdev *asd) > { > + struct v4l2_async_notifier *subdev_notifier; > int ret; > > - ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd); > - if (ret < 0) > + ret = v4l2_device_register_subdev( > + v4l2_async_notifier_get_v4l2_dev(notifier), sd); > + if (ret) > return ret; > > ret = v4l2_async_notifier_call_bound(notifier, sd, asd); > @@ -153,10 +262,20 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, > /* Move from the global subdevice list to notifier's done */ > list_move(&sd->async_list, ¬ifier->done); > > - if (list_empty(¬ifier->waiting)) > - return v4l2_async_notifier_call_complete(notifier); > + /* > + * See if the sub-device has a notifier. If it does, proceed > + * with checking for its async sub-devices. > + */ > + subdev_notifier = v4l2_async_get_subdev_notifier(sd); > + if (subdev_notifier && !subdev_notifier->master) { > + subdev_notifier->master = notifier; > + ret = v4l2_async_notifier_try_all_subdevs(subdev_notifier); > + if (ret) > + return ret; > + } > > - return 0; > + /* Try completing the notifier and its master(s). */ > + return v4l2_async_notifier_try_complete(notifier); > } > > static void v4l2_async_cleanup(struct v4l2_subdev *sd) > @@ -168,18 +287,17 @@ static void v4l2_async_cleanup(struct v4l2_subdev *sd) > sd->dev = NULL; > } > > -int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > - struct v4l2_async_notifier *notifier) > +static int __v4l2_async_notifier_register(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 || > + if (!notifier->v4l2_dev == !notifier->sd || !notifier->num_subdevs || With the changes suggested below this can be changed to: if (!notifier->num_subdevs || However, I have a question about that: why would it be wrong to call this with no subdevs in the list? It's perfectly valid to have no subdevs at all. There may be a fixed incoming video stream that is not controlled by a subdev. We have a case like that in fact. > notifier->num_subdevs > V4L2_MAX_SUBDEVS) > return -EINVAL; > > - notifier->v4l2_dev = v4l2_dev; > + INIT_LIST_HEAD(¬ifier->list); > INIT_LIST_HEAD(¬ifier->waiting); > INIT_LIST_HEAD(¬ifier->done); > > @@ -203,18 +321,10 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > > mutex_lock(&list_lock); > > - list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { > - int ret; > - > - asd = v4l2_async_find_match(notifier, sd); > - if (!asd) > - continue; > - > - ret = v4l2_async_match_notify(notifier, sd, asd); > - if (ret < 0) { > - mutex_unlock(&list_lock); > - return ret; > - } > + ret = v4l2_async_notifier_try_all_subdevs(notifier); > + if (ret) { > + mutex_unlock(&list_lock); > + return ret; > } > > /* Keep also completed notifiers on the list */ > @@ -224,28 +334,67 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > > return 0; > } > + > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > + struct v4l2_async_notifier *notifier) > +{ > + if (!v4l2_dev) I'd change this to: if (!v4l2_dev || notifier->sd) > + return -EINVAL; > + > + notifier->v4l2_dev = v4l2_dev; > + > + return __v4l2_async_notifier_register(notifier); > +} > EXPORT_SYMBOL(v4l2_async_notifier_register); > > -void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd, > + struct v4l2_async_notifier *notifier) > { > - struct v4l2_subdev *sd, *tmp; > + if (!sd) and this to: if (!sd || notifier->v4l2_dev) > + return -EINVAL; > > - if (!notifier->v4l2_dev) > - return; > + notifier->sd = sd; > > - mutex_lock(&list_lock); > + return __v4l2_async_notifier_register(notifier); > +} > +EXPORT_SYMBOL(v4l2_async_subdev_notifier_register); > > - list_del(¬ifier->list); > +/* Unbind all sub-devices in the notifier tree. */ > +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, ¬ifier->done, async_list) { > + struct v4l2_async_notifier *subdev_notifier = > + v4l2_async_get_subdev_notifier(sd); > + > + if (subdev_notifier) > + v4l2_async_notifier_unbind_all_subdevs(subdev_notifier); > + > v4l2_async_cleanup(sd); > > v4l2_async_notifier_call_unbind(notifier, sd, sd->asd); > + > + list_del(&sd->async_list); > + list_add(&sd->async_list, &subdev_list); > } > > - mutex_unlock(&list_lock); > + notifier->master = NULL; > +} > + > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) > +{ > + if (!notifier->v4l2_dev && !notifier->sd) > + return; > > - notifier->v4l2_dev = NULL; > + mutex_lock(&list_lock); > + > + v4l2_async_notifier_unbind_all_subdevs(notifier); > + > + list_del(¬ifier->list); > + > + mutex_unlock(&list_lock); > } > EXPORT_SYMBOL(v4l2_async_notifier_unregister); > > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h > index 3bc8a7c0d83f..12739be44bd1 100644 > --- a/include/media/v4l2-async.h > +++ b/include/media/v4l2-async.h > @@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations { > * @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 > + * @v4l2_dev: v4l2_device of the master, for subdev notifiers NULL > + * @sd: sub-device that registered the notifier, NULL otherwise > + * @master: master notifier carrying @v4l2_dev I think this description is out of date. It is really the parent notifier, right? Should 'master' be renamed to 'parent'? Same problem with the description of @v4l2_dev: it's the v4l2_device of the root/top-level notifier. > * @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 > @@ -113,6 +115,8 @@ struct v4l2_async_notifier { > unsigned int max_subdevs; > struct v4l2_async_subdev **subdevs; > struct v4l2_device *v4l2_dev; > + struct v4l2_subdev *sd; > + struct v4l2_async_notifier *master; > struct list_head waiting; > struct list_head done; > struct list_head list; > @@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, > struct v4l2_async_notifier *notifier); > > /** > + * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous > + * notifier for a sub-device > + * > + * @sd: pointer to &struct v4l2_subdev > + * @notifier: pointer to &struct v4l2_async_notifier > + */ > +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd, > + struct v4l2_async_notifier *notifier); > + > +/** > * v4l2_async_notifier_unregister - unregisters a subdevice asynchronous notifier > * > * @notifier: pointer to &struct v4l2_async_notifier > This v8 is much better and is getting close. Thanks! Hans