Message-ID: <CA+h21hpDyDLChzrqX19bU81+ss3psVesNftCqN7+7+GFkMe_-A@mail.gmail.com>
Date:   Wed, 5 Jun 2019 02:46:13 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ioana Ciornei <ioana.ciornei@....com>
Subject: Re: Cutting the link on ndo_stop - phy_stop or phy_disconnect?

On Wed, 5 Jun 2019 at 02:24, Russell King - ARM Linux admin
<linux@...linux.org.uk> wrote:
>
> On Wed, Jun 05, 2019 at 02:03:19AM +0300, Vladimir Oltean wrote:
> > On Wed, 5 Jun 2019 at 01:59, Russell King - ARM Linux admin
> > <linux@...linux.org.uk> wrote:
> > >
> > > On Wed, Jun 05, 2019 at 01:44:08AM +0300, Vladimir Oltean wrote:
> > > > You caught me.
> > > >
> > > > But even ignoring the NIC case, isn't the PHY state machine
> > > > inconsistent with itself? It is ok with callink phy_suspend upon
> > > > ndo_stop, but it won't call phy_suspend after phy_connect, when the
> > > > netdev is implicitly stopped?
> > >
> > > The PHY state machine isn't inconsistent with itself, but it does
> > > have strange behaviour.
> > >
> > > When the PHY is attached, the PHY is resumed and the state machine
> > > is in PHY_READY state.  If it goes through a start/stop cycle, the
> > > state machine transitions to PHY_HALTED and attempts to place the
> > > PHY into a low power state.  So the PHY state is consistent with
> > > the state machine state (we don't end up in the same state but with
> > > the PHY in a different state.)
> > >
> > > What we do have is a difference between the PHY state (and state
> > > machine state) between the boot scenario, and the interface up/down
> > > scenario, the latter behaviour having been introduced by a commit
> > > back in 2013:
> > >
> > >     net: phy: suspend phydev when going to HALTED
> > >
> > >     When phydev is going to HALTED state, we can try to suspend it to
> > >     safe more power. phy_suspend helper will check if PHY can be suspended,
> > >     so just call it when entering HALTED state.
> > >
> > > --
> > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > > According to speedtest.net: 11.9Mbps down 500kbps up
> >
> > I am really not into the PHYLIB internals, but basically what you're
> > telling me is that running "ip link set dev eth0 down" is a
> > stronger/more imperative condition than not running "ip link set dev
> > eth0 up"... Does it also suspend the PHY if I put the interface down
> > while it was already down?
>
> No - but that has nothing to do with phylib internals, more to do with
> the higher levels of networking.  ndo_stop() will not be called unless
> ndo_open() has already been called.  In other words, setting an already
> down device down via "ip link set dev eth0 down" is a no-op.
>
> So, let's a common scenario.  You power up a board.  The PHY comes up
> and establishes a link.  The boot loader runs, loads the kernel, which

This may or may not be the case. As you pointed out a few emails back,
this is a system-level issue that requires a system-level solution -
so cutting the link in U-boot is not out of the question.

> then boots.  Your network driver is a module, and hasn't been loaded
> yet.  The link is still up.
>
> The modular network driver gets loaded, and initialises.  Userspace
> does not bring the network device up, and the network driver does not
> attach or connect to the PHY (which is actually quite common).  So,
> the link is still up.
>
> The modular PHY driver gets loaded, and binds to the PHY.  The link
> is still up.

I would rather say, 'even if the link is not up, Linux brings it up
(possibly prematurely) via phy_resume'.
But let's consider the case where the link *was* up. The general idea
is 'implement your workarounds in whatever other way, that link is
welcome!'.

>
> Userspace configures the network interface, which causes the PHY
> device to be attached to the network device, and phy_start() to be
> called on it - the negotiation advertisement is configured, and
> negotiation restarted if necessary.

In general (the typical driver that isn't bothered by the presence of
an unrequested Ethernet link during initialization), U-boot may
negociate a mode without flow control, and Linux one with. So in
practice, AN is going to restart anyway, which makes the argument for
preserving that link established by the predecessors less strong.

>
> When userspace deconfigures the network interface, phy_stop() will
> be called, and, as the network driver attached the PHY in its
> ndo_open() function, the network driver will detach the PHY from
> the network interface to reverse those effects.  The PHY will be
> suspended, but more so than that, if there is a reset line, the
> reset line will be activated to the PHY.
>
> The above is an illustration of one sequence that /can/ happen.
> Other sequences are also possible.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

Regards,
-Vladimir

Powered by blists - more mailing lists