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

Philipp Zabel
2016-10-14T15:48:38Z

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