Multimaster I2C driver enhancement

[genosensor]

The work described here:
viewtopic.php?f=2&t=1502&start=10&hilit=i2c+slave
is now largely compete.
This makes it possible to network ChiBiOS microcontrollers via their I2C bus controllers.

The API of the existing Master Mode has not been changed.
The enhanced I2C driver API supports:
I2C Slave Mode
Multiple I2C Masters sharing the same bus
SMBus style "quick" messages (e.g. 0 byte writes and reads)
Ability to "lock" the bus for any arbitrary sequence of messages
Ability of slaves to detect whether or not the previous write received was ended with a STOP conditon (or repeated START)

The I2C driver still does not support:
10-bit addressing mode
SMBus CRC checking

The first implementation is for the STM32 MCUs that employ the STM32/I2Cv1 driver.
Testing has been carried out between STM32L1xx Discovery boards and older I2C controllers based on a (non-ChiBiOS) MSP430169 microcontroller.
Templates and headers are in place to facilitate enhancement of other I2C drivers in ChiBiOS.

The code is available at:

https://github.com/brentr/ChibiOS-RT

Anyone interested may clone the repo with:

Code: Select all

git  clone   git@github.com:brentr/ChibiOS-RT.git
cd  ChiBiOS-RT
git  checkout  i2cslave


This is all based on the 2.6x stable series.

Most of the changes are in i2cslave.h (new file) and i2c_lld.*
Note that there is no i2cslave.c, as the slave mode support is integrated into the existing i2c driver when, in halconf.h:

Code: Select all

#define HAL_USE_I2C_SLAVE           TRUE


To enable the optional i2cLock() function in the master mode API, allowing arbitrary sequences of "repeated starts" for atomic messaging, add:

Code: Select all

#define HAL_USE_I2C_LOCK           TRUE


The only change I made outside the driver was to add a macro to initialize virtual timers allocated in automatic storage.
I called this chVTInit(). If there's a better/more standard way to get this done, please let me know.

When slave messages arrive, the driver invokes callbacks in interrupt context to process them.
This is very efficient and appropriate for simple cases.
However, a small, hardware independent library (os/various/i2c_event.[c.h]) can convert these interrupt context callbacks into events that are typically serviced in a dedicated message processing thread.

Others have complained that the STM32's I2C controller is buggy. I have not found it to be especially so. The only hardware problems I see are:
1) Slaves do not get interrupted when repeated start event end the current message they are processing. As a result, slaves cannot process such messages until after the address for the next message is sent. This results in having one interrupt both end the previous message and start a new one and is why event queuing is absolutely required to interface with any sort of message processing thread.

2) Zero length writes do not provide any opportunity for slaves to stretch the I2C clock. They work fine to spite this. The queue size can be configured to store the longest rapid sequence of uninterrupted zero length writes to be expected. (usually, a queue depth of 4 events suffices)

3) Zero length reads are problematic. They can only work if the MSB of the first response byte from the receiving slave is 1. Otherwise, the STM32 will drive the SDA line low before the master has a chance to end the transaction by signalling a STOP condition.

The existing, still admittedly basic, tests are in the projects i2cmaster and i2cslave.
To run these without change, connect the PB8 and PB9 pins of two boards together and pull each up to VDD through approximately 4Kohm.
Run the i2cmaster project on one board and i2cslave on the other.
If you have the latest version of OpenOCD and are interfacing via STLINKv2, progress messages will appear in the debugger via DCC.
Otherwise, you'll need to modify the test code to output text to the serial port of your choosing.

[genosensor]

Christmas update:

I found that one could prevent the STM32 I2C controller from receiving short messages before being ready by delaying the clearing the I2C_SR1_ADDR status bit. The slave will now stretch the I2C clock directly after ACKing its address until it is ready to respond. Without this fix, messages whose length is < 2 bytes could be completely ACKed by the controller before the firmware indicated it was ready to receive them. This would cause missed messages if the master sent that another one immediately following the first short message. Note that I2C event interrupts are now disabled (to facilitate holding off clearing I2C_SR1_ADDR) until the firmware indicates it is ready to accept the message.

