Make usb_host.Port a singleton

tannewt · tannewt · commit 1629faf8b3c6 · 2023-07-18T10:40:54.000-07:00
This allows you to initialize usb_host.Port once successfully and
then returns the same object as long as you pass the same arguments
in. It does allow you to fix incorrect pins but not switching from
one valid set to another. (It needs a reset for that.)

This also moves hcd cache operations to RAM so that they don't
access the cache when doing maintenance.
diff --git a/lib/tinyusb b/lib/tinyusb
@@ -1 +1 @@
-Subproject commit f1e006d09bd32088ab421d0b519eb89c531eda4e
+Subproject commit 6c7c9f2ef5a80d5a6879e9c3558162188c6cf889
diff --git a/ports/mimxrt10xx/boards/imxrt1060_evk/board.c b/ports/mimxrt10xx/boards/imxrt1060_evk/board.c
@@ -28,6 +28,8 @@
 #include "supervisor/board.h"
 #include "shared-bindings/microcontroller/Pin.h"
 
+#include "shared-bindings/usb_host/Port.h"
+
 // These pins should never ever be reset; doing so could interfere with basic operation.
 // Used in common-hal/microcontroller/Pin.c
 const mcu_pin_obj_t *mimxrt10xx_reset_forbidden_pins[] = {
@@ -55,4 +57,8 @@ const mcu_pin_obj_t *mimxrt10xx_reset_forbidden_pins[] = {
     NULL,                       // Must end in NULL.
 };
 
+void board_init(void) {
+    common_hal_usb_host_port_construct(&pin_USB_OTG2_DP, &pin_USB_OTG2_DN);
+}
+
 // Use the MP_WEAK supervisor/shared/board.c versions of routines not defined here.
diff --git a/ports/mimxrt10xx/boards/imxrt1060_evkb/board.c b/ports/mimxrt10xx/boards/imxrt1060_evkb/board.c
@@ -28,6 +28,8 @@
 #include "supervisor/board.h"
 #include "shared-bindings/microcontroller/Pin.h"
 
+#include "shared-bindings/usb_host/Port.h"
+
 // These pins should never ever be reset; doing so could interfere with basic operation.
 // Used in common-hal/microcontroller/Pin.c
 const mcu_pin_obj_t *mimxrt10xx_reset_forbidden_pins[] = {
@@ -52,7 +54,11 @@ const mcu_pin_obj_t *mimxrt10xx_reset_forbidden_pins[] = {
     // USB Pins
     &pin_GPIO_AD_B0_01, // ID Pin
     &pin_GPIO_AD_B0_03, // OC/Fault Pin
-    NULL,                       // Must end in NULL.
+    NULL,               // Must end in NULL.
 };
 
+void board_init(void) {
+    common_hal_usb_host_port_construct(&pin_USB_OTG2_DP, &pin_USB_OTG2_DN);
+}
+
 // Use the MP_WEAK supervisor/shared/board.c versions of routines not defined here.
diff --git a/ports/mimxrt10xx/boards/teensy41/board.c b/ports/mimxrt10xx/boards/teensy41/board.c
@@ -28,6 +28,8 @@
 #include "supervisor/board.h"
 #include "shared-bindings/microcontroller/Pin.h"
 
+#include "shared-bindings/usb_host/Port.h"
+
 // These pins should never ever be reset; doing so could interfere with basic operation.
 // Used in common-hal/microcontroller/Pin.c
 const mcu_pin_obj_t *mimxrt10xx_reset_forbidden_pins[] = {
@@ -54,4 +56,8 @@ const mcu_pin_obj_t *mimxrt10xx_reset_forbidden_pins[] = {
     NULL,                       // Must end in NULL.
 };
 
+void board_init(void) {
+    common_hal_usb_host_port_construct(&pin_USB_OTG2_DP, &pin_USB_OTG2_DN);
+}
+
 // Use the MP_WEAK supervisor/shared/board.c versions of routines not defined here.
diff --git a/ports/mimxrt10xx/common-hal/usb_host/Port.c b/ports/mimxrt10xx/common-hal/usb_host/Port.c
@@ -29,9 +29,13 @@
 
 #include "py/runtime.h"
 
-bool usb_host_init;
+#include "tusb.h"
 
-void common_hal_usb_host_port_construct(usb_host_port_obj_t *self, const mcu_pin_obj_t *dp, const mcu_pin_obj_t *dm) {
+#include "imx_usb.h"
+
+usb_host_port_obj_t usb_host_instance;
+
+usb_host_port_obj_t *common_hal_usb_host_port_construct(const mcu_pin_obj_t *dp, const mcu_pin_obj_t *dm) {
     const mcu_pin_obj_t *supported_dp;
     const mcu_pin_obj_t *supported_dm;
     if (CIRCUITPY_USB_HOST_INSTANCE == 0) {
@@ -41,18 +45,27 @@ void common_hal_usb_host_port_construct(usb_host_port_obj_t *self, const mcu_pin
         supported_dp = &pin_USB_OTG2_DP;
         supported_dm = &pin_USB_OTG2_DN;
     }
+    // Return the singleton if given the same pins.
+    usb_host_port_obj_t *self = &usb_host_instance;
+    if (self->dp != NULL) {
+        if (self->dp != dp || self->dm != dm) {
+            mp_raise_msg_varg(&mp_type_RuntimeError, translate("%q in use"), MP_QSTR_usb_host);
+        }
+        return self;
+    }
     if (dp != supported_dp || dm != supported_dm) {
         raise_ValueError_invalid_pins();
     }
-    self->init = true;
-    usb_host_init = true;
-}
 
-void common_hal_usb_host_port_deinit(usb_host_port_obj_t *self) {
-    self->init = false;
-    usb_host_init = false;
-}
+    assert_pin_free(dp);
+    assert_pin_free(dm);
+
+    init_usb_instance(CIRCUITPY_USB_HOST_INSTANCE);
+    tuh_init(TUH_OPT_RHPORT);
+
+    self->base.type = &usb_host_port_type;
+    self->dp = dp;
+    self->dm = dm;
 
-bool common_hal_usb_host_port_deinited(usb_host_port_obj_t *self) {
-    return !self->init;
+    return self;
 }
diff --git a/ports/mimxrt10xx/common-hal/usb_host/Port.h b/ports/mimxrt10xx/common-hal/usb_host/Port.h
@@ -31,11 +31,8 @@
 
 typedef struct {
     mp_obj_base_t base;
-    bool init;
+    const mcu_pin_obj_t *dp;
+    const mcu_pin_obj_t *dm;
 } usb_host_port_obj_t;
 
-// Cheater state so that the usb module knows if it should return the TinyUSB
-// state.
-extern bool usb_host_init;
-
 #endif // MICROPY_INCLUDED_MIMXRT10XX_COMMON_HAL_USB_HOST_PORT_H
diff --git a/ports/mimxrt10xx/linking/common.ld b/ports/mimxrt10xx/linking/common.ld
@@ -71,7 +71,7 @@ SECTIONS
         . = ALIGN(4);
     *(EXCLUDE_FILE(
         *fsl_flexspi.o
-        *dcd_ci_hs.o
+        *cd_ci_hs.o
         *ehci.o
         *tusb_fifo.o
         *usbd.o
@@ -88,6 +88,9 @@ SECTIONS
         /* Keep USB processing functions out of RAM because we don't know which will be used.
            We try to only keep USB interrupt related functions. */
         *dcd_ci_hs.o(.text.process_*_request .text.dcd_edpt* .text.dcd_init .text.dcd_set_address)
+        /* Move hcd_dcache* routines to RAM so that we don't cross execution from
+           the cache during cache maintenance. Weird things happen when we do. */
+        *hcd_ci_hs.o(.text.hcd_i*)
         *usbd.o(.text.process_*_request .text.process_[gs]et* .text.tud_* .text.usbd_* .text.configuration_reset .text.invoke_*)
         *ehci.o(.text.hcd_edpt* .text.hcd_setup* .text.ehci_init* .text.hcd_port* .text.hcd_device* .text.qtd_init* .text.list_remove*)
 
diff --git a/ports/mimxrt10xx/supervisor/usb.c b/ports/mimxrt10xx/supervisor/usb.c
@@ -31,7 +31,9 @@
 #include "supervisor/linker.h"
 #include "supervisor/usb.h"
 
-STATIC void init_usb_instance(mp_int_t instance) {
+#include "imx_usb.h"
+
+void init_usb_instance(mp_int_t instance) {
     if (instance < 0) {
         return;
     }
@@ -72,9 +74,6 @@ STATIC void init_usb_instance(mp_int_t instance) {
 
     void init_usb_hardware(void) {
         init_usb_instance(CIRCUITPY_USB_DEVICE_INSTANCE);
-        // We can't dynamically start the USB Host port at the moment, so do it
-        // up front.
-        init_usb_instance(CIRCUITPY_USB_HOST_INSTANCE);
     }
 
 // Provide the prototypes for the interrupt handlers. The iMX RT SDK doesn't.
diff --git a/ports/raspberrypi/boards/adafruit_feather_rp2040_usb_host/board.c b/ports/raspberrypi/boards/adafruit_feather_rp2040_usb_host/board.c
@@ -31,13 +31,12 @@
 
 // Use the MP_WEAK supervisor/shared/board.c versions of routines not defined here.
 
-usb_host_port_obj_t _host_port;
 digitalio_digitalinout_obj_t _host_power;
 
 void board_init(void) {
     common_hal_digitalio_digitalinout_construct(&_host_power, &pin_GPIO18);
     common_hal_digitalio_digitalinout_never_reset(&_host_power);
     common_hal_digitalio_digitalinout_switch_to_output(&_host_power, true, DRIVE_MODE_PUSH_PULL);
 
-    common_hal_usb_host_port_construct(&_host_port, &pin_GPIO16, &pin_GPIO17);
+    common_hal_usb_host_port_construct(&pin_GPIO16, &pin_GPIO17);
 }
diff --git a/ports/raspberrypi/common-hal/usb_host/Port.c b/ports/raspberrypi/common-hal/usb_host/Port.c
@@ -45,7 +45,7 @@
 
 #include "supervisor/serial.h"
 
-bool usb_host_init;
+usb_host_port_obj_t usb_host_instance;
 
 STATIC PIO pio_instances[2] = {pio0, pio1};
 volatile bool _core1_ready = false;
@@ -102,10 +102,23 @@ STATIC bool _has_program_room(uint8_t pio_index, uint8_t program_size) {
     return pio_can_add_program(pio, &program_struct);
 }
 
-void common_hal_usb_host_port_construct(usb_host_port_obj_t *self, const mcu_pin_obj_t *dp, const mcu_pin_obj_t *dm) {
+usb_host_port_obj_t *common_hal_usb_host_port_construct(const mcu_pin_obj_t *dp, const mcu_pin_obj_t *dm) {
     if (dp->number + 1 != dm->number) {
         raise_ValueError_invalid_pins();
     }
+    usb_host_port_obj_t *self = &usb_host_instance;
+
+    // Return the singleton if given the same pins.
+    if (self->dp != NULL) {
+        if (self->dp != dp || self->dm != dm) {
+            mp_raise_msg_varg(&mp_type_RuntimeError, translate("%q in use"), MP_QSTR_usb_host);
+        }
+        return self;
+    }
+
+    assert_pin_free(dp);
+    assert_pin_free(dm);
+
     pio_usb_configuration_t pio_cfg = PIO_USB_DEFAULT_CONFIG;
     pio_cfg.skip_alarm_pool = true;
     pio_cfg.pin_dp = dp->number;
@@ -122,6 +135,10 @@ void common_hal_usb_host_port_construct(usb_host_port_obj_t *self, const mcu_pin
         mp_raise_RuntimeError(translate("All dma channels in use"));
     }
 
+    self->base.type = &usb_host_port_type;
+    self->dp = dp;
+    self->dm = dm;
+
     PIO tx_pio = pio_instances[pio_cfg.pio_tx_num];
     pio_cfg.sm_tx = pio_claim_unused_sm(tx_pio, false);
     PIO rx_pio = pio_instances[pio_cfg.pio_rx_num];
@@ -151,15 +168,5 @@ void common_hal_usb_host_port_construct(usb_host_port_obj_t *self, const mcu_pin
     tuh_configure(TUH_OPT_RHPORT, TUH_CFGID_RPI_PIO_USB_CONFIGURATION, &pio_cfg);
     tuh_init(TUH_OPT_RHPORT);
 
-    self->init = true;
-    usb_host_init = true;
-}
-
-void common_hal_usb_host_port_deinit(usb_host_port_obj_t *self) {
-    self->init = false;
-    usb_host_init = false;
-}
-
-bool common_hal_usb_host_port_deinited(usb_host_port_obj_t *self) {
-    return !self->init;
+    return self;
 }
diff --git a/ports/raspberrypi/common-hal/usb_host/Port.h b/ports/raspberrypi/common-hal/usb_host/Port.h
@@ -30,9 +30,6 @@
 
 typedef struct {
     mp_obj_base_t base;
-    bool init;
+    const mcu_pin_obj_t *dp;
+    const mcu_pin_obj_t *dm;
 } usb_host_port_obj_t;
-
-// Cheater state so that the usb module knows if it should return the TinyUSB
-// state.
-extern bool usb_host_init;
diff --git a/shared-bindings/usb_host/Port.c b/shared-bindings/usb_host/Port.c
@@ -35,59 +35,36 @@
 //|
 //|     def __init__(self, dp: microcontroller.Pin, dm: microcontroller.Pin) -> None:
 //|         """Create a USB host port on the given pins. Access attached devices
-//|         through the `usb` module. Keep this object referenced while
-//|         interacting with devices, otherwise they will be disconnected.
+//|         through the `usb` module.
+//|
+//|         The resulting object lives longer than the CircuitPython VM so that
+//|         USB devices such as keyboards can continue to be used. Subsequent
+//|         calls to this constructor will return the same object and *not*
+//|         reinitialize the USB host port. It will raise an exception when
+//|         given different arguments from the first successful call.
 //|
 //|         :param ~microcontroller.Pin dp: The data plus pin
 //|         :param ~microcontroller.Pin dm: The data minus pin
 //|         """
 //|         ...
+//|
 STATIC mp_obj_t usb_host_port_make_new(const mp_obj_type_t *type,
     size_t n_args, size_t n_kw, const mp_obj_t *args) {
     // check number of arguments
     mp_arg_check_num(n_args, n_kw, 2, 2, false);
 
-    const mcu_pin_obj_t *dp = validate_obj_is_free_pin(args[0], MP_QSTR_dp);
-    const mcu_pin_obj_t *dm = validate_obj_is_free_pin(args[1], MP_QSTR_dm);
-
-    usb_host_port_obj_t *self = m_new_obj(usb_host_port_obj_t);
-    self->base.type = &usb_host_port_type;
-    common_hal_usb_host_port_construct(self, dp, dm);
-
-    return (mp_obj_t)self;
-}
+    const mcu_pin_obj_t *dp = validate_obj_is_pin(args[0], MP_QSTR_dp);
+    const mcu_pin_obj_t *dm = validate_obj_is_pin(args[1], MP_QSTR_dm);
 
-//|     def deinit(self) -> None:
-//|         """Turn off the USB host port and release the pins for other use."""
-//|         ...
-STATIC mp_obj_t usb_host_port_obj_deinit(mp_obj_t self_in) {
-    usb_host_port_obj_t *self = MP_OBJ_TO_PTR(self_in);
-    common_hal_usb_host_port_deinit(self);
-    return mp_const_none;
-}
-MP_DEFINE_CONST_FUN_OBJ_1(usb_host_port_deinit_obj, usb_host_port_obj_deinit);
+    // Pin in use checks happen in the implementation so they can be ignored
+    // when returning the singleton.
 
-//|     def __enter__(self) -> Port:
-//|         """No-op used by Context Managers."""
-//|         ...
-//  Provided by context manager helper.
+    usb_host_port_obj_t *self = common_hal_usb_host_port_construct(dp, dm);
 
-//|     def __exit__(self) -> None:
-//|         """Automatically deinitializes the hardware when exiting a context. See
-//|         :ref:`lifetime-and-contextmanagers` for more info."""
-//|         ...
-//|
-STATIC mp_obj_t usb_host_port_obj___exit__(size_t n_args, const mp_obj_t *args) {
-    (void)n_args;
-    common_hal_usb_host_port_deinit(MP_OBJ_TO_PTR(args[0]));
-    return mp_const_none;
+    return (mp_obj_t)self;
 }
-STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(usb_host_port_obj___exit___obj, 4, 4, usb_host_port_obj___exit__);
 
 STATIC const mp_rom_map_elem_t usb_host_port_locals_dict_table[] = {
-    { MP_ROM_QSTR(MP_QSTR_deinit),             MP_ROM_PTR(&usb_host_port_deinit_obj) },
-    { MP_ROM_QSTR(MP_QSTR___enter__),          MP_ROM_PTR(&default___enter___obj) },
-    { MP_ROM_QSTR(MP_QSTR___exit__),           MP_ROM_PTR(&usb_host_port_obj___exit___obj) },
 };
 
 STATIC MP_DEFINE_CONST_DICT(usb_host_port_locals_dict, usb_host_port_locals_dict_table);
diff --git a/shared-bindings/usb_host/Port.h b/shared-bindings/usb_host/Port.h
@@ -35,8 +35,10 @@
 
 extern const mp_obj_type_t usb_host_port_type;
 
-void common_hal_usb_host_port_construct(usb_host_port_obj_t *self, const mcu_pin_obj_t *dp, const mcu_pin_obj_t *dm);
-void common_hal_usb_host_port_deinit(usb_host_port_obj_t *self);
-bool common_hal_usb_host_port_deinited(usb_host_port_obj_t *self);
+// This is unique to common_hal constructs because it returns a globally stored
+// object instead of taking on in that may be on the heap. This allows the
+// method to check the internals of the global object against the given arguments
+// to determine whether to return the singleton or raise an exception.
+usb_host_port_obj_t *common_hal_usb_host_port_construct(const mcu_pin_obj_t *dp, const mcu_pin_obj_t *dm);
 
 #endif  // MICROPY_INCLUDED_SHARED_BINDINGS_USB_HOST_PORT_H
diff --git a/supervisor/shared/usb/tusb_config.h b/supervisor/shared/usb/tusb_config.h
@@ -152,7 +152,8 @@ extern "C" {
 #endif
 
 #define CFG_TUH_HID                 2
-#define CFG_TUH_HUB                 1
+// 2 hubs so we can support "7 port" hubs which have two internal hubs.
+#define CFG_TUH_HUB                 2
 #define CFG_TUH_CDC                 0
 #define CFG_TUH_MSC                 0
 #define CFG_TUH_VENDOR              0
