Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gen 3] Fix micros/millis/unixtime becoming non-monotonic / Make I2C bus reset less destructive if performed in the middle of the transaction #2303

Merged
merged 11 commits into from
Apr 7, 2021

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Apr 6, 2021

Problem

  1. [Gen 3] Micros/millis/unixtime may become non-monotonic (especially with hot/cold ambient temperature) due to unaccounted clock drift between 32KHz RTC and DWT->CYCCNT
  2. [Gen 3] Non-monotonic micros may cause internal I2C HAL timeouts to expire early and cause I2C HAL to perform bus reset in the middle of the transaction.
  3. [Gen 2 / Gen 3] Current I2C bus reset procedure is destructive if happens in the middle of the unfinished transaction and may cause erroneous data to be written into the I2C slave. Devices populated with BQ24195 PMIC (cellular) or AM1805 RTC (tracker) may very well end up in a broken state (e.g. powered off)

See also https://github.com/particle-iot/device-os/issues/2291

Solution

  1. [Gen 3] Properly account for positive and negative clock drift between RTC and DWT->CYCCNT, so that the micros/millis/etc counters stay monotonic.
  2. [Gen 3] Every 512 seconds (RTC overflow) clocks are properly synchronized keeping the accumulated drift/bias between them intact, so there is no sudden jump in any direction after RTC overflow.
  3. [Gen 2 / Gen 3] I2C reset checks the state of the I2C bus to ensure the reset sequence is only sent when the bus is held busy by one of the slaves and attempts to finalize a potentially interrupted I2C transaction as soon as possible.
  4. [HAL] Add new OUTPUT_OPEN_DRAIN_PULLUP pin mode.
  5. [Gen 3] Add workaround for anomaly [219] TWIM: I2C timing spec is violated at 400 kHz
  6. [Cellular / Tracker] Perform PMIC/RTC bus reset on boot to make to avoid clocking erroneous data if booting after e.g. a hard reset

Steps to Test

  • wiring/no_fixture: I2C_07_bus_reset_is_not_destructive
  • wiring/timer_stress (requires temperature controlled environment, see README.md)

Example App

N/A

References


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@avtolstoy avtolstoy added this to the 2.1.0 milestone Apr 6, 2021
uint16_t config = buf[0] << 8 | buf[1];
assertEqual(config, MAX17043_DEFAULT_CONFIG);

// Validate that we can read exactly the same configuration with soft i2c implementation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you help me understand why we need to use software I2C to validate the communication with the fuel gauge?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use software I2C to create a situation of an interrupted transaction at a particular location e.g. clock 3 bits out 8 or clock 1 byte and 5 bits and then check whether hal_i2c_reset can reliably terminate this ongoing transaction without writing garbage data into the slave (previously it would have just clocked 9 high bits).

@avtolstoy avtolstoy merged commit 0b5c194 into develop Apr 7, 2021
@avtolstoy avtolstoy deleted the fix/i2c-timeout-micros-monotonic branch April 7, 2021 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants