From c85ddecae6e5e82ca3ae6f20c63f1d865e2ff5ea Mon Sep 17 00:00:00 2001 From: Heiner Kallweit <hkallweit1@gmail.com> Date: Fri, 23 Nov 2018 19:41:29 +0100 Subject: net: phy: add workaround for issue where PHY driver doesn't bind to the device After switching the r8169 driver to use phylib some user reported that their network is broken. This was caused by the genphy PHY driver being used instead of the dedicated PHY driver for the RTL8211B. Users reported that loading the Realtek PHY driver module upfront fixes the issue. See also this mail thread: https://marc.info/?t=154279781800003&r=1&w=2 The issue is quite weird and the root cause seems to be somewhere in the base driver core. The patch works around the issue and may be removed once the actual issue is fixed. The Fixes tag refers to the first reported occurrence of the issue. The issue itself may have been existing much longer and it may affect users of other network chips as well. Users typically will recognize this issue only if their PHY stops working when being used with the genphy driver. Fixes: f1e911d5d0df ("r8169: add basic phylib support") Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: David S. Miller <davem@davemloft.net> --- drivers/net/phy/phy_device.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/net/phy/phy_device.c') diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index ab33d1777132..23ee3967c166 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -2197,6 +2197,14 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner) new_driver->mdiodrv.driver.remove = phy_remove; new_driver->mdiodrv.driver.owner = owner; + /* The following works around an issue where the PHY driver doesn't bind + * to the device, resulting in the genphy driver being used instead of + * the dedicated driver. The root cause of the issue isn't known yet + * and seems to be in the base driver core. Once this is fixed we may + * remove this workaround. + */ + new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS; + retval = driver_register(&new_driver->mdiodrv.driver); if (retval) { pr_err("%s: Error %d in registering driver\n", -- cgit From d2a36971ef595069b7a600d1144c2e0881a930a1 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit <hkallweit1@gmail.com> Date: Mon, 3 Dec 2018 08:19:33 +0100 Subject: net: phy: don't allow __set_phy_supported to add unsupported modes Currently __set_phy_supported allows to add modes w/o checking whether the PHY supports them. This is wrong, it should never add modes but only remove modes we don't want to support. The commit marked as fixed didn't do anything wrong, it just copied existing functionality to the helper which is being fixed now. Fixes: f3a6bd393c2c ("phylib: Add phy_set_max_speed helper") Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: David S. Miller <davem@davemloft.net> --- drivers/net/phy/phy_device.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'drivers/net/phy/phy_device.c') diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 23ee3967c166..18e92c19c5ab 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1880,20 +1880,17 @@ EXPORT_SYMBOL(genphy_loopback); static int __set_phy_supported(struct phy_device *phydev, u32 max_speed) { - phydev->supported &= ~(PHY_1000BT_FEATURES | PHY_100BT_FEATURES | - PHY_10BT_FEATURES); - switch (max_speed) { - default: - return -ENOTSUPP; - case SPEED_1000: - phydev->supported |= PHY_1000BT_FEATURES; + case SPEED_10: + phydev->supported &= ~PHY_100BT_FEATURES; /* fall through */ case SPEED_100: - phydev->supported |= PHY_100BT_FEATURES; - /* fall through */ - case SPEED_10: - phydev->supported |= PHY_10BT_FEATURES; + phydev->supported &= ~PHY_1000BT_FEATURES; + break; + case SPEED_1000: + break; + default: + return -ENOTSUPP; } return 0; -- cgit From ef1b5bf506b1f0ee3edc98533e1f3ecb105eb46a Mon Sep 17 00:00:00 2001 From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Date: Wed, 28 Nov 2018 09:02:41 +0000 Subject: net: phy: Fix not to call phy_resume() if PHY is not attached This patch fixes an issue that mdio_bus_phy_resume() doesn't call phy_resume() if the PHY is not attached. Fixes: 803dd9c77ac3 ("net: phy: avoid suspending twice a PHY") Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Signed-off-by: David S. Miller <davem@davemloft.net> --- drivers/net/phy/phy_device.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'drivers/net/phy/phy_device.c') diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 18e92c19c5ab..c4b9008c52d2 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -220,7 +220,7 @@ static LIST_HEAD(phy_fixup_list); static DEFINE_MUTEX(phy_fixup_lock); #ifdef CONFIG_PM -static bool mdio_bus_phy_may_suspend(struct phy_device *phydev) +static bool mdio_bus_phy_may_suspend(struct phy_device *phydev, bool suspend) { struct device_driver *drv = phydev->mdio.dev.driver; struct phy_driver *phydrv = to_phy_driver(drv); @@ -232,10 +232,11 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev) /* PHY not attached? May suspend if the PHY has not already been * suspended as part of a prior call to phy_disconnect() -> * phy_detach() -> phy_suspend() because the parent netdev might be the - * MDIO bus driver and clock gated at this point. + * MDIO bus driver and clock gated at this point. Also may resume if + * PHY is not attached. */ if (!netdev) - return !phydev->suspended; + return suspend ? !phydev->suspended : phydev->suspended; if (netdev->wol_enabled) return false; @@ -270,7 +271,7 @@ static int mdio_bus_phy_suspend(struct device *dev) if (phydev->attached_dev && phydev->adjust_link) phy_stop_machine(phydev); - if (!mdio_bus_phy_may_suspend(phydev)) + if (!mdio_bus_phy_may_suspend(phydev, true)) return 0; return phy_suspend(phydev); @@ -281,7 +282,7 @@ static int mdio_bus_phy_resume(struct device *dev) struct phy_device *phydev = to_phy_device(dev); int ret; - if (!mdio_bus_phy_may_suspend(phydev)) + if (!mdio_bus_phy_may_suspend(phydev, false)) goto no_resume; ret = phy_resume(phydev); -- cgit From 7b566f70e1bf65b189b66eb3de6f431c30f7dff2 Mon Sep 17 00:00:00 2001 From: "David S. Miller" <davem@davemloft.net> Date: Tue, 4 Dec 2018 08:47:44 -0800 Subject: phy: Revert toggling reset changes. This reverts: ef1b5bf506b1 ("net: phy: Fix not to call phy_resume() if PHY is not attached") 8c85f4b81296 ("net: phy: micrel: add toggling phy reset if PHY is not attached") Andrew Lunn informs me that there are alternative efforts underway to fix this more properly. Signed-off-by: David S. Miller <davem@davemloft.net> --- drivers/net/phy/micrel.c | 8 -------- drivers/net/phy/phy_device.c | 11 +++++------ 2 files changed, 5 insertions(+), 14 deletions(-) (limited to 'drivers/net/phy/phy_device.c') diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index 1679a6ea104c..9265dea79412 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -23,7 +23,6 @@ * ksz9477 */ -#include <linux/delay.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/phy.h> @@ -836,13 +835,6 @@ static int kszphy_resume(struct phy_device *phydev) { int ret; - if (!phydev->attached_dev) { - /* If the PHY is not attached, toggle the reset */ - phy_device_reset(phydev, 1); - udelay(1); - phy_device_reset(phydev, 0); - } - genphy_resume(phydev); ret = kszphy_config_reset(phydev); diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index c4b9008c52d2..18e92c19c5ab 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -220,7 +220,7 @@ static LIST_HEAD(phy_fixup_list); static DEFINE_MUTEX(phy_fixup_lock); #ifdef CONFIG_PM -static bool mdio_bus_phy_may_suspend(struct phy_device *phydev, bool suspend) +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); @@ -232,11 +232,10 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev, bool suspend) /* PHY not attached? May suspend if the PHY has not already been * suspended as part of a prior call to phy_disconnect() -> * phy_detach() -> phy_suspend() because the parent netdev might be the - * MDIO bus driver and clock gated at this point. Also may resume if - * PHY is not attached. + * MDIO bus driver and clock gated at this point. */ if (!netdev) - return suspend ? !phydev->suspended : phydev->suspended; + return !phydev->suspended; if (netdev->wol_enabled) return false; @@ -271,7 +270,7 @@ static int mdio_bus_phy_suspend(struct device *dev) if (phydev->attached_dev && phydev->adjust_link) phy_stop_machine(phydev); - if (!mdio_bus_phy_may_suspend(phydev, true)) + if (!mdio_bus_phy_may_suspend(phydev)) return 0; return phy_suspend(phydev); @@ -282,7 +281,7 @@ static int mdio_bus_phy_resume(struct device *dev) struct phy_device *phydev = to_phy_device(dev); int ret; - if (!mdio_bus_phy_may_suspend(phydev, false)) + if (!mdio_bus_phy_may_suspend(phydev)) goto no_resume; ret = phy_resume(phydev); -- cgit