* 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