From 1629faf8b3c662d86a9f572b8388263cf9711800 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Thu, 13 Jul 2023 14:47:05 -0700 Subject: [PATCH 1/2] Make usb_host.Port a singleton 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. --- lib/tinyusb | 2 +- ports/mimxrt10xx/boards/imxrt1060_evk/board.c | 6 +++ .../mimxrt10xx/boards/imxrt1060_evkb/board.c | 8 ++- ports/mimxrt10xx/boards/teensy41/board.c | 6 +++ ports/mimxrt10xx/common-hal/usb_host/Port.c | 35 +++++++++---- ports/mimxrt10xx/common-hal/usb_host/Port.h | 7 +-- ports/mimxrt10xx/linking/common.ld | 5 +- ports/mimxrt10xx/supervisor/usb.c | 7 ++- .../adafruit_feather_rp2040_usb_host/board.c | 3 +- ports/raspberrypi/common-hal/usb_host/Port.c | 33 +++++++----- ports/raspberrypi/common-hal/usb_host/Port.h | 7 +-- shared-bindings/usb_host/Port.c | 51 +++++-------------- shared-bindings/usb_host/Port.h | 8 +-- supervisor/shared/usb/tusb_config.h | 3 +- 14 files changed, 97 insertions(+), 84 deletions(-) diff --git a/lib/tinyusb b/lib/tinyusb index f1e006d09b..6c7c9f2ef5 160000 --- 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 index 4a2e6e0913..12278acabb 100644 --- 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 index 4a2e6e0913..26dd28cbb4 100644 --- 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 index 8ece1546d7..3418cff43f 100644 --- 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 index 126160c69c..444c410729 100644 --- 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); -bool common_hal_usb_host_port_deinited(usb_host_port_obj_t *self) { - return !self->init; + 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; + + return self; } diff --git a/ports/mimxrt10xx/common-hal/usb_host/Port.h b/ports/mimxrt10xx/common-hal/usb_host/Port.h index dad1adf3a2..59f7735439 100644 --- 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 index 71e2c457c3..6e674a49f2 100644 --- 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 index 64a2639c16..ab0bd15b51 100644 --- 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 index 724fde1d27..1d054a03ac 100644 --- a/ports/raspberrypi/boards/adafruit_feather_rp2040_usb_host/board.c +++ b/ports/raspberrypi/boards/adafruit_feather_rp2040_usb_host/board.c @@ -31,7 +31,6 @@ // 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) { @@ -39,5 +38,5 @@ void board_init(void) { 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 index 9ad28c73ea..93d19acd69 100644 --- 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 index c9294debb2..69a7f8deaf 100644 --- 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 index 53b1fec54f..9be6da9df3 100644 --- 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); + 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); - 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); + // Pin in use checks happen in the implementation so they can be ignored + // when returning the singleton. + + usb_host_port_obj_t *self = common_hal_usb_host_port_construct(dp, dm); return (mp_obj_t)self; } -//| 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); - -//| def __enter__(self) -> Port: -//| """No-op used by Context Managers.""" -//| ... -// Provided by context manager helper. - -//| 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; -} -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 index 68645d1146..ba8d298517 100644 --- 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 index 19f5c0a0ce..d097005aac 100644 --- 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 From e81ed62cfd8e024929c70d6aeed6c2994de52cd4 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 19 Jul 2023 11:46:04 -0700 Subject: [PATCH 2/2] Add missing header file --- ports/mimxrt10xx/imx_usb.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 ports/mimxrt10xx/imx_usb.h diff --git a/ports/mimxrt10xx/imx_usb.h b/ports/mimxrt10xx/imx_usb.h new file mode 100644 index 0000000000..7ae7df8461 --- /dev/null +++ b/ports/mimxrt10xx/imx_usb.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);