[PATCH] i2c: (i2c-diolan-u2c) Fix master_xfer return code

[PATCH] i2c: (i2c-diolan-u2c) Fix master_xfer return code

[Guenter Roeck]

The master_xfer function returns 0 on success. It should return the number of
successful transactions.

Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-diolan-u2c.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
index aedb94f..dae3ddf 100644
--- a/drivers/i2c/busses/i2c-diolan-u2c.c
+++ b/drivers/i2c/busses/i2c-diolan-u2c.c
@@ -405,6 +405,7 @@ static int diolan_usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
 			}
 		}
 	}
+	ret = num;
 abort:
 	sret = diolan_i2c_stop(dev);
 	if (sret < 0 && ret >= 0)
-- 
1.7.9.7

Re: [PATCH] i2c: (i2c-diolan-u2c) Fix master_xfer return code

[Shubhrajyoti Datta]

Hi Gunter,
A minor suggestion below.

On Thu, Aug 9, 2012 at 9:17 PM, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
> The master_xfer function returns 0 on success. It should return the number of
> successful transactions.
>
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-diolan-u2c.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
> index aedb94f..dae3ddf 100644
> --- a/drivers/i2c/busses/i2c-diolan-u2c.c
> +++ b/drivers/i2c/busses/i2c-diolan-u2c.c
> @@ -405,6 +405,7 @@ static int diolan_usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
>                         }
>                 }
>         }
> +       ret = num;
So if all the messages get transferred this is right.

However if only a few go through should we return the number of
successful transactions
instead of error?

>  abort:
>         sret = diolan_i2c_stop(dev);
>         if (sret < 0 && ret >= 0)
> --
> 1.7.9.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c: (i2c-diolan-u2c) Fix master_xfer return code

[Guenter Roeck]

On Fri, Aug 10, 2012 at 11:28:26AM +0530, Shubhrajyoti Datta wrote:
> Hi Gunter,
> A minor suggestion below.
> 
> On Thu, Aug 9, 2012 at 9:17 PM, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
> > The master_xfer function returns 0 on success. It should return the number of
> > successful transactions.
> >
> > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-diolan-u2c.c |    1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-diolan-u2c.c b/drivers/i2c/busses/i2c-diolan-u2c.c
> > index aedb94f..dae3ddf 100644
> > --- a/drivers/i2c/busses/i2c-diolan-u2c.c
> > +++ b/drivers/i2c/busses/i2c-diolan-u2c.c
> > @@ -405,6 +405,7 @@ static int diolan_usb_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
> >                         }
> >                 }
> >         }
> > +       ret = num;
> So if all the messages get transferred this is right.
> 
> However if only a few go through should we return the number of
> successful transactions
> instead of error?
> 
Most other I2C bus drivers do the same, so I decided to not make the code
more complicated than necessary and do it the same way. Also, I prefer to have
the actual error code returned to the caller, not "I transferred x of your y
messages, but I won't tell you what went wrong with the rest".

If that prevents it from getting integrated, I'll be happy to change it. 

Thanks,
Guenter

Re: [PATCH] i2c: (i2c-diolan-u2c) Fix master_xfer return code

[Shubhrajyoti Datta]

On Fri, Aug 10, 2012 at 10:36 PM, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
[...]
>>
>> However if only a few go through should we return the number of
>> successful transactions
>> instead of error?
>>
> Most other I2C bus drivers do the same, so I decided to not make the code
> more complicated than necessary and do it the same way. Also, I prefer to have
> the actual error code returned to the caller, not "I transferred x of your y
> messages, but I won't tell you what went wrong with the rest".

depends on what the user decides however if ever his algo is that resend only
the remaining it would never work. Anyways thats a different story.

>
> If that prevents it from getting integrated, I'll be happy to change it.

I have no objections to the patch getting integrated.
Afterall anything that helps, helps:-)


>
> Thanks,
> Guenter

Re: [PATCH] i2c: (i2c-diolan-u2c) Fix master_xfer return code

[Guenter Roeck]

On Mon, Aug 13, 2012 at 11:57:15AM +0530, Shubhrajyoti Datta wrote:
> On Fri, Aug 10, 2012 at 10:36 PM, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
> [...]
> >>
> >> However if only a few go through should we return the number of
> >> successful transactions
> >> instead of error?
> >>
> > Most other I2C bus drivers do the same, so I decided to not make the code
> > more complicated than necessary and do it the same way. Also, I prefer to have
> > the actual error code returned to the caller, not "I transferred x of your y
> > messages, but I won't tell you what went wrong with the rest".
> 
> depends on what the user decides however if ever his algo is that resend only
> the remaining it would never work. Anyways thats a different story.
> 
Worse, many callers don't check if the return code matches the number of
messages. So errors can get lost if the return code after an error is not
negative.

Guenter

> >
> > If that prevents it from getting integrated, I'll be happy to change it.
> 
> I have no objections to the patch getting integrated.
> Afterall anything that helps, helps:-)
> 
> 
> >
> > Thanks,
> > Guenter
> 

Re: [PATCH] i2c: (i2c-diolan-u2c) Fix master_xfer return code

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

On Thu, Aug 09, 2012 at 08:47:20AM -0700, Guenter Roeck wrote:
> The master_xfer function returns 0 on success. It should return the number of
> successful transactions.
> 
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

Applied to -current, thanks! To add something to the discussion, an
excerpt for i2c-core.c:

1357 int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
1358 {
1359         int ret;
1360 
1361         /* REVISIT the fault reporting model here is weak:
1362          *
1363          *  - When we get an error after receiving N bytes from a slave,
1364          *    there is no way to report "N".
1365          *
1366          *  - When we get a NAK after transmitting N bytes to a slave,
1367          *    there is no way to report "N" ... or to let the master
1368          *    continue executing the rest of this combined message, if
1369          *    that's the appropriate response.
1370          *
1371          *  - When for example "num" is two and we successfully complete
1372          *    the first message but get an error part way through the
1373          *    second, it's unclear whether that should be reported as
1374          *    one (discarding status on the second message) or errno
1375          *    (discarding status on the first one).
1376          */
1377 

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]