qca8k_mdio_write() and qca8k_mdio_read() take the common exit path when
selecting the page of QCA8K_MDIO_MASTER_CTRL fails. That path clears
MASTER_CTRL[31:16] with a raw write to phy 0x10, reg 0x1f, which only
addresses MASTER_CTRL while page 0 is selected. After a failed page
select the switch is still on the previous page, so the write can clear
the upper half of an unrelated register at page * 0x200 + 0x3c.
No MDIO master transaction has been started at that point, so there is
nothing to clean up. Unlock and return the error directly.
qca8k_set_page() also keeps the old cached page when the page write
fails. If the write did reach the switch, a later access to the cached
page skips the page select and reaches the wrong register. Invalidate
the cache on failure so that the next access selects the page again.
Fixes: 759bafb8a322 ("net: dsa: qca8k: add support for internal phy and internal mdio")
Fixes: ba5707ec58cf ("net: dsa: qca8k: handle qca8k_set_page errors")
Signed-off-by: Yongzhao Chen <redacted>
Assisted-by: LLM
---
v2: new patch, addressing the Sashiko review of v1:
https://lore.kernel.org/netdev/179054715863.3145.10179961093285192493@kernel.org/ (local)
drivers/net/dsa/qca/qca8k-8xxx.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 0851e4d65b8..00a70c7880e 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -153,6 +153,8 @@ qca8k_set_page(struct qca8k_priv *priv, u16 page)
ret = bus->write(bus, 0x18, 0, page);
if (ret < 0) {
+ /* The switch may or may not have switched pages. */
+ *cached_page = 0xffff;
dev_err_ratelimited(&bus->dev,
"failed to set qca8k page\n");
return ret;@@ -836,7 +838,7 @@ qca8k_mdio_write(struct qca8k_priv *priv, int phy, int regnum, u16 data)
ret = qca8k_set_page(priv, page);
if (ret)
- goto exit;
+ goto unlock;
ret = qca8k_mii_write32(bus, 0x10 | r2, r1, val);
if (ret < 0)
@@ -851,6 +853,7 @@ qca8k_mdio_write(struct qca8k_priv *priv, int phy, int regnum, u16 data)
if (!ret)
ret = ret1;
+unlock:
mutex_unlock(&bus->mdio_lock);
return ret;
@@ -877,7 +880,7 @@ qca8k_mdio_read(struct qca8k_priv *priv, int phy, int regnum)
ret = qca8k_set_page(priv, page);
if (ret)
- goto exit;
+ goto unlock;
ret = qca8k_mii_write_hi(bus, 0x10 | r2, r1 + 1, val);
if (ret < 0)
@@ -896,6 +899,7 @@ qca8k_mdio_read(struct qca8k_priv *priv, int phy, int regnum)
if (!ret)
ret = ret1;
+unlock:
mutex_unlock(&bus->mdio_lock);
if (ret >= 0)
--
2.43.0