Merge pull request #8182 from tannewt/usb_host_singleton

Make usb_host.Port a singleton

Author: tannewt

lib/tinyusb

@@ -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.

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[] = {
@@ -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.

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[] = {
@@ -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.

ports/mimxrt10xx/boards/teensy41/board.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;
 }

ports/mimxrt10xx/common-hal/usb_host/Port.c

@@ -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

ports/mimxrt10xx/common-hal/usb_host/Port.h

@@ -0,0 +1,31 @@
+/*
+ * This file is part of the Micro Python project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2022 Scott Shawcroft for Adafruit Industries
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#pragma once
+
+// Provided by supervisor/usb.c that has a shared, non-port-specific header. So,
+// just define it here.
+void init_usb_instance(mp_int_t instance);

ports/mimxrt10xx/imx_usb.h

@@ -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*)
 

ports/mimxrt10xx/linking/common.ld

@@ -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.

ports/mimxrt10xx/supervisor/usb.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);
 }

ports/raspberrypi/boards/adafruit_feather_rp2040_usb_host/board.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;
 }

ports/raspberrypi/common-hal/usb_host/Port.c

@@ -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;

ports/raspberrypi/common-hal/usb_host/Port.h

@@ -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);

shared-bindings/usb_host/Port.c

@@ -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

shared-bindings/usb_host/Port.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