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: Thu, 31 May 2018 17:58:05 +0200 Message-ID: <1d79c2ae-6f63-694f-6c63-6369c854de69@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> 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]:36064 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755573AbeEaP6Q (ORCPT ); Thu, 31 May 2018 11:58:16 -0400 Received: by mail-wr0-f196.google.com with SMTP id f16-v6so18195757wrm.3 for ; Thu, 31 May 2018 08:58:15 -0700 (PDT) In-Reply-To: <20180530203512.GA16286@lunn.ch> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 30.05.2018 22:35, Andrew Lunn wrote: >> 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 .. > > Hi Heiner > > Please keep in mind that MDIO is a generic bus. Many Ethernet switches > are connected via MDIO. Some of those switches have MDIO busses of > their own. Also, some Broadcom devices have USB-PHYs controlled over > MDIO, etc. > > So you need a generic solution here. > > Andrew > The following proposed change (I combined three patches here) is quite small, generic, and solves my problem. Another advantage is that it doesn't impact existing code / drivers. We just would have to see whether Rafael likes the idea of adding this flag to the PM core. Other bus subsystems would be free to adopt the same mechanism with minimal effort. Alternatively we could just add a flag to struct mii_bus and not touch the PM core. But then the solution would be much less generic. By the way: The problem is related to an experimental patch series for splitting r8169/r8168 drivers and switching r8168 to phylib. Therefore the change to r8168.c won't apply to existing kernel code. Heiner --- drivers/net/ethernet/realtek/r8168.c | 1 + drivers/net/phy/phy_device.c | 9 ++++++++- include/linux/pm.h | 6 ++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/realtek/r8168.c b/drivers/net/ethernet/realtek/r8168.c index 473a147e..2e1af844 100644 --- a/drivers/net/ethernet/realtek/r8168.c +++ b/drivers/net/ethernet/realtek/r8168.c @@ -5063,6 +5063,7 @@ static int r8168_mdio_register(struct rtl8169_private *tp) new_bus->irq[0] = PHY_IGNORE_INTERRUPT; snprintf(new_bus->id, MII_BUS_ID_SIZE, "r8168-%x", PCI_DEVID(pdev->bus->number, pdev->devfn)); + dev_pm_set_driver_flags(&new_bus->dev, DPM_FLAG_IGNORE_PM); new_bus->read = r8168_mdio_read_reg; new_bus->write = r8168_mdio_write_reg; diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 9e4ba8e8..459fd677 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -76,13 +76,20 @@ static LIST_HEAD(phy_fixup_list); static DEFINE_MUTEX(phy_fixup_lock); #ifdef CONFIG_PM +static bool mdio_bus_ignore_pm(struct phy_device *phydev) +{ + struct mii_bus *bus = phydev->mdio.bus; + + return dev_pm_test_driver_flags(&bus->dev, DPM_FLAG_IGNORE_PM); +} + static bool mdio_bus_phy_may_suspend(struct phy_device *phydev) { struct device_driver *drv = phydev->mdio.dev.driver; struct phy_driver *phydrv = to_phy_driver(drv); struct net_device *netdev = phydev->attached_dev; - if (!drv || !phydrv->suspend) + if (!drv || !phydrv->suspend || mdio_bus_ignore_pm(phydev)) return false; /* PHY not attached? May suspend if the PHY has not already been diff --git a/include/linux/pm.h b/include/linux/pm.h index e723b78d..922d2ded 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -560,6 +560,7 @@ struct pm_subsys_data { * SMART_PREPARE: Check the return value of the driver's ->prepare callback. * SMART_SUSPEND: No need to resume the device from runtime suspend. * LEAVE_SUSPENDED: Avoid resuming the device during system resume if possible. + * IGNORE_PM: Skip suspend/resume because the parent takes care. * * Setting SMART_PREPARE instructs bus types and PM domains which may want * system suspend/resume callbacks to be skipped for the device to return 0 from @@ -576,11 +577,16 @@ struct pm_subsys_data { * * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that the * driver prefers the device to be left in suspend after system resume. + * + * Setting DPM_FLAG_IGNORE_PM instructs middle-layer code to skip suspending / + * resuming devices. This is meant for cases where the parent of a bus handles + * PM of the devices attached to the bus. */ #define DPM_FLAG_NEVER_SKIP BIT(0) #define DPM_FLAG_SMART_PREPARE BIT(1) #define DPM_FLAG_SMART_SUSPEND BIT(2) #define DPM_FLAG_LEAVE_SUSPENDED BIT(3) +#define DPM_FLAG_IGNORE_PM BIT(4) struct dev_pm_info { pm_message_t power_state;