Skip to content

Add library for reset interface#2629

Merged
kilograham merged 23 commits into
raspberrypi:developfrom
will-v-pi:reset-lib
May 28, 2026
Merged

Add library for reset interface#2629
kilograham merged 23 commits into
raspberrypi:developfrom
will-v-pi:reset-lib

Conversation

@will-v-pi

@will-v-pi will-v-pi commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

This separates the stdio_usb reset interface out into a separate library, which can be used by applications which use TinyUSB directly.

No change in behaviour when using the typical stdio_usb (eg in the hello_usb example).

Keeps the PICO_STDIO_USB_ prefix on the config defines to maintain compatibility, and also maintains compatibility with picotool.

Instructions to use this are added to the header file.

See implementation in debugprobe and the dev_multi_cdc example

Comment thread src/common/pico_usb_reset_interface_headers/include/pico/usb_reset_interface.h Outdated
Comment thread src/common/pico_usb_reset_interface_headers/include/pico/usb_reset_interface.h Outdated
@popcornmix

Copy link
Copy Markdown

Any reason this has stalled? I'd quite like to use this, so my project can handle reset to reconfigure in a new mode.

@mtfurlan

mtfurlan commented Mar 3, 2026

Copy link
Copy Markdown

I've been using this PR in a project for a while now, which isn't great, but it has been working.

@lurch

lurch commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Looks like this PR has conflicts which need resolving? @will-v-pi

@will-v-pi

Copy link
Copy Markdown
Contributor Author

Looks like this PR has conflicts which need resolving? @will-v-pi

I've rebased it onto develop to resolve those - this PR is awaiting review by @kilograham

@kilograham kilograham removed this from the 2.3.0 milestone Apr 15, 2026
@kilograham kilograham added this to the 2.2.1 milestone Apr 15, 2026
@kilograham

Copy link
Copy Markdown
Contributor

oops - sorry i seem to have ignored this!

  1. we should probably remove the STDIO_ from the define names, and add backwards-compatibility in the least ugly place possible
  2. we have hit a slight snafu obv with the fact that pico/usb_reset_interface (and particularly the header) existed before.. i'm kinda of loath to have a pico/foo where foo.h is not the API, so i wonder if we should rename this to something else

@will-v-pi

Copy link
Copy Markdown
Contributor Author

oops - sorry i seem to have ignored this!

  1. we should probably remove the STDIO_ from the define names, and add backwards-compatibility in the least ugly place possible
  2. we have hit a slight snafu obv with the fact that pico/usb_reset_interface (and particularly the header) existed before.. i'm kinda of loath to have a pico/foo where foo.h is not the API, so i wonder if we should rename this to something else

I can rename the new library to pico_usb_reset_interface_device, as the header name you actually include to use it is already usb_reset_interface_device.h. That will make it the longest library name, but it's only slightly longer than pico_bootsel_via_double_reset so it's probably fine. I can also move the new headers out of common and into that library.

Include backwards compatibility with old define names

Also fix bazel build
@will-v-pi

Copy link
Copy Markdown
Contributor Author

Actually, maybe just pico_usb_reset is fine for the library name, given it also includes the resetting via baud rate

…e_low bit (raspberrypi#2947)

commit 97ed432
Author: Graham Sanderson <graham.sanderson@raspberrypi.com>
Date:   Tue May 19 17:37:03 2026 -0500

    grr

commit 90c5d5a
Author: Graham Sanderson <graham.sanderson@raspberrypi.com>
Date:   Tue May 19 17:36:27 2026 -0500

    missing ) in comment

commit 082e459
Author: Graham Sanderson <graham.sanderson@raspberrypi.com>
Date:   Tue May 19 17:35:37 2026 -0500

    Re-arrange bit patterns to be more backwards compatible

