| Message ID | 20210204101550.21595-3-qiangqing.zhang@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | patch for stmmac driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 4 maintainers not CCed: mcoquelin.stm32@gmail.com linux-stm32@st-md-mailman.stormreply.com linux@armlinux.org.uk linux-arm-kernel@lists.infradead.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 26 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Thu, Feb 4, 2021 at 5:18 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote: > > Slightly adjust the order of the codes in stmmac_resume(), remove the > check "if (!device_may_wakeup(priv->device) || !priv->plat->pmt)". > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> This commit message says what the code does, but not why or why it's correct.
> -----Original Message----- > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > Sent: 2021年2月4日 21:20 > 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>; Andrew Lunn <andrew@lunn.ch>; > Florian Fainelli <f.fainelli@gmail.com>; Willem de Bruijn > <willemdebruijn.kernel@gmail.com> > Subject: Re: [PATCH net-next 2/2] net: stmmac: slightly adjust the order of the > codes in stmmac_resume() > > On Thu, Feb 4, 2021 at 5:18 AM Joakim Zhang <qiangqing.zhang@nxp.com> > wrote: > > > > Slightly adjust the order of the codes in stmmac_resume(), remove the > > check "if (!device_may_wakeup(priv->device) || !priv->plat->pmt)". > > > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > > This commit message says what the code does, but not why or why it's correct. This just slight changes the code order, there is no function change, I will improve the commit message if needed. Best Regards, Joakim Zhang
On Thu, Feb 4, 2021 at 8:18 PM Joakim Zhang <qiangqing.zhang@nxp.com> wrote: > > > > -----Original Message----- > > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > > Sent: 2021年2月4日 21:20 > > 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>; Andrew Lunn <andrew@lunn.ch>; > > Florian Fainelli <f.fainelli@gmail.com>; Willem de Bruijn > > <willemdebruijn.kernel@gmail.com> > > Subject: Re: [PATCH net-next 2/2] net: stmmac: slightly adjust the order of the > > codes in stmmac_resume() > > > > On Thu, Feb 4, 2021 at 5:18 AM Joakim Zhang <qiangqing.zhang@nxp.com> > > wrote: > > > > > > Slightly adjust the order of the codes in stmmac_resume(), remove the > > > check "if (!device_may_wakeup(priv->device) || !priv->plat->pmt)". > > > > > > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> > > > > This commit message says what the code does, but not why or why it's correct. > > This just slight changes the code order, there is no function change, I will improve the commit message if needed. And it is correct to move this phylink initialization before serdes_powerup? That is not immediately obvious to me. A comment would definitely be good. More relevant than just stating that the code is moved, which I see from the patch easily.
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 11e0b30b2e01..94d4f5d294f4 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -5295,6 +5295,12 @@ int stmmac_resume(struct device *dev) /* reset the phy so that it's ready */ if (priv->mii) stmmac_mdio_reset(priv->mii); + + rtnl_lock(); + phylink_start(priv->phylink); + /* We may have called phylink_speed_down before */ + phylink_speed_up(priv->phylink); + rtnl_unlock(); } if (priv->plat->serdes_powerup) { @@ -5305,14 +5311,6 @@ 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);
Slightly adjust the order of the codes in stmmac_resume(), remove the check "if (!device_may_wakeup(priv->device) || !priv->plat->pmt)". Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)