nrf: Fix I2C regression introduced by nrfx v3 update.#19220
Open
andrewleech wants to merge 1 commit into
Open
Conversation
In nrfx v3, when both SPIM0 and TWIM0 are enabled (PRS_BOX_0_ENABLED), nrfx_prs.c and nrfx_twim.c emit the same IRQ handler symbol via macro aliasing in nrfx_irqs_nrf52840.h. LTO resolves this to the PRS dispatcher, which never reaches twi_event_handler, causing I2C to stall indefinitely. Fix by switching to blocking mode (NULL handler), where nrfy_twim_tx_start polls the STOPPED/SUSPENDED event internally without IRQ dependency. Also fix the NOSTOP flag: the old code passed (flags & FLAG_STOP) == 0, evaluating to 0 or 1. In nrfx v3 TWIM, flag 1 is NRFX_TWIM_FLAG_TX_POSTINC (buffer post-increment), not no-stop. The correct flag is NRFX_TWIM_FLAG_TX_NO_STOP (1 << 5). Fixes: micropython#19214 Signed-off-by: Andrew Leech <andrew@alelec.net> Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Contributor
|
I can confirm I2C works with this update on my XIAO nrf52840 with expansion board. Both I2C(1) - the intermal IMU bus and I2C(0) work. |
Contributor
|
Tested with a Arduino Nano 33 BLE. I2C works. No SPI connected or enabled, besides the internal Flash of the UBLOX module. But I do not see if and where the supplied value for timeout is applied. |
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.
Summary
Fixes a regression introduced in #19125 where I2C stopped working on nRF52840 when SPI is also enabled (issue #19214).
The root cause is an IRQ handler symbol conflict in nrfx v3: both
nrfx_prs_box_0_irq_handler(PRS dispatcher) andnrfx_twim_0_irq_handlermap to the same final symbolSPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQHandlervia the nrfx irq alias headers. With LTO enabled, the PRS dispatcher wins and the TWIM event handler is never called, so I2C transfers stall indefinitely waiting for axfer_doneflag that never gets set.The fix is to use nrfx blocking mode (NULL event handler) where
nrfy_twim_tx_startpolls the STOPPED/SUSPENDED peripheral register directly without relying on the NVIC interrupt. This is appropriate for MicroPython's synchronousmachine.I2CAPI.Also fixes the NOSTOP flag: the old code passed
(flags & MP_MACHINE_I2C_FLAG_STOP) == 0as the xfer flags argument, which evaluates to 0 or 1. The value 1 isNRFX_TWIM_FLAG_TX_POSTINCin nrfx v3, not the no-stop flag. Now usesNRFX_TWIM_FLAG_TX_NO_STOPexplicitly.Testing
Tested that the nRF52840 port builds. Hardware testing not done - the regression is reported by users in #19214.
Generative AI
I used generative AI tools when creating this PR, but a human has checked the code and is responsible for the code and the description above.