Re: [PATCH net-next v2 3/3] net: phy: marvell10g: set the PHY in low power by default

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

On 3/1/2019 7:07 AM, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Fri, Mar 01, 2019 at 03:19:53PM +0100, Andrew Lunn wrote:
>> On Fri, Mar 01, 2019 at 12:00:47PM +0100, Antoine Tenart wrote:
>>> When the Marvell 10G PHYs are set out of reset, the LPOWER bit is set
>>> depending on an hardware configuration choice. We also do not know what
>>> is the PHY state at boot time. Hence, set the PHY in low power by
>>> default when this driver probes.
>>
>> Florian did some work for c22 PHYs so that the existing link state
>> could be used at boot. So for example, the bootloader configured the
>> PHY up and it got link, there is no need to down/up the PHY when linux
>> takes control. The networking comes up faster that way.
>>
>> Can this work for this PHY?
> 
> This use case (the bootloader configures the PHY, Linux boots and sets
> an interface using this PHY up) would work, and is what's happening in
> some situations right now (the 3310 reset is never asserted prior to
> this series).
> 
> But consider this case (let's say we use a 10G link):
> 
>   ----------------               ----------------
>   |    Board 1   |               |    Board 2   |
>   | MAC — 3310 — | — SFP cable — | — 3310 — MAC |
>   ----------------               ----------------
> 
> Board 1: The userspace do not set the interface up. The MAC is in reset
>          (default state during the MAC driver probe), the PHY was
> 	 configured by the bootloader.
> Board 2: The userspace set the interface up. The MAC is configured, the
>          PHY is configured as well.
> 
> The two PHY's PCS will establish a link and report it as being up. In
> this case, phylink's AN mode is MLO_AN_PHY and thus will report the
> overall link as being the PHY's link status: up.
> 
> My understanding is that the issue arises because the PHYs were never
> set in reset, or low power, and thus act as if the user wanted the port
> to be up. As the default behaviour for networking ports is to be down at
> boot, I thought to set the PHY as well in a default low power state.

The policy you are creating here for the marvell10g driver is entirely
applicable to any PHY <=> PHY configuration where either of the two
software agents on Board 1 or Board 2 has not had a chance to bring-up
its bootloader/OS/applications to control the PHY.

A number of PHYs come up fully on (or in isolate or super isolate mode)
and will AN with their link partner if connected. For some people it's a
feature, for some it is a waste of power. I don't necessarily have an
issue with your patch per-se, but it does create an one off behavior
that other PHY drivers may not follow.
-- 
Florian