Skip to contentSkip to conversation

net: dsa: mt7530: modernise register access and add two DSA ops

PATCH Superseded Daniel Golle 2 participants 15 messages Jun 13 – Jun 16

Subsystems (5)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Daniel Golle

author maintainer
#0 lore raw

v2 cover net: dsa: mt7530: modernise register access and add two DSA ops

The mt7530 driver carries its own register accessors that predate the regmap conversion and now largely duplicate what regmap already provides, including locking. Most of this series removes that layer.

It first moves the MDIO bus locking into the switch regmap via .lock/.unlock callbacks, matching the PCS regmaps, so any path reaching the regmap is serialised automatically. With the wrappers no longer adding locking, the thin mt7530_mii_* indirection is folded away and the remaining accessors are replaced mechanically with the plain regmap API, using the coccinelle semantic patches included in the commit messages. Open-coded register fields are then converted to FIELD_GET/FIELD_PREP. None of this is intended to change behaviour.

The last two patches implement .port_fast_age, which flushes dynamically learned MAC entries on topology changes, and .port_change_conduit, which moves a user port's CPU-port affinity at runtime.
---
v2:
* fix stray 'static void' left-over in 4/8 which had a fix accidentally folded into 5/8 (byte-identical state at 8/8, but bisectability is restored)
* extend port_change_conduit op commit message

Daniel Golle (8):
net: dsa: mt7530: move MDIO bus locking into regmap
net: dsa: mt7530: fold mt7530_mii_write/read into mt7530_write/read
net: dsa: mt7530: replace mt7530_write with regmap_write
net: dsa: mt7530: replace mt7530_rmw/set/clear with regmap API
net: dsa: mt7530: replace mt7530_read with regmap_read
net: dsa: mt7530: convert to use field accessor macros
net: dsa: mt7530: implement port_fast_age
net: dsa: mt7530: implement port_change_conduit op

 drivers/net/dsa/mt7530-mdio.c |   9 +-
 drivers/net/dsa/mt7530.c      | 791 +++++++++++++++++-----------------
 drivers/net/dsa/mt7530.h      | 209 +++++----
 3 files changed, 517 insertions(+), 492 deletions(-)
Signature · 1 line
2.54.0
author maintainer
#1 lore raw

PATCH v2 1/8 net: dsa: mt7530: move MDIO bus locking into regmap

The switch register regmap was created with .disable_locking = true, relying on callers to manually lock the MDIO bus. Move the locking into the regmap using .lock/.unlock callbacks, matching the PCS regmaps that already do this. This allows any code path reaching the regmap to be automatically protected.

With regmap handling bus locking, the manual mt7530_mutex_lock/unlock wrappers in mt7530_write(), _mt7530_read(), mt7530_rmw() and mt7530_port_change_mtu() become redundant and are removed.

The MT7531 indirect PHY access functions need serialization of their multi-step register sequences, but no longer need to hold bus->mdio_lock across the whole operation. Switch them to reg_mutex.

core_write()/core_rmw() are the only remaining callers of mt7530_mutex_lock(). They access TRGMII core PHY registers via the clause 22 MMD indirect protocol -- a separate register space that bypasses regmap and needs manual bus->mdio_lock protection.

Generated using the following semantic patch:

// Remove mt7530_mutex_lock/unlock around single regmap-based calls. @@
expression priv, reg, val;
@@
{
- mt7530_mutex_lock(priv);
-
mt7530_mii_write(priv, reg, val);
-
- mt7530_mutex_unlock(priv);
}

@@
expression priv, reg, mask, set;
@@
{
- mt7530_mutex_lock(priv);
-
regmap_update_bits(priv->regmap, reg, mask, set);
-
- mt7530_mutex_unlock(priv);
}

@@
expression p;
identifier val;
@@
{
- u32 val;
- mt7530_mutex_lock(p->priv);
- val = mt7530_mii_read(p->priv, p->reg);
- mt7530_mutex_unlock(p->priv);
- return val;
+ return mt7530_mii_read(p->priv, p->reg);
}

@@
expression priv;
@@
- mt7530_mutex_lock(priv);
val = mt7530_mii_read(priv, MT7530_GMACCR);
...
mt7530_mii_write(priv, MT7530_GMACCR, val);
- mt7530_mutex_unlock(priv);

@@
expression priv, port;
@@
INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
- mt7530_mutex_lock(priv);
+ mutex_lock(&priv->reg_mutex);

@@
expression priv;
@@
out:
- mt7530_mutex_unlock(priv);
+ mutex_unlock(&priv->reg_mutex);

---
v2: no changes

 drivers/net/dsa/mt7530-mdio.c |  9 ++++++---
 drivers/net/dsa/mt7530.c      | 38 +++++++++--------------------------
 2 files changed, 15 insertions(+), 32 deletions(-)
drivers/net/dsa/mt7530-mdio.c +6 -3
index 11ea924a9f35..f7c8eeb27211 100644
@@ -141,12 +141,14 @@ static const struct regmap_config regmap_config = {
 	.val_bits = 32,
 	.reg_stride = 4,
 	.max_register = MT7530_CREV,
-	.disable_locking = true,
+	.lock = mt7530_mdio_regmap_lock,
+	.unlock = mt7530_mdio_regmap_unlock,
 };
 
 static int
 mt7530_probe(struct mdio_device *mdiodev)
 {
+	struct regmap_config rc = regmap_config;
 	struct mt7530_priv *priv;
 	struct device_node *dn;
 	int ret;
@@ -200,8 +202,9 @@ mt7530_probe(struct mdio_device *mdiodev)
 			return PTR_ERR(priv->io_pwr);
 	}
 
-	priv->regmap = devm_regmap_init(priv->dev, &mt7530_regmap_bus, priv,
-					&regmap_config);
+	rc.lock_arg = &priv->bus->mdio_lock;
+	priv->regmap = devm_regmap_init(priv->dev, &mt7530_regmap_bus,
+					priv, &rc);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
drivers/net/dsa/mt7530.c +9 -29
index 3c2a3029b10c..5f56a423b147 100644
@@ -184,11 +184,7 @@ mt7530_mii_read(struct mt7530_priv *priv, u32 reg)
 static void
 mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val)
 {
-	mt7530_mutex_lock(priv);
-
 	mt7530_mii_write(priv, reg, val);
-
-	mt7530_mutex_unlock(priv);
 }
 
 static u32
@@ -200,15 +196,7 @@ _mt7530_unlocked_read(struct mt7530_dummy_poll *p)
 static u32
 _mt7530_read(struct mt7530_dummy_poll *p)
 {
-	u32 val;
-
-	mt7530_mutex_lock(p->priv);
-
-	val = mt7530_mii_read(p->priv, p->reg);
-
-	mt7530_mutex_unlock(p->priv);
-
-	return val;
+	return mt7530_mii_read(p->priv, p->reg);
 }
 
 static u32
@@ -224,11 +212,7 @@ static void
 mt7530_rmw(struct mt7530_priv *priv, u32 reg,
 	   u32 mask, u32 set)
 {
-	mt7530_mutex_lock(priv);
-
 	regmap_update_bits(priv->regmap, reg, mask, set);
-
-	mt7530_mutex_unlock(priv);
 }
 
 static void
@@ -555,7 +539,7 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
 
-	mt7530_mutex_lock(priv);
+	mutex_lock(&priv->reg_mutex);
 
 	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -588,7 +572,7 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 	ret = val & MT7531_MDIO_RW_DATA_MASK;
 out:
-	mt7530_mutex_unlock(priv);
+	mutex_unlock(&priv->reg_mutex);
 
 	return ret;
 }
@@ -603,7 +587,7 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
 
-	mt7530_mutex_lock(priv);
+	mutex_lock(&priv->reg_mutex);
 
 	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -635,7 +619,7 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
 	}
 
 out:
-	mt7530_mutex_unlock(priv);
+	mutex_unlock(&priv->reg_mutex);
 
 	return ret;
 }
@@ -649,7 +633,7 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum)
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
 
-	mt7530_mutex_lock(priv);
+	mutex_lock(&priv->reg_mutex);
 
 	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -672,7 +656,7 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum)
 
 	ret = val & MT7531_MDIO_RW_DATA_MASK;
 out:
-	mt7530_mutex_unlock(priv);
+	mutex_unlock(&priv->reg_mutex);
 
 	return ret;
 }
@@ -687,7 +671,7 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);
 
-	mt7530_mutex_lock(priv);
+	mutex_lock(&priv->reg_mutex);
 
 	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, reg,
 				 !(reg & MT7531_PHY_ACS_ST), 20, 100000);
@@ -709,7 +693,7 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
 	}
 
 out:
-	mt7530_mutex_unlock(priv);
+	mutex_unlock(&priv->reg_mutex);
 
 	return ret;
 }
@@ -1444,8 +1428,6 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 	if (!dsa_is_cpu_port(ds, port))
 		return 0;
 
-	mt7530_mutex_lock(priv);
-
 	val = mt7530_mii_read(priv, MT7530_GMACCR);
 	val &= ~MAX_RX_PKT_LEN_MASK;
 
@@ -1465,8 +1447,6 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 
 	mt7530_mii_write(priv, MT7530_GMACCR, val);
 
-	mt7530_mutex_unlock(priv);
-
 	return 0;
 }
 
Signature · 1 line
2.54.0
author maintainer
#2 lore raw

PATCH v2 2/8 net: dsa: mt7530: fold mt7530_mii_write/read into mt7530_write/read

In reply to Daniel Golle

With the lock wrappers removed in the previous commit, mt7530_write() was a trivial wrapper around mt7530_mii_write(), and mt7530_read() around mt7530_mii_read() via _mt7530_read(). Fold the function bodies and eliminate the intermediate functions.

The _mt7530_unlocked_read() and _mt7530_read() poll helpers, which existed as locked/unlocked variants for readx_poll_timeout(), are consolidated into a single mt7530_mii_poll() that calls mt7530_read().

Callers are updated using the following semantic patch:

@@
expression E1, E2, E3;
@@
-mt7530_mii_write(E1, E2, E3)
+mt7530_write(E1, E2, E3)

@@
expression E1, E2;
@@
-mt7530_mii_read(E1, E2)
+mt7530_read(E1, E2)

---
v2: no changes

 drivers/net/dsa/mt7530.c | 78 ++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 51 deletions(-)
drivers/net/dsa/mt7530.c +27 -51
index 5f56a423b147..9ccc848195cf 100644
@@ -150,22 +150,19 @@ core_clear(struct mt7530_priv *priv, u32 reg, u32 val)
 	core_rmw(priv, reg, val, 0);
 }
 
-static int
-mt7530_mii_write(struct mt7530_priv *priv, u32 reg, u32 val)
+static void
+mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val)
 {
 	int ret;
 
 	ret = regmap_write(priv->regmap, reg, val);
-
 	if (ret < 0)
 		dev_err(priv->dev,
 			"failed to write mt7530 register\n");
-
-	return ret;
 }
 
 static u32
-mt7530_mii_read(struct mt7530_priv *priv, u32 reg)
+mt7530_read(struct mt7530_priv *priv, u32 reg)
 {
 	int ret;
 	u32 val;
@@ -181,31 +178,10 @@ mt7530_mii_read(struct mt7530_priv *priv, u32 reg)
 	return val;
 }
 
-static void
-mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val)
-{
-	mt7530_mii_write(priv, reg, val);
-}
-
 static u32
-_mt7530_unlocked_read(struct mt7530_dummy_poll *p)
+mt7530_mii_poll(struct mt7530_dummy_poll *p)
 {
-	return mt7530_mii_read(p->priv, p->reg);
-}
-
-static u32
-_mt7530_read(struct mt7530_dummy_poll *p)
-{
-	return mt7530_mii_read(p->priv, p->reg);
-}
-
-static u32
-mt7530_read(struct mt7530_priv *priv, u32 reg)
-{
-	struct mt7530_dummy_poll p;
-
-	INIT_MT7530_DUMMY_POLL(&p, priv, reg);
-	return _mt7530_read(&p);
+	return mt7530_read(p->priv, p->reg);
 }
 
 static void
@@ -239,7 +215,7 @@ mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 	mt7530_write(priv, MT7530_ATC, val);
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_ATC);
-	ret = readx_poll_timeout(_mt7530_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & ATC_BUSY), 20, 20000);
 	if (ret < 0) {
 		dev_err(priv->dev, "reset timeout\n");
@@ -541,7 +517,7 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 	mutex_lock(&priv->reg_mutex);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -550,9 +526,9 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_ADDR | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad) | regnum;
-	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -561,9 +537,9 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_READ | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad);
-	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -589,7 +565,7 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
 
 	mutex_lock(&priv->reg_mutex);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -598,9 +574,9 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_ADDR | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad) | regnum;
-	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -609,9 +585,9 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_WRITE | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad) | data;
-	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -635,7 +611,7 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum)
 
 	mutex_lock(&priv->reg_mutex);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -645,9 +621,9 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum)
 	val = MT7531_MDIO_CL22_READ | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_REG_ADDR(regnum);
 
-	mt7530_mii_write(priv, MT7531_PHY_IAC, val | MT7531_PHY_ACS_ST);
+	mt7530_write(priv, MT7531_PHY_IAC, val | MT7531_PHY_ACS_ST);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -673,7 +649,7 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
 
 	mutex_lock(&priv->reg_mutex);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, reg,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, reg,
 				 !(reg & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -683,9 +659,9 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
 	reg = MT7531_MDIO_CL22_WRITE | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_REG_ADDR(regnum) | data;
 
-	mt7530_mii_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
-	ret = readx_poll_timeout(_mt7530_unlocked_read, &p, reg,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, reg,
 				 !(reg & MT7531_PHY_ACS_ST), 20, 100000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -1428,7 +1404,7 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 	if (!dsa_is_cpu_port(ds, port))
 		return 0;
 
-	val = mt7530_mii_read(priv, MT7530_GMACCR);
+	val = mt7530_read(priv, MT7530_GMACCR);
 	val &= ~MAX_RX_PKT_LEN_MASK;
 
 	/* RX length also includes Ethernet header, MTK tag, and FCS length */
@@ -1445,7 +1421,7 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 		val |= MAX_RX_PKT_LEN_JUMBO;
 	}
 
-	mt7530_mii_write(priv, MT7530_GMACCR, val);
+	mt7530_write(priv, MT7530_GMACCR, val);
 
 	return 0;
 }
@@ -1614,7 +1590,7 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
 	mt7530_write(priv, MT7530_VTCR, val);
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_VTCR);
-	ret = readx_poll_timeout(_mt7530_read, &p, val,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & VTCR_BUSY), 20, 20000);
 	if (ret < 0) {
 		dev_err(priv->dev, "poll timeout\n");
@@ -2465,7 +2441,7 @@ mt7530_setup(struct dsa_switch *ds)
 
 	/* Waiting for MT7530 got to stable */
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT753X_TRAP);
-	ret = readx_poll_timeout(_mt7530_read, &p, val, val != 0,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val, val != 0,
 				 20, 1000000);
 	if (ret < 0) {
 		dev_err(priv->dev, "reset timeout\n");
@@ -2706,7 +2682,7 @@ mt7531_setup(struct dsa_switch *ds)
 
 	/* Waiting for MT7530 got to stable */
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT753X_TRAP);
-	ret = readx_poll_timeout(_mt7530_read, &p, val, val != 0,
+	ret = readx_poll_timeout(mt7530_mii_poll, &p, val, val != 0,
 				 20, 1000000);
 	if (ret < 0) {
 		dev_err(priv->dev, "reset timeout\n");
Signature · 1 line
2.54.0
author maintainer
#3 lore raw

PATCH v2 3/8 net: dsa: mt7530: replace mt7530_write with regmap_write

In reply to Daniel Golle

Replace all mt7530_write() calls with direct regmap_write() calls and remove the wrapper function. The per-call error logging is dropped -- regmap has its own tracing infrastructure.

Generated using the following semantic patch:

@@
expression priv, reg, val;
@@
-mt7530_write(priv, reg, val)
+regmap_write(priv->regmap, reg, val)

---
v2: no changes

 drivers/net/dsa/mt7530.c | 126 ++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 67 deletions(-)
drivers/net/dsa/mt7530.c +59 -67
index 9ccc848195cf..ce4efcf1b3e6 100644
@@ -150,16 +150,6 @@ core_clear(struct mt7530_priv *priv, u32 reg, u32 val)
 	core_rmw(priv, reg, val, 0);
 }
 
-static void
-mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val)
-{
-	int ret;
-
-	ret = regmap_write(priv->regmap, reg, val);
-	if (ret < 0)
-		dev_err(priv->dev,
-			"failed to write mt7530 register\n");
-}
 
 static u32
 mt7530_read(struct mt7530_priv *priv, u32 reg)
