ESB: Fixes and improvements

- Improved `esb_suspend` to suspend the protocol.
- Changed `esb_flush_tx` to allow clearing the FIFO queue
  when ESB is in the IDLE state.
- Fixed `esb_pop_tx` to use the correct pointer.
- Renamed `tx_fifo_remove_last` to `tx_fifo_remove_first`

Ref: NCSDK-36153

Signed-off-by: Marcin Jelinski <[email protected]>

Author: maje-emb

include/esb.h

@@ -438,15 +438,22 @@ int esb_stop_rx(void);
  *
  * This function clears the TX FIFO buffer.
  *
- * @retval 0 If successful.
- *           Otherwise, a (negative) error code is returned.
+ * @note The radio must not be in transmission state for this operation to succeed.
+ *       This requirement prevents erroneous operations on the FIFO queue that could
+ *       occur if the buffer is cleared while the radio is transmitting.
+ *
+ * @retval 0       If successful (FIFO cleared).
+ * @retval -EACCES If ESB is not initialized.
+ * @retval -EBUSY  If radio is transmitting.
  */
 int esb_flush_tx(void);
 
 /** @brief Pop the first item from the TX buffer.
  *
  * @retval 0 If successful.
- *           Otherwise, a (negative) error code is returned.
+ * @retval -EACCES If ESB is not initialized.
+ * @retval -EBUSY  If radio is transmitting.
+ * @retval -ENODATA If TX FIFO is empty.
  */
 int esb_pop_tx(void);
 

subsys/esb/esb.c

@@ -1072,7 +1072,7 @@ static void initialize_fifos(void)
 	}
 }
 
