Bluetooth: controller: Fix MRAM latency race condition on nRF54H20

[Copilot]

• Review and understand the current implementation
• Add atomic variable to track MRAM request completion state
• Implement callback function for MRAM request completion
• Update radio_reset() to use sys_notify_init_callback()
• Update radio_stop() to wait for completion before cancel/release
• Address code review feedback: Add timeout and arch_spin_relax()
• Define named constant for timeout iterations
• Add documentation for error handling in ISR context
• Run final security check (no issues found)
• Add validation checks for state, res, and return values

Summary

Successfully implemented callback-based MRAM no latency synchronization to fix race conditions when requests and releases happen in quick succession. The solution includes:

1. Callback-based notification: Replaced sys_notify_init_spinwait() with sys_notify_init_callback() to properly track request completion
2. Completion tracking: Added atomic variable to track when MRAM request completes
3. Safe waiting: Wait for completion in radio_stop() before calling cancel/release
4. Timeout protection: Added conservative timeout (1M iterations) to prevent infinite loops
5. CPU-friendly waiting: Uses arch_spin_relax() to avoid wasting CPU cycles
6. Proper validation: Added LL_ASSERT_DBG checks for callback parameters and return values
7. Proper documentation: Added comments explaining the rationale and ISR context constraints

Original prompt

Problem

In the pull request zephyrproject-rtos#95417 (branch: github_nrf54h20_cpurad_bt_ll_sw_split_support), the mram_no_latency_request() and mram_no_latency_cancel_or_release() functions are used with sys_notify_init_spinwait() for synchronization.

The current implementation in subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c is:

#if defined(CONFIG_SOC_SERIES_NRF54H)
static struct onoff_client mram_cli;
static atomic_val_t mram_refcnt;
#endif /* CONFIG_SOC_SERIES_NRF54H */

void radio_reset(void)
{
    // ... other code ...
    
#if defined(CONFIG_SOC_SERIES_NRF54H)
    atomic_val_t refcnt;

    refcnt = atomic_inc(&mram_refcnt);
    if (refcnt == 0) {
        sys_notify_init_spinwait(&mram_cli.notify);

        (void)mram_no_latency_request(&mram_cli);
    } else {
        /* Nothing to do, reference count increased. */
    }
#endif /* CONFIG_SOC_SERIES_NRF54H */
    // ...
}

void radio_stop(void)
{
    // ... other code ...
    
#if defined(CONFIG_SOC_SERIES_NRF54H)
    atomic_val_t refcnt;

    refcnt = atomic_get(&mram_refcnt);
    if (refcnt > 0) {
        refcnt = atomic_dec(&mram_refcnt);
        if (refcnt == 1) {
            (void)mram_no_latency_cancel_or_release(&mram_cli);
        } else {
            /* Nothing to do, reference count decremented. */
        }
    } else {
        /* TODO: Development assertion on already cancel/release. */
    }
#endif /* CONFIG_SOC_SERIES_NRF54H */
}

Issue

When mram_no_latency_request() and mram_no_latency_cancel_or_release() are called in quick succession, the cancel/release call may fail because the request hasn't completed yet. This can leave the MRAM no latency request remaining on always, causing potential issues.

Required Changes

Instead of using sys_notify_init_spinwait(), implement a callback-based approach:

1. Add a state variable to track whether mram_no_latency_request() has completed successfully
2. Use sys_notify_init_callback() instead of sys_notify_init_spinwait()
3. Implement a callback function that sets the completion state when the request finishes
4. In radio_stop(), before calling mram_no_latency_cancel_or_release():
   • Check if the request has completed
   • If not completed, wait for completion (spin or use appropriate waiting mechanism)
   • Only then call mram_no_latency_cancel_or_release()

Implementation Guidelines

The callback approach should:

• Use a static atomic variable to track the completion state (e.g., mram_no_latency_complete)
• Define a callback function that atomically sets the completion state to true
• In radio_reset(), clear the completion state before making the request, then use sys_notify_init_callback() with the callback function
• In radio_stop(), wait (spin) until the completion state is true before calling mram_no_latency_cancel_or_release()

This ensures proper synchronization and prevents race conditions when requests and releases happen in quick succession.

Base Branch

This change should be based on the github_nrf54h20_cpurad_bt_ll_sw_split_support branch in cvinayak/zephyr repository.

This pull request was created from Copilot chat.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[cvinayak]

@copilot add implementation to check state and res values.

[Copilot]

Added validation checks for state and res parameters in the callback using LL_ASSERT_DBG. The callback now verifies that res == 0 (success) and state == ONOFF_STATE_ON (on state). Commit: f3f0574

[cvinayak]

@copilot add implementation to check return value.

[Copilot]

Added check for the return value of mram_no_latency_request(). The function now stores the return value and validates that it's non-negative using LL_ASSERT_DBG(ret >= 0). Commit: f3f0574

[cvinayak]

@copilot add implementation to check return value.

[Copilot]

Added check for the return value of mram_no_latency_cancel_or_release(). The function now stores the return value and validates that it's non-negative using LL_ASSERT_DBG(ret >= 0). Commit: f3f0574