@@ -212,7 +202,7 @@ mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 
 	/* Set the command operating upon the MAC address entries */
 	val = ATC_BUSY | ATC_MAT(0) | cmd;
-	mt7530_write(priv, MT7530_ATC, val);
+	regmap_write(priv->regmap, MT7530_ATC, val);
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_ATC);
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
@@ -288,7 +278,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
 
 	/* Write array into the ARL table */
 	for (i = 0; i < 3; i++)
-		mt7530_write(priv, MT7530_ATA1 + (i * 4), reg[i]);
+		regmap_write(priv->regmap, MT7530_ATA1 + (i * 4), reg[i]);
 }
 
 /* Set up switch core clock for MT7530 */
@@ -406,27 +396,27 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 	/* Step 1 : Disable MT7531 COREPLL */
 	val = mt7530_read(priv, MT7531_PLLGP_EN);
 	val &= ~EN_COREPLL;
-	mt7530_write(priv, MT7531_PLLGP_EN, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 
 	/* Step 2: switch to XTAL output */
 	val = mt7530_read(priv, MT7531_PLLGP_EN);
 	val |= SW_CLKSW;
-	mt7530_write(priv, MT7531_PLLGP_EN, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 
 	val = mt7530_read(priv, MT7531_PLLGP_CR0);
 	val &= ~RG_COREPLL_EN;
-	mt7530_write(priv, MT7531_PLLGP_CR0, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 
 	/* Step 3: disable PLLGP and enable program PLLGP */
 	val = mt7530_read(priv, MT7531_PLLGP_EN);
 	val |= SW_PLLGP;
-	mt7530_write(priv, MT7531_PLLGP_EN, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 
 	/* Step 4: program COREPLL output frequency to 500MHz */
 	val = mt7530_read(priv, MT7531_PLLGP_CR0);
 	val &= ~RG_COREPLL_POSDIV_M;
 	val |= 2 << RG_COREPLL_POSDIV_S;
-	mt7530_write(priv, MT7531_PLLGP_CR0, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 	usleep_range(25, 35);
 
 	switch (xtal) {
@@ -434,42 +424,42 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 		val = mt7530_read(priv, MT7531_PLLGP_CR0);
 		val &= ~RG_COREPLL_SDM_PCW_M;
 		val |= 0x140000 << RG_COREPLL_SDM_PCW_S;
-		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+		regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 		break;
 	case MT7531_XTAL_FSEL_40MHZ:
 		val = mt7530_read(priv, MT7531_PLLGP_CR0);
 		val &= ~RG_COREPLL_SDM_PCW_M;
 		val |= 0x190000 << RG_COREPLL_SDM_PCW_S;
-		mt7530_write(priv, MT7531_PLLGP_CR0, val);
+		regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 		break;
 	}
 
 	/* Set feedback divide ratio update signal to high */
 	val = mt7530_read(priv, MT7531_PLLGP_CR0);
 	val |= RG_COREPLL_SDM_PCW_CHG;
-	mt7530_write(priv, MT7531_PLLGP_CR0, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 	/* Wait for at least 16 XTAL clocks */
 	usleep_range(10, 20);
 
 	/* Step 5: set feedback divide ratio update signal to low */
 	val = mt7530_read(priv, MT7531_PLLGP_CR0);
 	val &= ~RG_COREPLL_SDM_PCW_CHG;
-	mt7530_write(priv, MT7531_PLLGP_CR0, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 
 	/* Enable 325M clock for SGMII */
-	mt7530_write(priv, MT7531_ANA_PLLGP_CR5, 0xad0000);
+	regmap_write(priv->regmap, MT7531_ANA_PLLGP_CR5, 0xad0000);
 
 	/* Enable 250SSC clock for RGMII */
-	mt7530_write(priv, MT7531_ANA_PLLGP_CR2, 0x4f40000);
+	regmap_write(priv->regmap, MT7531_ANA_PLLGP_CR2, 0x4f40000);
 
 	/* Step 6: Enable MT7531 PLL */
 	val = mt7530_read(priv, MT7531_PLLGP_CR0);
 	val |= RG_COREPLL_EN;
-	mt7530_write(priv, MT7531_PLLGP_CR0, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 
 	val = mt7530_read(priv, MT7531_PLLGP_EN);
 	val |= EN_COREPLL;
-	mt7530_write(priv, MT7531_PLLGP_EN, val);
+	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 	usleep_range(25, 35);
 }
 
@@ -478,8 +468,8 @@ mt7530_mib_reset(struct dsa_switch *ds)
 {
 	struct mt7530_priv *priv = ds->priv;
 
-	mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_FLUSH);
-	mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_ACTIVATE);
+	regmap_write(priv->regmap, MT7530_MIB_CCR, CCR_MIB_FLUSH);
+	regmap_write(priv->regmap, MT7530_MIB_CCR, CCR_MIB_ACTIVATE);
 }
 
 static int mt7530_phy_read_c22(struct mt7530_priv *priv, int port, int regnum)
@@ -526,7 +516,7 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_ADDR | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad) | regnum;
-	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	regmap_write(priv->regmap, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -537,7 +527,7 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_READ | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad);
-	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	regmap_write(priv->regmap, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -574,7 +564,7 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_ADDR | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad) | regnum;
-	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	regmap_write(priv->regmap, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -585,7 +575,7 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
 
 	reg = MT7531_MDIO_CL45_WRITE | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_DEV_ADDR(devad) | data;
-	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	regmap_write(priv->regmap, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -621,7 +611,7 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum)
 	val = MT7531_MDIO_CL22_READ | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_REG_ADDR(regnum);
 
-	mt7530_write(priv, MT7531_PHY_IAC, val | MT7531_PHY_ACS_ST);
+	regmap_write(priv->regmap, MT7531_PHY_IAC, val | MT7531_PHY_ACS_ST);
 
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
 				 !(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -659,7 +649,7 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
 	reg = MT7531_MDIO_CL22_WRITE | MT7531_MDIO_PHY_ADDR(port) |
 	      MT7531_MDIO_REG_ADDR(regnum) | data;
 
-	mt7530_write(priv, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
+	regmap_write(priv->regmap, MT7531_PHY_IAC, reg | MT7531_PHY_ACS_ST);
 
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, reg,
 				 !(reg & MT7531_PHY_ACS_ST), 20, 100000);
@@ -1012,7 +1002,8 @@ mt7530_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
 		}
 	}
 
-	mt7530_write(priv, MT7530_AAC, AGE_CNT(age_count) | AGE_UNIT(age_unit));
+	regmap_write(priv->regmap, MT7530_AAC,
+		     AGE_CNT(age_count) | AGE_UNIT(age_unit));
 
 	return 0;
 }
@@ -1050,7 +1041,7 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
 	/* MUX_PHY_P4: P4 -> P5 -> SoC MAC */
 	case MUX_PHY_P4:
 		/* Setup the MAC by default for the cpu port */
-		mt7530_write(priv, MT753X_PMCR_P(5), 0x56300);
+		regmap_write(priv->regmap, MT753X_PMCR_P(5), 0x56300);
 		break;
 
 	/* GMAC5: P5 -> SoC MAC or external PHY */
@@ -1064,7 +1055,8 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
 		val |= MT7530_P5_RGMII_MODE;
 
 		/* P5 RGMII RX Clock Control: delay setting for 1000M */
-		mt7530_write(priv, MT7530_P5RGMIIRXCR, CSR_RGMII_EDGE_ALIGN);
+		regmap_write(priv->regmap, MT7530_P5RGMIIRXCR,
+			     CSR_RGMII_EDGE_ALIGN);
 
 		/* Don't set delay in DSA mode */
 		if (!dsa_is_dsa_port(priv->ds, 5) &&
@@ -1073,15 +1065,15 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
 			tx_delay = 4; /* n * 0.5 ns */
 
 		/* P5 RGMII TX Clock Control: delay x */
-		mt7530_write(priv, MT7530_P5RGMIITXCR,
+		regmap_write(priv->regmap, MT7530_P5RGMIITXCR,
 			     CSR_RGMII_TXC_CFG(0x10 + tx_delay));
 
 		/* reduce P5 RGMII Tx driving, 8mA */
-		mt7530_write(priv, MT7530_IO_DRV_CR,
+		regmap_write(priv->regmap, MT7530_IO_DRV_CR,
 			     P5_IO_CLK_DRV(1) | P5_IO_DATA_DRV(1));
 	}
 
-	mt7530_write(priv, MT753X_MTRAP, val);
+	regmap_write(priv->regmap, MT753X_MTRAP, val);
 
 	dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, mode=%s, phy-mode=%s\n", val,
 		mt7530_p5_mode_str(priv->p5_mode), phy_modes(interface));
@@ -1303,8 +1295,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 	struct mt7530_priv *priv = ds->priv;
 
 	/* Enable Mediatek header mode on the cpu port */
-	mt7530_write(priv, MT7530_PVC_P(port),
-		     PORT_SPEC_TAG);
+	regmap_write(priv->regmap, MT7530_PVC_P(port), PORT_SPEC_TAG);
 
 	/* Enable flooding on the CPU port */
 	mt7530_set(priv, MT753X_MFC, BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) |
@@ -1321,7 +1312,7 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 	/* CPU port gets connected to all user ports of
 	 * the switch.
 	 */
-	mt7530_write(priv, MT7530_PCR_P(port),
+	regmap_write(priv->regmap, MT7530_PCR_P(port),
 		     PCR_MATRIX(dsa_user_ports(priv->ds)));
 
 	/* Set to fallback mode for independent VLAN learning */
@@ -1421,7 +1412,7 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 		val |= MAX_RX_PKT_LEN_JUMBO;
 	}
 
-	mt7530_write(priv, MT7530_GMACCR, val);
+	regmap_write(priv->regmap, MT7530_GMACCR, val);
 
 	return 0;
 }
@@ -1587,7 +1578,7 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
 	int ret;
 
 	val = VTCR_BUSY | VTCR_FUNC(cmd) | vid;
-	mt7530_write(priv, MT7530_VTCR, val);
+	regmap_write(priv->regmap, MT7530_VTCR, val);
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_VTCR);
 	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
@@ -1616,8 +1607,8 @@ mt7530_setup_vlan0(struct mt7530_priv *priv)
 	 */
 	val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) |
 	      VLAN_VALID;
-	mt7530_write(priv, MT7530_VAWD1, val);
-	mt7530_write(priv, MT7530_VAWD2, 0);
+	regmap_write(priv->regmap, MT7530_VAWD1, val);
+	regmap_write(priv->regmap, MT7530_VAWD2, 0);
 
 	return mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, 0);
 }
@@ -1887,7 +1878,7 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
 	 */
 	val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | FID(FID_BRIDGED) |
 	      VLAN_VALID;
-	mt7530_write(priv, MT7530_VAWD1, val);
+	regmap_write(priv->regmap, MT7530_VAWD1, val);
 
 	/* Decide whether adding tag or not for those outgoing packets from the
 	 * port inside the VLAN.
@@ -1926,10 +1917,10 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
 	if (new_members) {
 		val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
 		      VLAN_VALID;
-		mt7530_write(priv, MT7530_VAWD1, val);
+		regmap_write(priv->regmap, MT7530_VAWD1, val);
 	} else {
-		mt7530_write(priv, MT7530_VAWD1, 0);
-		mt7530_write(priv, MT7530_VAWD2, 0);
+		regmap_write(priv->regmap, MT7530_VAWD1, 0);
+		regmap_write(priv->regmap, MT7530_VAWD2, 0);
 	}
 }
 
@@ -2070,7 +2061,7 @@ static int mt753x_port_mirror_add(struct dsa_switch *ds, int port,
 	val |= MT753X_MIRROR_EN(priv->id);
 	val &= ~MT753X_MIRROR_PORT_MASK(priv->id);
 	val |= MT753X_MIRROR_PORT_SET(priv->id, mirror->to_local_port);
-	mt7530_write(priv, MT753X_MIRROR_REG(priv->id), val);
+	regmap_write(priv->regmap, MT753X_MIRROR_REG(priv->id), val);
 
 	val = mt7530_read(priv, MT7530_PCR_P(port));
 	if (ingress) {
@@ -2080,7 +2071,7 @@ static int mt753x_port_mirror_add(struct dsa_switch *ds, int port,
 		val |= PORT_TX_MIR;
 		priv->mirror_tx |= BIT(port);
 	}
-	mt7530_write(priv, MT7530_PCR_P(port), val);
+	regmap_write(priv->regmap, MT7530_PCR_P(port), val);
 
 	return 0;
 }
@@ -2099,12 +2090,12 @@ static void mt753x_port_mirror_del(struct dsa_switch *ds, int port,
 		val &= ~PORT_TX_MIR;
 		priv->mirror_tx &= ~BIT(port);
 	}
-	mt7530_write(priv, MT7530_PCR_P(port), val);
+	regmap_write(priv->regmap, MT7530_PCR_P(port), val);
 
 	if (!priv->mirror_rx && !priv->mirror_tx) {
 		val = mt7530_read(priv, MT753X_MIRROR_REG(priv->id));
 		val &= ~MT753X_MIRROR_EN(priv->id);
-		mt7530_write(priv, MT753X_MIRROR_REG(priv->id), val);
+		regmap_write(priv->regmap, MT753X_MIRROR_REG(priv->id), val);
 	}
 }
 
@@ -2202,9 +2193,9 @@ mt7530_setup_gpio(struct mt7530_priv *priv)
 	if (!gc)
 		return -ENOMEM;
 
-	mt7530_write(priv, MT7530_LED_GPIO_OE, 0);
-	mt7530_write(priv, MT7530_LED_GPIO_DIR, 0);
-	mt7530_write(priv, MT7530_LED_IO_MODE, 0);
+	regmap_write(priv->regmap, MT7530_LED_GPIO_OE, 0);
+	regmap_write(priv->regmap, MT7530_LED_GPIO_DIR, 0);
+	regmap_write(priv->regmap, MT7530_LED_IO_MODE, 0);
 
 	gc->label = "mt7530";
 	gc->parent = dev;
@@ -2462,13 +2453,12 @@ mt7530_setup(struct dsa_switch *ds)
 	}
 
 	/* Reset the switch through internal reset */
-	mt7530_write(priv, MT7530_SYS_CTRL,
-		     SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
-		     SYS_CTRL_REG_RST);
+	regmap_write(priv->regmap, MT7530_SYS_CTRL,
+		     SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST | SYS_CTRL_REG_RST);
 
 	/* Lower Tx driving for TRGMII path */
 	for (i = 0; i < NUM_TRGMII_CTRL; i++)
-		mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
+		regmap_write(priv->regmap, MT7530_TRGMII_TD_ODT(i),
 			     TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
 	for (i = 0; i < NUM_TRGMII_CTRL; i++)
@@ -2647,7 +2637,7 @@ mt7531_setup_common(struct dsa_switch *ds)
 
 	/* Enable Special Tag for rx frames */
 	if (priv->id == ID_EN7581 || priv->id == ID_AN7583)
-		mt7530_write(priv, MT753X_CPORT_SPTAG_CFG,
+		regmap_write(priv->regmap, MT753X_CPORT_SPTAG_CFG,
 			     CPORT_SW2FE_STAG_EN | CPORT_FE2SW_STAG_EN);
 
 	/* Flush the FDB table */
@@ -2705,10 +2695,12 @@ mt7531_setup(struct dsa_switch *ds)
 
 	/* Force link down on all ports before internal reset */
 	for (i = 0; i < priv->ds->num_ports; i++)
-		mt7530_write(priv, MT753X_PMCR_P(i), MT7531_FORCE_MODE_LNK);
+		regmap_write(priv->regmap, MT753X_PMCR_P(i),
+			     MT7531_FORCE_MODE_LNK);
 
 	/* Reset the switch through internal reset */
-	mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_SW_RST | SYS_CTRL_REG_RST);
+	regmap_write(priv->regmap, MT7530_SYS_CTRL,
+		     SYS_CTRL_SW_RST | SYS_CTRL_REG_RST);
 
 	if (!priv->p5_sgmii) {
 		mt7531_pll_setup(priv);
@@ -2917,7 +2909,7 @@ static void mt7531_rgmii_setup(struct mt7530_priv *priv,
 		}
 	}
 
-	mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
+	regmap_write(priv->regmap, MT7531_CLKGEN_CTRL, val);
 }
 
 static void
@@ -3254,7 +3246,7 @@ static int mt753x_tc_setup_qdisc_tbf(struct dsa_switch *ds, int port,
 		      FIELD_PREP(ERLCR_EXP_MASK, tick) |
 		      ERLCR_TBF_MODE_MASK |
 		      FIELD_PREP(ERLCR_MANT_MASK, 0xf);
-		mt7530_write(priv, MT753X_ERLCR_P(port), val);
+		regmap_write(priv->regmap, MT753X_ERLCR_P(port), val);
 		break;
 	}
 	default:
@@ -3296,7 +3288,7 @@ static int mt7988_setup(struct dsa_switch *ds)
 			   FIELD_PREP(AN7583_CSR_ETHER_AFE_PWD, 0));
 
 	/* Reset the switch PHYs */
-	mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
+	regmap_write(priv->regmap, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
 
 	return mt7531_setup_common(ds);
 }
Signature · 1 line
2.54.0
author maintainer
#4 lore raw

PATCH v2 4/8 net: dsa: mt7530: replace mt7530_rmw/set/clear with regmap API

In reply to Daniel Golle

Replace all mt7530_rmw() calls with regmap_update_bits(), mt7530_set() with regmap_set_bits(), and mt7530_clear() with regmap_clear_bits(). Remove the wrapper function definitions.

Generated using the following semantic patch:

@@
expression priv, reg, mask, set;
@@
-mt7530_rmw(priv, reg, mask, set)
+regmap_update_bits(priv->regmap, reg, mask, set)

@@
expression priv, reg, val;
@@
-mt7530_set(priv, reg, val)
+regmap_set_bits(priv->regmap, reg, val)

@@
expression priv, reg, val;
@@
-mt7530_clear(priv, reg, val)
+regmap_clear_bits(priv->regmap, reg, val)

---
v2: remove stray 'static void' leftover

 drivers/net/dsa/mt7530.c | 359 ++++++++++++++++++++-------------------
 1 file changed, 182 insertions(+), 177 deletions(-)
drivers/net/dsa/mt7530.c +182 -177
index ce4efcf1b3e6..fe7e4ab5ae9c 100644
@@ -174,25 +174,6 @@ mt7530_mii_poll(struct mt7530_dummy_poll *p)
 	return mt7530_read(p->priv, p->reg);
 }
 
-static void
-mt7530_rmw(struct mt7530_priv *priv, u32 reg,
-	   u32 mask, u32 set)
-{
-	regmap_update_bits(priv->regmap, reg, mask, set);
-}
-
-static void
-mt7530_set(struct mt7530_priv *priv, u32 reg, u32 val)
-{
-	mt7530_rmw(priv, reg, val, val);
-}
-
-static void
-mt7530_clear(struct mt7530_priv *priv, u32 reg, u32 val)
-{
-	mt7530_rmw(priv, reg, val, 0);
-}
-
 static int
 mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 {
@@ -332,12 +313,13 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
 	core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
 
 	if (interface == PHY_INTERFACE_MODE_RGMII) {
-		mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
-			   P6_INTF_MODE(0));
+		regmap_update_bits(priv->regmap, MT7530_P6ECR,
+				   P6_INTF_MODE_MASK, P6_INTF_MODE(0));
 		return;
 	}
 
-	mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK, P6_INTF_MODE(1));
+	regmap_update_bits(priv->regmap, MT7530_P6ECR, P6_INTF_MODE_MASK,
+			   P6_INTF_MODE(1));
 
 	xtal = mt7530_read(priv, MT753X_MTRAP) & MT7530_XTAL_MASK;
 
@@ -1258,35 +1240,35 @@ mt753x_trap_frames(struct mt7530_priv *priv)
 	 * switch egress VLAN tag processing. This preserves VLAN tags
 	 * for reception on VLAN sub-interfaces.
 	 */
-	mt7530_rmw(priv, MT753X_BPC,
-		   PAE_BPDU_FR | PAE_EG_TAG_MASK | PAE_PORT_FW_MASK |
-			   BPDU_EG_TAG_MASK | BPDU_PORT_FW_MASK,
-		   PAE_BPDU_FR | PAE_EG_TAG(MT7530_VLAN_EG_DISABLED) |
-			   PAE_PORT_FW(TO_CPU_FW_CPU_ONLY) |
-			   BPDU_EG_TAG(MT7530_VLAN_EG_DISABLED) |
-			   TO_CPU_FW_CPU_ONLY);
+	regmap_update_bits(priv->regmap, MT753X_BPC,
+			   PAE_BPDU_FR | PAE_EG_TAG_MASK | PAE_PORT_FW_MASK |
+				   BPDU_EG_TAG_MASK | BPDU_PORT_FW_MASK,
+			   PAE_BPDU_FR | PAE_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+				   PAE_PORT_FW(TO_CPU_FW_CPU_ONLY) |
+				   BPDU_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+				   TO_CPU_FW_CPU_ONLY);
 
 	/* Trap frames with :01 and :02 MAC DAs to the CPU port(s) and
 	 * egress them with EG_TAG disabled.
 	 */
-	mt7530_rmw(priv, MT753X_RGAC1,
-		   R02_BPDU_FR | R02_EG_TAG_MASK | R02_PORT_FW_MASK |
-			   R01_BPDU_FR | R01_EG_TAG_MASK | R01_PORT_FW_MASK,
-		   R02_BPDU_FR | R02_EG_TAG(MT7530_VLAN_EG_DISABLED) |
-			   R02_PORT_FW(TO_CPU_FW_CPU_ONLY) | R01_BPDU_FR |
-			   R01_EG_TAG(MT7530_VLAN_EG_DISABLED) |
-			   TO_CPU_FW_CPU_ONLY);
+	regmap_update_bits(priv->regmap, MT753X_RGAC1,
+			   R02_BPDU_FR | R02_EG_TAG_MASK | R02_PORT_FW_MASK |
+				   R01_BPDU_FR | R01_EG_TAG_MASK | R01_PORT_FW_MASK,
+			   R02_BPDU_FR | R02_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+				   R02_PORT_FW(TO_CPU_FW_CPU_ONLY) | R01_BPDU_FR |
+				   R01_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+				   TO_CPU_FW_CPU_ONLY);
 
 	/* Trap frames with :03 and :0E MAC DAs to the CPU port(s) and
 	 * egress them with EG_TAG disabled.
 	 */
-	mt7530_rmw(priv, MT753X_RGAC2,
-		   R0E_BPDU_FR | R0E_EG_TAG_MASK | R0E_PORT_FW_MASK |
-			   R03_BPDU_FR | R03_EG_TAG_MASK | R03_PORT_FW_MASK,
-		   R0E_BPDU_FR | R0E_EG_TAG(MT7530_VLAN_EG_DISABLED) |
-			   R0E_PORT_FW(TO_CPU_FW_CPU_ONLY) | R03_BPDU_FR |
-			   R03_EG_TAG(MT7530_VLAN_EG_DISABLED) |
-			   TO_CPU_FW_CPU_ONLY);
+	regmap_update_bits(priv->regmap, MT753X_RGAC2,
+			   R0E_BPDU_FR | R0E_EG_TAG_MASK | R0E_PORT_FW_MASK |
+				   R03_BPDU_FR | R03_EG_TAG_MASK | R03_PORT_FW_MASK,
+			   R0E_BPDU_FR | R0E_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+				   R0E_PORT_FW(TO_CPU_FW_CPU_ONLY) | R03_BPDU_FR |
+				   R03_EG_TAG(MT7530_VLAN_EG_DISABLED) |
+				   TO_CPU_FW_CPU_ONLY);
 }
 
 static void
@@ -1298,8 +1280,8 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 	regmap_write(priv->regmap, MT7530_PVC_P(port), PORT_SPEC_TAG);
 
 	/* Enable flooding on the CPU port */
-	mt7530_set(priv, MT753X_MFC, BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) |
-		   UNU_FFP(BIT(port)));
+	regmap_set_bits(priv->regmap, MT753X_MFC,
+			BC_FFP(BIT(port)) | UNM_FFP(BIT(port)) | UNU_FFP(BIT(port)));
 
 	/* Add the CPU port to the CPU port bitmap for MT7531 and the switch on
 	 * the MT7988 SoC. Trapped frames will be forwarded to the CPU port that
@@ -1307,7 +1289,8 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 	 */
 	if (priv->id == ID_MT7531 || priv->id == ID_MT7988 ||
 	    priv->id == ID_EN7581 || priv->id == ID_AN7583)
-		mt7530_set(priv, MT7531_CFC, MT7531_CPU_PMAP(BIT(port)));
+		regmap_set_bits(priv->regmap, MT7531_CFC,
+				MT7531_CPU_PMAP(BIT(port)));
 
 	/* CPU port gets connected to all user ports of
 	 * the switch.
@@ -1316,8 +1299,8 @@ mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 		     PCR_MATRIX(dsa_user_ports(priv->ds)));
 
 	/* Set to fallback mode for independent VLAN learning */
-	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
-		   MT7530_PORT_FALLBACK_MODE);
+	regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+			   PCR_PORT_VLAN_MASK, MT7530_PORT_FALLBACK_MODE);
 }
 
 static int
@@ -1339,8 +1322,8 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
 		priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));
 	}
 	priv->ports[port].enable = true;
-	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
-		   priv->ports[port].pm);
+	regmap_update_bits(priv->regmap, MT7530_PCR_P(port), PCR_MATRIX_MASK,
+			   priv->ports[port].pm);
 
 	mutex_unlock(&priv->reg_mutex);
 
@@ -1348,9 +1331,9 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
 		return 0;
 
 	if (port == 5)
-		mt7530_clear(priv, MT753X_MTRAP, MT7530_P5_DIS);
+		regmap_clear_bits(priv->regmap, MT753X_MTRAP, MT7530_P5_DIS);
 	else if (port == 6)
-		mt7530_clear(priv, MT753X_MTRAP, MT7530_P6_DIS);
+		regmap_clear_bits(priv->regmap, MT753X_MTRAP, MT7530_P6_DIS);
 
 	return 0;
 }
@@ -1366,8 +1349,8 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
 	 * enablement for the port.
 	 */
 	priv->ports[port].enable = false;
-	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK,
-		   PCR_MATRIX_CLR);
+	regmap_update_bits(priv->regmap, MT7530_PCR_P(port), PCR_MATRIX_MASK,
+			   PCR_MATRIX_CLR);
 
 	mutex_unlock(&priv->reg_mutex);
 
@@ -1376,9 +1359,9 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
 
 	/* Do not set MT7530_P5_DIS when port 5 is being used for PHY muxing. */
 	if (port == 5 && priv->p5_mode == GMAC5)
-		mt7530_set(priv, MT753X_MTRAP, MT7530_P5_DIS);
+		regmap_set_bits(priv->regmap, MT753X_MTRAP, MT7530_P5_DIS);
 	else if (port == 6)
-		mt7530_set(priv, MT753X_MTRAP, MT7530_P6_DIS);
+		regmap_set_bits(priv->regmap, MT753X_MTRAP, MT7530_P6_DIS);
 }
 
 static int
@@ -1448,8 +1431,9 @@ mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 		break;
 	}
 
-	mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK(FID_BRIDGED),
-		   FID_PST(FID_BRIDGED, stp_state));
+	regmap_update_bits(priv->regmap, MT7530_SSP_P(port),
+			   FID_PST_MASK(FID_BRIDGED),
+			   FID_PST(FID_BRIDGED, stp_state));
 }
 
 static void mt7530_update_port_member(struct mt7530_priv *priv, int port,
@@ -1488,8 +1472,9 @@ static void mt7530_update_port_member(struct mt7530_priv *priv, int port,
 		}
 
 		if (other_p->enable)
-			mt7530_rmw(priv, MT7530_PCR_P(other_port),
-				   PCR_MATRIX_MASK, other_p->pm);
+			regmap_update_bits(priv->regmap,
+					   MT7530_PCR_P(other_port),
+					   PCR_MATRIX_MASK, other_p->pm);
 	}
 
 	/* Add/remove the all other ports to this port matrix. For !join
@@ -1498,7 +1483,8 @@ static void mt7530_update_port_member(struct mt7530_priv *priv, int port,
 	 */
 	p->pm = PCR_MATRIX(port_bitmap);
 	if (priv->ports[port].enable)
-		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK, p->pm);
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+				   PCR_MATRIX_MASK, p->pm);
 }
 
 static int
@@ -1521,20 +1507,23 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
 	struct mt7530_priv *priv = ds->priv;
 
 	if (flags.mask & BR_LEARNING)
-		mt7530_rmw(priv, MT7530_PSC_P(port), SA_DIS,
-			   flags.val & BR_LEARNING ? 0 : SA_DIS);
+		regmap_update_bits(priv->regmap, MT7530_PSC_P(port), SA_DIS,
+				   flags.val & BR_LEARNING ? 0 : SA_DIS);
 
 	if (flags.mask & BR_FLOOD)
-		mt7530_rmw(priv, MT753X_MFC, UNU_FFP(BIT(port)),
-			   flags.val & BR_FLOOD ? UNU_FFP(BIT(port)) : 0);
+		regmap_update_bits(priv->regmap, MT753X_MFC,
+				   UNU_FFP(BIT(port)),
+				   flags.val & BR_FLOOD ? UNU_FFP(BIT(port)) : 0);
 
 	if (flags.mask & BR_MCAST_FLOOD)
-		mt7530_rmw(priv, MT753X_MFC, UNM_FFP(BIT(port)),
-			   flags.val & BR_MCAST_FLOOD ? UNM_FFP(BIT(port)) : 0);
+		regmap_update_bits(priv->regmap, MT753X_MFC,
+				   UNM_FFP(BIT(port)),
+				   flags.val & BR_MCAST_FLOOD ? UNM_FFP(BIT(port)) : 0);
 
 	if (flags.mask & BR_BCAST_FLOOD)
-		mt7530_rmw(priv, MT753X_MFC, BC_FFP(BIT(port)),
-			   flags.val & BR_BCAST_FLOOD ? BC_FFP(BIT(port)) : 0);
+		regmap_update_bits(priv->regmap, MT753X_MFC,
+				   BC_FFP(BIT(port)),
+				   flags.val & BR_BCAST_FLOOD ? BC_FFP(BIT(port)) : 0);
 
 	if (flags.mask & BR_ISOLATED) {
 		struct dsa_port *dp = dsa_to_port(ds, port);
@@ -1562,8 +1551,8 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 	mt7530_update_port_member(priv, port, bridge.dev, true);
 
 	/* Set to fallback mode for independent VLAN learning */
-	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
-		   MT7530_PORT_FALLBACK_MODE);
+	regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+			   PCR_PORT_VLAN_MASK, MT7530_PORT_FALLBACK_MODE);
 
 	mutex_unlock(&priv->reg_mutex);
 
