I2C implementation for STM32

[barthess]

Hi all.
Driver seams to be working after some time of usage. But I have a question about "sevent" field in I2CSlaveConfig structure.
Is it really need? In what situations it will be useful?

Is it better to combine the two functions (receive and transmit) in one to save some bytes of code and ROM?
Any suggestions, questions?

[Giovanni]

Hi,

If that structure is supposed to be constant then an EventSource makes no sense there, it is written by Event APIs.

Other problems I noticed:

i2c_lld.c line 45:

static __IO uint8_t *txBuffp, *rxBuffp, *datap;

Those variables are shared among I2C1 and I2C2 interrupt handlers, concurrent I2C operations would not work because the conflict.

In all source:

// is C99 only, it must be avoided.

Giovanni

[barthess]

If that structure is supposed to be constant then an EventSource makes no sense there, it is written by Event APIs.

This field moved to driver structure.

Other problems I noticed:

i2c_lld.c line 45:

static __IO uint8_t *txBuffp, *rxBuffp, *datap;

Those variables are shared among I2C1 and I2C2 interrupt handlers, concurrent I2C operations would not work because the conflict.

Fixed. I was surprised, but ROM usage reduced by 352 bytes after fixing. RAM usage increased by 11 bytes.

In all source:
// is C99 only, it must be avoided.

Fixed.

[Giovanni]

Fixed. I was surprised, but ROM usage reduced by 352 bytes after fixing. RAM usage increased by 11 bytes.

I am not surprised, accessing variables through a pointer (pointer to the driver structure) is much more efficient than accessing static variables, 2 less instructions are required.

In general it is better to avoid static variables in RISC architectures.

Giovanni

[Badger]

I've got a problem with the latest version of the I2C driver (i had the same problem with the driver a few days ago as well).

Code: Select all

   i2cMasterTransmit(&I2CD1, &lsm303_mag, LSM_MAG_ADDR, 4, 0);
   while(I2CD1.id_state != I2C_READY) {
      chThdSleepMilliseconds(10);
   }

the state stays at I2C_ACTIVE and the while loop runs indefinitely. The annoying thing is that this doesn't happen every time.. i'm going to try and trace through and see where it gets stuck.

[barthess]

I've got a problem with the latest version of the I2C driver (i had the same problem with the driver a few days ago as well).

Code: Select all

   i2cMasterTransmit(&I2CD1, &lsm303_mag, LSM_MAG_ADDR, 4, 0);
   while(I2CD1.id_state != I2C_READY) {
      chThdSleepMilliseconds(10);
   }

the state stays at I2C_ACTIVE and the while loop runs indefinitely. The annoying thing is that this doesn't happen every time.. i'm going to try and trace through and see where it gets stuck

Some times I have similar problem with my LIS3LV02 accelerometer manufactured by ST. It looks like this scenario occurs:
MCU halted by JTAG during transmission, but accelerometer keep lines low infinitely because
it waits data from MCU. It has no internal watchdog timer like MAX1236. Also, pushing reset on my
board not help, because accelerometer reset pin leaved unconnected. Only power cycle helps in this case.

[Badger]

Some times I have similar problem with my LIS3LV02 accelerometer manufactured by ST. It looks like this scenario occurs:
MCU halted by JTAG during transmission, but accelerometer keep lines low infinitely because
it waits data from MCU. It has no internal watchdog timer like MAX1236. Also, pushing reset on my
board not help, because accelerometer reset pin leaved unconnected. Only power cycle helps in this case.

Power cycling does seem to help, but I am also experiencing other issues. They may however be related to how I am using the library so perhaps you could help me with this:

I've looked through the library code, and it seems that my understanding of how the library works is incorrect. Am I right in thinking that regardless of whether or not I2C_USE_WAIT is defined, i2cMasterTransmit() will only return once the operation has completed? at the end of the function there is a little check (and a similar one in i2cMasterRead()):

Code: Select all

  if (i2cp->id_state == I2C_COMPLETE)
    i2cp->id_state = I2C_READY;

but I can't see anywhere else that the state could change to I2C_READY. If it is indeed the case that the function is blocking, it would be good to have it work in a similar way to the SPI driver; if SPI_USE_WAIT is not sent then the read/write functions are non blocking.

Is this correct? (line 179 i2c.c)

Code: Select all

#if !I2C_USE_WAIT
  i2c_lld_wait_bus_free(i2cp);
#endif

Shouldn't that be #if I2C_USE_WAIT?

Thanks!

[Cyrille]

Hi,

Created a branch here: https://chibios.svn.sourceforge.net/svn ... es/i2c_dev

Giovanni

I have tested the code "Revision 3075: /branches/i2c_dev/testhal/STM32/I2C", and when I built I have the following errors :

- error: unknown type name 'I2CConfig'
- error: 'OPMODE_I2C' undeclared here (not in a function)
- error: 'STD_DUTY_CYCLE' undeclared here (not in a function)
- error: unknown type name 'I2CConfig'
- error: 'I2CD1' undeclared (first use in this function)

Config : ChibiOS 2.2.0 / platform STM32 (Multipilot32)

Where is the pb, I suppose that I have forgotten something ?

Cyrille

[Badger]

Where is the pb, I suppose that I have forgotten something ?

you need to check out the entire i2c branch and work from that, making sure that the makefile references the i2c branch not chibios 2.2.0

[Badger]

I've been digging through a bit more of the code, and I think that the _i2c_isr_code macro should be changed to mirror how the SPI driver works:

Code: Select all

#define _i2c_isr_code(i2cp, i2cscfg) {                                 \
  (i2cp)->id_state = I2C_COMPLETE;                                     \
  if(((i2cp)->id_slave_config)->id_callback) {                         \
    ((i2cp)->id_slave_config)->id_callback(i2cp, i2cscfg);             \
    if((i2cp)->id_state == I2C_COMPLETE)                               \
      (i2cp)->id_state = I2C_READY;                                    \
  }                                                                    \
  else                                                                 \
    (i2cp)->id_state = I2C_READY;                                      \
  _i2c_wakeup_isr(i2cp);                                               \
}

This would allow the driver to transition to the ready state without blocking the thread.