linux-kernel - [PATCH RFC net-next 2/5] net: phy: support indirect c45 access in get_phy_c45

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220323183419.2278676-3-michael@walle.cc>
Date:   Wed, 23 Mar 2022 19:34:16 +0100
From:   Michael Walle <michael@...le.cc>
To:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Xu Liang <lxu@...linear.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michael Walle <michael@...le.cc>
Subject: [PATCH RFC net-next 2/5] net: phy: support indirect c45 access in get_phy_c45_ids()

There are some PHYs, namely the Maxlinear GPY215, whose driver is
explicitly supporting C45-over-C22 access. At least that was the
intention. In practice, it cannot work because get_phy_c45_ids()
will always issue c45 register accesses.

There is another issue at hand: the Microchip LAN8814, which is
a c22 only quad PHY, has issues with c45 accesses on the same bus
and its address decoder will find a match in the middle of another
c45 transaction. This will lead to spurious reads and writes. The
reads will corrupt the c45 in flight. The write will lead to random
writes to the LAN8814 registers. As a workaround for PHYs which
support C45-over-C22 register accesses, we can make the MDIO bus
c22-only.

For both reasons, extend the register accesses in get_phy_c45_ids()
to allow indirect accesses, indicated by the bus->probe_capabilities
bits. The probe_capabilites can then be degraded by a device tree
property, for example. Or it will just work when the MDIO driver
is c22-only and set the capabilities accordingly.

Signed-off-by: Michael Walle <michael@...le.cc>
---
 drivers/net/phy/phy_device.c | 46 ++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8406ac739def..c766f5bb421a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -649,6 +649,42 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
 }
 EXPORT_SYMBOL(phy_device_create);
 
+static int mdiobus_probe_mmd_read(struct mii_bus *bus, int prtad, int devad,
+				  u16 regnum)
+{
+	int ret;
+
+	/* For backwards compatibility, treat MDIOBUS_NO_CAP as c45 capable */
+	if (bus->probe_capabilities == MDIOBUS_NO_CAP ||
+	    bus->probe_capabilities >= MDIOBUS_C45)
+		return mdiobus_c45_read(bus, prtad, devad, regnum);
+
+	mutex_lock(&bus->mdio_lock);
+
+	/* Write the desired MMD Devad */
+	ret = __mdiobus_write(bus, prtad, MII_MMD_CTRL, devad);
+	if (ret)
+		goto out;
+
+	/* Write the desired MMD register address */
+	ret = __mdiobus_write(bus, prtad, MII_MMD_DATA, regnum);
+	if (ret)
+		goto out;
+
+	/* Select the Function : DATA with no post increment */
+	ret = __mdiobus_write(bus, prtad, MII_MMD_CTRL,
+			      devad | MII_MMD_CTRL_NOINCR);
+	if (ret)
+		goto out;
+
+	ret = __mdiobus_read(bus, prtad, MII_MMD_DATA);
+
+out:
+	mutex_unlock(&bus->mdio_lock);
+
+	return ret;
+}
+
 /* phy_c45_probe_present - checks to see if a MMD is present in the package
  * @bus: the target MII bus
  * @prtad: PHY package address on the MII bus
@@ -664,7 +700,7 @@ static int phy_c45_probe_present(struct mii_bus *bus, int prtad, int devad)
 {
 	int stat2;
 
-	stat2 = mdiobus_c45_read(bus, prtad, devad, MDIO_STAT2);
+	stat2 = mdiobus_probe_mmd_read(bus, prtad, devad, MDIO_STAT2);
 	if (stat2 < 0)
 		return stat2;
 
@@ -687,12 +723,12 @@ static int get_phy_c45_devs_in_pkg(struct mii_bus *bus, int addr, int dev_addr,
 {
 	int phy_reg;
 
-	phy_reg = mdiobus_c45_read(bus, addr, dev_addr, MDIO_DEVS2);
+	phy_reg = mdiobus_probe_mmd_read(bus, addr, dev_addr, MDIO_DEVS2);
 	if (phy_reg < 0)
 		return -EIO;
 	*devices_in_package = phy_reg << 16;
 
-	phy_reg = mdiobus_c45_read(bus, addr, dev_addr, MDIO_DEVS1);
+	phy_reg = mdiobus_probe_mmd_read(bus, addr, dev_addr, MDIO_DEVS1);
 	if (phy_reg < 0)
 		return -EIO;
 	*devices_in_package |= phy_reg;
@@ -776,12 +812,12 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr,
 				continue;
 		}
 
-		phy_reg = mdiobus_c45_read(bus, addr, i, MII_PHYSID1);
+		phy_reg = mdiobus_probe_mmd_read(bus, addr, i, MII_PHYSID1);
 		if (phy_reg < 0)
 			return -EIO;
 		c45_ids->device_ids[i] = phy_reg << 16;
 
-		phy_reg = mdiobus_c45_read(bus, addr, i, MII_PHYSID2);
+		phy_reg = mdiobus_probe_mmd_read(bus, addr, i, MII_PHYSID2);
 		if (phy_reg < 0)
 			return -EIO;
 		c45_ids->device_ids[i] |= phy_reg;
-- 
2.30.2