mbed-ce / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
79 stars 15 forks source link

LPC1768: Fix I2C pins not being open drain, fix destroying and recreating I2C making transactions fail #275

Closed multiplemonomials closed 4 months ago

multiplemonomials commented 5 months ago

Summary of changes

Testing with my CI test shield, I noticed an I2C issue with LPC1768. At first, it manifested as some of the test cases which constructed I2CEEBlockDevice objects failing on their first write command. After a bit more debugging (left the debug macros in in case we ever need to dig into this again), I realized it was actually just the act of destroying and recreating the I2C object that was causing the issue. When an I2C instance is recreated after being initialized previously, the very first transaction you do will always fail.

Why was this happening? It seems to be due to a bad interaction with the I2C::recover() function. This function remaps the I2C pins as digital IOs and performs a specific sequence to unstick any slave devices on the bus. The issue is, if this sequence occurs after the I2C peripheral has been enabled, it puts it in some sort of bad state that fouls up the next transaction. I'm not sure exactly why, as the I2C alternate functions aren't active on the pins during this sequence -- maybe it's because the I2C peripheral is still receiving inputs from the pins even when not configured to output to them?

Regardless of why, the fix is thankfully fairly easy. All one has to do is disable the peripheral before enabling it to reset its internal state. This fixes whatever was getting screwed up and allows I2C to work reliably again. If I2C had an i2c_free() function, that would be the place to do it, but the HAL currently doesn't have this function for some weird reason.

In the process of debugging this issue, I noticed another one as well: the I2C pins were never being configured for open drain. I'm not totally clear if this actually causes the pins to output push-pull or not (I definitely can remember using Mbed 2 on LPC1768 and seeing the I2C pins correctly working in open drain mode), but the manual does say to set this register setting, so definitely seems worth doing.

Impact of changes

Migration actions required

Documentation


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

LPC1768 now passes my I2C test shield tests:

The following tests passed:
    test-testshield-i2c-basic
    test-testshield-i2c-eeprom

100% tests passed, 0 tests failed out of 2

Reviewers