All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/6] ethernet: fixes for stmmac driver
@ 2021-01-26 11:58 Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock Joakim Zhang
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Joakim Zhang @ 2021-01-26 11:58 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba
  Cc: netdev, linux-imx, andrew, f.fainelli

Fixes for stmmac driver.

---
ChangeLogs:
V1->V2:
	* subject prefix: ethernet: stmmac: -> net: stmmac:
	* use dma_addr_t instead of unsigned int for physical address
	* use cpu_to_le32()
V2-V3:
	* fix the build issue pointed out by kbuild bot.
	* add error handling for stmmac_reinit_rx_buffers() function.

Joakim Zhang (6):
  net: stmmac: remove redundant null check for ptp clock
  net: stmmac: stop each tx channel independently
  net: stmmac: fix watchdog timeout during suspend/resume stress test
  net: stmmac: fix dma physical address of descriptor when display ring
  net: stmmac: fix wrongly set buffer2 valid when sph unsupport
  net: stmmac: re-init rx buffers when mac resume back

 .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  16 +-
 .../net/ethernet/stmicro/stmmac/dwmac4_lib.c  |   4 -
 .../ethernet/stmicro/stmmac/dwxgmac2_descs.c  |   2 +-
 .../net/ethernet/stmicro/stmmac/enh_desc.c    |   7 +-
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |   5 +-
 .../net/ethernet/stmicro/stmmac/norm_desc.c   |   7 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 154 +++++++++++++++---
 7 files changed, 153 insertions(+), 42 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock
  2021-01-26 11:58 [PATCH V3 0/6] ethernet: fixes for stmmac driver Joakim Zhang
@ 2021-01-26 11:58 ` Joakim Zhang
  2021-01-26 22:46   ` Willem de Bruijn
  2021-01-26 11:58 ` [PATCH V3 2/6] net: stmmac: stop each tx channel independently Joakim Zhang
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Joakim Zhang @ 2021-01-26 11:58 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba
  Cc: netdev, linux-imx, andrew, f.fainelli

Remove redundant null check for ptp clock.

Fixes: 1c35cc9cf6a0 ("net: stmmac: remove redundant null check before clk_disable_unprepare()")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 26b971cd4da5..11e0b30b2e01 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5291,8 +5291,7 @@ int stmmac_resume(struct device *dev)
 		/* enable the clk previously disabled */
 		clk_prepare_enable(priv->plat->stmmac_clk);
 		clk_prepare_enable(priv->plat->pclk);
-		if (priv->plat->clk_ptp_ref)
-			clk_prepare_enable(priv->plat->clk_ptp_ref);
+		clk_prepare_enable(priv->plat->clk_ptp_ref);
 		/* reset the phy so that it's ready */
 		if (priv->mii)
 			stmmac_mdio_reset(priv->mii);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 2/6] net: stmmac: stop each tx channel independently
  2021-01-26 11:58 [PATCH V3 0/6] ethernet: fixes for stmmac driver Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock Joakim Zhang
@ 2021-01-26 11:58 ` Joakim Zhang
  2021-01-26 23:09   ` Willem de Bruijn
  2021-01-26 11:58 ` [PATCH V3 3/6] net: stmmac: fix watchdog timeout during suspend/resume stress test Joakim Zhang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Joakim Zhang @ 2021-01-26 11:58 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba
  Cc: netdev, linux-imx, andrew, f.fainelli

If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users
may only want to stop secific tx channel.

Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
index 0b4ee2dbb691..71e50751ef2d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
@@ -53,10 +53,6 @@ void dwmac4_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 
 	value &= ~DMA_CONTROL_ST;
 	writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value &= ~GMAC_CONFIG_TE;
-	writel(value, ioaddr + GMAC_CONFIG);
 }
 
 void dwmac4_dma_start_rx(void __iomem *ioaddr, u32 chan)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 3/6] net: stmmac: fix watchdog timeout during suspend/resume stress test
  2021-01-26 11:58 [PATCH V3 0/6] ethernet: fixes for stmmac driver Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 2/6] net: stmmac: stop each tx channel independently Joakim Zhang
@ 2021-01-26 11:58 ` Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 4/6] net: stmmac: fix dma physical address of descriptor when display ring Joakim Zhang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Joakim Zhang @ 2021-01-26 11:58 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba
  Cc: netdev, linux-imx, andrew, f.fainelli

stmmac_xmit() call stmmac_tx_timer_arm() at the end to modify tx timer to
do the transmission cleanup work. Imagine such a situation, stmmac enters
suspend immediately after tx timer modified, it's expire callback
stmmac_tx_clean() would not be invoked. This could affect BQL, since
netdev_tx_sent_queue() has been called, but netdev_tx_completed_queue()
have not been involved, as a result, dql_avail(&dev_queue->dql) finally always
return a negative value.

__dev_queue_xmit->__dev_xmit_skb->qdisc_run->__qdisc_run->qdisc_restart->dequeue_skb:
	if ((q->flags & TCQ_F_ONETXQUEUE) &&
		netif_xmit_frozen_or_stopped(txq)) // __QUEUE_STATE_STACK_XOFF is set

Net core will stop transmitting any more. Finillay, net watchdong would timeout.
To fix this issue, we should call netdev_tx_reset_queue() in stmmac_resume().

Fixes: 54139cf3bb33 ("net: stmmac: adding multiple buffers for rx")
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 11e0b30b2e01..e4083bbc092f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5257,6 +5257,8 @@ static void stmmac_reset_queues_param(struct stmmac_priv *priv)
 		tx_q->cur_tx = 0;
 		tx_q->dirty_tx = 0;
 		tx_q->mss = 0;
+
+		netdev_tx_reset_queue(netdev_get_tx_queue(priv->dev, queue));
 	}
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 4/6] net: stmmac: fix dma physical address of descriptor when display ring
  2021-01-26 11:58 [PATCH V3 0/6] ethernet: fixes for stmmac driver Joakim Zhang
                   ` (2 preceding siblings ...)
  2021-01-26 11:58 ` [PATCH V3 3/6] net: stmmac: fix watchdog timeout during suspend/resume stress test Joakim Zhang
@ 2021-01-26 11:58 ` Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 5/6] net: stmmac: fix wrongly set buffer2 valid when sph unsupport Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 6/6] net: stmmac: re-init rx buffers when mac resume back Joakim Zhang
  5 siblings, 0 replies; 14+ messages in thread
From: Joakim Zhang @ 2021-01-26 11:58 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba
  Cc: netdev, linux-imx, andrew, f.fainelli

Driver uses dma_alloc_coherent to allocate dma memory for descriptors,
dma_alloc_coherent will return both the virtual address and physical
address. AFAIK, virt_to_phys could not convert virtual address to
physical address, for which memory is allocated by dma_alloc_coherent.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  7 +--
 .../net/ethernet/stmicro/stmmac/enh_desc.c    |  7 +--
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  3 +-
 .../net/ethernet/stmicro/stmmac/norm_desc.c   |  7 +--
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 54 ++++++++++++-------
 5 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index c6540b003b43..6f951adc5f90 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -402,7 +402,8 @@ static void dwmac4_rd_set_tx_ic(struct dma_desc *p)
 	p->des2 |= cpu_to_le32(TDES2_INTERRUPT_ON_COMPLETION);
 }
 
-static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
+static void dwmac4_display_ring(void *head, unsigned int size, bool rx,
+				dma_addr_t dma_rx_phy, unsigned int desc_size)
 {
 	struct dma_desc *p = (struct dma_desc *)head;
 	int i;
@@ -410,8 +411,8 @@ static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
 	pr_info("%s descriptor ring:\n", rx ? "RX" : "TX");
 
 	for (i = 0; i < size; i++) {
-		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-			i, (unsigned int)virt_to_phys(p),
+		pr_info("%03d [0x%llx]: 0x%x 0x%x 0x%x 0x%x\n",
+			i, (unsigned long long)(dma_rx_phy + i * desc_size),
 			le32_to_cpu(p->des0), le32_to_cpu(p->des1),
 			le32_to_cpu(p->des2), le32_to_cpu(p->des3));
 		p++;
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index d02cec296f51..eaea4cf02386 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -417,7 +417,8 @@ static int enh_desc_get_rx_timestamp_status(void *desc, void *next_desc,
 	}
 }
 
-static void enh_desc_display_ring(void *head, unsigned int size, bool rx)
+static void enh_desc_display_ring(void *head, unsigned int size, bool rx,
+				  dma_addr_t dma_rx_phy, unsigned int desc_size)
 {
 	struct dma_extended_desc *ep = (struct dma_extended_desc *)head;
 	int i;
@@ -428,8 +429,8 @@ static void enh_desc_display_ring(void *head, unsigned int size, bool rx)
 		u64 x;
 
 		x = *(u64 *)ep;
-		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-			i, (unsigned int)virt_to_phys(ep),
+		pr_info("%03d [0x%llx]: 0x%x 0x%x 0x%x 0x%x\n",
+			i, (unsigned long long)(dma_rx_phy + i * desc_size),
 			(unsigned int)x, (unsigned int)(x >> 32),
 			ep->basic.des2, ep->basic.des3);
 		ep++;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index b40b2e0667bb..7417db31402f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -78,7 +78,8 @@ struct stmmac_desc_ops {
 	/* get rx timestamp status */
 	int (*get_rx_timestamp_status)(void *desc, void *next_desc, u32 ats);
 	/* Display ring */
-	void (*display_ring)(void *head, unsigned int size, bool rx);
+	void (*display_ring)(void *head, unsigned int size, bool rx,
+			     dma_addr_t dma_rx_phy, unsigned int desc_size);
 	/* set MSS via context descriptor */
 	void (*set_mss)(struct dma_desc *p, unsigned int mss);
 	/* get descriptor skbuff address */
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index f083360e4ba6..36e769d2e312 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -269,7 +269,8 @@ static int ndesc_get_rx_timestamp_status(void *desc, void *next_desc, u32 ats)
 		return 1;
 }
 
-static void ndesc_display_ring(void *head, unsigned int size, bool rx)
+static void ndesc_display_ring(void *head, unsigned int size, bool rx,
+			       dma_addr_t dma_rx_phy, unsigned int desc_size)
 {
 	struct dma_desc *p = (struct dma_desc *)head;
 	int i;
@@ -280,8 +281,8 @@ static void ndesc_display_ring(void *head, unsigned int size, bool rx)
 		u64 x;
 
 		x = *(u64 *)p;
-		pr_info("%03d [0x%x]: 0x%x 0x%x 0x%x 0x%x",
-			i, (unsigned int)virt_to_phys(p),
+		pr_info("%03d [0x%llx]: 0x%x 0x%x 0x%x 0x%x",
+			i, (unsigned long long)(dma_rx_phy + i * desc_size),
 			(unsigned int)x, (unsigned int)(x >> 32),
 			p->des2, p->des3);
 		p++;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e4083bbc092f..8d1cc17a99a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1133,6 +1133,7 @@ static int stmmac_phy_setup(struct stmmac_priv *priv)
 static void stmmac_display_rx_rings(struct stmmac_priv *priv)
 {
 	u32 rx_cnt = priv->plat->rx_queues_to_use;
+	unsigned int desc_size;
 	void *head_rx;
 	u32 queue;
 
@@ -1142,19 +1143,24 @@ static void stmmac_display_rx_rings(struct stmmac_priv *priv)
 
 		pr_info("\tRX Queue %u rings\n", queue);
 
-		if (priv->extend_desc)
+		if (priv->extend_desc) {
 			head_rx = (void *)rx_q->dma_erx;
-		else
+			desc_size = sizeof(struct dma_extended_desc);
+		} else {
 			head_rx = (void *)rx_q->dma_rx;
+			desc_size = sizeof(struct dma_desc);
+		}
 
 		/* Display RX ring */
-		stmmac_display_ring(priv, head_rx, priv->dma_rx_size, true);
+		stmmac_display_ring(priv, head_rx, priv->dma_rx_size, true,
+				    rx_q->dma_rx_phy, desc_size);
 	}
 }
 
 static void stmmac_display_tx_rings(struct stmmac_priv *priv)
 {
 	u32 tx_cnt = priv->plat->tx_queues_to_use;
+	unsigned int desc_size;
 	void *head_tx;
 	u32 queue;
 
@@ -1164,14 +1170,19 @@ static void stmmac_display_tx_rings(struct stmmac_priv *priv)
 
 		pr_info("\tTX Queue %d rings\n", queue);
 
-		if (priv->extend_desc)
+		if (priv->extend_desc) {
 			head_tx = (void *)tx_q->dma_etx;
-		else if (tx_q->tbs & STMMAC_TBS_AVAIL)
+			desc_size = sizeof(struct dma_extended_desc);
+		} else if (tx_q->tbs & STMMAC_TBS_AVAIL) {
 			head_tx = (void *)tx_q->dma_entx;
-		else
+			desc_size = sizeof(struct dma_edesc);
+		} else {
 			head_tx = (void *)tx_q->dma_tx;
+			desc_size = sizeof(struct dma_desc);
+		}
 
-		stmmac_display_ring(priv, head_tx, priv->dma_tx_size, false);
+		stmmac_display_ring(priv, head_tx, priv->dma_tx_size, false,
+				    tx_q->dma_tx_phy, desc_size);
 	}
 }
 
@@ -3736,18 +3747,23 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	unsigned int count = 0, error = 0, len = 0;
 	int status = 0, coe = priv->hw->rx_csum;
 	unsigned int next_entry = rx_q->cur_rx;
+	unsigned int desc_size;
 	struct sk_buff *skb = NULL;
 
 	if (netif_msg_rx_status(priv)) {
 		void *rx_head;
 
 		netdev_dbg(priv->dev, "%s: descriptor ring:\n", __func__);
-		if (priv->extend_desc)
+		if (priv->extend_desc) {
 			rx_head = (void *)rx_q->dma_erx;
-		else
+			desc_size = sizeof(struct dma_extended_desc);
+		} else {
 			rx_head = (void *)rx_q->dma_rx;
+			desc_size = sizeof(struct dma_desc);
+		}
 
-		stmmac_display_ring(priv, rx_head, priv->dma_rx_size, true);
+		stmmac_display_ring(priv, rx_head, priv->dma_rx_size, true,
+				    rx_q->dma_rx_phy, desc_size);
 	}
 	while (count < limit) {
 		unsigned int buf1_len = 0, buf2_len = 0;
@@ -4315,7 +4331,7 @@ static int stmmac_set_mac_address(struct net_device *ndev, void *addr)
 static struct dentry *stmmac_fs_dir;
 
 static void sysfs_display_ring(void *head, int size, int extend_desc,
-			       struct seq_file *seq)
+			       struct seq_file *seq, dma_addr_t dma_phy_addr)
 {
 	int i;
 	struct dma_extended_desc *ep = (struct dma_extended_desc *)head;
@@ -4323,16 +4339,16 @@ static void sysfs_display_ring(void *head, int size, int extend_desc,
 
 	for (i = 0; i < size; i++) {
 		if (extend_desc) {
-			seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-				   i, (unsigned int)virt_to_phys(ep),
+			seq_printf(seq, "%d [0x%llx]: 0x%x 0x%x 0x%x 0x%x\n",
+				   i, (unsigned long long)(dma_phy_addr + i * sizeof(ep)),
 				   le32_to_cpu(ep->basic.des0),
 				   le32_to_cpu(ep->basic.des1),
 				   le32_to_cpu(ep->basic.des2),
 				   le32_to_cpu(ep->basic.des3));
 			ep++;
 		} else {
-			seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
-				   i, (unsigned int)virt_to_phys(p),
+			seq_printf(seq, "%d [0x%llx]: 0x%x 0x%x 0x%x 0x%x\n",
+				   i, (unsigned long long)(dma_phy_addr + i * sizeof(p)),
 				   le32_to_cpu(p->des0), le32_to_cpu(p->des1),
 				   le32_to_cpu(p->des2), le32_to_cpu(p->des3));
 			p++;
@@ -4360,11 +4376,11 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
 		if (priv->extend_desc) {
 			seq_printf(seq, "Extended descriptor ring:\n");
 			sysfs_display_ring((void *)rx_q->dma_erx,
-					   priv->dma_rx_size, 1, seq);
+					   priv->dma_rx_size, 1, seq, rx_q->dma_rx_phy);
 		} else {
 			seq_printf(seq, "Descriptor ring:\n");
 			sysfs_display_ring((void *)rx_q->dma_rx,
-					   priv->dma_rx_size, 0, seq);
+					   priv->dma_rx_size, 0, seq, rx_q->dma_rx_phy);
 		}
 	}
 
@@ -4376,11 +4392,11 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
 		if (priv->extend_desc) {
 			seq_printf(seq, "Extended descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_etx,
-					   priv->dma_tx_size, 1, seq);
+					   priv->dma_tx_size, 1, seq, tx_q->dma_tx_phy);
 		} else if (!(tx_q->tbs & STMMAC_TBS_AVAIL)) {
 			seq_printf(seq, "Descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_tx,
-					   priv->dma_tx_size, 0, seq);
+					   priv->dma_tx_size, 0, seq, tx_q->dma_tx_phy);
 		}
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 5/6] net: stmmac: fix wrongly set buffer2 valid when sph unsupport
  2021-01-26 11:58 [PATCH V3 0/6] ethernet: fixes for stmmac driver Joakim Zhang
                   ` (3 preceding siblings ...)
  2021-01-26 11:58 ` [PATCH V3 4/6] net: stmmac: fix dma physical address of descriptor when display ring Joakim Zhang
