I2C implementation for STM32

[Giovanni]

I adjusted some comments.

It has been a long process but this should be about final, could everybody give barthess and me some feedback about this driver? there is something else that could be done on it?

Giovanni

[Giovanni]

Question,

You wrote in the I2C documentation:

Code: Select all

 * @note    You must set I2C interrupts priority to highest level in the
 *          system. 

Is this still the case? if so what is the reason and is there any workaround?

Also, I modified the state diagram as follow:

i2c.jpg (34.39 KiB) Viewed 9887 times

This is done in order to force a driver restart if a timeout happens, because it is an anomalous condition that triggers an I2C hardware reset and that requires a driver restart.

Giovanni

[barthess]

Is this still the case?

Interesting question.
Some slaves can successfully recover from long preemption, some can not. I have performed test with irq storm. From my 6 slaves TMP75, MMA8452Q, MAX1236, MAG3110, BMP085 recover successfully but ITG3200 cause acknowledge fail error. So it totally depends on slaves. No workarounds. Most safe option is to set interrupts priority to highest level. In my case it was enough to set the priority of IRQ_STORM interrupts and I2C interrupts to the same level.

[Jacon]

Hello Giovanni,

As we approaching end of driver development point, what about making it more
similar to other drivers?

My proposals:

1. All of the "abc_lld.h" files, from other drivers with DMA support, have below enclosed blocks of defines:

Code: Select all

/**
 * @brief   I2Cx DMA priority (0..3|lowest..highest).
 * @note    The priority level is used for both the TX and RX DMA streams but
 *          because of the streams ordering the RX stream has always priority
 *          over the TX stream.
 */
#if !defined(STM32_I2C_I2Cx_DMA_PRIORITY) || defined(__DOXYGEN__)
#define STM32_I2C_I2Cx_DMA_PRIORITY         1
#endif

where x = no of I2C interface (should be 3 of them included in i2c_lld.h).

2. Right below that - we have only one common DMA error hoook, so comment should look like this:

Code: Select all

 * @brief   I2C DMA error hook

3. What about adding this extension into main driver structure:

Code: Select all

#if defined(I2C_DRIVER_EXT_FIELDS)
  I2C_DRIVER_EXT_FIELDS
#endif

?

4. As we have

Code: Select all

#include "i2c_lld.h"

in i2c.h file already, it should be removed from i2c_lld.c file.

5. Last, but most serious update:
Barthess used this local pointer

Code: Select all

  I2C_TypeDef *dp = i2cp->i2c;

in some, but not all of functions in i2c_lld.c, that use I2C registers directly .
I just added them to all remaining - immediate result:
32 bytes reduction of STM32F4 project code size (with -O2, of course)
And slightly better readability of shorter source lines, too.

[Giovanni]

All good points, I missed those.

Giovanni

[barthess]

I will add them.

[nad]

Here is my I2C usage report:
- platform: STM32L151CBT6
- compiler: Ride 7 / GCC variant

I2C driver in latest (unstable release) runs successfully on my hardware. I am using the I2C slave NXP LCD driver PCF8578HT with no other devices on the bus.

NOTE:
- the I2C pins on the STM chip should be configured as open drain and floating (no pullup/pull down) on both I2C signals.

[barthess]

@Jacon, Giovanni
Done.

@nad
thanks for your report.

[Jacon]

@Jacon, Giovanni
Done.

OK - now it looks really good
And works so, too

[Jacon]

Small editor bug:

i2c.h line 38 - double "Driver constants." header.