Skip to content

Commit

Permalink
Bluetooth: controller: split: correct timing calculation in PKT_US
Browse files Browse the repository at this point in the history 
A bug in the PKT_US resulted in wrong calculations for the 2M phy.
Fixes the bug, verified on EBQ.
Also adds some defines for improved readability.

Fixes zephyrproject-rtos#23482

Signed-off-by: Andries Kruithof <Andries.Kruithof@nordicsemi.no>
  • Loading branch information
@kruithofa @hakehuang
kruithofa authored and hakehuang committed Mar 18, 2020
1 parent 5139980 commit f88c777
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 35 deletions.
4 changes: 2 additions & 2 deletions subsys/bluetooth/controller/ll_sw/ull_adv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,8 @@ u8_t ll_adv_enable(u8_t enable)
/* Use the default 1M packet max time. Value of 0 is
* equivalent to using BIT(0).
*/
conn_lll->max_tx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0);
conn_lll->max_rx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0);
conn_lll->max_tx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M);
conn_lll->max_rx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M);
#endif /* CONFIG_BT_CTLR_PHY */
#endif /* CONFIG_BT_CTLR_DATA_LENGTH */

Expand Down
54 changes: 27 additions & 27 deletions subsys/bluetooth/controller/ll_sw/ull_conn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,12 +486,12 @@ void ll_length_max_get(u16_t *max_tx_octets, u16_t *max_tx_time,
*max_tx_octets = LL_LENGTH_OCTETS_RX_MAX;
*max_rx_octets = LL_LENGTH_OCTETS_RX_MAX;
#if defined(CONFIG_BT_CTLR_PHY)
*max_tx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, BIT(2));
*max_rx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, BIT(2));
*max_tx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_CODED);
*max_rx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_CODED);
#else /* !CONFIG_BT_CTLR_PHY */
/* Default is 1M packet timing */
*max_tx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, 0);
*max_rx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, 0);
*max_tx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_1M);
*max_rx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_1M);
#endif /* !CONFIG_BT_CTLR_PHY */
}
#endif /* CONFIG_BT_CTLR_DATA_LENGTH */
Expand Down Expand Up @@ -1596,7 +1596,7 @@ static int init_reset(void)
#if defined(CONFIG_BT_CTLR_DATA_LENGTH)
/* Initialize the DLE defaults */
default_tx_octets = PDU_DC_PAYLOAD_SIZE_MIN;
default_tx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0);
default_tx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M);
#endif /* CONFIG_BT_CTLR_DATA_LENGTH */

#if defined(CONFIG_BT_CTLR_PHY)
Expand Down Expand Up @@ -3189,31 +3189,31 @@ static inline void dle_max_time_get(const struct ll_conn *conn,

if (!conn->common.fex_valid ||
(!feature_coded_phy && !feature_phy_2m)) {
rx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, 0);
rx_time = PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_1M);
#if defined(CONFIG_BT_CTLR_PHY)
tx_time = MAX(MIN(PKT_US(LL_LENGTH_OCTETS_RX_MAX, 0),
tx_time = MAX(MIN(PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_1M),
conn->default_tx_time),
PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0));
PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M));
#else /* !CONFIG_BT_CTLR_PHY */
tx_time = PKT_US(conn->default_tx_octets, 0);
tx_time = PKT_US(conn->default_tx_octets, PHY_1M);
#endif /* !CONFIG_BT_CTLR_PHY */

#if defined(CONFIG_BT_CTLR_PHY)
#if defined(CONFIG_BT_CTLR_PHY_CODED)
} else if (feature_coded_phy) {
rx_time = MAX(PKT_US(LL_LENGTH_OCTETS_RX_MAX, BIT(2)),
PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, BIT(2)));
tx_time = MIN(PKT_US(LL_LENGTH_OCTETS_RX_MAX, BIT(2)),
rx_time = MAX(PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_CODED),
PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_CODED));
tx_time = MIN(PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_CODED),
conn->default_tx_time);
tx_time = MAX(PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0), tx_time);
tx_time = MAX(PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M), tx_time);
#endif /* CONFIG_BT_CTLR_PHY_CODED */

