Hi Andrew, On Fri, May 11, 2018 at 09:24:46PM +0200, Andrew Lunn wrote: > > I could reorder the probe function a little to initialize the PHY before > > performing the MAC reset, drop this patch and the AR803X hibernation > > stuff from patch 2 if you like. But again, I can't actually test the > > result on the affected hardware. > > Hi Paul > > I don't like a MAC driver poking around in PHY registers. > > So if you can rearrange the code, that would be great. > > Thanks > Andrew
Sure, I'll give it a shot. After digging into it I see 2 ways to go here: 1) We could just always reset the PHY before we reset the MAC. That would give us a window of however long the PHY takes to enter its low power state & stop providing the RX clock during which we'd need the MAC reset to complete. In the case of the AR8031 that's "about 10 seconds" according to its data sheet. In this particular case that feels like plenty, but it does also feel a bit icky to rely on the timing chosen by the PHY manufacturer to line up with that of the MAC reset. 2) We could introduce a couple of new phy_* functions to disable & enable low power states like the AR8031's hibernation feature, by calling new function pointers in struct phy_driver. Then pch_gbe & other MACs could call those to have the PHY driver disable hibernation at times where we know we'll need the RX clock and re-enable it afterwards. I'm currently leaning towards option 2. How does that sound to you? Or can you see another way to handle this? Thanks, Paul