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

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

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:

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