From: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
To: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
Subject: Re: [patch 2.6.26-rc1-git] i2c-core: return -Errno, not -1
Date: Sat, 17 May 2008 06:53:18 -0700 [thread overview]
Message-ID: <200805170653.19424.david-b@pacbell.net> (raw)
In-Reply-To: <20080517081537.17220cd0-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
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
_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c
next prev parent reply other threads:[~2008-05-17 13:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-05-03 20:06 [patch 2.6.25-git] i2c-core: return -Errno, not -1 David Brownell
[not found] ` <200805031306.16698.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-11 22:16 ` [patch 2.6.26-rc1-git] " David Brownell
[not found] ` <200805111516.08453.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-16 7:14 ` Jean Delvare
[not found] ` <20080516091458.28f9f5ca-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-05-17 6:15 ` Jean Delvare
[not found] ` <20080517081537.17220cd0-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2008-05-17 13:53 ` David Brownell [this message]
[not found] ` <200805170653.19424.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-05-17 14:43 ` Jean Delvare
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200805170653.19424.david-b@pacbell.net \
--to=david-b-ybekhbn/0ldr7s880joybq@public.gmane.org \
--cc=i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org \
--cc=khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox