I2C implementation for STM32

[Badger]

I've been using it a lot and I'd say it is working very nicely (using both I2C1 and I2C2 with a gyro, accelerometer and compass), and fairly easy to integrate. The last time I checked the test application in the i2c branch was broken because the i2c driver had changed since it was last updated, but I managed to work out how to get it going. I can post some sample code if that helps (the main point of confusion for me was that you have to set rx_bytes = 0 if you just want to transmit data, otherwise a write sequence will automatically take place).

[Giovanni]

Please go ahead and post any experience you had with the current implementation, I've had not time to review the code yet. This could be helpful for a lot of users.

Giovanni

[Badger]

Here's an example of how I've been using the i2c driver:

First set the driver to use 400khz fast mode on I2C1:

Code: Select all

static I2CConfig i2cfg1 = {
      .opMode          = opmodeI2C,
      .ClockSpeed       = 400000,
      .FastModeDutyCycle    = fastDutyCycle_2,
      .OwnAddress7       = 0,
      .OwnAddress10       = 0,
      .Ack            = 0,
      .nBitAddress      = 7
};

Then start the driver and wait for it to initialise:

Code: Select all

i2cStart(&I2CD1, &i2cfg1);
while(I2CD1.id_state != I2C_READY){
   chThdSleepMilliseconds(5);
}

Configure the ITG3200 slave struct:

Code: Select all

static i2cblock_t itg3200_txbuf[4];
static i2cblock_t itg3200_rxbuf[6];

static I2CSlaveConfig itg3200 = {
   .id_callback             = itg3200_cb,
   .id_err_callback          = itg3200_error_cb,
   .txbytes               = 2,
   .rxbytes               = 6,
   .txbuf                  = itg3200_txbuf,
   .rxbuf                  = itg3200_rxbuf,
   .slave_addr               = (ITG3200_ADDR),
   .nbit_address            = 7,
   .errors                  = 0,
   .flags                  = 0,
   .sevent                  = { .es_next = NULL }
};

I'm also using two call backs. The important one is itg3200_cb; it passes a pointer to the received data to the thread which is issuing the i2c calls. You could also make it wake the thread which is to process the data.

Code: Select all

static void itg3200_error_cb(I2CDriver *i2cp, I2CSlaveConfig *i2cscfg) {
   (void)i2cscfg;
   int status = 0;
   status = i2cp->id_i2c->SR1;
   while(TRUE);
}

static void itg3200_cb(I2CDriver *i2cp, I2CSlaveConfig *i2cscfg) {
   (void)i2cscfg;
   i2cReleaseBus(i2cp);

    if (gyro_thd != NULL) {
       ((imu_raw_readings_t*)gyro_thd->p_msg)->gyroData = itg3200.rxbuf;
      gyro_thd = NULL;
    }
}

To initialise the slave structure and device, I disable callbacks and perform an i2c operation which loops until it completes. In this example I write one byte (the devid register address) and then read one byte (the device id from the chip):

Code: Select all

   chEvtInit(&itg3200.sevent);
   itg3200.id_callback = NULL;

   while(I2CD2.id_state != I2C_READY)
      chThdSleepMilliseconds(1);

   /* lock bus */
   i2cAcquireBus(&I2CD2);

   // see if we find the gyro
   itg3200.txbytes = 1;
   itg3200.rxbytes = 1;
   itg3200.txbuf[0] = 0x00;

   i2cMasterTransmit(&I2CD2, &itg3200);
   while(I2CD2.id_state != I2C_READY)
      chThdSleepMilliseconds(1);

   /* check the received data matches expected device ID */
   if((itg3200.rxbuf[0] & ~0x01) != 0x68) {
      i2cReleaseBus(&I2CD2);
      return itg3200.rxbuf[0];
   }

If I just want to write some bytes but not read anything I do:

Code: Select all

   /* reset the gyro */
   itg3200.txbytes = 2;
   itg3200.rxbytes = 0;
   itg3200.txbuf[0] = ITG3200_PWR_MGM;
   itg3200.txbuf[1] = ITG3200_RESET;

   i2cMasterTransmit(&I2CD2, &itg3200);
   while(I2CD2.id_state != I2C_READY)
      chThdSleepMilliseconds(1);

