Skip to content

soc: nordic: nrf71: fix LMAC ROM patch entry address and WICR programming - #117358

Open
krish2718 wants to merge 4 commits into
zephyrproject-rtos:mainfrom
krish2718:nrf71-wicr-fixes
Open

soc: nordic: nrf71: fix LMAC ROM patch entry address and WICR programming#117358
krish2718 wants to merge 4 commits into
zephyrproject-rtos:mainfrom
krish2718:nrf71-wicr-fixes

Conversation

@krish2718

Copy link
Copy Markdown
Contributor

Two bugs and three cleanups in the nRF7120/nRF7120e WICR path, found while debugging a Wi-Fi core that came out of reset and ran but never completed initialisation.

LMAC ROM patch entry address is off by eight bytes

The LMAC ROM reads the patch address from WICR and calls it:

clear_bss_done:
        beqz  s0, patch_init_skip
        lui   a0, 0x3fd
        lw    a0, 8(a0)          ; LMACROMPATCHADDR
        jalr  a0

so it has to be a function entry. With no patch applied that function is patch_init_orig at 0x2808016a, which loads three arguments and tail-calls mvdmaCopyRegion to place the LMAC fast code:

patch_init_orig:                 ; 0x2808016a
        auipc a0, 0xfff87
        addi  a0, a0, 1374       ; __fast_start
        auipc a1, 0xfff8a        ; 0x28080172
        addi  a1, a1, -474       ; __fast_end
        auipc a2, 0x3f
        addi  a2, a2, -238       ; __fast_load_start
        j     mvdmaCopyRegion

The devicetree node used 0x28080172, which is patch_init_orig + 8 — the address that appears as a comment on the auipc a1 line above. a0 is therefore never set and the copy runs with a stale register. The LMAC starts and runs but its fast code is never placed, so it never reports RTPENABLED and never enables its bellboard receive interrupt: the application core's doorbell latches (EVENTS_TRIGGERED[2] = 1) but is never delivered, and no IPC is possible.

The UMAC node was already correct at its own patch_init_orig (0x2818011c), which is what made the asymmetry visible. The node is shared between nRF7120 and nRF7120e, which run the same firmware.

WICR wear-avoidance skip never triggered

wicr_setup() compares each WICR word against its intended value and skips the write when they match, to limit MRAM wear. The comparison was done before the CONFIGNVR page was unlocked, but the page permissions gate reads as well as writes: every read returned 0xFFFFFFFF, no word ever matched, and all ten words were rewritten to MRAM on every boot — the opposite of the comment's intent. Unlock before the comparison, and wait for MRAMC ready before the read-back and before locking again.

Cleanups

  • soc.h selected the antenna switch control register address using __NRF_TFM__, while soc.c had already moved to NRF_APPLICATION/__ZEPHYR__. Use !defined(__ZEPHYR__); the selection is unchanged in all three build configurations.
  • SOC_NRF7120_WICR_SETUP selected NRFX_MRAMC, which SOC_FLASH_NRF_MRAMC already implies under the same condition.
  • The comment above the ROM patch address override attributed the addresses to an out-of-tree patch; they come from the build system via the patch manifest as a compile definition.

Testing

nRF7120 FPGA, cpuapp, with and without a Wi-Fi ROM patch build. No silicon available yet.

Before: VPRSTATUS = 0x2, wifi_bellboard INTEN0 = 0, event mailbox never written.
After: VPRSTATUS = 0x12 (SLEEPING + RTPENABLED), INTEN0 = 0x4 — the channel the ipc0 node declares — and Wi-Fi IPC works.

Addresses were derived from the LMAC and UMAC ROM ELF symbol tables and disassembly.

wicr_setup() compares each WICR word against its intended value and
skips the write when they already match, to limit MRAM wear. The
comparison was done before the CONFIGNVR page was unlocked, but the page
permissions gate reads as well as writes, so every read returned
0xFFFFFFFF and no word ever matched. All ten words were rewritten to
MRAM on every boot.

Unlock the page before the loop so the comparison sees the stored
contents. Also wait for MRAMC to report ready before reading a written
value back and before locking the page again.

Signed-off-by: Chaitanya Tata <Chaitanya.Tata@nordicsemi.no>
Assisted-by: Claude:claude-opus-5
__NRF_TFM__ is defined out of tree, so soc.c selects the secure or
non-secure address of a peripheral from NRF_APPLICATION and __ZEPHYR__
instead. soc.h still used __NRF_TFM__ to pick the address of the antenna
switch control register, leaving the two files disagreeing on how a
secure build is identified.

Use !defined(__ZEPHYR__) for the same purpose. The selection is
unchanged in all three cases: a TF-M build does not define __ZEPHYR__, a
secure Zephyr build does not define CONFIG_TRUSTED_EXECUTION_NONSECURE,
and a non-secure Zephyr build defines the latter but not the former.

Signed-off-by: Chaitanya Tata <Chaitanya.Tata@nordicsemi.no>
Assisted-by: Claude:claude-opus-5
@krish2718

Copy link
Copy Markdown
Contributor Author

The comment says the patch addresses come from an out-of-tree patch,
which sends the reader looking for a drop-in file. They are supplied by
the build system when a Wi-Fi ROM patch is included: the address is read
from the patch manifest, reserved in MRAM, and passed in as a compile
definition. Say so, and note what the devicetree fallback means.

No functional change.

Signed-off-by: Chaitanya Tata <Chaitanya.Tata@nordicsemi.no>
Assisted-by: Claude:claude-opus-5
The LMAC ROM reads the patch address out of WICR and calls it:

  clear_bss_done:
        beqz  s0, patch_init_skip
        lui   a0, 0x3fd
        lw    a0, 8(a0)        ; LMACROMPATCHADDR
        jalr  a0

so the address has to be a function entry. With no patch applied that
function is patch_init_orig, which loads three arguments and tail-calls
mvdmaCopyRegion to place the LMAC fast code:

  patch_init_orig:              ; 0x2808016a
        auipc a0, 0xfff87
        addi  a0, a0, 1374     ; __fast_start
        auipc a1, 0xfff8a      ; 0x28080172
        addi  a1, a1, -474     ; __fast_end
        auipc a2, 0x3f
        addi  a2, a2, -238     ; __fast_load_start
        j     mvdmaCopyRegion

The node used 0x28080172, which is patch_init_orig plus eight bytes, so
the first argument is never set and the copy runs with whatever a0 held.
The LMAC starts and runs but its fast code is never placed, so it never
finishes initialising: it does not report RTPENABLED and never enables
its bellboard receive interrupt, leaving the application core's doorbell
latched but undelivered and no IPC possible.

Point the node at patch_init_orig. The UMAC node is already correct, at
its own patch_init_orig.

Signed-off-by: Chaitanya Tata <Chaitanya.Tata@nordicsemi.no>
Assisted-by: Claude:claude-opus-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants