| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180105010230.GC17719@n2100.armlinux.org.uk>
Date: Fri, 5 Jan 2018 01:02:30 +0000
From: Russell King - ARM Linux <linux@...linux.org.uk>
To: Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
Cc: Grygorii Strashko <grygorii.strashko@...com>,
Heiner Kallweit <hkallweit1@...il.com>,
Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Issue with commit fea23fb591cc "net: phy: convert
read-modify-write to phy_modify()"
On Fri, Jan 05, 2018 at 02:44:07AM +0200, Ivan Khoronzhuk wrote:
> + G.Strashko
> The below change also brokes phy connect for am572x..
>
> int genphy_restart_aneg(struct phy_device *phydev)
> {
> - int ctl = phy_read(phydev, MII_BMCR);
> -
> - if (ctl < 0)
> - return ctl;
> -
> - ctl |= BMCR_ANENABLE | BMCR_ANRESTART;
> -
> /* Don't isolate the PHY if we're negotiating */
> - ctl &= ~BMCR_ISOLATE;
> -
> - return phy_write(phydev, MII_BMCR, ctl);
> + return phy_modify(phydev, MII_BMCR, ~BMCR_ISOLATE,
> + BMCR_ANENABLE | BMCR_ANRESTART);
>
This should fix it, but it needs a bit more work, as it's also
recently been pointed out that the comment is wrong. Also, as I
said for the original patch, it needs a thorough review - and
because the last one didn't get that, the problem is now
compounded by two patches needing a thorough review, or both
being looked at as one patch combined.
I'm afraid I've not had time to fully complete this patch yet
because of the recent issues around CPU cache speculation, news
of which broke without warning on Wednesday.
I tested what's in net-next again, and it passes my tests locally -
it's only if I take an interface through an up-down-up sequence
that I see a problem (as discovered by Andrew) which explains why
my testing did not find it.
8<===
From: Russell King <rmk+kernel@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>,Florian Fainelli <f.fainelli@...il.com>
Cc: netdev@...r.kernel.org
Subject: [PATCH] net: phy: fix wrong masks to phy_modify()
The mask argument for phy_modify() in several locations was inverted.
Fixes: fea23fb591cc ("net: phy: convert read-modify-write to phy_modify()")
Reported-by: Heiner Kallweit <hkallweit1@...il.com>
Tested-by: Andrew Lunn <andrew@...n.ch>
Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
---
drivers/net/phy/at803x.c | 2 +-
drivers/net/phy/marvell.c | 19 +++++++++----------
drivers/net/phy/phy_device.c | 6 +++---
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index e86c1b8b1b51..a80cce11b252 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -231,7 +231,7 @@ static int at803x_suspend(struct phy_device *phydev)
static int at803x_resume(struct phy_device *phydev)
{
- return phy_modify(phydev, MII_BMCR, ~(BMCR_PDOWN | BMCR_ISOLATE), 0);
+ return phy_modify(phydev, MII_BMCR, BMCR_PDOWN | BMCR_ISOLATE, 0);
}
static int at803x_probe(struct phy_device *phydev)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 8212c2fd7fe1..5d28063e9327 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -668,7 +668,7 @@ static int m88e3016_config_init(struct phy_device *phydev)
/* Enable Scrambler and Auto-Crossover */
ret = phy_modify(phydev, MII_88E3016_PHY_SPEC_CTRL,
- ~MII_88E3016_DISABLE_SCRAMBLER,
+ MII_88E3016_DISABLE_SCRAMBLER,
MII_88E3016_AUTO_MDIX_CROSSOVER);
if (ret < 0)
return ret;
@@ -684,9 +684,9 @@ static int m88e1111_config_init_hwcfg_mode(struct phy_device *phydev,
mode |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
return phy_modify(phydev, MII_M1111_PHY_EXT_SR,
- (u16)~(MII_M1111_HWCFG_MODE_MASK |
- MII_M1111_HWCFG_FIBER_COPPER_AUTO |
- MII_M1111_HWCFG_FIBER_COPPER_RES),
+ MII_M1111_HWCFG_MODE_MASK |
+ MII_M1111_HWCFG_FIBER_COPPER_AUTO |
+ MII_M1111_HWCFG_FIBER_COPPER_RES,
mode);
}
@@ -705,8 +705,7 @@ static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
}
return phy_modify(phydev, MII_M1111_PHY_EXT_CR,
- (u16)~(MII_M1111_RGMII_RX_DELAY |
- MII_M1111_RGMII_TX_DELAY),
+ MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY,
delay);
}
@@ -833,7 +832,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
/* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1,
- ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
+ MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII);
if (err < 0)
return err;
@@ -957,7 +956,7 @@ static int m88e1145_config_init_rgmii(struct phy_device *phydev)
if (err < 0)
return err;
- err = phy_modify(phydev, 0x1e, 0xf03f,
+ err = phy_modify(phydev, 0x1e, 0x0fc0,
2 << 9 | /* 36 ohm */
2 << 6); /* 39 ohm */
if (err < 0)
@@ -1379,7 +1378,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
/* Setup LED[2] as interrupt pin (active low) */
err = __phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
- (u16)~MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+ MII_88E1318S_PHY_LED_TCR_FORCE_INT,
MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
if (err < 0)
@@ -1419,7 +1418,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
/* Clear WOL status and disable magic packet matching */
err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL,
- (u16)~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE,
+ MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE,
MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS);
if (err < 0)
goto error;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e5ddc5ae56d1..e1acb3052515 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1353,7 +1353,7 @@ EXPORT_SYMBOL(genphy_setup_forced);
int genphy_restart_aneg(struct phy_device *phydev)
{
/* Don't isolate the PHY if we're negotiating */
- return phy_modify(phydev, MII_BMCR, ~BMCR_ISOLATE,
+ return phy_modify(phydev, MII_BMCR, BMCR_ISOLATE,
BMCR_ANENABLE | BMCR_ANRESTART);
}
EXPORT_SYMBOL(genphy_restart_aneg);
@@ -1626,13 +1626,13 @@ EXPORT_SYMBOL(genphy_suspend);
int genphy_resume(struct phy_device *phydev)
{
- return phy_modify(phydev, MII_BMCR, ~BMCR_PDOWN, 0);
+ return phy_modify(phydev, MII_BMCR, BMCR_PDOWN, 0);
}
EXPORT_SYMBOL(genphy_resume);
int genphy_loopback(struct phy_device *phydev, bool enable)
{
- return phy_modify(phydev, MII_BMCR, ~BMCR_LOOPBACK,
+ return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
enable ? BMCR_LOOPBACK : 0);
}
EXPORT_SYMBOL(genphy_loopback);
--
2.7.4
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
Powered by blists - more mailing lists