-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[v2.6.4-NCSDK-34113-branch] Bluetooth: nRF Connect SDK v2.6.4 NCSIDB-1718 cherry-picks #24800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v2.6.4-NCSDK-34113-branch] Bluetooth: nRF Connect SDK v2.6.4 NCSIDB-1718 cherry-picks #24800
Conversation
|
The following west manifest projects have been modified in this Pull Request:
Note: This message is automatically posted and updated by the Manifest GitHub Action. |
Test specificationCI/Jenkins/NRF
CI/Jenkins/integration
test-sdk-sidewalk: added because there was no .github/test-spec.yml in 'sidewalk' Detailed information of selected test modules Note: This message is automatically posted and updated by the CI |
d0a8d8b to
b0eba10
Compare
bff7e09 to
0b8fc7d
Compare
5447117 to
bd1d3bc
Compare
a28b19b to
c537798
Compare
|
Seems there are many issues on the CI. I assume some are missing recent changes in the user_devconf files for test HW (see: https://jenkins-ncs.nordicsemi.no/blue/organizations/jenkins/latest%2Fsub%2Ftest-fw-nrfconnect-ble_samples/detail/v2.6-branch/122/pipeline), so probably some cherry picks unrelated to the fix at hand are missing, or how will it be handled with CI running red? As far as I can see all cherry-picks needed for the fix itself are present. |
57cb111 to
eb2a681
Compare
96da016 to
869113d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this file is named _testcase.yaml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unit testing was added post v2.6-branch, and it was not trivial for me to port it back. If you can/think it is a low hanging fruit, and i get assistance, this can be include in my task to bring in some latest tests to cover the cherry-picks.
This KConfig enables the feature bit for power class 1 in the controller. Required if a customer wants to transmit at >10dBm Signed-off-by: Sean Madigan <[email protected]> (cherry picked from commit d8f5a50) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Log an error and fail if an HCI packet was too big for the static HCI buffer. If for instance CONFIG_BT_BUF_RX_SIZE is too small, you get buffer overruns. Hopefully this can save some time for poor saps like me who spend all day trying to figure out why their code seems to produce random bus faults and other memory corruption symptoms Signed-off-by: Olivier Lesage <[email protected]> (cherry picked from commit 88b468e) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
This commit fixes deadlock in MPSL workq caused by bt_buf_get_rx with K_FOREVER. Previously, calling bt_buf_get_rx with K_FOREVER could block indefinitely if the requested pool had no available buffers. This blocking affected the MPSL work queue, preventing it from processing other work items, including critical timeslot events like MPSL_TIMESLOT_SIGNAL_CANCELLED and MPSL_TIMESLOT_SIGNAL_BLOCKED. The issue becomes more severe if a background flash operation coincides with this scenario. Flash operations often execute on the system work queue (sysworkq), which can delay host work items responsible for freeing buffers in the RX pool. In such cases: - Flash operations stall, waiting for a timeslot event from the MPSL work queue. - The MPSL work queue remains blocked by bt_buf_get_rx. - This results in a deadlock, causing the flash operation to timeout and triggering a warning. This commit modifies the HCI driver to call bt_buf_get_rx with K_NO_WAIT. If no buffer is immediately available, it relies on a callback to notify when a buffer is freed. This change ensures the MPSL work queue remains unblocked, allowing other work items to execute while waiting for a buffer. Signed-off-by: Pavel Vasilyev <[email protected]> (cherry picked from commit 52b6395) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
After replacing K_FOREVER with K_NO_WAIT when requesting a buffer from the host pool, it became possible to call `hci_driver_receive_process` directly from the SDC callback instead of going through `receive_signal_raise` as `hci_driver_receive_process` is now non-blocking. Signed-off-by: Pavel Vasilyev <[email protected]> (cherry picked from commit aec6383) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
The host will generate up to acl_pkts number of Host Number of Completed Packets plus a number of normal HCI commands, which use Num_HCI_Command_Packets. As such, if we do not provide enough (host) TX CMD buffers for the possible number of acl_pkts plus Num_HCI_Command_Packets, we may run into a deadlock. Currently the SDC controller delays sending disconnect events until it received all the ACKs for the ACL data packets sent to the host to avoid race conditions. Which, in case there aren't enough (host) TX CMD buffers, can result in a deadlock visible by a missing disconnect event. When Controller to Host data flow control is supported, this commit ensures that the `CONFIG_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. Note: The SDC controller (currently) does not support Num_HCI_Command_Packets > 1. Ncmd is always 1. Furthermore this commit restricts the `host_total_num_acl_data_packets` communicated to the controller to how many acknowledgements + Ncmd the controller is able to receive from the host. This is especially valuable in a controller-only build where it cannot be foreseen what settings are used for the host. Signed-off-by: Kyra Lengfeld <[email protected]> (cherry picked from commit 7766183) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Disable the tests/subsys/bluetooth/controller as the required framework is not present in this repository branch. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Refer to commit zephyrproject-rtos/zephyr@d382fca Signed-off-by: Vinayak Kariappa Chettimada <[email protected]> (cherry picked from commit beadde7)
When power class 1 was enabled the feature feature bit was not being set. Signed-off-by: Thomas Johansen <[email protected]> (cherry picked from commit 399a846) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
This is needed after changes in upstream zephyr. Signed-off-by: Timothy Keys <[email protected]> (cherry picked from commit 85ce826) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
869113d to
dbb2fd1
Compare
dbb2fd1 to
8900ece
Compare
…iew1 Update test-manifest to reference the tag ncs-v2.6-branch-NCSDK-34113-2-preview1 for test_nrf. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Update sdk-zephyr, sdk-sidewalk, sdk-nrfxlib and dragoon revision for NCSIDB-1718. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
8900ece to
a1646f2
Compare
For the 2.6.4-NCSDK-34113-2-preview1 version. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
a1646f2 to
858c259
Compare
| CONFIG_BT_CONN_TX_MAX=4 | ||
| CONFIG_BT_BUF_EVT_RX_COUNT=4 | ||
| CONFIG_BT_BUF_ACL_TX_COUNT=3 | ||
| CONFIG_BT_CONN_TX_MAX=3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that your intention here is to reduce RAM usage next to setting CONFIG_BT_BUF_EVT_RX_COUNT=4 (to prevent RAM overflows). I am slightly worried that the extra configuration changes added here might introduce edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E.g. IIRC CONFIG_BT_CONN_TX_MAX used value of 4 here to allow transmitting:
2 * GATT notification with HID mouse input report (required for motion data pipeline)
1 * GATT notification with battery data (BAS)
1 * GATT read response
Setting it to the value of 3 might not cover all of the edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates in dongle's configuration might require additional testing too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RAM overflows seem small:
- 52 bytes for nrf52810dmouse
- 24 bytes for nrf52820dongle
Still, I do not see a simple change that could allow us to reduce the memory usage here. This might require further investigation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see support for 52810 was dropped here: f66256c
Is this a concern for the v2.6.4-x? this is a preview tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think customers are likely to use the preview tag with the updated nRF Desktop devices? If so, then they might possibly reveal some edge cases.
Still I am not sure if it's mandatory to support these 2 devices from this particular tag (I think this would need to be consulted with PMT).
4912bb8
into
nrfconnect:v2.6.4-NCSDK-34113-branch
nRF Connect SDK v2.6.4 NCSIDB-1718 cherry-picks.