rom_reset_usb_boot_extra appears to pass p0/p1 to rom_reboot in the opposite order from the documented BOOTSEL ABI

[anoshyn]

What's wrong

There is a contradiction between (a) the documented REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL parameter ABI and (b) what the SDK's rom_reset_usb_boot_extra() wrapper actually passes to rom_reboot().

Documented ABI

src/common/boot_picoboot_headers/include/boot/picoboot_constants.h:15:

#define REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL      0x2 // param0 = gpio_pin_number, param1 = flags

src/rp2_common/pico_bootrom/include/pico/bootrom.h:469-475:

 * REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL - reboot into BOOTSEL mode.
 *  p0 - the GPIO number to use as an activity indicator (enabled by flag in p1).
 *  p1 - a set of flags:
 *   0x01 : DISABLE_MSD_INTERFACE
 *   0x02 : DISABLE_PICOBOOT_INTERFACE
 *   0x10 : GPIO_PIN_ACTIVE_LOW
 *   0x20 : GPIO_PIN_ENABLED

So per the docs: p0 = GPIO number, p1 = flags.

What the SDK does

src/rp2_common/pico_bootrom/bootrom.c:46 (RP2040 fallback path):

rom_reboot(REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL | REBOOT2_FLAG_NO_RETURN_ON_SUCCESS, 10, flags, usb_activity_gpio_pin_mask);
//                                                                                   ^p0    ^p1

src/rp2_common/pico_bootrom/bootrom.c:66 (newer path, used on RP2350):

rom_reboot(REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL | REBOOT2_FLAG_NO_RETURN_ON_SUCCESS, 10, flags, (uint)usb_activity_gpio_pin);
//                                                                                   ^p0    ^p1

Both pass p0 = flags, p1 = GPIO — the opposite of the documented ABI.

rom_reboot() itself just forwards (flags, delay_ms, p0, p1) to rom_func_lookup_inline(ROM_FUNC_REBOOT), so the wrapper isn't doing any swapping under the hood.

How I noticed

While preparing PR #2947 (fix for the wValue bit-overlap in pico_stdio_usb/reset_interface.c, issue #2713), I tried an end-to-end test on a Pico 2 (RP2350) using the bootsel-reset USB request to specify GPIO 25 as the activity-LED with active_low=true:

wValue = (25 << 10) | 0x200 | 0x100 = 0x6700

The expected observable behaviour during BOOTSEL: with active_low=true the bootrom should hold the activity GPIO HIGH during idle (so the Pico 2's onboard LED on GPIO 25 lights solid). What I actually observed: LED steady off, regardless of active_low value, in both active_low=false and active_low=true runs and with MSD enabled (disable_mask=0) so there was clear MSD enumeration / I/O activity.

The wValue parsing in reset_interface.c is correct (verified by reading the code), so the breakage is downstream of that — at the rom_reset_usb_boot_extra → rom_reboot call boundary, exactly where the apparent p0/p1 swap is.

Which side is wrong?

I'm not in a position to say with certainty which of the two is the source of truth (the bootrom's actual behaviour, vs. the SDK doc):

• If the doc is correct and the bootrom genuinely takes (p0=gpio, p1=flags), then bootrom.c:46 and :66 need to swap the last two args, and the activity-LED feature has been silently broken whenever usb_activity_gpio_pin >= 0 was passed. That matches what I observed.

• If the SDK code is correct and the bootrom actually takes (p0=flags, p1=gpio), then the doxygen in bootrom.h and the comment in picoboot_constants.h are stale and should be updated. In that case, the activity LED not lighting on my hardware would have a different explanation, which I'd want to understand before closing this.

Picotool's PC_REBOOT2 call (raspberrypi/picotool main.cpp) only passes dParam0=0, dParam1=0, so it doesn't exercise the activity-LED path and can't disambiguate.

Repro / environment

• SDK: latest develop (origin/develop, includes commit 2492b1c).
• Board: Pico 2 (RP2350), PID 2e8a:0009 in app mode, 2e8a:000f in BOOTSEL.
• Firmware: minimal app linking pico_stdlib with pico_enable_stdio_usb(... 1) and PICO_STDIO_USB_RESET_BOOTSEL_FIXED_ACTIVITY_LED=0.
• Host trigger: libusb control transfer, bmRequestType=0x21, bRequest=0x01 (RESET_REQUEST_BOOTSEL), wValue=0x6700, wIndex=2 (reset interface number from lsusb -v).
• Observed: device reboots into BOOTSEL (PID 2e8a:000f), MSD enumerates, files browsable; activity-LED on GPIO 25 remains off throughout, including during sustained MSD reads.

