From 3fde87e0a871ea88278d437703743d4c2c3bab26 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Wed, 10 Sep 2025 09:27:27 -0700 Subject: [PATCH] cleanup ringbuffer handling --- board/drivers/can_common.h | 41 +++--------------- board/drivers/ringbuffer.h | 89 ++++++++++++++++++++++++++++++++++++++ board/drivers/uart.h | 60 +++++-------------------- board/stm32h7/lluart.h | 19 +++----- 4 files changed, 112 insertions(+), 97 deletions(-) create mode 100644 board/drivers/ringbuffer.h diff --git a/board/drivers/can_common.h b/board/drivers/can_common.h index 23bc90f33cd..f70233e3e77 100644 --- a/board/drivers/can_common.h +++ b/board/drivers/can_common.h @@ -1,4 +1,5 @@ #include "can_common_declarations.h" +#include "ringbuffer.h" uint32_t safety_tx_blocked = 0; uint32_t safety_rx_invalid = 0; @@ -43,39 +44,11 @@ can_ring *can_queues[CAN_QUEUES_ARRAY_SIZE] = {&can_tx1_q, &can_tx2_q, &can_tx3_ // ********************* interrupt safe queue ********************* bool can_pop(can_ring *q, CANPacket_t *elem) { - bool ret = 0; - - ENTER_CRITICAL(); - if (q->w_ptr != q->r_ptr) { - *elem = q->elems[q->r_ptr]; - if ((q->r_ptr + 1U) == q->fifo_size) { - q->r_ptr = 0; - } else { - q->r_ptr += 1U; - } - ret = 1; - } - EXIT_CRITICAL(); - - return ret; + return rb_pop_u32(q->elems, sizeof(CANPacket_t), &q->w_ptr, &q->r_ptr, q->fifo_size, elem); } bool can_push(can_ring *q, const CANPacket_t *elem) { - bool ret = false; - uint32_t next_w_ptr; - - ENTER_CRITICAL(); - if ((q->w_ptr + 1U) == q->fifo_size) { - next_w_ptr = 0; - } else { - next_w_ptr = q->w_ptr + 1U; - } - if (next_w_ptr != q->r_ptr) { - q->elems[q->w_ptr] = *elem; - q->w_ptr = next_w_ptr; - ret = true; - } - EXIT_CRITICAL(); + bool ret = rb_push_u32(q->elems, sizeof(CANPacket_t), &q->w_ptr, &q->r_ptr, q->fifo_size, elem, false); if (!ret) { #ifdef DEBUG print("can_push to "); @@ -97,8 +70,7 @@ bool can_push(can_ring *q, const CANPacket_t *elem) { } uint32_t can_slots_empty(const can_ring *q) { - uint32_t ret = 0; - + uint32_t ret = 0U; ENTER_CRITICAL(); if (q->w_ptr >= q->r_ptr) { ret = q->fifo_size - 1U - q->w_ptr + q->r_ptr; @@ -106,14 +78,13 @@ uint32_t can_slots_empty(const can_ring *q) { ret = q->r_ptr - q->w_ptr - 1U; } EXIT_CRITICAL(); - return ret; } void can_clear(can_ring *q) { ENTER_CRITICAL(); - q->w_ptr = 0; - q->r_ptr = 0; + q->w_ptr = 0U; + q->r_ptr = 0U; EXIT_CRITICAL(); // handle TX buffer full with zero ECUs awake on the bus refresh_can_tx_slots_available(); diff --git a/board/drivers/ringbuffer.h b/board/drivers/ringbuffer.h new file mode 100644 index 00000000000..25efed13504 --- /dev/null +++ b/board/drivers/ringbuffer.h @@ -0,0 +1,89 @@ +#pragma once +#include +#include + +// Use existing critical-section macros if present; fall back to no-ops for tests. +#ifndef ENTER_CRITICAL +#define ENTER_CRITICAL() do {} while (0) +#endif +#ifndef EXIT_CRITICAL +#define EXIT_CRITICAL() do {} while (0) +#endif + +// Full push/pop helpers operating on element buffers. +// These are small static inline functions for zero call overhead. + +static inline bool rb_pop_u16(void *elems, uint16_t elem_size, + volatile uint16_t *w_ptr, volatile uint16_t *r_ptr, + uint16_t size, void *out) { + bool ret = false; + ENTER_CRITICAL(); + if (*w_ptr != *r_ptr) { + if (out != NULL) { + (void)memcpy(out, (uint8_t *)elems + ((uint32_t)(*r_ptr) * elem_size), elem_size); + } + *r_ptr = (uint16_t)((*r_ptr + 1U) % size); + ret = true; + } + EXIT_CRITICAL(); + return ret; +} + +static inline bool rb_push_u16(void *elems, uint16_t elem_size, + volatile uint16_t *w_ptr, volatile uint16_t *r_ptr, + uint16_t size, const void *in, bool overwrite) { + bool ret = false; + ENTER_CRITICAL(); + uint16_t next_w = (uint16_t)((*w_ptr + 1U) % size); + if ((next_w == *r_ptr) && overwrite) { + *r_ptr = (uint16_t)((*r_ptr + 1U) % size); + } + if (next_w != *r_ptr) { + if (in != NULL) { + (void)memcpy((uint8_t *)elems + ((uint32_t)(*w_ptr) * elem_size), in, elem_size); + } + *w_ptr = next_w; + ret = true; + } + EXIT_CRITICAL(); + return ret; +} + +static inline bool rb_pop_u32(void *elems, uint32_t elem_size, + volatile uint32_t *w_ptr, volatile uint32_t *r_ptr, + uint32_t size, void *out) { + bool ret = false; + ENTER_CRITICAL(); + if (*w_ptr != *r_ptr) { + if (out != NULL) { + (void)memcpy(out, (uint8_t *)elems + ((*r_ptr) * elem_size), elem_size); + } + uint32_t next_r = *r_ptr + 1U; + *r_ptr = (next_r == size) ? 0U : next_r; + ret = true; + } + EXIT_CRITICAL(); + return ret; +} + +static inline bool rb_push_u32(void *elems, uint32_t elem_size, + volatile uint32_t *w_ptr, volatile uint32_t *r_ptr, + uint32_t size, const void *in, bool overwrite) { + bool ret = false; + ENTER_CRITICAL(); + uint32_t next_w = *w_ptr + 1U; + next_w = (next_w == size) ? 0U : next_w; + if ((next_w == *r_ptr) && overwrite) { + uint32_t next_r = *r_ptr + 1U; + *r_ptr = (next_r == size) ? 0U : next_r; + } + if (next_w != *r_ptr) { + if (in != NULL) { + (void)memcpy((uint8_t *)elems + ((*w_ptr) * elem_size), in, elem_size); + } + *w_ptr = next_w; + ret = true; + } + EXIT_CRITICAL(); + return ret; +} diff --git a/board/drivers/uart.h b/board/drivers/uart.h index aeacf84b851..ca56dd43ec8 100644 --- a/board/drivers/uart.h +++ b/board/drivers/uart.h @@ -1,4 +1,5 @@ #include "uart_declarations.h" +#include "ringbuffer.h" // ***************************** Definitions ***************************** @@ -46,62 +47,16 @@ uart_ring *get_ring_by_number(int a) { // ************************* Low-level buffer functions ************************* bool get_char(uart_ring *q, char *elem) { - bool ret = false; - - ENTER_CRITICAL(); - if (q->w_ptr_rx != q->r_ptr_rx) { - if (elem != NULL) *elem = q->elems_rx[q->r_ptr_rx]; - q->r_ptr_rx = (q->r_ptr_rx + 1U) % q->rx_fifo_size; - ret = true; - } - EXIT_CRITICAL(); - - return ret; + return rb_pop_u16(q->elems_rx, 1U, &q->w_ptr_rx, &q->r_ptr_rx, (uint16_t)q->rx_fifo_size, elem); } bool injectc(uart_ring *q, char elem) { - int ret = false; - uint16_t next_w_ptr; - - ENTER_CRITICAL(); - next_w_ptr = (q->w_ptr_rx + 1U) % q->rx_fifo_size; - - if ((next_w_ptr == q->r_ptr_rx) && q->overwrite) { - // overwrite mode: drop oldest byte - q->r_ptr_rx = (q->r_ptr_rx + 1U) % q->rx_fifo_size; - } - - if (next_w_ptr != q->r_ptr_rx) { - q->elems_rx[q->w_ptr_rx] = elem; - q->w_ptr_rx = next_w_ptr; - ret = true; - } - EXIT_CRITICAL(); - - return ret; + return rb_push_u16(q->elems_rx, 1U, &q->w_ptr_rx, &q->r_ptr_rx, (uint16_t)q->rx_fifo_size, &elem, q->overwrite); } bool put_char(uart_ring *q, char elem) { - bool ret = false; - uint16_t next_w_ptr; - - ENTER_CRITICAL(); - next_w_ptr = (q->w_ptr_tx + 1U) % q->tx_fifo_size; - - if ((next_w_ptr == q->r_ptr_tx) && q->overwrite) { - // overwrite mode: drop oldest byte - q->r_ptr_tx = (q->r_ptr_tx + 1U) % q->tx_fifo_size; - } - - if (next_w_ptr != q->r_ptr_tx) { - q->elems_tx[q->w_ptr_tx] = elem; - q->w_ptr_tx = next_w_ptr; - ret = true; - } - EXIT_CRITICAL(); - + bool ret = rb_push_u16(q->elems_tx, 1U, &q->w_ptr_tx, &q->r_ptr_tx, (uint16_t)q->tx_fifo_size, &elem, q->overwrite); uart_tx_ring(q); - return ret; } @@ -147,3 +102,10 @@ static void hexdump(const void *a, int l) { print("\n"); } #endif + +void clear_uart_buff(uart_ring *q) { + ENTER_CRITICAL(); + q->w_ptr_rx = 0U; q->r_ptr_rx = 0U; + q->w_ptr_tx = 0U; q->r_ptr_tx = 0U; + EXIT_CRITICAL(); +} diff --git a/board/stm32h7/lluart.h b/board/stm32h7/lluart.h index e18f1e9f6f3..28b3840dd5a 100644 --- a/board/stm32h7/lluart.h +++ b/board/stm32h7/lluart.h @@ -5,17 +5,8 @@ static void uart_rx_ring(uart_ring *q){ // Read out RX buffer uint8_t c = q->uart->RDR; // This read after reading SR clears a bunch of interrupts - uint16_t next_w_ptr = (q->w_ptr_rx + 1U) % q->rx_fifo_size; - - if ((next_w_ptr == q->r_ptr_rx) && q->overwrite) { - // overwrite mode: drop oldest byte - q->r_ptr_rx = (q->r_ptr_rx + 1U) % q->rx_fifo_size; - } - - // Do not overwrite buffer data - if (next_w_ptr != q->r_ptr_rx) { - q->elems_rx[q->w_ptr_rx] = c; - q->w_ptr_rx = next_w_ptr; + bool pushed = rb_push_u16(q->elems_rx, 1U, &q->w_ptr_rx, &q->r_ptr_rx, (uint16_t)q->rx_fifo_size, &c, q->overwrite); + if (pushed) { if (q->callback != NULL) { q->callback(q); } @@ -30,8 +21,10 @@ void uart_tx_ring(uart_ring *q){ if (q->w_ptr_tx != q->r_ptr_tx) { // Only send if transmit register is empty (aka last byte has been sent) if ((q->uart->ISR & USART_ISR_TXE_TXFNF) != 0U) { - q->uart->TDR = q->elems_tx[q->r_ptr_tx]; // This clears TXE - q->r_ptr_tx = (q->r_ptr_tx + 1U) % q->tx_fifo_size; + uint8_t out; + if (rb_pop_u16(q->elems_tx, 1U, &q->w_ptr_tx, &q->r_ptr_tx, (uint16_t)q->tx_fifo_size, &out)) { + q->uart->TDR = out; // This clears TXE + } } // Enable TXE interrupt if there is still data to be sent