| Message ID | 20200904215141.20862-1-luca@lucaceresoli.net |
|---|---|
| State | Accepted |
| Commit | 6cab05cf699062a61ae56e6c8e28f15d176ab42d |
| Headers | show |
| Series | [1/3] media: docs: v4l2-subdev: fix typo | expand |
Hi Luca, On Fri, Sep 04, 2020 at 11:51:39PM +0200, Luca Ceresoli wrote: > Fix "will to" -> "will do". > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Thanks j > --- > Documentation/driver-api/media/v4l2-subdev.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst > index bc7e1fc40a9d..9a7dddd97f5a 100644 > --- a/Documentation/driver-api/media/v4l2-subdev.rst > +++ b/Documentation/driver-api/media/v4l2-subdev.rst > @@ -191,7 +191,7 @@ but it is better and easier to use this macro: > > err = v4l2_subdev_call(sd, core, g_std, &norm); > > -The macro will to the right ``NULL`` pointer checks and returns ``-ENODEV`` > +The macro will do the right ``NULL`` pointer checks and returns ``-ENODEV`` > if :c:type:`sd <v4l2_subdev>` is ``NULL``, ``-ENOIOCTLCMD`` if either > :c:type:`sd <v4l2_subdev>`->core or :c:type:`sd <v4l2_subdev>`->core->g_std is ``NULL``, or the actual result of the > :c:type:`sd <v4l2_subdev>`->ops->core->g_std ops. > -- > 2.28.0 >
Hi Luca, On Fri, Sep 04, 2020 at 11:51:41PM +0200, Luca Ceresoli wrote: > This paragraph provides generic information to explain what v4l2_subdev is > useful for. Placing it in the middle of paragraphs describing the details > of subdev registration does not make much sense. Move it near the beginning > of the section when the v4l2_subdev idea has just been introduced and > before going into its details. > > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> > --- > Documentation/driver-api/media/v4l2-subdev.rst | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst > index fb66163deb38..1c1e3f9da142 100644 > --- a/Documentation/driver-api/media/v4l2-subdev.rst > +++ b/Documentation/driver-api/media/v4l2-subdev.rst > @@ -12,6 +12,12 @@ Usually these are I2C devices, but not necessarily. In order to provide the > driver with a consistent interface to these sub-devices the > :c:type:`v4l2_subdev` struct (v4l2-subdev.h) was created. > > +The advantage of using :c:type:`v4l2_subdev` is that it is a generic struct and > +does not contain any knowledge about the underlying hardware. So a driver might > +contain several subdevs that use an I2C bus, but also a subdev that is > +controlled through GPIO pins. This distinction is only relevant when setting > +up the device, but once the subdev is registered it is completely transparent. > + > Each sub-device driver must have a :c:type:`v4l2_subdev` struct. This struct > can be stand-alone for simple sub-devices or it might be embedded in a larger > struct if more state information needs to be stored. Usually there is a > @@ -235,12 +241,6 @@ it can call ``v4l2_subdev_notify(sd, notification, arg)``. This macro checks > whether there is a ``notify()`` callback defined and returns ``-ENODEV`` if not. > Otherwise the result of the ``notify()`` call is returned. > > -The advantage of using :c:type:`v4l2_subdev` is that it is a generic struct and > -does not contain any knowledge about the underlying hardware. So a driver might > -contain several subdevs that use an I2C bus, but also a subdev that is > -controlled through GPIO pins. This distinction is only relevant when setting > -up the device, but once the subdev is registered it is completely transparent. > - Have you considered moving the whole part that describes how to call operations, which comes after the synchronous registration case to a dedicated sub-section ? The above paragraph makes sense in the context of describing why v4l2_subdev is advantageous as it abstract the underlying details under a unified call interface. This could become V4L2 sub-devices ---------------- Intro ~~~~~ Registration ~~~~~~~~~~~~ **synchronous** **asynchronous** Operations call (or a better name :) ~~~~~~~~~~~~~~~ What do you think ? Thanks j > In the **asynchronous** case subdevice probing can be invoked independently of > the bridge driver availability. The subdevice driver then has to verify whether > all the requirements for a successful probing are satisfied. This can include a > -- > 2.28.0 >
Hi Jacopo, thanks for reviewing. On 15/09/20 15:34, Jacopo Mondi wrote: > Hi Luca, > > On Fri, Sep 04, 2020 at 11:51:41PM +0200, Luca Ceresoli wrote: >> This paragraph provides generic information to explain what v4l2_subdev is >> useful for. Placing it in the middle of paragraphs describing the details >> of subdev registration does not make much sense. Move it near the beginning >> of the section when the v4l2_subdev idea has just been introduced and >> before going into its details. >> >> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> >> --- >> Documentation/driver-api/media/v4l2-subdev.rst | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst >> index fb66163deb38..1c1e3f9da142 100644 >> --- a/Documentation/driver-api/media/v4l2-subdev.rst >> +++ b/Documentation/driver-api/media/v4l2-subdev.rst >> @@ -12,6 +12,12 @@ Usually these are I2C devices, but not necessarily. In order to provide the >> driver with a consistent interface to these sub-devices the >> :c:type:`v4l2_subdev` struct (v4l2-subdev.h) was created. >> >> +The advantage of using :c:type:`v4l2_subdev` is that it is a generic struct and >> +does not contain any knowledge about the underlying hardware. So a driver might >> +contain several subdevs that use an I2C bus, but also a subdev that is >> +controlled through GPIO pins. This distinction is only relevant when setting >> +up the device, but once the subdev is registered it is completely transparent. >> + >> Each sub-device driver must have a :c:type:`v4l2_subdev` struct. This struct >> can be stand-alone for simple sub-devices or it might be embedded in a larger >> struct if more state information needs to be stored. Usually there is a >> @@ -235,12 +241,6 @@ it can call ``v4l2_subdev_notify(sd, notification, arg)``. This macro checks >> whether there is a ``notify()`` callback defined and returns ``-ENODEV`` if not. >> Otherwise the result of the ``notify()`` call is returned. >> >> -The advantage of using :c:type:`v4l2_subdev` is that it is a generic struct and >> -does not contain any knowledge about the underlying hardware. So a driver might >> -contain several subdevs that use an I2C bus, but also a subdev that is >> -controlled through GPIO pins. This distinction is only relevant when setting >> -up the device, but once the subdev is registered it is completely transparent. >> - > > Have you considered moving the whole part that describes how to call > operations, which comes after the synchronous registration case to a > dedicated sub-section ? The above paragraph makes sense in the context > of describing why v4l2_subdev is advantageous as it abstract the > underlying details under a unified call interface. > > This could become > > V4L2 sub-devices > ---------------- > > Intro > ~~~~~ > > Registration > ~~~~~~~~~~~~ > > **synchronous** > **asynchronous** > > Operations call (or a better name :) > ~~~~~~~~~~~~~~~ > > What do you think ? I think you're right. Incoming v2 with the new layout as you suggested. -- Luca
diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst index bc7e1fc40a9d..9a7dddd97f5a 100644 --- a/Documentation/driver-api/media/v4l2-subdev.rst +++ b/Documentation/driver-api/media/v4l2-subdev.rst @@ -191,7 +191,7 @@ but it is better and easier to use this macro: err = v4l2_subdev_call(sd, core, g_std, &norm); -The macro will to the right ``NULL`` pointer checks and returns ``-ENODEV`` +The macro will do the right ``NULL`` pointer checks and returns ``-ENODEV`` if :c:type:`sd <v4l2_subdev>` is ``NULL``, ``-ENOIOCTLCMD`` if either :c:type:`sd <v4l2_subdev>`->core or :c:type:`sd <v4l2_subdev>`->core->g_std is ``NULL``, or the actual result of the :c:type:`sd <v4l2_subdev>`->ops->core->g_std ops.
Fix "will to" -> "will do". Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net> --- Documentation/driver-api/media/v4l2-subdev.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)