@@ -1624,18 +1613,19 @@ mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
 	 * bridge. Don't set standalone ports to fallback mode.
 	 */
 	if (dsa_port_bridge_dev_get(dsa_to_port(ds, port)))
-		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
-			   MT7530_PORT_FALLBACK_MODE);
-
-	mt7530_rmw(priv, MT7530_PVC_P(port),
-		   VLAN_ATTR_MASK | PVC_EG_TAG_MASK | ACC_FRM_MASK,
-		   VLAN_ATTR(MT7530_VLAN_TRANSPARENT) |
-		   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
-		   MT7530_VLAN_ACC_ALL);
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+				   PCR_PORT_VLAN_MASK,
+				   MT7530_PORT_FALLBACK_MODE);
+
+	regmap_update_bits(priv->regmap, MT7530_PVC_P(port),
+			   VLAN_ATTR_MASK | PVC_EG_TAG_MASK | ACC_FRM_MASK,
+			   VLAN_ATTR(MT7530_VLAN_TRANSPARENT) |
+			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT) |
+			   MT7530_VLAN_ACC_ALL);
 
 	/* Set PVID to 0 */
-	mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK,
-		   G0_PORT_VID_DEF);
+	regmap_update_bits(priv->regmap, MT7530_PPBV1_P(port),
+			   G0_PORT_VID_MASK, G0_PORT_VID_DEF);
 
 	for (i = 0; i < priv->ds->num_ports; i++) {
 		if (i == port)
@@ -1666,24 +1656,27 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
 	 * table lookup.
 	 */
 	if (dsa_is_user_port(ds, port)) {
-		mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
-			   MT7530_PORT_SECURITY_MODE);
-		mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK,
-			   G0_PORT_VID(priv->ports[port].pvid));
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+				   PCR_PORT_VLAN_MASK,
+				   MT7530_PORT_SECURITY_MODE);
+		regmap_update_bits(priv->regmap, MT7530_PPBV1_P(port),
+				   G0_PORT_VID_MASK,
+				   G0_PORT_VID(priv->ports[port].pvid));
 
 		/* Only accept tagged frames if PVID is not set */
 		if (!priv->ports[port].pvid)
-			mt7530_rmw(priv, MT7530_PVC_P(port), ACC_FRM_MASK,
-				   MT7530_VLAN_ACC_TAGGED);
+			regmap_update_bits(priv->regmap, MT7530_PVC_P(port),
+					   ACC_FRM_MASK,
+					   MT7530_VLAN_ACC_TAGGED);
 
 		/* Set the port as a user port which is to be able to recognize
 		 * VID from incoming packets before fetching entry within the
 		 * VLAN table.
 		 */
-		mt7530_rmw(priv, MT7530_PVC_P(port),
-			   VLAN_ATTR_MASK | PVC_EG_TAG_MASK,
-			   VLAN_ATTR(MT7530_VLAN_USER) |
-			   PVC_EG_TAG(MT7530_VLAN_EG_DISABLED));
+		regmap_update_bits(priv->regmap, MT7530_PVC_P(port),
+				   VLAN_ATTR_MASK | PVC_EG_TAG_MASK,
+				   VLAN_ATTR(MT7530_VLAN_USER) |
+				   PVC_EG_TAG(MT7530_VLAN_EG_DISABLED));
 	} else {
 		/* Also set CPU ports to the "user" VLAN port attribute, to
 		 * allow VLAN classification, but keep the EG_TAG attribute as
@@ -1692,8 +1685,9 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
 		 * are forwarded to user ports as tagged, and untagged as
 		 * untagged.
 		 */
-		mt7530_rmw(priv, MT7530_PVC_P(port), VLAN_ATTR_MASK,
-			   VLAN_ATTR(MT7530_VLAN_USER));
+		regmap_update_bits(priv->regmap, MT7530_PVC_P(port),
+				   VLAN_ATTR_MASK,
+				   VLAN_ATTR(MT7530_VLAN_USER));
 	}
 }
 
@@ -1711,8 +1705,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 	 * back to the default as is at initial boot which is a VLAN-unaware
 	 * port.
 	 */
-	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
-		   MT7530_PORT_MATRIX_MODE);
+	regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+			   PCR_PORT_VLAN_MASK, MT7530_PORT_MATRIX_MODE);
 
 	mutex_unlock(&priv->reg_mutex);
 }
@@ -1893,9 +1887,9 @@ mt7530_hw_vlan_add(struct mt7530_priv *priv,
 		val = MT7530_VLAN_EGRESS_UNTAG;
 	else
 		val = MT7530_VLAN_EGRESS_TAG;
-	mt7530_rmw(priv, MT7530_VAWD2,
-		   ETAG_CTRL_P_MASK(entry->port),
-		   ETAG_CTRL_P(entry->port, val));
+	regmap_update_bits(priv->regmap, MT7530_VAWD2,
+			   ETAG_CTRL_P_MASK(entry->port),
+			   ETAG_CTRL_P(entry->port, val));
 }
 
 static void
@@ -1973,25 +1967,26 @@ mt7530_port_vlan_add(struct dsa_switch *ds, int port,
 		priv->ports[port].pvid = vlan->vid;
 
 		/* Accept all frames if PVID is set */
-		mt7530_rmw(priv, MT7530_PVC_P(port), ACC_FRM_MASK,
-			   MT7530_VLAN_ACC_ALL);
+		regmap_update_bits(priv->regmap, MT7530_PVC_P(port),
+				   ACC_FRM_MASK, MT7530_VLAN_ACC_ALL);
 
 		/* Only configure PVID if VLAN filtering is enabled */
 		if (dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
-			mt7530_rmw(priv, MT7530_PPBV1_P(port),
-				   G0_PORT_VID_MASK,
-				   G0_PORT_VID(vlan->vid));
+			regmap_update_bits(priv->regmap, MT7530_PPBV1_P(port),
+					   G0_PORT_VID_MASK,
+					   G0_PORT_VID(vlan->vid));
 	} else if (vlan->vid && priv->ports[port].pvid == vlan->vid) {
 		/* This VLAN is overwritten without PVID, so unset it */
 		priv->ports[port].pvid = G0_PORT_VID_DEF;
 
 		/* Only accept tagged frames if the port is VLAN-aware */
 		if (dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
-			mt7530_rmw(priv, MT7530_PVC_P(port), ACC_FRM_MASK,
-				   MT7530_VLAN_ACC_TAGGED);
+			regmap_update_bits(priv->regmap, MT7530_PVC_P(port),
+					   ACC_FRM_MASK,
+					   MT7530_VLAN_ACC_TAGGED);
 
-		mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK,
-			   G0_PORT_VID_DEF);
+		regmap_update_bits(priv->regmap, MT7530_PPBV1_P(port),
+				   G0_PORT_VID_MASK, G0_PORT_VID_DEF);
 	}
 
 	mutex_unlock(&priv->reg_mutex);
@@ -2025,11 +2020,12 @@ mt7530_port_vlan_del(struct dsa_switch *ds, int port,
 
 		/* Only accept tagged frames if the port is VLAN-aware */
 		if (dsa_port_is_vlan_filtering(dsa_to_port(ds, port)))
-			mt7530_rmw(priv, MT7530_PVC_P(port), ACC_FRM_MASK,
-				   MT7530_VLAN_ACC_TAGGED);
+			regmap_update_bits(priv->regmap, MT7530_PVC_P(port),
+					   ACC_FRM_MASK,
+					   MT7530_VLAN_ACC_TAGGED);
 
-		mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK,
-			   G0_PORT_VID_DEF);
+		regmap_update_bits(priv->regmap, MT7530_PPBV1_P(port),
+				   G0_PORT_VID_MASK, G0_PORT_VID_DEF);
 	}
 
 
@@ -2136,9 +2132,9 @@ mt7530_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
 	u32 bit = mt7530_gpio_to_bit(offset);
 
 	if (value)
-		mt7530_set(priv, MT7530_LED_GPIO_DATA, bit);
+		regmap_set_bits(priv->regmap, MT7530_LED_GPIO_DATA, bit);
 	else
-		mt7530_clear(priv, MT7530_LED_GPIO_DATA, bit);
+		regmap_clear_bits(priv->regmap, MT7530_LED_GPIO_DATA, bit);
 
 	return 0;
 }
@@ -2159,8 +2155,8 @@ mt7530_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
 	struct mt7530_priv *priv = gpiochip_get_data(gc);
 	u32 bit = mt7530_gpio_to_bit(offset);
 
-	mt7530_clear(priv, MT7530_LED_GPIO_OE, bit);
-	mt7530_clear(priv, MT7530_LED_GPIO_DIR, bit);
+	regmap_clear_bits(priv->regmap, MT7530_LED_GPIO_OE, bit);
+	regmap_clear_bits(priv->regmap, MT7530_LED_GPIO_DIR, bit);
 
 	return 0;
 }
@@ -2171,14 +2167,14 @@ mt7530_gpio_direction_output(struct gpio_chip *gc, unsigned int offset, int valu
 	struct mt7530_priv *priv = gpiochip_get_data(gc);
 	u32 bit = mt7530_gpio_to_bit(offset);
 
-	mt7530_set(priv, MT7530_LED_GPIO_DIR, bit);
+	regmap_set_bits(priv->regmap, MT7530_LED_GPIO_DIR, bit);
 
 	if (value)
-		mt7530_set(priv, MT7530_LED_GPIO_DATA, bit);
+		regmap_set_bits(priv->regmap, MT7530_LED_GPIO_DATA, bit);
 	else
-		mt7530_clear(priv, MT7530_LED_GPIO_DATA, bit);
+		regmap_clear_bits(priv->regmap, MT7530_LED_GPIO_DATA, bit);
 
-	mt7530_set(priv, MT7530_LED_GPIO_OE, bit);
+	regmap_set_bits(priv->regmap, MT7530_LED_GPIO_OE, bit);
 
 	return 0;
 }
@@ -2284,7 +2280,8 @@ mt7530_setup_irq(struct mt7530_priv *priv)
 
 	/* This register must be set for MT7530 to properly fire interrupts */
 	if (priv->id == ID_MT7530 || priv->id == ID_MT7621)
-		mt7530_set(priv, MT7530_TOP_SIG_CTRL, TOP_SIG_CTRL_NORMAL);
+		regmap_set_bits(priv->regmap, MT7530_TOP_SIG_CTRL,
+				TOP_SIG_CTRL_NORMAL);
 
 	ret = devm_regmap_add_irq_chip_fwnode(dev, dev_fwnode(dev),
 					      priv->regmap, irq,
@@ -2462,14 +2459,15 @@ mt7530_setup(struct dsa_switch *ds)
 			     TD_DM_DRVP(8) | TD_DM_DRVN(8));
 
 	for (i = 0; i < NUM_TRGMII_CTRL; i++)
-		mt7530_rmw(priv, MT7530_TRGMII_RD(i),
-			   RD_TAP_MASK, RD_TAP(16));
+		regmap_update_bits(priv->regmap, MT7530_TRGMII_RD(i),
+				   RD_TAP_MASK, RD_TAP(16));
 
 	/* Allow modifying the trap and directly access PHY registers via the
 	 * MDIO bus the switch is on.
 	 */
-	mt7530_rmw(priv, MT753X_MTRAP, MT7530_CHG_TRAP |
-		   MT7530_PHY_INDIRECT_ACCESS, MT7530_CHG_TRAP);
+	regmap_update_bits(priv->regmap, MT753X_MTRAP,
+			   MT7530_CHG_TRAP | MT7530_PHY_INDIRECT_ACCESS,
+			   MT7530_CHG_TRAP);
 
 	if ((val & MT7530_XTAL_MASK) == MT7530_XTAL_40MHZ)
 		mt7530_pll_setup(priv);
@@ -2483,17 +2481,16 @@ mt7530_setup(struct dsa_switch *ds)
 		/* Clear link settings and enable force mode to force link down
 		 * on all ports until they're enabled later.
 		 */
-		mt7530_rmw(priv, MT753X_PMCR_P(i),
-			   PMCR_LINK_SETTINGS_MASK |
-			   MT753X_FORCE_MODE(priv->id),
-			   MT753X_FORCE_MODE(priv->id));
+		regmap_update_bits(priv->regmap, MT753X_PMCR_P(i),
+				   PMCR_LINK_SETTINGS_MASK | MT753X_FORCE_MODE(priv->id),
+				   MT753X_FORCE_MODE(priv->id));
 
 		/* Disable forwarding by default on all ports */
-		mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
-			   PCR_MATRIX_CLR);
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(i),
+				   PCR_MATRIX_MASK, PCR_MATRIX_CLR);
 
 		/* Disable learning by default on all ports */
-		mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
+		regmap_set_bits(priv->regmap, MT7530_PSC_P(i), SA_DIS);
 
 		if (dsa_is_cpu_port(ds, i)) {
 			mt753x_cpu_port_enable(ds, i);
@@ -2501,16 +2498,17 @@ mt7530_setup(struct dsa_switch *ds)
 			mt7530_port_disable(ds, i);
 
 			/* Set default PVID to 0 on all user ports */
-			mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK,
-				   G0_PORT_VID_DEF);
+			regmap_update_bits(priv->regmap, MT7530_PPBV1_P(i),
+					   G0_PORT_VID_MASK, G0_PORT_VID_DEF);
 		}
 		/* Enable consistent egress tag */
-		mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK,
-			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
+		regmap_update_bits(priv->regmap, MT7530_PVC_P(i),
+				   PVC_EG_TAG_MASK,
+				   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
 	}
 
 	/* Allow mirroring frames received on the local port (monitor port). */
-	mt7530_set(priv, MT753X_AGC, LOCAL_EN);
+	regmap_set_bits(priv->regmap, MT753X_AGC, LOCAL_EN);
 
 	/* Setup VLAN ID 0 for VLAN-unaware bridges */
 	ret = mt7530_setup_vlan0(priv);
@@ -2557,7 +2555,8 @@ mt7530_setup(struct dsa_switch *ds)
 
 		if (priv->p5_mode == MUX_PHY_P0 ||
 		    priv->p5_mode == MUX_PHY_P4) {
-			mt7530_clear(priv, MT753X_MTRAP, MT7530_P5_DIS);
+			regmap_clear_bits(priv->regmap, MT753X_MTRAP,
+					  MT7530_P5_DIS);
 			mt7530_setup_port5(ds, interface);
 		}
 	}
@@ -2596,26 +2595,26 @@ mt7531_setup_common(struct dsa_switch *ds)
 	mt7530_mib_reset(ds);
 
 	/* Disable flooding on all ports */
-	mt7530_clear(priv, MT753X_MFC, BC_FFP_MASK | UNM_FFP_MASK |
-		     UNU_FFP_MASK);
+	regmap_clear_bits(priv->regmap, MT753X_MFC,
+			  BC_FFP_MASK | UNM_FFP_MASK | UNU_FFP_MASK);
 
 	for (i = 0; i < priv->ds->num_ports; i++) {
 		/* Clear link settings and enable force mode to force link down
 		 * on all ports until they're enabled later.
 		 */
-		mt7530_rmw(priv, MT753X_PMCR_P(i),
-			   PMCR_LINK_SETTINGS_MASK |
-			   MT753X_FORCE_MODE(priv->id),
-			   MT753X_FORCE_MODE(priv->id));
+		regmap_update_bits(priv->regmap, MT753X_PMCR_P(i),
+				   PMCR_LINK_SETTINGS_MASK | MT753X_FORCE_MODE(priv->id),
+				   MT753X_FORCE_MODE(priv->id));
 
 		/* Disable forwarding by default on all ports */
-		mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
-			   PCR_MATRIX_CLR);
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(i),
+				   PCR_MATRIX_MASK, PCR_MATRIX_CLR);
 
 		/* Disable learning by default on all ports */
-		mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
+		regmap_set_bits(priv->regmap, MT7530_PSC_P(i), SA_DIS);
 
