Skip to content

Fix I2C error-stop handling: 14ms stall per failed read, noisy NACK log, LP port reset - #240

Merged
lovyan03 merged 3 commits into
m5stack:developfrom
ainyan03:i2c_stop_fix
Aug 7, 2026
Merged

Fix I2C error-stop handling: 14ms stall per failed read, noisy NACK log, LP port reset#240
lovyan03 merged 3 commits into
m5stack:developfrom
ainyan03:i2c_stop_fix

Conversation

@ainyan03

@ainyan03 ainyan03 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Fixes three problems in the ESP32 I2C error handling, found while running an I2C bus scanner that probes absent addresses with 1-byte reads.

Symptoms

  • A read from an absent address took ~20 ms instead of sub-millisecond: scanning 112 addresses took 2.4 s per sweep.
  • The console was flooded with [W][common.cpp] readBytes(): [LGFX] i2c read error : read timeout for every probed address, which itself accounts for several ms per probe at 115200 bps.
  • On chips with a low power I2C port (ESP32-C5/C6), every forced stop on the LP port reset an unrelated register.

Fixes (one commit each)

  1. Reset a low power I2C port through its own reset register
    i2c_ll_reset_register(port) indexes the PCR I2C array with the port number, but the array only holds the HP ports (PCR.i2c[1] on C5), so an LP port number writes out of bounds. LP ports now use lp_i2c_ll_reset_register().

  2. Keep an errored I2C transaction on the forced stop path
    In i2c_wait(), int_raw was read uninitialized whenever the ack-wait stage had been skipped (e.g. endTransaction() after a failed read), so the STOP-path choice was undefined. When the normal STOP command path was taken on a peripheral that had already been force-stopped by the error handling, the completion interrupt never fired and the loop waited out the full 14 ms bound. int_raw is now initialized, an errored context always takes the forced stop, and the redundant second hardware stop is skipped (the error handler has already issued one).

  3. Report a pure address NACK on read as NACK instead of timeout
    An address NACK is the expected outcome when probing for devices; it is now logged at verbose level as nack. The timeout warning remains for genuine timeouts, and the NACK demotion only applies when neither the timeout flag nor the arbitration-lost flag is set.

With these, a failed 1-byte read probe drops from ~20 ms to sub-millisecond and the log stays quiet.

Testing

  • M5Stack Core2 (ESP32): full-bus scan sweep went from 2.4 s to well under 0.5 s; devices on the internal bus still detected correctly; no regressions in touch/RTC/PMIC traffic.
  • M5Stack ToughC5 (ESP32-C5, internal bus on the LP port): scan works, and NACK probes no longer hit the out-of-bounds HP reset.
  • Compile-checked for ESP32 and ESP32-C5 (HP and LP paths; the C5 branch of the chip conditionals is shared with S3/C2/C6/C61/P4/H2).

The HP helper i2c_ll_reset_register() indexes the PCR I2C register
array with the port number, and a low power port number is out of
range for that array (it holds only the high power ports), so a forced
stop on the low power port was resetting an unrelated register.
Route low power ports to lp_i2c_ll_reset_register() instead.
i2c_wait() chose between the normal STOP command and the forced stop
by reading int_raw, which was never initialized when the ack-wait
stage had been skipped, so the choice was undefined after a failed
read. The normal STOP path waits for a completion interrupt that
never fires on a peripheral that has already been force-stopped by
the error handling, costing the full 14ms timeout on every failed
transaction. Initialize int_raw, treat an errored context as needing
the forced stop, and skip the redundant hardware stop when the error
handler has already issued one.
A read from an absent address ended in the "read timeout" warning,
although the NACK is the expected outcome when probing for devices,
and printing the warning costs several milliseconds per probe on a
115200bps console, slowing bus scans considerably. Report it at
verbose level as a NACK, but only when neither the timeout flag nor
the arbitration-lost flag accompanies it.
@lovyan03
lovyan03 merged commit d83939f into m5stack:develop Aug 7, 2026
23 checks passed
@lovyan03
lovyan03 deleted the i2c_stop_fix branch August 7, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants