[PATCH V2 0/5] patches for stmmac

[PATCH V2 0/5] patches for stmmac

[Joakim Zhang]

A patch set for stmmac, fix some driver issues.

ChangeLogs:
V1->V2:
	* add Fixes tag.
	* add patch 5/5 into this patch set.

Fugang Duan (5):
  net: stmmac: increase the timeout for dma reset
  net: stmmac: start phylink instance before stmmac_hw_setup()
  net: stmmac: free tx skb buffer in stmmac_resume()
  net: stmmac: delete the eee_ctrl_timer after napi disabled
  net: stmmac: overwrite the dma_cap.addr64 according to HW design

 .../net/ethernet/stmicro/stmmac/dwmac-imx.c   |  9 +---
 .../net/ethernet/stmicro/stmmac/dwmac4_lib.c  |  2 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 51 +++++++++++++++----
 include/linux/stmmac.h                        |  1 +
 4 files changed, 43 insertions(+), 20 deletions(-)

-- 
2.17.1

[PATCH V2 1/5] net: stmmac: increase the timeout for dma reset

[Joakim Zhang]

From: Fugang Duan <fugang.duan@nxp.com>

Current timeout value is not enough for gmac5 dma reset
on imx8mp platform, increase the timeout range.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
index 6e30d7eb4983..0b4ee2dbb691 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
@@ -22,7 +22,7 @@ int dwmac4_dma_reset(void __iomem *ioaddr)
 
 	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
 				 !(value & DMA_BUS_MODE_SFT_RESET),
-				 10000, 100000);
+				 10000, 1000000);
 }
 
 void dwmac4_set_rx_tail_ptr(void __iomem *ioaddr, u32 tail_ptr, u32 chan)
-- 
2.17.1

[PATCH V2 2/5] net: stmmac: start phylink instance before stmmac_hw_setup()

[Joakim Zhang]

From: Fugang Duan <fugang.duan@nxp.com>

Start phylink instance and resume back the PHY to supply
RX clock to MAC before MAC layer initialization by calling
.stmmac_hw_setup(), since DMA reset depends on the RX clock,
otherwise DMA reset cost maximum timeout value then finally
timeout.

Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c    | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8c1ac75901ce..107761ef456a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5277,6 +5277,14 @@ int stmmac_resume(struct device *dev)
 			return ret;
 	}
 
+	if (!device_may_wakeup(priv->device) || !priv->plat->pmt) {
+		rtnl_lock();
+		phylink_start(priv->phylink);
+		/* We may have called phylink_speed_down before */
+		phylink_speed_up(priv->phylink);
+		rtnl_unlock();
+	}
+
 	rtnl_lock();
 	mutex_lock(&priv->lock);
 
@@ -5295,14 +5303,6 @@ int stmmac_resume(struct device *dev)
 	mutex_unlock(&priv->lock);
 	rtnl_unlock();
 
-	if (!device_may_wakeup(priv->device) || !priv->plat->pmt) {
-		rtnl_lock();
-		phylink_start(priv->phylink);
-		/* We may have called phylink_speed_down before */
-		phylink_speed_up(priv->phylink);
-		rtnl_unlock();
-	}
-
 	phylink_mac_change(priv->phylink, true);
 
 	netif_device_attach(ndev);
-- 
2.17.1

[PATCH V2 3/5] net: stmmac: free tx skb buffer in stmmac_resume()

[Joakim Zhang]

From: Fugang Duan <fugang.duan@nxp.com>

When do suspend/resume test, there have WARN_ON() log dump from
stmmac_xmit() funciton, the code logic:
	entry = tx_q->cur_tx;
	first_entry = entry;
	WARN_ON(tx_q->tx_skbuff[first_entry]);

In normal case, tx_q->tx_skbuff[txq->cur_tx] should be NULL because
the skb should be handled and freed in stmmac_tx_clean().

But stmmac_resume() reset queue parameters like below, skb buffers
may not be freed.
	tx_q->cur_tx = 0;
	tx_q->dirty_tx = 0;

