Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Thu, 7 Dec 2023 18:50:40 +0000
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Florian Fainelli <florian.fainelli@broadcom.com>
Cc:     Justin Chen <justin.chen@broadcom.com>,
        Andrew Lunn <andrew@lunn.ch>, Doug Berger <opendmb@gmail.com>,
        netdev@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com,
        Heiner Kallweit <hkallweit1@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: phy: Only resume phy if it is suspended
Message-ID: <ZXIUAPx1aClSsKED@shell.armlinux.org.uk>
References: <20231205234229.274601-1-justin.chen@broadcom.com>
 <7e3208aa-3adf-47ec-9e95-3c88a121e8a3@lunn.ch>
 <55a08719-cd18-4a01-9a2a-0115065c06a6@broadcom.com>
 <c2ce6d12-fb5e-4067-aa7c-4f57f4eb4613@broadcom.com>
 <f7eb20eb-3f0d-42c5-93fe-a622639c55a6@broadcom.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f7eb20eb-3f0d-42c5-93fe-a622639c55a6@broadcom.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

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.

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