From 7f4828ff27d4c92280222f6427981052fd3319e9 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sat, 2 Jun 2018 22:36:06 +0200 Subject: [PATCH 1/2] net: phy: add struct device_type representation of a PHY 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 Signed-off-by: David S. Miller --- 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 9e4ba8e80a18..bd0f339f69fd 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); From 9107c05e2e55c1c7abd02ab38f7694e0da08b643 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sat, 2 Jun 2018 22:37:24 +0200 Subject: [PATCH 2/2] net: phy: remove PM ops from MDIO bus 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 Signed-off-by: David S. Miller --- 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 24b5511222c8..98f4b1f706df 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 2cfffe586885..bfa7114167d7 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];