From e35eb08f1d91f4db7a65eea0987a8cdc4d1e1d65 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Mon, 28 Oct 2019 21:08:53 -0400 Subject: [PATCH 1/3] nrf: allocate two I2C on CPB --- .../mpconfigboard.mk | 7 ++++ ports/nrf/nrfx_config.h | 33 ++++++++++++++----- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/ports/nrf/boards/circuitplayground_bluefruit/mpconfigboard.mk b/ports/nrf/boards/circuitplayground_bluefruit/mpconfigboard.mk index dbb32629c6..d53e486f9d 100644 --- a/ports/nrf/boards/circuitplayground_bluefruit/mpconfigboard.mk +++ b/ports/nrf/boards/circuitplayground_bluefruit/mpconfigboard.mk @@ -24,3 +24,10 @@ NRF_DEFINES += -DNRF52840_XXAA -DNRF52840 QSPI_FLASH_FILESYSTEM = 1 EXTERNAL_FLASH_DEVICE_COUNT = 1 EXTERNAL_FLASH_DEVICES = "GD25Q16C" + +# Allocate two, not just one I2C peripheral for CPB, so that we have both +# on-board and off-board I2C available. +# When SPIM3 becomes available we'll be able to have two I2C and two SPI peripherals. +# We use a CFLAGS define here because there are include order issues +# if we try to include "mpconfigport.h" into nrfx_config.h . +CFLAGS += -DCIRCUITPY_NRF_NUM_I2C=2 diff --git a/ports/nrf/nrfx_config.h b/ports/nrf/nrfx_config.h index 8fa6721e2c..cafec6aa1d 100644 --- a/ports/nrf/nrfx_config.h +++ b/ports/nrf/nrfx_config.h @@ -5,12 +5,9 @@ #define NRFX_POWER_ENABLED 1 #define NRFX_POWER_CONFIG_IRQ_PRIORITY 7 -// Turn on nrfx supported workarounds for errata in Rev1/Rev2 of nRF52832 -#ifdef NRF52832_XXAA - #define NRFX_SPIS_NRF52_ANOMALY_109_WORKAROUND_ENABLED 1 -#endif - -// NOTE: THIS WORKAROUND CAUSES BLE CODE TO CRASH; tested on 2019-03-11. +// NOTE: THIS WORKAROUND CAUSES BLE CODE TO CRASH. +// It doesn't work with the SoftDevice. +// See https://devzone.nordicsemi.com/f/nordic-q-a/33982/sdk-15-software-crash-during-spi-session // Turn on nrfx supported workarounds for errata in Rev1 of nRF52840 #ifdef NRF52840_XXAA // #define NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED 1 @@ -24,11 +21,26 @@ // so out of the box TWIM0/SPIM0 and TWIM1/SPIM1 cannot be shared // between common-hal/busio/I2C.c and SPI.c. // We could write an interrupt handler that checks whether it's -// being used for SPI or I2C, but perhaps two I2C's and 1-2 SPI's are good enough for now. +// being used for SPI or I2C, but perhaps one I2C and two SPI or two I2C and one SPI +// are good enough for now. + +// CIRCUITPY_NRF_NUM_I2C is 1 or 2 to choose how many I2C (TWIM) peripherals +// to provide. +// This can go away once we have SPIM3 working: then we can have two +// I2C and two SPI. +#ifndef CIRCUITPY_NRF_NUM_I2C +#define CIRCUITPY_NRF_NUM_I2C 1 +#endif + +#if CIRCUITPY_NRF_NUM_I2C != 1 && CIRCUITPY_NRF_NUM_I2C != 2 +# error CIRCUITPY_NRF_NUM_I2C must be 1 or 2 +#endif // Enable SPIM1, SPIM2 and SPIM3 (if available) // No conflict with TWIM0. +#if CIRCUITPY_NRF_NUM_I2C == 1 #define NRFX_SPIM1_ENABLED 1 +#endif #define NRFX_SPIM2_ENABLED 1 // DON'T ENABLE SPIM3 DUE TO ANOMALY WORKAROUND FAILURE (SEE ABOVE). // #ifdef NRF52840_XXAA @@ -45,10 +57,13 @@ // QSPI #define NRFX_QSPI_ENABLED 1 -// TWI aka. I2C; enable a single bus: TWIM0 (no conflict with SPIM1 and SPIM2) +// TWI aka. I2C; always enable TWIM0 (no conflict with SPIM1 and SPIM2) #define NRFX_TWIM_ENABLED 1 #define NRFX_TWIM0_ENABLED 1 -//#define NRFX_TWIM1_ENABLED 1 + +#if CIRCUITPY_NRF_NUM_I2C == 2 +#define NRFX_TWIM1_ENABLED 1 +#endif #define NRFX_TWIM_DEFAULT_CONFIG_IRQ_PRIORITY 7 #define NRFX_TWIM_DEFAULT_CONFIG_FREQUENCY NRF_TWIM_FREQ_400K From 43b8d5e8ab07d9053fd55059a240891b6547626b Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 29 Oct 2019 09:58:44 -0400 Subject: [PATCH 2/3] Update I2C and SPI documentation --- shared-bindings/busio/I2C.c | 4 ++++ shared-bindings/busio/SPI.c | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/shared-bindings/busio/I2C.c b/shared-bindings/busio/I2C.c index 3bd89e2e20..50a95beb2e 100644 --- a/shared-bindings/busio/I2C.c +++ b/shared-bindings/busio/I2C.c @@ -60,6 +60,10 @@ //| :param int frequency: The clock frequency in Hertz //| :param int timeout: The maximum clock stretching timeut - (used only for bitbangio.I2C; ignored for busio.I2C) //| +//| .. note:: On the nRF52840, only one I2C object may be created, +//| except on the Circuit Playground Bluefruit, which allows two, +//| one for the onboard accelerometer, and one for offboard use. +//| STATIC mp_obj_t busio_i2c_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { busio_i2c_obj_t *self = m_new_obj(busio_i2c_obj_t); self->base.type = &busio_i2c_type; diff --git a/shared-bindings/busio/SPI.c b/shared-bindings/busio/SPI.c index d47bf499a2..d1791bef3b 100644 --- a/shared-bindings/busio/SPI.c +++ b/shared-bindings/busio/SPI.c @@ -154,11 +154,12 @@ STATIC void check_for_deinit(busio_spi_obj_t *self) { //| within spec for the SAMD21. //| //| .. note:: On the nRF52840, these baudrates are available: 125kHz, 250kHz, 1MHz, 2MHz, 4MHz, -//| and 8MHz. 16MHz and 32MHz are also available, but only on the first -//| `busio.SPI` object you create. Two more ``busio.SPI`` objects can be created, but they are restricted -//| to 8MHz maximum. This is a hardware restriction: there is only one high-speed SPI peripheral. +//| and 8MHz. //| If you pick a a baudrate other than one of these, the nearest lower //| baudrate will be chosen, with a minimum of 125kHz. +//| Two SPI objects may be created, except on the Circuit Playground Bluefruit, +//| which allows only one (to allow for an additional I2C object). +//| STATIC mp_obj_t busio_spi_configure(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { enum { ARG_baudrate, ARG_polarity, ARG_phase, ARG_bits }; static const mp_arg_t allowed_args[] = { From 85a648224fea027eb3635b7a3746cd6c12d738a6 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sat, 2 Nov 2019 22:47:03 -0400 Subject: [PATCH 3/3] Check for no pullups on I2C on nrf; give arduino nano 33 ble two I2C devices --- ports/atmel-samd/common-hal/busio/I2C.c | 1 + .../arduino_nano_33_ble/mpconfigboard.mk | 7 +++++ ports/nrf/common-hal/busio/I2C.c | 29 +++++++++++++++---- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/ports/atmel-samd/common-hal/busio/I2C.c b/ports/atmel-samd/common-hal/busio/I2C.c index cdc616bbc8..b0e5714a79 100644 --- a/ports/atmel-samd/common-hal/busio/I2C.c +++ b/ports/atmel-samd/common-hal/busio/I2C.c @@ -116,6 +116,7 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self, if (i2c_m_sync_set_baudrate(&self->i2c_desc, 0, frequency / 1000) != ERR_NONE) { reset_pin_number(sda->number); reset_pin_number(scl->number); + common_hal_busio_i2c_deinit(self); mp_raise_ValueError(translate("Unsupported baudrate")); } diff --git a/ports/nrf/boards/arduino_nano_33_ble/mpconfigboard.mk b/ports/nrf/boards/arduino_nano_33_ble/mpconfigboard.mk index 7a8e883f47..9a875ac442 100644 --- a/ports/nrf/boards/arduino_nano_33_ble/mpconfigboard.mk +++ b/ports/nrf/boards/arduino_nano_33_ble/mpconfigboard.mk @@ -22,3 +22,10 @@ endif NRF_DEFINES += -DNRF52840_XXAA -DNRF52840 INTERNAL_FLASH_FILESYSTEM = 1 + +# Allocate two, not just one I2C peripheral, so that we have both +# on-board and off-board I2C available. +# When SPIM3 becomes available we'll be able to have two I2C and two SPI peripherals. +# We use a CFLAGS define here because there are include order issues +# if we try to include "mpconfigport.h" into nrfx_config.h . +CFLAGS += -DCIRCUITPY_NRF_NUM_I2C=2 diff --git a/ports/nrf/common-hal/busio/I2C.c b/ports/nrf/common-hal/busio/I2C.c index 71835d16ad..8a2ebde3bd 100644 --- a/ports/nrf/common-hal/busio/I2C.c +++ b/ports/nrf/common-hal/busio/I2C.c @@ -28,13 +28,12 @@ */ #include "shared-bindings/busio/I2C.h" +#include "shared-bindings/microcontroller/__init__.h" #include "py/mperrno.h" #include "py/runtime.h" #include "supervisor/shared/translate.h" #include "nrfx_twim.h" -#include "nrf_gpio.h" - #include "nrfx_spim.h" #include "nrf_gpio.h" @@ -107,7 +106,7 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self, const mcu_pin_obj_t * for (size_t i = 0 ; i < MP_ARRAY_SIZE(twim_peripherals); i++) { if (!twim_peripherals[i].in_use) { self->twim_peripheral = &twim_peripherals[i]; - self->twim_peripheral->in_use = true; + // Mark it as in_use later after other validation is finished. break; } } @@ -116,10 +115,27 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self, const mcu_pin_obj_t * mp_raise_ValueError(translate("All I2C peripherals are in use")); } + // Test that the pins are in a high state. (Hopefully indicating they are pulled up.) + nrf_gpio_cfg_input(scl->number, NRF_GPIO_PIN_PULLDOWN); + nrf_gpio_cfg_input(sda->number, NRF_GPIO_PIN_PULLDOWN); + + common_hal_mcu_delay_us(10); + + nrf_gpio_cfg_input(scl->number, NRF_GPIO_PIN_NOPULL); + nrf_gpio_cfg_input(sda->number, NRF_GPIO_PIN_NOPULL); + + // We must pull up within 3us to achieve 400khz. + common_hal_mcu_delay_us(3); + + if (!nrf_gpio_pin_read(sda->number) || !nrf_gpio_pin_read(scl->number)) { + reset_pin_number(sda->number); + reset_pin_number(scl->number); + mp_raise_RuntimeError(translate("SDA or SCL needs a pull up")); + } + nrfx_twim_config_t config = NRFX_TWIM_DEFAULT_CONFIG; config.scl = scl->number; config.sda = sda->number; - // change freq. only if it's less than the default 400K if (frequency < 100000) { config.frequency = NRF_TWIM_FREQ_100K; @@ -132,6 +148,8 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self, const mcu_pin_obj_t * claim_pin(sda); claim_pin(scl); + // About to init. If we fail after this point, common_hal_busio_i2c_deinit() will set in_use to false. + self->twim_peripheral->in_use = true; nrfx_err_t err = nrfx_twim_init(&self->twim_peripheral->twim, &config, NULL, NULL); // A soft reset doesn't uninit the driver so we might end up with a invalid state @@ -152,8 +170,9 @@ bool common_hal_busio_i2c_deinited(busio_i2c_obj_t *self) { } void common_hal_busio_i2c_deinit(busio_i2c_obj_t *self) { - if (common_hal_busio_i2c_deinited(self)) + if (common_hal_busio_i2c_deinited(self)) { return; + } nrfx_twim_uninit(&self->twim_peripheral->twim);