embassy-nrf/twim: prevent extra STOPs on ERROR - #6726
Open
Wassasin wants to merge 1 commit into
Open
Conversation
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.
Solves #6722. Tested on NRF52840-DK and custom NRF9160 board.
The NRF TWIM can issue an ERROR event without a STOP event also being issued. This happens for example with an ANACK (address nack). The manual states:
What the manual does not say, that if you issue STOP when no transfer is happening, that the peripheral goes into a broken state.
Thus we should prevent STOP from being issued by the driver, unless it is necessary.
This PR moves only for the async driver the issuance of STOP in the event of an ERROR to the interrupt handler. STOP is still being issued in the blocking API, as well as in the transaction loop when there are no more pending transactions.
This PR loses the ability to emit
Error::Timeout, but it is not clear to me how this was ever going to be triggered. The ERROR event should never be pending withouterrorsrcalso containing something.Further work
This PR does not fix the rule:
We were already not following that rule currently, but peeking into the nrfx implementation got me thinking whether we maybe should.
Please discuss if we should tackle that, and if so in this PR or in another PR.