From 7686f93ef456aa46f538da3159d0e12cae9fa737 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Fri, 5 Apr 2019 19:06:37 -0700 Subject: Fix crash when getting board.SPI outside the VM If one of the default pins was already in use it would crash. The internal API has been refined to allow us to get the value without causing an init of the singleton. Fixes #1753 --- .../atmel-samd/boards/hallowing_m0_express/board.c | 2 +- .../atmel-samd/common-hal/digitalio/DigitalInOut.c | 5 +++ shared-bindings/digitalio/DigitalInOut.h | 1 + shared-module/displayio/__init__.c | 2 +- supervisor/shared/board_busses.c | 45 +++++++++++++++------- supervisor/shared/board_busses.h | 3 +- supervisor/shared/external_flash/spi_flash.c | 4 ++ supervisor/shared/safe_mode.c | 3 +- 8 files changed, 48 insertions(+), 17 deletions(-) diff --git a/ports/atmel-samd/boards/hallowing_m0_express/board.c b/ports/atmel-samd/boards/hallowing_m0_express/board.c index ababaef35..78c772323 100644 --- a/ports/atmel-samd/boards/hallowing_m0_express/board.c +++ b/ports/atmel-samd/boards/hallowing_m0_express/board.c @@ -72,7 +72,7 @@ void board_init(void) { displayio_fourwire_obj_t* bus = &displays[0].fourwire_bus; bus->base.type = &displayio_fourwire_type; common_hal_displayio_fourwire_construct(bus, - board_spi(), + common_hal_board_create_spi(), &pin_PA28, // Command or data &pin_PA01, // Chip select &pin_PA27); // Reset diff --git a/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c b/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c index 9537d6179..e167cbb69 100644 --- a/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c +++ b/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c @@ -49,6 +49,11 @@ digitalinout_result_t common_hal_digitalio_digitalinout_construct( return DIGITALINOUT_OK; } +void common_hal_digitalio_digitalinout_never_reset( + digitalio_digitalinout_obj_t *self) { + never_reset_pin_number(self->pin->number); +} + bool common_hal_digitalio_digitalinout_deinited(digitalio_digitalinout_obj_t* self) { return self->pin == mp_const_none; } diff --git a/shared-bindings/digitalio/DigitalInOut.h b/shared-bindings/digitalio/DigitalInOut.h index 2aaa31b7f..037979098 100644 --- a/shared-bindings/digitalio/DigitalInOut.h +++ b/shared-bindings/digitalio/DigitalInOut.h @@ -52,5 +52,6 @@ void common_hal_digitalio_digitalinout_set_drive_mode(digitalio_digitalinout_obj digitalio_drive_mode_t common_hal_digitalio_digitalinout_get_drive_mode(digitalio_digitalinout_obj_t* self); void common_hal_digitalio_digitalinout_set_pull(digitalio_digitalinout_obj_t* self, digitalio_pull_t pull); digitalio_pull_t common_hal_digitalio_digitalinout_get_pull(digitalio_digitalinout_obj_t* self); +void common_hal_digitalio_digitalinout_never_reset(digitalio_digitalinout_obj_t *self); #endif // MICROPY_INCLUDED_SHARED_BINDINGS_DIGITALIO_DIGITALINOUT_H diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c index ced5b0fef..49c85f277 100644 --- a/shared-module/displayio/__init__.c +++ b/shared-module/displayio/__init__.c @@ -190,7 +190,7 @@ void reset_displays(void) { if (((uint32_t) fourwire->bus) < ((uint32_t) &displays) || ((uint32_t) fourwire->bus) > ((uint32_t) &displays + CIRCUITPY_DISPLAY_LIMIT)) { busio_spi_obj_t* original_spi = fourwire->bus; - if (original_spi == board_spi()) { + if (original_spi == common_hal_board_get_spi()) { continue; } memcpy(&fourwire->inline_bus, original_spi, sizeof(busio_spi_obj_t)); diff --git a/supervisor/shared/board_busses.c b/supervisor/shared/board_busses.c index d77c4f331..b2d04207d 100644 --- a/supervisor/shared/board_busses.c +++ b/supervisor/shared/board_busses.c @@ -71,23 +71,42 @@ MP_DEFINE_CONST_FUN_OBJ_0(board_i2c_obj, board_i2c); STATIC busio_spi_obj_t spi_obj; STATIC mp_obj_t spi_singleton = NULL; -mp_obj_t board_spi(void) { - if (spi_singleton == NULL) { - busio_spi_obj_t *self = &spi_obj; - self->base.type = &busio_spi_type; - assert_pin_free(DEFAULT_SPI_BUS_SCK); - assert_pin_free(DEFAULT_SPI_BUS_MOSI); - assert_pin_free(DEFAULT_SPI_BUS_MISO); - const mcu_pin_obj_t* clock = MP_OBJ_TO_PTR(DEFAULT_SPI_BUS_SCK); - const mcu_pin_obj_t* mosi = MP_OBJ_TO_PTR(DEFAULT_SPI_BUS_MOSI); - const mcu_pin_obj_t* miso = MP_OBJ_TO_PTR(DEFAULT_SPI_BUS_MISO); - common_hal_busio_spi_construct(self, clock, mosi, miso); - spi_singleton = (mp_obj_t)self; +// TODO(tannewt): Move this to shared-bindings/board/__init__.c and corresponding shared-module. +mp_obj_t common_hal_board_get_spi(void) { + return spi_singleton; +} + +mp_obj_t common_hal_board_create_spi(void) { + if (spi_singleton != NULL) { + return spi_singleton; + } + busio_spi_obj_t *self = &spi_obj; + self->base.type = &busio_spi_type; + if (!common_hal_mcu_pin_is_free(DEFAULT_SPI_BUS_SCK) || + !common_hal_mcu_pin_is_free(DEFAULT_SPI_BUS_MOSI) || + !common_hal_mcu_pin_is_free(DEFAULT_SPI_BUS_MISO)) { + return NULL; } + const mcu_pin_obj_t* clock = MP_OBJ_TO_PTR(DEFAULT_SPI_BUS_SCK); + const mcu_pin_obj_t* mosi = MP_OBJ_TO_PTR(DEFAULT_SPI_BUS_MOSI); + const mcu_pin_obj_t* miso = MP_OBJ_TO_PTR(DEFAULT_SPI_BUS_MISO); + common_hal_busio_spi_construct(self, clock, mosi, miso); + spi_singleton = (mp_obj_t)self; return spi_singleton; } -#else + mp_obj_t board_spi(void) { + mp_obj_t singleton = common_hal_board_get_spi(); + if (singleton != NULL) { + return singleton; + } + assert_pin_free(DEFAULT_SPI_BUS_SCK); + assert_pin_free(DEFAULT_SPI_BUS_MOSI); + assert_pin_free(DEFAULT_SPI_BUS_MISO); + return common_hal_board_create_spi(); +} +#else +mp_obj_t common_hal_board_spi(void) { mp_raise_NotImplementedError(translate("No default SPI bus")); return NULL; } diff --git a/supervisor/shared/board_busses.h b/supervisor/shared/board_busses.h index 0ccb3ba6a..bdb3884f6 100644 --- a/supervisor/shared/board_busses.h +++ b/supervisor/shared/board_busses.h @@ -29,7 +29,8 @@ #include "py/obj.h" -mp_obj_t board_i2c(void); +mp_obj_t common_hal_board_get_spi(void); +mp_obj_t common_hal_board_create_spi(void); MP_DECLARE_CONST_FUN_OBJ_0(board_i2c_obj); mp_obj_t board_spi(void); diff --git a/supervisor/shared/external_flash/spi_flash.c b/supervisor/shared/external_flash/spi_flash.c index ec7101bc9..12888f2d3 100644 --- a/supervisor/shared/external_flash/spi_flash.c +++ b/supervisor/shared/external_flash/spi_flash.c @@ -132,11 +132,15 @@ bool spi_flash_read_data(uint32_t address, uint8_t* data, uint32_t data_length) } void spi_flash_init(void) { + cs_pin.base.type = &digitalio_digitalinout_type; common_hal_digitalio_digitalinout_construct(&cs_pin, SPI_FLASH_CS_PIN); + // Set CS high (disabled). common_hal_digitalio_digitalinout_switch_to_output(&cs_pin, true, DRIVE_MODE_PUSH_PULL); + common_hal_digitalio_digitalinout_never_reset(&cs_pin); + spi.base.type = &busio_spi_type; common_hal_busio_spi_construct(&spi, SPI_FLASH_SCK_PIN, SPI_FLASH_MOSI_PIN, SPI_FLASH_MISO_PIN); common_hal_busio_spi_never_reset(&spi); } diff --git a/supervisor/shared/safe_mode.c b/supervisor/shared/safe_mode.c index 3bb75b897..7182bbcca 100644 --- a/supervisor/shared/safe_mode.c +++ b/supervisor/shared/safe_mode.c @@ -77,7 +77,8 @@ safe_mode_t wait_for_safe_mode_reset(void) { return NO_SAFE_MODE; } -void reset_into_safe_mode(safe_mode_t reason) { +// Inline this so it's easy to break on it from GDB. +void __attribute__((noinline,)) reset_into_safe_mode(safe_mode_t reason) { if (current_safe_mode > BROWNOUT && reason > BROWNOUT) { while (true) { // This very bad because it means running in safe mode didn't save us. Only ignore brownout -- cgit v1.2.3