Skip to content

cpu/sam0_common/flashpage: disable interrupts while writing#21326

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
derMihai:mir/fix_flashpage
Mar 27, 2025
Merged

cpu/sam0_common/flashpage: disable interrupts while writing#21326
benpicco merged 1 commit intoRIOT-OS:masterfrom
derMihai:mir/fix_flashpage

Conversation

@derMihai
Copy link
Copy Markdown
Contributor

@derMihai derMihai commented Mar 26, 2025 •
edited
Loading

Contribution description

This reverts commit fd49d16.
Before, the MCU would hardfault somehwere in _erase_page():

void sam0_flashpage_aux_reset(const nvm_user_page_t *cfg)
{
    nvm_user_page_t old_cfg;

    if (cfg == NULL) {
        cfg = &old_cfg;
        memcpy(&old_cfg, (void*)NVMCTRL_USER, sizeof(*cfg));
    }

    _erase_page((void*)NVMCTRL_USER, _cmd_erase_aux);
    _write_row((void*)NVMCTRL_USER, cfg, sizeof(*cfg), AUX_CHUNK_SIZE, _cmd_write_aux);
}

I messed a bit around and looks like inserting some 100ms delay anywhere between _unlock() and _lock() in _erase_page() also works around the issue. Hope this helps with further debugging.

Testing procedure

Issues/PRs references

The reverted commit was introduced in #21043.

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Mar 26, 2025
@derMihai
Copy link
Copy Markdown
Contributor Author

derMihai commented Mar 26, 2025

@benpicco

@benpicco benpicco requested a review from maribu March 26, 2025 14:39
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 26, 2025
@mguetschow
Copy link
Copy Markdown
Contributor

mguetschow commented Mar 26, 2025

I messed a bit around and looks like inserting some 100ms delay anywhere between _unlock() and _lock() in _erase_page() also works around the issue. Hope this helps with further debugging.

Could you elaborate on why the fix from #21043 is no longer needed?

@riot-ci
Copy link
Copy Markdown

riot-ci commented Mar 26, 2025

Murdock results

✔️ PASSED

f6b9960 Revert "cpu/sam0_common: flashpage: don't disable interruipts while writing"

Success Failures Total Runtime
10287 0 10287 08m:49s

Artifacts

@derMihai
Copy link
Copy Markdown
Contributor Author

derMihai commented Mar 26, 2025

I messed a bit around and looks like inserting some 100ms delay anywhere between _unlock() and _lock() in _erase_page() also works around the issue. Hope this helps with further debugging.

Could you elaborate on why the fix from #21043 is no longer needed?

That PR is made out of two commits. The fix is still in there, this is only about the interrupts.

@benpicco benpicco added this pull request to the merge queue Mar 27, 2025
Merged via the queue into RIOT-OS:master with commit 4ad7df0 Mar 27, 2025
27 checks passed
@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@dylad dylad Awaiting requested review from dylad dylad is a code owner

@keestux keestux Awaiting requested review from keestux keestux is a code owner

@maribu maribu Awaiting requested review from maribu

Assignees

No one assigned

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms

Projects

None yet

Milestone

Release 2025.04

Development

Successfully merging this pull request may close these issues.

4 participants

@derMihai @mguetschow @riot-ci @benpicco