Hi Jean, Thank you for the new patch. I tried it and it works fine for our system with i2c_debug < 2. For i2c_debug >=2 the debug outputs leads to timeout condition when address should be acknowledged. I'll try to publish the "mlx90614" driver after doing some optimizations. Are you interested to review this driver before, because it is the first driver we try to publish? Do plan to include your patch on the "i2c-algo-bit" into the lastest kernel? I had applied the patch and done my tests on the 2.6.25.4 kernel, with a bunch of other patches needed for my system. Thanks a lot for your fast and good support for fixing this issue we had. Best regards Matthias Zacharias matthias.zacharias@xxxxxxxxxxxxxxxx >>> Jean Delvare <khali@xxxxxxxxxxxx> schrieb am Dienstag, 14. Dezember 2010 um 17:58 in Nachricht <20101214175828.0a62ce3f@xxxxxxxxxxxxxxxx>: > Hi Matthias, > > On Tue, 14 Dec 2010 15:14:13 +0100, Matthias Zacharias wrote: >> Thanks for your patch. It is working very well. >> But one problem is left: >> see "101214_1058_MLX_I2C_patched_0001.bmp" in my dropbox >> (http://www.dropbox.com/gallery/16457261/1/I2C_2_MLX90614?h=8e2a46) >> >> If between the read command (0x5a.0x6) and the answer (0x5a <3x >> data_bytes) the timeout condition: SCL = High for 43Âs match, the >> MLX90614 go thru reset and returns an invalid value (0xFFFF). >> >> Where can I place the call for "spin_lock_irqsave" and >> "spin_unlock_irqrestore" to block this timeout situation. > > Ah, I forgot the repeated start condition. The following updated patch > fixes this. I have also changed the code to no longer call scllo() when > the spinlock is held. That way we can release the spinlock (and thus > enable interrupts again) faster, which lowers the system latency. > > So please give a try to this new patch, and report. > > * * * * * > > Candidate fix for SCL getting stretched when high resulting in > slave timeouts. > > Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx> > --- > drivers/i2c/algos/i2c-algo-bit.c | 33 +++++++++++++++++++++++++++++---- > include/linux/i2c-algo-bit.h | 3 +++ > 2 files changed, 32 insertions(+), 4 deletions(-) > > --- linux-2.6.27.orig/drivers/i2c/algos/i2c-algo-bit.c 2010-12-14 17:34:48.000000000 > +0100 > +++ linux-2.6.27/drivers/i2c/algos/i2c-algo-bit.c 2010-12-14 17:34:50.000000000 > +0100 > @@ -30,6 +30,7 @@ > #include <linux/sched.h> > #include <linux/i2c.h> > #include <linux/i2c-algo-bit.h> > +#include <linux/spinlock.h> > > > /* ----- global defines ----------------------------------------------- */ > @@ -131,12 +132,17 @@ static void i2c_start(struct i2c_algo_bi > > static void i2c_repstart(struct i2c_algo_bit_data *adap) > { > + unsigned long flags; > + > /* assert: scl is low */ > sdahi(adap); > + spin_lock_irqsave(&adap->lockscl, flags); > sclhi(adap); > setsda(adap, 0); > udelay(adap->udelay); > - scllo(adap); > + setscl(adap, 0); > + spin_unlock_irqrestore(&adap->lockscl, flags); > + udelay(adap->udelay / 2); > } > > > @@ -164,15 +170,18 @@ static int i2c_outb(struct i2c_adapter * > int sb; > int ack; > struct i2c_algo_bit_data *adap = i2c_adap->algo_data; > + unsigned long flags; > > /* assert: scl is low */ > for (i = 7; i >= 0; i--) { > sb = (c >> i) & 1; > setsda(adap, sb); > udelay((adap->udelay + 1) / 2); > + spin_lock_irqsave(&adap->lockscl, flags); > if (sclhi(adap) < 0) { /* timed out */ > bit_dbg(1, &i2c_adap->dev, "i2c_outb: 0x%02x, " > "timeout at bit #%d\n", (int)c, i); > + spin_unlock_irqrestore(&adap->lockscl, flags); > return -ETIMEDOUT; > } > /* FIXME do arbitration here: > @@ -181,12 +190,16 @@ static int i2c_outb(struct i2c_adapter * > * Report a unique code, so higher level code can retry > * the whole (combined) message and *NOT* issue STOP. > */ > - scllo(adap); > + setscl(adap, 0); > + spin_unlock_irqrestore(&adap->lockscl, flags); > + udelay(adap->udelay / 2); > } > sdahi(adap); > + spin_lock_irqsave(&adap->lockscl, flags); > if (sclhi(adap) < 0) { /* timeout */ > bit_dbg(1, &i2c_adap->dev, "i2c_outb: 0x%02x, " > "timeout at ack\n", (int)c); > + spin_unlock_irqrestore(&adap->lockscl, flags); > return -ETIMEDOUT; > } > > @@ -197,7 +210,9 @@ static int i2c_outb(struct i2c_adapter * > bit_dbg(2, &i2c_adap->dev, "i2c_outb: 0x%02x %s\n", (int)c, > ack ? "A" : "NA"); > > - scllo(adap); > + setscl(adap, 0); > + spin_unlock_irqrestore(&adap->lockscl, flags); > + udelay(adap->udelay / 2); > return ack; > /* assert: scl is low (sda undef) */ > } > @@ -210,19 +225,23 @@ static int i2c_inb(struct i2c_adapter *i > int i; > unsigned char indata = 0; > struct i2c_algo_bit_data *adap = i2c_adap->algo_data; > + unsigned long flags; > > /* assert: scl is low */ > sdahi(adap); > for (i = 0; i < 8; i++) { > + spin_lock_irqsave(&adap->lockscl, flags); > if (sclhi(adap) < 0) { /* timeout */ > bit_dbg(1, &i2c_adap->dev, "i2c_inb: timeout at bit " > "#%d\n", 7 - i); > + spin_unlock_irqrestore(&adap->lockscl, flags); > return -ETIMEDOUT; > } > indata *= 2; > if (getsda(adap)) > indata |= 0x01; > setscl(adap, 0); > + spin_unlock_irqrestore(&adap->lockscl, flags); > udelay(i == 7 ? adap->udelay / 2 : adap->udelay); > } > /* assert: scl is low */ > @@ -385,16 +404,21 @@ static int sendbytes(struct i2c_adapter > static int acknak(struct i2c_adapter *i2c_adap, int is_ack) > { > struct i2c_algo_bit_data *adap = i2c_adap->algo_data; > + unsigned long flags; > > /* assert: sda is high */ > if (is_ack) /* send ack */ > setsda(adap, 0); > udelay((adap->udelay + 1) / 2); > + spin_lock_irqsave(&adap->lockscl, flags); > if (sclhi(adap) < 0) { /* timeout */ > dev_err(&i2c_adap->dev, "readbytes: ack/nak timeout\n"); > + spin_unlock_irqrestore(&adap->lockscl, flags); > return -ETIMEDOUT; > } > - scllo(adap); > + setscl(adap, 0); > + spin_unlock_irqrestore(&adap->lockscl, flags); > + udelay(adap->udelay / 2); > return 0; > } > > @@ -603,6 +627,7 @@ static int i2c_bit_prepare_bus(struct i2 > } > > /* register new adapter to i2c module... */ > + spin_lock_init(&bit_adap->lockscl); > adap->algo = &i2c_bit_algo; > > adap->timeout = 100; /* default values, should */ > --- linux-2.6.27.orig/include/linux/i2c-algo-bit.h 2010-12-14 17:34:49.000000000 > +0100 > +++ linux-2.6.27/include/linux/i2c-algo-bit.h 2010-12-14 17:35:30.000000000 +0100 > @@ -24,6 +24,8 @@ > #ifndef _LINUX_I2C_ALGO_BIT_H > #define _LINUX_I2C_ALGO_BIT_H > > +#include <linux/spinlock.h> > + > /* --- Defines for bit-adapters --------------------------------------- */ > /* > * This struct contains the hw-dependent functions of bit-style adapters to > @@ -36,6 +38,7 @@ struct i2c_algo_bit_data { > void (*setscl) (void *data, int state); > int (*getsda) (void *data); > int (*getscl) (void *data); > + spinlock_t lockscl; > > /* local settings */ > int udelay; /* half clock cycle time in us, -------------------- BMK electronic solutions GmbH Werner-von-Siemens-Str. 6, Eingang 18 f D-86159 Augsburg Tel. +49 (0) 821 / 207 88 - 700 Fax +49 (0) 821 / 207 88 - 721 info@xxxxxxxxxxxxxxxx GeschÃftsfÃhrer: Dipl.-oec. Alois KnÃferle Sitz: Augsburg HR-Nr.: B21197 --------------------- Diese E-mail kann vertrauliche Informationen enthalten. Falls Sie diese E-Mail irrtÃmlich erhalten haben, informieren Sie bitte unverzÃglich den Absender und lÃschen Sie diese E-Mail von jedem Rechner, auch von den Mailservern. Jede Verbreitung des Inhalts, auch die teilweise Verbreitung, ist in diesem Fall untersagt. AuÃer bei Vorsatz oder grober FahrlÃssigkeit schliessen wir jegliche Haftung fÃr Verluste oder SchÃden aus, die durch Viren befallene Software oder E-Mails verursacht werden. This e-mail may contain confidential information. If you received this e-mail in error, please contact the sender and delete this e-mail from your computer, including your mailservers. Any dissemination, even partly, is prohibited. Except in case of gross negligence or wilful misconduct we accept no liability for any loss or damage caused by software or e-mail viruses. -- 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
