Skip to content

Commit

Permalink
Bluetooth: Controller: Fix endianness issues for SyncInfo
Browse files Browse the repository at this point in the history 
Fix bitfield crossing byte boundary - replaced with macros for
setting/getting the values

Fix missing endianness conversion for evt_cntr

Changed aa from uint32_t to uint8_t[4] to align with the rest of
the code and avoid any potential endianness issues

Signed-off-by: Troels Nilsson <[email protected]>
  • Loading branch information
@Tronil
Tronil committed Oct 10, 2023
1 parent 493fe16 commit 45ea3e7
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 34 deletions.
8 changes: 4 additions & 4 deletions subsys/bluetooth/controller/hci/hci.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7053,10 +7053,10 @@ static void le_ext_adv_report(struct pdu_data *pdu_data,
LOG_DBG(" SyncInfo offs = %u, offs_unit = 0x%x, "
"interval = 0x%x, sca = 0x%x, "
"chan map = 0x%x 0x%x 0x%x 0x%x 0x%x, "
"AA = 0x%x, CRC = 0x%x 0x%x 0x%x, "
"AA = 0x%x%x%x%x, CRC = 0x%x 0x%x 0x%x, "
"evt cntr = 0x%x",
sys_le16_to_cpu(si->offs),
si->offs_units,
PDU_ADV_SYNC_INFO_OFFSET_GET(si),
PDU_ADV_SYNC_INFO_OFFS_UNITS_GET(si),
sys_le16_to_cpu(si->interval),
((si->sca_chm[PDU_SYNC_INFO_SCA_CHM_SCA_BYTE_OFFSET] &
PDU_SYNC_INFO_SCA_CHM_SCA_BIT_MASK) >>
Expand All @@ -7065,7 +7065,7 @@ static void le_ext_adv_report(struct pdu_data *pdu_data,
si->sca_chm[3],
(si->sca_chm[PDU_SYNC_INFO_SCA_CHM_SCA_BYTE_OFFSET] &
~PDU_SYNC_INFO_SCA_CHM_SCA_BIT_MASK),
sys_le32_to_cpu(si->aa),
si->aa[3], si->aa[2], si->aa[1], si->aa[0],
si->crc_init[0], si->crc_init[1],
si->crc_init[2], sys_le16_to_cpu(si->evt_cntr));
}
Expand Down
34 changes: 22 additions & 12 deletions subsys/bluetooth/controller/ll_sw/pdu.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,19 @@
(((aux_ptr)->offs_phy_packed[1] & 0x1F) << 8))
#define PDU_ADV_AUX_PTR_PHY_GET(aux_ptr) (((aux_ptr)->offs_phy_packed[1] >> 5) & 0x07)

/* Macros for getting/setting offset/offset_units/offset_adjust from pdu_adv_sync_info */
#define PDU_ADV_SYNC_INFO_OFFSET_GET(si) ((si)->offs_packed[0] | \
(((si)->offs_packed[1] & 0x1F) << 8))
#define PDU_ADV_SYNC_INFO_OFFS_UNITS_GET(si) (((si)->offs_packed[1] >> 5) & 0x01)
#define PDU_ADV_SYNC_INFO_OFFS_ADJUST_GET(si) (((si)->offs_packed[1] >> 6) & 0x01)
#define PDU_ADV_SYNC_INFO_OFFS_SET(si, offs, offs_units, offs_adjust) \
do { \
(si)->offs_packed[0] = (offs) & 0xFF; \
(si)->offs_packed[1] = (((offs) >> 8) & 0x1F) + \
(((offs_units) << 5) & 0x20) + \
(((offs_adjust) << 6) & 0x40); \
} while (0)

/* Advertiser's Sleep Clock Accuracy Value */
#define SCA_500_PPM 500 /* 51 ppm to 500 ppm */
#define SCA_50_PPM 50 /* 0 ppm to 50 ppm */
Expand Down Expand Up @@ -488,20 +501,17 @@ enum pdu_adv_aux_phy {
};

