| Message ID | Pine.LNX.4.64.1306170801590.22409@axis700.grange (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Guennadi, Thanks for the patch. On Mon, Jun 17, 2013 at 11:34 AM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Add documentation for the V4L2 clock and V4L2 asynchronous probing APIs > to v4l2-framework.txt. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> Reviewed-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, Thank you for the patch. Overall it looks pretty good, please see below for some small comments. If you've been wondering why your stock of comas is so low, wonder no more: I've found them :-D On Monday 17 June 2013 08:04:10 Guennadi Liakhovetski wrote: > Add documentation for the V4L2 clock and V4L2 asynchronous probing APIs > to v4l2-framework.txt. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > --- > > Hopefully we can commit the actual patches now, while we refine the > documentation. > > Documentation/video4linux/v4l2-framework.txt | 62 ++++++++++++++++++++++- > 1 files changed, 60 insertions(+), 2 deletions(-) > > diff --git a/Documentation/video4linux/v4l2-framework.txt > b/Documentation/video4linux/v4l2-framework.txt index a300b28..159a83a > 100644 > --- a/Documentation/video4linux/v4l2-framework.txt > +++ b/Documentation/video4linux/v4l2-framework.txt > @@ -326,8 +326,27 @@ that width, height and the media bus pixel code are > equal on both source and sink of the link. Subdev drivers are also free to > use this function to perform the checks mentioned above in addition to > their own checks. > > -A device (bridge) driver needs to register the v4l2_subdev with the > -v4l2_device: > +There are currently two ways to register subdevices with the V4L2 core. The > +first (traditional) possibility is to have subdevices registered by bridge > +drivers. This can be done, when the bridge driver has the complete s/done, /done/ > +information about subdevices, connected to it and knows exactly when to s/subdevices,/subdevices/ > +register them. This is typically the case for internal subdevices, like > +video data processing units within SoCs or complex pluggable boards, > +camera sensors in USB cameras or connected to SoCs, which pass information > +about them to bridge drivers, usually in their platform data. > + > +There are however also situations, where subdevices have to be registered s/situations,/situations/ > +asynchronously to bridge devices. An example of such a configuration is > +Device Tree based systems, on which information about subdevices is made s/systems,/systems/ > +available to the system indpendently from the bridge devices, e.g. when s/indpendently/independently/ > +subdevices are defined in DT as I2C device nodes. The API, used in this s/API,/API/ > +second case is described further below. > + > +Using one or the other registration method only affects the probing > +process, the run-time bridge-subdevice interaction is in both cases the > same. > + > +In the synchronous case a device (bridge) driver needs to register the > +v4l2_subdev with the v4l2_device: > > int err = v4l2_device_register_subdev(v4l2_dev, sd); > > @@ -394,6 +413,25 @@ controlled through GPIO pins. This distinction is only > relevant when setting up the device, but once the subdev is registered it > is completely transparent. > > > +In the asynchronous case subdevices register themselves using the > +v4l2_async_register_subdev() function. Unregistration is performed, using s/performed,/performed/ > +the v4l2_async_unregister_subdev() call. Subdevices registered this way > +are stored on a global list of subdevices, ready to be picked up by bridge s/on/in/ > +drivers. > + > +Bridge drivers in turn have to register a notifier object with an array of > +subdevice descriptors, that the bridge device needs for its operation. This s/descriptors,/descriptors/ > +is performed using the v4l2_async_notifier_register() call. To unregister > +the notifier the driver has to call v4l2_async_notifier_unregister(). The > +former of the two functions takes two arguments: a pointer to struct > +v4l2_device and a pointer to struct v4l2_async_notifier. The latter > +contains a pointer to an array of pointers to subdevice descriptors of > +type struct v4l2_async_subdev type. Isn't it the other way around ? > +The V4L2 core will then use these descriptors to match asynchronously > +registered subdevices to them. If a match is detected the .bound() notifier > +callback is called. After all subdevices have been located the .complete() > +callback is called. When a subdevice is removed from the system the > +.unbind() method is called. All three callbacks are optional. > + > + > V4L2 sub-device userspace API > ----------------------------- > > @@ -1061,3 +1099,23 @@ available event type is 'class base + 1'. > > An example on how the V4L2 events may be used can be found in the OMAP > 3 ISP driver (drivers/media/platform/omap3isp). > + > + > +V4L2 clocks > +----------- > + > +Many subdevices, like camera sensors, TV decoders and encoders, need a > +clock signal to be supplied by the system. Often this clock is supplied by > +the respective bridge device. The Linux kernel provides a Common Clock > +Framework for this purpose, however, it is not (yet) available on all > +architectures. Besides, the nature of the multi-functional (clock, data + > +synchronisation, I2C control) connection of subdevices to the system might > +impose special requirements on the clock API usage. For these reasons a > +V4L2 clock helper API has been developed and is provided to bridge and > +subdevice drivers. What special requirements are those ? I've always thought that the purpose of the V4L2 clock API was to provide an API on platforms where CCF wasn't available yet, and that it should then be removed. > +The API consists of two parts: two functions to register and unregister a > +V4L2 clock source: v4l2_clk_register() and v4l2_clk_unregister() and calls > +to control a clock object, similar to respective generic clock API calls: > +v4l2_clk_get(), v4l2_clk_put(), v4l2_clk_enable(), v4l2_clk_disable(), > +v4l2_clk_get_rate(), and v4l2_clk_set_rate(). Clock suppliers have to > +provide clock operations, that will be called when clock users invoke s/operations,/operations/ > respective API methods.
Hi Guennadi, I had hoped to review this earlier this week, but I didn't get around it. But better late than never... Comments below. On Mon June 17 2013 08:04:10 Guennadi Liakhovetski wrote: > Add documentation for the V4L2 clock and V4L2 asynchronous probing APIs > to v4l2-framework.txt. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > --- > > Hopefully we can commit the actual patches now, while we refine the > documentation. > > Documentation/video4linux/v4l2-framework.txt | 62 +++++++++++++++++++++++++- > 1 files changed, 60 insertions(+), 2 deletions(-) > > diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt > index a300b28..159a83a 100644 > --- a/Documentation/video4linux/v4l2-framework.txt > +++ b/Documentation/video4linux/v4l2-framework.txt > @@ -326,8 +326,27 @@ that width, height and the media bus pixel code are equal on both source and > sink of the link. Subdev drivers are also free to use this function to > perform the checks mentioned above in addition to their own checks. > > -A device (bridge) driver needs to register the v4l2_subdev with the > -v4l2_device: > +There are currently two ways to register subdevices with the V4L2 core. The > +first (traditional) possibility is to have subdevices registered by bridge > +drivers. This can be done, when the bridge driver has the complete information > +about subdevices, connected to it and knows exactly when to register them. This > +is typically the case for internal subdevices, like video data processing units > +within SoCs or complex pluggable boards, camera sensors in USB cameras or s/pluggable boards/PCI(e) boards/ That's more concrete than 'pluggable boards' IMHO. > +connected to SoCs, which pass information about them to bridge drivers, usually > +in their platform data. > + > +There are however also situations, where subdevices have to be registered > +asynchronously to bridge devices. An example of such a configuration is Device > +Tree based systems, on which information "a Device Tree based system where information" > about subdevices is made available to > +the system indpendently from the bridge devices, e.g. when subdevices are > +defined in DT as I2C device nodes. The API, used in this second case is > +described further below. > + > +Using one or the other registration method only affects the probing process, the > +run-time bridge-subdevice interaction is in both cases the same. > + > +In the synchronous case a device (bridge) driver needs to register the > +v4l2_subdev with the v4l2_device: > > int err = v4l2_device_register_subdev(v4l2_dev, sd); > > @@ -394,6 +413,25 @@ controlled through GPIO pins. This distinction is only relevant when setting > up the device, but once the subdev is registered it is completely transparent. > > > +In the asynchronous case subdevices register themselves using the > +v4l2_async_register_subdev() function. Unregistration is performed, using the > +v4l2_async_unregister_subdev() call. Subdevices registered this way are stored > +on a global list of subdevices, ready to be picked up by bridge drivers. > + > +Bridge drivers in turn have to register a notifier object with an array of > +subdevice descriptors, that the bridge device needs for its operation. This is > +performed using the v4l2_async_notifier_register() call. To unregister the > +notifier the driver has to call v4l2_async_notifier_unregister(). The former of > +the two functions takes two arguments: a pointer to struct v4l2_device and a > +pointer to struct v4l2_async_notifier. The latter contains a pointer to an array > +of pointers to subdevice descriptors of type struct v4l2_async_subdev type. The > +V4L2 core will then use these descriptors to match asynchronously registered > +subdevices to them. If a match is detected the .bound() notifier callback is > +called. After all subdevices have been located the .complete() callback is > +called. When a subdevice is removed from the system the .unbind() method is > +called. All three callbacks are optional. Is that true? Don't you need at least a bound or a complete callback? Something probable needs to be said about when to return -EPROBE_DEFER in a subdevice, and that any code that depends on the presence of subdevices in a probe() function will be moved to the complete callback. Also, what happens if one or more subdevices aren't found. Ideally a simple example would be helpful. > + > + > V4L2 sub-device userspace API > ----------------------------- > > @@ -1061,3 +1099,23 @@ available event type is 'class base + 1'. > > An example on how the V4L2 events may be used can be found in the OMAP > 3 ISP driver (drivers/media/platform/omap3isp). > + > + > +V4L2 clocks > +----------- > + > +Many subdevices, like camera sensors, TV decoders and encoders, need a clock > +signal to be supplied by the system. Often this clock is supplied by the > +respective bridge device. The Linux kernel provides a Common Clock Framework for > +this purpose, however, it is not (yet) available on all architectures. Besides, s/purpose, however,/purpose. However,/ > +the nature of the multi-functional (clock, data + synchronisation, I2C control) > +connection of subdevices to the system might impose special requirements on the > +clock API usage. For these reasons a V4L2 clock helper API has been developed > +and is provided to bridge and subdevice drivers. > + > +The API consists of two parts: two functions to register and unregister a V4L2 > +clock source: v4l2_clk_register() and v4l2_clk_unregister() and calls to control > +a clock object, similar to respective generic clock API calls: v4l2_clk_get(), s/to/to the/ > +v4l2_clk_put(), v4l2_clk_enable(), v4l2_clk_disable(), v4l2_clk_get_rate(), and > +v4l2_clk_set_rate(). Clock suppliers have to provide clock operations, that will > +be called when clock users invoke respective API methods. > It might be a good idea to mention that these v4l2 clocks will be replaced by the regular common clock framework once it is widespread enough. At least, that's my understanding. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, On Friday 21 June 2013 20:25:38 Hans Verkuil wrote: > Hi Guennadi, > > I had hoped to review this earlier this week, but I didn't get around it. > But better late than never... > > Comments below. > > On Mon June 17 2013 08:04:10 Guennadi Liakhovetski wrote: > > Add documentation for the V4L2 clock and V4L2 asynchronous probing APIs > > to v4l2-framework.txt. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > --- > > > > Hopefully we can commit the actual patches now, while we refine the > > documentation. > > > > Documentation/video4linux/v4l2-framework.txt | 62 > > +++++++++++++++++++++++++- 1 files changed, 60 insertions(+), 2 > > deletions(-) > > > > diff --git a/Documentation/video4linux/v4l2-framework.txt > > b/Documentation/video4linux/v4l2-framework.txt index a300b28..159a83a > > 100644 > > --- a/Documentation/video4linux/v4l2-framework.txt > > +++ b/Documentation/video4linux/v4l2-framework.txt [snip] > > @@ -394,6 +413,25 @@ controlled through GPIO pins. This distinction is > > only relevant when setting up the device, but once the subdev is > > registered it is completely transparent. > > > > +In the asynchronous case subdevices register themselves using the > > +v4l2_async_register_subdev() function. Unregistration is performed, using > > +the v4l2_async_unregister_subdev() call. Subdevices registered this way > > +are stored on a global list of subdevices, ready to be picked up by > > +bridge drivers. > > + > > +Bridge drivers in turn have to register a notifier object with an array > > +of subdevice descriptors, that the bridge device needs for its operation. > > +This is performed using the v4l2_async_notifier_register() call. To > > +unregister the notifier the driver has to call > > +v4l2_async_notifier_unregister(). The former of the two functions takes > > +two arguments: a pointer to struct v4l2_device and a pointer to struct > > +v4l2_async_notifier. The latter contains a pointer to an array of > > +pointers to subdevice descriptors of type struct v4l2_async_subdev type. > > +The V4L2 core will then use these descriptors to match asynchronously > > +registered subdevices to them. If a match is detected the .bound() > > +notifier callback is called. After all subdevices have been located the > > +.complete() callback is called. When a subdevice is removed from the > > +system the .unbind() method is called. All three callbacks are optional. > > Is that true? Don't you need at least a bound or a complete callback? A driver should implement at least either bound or complete, so in a sense all three callbacks are optional. This should probably be made a bit more explicit.
Hi Laurent Thanks for the review. On Mon, 17 Jun 2013, Laurent Pinchart wrote: [snip] > > +drivers. > > + > > +Bridge drivers in turn have to register a notifier object with an array of > > +subdevice descriptors, that the bridge device needs for its operation. This > > s/descriptors,/descriptors/ > > > +is performed using the v4l2_async_notifier_register() call. To unregister > > +the notifier the driver has to call v4l2_async_notifier_unregister(). The > > +former of the two functions takes two arguments: a pointer to struct > > +v4l2_device and a pointer to struct v4l2_async_notifier. The latter > > +contains a pointer to an array of pointers to subdevice descriptors of > > +type struct v4l2_async_subdev type. > > Isn't it the other way around ? I don't think I see anything above, that needs to be swapped. What exactly do you mean? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Monday 24 June 2013 10:53:35 Guennadi Liakhovetski wrote: > On Mon, 17 Jun 2013, Laurent Pinchart wrote: > > [snip] > > > > +drivers. > > > + > > > +Bridge drivers in turn have to register a notifier object with an array > > > +of subdevice descriptors, that the bridge device needs for its > > > +operation. This > > > > s/descriptors,/descriptors/ > > > > > +is performed using the v4l2_async_notifier_register() call. To > > > +unregister the notifier the driver has to call > > > +v4l2_async_notifier_unregister(). The > > > +former of the two functions takes two arguments: a pointer to struct > > > +v4l2_device and a pointer to struct v4l2_async_notifier. The latter > > > +contains a pointer to an array of pointers to subdevice descriptors of > > > +type struct v4l2_async_subdev type. > > > > Isn't it the other way around ? > > I don't think I see anything above, that needs to be swapped. What exactly > do you mean? My bad. I got confused by "the latter" (which should be "the later" ;-)) and thought it was referring to the later of the two functions, given that the previous sentence starts with "the former of the two functions". Maybe s/The format of the two functions/The first function/ and s/The latter/The second structure/ ?
diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt index a300b28..159a83a 100644 --- a/Documentation/video4linux/v4l2-framework.txt +++ b/Documentation/video4linux/v4l2-framework.txt @@ -326,8 +326,27 @@ that width, height and the media bus pixel code are equal on both source and sink of the link. Subdev drivers are also free to use this function to perform the checks mentioned above in addition to their own checks. -A device (bridge) driver needs to register the v4l2_subdev with the -v4l2_device: +There are currently two ways to register subdevices with the V4L2 core. The +first (traditional) possibility is to have subdevices registered by bridge +drivers. This can be done, when the bridge driver has the complete information +about subdevices, connected to it and knows exactly when to register them. This +is typically the case for internal subdevices, like video data processing units +within SoCs or complex pluggable boards, camera sensors in USB cameras or +connected to SoCs, which pass information about them to bridge drivers, usually +in their platform data. + +There are however also situations, where subdevices have to be registered +asynchronously to bridge devices. An example of such a configuration is Device +Tree based systems, on which information about subdevices is made available to +the system indpendently from the bridge devices, e.g. when subdevices are +defined in DT as I2C device nodes. The API, used in this second case is +described further below. + +Using one or the other registration method only affects the probing process, the +run-time bridge-subdevice interaction is in both cases the same. + +In the synchronous case a device (bridge) driver needs to register the +v4l2_subdev with the v4l2_device: int err = v4l2_device_register_subdev(v4l2_dev, sd); @@ -394,6 +413,25 @@ controlled through GPIO pins. This distinction is only relevant when setting up the device, but once the subdev is registered it is completely transparent. +In the asynchronous case subdevices register themselves using the +v4l2_async_register_subdev() function. Unregistration is performed, using the +v4l2_async_unregister_subdev() call. Subdevices registered this way are stored +on a global list of subdevices, ready to be picked up by bridge drivers. + +Bridge drivers in turn have to register a notifier object with an array of +subdevice descriptors, that the bridge device needs for its operation. This is +performed using the v4l2_async_notifier_register() call. To unregister the +notifier the driver has to call v4l2_async_notifier_unregister(). The former of +the two functions takes two arguments: a pointer to struct v4l2_device and a +pointer to struct v4l2_async_notifier. The latter contains a pointer to an array +of pointers to subdevice descriptors of type struct v4l2_async_subdev type. The +V4L2 core will then use these descriptors to match asynchronously registered +subdevices to them. If a match is detected the .bound() notifier callback is +called. After all subdevices have been located the .complete() callback is +called. When a subdevice is removed from the system the .unbind() method is +called. All three callbacks are optional. + + V4L2 sub-device userspace API ----------------------------- @@ -1061,3 +1099,23 @@ available event type is 'class base + 1'. An example on how the V4L2 events may be used can be found in the OMAP 3 ISP driver (drivers/media/platform/omap3isp). + + +V4L2 clocks +----------- + +Many subdevices, like camera sensors, TV decoders and encoders, need a clock +signal to be supplied by the system. Often this clock is supplied by the +respective bridge device. The Linux kernel provides a Common Clock Framework for +this purpose, however, it is not (yet) available on all architectures. Besides, +the nature of the multi-functional (clock, data + synchronisation, I2C control) +connection of subdevices to the system might impose special requirements on the +clock API usage. For these reasons a V4L2 clock helper API has been developed +and is provided to bridge and subdevice drivers. + +The API consists of two parts: two functions to register and unregister a V4L2 +clock source: v4l2_clk_register() and v4l2_clk_unregister() and calls to control +a clock object, similar to respective generic clock API calls: v4l2_clk_get(), +v4l2_clk_put(), v4l2_clk_enable(), v4l2_clk_disable(), v4l2_clk_get_rate(), and +v4l2_clk_set_rate(). Clock suppliers have to provide clock operations, that will +be called when clock users invoke respective API methods.
Add documentation for the V4L2 clock and V4L2 asynchronous probing APIs to v4l2-framework.txt. Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> --- Hopefully we can commit the actual patches now, while we refine the documentation. Documentation/video4linux/v4l2-framework.txt | 62 +++++++++++++++++++++++++- 1 files changed, 60 insertions(+), 2 deletions(-)