The mdio device core handles reset GPIOs and controls for PHYs and MDIOs after the MDIO or PHY device is created. However this does not cover the initial PHY ID read _before_ the PHY device is created, since the PHY ID is needed for the PHY device. This causes PHY devices that have reset GPIOs or controls to not work after a reboot if the GPIO is left in the reset state; neither will it work if the reset GPIO is by default (for example, missing a pull-up) in the reset state. One possible workaround is to place the reset GPIO or control property under the MDIO bus instead of under the PHY. However the common PHY device tree bindings already allow a reset for the PHY, so we should make some effort to support this. Rework get_phy_device() to allow passing in a fwnode handle for the PHY device, and use the handle to acquire the reset GPIO, control and delay timings for it. Before reading the PHY ID, deassert the reset. This reworked version is renamed to fwnode_get_phy_device(), with get_phy_device() calling the new version. Use this new version in fwnode_mdiobus_register_phy() so that PHY reset is handled. This allows the reset GPIO and reset control in the common Ethernet PHY device tree to be correctly handled without any gotchas. Signed-off-by: Chen-Yu Tsai --- This work was the result of Russell mentioning [1] that placing the reset GPIO under the PHY node in the device tree might result in it not working. I also talked about this at Plumbers in Tokyo last year during the Device Tree MC. There a few people mentioned that MDIO reset handling has been a pain point. [1] https://lore.kernel.org/linux-sunxi/aJy_qUbmqoOG-GBC@shell.armlinux.org.uk/ drivers/net/mdio/fwnode_mdio.c | 2 +- drivers/net/phy/phy_device.c | 47 +++++++++++++++++++++++++++++++--- include/linux/phy.h | 10 +++++++- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c index ba7091518265..f62a48583404 100644 --- a/drivers/net/mdio/fwnode_mdio.c +++ b/drivers/net/mdio/fwnode_mdio.c @@ -130,7 +130,7 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus, is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"); if (is_c45 || fwnode_get_phy_id(child, &phy_id)) - phy = get_phy_device(bus, addr, is_c45); + phy = fwnode_get_phy_device(bus, addr, child, is_c45); else phy = phy_device_create(bus, addr, phy_id, 0, NULL); if (IS_ERR(phy)) { diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 3bd415710bf3..62304fcacc7b 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -34,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -1050,14 +1052,17 @@ int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id) EXPORT_SYMBOL(fwnode_get_phy_id); /** - * get_phy_device - reads the specified PHY device and returns its @phy_device - * struct + * fwnode_get_phy_device - reads the specified PHY device and returns its + * @phy_device struct * @bus: the target MII bus * @addr: PHY address on the MII bus + * @fwnode: PHY fwnode handle * @is_c45: If true the PHY uses the 802.3 clause 45 protocol * * Probe for a PHY at @addr on @bus. * + * Transparently handle any reset GPIOs. + * * When probing for a clause 22 PHY, then read the ID registers. If we find * a valid ID, allocate and return a &struct phy_device. * @@ -1068,21 +1073,55 @@ EXPORT_SYMBOL(fwnode_get_phy_id); * Returns an allocated &struct phy_device on success, %-ENODEV if there is * no PHY present, or %-EIO on bus access error. */ -struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45) +struct phy_device *fwnode_get_phy_device(struct mii_bus *bus, int addr, + struct fwnode_handle *fwnode, bool is_c45) { struct phy_c45_device_ids c45_ids; + struct gpio_desc *gpiod = NULL; + struct reset_control *rstc = NULL; u32 phy_id = 0; + u32 delay = 0; int r; c45_ids.devices_in_package = 0; c45_ids.mmds_present = 0; memset(c45_ids.device_ids, 0xff, sizeof(c45_ids.device_ids)); + if (fwnode) { + /* Deassert the optional reset signal */ + gpiod = fwnode_gpiod_get_index(fwnode, "reset", 0, + GPIOD_OUT_LOW, "PHY reset"); + if (IS_ERR(gpiod)) { + if (PTR_ERR(gpiod) == -ENOENT) + gpiod = NULL; + else if (PTR_ERR(gpiod) == -ENOSYS) + gpiod = NULL; + else + return ERR_CAST(gpiod); + } + + if (is_of_node(fwnode)) { + rstc = of_reset_control_get_optional_exclusive(to_of_node(fwnode), "phy"); + if (IS_ERR(rstc)) + return ERR_CAST(rstc); + reset_control_deassert(rstc); + } + + /* Wait for PHY to come out of reset if needed */ + if (!fwnode_property_read_u32(fwnode, "reset-deassert-us", &delay)) + fsleep(delay); + } + if (is_c45) r = get_phy_c45_ids(bus, addr, &c45_ids); else r = get_phy_c22_id(bus, addr, &phy_id); + if (!IS_ERR_OR_NULL(rstc)) + reset_control_put(rstc); + if (!IS_ERR_OR_NULL(gpiod)) + gpiod_put(gpiod); + if (r) return ERR_PTR(r); @@ -1100,7 +1139,7 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45) return phy_device_create(bus, addr, phy_id, is_c45, &c45_ids); } -EXPORT_SYMBOL(get_phy_device); +EXPORT_SYMBOL(fwnode_get_phy_device); /** * phy_device_register - Register the phy device on the MDIO bus diff --git a/include/linux/phy.h b/include/linux/phy.h index 6f9979a26892..3bd3bbc1b281 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -2142,7 +2142,15 @@ int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id); struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode); struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode); struct fwnode_handle *fwnode_get_phy_node(const struct fwnode_handle *fwnode); -struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45); +struct phy_device *fwnode_get_phy_device(struct mii_bus *bus, int addr, + struct fwnode_handle *fwnode, bool is_c45); + +static inline +struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45) +{ + return fwnode_get_phy_device(bus, addr, NULL, is_c45); +} + int phy_device_register(struct phy_device *phy); void phy_device_free(struct phy_device *phydev); void phy_device_remove(struct phy_device *phydev); -- 2.47.3