Generally speaking, you don't want to wait until the i2c operation completes, so the callback is used:

Code: Select all

   // send the address we want to read from
   i2cAcquireBus(&I2CD2);

   itg3200.txbytes = 1;
   itg3200.txbuf[0] = ITG3200_GYRO_XOUT_H;
   itg3200.rxbytes = 6;

   itg3200.id_callback = itg3200_cb;

   /* start reading */
   i2cMasterTransmit(&I2CD2, &itg3200);

[barthess]

Hi, all. Sorry for the long absence.

The first thing I want to thank Alberto for his low level driver.
After the long and unsuccessful attempts to polish my
own realization I have decided to use his work with
my small hi level parts.
Examples in "testhal/STM32/I2C" dir also updated and tested
on the real hardware.

Giovanni, is it better to combine "slave_addr" and "nbit_address"
fields of I2CSlaveConfig in one uint16_t (using bitmask),
or leave it separated?

[Giovanni]

Hi barthess,

I don't know, I haven't had the time to study the driver yet, in general you want to save space in RAM structures.

From what I read the driver works but there are some "compatibility" issues, it looks a bit different from the other drivers:
- Naming conventions: structure fields should not use camel case for example.
- Single huge configuration structure (non constant) that includes about everything, some of those fields could/should be function parameters. I would try to have a const slave descriptor in order to save RAM and initialization.
- Are we sure that all the I2C scenarios (any combination of read, write, restarts) is covered by i2CMasterTransmit() ? I am not enough expert of I2C to say.
- Having to poll after i2cStart() for I2C_READY is very unlike the other drivers, usually there is a transient state internally to xxxStart(), if I remember well the CAN driver does that during canStart().

Anyway, I am glad that users have something to sue right now.

Giovanni

[barthess]

Hi all.
First, API BROKEN again (from rev. 3055).

I don't know, I haven't had the time to study the driver yet, in general you want to save space in RAM structures.

Merged together.

- Naming conventions: structure fields should not use camel case for example.

Fixed.

- Having to poll after i2cStart() for I2C_READY is very unlike the other
drivers, usually there is a transient state internally to xxxStart(),
if I remember well the CAN driver does that during canStart().

Hmm... That dead code was because of my carelessness. There is no need to poll.
Examples fixed.

- Single huge configuration structure (non constant) that includes
about everything, some of those fields could/should be function parameters.
I would try to have a const slave descriptor in order to save RAM and initialization.

Seting const qualifier for driver config is not a problem (already done).
But to do this for a slave config will too hard. Rxbytes and txbytes counters must
be preserved somewhere between context switches any way. Also, slave address could be
convenient to change if I have some identical devices with
different addresses. For example, 27 TMP175 sensors on single bus.

- Are we sure that all the I2C scenarios (any combination of read,
write, restarts) is covered by i2CMasterTransmit() ? I am not
enough expert of I2C to say.

This is most interesting question. In generally, I'm not sure that
it covers all known devices manufactured today.
But, driver works with my MAX1236, TMP75 on the first interface and
BMP085, LIS3LV02DQ on the second one.

[Badger]

There's a bug in i2c.c with the mutex code. line 282 is:

Code: Select all

chMtxLock(&i2cp->mutex);

when it should be:

Code: Select all

chMtxLock(&i2cp->id_mutex);

[barthess]

There's a bug in i2c.c with the mutex code. line 282 is:

Code: Select all

chMtxLock(&i2cp->mutex);

when it should be:

Code: Select all

chMtxLock(&i2cp->id_mutex);

Fixed.

[barthess]

Some news.
API broken again. I have move some fields from I2CSlaveConfig structure to the driver.
Now it has const qualifier.

Results: ram usage reduced by 88 bytes, rom usage reduced by 334 bytes.
This results was given on testhal/STM32/I2C project when compiled
with -o0 flag.

[Giovanni]

I don't know much about the driver yet but I like when the code size decreases

Good job.

Giovanni