Skip to content
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.

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

pico_bootsel_via_double_reset doesn't work on RP2350 #2043

Closed
earlephilhower opened this issue Nov 14, 2024 · 6 comments · Fixed by #2083
Closed

pico_bootsel_via_double_reset doesn't work on RP2350 #2043

earlephilhower opened this issue Nov 14, 2024 · 6 comments · Fixed by #2083
Assignees
Milestone

Comments

@earlephilhower
Copy link
Contributor

Overview

There is a small module that can be linked into an application to allow a quick double-reset to restart into UF2 loader mode. On the RP2040 this works as long as you simply include target_link_libraries(... pico_bootsel_via_double_reset ...) as it runs as part of the C++ constructor sequence.

Unfortunately, while this compiles and runs on the RP2350 (-DPICO_PLATFORM=rp2350) it doesn't function properly.

Debugging

On inspection, it seems like the magic numbers that are written to SRAM are actually overwritten by the Pico2 bootrom processing (with what looks to me to be repeatable, pseudo-random data) so that it can never actually match by the time the main app starts up and calls the C++ (constructor) that handles this.

I've increased the wait time via the define, but it's not related to this (i.e. it's not being reset too quick or slow).

When I did some snooping, it seems like main SRAM and the two 4K stack SRAMs are a) 0-filled after a UF2 upload, but b) pseudo-random filled after a HW (!RUN ) reset on the Pico2-ARM mode.

Maybe this is part of the secure booting process? If so, is there some RAM that's not wiped and not used during start up that we can stuff those magic values in?

Steps to reproduce

In pico-examples, update the basic blink_simple CMakefile.txt

earle@amd:~/src/pico-examples/blink_simple$ git diff .
diff --git a/blink_simple/CMakeLists.txt b/blink_simple/CMakeLists.txt
index 0d7c0cb..32e897b 100644
--- a/blink_simple/CMakeLists.txt
+++ b/blink_simple/CMakeLists.txt
@@ -3,7 +3,7 @@ add_executable(blink_simple
 )
 
 # pull in common dependencies
-target_link_libraries(blink_simple pico_stdlib)
+target_link_libraries(blink_simple pico_stdlib pico_bootsel_via_double_reset)
 
 # create map/bin/hex/uf2 file etc.
 pico_add_extra_outputs(blink_simple)

And build with -DPICO_PLATFORM=rp2350. Building the same for RP2040 works perfectly.

@kilograham
Copy link
Contributor

Ah, RP2350 has this support in the bootrom, but you have to enable it via:

// -----------------------------------------------------------------------------
// Field       : OTP_DATA_BOOT_FLAGS1_DOUBLE_TAP
// Description : Enable entering BOOTSEL mode via double-tap of the RUN/RSTn
//               pin. Adds a significant delay to boot time, as configured by
//               DOUBLE_TAP_DELAY.
//
//               This functions by waiting at startup (i.e. following a reset)
//               to see if a second reset is applied soon afterward. The second
//               reset is detected by the bootrom with help of the
//               POWMAN_CHIP_RESET_DOUBLE_TAP flag, which is not reset by the
//               external reset pin, and the bootrom enters BOOTSEL mode
//               (NSBOOT) to await further instruction over USB or UART.

As you suspected, RAM is zeroed in the boot path.

We likely forgoot about ramifications on this library. We should either make clear this library functionality is disabled on RP2350; or implement it differently for RP2350 (if having non OTP chosen way is useful which it might be). Thoughts @Wren6991

@kilograham kilograham added this to the 2.1.0 milestone Nov 14, 2024
@earlephilhower
Copy link
Contributor Author

Might also want to remove the uninitialized_data section in the RP2350 linker files, too, since they're gonna be overwritten by the ROM.

@Wren6991
Copy link
Contributor

Wren6991 commented Nov 21, 2024

As you suspected, RAM is zeroed in the boot path.

We don't unconditionally zero all of RAM, except when entering NSBOOT (as it's an S -> NS transition).

Where is that zeroing coming from? If it's due to LOAD_MAP then maybe we can teach picotool about the uninitialized_data section. This is not the right solution for double-tap on RP2350, but it is a solution. (Also people might be using uninitialized_data for other things.) Ignore this, I've now seen that you're getting random contents in the scratch RAMs, which is because they're getting powered down.

@Wren6991
Copy link
Contributor

Ah, the problem is more fundamental than that: asserting the RUN pin powers down the memories, and they power back up when it's released.

This is specific to double-tap, and I'm not sure there is actually any issue with uninitialized_data.

@Wren6991
Copy link
Contributor

The only state that is retained when the RUN pin is asserted is the DOUBLE_TAP flag itself, in POWMAN. (The whole point of RUN is to be a global reset, and RAM being retained on RP2040 was just due to the lack of power gating on the RAMs.)

If we want to keep that library then the best option is probably actually to use the same DOUBLE_TAP flag that the ROM uses.

@kilograham
Copy link
Contributor

merged into develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants