driver: eth: gmac: wait until the packet is sent before returning

Scale down the TX path of the GMAC driver by waiting for a packet to be
fully sent before returning from the send function. This has a small
performance impact, but has a few advantages:
- It allows the Ethernet code to modify the packet afterward, fixing PTP
  support on this board (see PR #12563).
- It returns an error to the IP stack in case of a transmit failure.
- It doesn't require net_buf to be thread safe.

This change can be reverted by changing GMAC_MULTIPLE_TX_PACKETS from 0
to 1.

Signed-off-by: Aurelien Jarno <[email protected]>

Author: aurel32

drivers/ethernet/eth_sam_gmac.c

@@ -96,6 +96,7 @@ static struct net_buf *rx_frag_list_que1[PRIORITY_QUEUE1_RX_DESC_COUNT];
 #if GMAC_PRIORITY_QUEUE_NO == 2
 static struct net_buf *rx_frag_list_que2[PRIORITY_QUEUE2_RX_DESC_COUNT];
 #endif
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 /* TX buffer accounting list */
 static struct net_buf *tx_frag_list_que0[MAIN_QUEUE_TX_DESC_COUNT];
 #if GMAC_PRIORITY_QUEUE_NO >= 1
@@ -114,6 +115,7 @@ static struct net_pkt *tx_frame_list_que1[CONFIG_NET_PKT_TX_COUNT + 1];
 static struct net_pkt *tx_frame_list_que2[CONFIG_NET_PKT_TX_COUNT + 1];
 #endif
 #endif
+#endif
 
 #define MODULO_INC(val, max) {val = (++val < max) ? val : 0; }
 
@@ -149,6 +151,7 @@ static inline void dcache_clean(u32_t addr, u32_t size)
 	SCB_CleanDCache_by_Addr((uint32_t *)start_addr, size_full);
 }
 
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 /*
  * Reset ring buffer
  */
@@ -185,6 +188,7 @@ static void ring_buf_put(struct ring_buf *rb, u32_t val)
 	__ASSERT(rb->tail != rb->head,
 		 "ring buffer overflow");
 }
+#endif
 
 /*
  * Free pre-reserved RX buffers
@@ -276,11 +280,13 @@ static void tx_descriptors_init(Gmac *gmac, struct gmac_queue *queue)
 	/* Set the wrap bit on the last descriptor */
 	tx_desc_list->buf[tx_desc_list->len - 1].w1 |= GMAC_TXW1_WRAP;
 
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 	/* Reset TX frame list */
 	ring_buf_reset(&queue->tx_frag_list);
 #if defined(CONFIG_PTP_CLOCK_SAM_GMAC)
 	ring_buf_reset(&queue->tx_frames);
 #endif
+#endif
 }
 
 #if defined(CONFIG_PTP_CLOCK_SAM_GMAC)
@@ -478,6 +484,9 @@ static inline struct net_if *get_iface(struct eth_sam_dev_data *ctx,
  */
 static void tx_completed(Gmac *gmac, struct gmac_queue *queue)
 {
+#if GMAC_MULTIPLE_TX_PACKETS == 0
+	k_sem_give(&queue->tx_sem);
+#else
 	struct gmac_desc_list *tx_desc_list = &queue->tx_desc_list;
 	struct gmac_desc *tx_desc;
 	struct net_buf *frag;
@@ -529,25 +538,29 @@ static void tx_completed(Gmac *gmac, struct gmac_queue *queue)
 			break;
 		}
 	}