The good news is that, now, the STM32's maximum queue size required for servicing slave message on a thread is only two entries. Those two queue entries are still required to deal with the fact that the STM32's I2C controller does not generate an event when a repeated start terminates an incoming message. Instead, the first indication one gets of is the address match of the subsequent message. But, what if that the subsequent message were targeted at a different controller...

That's the bad news.

In a sequence of messages separated by repeated starts, if the second message target address is that of another controller, the STM32 I2C slave receiving the first message will never see it terminate. The slave, in this case, sees the address match for the first message, it enables DMA to receive the message body, but there may be no stop condition nor another address match for it to signal the firmware that the first message it was receiving had terminated.

The errata sheet at:
http://www.st.com/st-web-ui/static/active/en/resource/technical/document/errata_sheet/CD00278726.pdf

Does not even mention this issue. It is a major problem if you are trying to implement an I2C slave that we be integrated into systems designed by others. However, if you can specify the design of the master I2C nodes, then it is a simple matter to avoid message sequences like those described above.

The only way I can see to workaround this is to have an external I2C start condition sensor generate an interrupt whenever a message is being received is terminated by a repeated start. I think one could configure an STM32 general purpose timer (GPT) for this, if its trigger and count inputs were jumpered over to the I2C channels SDA and SCL, respectively. The timer would be configured in gated external clock source mode 2.

But, I've yet to play with the STM32 GPTs.

Does anyone who has more experience with them have any ideas about how to configure a timer so that it would trigger an interrupt on an I2C START condition?
This is a falling SDA edge while SCL is high.

[Giovanni]

You would also have the problem of serving that IRQ in time. Using an external unit would be a huge problem for the driver anyway. It would assume TIM availability and specific external wiring.

Giovanni

[genosensor]

I don't think the serving the IRQ in time would be a problem.
As it is, the I2C repeated start generates no interrupt, so there's no race here.
The I2C controller will just stretch the clock if interrupts are not serviced in time.
The priority of the external start condition detector should, not fact, be set lower than that of the I2C controller's event interrupt.

I agree that the use of a TIM for the external start bit detection should not be integrated into the I2C driver. Rather, I'd envisioned an STM32 specific HAL flag like:

#define HAL_USE_STM32I2C_STARTFIX TRUE

When that #define is set, two callback pointers would be added to the I2CConfig struct.
Once would point to a function that "arms" the external start bit detector, the other function would disarm it.

The driver would export a function that could be called from an ISR to inform it that a start condition has been detected on the bus.

Admittedly, this is a corner case that won't effect most designers.
But, I'd like to address this issue while I'm still deep into this yoghurt

- brent

[genosensor]

I've added the support for an external I2C (re-)Start Condition detector.

In halconf.h, setting:

Code: Select all

/**
 * @brief   Enables the I2C slave external start bit detector.
 */
#if !defined(HAL_USE_I2C_STARTFIX) || defined(__DOXYGEN__)
#define HAL_USE_I2C_STARTFIX        HAL_USE_I2C_SLAVE
#endif


will include support in the slave mode STM32 I2C driver for an external start condition detector.
The example i2cslave.c attached demonstrates how a start condition detector can be made from TIM3 just by connecting SCL and SDA to the timer's TI1 and trigger, respectively. The core code looks like:

Code: Select all

#define I2C1TIM_CR1  (STM32_TIM_CR1_OPM)

static void armStartDetector(void)
{
  I2C1TIM->CNT = 1;
  I2C1TIM->CR1 = I2C1TIM_CR1 | STM32_TIM_CR1_CEN;
}

static void disarmStartDetector(void)
{
  I2C1TIM->CR1 = I2C1TIM_CR1;
  I2C1TIM->SR = 0;
}

/**
 * I2C1 start detector (TIMER) interrupt handler.
 */
I2C1TIM_IRQ_Handler {
  palTogglePad(GPIOB, GPIOB_LED4);  /* toggle LED when ISR starts */
  CH_IRQ_PROLOGUE();
  I2C1TIM->SR = 0;
  i2c_lld_startDetected(&I2CD1);
  CH_IRQ_EPILOGUE();
}

#include "i2c_event.h"

