linux-kernel - Re: [PATCH] net: phy: Only resume phy if it is suspended

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d217ce88-23c3-423e-b589-3a584dfbd4bd@broadcom.com>
Date:   Fri, 8 Dec 2023 09:36:53 -0800
From:   Florian Fainelli <florian.fainelli@...adcom.com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Justin Chen <justin.chen@...adcom.com>,
        Andrew Lunn <andrew@...n.ch>, Doug Berger <opendmb@...il.com>,
        netdev@...r.kernel.org, bcm-kernel-feedback-list@...adcom.com,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: phy: Only resume phy if it is suspended

On 12/7/23 10:50, Russell King (Oracle) wrote:
> On Thu, Dec 07, 2023 at 09:56:01AM -0800, Florian Fainelli wrote:
>> Hi Andrew,
>>
>> So we discussed with Justin and Doug about this yesterday and the main
>> reason for the phy_resume() ... MAC initialization ... phy_start() pattern
>> has to do with external RGMII PHYs and the clocking dependency between the
>> MAC and the PHY on the RX path. And also a tiny bit of cargo culting, but
>> shhh.
>>
>> When the external RGMII PHYs are suspended they will stop providing a RXC
>> back to the Ethernet MAC and our Ethernet MAC like a lot of designs out
>> there require the RXC in order to be functional and complete its reset
>> procedure correctly (you would think there would be a way to mux in a
>> different clock, but that does not appear to be the case). If we reset the
>> UniMAC block without a RXC we will typically see duplicate packets being
>> received, or absurdly long round trip times as soon as we try to use the RX
>> path.
> 
> This issue keeps appearing, and I think phylib ought to be doing more
> to support it, rather than ethernet drivers having to play fancy games.
> If one recalls stmmac, that has similar issues - it needs the RXC from
> the PHY to reset properly.
> 
> I did propose that we have:
> 
> +#define PHY_F_RXC_ALWAYS_ON    BIT(30)
> 
> that can be passed to phy_attach_direct()'s flags, which phylib drivers
> can then act upon to e.g. in the case of at803x, disable their
> hibernation mode which stops the RXC when the system isn't suspended.
> (AT803x Hibernation mode is enabled by default and the PHY automatically
> enters it when the link is down.)
> 
> Maybe this flag should be used to determine the resume behaviour,
> e.g. to ensure that the RXC is re-enabled early without the MAC driver
> needing to be involved?
> 
> WoL is a different problem - that depends whether the PHY is itself
> doing WoL independently from the MAC, or whether the MAC is involved.
> If the MAC is involved, then clearly the MII link between the PHY and
> MAC needs to be maintained while the system is in low power mode,
> which is an entirely different issue from the RXC being present
> while the MAC is being resumed.
> 
> Maybe PHY_F_RXC_ALWAYS_ON is a bad name, as I intended it to only refer
> to while the system is running, not while in low power mode.
> 

Sure, I would like to see something similar and be able to use it, 
especially during boot.

In our particular case however we have a "double" suspend and resume 
which is at best unnecessary and wasting time, and at worst causing some 
unidentified side effects.
-- 
Florian


Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)