From 0459e3fef543c8ea6372eb43291780aeca9b9847 Mon Sep 17 00:00:00 2001 From: Kyra Lengfeld Date: Mon, 13 Jan 2025 12:20:23 +0100 Subject: [PATCH] Bluetooth: Controller: Ensure controller can handle flow control buffers Host Number of Completed Packets command does not follow normal flow control of HCI commands and the Controller side HCI drivers that allocates HCI command buffers with K_NO_WAIT can end up running out of command buffers. Host will generate up to acl_pkts number of Host Number of Completed Packets command plus a number of normal HCI commands. Normal HCI commands follow the HCI command flow control using Num_HCI_Command_Packets return in HCI command complete and status. When Controller to Host data flow control is supported, this commit ensures that BT_BUF_CMD_TX_COUNT is greater than or equal to (BT_BUF_ACL_RX_COUNT + Ncmd), where Ncmd is the supported maximum Num_HCI_Command_Packets, for a controller + host, or host-only build. Furthermore this commit restricts the `host_total_num_acl_data_packets` communicated to the controller to how many acknoledgements the controller is able to receive from the host, in a controller-only build. Note: The SDC controller (currently) does not support Num_HCI_Command_Packets > 1, which means Ncmd is always 1. Signed-off-by: Kyra Lengfeld --- .../releases_and_maturity/known_issues.rst | 10 +++++++++ subsys/bluetooth/controller/hci_driver.c | 17 +++++++++++++++ subsys/bluetooth/controller/hci_internal.c | 21 ++++++++++++++++++- 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/doc/nrf/releases_and_maturity/known_issues.rst b/doc/nrf/releases_and_maturity/known_issues.rst index bb1a4db85d46..1b4f6b8c096c 100644 --- a/doc/nrf/releases_and_maturity/known_issues.rst +++ b/doc/nrf/releases_and_maturity/known_issues.rst @@ -189,6 +189,16 @@ KRKNWK-14299: NRPA MAC address cannot be set in Zephyr Bluetooth LE ============ +.. rst-class:: v2-9-0-nRF54H20-1 v2-9-0 v2-8-0 v2-7-0 v2-6-2 v2-6-1 v2-6-0 + +DRGN-24352: Missing disconnection events when HCI Controller to host flow control is enabled + The :zephyr:code-sample:`bluetooth_hci_ipc` sample and :ref:`ipc_radio` application do not support stream flow control and drop bytes. + With the deferred disconnection complete generation added in the |NCS| v2.6.0, there are cases where the :kconfig:option:`CONFIG_BT_HCI_ACL_FLOW_CONTROL` Kconfig option has value ``y``, and the value of :kconfig:option:`CONFIG_BT_BUF_CMD_TX_COUNT` is smaller than (:kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT` + 1). + This might result in dropped HCI command packets. + The visible symptom is a missing disconnect event and subsequent failure in connection, if a peripheral device falls out of range or is powered off. + + **Workaround:** Make sure that the value of the :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT` Kconfig option is smaller than the one for :kconfig:option:`CONFIG_BT_BUF_CMD_TX_COUNT` in your project. + .. rst-class:: v2-9-0-nRF54H20-rc1 v2-9-0 v2-8-0 v2-7-0 v2-6-2 v2-6-1 v2-6-0 NCSDK-31095: Issues with the :kconfig:option:`CONFIG_SEGGER_SYSVIEW` Kconfig option diff --git a/subsys/bluetooth/controller/hci_driver.c b/subsys/bluetooth/controller/hci_driver.c index 56e5251af63b..da371a8403e7 100644 --- a/subsys/bluetooth/controller/hci_driver.c +++ b/subsys/bluetooth/controller/hci_driver.c @@ -68,6 +68,23 @@ BUILD_ASSERT(!IS_ENABLED(CONFIG_BT_CENTRAL) || BUILD_ASSERT(!IS_ENABLED(CONFIG_BT_PERIPHERAL) || (CONFIG_BT_CTLR_SDC_PERIPHERAL_COUNT > 0)); +#if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) +/* + * The Host will generate up to acl_pkts number of Host Number of Completed Packets command plus a + * number of normal HCI commands, as such we need to ensure the tx command buffer count is big + * enough to not block incoming ACKs from the host. + * + * When Controller to Host data flow control is supported, ensure that BT_BUF_CMD_TX_COUNT is + * greater than or equal to (BT_BUF_ACL_RX_COUNT + Ncmd), where Ncmd is the supported maximum + * Num_HCI_Command_Packets. + * + * The SDC controller (currently) does not support Num_HCI_Command_Packets > 1, which means Ncmd + * is always 1. + */ +BUILD_ASSERT(BT_BUF_ACL_RX_COUNT < CONFIG_BT_BUF_CMD_TX_COUNT, + "Too low HCI command buffers compared to ACL Rx buffers."); +#endif + #if defined(CONFIG_BT_BROADCASTER) #if defined(CONFIG_BT_CTLR_ADV_EXT) #define SDC_ADV_SET_COUNT CONFIG_BT_CTLR_ADV_SET diff --git a/subsys/bluetooth/controller/hci_internal.c b/subsys/bluetooth/controller/hci_internal.c index 1726680163ff..047270a297ea 100644 --- a/subsys/bluetooth/controller/hci_internal.c +++ b/subsys/bluetooth/controller/hci_internal.c @@ -930,6 +930,25 @@ static uint8_t link_control_cmd_put(uint8_t const * const cmd) } #endif +/* Note: When host and controller are built separately, the RX Buffer Count communicated here from + * the host might be exeeding the controller's TX command buffer count. This may result in the + * controller sending up to host_total_num_acl_data_packets, it cannot receive the acknoledgements + * via its TX command buffers for. + * As we do not propogate the TX command buffer count information to the controller, the simplest + * approach is to align the communicated host RX Buffer Count to it here. + */ +#if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) +static int sdc_hci_cmd_cb_host_buffer_size_wrapper(void *cmd_params) +{ + sdc_hci_cmd_cb_host_buffer_size_t local_params = + *(const sdc_hci_cmd_cb_host_buffer_size_t *)cmd_params; + local_params.host_total_num_acl_data_packets = + MIN(local_params.host_total_num_acl_data_packets, (CONFIG_BT_BUF_CMD_TX_COUNT - 1)); + + return sdc_hci_cmd_cb_host_buffer_size((const void *)&local_params); +} +#endif + static uint8_t controller_and_baseband_cmd_put(uint8_t const * const cmd, uint8_t * const raw_event_out, uint8_t *param_length_out) @@ -956,7 +975,7 @@ static uint8_t controller_and_baseband_cmd_put(uint8_t const * const cmd, case SDC_HCI_OPCODE_CMD_CB_SET_CONTROLLER_TO_HOST_FLOW_CONTROL: return sdc_hci_cmd_cb_set_controller_to_host_flow_control((void *)cmd_params); case SDC_HCI_OPCODE_CMD_CB_HOST_BUFFER_SIZE: - return sdc_hci_cmd_cb_host_buffer_size((void *)cmd_params); + return sdc_hci_cmd_cb_host_buffer_size_wrapper((void *)cmd_params); case SDC_HCI_OPCODE_CMD_CB_HOST_NUMBER_OF_COMPLETED_PACKETS: return sdc_hci_cmd_cb_host_number_of_completed_packets((void *)cmd_params); #endif