i2cEventChannelCfg(I2C1channel, 2,
    OPMODE_I2C,
    100000,
    STD_DUTY_CYCLE,
    armStartDetector,  disarmStartDetector);
}

As you can see, the driver merely calls the arm or disarmStartDetector via function pointers added to the existing I2C configuration struct. And, the new ISR informs the driver when a START has been detected but calling i2c_lld_startDetected().

The attached versions of the i2cmaster and i2cslave projects demonstrate the failure that can result without an external start detector. In the master side, when the User button is pressed for more than 1 second, the master will emit repeated starts between messages for the next 15 seconds. As a result, the slave fails to see the end of messages it is receiving and times out. Once you jumper over the required lines:

Code: Select all

PC6 wired to PB8 (SCL)
PD2 wired to PB9 (SDA)


The timeouts no longer occur, as the slave is notified via the new TIM3 interrupt when messages being received are terminated with a repeated start.

This is about all I intend to do with the STM32 I2C driver until I get more feedback on it.
It does everything I need it to at this point.

[Giovanni]

I have no way to try it but I will consider the API for inclusion in 3.0.

My question: is this problem of repeated starts handled by the new I2C peripheral present in F0/F3 devices? is something that we should expect in other devices too?

Giovanni

[genosensor]

I just had a quick look at the STM32/I2Cv2 driver source.
The event ISR seems to be quite different.
There's a "transfer complete" interrupt that is entirely missing from the I2Cv1 device.
This would seem to imply that ST recognized the repeated start problem and tried to fix it in the new silicon.
I might try porting the multimastering support to the STM32/I2Cv2/i2c_lld sometime in the coming months.
That's the only way one can know for sure whether the repeated start problem is fixed. I suspect it is.
However, I read on their forums that the new device also has its share of new, improved bugs.

All anyone needs to try this out for themselves is a couple of the STM32Ldiscovery boards, a pair of 4kOhm resisters and a few jumper wires. That, and time...

[rpltn]

Hi and congratulations on your RTOS efforts. Personally, I'm not using ChibiOS at this time.

I've read some older posts regarding I2C implementation. As I understand your implementation, it is DMA only, no PEC, limited SMBus support.

I am working on an implementation of SMBus / PMBus over I2C using LeafLabs LibMaple (I rewrote a good bit of their I2C code, particularly the ISR, for my own efforts) that is working with repeat start and PEC. I'm using "method 1" with IRQ's set to highest priority, 400 khz - and it works (though, I'm not using the SMBus bit, it forces you to 100khz timing limitations, and I'm not using SMBalert nor ARP, etc). I still have more to do with it, but it's functional. I'm talking to 3 Dlynx DC to DC converters - so far, I've read the voltages from the converters, margined the voltage, and read it again to verify. Also, I don't disable the IRQ's unless there's an error (and I may change the error ISR to clear state to prevent the event/ buffer ISR from firing and reset for next, then remove all "disable IRQ" functions), there is no need to do so. I don't need to block for the entire I2C transaction, and I can check status of a transaction outside of the ISR from my main code (though for now, while testing, I am using a block, waiting until the transaction is done - easier to test this way). With the approach I'm using, 10 bit could be added, as could slave mode, etc. Ex, for method 2, the ISR will fire more often for Mode 2 vs Mode 1 as a result of BTF - and when this is expected, it can be handled.

In reading, I saw a question about resetting BTF. Once you've done "stop" properly, you can access DR twice to clear BTF, which prevents the ISR from firing unnecessarily. You shouldn't have to disable IRQ's within the main ISR, it won't fire unnecessarily if state is handled properly. At first, the code I had disabled IRQ's (which I didn't like doing), but I found that when re-enabling the IRQ's for the next transaction, the ISR fired due to state not cleared before SB was even set - that's how I found the solution to clear RXnE / BTF when done.

As for detecting "Stop", I think the answer is in checking if the MSL bit indicates slave mode - at least, that's what I plan to code (transaction done & MSL = 0 means done). The RM says the interface switches from slave to master on setting SB and indicates this on MSL, so it only makes sense that to be done, MSL switches back to slave - from here, it makes sense that SB can be set again and that the hardware won't be "confused".

