Hi Guennadi,
Overall it looks quite neat at this v10. :)
On 06/11/2013 10:23 AM, Guennadi Liakhovetski wrote:
Currently bridge device drivers register devices for all subdevices
synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
s/tupically/typically
is attached to a video bridge device, the bridge driver will create an I2C
+/**
+ * v4l2_async_subdev_list - provided by subdevices
+ * @list: links struct v4l2_async_subdev_list objects to a global list
+ * before probing, and onto notifier->done after probing
+ * @asd: pointer to respective struct v4l2_async_subdev
+ * @notifier: pointer to managing notifier
+ */
+struct v4l2_async_subdev_list {
+ struct list_head list;
+ struct v4l2_async_subdev *asd;
+ struct v4l2_async_notifier *notifier;
+};
I have a patch for this patch, which embeds members of this struct directly
into struct v4l2_subdev. My felling is that the code is simpler and easier
to follow this way, I might be missing some important details though.
+/**
+ * v4l2_async_notifier - v4l2_device notifier data
+ * @subdev_num: number of subdevices
+ * @subdev: array of pointers to subdevices
How about changing this to:
@subdevs: array of pointers to the subdevice descriptors
I think it would be more immediately clear this is the actual subdevs array
pointer, and perhaps we could have subdev_num renamed to num_subdevs ?
+ * @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.
So except this bikeshedding I don't really have other comments, I'm
going to
test this series with the s3c-camif/ov9650 drivers and will report back
soon.
It would have been a shame to not have this series in 3.11. I guess three
kernel cycles, since the initial implementation, time frame is sufficient
for having finally working camera devices on a device tree enabled system
in mainline.
Thanks for the idea and patience during reviews! :)
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
