All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  parent reply	other threads:[~2013-03-30  6:33 UTC|newest]

Thread overview: 42+ 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-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-30 18:17       ` Jean Delvare
2013-03-09 18:16 ` [PATCH 3/6] i2c: Ignore return value of i2c_del_adapter() Lars-Peter Clausen
2013-03-31  8:25   ` Jean Delvare
     [not found]   ` <1362853009-20789-4-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2013-03-11 17:52     ` Ben Hutchings
2013-03-11 17:52       ` Ben Hutchings
2013-03-30  6:34     ` Wolfram Sang
2013-03-30  6:34       ` Wolfram Sang
2013-03-30  7:55     ` Jean Delvare
2013-03-30  7:55       ` Jean Delvare
     [not found]       ` <20130330085539.442321fa-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-03-30  8:11         ` Lars-Peter Clausen
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-30  8:43               ` Jean Delvare
2013-03-31 13:12     ` Shawn Guo
2013-03-31 13:12       ` Shawn Guo
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-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-13  6:05       ` Guenter Roeck
2013-03-31  9:23     ` [PATCH 5/6] " Jean Delvare
2013-03-31  9:23       ` 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-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:13     ` 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  7:56           ` Jean Delvare
2013-03-31 11:30   ` Jean Delvare
2013-03-31 11:30     ` Jean Delvare
2013-04-02  5:09   ` Wolfram Sang
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.