gh-raspberrypi/pico-sdk

Watch

Fork 0

RP2350: `boot_stage2` never run? resulting in incorrect flash config #1903

New issue

Open

opened 2024-09-04 12:06:47 -05:00 by asumagic · 8 comments

asumagic commented

2024-09-04 12:06:47 -05:00

(Migrated from github.com)

I recently noticed that PICO_FLASH_SPI_CLKDIV was not having any effect on my project ported to RP2350, whereas it worked fine with RP2040 before.

This reproduced when using the stock Pico 2 and Pimoroni Pico Plus 2 board files, which use the same boot2_w25q080.S boot_stage2. It does get assembled.

The QMI timing values I am seeing on a MCVE (provided below) don't match my expectations reading the boot script.

qmi[0].timing = 60007203
qmi[0].rfmt   = 492a8
qmi[0].rcmd   = 00eb

When ripping out all the code from boot2_w25q080.S, behavior is unchanged! (Adding garbage to it, it fails to assemble, so it is still getting selected somewhere.)

The lower byte of timing should be 2 (the default CLKDIV), the rcmd is missing the upper byte that should have gotten configured, etc. -- so I am assuming it is inheriting whatever the bootrom had configured.

(EDIT: Writing to those regs works fine; I've been doing it.)

I observed this issue both selecting the 2.0.0 tag and the latest develop tip.

Repro:

#include <stdio.h>
#include <hardware/structs/qmi.h>
#include <hardware/structs/xip.h>
#include <pico/stdlib.h>

int main()
{
    stdio_init_all();
    while (true) {
        printf("qmi[0].timing = %04x\n", qmi_hw->m[0].timing);
        printf("qmi[0].rfmt   = %04x\n", qmi_hw->m[0].rfmt);
        printf("qmi[0].rcmd   = %04x\n", qmi_hw->m[0].rcmd);
        sleep_ms(1000);
    }

}

cmake_minimum_required(VERSION 3.13)

include(pico_sdk_import.cmake)

project(pico-sdk-boot2-rp2350-repro LANGUAGES C CXX ASM)
pico_sdk_init()

add_executable(repro main.c)
target_link_libraries(repro pico_stdlib pico_stdio)
pico_enable_stdio_usb(repro 1)
pico_enable_stdio_uart(repro 0)
pico_add_extra_outputs(repro)

#!/bin/bash
set -ex
mkdir build -p
cd build
cmake .. -DPICO_BOARD=pico2 -DPICO_PLATFORM=rp2350-arm-s -DPICO_COMPILER=pico_arm_cortex_m33_gcc -DPICO_SDK_PATH="<whatever_sdk_path_here>"  -GNinja
cmake --build .

I recently noticed that `PICO_FLASH_SPI_CLKDIV` was not having any effect on my project ported to RP2350, whereas it worked fine with RP2040 before. This reproduced when using the stock Pico 2 and Pimoroni Pico Plus 2 board files, which use the same `boot2_w25q080.S` `boot_stage2`. It _does_ get assembled. The QMI timing values I am seeing on a MCVE (provided below) don't match my expectations reading the boot script. ``` qmi[0].timing = 60007203 qmi[0].rfmt = 492a8 qmi[0].rcmd = 00eb ``` When ripping out all the code from `boot2_w25q080.S`, behavior is unchanged! (Adding garbage to it, it fails to assemble, so it is _still_ getting selected somewhere.) The lower byte of `timing` should be `2` (the default CLKDIV), the `rcmd` is missing the upper byte that should have gotten configured, etc. -- so I am assuming it is inheriting whatever the bootrom had configured. (EDIT: Writing to those regs works fine; I've been doing it.) I observed this issue both selecting the `2.0.0` tag and the latest `develop` tip. Repro: ```c #include <stdio.h> #include <hardware/structs/qmi.h> #include <hardware/structs/xip.h> #include <pico/stdlib.h> int main() { stdio_init_all(); while (true) { printf("qmi[0].timing = %04x\n", qmi_hw->m[0].timing); printf("qmi[0].rfmt = %04x\n", qmi_hw->m[0].rfmt); printf("qmi[0].rcmd = %04x\n", qmi_hw->m[0].rcmd); sleep_ms(1000); } } ``` ```cmake cmake_minimum_required(VERSION 3.13) include(pico_sdk_import.cmake) project(pico-sdk-boot2-rp2350-repro LANGUAGES C CXX ASM) pico_sdk_init() add_executable(repro main.c) target_link_libraries(repro pico_stdlib pico_stdio) pico_enable_stdio_usb(repro 1) pico_enable_stdio_uart(repro 0) pico_add_extra_outputs(repro) ``` ```shell #!/bin/bash set -ex mkdir build -p cd build cmake .. -DPICO_BOARD=pico2 -DPICO_PLATFORM=rp2350-arm-s -DPICO_COMPILER=pico_arm_cortex_m33_gcc -DPICO_SDK_PATH="<whatever_sdk_path_here>" -GNinja cmake --build . ```

