Message-ID: <20130330063340.GA3469@the-dreams.de>
Date:	Sat, 30 Mar 2013 07:33:40 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Lars-Peter Clausen <lars@...afoo.de>
Cc:	Ben Dooks <ben-linux@...ff.org>, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/6]  Make return type of i2c_del_adapter() (and
 i2c_del_mux_adapter()) void

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists