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: Wed, 30 May 2018 22:22:51 +0200 Message-ID: <5b9eceab-35b4-922c-7410-d8968d8364c7@gmail.com> References: <8fe93623-9d05-6182-fe5f-da0bd32bae0b@gmail.com> <20180523220418.GB5128@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-f196.google.com ([209.85.128.196]:38791 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932202AbeE3UXA (ORCPT ); Wed, 30 May 2018 16:23:00 -0400 Received: by mail-wr0-f196.google.com with SMTP id 94-v6so30760492wrf.5 for ; Wed, 30 May 2018 13:22:59 -0700 (PDT) In-Reply-To: <20180523220418.GB5128@lunn.ch> Sender: netdev-owner@vger.kernel.org List-ID: Am 24.05.2018 um 00:04 schrieb Andrew Lunn: > On Wed, May 23, 2018 at 10:15:29PM +0200, Heiner Kallweit wrote: >> I have the issue that suspending the MAC-integrated PHY gives an >> error during system suspend. The sequence is: >> >> 1. unconnected PHY/MAC are runtime-suspended already >> 2. system suspend commences >> 3. mdio_bus_phy_suspend is called >> 4. suspend callback of the network driver is called (implicitly >> MAC/PHY are runtime-resumed before) >> 5. suspend callback suspends MAC/PHY >> >> The problem occurs in step 3. phy_suspend() fails because the MDIO >> bus isn't accessible due to the chip being runtime-suspended. > > I think you are fixing the wrong problem. I've had the same with the > FEC driver. I fixed it by making the MDIO operations runtime-suspend > aware: > I checked the fec driver and there it's reletively easy because runtime suspend/resume is just about disabling/enabling one clock. In case of r8169 runtime suspend/resume do much more and there would be quite some potential deadlock issues to take care of. To provide one example: pm_runtime_get_sync() would be called with the mdio bus lock being held (from mdio write op), runtime-resuming however includes PHY writes what would result in a deadlock. I think we need a better solution than spending the effort needed to make the MDIO ops runtime-pm-aware. In general there seems to be just one network driver using both phylib and runtime pm, so most drivers aren't affected (yet). I will spend few more thoughts on a solution .. Regards, Heiner > commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3 > Author: Andrew Lunn > Date: Sat Jul 25 22:38:02 2015 +0200 > > net: fec: Ensure clocks are enabled while using mdio bus > > When a switch is attached to the mdio bus, the mdio bus can be used > while the interface is not open. If the IPG clock is not enabled, MDIO > reads/writes will simply time out. > > Add support for runtime PM to control this clock. Enable/disable this > clock using runtime PM, with open()/close() and mdio read()/write() > function triggering runtime PM operations. Since PM is optional, the > IPG clock is enabled at probe and is no longer modified by > fec_enet_clk_enable(), thus if PM is not enabled in the kernel, it is > guaranteed the clock is running when MDIO operations are performed. > > Don't copy this patch 1:1. I introduced a few bugs which took a while > to be shaken out :-( > > Andrew >