[nrf noup] boot: zephyr: cleanup of SPIM for nRF91#606
Conversation
|
none Note: This comment is automatically posted and updated by the Contribs GitHub Action. |
diegosolano
force-pushed
the
spim-cleanup
branch
from
0b53f87 to
0a97fd4
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Zephyr-based MCUboot Nordic peripheral teardown (nrf_cleanup_peripheral()) to also stop SPIM EasyDMA and disable SPIM instances before chain-loading, preventing RAMACCERR faults on nRF91 when TF-M changes SPU SRAM permissions after MCUboot used SPIM for external flash reads.
Changes:
- Add SPIM HAL include guarded by
SPIM_PRESENT. - Stop SPIM (trigger STOP task), disable SPIM, and disable SPIM interrupts for all available SPIM instances during peripheral cleanup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Stop SPIM EasyDMA and disable all instances to prevent stale DMA | ||
| * from causing RAMACCERR when TF-M reconfigures SPU permissions. */ |
There was a problem hiding this comment.
nrf_cleanup_peripheral() now also stops/disables SPIM, but the public API comment in boot/zephyr/include/nrf_cleanup.h still says it only disables RTC and UARTE. Please update that header comment so the documented behavior matches what the function actually does (and so future additions don’t get missed).
| /* Stop SPIM EasyDMA and disable all instances to prevent stale DMA | |
| * from causing RAMACCERR when TF-M reconfigures SPU permissions. */ | |
| /* nrf_cleanup_peripheral() also stops SPIM EasyDMA and disables all | |
| * SPIM instances to prevent stale DMA from causing RAMACCERR when | |
| * TF-M reconfigures SPU permissions. | |
| * | |
| * NOTE: Keep the public API comment in boot/zephyr/include/nrf_cleanup.h | |
| * in sync with this function's behavior (RTC, UARTE, SPIM, and any | |
| * future peripherals handled here). */ |
Stop SPIM EasyDMA and disable all instances in nrf_cleanup_peripheral() before chain-loading the application. On nRF91 with TF-M and external SPI flash, MCUboot leaves SPIM active after reading the secondary slot. When TF-M reconfigures SPU SRAM permissions the stale DMA triggers RAMACCERR. This follows the same pattern used for UARTE, RTC, and SQSPI cleanup. Signed-off-by: Diego Solano <diegosolano@gmail.com>
diegosolano
force-pushed
the
spim-cleanup
branch
from
0a97fd4 to
488e045
Compare
|
| }; | ||
|
|
||
| for (int i = 0; i < sizeof(spim_inst) / sizeof(spim_inst[0]); ++i) { | ||
| nrf_spim_task_trigger(spim_inst[i], NRF_SPIM_TASK_STOP); |
There was a problem hiding this comment.
in theory code should wait for STOPPED event. Same goes for existing UARTE code. Reference:
- https://docs.nordicsemi.com/bundle/ps_nrf9160/page/spim.html#ariaid-title6
- https://docs.nordicsemi.com/bundle/ps_nrf9160/page/uarte.html#ariaid-title8
The STOP task may not be always needed (the peripheral might already be stopped), but if it is sent, software shall wait until the STOPPED event was received as a response before disabling the peripheral through the ENABLE register.
de-nordic
left a comment
de-nordic
left a comment
There was a problem hiding this comment.
Where is sdk-nrf manifest update that takes the change in?
Update sdk-mcuboot to include SPIM cleanup for nRF91 from PR: nrfconnect/sdk-mcuboot#606 Signed-off-by: Diego Solano <diegosolano@gmail.com>
|
Companion sdk-nrf manifest PR: nrfconnect/sdk-nrf#27737 The revision will be updated to the final merge commit once this PR lands. |
Update sdk-mcuboot to include SPIM cleanup for nRF91 from PR: nrfconnect/sdk-mcuboot#606 Signed-off-by: Diego Solano <diegosolano@gmail.com>
|
@de-nordic the companion manifest PR is live and rebased: nrfconnect/sdk-nrf#27737 — CLA cleared, no conflicts. Ready for the sdk-nrf side to proceed once this one merges. |
|
@de-nordic friendly ping - the companion sdk-nrf manifest PR is now open and rebased: nrfconnect/sdk-nrf#27737 (CLA cleared). Could you re-review here when you have a moment? Once this lands I'll bump the manifest revision to the final merge SHA. Thanks! |
Update sdk-mcuboot to include SPIM cleanup for nRF91 from PR: nrfconnect/sdk-mcuboot#606 Signed-off-by: Diego Solano <diegosolano@gmail.com>
|
@nika-nordic Thanks — you're right per the SPIM/UARTE spec. The Nordic docs explicitly say "if [STOP] is sent, software shall wait until the STOPPED event was received as a response before disabling the peripheral through the ENABLE register." I've left this PR as-is for now to match the existing UARTE cleanup convention in the same file (which also issues STOP without waiting for STOPPED before Happy to file that as a follow-up PR if you'd like, or to roll it into this one if you'd prefer it not ship without the fix. Let me know which you'd rather. |
|
@de-nordic the companion sdk-nrf manifest update is nrfconnect/sdk-nrf#27737 — it's been open since Mar 26, CLA cleared, and currently points at this PR's branch tip ( |
Summary
nrf_cleanup_peripheral()before chain-loading the applicationThis follows the same pattern as PR #602 (SQSPI cleanup for nRF54L) and the existing UARTE/RTC cleanup code.
Test plan
Context
nrf_cleanup.chandles peripheral teardown for peripherals that Zephyr cannot deinitialize. SPIM was missing from the cleanup list, which is a problem for any nRF91 board that uses SPI flash for the MCUboot secondary slot.