Rethink STM32 I2C v2 HAL

[multiplemonomials]

Summary of changes

This PR is a major rethink of the STM32 I2C code for their v2 I2C peripheral. It tries to get it into a more API-compliant and less buggy state. It should fix several known bugs:

• Single-byte API never returns NACK result code on error, only timeout (STM32 I2C HAL does not handle NACKs properly in single-byte mode #61)
• Single-byte API returns errors one byte later than the byte that actually caused the error (STM32 I2C HAL does not handle NACKs properly in single-byte mode #61)
• Using the single-byte API puts the I2C peripheral in a bad state where the next operation will fail (STM32 I2Cv2 HAL: i2c_stop() silently fails, preventing single-byte I2C from operating correctly #62)

Additionally, I found more issues while making this PR that will also be fixed:

• Doing a single-byte operation that fails, then doing a transaction operation after that causes the transaction to return early and blindly report success, because the STOPF flag from earlier was not cleared
• Doing an asynchronous read transaction then a synchronous write transaction could cause the asynchronous read transaction to be done a second time due to incorrect logic in the interrupt handler
• The entire I2C peripheral was being reinitialized whenever a NACK was received, which is a fairly expensive operation, especially for an interrupt handler. Now NACKs are handled differently from other errors and don't cause a complete re-init.
• Doing a repeated start with the single-byte API did not work, because the RELOAD and NBYTES fields have to be zeroed before the repeated start can be sent.

The biggest change this PR makes is the addition of a new state enum, obj_s->state. This provides a simple indicator of what the I2C code is doing at any given time, making certain parts of the logic a lot simpler and enabling some of the fixes to work. I also added a lot of new checks based on this state variable, so if you use the I2C object in an incorrect way, you have a fighting chance of getting a useful error code back instead of undefined behavior. (it's still tough though because certain functions do not have the ability to return an error code...)

Impact of changes

Lots of issues are fixed (see above)

Migration actions required

Documentation

None

---

Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

---

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR

Working on creating new tests that run this code against hardware.

---

Reviewers

---

[multiplemonomials]

I have created an exhaustive new test suite that covers just about every pattern of I2C reads and writes you could do, including repeated starts and use of all three different APIs on the same peripheral one after the other. It helped me catch a large number of issues in the existing and the new code.

https://github.com/multiplemonomials/Mbed-CI-Test-Shield/blob/master/Software/I2CBasicTest.cpp

[multiplemonomials]

Tests passed on STM32L452RE:

1/1 Test #4: testshield-i2c-basic .............   Passed   11.40 sec

The following tests passed:
        testshield-i2c-basic

100% tests passed, 0 tests failed out of 1

Tests passed on STM32H743ZI:

1/1 Test #4: testshield-i2c-basic .............   Passed    7.42 sec

The following tests passed:
        testshield-i2c-basic

100% tests passed, 0 tests failed out of 1

[multiplemonomials]

Almost there, just need to look into why this PR makes the "Simple Read - Transaction" test fail for IP v1

[multiplemonomials]

OK, I can now say that there are no more failures with IP v1 on this branch than on master. Filed #96 about the remaining test failures.

[JojoS62]

it is difficult to test, I can connect some I2C devices and test with my currenct hardware. Maybe there is some I2C compliance test equiment, but I don't have access to it.

[JohnK1987]

@JojoS62, I believe it is about this piece of hardware - Mbed-CI-Test-Shield by @multiplemonomials.

[multiplemonomials]

Yup! I can send you one of the boards if you want to run these tests locally

[JohnK1987]

I would like also one, but I am from Czech republic and the transport from USA is probably expensive.
However, my employer has a PCB milling machine, I have it in my office. But for that is necessary have source files in Autodesk Eagle.

BR, Jan

[JojoS62]

and I'm living in germany. So it is probably easier to run some simple tests with I2C devices. Although, target implementations can have very different behaviour. I have seen that with SPI on H7, it can be very complicated.

[JohnK1987]

I saw your work was already Marged also in ArmMbed. May I close issues (61,62 and 94 mentioned above) connected to this?

BR, Jan

[multiplemonomials]

Sure on 61 and 62. I haven't fixed 94 yet.