[PATCHv3,1/2,RESEND] doc/media api: Try to make enum usage clearer

  This clarifies which side of the calls is responsible for doing what to which parts of the struct.
This also explicitly states that repeating values are disallowed.
It also expands the terse description of the access algorithm into more prose-like, active voice description, which trades conciseness for ease of comprehension.

Added: mbus codes must not repeat
Added: no holes in the enumeration
Added: enumerations per what?
Added: who fills in what in calls
Changed: "zero" -> "0"
Changed: "given" -> "specified"

Still unclear how it works so didn't describe: "which". What is a "try format" vs "active format"?

Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
---
Hi,

I took those patches out of the freezer. I made sure they still apply (they do), and corrected some errors pointed out by Jacopo in the previous round of reviews.

Thanks,
Dorota

 .../v4l/vidioc-subdev-enum-mbus-code.rst      | 38 +++++++++++++------
 1 file changed, 26 insertions(+), 12 deletions(-)
  

Hi Dorota,

On Sun, Feb 19, 2023 at 12:00:34PM +0100, Dorota Czaplejewicz wrote:
> This clarifies which side of the calls is responsible for doing what to which parts of the struct.
> This also explicitly states that repeating values are disallowed.
> It also expands the terse description of the access algorithm into more prose-like, active voice description, which trades conciseness for ease of comprehension.

The commit message lines should be wrapped at 74 characters. Please do use
checkpatch.pl in the future. There is also no apparent reason to add a
newline after a period.

The same applies to the other patch as well.

I've addressed the issues this time.

> 
> Added: mbus codes must not repeat
> Added: no holes in the enumeration
> Added: enumerations per what?
> Added: who fills in what in calls
> Changed: "zero" -> "0"
> Changed: "given" -> "specified"
> 
> Still unclear how it works so didn't describe: "which". What is a "try format" vs "active format"?
> 
> Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> ---
> Hi,
> 
> I took those patches out of the freezer. I made sure they still apply (they do), and corrected some errors pointed out by Jacopo in the previous round of reviews.

Ditto.

> 
> Thanks,
> Dorota
> 
>  .../v4l/vidioc-subdev-enum-mbus-code.rst      | 38 +++++++++++++------
>  1 file changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> index 417f1a19bcc4..63bff24d0520 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> @@ -31,15 +31,28 @@ Arguments
>  Description
>  ===========
>  
> -To enumerate media bus formats available at a given sub-device pad
> -applications initialize the ``pad``, ``which`` and ``index`` fields of
> -struct
> -:c:type:`v4l2_subdev_mbus_code_enum` and
> -call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl with a pointer to this
> -structure. Drivers fill the rest of the structure or return an ``EINVAL``
> -error code if either the ``pad`` or ``index`` are invalid. All media bus
> -formats are enumerable by beginning at index zero and incrementing by
> -one until ``EINVAL`` is returned.
> +This call is used by the application to access the enumeration
> +of media bus formats for the selected pad.
> +
> +The enumerations are defined by the driver, and indexed using the ``index`` field
> +of struct :c:type:`v4l2_subdev_mbus_code_enum`.
> +Each enumeration starts with the ``index`` of 0, and
> +the lowest invalid index marks the end of enumeration.
> +
> +Therefore, to enumerate media bus formats available at a given sub-device pad,
> +initialize the ``pad``, and ``which`` fields to desired values,
> +and set ``index`` to 0.
> +Then call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl
> +with a pointer to this structure.
> +
> +A successful call will return with the ``code`` field filled in
> +with a mbus code value.
> +Repeat with increasing ``index`` until ``EINVAL`` is received.
> +``EINVAL`` means that either ``pad`` is invalid,
> +or that there are no more codes available at this pad.
> +
> +The driver must not return the same value of ``code`` for different indices
> +at the same pad.
>  
>  Available media bus formats may depend on the current 'try' formats at
>  other pads of the sub-device, as well as on the current active links.
> @@ -57,14 +70,15 @@ information about the try formats.
>  
>      * - __u32
>        - ``pad``
> -      - Pad number as reported by the media controller API.
> +      - Pad number as reported by the media controller API. Filled in by the application.

Here, too...

>      * - __u32
>        - ``index``
> -      - Number of the format in the enumeration, set by the application.
> +      - Index of the mbus code in the enumeration belonging to the given pad.
> +    Filled in by the application.

Wrong indentation.

>      * - __u32
>        - ``code``
>        - The media bus format code, as defined in
> -	:ref:`v4l2-mbus-format`.
> +	:ref:`v4l2-mbus-format`. Filled in by the driver.
>      * - __u32
>        - ``which``
>        - Media bus format codes to be enumerated, from enum
  

Hi,

On Wed, 15 Mar 2023 12:14:28 +0200
Sakari Ailus <sakari.ailus@iki.fi> wrote:

