| Message ID | 20210426090447.14323-1-qiangqing.zhang@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V2,net] net: stmmac: fix MAC WoL unwork if PHY doesn't support WoL | expand |
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: 2021年4月26日 21:05 > To: Joakim Zhang <qiangqing.zhang@nxp.com> > Cc: peppe.cavallaro@st.com; alexandre.torgue@st.com; > joabreu@synopsys.com; davem@davemloft.net; kuba@kernel.org; > f.fainelli@gmail.com; Jisheng.Zhang@synaptics.com; netdev@vger.kernel.org; > dl-linux-imx <linux-imx@nxp.com> > Subject: Re: [PATCH V2 net] net: stmmac: fix MAC WoL unwork if PHY doesn't > support WoL > > > + if (wol->wolopts & WAKE_PHY) { > > + int ret = phylink_ethtool_set_wol(priv->phylink, wol); > > This is wrong. No PHY actually implements WAKE_PHY. > > What PHYs do implement is WAKE_MAGIC, WAKE_MAGICSEC, WAKE_UCAST, > and WAKE_BCAST. So there is a clear overlap with what the MAC can do. > > So you need to decide which is going to provide each of these wakeups, the > MAC or the PHY, and make sure only one does the actual implementation. Hi Andrew, Thanks for your review:-), PHY wakeup have not test at my side, need @Jisheng.Zhang@synaptics.com have a test if possible. According to your comments, I did a quick draft, and have not test yet, could you please review the logic to see if I understand you correctly? Thanks. --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -647,18 +647,7 @@ static void stmmac_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol) static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) { struct stmmac_priv *priv = netdev_priv(dev); - u32 support = WAKE_MAGIC | WAKE_UCAST; - - if (!device_can_wakeup(priv->device)) - return -EOPNOTSUPP; - - if (!priv->plat->pmt) { - int ret = phylink_ethtool_set_wol(priv->phylink, wol); - - if (!ret) - device_set_wakeup_enable(priv->device, !!wol->wolopts); - return ret; - } + u32 support = WAKE_MAGIC | WAKE_UCAST | WAKE_MAGICSECURE | WAKE_BCAST; /* By default almost all GMAC devices support the WoL via * magic frame but we can disable it if the HW capability @@ -669,13 +658,24 @@ static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) if (wol->wolopts & ~support) return -EINVAL; - if (wol->wolopts) { - pr_info("stmmac: wakeup enable\n"); - device_set_wakeup_enable(priv->device, 1); - enable_irq_wake(priv->wol_irq); - } else { + if (!wol->wolopts) { + device_set_wakeup_capable(priv->device, 0); device_set_wakeup_enable(priv->device, 0); disable_irq_wake(priv->wol_irq); + } else { + if (priv->plat->pmt && ((wol->wolopts & WAKE_MAGIC) || (wol->wolopts & WAKE_UCAST))) { + pr_info("stmmac: mac wakeup enable\n"); + enable_irq_wake(priv->wol_irq); + } else { + int ret = phylink_ethtool_set_wol(priv->phylink, wol); + + if (!ret) + pr_info("stmmac: phy wakeup enable\n"); + else + return ret; + } + device_set_wakeup_capable(priv->device, 1); + device_set_wakeup_enable(priv->device, 1); } mutex_lock(&priv->lock); Best Regards, Joakim Zhang > Andrew
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index c5642985ef95..13430419ddfd 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -629,36 +629,28 @@ static void stmmac_get_strings(struct net_device *dev, u32 stringset, u8 *data) /* Currently only support WOL through Magic packet. */ static void stmmac_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol) { + struct ethtool_wolinfo wol_phy = { .cmd = ETHTOOL_GWOL }; struct stmmac_priv *priv = netdev_priv(dev); - if (!priv->plat->pmt) - return phylink_ethtool_get_wol(priv->phylink, wol); - mutex_lock(&priv->lock); - if (device_can_wakeup(priv->device)) { + if (priv->plat->pmt) { wol->supported = WAKE_MAGIC | WAKE_UCAST; if (priv->hw_cap_support && !priv->dma_cap.pmt_magic_frame) wol->supported &= ~WAKE_MAGIC; - wol->wolopts = priv->wolopts; } + + phylink_ethtool_get_wol(priv->phylink, &wol_phy); + + wol->supported |= wol_phy.supported; + wol->wolopts = priv->wolopts; + mutex_unlock(&priv->lock); } static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) { struct stmmac_priv *priv = netdev_priv(dev); - u32 support = WAKE_MAGIC | WAKE_UCAST; - - if (!device_can_wakeup(priv->device)) - return -EOPNOTSUPP; - - if (!priv->plat->pmt) { - int ret = phylink_ethtool_set_wol(priv->phylink, wol); - - if (!ret) - device_set_wakeup_enable(priv->device, !!wol->wolopts); - return ret; - } + u32 support = WAKE_MAGIC | WAKE_UCAST | WAKE_PHY; /* By default almost all GMAC devices support the WoL via * magic frame but we can disable it if the HW capability @@ -669,11 +661,23 @@ static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) if (wol->wolopts & ~support) return -EINVAL; + if (wol->wolopts & WAKE_PHY) { + int ret = phylink_ethtool_set_wol(priv->phylink, wol); + + if (!ret) { + device_set_wakeup_capable(priv->device, 1); + device_set_wakeup_enable(priv->device, 1); + } + return ret; + } + if (wol->wolopts) { pr_info("stmmac: wakeup enable\n"); + device_set_wakeup_capable(priv->device, 1); device_set_wakeup_enable(priv->device, 1); enable_irq_wake(priv->wol_irq); } else { + device_set_wakeup_capable(priv->device, 0); device_set_wakeup_enable(priv->device, 0); disable_irq_wake(priv->wol_irq); } diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index c6f24abf6432..d62d8c28463d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1076,7 +1076,6 @@ static void stmmac_check_pcs_mode(struct stmmac_priv *priv) */ static int stmmac_init_phy(struct net_device *dev) { - struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL }; struct stmmac_priv *priv = netdev_priv(dev); struct device_node *node; int ret; @@ -1102,9 +1101,6 @@ static int stmmac_init_phy(struct net_device *dev) ret = phylink_connect_phy(priv->phylink, phydev); } - phylink_ethtool_get_wol(priv->phylink, &wol); - device_set_wakeup_capable(priv->device, !!wol.supported); - return ret; } @@ -4787,10 +4783,8 @@ static int stmmac_hw_init(struct stmmac_priv *priv) if (priv->plat->tx_coe) dev_info(priv->device, "TX Checksum insertion supported\n"); - if (priv->plat->pmt) { + if (priv->plat->pmt) dev_info(priv->device, "Wake-Up On Lan supported\n"); - device_set_wakeup_capable(priv->device, 1); - } if (priv->dma_cap.tsoen) dev_info(priv->device, "TSO supported\n");
Both get and set WoL will check device_can_wakeup(), if MAC supports PMT, it will set device wakeup capability. After commit 1d8e5b0f3f2c ("net: stmmac: Support WOL with phy"), device wakeup capability will be overwrite in stmmac_init_phy() according to phy's Wol feature. If phy doesn't support WoL, then MAC will lose wakeup capability. This patch combines WoL capabilities both MAC and PHY from stmmac_get_wol(), and set wakeup capability in stmmac_set_wol() when enable WoL. Fixes: commit 1d8e5b0f3f2c ("net: stmmac: Support WOL with phy") Suggested-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com> --- .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 38 ++++++++++--------- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +--- 2 files changed, 22 insertions(+), 24 deletions(-)