Re: [patch 2.6.26-rc1-git] i2c-core: return -Errno, not -1

Jean Delvare
2008-05-17T06:15:37Z

On Fri, 16 May 2008 09:14:58 +0200, Jean Delvare wrote:
> Hi David,
>
> On Sun, 11 May 2008 15:16:07 -0700, David Brownell wrote:
> > More updates to the I2C stack's fault reporting: make the core stop
> > returning "-1" (usually "-EPERM") for all faults. Instead, pass lower
> > level fault code up the stack, or return some appropriate errno.
> >
> > This patch happens to touch almost exclusively SMBus calls.
> >
> > Signed-off-by: David Brownell <dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> > ---
> > This is an updated version of:
> > http://marc.info/?l=i2c&m=120984528415259&w=2
> > with some return codes updated to address feedback from the similar
> > patch for the x86 I2C/SMBus adapters.
> >
> > drivers/i2c/i2c-core.c | 78 +++++++++++++++++++++++++++----------------------
> > 1 file changed, 44 insertions(+), 34 deletions(-)
> >
>
> Looks very good, with just one suggested change:
>
> > --- g26.orig/drivers/i2c/i2c-core.c 2008-05-11 13:07:56.000000000 -0700
> > +++ g26/drivers/i2c/i2c-core.c 2008-05-11 15:09:00.000000000 -0700
> > (...)
> > @@ -1568,7 +1576,7 @@ static s32 i2c_smbus_xfer_emulated(struc
> > default:
> > dev_err(&adapter->dev, "smbus_access called with invalid size (%d)\n",
> > size);
> > - return -1;
> > + return -EOPNOTSUPP;
> > }
>
> I'd rather use -EINVAL here. (...)

Hmh, scratch this. Thinking about it some more (night helps)
-EOPNOTSUPP is consistent with what we did for the bus drivers after
all, and it also anticipates addition of new transaction types to
<linux/i2c.h> or removal of support for some transaction types from
i2c_smbus_xfer_emulated() (which might as well happen, if you look
carefully you'll see that there are no in-kernel users of transaction
types I2C_SMBUS_PROC_CALL and I2C_SMBUS_BLOCK_PROC_CALL so we might
decide to remove thir support from i2c_smbus_xfer_emulated someday.)

> (..)
> BTW, feel free to adjust the debugging message above, as you did in
> some bus drivers already, to clearly say that we're speaking about a
> transaction type and not a "size".
>
> All the rest looks OK to me from a functional point of view. As far as
> style is concerned, please keep the alignment on opening parenthesis
> when the original code did that. i2c-core uses this style consistently
> and I like it.

I'll do all this myself now, no need to resend.

Thanks,