So free tx skb buffer in stmmac_resume() to avoid warning and
memory leak.

log:
[   46.139824] ------------[ cut here ]------------
[   46.144453] WARNING: CPU: 0 PID: 0 at drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3235 stmmac_xmit+0x7a0/0x9d0
[   46.154969] Modules linked in: crct10dif_ce vvcam(O) flexcan can_dev
[   46.161328] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G           O      5.4.24-2.1.0+g2ad925d15481 #1
[   46.170369] Hardware name: NXP i.MX8MPlus EVK board (DT)
[   46.175677] pstate: 80000005 (Nzcv daif -PAN -UAO)
[   46.180465] pc : stmmac_xmit+0x7a0/0x9d0
[   46.184387] lr : dev_hard_start_xmit+0x94/0x158
[   46.188913] sp : ffff800010003cc0
[   46.192224] x29: ffff800010003cc0 x28: ffff000177e2a100
[   46.197533] x27: ffff000176ef0840 x26: ffff000176ef0090
[   46.202842] x25: 0000000000000000 x24: 0000000000000000
[   46.208151] x23: 0000000000000003 x22: ffff8000119ddd30
[   46.213460] x21: ffff00017636f000 x20: ffff000176ef0cc0
[   46.218769] x19: 0000000000000003 x18: 0000000000000000
[   46.224078] x17: 0000000000000000 x16: 0000000000000000
[   46.229386] x15: 0000000000000079 x14: 0000000000000000
[   46.234695] x13: 0000000000000003 x12: 0000000000000003
[   46.240003] x11: 0000000000000010 x10: 0000000000000010
[   46.245312] x9 : ffff00017002b140 x8 : 0000000000000000
[   46.250621] x7 : ffff00017636f000 x6 : 0000000000000010
[   46.255930] x5 : 0000000000000001 x4 : ffff000176ef0000
[   46.261238] x3 : 0000000000000003 x2 : 00000000ffffffff
[   46.266547] x1 : ffff000177e2a000 x0 : 0000000000000000
[   46.271856] Call trace:
[   46.274302]  stmmac_xmit+0x7a0/0x9d0
[   46.277874]  dev_hard_start_xmit+0x94/0x158
[   46.282056]  sch_direct_xmit+0x11c/0x338
[   46.285976]  __qdisc_run+0x118/0x5f0
[   46.289549]  net_tx_action+0x110/0x198
[   46.293297]  __do_softirq+0x120/0x23c
[   46.296958]  irq_exit+0xb8/0xd8
[   46.300098]  __handle_domain_irq+0x64/0xb8
[   46.304191]  gic_handle_irq+0x5c/0x148
[   46.307936]  el1_irq+0xb8/0x180
[   46.311076]  cpuidle_enter_state+0x84/0x360
[   46.315256]  cpuidle_enter+0x34/0x48
[   46.318829]  call_cpuidle+0x18/0x38
[   46.322314]  do_idle+0x1e0/0x280
[   46.325539]  cpu_startup_entry+0x24/0x40
[   46.329460]  rest_init+0xd4/0xe0
[   46.332687]  arch_call_rest_init+0xc/0x14
[   46.336695]  start_kernel+0x420/0x44c
[   46.340353] ---[ end trace bc1ee695123cbacd ]---

Fixes: 47dd7a540b8a0 ("net: add support for STMicroelectronics Ethernet controllers.")
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 107761ef456a..53c5d77eba57 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1557,6 +1557,19 @@ static void dma_free_tx_skbufs(struct stmmac_priv *priv, u32 queue)
 		stmmac_free_tx_buffer(priv, queue, i);
 }
 
