Re: [PATCH net-next v10 04/11] net: ti: prueth: Adds link detection, RX and TX support.

[Parvathi Pudi <parvathi@couthit.com>]

Hi,

> On Wed, 16 Jul 2025 18:41:11 +0530 (IST) Parvathi Pudi wrote:
>> >> Something needs to stop the queue, right? Otherwise the stack will
>> >> send the frame right back to the driver.
>> > 
>> > Yes, we will notify upper layer with “netif_tx_stop_queue()” when returning
>> > “NETDEV_TX_BUSY” to not push again immediately.
>> 
>> We reviewed the flow and found that the reason for NETDEV_TX_BUSY being
>> notified to the upper layers is due lack of support for reliably detecting
>> the TX completion event.
>> 
>> In case of ICSSM PRU Ethernet, we do not have support for TX complete
>> notification back to the driver from firmware and its like store and
>> forget approach. So it will be tricky to enable back/resume the queue
>> if we stop it when we see busy status.
> 
> IIUC this is all implemented in SW / FW. You either need to add
> the notification or use a timer to unblock the queue.

We tried out a "hrtimer" based TX queue resume logic whenever
the driver finds that the queue is busy. The results look good.

Now the driver notifies the upper layer to stop re-queuing and
a timeout of HR_TIMER_TX_DELAY_US microseconds is used to
resume the queuing. Currently HR_TIMER_TX_DELAY_US is set as
100us as the PRU can approximately drain a maximum packet size
in this window.

Soon after timer expiry, the driver will notify the upper
layers to resume the queuing by invoking netif_tx_wake_queue().

This helps to avoid the stack from sending the frame right
back to the driver.

With these changes we have performed throughput tests for
various packet lengths using "iperf" and there is no
degradation in throughput for AM57x, AM437x and AM335x.

Below are the "hrtimer" changes for reference. We will post
the next version of patch series with reduced number of patches
and with this hrtimer logic.

We appreciate any feedback in the meantime.

diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
index a263df1fa511..9582246b1d87 100644
--- a/drivers/net/ethernet/ti/icssm/icssm_prueth.c
+++ b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
@@ -36,6 +36,7 @@
 
 #define TX_START_DELAY		0x40
 #define TX_CLK_DELAY_100M	0x6
+#define HR_TIMER_TX_DELAY_US	100
 
 static void icssm_prueth_write_reg(struct prueth *prueth,
 				   enum prueth_mem region,
@@ -1022,6 +1023,7 @@ static int icssm_emac_ndo_stop(struct net_device *ndev)
 	phy_stop(emac->phydev);
 
 	napi_disable(&emac->napi);
+	hrtimer_cancel(&emac->tx_hrtimer);
 
 	/* stop the PRU */
 	rproc_shutdown(emac->pru);
@@ -1109,6 +1111,9 @@ static enum netdev_tx icssm_emac_ndo_start_xmit(struct sk_buff *skb,
 
 fail_tx:
 	if (ret == -ENOBUFS) {
+		netif_stop_queue(ndev);
+		hrtimer_start(&emac->tx_hrtimer, us_to_ktime(HR_TIMER_TX_DELAY_US),
+			      HRTIMER_MODE_REL_PINNED);
 		ret = NETDEV_TX_BUSY;
 	} else {
 		/* error */
@@ -1161,6 +1166,17 @@ static int icssm_prueth_node_mac(struct device_node *eth_node)
 		return PRUETH_MAC_INVALID;
 }
 
+static enum hrtimer_restart icssm_emac_tx_timer_callback(struct hrtimer *timer)
+{
+        struct prueth_emac *emac =
+                        container_of(timer, struct prueth_emac, tx_hrtimer);
+
+	if (netif_queue_stopped(emac->ndev))
+		netif_wake_queue(emac->ndev);
+
+	return HRTIMER_NORESTART;
+}
+
 static int icssm_prueth_netdev_init(struct prueth *prueth,
 				    struct device_node *eth_node)
 {
@@ -1254,6 +1270,9 @@ static int icssm_prueth_netdev_init(struct prueth *prueth,
 
 	netif_napi_add(ndev, &emac->napi, icssm_emac_napi_poll);
 
+	hrtimer_setup(&emac->tx_hrtimer, &icssm_emac_tx_timer_callback,
+		      CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
+
 	return 0;
 free:
 	emac->ndev = NULL;
diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.h b/drivers/net/ethernet/ti/icssm/icssm_prueth.h
index 01586e6dbb66..c3f9c59ac6ff 100644
--- a/drivers/net/ethernet/ti/icssm/icssm_prueth.h
+++ b/drivers/net/ethernet/ti/icssm/icssm_prueth.h
@@ -221,6 +221,8 @@ struct prueth_emac {
 	 * during link configuration
 	 */
 	spinlock_t lock;
+
+	struct hrtimer tx_hrtimer;
 };
 
 struct prueth {


Thanks and Regards,
Parvathi.