kilograham commented

2024-09-04 12:14:31 -05:00

(Migrated from github.com)

Per release notes:

A boot_stage2 is not needed on RP2350, but one can be included via the define PICO_EMBED_XIP_SETUP=1.

Could perhaps be better documented (this is a compile definition).. also perhaps surprising that it builds without it but that is orthogonal

Per release notes: * A `boot_stage2` is not needed on RP2350, but one can be included via the define `PICO_EMBED_XIP_SETUP=1`. Could perhaps be better documented (this is a compile definition).. also perhaps surprising that it builds without it but that is orthogonal

👍 1

asumagic commented

2024-09-04 12:22:32 -05:00

(Migrated from github.com)

For clarification, by "ripping out all the code" from the stage2 I mostly meant the instructions under _stage2_boot.

But thank you, PICO_EMBED_XIP_SETUP=1 does indeed solve the problem. However, shouldn't that be the default for any non-default flash configuration?

For clarification, by "ripping out all the code" from the stage2 I mostly meant the instructions under `_stage2_boot`. But thank you, `PICO_EMBED_XIP_SETUP=1` does indeed solve the problem. However, shouldn't that be the default for any non-default flash configuration?

tonycarbon commented

2024-10-14 09:10:57 -05:00

(Migrated from github.com)

sorry can someone explain where to add the PICO_EMBED_XIP_SETUP=1 command ?

asumagic commented

2024-10-14 09:37:49 -05:00

(Migrated from github.com)

sorry can someone explain where to add the PICO_EMBED_XIP_SETUP=1 command ?

In CMakeLists.txt, after your target is specified:

target_compile_definitions(thenameofyourtarget PRIVATE
		PICO_EMBED_XIP_SETUP=1
)

Note that I had an issue enabling this where the program would immediately crash when debugging(?), which I haven't tried to test on a standard Pico 2. I haven't tried to debug it further.

I observed a weird issue where i haven't been able to enable continuous reads through the QMI registers, as it would just immediately crash upon returning to execution from flash.

> sorry can someone explain where to add the PICO_EMBED_XIP_SETUP=1 command ? In `CMakeLists.txt`, after your target is specified: ```cmake target_compile_definitions(thenameofyourtarget PRIVATE PICO_EMBED_XIP_SETUP=1 ) ``` Note that I had an issue enabling this where the program would immediately crash when debugging(?), which I haven't tried to test on a standard Pico 2. I haven't tried to debug it further. I observed a weird issue where i haven't been able to enable continuous reads through the QMI registers, as it would just immediately crash upon returning to execution from flash.

asumagic commented

2024-10-14 09:41:20 -05:00

(Migrated from github.com)

#include <hardware/structs/qmi.h>
#include <hardware/structs/xip.h>
#include <hardware/sync.h>

void __no_inline_not_in_flash_func(init_flash_frequency)() {
	uint32_t interrupt_state = save_and_disable_interrupts();

	qmi_hw->m[0].timing = 1 << QMI_M0_TIMING_COOLDOWN_LSB |
	                      2 << QMI_M0_TIMING_RXDELAY_LSB |
	                      3 << QMI_M0_TIMING_CLKDIV_LSB;   // <----- CLOCK DIVISOR HERE
	qmi_hw->m[0].rcmd =
		0xEB << QMI_M0_RCMD_PREFIX_LSB /* | 0xA0 << QMI_M0_RCMD_SUFFIX_LSB*/;

	// dummy read
	*reinterpret_cast<volatile char *>(XIP_NOCACHE_NOALLOC_BASE);

	restore_interrupts(interrupt_state);
}

Call that function before initializing the second core (if you use it at all) and before changing the clock frequency, if this is for core overclocking reasons.

