search

cnlohr / rv003usb

CH32V003 RISC-V Pure Software USB Controller
MIT License
436 stars 43 forks source link

Correct usage of USB_DM/USB_DP #44

Closed xsrf closed 6 months ago

xsrf commented 6 months ago

Before this, USB_DP was used for D- and USB_DM was used for D+ due to historical reasons during development of the USB stack. This however is frustratingly confusing, so this should fix it.

It also changes the demos and readme to reflect the change.

Additional documentation for the bootloader was added.

[!CAUTION] THIS IS A BREAKING CHANGE kind of ...

If RV003USB is updated but the old usb_config.h is kept untouched, the device will fail to enumerate.

I was considering adding a #define RV003USB_CONFIG_VERSION 2 to usb_config.h and throwing an error in rv003usb.c if it's not defined. (or #define RV003USB_REAL_PIN_NAMES)

I was also considering adding a red banner (like the one above) to the main readme.

Any thoughts if I should add something like this to alert users?

xsrf commented 6 months ago

So, since this will be a breaking change, I'd like to hear your feedback. I see three options how to handle this. Please vote for your favorite one with the respective reaction. The one implemented in the last commit is 🚀 but I'm open for changes.

👀 USB_PIN_DP / USB_PIN_DM / USB_PIN_DPU

Rename USB_DP, USB_DM and USB_DPU (for consistency) to USB_PIN_DM, USB_PIN_DP and USB_PIN_DPU. This will cause existing code to break with something like USB_PIN_DP not defined, did you mean USB_DP?. Because of this, I think most users will just rename their #defines but not actually understand why this is needed and will actually not swap DP/DM! So, in this case, I'd have to also throw an error if USB_PIN_DP is not defined, like

Error: Your "usb_config.h" is outdated! Please rename USB_DM/USB_DP/USB_DPU to USB_PIN_DP/USB_PIN_DM/USB_PIN_DPU and make sure DM/DP now reflect the correct signal!`

🚀 Versioned usb_config.h

I'll keep and just swap USB_DP and USB_DM. But to inform users with old configs about the change, I'll version the usb_config.h from now on by adding #define _USB_CONFIG_H_VER 2 to it. For users without this version defined, I'll issue an error like

Error: Your "usb_config.h" is outdated! Please swap USB_DP/USB_DM, they now reflect the correct signal! Add "#define _USB_CONFIG_H_VER 2" to suppress this error!

🎉 Fix specific flag

Like with the versioned usb_config.h I'll just keep USB_DP and USB_DM and add a fix specific flag to usb_config.h like #define _USB_CONFIG_DMDP_FIX. Like before, this flag will be required or an error will be thrown.

Error: Your "usb_config.h" is outdated! Please swap USB_DP/USB_DM, they now reflect the correct signal! Add "#define _USB_CONFIG_DMDP_FIX" to suppress this error!