> Hi Dorota,
> 
> On Sun, Feb 19, 2023 at 12:00:34PM +0100, Dorota Czaplejewicz wrote:
> > This clarifies which side of the calls is responsible for doing what to which parts of the struct.
> > This also explicitly states that repeating values are disallowed.
> > It also expands the terse description of the access algorithm into more prose-like, active voice description, which trades conciseness for ease of comprehension.  
> 
> The commit message lines should be wrapped at 74 characters. Please do use
> checkpatch.pl in the future. There is also no apparent reason to add a
> newline after a period.
> 
> The same applies to the other patch as well.
> 
> I've addressed the issues this time.

Thanks for taking a look. What is the next step?

--Dorota
> 
> > 
> > Added: mbus codes must not repeat
> > Added: no holes in the enumeration
> > Added: enumerations per what?
> > Added: who fills in what in calls
> > Changed: "zero" -> "0"
> > Changed: "given" -> "specified"
> > 
> > Still unclear how it works so didn't describe: "which". What is a "try format" vs "active format"?
> > 
> > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> > ---
> > Hi,
> > 
> > I took those patches out of the freezer. I made sure they still apply (they do), and corrected some errors pointed out by Jacopo in the previous round of reviews.  
> 
> Ditto.
> 
> > 
> > Thanks,
> > Dorota
> > 
> >  .../v4l/vidioc-subdev-enum-mbus-code.rst      | 38 +++++++++++++------
> >  1 file changed, 26 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> > index 417f1a19bcc4..63bff24d0520 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst
> > @@ -31,15 +31,28 @@ Arguments
> >  Description
> >  ===========
> >  
> > -To enumerate media bus formats available at a given sub-device pad
> > -applications initialize the ``pad``, ``which`` and ``index`` fields of
> > -struct
> > -:c:type:`v4l2_subdev_mbus_code_enum` and
> > -call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl with a pointer to this
> > -structure. Drivers fill the rest of the structure or return an ``EINVAL``
> > -error code if either the ``pad`` or ``index`` are invalid. All media bus
> > -formats are enumerable by beginning at index zero and incrementing by
> > -one until ``EINVAL`` is returned.
> > +This call is used by the application to access the enumeration
> > +of media bus formats for the selected pad.
> > +
> > +The enumerations are defined by the driver, and indexed using the ``index`` field
> > +of struct :c:type:`v4l2_subdev_mbus_code_enum`.
> > +Each enumeration starts with the ``index`` of 0, and
> > +the lowest invalid index marks the end of enumeration.
> > +
> > +Therefore, to enumerate media bus formats available at a given sub-device pad,
> > +initialize the ``pad``, and ``which`` fields to desired values,
> > +and set ``index`` to 0.
> > +Then call the :ref:`VIDIOC_SUBDEV_ENUM_MBUS_CODE` ioctl
> > +with a pointer to this structure.
> > +
> > +A successful call will return with the ``code`` field filled in
> > +with a mbus code value.
> > +Repeat with increasing ``index`` until ``EINVAL`` is received.
> > +``EINVAL`` means that either ``pad`` is invalid,
> > +or that there are no more codes available at this pad.
> > +
> > +The driver must not return the same value of ``code`` for different indices
> > +at the same pad.
> >  
> >  Available media bus formats may depend on the current 'try' formats at
> >  other pads of the sub-device, as well as on the current active links.
> > @@ -57,14 +70,15 @@ information about the try formats.
> >  
> >      * - __u32
> >        - ``pad``
> > -      - Pad number as reported by the media controller API.
> > +      - Pad number as reported by the media controller API. Filled in by the application.  
> 
> Here, too...
> 
> >      * - __u32
> >        - ``index``
> > -      - Number of the format in the enumeration, set by the application.
> > +      - Index of the mbus code in the enumeration belonging to the given pad.
> > +    Filled in by the application.  
> 
> Wrong indentation.
> 
> >      * - __u32
> >        - ``code``
> >        - The media bus format code, as defined in
> > -	:ref:`v4l2-mbus-format`.
> > +	:ref:`v4l2-mbus-format`. Filled in by the driver.
> >      * - __u32
> >        - ``which``
> >        - Media bus format codes to be enumerated, from enum  
>
  

Hi Dorota,

On Wed, Mar 15, 2023 at 02:09:14PM +0100, Dorota Czaplejewicz wrote:
> Hi,
> 
> On Wed, 15 Mar 2023 12:14:28 +0200
> Sakari Ailus <sakari.ailus@iki.fi> wrote:
> 
> > Hi Dorota,
> > 
> > On Sun, Feb 19, 2023 at 12:00:34PM +0100, Dorota Czaplejewicz wrote:
> > > This clarifies which side of the calls is responsible for doing what to which parts of the struct.
> > > This also explicitly states that repeating values are disallowed.
> > > It also expands the terse description of the access algorithm into more prose-like, active voice description, which trades conciseness for ease of comprehension.  
> > 
> > The commit message lines should be wrapped at 74 characters. Please do use
> > checkpatch.pl in the future. There is also no apparent reason to add a
> > newline after a period.
> > 
> > The same applies to the other patch as well.
> > 
> > I've addressed the issues this time.
> 
> Thanks for taking a look. What is the next step?

Just remember to use checkpatch.pl, on media tree in particular it's:

	$ checkpatch.pl --strict --max-line-length=80