Skip to content

Make sure the whole RTT structure is in RAM#683

Merged
bors[bot] merged 4 commits intoknurling-rs:mainfrom
jannic:avoid-flash-reads
Jul 14, 2022
Merged

Make sure the whole RTT structure is in RAM#683
bors[bot] merged 4 commits intoknurling-rs:mainfrom
jannic:avoid-flash-reads

Conversation

@jannic
Copy link
Contributor

@jannic jannic commented Jul 8, 2022

Currently, the name field of the RTT channel will be placed in
flash (rodata). That causes the debug probe to read from flash while
parsing the RTT header.

Usually this doesn't matter, but if the firmware disables flash access
before the debug probe scanned the header, it will break.

The situation where I stumbled over this was writing a firmware which
writes to flash on an RP2040. While writing to the (external) flash
chip, no concurrent flash accesses must happen.

This patch forces the name to the .data section, so the whole RTT
header can be read from RAM.

jannic added 2 commits July 8, 2022 11:31
Currently, the name field of the RTT channel will be placed in
flash (rodata). That causes the debug probe to read from flash while
parsing the RTT header.

Usually this doesn't matter, but if the firmware disables flash access
before the debug probe scanned the header, it will break.

The situation where I stumbled over this was writing a firmware which
writes to flash on an RP2040. While writing to the (external) flash
chip, no concurrent flash accesses must happen.

This patch forces the name to the `.data` section, so the whole RTT
header can be read from RAM.
@Urhengulas
Copy link
Member

Thank you for the PR! @jonas-schievink can you please take a brief look?

@jonas-schievink
Copy link
Contributor

Could you add an inline comment explaining the reasoning? Other than that, this seems reasonable

@Urhengulas
Copy link
Member

bors r=jonas-schievink

@bors
Copy link
Contributor

bors bot commented Jul 14, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants