michaelh/zephyr
1
0
Fork 0
Code Releases Activity

Bluetooth: controller: Fixup Broadcast ISO pre-transmission groups > 1

Browse source 
Fixup payload_index to be uint16_t to avoid index overflow.

Do not remember why ptc is 4 bits, where as it must be 5 bit
value similar to nse; added an assertion check until it is
fixed.

Fix ISO Broadcaster and ISO Sync Receiver for PTO > 1 and
use of Pre-Transmission Group Counts.

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
This commit is contained in:
Vinayak Kariappa Chettimada 2025-01-08 21:30:10 +01:00 • committed by Benjamin Cabé
commit 363c1431dd
4 changed files with 66 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

28
subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_iso.c
View file

@ -625,26 +625,36 @@ static void isr_tx_common(void *param,
/* Get ISO data PDU, not control subevent */ /* Get ISO data PDU, not control subevent */
if (!pdu) { if (!pdu) {
uint8_t payload_index; uint16_t payload_index;
if (lll->ptc_curr) { if (lll->ptc_curr) {
uint8_t ptx_idx = lll->ptc_curr - 1; /* FIXME: Do not remember why ptc is 4 bits, it should be 5 bits as ptc is a
* running buffer offset related to nse.
* Fix ptc and ptc_curr definitions, until then there is an assertion
* check when ptc is calculated in ptc_calc function.
*/
uint8_t ptx_idx = lll->ptc_curr - 1U; /* max. nse 5 bits */
uint8_t ptx_payload_idx; uint8_t ptx_payload_idx;
uint32_t ptx_group_mult; uint16_t ptx_group_mult;
uint8_t ptx_group_idx; uint8_t ptx_group_idx;
/* Calculate group index and multiplier for deriving /* Calculate group index and multiplier for deriving
* pre-transmission payload index. * pre-transmission payload index.
*/ */
ptx_group_idx = ptx_idx / lll->bn; ptx_group_idx = ptx_idx / lll->bn; /* 5 bits */
ptx_payload_idx = ptx_idx - ptx_group_idx * lll->bn; ptx_payload_idx = ptx_idx - ptx_group_idx * lll->bn; /* 8 bits */
ptx_group_mult = (ptx_group_idx + 1) * lll->pto; ptx_group_mult = (ptx_group_idx + 1U) * lll->pto; /* 9 bits */
payload_index = ptx_payload_idx + ptx_group_mult * lll->bn; payload_index = ptx_payload_idx + ptx_group_mult * lll->bn; /* 13 bits */
/* FIXME: memq_peek_n function does not support indices > UINT8_MAX,
* add assertion check to honor this limitation.
*/
LL_ASSERT(payload_index <= UINT8_MAX);
} else { } else {
payload_index = lll->bn_curr - 1U; payload_index = lll->bn_curr - 1U; /* 3 bits */
} }
payload_count = lll->payload_count - lll->bn + payload_index; payload_count = lll->payload_count + payload_index - lll->bn;
#if !TEST_WITH_DUMMY_PDU #if !TEST_WITH_DUMMY_PDU
struct lll_adv_iso_stream *stream; struct lll_adv_iso_stream *stream;

39
subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c
View file

@ -52,6 +52,7 @@ static void isr_rx_estab(void *param);
static void isr_rx(void *param); static void isr_rx(void *param);
static void isr_rx_done(void *param); static void isr_rx_done(void *param);
static void isr_done(void *param); static void isr_done(void *param);
static uint16_t payload_index_get(const struct lll_sync_iso *lll);
static void next_chan_calc(struct lll_sync_iso *lll, uint16_t event_counter, static void next_chan_calc(struct lll_sync_iso *lll, uint16_t event_counter,
uint16_t data_chan_id); uint16_t data_chan_id);
static void isr_rx_iso_data_valid(const struct lll_sync_iso *const lll, static void isr_rx_iso_data_valid(const struct lll_sync_iso *const lll,
@ -659,8 +660,7 @@ static void isr_rx(void *param)
* Ensure we are not having offset values over 255 in payload_count_max, used to * Ensure we are not having offset values over 255 in payload_count_max, used to
* allocate the buffers. * allocate the buffers.
*/ */
payload_offset = (lll->latency_event * lll->bn) + (lll->bn_curr - 1U) + payload_offset = (lll->latency_event * lll->bn) + payload_index_get(lll);
(lll->ptc_curr * lll->pto);
if (payload_offset >= lll->payload_count_max) { if (payload_offset >= lll->payload_count_max) {
goto isr_rx_done; goto isr_rx_done;
} }
@ -1031,8 +1031,7 @@ isr_rx_next_subevent:
if (bis) { if (bis) {
struct node_rx_pdu *node_rx; struct node_rx_pdu *node_rx;
payload_count += (lll->bn_curr - 1U) + payload_count += payload_index_get(lll);
(lll->ptc_curr * lll->pto);
/* By design, there shall always be one free node rx /* By design, there shall always be one free node rx
* available for setting up radio for new PDU reception. * available for setting up radio for new PDU reception.
@ -1336,6 +1335,35 @@ static void isr_done(void *param)
} }
} }
static uint16_t payload_index_get(const struct lll_sync_iso *lll)
{
uint16_t payload_index;
if (lll->ptc_curr) {
/* FIXME: Do not remember why ptc is 4 bits, it should be 5 bits as ptc is a
* running buffer offset related to nse.
* Fix ptc and ptc_curr definitions, until then there is an assertion
* check when ptc is calculated in ptc_calc function.
*/
uint8_t ptx_idx = lll->ptc_curr - 1U; /* max. nse 5 bits */
uint8_t ptx_payload_idx;
uint16_t ptx_group_mult;
uint8_t ptx_group_idx;
/* Calculate group index and multiplier for deriving
* pre-transmission payload index.
*/
ptx_group_idx = ptx_idx / lll->bn; /* 5 bits */
ptx_payload_idx = ptx_idx - ptx_group_idx * lll->bn; /* 8 bits */
ptx_group_mult = (ptx_group_idx + 1U) * lll->pto; /* 9 bits */
payload_index = ptx_payload_idx + ptx_group_mult * lll->bn; /* 13 bits */
} else {
payload_index = lll->bn_curr - 1U; /* 3 bits */
}
return payload_index;
}
static void next_chan_calc(struct lll_sync_iso *lll, uint16_t event_counter, static void next_chan_calc(struct lll_sync_iso *lll, uint16_t event_counter,
uint16_t data_chan_id) uint16_t data_chan_id)
{ {
@ -1377,8 +1405,7 @@ static void isr_rx_iso_data_valid(const struct lll_sync_iso *const lll,
node_rx->hdr.handle = handle; node_rx->hdr.handle = handle;
iso_meta = &node_rx->rx_iso_meta; iso_meta = &node_rx->rx_iso_meta;
iso_meta->payload_number = lll->payload_count + (lll->bn_curr - 1U) + iso_meta->payload_number = lll->payload_count + payload_index_get(lll);
(lll->ptc_curr * lll->pto);
/* Decrement BN as payload_count was pre-incremented */ /* Decrement BN as payload_count was pre-incremented */
iso_meta->payload_number -= lll->bn; iso_meta->payload_number -= lll->bn;

8
subsys/bluetooth/controller/ll_sw/ull_adv_iso.c
View file

@ -212,7 +212,7 @@ static uint8_t big_create(uint8_t big_handle, uint8_t adv_handle, uint8_t num_bi
return BT_HCI_ERR_INVALID_PARAM; return BT_HCI_ERR_INVALID_PARAM;
} }
if (!IN_RANGE(pto, 0x00, 0x0F)) { if (pto > 0x0F) {
return BT_HCI_ERR_INVALID_PARAM; return BT_HCI_ERR_INVALID_PARAM;
} }
@ -1211,6 +1211,12 @@ static uint8_t ptc_calc(const struct lll_adv_iso *lll, uint32_t event_spacing,
ptc = MIN(ptc, (lll->bn * BT_CTLR_ADV_ISO_PTO_GROUP_COUNT)); ptc = MIN(ptc, (lll->bn * BT_CTLR_ADV_ISO_PTO_GROUP_COUNT));
} }
/* FIXME: Do not remember why ptc is 4 bits, it should be 5 bits as ptc is a
* running buffer offset related to nse. Fix ptc and ptc_curr definitions,
* until then lets have an assert check here.
*/
LL_ASSERT(ptc <= BIT_MASK(4));
return ptc; return ptc;
} }

7
subsys/bluetooth/controller/ll_sw/ull_sync_iso.c
View file

@ -494,6 +494,13 @@ void ull_sync_iso_setup(struct ll_sync_iso_set *sync_iso,
} }
lll->ptc = lll->nse - nse; lll->ptc = lll->nse - nse;
/* FIXME: Do not remember why ptc is 4 bits, it should be 5 bits as ptc is a
* running buffer offset related to nse.
* Fix ptc and ptc_curr definitions, until then we keep an assertion check
* here.
*/
LL_ASSERT(lll->ptc <= BIT_MASK(4));
} else { } else {
lll->ptc = 0U; lll->ptc = 0U;
} }