Happy to gather any additional traces (e.g. a UART-stdio variant of the same firmware that prints gpio and active_low right before calling rom_reset_usb_boot_extra) if that would help triage.

[lurch]

• activity-LED on GPIO 25 remains off throughout, including during sustained MSD reads.

I guess if you could find some combination of fixes between this PR and/or #2947 where you are able to get the activity LED to blink as expected, that would help to track down "Which side is wrong?" 😉

[anoshyn]

Hardware test today inverts what I had concluded when filing this — the SDK code is right, the doc is stale.

What I ran

Minimal RP2350 firmware that calls rom_reboot directly with the doc-correct argument order — i.e. the swap of what rom_reset_usb_boot_extra() currently uses:

rom_reboot(
    REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL | REBOOT2_FLAG_NO_RETURN_ON_SUCCESS,
    /* delay_ms */ 10,
    /* p0       */ 25,    // gpio per the doc
    /* p1       */ 0x30   // flags per the doc: GPIO_PIN_SPECIFIED | GPIO_PIN_ACTIVE_LOW
);

If the doc were right, BOOTSEL should come back up with MSD enabled and the activity LED on GPIO 25 driven idle-high.

What I observed

• lsusb shows 2e8a:000f — BOOTSEL re-entered, PICOBOOT interface up.
• MSD interface is gone — no /media/$USER/RP2350/ mount, no /dev/sd* block device.
• LED stays off.

Why this is conclusive

Hypothesis	What 25 means	What 0x30 means	Expected outcome
Doc is right (p0=gpio, p1=flags)	gpio = 25	flags = GPIO_PIN_SPECIFIED | ACTIVE_LOW, no disable bits	MSD up, LED solid on
SDK is right (p0=flags, p1=gpio)	flags = 0x19 = DISABLE_MSD_INTERFACE | 0x08 | ACTIVE_LOW, GPIO_PIN_SPECIFIED not set	gpio = 48 (irrelevant, since GPIO_PIN_SPECIFIED is off)	MSD disabled, LED off

The MSD disappearing is the smoking gun. Under the doc-correct interpretation there was no way for the bootrom to disable MSD from these args. The only way it could have parsed DISABLE_MSD_INTERFACE (bit 0) out of them is by reading p0 as flags — which is what the SDK does. So the SDK code is correct; the doc and comment are reversed.

What actually needs to change

• src/common/boot_picoboot_headers/include/boot/picoboot_constants.h:15 — // param0 = gpio_pin_number, param1 = flags is reversed.
• src/rp2_common/pico_bootrom/include/pico/bootrom.h:469-475 — the REBOOT2_FLAG_REBOOT_TYPE_BOOTSEL doxygen has the p0/p1 descriptions swapped.

rom_reset_usb_boot_extra() itself stays as-is. Happy to put up a doc-only PR for the swap.

Separate, still-open question

The activity LED on GPIO 25 still doesn't light up in any SDK-order test I've run on this Pico 2 (active_low true or false, disable_mask = 0 so MSD is enumerating and serving reads). That's independent of the p0/p1 question. If there's a known-good configuration where the activity LED is observably blinking on RP2350, I'd appreciate a pointer — otherwise I'll file a separate follow-up once I've ruled out test-methodology issues on my end.

[anoshyn]

Separately from the doc fix in #2955, dug into the "activity LED never lights up" subplot today and confirmed it's a real, independent bug — filed as #2957. With the now-known-correct argument order (p0 = flags, p1 = gpio), GPIO 25 verified working from user code, and ACTIVE_LOW set (which should hold the pin idle-high regardless of any read/write activity), the bootrom still doesn't drive the pin. Keeping that work in #2957 so it doesn't get conflated with the doc-only swap here.

[lurch]

Documentation was fixed in #2955

[lurch]

@anoshyn Maybe you'd like to help test that #2971 works for you?

[anoshyn]

@lurch Thanks, I’ll test #2971