diff mbox series

[net] net: stmmac: fix double serdes powerdown

Message ID 20230621135537.376649-1-brgl@bgdev.pl (mailing list archive)
State Accepted
Commit c4fc88ad2a765224a648db8ab35f125e120fe41b
Delegated to: Netdev Maintainers
Headers show
Series [net] net: stmmac: fix double serdes powerdown | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers fail 1 blamed authors not CCed: weifeng.voon@intel.com; 1 maintainers not CCed: weifeng.voon@intel.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 12 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Bartosz Golaszewski June 21, 2023, 1:55 p.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Commit 49725ffc15fc ("net: stmmac: power up/down serdes in
stmmac_open/release") correctly added a call to the serdes_powerdown()
callback to stmmac_release() but did not remove the one from
stmmac_remove() which leads to a doubled call to serdes_powerdown().

This can lead to all kinds of problems: in the case of the qcom ethqos
driver, it caused an unbalanced regulator disable splat.

Fixes: 49725ffc15fc ("net: stmmac: power up/down serdes in stmmac_open/release")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ------
 1 file changed, 6 deletions(-)

Comments

Jiri Pirko June 22, 2023, 8:13 a.m. UTC | #1 
Wed, Jun 21, 2023 at 03:55:37PM CEST, brgl@bgdev.pl wrote:
>From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
>Commit 49725ffc15fc ("net: stmmac: power up/down serdes in
>stmmac_open/release") correctly added a call to the serdes_powerdown()
>callback to stmmac_release() but did not remove the one from
>stmmac_remove() which leads to a doubled call to serdes_powerdown().
>
>This can lead to all kinds of problems: in the case of the qcom ethqos
>driver, it caused an unbalanced regulator disable splat.
>
>Fixes: 49725ffc15fc ("net: stmmac: power up/down serdes in stmmac_open/release")
>Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Andrew Halaney June 22, 2023, 6:54 p.m. UTC | #2 
On Wed, Jun 21, 2023 at 03:55:37PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Commit 49725ffc15fc ("net: stmmac: power up/down serdes in
> stmmac_open/release") correctly added a call to the serdes_powerdown()
> callback to stmmac_release() but did not remove the one from
> stmmac_remove() which leads to a doubled call to serdes_powerdown().
> 
> This can lead to all kinds of problems: in the case of the qcom ethqos
> driver, it caused an unbalanced regulator disable splat.
> 
> Fixes: 49725ffc15fc ("net: stmmac: power up/down serdes in stmmac_open/release")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com>

> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 10e8a5606ba6..4727f7be4f86 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7461,12 +7461,6 @@ void stmmac_dvr_remove(struct device *dev)
>  	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);
> -
>  #ifdef CONFIG_DEBUG_FS
>  	stmmac_exit_fs(ndev);
>  #endif
> -- 
> 2.39.2
>
Chang, Junxiao June 23, 2023, 12:05 a.m. UTC | #3 
Acked-by: Junxiao Chang <junxiao.chang@intel.com>

-----Original Message-----
From: Andrew Halaney <ahalaney@redhat.com> 
Sent: Friday, June 23, 2023 2:54 AM
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>; David S . Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>; Chang, Junxiao <junxiao.chang@intel.com>; netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Subject: Re: [PATCH net] net: stmmac: fix double serdes powerdown

On Wed, Jun 21, 2023 at 03:55:37PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Commit 49725ffc15fc ("net: stmmac: power up/down serdes in
> stmmac_open/release") correctly added a call to the serdes_powerdown() 
> callback to stmmac_release() but did not remove the one from
> stmmac_remove() which leads to a doubled call to serdes_powerdown().
> 
> This can lead to all kinds of problems: in the case of the qcom ethqos 
> driver, it caused an unbalanced regulator disable splat.
> 
> Fixes: 49725ffc15fc ("net: stmmac: power up/down serdes in 
> stmmac_open/release")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com>

> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 10e8a5606ba6..4727f7be4f86 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7461,12 +7461,6 @@ void stmmac_dvr_remove(struct device *dev)
>  	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);
> -
>  #ifdef CONFIG_DEBUG_FS
>  	stmmac_exit_fs(ndev);
>  #endif
> --
> 2.39.2
>
patchwork-bot+netdevbpf@kernel.org June 23, 2023, 2:40 a.m. UTC | #4 
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 21 Jun 2023 15:55:37 +0200 you wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Commit 49725ffc15fc ("net: stmmac: power up/down serdes in
> stmmac_open/release") correctly added a call to the serdes_powerdown()
> callback to stmmac_release() but did not remove the one from
> stmmac_remove() which leads to a doubled call to serdes_powerdown().
> 
> [...]

Here is the summary with links:
  - [net] net: stmmac: fix double serdes powerdown
    https://git.kernel.org/netdev/net/c/c4fc88ad2a76

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 10e8a5606ba6..4727f7be4f86 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7461,12 +7461,6 @@  void stmmac_dvr_remove(struct device *dev)
 	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);
-
 #ifdef CONFIG_DEBUG_FS
 	stmmac_exit_fs(ndev);
 #endif