struct pdu_adv_sync_info {
#ifdef CONFIG_LITTLE_ENDIAN
uint16_t offs:13;
uint16_t offs_units:1;
uint16_t offs_adjust:1;
uint16_t rfu:1;
#else
uint16_t rfu:1;
uint16_t offs_adjust:1;
uint16_t offs_units:1;
uint16_t offs:13;
#endif /* CONFIG_LITTLE_ENDIAN */
/* offs:13
* offs_units:1
* offs_adjust:1
* rfu:1
* NOTE: This layout as bitfields is not portable for BE using
* endianness conversion macros.
*/
uint8_t offs_packed[2];
uint16_t interval;
uint8_t sca_chm[PDU_CHANNEL_MAP_SIZE];
uint32_t aa;
uint8_t aa[4];
uint8_t crc_init[3];
uint16_t evt_cntr;
} __packed;
Expand Down
25 changes: 12 additions & 13 deletions subsys/bluetooth/controller/ll_sw/ull_adv_sync.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1348,9 +1348,7 @@ void ull_adv_sync_info_fill(struct ll_adv_sync_set *sync,
* If sync_info is part of ADV PDU the offs_adjust field
* is always set to 0.
*/
si->offs_units = OFFS_UNIT_VALUE_30_US;
si->offs_adjust = 0U;
si->offs = 0U;
PDU_ADV_SYNC_INFO_OFFS_SET(si, 0U, OFFS_UNIT_VALUE_30_US, 0U);

/* Fill the interval, access address and CRC init */
si->interval = sys_cpu_to_le16(sync->interval);
Expand Down Expand Up @@ -2207,18 +2205,19 @@ void ull_adv_sync_lll_syncinfo_fill(struct pdu_adv *pdu, struct lll_adv_aux *lll

static void sync_info_offset_fill(struct pdu_adv_sync_info *si, uint32_t offs)
{
uint8_t offs_adjust = 0U;

if (offs >= OFFS_ADJUST_US) {
offs -= OFFS_ADJUST_US;
si->offs_adjust = 1U;
offs_adjust = 1U;
}

offs = offs / OFFS_UNIT_30_US;
if (!!(offs >> OFFS_UNIT_BITS)) {
si->offs = sys_cpu_to_le16(offs / (OFFS_UNIT_300_US / OFFS_UNIT_30_US));
si->offs_units = OFFS_UNIT_VALUE_300_US;
PDU_ADV_SYNC_INFO_OFFS_SET(si, offs / (OFFS_UNIT_300_US / OFFS_UNIT_30_US),
OFFS_UNIT_VALUE_300_US, offs_adjust);
} else {
si->offs = sys_cpu_to_le16(offs);
si->offs_units = OFFS_UNIT_VALUE_30_US;
PDU_ADV_SYNC_INFO_OFFS_SET(si, offs, OFFS_UNIT_VALUE_30_US, offs_adjust);
}
}

Expand Down Expand Up @@ -2328,22 +2327,22 @@ static void sync_info_offset_fill(struct pdu_adv_sync_info *si,
uint32_t remainder_us,
uint32_t start_us)
{
uint8_t offs_adjust = 0U;
uint32_t offs;

offs = HAL_TICKER_TICKS_TO_US(ticks_offset) + remainder_us - start_us;

if (offs >= OFFS_ADJUST_US) {
offs -= OFFS_ADJUST_US;
si->offs_adjust = 1U;
offs_adjust = 1U;
}

offs = offs / OFFS_UNIT_30_US;
if (!!(offs >> OFFS_UNIT_BITS)) {
si->offs = sys_cpu_to_le16(offs / (OFFS_UNIT_300_US / OFFS_UNIT_30_US));
si->offs_units = OFFS_UNIT_VALUE_300_US;
PDU_ADV_SYNC_INFO_OFFS_SET(si, offs / (OFFS_UNIT_300_US / OFFS_UNIT_30_US),
OFFS_UNIT_VALUE_300_US, offs_adjust);
} else {
si->offs = sys_cpu_to_le16(offs);
si->offs_units = OFFS_UNIT_VALUE_30_US;
PDU_ADV_SYNC_INFO_OFFS_SET(si, offs, OFFS_UNIT_VALUE_30_US, offs_adjust);
}
}

Expand Down
10 changes: 5 additions & 5 deletions subsys/bluetooth/controller/ll_sw/ull_sync.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,10 +711,10 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux,
* CONFIG_BT_CTLR_SYNC_PERIODIC_ADI_SUPPORT
*/

memcpy(lll->access_addr, &si->aa, sizeof(lll->access_addr));
memcpy(lll->access_addr, si->aa, sizeof(lll->access_addr));
lll->data_chan_id = lll_chan_id(lll->access_addr);
memcpy(lll->crc_init, si->crc_init, sizeof(lll->crc_init));
lll->event_counter = si->evt_cntr;
lll->event_counter = sys_le16_to_cpu(si->evt_cntr);
lll->phy = aux->lll.phy;

interval = sys_le16_to_cpu(si->interval);
Expand Down Expand Up @@ -762,7 +762,7 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux,
lll_clock_ppm_get(sca)) *
interval_us), USEC_PER_SEC);
lll->window_widening_max_us = (interval_us >> 1) - EVENT_IFS_US;
if (si->offs_units) {
if (PDU_ADV_SYNC_INFO_OFFS_UNITS_GET(si)) {
lll->window_size_event_us = OFFS_UNIT_300_US;
} else {
lll->window_size_event_us = OFFS_UNIT_30_US;
Expand Down Expand Up @@ -809,10 +809,10 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux,
ready_delay_us = lll_radio_rx_ready_delay_get(lll->phy, 1);

sync_offset_us = ftr->radio_end_us;
sync_offset_us += (uint32_t)sys_le16_to_cpu(si->offs) *
sync_offset_us += PDU_ADV_SYNC_INFO_OFFSET_GET(si) *
lll->window_size_event_us;
/* offs_adjust may be 1 only if sync setup by LL_PERIODIC_SYNC_IND */
sync_offset_us += (si->offs_adjust ? OFFS_ADJUST_US : 0U);
sync_offset_us += (PDU_ADV_SYNC_INFO_OFFS_ADJUST_GET(si) ? OFFS_ADJUST_US : 0U);
sync_offset_us -= PDU_AC_US(pdu->len, lll->phy, ftr->phy_flags);
sync_offset_us -= EVENT_TICKER_RES_MARGIN_US;
sync_offset_us -= EVENT_JITTER_US;
Expand Down

0 comments on commit 45ea3e7

Please sign in to comment.