Skip to content

Bluetooth: controller: nRF5: Back-to-Back Radio Tx interface #33118

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 22, 2021
Merged

Bluetooth: controller: nRF5: Back-to-Back Radio Tx interface #33118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0471175
Bluetooth: controller: Enclose macro parameters in paranthesis
cvinayak Mar 19, 2021
81b912a
Bluetooth: controller: Minor indentation and HAL code change
cvinayak Mar 19, 2021
d5c621e
Bluetooth: controller: nRF53 sw_switch always required Radio End
cvinayak Mar 19, 2021
f597fa4
Bluetooth: controller: nRF5: Back-to-Back Radio Tx interface
cvinayak Mar 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 99 additions & 35 deletions subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,28 +489,55 @@ void *radio_pkt_decrypt_get(void)
#endif

#if !defined(CONFIG_BT_CTLR_TIFS_HW)

#define SW_SWITCH_PREV_RX 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SW_SWITCH_DIR_* used for both TX and RX would be less confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are aliases to hardcoded 0 or 1, used to convey the parameter name supplied to inlined sw_switch file static function. They are not intended to be used as parameter values to other functions.

Do you mean to rename to

Suggested change
#define SW_SWITCH_PREV_RX 0
#define SW_SWITCH_PREV_DIR_RX 0
#define SW_SWITCH_NEXT_DIR_RX 0
#define SW_SWITCH_PREV_DIR_TX 1
#define SW_SWITCH_NEXT_DIR_TX 1

#define SW_SWITCH_NEXT_RX 0
#define SW_SWITCH_PREV_TX 1
#define SW_SWITCH_NEXT_TX 1
#define SW_SWITCH_PREV_PHY_1M 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better use PHY_LEGACY or even better PHY_1M instead since this is actual value passed from LLL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, macros are used as readable string (instead of using defines) which would convey the parameters purpose. Otherwise, the call at line 607 will look like:

sw_switch(SW_SWITCH_DIR_TX, SW_SWITCH_DIR_RX,
		  PHY_1M, SW_SWITCH_PREV_FLAGS_DONTCARE,
		  phy_rx, SW_SWITCH_NEXT_FLAGS_DONTCARE);

#define SW_SWITCH_PREV_FLAGS_DONTCARE 0
#define SW_SWITCH_NEXT_FLAGS_DONTCARE 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be defined in pdu.h and eventually also used in LLL since flags are magic values there as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again the macros are replacement to hardcoded values passed to sw_switch function which is vendor specific interface, not sure if other vendors will use 0 as the dont care values.

Change other magic values is outside the scope of this PR.


static uint8_t sw_tifs_toggle;

