Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gen2] usart: fix D0 AF unconditionally being changed in hal_usart_end() #2256

Merged
merged 2 commits into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 37 additions & 31 deletions hal/src/stm32f2xx/usart_hal.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ stm32_usart_info_t USART_MAP[HAL_PLATFORM_USART_NUM] = {
* <usart enabled> used internally and does not appear below
* <usart transmitting> used internally and does not appear below
*/
{ USART1, &RCC->APB2ENR, RCC_APB2Periph_USART1, USART1_IRQn, TX, RX, GPIO_PinSource9, GPIO_PinSource10, GPIO_AF_USART1 }, // USART 1
{ USART1, &RCC->APB2ENR, RCC_APB2Periph_USART1, USART1_IRQn, TX, RX, GPIO_PinSource9, GPIO_PinSource10, GPIO_AF_USART1, PIN_INVALID, PIN_INVALID }, // USART 1
{ USART2, &RCC->APB1ENR, RCC_APB1Periph_USART2, USART2_IRQn, RGBG, RGBB, GPIO_PinSource2, GPIO_PinSource3, GPIO_AF_USART2, A7, RGBR } // USART 2
#if PLATFORM_ID == PLATFORM_ELECTRON_PRODUCTION
,{ USART3, &RCC->APB1ENR, RCC_APB1Periph_USART3, USART3_IRQn, TXD_UC, RXD_UC, GPIO_PinSource10, GPIO_PinSource11, GPIO_AF_USART3, CTS_UC, RTS_UC } // USART 3
,{ UART4, &RCC->APB1ENR, RCC_APB1Periph_UART4, UART4_IRQn, C3, C2, GPIO_PinSource10, GPIO_PinSource11, GPIO_AF_UART4 } // UART 4
,{ UART5, &RCC->APB1ENR, RCC_APB1Periph_UART5, UART5_IRQn, C1, C0, GPIO_PinSource12, GPIO_PinSource2, GPIO_AF_UART5 } // UART 5
,{ UART4, &RCC->APB1ENR, RCC_APB1Periph_UART4, UART4_IRQn, C3, C2, GPIO_PinSource10, GPIO_PinSource11, GPIO_AF_UART4, PIN_INVALID, PIN_INVALID } // UART 4
,{ UART5, &RCC->APB1ENR, RCC_APB1Periph_UART5, UART5_IRQn, C1, C0, GPIO_PinSource12, GPIO_PinSource2, GPIO_AF_UART5, PIN_INVALID, PIN_INVALID } // UART 5
#endif // PLATFORM_ID == PLATFORM_ELECTRON_PRODUCTION
};

