| Message ID | 20230621135537.376649-1-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [net] net: stmmac: fix double serdes powerdown | expand |
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>
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 >
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 >
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 --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