From 0617e1aebdffedeca441d4675e4fe3b674f43ab7 Mon Sep 17 00:00:00 2001 From: Krzysztof Chruscinski Date: Tue, 1 Sep 2020 11:13:06 +0200 Subject: [PATCH] samples: bluetooth: hci_uart: Improve RX path Receiver part reworked to not block in interrupt and rely on assumption that uart_fifo_read is capable of reading incoming data from the HW. Signed-off-by: Krzysztof Chruscinski --- samples/bluetooth/hci_uart/src/main.c | 255 +++++++++++++------------- 1 file changed, 131 insertions(+), 124 deletions(-) diff --git a/samples/bluetooth/hci_uart/src/main.c b/samples/bluetooth/hci_uart/src/main.c index 46c6e1ed34..07cdcc219b 100644 --- a/samples/bluetooth/hci_uart/src/main.c +++ b/samples/bluetooth/hci_uart/src/main.c @@ -43,6 +43,12 @@ static K_FIFO_DEFINE(uart_tx_queue); #define H4_SCO 0x03 #define H4_EVT 0x04 +/* Receiver states. */ +#define ST_IDLE 0 /* Waiting for packet type. */ +#define ST_HDR 1 /* Receiving packet header. */ +#define ST_PAYLOAD 2 /* Receiving packet payload. */ +#define ST_DISCARD 3 /* Dropping packet. */ + /* Length of a discard/flush buffer. * This is sized to align with a BLE HCI packet: * 1 byte H:4 header + 32 bytes ACL/event data @@ -52,65 +58,129 @@ static K_FIFO_DEFINE(uart_tx_queue); */ #define H4_DISCARD_LEN 33 -static int h4_read(const struct device *uart, uint8_t *buf, - size_t len, size_t min) +static int h4_read(const struct device *uart, uint8_t *buf, size_t len) { - int total = 0; + int rx = uart_fifo_read(uart, buf, len); - while (len) { - int rx; + LOG_DBG("read %d req %d", rx, len); - rx = uart_fifo_read(uart, buf, len); - if (rx == 0) { - LOG_DBG("Got zero bytes from UART"); - if (total < min) { - continue; + return rx; +} + +static bool valid_type(uint8_t type) +{ + return (type == H4_CMD) | (type == H4_ACL); +} + +/* Function assumes that type is validated and only CMD or ACL will be used. */ +static uint32_t get_len(const uint8_t *hdr_buf, uint8_t type) +{ + return (type == BT_BUF_CMD) ? + ((const struct bt_hci_cmd_hdr *)hdr_buf)->param_len : + sys_le16_to_cpu(((const struct bt_hci_acl_hdr *)hdr_buf)->len); +} + +/* Function assumes that type is validated and only CMD or ACL will be used. */ +static int hdr_len(uint8_t type) +{ + return (type == H4_CMD) ? + sizeof(struct bt_hci_cmd_hdr) : sizeof(struct bt_hci_acl_hdr); +} + +static void rx_isr(void) +{ + static struct net_buf *buf; + static int remaining; + static uint8_t state; + static uint8_t type; + static uint8_t hdr_buf[MAX(sizeof(struct bt_hci_cmd_hdr), + sizeof(struct bt_hci_acl_hdr))]; + int read; + + do { + switch (state) { + case ST_IDLE: + /* Get packet type */ + read = h4_read(hci_uart_dev, &type, sizeof(type)); + /* since we read in loop until no data is in the fifo, + * it is possible that read = 0. + */ + if (read) { + if (valid_type(type)) { + /* Get expected header size and switch + * to receiving header. + */ + remaining = hdr_len(type); + state = ST_HDR; + } else { + LOG_WRN("Unknown header %d", type); + } } break; + case ST_HDR: + read = h4_read(hci_uart_dev, + &hdr_buf[hdr_len(type) - remaining], + remaining); + remaining -= read; + if (remaining == 0) { + /* Header received. Allocate buffer and get + * payload length. If allocation fails leave + * interrupt. On failed allocation state machine + * is reset. + */ + buf = bt_buf_get_tx(BT_BUF_H4, K_NO_WAIT, + &type, sizeof(type)); + if (!buf) { + state = ST_IDLE; + return; + } + + remaining = get_len(hdr_buf, type); + + net_buf_add_mem(buf, hdr_buf, hdr_len(type)); + if (remaining > net_buf_tailroom(buf)) { + LOG_ERR("Not enough space in buffer"); + net_buf_unref(buf); + state = ST_DISCARD; + } else { + state = ST_PAYLOAD; + } + + } + break; + case ST_PAYLOAD: + read = h4_read(hci_uart_dev, net_buf_tail(buf), + remaining); + buf->len += read; + remaining -= read; + if (remaining == 0) { + /* Packet received */ + LOG_DBG("putting RX packet in queue."); + net_buf_put(&tx_queue, buf); + state = ST_IDLE; + } + break; + case ST_DISCARD: + { + uint8_t discard[H4_DISCARD_LEN]; + size_t to_read = MIN(remaining, sizeof(discard)); + + read = h4_read(hci_uart_dev, discard, to_read); + remaining -= read; + if (remaining == 0) { + state = ST_IDLE; + } + + break; + } + default: + read = 0; + __ASSERT_NO_MSG(0); + break; - LOG_DBG("read %d remaining %d", rx, len - rx); - len -= rx; - total += rx; - buf += rx; - } - - return total; -} - -static size_t h4_discard(const struct device *uart, size_t len) -{ - uint8_t buf[H4_DISCARD_LEN]; - - return uart_fifo_read(uart, buf, MIN(len, sizeof(buf))); -} - -static void h4_cmd_recv(struct net_buf *buf, int *remaining) -{ - struct bt_hci_cmd_hdr hdr; - - /* We can ignore the return value since we pass len == min */ - h4_read(hci_uart_dev, (void *)&hdr, sizeof(hdr), sizeof(hdr)); - - *remaining = hdr.param_len; - - net_buf_add_mem(buf, &hdr, sizeof(hdr)); - - LOG_DBG("len %u", hdr.param_len); -} - -static void h4_acl_recv(struct net_buf *buf, int *remaining) -{ - struct bt_hci_acl_hdr hdr; - - /* We can ignore the return value since we pass len == min */ - h4_read(hci_uart_dev, (void *)&hdr, sizeof(hdr), sizeof(hdr)); - - net_buf_add_mem(buf, &hdr, sizeof(hdr)); - - *remaining = sys_le16_to_cpu(hdr.len); - - LOG_DBG("len %u", *remaining); + } + } while (read); } static void tx_isr(void) @@ -136,83 +206,20 @@ static void tx_isr(void) static void bt_uart_isr(const struct device *unused, void *user_data) { - static struct net_buf *buf; - static int remaining; - ARG_UNUSED(unused); ARG_UNUSED(user_data); - while (uart_irq_update(hci_uart_dev) && - uart_irq_is_pending(hci_uart_dev)) { - int read; + if (!(uart_irq_rx_ready(hci_uart_dev) || + uart_irq_tx_ready(hci_uart_dev))) { + LOG_DBG("spurious interrupt"); + } - if (uart_irq_tx_ready(hci_uart_dev)) { - tx_isr(); - } else if (!uart_irq_rx_ready(hci_uart_dev)) { - /* Only the UART TX and RX path are interrupt-enabled */ - LOG_DBG("spurious interrupt"); - break; - } + if (uart_irq_tx_ready(hci_uart_dev)) { + tx_isr(); + } - /* Beginning of a new packet */ - if (!remaining) { - uint8_t type; - - /* Get packet type */ - read = h4_read(hci_uart_dev, &type, sizeof(type), 0); - if (read != sizeof(type)) { - LOG_WRN("Unable to read H4 packet type"); - continue; - } - - buf = bt_buf_get_tx(BT_BUF_H4, K_NO_WAIT, &type, - sizeof(type)); - if (!buf) { - return; - } - - switch (bt_buf_get_type(buf)) { - case BT_BUF_CMD: - h4_cmd_recv(buf, &remaining); - break; - case BT_BUF_ACL_OUT: - h4_acl_recv(buf, &remaining); - break; - default: - LOG_ERR("Unknown H4 type %u", type); - return; - } - - LOG_DBG("need to get %u bytes", remaining); - - if (remaining > net_buf_tailroom(buf)) { - LOG_ERR("Not enough space in buffer"); - net_buf_unref(buf); - buf = NULL; - } - } - - if (!buf) { - read = h4_discard(hci_uart_dev, remaining); - LOG_WRN("Discarded %d bytes", read); - remaining -= read; - continue; - } - - read = h4_read(hci_uart_dev, net_buf_tail(buf), remaining, 0); - - buf->len += read; - remaining -= read; - - LOG_DBG("received %d bytes", read); - - if (!remaining) { - LOG_DBG("full packet received"); - - /* Put buffer into TX queue, thread will dequeue */ - net_buf_put(&tx_queue, buf); - buf = NULL; - } + if (uart_irq_rx_ready(hci_uart_dev)) { + rx_isr(); } }