+#endif
 }
 
 /*
  * Reset TX queue when errors are detected
  */
 static void tx_error_handler(Gmac *gmac, struct gmac_queue *queue)
 {
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 	struct net_buf *frag;
 	struct ring_buf *tx_frag_list = &queue->tx_frag_list;
 #if defined(CONFIG_PTP_CLOCK_SAM_GMAC)
 	struct net_pkt *pkt;
 	struct ring_buf *tx_frames = &queue->tx_frames;
+#endif
 #endif
 
 	queue->err_tx_flushed_count++;
 
 	/* Stop transmission, clean transmit pipeline and control registers */
 	gmac->GMAC_NCR &= ~GMAC_NCR_TXEN;
 
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 	/* Free all frag resources in the TX path */
 	while (tx_frag_list->tail != tx_frag_list->head) {
 		/* Release net buffer to the buffer pool */
@@ -570,10 +583,16 @@ static void tx_error_handler(Gmac *gmac, struct gmac_queue *queue)
 
 	/* Reinitialize TX descriptor list */
 	k_sem_reset(&queue->tx_desc_sem);
-	tx_descriptors_init(gmac, queue);
 	for (int i = 0; i < queue->tx_desc_list.len - 1; i++) {
 		k_sem_give(&queue->tx_desc_sem);
 	}
+#endif
+	tx_descriptors_init(gmac, queue);
+
+#if GMAC_MULTIPLE_TX_PACKETS == 0
+	/* Reinitialize TX mutex */
+	k_sem_give(&queue->tx_sem);
+#endif
 
 	/* Restart transmission */
 	gmac->GMAC_NCR |=  GMAC_NCR_TXEN;
@@ -923,12 +942,19 @@ static int nonpriority_queue_init(Gmac *gmac, struct gmac_queue *queue)
 
 	tx_descriptors_init(gmac, queue);
 
+#if GMAC_MULTIPLE_TX_PACKETS == 0
+	/* Initialize TX semaphore. This semaphore is used to wait until the TX
+	 * data has been sent.
+	 */
+	k_sem_init(&queue->tx_sem, 0, 1);
+#else
 	/* Initialize TX descriptors semaphore. The semaphore is required as the
 	 * size of the TX descriptor list is limited while the number of TX data
 	 * buffers is not.
 	 */
 	k_sem_init(&queue->tx_desc_sem, queue->tx_desc_list.len - 1,
 		   queue->tx_desc_list.len - 1);
+#endif
 
 	/* Set Receive Buffer Queue Pointer Register */
 	gmac->GMAC_RBQB = (u32_t)queue->rx_desc_list.buf;
@@ -983,8 +1009,12 @@ static int priority_queue_init(Gmac *gmac, struct gmac_queue *queue)
 
 	tx_descriptors_init(gmac, queue);
 
+#if GMAC_MULTIPLE_TX_PACKETS == 0
+	k_sem_init(&queue->tx_sem, 0, 1);
+#else
 	k_sem_init(&queue->tx_desc_sem, queue->tx_desc_list.len - 1,
 		   queue->tx_desc_list.len - 1);
+#endif
 
 	/* Setup RX buffer size for DMA */
 	gmac->GMAC_RBSRPQ[queue_index] =
@@ -1263,8 +1293,19 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 	u8_t *frag_data;
 	u16_t frag_len;
 	u32_t err_tx_flushed_count_at_entry;
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 	unsigned int key;
+#endif
 	u8_t pkt_prio;
+#if GMAC_MULTIPLE_TX_PACKETS == 0
+#if defined(CONFIG_PTP_CLOCK_SAM_GMAC)
+	u16_t vlan_tag = NET_VLAN_TAG_UNSPEC;
+	struct gptp_hdr *hdr;
+#if defined(CONFIG_NET_VLAN)
+	struct net_eth_hdr *eth_hdr;
+#endif
+#endif
+#endif
 
 	__ASSERT(pkt, "buf pointer is NULL");
 	__ASSERT(pkt->frags, "Frame data missing");
@@ -1297,6 +1338,7 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 		/* Assure cache coherency before DMA read operation */
 		dcache_clean((u32_t)frag_data, frag->size);
 
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 		k_sem_take(&queue->tx_desc_sem, K_FOREVER);
 
 		/* The following section becomes critical and requires IRQ lock
@@ -1311,6 +1353,7 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 			irq_unlock(key);
 			return -EIO;
 		}
+#endif
 
 		tx_desc = &tx_desc_list->buf[tx_desc_list->head];
 
@@ -1329,6 +1372,7 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 		/* Update descriptor position */
 		MODULO_INC(tx_desc_list->head, tx_desc_list->len);
 
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 		__ASSERT(tx_desc_list->head != tx_desc_list->tail,
 			 "tx_desc_list overflow");
 
@@ -1339,18 +1383,21 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 		net_pkt_frag_ref(frag);
 
 		irq_unlock(key);
+#endif
 
 		/* Continue with the rest of fragments (only data) */
 		frag = frag->frags;
 	}
 
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 	key = irq_lock();
 
 	/* Check if tx_error_handler() function was executed */
 	if (queue->err_tx_flushed_count != err_tx_flushed_count_at_entry) {
 		irq_unlock(key);
 		return -EIO;
 	}
+#endif
 
 	/* Ensure the descriptor following the last one is marked as used */
 	tx_desc_list->buf[tx_desc_list->head].w1 = GMAC_TXW1_USED;
@@ -1365,6 +1412,7 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 	 */
 	tx_first_desc->w1 &= ~GMAC_TXW1_USED;
 
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 #if defined(CONFIG_PTP_CLOCK_SAM_GMAC)
 	/* Account for a sent frame */
 	ring_buf_put(&queue->tx_frames, POINTER_TO_UINT(pkt));
@@ -1374,6 +1422,7 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 #endif
 
 	irq_unlock(key);
+#endif
 
 	/* Guarantee that the first fragment got its bit removed before starting
 	 * sending packets to avoid packets getting stuck.
@@ -1383,6 +1432,29 @@ static int eth_tx(struct device *dev, struct net_pkt *pkt)
 	/* Start transmission */
 	gmac->GMAC_NCR |= GMAC_NCR_TSTART;
 
+#if GMAC_MULTIPLE_TX_PACKETS == 0
+	/* Wait until the packet is sent */
+	k_sem_take(&queue->tx_sem, K_FOREVER);
+
+	/* Check if transmit successful or not */
+	if (queue->err_tx_flushed_count != err_tx_flushed_count_at_entry) {
+		return -EIO;
+	}
+#if defined(CONFIG_PTP_CLOCK_SAM_GMAC)
+#if defined(CONFIG_NET_VLAN)
+	eth_hdr = NET_ETH_HDR(pkt);
+	if (ntohs(eth_hdr->type) == NET_ETH_PTYPE_VLAN) {
+		vlan_tag = net_pkt_vlan_tag(pkt);
+	}
+#endif
+	hdr = check_gptp_msg(get_iface(dev_data, vlan_tag), pkt, true);
+	timestamp_tx_pkt(gmac, hdr, pkt);
+	if (hdr && need_timestamping(hdr)) {
+		net_if_add_tx_timestamp(pkt);
+	}
+#endif
+#endif
+
 	return 0;
 }
 
