I2C implementation for STM32

[Giovanni]

I think those should stay, just like in the SPI driver, those are useful when multiple threads need to "talk" with multiple devices on the bus.

Did the drive shrink after the change?

Giovanni

[barthess]

Did the drive shrink after the change?

Yea, some junk was removed.

[barthess]

Code merged to trunk. Branch deleted.

[Giovanni]

Hi,

Good work, I made some cosmetic adjustments to the high level and removed a couple of obsolete prototypes. Just a note, in the high level there should be no references to the STM32 or any other architecture, the checks must be not STM32-specific too, please remove those.

Any STM32-specific limitation must be handled in the low level only and documented in the STM32 HAL documentation.

Another note, the timeout can easily be handled by using chSchGoSleepTimeout() in the high level, the low level does not need to handle this.

Giovanni

[matis]

Driver switched to synchronous model. Callbacks and SlaveConfig struct were deleted.
matis
Now not responding nodes catched like there:

Code: Select all

  i2cAcquireBus(&I2CD1);
  errors = i2cMasterReceive(&I2CD1, addr, rx_data, 2);
  i2cReleaseBus(&I2CD1);

  if (errors == I2CD_ACK_FAILURE){
    ;
  }

Hurray, I preferred the syncronous version of I2C all the way. Ill give the trunk a go!

[matis]

That because of rotten I2C cell in stm32 family. There is not possible to read 1 byte via DMA. If your hardware allow than read 2 bytes and drop unneeded one. Otherwise you must to fall back to I2Cv1, but I not recommend to do that because it too knotty.

When receiving a 1 byte read request for I2C nodes, I read 2 bytes and return only the first received. Works splendid

[barthess]

Just a note, in the high level there should be no references to the STM32 or any other architecture, the checks must be not STM32-specific too, please remove those.

I forgot about other architectures . Fixed.

Added timeout and perform code cleanups (rev. 3584). Tested on F4x with I2C2(bmp085, tmp75, mma8451, itg3200, mag3110, max1236). And on F1x I2C1(tmp73, lis3lv02dl). All sensors polled from dedicated threads. Driver shrink little more.

matis
API slightly changed. Now error variable does not return by function (timeout status instead). Reference to error status variable must be pass in arguments. See in testhal\STM32F1xx\I2C\fake.c

Offtopic:
After looking at code now I feel "beauty of simplicity" and remember words of one my friend (he build small handmade airplanes and pilot them himself). He often saids "complication is simple, but simplifying is difficult" (in Russian "усложнять просто, а вот упрощать сложно"). Previously I think that synchronous driver must be based only on polling without interrupt handlers (lol). That has caused a protest in my mind. But now I see that driver can be interrupt (and DMA) based and can be synchronous at the same time.

[Giovanni]

Nice, I like simple and elegant things

Giovanni

[matis]

Just a note, in the high level there should be no references to the STM32 or any other architecture, the checks must be not STM32-specific too, please remove those.

matis
API slightly changed. Now error variable does not return by function (timeout status instead). Reference to error status variable must be pass in arguments. See in testhal\STM32F1xx\I2C\fake.c

The i2cflags_t *errors is a nice improvement, the systime_t timeout on the other hand seems unclear to me. The timeout should be greater than TIME_IMMEDIATE, so it will automatically be TIME_INFINITE ((systime_t)-1)? Can / will you enlight me on the need of that timeout?

[barthess]

Can / will you enlight me on the need of that timeout?

Thanks to your message I found one forgotten check. Update to rev. 3587.
For example:
Slave pulled down SCL or SDA and hanged up. I2C cell automatically set BUSY bit. Driver will be wait clearing of that bit infinitely or cause kernel panic (depending on debug features). Timeout allow you to catch situation and to do something (reset slave if possible, power cycle all I2C devices, etc.)

The timeout should be greater than TIME_IMMEDIATE

TIME_IMMEDIATE not allowed because of OS scheduler limitations (by speed reasons).

it will automatically be TIME_INFINITE ((systime_t)-1)?

No, systime_t is unsigneg int32 counter, so overflow wrap them to zero. So ((systime_t)-1) means not real infinite but maximum allowable time in future (2^32 - 1)ticks.