Skip to content

Commit 460dda0

Browse files
dhalbertdhalbert
authored
Merge pull request adafruit#7616 from dhalbert/8.0.x-fix-atmel-uart
Fix pad assignments on atmel-samd UART
2 parents 9b6abea + 2684aeb commit 460dda0

File tree

7 files changed

+120
-67
lines changed

7 files changed

+120
-67
lines changed

‎locale/circuitpython.pot

-6
Original file line numberDiff line numberDiff line change
@@ -1987,10 +1987,6 @@ msgstr ""
19871987
msgid "Stopping AP is not supported."
19881988
msgstr ""
19891989

1990-
#: ports/mimxrt10xx/common-hal/busio/UART.c ports/stm/common-hal/busio/UART.c
1991-
msgid "Supply at least one UART pin"
1992-
msgstr ""
1993-
19941990
#: shared-bindings/alarm/time/TimeAlarm.c
19951991
msgid "Supply one of monotonic_time or epoch_time"
19961992
msgstr ""
@@ -4116,8 +4112,6 @@ msgstr ""
41164112
msgid "twai_start returned esp-idf error #%d"
41174113
msgstr ""
41184114

4119-
#: ports/atmel-samd/common-hal/busio/UART.c
4120-
#: ports/espressif/common-hal/busio/UART.c ports/nrf/common-hal/busio/UART.c
41214115
#: shared-bindings/busio/UART.c shared-bindings/canio/CAN.c
41224116
msgid "tx and rx cannot both be None"
41234117
msgstr ""

‎ports/atmel-samd/common-hal/busio/UART.c

+92-43
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ static void usart_async_rxc_callback(const struct usart_async_descriptor *const
5858
// Nothing needs to be done by us.
5959
}
6060

