tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Revisiting I2C bus scanning



I've been doing a fair amount of research lately to figure out why scanning of I2C busses causes so much grief. It seems there are a number of write-only devices that react badly when you try to read from them!

The Linux folks have taken an alternative approach in their i2c-tools utlities. Rather than trying to read from each possible bus address, they use the quick_write bus operation for most addresses. They note only a single device with which this approach is known to have problems (Atmel AT24RF08 EEPROM, found on some IBM ThinkPads).

I've modified the bus scan loop in sys/dev/i2c/i2c.c to use quick_write instead of receive_byte for scanning most address. I've also changed the code to skip over many addresses that have predefined functionality which should never report a match. These changes are documented in the attached i2c.c.diff file.

Unfortunately, even though our I2C framework defines a quick_write function, it doesn't work! quick_write is a simple transaction which transmits ONLY the device's 7-bit bus address plus a "zero" bit! So in effect, this is a transfer of zero command bytes followed by zero data bytes. We have a iic_smbus_quick_write() routine (and the corresponding iic_smbus_quick_read() which transmits the device's 7-bit address and a 'one' bit), which calls iic_exec() with the right arguments, but iic_exec() doesn't know how to process the operation. The attached file i2c_exec.c.diff modifies iic_exec() to handle the quick_*() calls.

This still isn't enough, however, to make it work. Many (most?) of the actual I2C drivers provide their own xxx_exec() routine rather than using the generic routine discussed above. This routine is responsible for telling the controller to execute the appropriate transaction.

All of my systems have only nfsmb controllers, so I was able to modify the nfsmb driver to add the quick_* operations to nfsmb_exec(). These changes are in the attached nfsmb.c.diff file.

Any comments? I'd like to commit these changes in the not-too-distant future. Of course, other I2C drivers will need to be modified in order to take advantage of the new scan routine, and until they're changed those drivers will fail to find some devices. But this has no impact on "production" systems, since we don't depend on the scan results for anything, and we've warned people profusely that scanning the I2C bus is potentially dangerous. As other drivers get modified to implement the quick_* operations, we can relax that warning.


-------------------------------------------------------------------------
|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:      |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul at whooppee.com   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
|                  |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------
Index: i2c.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/i2c.c,v
retrieving revision 1.22
diff -u -p -r1.22 i2c.c
--- i2c.c       29 Sep 2008 22:55:08 -0000      1.22
+++ i2c.c       24 Jan 2009 02:52:06 -0000
@@ -131,17 +131,54 @@ iic_attach(device_t parent, device_t sel
 
 #if I2C_SCAN
        if (sc->sc_type == I2C_TYPE_SMBUS) {
+               int err;
                int found = 0;
                i2c_addr_t addr;
-               uint8_t cmd = 0, val;
+               uint8_t val;
 
-               for (addr = 0x0; addr < 0x80; addr++) {
-                       /* Skip i2c Alert Response Address */
-                       if (addr == 0x0c)
+               for (addr = 0x09; addr < 0x78; addr++) {
+                       /*
+                        * Skip certain i2c addresses:
+                        *      0x00            General Call / START
+                        *      0x01            CBUS Address
+                        *      0x02            Different Bus format
+                        *      0x03 - 0x07     Reserved
+                        *      0x08            Host Address
+                        *      0x0c            Alert Response Address
+                        *      0x28            ACCESS.Bus host
+                        *      0x37            ACCESS.Bus default address
+                        *      0x48 - 0x4b     Prototypes
+                        *      0x61            Device Default Address
+                        *      0x78 - 0x7b     10-bit addresses
+                        *      0x7c - 0x7f     Reserved
+                        *
+                        * Some of these are skipped by judicious selection
+                        * of the range of the above for (;;) statement.
+                        *
+                        * if (addr <= 0x08 || addr >= 0x78)
+                        *      continue;
+                        */
+                       if (addr == 0x0c || addr == 0x28 || addr == 0x37 ||
+                           addr == 0x61 || (addr & 0x7c) == 0x48)
                                continue;
+
                        iic_acquire_bus(ic, 0);
-                       if (iic_exec(ic, I2C_OP_READ_WITH_STOP, addr,
-                           &cmd, 1, &val, 1, 0) == 0) {
+                       /*
+                        * Use SMBus quick_write command to detect most
+                        * addresses;  should avoid hanging the bus on
+                        * some write-only devices (like clocks that show
+                        * up at address 0x69)
+                        *
+                        * XXX The quick_write() is allegedly known to
+                        * XXX corrupt the Atmel AT24RF08 EEPROM found
+                        * XXX on some IBM Thinkpads!
+                        */
+                       if ((addr & 0xf8) == 0x30 ||
+                           (addr & 0xf0) == 0x50)
+                               err = iic_smbus_receive_byte(ic, addr, &val, 0);
+                       else
+                               err = iic_smbus_quick_write(ic, addr, 0);
+                       if (err == 0) {
                                if (found == 0)
                                        aprint_normal("%s: devices at",
                                                        ic->ic_devname);
Index: i2c_exec.c
===================================================================
RCS file: /cvsroot/src/sys/dev/i2c/i2c_exec.c,v
retrieving revision 1.6
diff -u -p -r1.6 i2c_exec.c
--- i2c_exec.c  11 Dec 2007 12:09:22 -0000      1.6
+++ i2c_exec.c  24 Jan 2009 02:53:24 -0000
@@ -115,6 +115,17 @@ iic_exec(i2c_tag_t tag, i2c_op_t op, i2c
                        if ((error = iic_write_byte(tag, *cmd++, flags)) != 0)
                                goto bad;
                }
+       } else if (buflen == 0) {
+               /*
+                * This is a quick_read()/quick_write() command with
+                * no command or data
+                */
+               if (I2C_OP_STOP_P(op))
+                       flags |= I2C_F_STOP;
+               if (I2C_OP_READ_P(op))
+                       flags |= I2C_F_READ;
+               if ((error = iic_initiate_xfer(tag, addr, flags)) != 0)
+                       goto bad;
        }
 
        if (I2C_OP_READ_P(op))
Index: nfsmb.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/nfsmb.c,v
retrieving revision 1.15
diff -u -p -r1.15 nfsmb.c
--- nfsmb.c     15 Oct 2008 02:21:48 -0000      1.15
+++ nfsmb.c     24 Jan 2009 02:53:59 -0000
@@ -99,7 +99,8 @@ static int
     nfsmb_read_1(struct nfsmb_softc *, uint8_t, i2c_addr_t, i2c_op_t, int);
 static int
     nfsmb_read_2(struct nfsmb_softc *, uint8_t, i2c_addr_t, i2c_op_t, int);
-
+static int 
+    nfsmb_quick(struct nfsmb_softc *, i2c_addr_t, i2c_op_t, int);
 
 CFATTACH_DECL_NEW(nfsmbc, sizeof(struct nfsmbc_softc),
     nfsmbc_match, nfsmbc_attach, NULL, NULL);
@@ -282,6 +283,10 @@ nfsmb_exec(void *cookie, i2c_op_t op, i2
        uint8_t *p = vbuf;
        int rv;
 
+       if ((cmdlen == 0) && (buflen == 0)) {
+               return nfsmb_quick(sc, addr, op, flags);
+       }
+
        if (I2C_OP_READ_P(op) && (cmdlen == 0) && (buflen == 1)) {
                rv = nfsmb_receive_1(sc, addr, op, flags);
                if (rv == -1)
@@ -344,6 +349,23 @@ nfsmb_check_done(struct nfsmb_softc *sc)
 
 /* ARGSUSED */
 static int
+nfsmb_quick(struct nfsmb_softc *sc, i2c_addr_t addr, i2c_op_t op, int flags)
+{
+       uint8_t data;
+
+       /* write smbus slave address to register */
+       data = addr << 1;
+       bus_space_write_1(sc->sc_iot, sc->sc_ioh, NFORCE_SMB_ADDRESS, data);
+
+       /* write smbus protocol to register */
+       data = I2C_OP_READ_P(op) | NFORCE_SMB_PROTOCOL_QUICK;
+       bus_space_write_1(sc->sc_iot, sc->sc_ioh, NFORCE_SMB_PROTOCOL, data);
+
+       return nfsmb_check_done(sc);
+}
+
+/* ARGSUSED */
+static int
 nfsmb_send_1(struct nfsmb_softc *sc, uint8_t val, i2c_addr_t addr, i2c_op_t op,
             int flags)
 {


Home | Main Index | Thread Index | Old Index