-
Notifications
You must be signed in to change notification settings - Fork 2.1k
usbus: allow to set CONFIG_USB_PID/CONFIG_USB_VID by board #13652
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
Conversation
Some boards already have a unique PID:VID combination assigned to them. When running RIOT on these boards, we should keep this ID combination. Thus allow to set `CONFIG_USB_VID` and `CONFIG_USB_PID` by the board `Makefile.include`. Tests and examples have to be adapted so they don't set CFLAGS themselves anymore.
Could you elaborate on / provide pointers to how a board can have its own VID/PID pair? After all, it's the software running on a board (possibly in combination with the hardware) that determines the PID, not the board on its own. |
AFAIU VID:PID pairs are assigned to hardware, not software. |
My understanding so far was that it's quite important to have different IDs for different firmwares, for if you connect a thing with one VID/PID pair to Windows and it implements a mouse, and next time you flash it with the same pair as a block device, things don't work. (Could have been old bugs, but that's what the custom ID assignments ask one to consider). ... but if the concrete board vendors insist on this, so be it. The other part is where the configuration happens -- there's already #13382 that changes this, would you consider checking up on that and possibly rebasing this on that? |
Summarizing an IRC discussion, I understand that the use case is for boards that have terminals in their shipped configurations to reuse those IDs when providing a terminal in RIOT as well. That affects the builtin-peripherals IDs and not the custom specified ones. I'd suggest that instead of setting the CONFIG_USB_VID per board, the values used for built-in peripherals be overridable: diff --git a/sys/include/usb.h b/sys/include/usb.h
index 92dc6eaf7d..efaf94c57e 100644
--- a/sys/include/usb.h
+++ b/sys/include/usb.h
@@ -24,6 +24,15 @@
extern "C" {
#endif
+/* These can be overridden by boards that should come up with their board
+ * supplier VID/PID pair. Boards should only override this if the RIOT built-in
+ * peripherals are compatible with whatever is usually shipped with that pair
+ * */
+#ifndef RIOT_INTERNALPERIPHERAL_VID
+#define RIOT_INTERNALPERIPHERAL_VID (0x1209)
+#define RIOT_INTERNALPERIPHERAL_PID (0x7D00)
+#endif
+
/**
* @defgroup usb_conf USB peripheral compile time configurations
* @ingroup config
@@ -33,8 +42,8 @@ extern "C" {
#if !(defined(CONFIG_USB_VID) && defined(CONFIG_USB_PID))
#ifdef USB_H_USER_IS_RIOT_INTERNAL
/* Reserved for RIOT standard peripherals as per http://pid.codes/1209/7D00/ */
-#define CONFIG_USB_VID (0x1209)
-#define CONFIG_USB_PID (0x7D00)
+#define CONFIG_USB_VID RIOT_INTERNALPERIPHERAL_VID
+#define CONFIG_USB_PID RIOT_INTERNALPERIPHERAL_PID
#else
#error Please configure your vendor and product IDs. For development, you may \
set USB_VID=${USB_VID_TESTING} USB_PID=${USB_PID_TESTING}. (Not that I'd particularly recommend doing this, still, which is why that's text and not a PR -- but when it's done, that's how I think it'd be done best.) |
Contribution description
Some boards already have a unique PID:VID combination assigned to them.
When running RIOT on these boards, we should keep this ID combination.
Thus allow to set
CONFIG_USB_VID
andCONFIG_USB_PID
by the boardMakefile.include
.Tests and examples have to be adapted so they don't set CFLAGS themselves anymore.
Testing procedure
All boards that don't have a unique VID:PID should still use the Riot ID.
But boards can now specify their own ID.
Issues/PRs references
Will be used by #12778