61+
// shared-bindings validates that the tx and rx are not both missing,
62+
// and that the pins are distinct.
6163
void common_hal_busio_uart_construct(busio_uart_obj_t *self,
6264
const mcu_pin_obj_t *tx, const mcu_pin_obj_t *rx,
6365
const mcu_pin_obj_t *rts, const mcu_pin_obj_t *cts,
@@ -92,10 +94,6 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
9294
bool have_rts = rts != NULL;
9395
bool have_cts = cts != NULL;
9496

95-
if (!have_tx && !have_rx) {
96-
mp_raise_ValueError(translate("tx and rx cannot both be None"));
97-
}
98-
9997
if (have_rx && receiver_buffer_size > 0 && (receiver_buffer_size & (receiver_buffer_size - 1)) != 0) {
10098
mp_raise_ValueError_varg(translate("%q must be power of 2"), MP_QSTR_receiver_buffer_size);
10199
}
@@ -107,6 +105,20 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
107105
// This assignment is only here because the usart_async routines take a *const argument.
108106
struct usart_async_descriptor *const usart_desc_p = (struct usart_async_descriptor *const)&self->usart_desc;
109107

108+
// Allowed pads for USART. See the SAMD21 and SAMx5x datasheets.
109+
// TXPO:
110+
// (both) 0x0: TX pad 0; no RTS/CTS
111+
// (SAMD21) 0x1: TX pad 2; no RTS/CTS
112+
// (SAMx5x) 0x1: reserved
113+
// (both) 0x2: TX pad 0; RTS: pad 2, CTS: pad 3
114+
// (SAMD21) 0x3: reserved
115+
// (SAMx5x) 0x3: TX pad 0; RTS: pad 2; no CTS
116+
// RXPO:
117+
// 0x0: RX pad 0
118+
// 0x1: RX pad 1
119+
// 0x2: RX pad 2
120+
// 0x3: RX pad 3
121+
110122
for (int i = 0; i < NUM_SERCOMS_PER_PIN; i++) {
111123
Sercom *potential_sercom = NULL;
112124
if (have_tx) {
@@ -115,29 +127,71 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
115127
continue;
116128
}
117129
potential_sercom = sercom_insts[sercom_index];
130+
131+
// SAMD21 and SAMx5x have different requirements.
132+
118133
#ifdef SAMD21
119-
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0 ||
120-
!(tx->sercom[i].pad == 0 ||
121-
tx->sercom[i].pad == 2)) {
134+
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0) {
135+
// In use.
122136
continue;
123137
}
138+
if (tx->sercom[i].pad != 0 &&
139+
tx->sercom[i].pad != 2) {
140+
// TX must be on pad 0 or 2.
141+
continue;
142+
}
143+
if (have_rts) {
144+
if (rts->sercom[i].pad != 2 ||
145+
tx->sercom[i].pad == 2) {
146+
// RTS pin must be on pad 2, so if TX is also on pad 2, not possible
147+
continue;
148+
}
149+
}
150+
if (have_cts) {
151+
if (cts->sercom[i].pad != 3 ||
152+
(have_rx && rx->sercom[i].pad == 3)) {
153+
// CTS pin must be on pad 3, so if RX is also on pad 3, not possible
154+
continue;
155+
}
156+
}
124157
#endif
158+
125159
#ifdef SAM_D5X_E5X
126-
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0 ||
127-
!(tx->sercom[i].pad == 0)) {
160+
if (potential_sercom->USART.CTRLA.bit.ENABLE != 0) {
161+
// In use.
162+
continue;
163+
}
164+
if (tx->sercom[i].pad != 0) {
165+
// TX must be on pad 0
166+
continue;
167+
}
168+
169+
if (have_rts && rts->sercom[i].pad != 2) {
170+
// RTS pin must be on pad 2
128171
continue;
129172
}
173+
if (have_cts) {
174+
if (cts->sercom[i].pad != 3 ||
175+
(have_rx && rx->sercom[i].pad == 3)) {
176+
// CTS pin must be on pad 3, so if RX is also on pad 3, not possible
177+
continue;
178+
}
179+
}
130180
#endif
181+
131182
tx_pinmux = PINMUX(tx->number, (i == 0) ? MUX_C : MUX_D);
132183
tx_pad = tx->sercom[i].pad;
133184
if (have_rts) {
134185
rts_pinmux = PINMUX(rts->number, (i == 0) ? MUX_C : MUX_D);
135186
}
136-
if (rx == NULL) {
187+
if (!have_rx) {
188+
// TX only, so don't need to look further.
137189
sercom = potential_sercom;
138190
break;
139191
}
140192
}
193+
194+
// Have TX, now look for RX match. We know have_rx is true at this point.
141195
for (int j = 0; j < NUM_SERCOMS_PER_PIN; j++) {
142196
if (((!have_tx && rx->sercom[j].index < SERCOM_INST_NUM &&
143197
sercom_insts[rx->sercom[j].index]->USART.CTRLA.bit.ENABLE == 0) ||
@@ -160,20 +214,10 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
160214
if (sercom == NULL) {
161215
raise_ValueError_invalid_pins();
162216
}
163-
if (!have_tx) {
164-
tx_pad = 0;
165-
if (rx_pad == 0) {
166-
tx_pad = 2;
167-
}
168-
}
169-
if (!have_rx) {
170-
rx_pad = (tx_pad + 1) % 4;
171-
}
172-
173217
// Set up clocks on SERCOM.
174218
samd_peripherals_sercom_clock_init(sercom, sercom_index);
175219

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

207-
// Set pads computed for this SERCOM. Refer to the datasheet for details on pads.
208-
// TXPO:
209-
// 0x0: TX pad 0; no RTS/CTS
210-
// 0x1: reserved
211-
// 0x2: TX pad 0; RTS: pad 2, CTS: pad 3
212-
// 0x3: TX pad 0; RTS: pad 2; no CTS
213-
// RXPO:
214-
// 0x0: RX pad 0
215-
// 0x1: RX pad 1
216-
// 0x2: RX pad 2
217-
// 0x3: RX pad 3
251+
// See the TXPO/RXPO table above for how RXPO and TXPO are chosen below.
218252

219-
// Default to TXPO with no RTS/CTS
220-
uint8_t computed_txpo = 0;
221-
// If we have both CTS (with or without RTS), use second pinout
222-
if (have_cts) {
223-
computed_txpo = 2;
224-
}
225-
// If we have RTS only, use the third pinout
226-
if (have_rts && !have_cts) {
227-
computed_txpo = 3;
253+
// rxpo maps directly to rx_pad.
254+
// Set to 0x0 if no RX, but it doesn't matter because RX will not be enabled.
255+
const uint8_t rxpo = have_rx ? rx_pad : 0x0;
256+
257+
#ifdef SAMD21
258+
// SAMD21 has only one txpo value when using either CTS or RTS or both.
259+
// TX is on pad 0 or 2, or there is no TX.
260+
// 0x0 for pad 0, 0x1 for pad 2.
261+
uint8_t txpo;
262+
if (tx_pad == 2) {
263+
txpo = 0x1;
264+
} else {
265+
txpo = (have_cts || have_rts) ? 0x2 : 0x0;
228266
}
267+
#endif
268+
269+
#ifdef SAM_D5X_E5X
270+
// SAMx5x has two different possibilities, per the chart above.
271+
// We already know TX is on pad 0, or there is no TX.
272+
273+
// Without RTS or CTS, txpo can be 0x0.
274+
// It's not clear if 0x2 would cover all our cases, but this is known to be safe.
275+
uint8_t txpo = (have_rts || have_cts) ? 0x2: 0x0;
276+
#endif
229277

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

232280
sercom->USART.CTRLA.reg &= ~(SERCOM_USART_CTRLA_TXPO_Msk |
233281
SERCOM_USART_CTRLA_RXPO_Msk |
234282
SERCOM_USART_CTRLA_FORM_Msk);
235-
sercom->USART.CTRLA.reg |= SERCOM_USART_CTRLA_TXPO(computed_txpo) |
236-
SERCOM_USART_CTRLA_RXPO(rx_pad) |
283+
// See chart above for TXPO values and RXPO values.
284+
sercom->USART.CTRLA.reg |= SERCOM_USART_CTRLA_TXPO(txpo) |
285+
SERCOM_USART_CTRLA_RXPO(rxpo) |
237286
(parity == BUSIO_UART_PARITY_NONE ? 0 : SERCOM_USART_CTRLA_FORM(1));
238287

239288
// Enable tx and/or rx based on whether the pins were specified.

‎ports/espressif/common-hal/busio/UART.c

+2-3
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,8 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
112112

113113
uart_config_t uart_config = {0};
114114
bool have_rs485_dir = rs485_dir != NULL;
115-
if (!have_tx && !have_rx) {
116-
mp_raise_ValueError(translate("tx and rx cannot both be None"));
117-
}
115+
116+
// shared-bindings checks that TX and RX are not both None, so we don't need to check here.
118117

119118
// Filter for sane settings for RS485
120119
if (have_rs485_dir) {

‎ports/mimxrt10xx/common-hal/busio/UART.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,8 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
179179
break;
180180
}
181181
} else {
182-
mp_raise_ValueError(translate("Supply at least one UART pin"));
182+
// TX and RX are both None. But this is already handled in shared-bindings, so
183+
// we won't get here.
183184
}
184185

185186
if (rx && !rx_config) {

‎ports/nrf/common-hal/busio/UART.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,7 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
183183
mp_raise_ValueError(translate("All UART peripherals are in use"));
184184
}
185185

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

190188
mp_arg_validate_int_min(receiver_buffer_size, 1, MP_QSTR_receiver_buffer_size);
191189

‎ports/stm/common-hal/busio/UART.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ void common_hal_busio_uart_construct(busio_uart_obj_t *self,
8585
bool sigint_enabled) {
8686

8787
// match pins to UART objects
88-
USART_TypeDef *USARTx;
88+
USART_TypeDef *USARTx = NULL;
8989

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

166166
// Other errors

‎shared-bindings/busio/UART.c

+20-8
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,13 @@
4848
//|
4949
//| def __init__(
5050
//| self,
51-
//| tx: microcontroller.Pin,
52-
//| rx: microcontroller.Pin,
51+
//| tx: Optional[microcontroller.Pin] = None,
52+
//| rx: Optional[microcontroller.Pin] = None,
5353
//| *,
54+
//| rts: Optional[microcontroller.Pin] = None,
55+
//| cts: Optional[microcontroller.Pin] = None,
56+
//| rs485_dir: Optional[microcontroller.Pin] = None,
57+
//| rs485_invert: bool = False,
5458
//| baudrate: int = 9600,
5559
//| bits: int = 8,
5660
//| parity: Optional[Parity] = None,
@@ -74,11 +78,13 @@
7478
//| :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.
7579
//| :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.)
7680
//|
81+
//| ``tx`` and ``rx`` cannot both be ``None``.
82+
//|
7783
//| *New in CircuitPython 4.0:* ``timeout`` has incompatibly changed units from milliseconds to seconds.
7884
//| The new upper limit on ``timeout`` is meant to catch mistaken use of milliseconds.
7985
//|
8086
//| **Limitations:** RS485 is not supported on SAMD, nRF, Broadcom, Spresense, or STM.
81-
//| On i.MX and Raspberry Pi RP2040 support is implemented in software:
87+
//| On i.MX and Raspberry Pi RP2040, RS485 support is implemented in software:
8288
//| The timing for the ``rs485_dir`` pin signal is done on a best-effort basis, and may not meet
8389
//| RS485 specifications intermittently.
8490
//| """
@@ -118,11 +124,21 @@ STATIC mp_obj_t busio_uart_make_new(const mp_obj_type_t *type, size_t n_args, si
118124

119125
const mcu_pin_obj_t *rx = validate_obj_is_free_pin_or_none(args[ARG_rx].u_obj, MP_QSTR_rx);
120126
const mcu_pin_obj_t *tx = validate_obj_is_free_pin_or_none(args[ARG_tx].u_obj, MP_QSTR_tx);
121-
127+
const mcu_pin_obj_t *rts = validate_obj_is_free_pin_or_none(args[ARG_rts].u_obj, MP_QSTR_rts);
128+
const mcu_pin_obj_t *cts = validate_obj_is_free_pin_or_none(args[ARG_cts].u_obj, MP_QSTR_cts);
129+
const mcu_pin_obj_t *rs485_dir = validate_obj_is_free_pin_or_none(args[ARG_rs485_dir].u_obj, MP_QSTR_rs485_dir);
122130
if ((tx == NULL) && (rx == NULL)) {
123131
mp_raise_ValueError(translate("tx and rx cannot both be None"));
124132
}
125133

134+
// Pins must be distinct.
135+
if ((tx != NULL && (tx == rx || tx == rts || tx == cts || tx == rs485_dir)) ||
136+
(rx != NULL && (rx == rts || rx == cts || rx == rs485_dir)) ||
137+
(rts != NULL && (rts == cts || rts == rs485_dir)) ||
138+
(cts != NULL && (cts == rs485_dir))) {
139+
raise_ValueError_invalid_pins();
140+
}
141+
126142
uint8_t bits = (uint8_t)mp_arg_validate_int_range(args[ARG_bits].u_int, 5, 9, MP_QSTR_bits);
127143

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

140-
const mcu_pin_obj_t *rts = validate_obj_is_free_pin_or_none(args[ARG_rts].u_obj, MP_QSTR_rts);
141-
const mcu_pin_obj_t *cts = validate_obj_is_free_pin_or_none(args[ARG_cts].u_obj, MP_QSTR_cts);
142-
const mcu_pin_obj_t *rs485_dir = validate_obj_is_free_pin_or_none(args[ARG_rs485_dir].u_obj, MP_QSTR_rs485_dir);
143-
144156
const bool rs485_invert = args[ARG_rs485_invert].u_bool;
145157

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

0 commit comments

Comments
 (0)