Rob's Technical Article: Linux, I2C, and your driver

I have recently been assigned the task of writing a number of I2C drivers for Linux. Often, such drivers are written using read and write code like the following:

static int myDevice_i2c_ctrlbyte(struct device *dev, unsigned char byte)
{
        struct i2c_client *client = to_i2c_client(dev);
        int ret = 0;
        ret = i2c_master_send(client, &byte, 1);
        if (ret < 0)
                dev_err(&client->dev, "I2C write error\n");

        return ret;
}

static int myDevice_i2c_write(struct device *dev, unsigned char reg,
                                unsigned short data)
{
        struct i2c_client *client = to_i2c_client(dev);
        int ret = 0;
        u8 *_data = (u8 *)&data;
        u8 tx[3] = { reg, _data[1], _data[0] };

        ret = i2c_master_send(client, tx, 3);
        if (ret < 0)
                dev_err(&client->dev, "I2C write error i2c_master_send returned %d\n", ret);

        return ret;
}

static int myDevice_i2c_read(struct device *dev, unsigned char command,
                                unsigned short *data)
{
        struct i2c_client *client = to_i2c_client(dev);
        u8 *_data = (u8 *)data;
        u8 flipper[2];
        int ret;

        ret = i2c_master_send(client, &command, 1);
        if (ret >= 0)
                ret = i2c_master_recv(client, flipper, 2);
        if (unlikely(ret < 0))
        {
                dev_err(&client->dev, "I2C read error\n");
        }
        else
        {
                _data[1] = flipper[0];
                _data[0] = flipper[1];
        }
        return ret;
}

... and the code that would use it operates something like this:

        myDevice->readfunc(myDevice->dev, READREG(STATUS), &status);
        if(status & STATUS_INDEX)
                myDevice->readfunc(myDevice->dev, READREG(INDEX), &idx);
        if(status & STATUS_XREADY)
                myDevice->readfunc(myDevice->dev, READREG(XRESULT), &x);
        if(status & STATUS_YREADY)
                myDevice->readfunc(myDevice->dev, READREG(YRESULT), &y);
        if(status & STATUS_Z1READY)
                myDevice->readfunc(myDevice->dev, READREG(Z1RESULT), &z1);
        if(status & STATUS_Z2READY)
                myDevice->readfunc(myDevice->dev, READREG(Z2RESULT), &z2);

...  Do some processing and then finally ...

        tsc202x->ctrlfunc(tsc202x->dev, 0x84);

Here is an example of what happens when you do this. In this case, I am transferring one byte to tell the device to send me a bunch of data and grabbing the data in one transaction. Then I am setting up for the next conversion, finally I'm commanding the conversion. These communications happen in three separate I2C transfers. Note that SCL (the red line) goes high between block transfers at the 400uS and 550uS mark. The code that I've given is actually for a theoretical chip, which does not in any way reflect the operation of the real chip which I strapped to the logic analyzer to produce these graphs.

I suggest using i2c_transfer() to redesign your driver's communications. In the above code example, we've split up each register read into two discrete communications, one to command a read from a register, and second, the actual data is read from the register. Register writes are done in one transaction, similarly, sending the command byte is one transaction. Notice, however, we're using the register read much more often. Most chips will allow you to continue reading data by requesting to read the first register, but giving a buffer sufficient to read all data addresses from that point through the last register you require. That is what I am doing in this code example.

...
        struct i2c_msg msgs[] = {
                {
                        .addr = client->addr,
                        .flags = 0,
                        .len   = 1,
                        .buf   = &statuscmd,
                },
                {
                        .addr = client->addr,
                        .flags = I2C_M_RD,     /* This indicates that we're reading, not writing with this message. */
                        .len   = 2,
                        .buf   = statflip,     /* 2 element char array, remember endianness, on LE machines you'll have to flip your bytes */
                },
                {
                        .addr = client->addr,
                        .flags = 0,
                        .len   = 1,
                        .buf   = &readreg,
                },
                {
                        .addr = client->addr,
                        .flags = I2C_M_RD,
                        .len   = 10,
                        .buf   = flipper,     /* Be careful of endians or they'll shoot errors at you; 10 element char array */
                },
                {
                        .addr = client->addr,
                        .flags = 0,
                        .len   = 1,
                        .buf   = &cmd,
                },
        };

        i2c_transfer(client->adapter, msgs, 5);  /* This call should return 5 if the transfers succeed (5 messages) */

...

In this code example, I am reading the status register, which happens to be well after the other registers that I need to read. Regardless of whether the status indicates that the data is ready, I read it anyway. I figure that if the data isn't ready, I can deal with the (potentially junk) data that I got in an appropriate manner. I feel it's safe to assume that the data is not junk at this point, considering that the interrupt is triggered by DataReaDY (DRDY). If it is junk, the status register will indicate that that particular data field is not ready, and I can disregard it. I feel that the time lost by reinitiating communications is well worth spending because the high speed command which initiates all of these communications is quite expensive.

After reading the status and the values in the registers whether they're ready or not, I finally send the command byte to get another transfer going. In this implementation, the chip manufacturer never indicated that it's OK to request a conversion before collecting data, so I don't take that risk. In the example depicted by the provided graphs, it's obvious that starting conversions before reading the data would be foolhardy; I'd still be reading out data after the next conversion completes, a little over 100uS after the conversion command is initiated.

Here is an example of the previously analyzed driver with its three I2C transfers, however, it has been rewritten. All three communications are happening in one transfer, cutting 85uS off of the total time required to get another conversion. This time savings may seem inconsequential, however, keep in mind that I'm only reducing three transactions into one. Had I done the same with the code example given, I would be cutting 12 transactions down to 1, with a theoretical time saving of about 510uS. Also, the fact that I am reading all of the data registers in the code example in one fell swoop means that I'm getting a savings of about 70uS per register, for a total of 350uS. Keep in mind that the driver which I am using on this logic analyzer is not the same driver that I'm exposing in the code; the one that I'm exposing in the code would take significantly longer to communicate.

The top blue line is the DRDY line, which is hooked up to an interrupt GPIO. The red line is SCL, and the yellow line is SDA.