[PATCH net 1/1] net: stmmac: Fix VLAN filter delete timeout issue in Intel mGBE SGMII

[PATCH net 1/1] net: stmmac: Fix VLAN filter delete timeout issue in Intel mGBE SGMII

[Ong Boon Leong]

For Intel mGbE controller, MAC VLAN filter delete operation will time-out
if serdes power-down sequence happened first during driver remove() with
below message.

[82294.764958] intel-eth-pci 0000:00:1e.4 eth2: stmmac_dvr_remove: removing driver
[82294.778677] intel-eth-pci 0000:00:1e.4 eth2: Timeout accessing MAC_VLAN_Tag_Filter
[82294.779997] intel-eth-pci 0000:00:1e.4 eth2: failed to kill vid 0081/0
[82294.947053] intel-eth-pci 0000:00:1d.2 eth1: stmmac_dvr_remove: removing driver
[82295.002091] intel-eth-pci 0000:00:1d.1 eth0: stmmac_dvr_remove: removing driver

Therefore, we delay the serdes power-down to be after unregister_netdev()
which triggers the VLAN filter delete.

Fixes: b9663b7ca6ff ("net: stmmac: Enable SERDES power up/down sequence")
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0eba44e9c1f8..208cae344ffa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5249,13 +5249,16 @@ int stmmac_dvr_remove(struct device *dev)
 	netdev_info(priv->dev, "%s: removing driver", __func__);
 
 	stmmac_stop_all_dma(priv);
+	stmmac_mac_set(priv, priv->ioaddr, false);
+	netif_carrier_off(ndev);
+	unregister_netdev(ndev);
 
+	/* Serdes power down needs to happen after VLAN filter
+	 * is deleted that is triggered by unregister_netdev().
+	 */
 	if (priv->plat->serdes_powerdown)
 		priv->plat->serdes_powerdown(ndev, priv->plat->bsp_priv);
 
-	stmmac_mac_set(priv, priv->ioaddr, false);
-	netif_carrier_off(ndev);
-	unregister_netdev(ndev);
 #ifdef CONFIG_DEBUG_FS
 	stmmac_exit_fs(ndev);
 #endif
-- 
2.17.0

Re: [PATCH net 1/1] net: stmmac: Fix VLAN filter delete timeout issue in Intel mGBE SGMII

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Fri,  5 Mar 2021 13:49:30 +0800 you wrote:
> For Intel mGbE controller, MAC VLAN filter delete operation will time-out
> if serdes power-down sequence happened first during driver remove() with
> below message.
> 
> [82294.764958] intel-eth-pci 0000:00:1e.4 eth2: stmmac_dvr_remove: removing driver
> [82294.778677] intel-eth-pci 0000:00:1e.4 eth2: Timeout accessing MAC_VLAN_Tag_Filter
> [82294.779997] intel-eth-pci 0000:00:1e.4 eth2: failed to kill vid 0081/0
> [82294.947053] intel-eth-pci 0000:00:1d.2 eth1: stmmac_dvr_remove: removing driver
> [82295.002091] intel-eth-pci 0000:00:1d.1 eth0: stmmac_dvr_remove: removing driver
> 
> [...]

Here is the summary with links:
  - [net,1/1] net: stmmac: Fix VLAN filter delete timeout issue in Intel mGBE SGMII
    https://git.kernel.org/netdev/net/c/9a7b3950c7e1

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net 1/1] net: stmmac: Fix VLAN filter delete timeout issue in Intel mGBE SGMII

[Vladimir Oltean]

On Fri, Mar 05, 2021 at 01:49:30PM +0800, Ong Boon Leong wrote:
> For Intel mGbE controller, MAC VLAN filter delete operation will time-out
> if serdes power-down sequence happened first during driver remove() with
> below message.
> 
> [82294.764958] intel-eth-pci 0000:00:1e.4 eth2: stmmac_dvr_remove: removing driver
> [82294.778677] intel-eth-pci 0000:00:1e.4 eth2: Timeout accessing MAC_VLAN_Tag_Filter
> [82294.779997] intel-eth-pci 0000:00:1e.4 eth2: failed to kill vid 0081/0
> [82294.947053] intel-eth-pci 0000:00:1d.2 eth1: stmmac_dvr_remove: removing driver
> [82295.002091] intel-eth-pci 0000:00:1d.1 eth0: stmmac_dvr_remove: removing driver
> 
> Therefore, we delay the serdes power-down to be after unregister_netdev()
> which triggers the VLAN filter delete.
> 
> Fixes: b9663b7ca6ff ("net: stmmac: Enable SERDES power up/down sequence")
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 0eba44e9c1f8..208cae344ffa 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5249,13 +5249,16 @@ int stmmac_dvr_remove(struct device *dev)
>  	netdev_info(priv->dev, "%s: removing driver", __func__);
>  
>  	stmmac_stop_all_dma(priv);
> +	stmmac_mac_set(priv, priv->ioaddr, false);
> +	netif_carrier_off(ndev);
> +	unregister_netdev(ndev);
>  
> +	/* Serdes power down needs to happen after VLAN filter
> +	 * is deleted that is triggered by unregister_netdev().
> +	 */
>  	if (priv->plat->serdes_powerdown)
>  		priv->plat->serdes_powerdown(ndev, priv->plat->bsp_priv);
>  
> -	stmmac_mac_set(priv, priv->ioaddr, false);
> -	netif_carrier_off(ndev);
> -	unregister_netdev(ndev);
>  #ifdef CONFIG_DEBUG_FS
>  	stmmac_exit_fs(ndev);
>  #endif
> -- 
> 2.17.0
> 

Don't you also want to fix the probe path?

stmmac_dvr_probe:
	ret = register_netdev(ndev);
	if (ret) {
		dev_err(priv->device, "%s: ERROR %i registering the device\n",
			__func__, ret);
		goto error_netdev_register;
	}

	if (priv->plat->serdes_powerup) {
		ret = priv->plat->serdes_powerup(ndev,
						 priv->plat->bsp_priv);

		if (ret < 0)
			goto error_serdes_powerup;
	}

Since the device can start being used immediately after registration, it
depends upon the SERDES clock being powered up.