| From e4a09620e150ae0511db53d2eafb4cf674d61d1a Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 4 Oct 2023 11:19:04 +0200 |
| Subject: 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 |
| |
| From: Marek BehĂșn <kabel@kernel.org> |
| |
| [ Upstream commit 526c8ee04bdbd4d8d19a583b1f3b06700229a815 ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/dsa/qca/qca8k-8xxx.c | 11 +++++++++++ |
| 1 file changed, 11 insertions(+) |
| |
| diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c |
| index 8e1574c63954e..252089929eb92 100644 |
| --- a/drivers/net/dsa/qca/qca8k-8xxx.c |
| +++ b/drivers/net/dsa/qca/qca8k-8xxx.c |
| @@ -666,6 +666,15 @@ 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(&priv->bus->mdio_lock); |
| + |
| /* Actually start the request: |
| * 1. Send mdio master packet |
| * 2. Busy Wait for mdio master command |
| @@ -678,6 +687,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, |
| 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; |
| } |
| @@ -765,6 +775,7 @@ 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; |
| |
| -- |
| 2.40.1 |
| |