-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add board header for Waveshare RP2350-USB-A #2435
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
base: develop
Are you sure you want to change the base?
Conversation
|
||
// --- PIO USB --- | ||
#ifndef PICO_DEFAULT_PIO_USB_DP_PIN | ||
#define PICO_DEFAULT_PIO_USB_DP_PIN 12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no SDK support for this, so IMHO prefixing it with PICO_DEFAULT_
is a bit misleading. Perhaps WAVESHARE_RP2350_USB_A_USB_DP_PIN
would be better?
Also, if you're defining the USB_DP_PIN
, would it make sense to also define the USB_DM_PIN
??
EDIT: I also just removed the PIO
from my suggested define-name, because the PIO
part is a software-detail, and IMHO the board-headers in the SDK should just describe the raw hardware.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this define used by Pico-PIO-USB, if so im fine with it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i guess it doesn't ... i'd go with PIO_USB_DP_PIN
and then maybe encourage Pico-PIO-USB to provide a confif that uses those along with
#ifndef PIO_USB_DP_PIN
#define PIO_USB_DP_PIN PIO_USB_DP_PIN_DEFAULT
#endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That macro is the one used in TinyUSB for the Pico PIO implementation.
I agree it may be a bit misleading. For further context, as far as I know, the only other RP2040 board with a USB-A port is the Adafruit RP2040 USB Host, and it doesn’t define anything specific for the USB-A port in its board header.
TinyUSB sets the default pins for that board here.
It might be better to add the custom board header to the TinyUSB repo instead? I’m a bit torn...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That macro is the one used in TinyUSB for the Pico PIO implementation.
Ahh! https://github.com/search?q=repo%3Ahathach%2Ftinyusb%20PICO_DEFAULT_PIO_USB_DP_PIN&type=code
As a compromise, what about something like:
// --- PIO USB ---
#ifndef WAVESHARE_RP2350_USB_A_USB_DP_PIN
#define WAVESHARE_RP2350_USB_A_USB_DP_PIN 12
#endif
#ifndef WAVESHARE_RP2350_USB_A_USB_DM_PIN
#define WAVESHARE_RP2350_USB_A_USB_DM_PIN 13
#endif
#define PICO_DEFAULT_PIO_USB_DP_PIN WAVESHARE_RP2350_USB_A_USB_DP_PIN
?
ping @hathach in case he has any suggestions on how these defines should be structured and/or in which repo they belong.
The |
This PR introduces support for the Waveshare RP2350-USB-A.
This board combines an RP2350A with a USB-A port that can be used with a bit-banged implementation of the USB protocol such as Pico-PIO-USB.
Fixes #2434