* [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO @ 2018-06-02 20:33 Heiner Kallweit 2018-06-02 20:36 ` [PATCH net-next 1/2] net: phy: add struct device_type representation of a PHY Heiner Kallweit ` (4 more replies) 0 siblings, 5 replies; 8+ messages in thread From: Heiner Kallweit @ 2018-06-02 20:33 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev 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. Heiner Kallweit (2): net: phy: add struct device_type representation of a PHY net: phy: remove PM ops from MDIO bus drivers/net/phy/mdio_bus.c | 48 ------------------ drivers/net/phy/phy_device.c | 96 +++++++++++++++++++----------------- include/linux/mdio.h | 1 - 3 files changed, 50 insertions(+), 95 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 1/2] net: phy: add struct device_type representation of a PHY 2018-06-02 20:33 [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO Heiner Kallweit @ 2018-06-02 20:36 ` Heiner Kallweit 2018-06-02 20:37 ` [PATCH net-next 2/2] net: phy: remove PM ops from MDIO bus Heiner Kallweit ` (3 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Heiner Kallweit @ 2018-06-02 20:36 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev A PHY is a type of MDIO device, so let's model it as struct device_type and place PM ops, attribute groups and release callback on device type level. For this the attribute definitions have to be moved. This change allows us to get rid of the PM ops on a bus level in a second step. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/phy_device.c | 96 +++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 46 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 9e4ba8e8..bd0f339f 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -346,6 +346,55 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv) } } +static ssize_t +phy_id_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct phy_device *phydev = to_phy_device(dev); + + return sprintf(buf, "0x%.8lx\n", (unsigned long)phydev->phy_id); +} +static DEVICE_ATTR_RO(phy_id); + +static ssize_t +phy_interface_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct phy_device *phydev = to_phy_device(dev); + const char *mode = NULL; + + if (phy_is_internal(phydev)) + mode = "internal"; + else + mode = phy_modes(phydev->interface); + + return sprintf(buf, "%s\n", mode); +} +static DEVICE_ATTR_RO(phy_interface); + +static ssize_t +phy_has_fixups_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct phy_device *phydev = to_phy_device(dev); + + return sprintf(buf, "%d\n", phydev->has_fixups); +} +static DEVICE_ATTR_RO(phy_has_fixups); + +static struct attribute *phy_dev_attrs[] = { + &dev_attr_phy_id.attr, + &dev_attr_phy_interface.attr, + &dev_attr_phy_has_fixups.attr, + NULL, +}; +ATTRIBUTE_GROUPS(phy_dev); + +static const struct device_type mdio_bus_phy_type = { + .name = "PHY", + .groups = phy_dev_groups, + .release = phy_device_release, + .pm = MDIO_BUS_PHY_PM_OPS, +}; + struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id, bool is_c45, struct phy_c45_device_ids *c45_ids) @@ -359,11 +408,10 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id, return ERR_PTR(-ENOMEM); mdiodev = &dev->mdio; - mdiodev->dev.release = phy_device_release; mdiodev->dev.parent = &bus->dev; mdiodev->dev.bus = &mdio_bus_type; + mdiodev->dev.type = &mdio_bus_phy_type; mdiodev->bus = bus; - mdiodev->pm_ops = MDIO_BUS_PHY_PM_OPS; mdiodev->bus_match = phy_bus_match; mdiodev->addr = addr; mdiodev->flags = MDIO_DEVICE_FLAG_PHY; @@ -587,48 +635,6 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45) } EXPORT_SYMBOL(get_phy_device); -static ssize_t -phy_id_show(struct device *dev, struct device_attribute *attr, char *buf) -{ - struct phy_device *phydev = to_phy_device(dev); - - return sprintf(buf, "0x%.8lx\n", (unsigned long)phydev->phy_id); -} -static DEVICE_ATTR_RO(phy_id); - -static ssize_t -phy_interface_show(struct device *dev, struct device_attribute *attr, char *buf) -{ - struct phy_device *phydev = to_phy_device(dev); - const char *mode = NULL; - - if (phy_is_internal(phydev)) - mode = "internal"; - else - mode = phy_modes(phydev->interface); - - return sprintf(buf, "%s\n", mode); -} -static DEVICE_ATTR_RO(phy_interface); - -static ssize_t -phy_has_fixups_show(struct device *dev, struct device_attribute *attr, - char *buf) -{ - struct phy_device *phydev = to_phy_device(dev); - - return sprintf(buf, "%d\n", phydev->has_fixups); -} -static DEVICE_ATTR_RO(phy_has_fixups); - -static struct attribute *phy_dev_attrs[] = { - &dev_attr_phy_id.attr, - &dev_attr_phy_interface.attr, - &dev_attr_phy_has_fixups.attr, - NULL, -}; -ATTRIBUTE_GROUPS(phy_dev); - /** * phy_device_register - Register the phy device on the MDIO bus * @phydev: phy_device structure to be added to the MDIO bus @@ -651,8 +657,6 @@ int phy_device_register(struct phy_device *phydev) goto out; } - phydev->mdio.dev.groups = phy_dev_groups; - err = device_add(&phydev->mdio.dev); if (err) { pr_err("PHY %d failed to add\n", phydev->mdio.addr); -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] net: phy: remove PM ops from MDIO bus 2018-06-02 20:33 [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO Heiner Kallweit 2018-06-02 20:36 ` [PATCH net-next 1/2] net: phy: add struct device_type representation of a PHY Heiner Kallweit @ 2018-06-02 20:37 ` Heiner Kallweit 2018-06-04 19:41 ` [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO David Miller ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Heiner Kallweit @ 2018-06-02 20:37 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev 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. Now that a device type representation of PHY's as special type of MDIO devices was added (only user of MDIO bus PM ops), the MDIO bus PM ops can be removed including member pm of struct mdio_device. If for some other type of MDIO device PM ops are needed, it should be modeled as struct device_type as well. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/mdio_bus.c | 48 -------------------------------------- include/linux/mdio.h | 1 - 2 files changed, 49 deletions(-) diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 24b55112..98f4b1f7 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -717,58 +717,10 @@ static int mdio_uevent(struct device *dev, struct kobj_uevent_env *env) return 0; } -#ifdef CONFIG_PM -static int mdio_bus_suspend(struct device *dev) -{ - struct mdio_device *mdio = to_mdio_device(dev); - - if (mdio->pm_ops && mdio->pm_ops->suspend) - return mdio->pm_ops->suspend(dev); - - return 0; -} - -static int mdio_bus_resume(struct device *dev) -{ - struct mdio_device *mdio = to_mdio_device(dev); - - if (mdio->pm_ops && mdio->pm_ops->resume) - return mdio->pm_ops->resume(dev); - - return 0; -} - -static int mdio_bus_restore(struct device *dev) -{ - struct mdio_device *mdio = to_mdio_device(dev); - - if (mdio->pm_ops && mdio->pm_ops->restore) - return mdio->pm_ops->restore(dev); - - return 0; -} - -static const struct dev_pm_ops mdio_bus_pm_ops = { - .suspend = mdio_bus_suspend, - .resume = mdio_bus_resume, - .freeze = mdio_bus_suspend, - .thaw = mdio_bus_resume, - .restore = mdio_bus_restore, -}; - -#define MDIO_BUS_PM_OPS (&mdio_bus_pm_ops) - -#else - -#define MDIO_BUS_PM_OPS NULL - -#endif /* CONFIG_PM */ - struct bus_type mdio_bus_type = { .name = "mdio_bus", .match = mdio_bus_match, .uevent = mdio_uevent, - .pm = MDIO_BUS_PM_OPS, }; EXPORT_SYMBOL(mdio_bus_type); diff --git a/include/linux/mdio.h b/include/linux/mdio.h index 2cfffe58..bfa71141 100644 --- a/include/linux/mdio.h +++ b/include/linux/mdio.h @@ -29,7 +29,6 @@ enum mdio_mutex_lock_class { struct mdio_device { struct device dev; - const struct dev_pm_ops *pm_ops; struct mii_bus *bus; char modalias[MDIO_NAME_SIZE]; -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO 2018-06-02 20:33 [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO Heiner Kallweit 2018-06-02 20:36 ` [PATCH net-next 1/2] net: phy: add struct device_type representation of a PHY Heiner Kallweit 2018-06-02 20:37 ` [PATCH net-next 2/2] net: phy: remove PM ops from MDIO bus Heiner Kallweit @ 2018-06-04 19:41 ` David Miller 2018-06-04 21:48 ` Andrew Lunn 2018-06-05 12:50 ` David Miller 4 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2018-06-04 19:41 UTC (permalink / raw) To: hkallweit1; +Cc: andrew, f.fainelli, netdev From: Heiner Kallweit <hkallweit1@gmail.com> Date: Sat, 2 Jun 2018 22:33:36 +0200 > 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. Andrew and Florian, it would nice if one of you would review this patch series. Thank you. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO 2018-06-02 20:33 [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO Heiner Kallweit ` (2 preceding siblings ...) 2018-06-04 19:41 ` [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO David Miller @ 2018-06-04 21:48 ` Andrew Lunn 2018-06-04 22:06 ` Florian Fainelli 2018-06-05 6:08 ` Heiner Kallweit 2018-06-05 12:50 ` David Miller 4 siblings, 2 replies; 8+ messages in thread From: Andrew Lunn @ 2018-06-04 21:48 UTC (permalink / raw) To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev 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. 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@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO 2018-06-04 21:48 ` Andrew Lunn @ 2018-06-04 22:06 ` Florian Fainelli 2018-06-05 6:08 ` Heiner Kallweit 1 sibling, 0 replies; 8+ messages in thread From: Florian Fainelli @ 2018-06-04 22:06 UTC (permalink / raw) To: Andrew Lunn, Heiner Kallweit; +Cc: David Miller, netdev On 06/04/2018 02:48 PM, 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. > > 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 don't think so, but I will give it a spin on a board that has system wide suspend/resume support. Might take a few hours. > > I would prefer a bit more testing, but i guess that is what -rc > kernels are for. > > Tested-by: Andrew Lunn <andrew@lunn.ch> > > Andrew > -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO 2018-06-04 21:48 ` Andrew Lunn 2018-06-04 22:06 ` Florian Fainelli @ 2018-06-05 6:08 ` Heiner Kallweit 1 sibling, 0 replies; 8+ messages in thread From: Heiner Kallweit @ 2018-06-05 6:08 UTC (permalink / raw) To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev 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@lunn.ch> > > Andrew > . > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO 2018-06-02 20:33 [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO Heiner Kallweit ` (3 preceding siblings ...) 2018-06-04 21:48 ` Andrew Lunn @ 2018-06-05 12:50 ` David Miller 4 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2018-06-05 12:50 UTC (permalink / raw) To: hkallweit1; +Cc: andrew, f.fainelli, netdev From: Heiner Kallweit <hkallweit1@gmail.com> Date: Sat, 2 Jun 2018 22:33:36 +0200 > 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. Series applied, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-06-05 12:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-02 20:33 [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO Heiner Kallweit 2018-06-02 20:36 ` [PATCH net-next 1/2] net: phy: add struct device_type representation of a PHY Heiner Kallweit 2018-06-02 20:37 ` [PATCH net-next 2/2] net: phy: remove PM ops from MDIO bus Heiner Kallweit 2018-06-04 19:41 ` [PATCH net-next 0/2] net: phy: improve PM handling of PHY/MDIO David Miller 2018-06-04 21:48 ` Andrew Lunn 2018-06-04 22:06 ` Florian Fainelli 2018-06-05 6:08 ` Heiner Kallweit 2018-06-05 12:50 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).