@ 2021-01-26 11:58 ` Joakim Zhang
  2021-01-26 11:58 ` [PATCH V3 6/6] net: stmmac: re-init rx buffers when mac resume back Joakim Zhang
  5 siblings, 0 replies; 14+ messages in thread
From: Joakim Zhang @ 2021-01-26 11:58 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba
  Cc: netdev, linux-imx, andrew, f.fainelli

In current driver, buffer2 available only when hardware supports split
header. Wrongly set buffer2 valid in stmmac_rx_refill when refill buffer
address. You can see that desc3 is 0x81000000 after initialization, but
turn out to be 0x83000000 after refill.

Fixes: 67afd6d1cfdf ("net: stmmac: Add Split Header support and enable it in XGMAC cores")
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c   | 9 +++++++--
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 2 +-
 drivers/net/ethernet/stmicro/stmmac/hwif.h           | 2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 8 ++++++--
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 6f951adc5f90..af72be68ee8e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -500,10 +500,15 @@ static void dwmac4_get_rx_header_len(struct dma_desc *p, unsigned int *len)
 	*len = le32_to_cpu(p->des2) & RDES2_HL;
 }
 
-static void dwmac4_set_sec_addr(struct dma_desc *p, dma_addr_t addr)
+static void dwmac4_set_sec_addr(struct dma_desc *p, dma_addr_t addr, bool buf2_valid)
 {
 	p->des2 = cpu_to_le32(lower_32_bits(addr));
-	p->des3 = cpu_to_le32(upper_32_bits(addr) | RDES3_BUFFER2_VALID_ADDR);
+	p->des3 = cpu_to_le32(upper_32_bits(addr));
+
+	if (buf2_valid)
+		p->des3 |= cpu_to_le32(RDES3_BUFFER2_VALID_ADDR);
+	else
+		p->des3 &= cpu_to_le32(~RDES3_BUFFER2_VALID_ADDR);
 }
 
 static void dwmac4_set_tbs(struct dma_edesc *p, u32 sec, u32 nsec)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
index 0aaf19ab5672..ccfb0102dde4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
@@ -292,7 +292,7 @@ static void dwxgmac2_get_rx_header_len(struct dma_desc *p, unsigned int *len)
 		*len = le32_to_cpu(p->des2) & XGMAC_RDES2_HL;
 }
 
-static void dwxgmac2_set_sec_addr(struct dma_desc *p, dma_addr_t addr)
+static void dwxgmac2_set_sec_addr(struct dma_desc *p, dma_addr_t addr, bool is_valid)
 {
 	p->des2 = cpu_to_le32(lower_32_bits(addr));
 	p->des3 = cpu_to_le32(upper_32_bits(addr));
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 7417db31402f..979ac9fca23c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -92,7 +92,7 @@ struct stmmac_desc_ops {
 	int (*get_rx_hash)(struct dma_desc *p, u32 *hash,
 			   enum pkt_hash_types *type);
 	void (*get_rx_header_len)(struct dma_desc *p, unsigned int *len);
-	void (*set_sec_addr)(struct dma_desc *p, dma_addr_t addr);
+	void (*set_sec_addr)(struct dma_desc *p, dma_addr_t addr, bool buf2_valid);
 	void (*set_sarc)(struct dma_desc *p, u32 sarc_type);
 	void (*set_vlan_tag)(struct dma_desc *p, u16 tag, u16 inner_tag,
 			     u32 inner_type);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8d1cc17a99a5..2505e314f543 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1314,9 +1314,10 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, struct dma_desc *p,
 			return -ENOMEM;
 
 		buf->sec_addr = page_pool_get_dma_addr(buf->sec_page);
-		stmmac_set_desc_sec_addr(priv, p, buf->sec_addr);
+		stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, true);
 	} else {
 		buf->sec_page = NULL;
+		stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, false);
 	}
 
 	buf->addr = page_pool_get_dma_addr(buf->page);
