From: "Marek Behún" <kabel@kernel.org>
To: Christian Marangi <ansuelsmth@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org
Cc: "Marek Behún" <kabel@kernel.org>
Subject: [PATCH net v2 2/2] net: dsa: qca8k: fix potential MDIO bus conflict when accessing internal PHYs via management frames
Date: Wed, 4 Oct 2023 11:19:04 +0200 [thread overview]
Message-ID: <20231004091904.16586-3-kabel@kernel.org> (raw)
In-Reply-To: <20231004091904.16586-1-kabel@kernel.org>
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>
---
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 d2df30640269..4ce68e655a63 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.41.0
next prev parent reply other threads:[~2023-10-04 9:19 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-04 9:19 [PATCH net v2 0/2] net: dsa: qca8k: fix qca8k driver for Turris 1.x Marek Behún
2023-10-04 9:19 ` [PATCH net v2 1/2] net: dsa: qca8k: fix regmap bulk read/write methods on big endian systems Marek Behún
2023-10-05 10:48 ` Christian Marangi
2023-10-04 9:19 ` Marek Behún [this message]
2023-10-05 10:49 ` [PATCH net v2 2/2] net: dsa: qca8k: fix potential MDIO bus conflict when accessing internal PHYs via management frames Christian Marangi
2023-10-06 10:50 ` [PATCH net v2 0/2] net: dsa: qca8k: fix qca8k driver for Turris 1.x patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231004091904.16586-3-kabel@kernel.org \
--to=kabel@kernel.org \
--cc=ansuelsmth@gmail.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).