lkml.org 
[lkml]   [2023]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] net: phy: Only resume phy if it is suspended
From
Hi Andrew,

On 12/5/23 16:12, Florian Fainelli wrote:
> On 12/5/23 16:10, Justin Chen wrote:
>>
>>
>> On 12/5/23 4:03 PM, Andrew Lunn wrote:
>>> On Tue, Dec 05, 2023 at 03:42:29PM -0800, Justin Chen wrote:
>>>> Resuming the phy can take quite a bit of time. Lets only resume the
>>>> phy if it is suspended.
>>>
>>> Humm...
>>>
>>> https://lore.kernel.org/netdev/6d45f4da-c45e-4d35-869f-85dd4ec37b31@lunn.ch/T/
>>>
>>> If Broadcom PHYs are slow to resume, maybe you should solve this in
>>> the broadcom resume handler, read the status from the hardware and
>>> only do the resume if the hardware is suspended.
>>>
>>>       Andrew
>>
>> Right... Guess this won't work. It is odd that during resume we call
>> __phy_resume twice. Once from phy_resume() and another at phy_start().
>> Let me rethink this. Thanks for the feedback.
>
> This might be something for us to figure out on the driver side, I think
> historically I have always followed the pattern of doing:
>
> phy_suspend()
> phy_stop()
>
> and
>
> phy_resume()
> phy_start()
>
> because it used to be necessary to do that way back when...

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.

Doug lifted that requirement in GENET with:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=88f6c8bf1aaed5039923fb4c701cab4d42176275

by delaying the MAC reset until we have a link UP confirmation. Since
this is the same UniMAC design in the bcmasp driver we should be able to
apply the same strategy and remove the initial phy_resume().

There are also other opportunities for avoiding link disruption upon
suspend/resume when Wake-on-LAN is enabled and avoid a re-negotiation of
the link, though that's for another set of changes.
--
Florian

[unhandled content-type:application/pkcs7-signature]
\
 
 \ /
  Last update: 2023-12-07 18:56    [W:0.930 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site