mirror of
				git://git.openwrt.org/openwrt/openwrt.git
				synced 2025-11-03 14:34:27 -05:00 
			
		
		
		
	generic: 6.1: backport qca8k fixes for big endian and MDIO
Backport qca8k fixes for big endian system (to make them working again) and a patch fixing MDIO conflicts if other PHY are connected and mgmt eth is used to control the switch. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
This commit is contained in:
		
							parent
							
								
									b0048b3146
								
							
						
					
					
						commit
						20d74c6811
					
				@ -0,0 +1,61 @@
 | 
			
		||||
From 5652d1741574eb89cc02576e50ee3e348bd6dd77 Mon Sep 17 00:00:00 2001
 | 
			
		||||
From: =?UTF-8?q?Marek=20Beh=C3=BAn?= <kabel@kernel.org>
 | 
			
		||||
Date: Wed, 4 Oct 2023 11:19:03 +0200
 | 
			
		||||
Subject: [PATCH 1/2] net: dsa: qca8k: fix regmap bulk read/write methods on
 | 
			
		||||
 big endian systems
 | 
			
		||||
MIME-Version: 1.0
 | 
			
		||||
Content-Type: text/plain; charset=UTF-8
 | 
			
		||||
Content-Transfer-Encoding: 8bit
 | 
			
		||||
 | 
			
		||||
