[i2c] [PATCH] Add polling transfer for i2c-pxa

[Mike Rapoport]

Russell King - ARM Linux wrote:
> On Mon, Nov 19, 2007 at 05:24:23PM +0200, Mike Rapoport wrote:
>> Russell King - ARM Linux wrote:
>>> A couple of other points...
>>>
>>> On Wed, Nov 14, 2007 at 09:21:59AM +0200, Mike Rapoport wrote:
>>>>  #include "i2c-core.h"
>>>> @@ -870,7 +871,24 @@ int i2c_transfer(struct i2c_adapter * adap, struct i2c_msg *msgs, int num)
>>>>  {
>>>>  	int ret;
>>>>
>>>> -	if (adap->algo->master_xfer) {
>>>> +	if (in_atomic() || irqs_disabled()) {
>>> I wonder if it'd make more sense for this to be a per-adapter flag?
>>> Eg, so you could switch the bus with the PMIC on to always use PIO
>>> transfers.
>> I'm not sure that making the bus with the PMIC on to always use PIO
>> transfers is good idea. As Eric pointed there may be other devices
>> on the same bus.
> 
> That's exactly why I'm suggesting it - so that the bus is driven in the
> same way all the time.
> 
> Normal devices shouldn't care that it's being driven in PIO mode - they
> don't know that it is.

The test for irqs_disabled() is needed to avoid mutex_lock in interrupts-off
contexts. So, probably we should have both per-adapter flag and this test. Then
we can use PIO transfers in interrupts-off contexts for any bus that supports
it, and we can always use PIO for bus that has appropriate flag set. It can be,
for instance, something like:

int i2c_transfer(struct i2c_adapter * adap, struct i2c_msg *msgs, int num)
{
	int ret;
	int (*xfer)(struct i2c_adapter *adap,struct i2c_msg *msgs, int num);

	if (in_atomic() || irqs_disabled()) {
		int retries = 1;
		if (adap->algo->pio_xfer == NULL) {
			pr_debug("%s: algo has no poll xfer", __FUNCTION__);
			return -EINVAL;
		}
		ret = mutex_trylock(&adap->bus_lock);
		if (ret) { /* We get bus_lock, no ongoing I2C activity */
			ret = adap->algo->pio_xfer(adap, msgs, num);
			mutex_unlock(&adap->bus_lock);
			return ret;
		} else {
			/* I2C activity is ongoing. */
			/* FIXME: may be we should take the lock and
			   abandon ongoing transfer */
			return -EBUSY;
		}
	} else if (adap->use_pio && adap->algo->pio_xfer != NULL) {
		xfer = adap->algo->pio_xfer;
	} else if (adap->algo->master_xfer) {
		xfer = adap->algo->master_xfer;
	} else {
		dev_dbg(&adap->dev, "I2C level transfers not supported\n");
		return -ENOSYS;
	}

#ifdef DEBUG
	for (ret = 0; ret < num; ret++) {
		dev_dbg(&adap->dev, "master_xfer[%d] %c, addr=0x%02x, "
			"len=%d%s\n", ret, (msgs[ret].flags & I2C_M_RD)
			? 'R' : 'W', msgs[ret].addr, msgs[ret].len,
			(msgs[ret].flags & I2C_M_RECV_LEN) ? "+" : "");
	}
#endif
	
	mutex_lock_nested(&adap->bus_lock, adap->level);
	ret = xfer(adap,msgs,num);
	mutex_unlock(&adap->bus_lock);
	
	return ret;
}


-- 
Sincerely yours,
Mike.