Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jean,

Jean Delvare said the following:
...
>>  	if (adapter->algo->smbus_xfer) {
>> -		rt_mutex_lock(&adapter->bus_lock);
>> +		i2c_mux_tree_lock(adapter);
>>
>>  		/* Retry automatically on arbitration loss */
>>  		orig_jiffies = jiffies;
>> @@ -1871,7 +1950,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>>  				       orig_jiffies + adapter->timeout))
>>  				break;
>>  		}
>> -		rt_mutex_unlock(&adapter->bus_lock);
>> +		i2c_mux_tree_unlock(adapter);
>>  	} else
>>  		res = i2c_smbus_xfer_emulated(adapter,addr,flags,read_write,
>>  					      command, protocol, data);
>
> I'm worried that this approach is risky. We now use i2c_mux_tree_*lock
> internally, but still expose the i2c_*lock_adapter() API externally.
> This API is _not_ mux-aware, and will do the wrong thing if called on a
> bus segment behind a mux.
>
> I'd rather not have two APIs when we really only need one. Let's just
> update the current public API to be mux-aware, and use that everywhere.

Agree, but I'd vote to do it not inline as its now. Otherwise we start to export
mux implementation details in i2c.h

>
>> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
>> index f4110aa..da5327f 100755
>> --- a/drivers/i2c/i2c-dev.c
>> +++ b/drivers/i2c/i2c-dev.c
>> @@ -193,12 +193,56 @@ static int i2cdev_check(struct device *dev, void *addrp)
>>  	return dev->driver ? -EBUSY : 0;
>>  }
>>
>> +static int i2cdev_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
>> +{
>> +	return adapter->dev.parent != NULL
>> +		&& adapter->dev.parent->bus == &i2c_bus_type;
>> +}
>
> This is an exact duplicate of the same function in i2c-core. I'd rather
> make it an inline function in <linux/i2c.h>, or export the one from
> i2c-core.c. We have enough duplicate code in i2c-dev.c already :(

Must this and the above be done in an extra patch or can it be mixed into this mux patch?

-- 
KR
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux