-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 <[email protected]>
- Loading branch information
Showing
8 changed files
with
176 additions
and
9 deletions.
There are no files selected for viewing
61 changes: 61 additions & 0 deletions
61
...neric/backport-6.1/781-v6.6-01-net-dsa-qca8k-fix-regmap-bulk-read-write-methods-on-.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
From 5652d1741574eb89cc02576e50ee3e348bd6dd77 Mon Sep 17 00:00:00 2001 | ||
From: =?UTF-8?q?Marek=20Beh=C3=BAn?= <[email protected]> | ||
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 <[email protected]> | ||
Tested-by: Christian Marangi <[email protected]> | ||
Reviewed-by: Christian Marangi <[email protected]> | ||
Signed-off-by: David S. Miller <[email protected]> | ||
--- | ||
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 && |
106 changes: 106 additions & 0 deletions
106
...neric/backport-6.1/781-v6.6-02-net-dsa-qca8k-fix-potential-MDIO-bus-conflict-when-a.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
From 526c8ee04bdbd4d8d19a583b1f3b06700229a815 Mon Sep 17 00:00:00 2001 | ||
From: =?UTF-8?q?Marek=20Beh=C3=BAn?= <[email protected]> | ||
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 <[email protected]> | ||
Reviewed-by: Christian Marangi <[email protected]> | ||
Signed-off-by: David S. Miller <[email protected]> | ||
--- | ||
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; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ Signed-off-by: David S. Miller <[email protected]> | |
|
||
--- 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; | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,7 +71,7 @@ Signed-off-by: David S. Miller <[email protected]> | |
|
||
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; | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ Signed-off-by: Christian Marangi <[email protected]> | |
|
||
--- 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, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ Signed-off-by: Christian Marangi <[email protected]> | |
|
||
--- 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) | ||
} | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ Signed-off-by: Christian Marangi <[email protected]> | |
|
||
--- 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 <[email protected]> | |
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, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ Signed-off-by: Christian Marangi <[email protected]> | |
|
||
--- 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 <[email protected]> | |
} | ||
|
||
/* 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 <[email protected]> | |
/* 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; | ||
|
||
|