Re: [PATCH 02/22] [media] v4l2-async: allow subdevices to add further subdevices to the notifier waiting list

[Philipp Zabel <p.zabel@pengutronix.de>]

Am Samstag, den 08.10.2016, 01:43 +0300 schrieb Sakari Ailus:
[...]
> >  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);
> > +	unsigned int notif_n_subdev = 0;
> > +	unsigned int n_subdev;
> > +	struct list_head *list;
> >  	struct device **dev;
> >  	int i = 0;
> >  
> > @@ -218,6 +273,10 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> >  
> >  	list_del(&notifier->list);
> >  
> > +	list_for_each(list, &notifier->done)
> > +		++notif_n_subdev;
> > +	n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
> > +
> 
> Shouldn't this change go to a separate patch? It seems unrelated.

Thanks, this was intended to count the notifier done list instead of
relying on notifier->num_subdevs because of the additional asynchronous
subdevs added to the notifier that are not part of the original array.
Unfortunately this change is a few lines too late, it belongs before the
device cache is allocated. I'll fix this and add a comment.

I don't want to increment notifier->num_subdevs in
__v4l2_async_notifier_add_subdev because the caller of
v4l2_async_notifier_register might still use it to measure the original
array.

> >  	list_for_each_entry_safe(sd, tmp, &notifier->done, async_list) {
> >  		struct device *d;
> >  
> > @@ -294,8 +353,19 @@ 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) {
> > +			struct list_head *tail = notifier->waiting.prev;
> >  			int ret = v4l2_async_test_notify(notifier, sd, asd);
> > +
> > +			/*
> > +			 * If entries were added to the notifier waiting list,
> > +			 * check if the corresponding subdevices are already
> > +			 * available.
> > +			 */
> > +			if (tail != notifier->waiting.prev)
> > +				ret = v4l2_async_test_notify_all(notifier);
> > +
> >  			mutex_unlock(&list_lock);
> > +
> >  			return ret;
> >  		}
> >  	}
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > index 8e2a236..e4e4b11 100644
> > --- a/include/media/v4l2-async.h
> > +++ b/include/media/v4l2-async.h
> > @@ -114,6 +114,18 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> >  				 struct v4l2_async_notifier *notifier);
> >  
> >  /**
> > + * __v4l2_async_notifier_add_subdev - adds a subdevice to the notifier waitlist
> > + *
> > + * @v4l2_notifier: notifier the calling subdev is bound to
> 
> s/v4l2_//

I'd be happy to, but why should the v4l2 prefix be removed?

regards
Philipp