-		mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR);
+		regmap_set_bits(priv->regmap, MT7531_DBG_CNT(i),
+				MT7531_DIS_CLR);
 
 		if (dsa_is_cpu_port(ds, i)) {
 			mt753x_cpu_port_enable(ds, i);
@@ -2623,17 +2622,18 @@ mt7531_setup_common(struct dsa_switch *ds)
 			mt7530_port_disable(ds, i);
 
 			/* Set default PVID to 0 on all user ports */
-			mt7530_rmw(priv, MT7530_PPBV1_P(i), G0_PORT_VID_MASK,
-				   G0_PORT_VID_DEF);
+			regmap_update_bits(priv->regmap, MT7530_PPBV1_P(i),
+					   G0_PORT_VID_MASK, G0_PORT_VID_DEF);
 		}
 
 		/* Enable consistent egress tag */
-		mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK,
-			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
+		regmap_update_bits(priv->regmap, MT7530_PVC_P(i),
+				   PVC_EG_TAG_MASK,
+				   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
 	}
 
 	/* Allow mirroring frames received on the local port (monitor port). */
-	mt7530_set(priv, MT753X_AGC, LOCAL_EN);
+	regmap_set_bits(priv->regmap, MT753X_AGC, LOCAL_EN);
 
 	/* Enable Special Tag for rx frames */
 	if (priv->id == ID_EN7581 || priv->id == ID_AN7583)
@@ -2709,14 +2709,16 @@ mt7531_setup(struct dsa_switch *ds)
 		 * MT7531AE. Set the GPIO 11-12 pins to function as MDC and MDIO
 		 * to expose the MDIO bus of the switch.
 		 */
-		mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO11_RG_RXD2_MASK,
-			   MT7531_EXT_P_MDC_11);
-		mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO12_RG_RXD3_MASK,
-			   MT7531_EXT_P_MDIO_12);
+		regmap_update_bits(priv->regmap, MT7531_GPIO_MODE1,
+				   MT7531_GPIO11_RG_RXD2_MASK,
+				   MT7531_EXT_P_MDC_11);
+		regmap_update_bits(priv->regmap, MT7531_GPIO_MODE1,
+				   MT7531_GPIO12_RG_RXD3_MASK,
+				   MT7531_EXT_P_MDIO_12);
 	}
 
-	mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
-		   MT7531_GPIO0_INTERRUPT);
+	regmap_update_bits(priv->regmap, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
+			   MT7531_GPIO0_INTERRUPT);
 
 	/* Enable Energy-Efficient Ethernet (EEE) and PHY core PLL, since
 	 * phy_device has not yet been created provided for
@@ -2962,7 +2964,8 @@ mt753x_phylink_mac_config(struct phylink_config *config, unsigned int mode,
 
 	/* Are we connected to external phy */
 	if (port == 5 && dsa_is_user_port(ds, 5))
