Skip to content

cpu/sam0_common/periph_i2c: reliably unstuck bus#21891

Merged
maribu merged 1 commit intoRIOT-OS:masterfrom
maribu:cpu/sam0_common/periph_i2c/unstuck-for-real
Nov 27, 2025
Merged

cpu/sam0_common/periph_i2c: reliably unstuck bus#21891
maribu merged 1 commit intoRIOT-OS:masterfrom
maribu:cpu/sam0_common/periph_i2c/unstuck-for-real

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Nov 17, 2025

Contribution description

On a setup I have on my desk I can reliably get the I2C bus stuck. But the current unstuck logic will not get unstuck it. Adding a power cycle of the I2C bus to the unstuck logic does fix the issue, so let's do that.

Testing procedure

I2C should still work. E.g. as seen here on the same54-xpro:

> i2c_scan 1
2025-11-17 22:29:58,475 # i2c_scan 1
2025-11-17 22:29:58,477 # Scanning I2C device 1...
2025-11-17 22:29:58,483 # addr not ack'ed = "-", addr ack'ed = "X", addr reserved = "R", error = "E"
2025-11-17 22:29:58,487 #      0 1 2 3 4 5 6 7 8 9 a b c d e f
2025-11-17 22:29:58,491 # 0x00 R R R R R R R R - - - - - - - -
2025-11-17 22:29:58,495 # 0x10 - - - - - - - - - - - - - - - -
2025-11-17 22:29:58,500 # 0x20 - - - - - - - - X - - - - - - -
2025-11-17 22:29:58,505 # 0x30 - - - - - - X - - - - - - - - -
2025-11-17 22:29:58,510 # 0x40 - - - - - - - - - - - - - - - -
2025-11-17 22:29:58,515 # 0x50 - - - - - - X - - - - - - - X -
2025-11-17 22:29:58,520 # 0x60 X - - - - - - - - - - - - - - -
2025-11-17 22:29:58,524 # 0x70 - - - - - - - - R R R R R R R R

In addition, when the bus gets stuck, the current I2C transaction does indeed still fail. But subsequent transactions will work again.

In master using my cursed setup, the bus would stay stuck once it is stuck and never recover.

Issues/PRs references

None

@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 17, 2025
@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Nov 17, 2025
crasbe
crasbe reviewed Nov 17, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@crasbe crasbe left a comment

Choose a reason for hiding this comment

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

Would it make sense to alter the success variable if the bus-unsticking was successful?

Can't really comment on the function of the code, as I don't have the hardware nor experience with the SAM0 in general.

@maribu maribu marked this pull request as draft November 17, 2025 22:31
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 17, 2025

I think there are still some other factors that I do not fully understand yet.

@riot-ci
Copy link
Copy Markdown

riot-ci commented Nov 17, 2025 •
edited
Loading

Murdock results

✔️ PASSED

0967c6b cpu/sam0_common/periph_i2c: reliably unstuck bus

Success Failures Total Runtime
10931 0 10932 10m:12s

Artifacts

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 18, 2025

Would it make sense to alter the success variable if the bus-unsticking was successful?

Sadly not. When the bus got stuck, the I2C transfer failed. The I2C interface driver does not know how the failed communication can be fixed. E.g. i2c_read_reg() might be able to just retry by starting a new I2C transfer to read the requested register. But it is not clear if the interface is actually idempotent. Even a register read could change the device's state (even though that would usually be a sign of a poor interface).

In the end, only the device driver knows the exact communication semantics and could gracefully handle a failed communication. But each and every driver we have just passes on the buck to the application by reporting an error. That is usually good enough: Calling e.g. again some i2c_sensor_foo_read() should be good enough, even if the previous sensor read failed.

The goal of the bus-unstucking is to allow subsequent communication to succeed. If an app was designed with a noisy I2C bus in mind and actually retries on failure, it can then recover gracefully from the stuck bus situation without having someone to go on-site and power cycle the device.

image

@maribu @crasbe
cpu/sam0_common/periph_i2c: reliably unstuck bus
0967c6b 
On a setup I have on my desk I can reliably get the I2C bus stuck. But
the current unstuck logic will not get it unstuck. Adding a full reset
of the SERCOM does fix the issue, though.

Co-authored-by: crasbe <[email protected]>
@maribu maribu force-pushed the cpu/sam0_common/periph_i2c/unstuck-for-real branch from 1b5be2e to 0967c6b Compare November 27, 2025 13:02
@maribu maribu marked this pull request as ready for review November 27, 2025 13:26
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 27, 2025

This now reliably unstucks the I2C bus on my very very cursed setup. I'm quite confident that this will also reliably work on much less cursed setups.

benpicco
benpicco approved these changes Nov 27, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Changes look nice and small too

@maribu maribu added this pull request to the merge queue Nov 27, 2025
Merged via the queue into RIOT-OS:master with commit 6b536c6 Nov 27, 2025
26 checks passed
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 27, 2025

Thx a lot ❤️

@maribu maribu deleted the cpu/sam0_common/periph_i2c/unstuck-for-real branch November 27, 2025 16:23
@leandrolanzieri leandrolanzieri added this to the Release 2026.01 milestone Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crasbe crasbe crasbe left review comments

@benpicco benpicco benpicco approved these changes

@dylad dylad Awaiting requested review from dylad dylad is a code owner

@keestux keestux Awaiting requested review from keestux keestux is a code owner

Assignees

No one assigned

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2026.01

Development

Successfully merging this pull request may close these issues.

5 participants

@maribu @riot-ci @benpicco @crasbe @leandrolanzieri