From: Wolfram Sang <wsa@the-dreams.de>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: Ben Dooks <ben-linux@fluff.org>,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void
Date: Sat, 30 Mar 2013 07:33:40 +0100 [thread overview]
Message-ID: <20130330063340.GA3469@the-dreams.de> (raw)
In-Reply-To: <1362853009-20789-1-git-send-email-lars@metafoo.de>
On Sat, Mar 09, 2013 at 07:16:43PM +0100, Lars-Peter Clausen wrote:
> Currently i2c_del_adapter() returns 0 on success and potentially an error code
> on failure. Unfortunately this doesn't mix too well with the Linux device driver
> model. An i2c_adapter is usually registered in a drivers probe callback and
> removed in the drivers remove callback. The Linux device driver model assumes
> that a device may disappear at any time, e.g. because it is on a hot-plug-able
> bus and the device is physically removed or because it is unbound from it's
> driver. This means that a driver's remove callback is not allowed to fail. This
> has lead to code fragments like the following:
>
> rc = i2c_del_adapter(&board->i2c_adap);
> BUG_ON(rc);
>
> Currently there are two code paths which can cause to i2c_del_adapter() to
> return an error code:
> 1) If the adapter that is passed to i2c_del_adapter() is not registered
> i2c_del_adapter() returns -EINVAL
> 2) If an I2C client, which is registered as a child to the adapter, implements
> the detach_adapter callback and detach_adapter returns an error the removal of
> the adapter is aborted and i2c_del_adapter() returns the error returned by the
> clients detach_adapter callback.
>
> The first probably never happens unless there is a serious bug in the driver
> calling i2c_del_adapter(), but I think even then, for the sake of sanitizing the
> API we can argue that the purpose of i2c_del_adapter() is to remove the adapter
> from the system. If the adapter currently isn't registered this becomes a no-op
> and we can return success, without having to do anything. The second also never
> happens, since the detach_adapter callback has been deprecated for quite some
> time now and there are currently no drivers left which implement it.
>
> So realisticly i2c_del_adapter() already always returns 0 and all code checking
> for errors is more or less dead code. And in fact the majority of the users of
> i2c_del_adapter() already ignore the return value, but there are still some
> drivers (both newer and older) which assume that i2c_del_adapter() might fail.
> This series aims at making it explicit that i2c_del_adapter() can not fail by
> making its return type void.
>
> The series will start by making i2c_del_adapter() always return success. For
> this it will update the case, where a non-registered adapter is passed in, to
> return 0 instead of -EINVAL and remove all code related to the unused
> detach_adapter callback. Afterward the series will update all users of
> i2c_del_adapter() to ignore the return value (since it is always 0 now). And
> finally the series will change the return type of i2c_del_adapter() to void.
>
> The same is true for i2c_del_mux_adapter() which is mostly just a wrapper
> around i2c_del_adapter(), so it is also updated in this series.
Looks great from a high level view. Will now dive into more detailed
review, but really looking good. Thanks for doing this.
next prev parent reply other threads:[~2013-03-30 6:33 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-09 18:16 [PATCH 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void Lars-Peter Clausen
2013-03-09 18:16 ` [PATCH 1/6] i2c: Remove detach_adapter Lars-Peter Clausen
[not found] ` <1362853009-20789-2-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 14:52 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 2/6] i2c: i2c_del_adapter: Don't treat removing a non-registered adapter as error Lars-Peter Clausen
[not found] ` <1362853009-20789-3-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 18:17 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 3/6] i2c: Ignore return value of i2c_del_adapter() Lars-Peter Clausen
[not found] ` <1362853009-20789-4-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-11 17:52 ` Ben Hutchings
2013-03-30 6:34 ` Wolfram Sang
2013-03-30 7:55 ` Jean Delvare
[not found] ` <20130330085539.442321fa-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-03-30 8:11 ` Lars-Peter Clausen
[not found] ` <51569E4A.2080006-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 8:43 ` Jean Delvare
2013-03-31 13:12 ` Shawn Guo
2013-03-31 8:25 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 4/6] i2c: Make return type of i2c_del_adapter() void Lars-Peter Clausen
[not found] ` <1362853009-20789-5-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-31 9:04 ` Jean Delvare
2013-03-09 18:16 ` [PATCH 5/6] i2c: Ignore the return value of i2c_del_mux_adapter() Lars-Peter Clausen
[not found] ` <1362853009-20789-6-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-13 6:05 ` [5/6] " Guenter Roeck
2013-03-31 9:23 ` [PATCH 5/6] " Jean Delvare
2013-03-09 18:16 ` [PATCH 6/6] i2c: Make the return type of i2c_del_mux_adapter() void Lars-Peter Clausen
[not found] ` <1362853009-20789-7-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-31 9:24 ` Jean Delvare
2013-03-30 6:33 ` Wolfram Sang [this message]
[not found] ` <1362853009-20789-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-30 20:13 ` [PATCH 0/6] Make return type of i2c_del_adapter() (and i2c_del_mux_adapter()) void Jean Delvare
2013-03-30 20:43 ` Lars-Peter Clausen
[not found] ` <51574E71.7010403-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-31 7:56 ` Jean Delvare
2013-03-31 11:30 ` Jean Delvare
2013-04-02 5:09 ` Wolfram Sang
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=20130330063340.GA3469@the-dreams.de \
--to=wsa@the-dreams.de \
--cc=ben-linux@fluff.org \
--cc=lars@metafoo.de \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.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;
as well as URLs for NNTP newsgroup(s).