-static void tx_fifo_remove_last(void)
+static void tx_fifo_remove_first(void)
 {
 	if (tx_fifo.count == 0) {
 		return;
@@ -1332,7 +1332,7 @@ static void on_timer_compare1_tx_noack(void)
 	esb_ppi_for_wait_for_rx_clear();
 
 	interrupt_flags |= INT_TX_SUCCESS_MSK;
-	tx_fifo_remove_last();
+	tx_fifo_remove_first();
 
 	if (tx_fifo.count == 0) {
 		esb_state = ESB_STATE_PTX_TXIDLE;
@@ -1349,7 +1349,7 @@ static void on_radio_disabled_tx_noack(void)
 	esb_ppi_for_txrx_clear(false, false);
 
 	interrupt_flags |= INT_TX_SUCCESS_MSK;
-	tx_fifo_remove_last();
+	tx_fifo_remove_first();
 
 	if (tx_fifo.count == 0) {
 		esb_state = ESB_STATE_IDLE;
@@ -1440,7 +1440,7 @@ static void on_radio_disabled_tx_wait_for_ack(void)
 		interrupt_flags |= INT_TX_SUCCESS_MSK;
 		last_tx_attempts = esb_cfg.retransmit_count - retransmits_remaining + 1;
 
-		tx_fifo_remove_last();
+		tx_fifo_remove_first();
 
 		if ((esb_cfg.protocol != ESB_PROTOCOL_ESB) && (rx_pdu->type.dpl_pdu.length > 0)) {
 			if (rx_fifo_push_rfbuf(
@@ -2100,15 +2100,31 @@ int esb_init(const struct esb_config *config)
 
 int esb_suspend(void)
 {
-	if (esb_state != ESB_STATE_IDLE) {
-		return -EBUSY;
+	if (esb_state == ESB_STATE_IDLE) {
+		return -EALREADY;
+	}
+	on_radio_disabled = NULL;
+
+	/* Stop radio */
+	nrf_radio_shorts_disable(NRF_RADIO, 0xFFFFFFFF);
+	nrf_radio_int_disable(NRF_RADIO, 0xFFFFFFFF);
+
+	nrf_radio_event_clear(NRF_RADIO, NRF_RADIO_EVENT_DISABLED);
+	nrf_radio_task_trigger(NRF_RADIO, NRF_RADIO_TASK_DISABLE);
+
+	while (!nrf_radio_event_check(NRF_RADIO, NRF_RADIO_EVENT_DISABLED)) {
+		/* wait for register to settle */
 	}
 
-	/*  Clear PPI */
+	/* Stop timer */
+	nrf_timer_shorts_disable(esb_timer.p_reg, 0xFFFFFFFF);
+	nrf_timer_int_disable(esb_timer.p_reg, 0xFFFFFFFF);
+
 	esb_ppi_disable_all();
+	esb_fem_reset();
 
-	esb_state = ESB_STATE_IDLE;
 	errata_216_off();
+	esb_state = ESB_STATE_IDLE;
 
 	return 0;
 }
@@ -2313,34 +2329,20 @@ int esb_stop_rx(void)
 		return -EINVAL;
 	}
 
-	on_radio_disabled = NULL;
-
-	esb_ppi_for_txrx_clear(true, false);
-	esb_fem_reset();
-
-	nrf_radio_shorts_disable(NRF_RADIO, 0xFFFFFFFF);
-	nrf_radio_int_disable(NRF_RADIO, 0xFFFFFFFF);
-
-	nrf_radio_event_clear(NRF_RADIO, NRF_RADIO_EVENT_DISABLED);
-	nrf_radio_task_trigger(NRF_RADIO, NRF_RADIO_TASK_DISABLE);
-
-	while (!nrf_radio_event_check(NRF_RADIO, NRF_RADIO_EVENT_DISABLED)) {
-		/* wait for register to settle */
-	}
-
-	nrf_radio_event_clear(NRF_RADIO, NRF_RADIO_EVENT_DISABLED);
-
-	esb_state = ESB_STATE_IDLE;
-	errata_216_off();
-
-	return 0;
+	return esb_suspend();
 }
 
 int esb_flush_tx(void)
 {
 	if (!esb_initialized) {
 		return -EACCES;
 	}
+	if (esb_state != ESB_STATE_IDLE) {
+		return -EBUSY;
+	}
+	if (tx_fifo.count == 0) {
+		return 0;
+	}
 
 	unsigned int key = irq_lock();
 
@@ -2367,14 +2369,17 @@ int esb_pop_tx(void)
 	if (!esb_initialized) {
 		return -EACCES;
 	}
+	if (esb_state != ESB_STATE_IDLE) {
+		return -EBUSY;
+	}
 	if (tx_fifo.count == 0) {
 		return -ENODATA;
 	}
 
 	unsigned int key = irq_lock();
 
-	if (++tx_fifo.back >= CONFIG_ESB_TX_FIFO_SIZE) {
-		tx_fifo.back = 0;
+	if (++tx_fifo.front >= CONFIG_ESB_TX_FIFO_SIZE) {
+		tx_fifo.front = 0;
 	}
 	tx_fifo.count--;
 

subsys/esb/esb_dppi.c

@@ -289,11 +289,35 @@ void esb_ppi_disable_all(void)
 				  BIT(egu_timer_start) |
 				  BIT(radio_address_timer_stop) |
 				  BIT(timer_compare0_radio_disable) |
-				  BIT(radio_end_timer_start) |
+				  BIT(timer_compare1_radio_txen) |
 				  (IS_ENABLED(CONFIG_ESB_NEVER_DISABLE_TX) ?
-					BIT(timer_compare1_radio_txen) : 0));
+					BIT(radio_end_timer_start) : 0));
 
 	nrf_dppi_channels_disable(ESB_DPPIC, channels_mask);
+
+	/* Clear all publish/subscribe connections to fully disconnect peripherals */
+	/* Clear EGU */
+	nrf_egu_publish_clear(ESB_EGU, ESB_EGU_EVENT);
+	nrf_egu_publish_clear(ESB_EGU, ESB_EGU_DPPI_EVENT);
+	nrf_egu_subscribe_clear(ESB_EGU, ESB_EGU_TASK);
+	nrf_egu_subscribe_clear(ESB_EGU, ESB_EGU_DPPI_TASK);
+
+	/* Clear Radio */
+	nrf_radio_publish_clear(NRF_RADIO, NRF_RADIO_EVENT_ADDRESS);
+	nrf_radio_subscribe_clear(NRF_RADIO, NRF_RADIO_TASK_RXEN);
+	nrf_radio_subscribe_clear(NRF_RADIO, NRF_RADIO_TASK_TXEN);
+	nrf_radio_subscribe_clear(NRF_RADIO, NRF_RADIO_TASK_DISABLE);
+
+	/* Clear Timer */
+	nrf_timer_publish_clear(ESB_NRF_TIMER_INSTANCE, NRF_TIMER_EVENT_COMPARE0);
+	nrf_timer_publish_clear(ESB_NRF_TIMER_INSTANCE, NRF_TIMER_EVENT_COMPARE1);
+	nrf_timer_subscribe_clear(ESB_NRF_TIMER_INSTANCE, NRF_TIMER_TASK_START);
+	nrf_timer_subscribe_clear(ESB_NRF_TIMER_INSTANCE, NRF_TIMER_TASK_STOP);
+
+	/* Clear DPPI */
+	nrf_dppi_subscribe_clear(ESB_DPPIC,
+				 nrf_dppi_group_disable_task_get((uint8_t)ramp_up_dppi_group));
+	nrf_dppi_channels_remove_from_group(ESB_DPPIC, BIT(egu_ramp_up), ramp_up_dppi_group);
 }
 
 void esb_ppi_deinit(void)

subsys/esb/esb_ppi.c

@@ -249,11 +249,26 @@ void esb_ppi_disable_all(void)
 				  BIT(egu_timer_start) |
 				  BIT(radio_address_timer_stop) |
 				  BIT(timer_compare0_radio_disable) |
-				  BIT(radio_end_timer_start) |
+				  BIT(timer_compare1_radio_txen) |
 				  (IS_ENABLED(CONFIG_ESB_NEVER_DISABLE_TX) ?
-					BIT(timer_compare1_radio_txen) : 0));
+					BIT(radio_end_timer_start) : 0));
 
 	nrf_ppi_channels_disable(NRF_PPI, channels_mask);
+
+	/* Clear all PPI endpoints to fully disconnect peripherals */
+	nrf_ppi_channel_and_fork_endpoint_setup(NRF_PPI, egu_ramp_up, 0, 0, 0);
+	nrf_ppi_channel_endpoint_setup(NRF_PPI, disabled_egu, 0, 0);
+	nrf_ppi_channel_endpoint_setup(NRF_PPI, egu_timer_start, 0, 0);
+	nrf_ppi_channel_endpoint_setup(NRF_PPI, radio_address_timer_stop, 0, 0);
+	nrf_ppi_channel_endpoint_setup(NRF_PPI, timer_compare0_radio_disable, 0, 0);
+	nrf_ppi_channel_endpoint_setup(NRF_PPI, radio_end_timer_start, 0, 0);
+
+	if (IS_ENABLED(CONFIG_ESB_NEVER_DISABLE_TX)) {
+		nrf_ppi_channel_endpoint_setup(NRF_PPI, timer_compare1_radio_txen, 0, 0);
+	}
+
+	/* Remove channel from group */
+	nrf_ppi_channel_remove_from_group(NRF_PPI, egu_ramp_up, ramp_up_ppi_group);
 }
 
 void esb_ppi_deinit(void)