On Wed, 5 Jun 2019 at 06:06, Florian Fainelli <[email protected]> wrote: > > > > On 6/4/2019 4:46 PM, Vladimir Oltean wrote: > > On Wed, 5 Jun 2019 at 02:24, Russell King - ARM Linux admin > > <[email protected]> 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 > >>> <[email protected]> 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!'. > > With the systems that I work with, we have enforced the following > behavior to happen: the boot loader and kernel only turn on what they > needs, at the time they need it, and nothing more, once done, they put > the blocks back into lowest power mode (clock and power gated if > available). So yes, there are multiple link re-negotiations throughput > the boot process, but when there is no device bound to a driver the > system conserves power by default which is deemed a higher goal than > speed. Your mileage may vary of course. > > There is not exactly a simple way of enforcing that kind (or another > kind for that matter) of policy kernel wide, so it's unfortunately up to > the driver writer to propose something that is deemed sensible. > > We could however, extend existing tools like iproute2 to offer the > ability to control whether the PHY should be completely powered off, in > a low power state allowing WoL, or remain UP when the network device is > brought down. That would not cover the case that Russell explained, but > it would be another monkey wrench you can throw at the system. > -- > Florian
Hi Florian, By going to HALTED on phy_stop, the system already achieves what I am looking after - although maybe it is an unintended consequence for you. I'm only trying to make an argument for removing the phy_resume from phy_attach_direct now. If there was a link, and it doesn't need re-negociation, fine, use it in phy_start, but at most leave U-boot to put that link down and don't force it up prior to the netdev's ndo_open. Regards, -Vladimir
