On 02.06.2018 22:27, Heiner Kallweit wrote: > On 01.06.2018 02:10, Andrew Lunn wrote: >>> Configuring the different WoL options isn't handled by writing to >>> the PHY registers but by writing to chip / MAC registers. >>> Therefore phy_suspend() isn't able to figure out whether WoL is >>> enabled or not. Only the parent has the full picture. >> >> Hi Heiner >> >> I think you need to look at your different runtime PM domains. If i >> understand the code right, you runtime suspend if there is no >> link. But for this to work correctly, your PHY needs to keep working. >> You also cannot assume all accesses to the PHY go via the MAC. Some >> calls will go direct to the PHY, and they can trigger MDIO bus >> accesses. So i think you need two runtime PM domains. MAC and MDIO >> bus. Maybe just the pll? An MDIO bus is a device, so it can have its >> on PM callbacks. It is not clear what you need to resume in order to >> make MDIO work. >> > Thanks for your comments! > The actual problem is quite small: I get an error at MDIO suspend, > the PHY however is suspended later by the driver's suspend callback > anyway. Because the problem is small I'm somewhat reluctant to > consider bigger changes like introducing different PM domains. > > Primary reason for the error is that the network chip is in PCI D3hot > at that moment. In addition to that for some of the chips supported by > the driver also MDIO-relevant PLL's might be disabled. > > By the way: > When checking PM handling for PHY/MDIO I stumbled across something > that can be improved IMO, I'll send a patch for your review. > I experimented a little and with adding Runtime PM to MDIO bus and PHY device I can make it work: PHY runtime-resumes before entering suspend and resumes its parent (MDIO bus) which then resumes its parent (PCI device). However this needs quite some code and is hard to read / understand w/o reading through this mail thread.
And in general I still have doubts this is the right way. Let's consider the following scenario: A network driver configures WoL in its suspend callback (incl. setting the device to wakeup-enabled). The suspend callback of the PHY is called before this and therefore has no clue that WoL will be configured a little later, with the consequence that it will do an unsolicited power-down. The network driver then has to detect this and power-up the PHY again. This doesn't seem to make much sense and still my best idea is to establish a mechanism that a device can state: I orchestrate PM of my children. Heiner >> It might also help if you do the phy_connect in .ndo_open and >> disconnect in .ndo_stop. This is a common pattern in drivers. But some >> also do it is probe and remove. >> > Thanks for the hint. I will move phy_connect_direct accordingly. > >> Andrew >> > Heiner >
