| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <34478e1c-b3ba-4da3-839a-4cec9ac5f51e@broadcom.com>
Date: Sat, 27 Sep 2025 14:04:15 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>,
Gatien Chevallier <gatien.chevallier@...s.st.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Christophe Roullier <christophe.roullier@...s.st.com>,
Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
Simon Horman <horms@...nel.org>, Tristram Ha <Tristram.Ha@...rochip.com>,
netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 2/4] net: stmmac: stm32: add WoL from PHY
support
On 9/26/2025 12:05 PM, Florian Fainelli wrote:
>
>
> On 9/26/2025 10:59 AM, Russell King (Oracle) wrote:
>> On Wed, Sep 17, 2025 at 05:31:16PM +0100, Russell King (Oracle) wrote:
>>> On Wed, Sep 17, 2025 at 05:36:37PM +0200, Gatien Chevallier wrote:
>>>> If the "st,phy-wol" property is present in the device tree node,
>>>> set the STMMAC_FLAG_USE_PHY_WOL flag to use the WoL capability of
>>>> the PHY.
>>>>
>>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@...s.st.com>
>>>> ---
>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/
>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>>> index
>>>> 77a04c4579c9dbae886a0b387f69610a932b7b9e..6f197789cc2e8018d6959158b795e4bca46869c5 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>>> @@ -106,6 +106,7 @@ struct stm32_dwmac {
>>>> u32 speed;
>>>> const struct stm32_ops *ops;
>>>> struct device *dev;
>>>> + bool phy_wol;
>>>> };
>>>> struct stm32_ops {
>>>> @@ -433,6 +434,8 @@ static int stm32_dwmac_parse_data(struct
>>>> stm32_dwmac *dwmac,
>>>> }
>>>> }
>>>> + dwmac->phy_wol = of_property_read_bool(np, "st,phy-wol");
>>>> +
>>>> return err;
>>>> }
>>>> @@ -557,6 +560,8 @@ static int stm32_dwmac_probe(struct
>>>> platform_device *pdev)
>>>> plat_dat->bsp_priv = dwmac;
>>>> plat_dat->suspend = stm32_dwmac_suspend;
>>>> plat_dat->resume = stm32_dwmac_resume;
>>>> + if (dwmac->phy_wol)
>>>> + plat_dat->flags |= STMMAC_FLAG_USE_PHY_WOL;
>>>
>>> I would much rather we found a different approach, rather than adding
>>> custom per-driver DT properties to figure this out.
>>>
>>> Andrew has previously suggested that MAC drivers should ask the PHY
>>> whether WoL is supported, but this pre-supposes that PHY drivers are
>>> coded correctly to only report WoL capabilities if they are really
>>> capable of waking the system. As shown in your smsc PHY driver patch,
>>> this may not be the case.
>>>
>>> Given that we have historically had PHY drivers reporting WoL
>>> capabilities without being able to wake the system, we can't
>>> implement Andrew's suggestion easily.
>>>
>>> The only approach I can think that would allow us to transition is
>>> to add:
>>>
>>> static inline bool phy_can_wakeup(struct phy_device *phy_dev)
>>> {
>>> return device_can_wakeup(&phy_dev->mdio.dev);
>>> }
>>>
>>> to include/linux/phy.h, and a corresponding wrapper for phylink.
>>> This can then be used to determine whether to attempt to use PHY-based
>>> Wol in stmmac_get_wol() and rtl8211f_set_wol(), falling back to
>>> PMT-based WoL if supported at the MAC.
>>>
>>> So, maybe something like:
>>>
>>> static u32 stmmac_wol_support(struct stmmac_priv *priv)
>>> {
>>> u32 support = 0;
>>>
>>> if (priv->plat->pmt && device_can_wakeup(priv->device)) {
>>> support = WAKE_UCAST;
>>> if (priv->hw_cap_support && priv->dma_cap.pmt_magic_frame)
>>> support |= WAKE_MAGIC;
>>> }
>>>
>>> return support;
>>> }
>>>
>>> static void stmmac_get_wol(struct net_device *dev, struct
>>> ethtool_wolinfo *wol)
>>> {
>>> struct stmmac_priv *priv = netdev_priv(dev);
>>> int err;
>>>
>>> /* Check STMMAC_FLAG_USE_PHY_WOL for legacy */
>>> if (phylink_can_wakeup(priv->phylink) ||
>>> priv->plat->flags & STMMAC_FLAG_USE_PHY_WOL) {
>>> err = phylink_ethtool_get_wol(priv->phylink, wol);
>>> if (err != 0 && err != -EOPNOTSUPP)
>>> return;
>>> }
>>>
>>> wol->supported |= stmmac_wol_support(priv);
>>>
>>> /* A read of priv->wolopts is single-copy atomic. Locking
>>> * doesn't add any benefit.
>>> */
>>> wol->wolopts |= priv->wolopts;
>>> }
>>>
>>> static int stmmac_set_wol(struct net_device *dev, struct
>>> ethtool_wolinfo *wol)
>>> {
>>> struct stmmac_priv *priv = netdev_priv(dev);
>>> u32 support, wolopts;
>>> int err;
>>>
>>> wolopts = wol->wolopts;
>>>
>>> /* Check STMMAC_FLAG_USE_PHY_WOL for legacy */
>>> if (phylink_can_wakeup(priv->phylink) ||
>>> priv->plat->flags & STMMAC_FLAG_USE_PHY_WOL) {
>>> struct ethtool_wolinfo w;
>>>
>>> err = phylink_ethtool_set_wol(priv->phylink, wol);
>>> if (err != -EOPNOTSUPP)
>>> return err;
>>>
>>> /* Remove the WoL modes that the PHY is handling */
>>> if (!phylink_ethtool_get_wol(priv->phylink, &w))
>>> wolopts &= ~w.wolopts;
>>> }
>>>
>>> support = stmmac_wol_support(priv);
>>>
>>> mutex_lock(&priv->lock);
>>> priv->wolopts = wolopts & support;
>>> device_set_wakeup_enable(priv->device, !!priv->wolopts);
>>> mutex_unlock(&priv->lock);
>>>
>>> return 0;
>>> }
>>>
>>> ... and now I'm wondering whether this complexity is something that
>>> phylink should handle internally, presenting a mac_set_wol() method
>>> to configure the MAC-side WoL settings. What makes it difficult to
>>> just move into phylink is the STMMAC_FLAG_USE_PHY_WOL flag, but
>>> that could be a "force_phy_wol" flag in struct phylink_config as
>>> a transitionary measure... so long as PHY drivers get fixed.
>>
>> I came up with this as an experiment - I haven't tested it beyond
>> running it through the compiler (didn't let it get to the link stage
>> yet.) Haven't even done anything with it for stmmac yet.
>>
>
> I like the direction this is going, we could probably take one step
> further and extract the logic present in bcmgenet_wol.c and make those
> helper functions for other drivers to get the overlay of PHY+MAC WoL
> options/password consistent across all drivers. What do you think?
+ if (wolopts & WAKE_MAGIC)
+ changed |= !!memcmp(wol->sopass, pl->wol_sopass,
+ sizeof(wol->sopass));
Should not the hunk above be wolopts & WAKE_MAGICSECURE?
--
Florian
Powered by blists - more mailing lists