Re: [PATCH net-next 2/2] net: phy: add phy_speed_down and phy_speed_up

[Florian Fainelli <f.fainelli@gmail.com>]

On 07/12/2018 12:25 PM, Florian Fainelli wrote:
> 
> 
> On 07/12/2018 12:10 PM, Heiner Kallweit wrote:
>> On 12.07.2018 21:09, Andrew Lunn wrote:
>>>> Like r8169 also tg3 driver doesn't wait for the speed-down-renegotiation
>>>> to finish. Therefore, even though I share Andrew's concerns, there seem
>>>> to be chips where it's safe to not wait for the renegotiation to finish
>>>> (e.g. because device is in PCI D3 already and can't generate an interrupt).
>>>> Having said that I'd keep the sync parameter for phy_speed_down so that
>>>> the driver can decide.
>>>
>>> Hi Heiner
>>>
>>> Please put a big fat comment about the dangers of sync=false in the
>>> function header. We want people to known it is dangerous by default,
>>> and should only be used in special conditions, when it is known to be
>>> safe.
>>> 	Andrew
>>>
>> OK ..
> 
> What part do you find dangerous? Magic Packets are UDP packets and they
> are not routed (unless specifically taken care of) so there is already
> some "lossy" behavior involved with waking-up an Ethernet MAC, I don't
> think that is too bad to retry several times until the link comes up.

I see the concern with the comment from v2, and indeed you could get an
interrupt signaling the PHY auto-negotiated the link before or at the
time we are suspending causing potentially an early wake-up. Not that
this should be a problem though since there is usually a point of not
return past which you can't do early wake-up anyway.
-- 
Florian