On 04.06.2019 22:42, Vladimir Oltean wrote: > On Tue, 4 Jun 2019 at 23:07, Andrew Lunn <and...@lunn.ch> wrote: >> >> On Tue, Jun 04, 2019 at 10:58:41PM +0300, Vladimir Oltean wrote: >>> Hi, >>> >>> I've been wondering what is the correct approach to cut the Ethernet link >>> when the user requests it to be administratively down (aka ip link set dev >>> eth0 down). >>> Most of the Ethernet drivers simply call phy_stop or the phylink equivalent. >>> This leaves an Ethernet link between the PHY and its link partner. >>> The Freescale gianfar driver (authored by Andy Fleming who also authored the >>> phylib) does a phy_disconnect here. It may seem a bit overkill, but of the >>> extra things it does, it calls phy_suspend where most PHY drivers set the >>> BMCR_PDOWN bit. Only this achieves the intended purpose of also cutting the >>> link partner's link on 'ip link set dev eth0 down'. >> >> Hi Vladimir >> >> Heiner knows the state machine better than i. But when we transition >> to PHY_HALTED, as part of phy_stop(), it should do a phy_suspend(). >> >> Andrew > > Hi Andrew, Florian, > > Thanks for giving me the PHY_HALTED hint! > Indeed it looks like I conflated two things - the Ehernet port that > uses phy_disconnect also happens to be connected to a PHY that has > phy_suspend implemented. Whereas the one that only does phy_stop is > connected to a PHY that doesn't have that... I thought that in absence > of .suspend, the PHY library automatically calls genphy_suspend. Oh > well, looks like it doesn't. So of course, phy_stop calls phy_suspend > too. > But now the second question: between a phy_connect and a phy_start, > shouldn't the PHY be suspended too? Experimentally it looks like it > still isn't. > By the way, Florian, yes, PHY drivers that use WOL still set > BMCR_ISOLATE, which cuts the MII-side, so that's ok. However that's > not the case here - no WOL. > Right, some PHY driver callbacks fall back to the generic functionality, for the suspend/resume callbacks that's not the case. phy_connect() eventually calls phy_attach_direct() that has a call to phy_resume(). So your observation is correct, phy_connect() wakes the PHY. I'm not 100% sure whether this is needed because also phy_start() resumes the PHY.
BMCR_ISOLATE isn't set by any phylib function. We just have few calls where BMCR_ISOLATE is cleared as part of the functionality. > Regards, > -Vladimir > Heiner