@@ -1421,10 +1493,12 @@ static void queue0_isr(void *arg)
 	if (isr & GMAC_INT_TX_ERR_BITS) {
 		tx_error_handler(gmac, queue);
 	} else if (isr & GMAC_ISR_TCOMP) {
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 		tail_desc = &tx_desc_list->buf[tx_desc_list->tail];
 		LOG_DBG("tx.w1=0x%08x, tail=%d",
 			tail_desc->w1,
 			tx_desc_list->tail);
+#endif
 
 		tx_completed(gmac, queue);
 	}
@@ -1469,10 +1543,12 @@ static inline void priority_queue_isr(void *arg, unsigned int queue_idx)
 	if (isrpq & GMAC_INTPQ_TX_ERR_BITS) {
 		tx_error_handler(gmac, queue);
 	} else if (isrpq & GMAC_ISRPQ_TCOMP) {
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 		tail_desc = &tx_desc_list->buf[tx_desc_list->tail];
 		LOG_DBG("tx.w1=0x%08x, tail=%d",
 			tail_desc->w1,
 			tx_desc_list->tail);
+#endif
 
 		tx_completed(gmac, queue);
 	}
@@ -1894,6 +1970,7 @@ static struct eth_sam_dev_data eth0_data = {
 				.buf = (u32_t *)rx_frag_list_que0,
 				.len = ARRAY_SIZE(rx_frag_list_que0),
 			},
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 			.tx_frag_list = {
 				.buf = (u32_t *)tx_frag_list_que0,
 				.len = ARRAY_SIZE(tx_frag_list_que0),
@@ -1903,6 +1980,7 @@ static struct eth_sam_dev_data eth0_data = {
 				.buf = (u32_t *)tx_frame_list_que0,
 				.len = ARRAY_SIZE(tx_frame_list_que0),
 			},
+#endif
 #endif
 		}, {
 			.que_idx = GMAC_QUE_1,
@@ -1919,6 +1997,7 @@ static struct eth_sam_dev_data eth0_data = {
 				.buf = (u32_t *)rx_frag_list_que1,
 				.len = ARRAY_SIZE(rx_frag_list_que1),
 			},
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 			.tx_frag_list = {
 				.buf = (u32_t *)tx_frag_list_que1,
 				.len = ARRAY_SIZE(tx_frag_list_que1),
@@ -1929,6 +2008,7 @@ static struct eth_sam_dev_data eth0_data = {
 				.len = ARRAY_SIZE(tx_frame_list_que1),
 			}
 #endif
+#endif
 #endif
 		}, {
 			.que_idx = GMAC_QUE_2,
@@ -1945,6 +2025,7 @@ static struct eth_sam_dev_data eth0_data = {
 				.buf = (u32_t *)rx_frag_list_que2,
 				.len = ARRAY_SIZE(rx_frag_list_que2),
 			},
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 			.tx_frag_list = {
 				.buf = (u32_t *)tx_frag_list_que2,
 				.len = ARRAY_SIZE(tx_frag_list_que2),
@@ -1955,6 +2036,7 @@ static struct eth_sam_dev_data eth0_data = {
 				.len = ARRAY_SIZE(tx_frame_list_que2),
 			}
 #endif
+#endif
 #endif
 		}
 	},

drivers/ethernet/eth_sam_gmac_priv.h

@@ -12,6 +12,13 @@
 
 #include <zephyr/types.h>
 
+/* This option enables support to push multiple packets to the DMA engine.
+ * This currently doesn't work given the current version of net_pkt or
+ * net_buf does not allowed access from multiple threads. This option is
+ * therefore currenlty disabled.
+ */
+#define GMAC_MULTIPLE_TX_PACKETS 0
+
 #define GMAC_MTU 1500
 #define GMAC_FRAME_SIZE_MAX (GMAC_MTU + 18)
 
@@ -164,12 +171,19 @@ struct gmac_desc_list {
 struct gmac_queue {
 	struct gmac_desc_list rx_desc_list;
 	struct gmac_desc_list tx_desc_list;
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 	struct k_sem tx_desc_sem;
+#else
+	struct k_sem tx_sem;
+#endif
 
 	struct ring_buf rx_frag_list;
+
+#if GMAC_MULTIPLE_TX_PACKETS == 1
 	struct ring_buf tx_frag_list;
 #if defined(CONFIG_PTP_CLOCK_SAM_GMAC)
 	struct ring_buf tx_frames;
+#endif
 #endif
 
 	/** Number of RX frames dropped by the driver */