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

[Florian Fainelli <florian.fainelli@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]

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


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]