Commit c766e077d927 ("net: dsa: qca8k: convert to regmap read/write
 | 
			
		||||
API") introduced bulk read/write methods to qca8k's regmap.
 | 
			
		||||
 | 
			
		||||
The regmap bulk read/write methods get the register address in a buffer
 | 
			
		||||
passed as a void pointer parameter (the same buffer contains also the
 | 
			
		||||
read/written values). The register address occupies only as many bytes
 | 
			
		||||
as it requires at the beginning of this buffer. For example if the
 | 
			
		||||
.reg_bits member in regmap_config is 16 (as is the case for this
 | 
			
		||||
driver), the register address occupies only the first 2 bytes in this
 | 
			
		||||
buffer, so it can be cast to u16.
 | 
			
		||||
 | 
			
		||||
But the original commit implementing these bulk read/write methods cast
 | 
			
		||||
the buffer to u32:
 | 
			
		||||
  u32 reg = *(u32 *)reg_buf & U16_MAX;
 | 
			
		||||
taking the first 4 bytes. This works on little endian systems where the
 | 
			
		||||
first 2 bytes of the buffer correspond to the low 16-bits, but it
 | 
			
		||||
obviously cannot work on big endian systems.
 | 
			
		||||
 | 
			
		||||
Fix this by casting the beginning of the buffer to u16 as
 | 
			
		||||
   u32 reg = *(u16 *)reg_buf;
 | 
			
		||||
 | 
			
		||||
Fixes: c766e077d927 ("net: dsa: qca8k: convert to regmap read/write API")
 | 
			
		||||
Signed-off-by: Marek Behún <kabel@kernel.org>
 | 
			
		||||
Tested-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
Reviewed-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
Signed-off-by: David S. Miller <davem@davemloft.net>
 | 
			
		||||
---
 | 
			
		||||
 drivers/net/dsa/qca/qca8k-8xxx.c | 4 ++--
 | 
			
		||||
 1 file changed, 2 insertions(+), 2 deletions(-)
 | 
			
		||||
 | 
			
		||||
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
@@ -504,8 +504,8 @@ qca8k_bulk_read(void *ctx, const void *r
 | 
			
		||||
 		void *val_buf, size_t val_len)
 | 
			
		||||
 {
 | 
			
		||||
 	int i, count = val_len / sizeof(u32), ret;
 | 
			
		||||
-	u32 reg = *(u32 *)reg_buf & U16_MAX;
 | 
			
		||||
 	struct qca8k_priv *priv = ctx;
 | 
			
		||||
+	u32 reg = *(u16 *)reg_buf;
 | 
			
		||||
 
 | 
			
		||||
 	if (priv->mgmt_master &&
 | 
			
		||||
 	    !qca8k_read_eth(priv, reg, val_buf, val_len))
 | 
			
		||||
@@ -526,8 +526,8 @@ qca8k_bulk_gather_write(void *ctx, const
 | 
			
		||||
 			const void *val_buf, size_t val_len)
 | 
			
		||||
 {
 | 
			
		||||
 	int i, count = val_len / sizeof(u32), ret;
 | 
			
		||||
-	u32 reg = *(u32 *)reg_buf & U16_MAX;
 | 
			
		||||
 	struct qca8k_priv *priv = ctx;
 | 
			
		||||
+	u32 reg = *(u16 *)reg_buf;
 | 
			
		||||
 	u32 *val = (u32 *)val_buf;
 | 
			
		||||
 
 | 
			
		||||
 	if (priv->mgmt_master &&
 | 
			
		||||
@ -0,0 +1,106 @@
 | 
			
		||||
From 526c8ee04bdbd4d8d19a583b1f3b06700229a815 Mon Sep 17 00:00:00 2001
 | 
			
		||||
From: =?UTF-8?q?Marek=20Beh=C3=BAn?= <kabel@kernel.org>
 | 
			
		||||
Date: Wed, 4 Oct 2023 11:19:04 +0200
 | 
			
		||||
Subject: [PATCH 2/2] net: dsa: qca8k: fix potential MDIO bus conflict when
 | 
			
		||||
 accessing internal PHYs via management frames
 | 
			
		||||
MIME-Version: 1.0
 | 
			
		||||
Content-Type: text/plain; charset=UTF-8
 | 
			
		||||
Content-Transfer-Encoding: 8bit
 | 
			
		||||
 | 
			
		||||
Besides the QCA8337 switch the Turris 1.x device has on it's MDIO bus
 | 
			
		||||
also Micron ethernet PHY (dedicated to the WAN port).
 | 
			
		||||
 | 
			
		||||
We've been experiencing a strange behavior of the WAN ethernet
 | 
			
		||||
interface, wherein the WAN PHY started timing out the MDIO accesses, for
 | 
			
		||||
example when the interface was brought down and then back up.
 | 
			
		||||
 | 
			
		||||
Bisecting led to commit 2cd548566384 ("net: dsa: qca8k: add support for
 | 
			
		||||
phy read/write with mgmt Ethernet"), which added support to access the
 | 
			
		||||
QCA8337 switch's internal PHYs via management ethernet frames.
 | 
			
		||||
 | 
			
		||||
Connecting the MDIO bus pins onto an oscilloscope, I was able to see
 | 
			
		||||
that the MDIO bus was active whenever a request to read/write an
 | 
			
		||||
internal PHY register was done via an management ethernet frame.
 | 
			
		||||
 | 
			
		||||
My theory is that when the switch core always communicates with the
 | 
			
		||||
internal PHYs via the MDIO bus, even when externally we request the
 | 
			
		||||
access via ethernet. This MDIO bus is the same one via which the switch
 | 
			
		||||
and internal PHYs are accessible to the board, and the board may have
 | 
			
		||||
other devices connected on this bus. An ASCII illustration may give more
 | 
			
		||||
insight:
 | 
			
		||||
 | 
			
		||||
           +---------+
 | 
			
		||||
      +----|         |
 | 
			
		||||
      |    | WAN PHY |
 | 
			
		||||
      | +--|         |
 | 
			
		||||
      | |  +---------+
 | 
			
		||||
      | |
 | 
			
		||||
      | |  +----------------------------------+
 | 
			
		||||
      | |  | QCA8337                          |
 | 
			
		||||
MDC   | |  |                        +-------+ |
 | 
			
		||||
------o-+--|--------o------------o--|       | |
 | 
			
		||||
MDIO    |  |        |            |  | PHY 1 |-|--to RJ45
 | 
			
		||||
--------o--|---o----+---------o--+--|       | |
 | 
			
		||||
           |   |    |         |  |  +-------+ |
 | 
			
		||||
	   | +-------------+  |  o--|       | |
 | 
			
		||||
	   | | MDIO MDC    |  |  |  | PHY 2 |-|--to RJ45
 | 
			
		||||
eth1	   | |             |  o--+--|       | |
 | 
			
		||||
-----------|-|port0        |  |  |  +-------+ |
 | 
			
		||||
           | |             |  |  o--|       | |
 | 
			
		||||
	   | | switch core |  |  |  | PHY 3 |-|--to RJ45
 | 
			
		||||
           | +-------------+  o--+--|       | |
 | 
			
		||||
	   |                  |  |  +-------+ |
 | 
			
		||||
	   |                  |  o--|  ...  | |
 | 
			
		||||
	   +----------------------------------+
 | 
			
		||||
 | 
			
		||||
When we send a request to read an internal PHY register via an ethernet
 | 
			
		||||
management frame via eth1, the switch core receives the ethernet frame
 | 
			
		||||
on port 0 and then communicates with the internal PHY via MDIO. At this
 | 
			
		||||
time, other potential devices, such as the WAN PHY on Turris 1.x, cannot
 | 
			
		||||
use the MDIO bus, since it may cause a bus conflict.
 | 
			
		||||
 | 
			
		||||
Fix this issue by locking the MDIO bus even when we are accessing the
 | 
			
		||||
PHY registers via ethernet management frames.
 | 
			
		||||
 | 
			
		||||
Fixes: 2cd548566384 ("net: dsa: qca8k: add support for phy read/write with mgmt Ethernet")
 | 
			
		||||
Signed-off-by: Marek Behún <kabel@kernel.org>
 | 
			
		||||
Reviewed-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
Signed-off-by: David S. Miller <davem@davemloft.net>
 | 
			
		||||
---
 | 
			
		||||
 drivers/net/dsa/qca/qca8k-8xxx.c | 11 +++++++++++
 | 
			
		||||
 1 file changed, 11 insertions(+)
 | 
			
		||||
 | 
			
		||||
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
@@ -665,6 +665,15 @@ qca8k_phy_eth_command(struct qca8k_priv
 | 
			
		||||
 		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(&priv->bus->mdio_lock);
 | 
			
		||||
+
 | 
			
		||||
 	/* Actually start the request:
 | 
			
		||||
 	 * 1. Send mdio master packet
 | 
			
		||||
 	 * 2. Busy Wait for mdio master command
 | 
			
		||||
@@ -677,6 +686,7 @@ qca8k_phy_eth_command(struct qca8k_priv
 | 
			
		||||
 	mgmt_master = priv->mgmt_master;
 | 
			
		||||
 	if (!mgmt_master) {
 | 
			
		||||
 		mutex_unlock(&mgmt_eth_data->mutex);
 | 
			
		||||
+		mutex_unlock(&priv->bus->mdio_lock);
 | 
			
		||||
 		ret = -EINVAL;
 | 
			
		||||
 		goto err_mgmt_master;
 | 
			
		||||
 	}
 | 
			
		||||
@@ -764,6 +774,7 @@ exit:
 | 
			
		||||
 				    QCA8K_ETHERNET_TIMEOUT);
 | 
			
		||||
 
 | 
			
		||||
 	mutex_unlock(&mgmt_eth_data->mutex);
 | 
			
		||||
+	mutex_unlock(&priv->bus->mdio_lock);
 | 
			
		||||
 
 | 
			
		||||
 	return ret;
 | 
			
		||||
 
 | 
			
		||||
@ -20,7 +20,7 @@ Signed-off-by: David S. Miller <davem@davemloft.net>
 | 
			
		||||
 | 
			
		||||
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
@@ -778,21 +778,6 @@ err_clear_skb:
 | 
			
		||||
@@ -789,21 +789,6 @@ err_clear_skb:
 | 
			
		||||
 	return ret;
 | 
			
		||||
 }
 | 
			
		||||
 
 | 
			
		||||
 | 
			
		||||
@ -71,7 +71,7 @@ Signed-off-by: David S. Miller <davem@davemloft.net>
 | 
			
		||||
 
 | 
			
		||||
 static void
 | 
			
		||||
 qca8k_split_addr(u32 regaddr, u16 *r1, u16 *r2, u16 *page)
 | 
			
		||||
@@ -1829,6 +1830,10 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
@@ -1840,6 +1841,10 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
 	if (ret)
 | 
			
		||||
 		return ret;
 | 
			
		||||
 
 | 
			
		||||
 | 
			
		||||
@ -16,7 +16,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
 | 
			
		||||
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
@@ -1993,6 +1993,8 @@ static const struct dsa_switch_ops qca8k
 | 
			
		||||
@@ -2004,6 +2004,8 @@ static const struct dsa_switch_ops qca8k
 | 
			
		||||
 	.port_fdb_add		= qca8k_port_fdb_add,
 | 
			
		||||
 	.port_fdb_del		= qca8k_port_fdb_del,
 | 
			
		||||
 	.port_fdb_dump		= qca8k_port_fdb_dump,
 | 
			
		||||
 | 
			
		||||
@ -14,7 +14,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
 | 
			
		||||
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
@@ -1882,15 +1882,12 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
@@ -1893,15 +1893,12 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
 		}
 | 
			
		||||
 	}
 | 
			
		||||
 
 | 
			
		||||
 | 
			
		||||
@ -26,7 +26,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
 | 
			
		||||
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
@@ -1719,6 +1719,117 @@ qca8k_get_tag_protocol(struct dsa_switch
 | 
			
		||||
@@ -1730,6 +1730,117 @@ qca8k_get_tag_protocol(struct dsa_switch
 | 
			
		||||
 	return DSA_TAG_PROTO_QCA;
 | 
			
		||||
 }
 | 
			
		||||
 
 | 
			
		||||
@ -144,7 +144,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
 static void
 | 
			
		||||
 qca8k_master_change(struct dsa_switch *ds, const struct net_device *master,
 | 
			
		||||
 		    bool operational)
 | 
			
		||||
@@ -2005,8 +2116,9 @@ static const struct dsa_switch_ops qca8k
 | 
			
		||||
@@ -2016,8 +2127,9 @@ static const struct dsa_switch_ops qca8k
 | 
			
		||||
 	.phylink_mac_link_down	= qca8k_phylink_mac_link_down,
 | 
			
		||||
 	.phylink_mac_link_up	= qca8k_phylink_mac_link_up,
 | 
			
		||||
 	.get_phy_flags		= qca8k_get_phy_flags,
 | 
			
		||||
 | 
			
		||||
@ -20,7 +20,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
 | 
			
		||||
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
 | 
			
		||||
@@ -1991,6 +1991,12 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
@@ -2002,6 +2002,12 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
 			dev_err(priv->dev, "failed enabling QCA header mode on port %d", dp->index);
 | 
			
		||||
 			return ret;
 | 
			
		||||
 		}
 | 
			
		||||
@ -33,7 +33,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
 	}
 | 
			
		||||
 
 | 
			
		||||
 	/* Forward all unknown frames to CPU port for Linux processing */
 | 
			
		||||
@@ -2020,11 +2026,6 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
@@ -2031,11 +2037,6 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
 		if (ret)
 | 
			
		||||
 			return ret;
 | 
			
		||||
 
 | 
			
		||||
@ -45,7 +45,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 | 
			
		||||
 		/* For port based vlans to work we need to set the
 | 
			
		||||
 		 * default egress vid
 | 
			
		||||
 		 */
 | 
			
		||||
@@ -2076,6 +2077,9 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
@@ -2087,6 +2088,9 @@ qca8k_setup(struct dsa_switch *ds)
 | 
			
		||||
 	/* Set max number of LAGs supported */
 | 
			
		||||
 	ds->num_lag_ids = QCA8K_NUM_LAGS;
 | 
			
		||||
 
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user