Re: [PATCH net v2 2/2] net: fec: Suspend and resume the PHY

[John Ernberg]

Hi Wei,

On 3/1/24 02:49, Wei Fang wrote:
>> -----Original Message-----
>> From: John Ernberg <john.ernberg@xxxxxxxx>
>> Sent: 2024年2月29日 18:53
>> To: Wei Fang <wei.fang@xxxxxxx>
>> Cc: Shenwei Wang <shenwei.wang@xxxxxxx>; Clark Wang
>> <xiaoning.wang@xxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>; David S.
>> Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>;
>> Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>;
>> Heiner Kallweit <hkallweit1@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx;
>> linux-kernel@xxxxxxxxxxxxxxx; John Ernberg <john.ernberg@xxxxxxxx>
>> Subject: [PATCH net v2 2/2] net: fec: Suspend and resume the PHY
>>
>> PHYs that are always-on will not enter their low power modes otherwise as
>> they have no regulator to be powered off with.
>>
>> Since the PHY is picked up via {of_,}phy_connect() and dropped with
>> phy_disconnect() when the link is brought up and down respectively the only
>> cases were pm is needed is when the netif is running or or when the link
> nit: where
> 
>> has never been up.
>>
>> To deal with the latter case the PHY is suspended on discovery in probe,
>> since it won't be needed until link up.
>>
>> Fixes: 557d5dc83f68 ("net: fec: use mac-managed PHY PM")
> I'm not sure whether this commit should be blamed. After checking my local
> code (not the recent upstream code), fec_suspend() will make the PHY enter
> suspend mode when calling phy_stop(), the specific logic is fec_suspend() -->
> phy_stop() --> phy_state_machine() --> phy_suspend (). But the latest upstream
> code may have changed this logic. I'm sorry that I don't have time to sit down
> and look at the latest code.

I missed this flow, and also didn't see it take place when I look at the 
MDIO traffic. But the code has been there since 2013.

I will check why that isn't happening.

Thanks! // John Ernberg