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

David Brownell
2008-05-17T13:53:18Z

On Friday 16 May 2008, Jean Delvare wrote:
> > >     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() ..

Right.

> > (..)
> > 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".

I must have missed a few. Those updates weren't initially on
a list of things-to-change, but I guess some of the issues were
egregious enough to make me add them -- after that particular
confusion percolated long enough. The third or fourth time
through a piece of code, it's a bit easier to notice that sort
of confusion. (Probably worth someone's time to switch *ALL*
drivers from calling that a "size". I think that bit of history
needs to be edited out of existence...)

> > 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.

Good. In general, when it's down to the last set of small
comments like this, it's probably easier all around if you
do stuff like that (rather than writing it up in email, having
someone like me both read it and make time to act on it, then
have you re-review). That's fairly common maintainer practice.

- Dave