Expand All @@ -129,10 +129,11 @@ inline void storeChar(uint16_t c, hal_usart_ring_buffer_t *buffer) {

static uint8_t calculateWordLength(uint32_t config, uint8_t noparity);
static uint32_t calculateDataBitsMask(uint32_t config);
static uint8_t validateConfig(uint32_t config);
static bool validateConfig(hal_usart_interface_t serial, uint32_t config);
static void configureTransmitReceive(hal_usart_interface_t serial, uint8_t transmit, uint8_t receive);
static void configurePinsMode(hal_usart_interface_t serial, uint32_t config);
static void usartEndImpl(hal_usart_interface_t serial, bool end);
static bool hardwareFlowControlSupported(hal_usart_interface_t serial);

uint8_t calculateWordLength(uint32_t config, uint8_t noparity) {
// STM32F2 USARTs support only 8-bit or 9-bit communication, however
Expand Down Expand Up @@ -169,24 +170,24 @@ uint32_t calculateDataBitsMask(uint32_t config) {
return (1 << calculateWordLength(config, 1)) - 1;
}

uint8_t validateConfig(uint32_t config) {
bool validateConfig(hal_usart_interface_t serial, uint32_t config) {
// Total word length should be either 8 or 9 bits
if (calculateWordLength(config, 0) == 0) {
return 0;
return false;
}
// Either No, Even or Odd parity
if ((config & SERIAL_PARITY) == (SERIAL_PARITY_EVEN | SERIAL_PARITY_ODD)) {
return 0;
return false;
}
if ((config & SERIAL_HALF_DUPLEX) && (config & LIN_MODE)) {
return 0;
return false;
}

if (config & LIN_MODE) {
// Either Master or Slave mode
// Break detection can still be enabled in both Master and Slave modes
if ((config & LIN_MODE_MASTER) && (config & LIN_MODE_SLAVE)) {
return 0;
return false;
}
switch (config & LIN_BREAK_BITS) {
case LIN_BREAK_13B:
Expand All @@ -195,12 +196,16 @@ uint8_t validateConfig(uint32_t config) {
break;
}
default: {
return 0;
return false;
}
}
}

return 1;
if ((config & SERIAL_FLOW_CONTROL) && !hardwareFlowControlSupported(serial)) {
return false;
}

return true;
}

void configureTransmitReceive(hal_usart_interface_t serial, uint8_t transmit, uint8_t receive) {
Expand Down Expand Up @@ -253,9 +258,13 @@ void configurePinsMode(hal_usart_interface_t serial, uint32_t config) {
}
}

bool hardwareFlowControlSupported(hal_usart_interface_t serial) {
return (usartMap[serial]->rts_pin != PIN_INVALID) && (usartMap[serial]->cts_pin != PIN_INVALID);
}

void usartEndImpl(hal_usart_interface_t serial, bool end) {
Hal_Pin_Info* PIN_MAP = HAL_Pin_Map();
if ((usartMap[serial]->conf.config & SERIAL_FLOW_CONTROL) == SERIAL_FLOW_CONTROL_RTS) {
if (usartMap[serial]->conf.config & SERIAL_FLOW_CONTROL_RTS) {
// nRTS is still under control of USART peripheral, release it.
GPIO_PinAFConfig(PIN_MAP[usartMap[serial]->rts_pin].gpio_peripheral, PIN_MAP[usartMap[serial]->rts_pin].gpio_pin_source, 0/*default after reset*/);
HAL_Pin_Mode(usartMap[serial]->rts_pin, OUTPUT);
Expand Down Expand Up @@ -291,28 +300,19 @@ void usartEndImpl(hal_usart_interface_t serial, bool end) {

GPIO_PinAFConfig(PIN_MAP[usartMap[serial]->rx_pin].gpio_peripheral, usartMap[serial]->rx_pinsource, 0);
GPIO_PinAFConfig(PIN_MAP[usartMap[serial]->tx_pin].gpio_peripheral, usartMap[serial]->tx_pinsource, 0);
GPIO_PinAFConfig(PIN_MAP[usartMap[serial]->cts_pin].gpio_peripheral, PIN_MAP[usartMap[serial]->cts_pin].gpio_pin_source, 0);

// Switch pins to INPUT
HAL_Pin_Mode(usartMap[serial]->rx_pin, INPUT);
HAL_Pin_Mode(usartMap[serial]->tx_pin, end ? INPUT : INPUT_PULLUP);
switch (usartMap[serial]->conf.config & SERIAL_FLOW_CONTROL) {
case SERIAL_FLOW_CONTROL_CTS: {
HAL_Pin_Mode(usartMap[serial]->cts_pin, INPUT);
break;
}
case SERIAL_FLOW_CONTROL_RTS: {
HAL_Pin_Mode(usartMap[serial]->rts_pin, end ? INPUT : INPUT_PULLUP);
break;
}
case SERIAL_FLOW_CONTROL_RTS_CTS: {
HAL_Pin_Mode(usartMap[serial]->rts_pin, end ? INPUT : INPUT_PULLUP);
HAL_Pin_Mode(usartMap[serial]->cts_pin, INPUT);
break;
}
default: {
break;
}

if (usartMap[serial]->conf.config & SERIAL_FLOW_CONTROL_RTS) {
// NOTE: AF setting has already been reset above
HAL_Pin_Mode(usartMap[serial]->rts_pin, end ? INPUT : INPUT_PULLUP);
}

if (usartMap[serial]->conf.config & SERIAL_FLOW_CONTROL_CTS) {
GPIO_PinAFConfig(PIN_MAP[usartMap[serial]->cts_pin].gpio_peripheral, PIN_MAP[usartMap[serial]->cts_pin].gpio_pin_source, 0);
HAL_Pin_Mode(usartMap[serial]->cts_pin, INPUT);
}
}

Expand Down Expand Up @@ -355,7 +355,7 @@ void hal_usart_begin_config(hal_usart_interface_t serial, uint32_t baud, uint32_
return;
}
// Verify UART configuration, exit if it's invalid.
if (!validateConfig(config)) {
if (!validateConfig(serial, config)) {
return;
}

Expand Down Expand Up @@ -562,6 +562,10 @@ void hal_usart_begin_config(hal_usart_interface_t serial, uint32_t baud, uint32_
}

void hal_usart_end(hal_usart_interface_t serial) {
if (usartMap[serial]->state == HAL_USART_STATE_DISABLED) {
// Invalid state
return;
}
usartEndImpl(serial, true);

// clear any received data
Expand All @@ -573,6 +577,8 @@ void hal_usart_end(hal_usart_interface_t serial) {

usartMap[serial]->state = HAL_USART_STATE_DISABLED;
usartMap[serial]->transmitting = false;
usartMap[serial]->conf.baud_rate = 0;
usartMap[serial]->conf.config = 0;
}

uint32_t hal_usart_write(hal_usart_interface_t serial, uint8_t data) {
Expand Down
106 changes: 106 additions & 0 deletions user/tests/wiring/no_fixture/uart.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright (c) 2020 Particle Industries, Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation, either
* version 3 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/

#include "application.h"
#include "unit-test/unit-test.h"
#include "Serial2/Serial2.h"
#include "Serial3/Serial3.h"
#include "Serial4/Serial4.h"
#include "Serial5/Serial5.h"

#if HAL_PLATFORM_GEN == 2

namespace {

uint32_t getPinAf(pin_t pin) {
auto pinmap = HAL_Pin_Map();
auto gpiox = pinmap[pin].gpio_peripheral;
auto pinSource = pinmap[pin].gpio_pin_source;
auto afr = gpiox->AFR[pinSource >> 0x03];
return (afr >> ((pinSource & 0x07) * 4)) & 0xF;
}

} // anomymous

test(UART_00_HardwareFlowControlCannotBeEnabledOnUnsupportedPeripheral) {
// On Gen 2 not supported on Serial1 and Serial4/5
Serial1.end();
Serial1.begin(115200, SERIAL_8N1 | SERIAL_FLOW_CONTROL_RTS_CTS);
assertFalse(Serial1.isEnabled());

Serial1.begin(115200, SERIAL_8N1);
assertTrue(Serial1.isEnabled());
Serial1.end();

#if Wiring_Serial4
Serial4.end();
Serial4.begin(115200, SERIAL_8N1 | SERIAL_FLOW_CONTROL_RTS_CTS);
assertFalse(Serial4.isEnabled());

Serial4.begin(115200, SERIAL_8N1);
assertTrue(Serial4.isEnabled());
Serial4.end();
#endif // Wiring_Serial4

#if Wiring_Serial5
Serial5.end();
Serial5.begin(115200, SERIAL_8N1 | SERIAL_FLOW_CONTROL_RTS_CTS);
assertFalse(Serial5.isEnabled());

Serial5.begin(115200, SERIAL_8N1);
assertTrue(Serial5.isEnabled());
Serial5.end();
#endif // Wiring_Serial5
}

test(UART_01_D0ConfigurationIsIntactWhenSerial1IsDeinitialized) {
Serial1.end();

Serial1.begin(115200, SERIAL_8N1);
assertTrue(Serial1.isEnabled());

Wire.end();
Wire.begin();
assertTrue(Wire.isEnabled());
assertEqual(getPinAf(D0), (uint32_t)GPIO_AF_I2C1);

Serial1.end();
assertEqual(getPinAf(D0), (uint32_t)GPIO_AF_I2C1);
Wire.end();
}

#endif // HAL_PLATFORM_GEN == 2

test(UART_02_PinConfigurationNotAffectedWhenEndCalledMultipleTimes) {
Serial1.begin(115200, SERIAL_8N1);
assertTrue(Serial1.isEnabled());
Serial1.end();
assertFalse(Serial1.isEnabled());

pinMode(TX, INPUT_PULLDOWN);
pinMode(RX, INPUT_PULLDOWN);

assertEqual(getPinMode(TX), INPUT_PULLDOWN);
assertEqual(getPinMode(RX), INPUT_PULLDOWN);

Serial1.end();
assertEqual(getPinMode(TX), INPUT_PULLDOWN);
assertEqual(getPinMode(RX), INPUT_PULLDOWN);

pinMode(TX, INPUT);
pinMode(RX, INPUT);
}