Skip to content

Commit

Permalink
Merge pull request #7616 from dhalbert/8.0.x-fix-atmel-uart
Browse files Browse the repository at this point in the history 
Fix pad assignments on atmel-samd UART
  • Loading branch information
@dhalbert
dhalbert committed Feb 22, 2023
2 parents 9b6abea + 2684aeb commit 460dda0
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 67 deletions.
6 changes: 0 additions & 6 deletions locale/circuitpython.pot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1987,10 +1987,6 @@ msgstr ""
msgid "Stopping AP is not supported."
msgstr ""

#: ports/mimxrt10xx/common-hal/busio/UART.c ports/stm/common-hal/busio/UART.c
msgid "Supply at least one UART pin"
msgstr ""

#: shared-bindings/alarm/time/TimeAlarm.c
msgid "Supply one of monotonic_time or epoch_time"
msgstr ""
Expand Down Expand Up @@ -4116,8 +4112,6 @@ msgstr ""
msgid "twai_start returned esp-idf error #%d"
msgstr ""

#: ports/atmel-samd/common-hal/busio/UART.c
#: ports/espressif/common-hal/busio/UART.c ports/nrf/common-hal/busio/UART.c
#: shared-bindings/busio/UART.c shared-bindings/canio/CAN.c
msgid "tx and rx cannot both be None"
msgstr ""
Expand Down
135 changes: 92 additions & 43 deletions ports/atmel-samd/common-hal/busio/UART.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ static void usart_async_rxc_callback(const struct usart_async_descriptor *const
// Nothing needs to be done by us.
}

// shared-bindings validates that the tx and rx are not both missing,
// and that the pins are distinct.
void common_hal_busio_uart_construct(busio_uart_obj_t *self,
const mcu_pin_obj_t *tx, const mcu_pin_obj_t *rx,
const mcu_pin_obj_t *rts, const mcu_pin_obj_t *cts,
Expand Down Expand Up @@ -92,10 +94,6 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
bool have_rts = rts != NULL;
bool have_cts = cts != NULL;

if (!have_tx && !have_rx) {
mp_raise_ValueError(translate("tx and rx cannot both be None"));
}

if (have_rx && receiver_buffer_size > 0 && (receiver_buffer_size & (receiver_buffer_size - 1)) != 0) {
mp_raise_ValueError_varg(translate("%q must be power of 2"), MP_QSTR_receiver_buffer_size);
}
Expand All @@ -107,6 +105,20 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
// This assignment is only here because the usart_async routines take a *const argument.
struct usart_async_descriptor *const usart_desc_p = (struct usart_async_descriptor *const)&self->usart_desc;

// Allowed pads for USART. See the SAMD21 and SAMx5x datasheets.
// TXPO:
// (both) 0x0: TX pad 0; no RTS/CTS
// (SAMD21) 0x1: TX pad 2; no RTS/CTS
// (SAMx5x) 0x1: reserved
// (both) 0x2: TX pad 0; RTS: pad 2, CTS: pad 3
// (SAMD21) 0x3: reserved
// (SAMx5x) 0x3: TX pad 0; RTS: pad 2; no CTS
// RXPO:
// 0x0: RX pad 0
// 0x1: RX pad 1
// 0x2: RX pad 2
// 0x3: RX pad 3

for (int i = 0; i < NUM_SERCOMS_PER_PIN; i++) {
Sercom *potential_sercom = NULL;
if (have_tx) {
Expand All @@ -115,29 +127,71 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
continue;
}
potential_sercom = sercom_insts[sercom_index];

// SAMD21 and SAMx5x have different requirements.

#ifdef SAMD21
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0 ||
!(tx->sercom[i].pad == 0 ||
tx->sercom[i].pad == 2)) {
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0) {
// In use.
continue;
}
if (tx->sercom[i].pad != 0 &&
tx->sercom[i].pad != 2) {
// TX must be on pad 0 or 2.
continue;
}
if (have_rts) {
if (rts->sercom[i].pad != 2 ||
tx->sercom[i].pad == 2) {
// RTS pin must be on pad 2, so if TX is also on pad 2, not possible
continue;
}
}
if (have_cts) {
if (cts->sercom[i].pad != 3 ||
(have_rx && rx->sercom[i].pad == 3)) {
// CTS pin must be on pad 3, so if RX is also on pad 3, not possible
continue;
}
}
#endif

#ifdef SAM_D5X_E5X
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0 ||
!(tx->sercom[i].pad == 0)) {
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0) {
// In use.
continue;
}
if (tx->sercom[i].pad != 0) {
// TX must be on pad 0
continue;
}

