From e00b3269febff0d7e1224d76d8301d8c95850536 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Wed, 12 Feb 2020 15:04:19 -0500 Subject: [PATCH] use properly-sized SPI transactions --- ports/nrf/boards/common.template.ld | 4 +- ports/nrf/common-hal/busio/SPI.c | 114 ++++++++++------------------ ports/nrf/common-hal/busio/SPI.h | 2 +- ports/nrf/ld_defines.c | 3 + ports/nrf/mpconfigport.h | 7 ++ 5 files changed, 51 insertions(+), 79 deletions(-) diff --git a/ports/nrf/boards/common.template.ld b/ports/nrf/boards/common.template.ld index 427b6b35b3..2fca167079 100644 --- a/ports/nrf/boards/common.template.ld +++ b/ports/nrf/boards/common.template.ld @@ -22,8 +22,8 @@ MEMORY /* SoftDevice 6.1.0 with 5 connections and various increases takes just under 64kiB. /* To measure the minimum required amount of memory for given configuration, set this number high enough to work and then check the mutation of the value done by sd_ble_enable. */ - SPIM3_RAM (rw) : ORIGIN = 0x20000000 + 64K, LENGTH = 8K - RAM (xrw) : ORIGIN = 0x20000000 + 64K + 8K, LENGTH = 256K - 64K -8K + SPIM3_RAM (rw) : ORIGIN = 0x20000000 + ${SOFTDEVICE_RAM_SIZE}, LENGTH = ${SPIM3_BUFFER_SIZE} + RAM (xrw) : ORIGIN = 0x20000000 + ${SOFTDEVICE_RAM_SIZE} + ${SPIM3_BUFFER_SIZE}, LENGTH = 256K - ${SOFTDEVICE_RAM_SIZE} -${SPIM3_BUFFER_SIZE} } diff --git a/ports/nrf/common-hal/busio/SPI.c b/ports/nrf/common-hal/busio/SPI.c index fe943a2215..43aced1a5f 100644 --- a/ports/nrf/common-hal/busio/SPI.c +++ b/ports/nrf/common-hal/busio/SPI.c @@ -31,35 +31,35 @@ #include "nrfx_spim.h" #include "nrf_gpio.h" -// These are in order from ighest available frequency to lowest (32MHz first, then 8MHz). +// These are in order from highest available frequency to lowest (32MHz first, then 8MHz). STATIC spim_peripheral_t spim_peripherals[] = { #if NRFX_CHECK(NRFX_SPIM3_ENABLED) // SPIM3 exists only on nRF52840 and supports 32MHz max. All other SPIM's are only 8MHz max. // Allocate SPIM3 first. { .spim = NRFX_SPIM_INSTANCE(3), .max_frequency = 32000000, - .max_xfer_size = SPIM3_EASYDMA_MAXCNT_SIZE, + .max_xfer_size = MIN(SPIM3_BUFFER_SIZE, (1UL << SPIM3_EASYDMA_MAXCNT_SIZE) - 1) }, #endif #if NRFX_CHECK(NRFX_SPIM2_ENABLED) // SPIM2 is not shared with a TWIM, so allocate before the shared ones. { .spim = NRFX_SPIM_INSTANCE(2), .max_frequency = 8000000, - .max_xfer_size = SPIM2_EASYDMA_MAXCNT_SIZE, + .max_xfer_size = (1UL << SPIM2_EASYDMA_MAXCNT_SIZE) - 1 }, #endif #if NRFX_CHECK(NRFX_SPIM1_ENABLED) // SPIM1 and TWIM1 share an address. { .spim = NRFX_SPIM_INSTANCE(1), .max_frequency = 8000000, - .max_xfer_size = SPIM1_EASYDMA_MAXCNT_SIZE, + .max_xfer_size = (1UL << SPIM1_EASYDMA_MAXCNT_SIZE) - 1 }, #endif #if NRFX_CHECK(NRFX_SPIM0_ENABLED) // SPIM0 and TWIM0 share an address. { .spim = NRFX_SPIM_INSTANCE(0), .max_frequency = 8000000, - .max_xfer_size = SPIM0_EASYDMA_MAXCNT_SIZE, + .max_xfer_size = (1UL << SPIM0_EASYDMA_MAXCNT_SIZE) - 1 }, #endif }; @@ -232,104 +232,66 @@ void common_hal_busio_spi_unlock(busio_spi_obj_t *self) { } bool common_hal_busio_spi_write(busio_spi_obj_t *self, const uint8_t *data, size_t len) { - if (len == 0) { - return true; - } - const bool is_spim3 = self->spim_peripheral->spim.p_reg == NRF_SPIM3; + uint8_t *next_chunk = (uint8_t *) data; - const uint32_t max_xfer_size = self->spim_peripheral->max_xfer_size; - const uint32_t parts = len / max_xfer_size; - const uint32_t remainder = len % max_xfer_size; - - for (uint32_t i = 0; i < parts; ++i) { - uint8_t *start = (uint8_t *) (data + i * max_xfer_size); + while (len > 0) { + size_t chunk_size = MIN(len, self->spim_peripheral->max_xfer_size); + uint8_t *chunk = next_chunk; if (is_spim3) { // If SPIM3, copy into unused RAM block, and do DMA from there. - memcpy(spim3_transmit_buffer, start, max_xfer_size); - start = spim3_transmit_buffer; + memcpy(spim3_transmit_buffer, chunk, chunk_size); + chunk = spim3_transmit_buffer; } - const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_XFER_TX(start, max_xfer_size); - if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) + const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_XFER_TX(chunk, chunk_size); + if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) { return false; - } - - if (remainder > 0) { - uint8_t *start = (uint8_t *) (data + parts * max_xfer_size); - if (is_spim3) { - // If SPIM3, copy into unused RAM block, and do DMA from there. - memcpy(spim3_transmit_buffer, start, remainder); - start = spim3_transmit_buffer; } - const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_XFER_TX(start, remainder); - if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) - return false; + next_chunk += chunk_size; + len -= chunk_size; } - return true; } bool common_hal_busio_spi_read(busio_spi_obj_t *self, uint8_t *data, size_t len, uint8_t write_value) { - if (len == 0) { - return true; - } + uint8_t *next_chunk = data; - const uint32_t max_xfer_size = self->spim_peripheral->max_xfer_size; - const uint32_t parts = len / max_xfer_size; - const uint32_t remainder = len % max_xfer_size; - - for (uint32_t i = 0; i < parts; ++i) { - const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_XFER_RX(data + i * max_xfer_size, max_xfer_size); - if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) + while (len > 0) { + size_t chunk_size = MIN(len, self->spim_peripheral->max_xfer_size); + const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_XFER_RX(next_chunk, chunk_size); + if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) { return false; + } + next_chunk += chunk_size; + len -= chunk_size; } - - if (remainder > 0) { - const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_XFER_RX(data + parts * max_xfer_size, remainder); - if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) - return false; - } - return true; } bool common_hal_busio_spi_transfer(busio_spi_obj_t *self, uint8_t *data_out, uint8_t *data_in, size_t len) { - if (len == 0) { - return true; - } - const bool is_spim3 = self->spim_peripheral->spim.p_reg == NRF_SPIM3; + uint8_t *next_chunk_out = data_out; + uint8_t *next_chunk_in = data_in; - const uint32_t max_xfer_size = self->spim_peripheral->max_xfer_size; - const uint32_t parts = len / max_xfer_size; - const uint32_t remainder = len % max_xfer_size; - - for (uint32_t i = 0; i < parts; ++i) { - uint8_t *out_start = (uint8_t *) (data_out + i * max_xfer_size); + while (len > 0) { + uint8_t *chunk_out = next_chunk_out; + size_t chunk_size = MIN(len, self->spim_peripheral->max_xfer_size); if (is_spim3) { // If SPIM3, copy into unused RAM block, and do DMA from there. - memcpy(spim3_transmit_buffer, out_start, max_xfer_size); - out_start = spim3_transmit_buffer; + memcpy(spim3_transmit_buffer, chunk_out, chunk_size); + chunk_out = spim3_transmit_buffer; } - const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_SINGLE_XFER(out_start, max_xfer_size, - data_in + i * max_xfer_size, max_xfer_size); - if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) + const nrfx_spim_xfer_desc_t xfer = + NRFX_SPIM_SINGLE_XFER(next_chunk_out, chunk_size, + next_chunk_in, chunk_size); + if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) { return false; - } - - if (remainder > 0) { - uint8_t *out_start = (uint8_t *) (data_out + parts * max_xfer_size); - if (is_spim3) { - // If SPIM3, copy into unused RAM block, and do DMA from there. - memcpy(spim3_transmit_buffer, out_start, remainder); - out_start = spim3_transmit_buffer; } - const nrfx_spim_xfer_desc_t xfer = NRFX_SPIM_SINGLE_XFER(out_start, remainder, - data_in + parts * max_xfer_size, remainder); - if (nrfx_spim_xfer(&self->spim_peripheral->spim, &xfer, 0) != NRFX_SUCCESS) - return false; - } + next_chunk_out += chunk_size; + next_chunk_in += chunk_size; + len -= chunk_size; + } return true; } diff --git a/ports/nrf/common-hal/busio/SPI.h b/ports/nrf/common-hal/busio/SPI.h index 738a1de788..ef3ac9531e 100644 --- a/ports/nrf/common-hal/busio/SPI.h +++ b/ports/nrf/common-hal/busio/SPI.h @@ -33,7 +33,7 @@ typedef struct { nrfx_spim_t spim; uint32_t max_frequency; - uint8_t max_xfer_size; + uint32_t max_xfer_size; } spim_peripheral_t; typedef struct { diff --git a/ports/nrf/ld_defines.c b/ports/nrf/ld_defines.c index 0ec6dfdb5d..ebe9c4929e 100644 --- a/ports/nrf/ld_defines.c +++ b/ports/nrf/ld_defines.c @@ -37,3 +37,6 @@ /*BOOTLOADER_SETTINGS_START_ADDR=*/ BOOTLOADER_SETTINGS_START_ADDR; /*BOOTLOADER_SETTINGS_SIZE=*/ BOOTLOADER_SETTINGS_SIZE; + +/*SOFTDEVICE_RAM_SIZE=*/ SOFTDEVICE_RAM_SIZE; +/*SPIM3_BUFFER_SIZE=*/ SPIM3_BUFFER_SIZE; diff --git a/ports/nrf/mpconfigport.h b/ports/nrf/mpconfigport.h index b6635965de..db082ab51c 100644 --- a/ports/nrf/mpconfigport.h +++ b/ports/nrf/mpconfigport.h @@ -34,9 +34,16 @@ #include "nrf_sdm.h" // for SD_FLASH_SIZE #include "peripherals/nrf/nvm.h" // for FLASH_PAGE_SIZE +// Max RAM used by SoftDevice. Can be changed when SoftDevice parameters are changed. +// See common.template.ld. +#define SOFTDEVICE_RAM_SIZE (64*1024) + #ifdef NRF52840 #define MICROPY_PY_SYS_PLATFORM "nRF52840" #define FLASH_SIZE (0x100000) // 1MiB +// Special RAM area for SPIM3 transmit buffer, to work around hardware bug. +// See common.template.ld. +#define SPIM3_BUFFER_SIZE (8192) #endif #define MICROPY_PY_COLLECTIONS_ORDEREDDICT (1)