From 1117edae63df7b66f1575980bf54d6836f9a8aa8 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Sat, 24 Oct 2020 17:13:10 +0200 Subject: [PATCH 1/5] Fix erroneous claim of board.I2C() by deinited display. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the end of a session that called displayio.release_displays() (and did not initialize a new display), a board.I2C() bus that was previously used by a display would wrongly be considered still in use. While I can’t think of any unrecoverable problem this would cause in the next session, it violates the assumption that a soft reboot resets everything not needed by displays, potentially leading to confusion. By itself, this change does not fix the problem yet - rather, it introduces the same issue as in #3581 for SPI. This needs to be solved in the same way for I2C and SPI. --- shared-module/board/__init__.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-module/board/__init__.c b/shared-module/board/__init__.c index 39b68a0f11..856103b7b6 100644 --- a/shared-module/board/__init__.c +++ b/shared-module/board/__init__.c @@ -139,7 +139,7 @@ void reset_board_busses(void) { bool display_using_i2c = false; #if CIRCUITPY_DISPLAYIO for (uint8_t i = 0; i < CIRCUITPY_DISPLAY_LIMIT; i++) { - if (displays[i].i2cdisplay_bus.bus == i2c_singleton) { + if (displays[i].bus_base.type == &displayio_i2cdisplay_type && displays[i].i2cdisplay_bus.bus == i2c_singleton) { display_using_i2c = true; break; } From f4f80e07ca3dc2f5a93484e5342540837a94a505 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Sat, 24 Oct 2020 17:23:17 +0200 Subject: [PATCH 2/5] Fix lost board.SPI and board.I2C after being used by display. Fixes #3581. Pins were marked as never_reset by common_hal_displayio_fourwire_construct() and common_hal_sharpdisplay_framebuffer_construct(), but these marks were never removed, so at the end of a session after displayio.release_displays(), {spi|i2c}_singleton would be set to NULL but the pins would not be reset. In the next session, board.SPI() and board.I2C() were unable to reconstruct the object because the pins were still in use. For symmetry with creation of the singleton, add deinitialization before setting it to NULL in reset_board_busses(). This makes the pins resettable, so that reset_port(), moved behind it, then resets them. --- main.c | 4 +++- shared-module/board/__init__.c | 14 +++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/main.c b/main.c index b43b3b8c80..80b163f607 100755 --- a/main.c +++ b/main.c @@ -234,10 +234,12 @@ void cleanup_after_vm(supervisor_allocation* heap) { common_hal_canio_reset(); #endif - reset_port(); + // reset_board_busses() first because it may release pins from the never_reset state, so that + // reset_port() can reset them. #if CIRCUITPY_BOARD reset_board_busses(); #endif + reset_port(); reset_board(); reset_status_led(); } diff --git a/shared-module/board/__init__.c b/shared-module/board/__init__.c index 856103b7b6..967b430d2b 100644 --- a/shared-module/board/__init__.c +++ b/shared-module/board/__init__.c @@ -145,8 +145,11 @@ void reset_board_busses(void) { } } #endif - if (!display_using_i2c) { - i2c_singleton = NULL; + if (i2c_singleton != NULL) { + if (!display_using_i2c) { + common_hal_busio_i2c_deinit(i2c_singleton); + i2c_singleton = NULL; + } } #endif #if BOARD_SPI @@ -169,9 +172,10 @@ void reset_board_busses(void) { // make sure SPI lock is not held over a soft reset if (spi_singleton != NULL) { common_hal_busio_spi_unlock(spi_singleton); - } - if (!display_using_spi) { - spi_singleton = NULL; + if (!display_using_spi) { + common_hal_busio_spi_deinit(spi_singleton); + spi_singleton = NULL; + } } #endif #if BOARD_UART From 99a3750a2cfd1da8cdd266ca9a0367f0c3a54092 Mon Sep 17 00:00:00 2001 From: Christian Walther Date: Sat, 24 Oct 2020 17:50:11 +0200 Subject: [PATCH 3/5] Fix lost board.SPI and board.I2C after explicitly deiniting them. After calling board.SPI().deinit(), calling board.SPI() again would return the unusable deinited object and there was no way of getting it back into an initialized state until the end of the session. --- shared-bindings/board/__init__.c | 10 ++++++++-- shared-module/board/__init__.c | 10 ++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/shared-bindings/board/__init__.c b/shared-bindings/board/__init__.c index 6837fc41c2..717698408a 100644 --- a/shared-bindings/board/__init__.c +++ b/shared-bindings/board/__init__.c @@ -28,6 +28,12 @@ #include "py/runtime.h" #include "shared-bindings/board/__init__.h" +#if BOARD_I2C +#include "shared-bindings/busio/I2C.h" +#endif +#if BOARD_SPI +#include "shared-bindings/busio/SPI.h" +#endif //| """Board specific pin names //| @@ -45,7 +51,7 @@ #if BOARD_I2C mp_obj_t board_i2c(void) { mp_obj_t singleton = common_hal_board_get_i2c(); - if (singleton != NULL) { + if (singleton != NULL && !common_hal_busio_i2c_deinited(singleton)) { return singleton; } assert_pin_free(DEFAULT_I2C_BUS_SDA); @@ -69,7 +75,7 @@ MP_DEFINE_CONST_FUN_OBJ_0(board_i2c_obj, board_i2c); #if BOARD_SPI mp_obj_t board_spi(void) { mp_obj_t singleton = common_hal_board_get_spi(); - if (singleton != NULL) { + if (singleton != NULL && !common_hal_busio_spi_deinited(singleton)) { return singleton; } assert_pin_free(DEFAULT_SPI_BUS_SCK); diff --git a/shared-module/board/__init__.c b/shared-module/board/__init__.c index 967b430d2b..2f1c34e565 100644 --- a/shared-module/board/__init__.c +++ b/shared-module/board/__init__.c @@ -55,9 +55,8 @@ mp_obj_t common_hal_board_get_i2c(void) { } mp_obj_t common_hal_board_create_i2c(void) { - if (i2c_singleton != NULL) { - return i2c_singleton; - } + // All callers have either already verified this or come so early that it can't be otherwise. + assert(i2c_singleton == NULL || common_hal_busio_i2c_deinited(i2c_singleton)); busio_i2c_obj_t *self = &i2c_obj; self->base.type = &busio_i2c_type; @@ -79,9 +78,8 @@ mp_obj_t common_hal_board_get_spi(void) { } mp_obj_t common_hal_board_create_spi(void) { - if (spi_singleton != NULL) { - return spi_singleton; - } + // All callers have either already verified this or come so early that it can't be otherwise. + assert(spi_singleton == NULL || common_hal_busio_spi_deinited(spi_singleton)); busio_spi_obj_t *self = &spi_obj; self->base.type = &busio_spi_type; From 59b9ca409cc6d1835567f866f9a96c04bc78d2c3 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Wed, 28 Oct 2020 20:33:10 -0400 Subject: [PATCH 4/5] matrixportal ESP TX and RX pins were swapped --- ports/atmel-samd/boards/matrixportal_m4/pins.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ports/atmel-samd/boards/matrixportal_m4/pins.c b/ports/atmel-samd/boards/matrixportal_m4/pins.c index 34865597b6..1f9956d9ec 100644 --- a/ports/atmel-samd/boards/matrixportal_m4/pins.c +++ b/ports/atmel-samd/boards/matrixportal_m4/pins.c @@ -16,8 +16,8 @@ STATIC const mp_rom_map_elem_t board_global_dict_table[] = { { MP_OBJ_NEW_QSTR(MP_QSTR_ESP_BUSY), MP_ROM_PTR(&pin_PA22) }, { MP_OBJ_NEW_QSTR(MP_QSTR_ESP_RESET), MP_ROM_PTR(&pin_PA21) }, { MP_OBJ_NEW_QSTR(MP_QSTR_ESP_RTS), MP_ROM_PTR(&pin_PA18) }, - { MP_OBJ_NEW_QSTR(MP_QSTR_ESP_TX), MP_ROM_PTR(&pin_PA12) }, - { MP_OBJ_NEW_QSTR(MP_QSTR_ESP_RX), MP_ROM_PTR(&pin_PA13) }, + { MP_OBJ_NEW_QSTR(MP_QSTR_ESP_TX), MP_ROM_PTR(&pin_PA13) }, + { MP_OBJ_NEW_QSTR(MP_QSTR_ESP_RX), MP_ROM_PTR(&pin_PA12) }, { MP_OBJ_NEW_QSTR(MP_QSTR_SCL),MP_ROM_PTR(&pin_PB30) }, { MP_OBJ_NEW_QSTR(MP_QSTR_SDA),MP_ROM_PTR(&pin_PB31) }, From 345d84ffde527b16f50c203f7b6a718886be34f6 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 30 Oct 2020 22:16:12 -0400 Subject: [PATCH 5/5] improve USB CDC disconnect/reconnect checking --- supervisor/serial.h | 1 - supervisor/shared/serial.c | 6 +++--- supervisor/shared/usb/usb.c | 3 --- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/supervisor/serial.h b/supervisor/serial.h index 9c2d44737a..066886303e 100644 --- a/supervisor/serial.h +++ b/supervisor/serial.h @@ -47,5 +47,4 @@ char serial_read(void); bool serial_bytes_available(void); bool serial_connected(void); -extern volatile bool _serial_connected; #endif // MICROPY_INCLUDED_SUPERVISOR_SERIAL_H diff --git a/supervisor/shared/serial.c b/supervisor/shared/serial.c index 91e90671d2..7383cc2282 100644 --- a/supervisor/shared/serial.c +++ b/supervisor/shared/serial.c @@ -47,8 +47,6 @@ busio_uart_obj_t debug_uart; byte buf_array[64]; #endif -volatile bool _serial_connected; - void serial_early_init(void) { #if defined(DEBUG_UART_TX) && defined(DEBUG_UART_RX) debug_uart.base.type = &busio_uart_type; @@ -71,7 +69,9 @@ bool serial_connected(void) { #if defined(DEBUG_UART_TX) && defined(DEBUG_UART_RX) return true; #else - return _serial_connected; + // True if DTR is asserted, and the USB connection is up. + // tud_cdc_get_line_state(): bit 0 is DTR, bit 1 is RTS + return (tud_cdc_get_line_state() & 1) && tud_ready(); #endif } diff --git a/supervisor/shared/usb/usb.c b/supervisor/shared/usb/usb.c index 89fbf56f37..93d3436e9d 100644 --- a/supervisor/shared/usb/usb.c +++ b/supervisor/shared/usb/usb.c @@ -116,7 +116,6 @@ void tud_umount_cb(void) { // remote_wakeup_en : if host allows us to perform remote wakeup // USB Specs: Within 7ms, device must draw an average current less than 2.5 mA from bus void tud_suspend_cb(bool remote_wakeup_en) { - _serial_connected = false; } // Invoked when usb bus is resumed @@ -128,8 +127,6 @@ void tud_resume_cb(void) { void tud_cdc_line_state_cb(uint8_t itf, bool dtr, bool rts) { (void) itf; // interface ID, not used - _serial_connected = dtr; - // DTR = false is counted as disconnected if ( !dtr ) {