From 05c104dcff7f5604bcd7c59afafbffa11ee1c767 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Tue, 13 Jun 2023 08:28:41 +0530 Subject: [PATCH 1/5] Bluetooth: Controller: Fix CIS accept fails with unsupp parameters Fix CIS accepted by Host being failed in the Controller with reason 0x20 Unsupported LL Parameter Value, but relaxing the check that the ACL connection is sufficiently placed such that the time reservation using in the Controller implementation does not overlap with the CIG event. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/Kconfig.ll_sw_split | 10 ++++++++++ subsys/bluetooth/controller/ll_sw/ull_peripheral_iso.c | 3 ++- tests/bluetooth/init/prj_ctlr_5_x_dbg.conf | 1 + tests/bluetooth/init/prj_ctlr_dbg.conf | 2 ++ 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/subsys/bluetooth/controller/Kconfig.ll_sw_split b/subsys/bluetooth/controller/Kconfig.ll_sw_split index b3cd1e9f8f8e..3cba9553aa10 100644 --- a/subsys/bluetooth/controller/Kconfig.ll_sw_split +++ b/subsys/bluetooth/controller/Kconfig.ll_sw_split @@ -536,6 +536,16 @@ config BT_CTLR_SCAN_ENABLE_STRICT Enforce returning HCI Error Command Disallowed on enabling/disabling already enabled/disabled scanning. +config BT_CTLR_CIS_ACCEPT_MIN_OFFSET_STRICT + bool "Enforce Strict CIS Minimum Offset Check" + depends on BT_CTLR_PERIPHERAL_ISO + help + Enforce strict check of CIS minimum offset accepted by the peripheral + considering that there will be no overlap of ACL connection with the + CIG events. Radio and CPU overheads for an ACL connection event is + considered and checks the CIS minimum offset is greater than the time + reservation for the ACL connection. + config BT_CTLR_ISOAL_SN_STRICT bool "Enforce Strict Tx ISO Data Sequence Number use" depends on !BT_CTLR_ISOAL_PSN_IGNORE && (BT_CTLR_ADV_ISO || \ diff --git a/subsys/bluetooth/controller/ll_sw/ull_peripheral_iso.c b/subsys/bluetooth/controller/ll_sw/ull_peripheral_iso.c index 843bf8779354..b7718a3091f8 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_peripheral_iso.c +++ b/subsys/bluetooth/controller/ll_sw/ull_peripheral_iso.c @@ -104,7 +104,8 @@ uint8_t ll_cis_accept(uint16_t handle) if (conn) { uint32_t cis_offset_min; - if (IS_ENABLED(CONFIG_BT_CTLR_PERIPHERAL_ISO_EARLY_CIG_START)) { + if (IS_ENABLED(CONFIG_BT_CTLR_PERIPHERAL_ISO_EARLY_CIG_START) || + !IS_ENABLED(CONFIG_BT_CTLR_CIS_ACCEPT_MIN_OFFSET_STRICT)) { /* Early start allows offset down to spec defined minimum */ cis_offset_min = CIS_MIN_OFFSET_MIN; } else { diff --git a/tests/bluetooth/init/prj_ctlr_5_x_dbg.conf b/tests/bluetooth/init/prj_ctlr_5_x_dbg.conf index 0436807e8d2a..3e9ba3921a9f 100644 --- a/tests/bluetooth/init/prj_ctlr_5_x_dbg.conf +++ b/tests/bluetooth/init/prj_ctlr_5_x_dbg.conf @@ -39,6 +39,7 @@ CONFIG_BT_CTLR_ADV_RESERVE_MAX=n CONFIG_BT_CTLR_ADV_ISO_RESERVE_MAX=n CONFIG_BT_CTLR_SYNC_ISO_RESERVE_MAX=n CONFIG_BT_CTLR_CENTRAL_RESERVE_MAX=n +CONFIG_BT_CTLR_CIS_ACCEPT_MIN_OFFSET_STRICT=y CONFIG_BT_CTLR_EVENT_OVERHEAD_RESERVE_MAX=n CONFIG_BT_CTLR_PROFILE_ISR=y CONFIG_BT_CTLR_DEBUG_PINS=y diff --git a/tests/bluetooth/init/prj_ctlr_dbg.conf b/tests/bluetooth/init/prj_ctlr_dbg.conf index ac9efeb93389..9f30378a0637 100644 --- a/tests/bluetooth/init/prj_ctlr_dbg.conf +++ b/tests/bluetooth/init/prj_ctlr_dbg.conf @@ -21,6 +21,8 @@ CONFIG_BT_CTLR_LLL_PRIO=0 CONFIG_BT_CTLR_ULL_HIGH_PRIO=1 CONFIG_BT_CTLR_XTAL_ADVANCED=n CONFIG_BT_CTLR_SCHED_ADVANCED=n +CONFIG_BT_CTLR_CIS_ACCEPT_MIN_OFFSET_STRICT=n +CONFIG_BT_CTLR_EVENT_OVERHEAD_RESERVE_MAX=y CONFIG_BT_CTLR_RADIO_ENABLE_FAST=y CONFIG_BT_CTLR_TIFS_HW=n CONFIG_BT_CTLR_TX_RETRY_DISABLE=y From 79bc1effab27fe7fee5fc9d6027eb03f4695fda3 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 9 Feb 2024 15:21:36 +0100 Subject: [PATCH 2/5] Bluetooth: Controller: Workaround peer CIS establishment limitations Some peer controller implementations are not able to establish CIS if the instant is equal to the current event count where CIS_IND PDU is received. Workaroud by having the instant one ahead of the event count where CIS_IND PDU is transmitted. Signed-off-by: Vinayak Kariappa Chettimada --- .../controller/ll_sw/ull_central_iso.c | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_central_iso.c b/subsys/bluetooth/controller/ll_sw/ull_central_iso.c index 7c1c59e4e4c2..46941ec40699 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_central_iso.c +++ b/subsys/bluetooth/controller/ll_sw/ull_central_iso.c @@ -836,8 +836,17 @@ uint8_t ull_central_iso_setup(uint16_t cis_handle, /* ACL connection of the new CIS */ conn = ll_conn_get(cis->lll.acl_handle); + + /* WORKAROUND: + * Some peer controller implementations are not able to establish CIS + * if the instant is equal to the current event count where CIS_IND PDU + * is received. + * ull_conn_event_counter() returns the current event count, CIS_IND PDU + * will be transmitted in the next event, hence add 2 so that the + * instant is one ahead of the event where CIS_IND PDU is transmitted. + */ event_counter = ull_conn_event_counter(conn); - instant = MAX(*conn_event_count, event_counter + 1); + instant = MAX(*conn_event_count, event_counter + 2U); #if defined(CONFIG_BT_CTLR_JIT_SCHEDULING) uint32_t cis_offset; @@ -948,7 +957,15 @@ int ull_central_iso_cis_offset_get(uint16_t cis_handle, conn = ll_conn_get(cis->lll.acl_handle); - cis->central.instant = ull_conn_event_counter(conn) + 3U; + /* WORKAROUND: + * Some peer controller implementations are not able to establish CIS + * if the instant is equal to the current event count where CIS_IND PDU + * is received. + * ull_conn_event_counter() returns the current event count, CIS_REQ PDU + * CIS_RSP PDU and CIS_IND PDU take 3 connection events. Add 1 extra so + * that the instant is one ahead of where CIS_IND PDU is transmitted. + */ + cis->central.instant = ull_conn_event_counter(conn) + 4U; *conn_event_count = cis->central.instant; /* Provide CIS offset range From 165e16c5d37a61725304d27816e4f53c09c330bb Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Mon, 29 Jan 2024 20:45:08 +0100 Subject: [PATCH 3/5] tests: Bluetooth: tester: Do not use PERIPHERAL_ISO_EARLY_CIG_START Do not use PERIPHERAL_ISO_EARLY_CIG_START, instead CIS offset minimum check is relaxed permitting the minimum of 500 us offset to be accepted by our peripheral implementation. Signed-off-by: Vinayak Kariappa Chettimada --- tests/bluetooth/tester/nrf5340_hci_ipc_cpunet.conf | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/bluetooth/tester/nrf5340_hci_ipc_cpunet.conf b/tests/bluetooth/tester/nrf5340_hci_ipc_cpunet.conf index b5b8a8971ceb..5696ebc072d5 100644 --- a/tests/bluetooth/tester/nrf5340_hci_ipc_cpunet.conf +++ b/tests/bluetooth/tester/nrf5340_hci_ipc_cpunet.conf @@ -1,6 +1,6 @@ # Apply this overlay at hci_ipc controller build -CONFIG_BT_CTLR_CONN_ISO_LOW_LATENCY_POLICY=y -CONFIG_BT_CTLR_DATA_LENGTH_MAX=100 CONFIG_BT_BUF_ACL_RX_SIZE=100 -CONFIG_BT_CTLR_PERIPHERAL_ISO_EARLY_CIG_START=y +CONFIG_BT_CTLR_DATA_LENGTH_MAX=100 +CONFIG_BT_CTLR_CONN_ISO_LOW_LATENCY_POLICY=y CONFIG_BT_CTLR_ISOAL_PSN_IGNORE=y +CONFIG_BT_CTLR_PHY_CODED=n From 79eba73405db797d57a13cf0be62b8d870ee6766 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 9 Feb 2024 10:06:13 +0100 Subject: [PATCH 4/5] tests: Bluetooth: tester: Handle reception of valid ISO data only Handle reception of valid ISO data only. Signed-off-by: Vinayak Kariappa Chettimada --- tests/bluetooth/tester/src/btp_bap_broadcast.c | 2 +- tests/bluetooth/tester/src/btp_bap_unicast.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bluetooth/tester/src/btp_bap_broadcast.c b/tests/bluetooth/tester/src/btp_bap_broadcast.c index bd0549e24cb1..ca2b5210bcea 100644 --- a/tests/bluetooth/tester/src/btp_bap_broadcast.c +++ b/tests/bluetooth/tester/src/btp_bap_broadcast.c @@ -166,7 +166,7 @@ static void stream_recv(struct bt_bap_stream *stream, struct btp_bap_broadcast_remote_source *broadcaster; struct btp_bap_broadcast_stream *b_stream = stream_bap_to_broadcast(stream); - if (b_stream->already_sent == false) { + if ((b_stream->already_sent == false) && (info->flags & BT_ISO_FLAGS_VALID)) { /* For now, send just a first packet, to limit the number * of logs and not unnecessarily spam through btp. */ diff --git a/tests/bluetooth/tester/src/btp_bap_unicast.c b/tests/bluetooth/tester/src/btp_bap_unicast.c index 06b9219697c4..1dc909ba2b3e 100644 --- a/tests/bluetooth/tester/src/btp_bap_unicast.c +++ b/tests/bluetooth/tester/src/btp_bap_unicast.c @@ -677,7 +677,7 @@ static void stream_recv(struct bt_bap_stream *stream, { struct btp_bap_unicast_stream *u_stream = stream_bap_to_unicast(stream); - if (u_stream->already_sent == false) { + if ((u_stream->already_sent == false) && (info->flags & BT_ISO_FLAGS_VALID)) { /* For now, send just a first packet, to limit the number * of logs and not unnecessarily spam through btp. */ From ab9dcad525590927f9fa8c034988e261a5fae8d8 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Mon, 12 Feb 2024 15:41:24 +0100 Subject: [PATCH 5/5] Bluetooth: Controller: Fix Central ISO sub_interval calculation Fix Central ISO sub_interval calculation, include active clock jitter in the calculation. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ll_sw/ull_central_iso.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_central_iso.c b/subsys/bluetooth/controller/ll_sw/ull_central_iso.c index 46941ec40699..8497efc0ac28 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_central_iso.c +++ b/subsys/bluetooth/controller/ll_sw/ull_central_iso.c @@ -394,7 +394,7 @@ uint8_t ll_cig_parameters_commit(uint8_t cig_id, uint16_t *handles) mpt_c = PDU_CIS_MAX_US(cis->lll.tx.max_pdu, tx, cis->lll.tx.phy); mpt_p = PDU_CIS_MAX_US(cis->lll.rx.max_pdu, rx, cis->lll.rx.phy); - se[i].length = mpt_c + EVENT_IFS_US + mpt_p + EVENT_MSS_US; + se[i].length = mpt_c + EVENT_IFS_MAX_US + mpt_p + EVENT_MSS_MAX_US; max_se_length = MAX(max_se_length, se[i].length); /* Total number of subevents needed */