#if defined(CONFIG_BT_CTLR_PHY_2M)
} else if (feature_phy_2m) {
rx_time = MAX(PKT_US(LL_LENGTH_OCTETS_RX_MAX, BIT(1)),
PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, BIT(1)));
tx_time = MAX(PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0),
MIN(PKT_US(LL_LENGTH_OCTETS_RX_MAX, BIT(1)),
rx_time = MAX(PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_2M),
PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M));
tx_time = MAX(PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M),
MIN(PKT_US(LL_LENGTH_OCTETS_RX_MAX, PHY_2M),
conn->default_tx_time));
#endif /* CONFIG_BT_CTLR_PHY_2M */
#endif /* CONFIG_BT_CTLR_PHY */
Expand Down Expand Up @@ -3355,8 +3355,8 @@ static inline void event_len_prep(struct ll_conn *conn)
lr->max_tx_octets = sys_cpu_to_le16(tx_octets);
#if !defined(CONFIG_BT_CTLR_PHY)
lr->max_rx_time =
sys_cpu_to_le16(PKT_US(lll->max_rx_octets, 0));
lr->max_tx_time = sys_cpu_to_le16(PKT_US(tx_octets, 0));
sys_cpu_to_le16(PKT_US(lll->max_rx_octets, PHY_1M));
lr->max_tx_time = sys_cpu_to_le16(PKT_US(tx_octets, PHY_1M));
#else /* CONFIG_BT_CTLR_PHY */
lr->max_rx_time = sys_cpu_to_le16(lll->max_rx_time);
lr->max_tx_time = sys_cpu_to_le16(tx_time);
Expand Down Expand Up @@ -4303,9 +4303,9 @@ static inline int reject_ind_dle_recv(struct ll_conn *conn,
lr->max_tx_octets = sys_cpu_to_le16(conn->lll.max_tx_octets);
#if !defined(CONFIG_BT_CTLR_PHY)
lr->max_rx_time =
sys_cpu_to_le16(PKT_US(conn->lll.max_rx_octets, 0));
sys_cpu_to_le16(PKT_US(conn->lll.max_rx_octets, PHY_1M));
lr->max_tx_time =
sys_cpu_to_le16(PKT_US(conn->lll.max_tx_octets, 0));
sys_cpu_to_le16(PKT_US(conn->lll.max_tx_octets, PHY_1M));
#else /* CONFIG_BT_CTLR_PHY */
lr->max_rx_time = sys_cpu_to_le16(conn->lll.max_rx_time);
lr->max_tx_time = sys_cpu_to_le16(conn->lll.max_tx_time);
Expand Down Expand Up @@ -4459,9 +4459,9 @@ static void length_resp_send(struct ll_conn *conn, struct node_tx *tx,

#if !defined(CONFIG_BT_CTLR_PHY)
pdu_tx->llctrl.length_rsp.max_rx_time =
sys_cpu_to_le16(PKT_US(eff_rx_octets, 0));
sys_cpu_to_le16(PKT_US(eff_rx_octets, PHY_1M));
pdu_tx->llctrl.length_rsp.max_tx_time =
sys_cpu_to_le16(PKT_US(eff_tx_octets, 0));
sys_cpu_to_le16(PKT_US(eff_tx_octets, PHY_1M));
#else /* CONFIG_BT_CTLR_PHY */
pdu_tx->llctrl.length_rsp.max_rx_time = sys_cpu_to_le16(eff_rx_time);
pdu_tx->llctrl.length_rsp.max_tx_time = sys_cpu_to_le16(eff_tx_time);
Expand Down Expand Up @@ -4559,7 +4559,7 @@ static inline int length_req_rsp_recv(struct ll_conn *conn, memq_link_t *link,
lr_rx_time = sys_le16_to_cpu(lr->max_rx_time);
lr_tx_time = sys_le16_to_cpu(lr->max_tx_time);

if (lr_rx_time >= PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0)) {
if (lr_rx_time >= PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M)) {
eff_tx_time = MIN(lr_rx_time, max_tx_time);
#if defined(CONFIG_BT_CTLR_PHY_CODED)
eff_tx_time = MAX(eff_tx_time,
Expand All @@ -4571,7 +4571,7 @@ static inline int length_req_rsp_recv(struct ll_conn *conn, memq_link_t *link,
/* use the minimal of our max supported and
* peer max_tx_time
*/
if (lr_tx_time >= PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0)) {
if (lr_tx_time >= PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M)) {
eff_rx_time = MIN(lr_tx_time, max_rx_time);
#if defined(CONFIG_BT_CTLR_PHY_CODED)
eff_rx_time = MAX(eff_rx_time,
Expand Down Expand Up @@ -4675,9 +4675,9 @@ static inline int length_req_rsp_recv(struct ll_conn *conn, memq_link_t *link,

#if !defined(CONFIG_BT_CTLR_PHY)
lr->max_rx_time =
sys_cpu_to_le16(PKT_US(eff_rx_octets, 0));
sys_cpu_to_le16(PKT_US(eff_rx_octets, PHY_1M));
lr->max_tx_time =
sys_cpu_to_le16(PKT_US(eff_tx_octets, 0));
sys_cpu_to_le16(PKT_US(eff_tx_octets, PHY_1M));
#else /* CONFIG_BT_CTLR_PHY */
lr->max_rx_time = sys_cpu_to_le16(eff_rx_time);
lr->max_tx_time = sys_cpu_to_le16(eff_tx_time);
Expand Down
11 changes: 7 additions & 4 deletions subsys/bluetooth/controller/ll_sw/ull_conn_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
*/
#define PAYLOAD_OVERHEAD_SIZE (2 + 4)

#define PHY_1M BIT(0)
#define PHY_2M BIT(1)
#define PHY_CODED BIT(2)
#if defined(CONFIG_BT_CTLR_PHY_CODED)
#define CODED_PHY_PREAMBLE_TIME_US (80)
#define CODED_PHY_ACCESS_ADDRESS_TIME_US (256)
Expand All @@ -33,23 +36,23 @@
CODED_PHY_CRC_SIZE + \
CODED_PHY_TERM2_SIZE) * 8)

#define PKT_US(octets, phy) (((phy) & BIT(2)) ? \
#define PKT_US(octets, phy) (((phy) & PHY_CODED) ? \
(CODED_PHY_PREAMBLE_TIME_US + \
FEC_BLOCK1_TIME_US + \
FEC_BLOCK2_TIME_US(octets)) : \
(((PREAMBLE_SIZE(1) + \
(((PREAMBLE_SIZE(phy) + \
ACCESS_ADDR_SIZE + \
PAYLOAD_OVERHEAD_SIZE + \
(octets) + \
CRC_SIZE) * 8) / \
BIT(((phy) & 0x03) >> 1)))
#else /* !CONFIG_BT_CTLR_PHY_CODED */
#define PKT_US(octets, phy) (((PREAMBLE_SIZE(1) + \
#define PKT_US(octets, phy) ((((PREAMBLE_SIZE(phy)) + \
ACCESS_ADDR_SIZE + \
PAYLOAD_OVERHEAD_SIZE + \
(octets) + \
CRC_SIZE) * 8) / \
BIT(((phy) & 0x03) >> 1))
BIT(((phy) & 0x03) >> 1))
#endif /* !CONFIG_BT_CTLR_PHY_CODED */

struct ll_conn *ll_conn_acquire(void);
Expand Down
4 changes: 2 additions & 2 deletions subsys/bluetooth/controller/ll_sw/ull_master.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ u8_t ll_create_connection(u16_t scan_interval, u16_t scan_window,
conn_lll->max_rx_octets = PDU_DC_PAYLOAD_SIZE_MIN;

#if defined(CONFIG_BT_CTLR_PHY)
conn_lll->max_tx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0);
conn_lll->max_rx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, 0);
conn_lll->max_tx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M);
conn_lll->max_rx_time = PKT_US(PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M);
#endif /* CONFIG_BT_CTLR_PHY */
#endif /* CONFIG_BT_CTLR_DATA_LENGTH */

Expand Down

0 comments on commit f88c777

Please sign in to comment.