@@ -3659,7 +3660,10 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
 					   DMA_FROM_DEVICE);
 
 		stmmac_set_desc_addr(priv, p, buf->addr);
-		stmmac_set_desc_sec_addr(priv, p, buf->sec_addr);
+		if (priv->sph)
+			stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, true);
+		else
+			stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, false);
 		stmmac_refill_desc3(priv, rx_q, p);
 
 		rx_q->rx_count_frames++;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V3 6/6] net: stmmac: re-init rx buffers when mac resume back
  2021-01-26 11:58 [PATCH V3 0/6] ethernet: fixes for stmmac driver Joakim Zhang
                   ` (4 preceding siblings ...)
  2021-01-26 11:58 ` [PATCH V3 5/6] net: stmmac: fix wrongly set buffer2 valid when sph unsupport Joakim Zhang
@ 2021-01-26 11:58 ` Joakim Zhang
  5 siblings, 0 replies; 14+ messages in thread
From: Joakim Zhang @ 2021-01-26 11:58 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, joabreu, davem, kuba
  Cc: netdev, linux-imx, andrew, f.fainelli

During suspend/resume stress test, we found descriptor write back by DMA
could exhibit unusual behavior, e.g.:
	003 [0xc4310030]: 0x0 0x40 0x0 0xb5010040

We can see that desc3 write back is 0xb5010040, it is still ownd by DMA,
so application would not recycle this buffer. It will trigger fatal bus
error when DMA try to use this descriptor again. To fix this issue, we
should re-init all rx buffers when mac resume back.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 87 ++++++++++++++++++-
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2505e314f543..e657e879c8bf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1379,6 +1379,91 @@ static void stmmac_free_tx_buffer(struct stmmac_priv *priv, u32 queue, int i)
 	}
 }
 
+/**
+ * stmmac_reinit_rx_buffers - reinit the RX descriptor buffer.
+ * @priv: driver private structure
+ * Description: this function is called to re-allocate a receive buffer, perform
+ * the DMA mapping and init the descriptor.
+ */
+static int stmmac_reinit_rx_buffers(struct stmmac_priv *priv)
+{
+	u32 rx_count = priv->plat->rx_queues_to_use;
+	u32 queue;
+	int i;
+
+	for (queue = 0; queue < rx_count; queue++) {
+		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
+
+		for (i = 0; i < priv->dma_rx_size; i++) {
+			struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
+
+			if (buf->page) {
+				page_pool_recycle_direct(rx_q->page_pool, buf->page);
+				buf->page = NULL;
+			}
+
+			if (priv->sph && buf->sec_page) {
+				page_pool_recycle_direct(rx_q->page_pool, buf->sec_page);
+				buf->sec_page = NULL;
+			}
+		}
+	}
+
+	for (queue = 0; queue < rx_count; queue++) {
+		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
+
+		for (i = 0; i < priv->dma_rx_size; i++) {
+			struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
+			struct dma_desc *p;
+
+			if (priv->extend_desc)
+				p = &((rx_q->dma_erx + i)->basic);
+			else
+				p = rx_q->dma_rx + i;
+
+			if (!buf->page) {
+				buf->page = page_pool_dev_alloc_pages(rx_q->page_pool);
+				if (!buf->page)
+					goto err_reinit_rx_buffers;
+
+				buf->addr = page_pool_get_dma_addr(buf->page);
+			}
+
+			if (priv->sph && !buf->sec_page) {
+				buf->sec_page = page_pool_dev_alloc_pages(rx_q->page_pool);
+				if (!buf->sec_page)
+					goto err_reinit_rx_buffers;
+
+				buf->sec_addr = page_pool_get_dma_addr(buf->sec_page);
+			}
+
+			stmmac_set_desc_addr(priv, p, buf->addr);
+			if (priv->sph)
+				stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, true);
+			else
+				stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, false);
+			if (priv->dma_buf_sz == BUF_SIZE_16KiB)
+				stmmac_init_desc3(priv, p);
+		}
+	}
+
+	return 0;
+
+err_reinit_rx_buffers:
+	while (queue >= 0) {
+		while (--i >= 0)
+			stmmac_free_rx_buffer(priv, queue, i);
+
+		if (queue == 0)
+			break;
+
+		i = priv->dma_rx_size;
+		queue--;
+	}
+
+	return -ENOMEM;
+}
+
 /**
  * init_dma_rx_desc_rings - init the RX descriptor rings
  * @dev: net device structure
@@ -5339,7 +5424,7 @@ int stmmac_resume(struct device *dev)
 	mutex_lock(&priv->lock);
 
 	stmmac_reset_queues_param(priv);
-
+	stmmac_reinit_rx_buffers(priv);
 	stmmac_free_tx_skbufs(priv);
 	stmmac_clear_descriptors(priv);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock
  2021-01-26 11:58 ` [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock Joakim Zhang
@ 2021-01-26 22:46   ` Willem de Bruijn
  2021-01-27  1:30     ` Joakim Zhang
  0 siblings, 1 reply; 14+ messages in thread
From: Willem de Bruijn @ 2021-01-26 22:46 UTC (permalink / raw)
  To: Joakim Zhang
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David Miller,
	Jakub Kicinski, Network Development, linux-imx, Andrew Lunn,
	Florian Fainelli

On Tue, Jan 26, 2021 at 7:05 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
> Remove redundant null check for ptp clock.
>
> Fixes: 1c35cc9cf6a0 ("net: stmmac: remove redundant null check before clk_disable_unprepare()")

This does not look like a fix to that patch, but another instance of a cleanup.

The patchset also does not explicitly target net (for fixes) or
net-next (for new improvements). I suppose this patch targets
net-next.

> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 26b971cd4da5..11e0b30b2e01 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5291,8 +5291,7 @@ int stmmac_resume(struct device *dev)
>                 /* enable the clk previously disabled */
>                 clk_prepare_enable(priv->plat->stmmac_clk);
>                 clk_prepare_enable(priv->plat->pclk);
> -               if (priv->plat->clk_ptp_ref)
> -                       clk_prepare_enable(priv->plat->clk_ptp_ref);
> +               clk_prepare_enable(priv->plat->clk_ptp_ref);
>                 /* reset the phy so that it's ready */
>                 if (priv->mii)
>                         stmmac_mdio_reset(priv->mii);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 2/6] net: stmmac: stop each tx channel independently
  2021-01-26 11:58 ` [PATCH V3 2/6] net: stmmac: stop each tx channel independently Joakim Zhang
@ 2021-01-26 23:09   ` Willem de Bruijn
  2021-01-27  1:44     ` Joakim Zhang
  0 siblings, 1 reply; 14+ messages in thread
From: Willem de Bruijn @ 2021-01-26 23:09 UTC (permalink / raw)
  To: Joakim Zhang
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David Miller,
	Jakub Kicinski, Network Development, linux-imx, Andrew Lunn,
	Florian Fainelli

On Tue, Jan 26, 2021 at 7:03 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
> If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users
> may only want to stop secific tx channel.

secific -> specific

>
> Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> index 0b4ee2dbb691..71e50751ef2d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> @@ -53,10 +53,6 @@ void dwmac4_dma_stop_tx(void __iomem *ioaddr, u32 chan)
>
>         value &= ~DMA_CONTROL_ST;
>         writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
> -
> -       value = readl(ioaddr + GMAC_CONFIG);
> -       value &= ~GMAC_CONFIG_TE;
> -       writel(value, ioaddr + GMAC_CONFIG);

Is it safe to partially unwind the actions of dwmac4_dma_start_tx

And would the same reasoning apply to dwmac4_(dma_start|stop)_rx?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock
  2021-01-26 22:46   ` Willem de Bruijn
