Repository navigation
i2c: designware: Fix SDA hold default being computed before LCNT - #7741
Merged
pelwell merged 1 commit intoNov 6, 2026
Merged
Conversation
The "SDA hold time = LCNT / 2" default added by commit "i2c: designware: Make the SDA hold time half LCNT" (see raspberrypi#6091) is applied in i2c_dw_probe(), before i2c_dw_probe_master() has run i2c_dw_set_timings_master() to populate dev->fs_lcnt. fs_lcnt is therefore still 0, the default evaluates to 0, and i2c_dw_set_sda_hold() keeps the hardware reset value (1 cycle, ~5 ns) instead of the intended ~3 us. On a Pi 5 (200 MHz clk_sys, 100 kHz bus) this leaves IC_SDA_HOLD at 0x10001 rather than 0x10251 as on 6.18, and reintroduces the early-ACK failures that raspberrypi#6091 and raspberrypi#6056 fixed: an Adafruit PiOLED (SSD1306, 0x3c) fails with "SDA stuck at low" / "Remote I/O error" on 7.3.1 while it works on 6.18. Do the lcnt/2 default and i2c_dw_set_sda_hold() after i2c_dw_probe_master(), so fs_lcnt is valid. i2c_dw_init(), which writes the register, still runs afterwards. Tested on a Pi 5 running 7.3.1: IC_SDA_HOLD reads 0x10251 and the OLED works. Assisted-by: GitHub Copilot CLI:claude-sonnet-5.5 Signed-off-by: jflecool2 <1393416+jflecool2@users.noreply.github.com>
Collaborator
|
Claude review: Thanks for the clear report and the register readings — the analysis is correct. Upstream's "Combine the init functions" rework moved The reordering looks right to me:
The same change applies cleanly to rpi-7.4.y as well. After merging, this can be squashed into the original "Make the SDA hold time half LCNT" commit at the next rebase. |
popcornmix
approved these changes
Nov 6, 2026
Contributor
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7740.
The "SDA hold = LCNT / 2" default from #6091 is computed in
i2c_dw_probe()beforei2c_dw_probe_master()fills infs_lcnt, so it is always 0 on rpi-7.x and the hardware reset value (~5 ns) is used. This moves the default andi2c_dw_set_sda_hold()afteri2c_dw_probe_master().i2c_dw_init()(which writes the register) still runs afterwards.Tested on Pi 5 (7.3.1, custom build):
IC_SDA_HOLD0x10001 -> 0x10251 (same as 6.18), Adafruit PiOLED SSD1306 works again.Not tested: other boards, slave mode. The same ordering is present in rpi-7.0.y, 7.1.y and 7.2.y, so this will need backporting there.