+/**
+ * stmmac_free_tx_skbufs - free TX skb buffers
+ * @priv: private structure
+ */
+static void stmmac_free_tx_skbufs(struct stmmac_priv *priv)
+{
+	u32 tx_queue_cnt = priv->plat->tx_queues_to_use;
+	u32 queue;
+
+	for (queue = 0; queue < tx_queue_cnt; queue++)
+		dma_free_tx_skbufs(priv, queue);
+}
+
 /**
  * free_dma_rx_desc_resources - free RX dma desc resources
  * @priv: private structure
@@ -5290,6 +5303,7 @@ int stmmac_resume(struct device *dev)
 
 	stmmac_reset_queues_param(priv);
 
+	stmmac_free_tx_skbufs(priv);
 	stmmac_clear_descriptors(priv);
 
 	stmmac_hw_setup(ndev, false);
-- 
2.17.1

[PATCH V2 4/5] net: stmmac: delete the eee_ctrl_timer after napi disabled

[Joakim Zhang]

From: Fugang Duan <fugang.duan@nxp.com>

There have chance to re-enable the eee_ctrl_timer and fire the timer
in napi callback after delete the timer in .stmmac_release(), which
introduces to access eee registers in the timer function after clocks
are disabled then causes system hang. Found this issue when do
suspend/resume and reboot stress test.

It is safe to delete the timer after napi disabled and disable lpi mode.

Fixes: d765955d2ae0b ("stmmac: add the Energy Efficient Ethernet support")
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 53c5d77eba57..03c6995d276a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2938,9 +2938,6 @@ static int stmmac_release(struct net_device *dev)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	u32 chan;
 
-	if (priv->eee_enabled)
-		del_timer_sync(&priv->eee_ctrl_timer);
-
 	if (device_may_wakeup(priv->device))
 		phylink_speed_down(priv->phylink, false);
 	/* Stop and disconnect the PHY */
@@ -2959,6 +2956,11 @@ static int stmmac_release(struct net_device *dev)
 	if (priv->lpi_irq > 0)
 		free_irq(priv->lpi_irq, dev);
 
+	if (priv->eee_enabled) {
+		priv->tx_path_in_lpi_mode = false;
+		del_timer_sync(&priv->eee_ctrl_timer);
+	}
+
 	/* Stop TX/RX DMA and clear the descriptors */
 	stmmac_stop_all_dma(priv);
 
@@ -5185,6 +5187,11 @@ int stmmac_suspend(struct device *dev)
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
 		hrtimer_cancel(&priv->tx_queue[chan].txtimer);
 
+	if (priv->eee_enabled) {
+		priv->tx_path_in_lpi_mode = false;
+		del_timer_sync(&priv->eee_ctrl_timer);
+	}
+
 	/* Stop TX/RX DMA */
 	stmmac_stop_all_dma(priv);
 
-- 
2.17.1

[PATCH V2 5/5] net: stmmac: overwrite the dma_cap.addr64 according to HW design

[Joakim Zhang]

From: Fugang Duan <fugang.duan@nxp.com>

The current IP register MAC_HW_Feature1[ADDR64] only defines
32/40/64 bit width, but some SOCs support others like i.MX8MP
support 34 bits but it maps to 40 bits width in MAC_HW_Feature1[ADDR64].
So overwrite dma_cap.addr64 according to HW real design.

Fixes: 94abdad6974a ("net: ethernet: dwmac: add ethernet glue logic for NXP imx8 chip")
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c   | 9 +--------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++++++
 include/linux/stmmac.h                            | 1 +
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index efef5476a577..223f69da7e95 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -246,13 +246,7 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 		goto err_parse_dt;
 	}
 
-	ret = dma_set_mask_and_coherent(&pdev->dev,
-					DMA_BIT_MASK(dwmac->ops->addr_width));
-	if (ret) {
-		dev_err(&pdev->dev, "DMA mask set failed\n");
-		goto err_dma_mask;
-	}
-
+	plat_dat->addr64 = dwmac->ops->addr_width;
 	plat_dat->init = imx_dwmac_init;
 	plat_dat->exit = imx_dwmac_exit;
 	plat_dat->fix_mac_speed = imx_dwmac_fix_speed;
