From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO Date: Tue, 5 Jun 2018 08:08:24 +0200 Message-ID: References: <20180604214829.GA14873@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-f194.google.com ([209.85.128.194]:33205 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751582AbeFEGId (ORCPT ); Tue, 5 Jun 2018 02:08:33 -0400 Received: by mail-wr0-f194.google.com with SMTP id k16-v6so982435wro.0 for ; Mon, 04 Jun 2018 23:08:32 -0700 (PDT) In-Reply-To: <20180604214829.GA14873@lunn.ch> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 04.06.2018 23:48, Andrew Lunn wrote: > On Sat, Jun 02, 2018 at 10:33:36PM +0200, Heiner Kallweit wrote: >> Current implementation of MDIO bus PM ops doesn't actually implement >> bus-specific PM ops but just calls PM ops defined on a device level >> what doesn't seem to be fully in line with the core PM model. >> >> When looking e.g. at __device_suspend() the PM core looks for PM ops >> of a device in a specific order: >> 1. device PM domain >> 2. device type >> 3. device class >> 4. device bus >> >> I think it has good reason that there's no PM ops on device level. >> The situation can be improved by modeling PHY's as device type of >> a MDIO device. If for some other type of MDIO device PM ops are >> needed, it could be modeled as struct device_type as well. > > Hi Heiner > > I tested that the files in /sys/class/bus/mdio/devices/* are still > there. And also not there for MDIO devices which are not PHYs, > e.g. Ethernet switches. > > I don't have any boards which do PM. So i cannot test suspend/resume. > Thanks for reviewing! I tested suspend / resume by manually suspending via "systemctl suspend" and resuming via power button or WoL. The only behavior change I expect is an additional parameter in the PHY udev calls, see following comment in device.h: If "name" is specified, the uevent will contain it in the DEVTYPE variable. This however shouldn't have any impact. Heiner > I also took a look at drivers/net/dsa/qca8k.c. This is an MDIO switch > which has PM operations. I don't think this change will break it. > > I would prefer a bit more testing, but i guess that is what -rc > kernels are for. > > Tested-by: Andrew Lunn > > Andrew > . >