I2C implementation for STM32

[barthess]

Nice solution with DMA init. I have tested changes in hardware. Works fine. Code merged in rev. 3729.
About timeout issue. If it will helpful, I have found that RDY_OK returns only in case of read through write operations.

[Giovanni]

Thanks, tomorrow I will look into the timeout thing then it is ready.

I read a TODO inside the code about DMA errors handling, is that still missing?

Giovanni

[barthess]

I read a TODO inside the code about DMA errors handling, is that still missing?

Looks like DMA subsystem has it own error handling and passed values checks in chDbgCheck(). I have
no suggestion about additional error handling. If you not have too than remove them.

[Giovanni]

Hi,

I made several changes to the driver:
1) Implemented timeouts globally.
2) The CR1 and CR2 registers were apparently not reinitialized between a transaction and next one.
3) Moved some initialization from I/O function to the start().
4) Moved some code from the header to the driver, it was private.
5) Implemented DMA errors handling in DMA ISRs.
6) Inlined some code in order to avoid some duplication.

I hope to not have introduced bugs, assuming it works this could be final.

Giovanni

[barthess]

Giovanni,
Take a look at changes in rev. 3742. Are there were copypaste errors or something unobviouse for me?
On that revision the following tests successfully works:
- testhal\STM32F1xx\I2C
- testhal\STM32F4xx\I2C
- my own project (6 different slaves in different threads on single bus) with IRQ_STORM in background

P.S. Your code looks much cleaner than mine.

[Giovanni]

First you write code and it is ugly and doesn't work.
Then you write code and it is ugly but works.
After a while you write code and it is beautiful and it works.
Finally you write code and it is so beautiful that it doesn't matter if it works.

Jokes apart, I have doubts on the CR2 handling.

You initialize CR2 in the start() function then raise some bits in the interrupt handler:

i2cp->i2c->CR2 |= I2C_CR2_DMAEN | I2C_CR2_LAST;

Those bits are not cleared anywhere, so next calls find them alread raised, this means that you could put those bits active in start() and not touch CR2 anymore.

Giovanni

[barthess]

Yes, it must be initialized only once, moved to start() function.
Observe strange behavior with my BMP085 + IRQ_STORM + -O2 optimization compiler flag. Polling thread hangs and returns with RDY_TIMOUT. Will check it tomorrow with scope.

[Badger]

A very minor point, but the mcuconf.h in the i2c testhal projects is out of date and also includes a few comments like //RF Link

[barthess]

Rev. 3747.
Got it! Problem occurred in tx-only transactions because of repeated STOP on bus, generated from ISR. Much more interesting thing - why that error never occurred with compilation flag -O0? Any ideas?

A very minor point, but the mcuconf.h in the i2c testhal projects is out of date and also includes a few comments like //RF Link

Thanks, cleaned.

P.S. Interesting to see on oscilloscope increasing/decreasing of jitter on I2C bus during irq storm.

[barthess]

Rev. 3749
Now interrupts enabled only if they really need. Previously they were enabled all time that causes ISR every transmitted/received byte.