2019-11-19 09:01:08

by Lei YU

[permalink] [raw]
Subject: [PATCH] docs: i2c: Fix return value of i2c_smbus_xxx functions

In i2c/dev-interface.rst it said

> All these transactions return -1 on failure

But actually the i2c_smbus_xxx functions return negative error numbers
on failure, instead of -1.

Fix the document.

Signed-off-by: Lei YU <[email protected]>
---
Documentation/i2c/dev-interface.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/i2c/dev-interface.rst b/Documentation/i2c/dev-interface.rst
index 69c23a3..73b77c3 100644
--- a/Documentation/i2c/dev-interface.rst
+++ b/Documentation/i2c/dev-interface.rst
@@ -163,8 +163,8 @@ for details) through the following functions::
__s32 i2c_smbus_write_block_data(int file, __u8 command, __u8 length,
__u8 *values);

-All these transactions return -1 on failure; you can read errno to see
-what happened. The 'write' transactions return 0 on success; the
+All these transactions return negative value on failure; you can read errno to
+see what happened. The 'write' transactions return 0 on success; the
'read' transactions return the read value, except for read_block, which
returns the number of values read. The block buffers need not be longer
than 32 bytes.
--
2.7.4



2019-11-19 09:34:37

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH] docs: i2c: Fix return value of i2c_smbus_xxx functions

On 2019-11-19 09:56, Lei YU wrote:
> In i2c/dev-interface.rst it said
>
>> All these transactions return -1 on failure
>
> But actually the i2c_smbus_xxx functions return negative error numbers
> on failure, instead of -1.
>
> Fix the document.
>
> Signed-off-by: Lei YU <[email protected]>
> ---
> Documentation/i2c/dev-interface.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/i2c/dev-interface.rst b/Documentation/i2c/dev-interface.rst
> index 69c23a3..73b77c3 100644
> --- a/Documentation/i2c/dev-interface.rst
> +++ b/Documentation/i2c/dev-interface.rst
> @@ -163,8 +163,8 @@ for details) through the following functions::
> __s32 i2c_smbus_write_block_data(int file, __u8 command, __u8 length,
> __u8 *values);
>
> -All these transactions return -1 on failure; you can read errno to see
> -what happened. The 'write' transactions return 0 on success; the
> +All these transactions return negative value on failure; you can read errno to
> +see what happened. The 'write' transactions return 0 on success; the

s/return negative/return a negative/

And the line is now too long compared to the rest of the text, so you
need to rewrap the paragraph.

And why do you need to dig around in errno if the negative errno has
already been returned?

> 'read' transactions return the read value, except for read_block, which
> returns the number of values read. The block buffers need not be longer
> than 32 bytes.
>

Hmm, unrelated, but should that perhaps be "must not" instead of "need not"?

Cheers,
Peter

2019-11-19 09:38:10

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH] docs: i2c: Fix return value of i2c_smbus_xxx functions

On 2019-11-19 10:33, Peter Rosin wrote:
>> 'read' transactions return the read value, except for read_block, which
>> returns the number of values read. The block buffers need not be longer
>> than 32 bytes.
>>
>
> Hmm, unrelated, but should that perhaps be "must not" instead of "need not"?

Ahhh, hit 'send' and it all becomes crystal clear. The *read* buffers need
not be larger than 32 bytes. Duh!

Cheers,
Peter

2019-11-19 09:47:30

by Lei YU

[permalink] [raw]
Subject: Re: [PATCH] docs: i2c: Fix return value of i2c_smbus_xxx functions

On Tue, Nov 19, 2019 at 5:33 PM Peter Rosin <[email protected]> wrote:
>
> On 2019-11-19 09:56, Lei YU wrote:
> > In i2c/dev-interface.rst it said
> >
> >> All these transactions return -1 on failure
> >
> > But actually the i2c_smbus_xxx functions return negative error numbers
> > on failure, instead of -1.
> >
> > Fix the document.
> >
> > Signed-off-by: Lei YU <[email protected]>
> > ---
> > Documentation/i2c/dev-interface.rst | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/i2c/dev-interface.rst b/Documentation/i2c/dev-interface.rst
> > index 69c23a3..73b77c3 100644
> > --- a/Documentation/i2c/dev-interface.rst
> > +++ b/Documentation/i2c/dev-interface.rst
> > @@ -163,8 +163,8 @@ for details) through the following functions::
> > __s32 i2c_smbus_write_block_data(int file, __u8 command, __u8 length,
> > __u8 *values);
> >
> > -All these transactions return -1 on failure; you can read errno to see
> > -what happened. The 'write' transactions return 0 on success; the
> > +All these transactions return negative value on failure; you can read errno to
> > +see what happened. The 'write' transactions return 0 on success; the
>
> s/return negative/return a negative/

Ack, will send v2 patch.

>
> And the line is now too long compared to the rest of the text, so you
> need to rewrap the paragraph.

In this patch it's at column 78, that should be OK.
But after adding the "a" it will exceed and will rewrap in v2 patch.

>
> And why do you need to dig around in errno if the negative errno has
> already been returned?

Yeah, good question, probably we could remove the following sentence?

>
> > 'read' transactions return the read value, except for read_block, which
> > returns the number of values read. The block buffers need not be longer
> > than 32 bytes.
> >
>
> Hmm, unrelated, but should that perhaps be "must not" instead of "need not"?
>
> Cheers,
> Peter

2019-11-19 10:27:33

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH] docs: i2c: Fix return value of i2c_smbus_xxx functions

On 2019-11-19 10:43, Lei YU wrote:
> On Tue, Nov 19, 2019 at 5:33 PM Peter Rosin <[email protected]> wrote:
>> And the line is now too long compared to the rest of the text, so you
>> need to rewrap the paragraph.
>
> In this patch it's at column 78, that should be OK.

No, that's not ok. If the rest
of the paragraph is wrapped at 72 (or something such), a single line
breaking that pattern will be
disturbing to the reader. The
fact that you don't get some
checkpatch warning has nothing
to do with that.

Cheers,
Peter