Skip to content

Commit 2b2a431

Browse files
robert-hhdpgeorge
authored andcommitted
rp2/machine_uart: Fix unintended UART buffer allocation on init().
The buffer was be reset on every call to uart.init(). If no sizes were given, the buffer was set to the default size 256. That made problems e.g. with PPP. This commit fixes it, keeping the buffer size if not deliberately changed and allocating new buffers only if the size was changed. Signed-off-by: robert-hh <robert@hammelrath.com>
1 parent 35d4d2d commit 2b2a431

1 file changed

Lines changed: 29 additions & 12 deletions

File tree

ports/rp2/machine_uart.c

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,10 @@ typedef struct _machine_uart_obj_t {
116116
uint16_t timeout_char; // timeout waiting between chars (in ms)
117117
uint8_t invert;
118118
uint8_t flow;
119+
uint16_t rxbuf_len;
119120
ringbuf_t read_buffer;
120121
mutex_t *read_mutex;
122+
uint16_t txbuf_len;
121123
ringbuf_t write_buffer;
122124
mutex_t *write_mutex;
123125
uint16_t mp_irq_trigger; // user IRQ trigger mask
@@ -128,10 +130,10 @@ typedef struct _machine_uart_obj_t {
128130
static machine_uart_obj_t machine_uart_obj[] = {
129131
{{&machine_uart_type}, uart0, 0, 0, DEFAULT_UART_BITS, UART_PARITY_NONE, DEFAULT_UART_STOP,
130132
MICROPY_HW_UART0_TX, MICROPY_HW_UART0_RX, MICROPY_HW_UART0_CTS, MICROPY_HW_UART0_RTS,
131-
0, 0, 0, 0, {NULL, 1, 0, 0}, &read_mutex_0, {NULL, 1, 0, 0}, &write_mutex_0, 0, 0, NULL},
133+
0, 0, 0, 0, 0, {NULL, 1, 0, 0}, &read_mutex_0, 0, {NULL, 1, 0, 0}, &write_mutex_0, 0, 0, NULL},
132134
{{&machine_uart_type}, uart1, 1, 0, DEFAULT_UART_BITS, UART_PARITY_NONE, DEFAULT_UART_STOP,
133135
MICROPY_HW_UART1_TX, MICROPY_HW_UART1_RX, MICROPY_HW_UART1_CTS, MICROPY_HW_UART1_RTS,
134-
0, 0, 0, 0, {NULL, 1, 0, 0}, &read_mutex_1, {NULL, 1, 0, 0}, &write_mutex_1},
136+
0, 0, 0, 0, 0, {NULL, 1, 0, 0}, &read_mutex_1, 0, {NULL, 1, 0, 0}, &write_mutex_1, 0, 0, NULL},
135137
};
136138

137139
static const char *_parity_name[] = {"None", "0", "1"};
@@ -252,7 +254,7 @@ static void mp_machine_uart_print(const mp_print_t *print, mp_obj_t self_in, mp_
252254
mp_printf(print, "UART(%u, baudrate=%u, bits=%u, parity=%s, stop=%u, tx=%d, rx=%d, "
253255
"txbuf=%d, rxbuf=%d, timeout=%u, timeout_char=%u, invert=%s, irq=%d)",
254256
self->uart_id, self->baudrate, self->bits, _parity_name[self->parity],
255-
self->stop, self->tx, self->rx, self->write_buffer.size - 1, self->read_buffer.size - 1,
257+
self->stop, self->tx, self->rx, self->txbuf_len, self->rxbuf_len,
256258
self->timeout, self->timeout_char, _invert_name[self->invert], self->mp_irq_trigger);
257259
}
258260

@@ -365,25 +367,33 @@ static void mp_machine_uart_init_helper(machine_uart_obj_t *self, size_t n_args,
365367
}
366368

