On Sat, 19 May 2007, e9hack wrote:
> Trent Piepho wrote:
> > On Fri, 18 May 2007, Johannes Stezenbach wrote:
> >> Or you could propose a change to i2c-core to add
> >> a I2C_M_STOP flag (analogous to I2C_M_NOSTART), which
> >> then would have to be implemented by all i2c bus drivers.
> >
> > It seems like this is the only way to send multiple stops in a single
> > atomic transaction. All drivers wouldn't have to implement it, just the
> > ones where someone actually wants this ability. Few drivers actually
> > implement all of the existing i2c features.
> >
>
> It is a big difference between the I2C_M_STOP and the I2C_M_NOSTART flag. I2C_M_NOSTART is a nice feature, which a
> master may implement. If I look through the different implementations, it isn't defined exactly, what it means.
> I2C_M_STOP is a must have for the master, if a slave is connected to the bus like the stv0297.
It doesn't seem any different than I2C_M_TEN, I2C_M_NO_RD_ACK, etc. in that a
slave that needs one of these features won't work with a master (i2c_adapter
in Linux I2C terms) that doesn't implement them. There are plenty of i2c
adapters that don't even support messages longer than two bytes or
transactions with more than one or two parts.
> It isn't necessary, that I2C_M_STOP is implemented by the master itself. It is possible to implement it in i2ctransfer.
> If the flag is set, each entry of the message array is send as a single request to the underlaying transfer function of
That's a good point, i2c_tranfer() is basically:
mutex_lock(i2c_adapter->mutex);
i2c_adapter->master_xfer(...);
mutex_unlock(i2c_adapter->mutex);
The problem with stv0297 is that one wants multiple calls to master_xfer()
inside the mutex protected region. It would be easy to have this code break a
series of i2c messages into multiple calls to master_xfer() with a I2C_M_STOP
flag. Then all i2c_adapters could support I2C_M_STOP without needing to be
changed. One could also support the flag inside an i2c_adapter driver, but
there doesn't seem to be much point.
I've written a patch that implements I2C_M_STOP. It would be used like this:
char buf1[2] = {0x0b,0x3c}, buf2[1];
struct i2c_msg msgs[2] = {
{ .addr = 0x61, .buf = buf1, .len = 2, .flags = I2C_M_STOP },
{ .addr = 0x61, .buf = buf2, .len = 1, .flags = I2C_M_RD } };
i2c_transfer(adap, msgs, 2);
_Without_ the I2C_M_STOP flag, one would see a transfer like this:
i2c-adapter i2c-0: emitting start condition
i2c-adapter i2c-0: i2c_outb: 0xc2 A
i2c-adapter i2c-0: i2c_outb: 0x0b A
i2c-adapter i2c-0: i2c_outb: 0x3c A
i2c-adapter i2c-0: wrote 2 bytes
i2c-adapter i2c-0: emitting repeated start condition
i2c-adapter i2c-0: i2c_outb: 0xc3 A
i2c-adapter i2c-0: i2c_inb: 0x74 NA
i2c-adapter i2c-0: read 1 byte
i2c-adapter i2c-0: emitting stop condition
_With_ the I2C_M_STOP flag, one sees this:
i2c-adapter i2c-0: emitting start condition
i2c-adapter i2c-0: i2c_outb: 0xc2 A
i2c-adapter i2c-0: i2c_outb: 0x0b A
i2c-adapter i2c-0: i2c_outb: 0x3c A
i2c-adapter i2c-0: wrote 2 bytes
i2c-adapter i2c-0: emitting stop condition
i2c-adapter i2c-0: emitting start condition
i2c-adapter i2c-0: i2c_outb: 0xc3 A
i2c-adapter i2c-0: i2c_inb: 0x74 NA
i2c-adapter i2c-0: read 1 byte
i2c-adapter i2c-0: emitting stop condition
This would work for stv0297, wouldn't it?
--------------------------------------------------------------------
diff -r 56b4c3e8f350 drivers/i2c/i2c-core.c
--- a/drivers/i2c/i2c-core.c Sat May 19 05:00:32 2007 +0000
+++ b/drivers/i2c/i2c-core.c Sat May 19 17:35:19 2007 -0700
@@ -862,6 +862,7 @@ int i2c_transfer(struct i2c_adapter * ad
int ret;
if (adap->algo->master_xfer) {
+ int n, total = 0;
#ifdef DEBUG
for (ret = 0; ret < num; ret++) {
dev_dbg(&adap->dev, "master_xfer[%d] %c, addr=0x%02x, "
@@ -872,10 +873,28 @@ int i2c_transfer(struct i2c_adapter * ad
#endif
mutex_lock_nested(&adap->bus_lock, adap->level);
- ret = adap->algo->master_xfer(adap,msgs,num);
+ for (n=1; n<=num; n++) {
+ /* xfer the message(s) if we want a stop or if
+ it's the last message. */
+ if (n == num || (msgs[n-1].flags & I2C_M_STOP)) {
+ ret = adap->algo->master_xfer(adap,msgs,n);
+
+ total += ret;
+ if (ret < n) {
+ /* Return error code, if any */
+ if (ret < 0)
+ total = ret;
+ break;
+ }
+
+ /* Process any remaining messages */
+ msgs += n;
+ num -= n;
+ n = 0;
+ }
+ }
mutex_unlock(&adap->bus_lock);
-
- return ret;
+ return total;
} else {
dev_dbg(&adap->dev, "I2C level transfers not supported\n");
return -ENOSYS;
diff -r 56b4c3e8f350 include/linux/i2c.h
--- a/include/linux/i2c.h Sat May 19 05:00:32 2007 +0000
+++ b/include/linux/i2c.h Sat May 19 17:35:03 2007 -0700
@@ -448,6 +448,7 @@ struct i2c_msg {
#define I2C_M_IGNORE_NAK 0x1000
#define I2C_M_NO_RD_ACK 0x0800
#define I2C_M_RECV_LEN 0x0400 /* length will be first received byte */
+#define I2C_M_STOP 0x0200 /* send STOP condition after this msg */
__u16 len; /* msg length */
__u8 *buf; /* pointer to msg data */
};
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
