Skip to content

Conversation

@qdotme
Copy link

@qdotme qdotme commented Nov 13, 2023

We have a use-case involving reusing RP2040 supervisor as an SWD debug interface during initial testing. This implements standard Pico SDK C++ usb reset endpoints to interoperate with appropriately patched picotool. The patch enables this functionality by default, but it can be disabled with a config switch to avoid accidentally rebooting the incorrect RP2040 device.

raspberrypi/picotool@master...FarProbe:picotool:master

Copy link
Contributor

@P33M P33M left a comment

Choose a reason for hiding this comment

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

Thanks for the PR - I have left review comments below.

#define PROBE_PRODUCT_STRING "Picoprobe (CMSIS-DAP)"

// Host Reset Config
#define PICOPROBE_RESET_CAPABLE 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer disabled by default. I would guess that for other users the feature would aid remote development of the probe firmware itself, but the vast majority will be doing uf2 drag-and-drop with a Pico or a Debug Probe and a standard release.

So, please move this definition to include/board_example_config.h with a suitable description.

#if PICOPROBE_RESET_CAPABLE
if (request->wIndex == 3) {
if (request->bRequest == RESET_REQUEST_BOOTSEL) {
reset_usb_boot(0, (request->wValue & 0x7f) | 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Superfluous bitwise OR with zero

@@ -0,0 +1,28 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the reset interface definitions be included with a Cmake library linkage instead of a repeat inclusion of the sdk header?

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

Labels

None yet

2 participants