Skip to content

Commit

Permalink
Merge branch 'bugfix/uart_poll_fails_for_pollout_event_v5.1' into 're…
Browse files Browse the repository at this point in the history 
…lease/v5.1'

UART: UART_SELECT_WRITE_NOTIF event added in UART driver (v5.1)

See merge request espressif/esp-idf!26191
  • Loading branch information
@jack0c
jack0c committed Sep 28, 2023
2 parents d02cfc2 + 1992050 commit 6b1f40b
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 15 deletions.
33 changes: 28 additions & 5 deletions components/driver/uart/uart.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -771,7 +771,9 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
uint32_t uart_intr_status = 0;
uart_event_t uart_event;
portBASE_TYPE HPTaskAwoken = 0;
bool need_yield = false;
static uint8_t pat_flg = 0;
BaseType_t sent = pdFALSE;
while (1) {
// The `continue statement` may cause the interrupt to loop infinitely
// we exit the interrupt here
Expand All @@ -793,6 +795,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
if (p_uart->tx_waiting_fifo == true && p_uart->tx_buf_size == 0) {
p_uart->tx_waiting_fifo = false;
xSemaphoreGiveFromISR(p_uart->tx_fifo_sem, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
} else {
//We don't use TX ring buffer, because the size is zero.
if (p_uart->tx_buf_size == 0) {
Expand All @@ -819,6 +822,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
}
//We have saved the data description from the 1st item, return buffer.
vRingbufferReturnItemFromISR(p_uart->tx_ring_buf, p_uart->tx_head, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
} else if (p_uart->tx_ptr == NULL) {
//Update the TX item pointer, we will need this to return item to buffer.
p_uart->tx_ptr = (uint8_t *)p_uart->tx_head;
Expand All @@ -841,6 +845,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
if (p_uart->tx_len_cur == 0) {
//Return item to ring buffer.
vRingbufferReturnItemFromISR(p_uart->tx_ring_buf, p_uart->tx_head, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
p_uart->tx_head = NULL;
p_uart->tx_ptr = NULL;
//Sending item done, now we need to send break if there is a record.
Expand All @@ -858,6 +863,12 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
//enable TX empty interrupt
en_tx_flg = true;
}
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_WRITE_NOTIF, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
UART_EXIT_CRITICAL_ISR(&uart_selectlock);
} else {
//enable TX empty interrupt
en_tx_flg = true;
Expand Down Expand Up @@ -905,13 +916,16 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_READ_NOTIF, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
UART_EXIT_CRITICAL_ISR(&uart_selectlock);
}
p_uart->rx_stash_len = rx_fifo_len;
//If we fail to push data to ring buffer, we will have to stash the data, and send next time.
//Mainly for applications that uses flow control or small ring buffer.
if (pdFALSE == xRingbufferSendFromISR(p_uart->rx_ring_buf, p_uart->rx_data_buf, p_uart->rx_stash_len, &HPTaskAwoken)) {
sent = xRingbufferSendFromISR(p_uart->rx_ring_buf, p_uart->rx_data_buf, p_uart->rx_stash_len, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
if (sent == pdFALSE) {
p_uart->rx_buffer_full_flg = true;
UART_ENTER_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
uart_hal_disable_intr_mask(&(uart_context[uart_num].hal), UART_INTR_RXFIFO_TOUT | UART_INTR_RXFIFO_FULL);
Expand All @@ -930,7 +944,9 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
p_uart->rx_buffered_len + pat_idx);
}
UART_EXIT_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
if ((p_uart->event_queue != NULL) && (pdFALSE == xQueueSendFromISR(p_uart->event_queue, (void * )&uart_event, &HPTaskAwoken))) {
sent = xQueueSendFromISR(p_uart->event_queue, (void * )&uart_event, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
if ((p_uart->event_queue != NULL) && (sent == pdFALSE)) {
#ifndef CONFIG_UART_ISR_IN_IRAM //Only log if ISR is not in IRAM
ESP_EARLY_LOGV(UART_TAG, "UART event queue full");
#endif
Expand Down Expand Up @@ -971,6 +987,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_ERROR_NOTIF, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
UART_EXIT_CRITICAL_ISR(&uart_selectlock);
uart_hal_clr_intsts_mask(&(uart_context[uart_num].hal), UART_INTR_RXFIFO_OVF);
Expand All @@ -982,6 +999,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_ERROR_NOTIF, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
UART_EXIT_CRITICAL_ISR(&uart_selectlock);
uart_hal_clr_intsts_mask(&(uart_context[uart_num].hal), UART_INTR_FRAM_ERR);
Expand All @@ -990,6 +1008,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_ERROR_NOTIF, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
UART_EXIT_CRITICAL_ISR(&uart_selectlock);
uart_hal_clr_intsts_mask(&(uart_context[uart_num].hal), UART_INTR_PARITY_ERR);
Expand All @@ -1008,6 +1027,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
p_uart->tx_waiting_brk = 0;
} else {
xSemaphoreGiveFromISR(p_uart->tx_brk_sem, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
} else if (uart_intr_status & UART_INTR_TX_BRK_IDLE) {
UART_ENTER_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
Expand Down Expand Up @@ -1046,6 +1066,7 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
}
UART_EXIT_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
xSemaphoreGiveFromISR(p_uart_obj[uart_num]->tx_done_sem, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
}
#if SOC_UART_SUPPORT_WAKEUP_INT
Expand All @@ -1060,14 +1081,16 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
}

if (uart_event.type != UART_EVENT_MAX && p_uart->event_queue) {
if (pdFALSE == xQueueSendFromISR(p_uart->event_queue, (void * )&uart_event, &HPTaskAwoken)) {
sent = xQueueSendFromISR(p_uart->event_queue, (void * )&uart_event, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
if (sent == pdFALSE) {
#ifndef CONFIG_UART_ISR_IN_IRAM //Only log if ISR is not in IRAM
ESP_EARLY_LOGV(UART_TAG, "UART event queue full");
#endif
}
}
}
if (HPTaskAwoken == pdTRUE) {
if (need_yield) {
portYIELD_FROM_ISR();
}
}
Expand Down
64 changes: 54 additions & 10 deletions components/vfs/test/test_vfs_select.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ TEST_CASE("UART can do select()", "[vfs]")
deinit(uart_fd, socket_fd);
}

TEST_CASE("UART can do poll()", "[vfs]")
TEST_CASE("UART can do poll() with POLLIN event", "[vfs]")
{
int uart_fd;
int socket_fd;
Expand Down Expand Up @@ -259,6 +259,50 @@ TEST_CASE("UART can do poll()", "[vfs]")

deinit(uart_fd, socket_fd);
}

TEST_CASE("UART can do poll() with POLLOUT event", "[vfs]")
{
int uart_fd;
int socket_fd;
char recv_message[sizeof(message)];

init(&uart_fd, &socket_fd);

struct pollfd poll_fds[] = {
{
.fd = uart_fd,
.events = POLLOUT,
},
{
.fd = -1, // should be ignored according to the documentation of poll()
},
};

const test_task_param_t test_task_param = {
.fd = uart_fd,
.delay_ms = 50,
.sem = xSemaphoreCreateBinary(),
};
TEST_ASSERT_NOT_NULL(test_task_param.sem);
start_task(&test_task_param);

poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100);
TEST_ASSERT_EQUAL(uart_fd, poll_fds[0].fd);
TEST_ASSERT_EQUAL(POLLOUT, poll_fds[0].revents);
TEST_ASSERT_EQUAL(-1, poll_fds[1].fd);
TEST_ASSERT_EQUAL(0, poll_fds[1].revents);

int read_bytes = read(uart_fd, recv_message, sizeof(message));
TEST_ASSERT_EQUAL(read_bytes, sizeof(message));
TEST_ASSERT_EQUAL_MEMORY(message, recv_message, sizeof(message));

TEST_ASSERT_EQUAL(xSemaphoreTake(test_task_param.sem, 1000 / portTICK_PERIOD_MS), pdTRUE);

vSemaphoreDelete(test_task_param.sem);

deinit(uart_fd, socket_fd);
}

#endif

TEST_CASE("socket can do select()", "[vfs]")
Expand Down Expand Up @@ -509,26 +553,26 @@ TEST_CASE("concurrent selects work", "[vfs]")
.errfds = NULL,
.maxfds = uart_fd + 1,
.tv = &tv,
.select_ret = 0, // expected timeout
.select_ret = 1,
.sem = xSemaphoreCreateBinary(),
};
TEST_ASSERT_NOT_NULL(param.sem);

start_select_task(&param);

fd_set rdfds2;
FD_ZERO(&rdfds2);
FD_SET(uart_fd, &rdfds2);
FD_SET(socket_fd, &rdfds2);
FD_SET(dummy_socket_fd, &rdfds2);

const test_task_param_t send_param = {
.fd = uart_fd,
.delay_ms = 50,
.sem = xSemaphoreCreateBinary(),
};
TEST_ASSERT_NOT_NULL(send_param.sem);
start_task(&send_param); // This task will write to UART which will be detected by select()
start_select_task(&param);
vTaskDelay(100 / portTICK_PERIOD_MS); //make sure the task has started and waits in select()

fd_set rdfds2;
FD_ZERO(&rdfds2);
FD_SET(uart_fd, &rdfds2);
FD_SET(socket_fd, &rdfds2);
FD_SET(dummy_socket_fd, &rdfds2);

int s = select(MAX(MAX(uart_fd, dummy_socket_fd), socket_fd) + 1, &rdfds2, NULL, NULL, &tv);
TEST_ASSERT_EQUAL(1, s);
Expand Down

0 comments on commit 6b1f40b

Please sign in to comment.