From 9a13a0c7326767a0d243793cfe2a8fd8213ba10e Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Tue, 11 Jul 2017 09:47:08 +0200 Subject: [PATCH] Bluetooth: Add BUILD_ASSERT to check Tx and Rx thread priorities Added BUILD_ASSERT check for Tx and Rx thread priorities. The Tx thread priority shall be higher than Rx thread priority in order to correctly detect transaction violations in ATT and SMP protocols. The Number of Completed Packets for a connection shall be processed before any new data is received and processed for that connection. The Controller's priority receive thread priority shall be higher than the Host's Tx and the Controller's Rx thread priority. Signed-off-by: Vinayak Kariappa Chettimada --- drivers/bluetooth/hci/h4.c | 6 ++++-- drivers/bluetooth/hci/h5.c | 10 ++++++---- drivers/bluetooth/hci/spi.c | 5 +++-- subsys/bluetooth/common/dummy.c | 17 +++++++++++++++++ subsys/bluetooth/controller/Kconfig | 6 ++++++ subsys/bluetooth/controller/hci/hci_driver.c | 11 +++++++---- subsys/bluetooth/host/Kconfig | 11 +++++++++++ subsys/bluetooth/host/hci_core.c | 8 +++++--- 8 files changed, 59 insertions(+), 15 deletions(-) diff --git a/drivers/bluetooth/hci/h4.c b/drivers/bluetooth/hci/h4.c index bf4a664b278..e9b7673cee7 100644 --- a/drivers/bluetooth/hci/h4.c +++ b/drivers/bluetooth/hci/h4.c @@ -440,8 +440,10 @@ static int h4_open(void) uart_irq_callback_set(h4_dev, bt_uart_isr); k_thread_create(&rx_thread_data, rx_thread_stack, - K_THREAD_STACK_SIZEOF(rx_thread_stack), rx_thread, - NULL, NULL, NULL, K_PRIO_COOP(8), 0, K_NO_WAIT); + K_THREAD_STACK_SIZEOF(rx_thread_stack), + rx_thread, NULL, NULL, NULL, + K_PRIO_COOP(CONFIG_BLUETOOTH_RX_PRIO), + 0, K_NO_WAIT); return 0; } diff --git a/drivers/bluetooth/hci/h5.c b/drivers/bluetooth/hci/h5.c index 1ebc19591bf..82c1ee36f77 100644 --- a/drivers/bluetooth/hci/h5.c +++ b/drivers/bluetooth/hci/h5.c @@ -719,14 +719,16 @@ static void h5_init(void) k_fifo_init(&h5.tx_queue); k_thread_create(&tx_thread_data, tx_stack, K_THREAD_STACK_SIZEOF(tx_stack), - (k_thread_entry_t)tx_thread, - NULL, NULL, NULL, K_PRIO_COOP(7), 0, K_NO_WAIT); + (k_thread_entry_t)tx_thread, NULL, NULL, NULL, + K_PRIO_COOP(CONFIG_BLUETOOTH_HCI_TX_PRIO), + 0, K_NO_WAIT); k_fifo_init(&h5.rx_queue); k_thread_create(&rx_thread_data, rx_stack, K_THREAD_STACK_SIZEOF(rx_stack), - (k_thread_entry_t)rx_thread, - NULL, NULL, NULL, K_PRIO_COOP(8), 0, K_NO_WAIT); + (k_thread_entry_t)rx_thread, NULL, NULL, NULL, + K_PRIO_COOP(CONFIG_BLUETOOTH_RX_PRIO), + 0, K_NO_WAIT); /* Unack queue */ k_fifo_init(&h5.unack_queue); diff --git a/drivers/bluetooth/hci/spi.c b/drivers/bluetooth/hci/spi.c index 67adf523fad..5ddeb5ebe05 100644 --- a/drivers/bluetooth/hci/spi.c +++ b/drivers/bluetooth/hci/spi.c @@ -327,8 +327,9 @@ static int bt_spi_open(void) /* Start RX thread */ k_thread_create(&rx_thread_data, rx_stack, K_THREAD_STACK_SIZEOF(rx_stack), - (k_thread_entry_t)bt_spi_rx_thread, - NULL, NULL, NULL, K_PRIO_COOP(8), 0, K_NO_WAIT); + (k_thread_entry_t)bt_spi_rx_thread, NULL, NULL, NULL, + K_PRIO_COOP(CONFIG_BLUETOOTH_RX_PRIO), + 0, K_NO_WAIT); /* Take BLE out of reset */ gpio_pin_write(rst_dev, GPIO_RESET_PIN, 1); diff --git a/subsys/bluetooth/common/dummy.c b/subsys/bluetooth/common/dummy.c index 9349262005c..9a8cfdc18b0 100644 --- a/subsys/bluetooth/common/dummy.c +++ b/subsys/bluetooth/common/dummy.c @@ -17,3 +17,20 @@ * and that the system workqueue priority is negative (cooperative). */ BUILD_ASSERT(CONFIG_SYSTEM_WORKQUEUE_PRIORITY < 0); + +/* The Bluetooth subsystem requires the Tx thread to execute at higher priority + * than the Rx thread as the Tx thread needs to process the acknowledgements + * before new Rx data is processed. This is a necessity to correctly detect + * transaction violations in ATT and SMP protocols. + */ +BUILD_ASSERT(CONFIG_BLUETOOTH_HCI_TX_PRIO < CONFIG_BLUETOOTH_RX_PRIO); + +#if defined(CONFIG_BLUETOOTH_CONTROLLER) +/* The Bluetooth Controller's priority receive thread priority shall be higher + * than the Bluetooth Host's Tx and the Controller's receive thread priority. + * This is required in order to dispatch Number of Completed Packets event + * before any new data arrives on a connection to the Host threads. + */ +BUILD_ASSERT(CONFIG_BLUETOOTH_CONTROLLER_RX_PRIO < + CONFIG_BLUETOOTH_HCI_TX_PRIO); +#endif /* CONFIG_BLUETOOTH_CONTROLLER */ diff --git a/subsys/bluetooth/controller/Kconfig b/subsys/bluetooth/controller/Kconfig index db1401e585e..dc2c1c03666 100644 --- a/subsys/bluetooth/controller/Kconfig +++ b/subsys/bluetooth/controller/Kconfig @@ -95,6 +95,12 @@ config BLUETOOTH_CONTROLLER_RX_PRIO_STACK_SIZE int default 448 +config BLUETOOTH_CONTROLLER_RX_PRIO + # Hidden option for Controller's Co-Operative high priority Rx thread + # priority. + int + default 6 + comment "BLE Controller features" if BLUETOOTH_CONN diff --git a/subsys/bluetooth/controller/hci/hci_driver.c b/subsys/bluetooth/controller/hci/hci_driver.c index f1334a525cc..2f336f9de86 100644 --- a/subsys/bluetooth/controller/hci/hci_driver.c +++ b/subsys/bluetooth/controller/hci/hci_driver.c @@ -402,12 +402,15 @@ static int hci_driver_open(void) k_thread_create(&prio_recv_thread_data, prio_recv_thread_stack, K_THREAD_STACK_SIZEOF(prio_recv_thread_stack), - prio_recv_thread, - NULL, NULL, NULL, K_PRIO_COOP(6), 0, K_NO_WAIT); + prio_recv_thread, NULL, NULL, NULL, + K_PRIO_COOP(CONFIG_BLUETOOTH_CONTROLLER_RX_PRIO), + 0, K_NO_WAIT); k_thread_create(&recv_thread_data, recv_thread_stack, - K_THREAD_STACK_SIZEOF(recv_thread_stack), recv_thread, - NULL, NULL, NULL, K_PRIO_COOP(8), 0, K_NO_WAIT); + K_THREAD_STACK_SIZEOF(recv_thread_stack), + recv_thread, NULL, NULL, NULL, + K_PRIO_COOP(CONFIG_BLUETOOTH_RX_PRIO), + 0, K_NO_WAIT); BT_DBG("Success."); diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 15b6d7445a9..3f4bc8e10a8 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -57,6 +57,11 @@ config BLUETOOTH_HCI_TX_STACK_SIZE default 256 if BLUETOOTH_SPI default 640 if BLUETOOTH_CONTROLLER +config BLUETOOTH_HCI_TX_PRIO + # Hidden option for Co-Operative Tx thread priority + int + default 7 + config BLUETOOTH_RECV_IS_RX_THREAD # Virtual option set by the HCI driver to indicate that there's # no need for the host to have its own RX thread, rather the @@ -80,6 +85,12 @@ config BLUETOOTH_RX_STACK_SIZE require extra stack space, this value can be increased to accommodate for that. +config BLUETOOTH_RX_PRIO + # Hidden option for Co-Operative Rx thread priority + int + depends on BLUETOOTH_HCI_HOST || BLUETOOTH_RECV_IS_RX_THREAD + default 8 + if BLUETOOTH_HCI_HOST config BLUETOOTH_HOST_CRYPTO # Hidden option that compiles in random number generation and AES diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 98b8aa3621e..79367298ad2 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -4067,15 +4067,17 @@ int bt_enable(bt_ready_cb_t cb) /* TX thread */ k_thread_create(&tx_thread_data, tx_thread_stack, K_THREAD_STACK_SIZEOF(tx_thread_stack), - hci_tx_thread, NULL, NULL, - NULL, K_PRIO_COOP(7), 0, K_NO_WAIT); + hci_tx_thread, NULL, NULL, NULL, + K_PRIO_COOP(CONFIG_BLUETOOTH_HCI_TX_PRIO), + 0, K_NO_WAIT); #if !defined(CONFIG_BLUETOOTH_RECV_IS_RX_THREAD) /* RX thread */ k_thread_create(&rx_thread_data, rx_thread_stack, K_THREAD_STACK_SIZEOF(rx_thread_stack), (k_thread_entry_t)hci_rx_thread, NULL, NULL, NULL, - K_PRIO_COOP(8), 0, K_NO_WAIT); + K_PRIO_COOP(CONFIG_BLUETOOTH_RX_PRIO), + 0, K_NO_WAIT); #endif if (IS_ENABLED(CONFIG_BLUETOOTH_TINYCRYPT_ECC)) {