Skip to content

machine,nrf528,i2c: fix multiple resume-stop on bus error #5007

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

gen2thomas
Copy link

@gen2thomas gen2thomas commented Aug 21, 2025

As already shown in #4998, in case of an error the i2c TX method in nrf528xx loops until a stop condition is fulfilled. In case of an error, a resume-stop command on bus will be called, so the bus can finish its work and exit the loop afterwards - no "hard" return in case of error is done and needed.

With the current code, after the first error occurs, each loop checks again for an error and practically (assuming the error is not gone) sends the resume-stop sequence again and again until the stop state is reached (triggered by the first resume-stop-sequence) and the loop is finished. On long run with existing errors, e.g the device is not connected on the bus, and cyclic calls (e.g. each 10 seconds) this leads reproducible to hang of the loop with the bme280 driver.

The provided code change improves the code for this topics:

  • checks for the error code before the resume-stop is called, so the real error is finally contained in err variable
  • skips the complete error check if the first error was detected - this fixes the originated problem

@gen2thomas gen2thomas force-pushed the fix/nrf528_i2c_hang branch from 768d21d to 5e6e91e Compare August 21, 2025 16:35
@eliasnaur
Copy link
Contributor

LGTM, but please copy the relevant parts of the PR explanation to the commit message.

@gen2thomas gen2thomas force-pushed the fix/nrf528_i2c_hang branch from 5e6e91e to 3a580c1 Compare August 22, 2025 15:51
@gen2thomas
Copy link
Author

@eliasnaur second topic added to the commit message - is the prefix ok? Is there a special guideline for commit messages on tinygo?

@eliasnaur
Copy link
Contributor

I'm fairly new here, and not aware of particular guidelines for commit messages. My personal preference is to retain details in the commit message, because GitHub PRs are separate from the repository. That is, PR descriptions are for helping reviewers, and commit messages to describe the changes.

For your commit, I suggest something like

nrf528: stop the bus only once on I2C bus error

In some cases, e.g nothing connected on the bus, repeated resume-stop sequences can lead to the bus never reaching the stop state, hanging Tx.
This change ensures the resume-stop sequence is submitted once on error. It also moves the error code read to before the sequence to ensure it's valid.

Fixes: #4998

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