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.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature