Message-ID: <20250425151309.30493-1-kabel@kernel.org>
Date: Fri, 25 Apr 2025 17:13:09 +0200
From: Marek Behún <kabel@...nel.org>
To: netdev@...r.kernel.org
Cc: Marek Behún <kabel@...nel.org>,
	Andrew Lunn <andrew@...n.ch>,
	Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
	"Rob Herring (Arm)" <robh@...nel.org>,
	Javier Carrasco <javier.carrasco.cruz@...il.com>,
	Wojciech Drewek <wojciech.drewek@...el.com>,
	Matthias Schiffer <mschiffer@...verse-factory.net>,
	Christian Marangi <ansuelsmth@...il.com>,
	Marek Mojík <marek.mojik@....cz>
Subject: [PATCH net] net: dsa: qca8k: forbid management frames access to internal PHYs if another device is on the MDIO bus

Completely forbid access to the internal switch PHYs via management
frames if there is another MDIO device on the MDIO bus besides the QCA8K
switch.

It seems that when internal PHYs are accessed via management frames,
internally the switch core translates these requests to MDIO accesses
and communicates with the internal PHYs via the MDIO bus. This
communication leaks outside on the MDC and MDIO pins. If there is
another PHY device connected on the MDIO bus besides the QCA8K switch,
and the kernel tries to communicate with the PHY at the same time, the
communication gets corrupted.

This makes the WAN PHY break on the Turris 1.x device.

In commit 526c8ee04bdbd4d8 ("net: dsa: qca8k: fix potential MDIO bus
conflict when accessing internal PHYs via management frames") we tried
to solve this issue by locking access to the MDIO bus when accessing
internal PHYs via management frames. It seems that the bug still
prevails though, and we are not able to determine why. Therefore this
seems the only viable fix for now.

Fixes: 526c8ee04bdbd4d8 ("net: dsa: qca8k: fix potential MDIO bus conflict when accessing internal PHYs via management frames")
Signed-off-by: Marek Behún <kabel@...nel.org>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 53 ++++++++++++++++++++++++--------
 drivers/net/dsa/qca/qca8k.h      |  1 +
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index a36b8b07030e..7f9fd3fc0d75 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -633,6 +633,9 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
 	if (regnum >= QCA8K_MDIO_MASTER_MAX_REG)
 		return -EINVAL;
 
+	if (!priv->can_access_phys_over_mgmt)
+		return -EBUSY;
+
 	mgmt_eth_data = &priv->mgmt_eth_data;
 
 	write_val = QCA8K_MDIO_MASTER_BUSY | QCA8K_MDIO_MASTER_EN |
@@ -666,15 +669,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
 		goto err_read_skb;
 	}
 
-	/* It seems that accessing the switch's internal PHYs via management
-	 * packets still uses the MDIO bus within the switch internally, and
-	 * these accesses can conflict with external MDIO accesses to other
-	 * devices on the MDIO bus.
-	 * We therefore need to lock the MDIO bus onto which the switch is
-	 * connected.
-	 */
-	mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
-
 	/* Actually start the request:
 	 * 1. Send mdio master packet
 	 * 2. Busy Wait for mdio master command
@@ -687,7 +681,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
 	mgmt_conduit = priv->mgmt_conduit;
 	if (!mgmt_conduit) {
 		mutex_unlock(&mgmt_eth_data->mutex);
-		mutex_unlock(&priv->bus->mdio_lock);
 		ret = -EINVAL;
 		goto err_mgmt_conduit;
 	}
@@ -775,7 +768,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
 				    QCA8K_ETHERNET_TIMEOUT);
 
 	mutex_unlock(&mgmt_eth_data->mutex);
-	mutex_unlock(&priv->bus->mdio_lock);
 
 	return ret;
 
@@ -943,6 +935,39 @@ qca8k_legacy_mdio_read(struct mii_bus *slave_bus, int port, int regnum)
 	return qca8k_internal_mdio_read(slave_bus, port, regnum);
 }
 
+static int qca8k_mdio_determine_access_over_eth(struct qca8k_priv *priv)
+{
+	struct device_node *parent = of_get_parent(priv->dev->of_node);
+	int addr = to_mdio_device(priv->dev)->addr;
+	bool result = true;
+	u32 reg;
+	int ret;
+
+	/* It seems that accessing the switch's internal PHYs via management
+	 * packets still uses the MDIO bus within the switch internally, and
+	 * these accesses can conflict with external MDIO accesses to other
+	 * devices on the MDIO bus.
+	 *
+	 * Determine whether there are other devices on the MDIO bus besides
+	 * the switch, and if there are, forbid access to the internal PHYs
+	 * via management frames.
+	 */
+	for_each_available_child_of_node_scoped(parent, sibling) {
+		ret = of_property_read_u32(sibling, "reg", &reg);
+		if (ret)
+			return ret;
+
+		if (reg != addr) {
+			result = false;
+			break;
+		}
+	}
+
+	priv->can_access_phys_over_mgmt = result;
+
+	return 0;
+}
+
 static int
 qca8k_mdio_register(struct qca8k_priv *priv)
 {
@@ -950,7 +975,11 @@ qca8k_mdio_register(struct qca8k_priv *priv)
 	struct device *dev = ds->dev;
 	struct device_node *mdio;
 	struct mii_bus *bus;
-	int ret = 0;
+	int ret;
+
+	ret = qca8k_mdio_determine_access_over_eth(priv);
+	if (ret)
+		return ret;
 
 	mdio = of_get_child_by_name(dev->of_node, "mdio");
 	if (mdio && !of_device_is_available(mdio))
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index d046679265fa..346f79f81fe5 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -463,6 +463,7 @@ struct qca8k_priv {
 	struct net_device *mgmt_conduit; /* Track if mdio/mib Ethernet is available */
 	struct qca8k_mgmt_eth_data mgmt_eth_data;
 	struct qca8k_mib_eth_data mib_eth_data;
+	bool can_access_phys_over_mgmt;
 	struct qca8k_mdio_cache mdio_cache;
 	struct qca8k_pcs pcs_port_0;
 	struct qca8k_pcs pcs_port_6;
-- 
2.49.0

Powered by blists - more mailing lists