@@ -272,7 +266,6 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 err_dwmac_init:
 err_drv_probe:
 	imx_dwmac_exit(pdev, plat_dat->bsp_priv);
-err_dma_mask:
 err_parse_dt:
 err_match_data:
 	stmmac_remove_config_dt(pdev, plat_dat);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 03c6995d276a..5b1c12ff98c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4975,6 +4975,14 @@ int stmmac_dvr_probe(struct device *device,
 		dev_info(priv->device, "SPH feature enabled\n");
 	}
 
+	/* The current IP register MAC_HW_Feature1[ADDR64] only define
+	 * 32/40/64 bit width, but some SOC support others like i.MX8MP
+	 * support 34 bits but it map to 40 bits width in MAC_HW_Feature1[ADDR64].
+	 * So overwrite dma_cap.addr64 according to HW real design.
+	 */
+	if (priv->plat->addr64)
+		priv->dma_cap.addr64 = priv->plat->addr64;
+
 	if (priv->dma_cap.addr64) {
 		ret = dma_set_mask_and_coherent(device,
 				DMA_BIT_MASK(priv->dma_cap.addr64));
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 628e28903b8b..15ca6b4167cc 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -170,6 +170,7 @@ struct plat_stmmacenet_data {
 	int unicast_filter_entries;
 	int tx_fifo_size;
 	int rx_fifo_size;
+	u32 addr64;
 	u32 rx_queues_to_use;
 	u32 tx_queues_to_use;
 	u8 rx_sched_algorithm;
-- 
2.17.1

Re: [PATCH V2 0/5] patches for stmmac

[Jakub Kicinski]

On Fri,  4 Dec 2020 10:46:33 +0800 Joakim Zhang wrote:
> A patch set for stmmac, fix some driver issues.

These don't apply cleanly to the net tree where fixes go:

https://patchwork.kernel.org/project/netdevbpf/list/?delegate=netdev&param=1&order=date

Please rebase / retest / repost.

RE: [PATCH V2 0/5] patches for stmmac

[Joakim Zhang]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: 2020年12月6日 5:40
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: peppe.cavallaro@st.com; alexandre.torgue@st.com;
> joabreu@synopsys.com; davem@davemloft.net; netdev@vger.kernel.org;
> dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH V2 0/5] patches for stmmac
> 
> On Fri,  4 Dec 2020 10:46:33 +0800 Joakim Zhang wrote:
> > A patch set for stmmac, fix some driver issues.
> 
> These don't apply cleanly to the net tree where fixes go:
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.kernel.org%2Fproject%2Fnetdevbpf%2Flist%2F%3Fdelegate%3Dnetdev%26
> param%3D1%26order%3Ddate&amp;data=04%7C01%7Cqiangqing.zhang%40
> nxp.com%7C78a0b4496e7a49d8fcfc08d899664aff%7C686ea1d3bc2b4c6fa92cd
> 99c5c301635%7C0%7C1%7C637428011934975450%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D%7C2000&amp;sdata=TO7GoQGWml8hlMyYV84bks1hXsAb%2FLQYue1U
> Y%2FpmIrM%3D&amp;reserved=0
> 
> Please rebase / retest / repost.

Hi Jakub,

I will rebase to the latest net tree, thanks.


Hi all guys,

I also want to report a stmmac driver issue here, someone may also suffer from it.

After I do hundreds of suspend/resume stress test, I can encounter below netdev watchdog timeout issue. Tx queue timed out then reset adapter.
===============================
suspend 1000 times
===============================

----  Test < suspend_quick_auto.sh > ended  ----
root@imx8mpevk:/unit_tests/Power_Management# [ 1347.976688] imx-dwmac 30bf0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
[ 1358.022784] ------------[ cut here ]------------
[ 1358.027430] NETDEV WATCHDOG: eth0 (imx-dwmac): transmit queue 0 timed out
[ 1358.035469] WARNING: CPU: 2 PID: 0 at net/sched/sch_generic.c:450 dev_watchdog+0x2fc/0x30c
[ 1358.043736] Modules linked in:
[ 1358.046798] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G        W         5.8.0-rc5-next-20200717-00007-g30d24ae22e81-dirty #333
[ 1358.058011] Hardware name: NXP i.MX8MPlus EVK board (DT)
[ 1358.063324] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--)
[ 1358.068898] pc : dev_watchdog+0x2fc/0x30c
[ 1358.072908] lr : dev_watchdog+0x2fc/0x30c
[ 1358.076915] sp : ffff800011c5bd90
[ 1358.080228] x29: ffff800011c5bd90 x28: ffff0001767f1940
[ 1358.085542] x27: 0000000000000004 x26: ffff000176e88440
[ 1358.090857] x25: 0000000000000140 x24: 00000000ffffffff
[ 1358.096171] x23: ffff000176e8839c x22: 0000000000000002
[ 1358.101484] x21: ffff8000119f6000 x20: ffff000176e88000
[ 1358.106799] x19: 0000000000000000 x18: 0000000000000030
[ 1358.112112] x17: 0000000000000001 x16: 00000018bf1a354e
[ 1358.117426] x15: ffff0001760eae70 x14: ffffffffffffffff
[ 1358.122740] x13: ffff800091c5ba77 x12: ffff800011c5ba80
[ 1358.128054] x11: ffffffffffffffff x10: ffff00017f38b7c0
[ 1358.133368] x9 : 00000000000c0000 x8 : 6928203068746520
[ 1358.138682] x7 : 3a474f4448435441 x6 : 0000000000000003
[ 1358.143996] x5 : 0000000000000000 x4 : 0000000000000000
[ 1358.149310] x3 : 0000000000000004 x2 : 0000000000000100
[ 1358.154624] x1 : b54950db346c9600 x0 : 0000000000000000
[ 1358.159939] Call trace:
[ 1358.162389]  dev_watchdog+0x2fc/0x30c
[ 1358.166055]  call_timer_fn.constprop.0+0x24/0x80
[ 1358.170673]  expire_timers+0x98/0xc4
[ 1358.174249]  run_timer_softirq+0xd0/0x200
[ 1358.178261]  efi_header_end+0x124/0x284
[ 1358.182098]  irq_exit+0xdc/0xfc
[ 1358.185241]  __handle_domain_irq+0x80/0xe0
[ 1358.189338]  gic_handle_irq+0xc8/0x170
[ 1358.193087]  el1_irq+0xbc/0x180
[ 1358.196230]  arch_cpu_idle+0x14/0x20
[ 1358.199807]  cpu_startup_entry+0x24/0x80
[ 1358.203732]  secondary_start_kernel+0x138/0x184
[ 1358.208262] ---[ end trace b422761fd811b2a7 ]---
[ 1358.213588] imx-dwmac 30bf0000.ethernet eth0: Reset adapter.
[ 1358.228037] imx-dwmac 30bf0000.ethernet eth0: PHY [stmmac-1:01] driver [RTL8211F Gigabit Ethernet] (irq=POLL)
[ 1358.246815] imx-dwmac 30bf0000.ethernet eth0: No Safety Features support found
[ 1358.254062] imx-dwmac 30bf0000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[ 1358.264130] imx-dwmac 30bf0000.ethernet eth0: registered PTP clock
[ 1358.270374] imx-dwmac 30bf0000.ethernet eth0: configuring for phy/rgmii-id link mode
[ 1358.279481] 8021q: adding VLAN 0 to HW filter on device eth0
[ 1360.328695] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[ 1360.335007] imx-dwmac 30bf0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx

I found this issue first in latest 5.10, and I confirm it is fine in 5.4. After a period of time digging into driver commit history, I got nothing. It should be related to stmmac core driver, un-related to platform driver.
So I think it could be reproduced on other platforms.

Could you please point me how to debug this issue? Now I don't know how to look into this issue further, as I take over ethernet driver in a short time.

Any feedback could be appreciated!

Joakim Zhang