linux-kernel - Re: [PATCH 3/3] media: docs: v4l2-subdev: move generic paragraph to the introduction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200915133451.jydprottrtskqe7y@uno.localdomain>
Date:   Tue, 15 Sep 2020 15:34:51 +0200
From:   Jacopo Mondi <jacopo@...ndi.org>
To:     Luca Ceresoli <luca@...aceresoli.net>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Subject: Re: [PATCH 3/3] media: docs: v4l2-subdev: move generic paragraph to
 the introduction

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@...aceresoli.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
>