static void sw_switch(uint8_t dir, uint8_t phy_curr, uint8_t flags_curr, uint8_t phy_next,
uint8_t flags_next)
static inline void sw_switch(uint8_t dir_curr, uint8_t dir_next,
uint8_t phy_curr, uint8_t flags_curr,
uint8_t phy_next, uint8_t flags_next)
{
uint8_t ppi = HAL_SW_SWITCH_RADIO_ENABLE_PPI(sw_tifs_toggle);
uint8_t cc = SW_SWITCH_TIMER_EVTS_COMP(sw_tifs_toggle);
uint32_t delay;

hal_radio_sw_switch_setup(cc, ppi, sw_tifs_toggle);

if (dir) {
/* NOTE: As constants are passed to dir_curr and dir_next, the
* compiler should optimize out the redundant code path as
* this is an inline function.
*/
if (dir_next) {
/* TX */

/* Calculate delay with respect to current (RX) and next
* (TX) PHY. If RX PHY is LE Coded, assume S8 coding scheme.
/* Calculate delay with respect to current and next PHY.
*/
delay = HAL_RADIO_NS2US_ROUND(
hal_radio_tx_ready_delay_ns_get(phy_next, flags_next) +
hal_radio_rx_chain_delay_ns_get(phy_curr, 1));

hal_radio_txen_on_sw_switch(ppi);
if (dir_curr) {
delay = HAL_RADIO_NS2US_ROUND(
hal_radio_tx_ready_delay_ns_get(phy_next,
flags_next) +
hal_radio_tx_chain_delay_ns_get(phy_curr,
flags_curr));

hal_radio_b2b_txen_on_sw_switch(ppi);
} else {
/* If RX PHY is LE Coded, calculate for S8 coding.
* Assumption being, S8 has higher delay.
*/
delay = HAL_RADIO_NS2US_ROUND(
hal_radio_tx_ready_delay_ns_get(phy_next,
flags_next) +
hal_radio_rx_chain_delay_ns_get(phy_curr, 1));

hal_radio_txen_on_sw_switch(ppi);
}

#if defined(CONFIG_BT_CTLR_PHY_CODED)
#if defined(CONFIG_HAS_HW_NRF_RADIO_BLE_CODED)
Expand All @@ -520,7 +547,7 @@ static void sw_switch(uint8_t dir, uint8_t phy_curr, uint8_t flags_curr, uint8_t
HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI(
sw_tifs_toggle);

if (phy_curr & PHY_CODED) {
if (!dir_curr && (phy_curr & PHY_CODED)) {
/* Switching to TX after RX on LE Coded PHY. */

uint8_t cc_s2 =
Expand All @@ -530,12 +557,11 @@ static void sw_switch(uint8_t dir, uint8_t phy_curr, uint8_t flags_curr, uint8_t

/* Calculate assuming reception on S2 coding scheme. */
delay_s2 = HAL_RADIO_NS2US_ROUND(
hal_radio_tx_ready_delay_ns_get(
phy_next, flags_next) +
hal_radio_tx_ready_delay_ns_get(phy_next,
flags_next) +
hal_radio_rx_chain_delay_ns_get(phy_curr, 0));

SW_SWITCH_TIMER->CC[cc_s2] =
SW_SWITCH_TIMER->CC[cc];
SW_SWITCH_TIMER->CC[cc_s2] = SW_SWITCH_TIMER->CC[cc];

if (delay_s2 < SW_SWITCH_TIMER->CC[cc_s2]) {
SW_SWITCH_TIMER->CC[cc_s2] -= delay_s2;
Expand All @@ -545,7 +571,8 @@ static void sw_switch(uint8_t dir, uint8_t phy_curr, uint8_t flags_curr, uint8_t

hal_radio_sw_switch_coded_tx_config_set(ppi_en, ppi_dis,
cc_s2, sw_tifs_toggle);
} else {

} else if (!dir_curr) {
/* Switching to TX after RX on LE 1M/2M PHY */

hal_radio_sw_switch_coded_config_clear(ppi_en,
Expand All @@ -564,12 +591,10 @@ static void sw_switch(uint8_t dir, uint8_t phy_curr, uint8_t flags_curr, uint8_t
#if defined(CONFIG_BT_CTLR_PHY_CODED)
#if defined(CONFIG_HAS_HW_NRF_RADIO_BLE_CODED)
if (1) {
uint8_t ppi_en =
HAL_SW_SWITCH_RADIO_ENABLE_S2_PPI(
sw_tifs_toggle);
uint8_t ppi_dis =
HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI(
sw_tifs_toggle);
uint8_t ppi_en = HAL_SW_SWITCH_RADIO_ENABLE_S2_PPI(
sw_tifs_toggle);
uint8_t ppi_dis = HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI(
sw_tifs_toggle);

hal_radio_sw_switch_coded_config_clear(ppi_en,
ppi_dis, cc, sw_tifs_toggle);
Expand All @@ -578,26 +603,32 @@ static void sw_switch(uint8_t dir, uint8_t phy_curr, uint8_t flags_curr, uint8_t
#endif /* CONFIG_BT_CTLR_PHY_CODED */
}

if (delay <
SW_SWITCH_TIMER->CC[cc]) {
if (delay < SW_SWITCH_TIMER->CC[cc]) {
nrf_timer_cc_set(SW_SWITCH_TIMER, cc,
SW_SWITCH_TIMER->CC[cc] - delay);
(SW_SWITCH_TIMER->CC[cc] - delay));
} else {
nrf_timer_cc_set(SW_SWITCH_TIMER, cc, 1);
}

hal_radio_nrf_ppi_channels_enable(BIT(HAL_SW_SWITCH_TIMER_CLEAR_PPI) |
BIT(HAL_SW_SWITCH_GROUP_TASK_ENABLE_PPI));

#if defined(CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER)
/* Since the event timer is cleared on END, we
* always need to capture the PDU END time-stamp.
#if defined(CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER) || \
defined(CONFIG_SOC_SERIES_NRF53X)
/* NOTE: nRF5340 shares the DPPI channel being triggered by Radio End
* for End time capture and sw_switch DPPI channel toggling hence
* always need to capture End time. Or when using single event
* timer since the timer is cleared on Radio End, we always need
* to capture the Radio End time-stamp.
*/
radio_tmr_end_capture();
#endif /* CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER */
hal_radio_end_time_capture_ppi_config();
#if !defined(CONFIG_SOC_SERIES_NRF53X)
hal_radio_nrf_ppi_channels_enable(BIT(HAL_RADIO_END_TIME_CAPTURE_PPI));
#endif /* !CONFIG_SOC_SERIES_NRF53X */
#endif /* CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER || CONFIG_SOC_SERIES_NRF53X */

sw_tifs_toggle += 1U;
sw_tifs_toggle &= 1;
sw_tifs_toggle &= 1U;
}
#endif /* CONFIG_BT_CTLR_TIFS_HW */

Expand All @@ -610,12 +641,19 @@ void radio_switch_complete_and_rx(uint8_t phy_rx)
#else /* !CONFIG_BT_CTLR_TIFS_HW */
NRF_RADIO->SHORTS = RADIO_SHORTS_READY_START_Msk |
RADIO_SHORTS_END_DISABLE_Msk;
sw_switch(0, 0, 0, phy_rx, 0);

/* NOTE: As Tx chain delays are negligible constant values (~1 us)
* across nRF5x radios, sw_switch assumes the 1M chain delay for
* calculations.
*/
sw_switch(SW_SWITCH_PREV_TX, SW_SWITCH_NEXT_RX,
SW_SWITCH_PREV_PHY_1M, SW_SWITCH_PREV_FLAGS_DONTCARE,
phy_rx, SW_SWITCH_NEXT_FLAGS_DONTCARE);
#endif /* !CONFIG_BT_CTLR_TIFS_HW */
}

void radio_switch_complete_and_tx(uint8_t phy_rx, uint8_t flags_rx, uint8_t phy_tx,
uint8_t flags_tx)
void radio_switch_complete_and_tx(uint8_t phy_rx, uint8_t flags_rx,
uint8_t phy_tx, uint8_t flags_tx)
{
#if defined(CONFIG_BT_CTLR_TIFS_HW)
NRF_RADIO->SHORTS = RADIO_SHORTS_READY_START_Msk |
Expand All @@ -624,7 +662,25 @@ void radio_switch_complete_and_tx(uint8_t phy_rx, uint8_t flags_rx, uint8_t phy_
#else /* !CONFIG_BT_CTLR_TIFS_HW */
NRF_RADIO->SHORTS = RADIO_SHORTS_READY_START_Msk |
RADIO_SHORTS_END_DISABLE_Msk;
sw_switch(1, phy_rx, flags_rx, phy_tx, flags_tx);

sw_switch(SW_SWITCH_PREV_RX, SW_SWITCH_NEXT_TX,
phy_rx, flags_rx, phy_tx, flags_tx);
#endif /* !CONFIG_BT_CTLR_TIFS_HW */
}

void radio_switch_complete_and_b2b_tx(uint8_t phy_curr, uint8_t flags_curr,
uint8_t phy_next, uint8_t flags_next)
{
#if defined(CONFIG_BT_CTLR_TIFS_HW)
NRF_RADIO->SHORTS = RADIO_SHORTS_READY_START_Msk |
RADIO_SHORTS_END_DISABLE_Msk |
RADIO_SHORTS_DISABLED_TXEN_Msk;
#else /* !CONFIG_BT_CTLR_TIFS_HW */
NRF_RADIO->SHORTS = RADIO_SHORTS_READY_START_Msk |
RADIO_SHORTS_END_DISABLE_Msk;

sw_switch(SW_SWITCH_PREV_TX, SW_SWITCH_NEXT_TX,
phy_curr, flags_curr, phy_next, flags_next);
#endif /* !CONFIG_BT_CTLR_TIFS_HW */
}

Expand Down Expand Up @@ -954,8 +1010,16 @@ uint32_t radio_tmr_ready_get(void)

void radio_tmr_end_capture(void)
{
/* NOTE: nRF5340 shares the DPPI channel being triggered by Radio End
* for End time capture and sw_switch DPPI channel toggling hence
* always need to capture End time. Hence, the below code is
* present in hal_sw_switch_timer_clear_ppi_config() and
* sw_switch().
*/
#if defined(CONFIG_BT_CTLR_TIFS_HW) || !defined(CONFIG_SOC_SERIES_NRF53X)
hal_radio_end_time_capture_ppi_config();
hal_radio_nrf_ppi_channels_enable(BIT(HAL_RADIO_END_TIME_CAPTURE_PPI));
#endif /* CONFIG_BT_CTLR_TIFS_HW || !CONFIG_SOC_SERIES_NRF53X */
}

uint32_t radio_tmr_end_get(void)
Expand Down
2 changes: 2 additions & 0 deletions subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ void *radio_pkt_decrypt_get(void);
void radio_switch_complete_and_rx(uint8_t phy_rx);
void radio_switch_complete_and_tx(uint8_t phy_rx, uint8_t flags_rx, uint8_t phy_tx,
uint8_t flags_tx);
void radio_switch_complete_and_b2b_tx(uint8_t phy_curr, uint8_t flags_curr,
uint8_t phy_next, uint8_t flags_next);
void radio_switch_complete_and_disable(void);

void radio_rssi_measure(void);
Expand Down
46 changes: 32 additions & 14 deletions subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_dppi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ static inline void hal_radio_enable_on_tick_ppi_config_and_enable(uint8_t trx)
}
}

nrf_dppi_channels_enable(
NRF_DPPIC, BIT(HAL_RADIO_ENABLE_ON_TICK_PPI));
nrf_dppi_channels_enable(NRF_DPPIC, BIT(HAL_RADIO_ENABLE_ON_TICK_PPI));
}

/*******************************************************************************
Expand Down Expand Up @@ -290,26 +289,33 @@ static inline void hal_sw_switch_timer_clear_ppi_config(void)
nrf_radio_publish_set(NRF_RADIO, NRF_RADIO_EVENT_END, HAL_SW_SWITCH_TIMER_CLEAR_PPI);
nrf_timer_subscribe_set(SW_SWITCH_TIMER,
NRF_TIMER_TASK_CLEAR, HAL_SW_SWITCH_TIMER_CLEAR_PPI);

/* NOTE: nRF5340 shares the DPPI channel being triggered by Radio End,
* for End time capture and sw_switch DPPI channel toggling, hence
* always need to capture End time.
*/
nrf_dppi_channels_enable(NRF_DPPIC,
BIT(HAL_RADIO_END_TIME_CAPTURE_PPI));
}

/* The 2 adjacent PPI groups used for implementing SW_SWITCH_TIMER-based
* auto-switch for TIFS. 'index' must be 0 or 1.
*/
#define SW_SWITCH_TIMER_TASK_GROUP(index) \
(SW_SWITCH_TIMER_TASK_GROUP_BASE + index)
(SW_SWITCH_TIMER_TASK_GROUP_BASE + (index))

/* The 2 adjacent TIMER EVENTS_COMPARE event offsets used for implementing
* SW_SWITCH_TIMER-based auto-switch for TIFS. 'index' must be 0 or 1.
*/
#define SW_SWITCH_TIMER_EVTS_COMP(index) \
(SW_SWITCH_TIMER_EVTS_COMP_BASE + index)
(SW_SWITCH_TIMER_EVTS_COMP_BASE + (index))

/* The 2 adjacent TIMER EVENTS_COMPARE event offsets used for implementing
* SW_SWITCH_TIMER-based auto-switch for TIFS, when receiving on LE Coded
* PHY. 'index' must be 0 or 1.
*/
#define SW_SWITCH_TIMER_S2_EVTS_COMP(index) \
(SW_SWITCH_TIMER_EVTS_COMP_S2_BASE + index)
(SW_SWITCH_TIMER_EVTS_COMP_S2_BASE + (index))

/* Wire a SW SWITCH TIMER EVENTS_COMPARE[<cc_offset>] event
* to a PPI GROUP TASK DISABLE task (PPI group with index <index>).
Expand All @@ -318,7 +324,7 @@ static inline void hal_sw_switch_timer_clear_ppi_config(void)
*/
#define HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI_BASE 14
#define HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI(index) \
(HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI_BASE + index)
(HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI_BASE + (index))

#define HAL_SW_SWITCH_GROUP_TASK_DISABLE_PPI_REGISTER_EVT(cc_offset) \
SW_SWITCH_TIMER->PUBLISH_COMPARE[cc_offset]
Expand Down Expand Up @@ -373,12 +379,12 @@ static inline void hal_sw_switch_timer_clear_ppi_config(void)
*/
#define HAL_SW_SWITCH_RADIO_ENABLE_PPI_BASE 14
#define HAL_SW_SWITCH_RADIO_ENABLE_PPI(index) \
(HAL_SW_SWITCH_RADIO_ENABLE_PPI_BASE + index)
(HAL_SW_SWITCH_RADIO_ENABLE_PPI_BASE + (index))

#define HAL_SW_SWITCH_RADIO_ENABLE_S2_PPI_BASE \
HAL_SW_SWITCH_RADIO_ENABLE_PPI_BASE
#define HAL_SW_SWITCH_RADIO_ENABLE_S2_PPI(index) \
(HAL_SW_SWITCH_RADIO_ENABLE_S2_PPI_BASE + index)
(HAL_SW_SWITCH_RADIO_ENABLE_S2_PPI_BASE + (index))

#define HAL_SW_SWITCH_RADIO_ENABLE_PPI_REGISTER_EVT(cc_offset) \
SW_SWITCH_TIMER->PUBLISH_COMPARE[cc_offset]
Expand Down Expand Up @@ -445,11 +451,8 @@ static inline void hal_radio_sw_switch_setup(
HAL_SW_SWITCH_GROUP_TASK_ENABLE_PPI_TASK;

/* We need to un-subscribe the other group from the PPI channel. */
if (ppi_group_index == 0) {
HAL_SW_SWITCH_GROUP_TASK_ENABLE_PPI_REGISTER_TASK(1) = 0;
} else if (ppi_group_index == 1) {
HAL_SW_SWITCH_GROUP_TASK_ENABLE_PPI_REGISTER_TASK(0) = 0;
}
HAL_SW_SWITCH_GROUP_TASK_ENABLE_PPI_REGISTER_TASK(
(ppi_group_index + 1) & 0x01) = 0;

/* Wire SW Switch timer event <compare_reg> to the
* PPI[<radio_enable_ppi>] for enabling Radio. Do
Expand All @@ -466,6 +469,21 @@ static inline void hal_radio_txen_on_sw_switch(uint8_t ppi)
nrf_radio_subscribe_set(NRF_RADIO, NRF_RADIO_TASK_TXEN, ppi);
}

static inline void hal_radio_b2b_txen_on_sw_switch(uint8_t ppi)
{
/* NOTE: Calling radio_tmr_start/radio_tmr_start_us/radio_tmr_start_now
* after the radio_switch_complete_and_b2b_tx() call would have
* changed the PPI channel to HAL_RADIO_ENABLE_ON_TICK_PPI as we
* cannot double buffer the subscribe buffer. Hence, lets have
* both DPPI channel enabled (other one was enabled by the DPPI
* group when the Radio End occurred) so that when both timer
* trigger one of the DPPI is correct in the radio tx
* subscription.
*/
nrf_radio_subscribe_set(NRF_RADIO, NRF_RADIO_TASK_TXEN, ppi);
nrf_dppi_channels_enable(NRF_DPPIC, BIT(ppi));
}

static inline void hal_radio_rxen_on_sw_switch(uint8_t ppi)
{
nrf_radio_subscribe_set(NRF_RADIO, NRF_RADIO_TASK_RXEN, ppi);
Expand Down Expand Up @@ -521,7 +539,7 @@ static inline void hal_radio_sw_switch_coded_tx_config_set(uint8_t ppi_en,
HAL_SW_SWITCH_TIMER_S8_DISABLE_PPI_TASK;

nrf_dppi_channels_enable(NRF_DPPIC,
BIT(HAL_SW_SWITCH_TIMER_S8_DISABLE_PPI));
BIT(HAL_SW_SWITCH_TIMER_S8_DISABLE_PPI));
}

static inline void hal_radio_sw_switch_coded_config_clear(uint8_t ppi_en,
Expand Down
Loading