Support boards with internal *and* external pullups again

These boards were broken in commit e1d409f Refactor USB pullup
handling. Before that commit, all boards without external controllable
pullups were assumed to have fixed external pullups and use the DP write
trick. Since that commit, boards that have internal pullups are assumed
to *not* have any external pullups and the internal pullups are
automatically used.

It turns out there exist some boards that have internal pullups in the
chips, but also have an external fixed pullup. This can probably be
considered a hardware bug, but since the boards exist, we should support
them.

This commit allows variants to define USBD_FIXED_PULLUP to explicitly
state they have a fixed pullup on the D+ line. This will cause the D+
output trick to be applied even when internal pullups are present,
fixing these boards.

This define is only needed on these specific boards, but it can also be
defined on boards with a fixed external pullup without internal pullups.

This problem was prompted by the "Black F407VE" board, which has the
problematic pullup. All other F4 boards were checked and one other was
found to have the pullup, all others seem ok. None of the other series
have been checked, so there might still be board broken.

See also STM32-base/STM32-base.github.io#26 for
some additional inventarisation of this problem.

This fixes #1029.

Author: matthijskooijman

cores/arduino/stm32/usb/usbd_if.c

@@ -68,6 +68,9 @@
 #if defined(USBD_DETACH_PIN) && !defined(USBD_DETACH_LEVEL)
   #error "USBD_DETACH_PIN also needs USBD_DETACH_LEVEL defined"
 #endif /* defined(USBD_DETACH_PIN) && !defined(USBD_DETACH_LEVEL) */
+#if (defined(USBD_DETACH_PIN) || defined(USBD_ATTACH_PIN)) && defined(USBD_FIXED_PULLUP)
+  #error "Cannot define both USBD_FIXED_PULLUP and USBD_ATTACH_PIN or USBD_DETACH_PIN"
+#endif /* (defined(USBD_DETACH_PIN) || defined(USBD_ATTACH_PIN)) && defined(USBD_FIXED_PULLUP) */
 
 /* Either of these bits indicate that there are internal pullups */
 #if defined(USB_BCDR_DPPU) || defined(USB_OTG_DCTL_SDIS)
@@ -78,10 +81,13 @@
  * in USBD_LL_Init in usbd_conf.c. */
 #if defined(USE_USB_HS)
   #define USBD_USB_INSTANCE USB_OTG_HS
+  #define USBD_DP_PINNAME USB_OTG_HS_DP
 #elif defined(USB_OTG_FS)
   #define USBD_USB_INSTANCE USB_OTG_FS
+  #define USBD_DP_PINNAME USB_OTG_FS_DP
 #elif defined(USB)
   #define USBD_USB_INSTANCE USB
+  #define USBD_DP_PINNAME USB_DP
 #endif
 
 /*
@@ -97,15 +103,13 @@
 #elif defined(USBD_DETACH_PIN)
   #define USBD_PULLUP_CONTROL_PINNAME digitalPinToPinName(USBD_DETACH_PIN)
   #define USBD_ATTACH_LEVEL !(USBD_DETACH_LEVEL)
-#elif !defined(USBD_HAVE_INTERNAL_PULLUPS)
+#elif !defined(USBD_HAVE_INTERNAL_PULLUPS) || defined(USBD_FIXED_PULLUP)
   /* When no USB attach and detach pins were defined, and there are also
   * no internal pullups, assume there is a fixed external pullup and apply
-  * the D+ trick. This should happen only for the USB peripheral, since
-  * USB_OTG_HS and USB_OTG_FS always have internal pullups. */
-  #if !defined(USB)
-    #error "Unexpected USB configuration"
-  #endif
-  #define USBD_PULLUP_CONTROL_PINNAME USB_DP
+  * the D+ trick. Also do this when there are internal *and* external
+  * pulups (which is a hardware bug, but there are boards out there with
+  * this). */
+  #define USBD_PULLUP_CONTROL_PINNAME USBD_DP_PINNAME
   #define USBD_DETACH_LEVEL LOW
   // USBD_ATTACH_LEVEL not needed.
   #define USBD_DP_TRICK

variants/BLACK_F407XX/variant.h

@@ -301,6 +301,13 @@ extern "C" {
 #define HAL_DAC_MODULE_ENABLED
 #define HAL_SD_MODULE_ENABLED
 
+// This indicates that there is an external and fixed 1.5k pullup
+// on the D+ line. This define is only needed on boards that have
+// internal pullups *and* an external pullup. Note that it would have
+// been better to omit the pullup and exclusively use the internal
+// pullups instead.
+#define USBD_FIXED_PULLUP
+
 #ifdef __cplusplus
 } // extern "C"
 #endif

variants/BLUE_F407VE_Mini/variant.h

@@ -184,6 +184,13 @@ extern "C" {
 #define HAL_DAC_MODULE_ENABLED
 #define HAL_SD_MODULE_ENABLED
 
+// This indicates that there is an external and fixed 1.5k pullup
+// on the D+ line. This define is only needed on boards that have
+// internal pullups *and* an external pullup. Note that it would have
+// been better to omit the pullup and exclusively use the internal
+// pullups instead.
+#define USBD_FIXED_PULLUP
+
 #ifdef __cplusplus
 } // extern "C"
 #endif

variants/board_template/variant.h

@@ -174,7 +174,17 @@ extern "C" {
 //#define USBD_ATTACH_LEVEL LOW
 //#define USBD_DETACH_PIN x
 //#define USBD_DETACH_LEVEL LOW
-
+//
+// This indicates that there is an external and fixed 1.5k pullup
+// on the D+ line. This define is not normally needed, since a
+// fixed pullup is assumed by default. It is only required when
+// the USB peripheral has an internal pullup *and* an external
+// fixed pullup is present (which is actually a hardware bug, since just
+// the internal pullup is sufficient and having two pullups violates the
+// USB specification). In this case, defining this forces
+// the "write D+ LOW"-trick to be used. In the future, it might also
+// disable the internal pullups, but this is not currently implemented.
+// #define USBD_FIXED_PULLUP
 #ifdef __cplusplus
 } // extern "C"
 #endif