commit e243616
Author: Andrii Anoshyn <anoshyn.andrii@gmail.com>
Date:   Sat May 16 14:29:23 2026 +0300

    pico_stdio_usb: fix overlap between activity-LED GPIO and active_low bit

    The bootsel reset USB control transfer encodes the activity-LED GPIO in
    wValue. The current layout reads:

      if (request->wValue & 0x100) {
          gpio = request->wValue >> 9u;
      }
      active_low = request->wValue & 0x200;

    Bit 9 (0x200) is the active_low flag, but `gpio = wValue >> 9` also
    makes bit 9 the low bit of the extracted GPIO number. As a result:
    - Any odd-numbered activity-LED GPIO is silently flagged active_low.
    - Any client wanting active_low has the GPIO number's low bit forced.

    The active_low line was added in commit 5a98889 (\"add
    rom_reset_usb_boot_extra which supports >32 pins and ACTIVE_LOW\") on
    top of the original \"gpio = wValue >> 9\" encoding from commit 383e88e,
    without shifting the gpio over by an extra bit to make room. See raspberrypi#2713.

    Shift the GPIO extraction to (wValue >> 10) so bit 9 is reserved for
    active_low alone. The resulting GPIO field is 6 bits (0-63), enough
    for RP2040 (30 GPIOs) and RP2350 (48 GPIOs). The canonical client
    (picotool) only sends wValue=disable_mask with bits 0-1 set, so this
    does not change any in-tree behavior; the change is only visible to
    out-of-tree clients that build wValue with the GPIO-specified bit set.

    Fixes raspberrypi#2713.
@lurch lurch linked an issue May 26, 2026 that may be closed by this pull request
Comment thread src/rp2_common/pico_usb_reset/usb_reset.c
@kilograham

Copy link
Copy Markdown
Contributor

ok, so there was maybe a bit of a miscommunication on the renaming, but i have looked at it again anyway.

I am cool with leaving it called pico_usb_reset_interface since thats really what it is - the original headers can remain.

even though they are long, we should probably keep PICO_USB_RESET_INTERFACE_ in the names to be consistent (i could handle contraction to IFC i guess)

It's kinda ugly, but the real headers really belong in rp2_common, so i'd actually put them with the code in rp2_common, but add them to the pico_usb_reset_interface_headers, and #include them from the original one guarded by a #if LIB_PICO_USB_RESET_INTERFACE

@will-v-pi

Copy link
Copy Markdown
Contributor Author

ok, so there was maybe a bit of a miscommunication on the renaming, but i have looked at it again anyway.

I am cool with leaving it called pico_usb_reset_interface since thats really what it is - the original headers can remain.

even though they are long, we should probably keep PICO_USB_RESET_INTERFACE_ in the names to be consistent (i could handle contraction to IFC i guess)

It's kinda ugly, but the real headers really belong in rp2_common, so i'd actually put them with the code in rp2_common, but add them to the pico_usb_reset_interface_headers, and #include them from the original one guarded by a #if LIB_PICO_USB_RESET_INTERFACE

Sorry, I was occupied with other stuff - I think the rename to pico_usb_reset makes all of this cleaner so I will do that, as it allows for a separate pico_usb_reset_headers library so everything can be in rp2_common without any hacks in the existing header

Comment thread src/rp2_common/pico_usb_reset/include/pico/usb_reset_config.h Outdated
Comment thread src/rp2_common/pico_usb_reset/include/pico/usb_reset_config.h Outdated
Comment thread src/rp2_common/pico_usb_reset/include/pico/usb_reset_tusb.h
will-v-pi added 3 commits May 27, 2026 11:16
Place vendor interface defines after PICO_ENABLE_USB_RESET_VIA_VENDOR_INTERFACE, and bootsel defines after PICO_USB_RESET_SUPPORT_RESET_TO_BOOTSEL
Comment on lines +30 to +31
'D', 0x00, 'e', 0x00, 'v', 0x00, 'i', 0x00, 'c', 0x00, 'e', 0x00, 'I', 0x00, 'n', 0x00, 't', 0x00, 'e', 0x00, \
'r', 0x00, 'f', 0x00, 'a', 0x00, 'c', 0x00, 'e', 0x00, 'G', 0x00, 'U', 0x00, 'I', 0x00, 'D', 0x00, 0x00, 0x00, \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly-related to this PR (so feel free to ignore this suggestion), but maybe this could be reformatted to:

Suggested change
'D', 0x00, 'e', 0x00, 'v', 0x00, 'i', 0x00, 'c', 0x00, 'e', 0x00, 'I', 0x00, 'n', 0x00, 't', 0x00, 'e', 0x00, \
'r', 0x00, 'f', 0x00, 'a', 0x00, 'c', 0x00, 'e', 0x00, 'G', 0x00, 'U', 0x00, 'I', 0x00, 'D', 0x00, 0x00, 0x00, \
'D', 0, 'e', 0, 'v', 0, 'i', 0, 'c', 0, 'e', 0, \
'I', 0, 'n', 0, 't', 0, 'e', 0, 'r', 0, 'f', 0, \
'a', 0, 'c', 0, 'e', 0, 'G', 0, 'U', 0, 'I', 0, \
'D', 0, 0, 0, \

as this would then match the formatting of the {bc7398c1-... UTF-16 string below?

@kilograham kilograham merged commit cd05d20 into raspberrypi:develop May 28, 2026
7 checks passed
@will-v-pi will-v-pi deleted the reset-lib branch May 28, 2026 14:56
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.

Bug in !PICO_STDIO_USB_RESET_BOOTSEL_FIXED_ACTIVITY_LED implementation?

5 participants