Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 27, 2025
Merged

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

merged 1 commit into from
Mar 27, 2025

Conversation

derMihai
Copy link
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
Contributor Author

@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
Contributor

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

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
Contributor Author

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