From 6ab75fe8c5e2e4ca930323f0e1e8ef0fcf009627 Mon Sep 17 00:00:00 2001 From: Hierophect Date: Tue, 7 Jan 2020 13:19:00 -0500 Subject: Allow MOSI/MISO exclusion --- ports/stm32f4/common-hal/busio/SPI.c | 143 +++++++++++++++++++++++------------ 1 file changed, 95 insertions(+), 48 deletions(-) diff --git a/ports/stm32f4/common-hal/busio/SPI.c b/ports/stm32f4/common-hal/busio/SPI.c index 32089887f..fe2041683 100644 --- a/ports/stm32f4/common-hal/busio/SPI.c +++ b/ports/stm32f4/common-hal/busio/SPI.c @@ -36,6 +36,9 @@ #include "supervisor/shared/translate.h" #include "common-hal/microcontroller/Pin.h" +// Note that any bugs introduced in this file can cause crashes at startup +// for chips using external SPI flash. + #define MAX_SPI 6 //TODO; replace this as part of periph cleanup #define ALL_CLOCKS 0xFF @@ -79,38 +82,80 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self, uint8_t sck_len = sizeof(mcu_spi_sck_list)/sizeof(*mcu_spi_sck_list); uint8_t mosi_len = sizeof(mcu_spi_mosi_list)/sizeof(*mcu_spi_mosi_list); uint8_t miso_len = sizeof(mcu_spi_miso_list)/sizeof(*mcu_spi_miso_list); - bool spi_taken = false; - //sck + + //sck is not optional. MOSI and MISO are for(uint i=0; isck = &mcu_spi_sck_list[i]; + self->mosi = &mcu_spi_mosi_list[j]; + self->miso = &mcu_spi_miso_list[k]; + break; } - //store pins if not - self->sck = &mcu_spi_sck_list[i]; - self->mosi = &mcu_spi_mosi_list[j]; - self->miso = &mcu_spi_miso_list[k]; - break; - } - } + } + } } + // if just miso, reduce search + } else if (miso != mp_const_none) { + for(uint j=0; jsck = &mcu_spi_sck_list[i]; + self->mosi = NULL; + self->miso = &mcu_spi_miso_list[j]; + break; + } + } + // if just mosi, reduce search + } else if (mosi != mp_const_none) { + for(uint j=0; jsck = &mcu_spi_sck_list[i]; + self->mosi = &mcu_spi_mosi_list[j]; + self->miso = NULL; + break; + } + } + } else { + //throw an error immediately + mp_raise_ValueError(translate("Must provide MISO or MOSI pin")); } } } //handle typedef selection, errors - if(self->sck!=NULL && self->mosi!=NULL && self->miso!=NULL ) { + if( (self->sck!=NULL && self->mosi!=NULL && self->miso != NULL) || + (self->sck!=NULL && self->mosi!=NULL && miso == mp_const_none) || + (self->sck!=NULL && self->miso!=NULL && mosi == mp_const_none)) { SPIx = mcu_spi_banks[self->sck->spi_index-1]; } else { if (spi_taken) { @@ -129,26 +174,31 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self, GPIO_InitStruct.Alternate = self->sck->altfn_index; HAL_GPIO_Init(pin_port(sck->port), &GPIO_InitStruct); - GPIO_InitStruct.Pin = pin_mask(mosi->number); - GPIO_InitStruct.Mode = GPIO_MODE_AF_PP; - GPIO_InitStruct.Pull = GPIO_NOPULL; - GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_VERY_HIGH; - GPIO_InitStruct.Alternate = self->mosi->altfn_index; - HAL_GPIO_Init(pin_port(mosi->port), &GPIO_InitStruct); + if (self->mosi != NULL) { + GPIO_InitStruct.Pin = pin_mask(mosi->number); + GPIO_InitStruct.Mode = GPIO_MODE_AF_PP; + GPIO_InitStruct.Pull = GPIO_NOPULL; + GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_VERY_HIGH; + GPIO_InitStruct.Alternate = self->mosi->altfn_index; + HAL_GPIO_Init(pin_port(mosi->port), &GPIO_InitStruct); + } - GPIO_InitStruct.Pin = pin_mask(miso->number); - GPIO_InitStruct.Mode = GPIO_MODE_AF_PP; - GPIO_InitStruct.Pull = GPIO_NOPULL; - GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_VERY_HIGH; - GPIO_InitStruct.Alternate = self->miso->altfn_index; - HAL_GPIO_Init(pin_port(miso->port), &GPIO_InitStruct); + if (self->miso != NULL) { + GPIO_InitStruct.Pin = pin_mask(miso->number); + GPIO_InitStruct.Mode = GPIO_MODE_AF_PP; + GPIO_InitStruct.Pull = GPIO_NOPULL; + GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_VERY_HIGH; + GPIO_InitStruct.Alternate = self->miso->altfn_index; + HAL_GPIO_Init(pin_port(miso->port), &GPIO_InitStruct); + } spi_clock_enable(1<<(self->sck->spi_index - 1)); reserved_spi[self->sck->spi_index - 1] = true; self->handle.Instance = SPIx; self->handle.Init.Mode = SPI_MODE_MASTER; - self->handle.Init.Direction = SPI_DIRECTION_2LINES; + // Direction change only required for RX only, see RefMan RM0090:884 + self->handle.Init.Direction = (self->mosi == NULL) ? SPI_CR1_RXONLY : SPI_DIRECTION_2LINES; self->handle.Init.DataSize = SPI_DATASIZE_8BIT; self->handle.Init.CLKPolarity = SPI_POLARITY_LOW; self->handle.Init.CLKPhase = SPI_PHASE_1EDGE; @@ -169,8 +219,8 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self, self->bits = 8; claim_pin(sck); - claim_pin(mosi); - claim_pin(miso); + if (self->mosi != NULL) claim_pin(mosi); + if (self->miso != NULL) claim_pin(miso); } void common_hal_busio_spi_never_reset(busio_spi_obj_t *self) { @@ -178,8 +228,8 @@ void common_hal_busio_spi_never_reset(busio_spi_obj_t *self) { if (mcu_spi_banks[i] == self->handle.Instance) { never_reset_spi[i] = true; never_reset_pin_number(self->sck->pin->port, self->sck->pin->number); - never_reset_pin_number(self->mosi->pin->port, self->mosi->pin->number); - never_reset_pin_number(self->miso->pin->port, self->miso->pin->number); + if (self->mosi != NULL) never_reset_pin_number(self->mosi->pin->port, self->mosi->pin->number); + if (self->miso != NULL) never_reset_pin_number(self->miso->pin->port, self->miso->pin->number); break; } } @@ -195,8 +245,8 @@ void common_hal_busio_spi_deinit(busio_spi_obj_t *self) { never_reset_spi[self->sck->spi_index - 1] = false; reset_pin_number(self->sck->pin->port,self->sck->pin->number); - reset_pin_number(self->mosi->pin->port,self->mosi->pin->number); - reset_pin_number(self->miso->pin->port,self->miso->pin->number); + if (self->mosi != NULL) reset_pin_number(self->mosi->pin->port,self->mosi->pin->number); + if (self->miso != NULL) reset_pin_number(self->miso->pin->port,self->miso->pin->number); self->sck = mp_const_none; self->mosi = mp_const_none; self->miso = mp_const_none; @@ -243,13 +293,6 @@ bool common_hal_busio_spi_configure(busio_spi_obj_t *self, self->handle.Init.BaudRatePrescaler = stm32_baud_to_spi_div(baudrate, &self->prescaler, get_busclock(self->handle.Instance)); - self->handle.Init.Mode = SPI_MODE_MASTER; - self->handle.Init.Direction = SPI_DIRECTION_2LINES; - self->handle.Init.NSS = SPI_NSS_SOFT; - self->handle.Init.FirstBit = SPI_FIRSTBIT_MSB; - self->handle.Init.TIMode = SPI_TIMODE_DISABLE; - self->handle.Init.CRCCalculation = SPI_CRCCALCULATION_DISABLE; - self->handle.Init.CRCPolynomial = 10; if (HAL_SPI_Init(&self->handle) != HAL_OK) { @@ -292,18 +335,22 @@ 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 (self->mosi == NULL) mp_raise_ValueError(translate("No MOSI Pin")); HAL_StatusTypeDef result = HAL_SPI_Transmit (&self->handle, (uint8_t *)data, (uint16_t)len, HAL_MAX_DELAY); return result == HAL_OK; } bool common_hal_busio_spi_read(busio_spi_obj_t *self, uint8_t *data, size_t len, uint8_t write_value) { + if (self->miso == NULL) mp_raise_ValueError(translate("No MISO Pin")); HAL_StatusTypeDef result = HAL_SPI_Receive (&self->handle, data, (uint16_t)len, HAL_MAX_DELAY); return result == HAL_OK; } bool common_hal_busio_spi_transfer(busio_spi_obj_t *self, uint8_t *data_out, uint8_t *data_in, size_t len) { + if (self->miso == NULL) mp_raise_ValueError(translate("No MISO Pin")); + if (self->mosi == NULL) mp_raise_ValueError(translate("No MOSI Pin")); HAL_StatusTypeDef result = HAL_SPI_TransmitReceive (&self->handle, data_out, data_in, (uint16_t)len,HAL_MAX_DELAY); return result == HAL_OK; -- cgit v1.2.3 From d645ea222d121fef0d82be309f917ea856eef030 Mon Sep 17 00:00:00 2001 From: Hierophect Date: Tue, 7 Jan 2020 13:25:24 -0500 Subject: Text fixes for consistency --- ports/stm32f4/common-hal/busio/SPI.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ports/stm32f4/common-hal/busio/SPI.c b/ports/stm32f4/common-hal/busio/SPI.c index fe2041683..d1218c9ab 100644 --- a/ports/stm32f4/common-hal/busio/SPI.c +++ b/ports/stm32f4/common-hal/busio/SPI.c @@ -84,15 +84,15 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self, uint8_t miso_len = sizeof(mcu_spi_miso_list)/sizeof(*mcu_spi_miso_list); bool spi_taken = false; - //sck is not optional. MOSI and MISO are + //SCK is not optional. MOSI and MISO are for(uint i=0; ihandle.Instance = SPIx; self->handle.Init.Mode = SPI_MODE_MASTER; - // Direction change only required for RX only, see RefMan RM0090:884 + // Direction change only required for RX-only, see RefMan RM0090:884 self->handle.Init.Direction = (self->mosi == NULL) ? SPI_CR1_RXONLY : SPI_DIRECTION_2LINES; self->handle.Init.DataSize = SPI_DATASIZE_8BIT; self->handle.Init.CLKPolarity = SPI_POLARITY_LOW; -- cgit v1.2.3 From 3d5528b88f07b3ebb621d2c525bff3cadc3232b4 Mon Sep 17 00:00:00 2001 From: Hierophect Date: Wed, 8 Jan 2020 10:53:53 -0500 Subject: style edits --- ports/stm32f4/common-hal/busio/SPI.c | 65 +++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/ports/stm32f4/common-hal/busio/SPI.c b/ports/stm32f4/common-hal/busio/SPI.c index d1218c9ab..deacedb50 100644 --- a/ports/stm32f4/common-hal/busio/SPI.c +++ b/ports/stm32f4/common-hal/busio/SPI.c @@ -52,10 +52,10 @@ STATIC void spi_clock_disable(uint8_t mask); STATIC uint32_t get_busclock(SPI_TypeDef * instance) { //SPI2 and 3 are on PCLK1, if they exist. #ifdef SPI2 - if(instance == SPI2) return HAL_RCC_GetPCLK1Freq(); + if (instance == SPI2) return HAL_RCC_GetPCLK1Freq(); #endif #ifdef SPI3 - if(instance == SPI3) return HAL_RCC_GetPCLK1Freq(); + if (instance == SPI3) return HAL_RCC_GetPCLK1Freq(); #endif return HAL_RCC_GetPCLK2Freq(); } @@ -85,20 +85,20 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self, bool spi_taken = false; //SCK is not optional. MOSI and MISO are - for(uint i=0; isck!=NULL && self->mosi!=NULL && self->miso != NULL) || + if ( (self->sck!=NULL && self->mosi!=NULL && self->miso != NULL) || (self->sck!=NULL && self->mosi!=NULL && miso == mp_const_none) || (self->sck!=NULL && self->miso!=NULL && mosi == mp_const_none)) { SPIx = mcu_spi_banks[self->sck->spi_index-1]; @@ -219,17 +219,25 @@ void common_hal_busio_spi_construct(busio_spi_obj_t *self, self->bits = 8; claim_pin(sck); - if (self->mosi != NULL) claim_pin(mosi); - if (self->miso != NULL) claim_pin(miso); + if (self->mosi != NULL) { + claim_pin(mosi); + } + if (self->miso != NULL) { + claim_pin(miso); + } } void common_hal_busio_spi_never_reset(busio_spi_obj_t *self) { - for(size_t i = 0 ; i < MP_ARRAY_SIZE(mcu_spi_banks); i++) { + for (size_t i = 0 ; i < MP_ARRAY_SIZE(mcu_spi_banks); i++) { if (mcu_spi_banks[i] == self->handle.Instance) { never_reset_spi[i] = true; never_reset_pin_number(self->sck->pin->port, self->sck->pin->number); - if (self->mosi != NULL) never_reset_pin_number(self->mosi->pin->port, self->mosi->pin->number); - if (self->miso != NULL) never_reset_pin_number(self->miso->pin->port, self->miso->pin->number); + if (self->mosi != NULL) { + never_reset_pin_number(self->mosi->pin->port, self->mosi->pin->number); + } + if (self->miso != NULL) { + never_reset_pin_number(self->miso->pin->port, self->miso->pin->number); + } break; } } @@ -245,8 +253,12 @@ void common_hal_busio_spi_deinit(busio_spi_obj_t *self) { never_reset_spi[self->sck->spi_index - 1] = false; reset_pin_number(self->sck->pin->port,self->sck->pin->number); - if (self->mosi != NULL) reset_pin_number(self->mosi->pin->port,self->mosi->pin->number); - if (self->miso != NULL) reset_pin_number(self->miso->pin->port,self->miso->pin->number); + if (self->mosi != NULL) { + reset_pin_number(self->mosi->pin->port,self->mosi->pin->number); + } + if (self->miso != NULL) { + reset_pin_number(self->miso->pin->port,self->miso->pin->number); + } self->sck = mp_const_none; self->mosi = mp_const_none; self->miso = mp_const_none; @@ -282,7 +294,9 @@ bool common_hal_busio_spi_configure(busio_spi_obj_t *self, uint32_t baudrate, uint8_t polarity, uint8_t phase, uint8_t bits) { //This resets the SPI, so check before updating it redundantly if (baudrate == self->baudrate && polarity== self->polarity - && phase == self->phase && bits == self->bits) return true; + && phase == self->phase && bits == self->bits) { + return true; + } //Deinit SPI HAL_SPI_DeInit(&self->handle); @@ -335,22 +349,27 @@ 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 (self->mosi == NULL) mp_raise_ValueError(translate("No MOSI Pin")); + if (self->mosi == NULL) { + mp_raise_ValueError(translate("No MOSI Pin")); + } HAL_StatusTypeDef result = HAL_SPI_Transmit (&self->handle, (uint8_t *)data, (uint16_t)len, HAL_MAX_DELAY); return result == HAL_OK; } bool common_hal_busio_spi_read(busio_spi_obj_t *self, uint8_t *data, size_t len, uint8_t write_value) { - if (self->miso == NULL) mp_raise_ValueError(translate("No MISO Pin")); + if (self->miso == NULL) { + mp_raise_ValueError(translate("No MISO Pin")); + } HAL_StatusTypeDef result = HAL_SPI_Receive (&self->handle, data, (uint16_t)len, HAL_MAX_DELAY); return result == HAL_OK; } bool common_hal_busio_spi_transfer(busio_spi_obj_t *self, uint8_t *data_out, uint8_t *data_in, size_t len) { - if (self->miso == NULL) mp_raise_ValueError(translate("No MISO Pin")); - if (self->mosi == NULL) mp_raise_ValueError(translate("No MOSI Pin")); + if (self->miso == NULL || self->mosi == NULL) { + mp_raise_ValueError(translate("Missing MISO or MOSI Pin")); + } HAL_StatusTypeDef result = HAL_SPI_TransmitReceive (&self->handle, data_out, data_in, (uint16_t)len,HAL_MAX_DELAY); return result == HAL_OK; -- cgit v1.2.3