From 6e2d85ec05591b739059f65fe8438c9c5999f7d8 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 25 Sep 2018 11:28:45 -0700 Subject: [PATCH 1/2] net: phy: Stop with excessive soft reset While consolidating the PHY reset in phy_init_hw() an unconditionaly BMCR soft-reset I became quite trigger happy with those. This was later on deactivated for the Generic PHY driver on the premise that a prior software entity (e.g: bootloader) might have applied workarounds in commit 0878fff1f42c ("net: phy: Do not perform software reset for Generic PHY"). Since we have a hook to wire-up a soft_reset callback, just use that and get rid of the call to genphy_soft_reset() entirely. This speeds up initialization and link establishment for most PHYs out there that do not require a reset. Fixes: 87aa9f9c61ad ("net: phy: consolidate PHY reset in phy_init_hw()") Tested-by: Wang, Dongsheng Tested-by: Chris Healy Tested-by: Andrew Lunn Tested-by: Clemens Gruber Signed-off-by: Florian Fainelli Signed-off-by: David S. Miller --- drivers/net/phy/phy_device.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index af64a9320fb0..ee676d75fe02 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -880,8 +880,6 @@ int phy_init_hw(struct phy_device *phydev) if (phydev->drv->soft_reset) ret = phydev->drv->soft_reset(phydev); - else - ret = genphy_soft_reset(phydev); if (ret < 0) return ret; From d6ab93364734bd88a1011432faa9253680f7e9da Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 25 Sep 2018 11:28:46 -0700 Subject: [PATCH 2/2] net: phy: marvell: Avoid unnecessary soft reset The BMCR.RESET bit on the Marvell PHYs has a special meaning in that it commits the register writes into the HW for it to latch and be configured appropriately. Doing software resets causes link drops, and this is unnecessary disruption if nothing changed. Determine from marvell_set_polarity()'s return code whether the register value was changed and if it was, propagate that to the logic that hits the software reset bit. This avoids doing unnecessary soft reset if the PHY is configured in the same state it was previously, this also eliminates the need for a m88e1111_config_aneg() function since it now is the same as marvell_config_aneg(). Tested-by: Wang, Dongsheng Tested-by: Chris Healy Tested-by: Andrew Lunn Tested-by: Clemens Gruber Signed-off-by: Florian Fainelli Signed-off-by: David S. Miller --- drivers/net/phy/marvell.c | 67 ++++++++++++++------------------------- 1 file changed, 23 insertions(+), 44 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index f7c69ca34056..b55a7376bfdc 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -265,7 +265,7 @@ static int marvell_set_polarity(struct phy_device *phydev, int polarity) return err; } - return 0; + return val != reg; } static int marvell_set_downshift(struct phy_device *phydev, bool enable, @@ -287,12 +287,15 @@ static int marvell_set_downshift(struct phy_device *phydev, bool enable, static int marvell_config_aneg(struct phy_device *phydev) { + int changed = 0; int err; err = marvell_set_polarity(phydev, phydev->mdix_ctrl); if (err < 0) return err; + changed = err; + err = phy_write(phydev, MII_M1111_PHY_LED_CONTROL, MII_M1111_PHY_LED_DIRECT); if (err < 0) @@ -302,7 +305,7 @@ static int marvell_config_aneg(struct phy_device *phydev) if (err < 0) return err; - if (phydev->autoneg != AUTONEG_ENABLE) { + if (phydev->autoneg != AUTONEG_ENABLE || changed) { /* A write to speed/duplex bits (that is performed by * genphy_config_aneg() call above) must be followed by * a software reset. Otherwise, the write has no effect. @@ -350,42 +353,6 @@ static int m88e1101_config_aneg(struct phy_device *phydev) return marvell_config_aneg(phydev); } -static int m88e1111_config_aneg(struct phy_device *phydev) -{ - int err; - - /* The Marvell PHY has an errata which requires - * that certain registers get written in order - * to restart autonegotiation - */ - err = genphy_soft_reset(phydev); - - err = marvell_set_polarity(phydev, phydev->mdix_ctrl); - if (err < 0) - return err; - - err = phy_write(phydev, MII_M1111_PHY_LED_CONTROL, - MII_M1111_PHY_LED_DIRECT); - if (err < 0) - return err; - - err = genphy_config_aneg(phydev); - if (err < 0) - return err; - - if (phydev->autoneg != AUTONEG_ENABLE) { - /* A write to speed/duplex bits (that is performed by - * genphy_config_aneg() call above) must be followed by - * a software reset. Otherwise, the write has no effect. - */ - err = genphy_soft_reset(phydev); - if (err < 0) - return err; - } - - return 0; -} - #ifdef CONFIG_OF_MDIO /* Set and/or override some configuration registers based on the * marvell,reg-init property stored in the of_node for the phydev. @@ -479,6 +446,7 @@ static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev) static int m88e1121_config_aneg(struct phy_device *phydev) { + int changed = 0; int err = 0; if (phy_interface_is_rgmii(phydev)) { @@ -487,15 +455,26 @@ static int m88e1121_config_aneg(struct phy_device *phydev) return err; } - err = genphy_soft_reset(phydev); - if (err < 0) - return err; - err = marvell_set_polarity(phydev, phydev->mdix_ctrl); if (err < 0) return err; - return genphy_config_aneg(phydev); + changed = err; + + err = genphy_config_aneg(phydev); + if (err < 0) + return err; + + if (phydev->autoneg != autoneg || changed) { + /* A software reset is used to ensure a "commit" of the + * changes is done. + */ + err = genphy_soft_reset(phydev); + if (err < 0) + return err; + } + + return 0; } static int m88e1318_config_aneg(struct phy_device *phydev) @@ -2067,7 +2046,7 @@ static struct phy_driver marvell_drivers[] = { .flags = PHY_HAS_INTERRUPT, .probe = marvell_probe, .config_init = &m88e1111_config_init, - .config_aneg = &m88e1111_config_aneg, + .config_aneg = &marvell_config_aneg, .read_status = &marvell_read_status, .ack_interrupt = &marvell_ack_interrupt, .config_intr = &marvell_config_intr,