-		mt7530_set(priv, MT753X_PMCR_P(port), PMCR_EXT_PHY);
+		regmap_set_bits(priv->regmap, MT753X_PMCR_P(port),
+				PMCR_EXT_PHY);
 }
 
 static void mt753x_phylink_mac_link_down(struct phylink_config *config,
@@ -2972,7 +2975,8 @@ static void mt753x_phylink_mac_link_down(struct phylink_config *config,
 	struct dsa_port *dp = dsa_phylink_to_port(config);
 	struct mt7530_priv *priv = dp->ds->priv;
 
-	mt7530_clear(priv, MT753X_PMCR_P(dp->index), PMCR_LINK_SETTINGS_MASK);
+	regmap_clear_bits(priv->regmap, MT753X_PMCR_P(dp->index),
+			  PMCR_LINK_SETTINGS_MASK);
 }
 
 static void mt753x_phylink_mac_link_up(struct phylink_config *config,
@@ -3006,7 +3010,7 @@ static void mt753x_phylink_mac_link_up(struct phylink_config *config,
 			mcr |= PMCR_FORCE_RX_FC_EN;
 	}
 
-	mt7530_set(priv, MT753X_PMCR_P(dp->index), mcr);
+	regmap_set_bits(priv->regmap, MT753X_PMCR_P(dp->index), mcr);
 }
 
 static void mt753x_phylink_mac_disable_tx_lpi(struct phylink_config *config)
@@ -3014,8 +3018,8 @@ static void mt753x_phylink_mac_disable_tx_lpi(struct phylink_config *config)
 	struct dsa_port *dp = dsa_phylink_to_port(config);
 	struct mt7530_priv *priv = dp->ds->priv;
 
-	mt7530_clear(priv, MT753X_PMCR_P(dp->index),
-		     PMCR_FORCE_EEE1G | PMCR_FORCE_EEE100);
+	regmap_clear_bits(priv->regmap, MT753X_PMCR_P(dp->index),
+			  PMCR_FORCE_EEE1G | PMCR_FORCE_EEE100);
 }
 
 static int mt753x_phylink_mac_enable_tx_lpi(struct phylink_config *config,
@@ -3036,11 +3040,11 @@ static int mt753x_phylink_mac_enable_tx_lpi(struct phylink_config *config,
 	else
 		val = LPI_THRESH_MASK;
 
-	mt7530_rmw(priv, MT753X_PMEEECR_P(dp->index),
-		   LPI_THRESH_MASK | LPI_MODE_EN, val);
+	regmap_update_bits(priv->regmap, MT753X_PMEEECR_P(dp->index),
+			   LPI_THRESH_MASK | LPI_MODE_EN, val);
 
-	mt7530_set(priv, MT753X_PMCR_P(dp->index),
-		   PMCR_FORCE_EEE1G | PMCR_FORCE_EEE100);
+	regmap_set_bits(priv->regmap, MT753X_PMCR_P(dp->index),
+			PMCR_FORCE_EEE1G | PMCR_FORCE_EEE100);
 
 	return 0;
 }
@@ -3217,7 +3221,8 @@ mt753x_conduit_state_change(struct dsa_switch *ds,
 		      MT7530_CPU_PORT(__ffs(priv->active_cpu_ports));
 	}
 
-	mt7530_rmw(priv, MT753X_MFC, MT7530_CPU_EN | MT7530_CPU_PORT_MASK, val);
+	regmap_update_bits(priv->regmap, MT753X_MFC,
+			   MT7530_CPU_EN | MT7530_CPU_PORT_MASK, val);
 }
 
 static int mt753x_tc_setup_qdisc_tbf(struct dsa_switch *ds, int port,
@@ -3234,8 +3239,8 @@ static int mt753x_tc_setup_qdisc_tbf(struct dsa_switch *ds, int port,
 	case TC_TBF_DESTROY: {
 		u32 val, tick;
 
-		mt7530_rmw(priv, MT753X_GERLCR, EGR_BC_MASK,
-			   EGR_BC_CRC_IPG_PREAMBLE);
+		regmap_update_bits(priv->regmap, MT753X_GERLCR, EGR_BC_MASK,
+				   EGR_BC_CRC_IPG_PREAMBLE);
 
 		/* if rate is greater than 10Mbps tick is 1/32 ms,
 		 * 1ms otherwise
@@ -3279,13 +3284,13 @@ static int mt7988_setup(struct dsa_switch *ds)
 
 	/* AN7583 require additional tweak to CONN_CFG */
 	if (priv->id == ID_AN7583)
-		mt7530_rmw(priv, AN7583_GEPHY_CONN_CFG,
-			   AN7583_CSR_DPHY_CKIN_SEL |
-			   AN7583_CSR_PHY_CORE_REG_CLK_SEL |
-			   AN7583_CSR_ETHER_AFE_PWD,
-			   AN7583_CSR_DPHY_CKIN_SEL |
-			   AN7583_CSR_PHY_CORE_REG_CLK_SEL |
-			   FIELD_PREP(AN7583_CSR_ETHER_AFE_PWD, 0));
+		regmap_update_bits(priv->regmap, AN7583_GEPHY_CONN_CFG,
+				   AN7583_CSR_DPHY_CKIN_SEL |
+				   AN7583_CSR_PHY_CORE_REG_CLK_SEL |
+				   AN7583_CSR_ETHER_AFE_PWD,
+				   AN7583_CSR_DPHY_CKIN_SEL |
+				   AN7583_CSR_PHY_CORE_REG_CLK_SEL |
+				   FIELD_PREP(AN7583_CSR_ETHER_AFE_PWD, 0));
 
 	/* Reset the switch PHYs */
 	regmap_write(priv->regmap, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
Signature · 1 line
2.54.0
author maintainer
#5 lore raw

PATCH v2 5/8 net: dsa: mt7530: replace mt7530_read with regmap_read

In reply to Daniel Golle

Replace all mt7530_read() calls with direct regmap_read() calls and remove the wrapper function. The WARN_ON_ONCE error logging is dropped as regmap provides its own error handling.

Most callsites follow the val = mt7530_read(priv, reg) pattern and are converted mechanically using the following semantic patch:

@@
expression priv, reg;
identifier val;
@@
-val = mt7530_read(priv, reg);
+regmap_read(priv->regmap, reg, &val);

Remaining inline uses are converted by hand.

---
v2: drop fix for stray 'static void' leftover now correctly squashed

    into 4/8
 drivers/net/dsa/mt7530.c | 113 +++++++++++++++++++--------------------
 1 file changed, 56 insertions(+), 57 deletions(-)
drivers/net/dsa/mt7530.c +56 -57
index fe7e4ab5ae9c..4168adca949f 100644
@@ -152,28 +152,15 @@ core_clear(struct mt7530_priv *priv, u32 reg, u32 val)
 
 
 static u32
-mt7530_read(struct mt7530_priv *priv, u32 reg)
+mt7530_mii_poll(struct mt7530_dummy_poll *p)
 {
-	int ret;
 	u32 val;
 
-	ret = regmap_read(priv->regmap, reg, &val);
-	if (ret) {
-		WARN_ON_ONCE(1);
-		dev_err(priv->dev,
-			"failed to read mt7530 register\n");
-		return 0;
-	}
+	regmap_read(p->priv->regmap, p->reg, &val);
 
 	return val;
 }
 
-static u32
-mt7530_mii_poll(struct mt7530_dummy_poll *p)
-{
-	return mt7530_read(p->priv, p->reg);
-}
-
 static int
 mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 {
@@ -196,7 +183,7 @@ mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 	/* Additional sanity for read command if the specified
 	 * entry is invalid
 	 */
-	val = mt7530_read(priv, MT7530_ATC);
+	regmap_read(priv->regmap, MT7530_ATC, &val);
 	if ((cmd == MT7530_FDB_READ) && (val & ATC_INVALID))
 		return -EINVAL;
 
@@ -214,7 +201,8 @@ mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
 
 	/* Read from ARL table into an array */
 	for (i = 0; i < 3; i++) {
-		reg[i] = mt7530_read(priv, MT7530_TSRA1 + (i * 4));
+		regmap_read(priv->regmap, MT7530_TSRA1 + (i * 4),
+			    &reg[i]);
 
 		dev_dbg(priv->dev, "%s(%d) reg[%d]=0x%x\n",
 			__func__, __LINE__, i, reg[i]);
@@ -321,7 +309,8 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
 	regmap_update_bits(priv->regmap, MT7530_P6ECR, P6_INTF_MODE_MASK,
 			   P6_INTF_MODE(1));
 
-	xtal = mt7530_read(priv, MT753X_MTRAP) & MT7530_XTAL_MASK;
+	regmap_read(priv->regmap, MT753X_MTRAP, &xtal);
+	xtal &= MT7530_XTAL_MASK;
 
 	if (xtal == MT7530_XTAL_25MHZ)
 		ssc_delta = 0x57;
@@ -365,9 +354,9 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 	u32 hwstrap;
 	u32 val;
 
-	val = mt7530_read(priv, MT7531_CREV);
-	top_sig = mt7530_read(priv, MT7531_TOP_SIG_SR);
-	hwstrap = mt7530_read(priv, MT753X_TRAP);
+	regmap_read(priv->regmap, MT7531_CREV, &val);
+	regmap_read(priv->regmap, MT7531_TOP_SIG_SR, &top_sig);
+	regmap_read(priv->regmap, MT753X_TRAP, &hwstrap);
 	if ((val & CHIP_REV_M) > 0)
 		xtal = (top_sig & PAD_MCM_SMI_EN) ? MT7531_XTAL_FSEL_40MHZ :
 						    MT7531_XTAL_FSEL_25MHZ;
@@ -376,26 +365,26 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 						   MT7531_XTAL_FSEL_40MHZ;
 
 	/* Step 1 : Disable MT7531 COREPLL */
-	val = mt7530_read(priv, MT7531_PLLGP_EN);
+	regmap_read(priv->regmap, MT7531_PLLGP_EN, &val);
 	val &= ~EN_COREPLL;
 	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 
 	/* Step 2: switch to XTAL output */
-	val = mt7530_read(priv, MT7531_PLLGP_EN);
+	regmap_read(priv->regmap, MT7531_PLLGP_EN, &val);
 	val |= SW_CLKSW;
 	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 
-	val = mt7530_read(priv, MT7531_PLLGP_CR0);
+	regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
 	val &= ~RG_COREPLL_EN;
 	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 
 	/* Step 3: disable PLLGP and enable program PLLGP */
-	val = mt7530_read(priv, MT7531_PLLGP_EN);
+	regmap_read(priv->regmap, MT7531_PLLGP_EN, &val);
 	val |= SW_PLLGP;
 	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 
 	/* Step 4: program COREPLL output frequency to 500MHz */
-	val = mt7530_read(priv, MT7531_PLLGP_CR0);
+	regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
 	val &= ~RG_COREPLL_POSDIV_M;
 	val |= 2 << RG_COREPLL_POSDIV_S;
 	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
@@ -403,13 +392,13 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 
 	switch (xtal) {
 	case MT7531_XTAL_FSEL_25MHZ:
-		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
 		val &= ~RG_COREPLL_SDM_PCW_M;
 		val |= 0x140000 << RG_COREPLL_SDM_PCW_S;
 		regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 		break;
 	case MT7531_XTAL_FSEL_40MHZ:
-		val = mt7530_read(priv, MT7531_PLLGP_CR0);
+		regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
 		val &= ~RG_COREPLL_SDM_PCW_M;
 		val |= 0x190000 << RG_COREPLL_SDM_PCW_S;
 		regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
@@ -417,14 +406,14 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 	}
 
 	/* Set feedback divide ratio update signal to high */
-	val = mt7530_read(priv, MT7531_PLLGP_CR0);
+	regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
 	val |= RG_COREPLL_SDM_PCW_CHG;
 	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 	/* Wait for at least 16 XTAL clocks */
 	usleep_range(10, 20);
 
 	/* Step 5: set feedback divide ratio update signal to low */
-	val = mt7530_read(priv, MT7531_PLLGP_CR0);
+	regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
 	val &= ~RG_COREPLL_SDM_PCW_CHG;
 	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 
@@ -435,11 +424,11 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 	regmap_write(priv->regmap, MT7531_ANA_PLLGP_CR2, 0x4f40000);
 
 	/* Step 6: Enable MT7531 PLL */
-	val = mt7530_read(priv, MT7531_PLLGP_CR0);
+	regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
 	val |= RG_COREPLL_EN;
 	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 
-	val = mt7530_read(priv, MT7531_PLLGP_EN);
+	regmap_read(priv->regmap, MT7531_PLLGP_EN, &val);
 	val |= EN_COREPLL;
 	regmap_write(priv->regmap, MT7531_PLLGP_EN, val);
 	usleep_range(25, 35);
@@ -698,11 +687,11 @@ mt7530_read_port_stats(struct mt7530_priv *priv, int port,
 {
 	u32 val, reg = MT7530_PORT_MIB_COUNTER(port) + offset;
 
-	val = mt7530_read(priv, reg);
+	regmap_read(priv->regmap, reg, &val);
 	*data = val;
 
 	if (size == 2) {
-		val = mt7530_read(priv, reg + 4);
+		regmap_read(priv->regmap, reg + 4, &val);
 		*data |= (u64)val << 32;
 	}
 }
@@ -1010,7 +999,7 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
 
 	mutex_lock(&priv->reg_mutex);
 
-	val = mt7530_read(priv, MT753X_MTRAP);
+	regmap_read(priv->regmap, MT753X_MTRAP, &val);
 
 	val &= ~MT7530_P5_PHY0_SEL & ~MT7530_P5_MAC_SEL & ~MT7530_P5_RGMII_MODE;
 
@@ -1378,7 +1367,7 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 	if (!dsa_is_cpu_port(ds, port))
 		return 0;
 
-	val = mt7530_read(priv, MT7530_GMACCR);
+	regmap_read(priv->regmap, MT7530_GMACCR, &val);
 	val &= ~MAX_RX_PKT_LEN_MASK;
 
 	/* RX length also includes Ethernet header, MTK tag, and FCS length */
@@ -1577,7 +1566,7 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
 		return ret;
 	}
 
-	val = mt7530_read(priv, MT7530_VTCR);
+	regmap_read(priv->regmap, MT7530_VTCR, &val);
 	if (val & VTCR_INVALID) {
 		dev_err(priv->dev, "read VTCR invalid\n");
 		return -EINVAL;
@@ -1789,14 +1778,16 @@ mt7530_port_mdb_add(struct dsa_switch *ds, int port,
 	const u8 *addr = mdb->addr;
 	u16 vid = mdb->vid;
 	u8 port_mask = 0;
+	u32 val;
 	int ret;
 
 	mutex_lock(&priv->reg_mutex);
 
 	mt7530_fdb_write(priv, vid, 0, addr, 0, STATIC_EMP);
-	if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL))
-		port_mask = (mt7530_read(priv, MT7530_ATRD) >> PORT_MAP)
-			    & PORT_MAP_MASK;
+	if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL)) {
+		regmap_read(priv->regmap, MT7530_ATRD, &val);
+		port_mask = (val >> PORT_MAP) & PORT_MAP_MASK;
+	}
 
 	port_mask |= BIT(port);
 	mt7530_fdb_write(priv, vid, port_mask, addr, -1, STATIC_ENT);
@@ -1816,14 +1807,16 @@ mt7530_port_mdb_del(struct dsa_switch *ds, int port,
 	const u8 *addr = mdb->addr;
 	u16 vid = mdb->vid;
 	u8 port_mask = 0;
+	u32 val;
 	int ret;
 
 	mutex_lock(&priv->reg_mutex);
 
 	mt7530_fdb_write(priv, vid, 0, addr, 0, STATIC_EMP);
-	if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL))
-		port_mask = (mt7530_read(priv, MT7530_ATRD) >> PORT_MAP)
-			    & PORT_MAP_MASK;
+	if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL)) {
+		regmap_read(priv->regmap, MT7530_ATRD, &val);
+		port_mask = (val >> PORT_MAP) & PORT_MAP_MASK;
+	}
 
 	port_mask &= ~BIT(port);
 	mt7530_fdb_write(priv, vid, port_mask, addr, -1,
@@ -1901,7 +1894,7 @@ mt7530_hw_vlan_del(struct mt7530_priv *priv,
 
 	new_members = entry->old_members & ~BIT(entry->port);
 
-	val = mt7530_read(priv, MT7530_VAWD1);
+	regmap_read(priv->regmap, MT7530_VAWD1, &val);
 	if (!(val & VLAN_VALID)) {
 		dev_err(priv->dev,
 			"Cannot be deleted due to invalid entry\n");
@@ -1928,7 +1921,7 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid,
 	/* Fetch entry */
 	mt7530_vlan_cmd(priv, MT7530_VTCR_RD_VID, vid);
 
-	val = mt7530_read(priv, MT7530_VAWD1);
+	regmap_read(priv->regmap, MT7530_VAWD1, &val);
 
 	entry->old_members = (val >> PORT_MEM_SHFT) & PORT_MEM_MASK;
 
@@ -2046,7 +2039,7 @@ static int mt753x_port_mirror_add(struct dsa_switch *ds, int port,
 	if ((ingress ? priv->mirror_rx : priv->mirror_tx) & BIT(port))
 		return -EEXIST;
 
-	val = mt7530_read(priv, MT753X_MIRROR_REG(priv->id));
+	regmap_read(priv->regmap, MT753X_MIRROR_REG(priv->id), &val);
 
 	/* MT7530 only supports one monitor port */
 	monitor_port = MT753X_MIRROR_PORT_GET(priv->id, val);
@@ -2059,7 +2052,7 @@ static int mt753x_port_mirror_add(struct dsa_switch *ds, int port,
 	val |= MT753X_MIRROR_PORT_SET(priv->id, mirror->to_local_port);
 	regmap_write(priv->regmap, MT753X_MIRROR_REG(priv->id), val);
 
-	val = mt7530_read(priv, MT7530_PCR_P(port));
+	regmap_read(priv->regmap, MT7530_PCR_P(port), &val);
 	if (ingress) {
 		val |= PORT_RX_MIR;
 		priv->mirror_rx |= BIT(port);
@@ -2078,7 +2071,7 @@ static void mt753x_port_mirror_del(struct dsa_switch *ds, int port,
 	struct mt7530_priv *priv = ds->priv;
 	u32 val;
 
-	val = mt7530_read(priv, MT7530_PCR_P(port));
+	regmap_read(priv->regmap, MT7530_PCR_P(port), &val);
 	if (mirror->ingress) {
 		val &= ~PORT_RX_MIR;
 		priv->mirror_rx &= ~BIT(port);
@@ -2089,7 +2082,7 @@ static void mt753x_port_mirror_del(struct dsa_switch *ds, int port,
 	regmap_write(priv->regmap, MT7530_PCR_P(port), val);
 
 	if (!priv->mirror_rx && !priv->mirror_tx) {
-		val = mt7530_read(priv, MT753X_MIRROR_REG(priv->id));
+		regmap_read(priv->regmap, MT753X_MIRROR_REG(priv->id), &val);
 		val &= ~MT753X_MIRROR_EN(priv->id);
 		regmap_write(priv->regmap, MT753X_MIRROR_REG(priv->id), val);
 	}
@@ -2121,8 +2114,11 @@ mt7530_gpio_get(struct gpio_chip *gc, unsigned int offset)
 {
 	struct mt7530_priv *priv = gpiochip_get_data(gc);
 	u32 bit = mt7530_gpio_to_bit(offset);
+	u32 val;
+
+	regmap_read(priv->regmap, MT7530_LED_GPIO_DATA, &val);
 
-	return !!(mt7530_read(priv, MT7530_LED_GPIO_DATA) & bit);
+	return !!(val & bit);
 }
 
 static int
@@ -2144,8 +2140,11 @@ mt7530_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
 	struct mt7530_priv *priv = gpiochip_get_data(gc);
 	u32 bit = mt7530_gpio_to_bit(offset);
+	u32 val;
+
+	regmap_read(priv->regmap, MT7530_LED_GPIO_DIR, &val);
 
-	return (mt7530_read(priv, MT7530_LED_GPIO_DIR) & bit) ?
+	return (val & bit) ?
 		GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
 }
 
@@ -2436,7 +2435,7 @@ mt7530_setup(struct dsa_switch *ds)
 		return ret;
 	}
 
-	id = mt7530_read(priv, MT7530_CREV);
+	regmap_read(priv->regmap, MT7530_CREV, &id);
 	id >>= CHIP_NAME_SHIFT;
 	if (id != MT7530_ID) {
 		dev_err(priv->dev, "chip %x can't be supported\n", id);
@@ -2679,7 +2678,7 @@ mt7531_setup(struct dsa_switch *ds)
 		return ret;
 	}
 
-	id = mt7530_read(priv, MT7531_CREV);
+	regmap_read(priv->regmap, MT7531_CREV, &id);
 	id >>= CHIP_NAME_SHIFT;
 
 	if (id != MT7531_ID) {
@@ -2690,7 +2689,7 @@ mt7531_setup(struct dsa_switch *ds)
 	/* MT7531AE has got two SGMII units. One for port 5, one for port 6.
 	 * MT7531BE has got only one SGMII unit which is for port 6.
 	 */
-	val = mt7530_read(priv, MT7531_TOP_SIG_SR);
+	regmap_read(priv->regmap, MT7531_TOP_SIG_SR, &val);
 	priv->p5_sgmii = !!(val & PAD_DUAL_SGMII_EN);
 
 	/* Force link down on all ports before internal reset */
@@ -2880,7 +2879,7 @@ static void mt7531_rgmii_setup(struct mt7530_priv *priv,
 {
 	u32 val;
 
-	val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
+	regmap_read(priv->regmap, MT7531_CLKGEN_CTRL, &val);
 	val |= GP_CLK_EN;
 	val &= ~GP_MODE_MASK;
 	val |= GP_MODE(MT7531_GP_MODE_RGMII);
@@ -3059,7 +3058,7 @@ static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
 
 	config->lpi_capabilities = MAC_100FD | MAC_1000FD | MAC_2500FD;
 
-	eeecr = mt7530_read(priv, MT753X_PMEEECR_P(port));
+	regmap_read(priv->regmap, MT753X_PMEEECR_P(port), &eeecr);
 	/* tx_lpi_timer should be in microseconds. The time units for
 	 * LPI threshold are unspecified.
 	 */
@@ -3087,7 +3086,7 @@ static void mt7530_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
 	int port = pcs_to_mt753x_pcs(pcs)->port;
 	u32 pmsr;
 
-	pmsr = mt7530_read(priv, MT7530_PMSR_P(port));
+	regmap_read(priv->regmap, MT7530_PMSR_P(port), &pmsr);
 
 	state->link = (pmsr & PMSR_LINK);
 	state->an_complete = state->link;
Signature · 1 line
2.54.0
maintainer
#6 lore raw

Re: [PATCH net-next v2 5/8] net: dsa: mt7530: replace mt7530_read with regmap_read

This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
---
net: dsa: mt7530: replace mt7530_read with regmap_read

This commit removes the mt7530_read() wrapper and converts every caller to use regmap_read() directly. The WARN_ON_ONCE / dev_err and the return-zero-on-failure behaviour the wrapper provided are dropped.

Quoted 15 lines
drivers/net/dsa/mt7530.c +1 -3
index fe7e4ab5ae9c..4168adca949f 100644
@@ -152,28 +152,15 @@ core_clear(struct mt7530_priv *priv, u32 reg, u32 val)
 
 
 static u32
-mt7530_read(struct mt7530_priv *priv, u32 reg)
+mt7530_mii_poll(struct mt7530_dummy_poll *p)
 {
-	int ret;
 	u32 val;
 
-	ret = regmap_read(priv->regmap, reg, &val);

drivers/net/dsa/mt7530.c core_clear(struct mt7530_priv *priv, u32 reg, u32 val) · line 162

Quoted 10 lines
drivers/net/dsa/mt7530.c +1 -6
-	if (ret) {
-		WARN_ON_ONCE(1);
-		dev_err(priv->dev,
-			"failed to read mt7530 register\n");
-		return 0;
-	}
+	regmap_read(p->priv->regmap, p->reg, &val);
 
 	return val;
 }

[Medium]
The old wrapper guaranteed val == 0 on failure. The new mt7530_mii_poll() declares val without an initializer and ignores the regmap_read() return code, so on a failed underlying transfer val is whatever was on the stack.

Looking at drivers/net/dsa/mt7530-mdio.c::mt7530_regmap_read():

    ret = bus->write(bus, priv->mdiodev->addr, 0x1f, page);
    if (ret < 0)
        return ret;

    lo = bus->read(bus, priv->mdiodev->addr, r);
    hi = bus->read(bus, priv->mdiodev->addr, 0x10);

    *val = (hi << 16) | (lo & 0xffff);

When the page-select write fails, *val is not written. mt7530_mii_poll() is the polling callback used by readx_poll_timeout() in mt7530_fdb_cmd(), mt7530_vlan_cmd(), mt7530_setup() and mt7531_setup() with predicates such as val != 0 (post-reset stability), !(val & ATC_BUSY) and !(val & VTCR_BUSY).

Previously a broken MDIO transport produced val == 0 and the post-reset poll deterministically timed out. Can the new code spuriously satisfy the val != 0 predicate from an uninitialized stack slot, then feed that same garbage into (val & MT7530_XTAL_MASK) to choose 25/40 MHz settings in mt7530_pll_setup()?

drivers/net/dsa/mt7530.c mt7531_pll_setup(struct mt7530_priv *priv) · line 359

Quoted 10 lines
drivers/net/dsa/mt7530.c +3 -3
@@ -365,9 +354,9 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 	u32 hwstrap;
 	u32 val;
 
-	val = mt7530_read(priv, MT7531_CREV);
-	top_sig = mt7530_read(priv, MT7531_TOP_SIG_SR);
-	hwstrap = mt7530_read(priv, MT753X_TRAP);
+	regmap_read(priv->regmap, MT7531_CREV, &val);
+	regmap_read(priv->regmap, MT7531_TOP_SIG_SR, &top_sig);
+	regmap_read(priv->regmap, MT753X_TRAP, &hwstrap);

[Medium]
Same concern applies to all the read-modify-write sequences in mt7531_pll_setup(), e.g.:

    regmap_read(priv->regmap, MT7531_PLLGP_EN, &val);
    val &= ~EN_COREPLL;
    regmap_write(priv->regmap, MT7531_PLLGP_EN, val);

If regmap_read() fails, val is uninitialized and is then OR'd / AND'd with constants and written straight back into PLLGP / COREPLL control registers. Was there a reason to drop the wrapper's return-zero (and WARN_ON_ONCE / dev_err) behaviour without either initializing these locals to 0 or checking the return code?

drivers/net/dsa/mt7530.c static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface) · line 1002

Quoted 6 lines
drivers/net/dsa/mt7530.c +1 -1
@@ -1010,7 +999,7 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
 
 	mutex_lock(&priv->reg_mutex);
 
-	val = mt7530_read(priv, MT753X_MTRAP);
+	regmap_read(priv->regmap, MT753X_MTRAP, &val);

[Low]
mt7530_setup_port5() declares val as int, but regmap_read()'s third parameter is unsigned int *. The previous code hid this through the u32 return of mt7530_read() and an implicit assignment conversion; the direct call now passes &val with mismatched signedness. Should this be u32 val; like the other call sites in this file (sparse and -Wpointer-sign would flag this)?

drivers/net/dsa/mt7530.c mt7530_setup(struct dsa_switch *ds) · line 2441

Quoted 9 lines
drivers/net/dsa/mt7530.c +1 -1
@@ -2436,7 +2435,7 @@ mt7530_setup(struct dsa_switch *ds)
 		return ret;
 	}
 
-	id = mt7530_read(priv, MT7530_CREV);
+	regmap_read(priv->regmap, MT7530_CREV, &id);
 	id >>= CHIP_NAME_SHIFT;
 	if (id != MT7530_ID) {
 		dev_err(priv->dev, "chip %x can't be supported\n", id);

[Medium]
The chip-ID gating in mt7530_setup() and mt7531_setup() was previously deterministic-fail on MDIO read errors (id == 0 after the wrapper). With id now uninitialized on failure, can this check spuriously match or mismatch on garbage and either mis-bind a wrong-id chip or refuse a real one?

drivers/net/dsa/mt7530.c mt7531_setup(struct dsa_switch *ds) · line 2693

Quoted 7 lines
drivers/net/dsa/mt7530.c +1 -1
@@ -2690,7 +2689,7 @@ mt7531_setup(struct dsa_switch *ds)
 	/* MT7531AE has got two SGMII units. One for port 5, one for port 6.
 	 * MT7531BE has got only one SGMII unit which is for port 6.
 	 */
-	val = mt7530_read(priv, MT7531_TOP_SIG_SR);
+	regmap_read(priv->regmap, MT7531_TOP_SIG_SR, &val);
 	priv->p5_sgmii = !!(val & PAD_DUAL_SGMII_EN);

[Medium]
And here, p5_sgmii is set from val & PAD_DUAL_SGMII_EN after an unchecked read; if the read fails, would this latch a wrong p5_sgmii state from stack contents and then drive different mt7531_pll_setup() vs. GPIO mode programming below?

The commit message says:

The WARN_ON_ONCE error logging is dropped as regmap provides its own error handling.

Is that accurate for this driver? regmap_read() does not zero *val on error and does not log; with all new call sites discarding the return value, the converted paths appear to have no error handling at all, which seems like a behavioural regression compared to the prior wrapper. Would initializing the affected locals to 0 (matching the old return-zero-on-failure semantics), or checking the regmap_read() return code at the sites that derive subsequent hardware programming (chip ID, XTAL select, PLL RMW, post-reset poll), be appropriate here?

Signature · 1 line
pw-bot: cr
maintainer
#7 lore raw

Re: [PATCH net-next v2 5/8] net: dsa: mt7530: replace mt7530_read with regmap_read

Quoted 26 lines · On Mon, 15 Jun 2026 16:56:49 -0700 Jakub Kicinski wrote:
Quoted 8 lines

static u32

drivers/net/dsa/mt7530.c +1 -3
-mt7530_read(struct mt7530_priv *priv, u32 reg)
+mt7530_mii_poll(struct mt7530_dummy_poll *p)
 {
-	int ret;
 	u32 val;
 
-	ret = regmap_read(priv->regmap, reg, &val);

drivers/net/dsa/mt7530.c core_clear(struct mt7530_priv *priv, u32 reg, u32 val) · line 162

Quoted 10 lines
drivers/net/dsa/mt7530.c +1 -6
-	if (ret) {
-		WARN_ON_ONCE(1);
-		dev_err(priv->dev,
-			"failed to read mt7530 register\n");
-		return 0;
-	}
+	regmap_read(p->priv->regmap, p->reg, &val);
 
 	return val;
 }  

[Medium]
The old wrapper guaranteed val == 0 on failure. The new mt7530_mii_poll() declares val without an initializer and ignores the regmap_read() return code, so on a failed underlying transfer val is whatever was on the stack.

Looking at drivers/net/dsa/mt7530-mdio.c::mt7530_regmap_read():

Quoted 8 lines
    ret = bus->write(bus, priv->mdiodev->addr, 0x1f, page);
    if (ret < 0)
        return ret;

    lo = bus->read(bus, priv->mdiodev->addr, r);
    hi = bus->read(bus, priv->mdiodev->addr, 0x10);

    *val = (hi << 16) | (lo & 0xffff);

This looks legit, AFAICT, sadly.

Maybe just write *val = 0; in mt7530_regmap_read() unconditionally ?

author maintainer
#8 lore raw

PATCH v2 6/8 net: dsa: mt7530: convert to use field accessor macros

In reply to Daniel Golle

Use FIELD_GET and FIELD_PREP instead of open-coding register fields. Replace 0x1f constant with (PHY_MAX_ADDR - 1)

---
v2: no changes

 drivers/net/dsa/mt7530.c |  64 ++++++------
 drivers/net/dsa/mt7530.h | 208 ++++++++++++++++++++++-----------------
 2 files changed, 148 insertions(+), 124 deletions(-)
drivers/net/dsa/mt7530.c +32 -32
index 4168adca949f..dcf72ab0cd66 100644
@@ -208,16 +208,16 @@ mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
 			__func__, __LINE__, i, reg[i]);
 	}
 
-	fdb->vid = (reg[1] >> CVID) & CVID_MASK;
-	fdb->aging = (reg[2] >> AGE_TIMER) & AGE_TIMER_MASK;
-	fdb->port_mask = (reg[2] >> PORT_MAP) & PORT_MAP_MASK;
-	fdb->mac[0] = (reg[0] >> MAC_BYTE_0) & MAC_BYTE_MASK;
-	fdb->mac[1] = (reg[0] >> MAC_BYTE_1) & MAC_BYTE_MASK;
-	fdb->mac[2] = (reg[0] >> MAC_BYTE_2) & MAC_BYTE_MASK;
-	fdb->mac[3] = (reg[0] >> MAC_BYTE_3) & MAC_BYTE_MASK;
-	fdb->mac[4] = (reg[1] >> MAC_BYTE_4) & MAC_BYTE_MASK;
-	fdb->mac[5] = (reg[1] >> MAC_BYTE_5) & MAC_BYTE_MASK;
-	fdb->noarp = ((reg[2] >> ENT_STATUS) & ENT_STATUS_MASK) == STATIC_ENT;
+	fdb->vid = FIELD_GET(CVID_MASK, reg[1]);
+	fdb->aging = FIELD_GET(AGE_TIMER_RD_MASK, reg[2]);
+	fdb->port_mask = FIELD_GET(PORT_MAP_MASK, reg[2]);
+	fdb->mac[0] = FIELD_GET(MAC_BYTE_0_MASK, reg[0]);
+	fdb->mac[1] = FIELD_GET(MAC_BYTE_1_MASK, reg[0]);
+	fdb->mac[2] = FIELD_GET(MAC_BYTE_2_MASK, reg[0]);
+	fdb->mac[3] = FIELD_GET(MAC_BYTE_3_MASK, reg[0]);
+	fdb->mac[4] = FIELD_GET(MAC_BYTE_4_MASK, reg[1]);
+	fdb->mac[5] = FIELD_GET(MAC_BYTE_5_MASK, reg[1]);
+	fdb->noarp = FIELD_GET(ENT_STATUS_MASK, reg[2]) == STATIC_ENT;
 }
 
 static void
@@ -228,22 +228,22 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
 	u32 reg[3] = { 0 };
 	int i;
 
-	reg[1] |= vid & CVID_MASK;
+	reg[1] |= FIELD_PREP(CVID_MASK, vid);
 	reg[1] |= ATA2_IVL;
 	reg[1] |= ATA2_FID(FID_BRIDGED);
-	reg[2] |= (aging & AGE_TIMER_MASK) << AGE_TIMER;
-	reg[2] |= (port_mask & PORT_MAP_MASK) << PORT_MAP;
+	reg[2] |= FIELD_PREP(AGE_TIMER_RD_MASK, aging);
+	reg[2] |= FIELD_PREP(PORT_MAP_MASK, port_mask);
 	/* STATIC_ENT indicate that entry is static wouldn't
 	 * be aged out and STATIC_EMP specified as erasing an
 	 * entry
 	 */
-	reg[2] |= (type & ENT_STATUS_MASK) << ENT_STATUS;
-	reg[1] |= mac[5] << MAC_BYTE_5;
-	reg[1] |= mac[4] << MAC_BYTE_4;
-	reg[0] |= mac[3] << MAC_BYTE_3;
-	reg[0] |= mac[2] << MAC_BYTE_2;
-	reg[0] |= mac[1] << MAC_BYTE_1;
-	reg[0] |= mac[0] << MAC_BYTE_0;
+	reg[2] |= FIELD_PREP(ENT_STATUS_MASK, type);
+	reg[1] |= FIELD_PREP(MAC_BYTE_5_MASK, mac[5]);
+	reg[1] |= FIELD_PREP(MAC_BYTE_4_MASK, mac[4]);
+	reg[0] |= FIELD_PREP(MAC_BYTE_3_MASK, mac[3]);
+	reg[0] |= FIELD_PREP(MAC_BYTE_2_MASK, mac[2]);
+	reg[0] |= FIELD_PREP(MAC_BYTE_1_MASK, mac[1]);
+	reg[0] |= FIELD_PREP(MAC_BYTE_0_MASK, mac[0]);
 
 	/* Write array into the ARL table */
 	for (i = 0; i < 3; i++)
@@ -385,22 +385,22 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 
 	/* Step 4: program COREPLL output frequency to 500MHz */
 	regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
-	val &= ~RG_COREPLL_POSDIV_M;
-	val |= 2 << RG_COREPLL_POSDIV_S;
+	val &= ~RG_COREPLL_POSDIV_MASK;
+	val |= RG_COREPLL_POSDIV(2);
 	regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 	usleep_range(25, 35);
 
 	switch (xtal) {
 	case MT7531_XTAL_FSEL_25MHZ:
 		regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
-		val &= ~RG_COREPLL_SDM_PCW_M;
-		val |= 0x140000 << RG_COREPLL_SDM_PCW_S;
+		val &= ~RG_COREPLL_SDM_PCW_MASK;
+		val |= RG_COREPLL_SDM_PCW(0x140000);
 		regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 		break;
 	case MT7531_XTAL_FSEL_40MHZ:
 		regmap_read(priv->regmap, MT7531_PLLGP_CR0, &val);
-		val &= ~RG_COREPLL_SDM_PCW_M;
-		val |= 0x190000 << RG_COREPLL_SDM_PCW_S;
+		val &= ~RG_COREPLL_SDM_PCW_MASK;
+		val |= RG_COREPLL_SDM_PCW(0x190000);
 		regmap_write(priv->regmap, MT7531_PLLGP_CR0, val);
 		break;
 	}
@@ -1555,7 +1555,7 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
 	u32 val;
 	int ret;
 
-	val = VTCR_BUSY | VTCR_FUNC(cmd) | vid;
+	val = VTCR_BUSY | VTCR_FUNC(cmd) | VTCR_VID(vid);
 	regmap_write(priv->regmap, MT7530_VTCR, val);
 
 	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_VTCR);
@@ -1786,7 +1786,7 @@ mt7530_port_mdb_add(struct dsa_switch *ds, int port,
 	mt7530_fdb_write(priv, vid, 0, addr, 0, STATIC_EMP);
 	if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL)) {
 		regmap_read(priv->regmap, MT7530_ATRD, &val);
-		port_mask = (val >> PORT_MAP) & PORT_MAP_MASK;
+		port_mask = FIELD_GET(PORT_MAP_MASK, val);
 	}
 
 	port_mask |= BIT(port);
@@ -1815,7 +1815,7 @@ mt7530_port_mdb_del(struct dsa_switch *ds, int port,
 	mt7530_fdb_write(priv, vid, 0, addr, 0, STATIC_EMP);
 	if (!mt7530_fdb_cmd(priv, MT7530_FDB_READ, NULL)) {
 		regmap_read(priv->regmap, MT7530_ATRD, &val);
-		port_mask = (val >> PORT_MAP) & PORT_MAP_MASK;
+		port_mask = FIELD_GET(PORT_MAP_MASK, val);
 	}
 
 	port_mask &= ~BIT(port);
@@ -1923,7 +1923,7 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid,
 
 	regmap_read(priv->regmap, MT7530_VAWD1, &val);
 
-	entry->old_members = (val >> PORT_MEM_SHFT) & PORT_MEM_MASK;
+	entry->old_members = FIELD_GET(PORT_MEM_MASK, val);
 
 	/* Manipulate entry */
 	vlan_op(priv, entry);
@@ -2436,7 +2436,7 @@ mt7530_setup(struct dsa_switch *ds)
 	}
 
 	regmap_read(priv->regmap, MT7530_CREV, &id);
-	id >>= CHIP_NAME_SHIFT;
+	id = FIELD_GET(CHIP_NAME_MASK, id);
 	if (id != MT7530_ID) {
 		dev_err(priv->dev, "chip %x can't be supported\n", id);
 		return -ENODEV;
@@ -2679,7 +2679,7 @@ mt7531_setup(struct dsa_switch *ds)
 	}
 
 	regmap_read(priv->regmap, MT7531_CREV, &id);
-	id >>= CHIP_NAME_SHIFT;
+	id = FIELD_GET(CHIP_NAME_MASK, id);
 
 	if (id != MT7531_ID) {
 		dev_err(priv->dev, "chip %x can't be supported\n", id);
drivers/net/dsa/mt7530.h +116 -92

410 lines not on this page · raw message

Signature · 1 line
2.54.0
maintainer
#9 lore raw

Re: [PATCH net-next v2 6/8] net: dsa: mt7530: convert to use field accessor macros

This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
---
net: dsa: mt7530: convert to use field accessor macros

This patch converts the mt7530 DSA driver to use FIELD_GET and FIELD_PREP instead of open-coded shift/mask constructs, and replaces a 0x1f literal in the PHY address helper with (PHY_MAX_ADDR - 1).

Quoted 4 lines
drivers/net/dsa/mt7530.c +0 -0
index 4168adca949f..dcf72ab0cd66 100644

[ ... ]

drivers/net/dsa/mt7530.c mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid) · line 1558

Quoted 6 lines
drivers/net/dsa/mt7530.c +1 -1
@@ -1555,7 +1555,7 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
 	u32 val;
 	int ret;
 
-	val = VTCR_BUSY | VTCR_FUNC(cmd) | vid;
+	val = VTCR_BUSY | VTCR_FUNC(cmd) | VTCR_VID(vid);

[Low]
The commit message describes this change as a refactor:
"Use FIELD_GET and FIELD_PREP instead of open-coding register fields."

The mt7530_vlan_cmd() hunk is not quite a pure conversion though. The previous code OR'd in vid directly:

    val = VTCR_BUSY | VTCR_FUNC(cmd) | vid;

while the new code applies a GENMASK(11, 0) truncation through VTCR_VID():

    val = VTCR_BUSY | VTCR_FUNC(cmd) | VTCR_VID(vid);

Spec-conformant VIDs always fit in 12 bits, so the runtime result is the same, but the value is now masked where it previously was not.

A related observation is that the prior VTCR_VID and ATC_HASH macros referenced an undeclared x:

    #define  ATC_HASH                       (((x) & 0xfff) << 16)
    #define  VTCR_VID                       ((x) & 0xfff)

so they could not have been used by any caller before this patch. Adding the (x) parameter and a GENMASK is effectively a fix for those macros.

Would it be worth calling out the VTCR_VID() truncation and the macro parameter fix in the commit message, or splitting them into a separate patch so the conversion remains a no-op refactor?

Quoted 1 line
drivers/net/dsa/mt7530.h +0 -0

[ ... ]

Quoted 4 lines
drivers/net/dsa/mt7530.h +0 -0
@@ -171,32 +176,29 @@ enum mt7530_fdb_cmd {
 
 /* Registers for table search read address */
 #define MT7530_TSRA1			0x84

[ ... ]

drivers/net/dsa/mt7530.h enum mt7530_fdb_cmd { · line 192

Quoted 10 lines

#define MT7530_ATRD 0x8C

drivers/net/dsa/mt7530.h +3 -6
-#define	 AGE_TIMER			24
-#define  AGE_TIMER_MASK			0xff
-#define  PORT_MAP			4
-#define  PORT_MAP_MASK			0xff
-#define  ENT_STATUS			2
-#define  ENT_STATUS_MASK		0x3
+#define  AGE_TIMER_RD_MASK		GENMASK(31, 24)
+#define  PORT_MAP_MASK			GENMASK(11, 4)
+#define  ENT_STATUS_MASK		GENMASK(3, 2)

[Low]
The mask formerly named AGE_TIMER_MASK is renamed to AGE_TIMER_RD_MASK, but the same mask is consumed on both the read and write paths (mt7530_fdb_read() and mt7530_fdb_write()), so the _RD_ infix reads as "read-side only".

Its sibling fields in the same MT7530_ATRD register keep plain names (PORT_MAP_MASK, ENT_STATUS_MASK), and the related ageing counters in MT7530_AAC use AGE_CNT_MASK / AGE_UNIT_MASK rather than an _RD_ disambiguation.

Was the _RD_ infix intentional, and if so, what does it disambiguate from?

[ ... ]

drivers/net/dsa/mt7530.h enum mt7531_xtal_fsel { · line 748

Quoted 6 lines
drivers/net/dsa/mt7530.h +1 -1
@@ -731,38 +745,48 @@ enum mt7531_xtal_fsel {
 #define  MT7531_PHY_PLL_OFF		BIT(5)
 #define  MT7531_PHY_PLL_BYPASS_MODE	BIT(4)
 
-#define MT753X_CTRL_PHY_ADDR(addr)	((addr + 1) & 0x1f)
+#define MT753X_CTRL_PHY_ADDR(addr)	(((addr) + 1) & (PHY_MAX_ADDR - 1))

[Low]
The replacement evaluates to the same value today, since PHY_MAX_ADDR is 32 in include/linux/phy.h and PHY_MAX_ADDR - 1 is 0x1f, and both forms ultimately derive from the IEEE 802.3 5-bit MDIO PHY-address field.

The original 0x1f represented the width of the hardware register field, while PHY_MAX_ADDR - 1 represents the maximum software PHY address minus one and additionally relies on PHY_MAX_ADDR being a power of two, which phy.h does not formally guarantee.

Given the rest of the patch introduces explicit GENMASK based field masks, would something like:

    #define MT753X_CTRL_PHY_ADDR_MASK GENMASK(4, 0)
    #define MT753X_CTRL_PHY_ADDR(addr) FIELD_PREP(MT753X_CTRL_PHY_ADDR_MASK, (addr) + 1)

be more consistent and make the hardware-field intent clearer?

author maintainer
#10 lore raw

PATCH v2 7/8 net: dsa: mt7530: implement port_fast_age

In reply to Daniel Golle

Implement the .port_fast_age DSA operation by flushing all non-static (dynamically learned) MAC address entries from the address table.

The switch does not offer a combined "non-static AND per-port" match mode, so flush all dynamic entries globally. This is consistent with what other DSA drivers do (b53, realtek) and relearning is fast.

---
v2: no changes

 drivers/net/dsa/mt7530.c | 16 ++++++++++++++++
 drivers/net/dsa/mt7530.h |  1 +
 2 files changed, 17 insertions(+)
drivers/net/dsa/mt7530.c +16 -0
index dcf72ab0cd66..c96420c291d5 100644
@@ -193,6 +193,21 @@ mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 	return 0;
 }
 
+static void mt7530_port_fast_age(struct dsa_switch *ds, int port)
+{
+	struct mt7530_priv *priv = ds->priv;
+	struct mt7530_dummy_poll p;
+	u32 val;
+
+	/* Flush all non-static MAC address entries */
+	val = ATC_BUSY | ATC_MAT_NON_STATIC_MAC | MT7530_FDB_FLUSH;
+	regmap_write(priv->regmap, MT7530_ATC, val);
+
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_ATC);
+	readx_poll_timeout(mt7530_mii_poll, &p, val,
+			   !(val & ATC_BUSY), 20, 20000);
+}
+
 static void
 mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
 {
@@ -3319,6 +3334,7 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.port_bridge_flags	= mt7530_port_bridge_flags,
 	.port_bridge_join	= mt7530_port_bridge_join,
 	.port_bridge_leave	= mt7530_port_bridge_leave,
+	.port_fast_age		= mt7530_port_fast_age,
 	.port_fdb_add		= mt7530_port_fdb_add,
 	.port_fdb_del		= mt7530_port_fdb_del,
 	.port_fdb_dump		= mt7530_port_fdb_dump,
drivers/net/dsa/mt7530.h +1 -0
index abf19aa69520..decad7a93dbd 100644
@@ -165,6 +165,7 @@ enum mt753x_to_cpu_fw {
 #define  ATC_MAT_MASK			GENMASK(11, 8)
 #define  ATC_MAT(x)			FIELD_PREP(ATC_MAT_MASK, x)
 #define  ATC_MAT_MACTAB			ATC_MAT(0)
+#define  ATC_MAT_NON_STATIC_MAC	ATC_MAT(4)
 
 enum mt7530_fdb_cmd {
 	MT7530_FDB_READ	= 0,
Signature · 1 line
2.54.0
maintainer
#11 lore raw

Re: [PATCH net-next v2 7/8] net: dsa: mt7530: implement port_fast_age

This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
---
Subject: net: dsa: mt7530: implement port_fast_age

This change adds a .port_fast_age operation for the mt7530 DSA driver. The implementation triggers an ATC flush of all non-static MAC address entries and polls ATC_BUSY for completion.

Quoted 7 lines
drivers/net/dsa/mt7530.c +0 -0
index dcf72ab0cd66..c96420c291d5 100644
@@ -193,6 +193,21 @@ mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 	return 0;
 }

drivers/net/dsa/mt7530.c mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp) · line 204

Quoted 10 lines
drivers/net/dsa/mt7530.c +9 -0
 
+static void mt7530_port_fast_age(struct dsa_switch *ds, int port)
+{
+	struct mt7530_priv *priv = ds->priv;
+	struct mt7530_dummy_poll p;
+	u32 val;
+
+	/* Flush all non-static MAC address entries */
+	val = ATC_BUSY | ATC_MAT_NON_STATIC_MAC | MT7530_FDB_FLUSH;
+	regmap_write(priv->regmap, MT7530_ATC, val);

[High]
Should this transaction be serialised against priv->reg_mutex?

Every other accessor that touches MT7530_ATC wraps the write+poll sequence in mutex_lock(&priv->reg_mutex)/mutex_unlock(&priv->reg_mutex). For example mt7530_port_fdb_add(), mt7530_port_fdb_del(), mt7530_port_fdb_dump(), mt7530_port_mdb_add() and mt7530_port_mdb_del() all hold reg_mutex around the call to mt7530_fdb_cmd():

	mutex_lock(&priv->reg_mutex);
	mt7530_fdb_write(priv, vid, port_mask, addr, -1, STATIC_ENT);
	ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, NULL);
	mutex_unlock(&priv->reg_mutex);

If .port_fast_age runs concurrently with one of those operations, can the two write-then-busy-poll sequences on the same MT7530_ATC register overlap?

Could one thread's poll loop observe ATC_BUSY clearing for the other thread's command, leading mt7530_port_fdb_dump() to read stale ATRD data, or could one thread's freshly written ATC command be overwritten by the other before the controller has executed it?

drivers/net/dsa/mt7530.c mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp) · line 209

Quoted 5 lines
drivers/net/dsa/mt7530.c +5 -0
+
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_ATC);
+	readx_poll_timeout(mt7530_mii_poll, &p, val,
+			   !(val & ATC_BUSY), 20, 20000);
+}

[Medium]
Is there a reason the return value of readx_poll_timeout() is ignored here?

The structurally identical helper mt7530_fdb_cmd() in the same file checks the result and emits a diagnostic on timeout:

	ret = readx_poll_timeout(mt7530_mii_poll, &p, val,
				 !(val & ATC_BUSY), 20, 20000);
	if (ret < 0) {
		dev_err(priv->dev, "reset timeout\n");
		return ret;
	}

Peer DSA drivers also log on failure (b53_br_fast_age prints "fast ageing failed"; rtl83xx_port_fast_age prints "failed to fast age on port %d: %d").

If the ATC engine fails to clear ATC_BUSY within the 20ms timeout, mt7530_port_fast_age() returns silently and a subsequent mt7530_fdb_cmd() will write a new command on top of the in-flight operation. Should this path emit dev_err() on timeout to make a wedged ATC engine visible to operators?

drivers/net/dsa/mt7530.c mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp) · line 213

Quoted 4 lines
drivers/net/dsa/mt7530.c +1 -0
+
 static void
 mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
 {

[ ... ]

[Low]
Is the claim in the commit message accurate?

"This is consistent with what other DSA drivers do (b53, realtek) and relearning is fast."

Both drivers cited actually implement per-port fast aging.

In b53, b53_br_fast_age() calls b53_fast_age_port() which writes the target port to B53_FAST_AGE_PORT_CTRL and selects FAST_AGE_PORT mode.

In realtek, rtl83xx_port_fast_age() calls priv->ops->l2_flush(priv, port, 0), and rtl8366rb_port_fast_age() flushes only BIT(port) via RTL8366RB_SECURITY_CTRL.

The mt7530 implementation flushes globally and ignores the port argument entirely, which is the opposite of the cited examples. The hardware-limitation rationale ("switch does not offer combined non-static AND per-port match mode") may be valid, but the b53/realtek comparison appears to be the wrong precedent.

A side effect not mentioned is that an STP transition on one port will flush dynamic FDB entries on every other port and every other bridge sharing the switch. Should this divergence from the typical .port_fast_age contract be called out?

drivers/net/dsa/mt7530.h enum mt753x_to_cpu_fw { · line 168

Quoted 9 lines
drivers/net/dsa/mt7530.h +1 -0
index abf19aa69520..decad7a93dbd 100644
@@ -165,6 +165,7 @@ enum mt753x_to_cpu_fw {
 #define  ATC_MAT_MASK			GENMASK(11, 8)
 #define  ATC_MAT(x)			FIELD_PREP(ATC_MAT_MASK, x)
 #define  ATC_MAT_MACTAB			ATC_MAT(0)
+#define  ATC_MAT_NON_STATIC_MAC	ATC_MAT(4)

[Low]
The new ATC_MAT_NON_STATIC_MAC define uses a single tab between the macro name and its value, while the surrounding ATC_MAT_* macros use multiple tabs and align the values at the same column. Should this follow the existing alignment?

Quoted 3 lines
drivers/net/dsa/mt7530.h +0 -0
 enum mt7530_fdb_cmd {
 	MT7530_FDB_READ	= 0,
author maintainer
#12 lore raw

PATCH v2 8/8 net: dsa: mt7530: implement port_change_conduit op

In reply to Daniel Golle

Allow changing the CPU port affinity of user ports at runtime via the IFLA_DSA_CONDUIT netlink attribute. This updates the port matrix to forward to the new CPU port instead of the old one.

Limit the operation to MT7531. There, trapped link-local frames follow the per-port affinity, as the MT7531_CPU_PMAP destination mask is further restricted by the port matrix. A conduit change is hence fully honoured by the hardware, for regular traffic as well as for trapped frames.

The MT7530 switch, including the variant embedded in the MT7621 SoC, instead traps frames to the single CPU port set in the CPU_PORT field of the MFC register, regardless of the affinity of the inbound user port. With user ports affine to different CPU ports there is no correct value for that field, so per-port CPU affinity cannot be fully implemented for trapped frames. Routing a WAN port via the second SoC GMAC is conventionally covered by the PHY muxing feature on these switches, which bypasses the switch fabric and does not involve a CPU port at all.

The switches on the MT7988, EN7581 and AN7583 SoCs only have a single CPU port, leaving no other conduit to change to.

---
v2: extend commit message

 drivers/net/dsa/mt7530.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
drivers/net/dsa/mt7530.c +29 -0
index c96420c291d5..2f3e734b9f53 100644
@@ -3206,6 +3206,34 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt753x_port_change_conduit(struct dsa_switch *ds, int port,
+			   struct net_device *conduit,
+			   struct netlink_ext_ack *extack)
+{
+	struct dsa_port *new_cpu_dp = conduit->dsa_ptr;
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct mt7530_priv *priv = ds->priv;
+
+	if (priv->id != ID_MT7531)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&priv->reg_mutex);
+
+	/* dp->cpu_dp still points to the old CPU port */
+	priv->ports[port].pm &= ~PCR_MATRIX(BIT(dp->cpu_dp->index));
+	priv->ports[port].pm |= PCR_MATRIX(BIT(new_cpu_dp->index));
+	if (priv->ports[port].enable)
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+				   PCR_MATRIX_MASK, priv->ports[port].pm);
+
+	mutex_unlock(&priv->reg_mutex);
+
+	mt7530_port_fast_age(ds, port);
+
+	return 0;
+}
+
 static void
 mt753x_conduit_state_change(struct dsa_switch *ds,
 			    const struct net_device *conduit,
@@ -3317,6 +3345,7 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.setup			= mt753x_setup,
 	.teardown		= mt753x_teardown,
 	.preferred_default_local_cpu_port = mt753x_preferred_default_local_cpu_port,
+	.port_change_conduit	= mt753x_port_change_conduit,
 	.get_strings		= mt7530_get_strings,
 	.get_ethtool_stats	= mt7530_get_ethtool_stats,
 	.get_sset_count		= mt7530_get_sset_count,
Signature · 1 line
2.54.0
author maintainer Acked-by Chester A. Unal on v2 8/8
#13 lore raw

Re: [PATCH net-next v2 8/8] net: dsa: mt7530: implement port_change_conduit op

Quoted 20 lines · Daniel Golle

Allow changing the CPU port affinity of user ports at runtime via the IFLA_DSA_CONDUIT netlink attribute. This updates the port matrix to forward to the new CPU port instead of the old one.

Limit the operation to MT7531. There, trapped link-local frames follow the per-port affinity, as the MT7531_CPU_PMAP destination mask is further restricted by the port matrix. A conduit change is hence fully honoured by the hardware, for regular traffic as well as for trapped frames.

The MT7530 switch, including the variant embedded in the MT7621 SoC, instead traps frames to the single CPU port set in the CPU_PORT field of the MFC register, regardless of the affinity of the inbound user port. With user ports affine to different CPU ports there is no correct value for that field, so per-port CPU affinity cannot be fully implemented for trapped frames. Routing a WAN port via the second SoC GMAC is conventionally covered by the PHY muxing feature on these switches, which bypasses the switch fabric and does not involve a CPU port at all.

Quoted 4 lines

The switches on the MT7988, EN7581 and AN7583 SoCs only have a single CPU port, leaving no other conduit to change to.

I forgot to include the previously received

See also:

https://patchwork.kernel.org/comment/27003848/

https://lore.kernel.org/all/[email protected]/

maintainer
#14 lore raw

Re: [PATCH net-next v2 8/8] net: dsa: mt7530: implement port_change_conduit op

In reply to Daniel Golle

This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
---
net: dsa: mt7530: implement port_change_conduit op

Implements the port_change_conduit operation for the mt7530 driver, updating the port matrix to forward to a new CPU port. The op is restricted to MT7531 because MT7530/MT7621 cannot honour per-port CPU affinity for trapped frames and the MT7988/EN7581/AN7583 switches have only a single CPU port.

Quoted 9 lines
drivers/net/dsa/mt7530.c +1 -0
index c96420c291d5..2f3e734b9f53 100644
@@ -3206,6 +3206,34 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int

drivers/net/dsa/mt7530.c static int mt753x_set_mac_eee(struct dsa_switch *ds, int port, · line 3219

Quoted 10 lines
drivers/net/dsa/mt7530.c +10 -0
+mt753x_port_change_conduit(struct dsa_switch *ds, int port,
+			   struct net_device *conduit,
+			   struct netlink_ext_ack *extack)
+{
+	struct dsa_port *new_cpu_dp = conduit->dsa_ptr;
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct mt7530_priv *priv = ds->priv;
+
+	if (priv->id != ID_MT7531)
+		return -EOPNOTSUPP;

[Low]
Should this branch populate the supplied extack with NL_SET_ERR_MSG_MOD() before returning -EOPNOTSUPP?

Because port_change_conduit is registered in the shared mt7530_switch_ops table, the DSA core's fallback message in dsa_user_change_conduit() ("Driver does not support changing DSA conduit") is bypassed; that branch only runs when ds->ops->port_change_conduit is NULL. Neither dsa_port_change_conduit() nor __dsa_port_change_conduit() in net/dsa/port.c synthesize an extack on callback failure either.

The result is that on MT7530, MT7621, MT7988, EN7581 and AN7583, userspace invoking IFLA_DSA_CONDUIT receives EOPNOTSUPP with an empty extack, with none of the per-chip context that the commit message itself spells out (single CPU port; MT7530 MFC CPU_PORT trapping limitation). Would a short NL_SET_ERR_MSG_MOD() naming the reason be appropriate here?

Quoted 16 lines
drivers/net/dsa/mt7530.c +16 -0
+
+	mutex_lock(&priv->reg_mutex);
+
+	/* dp->cpu_dp still points to the old CPU port */
+	priv->ports[port].pm &= ~PCR_MATRIX(BIT(dp->cpu_dp->index));
+	priv->ports[port].pm |= PCR_MATRIX(BIT(new_cpu_dp->index));
+	if (priv->ports[port].enable)
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+				   PCR_MATRIX_MASK, priv->ports[port].pm);
+
+	mutex_unlock(&priv->reg_mutex);
+
+	mt7530_port_fast_age(ds, port);
+
+	return 0;
+}