Re: [PATCH v2 2/2] i2c: Multiplexed I2C bus multiplexer driver pca954x

[Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>]

Jean Delvare said the following:
> Hi Michael,
> 
> I just hit a bug...
> 
> On Tue, 04 May 2010 14:48:21 +0200, Michael Lawnick wrote:
>> +static int pca954x_select_chan(struct i2c_adapter *adap,
>> +			       void *client, u32 chan)
>> +{
>> +	struct pca954x *data = i2c_get_clientdata(client);
>> +	const struct chip_desc *chip = &chips[data->type];
>> +	u8 regval;
>> +	int ret = 0;
>> +
>> +	/* we make switches look like muxes, not sure how to be smarter */
>> +	if (chip->muxtype == pca954x_ismux)
>> +		regval = chan | chip->enable;
>> +	else
>> +		regval = 1 << chan;
>> +
>> +	/* Only select the channel if its different from the last channel */
>> +	if (data->last_chan != chan) {
> 
> That's not right. data->last_chan is 0 after initialization or
> deselection, but selecting channel 0 is valid and must be possible.
> 
> So you either want to use a different initialization value (e.g. 0xff)
> or store the register value in data->last_chan instead of the channel
> number. I went for the latter.

ACK
-- 
KR
Michael