* DocBook fixes
@ 2013-01-11 11:26 Hans Verkuil
2013-01-11 11:26 ` [REVIEWv2 PATCH 1/2] DocBook: fix various validation errors Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2013-01-11 11:26 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart
Two patches: the first fixes a number of validation errors in the DocBook
code (unchanged from v1), the second clarifies how the error_idx is used.
The second patch is almost rewritten completely from v1 based on the
comments from Laurent.
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
* [REVIEWv2 PATCH 1/2] DocBook: fix various validation errors
2013-01-11 11:26 DocBook fixes Hans Verkuil
@ 2013-01-11 11:26 ` Hans Verkuil
2013-01-11 11:26 ` [REVIEWv2 PATCH 2/2] DocBook: improve the error_idx field documentation Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2013-01-11 11:26 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Fixed the following errors (with exception of the SVG errors):
GEN /home/hans/work/src/v4l/media-git/Documentation/DocBook//v4l2.xml
rm -rf Documentation/DocBook/index.html; echo '<h1>Linux Kernel HTML Documentation</h1>' >> Documentation/DocBook/index.html && echo '<h2>Kernel Version: 3.8.0-rc1</h2>' >> Documentation/DocBook/index.html && cat Documentation/DocBook/media_api.html >> Documentation/DocBook/index.html
/tmp/x.xml:883: element revremark: validity error : Element structname is not declared in revremark list of possible children
/tmp/x.xml:883: element revremark: validity error : Element xref is not declared in revremark list of possible children
/tmp/x.xml:1829: element footnote: validity error : Element footnote content does not follow the DTD, expecting (calloutlist | glosslist | itemizedlist | orderedlist | segmentedlist | simplelist | variablelist | literallayout | programlisting | programlistingco | screen | screenco | screenshot | synopsis | cmdsynopsis | funcsynopsis | classsynopsis | fieldsynopsis | constructorsynopsis | destructorsynopsis | methodsynopsis | formalpara | para | simpara | address | blockquote | graphic | graphicco | mediaobject | mediaobjectco | informalequation | informalexample | informalfigure | informaltable)+, got (para para errorcode CDATA para )
/tmp/x.xml:9580: element xref: validity error : Element xref was declared EMPTY this one has content
/tmp/x.xml:13508: element link: validity error : Element link does not carry attribute linkend
/tmp/x.xml:13508: element link: validity error : No declaration for attribute linked of element link
/tmp/x.xml:16986: element imagedata: validity error : Value "SVG" for attribute format of imagedata is not among the enumerated set
/tmp/x.xml:17003: element imagedata: validity error : Value "SVG" for attribute format of imagedata is not among the enumerated set
/tmp/x.xml:17022: element imagedata: validity error : Value "SVG" for attribute format of imagedata is not among the enumerated set
/tmp/x.xml:26795: element refsect1: validity error : Element refsect1 content does not follow the DTD, expecting (refsect1info? , (title , subtitle? , titleabbrev?) , (((calloutlist | glosslist | itemizedlist | orderedlist | segmentedlist | simplelist | variablelist | caution | important | note | tip | warning | literallayout | programlisting | programlistingco | screen | screenco | screenshot | synopsis | cmdsynopsis | funcsynopsis | classsynopsis | fieldsynopsis | constructorsynopsis | destructorsynopsis | methodsynopsis | formalpara | para | simpara | address | blockquote | graphic | graphicco | mediaobject | mediaobjectco | informalequation | informalexample | informalfigure | informaltable | equation | example | figure | table | msgset | procedure | sidebar | qandaset | anchor | bridgehead | remark | highlights | abstract | authorblurb | epigraph | indexterm | beginpage)+ , refsect2*) | refsect2+)), got (section )
/tmp/x.xml:26852: element refsect1: validity error : Element refsect1 content does not follow the DTD, expecting (refsect1info? , (title , subtitle? , titleabbrev?) , (((calloutlist | glosslist | itemizedlist | orderedlist | segmentedlist | simplelist | variablelist | caution | important | note | tip | warning | literallayout | programlisting | programlistingco | screen | screenco | screenshot | synopsis | cmdsynopsis | funcsynopsis | classsynopsis | fieldsynopsis | constructorsynopsis | destructorsynopsis | methodsynopsis | formalpara | para | simpara | address | blockquote | graphic | graphicco | mediaobject | mediaobjectco | informalequation | informalexample | informalfigure | informaltable | equation | example | figure | table | msgset | procedure | sidebar | qandaset | anchor | bridgehead | remark | highlights | abstract | authorblurb | epigraph | indexterm | beginpage)+ , refsect2*) | refsect2+)), got (table )
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
Documentation/DocBook/media/v4l/common.xml | 2 +-
Documentation/DocBook/media/v4l/io.xml | 4 +--
.../DocBook/media/v4l/pixfmt-srggb10alaw8.xml | 2 +-
Documentation/DocBook/media/v4l/v4l2.xml | 5 +---
Documentation/DocBook/media/v4l/vidioc-expbuf.xml | 28 +++++++++-----------
5 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/common.xml b/Documentation/DocBook/media/v4l/common.xml
index 73c6847..ae06afb 100644
--- a/Documentation/DocBook/media/v4l/common.xml
+++ b/Documentation/DocBook/media/v4l/common.xml
@@ -609,7 +609,7 @@ to zero and the <constant>VIDIOC_G_STD</constant>,
<para>Applications can make use of the <xref linkend="input-capabilities" /> and
<xref linkend="output-capabilities"/> flags to determine whether the video standard ioctls
are available for the device.</para>
-&ENOTTY;.
+
<para>See <xref linkend="buffer" /> for a rationale. Probably
even USB cameras follow some well known video standard. It might have
been better to explicitly indicate elsewhere if a device cannot live
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 2c4646d..e6c5855 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -477,7 +477,7 @@ rest should be evident.</para>
<note>
<title>Experimental</title>
- <para>This is an <link linkend="experimental"> experimental </link>
+ <para>This is an <link linkend="experimental">experimental</link>
interface and may change in the future.</para>
</note>
@@ -488,7 +488,7 @@ DMA buffer from userspace using a file descriptor previously exported for a
different or the same device (known as the importer role), or both. This
section describes the DMABUF importer role API in V4L2.</para>
- <para>Refer to <link linked="vidioc-expbuf"> DMABUF exporting </link> for
+ <para>Refer to <link linkend="vidioc-expbuf">DMABUF exporting</link> for
details about exporting V4L2 buffers as DMABUF file descriptors.</para>
<para>Input and output devices support the streaming I/O method when the
diff --git a/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml b/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml
index c934192..29acc20 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml
@@ -29,6 +29,6 @@
formats with 10 bits per color compressed to 8 bits each,
using the A-LAW algorithm. Each color component consumes 8
bits of memory. In other respects this format is similar to
- <xref linkend="V4L2-PIX-FMT-SRGGB8">.</xref></para>
+ <xref linkend="V4L2-PIX-FMT-SRGGB8"></xref>.</para>
</refsect1>
</refentry>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 8fe2942..94ab0e1 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -143,10 +143,7 @@ applications. -->
<revnumber>3.9</revnumber>
<date>2012-12-03</date>
<authorinitials>sa</authorinitials>
- <revremark>Added timestamp types to
- <structname>v4l2_buffer</structname>, see <xref
- linkend="buffer-flags" />.
- </revremark>
+ <revremark>Added timestamp types to v4l2_buffer.</revremark>
</revision>
<revision>
diff --git a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
index 72dfbd2..e287c8f 100644
--- a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
@@ -83,15 +83,14 @@ descriptor. The application may pass it to other DMABUF-aware devices. Refer to
<link linkend="dmabuf">DMABUF importing</link> for details about importing
DMABUF files into V4L2 nodes. It is recommended to close a DMABUF file when it
is no longer used to allow the associated memory to be reclaimed. </para>
-
</refsect1>
+
<refsect1>
- <section>
- <title>Examples</title>
+ <title>Examples</title>
- <example>
- <title>Exporting a buffer.</title>
- <programlisting>
+ <example>
+ <title>Exporting a buffer.</title>
+ <programlisting>
int buffer_export(int v4lfd, &v4l2-buf-type; bt, int index, int *dmafd)
{
&v4l2-exportbuffer; expbuf;
@@ -108,12 +107,12 @@ int buffer_export(int v4lfd, &v4l2-buf-type; bt, int index, int *dmafd)
return 0;
}
- </programlisting>
- </example>
+ </programlisting>
+ </example>
- <example>
- <title>Exporting a buffer using the multi-planar API.</title>
- <programlisting>
+ <example>
+ <title>Exporting a buffer using the multi-planar API.</title>
+ <programlisting>
int buffer_export_mp(int v4lfd, &v4l2-buf-type; bt, int index,
int dmafd[], int n_planes)
{
@@ -137,12 +136,9 @@ int buffer_export_mp(int v4lfd, &v4l2-buf-type; bt, int index,
return 0;
}
- </programlisting>
- </example>
- </section>
- </refsect1>
+ </programlisting>
+ </example>
- <refsect1>
<table pgwide="1" frame="none" id="v4l2-exportbuffer">
<title>struct <structname>v4l2_exportbuffer</structname></title>
<tgroup cols="3">
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [REVIEWv2 PATCH 2/2] DocBook: improve the error_idx field documentation.
2013-01-11 11:26 ` [REVIEWv2 PATCH 1/2] DocBook: fix various validation errors Hans Verkuil
@ 2013-01-11 11:26 ` Hans Verkuil
2013-01-11 12:13 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2013-01-11 11:26 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
The documentation of the error_idx field was incomplete and confusing.
This patch improves it.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
.../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 44 ++++++++++++++++----
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
index 0a4b90f..e9f9735 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
@@ -199,13 +199,43 @@ also be zero.</entry>
<row>
<entry>__u32</entry>
<entry><structfield>error_idx</structfield></entry>
- <entry>Set by the driver in case of an error. If it is equal
-to <structfield>count</structfield>, then no actual changes were made to
-controls. In other words, the error was not associated with setting a particular
-control. If it is another value, then only the controls up to <structfield>error_idx-1</structfield>
-were modified and control <structfield>error_idx</structfield> is the one that
-caused the error. The <structfield>error_idx</structfield> value is undefined
-if the ioctl returned 0 (success).</entry>
+ <entry><para>Set by the driver in case of an error. If the error is
+associated with a particular control, then <structfield>error_idx</structfield>
+is set to the index of that control. If the error is not related to a specific
+control, or the pre-validation step failed (see below), then
+<structfield>error_idx</structfield> is set to <structfield>count</structfield>.
+The value is undefined if the ioctl returned 0 (success).</para>
+
+<para>Before controls are read from/written to hardware a pre-validation step
+takes place: this checks if all controls in the list are all valid controls,
+if no attempt is made to write to a read-only control or read from a write-only
+control, and any other up-front checks that can be done without accessing the
+hardware.</para>
+
+<para>This check is done to avoid leaving the hardware in an inconsistent state due
+to easy-to-avoid problems. But it leads to another problem: the application needs to
+know whether an error came from the pre-validation step (meaning that the hardware
+was not touched) or from an error during the actual reading from/writing to hardware.</para>
+
+<para>The, in hindsight quite poor, solution for that is to set <structfield>error_idx</structfield>
+to <structfield>count</structfield> if the pre-validation failed. This has the
+unfortunate side-effect that it is not possible to see which control failed the
+pre-validation. If the pre-validation was successful and the error happened while
+accessing the hardware, then <structfield>error_idx</structfield> is less than
+<structfield>count</structfield> and only the controls up to
+<structfield>error_idx-1</structfield> were read or written correctly, and the
+state of the remaining controls is undefined.</para>
+
+<para>Since <constant>VIDIOC_TRY_EXT_CTRLS</constant> does not access hardware
+there is also no need to handle the pre-validation step in this special way,
+so <structfield>error_idx</structfield> will just be set to the control that
+failed the pre-validation step instead of to <structfield>count</structfield>.
+This means that if <constant>VIDIOC_S_EXT_CTRLS</constant> fails with
+<structfield>error_idx</structfield> set to <structfield>count</structfield>,
+then you can call <constant>VIDIOC_TRY_EXT_CTRLS</constant> to try to discover
+the actual control that failed the pre-validation step. Unfortunately, there
+is no <constant>TRY</constant> equivalent for <constant>VIDIOC_G_EXT_CTRLS</constant>.
+</para></entry>
</row>
<row>
<entry>__u32</entry>
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [REVIEWv2 PATCH 2/2] DocBook: improve the error_idx field documentation.
2013-01-11 11:26 ` [REVIEWv2 PATCH 2/2] DocBook: improve the error_idx field documentation Hans Verkuil
@ 2013-01-11 12:13 ` Laurent Pinchart
2013-01-11 12:22 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2013-01-11 12:13 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil
Hi Hans,
Thanks for the patch. This is much better in my opinion, please see below for
two small comments.
On Friday 11 January 2013 12:26:03 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The documentation of the error_idx field was incomplete and confusing.
> This patch improves it.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 44 +++++++++++++----
> 1 file changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml index
> 0a4b90f..e9f9735 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> @@ -199,13 +199,43 @@ also be zero.</entry>
> <row>
> <entry>__u32</entry>
> <entry><structfield>error_idx</structfield></entry>
> - <entry>Set by the driver in case of an error. If it is equal
> -to <structfield>count</structfield>, then no actual changes were made to
> -controls. In other words, the error was not associated with setting a
> -particular control. If it is another value, then only the controls up to
> -<structfield>error_idx-1</structfield> were modified and control
> -<structfield>error_idx</structfield> is the one that caused the error. The
> -<structfield>error_idx</structfield> value is undefined if the ioctl
> -returned 0 (success).</entry>
> + <entry><para>Set by the driver in case of an error. If the error is
> +associated with a particular control, then
> +<structfield>error_idx</structfield> is set to the index of that control.
> +If the error is not related to a specific control, or the pre-validation
> +step failed (see below), then <structfield>error_idx</structfield> is set
> +to <structfield>count</structfield>. The value is undefined if the ioctl
> +returned 0 (success).</para>
> +
> +<para>Before controls are read from/written to hardware a pre-validation
Maybe s/pre-validation/validation/ through the text ? We have a single
validation step, it feels a bit weird to talk about pre-validation when
there's no further validation :-)
> +step takes place: this checks if all controls in the list are all valid
s/all valid/valid/ ?
> +controls, if no attempt is made to write to a read-only control or read
> +from a write-only control, and any other up-front checks that can be done
> +without accessing the hardware.</para>
> +
> +<para>This check is done to avoid leaving the hardware in an inconsistent
> +state due to easy-to-avoid problems. But it leads to another problem: the
> +application needs to know whether an error came from the pre-validation
> +step (meaning that the hardware was not touched) or from an error during
> +the actual reading from/writing to hardware.</para>
> +
> +<para>The, in hindsight quite poor, solution for that is to set
> +<structfield>error_idx</structfield> to <structfield>count</structfield>
> +if the pre-validation failed. This has the unfortunate side-effect that it
> +is not possible to see which control failed the pre-validation. If the
> +pre-validation was successful and the error happened while accessing the
> +hardware, then <structfield>error_idx</structfield> is less than
> +<structfield>count</structfield> and only the controls up to
> +<structfield>error_idx-1</structfield> were read or written correctly, and
> +the state of the remaining controls is undefined.</para>
> +
> +<para>Since <constant>VIDIOC_TRY_EXT_CTRLS</constant> does not access
> +hardware there is also no need to handle the pre-validation step in this
> +special way, so <structfield>error_idx</structfield> will just be set to
> +the control that failed the pre-validation step instead of to
> +<structfield>count</structfield>. This means that if
> +<constant>VIDIOC_S_EXT_CTRLS</constant> fails with
> +<structfield>error_idx</structfield> set to
> +<structfield>count</structfield>, then you can call
> +<constant>VIDIOC_TRY_EXT_CTRLS</constant> to try to discover the actual
> +control that failed the pre-validation step. Unfortunately, there is no
> +<constant>TRY</constant> equivalent for
> +<constant>VIDIOC_G_EXT_CTRLS</constant>. </para></entry>
> </row>
> <row>
> <entry>__u32</entry>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [REVIEWv2 PATCH 2/2] DocBook: improve the error_idx field documentation.
2013-01-11 12:13 ` Laurent Pinchart
@ 2013-01-11 12:22 ` Hans Verkuil
2013-01-11 12:27 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2013-01-11 12:22 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Hans Verkuil
On Fri January 11 2013 13:13:47 Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch. This is much better in my opinion, please see below for
> two small comments.
>
> On Friday 11 January 2013 12:26:03 Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> >
> > The documentation of the error_idx field was incomplete and confusing.
> > This patch improves it.
> >
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> > .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 44 +++++++++++++----
> > 1 file changed, 37 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> > b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml index
> > 0a4b90f..e9f9735 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> > @@ -199,13 +199,43 @@ also be zero.</entry>
> > <row>
> > <entry>__u32</entry>
> > <entry><structfield>error_idx</structfield></entry>
> > - <entry>Set by the driver in case of an error. If it is equal
> > -to <structfield>count</structfield>, then no actual changes were made to
> > -controls. In other words, the error was not associated with setting a
> > -particular control. If it is another value, then only the controls up to
> > -<structfield>error_idx-1</structfield> were modified and control
> > -<structfield>error_idx</structfield> is the one that caused the error. The
> > -<structfield>error_idx</structfield> value is undefined if the ioctl
> > -returned 0 (success).</entry>
> > + <entry><para>Set by the driver in case of an error. If the error is
> > +associated with a particular control, then
> > +<structfield>error_idx</structfield> is set to the index of that control.
> > +If the error is not related to a specific control, or the pre-validation
> > +step failed (see below), then <structfield>error_idx</structfield> is set
> > +to <structfield>count</structfield>. The value is undefined if the ioctl
> > +returned 0 (success).</para>
> > +
> > +<para>Before controls are read from/written to hardware a pre-validation
>
> Maybe s/pre-validation/validation/ through the text ? We have a single
> validation step, it feels a bit weird to talk about pre-validation when
> there's no further validation :-)
OK.
> > +step takes place: this checks if all controls in the list are all valid
>
> s/all valid/valid/ ?
Indeed.
>
> > +controls, if no attempt is made to write to a read-only control or read
> > +from a write-only control, and any other up-front checks that can be done
> > +without accessing the hardware.</para>
How about adding this sentence to the end of the paragraph:
"The exact validations done during this step are driver dependent since some
checks might require hardware access for some devices, thus making it impossible
to do those checks up-front. However, drivers should make a best-effort to do
as many up-front checks as possible."
Regards,
Hans
> > +
> > +<para>This check is done to avoid leaving the hardware in an inconsistent
> > +state due to easy-to-avoid problems. But it leads to another problem: the
> > +application needs to know whether an error came from the pre-validation
> > +step (meaning that the hardware was not touched) or from an error during
> > +the actual reading from/writing to hardware.</para>
> > +
> > +<para>The, in hindsight quite poor, solution for that is to set
> > +<structfield>error_idx</structfield> to <structfield>count</structfield>
> > +if the pre-validation failed. This has the unfortunate side-effect that it
> > +is not possible to see which control failed the pre-validation. If the
> > +pre-validation was successful and the error happened while accessing the
> > +hardware, then <structfield>error_idx</structfield> is less than
> > +<structfield>count</structfield> and only the controls up to
> > +<structfield>error_idx-1</structfield> were read or written correctly, and
> > +the state of the remaining controls is undefined.</para>
> > +
> > +<para>Since <constant>VIDIOC_TRY_EXT_CTRLS</constant> does not access
> > +hardware there is also no need to handle the pre-validation step in this
> > +special way, so <structfield>error_idx</structfield> will just be set to
> > +the control that failed the pre-validation step instead of to
> > +<structfield>count</structfield>. This means that if
> > +<constant>VIDIOC_S_EXT_CTRLS</constant> fails with
> > +<structfield>error_idx</structfield> set to
> > +<structfield>count</structfield>, then you can call
> > +<constant>VIDIOC_TRY_EXT_CTRLS</constant> to try to discover the actual
> > +control that failed the pre-validation step. Unfortunately, there is no
> > +<constant>TRY</constant> equivalent for
> > +<constant>VIDIOC_G_EXT_CTRLS</constant>. </para></entry>
> > </row>
> > <row>
> > <entry>__u32</entry>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [REVIEWv2 PATCH 2/2] DocBook: improve the error_idx field documentation.
2013-01-11 12:22 ` Hans Verkuil
@ 2013-01-11 12:27 ` Laurent Pinchart
0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2013-01-11 12:27 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil
Hi Hans,
On Friday 11 January 2013 13:22:08 Hans Verkuil wrote:
> On Fri January 11 2013 13:13:47 Laurent Pinchart wrote:
> > Hi Hans,
> >
> > Thanks for the patch. This is much better in my opinion, please see below
> > for two small comments.
> >
> > On Friday 11 January 2013 12:26:03 Hans Verkuil wrote:
> > > From: Hans Verkuil <hans.verkuil@cisco.com>
> > >
> > > The documentation of the error_idx field was incomplete and confusing.
> > > This patch improves it.
> > >
> > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > > ---
> > >
> > > .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 44
> > > +++++++++++++----
> > > 1 file changed, 37 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> > > b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml index
> > > 0a4b90f..e9f9735 100644
> > > --- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> > > +++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> > > @@ -199,13 +199,43 @@ also be zero.</entry>
> > >
> > > <row>
> > >
> > > <entry>__u32</entry>
> > > <entry><structfield>error_idx</structfield></entry>
> > >
> > > - <entry>Set by the driver in case of an error. If it is equal
> > > -to <structfield>count</structfield>, then no actual changes were made
> > > -to controls. In other words, the error was not associated with setting
> > > -a particular control. If it is another value, then only the controls up
> > > -to <structfield>error_idx-1</structfield> were modified and control
> > > -<structfield>error_idx</structfield> is the one that caused the error.
> > > -The <structfield>error_idx</structfield> value is undefined if the
> > > -ioctl returned 0 (success).</entry>
> > > + <entry><para>Set by the driver in case of an error. If the error
> > > +is associated with a particular control, then
> > > +<structfield>error_idx</structfield> is set to the index of that
> > > +control. If the error is not related to a specific control, or the
> > > +pre-validation step failed (see below), then
> > > +<structfield>error_idx</structfield> is set to
> > > +<structfield>count</structfield>. The value is undefined if the ioctl
> > > +returned 0 (success).</para>
> > > +
> > > +<para>Before controls are read from/written to hardware a
> > > +pre-validation
> >
> > Maybe s/pre-validation/validation/ through the text ? We have a single
> > validation step, it feels a bit weird to talk about pre-validation when
> > there's no further validation :-)
>
> OK.
>
> > > +step takes place: this checks if all controls in the list are all valid
> >
> > s/all valid/valid/ ?
>
> Indeed.
>
> > > +controls, if no attempt is made to write to a read-only control or read
> > > +from a write-only control, and any other up-front checks that can be
> > > done
> > > +without accessing the hardware.</para>
>
> How about adding this sentence to the end of the paragraph:
>
> "The exact validations done during this step are driver dependent since some
> checks might require hardware access for some devices, thus making it
> impossible to do those checks up-front. However, drivers should make a
> best-effort to do as many up-front checks as possible."
Sounds very good to me. With all those changes,
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-11 12:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-11 11:26 DocBook fixes Hans Verkuil
2013-01-11 11:26 ` [REVIEWv2 PATCH 1/2] DocBook: fix various validation errors Hans Verkuil
2013-01-11 11:26 ` [REVIEWv2 PATCH 2/2] DocBook: improve the error_idx field documentation Hans Verkuil
2013-01-11 12:13 ` Laurent Pinchart
2013-01-11 12:22 ` Hans Verkuil
2013-01-11 12:27 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox