Refactor USB pullup handling

Previously, variants could define USB_DISC_PIN when they had an USB
attach pullup that was disabled by default and needed a pin to
be written LOW to enable it. Other hardware configurations could only
overwrite the USBD_reenumerate function, like the M200 board did.

This commit makes the pullup configuration more flexible. By defining
the appropriate macros, enabled-by-default and disabled-by-default
pullups are both supported. The output level of the pin can also be
configured. Finally, for disabled-by-default, the output mode for the
enabled state can also be configured (INPUT to leave the pin floating
and let any external transistor gate pullup enable the USB pullup, or
OUTPUT to keep the pin as OUTPUT an actively drive the transistor gate).

The new macros also take PinName constants rather than pin numbers. This
allows merging the code that controls an external pullup with the code
that writes directly to the USB DP pin to fake a disabled pullup (this
code only knows the pin name of the DP pin, not the number).

Finally, for CPUs that have internal USB pullup (as indicated by the
presence of a SDIS config bit), the write-to-DP-trick is now not
performed (since the pullup is automatically managed by the USB hardware
already).

This fixes stm32duino#885, also see that issue for discussion leading up to this
change.

Author: matthijskooijman

cores/arduino/stm32/usb/usbd_if.c

@@ -10,33 +10,76 @@
 #include "usbd_if.h"
 #include "usbd_cdc_if.h"
 
+#if defined(USB_DISC_PIN)
+// Compatibility with the old way to specify this
+#define USBD_ATTACH_PIN digitalPinToPinName(USB_DISC_PIN)
+#define USBD_ATTACH_LEVEL LOW
+#warning "USB_DISC_PIN is deprecated, use USBD_ATTACH_PINNAME instead"
+#endif /* USB_DISC_PIN */
+
+// When no USB attach and detach pins were defined, and the USB module
+// also does not have any builtin autocontrolled pullups (indicated by
+// the SDIS bit), assume there are fixed external pullups. This uses a
+// bit of a hack by using the DP pin as a DETACH pin, so it will be
+// driven low very briefly, which looks like a missing pullup to the
+// host. Probably violates USB specs, but works properly in practice.
+#if !defined(USBD_ATTACH_PINNAME) && !defined(USBD_DETACH_PIN) && !defined(USB_OTG_DCTL_SDIS)
+#if defined(USE_USB_HS_IN_FS)
+#define USBD_DETACH_PINNAME USB_OTG_HS_DP
+#elif defined(USB_OTG_FS)
+#define USBD_DETACH_PINNAME USB_OTG_FS_DP
+#else /* USB */
+#define USBD_DETACH_PINNAME  USB_DP
+#endif
+#define USBD_DETACH_LEVEL LOW
+// Make sure to revert to INPUT (instead of OUTPUT HIGH) after the
+// detach "pulse" to restore normal operation.
+#define USBD_ATTACH_MODE INPUT
+#endif /* !defined(USBD_ATTACH_PINNAME) && !defined(USBD_DETACH_PIN) && !defined(USB_OTG_DCTL_SDIS) */
+
+#if defined(USBD_ATTACH_PINNAME) && !defined(USBD_ATTACH_MODE)
+// Normally (e.g. when controlling a transistor), keep the pin in
+// OUTPUT mode after the detach "pulse" to actively drive the
+// transistor.
+#define USBD_ATTACH_MODE OUTPUT
+#endif /* defined(USBD_ATTACH_PINNAME) && !defined(USBD_ATTACH_MODE) */
+
+// Some sanity checks
+#if defined(USBD_ATTACH_PINNAME) && defined(USBD_DETACH_PIN)
+#error "Cannot define both USBD_ATTACH_PINNAME and USBD_DETACH_PIN"
+#endif /* defined(USBD_ATTACH_PINNAME) && defined(USBD_DETACH_PIN) */
+#if defined(USBD_ATTACH_PINNAME) && !defined(USBD_ATTACH_LEVEL)
+#error "USBD_ATTACH_PINNAME also needs USBD_ATTACH_LEVEL defined"
+#endif /* defined(USBD_ATTACH_PINNAME) && !defined(USBD_ATTACH_LEVEL) */
+#if defined(USBD_DETACH_PINNAME) && !defined(USBD_DETACH_LEVEL)
+#error "USBD_DETACH_PINNAME also needs USBD_DETACH_LEVEL defined"
+#endif /* defined(USBD_DETACH_PINNAME) && !defined(USBD_DETACH_LEVEL) */
+
 /**
   * @brief  Force to re-enumerate USB
   * @param  None
   * @retval None
   */
