Fix #1 looks fine. Fix #2 (test_bus) I'm sure is an improvement. If you are really motivated, try fixing the printk's for debug=1. As I recall from long ago some of them are not correct. If you want to be truly frightened try debug=9. As far as the timing, yes it's horrid. And not checking for the bus being high, that's a byproduct of not really implementing multi-master. See the TODO file. The actual cycle time is 2/3 of advertised (yes it's 3 states total, not 2 or 4). What you're starting to see is the problems that caused Kyosti to start a totally new driver i2c-algo-biths. I'll try and find some of the discussions about the i2c-algo-bit driver, and what Kyosti hoped to accomplish with i2c-algo-biths, and put it in the TODO file. What's in TODO now is only part of the problems. mds Jean Delvare wrote: > > > > 1* Fixed sclhi() for adapters that do not have getscl(). It looks > > > like there was a udelay call missing in this case. > > > > If you can't read SCL, you can't have a true I2C master. SCL is > > wired-AND which means that any device on the bus can hold it low > > until that device is ready. > > You're of course right. Still some masters can't read SCL back. This is > the case for the ADM1032 evaluation I received from Analog Devices. The > i2c-algo-bit module was supposed to handle that case already, so it must > not be that uncommon. > > > In theory, any I2C bus master which is implemented by bit-bashing > > yet which cannot read SCL is fundamentally broken. In practice, > > such a system will work fine if the slaves never hold the clock low. > > For bus arbitration (more than one master), the ability to read SCL > > is definitely required. > > I guess that evalution boards must be matching that criteria. Mine must > be, at least, since it's working OK. > > > > 1* I don't understand how the whole thing works. Each of sdalo(), > > > sdahi(), scllo() and sclhi() are waiting adap->udelay before > > > returning. How in this condition can the driver change both SCL and > > > SDA values in a row? > > > > Do you mean to transition SCL and SDA at the same time? It cannot, > > nor does it have to. Take another look at the I2C protocol spec, > > especially figure 4 on page 9. > > Yes, I know that. I've read the I2C spec, mind you ;) OK, I stopped > before the end, I admit. But I've at least understood that SDA changes > on SCL low, and "sampled" (if that make sense for a one-bit value) on > SCL high. > > What confuses me is that comment in i2c-algo-bit.h: > > int udelay; /* half-clock-cycle time in microsecs */ > /* i.e. clock is (500 / udelay) KHz */ > > Since each of sdalo(), sdahi(), scllo() and sclhi() is waiting udelay > before returning, the clock-cycle would more likely be four times > udelay, not two. Or am I missing something? Sending one bit on the bus > being setting SCL low, setting SDA to the wanted value, setting SCL > high, waiting for SDA to be sampled. Four times udelay, no less. Please > tell me if I'm wrong somehow. > > > > 2* It looks like i2c-algo-bit assumes that both SDA and SCL are high > > > by the time it is called. Why that? Looks like not all drivers will > > > do so. > > > > Because that is the bus idle condition? If the hardware has pullups > > on SCL and SDA then the driver doesn't need to do anything to cause > > bus idle. OTOH if SCL/SDA are floating, then the driver might have > > to manually establish bus idle first. Again, I haven't looked at the > > source... but it makes sense if that's how it's implemented. The > > algorithm simply expects an idle bus. The driver does whatever it > > must do (possibly nothing) to create that condition. > > That makes sense - providing all bus drivers play the game. The > parallel-port bus driver, for example, has to set the lines high > manually. The pins have to be set or not, they can't be left floating. > > > > Especially, I tried my changes using i2c-matroxfb and it looks like > > > this driver doesn't set SDA not SCL before registering with > > > i2c-algo-bit, causing bit_test to sometimes fail with the message > > > "seems to be busy". I wonder if it wouldn't be easier (and safer) > > > not to assume anything about SDA and SCL in > > > i2c-algo-bit.c:test_bus() (that is, remove that"may be busy" test). > > > > In this case, do you remove and reload the modules, or are you forced > > to reboot the machine? The bus really might just be stuck. The I2C > > bus is especially vulnerable to SCL stuck low because start, idle, and > > stop conditions are all impossible in that case; and the > > wired-AND-ness of SCL means it only takes one confused slave to kill > > the bus. > > Reloading the i2c-matroxfb module didn't help, but reloading > i2c-algo-bit without the bit_test parameter worked without a hitch. > That's what made me think the test wasn't good. Now I understand that > i2c-algo-bit doesn't want to trouble the bus if someone else is talking > (multi-master case for example), but in my case it looks like > something's not working as intended. I don't see what bad could happen > setting the lines high at the beginning of the test. If someone else is > holding the line low at the same time, if my understanding of the I2C > bus is correct, that other device won't even notice it, since setting > low always wins over setting high. This is why I believe we could set > the lines high in i2c-algo-bit rather than in drivers (or at least set > the lines high before trying to test wether the lines are stuck or not). > > Of course, the other possibility is to fix i2c-matroxfb, if it is > actually broken. > > Thanks for your advice :) > > -- > Jean Delvare > http://www.ensicaen.ismra.fr/~delvare/
