[PATCH net-next v2 1/2] net: dsa: qca8k: serialize register access

[Stanislaw Pal]

The regmap is created with .disable_locking, on the ground that
qca8k's read and write callbacks do their own locking. They only lock
the bus transaction, though: regmap formats the register and parses
the value in the map's shared work_buf, outside of them, so two
concurrent users can swap or corrupt each other's register and value.
Most register access runs under rtnl, but not all of it - phylink's
resolve work (mac_link_up/down) and the LED class callbacks do not -
and the next change adds a delayed work that reads the MIB. Let regmap
take its own lock; every user sleeps already, on the MDIO bus or on
the management frame round trip.

An autocast MIB read holds the MIB mutex while its register write
takes the management mutex, and qca8k_conduit_change() takes the two
the other way round. Both run under rtnl today; with a reader outside
rtnl that would be an ABBA deadlock. Take the MIB mutex first there
too.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stanislaw Pal <kuncy7@xxxxxxxxx>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 07227ae8b..9a8b9e34a 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -579,7 +579,6 @@ static const struct regmap_config qca8k_regmap_config = {
 	.write = qca8k_bulk_write,
 	.reg_update_bits = qca8k_regmap_update_bits,
 	.rd_table = &qca8k_readable_table,
-	.disable_locking = true, /* Locking is handled by qca8k read/write */
 	.cache_type = REGCACHE_NONE, /* Explicitly disable CACHE */
 	.max_raw_read = 32, /* mgmt eth can read up to 8 registers at time */
 	/* ATU regs suffer from a bug where some data are not correctly
@@ -1782,13 +1781,16 @@ qca8k_conduit_change(struct dsa_switch *ds, const struct net_device *conduit,
 	if (dp->index != 0)
 		return;
 
-	mutex_lock(&priv->mgmt_eth_data.mutex);
+	/* Same order as an autocast, which holds the MIB mutex while its
+	 * register write takes the management one.
+	 */
 	mutex_lock(&priv->mib_eth_data.mutex);
+	mutex_lock(&priv->mgmt_eth_data.mutex);
 
 	priv->mgmt_conduit = operational ? (struct net_device *)conduit : NULL;
 
-	mutex_unlock(&priv->mib_eth_data.mutex);
 	mutex_unlock(&priv->mgmt_eth_data.mutex);
+	mutex_unlock(&priv->mib_eth_data.mutex);
 }
 
 static int qca8k_connect_tag_protocol(struct dsa_switch *ds,
-- 
2.43.0