STM32 I2Cv2 HAL: i2c_stop() silently fails, preventing single-byte I2C from operating correctly

[multiplemonomials]

I've been working on a test program that uses Mbed boards to talk to a real EEPROM on a board. When I tried it, I immediately saw weird results coming out of the I2C transactions. For context, this is an issue I've been trying to track down for a while (originally saw it over 3 years ago!), and it's been awfully intermittent, but it seems like it reproduces reliably here.

Basically, this is what the I2C transaction with the EEPROM is supposed to look like:

However, when I tested it, I got this:

Essentially, after the first data byte is transmitted, there's a long pause, then a ninth SCL pulse, then no STOP condition at all. We should just see a final SCL pulse Furthermore, the next I2C operation never even occurs.

Enabling DEBUG_STDIO in i2c_api.c and adding some prints to the i2c_stop() function makes the source of the problem fairly clear:

[1664344852.31][CONN][RXD] timeout in i2c_stop waiting for I2C_FLAG_TXIS
[1664344852.34][CONN][RXD] TIMEOUT or error in i2c_read

Basically, it gets to the i2c_stop() function and hits this line. It never gets the TXIS flag, so then it times out. I believe that this leaves the peripheral in a bad state (since it does not send a stop condition and it never gets to i2c_sw_reset(obj);), causing subsequent I2C operations to also fail.

This issue has been exacerbated by the bad situation with return codes from i2c_stop(). The C API function has one, but it it's undocumented 🤦 , and seems like the designers assumed I2C::stop() could not fail, so the C++ API function simply discards the return code from the lower layer. I'm pretty sure this function has been failing since it was first introduced, but no one ever noticed because of this.

Back to the main question: why doesn't the TXIS flag that it's looking for set? This one took a bit of work, but I believe it's answered by the following diagram from the STM32L4 reference manual:

Basically, the TXIS flag is an indicator that the Tx register is ready to receive another byte to be shifted out. It sets as soon as the current byte starts to be shifted out and the TXDR register empties. But, crucially, as shown above, it doesn't set after the last byte in a transfer is shifted out! This is because the hardware knows that you aren't planning to shift out another byte so it doesn't want to interrupt you needlessly.

Now, if you look here, you can see that the hardware is being configured with a transfer size of 1 (necessary since we don't know the size and want to reload a new byte each time). Crucially, this means that every byte is the "last" byte, so the TXIS flag never sets!

Instead, what this code should be doing is looking at the TCR flag, which sets when the current transfer completes. It also needs to be waiting BYTE_TIMEOUT instead of FLAG_TIMEOUT, because we might potentially need to wait for an entire byte to transmit.

Indeed, if I make these changes, the errors go away, and we get a much better looking waveform:

[JohnK1987]

Solved in PR #78