@ 2021-01-27  1:30     ` Joakim Zhang
  2021-01-27  1:33       ` Florian Fainelli
  0 siblings, 1 reply; 14+ messages in thread
From: Joakim Zhang @ 2021-01-27  1:30 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David Miller,
	Jakub Kicinski, Network Development, dl-linux-imx, Andrew Lunn,
	Florian Fainelli


> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: 2021年1月27日 6:46
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David
> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Network
> Development <netdev@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>;
> Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>
> Subject: Re: [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp
> clock
> 
> On Tue, Jan 26, 2021 at 7:05 AM Joakim Zhang <qiangqing.zhang@nxp.com>
> wrote:
> >
> > Remove redundant null check for ptp clock.
> >
> > Fixes: 1c35cc9cf6a0 ("net: stmmac: remove redundant null check before
> > clk_disable_unprepare()")
> 
> This does not look like a fix to that patch, but another instance of a cleanup.
> 
> The patchset also does not explicitly target net (for fixes) or net-next (for new
> improvements). I suppose this patch targets net-next.

I forgot to explicitly target as net when format the patch set again. This could be a fix even original patch(1c35cc9cf6a0) doesn't break anything, but it didn't do all the work as commit message commit.
This patch targets net or net-next, this matter doesn't seem to be that important. If it is necessary, I can repost it next time as a separate patch for net-next. Thanks.

Best Regards,
Joakim Zhang
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 26b971cd4da5..11e0b30b2e01 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -5291,8 +5291,7 @@ int stmmac_resume(struct device *dev)
> >                 /* enable the clk previously disabled */
> >                 clk_prepare_enable(priv->plat->stmmac_clk);
> >                 clk_prepare_enable(priv->plat->pclk);
> > -               if (priv->plat->clk_ptp_ref)
> > -                       clk_prepare_enable(priv->plat->clk_ptp_ref);
> > +               clk_prepare_enable(priv->plat->clk_ptp_ref);
> >                 /* reset the phy so that it's ready */
> >                 if (priv->mii)
> >                         stmmac_mdio_reset(priv->mii);
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock
  2021-01-27  1:30     ` Joakim Zhang
@ 2021-01-27  1:33       ` Florian Fainelli
  2021-01-27  1:49         ` Joakim Zhang
  0 siblings, 1 reply; 14+ messages in thread
From: Florian Fainelli @ 2021-01-27  1:33 UTC (permalink / raw)
  To: Joakim Zhang, Willem de Bruijn
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David Miller,
	Jakub Kicinski, Network Development, dl-linux-imx, Andrew Lunn



On 1/26/2021 5:30 PM, Joakim Zhang wrote:
> 
>> -----Original Message-----
>> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
>> Sent: 2021年1月27日 6:46
>> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
>> <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David
>> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Network
>> Development <netdev@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>;
>> Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>
>> Subject: Re: [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp
>> clock
>>
>> On Tue, Jan 26, 2021 at 7:05 AM Joakim Zhang <qiangqing.zhang@nxp.com>
>> wrote:
>>>
>>> Remove redundant null check for ptp clock.
>>>
>>> Fixes: 1c35cc9cf6a0 ("net: stmmac: remove redundant null check before
>>> clk_disable_unprepare()")
>>
>> This does not look like a fix to that patch, but another instance of a cleanup.
>>
>> The patchset also does not explicitly target net (for fixes) or net-next (for new
>> improvements). I suppose this patch targets net-next.
> 
> I forgot to explicitly target as net when format the patch set again. This could be a fix even original patch(1c35cc9cf6a0) doesn't break anything, but it didn't do all the work as commit message commit.
> This patch targets net or net-next, this matter doesn't seem to be that important. If it is necessary, I can repost it next time as a separate patch for net-next. Thanks.

Your patch series is titled "ethernet: fixes for stmmac driver" so we
sort of expect to find only bug fixes in there.

Given this patch has no dependency and does not create one on the
others, you should post that as a separate patch targeting the
'net-next' tree.
-- 
Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH V3 2/6] net: stmmac: stop each tx channel independently
  2021-01-26 23:09   ` Willem de Bruijn
@ 2021-01-27  1:44     ` Joakim Zhang
  2021-01-27  2:16       ` Willem de Bruijn
  0 siblings, 1 reply; 14+ messages in thread
From: Joakim Zhang @ 2021-01-27  1:44 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David Miller,
	Jakub Kicinski, Network Development, dl-linux-imx, Andrew Lunn,
	Florian Fainelli


> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: 2021年1月27日 7:10
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David
> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Network
> Development <netdev@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>;
> Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>
> Subject: Re: [PATCH V3 2/6] net: stmmac: stop each tx channel independently
> 
> On Tue, Jan 26, 2021 at 7:03 AM Joakim Zhang <qiangqing.zhang@nxp.com>
> wrote:
> >
> > If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users
> > may only want to stop secific tx channel.
> 
> secific -> specific

Thanks. Will correct it.

> >
> > Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > index 0b4ee2dbb691..71e50751ef2d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > @@ -53,10 +53,6 @@ void dwmac4_dma_stop_tx(void __iomem *ioaddr,
> u32
> > chan)
> >
> >         value &= ~DMA_CONTROL_ST;
> >         writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
> > -
> > -       value = readl(ioaddr + GMAC_CONFIG);
> > -       value &= ~GMAC_CONFIG_TE;
> > -       writel(value, ioaddr + GMAC_CONFIG);
> 
> Is it safe to partially unwind the actions of dwmac4_dma_start_tx
> 
> And would the same reasoning apply to dwmac4_(dma_start|stop)_rx?

Sorry, I am not quite understand what you means.

What this patch did is to align to dwmac4_(dma_start|stop)_rx.

dwmac4_dma_start_rx: assert DMA_CONTROL_SR bit for each channel, and assert GMAC_CONFIG_RE bit which targets all channels.
dwmac4_dma_stop_rx: only need clear DMA_CONTROL_SR bit for each channel.

After this patch applied:
dwmac4_dma_start_tx: assert DMA_CONTROL_ST bit for each channel, and assert GMAC_CONFIG_TE bit which targets all channels.
dwmac4_dma_stop_tx: only need clear DMA_CONTROL_ST bit for each channel.

You can refer to drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c, this is the correct sequence. 

Thanks.

Best Regards,
Joakim Zhang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock
  2021-01-27  1:33       ` Florian Fainelli
@ 2021-01-27  1:49         ` Joakim Zhang
  0 siblings, 0 replies; 14+ messages in thread
From: Joakim Zhang @ 2021-01-27  1:49 UTC (permalink / raw)
  To: Florian Fainelli, Willem de Bruijn
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David Miller,
	Jakub Kicinski, Network Development, dl-linux-imx, Andrew Lunn


> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: 2021年1月27日 9:33
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Willem de Bruijn
> <willemdebruijn.kernel@gmail.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David
> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Network
> Development <netdev@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>;
> Andrew Lunn <andrew@lunn.ch>
> Subject: Re: [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp
> clock
> 
> 
> 
> On 1/26/2021 5:30 PM, Joakim Zhang wrote:
> >
> >> -----Original Message-----
> >> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> >> Sent: 2021年1月27日 6:46
> >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> >> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> >> <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David
> >> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>;
> >> Network Development <netdev@vger.kernel.org>; dl-linux-imx
> >> <linux-imx@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
> >> <f.fainelli@gmail.com>
> >> Subject: Re: [PATCH V3 1/6] net: stmmac: remove redundant null check
> >> for ptp clock
> >>
> >> On Tue, Jan 26, 2021 at 7:05 AM Joakim Zhang
> >> <qiangqing.zhang@nxp.com>
> >> wrote:
> >>>
> >>> Remove redundant null check for ptp clock.
> >>>
> >>> Fixes: 1c35cc9cf6a0 ("net: stmmac: remove redundant null check
> >>> before
> >>> clk_disable_unprepare()")
> >>
> >> This does not look like a fix to that patch, but another instance of a cleanup.
> >>
> >> The patchset also does not explicitly target net (for fixes) or
> >> net-next (for new improvements). I suppose this patch targets net-next.
> >
> > I forgot to explicitly target as net when format the patch set again. This could
> be a fix even original patch(1c35cc9cf6a0) doesn't break anything, but it didn't
> do all the work as commit message commit.
> > This patch targets net or net-next, this matter doesn't seem to be that
> important. If it is necessary, I can repost it next time as a separate patch for
> net-next. Thanks.
> 
> Your patch series is titled "ethernet: fixes for stmmac driver" so we sort of
> expect to find only bug fixes in there.
> 
> Given this patch has no dependency and does not create one on the others, you
> should post that as a separate patch targeting the 'net-next' tree.

Ok, I will remove this patch from patch set next version. Thanks.

Best Regards,
Joakim Zhang
> --
> Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH V3 2/6] net: stmmac: stop each tx channel independently
  2021-01-27  1:44     ` Joakim Zhang
@ 2021-01-27  2:16       ` Willem de Bruijn
  0 siblings, 0 replies; 14+ messages in thread
From: Willem de Bruijn @ 2021-01-27  2:16 UTC (permalink / raw)
  To: Joakim Zhang
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David Miller,
	Jakub Kicinski, Network Development, dl-linux-imx, Andrew Lunn,
	Florian Fainelli

On Tue, Jan 26, 2021 at 8:44 PM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
>
> > -----Original Message-----
> > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Sent: 2021年1月27日 7:10
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> > <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David
> > Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Network
> > Development <netdev@vger.kernel.org>; dl-linux-imx <linux-imx@nxp.com>;
> > Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>
> > Subject: Re: [PATCH V3 2/6] net: stmmac: stop each tx channel independently
> >
> > On Tue, Jan 26, 2021 at 7:03 AM Joakim Zhang <qiangqing.zhang@nxp.com>
> > wrote:
> > >
> > > If clear GMAC_CONFIG_TE bit, it would stop all tx channels, but users
> > > may only want to stop secific tx channel.
> >
> > secific -> specific
>
> Thanks. Will correct it.
>
> > >
> > > Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")
> > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > > ---
> > >  drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 4 ----
> > >  1 file changed, 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > > b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > > index 0b4ee2dbb691..71e50751ef2d 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
> > > @@ -53,10 +53,6 @@ void dwmac4_dma_stop_tx(void __iomem *ioaddr,
> > u32
> > > chan)
> > >
> > >         value &= ~DMA_CONTROL_ST;
> > >         writel(value, ioaddr + DMA_CHAN_TX_CONTROL(chan));
> > > -
> > > -       value = readl(ioaddr + GMAC_CONFIG);
> > > -       value &= ~GMAC_CONFIG_TE;
> > > -       writel(value, ioaddr + GMAC_CONFIG);
> >
> > Is it safe to partially unwind the actions of dwmac4_dma_start_tx
> >
> > And would the same reasoning apply to dwmac4_(dma_start|stop)_rx?
>
> Sorry, I am not quite understand what you means.
>
> What this patch did is to align to dwmac4_(dma_start|stop)_rx.
>
> dwmac4_dma_start_rx: assert DMA_CONTROL_SR bit for each channel, and assert GMAC_CONFIG_RE bit which targets all channels.
> dwmac4_dma_stop_rx: only need clear DMA_CONTROL_SR bit for each channel.
>
> After this patch applied:
> dwmac4_dma_start_tx: assert DMA_CONTROL_ST bit for each channel, and assert GMAC_CONFIG_TE bit which targets all channels.
> dwmac4_dma_stop_tx: only need clear DMA_CONTROL_ST bit for each channel.

Oh indeed. Sorry, I should have seen that it exactly brings the tx
logic into agreement with rx. Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-01-27 13:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 11:58 [PATCH V3 0/6] ethernet: fixes for stmmac driver Joakim Zhang
2021-01-26 11:58 ` [PATCH V3 1/6] net: stmmac: remove redundant null check for ptp clock Joakim Zhang
2021-01-26 22:46   ` Willem de Bruijn
2021-01-27  1:30     ` Joakim Zhang
2021-01-27  1:33       ` Florian Fainelli
2021-01-27  1:49         ` Joakim Zhang
2021-01-26 11:58 ` [PATCH V3 2/6] net: stmmac: stop each tx channel independently Joakim Zhang
2021-01-26 23:09   ` Willem de Bruijn
2021-01-27  1:44     ` Joakim Zhang
2021-01-27  2:16       ` Willem de Bruijn
2021-01-26 11:58 ` [PATCH V3 3/6] net: stmmac: fix watchdog timeout during suspend/resume stress test Joakim Zhang
2021-01-26 11:58 ` [PATCH V3 4/6] net: stmmac: fix dma physical address of descriptor when display ring Joakim Zhang
2021-01-26 11:58 ` [PATCH V3 5/6] net: stmmac: fix wrongly set buffer2 valid when sph unsupport Joakim Zhang
2021-01-26 11:58 ` [PATCH V3 6/6] net: stmmac: re-init rx buffers when mac resume back Joakim Zhang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.