From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Joakim Zhang <qiangqing.zhang@nxp.com>
Cc: Andrew Lunn <andrew@lunn.ch>, Vladimir Oltean <olteanv@gmail.com>,
"peppe.cavallaro@st.com" <peppe.cavallaro@st.com>,
"alexandre.torgue@foss.st.com" <alexandre.torgue@foss.st.com>,
"joabreu@synopsys.com" <joabreu@synopsys.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"kuba@kernel.org" <kuba@kernel.org>,
"mcoquelin.stm32@gmail.com" <mcoquelin.stm32@gmail.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
"hkallweit1@gmail.com" <hkallweit1@gmail.com>,
dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH] net: stmmac: fix MAC not working when system resume back with WoL enabled
Date: Fri, 3 Sep 2021 21:12:10 +0100 [thread overview]
Message-ID: <20210903201210.GF1350@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210903120127.GW22278@shell.armlinux.org.uk>
Hi,
Here's a patch to try - you'll need to integrate the new calls into
stmmac's suspend and resume hooks. Obviously, given my previous
comments, this isn't tested!
I didn't need to repeat the mac_wol boolean to phylink_resume as we
can record the state internally - mac_wol should not change between
a call to phylink_suspend() and subsequent phylink_resume() anyway.
mac_wol should only be true if the MAC is involved in processing
packets for WoL, false otherwise.
Please let me know if this resolves your stmmac WoL issue.
Thanks.
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index f0c769027145..c4d0de04416a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -33,6 +33,7 @@
enum {
PHYLINK_DISABLE_STOPPED,
PHYLINK_DISABLE_LINK,
+ PHYLINK_DISABLE_MAC_WOL,
};
/**
@@ -1313,6 +1314,9 @@ EXPORT_SYMBOL_GPL(phylink_start);
* network device driver's &struct net_device_ops ndo_stop() method. The
* network device's carrier state should not be changed prior to calling this
* function.
+ *
+ * This will synchronously bring down the link if the link is not already
+ * down (in other words, it will trigger a mac_link_down() method call.)
*/
void phylink_stop(struct phylink *pl)
{
@@ -1338,6 +1342,81 @@ void phylink_stop(struct phylink *pl)
}
EXPORT_SYMBOL_GPL(phylink_stop);
+
+/**
+ * phylink_suspend() - handle a network device suspend event
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @mac_wol: true if the MAC needs to receive packets for Wake-on-Lan
+ *
+ * Handle a network device suspend event. There are several cases:
+ * - If Wake-on-Lan is not active, we can bring down the link between
+ * the MAC and PHY by calling phylink_stop().
+ * - If Wake-on-Lan is active, and being handled only by the PHY, we
+ * can also bring down the link between the MAC and PHY.
+ * - If Wake-on-Lan is active, but being handled by the MAC, the MAC
+ * still needs to receive packets, so we can not bring the link down.
+ */
+void phylink_suspend(struct phylink *pl, bool mac_wol)
+{
+ ASSERT_RTNL();
+
+ if (mac_wol && (!pl->netdev || pl->netdev->wol_enabled)) {
+ /* Wake-on-Lan enabled, MAC handling */
+ mutex_lock(&pl->state_mutex);
+
+ /* Stop the resolver bringing the link up */
+ __set_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state);
+
+ /* Disable the carrier, to prevent transmit timeouts,
+ * but one would hope all packets have been sent.
+ */
+ netif_carrier_off(pl->netdev);
+
+ /* We do not call mac_link_down() here as we want the
+ * link to remain up to receive the WoL packets.
+ */
+ mutex_unlock(&pl->state_mutex);
+ } else {
+ phylink_stop(pl);
+ }
+}
+EXPORT_SYMBOL_GPL(phylink_suspend);
+
+/**
+ * phylink_resume() - handle a network device resume event
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * Undo the effects of phylink_suspend(), returning the link to an
+ * operational state.
+ */
+void phylink_resume(struct phylink *pl)
+{
+ ASSERT_RTNL();
+
+ if (test_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state)) {
+ /* Wake-on-Lan enabled, MAC handling */
+
+ /* Call mac_link_down() so we keep the overall state balanced.
+ * Do this under the state_mutex lock for consistency. This
+ * will cause a "Link Down" message to be printed during
+ * resume, which is harmless - the true link state will be
+ * printed when we run a resolve.
+ */
+ mutex_lock(&pl->state_mutex);
+ phylink_link_down(pl);
+ mutex_unlock(&pl->state_mutex);
+
+ /* Re-apply the link parameters so that all the settings get
+ * restored to the MAC.
+ */
+ phylink_mac_initial_config(pl, true);
+ phylink_enable_and_run_resolve(pl, PHYLINK_DISABLE_MAC_WOL);
+ } else {
+ phylink_start(pl);
+ }
+}
+EXPORT_SYMBOL_GPL(phylink_resume);
+
/**
* phylink_ethtool_get_wol() - get the wake on lan parameters for the PHY
* @pl: a pointer to a &struct phylink returned from phylink_create()
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index bdeec800da5c..ba0ab7126b96 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -462,6 +462,9 @@ void phylink_mac_change(struct phylink *, bool up);
void phylink_start(struct phylink *);
void phylink_stop(struct phylink *);
+void phylink_suspend(struct phylink *pl, bool mac_wol);
+void phylink_resume(struct phylink *pl);
+
void phylink_ethtool_get_wol(struct phylink *, struct ethtool_wolinfo *);
int phylink_ethtool_set_wol(struct phylink *, struct ethtool_wolinfo *);
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2021-09-03 20:12 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-01 9:02 [PATCH] net: stmmac: fix MAC not working when system resume back with WoL enabled Joakim Zhang
2021-09-01 9:13 ` Russell King (Oracle)
2021-09-01 10:21 ` Joakim Zhang
2021-09-01 12:56 ` Russell King (Oracle)
2021-09-02 7:01 ` Joakim Zhang
2021-09-01 15:40 ` Heiner Kallweit
2021-09-02 7:35 ` Joakim Zhang
2021-09-01 9:21 ` Vladimir Oltean
2021-09-01 10:25 ` Joakim Zhang
2021-09-01 10:56 ` Vladimir Oltean
2021-09-01 11:42 ` Joakim Zhang
2021-09-01 13:25 ` Russell King (Oracle)
2021-09-02 7:28 ` Joakim Zhang
2021-09-02 8:32 ` Russell King (Oracle)
2021-09-02 10:26 ` Joakim Zhang
2021-09-02 10:49 ` Russell King (Oracle)
2021-09-02 11:15 ` Joakim Zhang
2021-09-02 12:24 ` Andrew Lunn
2021-09-03 6:51 ` Joakim Zhang
2021-09-03 8:01 ` Russell King (Oracle)
2021-09-03 8:39 ` Joakim Zhang
2021-09-03 9:32 ` Russell King (Oracle)
2021-09-03 11:04 ` Joakim Zhang
2021-09-03 12:01 ` Russell King (Oracle)
2021-09-03 20:12 ` Russell King - ARM Linux admin [this message]
2021-09-06 2:29 ` Joakim Zhang
2021-09-06 9:34 ` Russell King (Oracle)
2021-09-06 10:41 ` Joakim Zhang
2021-09-06 11:21 ` Russell King (Oracle)
2021-09-06 13:23 ` Andrew Lunn
2021-09-07 8:52 ` Russell King (Oracle)
2021-09-06 2:21 ` Joakim Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210903201210.GF1350@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=joabreu@synopsys.com \
--cc=kuba@kernel.org \
--cc=linux-imx@nxp.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=peppe.cavallaro@st.com \
--cc=qiangqing.zhang@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).