If you only care about changing the clock divisor for flash, you can do this at runtime, with something like this (in C++), assuming the Pico 2 flash (including the Pimoroni Pico Plus 2) -- at least AFAIK: ```cpp #include <hardware/structs/qmi.h> #include <hardware/structs/xip.h> #include <hardware/sync.h> void __no_inline_not_in_flash_func(init_flash_frequency)() { uint32_t interrupt_state = save_and_disable_interrupts(); qmi_hw->m[0].timing = 1 << QMI_M0_TIMING_COOLDOWN_LSB | 2 << QMI_M0_TIMING_RXDELAY_LSB | 3 << QMI_M0_TIMING_CLKDIV_LSB; // <----- CLOCK DIVISOR HERE qmi_hw->m[0].rcmd = 0xEB << QMI_M0_RCMD_PREFIX_LSB /* | 0xA0 << QMI_M0_RCMD_SUFFIX_LSB*/; // dummy read *reinterpret_cast<volatile char *>(XIP_NOCACHE_NOALLOC_BASE); restore_interrupts(interrupt_state); } ``` Call that function _before_ initializing the second core (if you use it at all) and _before_ changing the clock frequency, if this is for core overclocking reasons.

XZCE commented

2024-11-04 23:54:25 -06:00

(Migrated from github.com)

@asumagic your code in the post above works if you just want to read flash after overclocking, but it didn't work for me when I start to write flash. I suspect the SDK reverts the divider at the end of each call to the write function. PICO_EMBED_XIP_SETUP=1 does however work for both.

But thanks for raising this, it helped me a lot!

asumagic commented

2024-11-05 02:28:00 -06:00

(Migrated from github.com)

@asumagic your code in the post above works if you just want to read flash after overclocking, but it didn't work for me when I start to write flash. I suspect the SDK reverts the divider at the end of each call to the write function. PICO_EMBED_XIP_SETUP=1 does however work for both.

But thanks for raising this, it helped me a lot!

You are correct: #1983

> @asumagic your code in the post above works if you just want to read flash after overclocking, but it didn't work for me when I start to write flash. I suspect the SDK reverts the divider at the end of each call to the write function. PICO_EMBED_XIP_SETUP=1 does however work for both. > > But thanks for raising this, it helped me a lot! You are correct: #1983

Wren6991 commented

2024-11-21 14:28:06 -06:00

(Migrated from github.com)

I suspect the SDK reverts the divider at the end of each call to the write function.

Yeah, the ROM creates a tiny stub for you in boot RAM (where the copy of the XIP setup is stored if you have one) which restores whatever flash parameters it found during startup. The SDK executes that XIP setup from boot RAM after completing a flash operation, to set flash back up again. This is mostly a big improvement over RP2040 since you can just wire up say a dual-SPI flash on your board and the ROM will figure things out and boot into your flash application without any configuration.

The intent when modifying the XIP configuration from the parameters found by the ROM is:

you have a small function to set up XIP (XIP setup func, similar to RP2040 boot2 except you call it instead of the ROM calling it)
you execute this function during startup
you make a copy of this function (or a stub that calls it) in boot RAM so that the SDK flash API can call it when it needs to set XIP back up after tearing things down for serial programming operations

PICO_EMBED_XIP_SETUP=1 does exactly this. It just doesn't help you out if you've done raw register accesses.

We could improve the setup-less behaviour, e.g. by creating a new stub that sets your divider if you have specified a PICO_FLASH_SPI_CLKDIV but haven't specified a boot2. Something else I think would ease confusion is making PICO_EMBED_XIP_SETUP=1 be implied by selecting a boot2. I don't think it's going to happen for this upcoming release though.

> I suspect the SDK reverts the divider at the end of each call to the write function. Yeah, the ROM creates a tiny stub for you in boot RAM (where the copy of the XIP setup is stored if you have one) which restores whatever flash parameters it found during startup. The SDK executes that XIP setup from boot RAM after completing a flash operation, to set flash back up again. This is mostly a big improvement over RP2040 since you can just wire up say a dual-SPI flash on your board and the ROM will figure things out and boot into your flash application without any configuration. The intent when modifying the XIP configuration from the parameters found by the ROM is: * you have a small function to set up XIP (XIP setup func, similar to RP2040 boot2 except *you* call it instead of the ROM calling it) * you execute this function during startup * you make a copy of this function (or a stub that calls it) in boot RAM so that the SDK flash API can call it when it needs to set XIP back up after tearing things down for serial programming operations `PICO_EMBED_XIP_SETUP=1` does exactly this. It just doesn't help you out if you've done raw register accesses. We could improve the setup-less behaviour, e.g. by creating a new stub that sets your divider if you have specified a `PICO_FLASH_SPI_CLKDIV` but haven't specified a boot2. Something else I think would ease confusion is making `PICO_EMBED_XIP_SETUP=1` be implied by selecting a boot2. I don't think it's going to happen for this upcoming release though.