Skip to content

Commit

Permalink
[Silabs] Fix some uart cli hang (#35723)
Browse files Browse the repository at this point in the history 
* Add macros to abstact eusart api between series

* Fix for uart cpp with irq handling with series3. Tweak uart and shell task priority.

* [MATTER-4132]force implementation of _read and _write from newlib to use our UartConsole api when SILABS_LOG_OUT_UART is enabled

* remove segger_rtt_syscalls when uartlog is enabled

* Apply suggestions from code review

Co-authored-by: Andrei Litvin <[email protected]>

* Restyled by gn

---------

Co-authored-by: Andrei Litvin <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
@jmartinez-silabs @andy31415 @restyled-commits
3 people authored Sep 24, 2024
1 parent e527611 commit 222338e
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 26 deletions.
42 changes: 34 additions & 8 deletions examples/platform/silabs/syscalls_stubs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,31 @@ extern "C" {
#include "uart.h"
#endif

int _open(char * path, int flags, ...);
int _close(int file);
int _fstat(int file, struct stat * st);
int _isatty(int file);
int _lseek(int file, int ptr, int dir);
int _read(int file, char * ptr, int len);
int _write(int file, const char * ptr, int len);

/**************************************************************************
* @brief
* Open a file.
*
* @param[in] file
* File you want to open.
*
* @return
* Returns -1 since there is not logic here to open file.
**************************************************************************/

int __attribute__((weak)) _open(char * path, int flags, ...)
{
/* Pretend like we always fail */
return -1;
}

/**************************************************************************
* @brief
* Close a file.
Expand Down Expand Up @@ -170,17 +188,22 @@ int __attribute__((weak)) _lseek(int file, int ptr, int dir)
* @return
* Number of characters that have been read.
*************************************************************************/
int __attribute__((weak)) _read(int file, char * ptr, int len)
#if SILABS_LOG_OUT_UART
int _read(int file, char * ptr, int len)
{
(void) file;
#if SILABS_LOG_OUT_UART
return uartConsoleRead(ptr, len);
}
#else
int __attribute__((weak)) _read(int file, char * ptr, int len)
{
(void) file;
(void) ptr;
(void) len;
#endif

return 0;
}
#endif // SILABS_LOG_OUT_UART

/**************************************************************************
* @brief
Expand All @@ -198,17 +221,20 @@ int __attribute__((weak)) _read(int file, char * ptr, int len)
* @return
* Number of characters that have been written.
**************************************************************************/
int __attribute__((weak)) _write(int file, const char * ptr, int len)
#if SILABS_LOG_OUT_UART
int _write(int file, const char * ptr, int len)
{
(void) file;
#if SILABS_LOG_OUT_UART
uartConsoleWrite(ptr, len);
return uartConsoleWrite(ptr, len);
}
#else
int __attribute__((weak)) _write(int file, const char * ptr, int len)
{
(void) file;
(void) ptr;
#endif

return len;
}
#endif // SILABS_LOG_OUT_UART

#ifdef __cplusplus
}
Expand Down
68 changes: 53 additions & 15 deletions examples/platform/silabs/uart.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ extern "C" {
#include "rsi_rom_egpio.h"
#include "sl_si91x_usart.h"
#else // For EFR32
#if (_SILICON_LABS_32B_SERIES < 3)
#include "em_core.h"
#include "em_usart.h"
#else
#include "sl_hal_eusart.h"
#endif //_SILICON_LABS_32B_SERIES
#include "uartdrv.h"
#ifdef SL_BOARD_NAME
#include "sl_board_control.h"
Expand Down Expand Up @@ -86,11 +90,34 @@ extern "C" {
#define USART_IRQ HELPER2(SL_UARTDRV_EUSART_VCOM_PERIPHERAL_NO)
#define USART_IRQHandler HELPER4(SL_UARTDRV_EUSART_VCOM_PERIPHERAL_NO)
#define vcom_handle sl_uartdrv_eusart_vcom_handle

#if (_SILICON_LABS_32B_SERIES < 3)
#define EUSART_INT_ENABLE EUSART_IntEnable
#define EUSART_INT_DISABLE EUSART_IntDisable
#define EUSART_INT_CLEAR EUSART_IntClear
#define EUSART_CLEAR_RX
#define EUSART_GET_PENDING_INT EUSART_IntGet
#define EUSART_ENABLE(eusart) EUSART_Enable(eusart, eusartEnable)
#else
#define EUSART_INT_ENABLE sl_hal_eusart_enable_interrupts
#define EUSART_INT_DISABLE sl_hal_eusart_disable_interrupts
#define EUSART_INT_SET sl_hal_eusart_set_interrupts
#define EUSART_INT_CLEAR sl_hal_eusart_clear_interrupts
#define EUSART_CLEAR_RX sl_hal_eusart_clear_rx
#define EUSART_GET_PENDING_INT sl_hal_eusart_get_pending_interrupts
#define EUSART_ENABLE(eusart) \
{ \
sl_hal_eusart_enable(eusart); \
sl_hal_eusart_enable_tx(eusart); \
sl_hal_eusart_enable_rx(eusart); \
}
#endif //_SILICON_LABS_32B_SERIES

#else
#define USART_IRQ HELPER2(SL_UARTDRV_USART_VCOM_PERIPHERAL_NO)
#define USART_IRQHandler HELPER4(SL_UARTDRV_USART_VCOM_PERIPHERAL_NO)
#define vcom_handle sl_uartdrv_usart_vcom_handle
#endif // EFR32MG24
#endif // SL_CATALOG_UARTDRV_EUSART_PRESENT

namespace {
// In order to reduce the probability of data loss during the dmaFull callback handler we use
Expand Down Expand Up @@ -118,7 +145,11 @@ typedef struct
#if SILABS_LOG_OUT_UART
#define UART_MAX_QUEUE_SIZE 125
#else
#if (_SILICON_LABS_32B_SERIES < 3)
#define UART_MAX_QUEUE_SIZE 25
#else
#define UART_MAX_QUEUE_SIZE 50
#endif
#endif

#ifdef CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE
Expand All @@ -138,13 +169,7 @@ constexpr osThreadAttr_t kUartTaskAttr = { .name = "UART",
.cb_size = osThreadCbSize,
.stack_mem = uartStack,
.stack_size = kUartTaskSize,
#if SLI_SI91X_MCU_INTERFACE
// Reducing the priority of the UART task to avoid priority inversion
.priority = osPriorityNormal
#else
.priority = osPriorityRealtime
#endif // SLI_SI91X_MCU_INTERFACE
};
.priority = osPriorityBelowNormal };

typedef struct
{
Expand Down Expand Up @@ -309,16 +334,17 @@ void uartConsoleInit(void)

#ifdef SL_CATALOG_UARTDRV_EUSART_PRESENT
// Clear previous RX interrupts
EUSART_IntClear(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, EUSART_IF_RXFL);
EUSART_INT_CLEAR(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, (EUSART_IF_RXFL | EUSART_IF_RXOF));
EUSART_CLEAR_RX(SL_UARTDRV_EUSART_VCOM_PERIPHERAL);

// Enable RX interrupts
EUSART_IntEnable(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, EUSART_IF_RXFL);
EUSART_INT_ENABLE(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, EUSART_IF_RXFL);

// Enable EUSART
EUSART_Enable(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, eusartEnable);
EUSART_ENABLE(SL_UARTDRV_EUSART_VCOM_PERIPHERAL);
#else
USART_IntEnable(SL_UARTDRV_USART_VCOM_PERIPHERAL, USART_IF_RXDATAV);
#endif // EFR32MG24
#endif // SL_CATALOG_UARTDRV_EUSART_PRESENT
#endif // SLI_SI91X_MCU_INTERFACE == 0
}

Expand All @@ -344,9 +370,15 @@ void USART_IRQHandler(void)
#elif !defined(PW_RPC_ENABLED) && !defined(SL_WIFI)
otSysEventSignalPending();
#endif

#ifdef SL_CATALOG_UARTDRV_EUSART_PRESENT
EUSART_IntClear(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, EUSART_IF_RXFL);
// disable RXFL IRQ until data read by uartConsoleRead
EUSART_INT_DISABLE(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, EUSART_IF_RXFL);
EUSART_INT_CLEAR(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, EUSART_IF_RXFL);

if (EUSART_GET_PENDING_INT(SL_UARTDRV_EUSART_VCOM_PERIPHERAL) & EUSART_IF_RXOF)
{
EUSART_CLEAR_RX(SL_UARTDRV_EUSART_VCOM_PERIPHERAL);
}
#endif
}

Expand Down Expand Up @@ -418,7 +450,8 @@ int16_t uartConsoleWrite(const char * Buf, uint16_t BufLength)
memcpy(workBuffer.data, Buf, BufLength);
workBuffer.length = BufLength;

if (osMessageQueuePut(sUartTxQueue, &workBuffer, osPriorityNormal, 0) == osOK)
// this is usually a command response. Wait on queue if full.
if (osMessageQueuePut(sUartTxQueue, &workBuffer, osPriorityNormal, osWaitForever) == osOK)
{
return BufLength;
}
Expand All @@ -445,6 +478,7 @@ int16_t uartLogWrite(const char * log, uint16_t length)
memcpy(workBuffer.data + length, "\r\n", 2);
workBuffer.length = length + 2;

// Don't wait when queue is full. Drop the log and return UART_CONSOLE_ERR
if (osMessageQueuePut(sUartTxQueue, &workBuffer, osPriorityNormal, 0) == osOK)
{
return length;
Expand All @@ -462,6 +496,10 @@ int16_t uartConsoleRead(char * Buf, uint16_t NbBytesToRead)
{
uint8_t * data;

#ifdef SL_CATALOG_UARTDRV_EUSART_PRESENT
EUSART_INT_ENABLE(SL_UARTDRV_EUSART_VCOM_PERIPHERAL, EUSART_IF_RXFL);
#endif

if (Buf == NULL || NbBytesToRead < 1)
{
return UART_CONSOLE_ERR;
Expand Down
8 changes: 7 additions & 1 deletion src/lib/shell/streamer_silabs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,13 @@ ssize_t streamer_efr_read(streamer_t * streamer, char * buffer, size_t length)
ssize_t streamer_efr_write(streamer_t * streamer, const char * buffer, size_t length)
{
(void) streamer;
return uartConsoleWrite(buffer, (uint16_t) length);
int16_t bytesWritten = uartConsoleWrite(buffer, (uint16_t) length);
if (bytesWritten < 0) // The Write failed
{
bytesWritten = 0;
}

return bytesWritten;
}

static streamer_t streamer_efr = {
Expand Down
6 changes: 5 additions & 1 deletion third_party/silabs/SiWx917_sdk.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,8 +773,12 @@ template("siwx917_sdk") {
public_deps = [
"${segger_rtt_root}:segger_rtt",
"${segger_rtt_root}:segger_rtt_printf",
"${segger_rtt_root}:segger_rtt_syscalls",
]

if (!sl_uart_log_output) {
public_deps += [ "${segger_rtt_root}:segger_rtt_syscalls" ]
}

if (chip_crypto == "platform") {
if (sl_si91x_crypto_flavor == "tinycrypt") {
public_deps += [ ":siwx917_tinycrypt" ]
Expand Down
5 changes: 4 additions & 1 deletion third_party/silabs/efr32_sdk.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1072,9 +1072,12 @@ template("efr32_sdk") {
":efr32_mbedtls_config",
"${segger_rtt_root}:segger_rtt",
"${segger_rtt_root}:segger_rtt_printf",
"${segger_rtt_root}:segger_rtt_syscalls",
]

if (!sl_uart_log_output) {
public_deps += [ "${segger_rtt_root}:segger_rtt_syscalls" ]
}

if (defined(invoker.sources)) {
sources += invoker.sources
}
Expand Down

0 comments on commit 222338e

Please sign in to comment.