if (have_rts && rts->sercom[i].pad != 2) {
// RTS pin must be on pad 2
continue;
}
if (have_cts) {
if (cts->sercom[i].pad != 3 ||
(have_rx && rx->sercom[i].pad == 3)) {
// CTS pin must be on pad 3, so if RX is also on pad 3, not possible
continue;
}
}
#endif

tx_pinmux = PINMUX(tx->number, (i == 0) ? MUX_C : MUX_D);
tx_pad = tx->sercom[i].pad;
if (have_rts) {
rts_pinmux = PINMUX(rts->number, (i == 0) ? MUX_C : MUX_D);
}
if (rx == NULL) {
if (!have_rx) {
// TX only, so don't need to look further.
sercom = potential_sercom;
break;
}
}

// Have TX, now look for RX match. We know have_rx is true at this point.
for (int j = 0; j < NUM_SERCOMS_PER_PIN; j++) {
if (((!have_tx && rx->sercom[j].index < SERCOM_INST_NUM &&
sercom_insts[rx->sercom[j].index]->USART.CTRLA.bit.ENABLE == 0) ||
Expand All @@ -160,20 +214,10 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
if (sercom == NULL) {
raise_ValueError_invalid_pins();
}
if (!have_tx) {
tx_pad = 0;
if (rx_pad == 0) {
tx_pad = 2;
}
}
if (!have_rx) {
rx_pad = (tx_pad + 1) % 4;
}

// Set up clocks on SERCOM.
samd_peripherals_sercom_clock_init(sercom, sercom_index);

if (rx && receiver_buffer_size > 0) {
if (have_rx && receiver_buffer_size > 0) {
self->buffer_length = receiver_buffer_size;
if (NULL != receiver_buffer) {
self->buffer = receiver_buffer;
Expand Down Expand Up @@ -204,36 +248,41 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
// which don't necessarily match what we need. After calling it, set the values
// specific to this instantiation of UART.

// Set pads computed for this SERCOM. Refer to the datasheet for details on pads.
// TXPO:
// 0x0: TX pad 0; no RTS/CTS
// 0x1: reserved
// 0x2: TX pad 0; RTS: pad 2, CTS: pad 3
// 0x3: TX pad 0; RTS: pad 2; no CTS
// RXPO:
// 0x0: RX pad 0
// 0x1: RX pad 1
// 0x2: RX pad 2
// 0x3: RX pad 3
// See the TXPO/RXPO table above for how RXPO and TXPO are chosen below.

// Default to TXPO with no RTS/CTS
uint8_t computed_txpo = 0;
// If we have both CTS (with or without RTS), use second pinout
if (have_cts) {
computed_txpo = 2;
}
// If we have RTS only, use the third pinout
if (have_rts && !have_cts) {
computed_txpo = 3;
// rxpo maps directly to rx_pad.
// Set to 0x0 if no RX, but it doesn't matter because RX will not be enabled.
const uint8_t rxpo = have_rx ? rx_pad : 0x0;

#ifdef SAMD21
// SAMD21 has only one txpo value when using either CTS or RTS or both.
// TX is on pad 0 or 2, or there is no TX.
// 0x0 for pad 0, 0x1 for pad 2.
uint8_t txpo;
if (tx_pad == 2) {
txpo = 0x1;
} else {
txpo = (have_cts || have_rts) ? 0x2 : 0x0;
}
#endif

#ifdef SAM_D5X_E5X
// SAMx5x has two different possibilities, per the chart above.
// We already know TX is on pad 0, or there is no TX.

// Without RTS or CTS, txpo can be 0x0.
// It's not clear if 0x2 would cover all our cases, but this is known to be safe.
uint8_t txpo = (have_rts || have_cts) ? 0x2: 0x0;
#endif

// Doing a group mask and set of the registers saves 60 bytes over setting the bitfields individually.

sercom->USART.CTRLA.reg &= ~(SERCOM_USART_CTRLA_TXPO_Msk |
SERCOM_USART_CTRLA_RXPO_Msk |
SERCOM_USART_CTRLA_FORM_Msk);
sercom->USART.CTRLA.reg |= SERCOM_USART_CTRLA_TXPO(computed_txpo) |
SERCOM_USART_CTRLA_RXPO(rx_pad) |
// See chart above for TXPO values and RXPO values.
sercom->USART.CTRLA.reg |= SERCOM_USART_CTRLA_TXPO(txpo) |
SERCOM_USART_CTRLA_RXPO(rxpo) |
(parity == BUSIO_UART_PARITY_NONE ? 0 : SERCOM_USART_CTRLA_FORM(1));

// Enable tx and/or rx based on whether the pins were specified.
Expand Down
5 changes: 2 additions & 3 deletions ports/espressif/common-hal/busio/UART.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,8 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,

uart_config_t uart_config = {0};
bool have_rs485_dir = rs485_dir != NULL;
if (!have_tx && !have_rx) {
mp_raise_ValueError(translate("tx and rx cannot both be None"));
}

// shared-bindings checks that TX and RX are not both None, so we don't need to check here.

// Filter for sane settings for RS485
if (have_rs485_dir) {
Expand Down
3 changes: 2 additions & 1 deletion ports/mimxrt10xx/common-hal/busio/UART.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
break;
}
} else {
mp_raise_ValueError(translate("Supply at least one UART pin"));
// TX and RX are both None. But this is already handled in shared-bindings, so
// we won't get here.
}

if (rx && !rx_config) {
Expand Down
4 changes: 1 addition & 3 deletions ports/nrf/common-hal/busio/UART.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
mp_raise_ValueError(translate("All UART peripherals are in use"));
}

if ((tx == NULL) && (rx == NULL)) {
mp_raise_ValueError(translate("tx and rx cannot both be None"));
}
// shared-bindings checks that TX and RX are not both None, so we don't need to check here.

mp_arg_validate_int_min(receiver_buffer_size, 1, MP_QSTR_receiver_buffer_size);

Expand Down
6 changes: 3 additions & 3 deletions ports/stm/common-hal/busio/UART.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
bool sigint_enabled) {

// match pins to UART objects
USART_TypeDef *USARTx;
USART_TypeDef *USARTx = NULL;

uint8_t tx_len = MP_ARRAY_SIZE(mcu_uart_tx_list);
uint8_t rx_len = MP_ARRAY_SIZE(mcu_uart_rx_list);
Expand Down Expand Up @@ -159,8 +159,8 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
USARTx = assign_uart_or_throw(self, (self->tx != NULL),
periph_index, uart_taken);
} else {
// both pins cannot be empty
mp_raise_ValueError(translate("Supply at least one UART pin"));
// TX and RX are both None. But this is already handled in shared-bindings, so
// we won't get here.
}

// Other errors
Expand Down
28 changes: 20 additions & 8 deletions shared-bindings/busio/UART.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,13 @@
//|
//| def __init__(
//| self,
//| tx: microcontroller.Pin,
//| rx: microcontroller.Pin,
//| tx: Optional[microcontroller.Pin] = None,
//| rx: Optional[microcontroller.Pin] = None,
//| *,
//| rts: Optional[microcontroller.Pin] = None,
//| cts: Optional[microcontroller.Pin] = None,
//| rs485_dir: Optional[microcontroller.Pin] = None,
//| rs485_invert: bool = False,
//| baudrate: int = 9600,
//| bits: int = 8,
//| parity: Optional[Parity] = None,
Expand All @@ -74,11 +78,13 @@
//| :param float timeout: the timeout in seconds to wait for the first character and between subsequent characters when reading. Raises ``ValueError`` if timeout >100 seconds.
//| :param int receiver_buffer_size: the character length of the read buffer (0 to disable). (When a character is 9 bits the buffer will be 2 * receiver_buffer_size bytes.)
//|
//| ``tx`` and ``rx`` cannot both be ``None``.
//|
//| *New in CircuitPython 4.0:* ``timeout`` has incompatibly changed units from milliseconds to seconds.
//| The new upper limit on ``timeout`` is meant to catch mistaken use of milliseconds.
//|
//| **Limitations:** RS485 is not supported on SAMD, nRF, Broadcom, Spresense, or STM.
//| On i.MX and Raspberry Pi RP2040 support is implemented in software:
//| On i.MX and Raspberry Pi RP2040, RS485 support is implemented in software:
//| The timing for the ``rs485_dir`` pin signal is done on a best-effort basis, and may not meet
//| RS485 specifications intermittently.
//| """
Expand Down Expand Up @@ -118,11 +124,21 @@ STATIC mp_obj_t busio_uart_make_new(const mp_obj_type_t *type, size_t n_args, si

const mcu_pin_obj_t *rx = validate_obj_is_free_pin_or_none(args[ARG_rx].u_obj, MP_QSTR_rx);
const mcu_pin_obj_t *tx = validate_obj_is_free_pin_or_none(args[ARG_tx].u_obj, MP_QSTR_tx);

const mcu_pin_obj_t *rts = validate_obj_is_free_pin_or_none(args[ARG_rts].u_obj, MP_QSTR_rts);
const mcu_pin_obj_t *cts = validate_obj_is_free_pin_or_none(args[ARG_cts].u_obj, MP_QSTR_cts);
const mcu_pin_obj_t *rs485_dir = validate_obj_is_free_pin_or_none(args[ARG_rs485_dir].u_obj, MP_QSTR_rs485_dir);
if ((tx == NULL) && (rx == NULL)) {
mp_raise_ValueError(translate("tx and rx cannot both be None"));
}

// Pins must be distinct.
if ((tx != NULL && (tx == rx || tx == rts || tx == cts || tx == rs485_dir)) ||
(rx != NULL && (rx == rts || rx == cts || rx == rs485_dir)) ||
(rts != NULL && (rts == cts || rts == rs485_dir)) ||
(cts != NULL && (cts == rs485_dir))) {
raise_ValueError_invalid_pins();
}

uint8_t bits = (uint8_t)mp_arg_validate_int_range(args[ARG_bits].u_int, 5, 9, MP_QSTR_bits);

busio_uart_parity_t parity = BUSIO_UART_PARITY_NONE;
Expand All @@ -137,10 +153,6 @@ STATIC mp_obj_t busio_uart_make_new(const mp_obj_type_t *type, size_t n_args, si
mp_float_t timeout = mp_obj_get_float(args[ARG_timeout].u_obj);
validate_timeout(timeout);

const mcu_pin_obj_t *rts = validate_obj_is_free_pin_or_none(args[ARG_rts].u_obj, MP_QSTR_rts);
const mcu_pin_obj_t *cts = validate_obj_is_free_pin_or_none(args[ARG_cts].u_obj, MP_QSTR_cts);
const mcu_pin_obj_t *rs485_dir = validate_obj_is_free_pin_or_none(args[ARG_rs485_dir].u_obj, MP_QSTR_rs485_dir);

const bool rs485_invert = args[ARG_rs485_invert].u_bool;

// Always initially allocate the UART object within the long-lived heap.
Expand Down

0 comments on commit 460dda0

Please sign in to comment.