From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean Delvare Subject: Re: [PATCH v1 4/4] i2c-mux: i2c_add_mux_adapter() should use -1 for auto bus num Date: Thu, 14 Feb 2013 08:15:58 +0100 Message-ID: <20130214081558.5e3d636e@endymion.delvare> References: <1360778532-7480-1-git-send-email-dianders@chromium.org> <1360778532-7480-4-git-send-email-dianders@chromium.org> <511C00F4.4080708@wwwdotorg.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <511C00F4.4080708-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> Sender: linux-i2c-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Stephen Warren Cc: Doug Anderson , Wolfram Sang , Simon Glass , Naveen Krishna Chatradhi , Grant Likely , Yuvaraj Kumar , Ben Dooks , u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, Mark Brown , Girish Shivananjappa , bhushan.r-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, sreekumar.c-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, Prashanth G , Olof Johansson , Daniel Kurtz , Grant Grundler , "Ben Dooks (embedded platforms)" , Peter Korsgaard , Guenter Roeck , David Daney , Axel Lin , Stephen Warren , Barry Song List-Id: linux-i2c@vger.kernel.org On Wed, 13 Feb 2013 14:09:08 -0700, Stephen Warren wrote: > On 02/13/2013 11:02 AM, Doug Anderson wrote: > > The force_nr parameter to i2c_add_mux_adapter() uses 0 to signify that > > we don't want to force the bus number of the adapter. This is > > non-ideal because: > > * 0 is actually a valid bus number to request > > * i2c_add_numbered_adapter() (which i2c_add_mux_adapter() calls) uses > > -1 to mean the same thing. That means extra logic in > > i2c_add_mux_adapter(). > > > > Fix i2c_add_mux_adapter() to use -1 and update all mux drivers > > accordingly. > > > > Signed-off-by: Doug Anderson > > --- > > Notes: > > - If there's a good reason that force_nr uses 0 for auto then feel > > free to drop this patch. I've place it at the end of the series to > > make it easy to just drop it. > > IIRC (and I only vaguely do...) it's because: > > > diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c > > index 9f50ef0..301ed0b 100644 > > --- a/drivers/i2c/muxes/i2c-mux-gpio.c > > +++ b/drivers/i2c/muxes/i2c-mux-gpio.c > > @@ -208,7 +208,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev) > > } > > > > for (i = 0; i < mux->data.n_values; i++) { > > - u32 nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0; > > + int nr = mux->data.base_nr ? (mux->data.base_nr + i) : -1; > > Here, mux->data.base_nr is platform data (or copied directly from it), > and any field in a platform data struct stored in a global variable not > explicitly initialized would be 0, hence 0 would typically mean "no > explicit bus number desired". Since a mux can't exist without a parent > I2C bus, it's unlikely anyone would want a mux to be I2C bus 0, but > rather the parent to have that number. Yes, as I recall this is exactly the reason why the current code is the way it is. -- Jean Delvare