From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net-next v2 0/2] net: phy: improve PHY suspend/resume Date: Sat, 2 Jun 2018 22:27:50 +0200 Message-ID: <28a8846a-f7a7-3266-3aec-7f58fe8dac72@gmail.com> References: <8fe93623-9d05-6182-fe5f-da0bd32bae0b@gmail.com> <20180523220418.GB5128@lunn.ch> <5b9eceab-35b4-922c-7410-d8968d8364c7@gmail.com> <20180530203512.GA16286@lunn.ch> <1d79c2ae-6f63-694f-6c63-6369c854de69@gmail.com> <20180531183040.GA7378@lunn.ch> <20180601001043.GA2303@lunn.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Florian Fainelli , David Miller , "netdev@vger.kernel.org" To: Andrew Lunn Return-path: Received: from mail-wr0-f193.google.com ([209.85.128.193]:35987 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750964AbeFBU16 (ORCPT ); Sat, 2 Jun 2018 16:27:58 -0400 Received: by mail-wr0-f193.google.com with SMTP id f16-v6so24018236wrm.3 for ; Sat, 02 Jun 2018 13:27:58 -0700 (PDT) In-Reply-To: <20180601001043.GA2303@lunn.ch> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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. > 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