-
 WEAK void USBD_reenumerate(void)
 {
 #ifndef USBD_REENUM_DISABLED
   /* Re-enumerate the USB */
-#ifdef USB_DISC_PIN
-  pinMode(USB_DISC_PIN, OUTPUT);
-  digitalWrite(USB_DISC_PIN, LOW);
-#else
-#ifdef USE_USB_HS_IN_FS
-  PinName pinDP = USB_OTG_HS_DP;
-#elif defined(USB_OTG_FS)
-  PinName pinDP = USB_OTG_FS_DP;
-#else /* USB */
-  PinName pinDP =  USB_DP;
-#endif
-  pin_function(pinDP, STM_PIN_DATA(STM_MODE_OUTPUT_PP, GPIO_NOPULL, 0));
-  digitalWriteFast(pinDP, LOW);
+#ifdef USBD_DETACH_PINNAME
+  // Default is attached, so detach, wait, and reattach.
+  pin_function(USBD_DETACH_PINNAME, STM_PIN_DATA(STM_MODE_OUTPUT_PP, GPIO_NOPULL, 0));
+  digitalWriteFast(USBD_DETACH_PINNAME, USBD_DETACH_LEVEL);
   delay(USBD_ENUM_DELAY);
-  pin_function(pinDP, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, 0));
-  /*delay(USBD_ENUM_DELAY);*/
-#endif /* USB_DISC_PIN */
+  if (USBD_ATTACH_MODE_INPUT == OUTPUT)
+    digitalWriteFast(USBD_DETACH_PINNAME, !USBD_DETACH_LEVEL);
+  else
+    pin_function(USBD_DETACH_PINNAME, STM_PIN_DATA(STM_MODE_INPUT, GPIO_NOPULL, 0));
+#elif USBD_ATTACH_PINNAME
+  // Default is detached, so just attach
+  pin_function(USBD_ATTACH_PINNAME, STM_PIN_DATA(STM_MODE_OUTPUT_PP, GPIO_NOPULL, 0));
+  digitalWriteFast(USBD_ATTACH_PINNAME, USBD_ATTACH_LEVEL);
+#elif !defined(USB_OTG_DCTL_SDIS)
+#warning "No USB attac/detach method, USB might not be reliable through system resets"
+#endif
 #endif /* USBD_REENUM_DISABLED */
 }
 

variants/MALYANM200_F103CB/variant.cpp

@@ -86,25 +86,6 @@ const PinName digitalPin[] = {
 extern "C" {
 #endif
 
-/**
-  * Initialize the variant. Turn on PB9 for USB.
-  */
-void initVariant()
-{
-  pinMode(PB9, OUTPUT);
-  digitalWrite(PB9, 1);
-}
-
-void USBD_reenumerate(void)
-{
-  pinMode(PB9, OUTPUT);
-  digitalWrite(PB9, HIGH);
-  delay(10);
-  digitalWrite(PB9, LOW);
-  delay(10);
-  digitalWrite(PB9, HIGH);
-}
-
 /**
   * @brief  System Clock Configuration
   *         The system Clock is configured as follow :

variants/MALYANM200_F103CB/variant.h

@@ -105,6 +105,10 @@ extern "C" {
 #define PIN_SERIAL_RX           PA10
 #define PIN_SERIAL_TX           PA9
 
+// USB, pull this pin low to *disable* the USB attach pullup
+#define USBD_DETACH_PINNAME     PB_9
+#define USBD_DETACH_LEVEL       LOW
+
 /* Dedicated definitions */
 #ifndef MAX_PRIORITY
 #define MAX_PRIORITY            15

variants/MAPLEMINI_F103CB/variant.h

@@ -108,8 +108,9 @@ extern "C" {
 #define PIN_SERIAL_RX           PA10
 #define PIN_SERIAL_TX           PA9
 
-// USB
-#define USB_DISC_PIN            PB9
+// USB, pull this pin low to enable the USB attach pullup
+#define USBD_ATTACH_PINNAME     PB_9
+#define USBD_ATTACH_LEVEL       LOW
 
 #ifdef __cplusplus
 } // extern "C"

variants/board_template/variant.h

@@ -162,6 +162,21 @@ extern "C" {
 // See AN4879 https://www.st.com/content/st_com/en/search.html#q=AN4879-t=resources-page=1
 //#define USBD_VBUS_DETECTION_ENABLE
 
+// If the board has external USB pullup (on DP/DM depending on speed)
+// that can be controlled using a GPIO pin, define these:
+//  - If the the pullup is disabled (USB detached) by default, define
+//    USBD_ATTACH_PINNAME to the pin that, when written to
+//    USBD_ATTACH_LEVEL, attaches the pullup.
+//  - If the the pullup is enabled (attached) by default, define
+//    USBD_DETACH_PINNAME to the pin that, when written to
+//    USBD_DETACH_LEVEL, detaches the pullup.
+// Note that these constants need PinName values (e.g. PB_5) not pin
+// numbers (e.g. PB5).
+//#define USBD_ATTACH_PINNAME x
+//#define USBD_ATTACH_LEVEL LOW
+//#define USBD_DETACH_PINNAME NAMEx
+//#define USBD_DETACH_LEVEL LOW
+
 #ifdef __cplusplus
 } // extern "C"
 #endif