From e0c50a90fb1847eb997f2a81437a555024167689 Mon Sep 17 00:00:00 2001 From: kurte Date: Sat, 29 Jul 2023 09:43:11 -0700 Subject: [PATCH 1/2] Serial: Tx use FIFO plus fixes This is a reasonably major update to the Serial(UART) code base, which is mostly concentrated on the Output(TX) side of it. Simple part of the description: The currently released code, did not use the FIFO object that was built into the Serial object. Instead it would output one character at a time and then wait for a callback from the lower level code base, which could be TDRE or TEND (mostly likely TDRE) before it would return from the call. There were several other methods that did not work or were not implemented including: flush(), availableForWrite(). Also the write with buffer and count did not work as it did not have the same parameters as the one in the Print class, so the print class simply enumerated each byte. **More Complex stuff:** **Write Call** I added a 16 byte buffer to the UART class that I use to pass down to the R_SCI_UART_Write calls, with as much data as I have up to 16 from the write method if a write is not currently in process. If not active and the FIFO is empty. I detect that and bypass using FIFO up to the first 16 characters. If write is active, the new data is added to the FIFO. **UART_EVENT_TX_DATA_EMPTY** Note: I mention TDRE here, but also cases for FIFO which are different. When lower level completes the write, it will do a callback with the event: UART_EVENT_TX_DATA_EMPTY. First try was to simply grab what is available up to 16 bytes and recall R_SCI_UART_Write with that. But that can lose data, as the callback is called when the ISR code has put the last byte into the TDR register, and the Write call always tries to put the first character out, which if the TDRE state is not set, will lose the last byte. Found I needed to handle two cases: 1) If TDRE I call the write function again. 2) if not TDRE, I update the Buffer/count member variables and reset which interrupt events which will happen. **TIMING** I was still running into conditions where either data was lost or the transfers did not work correctly. RingBuffer: Found in several cases we were spending too much time in the ISR and we could lose interrupts on either UARTS or the same UARTS but different Interrupt. Found a lot of the time was spent in the SafeRingBufferN code, where it creates a class object which disables all interrupts and maybe reenables them at the end. This is needed as the API/ RingBufferN code is inherently unsafe to use here. I created my own versions of these classes that make the RingBuffer class safer such that the Safe version could be a minimal subset. That decreased the overhead a lot. FIFO Size: When using one of the UARTS with FIFO, we filled the RX before any was returned. So the ISR might need to process 16 characters, which took awhile. I have it currently set to 8. I updated all of the Variants to allow each to set their preference. **RX** As mentioned above, when we receive bytes and the ISR is triggered, it calls our callback with the UART_EVENT_RX_CHAR event. For simple double buffer UARTS, we simply store one character away. However for FIFO UARTS, it will callback to us one time for each character in the FIFO. Which before took a lot of time. As for character it would setup the callback, call it, where we disabled the interrupts... The new cod instead extracts the remaining characters from the FIFO and stuffs them directly back into our fifo queue. To keep from having to look each time at, am I running on a FIFO queue or not, I have two different callbacks, so they don't have to keep asking. **DEBUG CODE** Currently I have a fair amount of debug code built into the Serial class, to help debug, including fast functions to set/clear/toggle digital pins as well as to maybe save some state information. I have hopefully all of this under #ifdef --- cores/arduino/FasterSafeRingBuffer.h | 196 +++++++++++ cores/arduino/IRQManager.cpp | 11 +- cores/arduino/Serial.cpp | 329 ++++++++++++++++-- cores/arduino/Serial.h | 49 ++- cores/arduino/cm_backtrace/cm_backtrace.c | 6 +- .../ra/fsp/inc/instances/r_sci_uart.h | 6 +- .../ra/fsp/inc/instances/r_sci_uart.h | 5 +- .../ra/fsp/inc/instances/r_sci_uart.h | 5 +- .../ra/fsp/inc/instances/r_sci_uart.h | 2 + 9 files changed, 567 insertions(+), 42 deletions(-) create mode 100644 cores/arduino/FasterSafeRingBuffer.h diff --git a/cores/arduino/FasterSafeRingBuffer.h b/cores/arduino/FasterSafeRingBuffer.h new file mode 100644 index 000000000..1696f6f1a --- /dev/null +++ b/cores/arduino/FasterSafeRingBuffer.h @@ -0,0 +1,196 @@ +/* + Copyright (c) 2020 Arduino. All right 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 2.1 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, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +*/ + +#ifdef __cplusplus + +#ifndef _FASTER_SAFE_RING_BUFFER_ +#define _FASTER_SAFE_RING_BUFFER_ +#include "sync.h" + + +// Note: Below is a modified version of the RingBuffer.h, that removes the +// count member variable, such that the producer and consumer don't need to +// modify the same members. Allows one or both of them to run without needing +// to disable interrupts for reading and writing members. + +// Define constants and variables for buffering incoming serial data. We're +// using a ring buffer (I think), in which head is the index of the location +// to which to write the next incoming character and tail is the index of the +// location from which to read. +namespace arduino { + +#define SERIAL_BUFFER_SIZE 64 + +template +class SaferRingBufferN +{ + public: + uint8_t _aucBuffer[N] ; + volatile int _iHead ; + volatile int _iTail ; + + public: + SaferRingBufferN( void ) ; + void store_char( uint8_t c ) ; + void clear(); + int read_char(); + int available(); + int availableForStore(); + int peek(); + bool isFull(); + + private: + int nextIndex(int index); + inline bool isEmpty() const { return (_iHead == _iTail); } +}; + +typedef SaferRingBufferN SaferRingBuffer; + + +template +SaferRingBufferN::SaferRingBufferN( void ) +{ + memset( _aucBuffer, 0, N ) ; + clear(); +} + +template +void SaferRingBufferN::store_char( uint8_t c ) +{ + // if we should be storing the received character into the location + // just before the tail (meaning that the head would advance to the + // current location of the tail), we're about to overflow the buffer + // and so we don't write the character or advance the head. + int newhead = nextIndex(_iHead); + if (newhead != _iTail) + { + _aucBuffer[_iHead] = c ; + _iHead = newhead; + } +} + +template +void SaferRingBufferN::clear() +{ + _iHead = 0; + _iTail = 0; +} + +template +int SaferRingBufferN::read_char() +{ + if (_iHead == _iTail) + return -1; + + uint8_t value = _aucBuffer[_iTail]; + _iTail = nextIndex(_iTail); + + return value; +} + +template +int SaferRingBufferN::available() +{ + // grab state of head and tail, to keep result consistent + int head = _iHead; + int tail = _iTail; + + if (head >= tail) + return head - tail; + return N + head - tail; +} + +template +int SaferRingBufferN::availableForStore() +{ + // grab state of head and tail, to keep result consistent + int head = _iHead; + int tail = _iTail; + if (head >= tail) return N - 1 - head + tail; + return tail - head - 1; +} + +template +int SaferRingBufferN::peek() +{ + if (isEmpty()) + return -1; + + return _aucBuffer[_iTail]; +} + +template +int SaferRingBufferN::nextIndex(int index) +{ + return (uint32_t)(index + 1) % N; +} + +template +bool SaferRingBufferN::isFull() +{ + int newhead = nextIndex(_iHead); + return (newhead == _iTail); +} + +/////////////////////////////////// + +// Protect writes for potential multiple writers +template +class FasterSafeWriteRingBufferN : public SaferRingBufferN +{ + public: + void store_char( uint8_t c ) ; +}; + +typedef FasterSafeWriteRingBufferN FasterSafeWriteRingBuffer; + + +template +void FasterSafeWriteRingBufferN::store_char(uint8_t c) { + synchronized { + SaferRingBufferN::store_char(c); + } +} + +template +class FasterSafeReadRingBufferN : public SaferRingBufferN +{ + public: + int read_char(); +}; + +typedef FasterSafeReadRingBufferN FasterSafeReadRingBuffer; + +template +int FasterSafeReadRingBufferN::read_char() { + synchronized { + return SaferRingBufferN::read_char(); + } + + // We should never reached this line because the synchronized {} block gets + // executed at least once. However the compiler gets confused and prints a + // warning about control reaching the end of a non-void function. This + // silences that warning. + return -1; +} + + +} + +#endif /* _SAFE_RING_BUFFER_ */ +#endif /* __cplusplus */ diff --git a/cores/arduino/IRQManager.cpp b/cores/arduino/IRQManager.cpp index 4d9e6e1e8..2b5cb40ad 100644 --- a/cores/arduino/IRQManager.cpp +++ b/cores/arduino/IRQManager.cpp @@ -12,6 +12,7 @@ #define SDCARD_CARD_PRIORITY 12 #define EXTERNAL_PIN_PRIORITY 12 #define UART_SCI_PRIORITY 12 +#define UART_SCI_RX_PRIORITY 10 #define USB_PRIORITY 12 #define AGT_PRIORITY 14 #define RTC_PRIORITY 12 @@ -487,14 +488,14 @@ bool IRQManager::addPeripheral(Peripheral_t p, void *cfg) { last_interrupt_index++; /* RX interrupt */ - p_cfg->rxi_ipl = UART_SCI_PRIORITY; + p_cfg->rxi_ipl = UART_SCI_RX_PRIORITY; p_cfg->rxi_irq = (IRQn_Type)last_interrupt_index; *(irq_ptr + last_interrupt_index) = (uint32_t)sci_uart_rxi_isr; set_sci_rx_link_event(last_interrupt_index, p_cfg->channel); last_interrupt_index++; /* RX-ERROR interrupt */ - p_cfg->eri_ipl = UART_SCI_PRIORITY; + p_cfg->eri_ipl = UART_SCI_RX_PRIORITY; p_cfg->eri_irq = (IRQn_Type)last_interrupt_index; *(irq_ptr + last_interrupt_index) = (uint32_t)sci_uart_eri_isr; set_sci_eri_link_event(last_interrupt_index, p_cfg->channel); @@ -1672,7 +1673,7 @@ void IRQManager::set_can_tx_link_event(int li, int ch) #endif } -void IRQManager::set_canfd_error_link_event(int li, int ch) +void IRQManager::set_canfd_error_link_event(__attribute__((unused)) int li, __attribute__((unused)) int ch) { if (0) {} #ifdef ELC_EVENT_CAN0_CHERR @@ -1687,7 +1688,7 @@ void IRQManager::set_canfd_error_link_event(int li, int ch) #endif } -void IRQManager::set_canfd_rx_link_event(int li, int ch) +void IRQManager::set_canfd_rx_link_event(__attribute__((unused)) int li, __attribute__((unused)) int ch) { if (0) {} #ifdef ELC_EVENT_CAN0_COMFRX @@ -1702,7 +1703,7 @@ void IRQManager::set_canfd_rx_link_event(int li, int ch) #endif } -void IRQManager::set_canfd_tx_link_event(int li, int ch) +void IRQManager::set_canfd_tx_link_event(__attribute__((unused)) int li, __attribute__((unused)) int ch) { if (0) {} #ifdef ELC_EVENT_CAN0_TX diff --git a/cores/arduino/Serial.cpp b/cores/arduino/Serial.cpp index d905edd90..29eb1242f 100644 --- a/cores/arduino/Serial.cpp +++ b/cores/arduino/Serial.cpp @@ -29,6 +29,41 @@ #undef Serial #endif +//#define ENABLE_DEBUG + +#define DEBUG_PIN_WRITE_CALL 3 +#define DEBUG_PIN_WRITE_TOGGLE 4 +#define DEBUG_PIN_CALLBACK_TXDE 5 +#define DEBUG_PIN_CALLBACK_TE 6 +#define DEBUG_PIN_CALLBACK_RX 7 +#define DEBUG_PIN_CALLBACK_ERROR 8 +#ifdef ENABLE_DEBUG +static R_PORT0_Type *port_table[] = { R_PORT0, R_PORT1, R_PORT2, R_PORT3, R_PORT4, R_PORT5, R_PORT6, R_PORT7 }; +static const uint16_t mask_table[] = { 1 << 0, 1 << 1, 1 << 2, 1 << 3, 1 << 4, 1 << 5, 1 << 6, 1 << 7, + 1 << 8, 1 << 9, 1 << 10, 1 << 11, 1 << 12, 1 << 13, 1 << 14, 1 << 15 }; +// quick and dirty digitalWriteFast +static inline void DBGDigitalWrite(pin_size_t pin, PinStatus val) { + uint16_t hardware_port_pin = g_pin_cfg[pin].pin; + //uint16_t mask = 1 << (hardware_port_pin & 0xf); + uint16_t pin_mask = mask_table[hardware_port_pin & 0xf]; + R_PORT0_Type *portX = port_table[hardware_port_pin >> 8]; + + if (val) portX->POSR = pin_mask; + else portX->PORR = pin_mask; +} +static inline void DBGDigitalToggle(pin_size_t pin) { + uint16_t hardware_port_pin = g_pin_cfg[pin].pin; + uint16_t pin_mask = mask_table[hardware_port_pin & 0xf]; + R_PORT0_Type *portX = port_table[hardware_port_pin >> 8]; + + if (portX->PODR & pin_mask) portX->PORR = pin_mask; + else portX->POSR = pin_mask; +} +#else +inline void DBGDigitalWrite(__attribute__((unused)) int pin, __attribute__((unused))int state) {}; +inline void DBGDigitalToggle(__attribute__((unused)) int pin) {}; +#endif + UART * UART::g_uarts[MAX_UARTS] = {nullptr}; void uart_callback(uart_callback_args_t __attribute((unused)) *p_args) @@ -56,21 +91,160 @@ void UART::WrapperCallback(uart_callback_args_t *p_args) { case UART_EVENT_ERR_OVERFLOW: case UART_EVENT_RX_COMPLETE: // This is called when all the "expected" data are received { + DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, HIGH); + DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, LOW); break; } - case UART_EVENT_TX_COMPLETE: case UART_EVENT_TX_DATA_EMPTY: { - //uint8_t to_enqueue = uart_ptr->txBuffer.available() < uart_ptr->uart_ctrl.fifo_depth ? uart_ptr->txBuffer.available() : uart_ptr->uart_ctrl.fifo_depth; - //while (to_enqueue) { - uart_ptr->tx_done = true; + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, HIGH); + if (uart_ptr->txBuffer.available() == 0) { + uart_ptr->tx_fsi_state = TX_FSI_WAITING_TE; // maybe... + uart_ptr->save_tx_info(0x40); // top nibble bits 0100 + } else { + size_t cb = 0; + while (cb < sizeof(tx_fsi_buffer)) { + int ch = uart_ptr->txBuffer.read_char(); + if (ch == -1) break; + uart_ptr->tx_fsi_buffer[cb++] = ch; + } + uart_ptr->tx_fsi_state = TX_FSI_ACTIVE; + DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + // See if we can simply stuff out the new buffer and count + // Maybe special case if we get here and TDR is already empty + // maybe different test for FIFO. + if (uart_ptr->uart_ctrl.p_reg->SSR_b.TDRE) { + uart_ptr->save_tx_info(cb | 0xA0); // top nibble bits 1010 + R_SCI_UART_Write(&(uart_ptr->uart_ctrl), uart_ptr->tx_fsi_buffer, cb); + } else { + uart_ptr->save_tx_info(cb | 0x60); // top nibble bits 0110 + + uart_ptr->uart_ctrl.tx_src_bytes = cb; + uart_ptr->uart_ctrl.p_tx_src = uart_ptr->tx_fsi_buffer; + + // and reenable the TIE and not TEIE + uart_ptr->uart_ctrl.p_reg->SCR &= (uint8_t) ~(R_SCI0_SCR_TEIE_Msk); // don't wait on transer end interrupt. + uart_ptr->uart_ctrl.p_reg->SCR |= (uint8_t)R_SCI0_SCR_TIE_Msk; + } + } + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, LOW); + break; + } + case UART_EVENT_TX_COMPLETE: + { + // + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, HIGH); + uart_ptr->tx_fsi_state = TX_FSI_COMPLETE; + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, LOW); + uart_ptr->save_tx_info(0x80); // top nibble bits 0110 break; } case UART_EVENT_RX_CHAR: { + + DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); if (uart_ptr->rxBuffer.availableForStore()) { uart_ptr->rxBuffer.store_char(p_args->data); } + DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); + break; + } + case UART_EVENT_BREAK_DETECT: + { + break; + } + } + +} + +/* -------------------------------------------------------------------------- */ +void UART::WrapperCallbackFIFO(uart_callback_args_t *p_args) { +/* -------------------------------------------------------------------------- */ + + uint32_t channel = p_args->channel; + + UART *uart_ptr = UART::g_uarts[channel]; + + if(uart_ptr == nullptr) { + return; + } + + + switch (p_args->event){ + case UART_EVENT_ERR_PARITY: + case UART_EVENT_ERR_FRAMING: + case UART_EVENT_ERR_OVERFLOW: + case UART_EVENT_RX_COMPLETE: // This is called when all the "expected" data are received + { + DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, HIGH); + DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, LOW); + break; + } + case UART_EVENT_TX_DATA_EMPTY: + { + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, HIGH); + if (uart_ptr->txBuffer.available() == 0) { + uart_ptr->tx_fsi_state = TX_FSI_WAITING_TE; // maybe... + uart_ptr->save_tx_info(0x40); // top nibble bits 0100 + } else { + size_t cb = 0; + while (cb < sizeof(tx_fsi_buffer)) { + int ch = uart_ptr->txBuffer.read_char(); + if (ch == -1) break; + uart_ptr->tx_fsi_buffer[cb++] = ch; + } + uart_ptr->tx_fsi_state = TX_FSI_ACTIVE; + DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + // we are losing chars. See if the UART is full, before and see if waiting helps + // two cases with or without fifo. + // but don't see any way we can get here without... + // See if we can simply stuff out the new buffer and count + // Maybe special case if we get here and TDR is already empty + // maybe different test for FIFO. + if (((uart_ptr->uart_ctrl.fifo_depth == 0) && (uart_ptr->uart_ctrl.p_reg->SSR_b.TDRE)) + /*|| ((uart_ptr->uart_ctrl.fifo_depth > 0) && (uart_ptr->uart_ctrl.p_reg->FDR_b.T ==0))*/) { + uart_ptr->save_tx_info(cb | 0xA0); // top nibble bits 1010 + R_SCI_UART_Write(&(uart_ptr->uart_ctrl), uart_ptr->tx_fsi_buffer, cb); + } else { + uart_ptr->save_tx_info(cb | 0x60); // top nibble bits 0110 + + uart_ptr->uart_ctrl.tx_src_bytes = cb; + uart_ptr->uart_ctrl.p_tx_src = uart_ptr->tx_fsi_buffer; + + // and reenable the TIE and not TEIE + uart_ptr->uart_ctrl.p_reg->SCR &= (uint8_t) ~(R_SCI0_SCR_TEIE_Msk); // don't wait on transer end interrupt. + uart_ptr->uart_ctrl.p_reg->SCR |= (uint8_t)R_SCI0_SCR_TIE_Msk; + } + + } + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, LOW); + break; + } + case UART_EVENT_TX_COMPLETE: + { + // + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, HIGH); + uart_ptr->tx_fsi_state = TX_FSI_COMPLETE; + DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, LOW); + uart_ptr->save_tx_info(0x80); // top nibble bits 0110 + break; + } + case UART_EVENT_RX_CHAR: + { + // See if we can minimize the callbacks, by checking ourself if there is + // more data in the FIFO... + // Don't look to see if there is room, as the store_char does the same + // and in either case we toss it anyway + DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); + uart_ptr->rxBuffer.store_char(p_args->data); + DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); + + while (uart_ptr->uart_ctrl.p_reg->FDR_b.R > 0U) { + DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); + uart_ptr->rxBuffer.store_char( uart_ptr->uart_ctrl.p_reg->FRDRHL & 0xff); + DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); + } + break; } case UART_EVENT_BREAK_DETECT: @@ -108,22 +282,81 @@ bool UART::setUpUartIrqs(uart_cfg_t &cfg) { /* -------------------------------------------------------------------------- */ size_t UART::write(uint8_t c) { /* -------------------------------------------------------------------------- */ - if(init_ok) { - tx_done = false; - R_SCI_UART_Write(&uart_ctrl, &c, 1); - while (!tx_done) {} - return 1; - } - else { - return 0; - } + return write(&c, 1); } -size_t UART::write(uint8_t* c, size_t len) { - if(init_ok) { - tx_done = false; - R_SCI_UART_Write(&uart_ctrl, c, len); - while (!tx_done) {} +size_t UART::write(const uint8_t* c, size_t len) { + if(init_ok && (len > 0)) { + DBGDigitalWrite(DEBUG_PIN_WRITE_CALL, HIGH); + size_t cb_left = len; + + // If there is no transfer active, try to bypass putting stuff into + // the txBuffer, just to pull it out again, for up to the size + // of our temporary buffer. + if (tx_fsi_state == TX_FSI_COMPLETE) { + size_t cb_copy = len; + if (cb_copy > sizeof(tx_fsi_buffer)) cb_copy = sizeof(tx_fsi_buffer); + memcpy(tx_fsi_buffer, c, cb_copy); + cb_left -= cb_copy; + c += cb_copy; + tx_fsi_state = TX_FSI_ACTIVE; + DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + save_tx_info(cb_copy); // top nibble bits 0010 + R_SCI_UART_Write(&(uart_ctrl), tx_fsi_buffer, cb_copy); + } + + while (cb_left) { + // Put as much of data into txBuffer as will fit + while(cb_left && !txBuffer.isFull()) { + txBuffer.store_char(*c++); + cb_left--; + } + + // was active, see if we can always make the callback handle setting up the next transfer +#if 1 + if (tx_fsi_state != TX_FSI_ACTIVE) { + // Does not have an active buffer so we need to send a new one. + // need to setup the next buffer... Unsure yet how much we should do. + uint16_t cb_copy = txBuffer.available(); + if (cb_copy > sizeof(tx_fsi_buffer)) cb_copy = sizeof(tx_fsi_buffer); + for (uint16_t i = 0; i < cb_copy; i++) tx_fsi_buffer[i] = txBuffer.read_char(); + tx_fsi_state = TX_FSI_ACTIVE; + DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + + // we were losing chars. See if the UART is full, before and see if waiting helps + // two cases with or without fifo. + // but don't see any way we can get here without... + uint32_t start_time = micros(); + if (uart_ctrl.fifo_depth) { + // Hopefully they don't completely fill the FIFO before calling us. + } else { + // Uart does not have FIFO so check TDRE.. + while ((((uint32_t)micros() - start_time) < 250) && !uart_ctrl.p_reg->SSR_b.TDRE) { + DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + } + } + save_tx_info(cb_copy | 0x20); // top nibble bits 0010 + R_SCI_UART_Write(&(uart_ctrl), tx_fsi_buffer, cb_copy); + } +#else + synchronized { + do_R_SCI_UART_Write = txBuffer.available() && (tx_fsi_state != TX_FSI_ACTIVE); + if (txBuffer.available()) { + tx_fsi_state = TX_FSI_ACTIVE; + + // if the TIE is enabled already, nothing to do. + if ((uart_ctrl.p_reg->SCR & R_SCI0_SCR_TIE_Msk) == 0) { + // probably in the case of either not active or waiting for TEIE so reset to + // looking for TIE + uart_ctrl.p_reg->SCR &= (uint8_t) ~(R_SCI0_SCR_TEIE_Msk); // don't wait on transer end interrupt. + uart_ctrl.p_reg->SCR |= (uint8_t)R_SCI0_SCR_TIE_Msk; + } + } + } +#endif + DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + } + DBGDigitalWrite(DEBUG_PIN_WRITE_CALL, LOW); return len; } else { @@ -212,7 +445,8 @@ void UART::begin(unsigned long baudrate, uint16_t config) { uart_cfg_extend.clock = SCI_UART_CLOCK_INT; uart_cfg_extend.rx_edge_start = SCI_UART_START_BIT_FALLING_EDGE; uart_cfg_extend.noise_cancel = SCI_UART_NOISE_CANCELLATION_DISABLE; - uart_cfg_extend.rx_fifo_trigger = SCI_UART_RX_FIFO_TRIGGER_MAX; + //uart_cfg_extend.rx_fifo_trigger = SCI_UART_RX_FIFO_TRIGGER_MAX; + uart_cfg_extend.rx_fifo_trigger = SCI_UART_RX_FIFO_TRIGGER_DEFAULT; uart_cfg_extend.p_baud_setting = &uart_baud; uart_cfg_extend.flow_control = SCI_UART_FLOW_CONTROL_RTS; uart_cfg_extend.flow_control_pin = (bsp_io_port_pin_t) UINT16_MAX; @@ -263,6 +497,7 @@ void UART::begin(unsigned long baudrate, uint16_t config) { } uart_cfg.p_callback = UART::WrapperCallback; + tx_fsi_state = TX_FSI_COMPLETE; } else { return; @@ -278,11 +513,20 @@ void UART::begin(unsigned long baudrate, uint16_t config) { if (uart_baud.mddr == 0) { err = R_SCI_UART_BaudCalculate(baudrate, false, err_rate, &uart_baud); } + + // TODO: Do you really want to hang the processor if it fails to open the Serial port? + // or set the Baud rate? err = R_SCI_UART_Open (&uart_ctrl, &uart_cfg); if(err != FSP_SUCCESS) while(1); err = R_SCI_UART_BaudSet(&uart_ctrl, (void *) &uart_baud); if(err != FSP_SUCCESS) while(1); + // Set a different callback if the uart has a FIFO so we don't have to keep + // checking it + if (uart_ctrl.fifo_depth > 0) { + R_SCI_UART_CallbackSet (&uart_ctrl, UART::WrapperCallbackFIFO, nullptr, nullptr); + } + rxBuffer.clear(); txBuffer.clear(); } @@ -322,9 +566,20 @@ int UART::read() { /* -------------------------------------------------------------------------- */ void UART::flush() { /* -------------------------------------------------------------------------- */ - while(txBuffer.available()); + // wait until our software queue is not empty and we are not at TEND. +// while(txBuffer.available()); +// while ((uart_ctrl.p_reg->SSR_b.TEND == 0) || (uart_ctrl.p_reg->SSR_b.TDRE == 0)) {} + while(tx_fsi_state != TX_FSI_COMPLETE) ; + +} + +/* -------------------------------------------------------------------------- */ +int UART::availableForWrite() { +/* -------------------------------------------------------------------------- */ + return txBuffer.availableForStore(); } + /* -------------------------------------------------------------------------- */ size_t UART::write_raw(uint8_t* c, size_t len) { /* -------------------------------------------------------------------------- */ @@ -335,4 +590,36 @@ size_t UART::write_raw(uint8_t* c, size_t len) { i++; } return len; -} \ No newline at end of file +} + +void UART::printDebugInfo(Stream *pstream) { + pstream->print("\tChannel: "); + pstream->print(channel, DEC); + pstream->print("\n\tPins:(TR) "); + pstream->print(tx_pin, DEC); + pstream->print(" "); + pstream->print(rx_pin, DEC); + pstream->println(); + pstream->print("\ttx_fsi_state: "); + pstream->print(tx_fsi_state, HEX); + #ifdef SERIAL_TEST_AND_DEBUG_TX_CODE + + pstream->print(" : "); + uint8_t info_index = last_tx_info_index; + for (uint8_t i = 0; i < 16; i++) { + info_index = (info_index == 0)? 15 : info_index - 1; + uint8_t val = last_tx_info[info_index]; + switch (val & 0xe0) { + case 0: pstream->print("WIDLE:"); break; + case 0x20: pstream->print("WFBUF:"); break; + case 0x40: pstream->print("TDRE->TEND:"); break; + case 0x60: pstream->print("TDRE-CONT:"); break; + case 0x80: pstream->print("TEND:"); break; + case 0xA0: pstream->print("TDRE-WR:"); break; + }; + pstream->print(val & 0x1f, DEC); + pstream->print(" "); + } + #endif + pstream->println(); +} diff --git a/cores/arduino/Serial.h b/cores/arduino/Serial.h index cc818d466..031f3fb2d 100644 --- a/cores/arduino/Serial.h +++ b/cores/arduino/Serial.h @@ -35,10 +35,15 @@ #include "r_sci_uart.h" #include "r_uart_api.h" -#include "SafeRingBuffer.h" +//#include "SafeRingBuffer.h" +#include "FasterSafeRingBuffer.h" #undef SERIAL_BUFFER_SIZE #define SERIAL_BUFFER_SIZE 512 +#define SERIAL_FSI_BUFFER_SIZE 16 + +// Uncomment to enable additional Debug code to be enabled +//#define SERIAL_TEST_AND_DEBUG_TX_CODE #define MAX_UARTS 10 @@ -52,6 +57,8 @@ class UART : public arduino::HardwareSerial { public: static UART *g_uarts[MAX_UARTS]; static void WrapperCallback(uart_callback_args_t *p_args); + static void WrapperCallbackFIFO(uart_callback_args_t *p_args); + static void uart_txi_isr(void); UART(int _pin_tx, int _pin_rx, int pin_rts = -1, int pin_cts = -1); void begin(unsigned long); @@ -62,7 +69,8 @@ class UART : public arduino::HardwareSerial { int read(void); void flush(void); size_t write(uint8_t c); - size_t write(uint8_t* c, size_t len); + size_t write(const uint8_t* c, size_t len); + virtual int availableForWrite() override; size_t write_raw(uint8_t* c, size_t len); using Print::write; operator bool(); // { return true; } @@ -75,11 +83,35 @@ class UART : public arduino::HardwareSerial { bool cfg_pins(int max_index); int channel; - arduino::SafeRingBufferN rxBuffer; - arduino::SafeRingBufferN txBuffer; - - volatile bool tx_done; - + //arduino::SafeRingBufferN rxBuffer; + //arduino::SafeRingBufferN txBuffer; + arduino::FasterSafeReadRingBufferN rxBuffer; + arduino::FasterSafeWriteRingBufferN txBuffer; + + uint8_t tx_fsi_buffer[SERIAL_FSI_BUFFER_SIZE]; + + // 0 - not active, 1 - active, 2 - TDRE state empty waiting for TE + enum {TX_FSI_COMPLETE = 0, TX_FSI_ACTIVE, TX_FSI_WAITING_TE}; + + volatile uint8_t tx_fsi_state; + + // tx debug + // Top 3 bits - what + // 0000 - FSI_Write was empty + // 0010 - Not in complete tate + // 0100 TDRE->TEND + // 0110 TDRE Continue + // 1000 TEND + #ifdef SERIAL_TEST_AND_DEBUG_TX_CODE + volatile uint8_t last_tx_info_index = 0; + volatile uint8_t last_tx_info[16]; + inline void save_tx_info(uint8_t val) { + last_tx_info[last_tx_info_index] = val; + last_tx_info_index = (last_tx_info_index + 1) & 0xf; + } + #else + inline void save_tx_info(__attribute__((unused)) uint8_t val) {} + #endif sci_uart_instance_ctrl_t uart_ctrl; uart_cfg_t uart_cfg; baud_setting_t uart_baud; @@ -91,6 +123,9 @@ class UART : public arduino::HardwareSerial { protected: bool init_ok; + public: + R_SCI0_Type * get_p_reg() {return uart_ctrl.p_reg;} + void printDebugInfo(Stream *pstream); }; diff --git a/cores/arduino/cm_backtrace/cm_backtrace.c b/cores/arduino/cm_backtrace/cm_backtrace.c index 2707da1a1..1bff9de91 100644 --- a/cores/arduino/cm_backtrace/cm_backtrace.c +++ b/cores/arduino/cm_backtrace/cm_backtrace.c @@ -117,9 +117,9 @@ static const char * const print_info[] = { #endif }; -static char* fw_name; -static char* hw_ver; -static char* sw_ver; +const static char* fw_name; +const static char* hw_ver; +const static char* sw_ver; static uint32_t main_stack_start_addr = 0; static size_t main_stack_size = 0; static uint32_t code_start_addr = 0; diff --git a/variants/MINIMA/includes/ra/fsp/inc/instances/r_sci_uart.h b/variants/MINIMA/includes/ra/fsp/inc/instances/r_sci_uart.h index d83b397c5..cb735d0d3 100644 --- a/variants/MINIMA/includes/ra/fsp/inc/instances/r_sci_uart.h +++ b/variants/MINIMA/includes/ra/fsp/inc/instances/r_sci_uart.h @@ -102,8 +102,10 @@ typedef struct st_sci_uart_instance_ctrl /** Receive FIFO trigger configuration. */ typedef enum e_sci_uart_rx_fifo_trigger { - SCI_UART_RX_FIFO_TRIGGER_1 = 0x1, ///< Callback after each byte is received without buffering - SCI_UART_RX_FIFO_TRIGGER_MAX = 0xF, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) + SCI_UART_RX_FIFO_TRIGGER_1 = 0x1, ///< Callback after each byte is received without buffering + SCI_UART_RX_FIFO_TRIGGER_HALF = 0x8, ///< Callback when FIFO is half full or (fewer interrupts, but leaves room) + SCI_UART_RX_FIFO_TRIGGER_MAX = 0xF, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) + SCI_UART_RX_FIFO_TRIGGER_DEFAULT = SCI_UART_RX_FIFO_TRIGGER_HALF, ///< Callback when FIFO is half full or (fewer interrupts, but leaves room) } sci_uart_rx_fifo_trigger_t; /** Asynchronous Start Bit Edge Detection configuration. */ diff --git a/variants/MUXTO/includes/ra/fsp/inc/instances/r_sci_uart.h b/variants/MUXTO/includes/ra/fsp/inc/instances/r_sci_uart.h index d83b397c5..a729286f8 100644 --- a/variants/MUXTO/includes/ra/fsp/inc/instances/r_sci_uart.h +++ b/variants/MUXTO/includes/ra/fsp/inc/instances/r_sci_uart.h @@ -102,8 +102,9 @@ typedef struct st_sci_uart_instance_ctrl /** Receive FIFO trigger configuration. */ typedef enum e_sci_uart_rx_fifo_trigger { - SCI_UART_RX_FIFO_TRIGGER_1 = 0x1, ///< Callback after each byte is received without buffering - SCI_UART_RX_FIFO_TRIGGER_MAX = 0xF, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) + SCI_UART_RX_FIFO_TRIGGER_1 = 0x1, ///< Callback after each byte is received without buffering + SCI_UART_RX_FIFO_TRIGGER_MAX = 0xF, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) + SCI_UART_RX_FIFO_TRIGGER_DEFAULT = SCI_UART_RX_FIFO_TRIGGER_MAX, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) } sci_uart_rx_fifo_trigger_t; /** Asynchronous Start Bit Edge Detection configuration. */ diff --git a/variants/PORTENTA_C33/includes/ra/fsp/inc/instances/r_sci_uart.h b/variants/PORTENTA_C33/includes/ra/fsp/inc/instances/r_sci_uart.h index d83b397c5..a729286f8 100644 --- a/variants/PORTENTA_C33/includes/ra/fsp/inc/instances/r_sci_uart.h +++ b/variants/PORTENTA_C33/includes/ra/fsp/inc/instances/r_sci_uart.h @@ -102,8 +102,9 @@ typedef struct st_sci_uart_instance_ctrl /** Receive FIFO trigger configuration. */ typedef enum e_sci_uart_rx_fifo_trigger { - SCI_UART_RX_FIFO_TRIGGER_1 = 0x1, ///< Callback after each byte is received without buffering - SCI_UART_RX_FIFO_TRIGGER_MAX = 0xF, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) + SCI_UART_RX_FIFO_TRIGGER_1 = 0x1, ///< Callback after each byte is received without buffering + SCI_UART_RX_FIFO_TRIGGER_MAX = 0xF, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) + SCI_UART_RX_FIFO_TRIGGER_DEFAULT = SCI_UART_RX_FIFO_TRIGGER_MAX, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) } sci_uart_rx_fifo_trigger_t; /** Asynchronous Start Bit Edge Detection configuration. */ diff --git a/variants/UNOWIFIR4/includes/ra/fsp/inc/instances/r_sci_uart.h b/variants/UNOWIFIR4/includes/ra/fsp/inc/instances/r_sci_uart.h index d83b397c5..fe53204a0 100644 --- a/variants/UNOWIFIR4/includes/ra/fsp/inc/instances/r_sci_uart.h +++ b/variants/UNOWIFIR4/includes/ra/fsp/inc/instances/r_sci_uart.h @@ -103,7 +103,9 @@ typedef struct st_sci_uart_instance_ctrl typedef enum e_sci_uart_rx_fifo_trigger { SCI_UART_RX_FIFO_TRIGGER_1 = 0x1, ///< Callback after each byte is received without buffering + SCI_UART_RX_FIFO_TRIGGER_HALF = 0x8, ///< Callback when FIFO is half full or (fewer interrupts, but leaves room) SCI_UART_RX_FIFO_TRIGGER_MAX = 0xF, ///< Callback when FIFO is full or after 15 bit times with no data (fewer interrupts) + SCI_UART_RX_FIFO_TRIGGER_DEFAULT = SCI_UART_RX_FIFO_TRIGGER_HALF, ///< CCallback when FIFO is half full or (fewer interrupts, but leaves room) } sci_uart_rx_fifo_trigger_t; /** Asynchronous Start Bit Edge Detection configuration. */ From 0e27ef27cc84a571897e5aa99e684c4ff395ea24 Mon Sep 17 00:00:00 2001 From: kurte Date: Fri, 29 Dec 2023 09:33:29 -0800 Subject: [PATCH 2/2] remove stuff from PR this includes a lot of debug code. A better implementation for ringbuffers --- cores/arduino/FasterSafeRingBuffer.h | 196 ---------------------- cores/arduino/Serial.cpp | 120 +++---------- cores/arduino/Serial.h | 12 +- cores/arduino/cm_backtrace/cm_backtrace.c | 6 +- 4 files changed, 33 insertions(+), 301 deletions(-) delete mode 100644 cores/arduino/FasterSafeRingBuffer.h diff --git a/cores/arduino/FasterSafeRingBuffer.h b/cores/arduino/FasterSafeRingBuffer.h deleted file mode 100644 index 1696f6f1a..000000000 --- a/cores/arduino/FasterSafeRingBuffer.h +++ /dev/null @@ -1,196 +0,0 @@ -/* - Copyright (c) 2020 Arduino. All right 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 2.1 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, write to the Free Software - Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA -*/ - -#ifdef __cplusplus - -#ifndef _FASTER_SAFE_RING_BUFFER_ -#define _FASTER_SAFE_RING_BUFFER_ -#include "sync.h" - - -// Note: Below is a modified version of the RingBuffer.h, that removes the -// count member variable, such that the producer and consumer don't need to -// modify the same members. Allows one or both of them to run without needing -// to disable interrupts for reading and writing members. - -// Define constants and variables for buffering incoming serial data. We're -// using a ring buffer (I think), in which head is the index of the location -// to which to write the next incoming character and tail is the index of the -// location from which to read. -namespace arduino { - -#define SERIAL_BUFFER_SIZE 64 - -template -class SaferRingBufferN -{ - public: - uint8_t _aucBuffer[N] ; - volatile int _iHead ; - volatile int _iTail ; - - public: - SaferRingBufferN( void ) ; - void store_char( uint8_t c ) ; - void clear(); - int read_char(); - int available(); - int availableForStore(); - int peek(); - bool isFull(); - - private: - int nextIndex(int index); - inline bool isEmpty() const { return (_iHead == _iTail); } -}; - -typedef SaferRingBufferN SaferRingBuffer; - - -template -SaferRingBufferN::SaferRingBufferN( void ) -{ - memset( _aucBuffer, 0, N ) ; - clear(); -} - -template -void SaferRingBufferN::store_char( uint8_t c ) -{ - // if we should be storing the received character into the location - // just before the tail (meaning that the head would advance to the - // current location of the tail), we're about to overflow the buffer - // and so we don't write the character or advance the head. - int newhead = nextIndex(_iHead); - if (newhead != _iTail) - { - _aucBuffer[_iHead] = c ; - _iHead = newhead; - } -} - -template -void SaferRingBufferN::clear() -{ - _iHead = 0; - _iTail = 0; -} - -template -int SaferRingBufferN::read_char() -{ - if (_iHead == _iTail) - return -1; - - uint8_t value = _aucBuffer[_iTail]; - _iTail = nextIndex(_iTail); - - return value; -} - -template -int SaferRingBufferN::available() -{ - // grab state of head and tail, to keep result consistent - int head = _iHead; - int tail = _iTail; - - if (head >= tail) - return head - tail; - return N + head - tail; -} - -template -int SaferRingBufferN::availableForStore() -{ - // grab state of head and tail, to keep result consistent - int head = _iHead; - int tail = _iTail; - if (head >= tail) return N - 1 - head + tail; - return tail - head - 1; -} - -template -int SaferRingBufferN::peek() -{ - if (isEmpty()) - return -1; - - return _aucBuffer[_iTail]; -} - -template -int SaferRingBufferN::nextIndex(int index) -{ - return (uint32_t)(index + 1) % N; -} - -template -bool SaferRingBufferN::isFull() -{ - int newhead = nextIndex(_iHead); - return (newhead == _iTail); -} - -/////////////////////////////////// - -// Protect writes for potential multiple writers -template -class FasterSafeWriteRingBufferN : public SaferRingBufferN -{ - public: - void store_char( uint8_t c ) ; -}; - -typedef FasterSafeWriteRingBufferN FasterSafeWriteRingBuffer; - - -template -void FasterSafeWriteRingBufferN::store_char(uint8_t c) { - synchronized { - SaferRingBufferN::store_char(c); - } -} - -template -class FasterSafeReadRingBufferN : public SaferRingBufferN -{ - public: - int read_char(); -}; - -typedef FasterSafeReadRingBufferN FasterSafeReadRingBuffer; - -template -int FasterSafeReadRingBufferN::read_char() { - synchronized { - return SaferRingBufferN::read_char(); - } - - // We should never reached this line because the synchronized {} block gets - // executed at least once. However the compiler gets confused and prints a - // warning about control reaching the end of a non-void function. This - // silences that warning. - return -1; -} - - -} - -#endif /* _SAFE_RING_BUFFER_ */ -#endif /* __cplusplus */ diff --git a/cores/arduino/Serial.cpp b/cores/arduino/Serial.cpp index 29eb1242f..993dad297 100644 --- a/cores/arduino/Serial.cpp +++ b/cores/arduino/Serial.cpp @@ -29,40 +29,6 @@ #undef Serial #endif -//#define ENABLE_DEBUG - -#define DEBUG_PIN_WRITE_CALL 3 -#define DEBUG_PIN_WRITE_TOGGLE 4 -#define DEBUG_PIN_CALLBACK_TXDE 5 -#define DEBUG_PIN_CALLBACK_TE 6 -#define DEBUG_PIN_CALLBACK_RX 7 -#define DEBUG_PIN_CALLBACK_ERROR 8 -#ifdef ENABLE_DEBUG -static R_PORT0_Type *port_table[] = { R_PORT0, R_PORT1, R_PORT2, R_PORT3, R_PORT4, R_PORT5, R_PORT6, R_PORT7 }; -static const uint16_t mask_table[] = { 1 << 0, 1 << 1, 1 << 2, 1 << 3, 1 << 4, 1 << 5, 1 << 6, 1 << 7, - 1 << 8, 1 << 9, 1 << 10, 1 << 11, 1 << 12, 1 << 13, 1 << 14, 1 << 15 }; -// quick and dirty digitalWriteFast -static inline void DBGDigitalWrite(pin_size_t pin, PinStatus val) { - uint16_t hardware_port_pin = g_pin_cfg[pin].pin; - //uint16_t mask = 1 << (hardware_port_pin & 0xf); - uint16_t pin_mask = mask_table[hardware_port_pin & 0xf]; - R_PORT0_Type *portX = port_table[hardware_port_pin >> 8]; - - if (val) portX->POSR = pin_mask; - else portX->PORR = pin_mask; -} -static inline void DBGDigitalToggle(pin_size_t pin) { - uint16_t hardware_port_pin = g_pin_cfg[pin].pin; - uint16_t pin_mask = mask_table[hardware_port_pin & 0xf]; - R_PORT0_Type *portX = port_table[hardware_port_pin >> 8]; - - if (portX->PODR & pin_mask) portX->PORR = pin_mask; - else portX->POSR = pin_mask; -} -#else -inline void DBGDigitalWrite(__attribute__((unused)) int pin, __attribute__((unused))int state) {}; -inline void DBGDigitalToggle(__attribute__((unused)) int pin) {}; -#endif UART * UART::g_uarts[MAX_UARTS] = {nullptr}; @@ -91,13 +57,13 @@ void UART::WrapperCallback(uart_callback_args_t *p_args) { case UART_EVENT_ERR_OVERFLOW: case UART_EVENT_RX_COMPLETE: // This is called when all the "expected" data are received { - DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, HIGH); - DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_ERROR, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_ERROR, LOW); break; } case UART_EVENT_TX_DATA_EMPTY: { - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_TXDE, HIGH); if (uart_ptr->txBuffer.available() == 0) { uart_ptr->tx_fsi_state = TX_FSI_WAITING_TE; // maybe... uart_ptr->save_tx_info(0x40); // top nibble bits 0100 @@ -109,7 +75,7 @@ void UART::WrapperCallback(uart_callback_args_t *p_args) { uart_ptr->tx_fsi_buffer[cb++] = ch; } uart_ptr->tx_fsi_state = TX_FSI_ACTIVE; - DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + //digitalToggle(DEBUG_PIN_WRITE_TOGGLE); // See if we can simply stuff out the new buffer and count // Maybe special case if we get here and TDR is already empty // maybe different test for FIFO. @@ -127,26 +93,26 @@ void UART::WrapperCallback(uart_callback_args_t *p_args) { uart_ptr->uart_ctrl.p_reg->SCR |= (uint8_t)R_SCI0_SCR_TIE_Msk; } } - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_TXDE, LOW); break; } case UART_EVENT_TX_COMPLETE: { // - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_TE, HIGH); uart_ptr->tx_fsi_state = TX_FSI_COMPLETE; - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_TE, LOW); uart_ptr->save_tx_info(0x80); // top nibble bits 0110 break; } case UART_EVENT_RX_CHAR: { - DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); if (uart_ptr->rxBuffer.availableForStore()) { uart_ptr->rxBuffer.store_char(p_args->data); } - DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); break; } case UART_EVENT_BREAK_DETECT: @@ -176,13 +142,13 @@ void UART::WrapperCallbackFIFO(uart_callback_args_t *p_args) { case UART_EVENT_ERR_OVERFLOW: case UART_EVENT_RX_COMPLETE: // This is called when all the "expected" data are received { - DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, HIGH); - DBGDigitalWrite(DEBUG_PIN_CALLBACK_ERROR, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_ERROR, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_ERROR, LOW); break; } case UART_EVENT_TX_DATA_EMPTY: { - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_TXDE, HIGH); if (uart_ptr->txBuffer.available() == 0) { uart_ptr->tx_fsi_state = TX_FSI_WAITING_TE; // maybe... uart_ptr->save_tx_info(0x40); // top nibble bits 0100 @@ -194,7 +160,7 @@ void UART::WrapperCallbackFIFO(uart_callback_args_t *p_args) { uart_ptr->tx_fsi_buffer[cb++] = ch; } uart_ptr->tx_fsi_state = TX_FSI_ACTIVE; - DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + //digitalToggle(DEBUG_PIN_WRITE_TOGGLE); // we are losing chars. See if the UART is full, before and see if waiting helps // two cases with or without fifo. // but don't see any way we can get here without... @@ -212,20 +178,20 @@ void UART::WrapperCallbackFIFO(uart_callback_args_t *p_args) { uart_ptr->uart_ctrl.p_tx_src = uart_ptr->tx_fsi_buffer; // and reenable the TIE and not TEIE - uart_ptr->uart_ctrl.p_reg->SCR &= (uint8_t) ~(R_SCI0_SCR_TEIE_Msk); // don't wait on transer end interrupt. + uart_ptr->uart_ctrl.p_reg->SCR &= (uint8_t) ~(R_SCI0_SCR_TEIE_Msk); // don't wait on transfer end interrupt. uart_ptr->uart_ctrl.p_reg->SCR |= (uint8_t)R_SCI0_SCR_TIE_Msk; } } - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TXDE, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_TXDE, LOW); break; } case UART_EVENT_TX_COMPLETE: { // - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_TE, HIGH); uart_ptr->tx_fsi_state = TX_FSI_COMPLETE; - DBGDigitalWrite(DEBUG_PIN_CALLBACK_TE, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_TE, LOW); uart_ptr->save_tx_info(0x80); // top nibble bits 0110 break; } @@ -235,14 +201,14 @@ void UART::WrapperCallbackFIFO(uart_callback_args_t *p_args) { // more data in the FIFO... // Don't look to see if there is room, as the store_char does the same // and in either case we toss it anyway - DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); uart_ptr->rxBuffer.store_char(p_args->data); - DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); while (uart_ptr->uart_ctrl.p_reg->FDR_b.R > 0U) { - DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); + //digitalWrite(DEBUG_PIN_CALLBACK_RX, HIGH); uart_ptr->rxBuffer.store_char( uart_ptr->uart_ctrl.p_reg->FRDRHL & 0xff); - DBGDigitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); + //digitalWrite(DEBUG_PIN_CALLBACK_RX, LOW); } break; @@ -287,7 +253,7 @@ size_t UART::write(uint8_t c) { size_t UART::write(const uint8_t* c, size_t len) { if(init_ok && (len > 0)) { - DBGDigitalWrite(DEBUG_PIN_WRITE_CALL, HIGH); + //digitalWrite(DEBUG_PIN_WRITE_CALL, HIGH); size_t cb_left = len; // If there is no transfer active, try to bypass putting stuff into @@ -300,7 +266,7 @@ size_t UART::write(const uint8_t* c, size_t len) { cb_left -= cb_copy; c += cb_copy; tx_fsi_state = TX_FSI_ACTIVE; - DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + //digitalToggle(DEBUG_PIN_WRITE_TOGGLE); save_tx_info(cb_copy); // top nibble bits 0010 R_SCI_UART_Write(&(uart_ctrl), tx_fsi_buffer, cb_copy); } @@ -321,7 +287,7 @@ size_t UART::write(const uint8_t* c, size_t len) { if (cb_copy > sizeof(tx_fsi_buffer)) cb_copy = sizeof(tx_fsi_buffer); for (uint16_t i = 0; i < cb_copy; i++) tx_fsi_buffer[i] = txBuffer.read_char(); tx_fsi_state = TX_FSI_ACTIVE; - DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + //digitalToggle(DEBUG_PIN_WRITE_TOGGLE); // we were losing chars. See if the UART is full, before and see if waiting helps // two cases with or without fifo. @@ -332,7 +298,7 @@ size_t UART::write(const uint8_t* c, size_t len) { } else { // Uart does not have FIFO so check TDRE.. while ((((uint32_t)micros() - start_time) < 250) && !uart_ctrl.p_reg->SSR_b.TDRE) { - DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + //digitalToggle(DEBUG_PIN_WRITE_TOGGLE); } } save_tx_info(cb_copy | 0x20); // top nibble bits 0010 @@ -354,9 +320,9 @@ size_t UART::write(const uint8_t* c, size_t len) { } } #endif - DBGDigitalToggle(DEBUG_PIN_WRITE_TOGGLE); + //digitalToggle(DEBUG_PIN_WRITE_TOGGLE); } - DBGDigitalWrite(DEBUG_PIN_WRITE_CALL, LOW); + //digitalWrite(DEBUG_PIN_WRITE_CALL, LOW); return len; } else { @@ -591,35 +557,3 @@ size_t UART::write_raw(uint8_t* c, size_t len) { } return len; } - -void UART::printDebugInfo(Stream *pstream) { - pstream->print("\tChannel: "); - pstream->print(channel, DEC); - pstream->print("\n\tPins:(TR) "); - pstream->print(tx_pin, DEC); - pstream->print(" "); - pstream->print(rx_pin, DEC); - pstream->println(); - pstream->print("\ttx_fsi_state: "); - pstream->print(tx_fsi_state, HEX); - #ifdef SERIAL_TEST_AND_DEBUG_TX_CODE - - pstream->print(" : "); - uint8_t info_index = last_tx_info_index; - for (uint8_t i = 0; i < 16; i++) { - info_index = (info_index == 0)? 15 : info_index - 1; - uint8_t val = last_tx_info[info_index]; - switch (val & 0xe0) { - case 0: pstream->print("WIDLE:"); break; - case 0x20: pstream->print("WFBUF:"); break; - case 0x40: pstream->print("TDRE->TEND:"); break; - case 0x60: pstream->print("TDRE-CONT:"); break; - case 0x80: pstream->print("TEND:"); break; - case 0xA0: pstream->print("TDRE-WR:"); break; - }; - pstream->print(val & 0x1f, DEC); - pstream->print(" "); - } - #endif - pstream->println(); -} diff --git a/cores/arduino/Serial.h b/cores/arduino/Serial.h index 031f3fb2d..933c83aef 100644 --- a/cores/arduino/Serial.h +++ b/cores/arduino/Serial.h @@ -35,16 +35,12 @@ #include "r_sci_uart.h" #include "r_uart_api.h" -//#include "SafeRingBuffer.h" -#include "FasterSafeRingBuffer.h" +#include "SafeRingBuffer.h" #undef SERIAL_BUFFER_SIZE #define SERIAL_BUFFER_SIZE 512 #define SERIAL_FSI_BUFFER_SIZE 16 -// Uncomment to enable additional Debug code to be enabled -//#define SERIAL_TEST_AND_DEBUG_TX_CODE - #define MAX_UARTS 10 typedef enum { @@ -83,10 +79,8 @@ class UART : public arduino::HardwareSerial { bool cfg_pins(int max_index); int channel; - //arduino::SafeRingBufferN rxBuffer; - //arduino::SafeRingBufferN txBuffer; - arduino::FasterSafeReadRingBufferN rxBuffer; - arduino::FasterSafeWriteRingBufferN txBuffer; + arduino::SafeRingBufferN rxBuffer; + arduino::SafeRingBufferN txBuffer; uint8_t tx_fsi_buffer[SERIAL_FSI_BUFFER_SIZE]; diff --git a/cores/arduino/cm_backtrace/cm_backtrace.c b/cores/arduino/cm_backtrace/cm_backtrace.c index 1bff9de91..2707da1a1 100644 --- a/cores/arduino/cm_backtrace/cm_backtrace.c +++ b/cores/arduino/cm_backtrace/cm_backtrace.c @@ -117,9 +117,9 @@ static const char * const print_info[] = { #endif }; -const static char* fw_name; -const static char* hw_ver; -const static char* sw_ver; +static char* fw_name; +static char* hw_ver; +static char* sw_ver; static uint32_t main_stack_start_addr = 0; static size_t main_stack_size = 0; static uint32_t code_start_addr = 0;