netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Gatien CHEVALLIER <gatien.chevallier@foss.st.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Christophe Roullier <christophe.roullier@foss.st.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Simon Horman <horms@kernel.org>,
	Tristram Ha <Tristram.Ha@microchip.com>,
	Florian Fainelli <florian.fainelli@broadcom.com>,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 1/4] dt-bindings: net: document st,phy-wol property
Date: Tue, 22 Jul 2025 10:13:49 +0100	[thread overview]
Message-ID: <aH9WTYwty-tso66J@shell.armlinux.org.uk> (raw)
In-Reply-To: <5b8608cb-1369-4638-9cda-1cf90412fc0f@lunn.ch>

On Mon, Jul 21, 2025 at 07:07:11PM +0200, Andrew Lunn wrote:
> > Regarding this property, somewhat similar to "mediatek,mac-wol",
> > I need to position a flag at the mac driver level. I thought I'd go
> > using the same approach.
> 
> Ideally, you don't need such a flag. WoL should be done as low as
> possible. If the PHY can do the WoL, the PHY should be used. If not,
> fall back to MAC.
> 
> Many MAC drivers don't support this, or they get the implementation
> wrong. So it could be you need to fix the MAC driver.
> 
> MAC get_wol() should ask the PHY what it supports, and then OR in what
> the MAC supports.
> 
> When set_wol() is called, the MAC driver should ask the PHY driver to
> do it. If it return 0, all is good, and the MAC driver can be
> suspended when times comes. If the PHY driver returns EOPNOTSUPP, it
> means it cannot support all the enabled WoL operations, so the MAC
> driver needs to do some of them. The MAC driver then needs to ensure
> it is not suspended.
> 
> If the PHY driver is missing the interrupt used to wake the system,
> the get_wol() call should not return any supported WoL modes. The MAC
> will then do WoL. Your "vendor,mac-wol" property is then pointless.
> 
> Correctly describe the PHY in DT, list the interrupt it uses for
> waking the system.

This would be a good idea if we were talking about a new PHY and MAC
driver, but we aren't.

Given the number of platform drivers that stmmac has with numerous
PHY drivers, changing how this works _now_ will likely break current
setups. Whether PHY-side WoL is used is dependent on a
priv->plat->pmt flag which depends on MAC capabilties and also
whether the platform glue sets/clears the STMMAC_FLAG_USE_PHY_WOL
flag.

Yes, it's a mess, and it could do with being improved, which will
likely take considerable time to do to shake out any regressions
caused - both in stmmac and PHY drivers.

I bet there are _numerous_ PHY drivers that report and accept WoL
even when the hardware isn't wired to support WoL. For example,
AT8031 reports that it supports WAKE_KAGIC irrespective of whether
WOL_INT is wired, and whether or not the INT pin is capable of
waking the system. I don't think we have any way that a driver can
determine whether a particular interrupt _can_ wake the system.

The problem for stmmac is that the PHY driver may support WAKE_MAGIC,
but so can the MAC core. If the PHY isn't electrically capable of
waking the system for whatever reason, but the PHY driver still
reports that it can (like AT803x) and we don't program the MAC core,
then under your idea, WoL will no longer work.

The only thing I can think we can now do is to have yet another
STMMAC_FLAG_xxx which platform glue can set to enable a new behaviour.
Yay, a driver with multiple different behaviours depending on flags
for the same feature.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

  parent reply	other threads:[~2025-07-22  9:14 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-21 11:14 [PATCH net-next 0/4] net: add WoL from PHY support for stm32mp135f-dk Gatien Chevallier
2025-07-21 11:14 ` [PATCH net-next 1/4] dt-bindings: net: document st,phy-wol property Gatien Chevallier
2025-07-21 11:30   ` Krzysztof Kozlowski
2025-07-21 12:10     ` Gatien CHEVALLIER
2025-07-21 12:16       ` Krzysztof Kozlowski
2025-07-21 12:54         ` Gatien CHEVALLIER
2025-07-21 13:18       ` Andrew Lunn
2025-07-21 15:56         ` Gatien CHEVALLIER
2025-07-21 17:07           ` Andrew Lunn
2025-07-21 18:08             ` Florian Fainelli
2025-07-22  9:08             ` Gatien CHEVALLIER
2025-07-22 13:40               ` Andrew Lunn
2025-07-22 20:20                 ` Russell King (Oracle)
2025-07-22 20:30                   ` Florian Fainelli
2025-07-22 20:59                   ` Andrew Lunn
2025-07-22 21:39                     ` Russell King (Oracle)
2025-07-22 22:00                       ` Russell King (Oracle)
2025-07-22 22:57                         ` Russell King (Oracle)
2025-07-23 14:02                         ` Andrew Lunn
2025-07-23 14:23                       ` Andrew Lunn
2025-07-23 18:13                         ` Florian Fainelli
2025-07-23  8:50                   ` Gatien CHEVALLIER
2025-07-23  8:53                     ` Gatien CHEVALLIER
2025-07-23  9:25                       ` Russell King (Oracle)
2025-07-23  9:20                     ` Russell King (Oracle)
2025-07-23 14:35                       ` Gatien CHEVALLIER
2025-07-22  9:13             ` Russell King (Oracle) [this message]
2025-07-22  7:32           ` Russell King (Oracle)
2025-07-22  9:10             ` Gatien CHEVALLIER
2025-07-21 11:14 ` [PATCH net-next 2/4] net: stmmac: stm32: add WoL from PHY support Gatien Chevallier
2025-07-21 11:14 ` [PATCH net-next 3/4] net: phy: smsc: fix and improve WoL support Gatien Chevallier
2025-07-21 11:28   ` Russell King (Oracle)
2025-07-21 12:23     ` Gatien CHEVALLIER
2025-07-21 13:26   ` Andrew Lunn
2025-07-21 14:19     ` Gatien CHEVALLIER
2025-07-21 14:23       ` Andrew Lunn
2025-07-21 11:14 ` [PATCH net-next 4/4] arm: dts: st: activate ETH1 WoL from PHY on stm32mp135f-dk Gatien Chevallier

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=aH9WTYwty-tso66J@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Tristram.Ha@microchip.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=christophe.roullier@foss.st.com \
    --cc=conor+dt@kernel.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=florian.fainelli@broadcom.com \
    --cc=gatien.chevallier@foss.st.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=robh@kernel.org \
    /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).