367369
// Set the RX buffer size if configured.
368-
size_t rxbuf_len = DEFAULT_BUFFER_SIZE;
369370
if (args[ARG_rxbuf].u_int > 0) {
370-
rxbuf_len = args[ARG_rxbuf].u_int;
371+
size_t rxbuf_len = args[ARG_rxbuf].u_int;
371372
if (rxbuf_len < MIN_BUFFER_SIZE) {
372373
rxbuf_len = MIN_BUFFER_SIZE;
373374
} else if (rxbuf_len > MAX_BUFFER_SIZE) {
374375
mp_raise_ValueError(MP_ERROR_TEXT("rxbuf too large"));
375376
}
377+
// Force re-allocting of the buffer if the size changed
378+
if (rxbuf_len != self->rxbuf_len) {
379+
self->read_buffer.buf = NULL;
380+
self->rxbuf_len = rxbuf_len;
381+
}
376382
}
377383

378384
// Set the TX buffer size if configured.
379-
size_t txbuf_len = DEFAULT_BUFFER_SIZE;
380385
if (args[ARG_txbuf].u_int > 0) {
381-
txbuf_len = args[ARG_txbuf].u_int;
386+
size_t txbuf_len = args[ARG_txbuf].u_int;
382387
if (txbuf_len < MIN_BUFFER_SIZE) {
383388
txbuf_len = MIN_BUFFER_SIZE;
384389
} else if (txbuf_len > MAX_BUFFER_SIZE) {
385390
mp_raise_ValueError(MP_ERROR_TEXT("txbuf too large"));
386391
}
392+
// Force re-allocting of the buffer if the size changed
393+
if (txbuf_len != self->txbuf_len) {
394+
self->write_buffer.buf = NULL;
395+
self->txbuf_len = txbuf_len;
396+
}
387397
}
388398

389399
// Initialise the UART peripheral if any arguments given, or it was not initialised previously.
@@ -423,11 +433,14 @@ static void mp_machine_uart_init_helper(machine_uart_obj_t *self, size_t n_args,
423433
uart_set_hw_flow(self->uart, self->flow & UART_HWCONTROL_CTS, self->flow & UART_HWCONTROL_RTS);
424434

425435
// Allocate the RX/TX buffers.
426-
ringbuf_alloc(&(self->read_buffer), rxbuf_len + 1);
427-
MP_STATE_PORT(rp2_uart_rx_buffer[self->uart_id]) = self->read_buffer.buf;
428-
429-
ringbuf_alloc(&(self->write_buffer), txbuf_len + 1);
430-
MP_STATE_PORT(rp2_uart_tx_buffer[self->uart_id]) = self->write_buffer.buf;
436+
if (self->read_buffer.buf == NULL) {
437+
ringbuf_alloc(&(self->read_buffer), self->rxbuf_len + 1);
438+
MP_STATE_PORT(rp2_uart_rx_buffer[self->uart_id]) = self->read_buffer.buf;
439+
}
440+
if (self->write_buffer.buf == NULL) {
441+
ringbuf_alloc(&(self->write_buffer), self->txbuf_len + 1);
442+
MP_STATE_PORT(rp2_uart_tx_buffer[self->uart_id]) = self->write_buffer.buf;
443+
}
431444

432445
// Set the irq handler.
433446
if (self->uart_id == 0) {
@@ -454,6 +467,10 @@ static mp_obj_t mp_machine_uart_make_new(const mp_obj_type_t *type, size_t n_arg
454467

455468
// Get static peripheral object.
456469
machine_uart_obj_t *self = (machine_uart_obj_t *)&machine_uart_obj[uart_id];
470+
self->rxbuf_len = DEFAULT_BUFFER_SIZE;
471+
self->read_buffer.buf = NULL;
472+
self->txbuf_len = DEFAULT_BUFFER_SIZE;
473+
self->write_buffer.buf = NULL;
457474

458475
// Initialise the UART peripheral.
459476
mp_map_t kw_args;

0 commit comments

Comments
 (0)