Once I managed to better understand the I2C hardware in the STM32F103, it doesn't seem quite so butchered as many seem to think (particularly Method 1 of the RM, though I could probably get Method 2 to work with my approach) - you simply have to take the RM with a grain of salt, and understand that their "events" are merely concepts, not explicit state to code to - ex, SB gets the ISR to fire, then you send the address to clear SB, then read SR2 to clear ADDR. The explanation (diagrams in the RM) using events is useful only as an explanation, the events themselves are what seems to confuse people. Instead, think of EVx as a variable for the footnote only. Surely, their diagrams don't explain all possible scenarios - they only provide insight in how to handle state in the low byte of SR1, which is what triggered the ISR. From there, what you do is what's needed based on what happened in SR1.

I haven't seen the original interrupt-based ISR, how much code was in there?

[genosensor]

Hi rpltn,

Yes, the I2C implementation is DMA based for all messages > 2 bytes in length and has no support for PEC or 10-bit addressing.
Basic SMBus support is implemented, but some aspects are untested. There's no support for SMBus ARP.

Regarding your detailed comments on disabling interrupts and BTF handling:

Have you looked at my branch of os/hal/platforms/STM32/I2Cv1/i2c_lld.c ?
at:
https://github.com/brentr/ChibiOS-RT/tree/i2cslave

This already clears the BTF by reading the DR reg and I've already gotten rid of all masking of interrupts.
However, I don't see how one could check the MSL status bit to detect a repeated start.
The MSL bit only is set when the chip is mastering the I2C bus.
It would remain clear for the duration of a sequence of received repeated start events in slave mode.
Could you please elaborate how the MSL might be used when while the interface remains in slave mode?

I agree with you that the STM32v1 I2C hardware really is not that bad, aside from the repeated start issue,
for which I have a workaround the version of i2c_lld.c to which I referred you. Details are explained earlier in this thread.
And, yes, one key to successful implementation is to use all STM's pretty diagrams as a departure point, not as gospel.

I'm not sure which "original interrupt-based ISR" you mean.

[rpltn]

Hi Genosensor,

Ah, maybe I wasn't clear...

I wasn't speaking of using MSL for detecting repeat start, but instead for detecting end-of-transaction (stop done).

For detecting "Start", that's indicated by SB in CR1, for first start OR repeat start, after generation of Start condition on the bus. In the case of repeated start, you get TxE and SB in CR1 (for SMBus-style transactions - the TxE is from the CMD being transmitted).

My work is all IRQ-based, no DMA at this time (the RM says revert to IRQ / ISR-based approach for single byte, from reading posts here, DMA was chosen instead of IRQ-based for a few different reasons). First byte of SMbus packet is ADDR, second is CMD, then Start must follow on the TXE corresponding to CMD sent. Then, ADDR is sent a second time, with R/w bit set to 1. PEC gets set on last RXnE, PEC Enable must be set prior to sending first byte of the SMBus transaction (Address byte) for proper PEC calculation.

With DMA, I can't say how you'd handle this. I checked out what you have there, all DMA, correct? That makes what you're doing quite different from what I have. Using IRQ-based, it's just a check of the low byte of SR1 when the ISR is called, for SB=1 to identify repeat start sent.

Regarding the original interrupt-based ISR, I'm referring to the last version before interrupt-based method was removed. I can dig for the post, but if your intentions are to stick with DMA only, my work is of no use to you.

But yes, repeat start can be done, PEC can be done, SMBus Tx / Rx transactions can be done WITH PEC (and without using SMBus bit, no ARP, no SMBAlert), at 400khz, even alongside "regular" I2C devices on the same bus (depending on devices chosen, etc) - with IRQ-based handling. I could do 10 bit addressing without much effort, but I have no 10 bit devices to check (though, this straightforward enough to set up without having a device). I have a Dlynx UDT020 and two Dlynx PDT012's on the same bus with a PCA9685 chip, working together as of this past weekend. The UDT020 and PDT012's are SMBus and require PEC. I'm able to command these devices and request from them as well (PEC working on Tx and Rx SMBus transactions). I'm using Repeat Start in SMBus Rx.