Merge pull request stm32duino#1048 from matthijskooijman/fix-external-and-internal-usb-pullup

Fix external and internal usb pullup

Author: fpistm

boards.txt

@@ -1210,6 +1210,7 @@ GenF4.build.series=STM32F4xx
 GenF4.build.cmsis_lib_gcc=arm_cortexM4lf_math
 
 # Black F407VE
+# https://github.com/mcauser/BLACK_F407VEZ
 GenF4.menu.pnum.BLACK_F407VE=Black F407VE
 GenF4.menu.pnum.BLACK_F407VE.upload.maximum_size=524288
 GenF4.menu.pnum.BLACK_F407VE.upload.maximum_data_size=131072
@@ -1218,6 +1219,7 @@ GenF4.menu.pnum.BLACK_F407VE.build.product_line=STM32F407xx
 GenF4.menu.pnum.BLACK_F407VE.build.variant=BLACK_F407XX
 
 # Black F407VG
+# https://github.com/mcauser/BLACK_F407VEZ with bigger chip
 GenF4.menu.pnum.BLACK_F407VG=Black F407VG
 GenF4.menu.pnum.BLACK_F407VG.upload.maximum_size=1048576
 GenF4.menu.pnum.BLACK_F407VG.upload.maximum_data_size=131072
@@ -1226,6 +1228,7 @@ GenF4.menu.pnum.BLACK_F407VG.build.product_line=STM32F407xx
 GenF4.menu.pnum.BLACK_F407VG.build.variant=BLACK_F407XX
 
 # Black F407ZE
+# https://github.com/mcauser/BLACK_F407ZE
 GenF4.menu.pnum.BLACK_F407ZE=Black F407ZE
 GenF4.menu.pnum.BLACK_F407ZE.upload.maximum_size=524288
 GenF4.menu.pnum.BLACK_F407ZE.upload.maximum_data_size=131072
@@ -1234,6 +1237,7 @@ GenF4.menu.pnum.BLACK_F407ZE.build.product_line=STM32F407xx
 GenF4.menu.pnum.BLACK_F407ZE.build.variant=BLACK_F407XX
 
 # Black F407ZG
+# https://github.com/mcauser/BLACK_F407ZG
 GenF4.menu.pnum.BLACK_F407ZG=Black F407ZG
 GenF4.menu.pnum.BLACK_F407ZG.upload.maximum_size=1048576
 GenF4.menu.pnum.BLACK_F407ZG.upload.maximum_data_size=131072
@@ -1274,6 +1278,7 @@ GenF4.menu.pnum.BLACKPILL_F401CC.build.product_line=STM32F401xC
 GenF4.menu.pnum.BLACKPILL_F401CC.build.variant=Generic_F401Cx
 
 # BlackPill F411CE
+# https://github.com/mcauser/WEACT_F411CEU6
 GenF4.menu.pnum.BLACKPILL_F411CE=BlackPill F411CE
 GenF4.menu.pnum.BLACKPILL_F411CE.upload.maximum_size=524288
 GenF4.menu.pnum.BLACKPILL_F411CE.upload.maximum_data_size=131072
@@ -1282,6 +1287,7 @@ GenF4.menu.pnum.BLACKPILL_F411CE.build.product_line=STM32F411xE
 GenF4.menu.pnum.BLACKPILL_F411CE.build.variant=Generic_F411Cx
 
 # Core board F401RCT6
+# https://stm32-base.org/boards/STM32F401RCT6-STM32F-Core-Board
 GenF4.menu.pnum.CoreBoard_F401RC=Core board F401RCT6
 GenF4.menu.pnum.CoreBoard_F401RC.upload.maximum_size=262144
 GenF4.menu.pnum.CoreBoard_F401RC.upload.maximum_data_size=65536
@@ -1298,6 +1304,7 @@ GenF4.menu.pnum.FEATHER_F405.build.product_line=STM32F405xx
 GenF4.menu.pnum.FEATHER_F405.build.variant=FEATHER_F405
 
 # ThunderPack F411xxE
+# https://github.com/jgillick/ThunderPack/tree/STM32F4
 GenF4.menu.pnum.THUNDERPACK_F411=ThunderPack v1.1+
 GenF4.menu.pnum.THUNDERPACK_F411.upload.maximum_size=524288
 GenF4.menu.pnum.THUNDERPACK_F411.upload.maximum_data_size=131072

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/Generic_F401Rx/variant.h

@@ -127,8 +127,11 @@ extern "C" {
 
 #ifdef ARDUINO_CoreBoard_F401RC
 // USB, pull this pin low to enable the USB attach pullup
-#define USBD_ATTACH_PIN         PD2
-#define USBD_ATTACH_LEVEL       LOW
+// It is documented here, but not actually used, since there are also
+// internal pullups which are automatically used and using both would
+// violate the USB specification for pullup strength.
+//#define USBD_ATTACH_PIN         PD2
+//#define USBD_ATTACH_LEVEL       LOW
 #endif
 
 #ifdef __cplusplus

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