Hi Jean, >>> Jean Delvare <khali@xxxxxxxxxxxx> schrieb am Samstag, 11. Dezember 2010 um 17:23 in Nachricht <20101211172336.35c434ef@xxxxxxxxxxxxxxxx>: > Hi Matthias, > > On Fri, 10 Dec 2010 16:21:43 +0100, Matthias Zacharias wrote: >> Hi Jean, >> >> >>> Jean Delvare <khali@xxxxxxxxxxxx> schrieb am Montag, 6. Dezember >> 2010 um 11:25 in Nachricht <20101206112557.3dc8ffe3@xxxxxxxxxxxxxxxx> >> > I know that. My question was, did you try an alternative way? For >> > example you could use the i2cget command line tool (part of >> > i2c-tools) for user-space access. It would be interesting to see if >> > it makes a difference. >> >> I don't get the i2c-tools compiled for my armv5l platform. So can't try >> to work with these tools. Do you know how to get the i2c-tools working >> on an ARM9 microcontroller. > > How do you expect me to help you here? You did not provide any hint at > what is failing. I don't have any ARM platform at hand. > >> I think it's a problem of setting the right configuration. > > "Setting the right configuration" is awfully vague. What do you mean? > If you have trouble building i2c-tools, please start a new discussion > thread for that with all the details and I'll look into it. > I'll start a new thread for compiling the i2c-tools on ARM platforms >> > You really want a bidirectional SCL on the master side, otherwise >> > reliability cannot be guaranteed. In fact, i2c-algo-bit with no SCL >> > read callback is not I2C-compliant. It's just a hack because >> > sometimes we have no other way, but in real systems you really don't >> > want to do that. Maybe I should make i2c-algo-bit complain when SCL >> > read callback is missing, to make it more obvious. >> > >> > So please fix this, set a proper callback for SCL read. This alone >> > might fix your problem. >> >> I remove the SCL Output only setting and as open drain line. > > Good. > >> Then I take some screenshots with the I2C analyzer. using series >> resistors on the SMBus I could visualize which of the devices connected >> to the SMBus throws the SCL or the SDA. The higher logic-Low-level is >> thrown by the SMBus slave and the lower from the SMBus-master. > > My electronics knowledge is fairly limited, I didn't even know this was > possible. So I(ll trust you ;) > >> I drop these screenshots on my Dropbox: >> http://www.dropbox.com/gallery/16457261/1/I2C_2_MLX90614?h=8e2a46 > > Interesting. I see that you included both traces from the MLX90614 and > traces from the EEPROM, and both exhibit the problem. This, and the fact > that the clock line can be stuck both high and low, clearly points at > the master. > > If EEPROM access still works fine, this is probably because the EEPROM > is more tolerant to clock stalls. > > If I read the screenshots correctly, the problem only happens when SCL > stalls high, not low, and only for the MLX90614, right? > > This would match the timeouts documented in the MLX90614 datasheet: > only 50 Âs when SCL is high. This matches the SMBus specification, BTW, > while I2C doesn't have any such timeout. > >> Where are the these callback functions located? In i2c-algo-bits, >> i2c-core, or i2c-gpio? > > Your code (user-space or kernel driver) calls into i2c-core, which > calls i2c-algo-bit. The functions in i2c-algo-bit ultimately call into > i2c-gpio, which forwards the requests directly to the low-level GPIO > code. > > Such timing problems can't be caused by i2c-core, which is way too high > level. i2c-gpio is only a middle man between the software bit-banging > logic (i2c-algo-bit) and the physical level (gpio), so I doubt anything > can be wrong there. If something is actually wrong in i2c-gpio, it > would be in the initial setup (i2c_gpio_probe), not the callbacks. I > definitely hope that you already double-checked that the setup data for > i2c-gpio is correct (including pdata->sda_is_open_drain and > pdata->scl_is_op en_drain). > > So the suspects are i2c-algo-bit and the low-level GPIO code. > > For i2c-algo-bit, as I already said, the known issue is that the bus is > software-driven, and the code driving the bus can be preempted by an > interrupt. I already suggested that you should check what interrupts > are happening on your system during the I2C transfers. > > The low-level GPIO code is suspect too. i2c-algo-bit assumes that > setting SDA or SCL is very fast, and reading them too. If any of the > callbacks takes a significant amount of time, then the timing > assumptions made by i2c-algo-bit will tear apart. Unfortunately I am not > familiar with GPIOs, and I have no idea how to accurately measure how > long the GPIO callbacks last. > > Actually, what I see on your analyzer screenshots could be caused by > either of these two problems. > >> (...) >> When printk output is called and as well in the debug output >> functions, the SCL signal is strechted until the output function has >> finished to send data to the console on ttyS0. > > This is correct. But in your case, the problem happens even without > printk being ever called, right? So that's not the problem. > > Here is a patch I came up with, which I would like you to try. It blocks > interrupts while SCL is high. Please use your I2C analyzer again with > this patch applied. What I expect is that you will still see stretched > clocks, but only when SCL is low. The timeout of the MLX90614 is much > higher when SCL is low, so I hope that the clock stretching will be > properly handled this time and the MLX90614 will work fine. > > --- > drivers/i2c/algos/i2c-algo-bit.c | 17 +++++++++++++++++ > include/linux/i2c-algo-bit.h | 3 +++ > 2 files changed, 20 insertions(+) > > --- linux-2.6.27.orig/drivers/i2c/algos/i2c-algo-bit.c 2010-12-11 16:09:18.000000000 > +0100 > +++ linux-2.6.27/drivers/i2c/algos/i2c-algo-bit.c 2010-12-11 16:52:30.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 ----------------------------------------------- */ > @@ -164,15 +165,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: > @@ -182,11 +186,14 @@ static int i2c_outb(struct i2c_adapter * > * the whole (combined) message and *NOT* issue STOP. > */ > scllo(adap); > + spin_unlock_irqrestore(&adap->lockscl, flags); > } > 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; > } > > @@ -198,6 +205,7 @@ static int i2c_outb(struct i2c_adapter * > ack ? "A" : "NA"); > > scllo(adap); > + spin_unlock_irqrestore(&adap->lockscl, flags); > return ack; > /* assert: scl is low (sda undef) */ > } > @@ -210,19 +218,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_irqr estore(&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 +397,20 @@ 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); > + spin_unlock_irqrestore(&adap->lockscl, flags); > return 0; > } > > @@ -603,6 +619,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 2008-04-17 04:49:44.000000000 > +0200 > +++ linux-2.6.27/include/linux/i2c-algo-bit.h 2010-12-11 16:53:22.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, 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. best regards Matthias Zacharias -------------------- 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
