From 1cb83a81f09c696d2a8eeb54398c150a64fd498f Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Mon, 22 Jan 2024 15:51:24 +0100 Subject: [PATCH] Bluetooth: Host: Remove use of `bt_buf_get_cmd_complete` `bt_buf_get_cmd_complete` is broken due to https://github.com/zephyrproject-rtos/zephyr/issues/64158, and fixing it would require changing its signature and put even more complexity into the HCI drivers, as it would require the drivers to perform an even deeper peek into the event in order to observe the opcode. Instead of the above, this patch removes the use of `bt_buf_get_cmd_complete` and adds logic to allow the host to accept command complete events in normal event buffers. The above means performing a copy into the destination buffer, which is the original command buffer. This is a small inefficiency for now, but we should strive to redesign the host into a streaming architecture as much as possible and handle events immediately instead of retaining buffers. This fixes https://github.com/zephyrproject-rtos/zephyr/issues/64158: Like all command completed events, the completion event for `BT_HCI_OP_HOST_NUM_COMPLETED_PACKETS` is now placed in normal event buffers. The the logic where the host discards this event is already present. Since it's discarded, it will not interfere with the logic around `bt_dev.cmd_send`. Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/buf.c | 3 --- subsys/bluetooth/host/hci_core.c | 46 ++++++++++++++++++++++++-------- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/subsys/bluetooth/host/buf.c b/subsys/bluetooth/host/buf.c index 296d2aa0b5e..2623871c502 100644 --- a/subsys/bluetooth/host/buf.c +++ b/subsys/bluetooth/host/buf.c @@ -120,9 +120,6 @@ struct net_buf *bt_buf_get_evt(uint8_t evt, bool discardable, return buf; } #endif /* CONFIG_BT_CONN || CONFIG_BT_ISO */ - case BT_HCI_EVT_CMD_COMPLETE: - case BT_HCI_EVT_CMD_STATUS: - return bt_buf_get_cmd_complete(timeout); default: if (discardable) { struct net_buf *buf; diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 2f54298b41b..b751cb989f1 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -2284,25 +2284,44 @@ static void hci_reset_complete(struct net_buf *buf) atomic_set(bt_dev.flags, flags); } -static void hci_cmd_done(uint16_t opcode, uint8_t status, struct net_buf *buf) +static void hci_cmd_done(uint16_t opcode, uint8_t status, struct net_buf *evt_buf) { - LOG_DBG("opcode 0x%04x status 0x%02x buf %p", opcode, status, buf); + /* Original command buffer. */ + struct net_buf *buf = NULL; - if (net_buf_pool_get(buf->pool_id) != &hci_cmd_pool) { - LOG_WRN("opcode 0x%04x pool id %u pool %p != &hci_cmd_pool %p", opcode, - buf->pool_id, net_buf_pool_get(buf->pool_id), &hci_cmd_pool); - return; + LOG_DBG("opcode 0x%04x status 0x%02x buf %p", opcode, status, evt_buf); + + /* Unsolicited cmd complete. This does not complete a command. + * The controller can send these for effect of the `ncmd` field. + */ + if (opcode == 0) { + goto exit; + } + + /* Take the original command buffer reference. */ + buf = atomic_ptr_clear((atomic_ptr_t *)&bt_dev.sent_cmd); + + if (!buf) { + LOG_ERR("No command sent for cmd complete 0x%04x", opcode); + goto exit; } if (cmd(buf)->opcode != opcode) { - LOG_WRN("OpCode 0x%04x completed instead of expected 0x%04x", opcode, + LOG_ERR("OpCode 0x%04x completed instead of expected 0x%04x", opcode, cmd(buf)->opcode); - return; + buf = atomic_ptr_set((atomic_ptr_t *)&bt_dev.sent_cmd, buf); + __ASSERT_NO_MSG(!buf); + goto exit; } - if (bt_dev.sent_cmd) { - net_buf_unref(bt_dev.sent_cmd); - bt_dev.sent_cmd = NULL; + /* Response data is to be delivered in the original command + * buffer. + */ + if (evt_buf != buf) { + net_buf_reset(buf); + bt_buf_set_type(buf, BT_BUF_EVT); + net_buf_reserve(buf, BT_BUF_RESERVE); + net_buf_add_mem(buf, evt_buf->data, evt_buf->len); } if (cmd(buf)->state && !status) { @@ -2316,6 +2335,11 @@ static void hci_cmd_done(uint16_t opcode, uint8_t status, struct net_buf *buf) cmd(buf)->status = status; k_sem_give(cmd(buf)->sync); } + +exit: + if (buf) { + net_buf_unref(buf); + } } static void hci_cmd_complete(struct net_buf *buf)