Re: [PATCH v10 16/21] V4L2: support asynchronous subdevice registration

Guennadi Liakhovetski
2013-06-14T09:44:49Z

On Fri, 14 Jun 2013, Sylwester Nawrocki wrote:

> Hi,
>
> On 06/14/2013 11:14 AM, Guennadi Liakhovetski wrote:
> > On Fri, 14 Jun 2013, Hans Verkuil wrote:
> >> On Fri 14 June 2013 09:14:48 Guennadi Liakhovetski wrote:
> >>> On Thu, 13 Jun 2013, Sylwester Nawrocki wrote:
> >>>> On 06/11/2013 10:23 AM, Guennadi Liakhovetski wrote:
> [...]
> >>>>> + * @v4l2_dev: pointer to struct v4l2_device
> >>>>> + * @waiting: list of struct v4l2_async_subdev, waiting for their drivers
> >>>>> + * @done: list of struct v4l2_async_subdev_list, already probed
> >>>>> + * @list: member in a global list of notifiers
> >>>>> + * @bound: a subdevice driver has successfully probed one of subdevices
> >>>>> + * @complete: all subdevices have been probed successfully
> >>>>> + * @unbind: a subdevice is leaving
> >>>>> + */
> >>>>> +struct v4l2_async_notifier {
> >>>>> + unsigned int subdev_num;
> >>>>> + struct v4l2_async_subdev **subdev;
> >>>>> + struct v4l2_device *v4l2_dev;
> >>>>> + struct list_head waiting;
> >>>>> + struct list_head done;
> >>>>> + struct list_head list;
> >>>>> + int (*bound)(struct v4l2_async_notifier *notifier,
> >>>>> + struct v4l2_subdev *subdev,
> >>>>> + struct v4l2_async_subdev *asd);
> >>>>> + int (*complete)(struct v4l2_async_notifier *notifier);
> >>>>> + void (*unbind)(struct v4l2_async_notifier *notifier,
> >>>>> + struct v4l2_subdev *subdev,
> >>>>> + struct v4l2_async_subdev *asd);
> >>>>> +};
> >>>>> +
> >>>>> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> >>>>> + struct v4l2_async_notifier *notifier);
> >>>>> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
> >>>>> +int v4l2_async_register_subdev(struct v4l2_subdev *sd);
> >>>>> +void v4l2_async_unregister_subdev(struct v4l2_subdev *sd);
> >>>>
> >>>> I still think "async_" in this public API is unnecessary, since we register/
> >>>> unregister a subdev with the core and notifiers are intrinsically
> >>>> asynchronous.
> >>>> But your preference seems be otherwise, what could I do... :) At most it just
> >>>> means one less happy user of this interface.
> >>
> >> I think v4l2_register_subdev looks awfully similar to v4l2_device_register_subdev.
> >> It becomes very confusing naming it like that. I prefer v4l2_async where 'async'
> >> refers to the v4l2-async module.
>
> Ok, let's leave v4l2_async then.
>
> > And v4l2(_async)_notifier_(un)register()?
>
> I guess it would be better to have all or none of the functions
> with that prefix. So either:

Exactly.

Thanks
Guennadi

> v4l2_async_notifier_register
> v4l2_async_notifier_unregister
> v4l2_async_register_subdev
> v4l2_async_unregister_subdev
>
> or
>
> v4l2_subdev_notifier_register
> v4l2_subdev_notifier_unregister
> v4l2_subdev_register
> v4l2_subdev_unregister
>
> Thanks,
> Sylwester
>

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer