[net-next PATCH v2 0/9] Multiple cleanup and feature for qca8k

[net-next PATCH v2 0/9] Multiple cleanup and feature for qca8k

[Ansuel Smith]

This is a reduced version of the old massive series.
Refer to the changelog to know what is removed from this.

THIS IS BASED ON net-next WITH THE 2 FIXES FROM net ALREADY REVIEWED
net: dsa: qca8k: fix MTU calculation
net: dsa: qca8k: fix internal delay applied to the wrong PAD config

We clean and convert the driver to GENMASK FIELD_PREP to clean multiple
use of various naming scheme. (example we have a mix of _MASK, _S _M,
and various name) The idea is to ""simplify"" and remove some shift and
data handling by using FIELD API. 
The patch contains various checkpatch warning and are ignored to not
create more mess in the header file. (fixing the too long line warning
would results in regs definition less readable)

We conver the driver to regmap API as ipq40xx SoC is based on the same
reg structure and we need to generilize the read/write access to split
the driver to commond and specific code.

We also add a minor patch for MIB counter as qca8337 is missing 2 extra
counter, support for mdb and ageing settings.

v2:
- Move regmap init to sw_probe instead of moving switch id check.
- Removed LAGs, mirror extra features will come later in another
  smaller series.
- Squash 2 ageing patch
- Add more description about the regmap patch
- Rework mdb patch to do operation under the same lock

Ansuel Smith (9):
  net: dsa: qca8k: remove redundant check in parse_port_config
  net: dsa: qca8k: convert to GENMASK/FIELD_PREP/FIELD_GET
  net: dsa: qca8k: remove extra mutex_init in qca8k_setup
  net: dsa: qca8k: move regmap init in probe and set it mandatory
  net: dsa: qca8k: convert qca8k to regmap helper
  net: dsa: qca8k: add additional MIB counter and make it dynamic
  net: dsa: qca8k: add support for port fast aging
  net: dsa: qca8k: add set_ageing_time support
  net: dsa: qca8k: add support for mdb_add/del

 drivers/net/dsa/qca8k.c | 531 ++++++++++++++++++++++++----------------
 drivers/net/dsa/qca8k.h | 161 ++++++------
 2 files changed, 416 insertions(+), 276 deletions(-)

-- 
2.32.0

[net-next PATCH v2 1/9] net: dsa: qca8k: remove redundant check in parse_port_config

[Ansuel Smith]

The very next check for port 0 and 6 already makes sure we don't go out
of bounds with the ports_config delay table.
Remove the redundant check.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/qca8k.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 147ca39531a3..783298cf859f 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -983,7 +983,7 @@ qca8k_parse_port_config(struct qca8k_priv *priv)
 	u32 delay;
 
 	/* We have 2 CPU port. Check them */
-	for (port = 0; port < QCA8K_NUM_PORTS && cpu_port_index < QCA8K_NUM_CPU_PORTS; port++) {
+	for (port = 0; port < QCA8K_NUM_PORTS; port++) {
 		/* Skip every other port */
 		if (port != 0 && port != 6)
 			continue;
-- 
2.32.0

[net-next PATCH v2 2/9] net: dsa: qca8k: convert to GENMASK/FIELD_PREP/FIELD_GET

[Ansuel Smith]

Convert and try to standardize bit fields using
GENMASK/FIELD_PREP/FIELD_GET macros. Rework some logic to support the
standard macro and tidy things up. No functional change intended.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c |  98 ++++++++++++-------------
 drivers/net/dsa/qca8k.h | 153 ++++++++++++++++++++++------------------
 2 files changed, 130 insertions(+), 121 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 783298cf859f..6783a3c2620f 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 #include <linux/netdevice.h>
+#include <linux/bitfield.h>
 #include <net/dsa.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
@@ -319,18 +320,18 @@ qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
 	}
 
 	/* vid - 83:72 */
-	fdb->vid = (reg[2] >> QCA8K_ATU_VID_S) & QCA8K_ATU_VID_M;
+	fdb->vid = FIELD_GET(QCA8K_ATU_VID_MASK, reg[2]);
 	/* aging - 67:64 */
-	fdb->aging = reg[2] & QCA8K_ATU_STATUS_M;
+	fdb->aging = FIELD_GET(QCA8K_ATU_STATUS_MASK, reg[2]);
 	/* portmask - 54:48 */
-	fdb->port_mask = (reg[1] >> QCA8K_ATU_PORT_S) & QCA8K_ATU_PORT_M;
+	fdb->port_mask = FIELD_GET(QCA8K_ATU_PORT_MASK, reg[1]);
 	/* mac - 47:0 */
-	fdb->mac[0] = (reg[1] >> QCA8K_ATU_ADDR0_S) & 0xff;
-	fdb->mac[1] = reg[1] & 0xff;
-	fdb->mac[2] = (reg[0] >> QCA8K_ATU_ADDR2_S) & 0xff;
-	fdb->mac[3] = (reg[0] >> QCA8K_ATU_ADDR3_S) & 0xff;
-	fdb->mac[4] = (reg[0] >> QCA8K_ATU_ADDR4_S) & 0xff;
-	fdb->mac[5] = reg[0] & 0xff;
+	fdb->mac[0] = FIELD_GET(QCA8K_ATU_ADDR0_MASK, reg[1]);
+	fdb->mac[1] = FIELD_GET(QCA8K_ATU_ADDR1_MASK, reg[1]);
+	fdb->mac[2] = FIELD_GET(QCA8K_ATU_ADDR2_MASK, reg[0]);
+	fdb->mac[3] = FIELD_GET(QCA8K_ATU_ADDR3_MASK, reg[0]);
+	fdb->mac[4] = FIELD_GET(QCA8K_ATU_ADDR4_MASK, reg[0]);
+	fdb->mac[5] = FIELD_GET(QCA8K_ATU_ADDR5_MASK, reg[0]);
 
 	return 0;
 }
@@ -343,18 +344,18 @@ qca8k_fdb_write(struct qca8k_priv *priv, u16 vid, u8 port_mask, const u8 *mac,
 	int i;
 
 	/* vid - 83:72 */
-	reg[2] = (vid & QCA8K_ATU_VID_M) << QCA8K_ATU_VID_S;
+	reg[2] = FIELD_PREP(QCA8K_ATU_VID_MASK, vid);
 	/* aging - 67:64 */
-	reg[2] |= aging & QCA8K_ATU_STATUS_M;
+	reg[2] |= FIELD_PREP(QCA8K_ATU_STATUS_MASK, aging);
 	/* portmask - 54:48 */
-	reg[1] = (port_mask & QCA8K_ATU_PORT_M) << QCA8K_ATU_PORT_S;
+	reg[1] = FIELD_PREP(QCA8K_ATU_PORT_MASK, port_mask);
 	/* mac - 47:0 */
-	reg[1] |= mac[0] << QCA8K_ATU_ADDR0_S;
-	reg[1] |= mac[1];
-	reg[0] |= mac[2] << QCA8K_ATU_ADDR2_S;
-	reg[0] |= mac[3] << QCA8K_ATU_ADDR3_S;
-	reg[0] |= mac[4] << QCA8K_ATU_ADDR4_S;
-	reg[0] |= mac[5];
+	reg[1] |= FIELD_PREP(QCA8K_ATU_ADDR0_MASK, mac[0]);
+	reg[1] |= FIELD_PREP(QCA8K_ATU_ADDR1_MASK, mac[1]);
+	reg[0] |= FIELD_PREP(QCA8K_ATU_ADDR2_MASK, mac[2]);
+	reg[0] |= FIELD_PREP(QCA8K_ATU_ADDR3_MASK, mac[3]);
+	reg[0] |= FIELD_PREP(QCA8K_ATU_ADDR4_MASK, mac[4]);
+	reg[0] |= FIELD_PREP(QCA8K_ATU_ADDR5_MASK, mac[5]);
 
 	/* load the array into the ARL table */
 	for (i = 0; i < 3; i++)
@@ -372,7 +373,7 @@ qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port)
 	reg |= cmd;
 	if (port >= 0) {
 		reg |= QCA8K_ATU_FUNC_PORT_EN;
-		reg |= (port & QCA8K_ATU_FUNC_PORT_M) << QCA8K_ATU_FUNC_PORT_S;
+		reg |= FIELD_PREP(QCA8K_ATU_FUNC_PORT_MASK, port);
 	}
 
 	/* Write the function register triggering the table access */
@@ -454,7 +455,7 @@ qca8k_vlan_access(struct qca8k_priv *priv, enum qca8k_vlan_cmd cmd, u16 vid)
 	/* Set the command and VLAN index */
 	reg = QCA8K_VTU_FUNC1_BUSY;
 	reg |= cmd;
-	reg |= vid << QCA8K_VTU_FUNC1_VID_S;
+	reg |= FIELD_PREP(QCA8K_VTU_FUNC1_VID_MASK, vid);
 
 	/* Write the function register triggering the table access */
 	ret = qca8k_write(priv, QCA8K_REG_VTU_FUNC1, reg);
@@ -500,13 +501,11 @@ qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged)
 	if (ret < 0)
 		goto out;
 	reg |= QCA8K_VTU_FUNC0_VALID | QCA8K_VTU_FUNC0_IVL_EN;
-	reg &= ~(QCA8K_VTU_FUNC0_EG_MODE_MASK << QCA8K_VTU_FUNC0_EG_MODE_S(port));
+	reg &= ~QCA8K_VTU_FUNC0_EG_MODE_PORT_MASK(port);
 	if (untagged)
-		reg |= QCA8K_VTU_FUNC0_EG_MODE_UNTAG <<
-				QCA8K_VTU_FUNC0_EG_MODE_S(port);
+		reg |= QCA8K_VTU_FUNC0_EG_MODE_PORT_UNTAG(port);
 	else
-		reg |= QCA8K_VTU_FUNC0_EG_MODE_TAG <<
-				QCA8K_VTU_FUNC0_EG_MODE_S(port);
+		reg |= QCA8K_VTU_FUNC0_EG_MODE_PORT_TAG(port);
 
 	ret = qca8k_write(priv, QCA8K_REG_VTU_FUNC0, reg);
 	if (ret)
@@ -534,15 +533,13 @@ qca8k_vlan_del(struct qca8k_priv *priv, u8 port, u16 vid)
 	ret = qca8k_read(priv, QCA8K_REG_VTU_FUNC0, &reg);
 	if (ret < 0)
 		goto out;
-	reg &= ~(3 << QCA8K_VTU_FUNC0_EG_MODE_S(port));
-	reg |= QCA8K_VTU_FUNC0_EG_MODE_NOT <<
-			QCA8K_VTU_FUNC0_EG_MODE_S(port);
+	reg &= ~QCA8K_VTU_FUNC0_EG_MODE_PORT_MASK(port);
+	reg |= QCA8K_VTU_FUNC0_EG_MODE_PORT_NOT(port);
 
 	/* Check if we're the last member to be removed */
 	del = true;
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
-		mask = QCA8K_VTU_FUNC0_EG_MODE_NOT;
-		mask <<= QCA8K_VTU_FUNC0_EG_MODE_S(i);
+		mask = QCA8K_VTU_FUNC0_EG_MODE_PORT_NOT(i);
 
 		if ((reg & mask) != mask) {
 			del = false;
@@ -1014,7 +1011,7 @@ qca8k_parse_port_config(struct qca8k_priv *priv)
 				 mode == PHY_INTERFACE_MODE_RGMII_TXID)
 				delay = 1;
 
-			if (delay > QCA8K_MAX_DELAY) {
+			if (!FIELD_FIT(QCA8K_PORT_PAD_RGMII_TX_DELAY_MASK, delay)) {
 				dev_err(priv->dev, "rgmii tx delay is limited to a max value of 3ns, setting to the max value");
 				delay = 3;
 			}
@@ -1030,7 +1027,7 @@ qca8k_parse_port_config(struct qca8k_priv *priv)
 				 mode == PHY_INTERFACE_MODE_RGMII_RXID)
 				delay = 2;
 
-			if (delay > QCA8K_MAX_DELAY) {
+			if (!FIELD_FIT(QCA8K_PORT_PAD_RGMII_RX_DELAY_MASK, delay)) {
 				dev_err(priv->dev, "rgmii rx delay is limited to a max value of 3ns, setting to the max value");
 				delay = 3;
 			}
@@ -1141,8 +1138,8 @@ qca8k_setup(struct dsa_switch *ds)
 		/* Enable QCA header mode on all cpu ports */
 		if (dsa_is_cpu_port(ds, i)) {
 			ret = qca8k_write(priv, QCA8K_REG_PORT_HDR_CTRL(i),
-					  QCA8K_PORT_HDR_CTRL_ALL << QCA8K_PORT_HDR_CTRL_TX_S |
-					  QCA8K_PORT_HDR_CTRL_ALL << QCA8K_PORT_HDR_CTRL_RX_S);
+					  FIELD_PREP(QCA8K_PORT_HDR_CTRL_TX_MASK, QCA8K_PORT_HDR_CTRL_ALL) |
+					  FIELD_PREP(QCA8K_PORT_HDR_CTRL_RX_MASK, QCA8K_PORT_HDR_CTRL_ALL));
 			if (ret) {
 				dev_err(priv->dev, "failed enabling QCA header mode");
 				return ret;
@@ -1159,10 +1156,10 @@ qca8k_setup(struct dsa_switch *ds)
 	 * for igmp, unknown, multicast and broadcast packet
 	 */
 	ret = qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
-			  BIT(cpu_port) << QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_S |
-			  BIT(cpu_port) << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
-			  BIT(cpu_port) << QCA8K_GLOBAL_FW_CTRL1_MC_DP_S |
-			  BIT(cpu_port) << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
+			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_MASK, BIT(cpu_port)) |
+			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_BC_DP_MASK, BIT(cpu_port)) |
+			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_MC_DP_MASK, BIT(cpu_port)) |
+			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_UC_DP_MASK, BIT(cpu_port)));
 	if (ret)
 		return ret;
 
@@ -1180,8 +1177,6 @@ qca8k_setup(struct dsa_switch *ds)
 
 		/* Individual user ports get connected to CPU port only */
 		if (dsa_is_user_port(ds, i)) {
-			int shift = 16 * (i % 2);
-
 			ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
 					QCA8K_PORT_LOOKUP_MEMBER,
 					BIT(cpu_port));
@@ -1198,8 +1193,8 @@ qca8k_setup(struct dsa_switch *ds)
 			 * default egress vid
 			 */
 			ret = qca8k_rmw(priv, QCA8K_EGRESS_VLAN(i),
-					0xfff << shift,
-					QCA8K_PORT_VID_DEF << shift);
+					QCA8K_EGREES_VLAN_PORT_MASK(i),
+					QCA8K_EGREES_VLAN_PORT(i, QCA8K_PORT_VID_DEF));
 			if (ret)
 				return ret;
 
@@ -1246,7 +1241,7 @@ qca8k_setup(struct dsa_switch *ds)
 			QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
 			QCA8K_PORT_HOL_CTRL1_WRED_EN;
 			qca8k_rmw(priv, QCA8K_REG_PORT_HOL_CTRL1(i),
-				  QCA8K_PORT_HOL_CTRL1_ING_BUF |
+				  QCA8K_PORT_HOL_CTRL1_ING_BUF_MASK |
 				  QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
 				  QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
 				  QCA8K_PORT_HOL_CTRL1_WRED_EN,
@@ -1269,8 +1264,8 @@ qca8k_setup(struct dsa_switch *ds)
 		mask = QCA8K_GLOBAL_FC_GOL_XON_THRES(288) |
 		       QCA8K_GLOBAL_FC_GOL_XOFF_THRES(496);
 		qca8k_rmw(priv, QCA8K_REG_GLOBAL_FC_THRESH,
-			  QCA8K_GLOBAL_FC_GOL_XON_THRES_S |
-			  QCA8K_GLOBAL_FC_GOL_XOFF_THRES_S,
+			  QCA8K_GLOBAL_FC_GOL_XON_THRES_MASK |
+			  QCA8K_GLOBAL_FC_GOL_XOFF_THRES_MASK,
 			  mask);
 	}
 
@@ -1916,11 +1911,11 @@ qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 
 	if (vlan_filtering) {
 		ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-				QCA8K_PORT_LOOKUP_VLAN_MODE,
+				QCA8K_PORT_LOOKUP_VLAN_MODE_MASK,
 				QCA8K_PORT_LOOKUP_VLAN_MODE_SECURE);
 	} else {
 		ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-				QCA8K_PORT_LOOKUP_VLAN_MODE,
+				QCA8K_PORT_LOOKUP_VLAN_MODE_MASK,
 				QCA8K_PORT_LOOKUP_VLAN_MODE_NONE);
 	}
 
@@ -1944,10 +1939,9 @@ qca8k_port_vlan_add(struct dsa_switch *ds, int port,
 	}
 
 	if (pvid) {
-		int shift = 16 * (port % 2);
-
 		ret = qca8k_rmw(priv, QCA8K_EGRESS_VLAN(port),
-				0xfff << shift, vlan->vid << shift);
+				QCA8K_EGREES_VLAN_PORT_MASK(port),
+				QCA8K_EGREES_VLAN_PORT(port, vlan->vid));
 		if (ret)
 			return ret;
 
@@ -2041,7 +2035,7 @@ static int qca8k_read_switch_id(struct qca8k_priv *priv)
 	if (ret < 0)
 		return -ENODEV;
 
-	id = QCA8K_MASK_CTRL_DEVICE_ID(val & QCA8K_MASK_CTRL_DEVICE_ID_MASK);
+	id = QCA8K_MASK_CTRL_DEVICE_ID(val);
 	if (id != data->id) {
 		dev_err(priv->dev, "Switch id detected %x but expected %x", id, data->id);
 		return -ENODEV;
@@ -2050,7 +2044,7 @@ static int qca8k_read_switch_id(struct qca8k_priv *priv)
 	priv->switch_id = id;
 
 	/* Save revision to communicate to the internal PHY driver */
-	priv->switch_revision = (val & QCA8K_MASK_CTRL_REV_ID_MASK);
+	priv->switch_revision = QCA8K_MASK_CTRL_REV_ID(val);
 
 	return 0;
 }
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 128b8cf85e08..085885275398 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -30,9 +30,9 @@
 /* Global control registers */
 #define QCA8K_REG_MASK_CTRL				0x000
 #define   QCA8K_MASK_CTRL_REV_ID_MASK			GENMASK(7, 0)
-#define   QCA8K_MASK_CTRL_REV_ID(x)			((x) >> 0)
+#define   QCA8K_MASK_CTRL_REV_ID(x)			FIELD_GET(QCA8K_MASK_CTRL_REV_ID_MASK, x)
 #define   QCA8K_MASK_CTRL_DEVICE_ID_MASK		GENMASK(15, 8)
-#define   QCA8K_MASK_CTRL_DEVICE_ID(x)			((x) >> 8)
+#define   QCA8K_MASK_CTRL_DEVICE_ID(x)			FIELD_GET(QCA8K_MASK_CTRL_DEVICE_ID_MASK, x)
 #define QCA8K_REG_PORT0_PAD_CTRL			0x004
 #define   QCA8K_PORT0_PAD_MAC06_EXCHANGE_EN		BIT(31)
 #define   QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE	BIT(19)
@@ -41,12 +41,11 @@
 #define QCA8K_REG_PORT6_PAD_CTRL			0x00c
 #define   QCA8K_PORT_PAD_RGMII_EN			BIT(26)
 #define   QCA8K_PORT_PAD_RGMII_TX_DELAY_MASK		GENMASK(23, 22)
-#define   QCA8K_PORT_PAD_RGMII_TX_DELAY(x)		((x) << 22)
+#define   QCA8K_PORT_PAD_RGMII_TX_DELAY(x)		FIELD_PREP(QCA8K_PORT_PAD_RGMII_TX_DELAY_MASK, x)
 #define   QCA8K_PORT_PAD_RGMII_RX_DELAY_MASK		GENMASK(21, 20)
-#define   QCA8K_PORT_PAD_RGMII_RX_DELAY(x)		((x) << 20)
+#define   QCA8K_PORT_PAD_RGMII_RX_DELAY(x)		FIELD_PREP(QCA8K_PORT_PAD_RGMII_RX_DELAY_MASK, x)
 #define	  QCA8K_PORT_PAD_RGMII_TX_DELAY_EN		BIT(25)
 #define   QCA8K_PORT_PAD_RGMII_RX_DELAY_EN		BIT(24)
-#define   QCA8K_MAX_DELAY				3
 #define   QCA8K_PORT_PAD_SGMII_EN			BIT(7)
 #define QCA8K_REG_PWS					0x010
 #define   QCA8K_PWS_POWER_ON_SEL			BIT(31)
@@ -68,10 +67,12 @@
 #define   QCA8K_MDIO_MASTER_READ			BIT(27)
 #define   QCA8K_MDIO_MASTER_WRITE			0
 #define   QCA8K_MDIO_MASTER_SUP_PRE			BIT(26)
-#define   QCA8K_MDIO_MASTER_PHY_ADDR(x)			((x) << 21)
-#define   QCA8K_MDIO_MASTER_REG_ADDR(x)			((x) << 16)
-#define   QCA8K_MDIO_MASTER_DATA(x)			(x)
+#define   QCA8K_MDIO_MASTER_PHY_ADDR_MASK		GENMASK(25, 21)
+#define   QCA8K_MDIO_MASTER_PHY_ADDR(x)			FIELD_PREP(QCA8K_MDIO_MASTER_PHY_ADDR_MASK, x)
+#define   QCA8K_MDIO_MASTER_REG_ADDR_MASK		GENMASK(20, 16)
+#define   QCA8K_MDIO_MASTER_REG_ADDR(x)			FIELD_PREP(QCA8K_MDIO_MASTER_REG_ADDR_MASK, x)
 #define   QCA8K_MDIO_MASTER_DATA_MASK			GENMASK(15, 0)
+#define   QCA8K_MDIO_MASTER_DATA(x)			FIELD_PREP(QCA8K_MDIO_MASTER_DATA_MASK, x)
 #define   QCA8K_MDIO_MASTER_MAX_PORTS			5
 #define   QCA8K_MDIO_MASTER_MAX_REG			32
 #define QCA8K_GOL_MAC_ADDR0				0x60
@@ -93,9 +94,7 @@
 #define   QCA8K_PORT_STATUS_FLOW_AUTO			BIT(12)
 #define QCA8K_REG_PORT_HDR_CTRL(_i)			(0x9c + (_i * 4))
 #define   QCA8K_PORT_HDR_CTRL_RX_MASK			GENMASK(3, 2)
-#define   QCA8K_PORT_HDR_CTRL_RX_S			2
 #define   QCA8K_PORT_HDR_CTRL_TX_MASK			GENMASK(1, 0)
-#define   QCA8K_PORT_HDR_CTRL_TX_S			0
 #define   QCA8K_PORT_HDR_CTRL_ALL			2
 #define   QCA8K_PORT_HDR_CTRL_MGMT			1
 #define   QCA8K_PORT_HDR_CTRL_NONE			0
@@ -105,10 +104,11 @@
 #define   QCA8K_SGMII_EN_TX				BIT(3)
 #define   QCA8K_SGMII_EN_SD				BIT(4)
 #define   QCA8K_SGMII_CLK125M_DELAY			BIT(7)
-#define   QCA8K_SGMII_MODE_CTRL_MASK			(BIT(22) | BIT(23))
-#define   QCA8K_SGMII_MODE_CTRL_BASEX			(0 << 22)
-#define   QCA8K_SGMII_MODE_CTRL_PHY			(1 << 22)
-#define   QCA8K_SGMII_MODE_CTRL_MAC			(2 << 22)
+#define   QCA8K_SGMII_MODE_CTRL_MASK			GENMASK(23, 22)
+#define   QCA8K_SGMII_MODE_CTRL(x)			FIELD_PREP(QCA8K_SGMII_MODE_CTRL_MASK, x)
+#define   QCA8K_SGMII_MODE_CTRL_BASEX			QCA8K_SGMII_MODE_CTRL(0x0)
+#define   QCA8K_SGMII_MODE_CTRL_PHY			QCA8K_SGMII_MODE_CTRL(0x1)
+#define   QCA8K_SGMII_MODE_CTRL_MAC			QCA8K_SGMII_MODE_CTRL(0x2)
 
 /* MAC_PWR_SEL registers */
 #define QCA8K_REG_MAC_PWR_SEL				0x0e4
@@ -121,100 +121,115 @@
 
 /* ACL registers */
 #define QCA8K_REG_PORT_VLAN_CTRL0(_i)			(0x420 + (_i * 8))
-#define   QCA8K_PORT_VLAN_CVID(x)			(x << 16)
-#define   QCA8K_PORT_VLAN_SVID(x)			x
+#define   QCA8K_PORT_VLAN_CVID_MASK			GENMASK(27, 16)
+#define   QCA8K_PORT_VLAN_CVID(x)			FIELD_PREP(QCA8K_PORT_VLAN_CVID_MASK, x)
+#define   QCA8K_PORT_VLAN_SVID_MASK			GENMASK(11, 0)
+#define   QCA8K_PORT_VLAN_SVID(x)			FIELD_PREP(QCA8K_PORT_VLAN_SVID_MASK, x)
 #define QCA8K_REG_PORT_VLAN_CTRL1(_i)			(0x424 + (_i * 8))
 #define QCA8K_REG_IPV4_PRI_BASE_ADDR			0x470
 #define QCA8K_REG_IPV4_PRI_ADDR_MASK			0x474
 
 /* Lookup registers */
 #define QCA8K_REG_ATU_DATA0				0x600
-#define   QCA8K_ATU_ADDR2_S				24
-#define   QCA8K_ATU_ADDR3_S				16
-#define   QCA8K_ATU_ADDR4_S				8
+#define   QCA8K_ATU_ADDR2_MASK				GENMASK(31, 24)
+#define   QCA8K_ATU_ADDR3_MASK				GENMASK(23, 16)
+#define   QCA8K_ATU_ADDR4_MASK				GENMASK(15, 8)
+#define   QCA8K_ATU_ADDR5_MASK				GENMASK(7, 0)
 #define QCA8K_REG_ATU_DATA1				0x604
-#define   QCA8K_ATU_PORT_M				0x7f
-#define   QCA8K_ATU_PORT_S				16
-#define   QCA8K_ATU_ADDR0_S				8
+#define   QCA8K_ATU_PORT_MASK				GENMASK(22, 16)
+#define   QCA8K_ATU_ADDR0_MASK				GENMASK(15, 8)
+#define   QCA8K_ATU_ADDR1_MASK				GENMASK(7, 0)
 #define QCA8K_REG_ATU_DATA2				0x608
-#define   QCA8K_ATU_VID_M				0xfff
-#define   QCA8K_ATU_VID_S				8
-#define   QCA8K_ATU_STATUS_M				0xf
+#define   QCA8K_ATU_VID_MASK				GENMASK(19, 8)
+#define   QCA8K_ATU_STATUS_MASK				GENMASK(3, 0)
 #define   QCA8K_ATU_STATUS_STATIC			0xf
 #define QCA8K_REG_ATU_FUNC				0x60c
 #define   QCA8K_ATU_FUNC_BUSY				BIT(31)
 #define   QCA8K_ATU_FUNC_PORT_EN			BIT(14)
 #define   QCA8K_ATU_FUNC_MULTI_EN			BIT(13)
 #define   QCA8K_ATU_FUNC_FULL				BIT(12)
-#define   QCA8K_ATU_FUNC_PORT_M				0xf
-#define   QCA8K_ATU_FUNC_PORT_S				8
+#define   QCA8K_ATU_FUNC_PORT_MASK			GENMASK(11, 8)
 #define QCA8K_REG_VTU_FUNC0				0x610
 #define   QCA8K_VTU_FUNC0_VALID				BIT(20)
 #define   QCA8K_VTU_FUNC0_IVL_EN			BIT(19)
-#define   QCA8K_VTU_FUNC0_EG_MODE_S(_i)			(4 + (_i) * 2)
-#define   QCA8K_VTU_FUNC0_EG_MODE_MASK			3
-#define   QCA8K_VTU_FUNC0_EG_MODE_UNMOD			0
-#define   QCA8K_VTU_FUNC0_EG_MODE_UNTAG			1
-#define   QCA8K_VTU_FUNC0_EG_MODE_TAG			2
-#define   QCA8K_VTU_FUNC0_EG_MODE_NOT			3
+/*        QCA8K_VTU_FUNC0_EG_MODE_MASK			GENMASK(17, 4)
+ *          It does contain VLAN_MODE for each port [5:4] for port0,
+ *          [7:6] for port1 ... [17:16] for port6. Use virtual port
+ *          define to handle this.
+ */
+#define   QCA8K_VTU_FUNC0_EG_MODE_PORT_SHIFT(_i)	(4 + (_i) * 2)
+#define   QCA8K_VTU_FUNC0_EG_MODE_MASK			GENMASK(1, 0)
+#define   QCA8K_VTU_FUNC0_EG_MODE_PORT_MASK(_i)		(GENMASK(1, 0) << QCA8K_VTU_FUNC0_EG_MODE_PORT_SHIFT(_i))
+#define   QCA8K_VTU_FUNC0_EG_MODE_UNMOD			FIELD_PREP(QCA8K_VTU_FUNC0_EG_MODE_MASK, 0x0)
+#define   QCA8K_VTU_FUNC0_EG_MODE_PORT_UNMOD(_i)	(QCA8K_VTU_FUNC0_EG_MODE_UNMOD << QCA8K_VTU_FUNC0_EG_MODE_PORT_SHIFT(_i))
+#define   QCA8K_VTU_FUNC0_EG_MODE_UNTAG			FIELD_PREP(QCA8K_VTU_FUNC0_EG_MODE_MASK, 0x1)
+#define   QCA8K_VTU_FUNC0_EG_MODE_PORT_UNTAG(_i)	(QCA8K_VTU_FUNC0_EG_MODE_UNTAG << QCA8K_VTU_FUNC0_EG_MODE_PORT_SHIFT(_i))
+#define   QCA8K_VTU_FUNC0_EG_MODE_TAG			FIELD_PREP(QCA8K_VTU_FUNC0_EG_MODE_MASK, 0x2)
+#define   QCA8K_VTU_FUNC0_EG_MODE_PORT_TAG(_i)		(QCA8K_VTU_FUNC0_EG_MODE_TAG << QCA8K_VTU_FUNC0_EG_MODE_PORT_SHIFT(_i))
+#define   QCA8K_VTU_FUNC0_EG_MODE_NOT			FIELD_PREP(QCA8K_VTU_FUNC0_EG_MODE_MASK, 0x3)
+#define   QCA8K_VTU_FUNC0_EG_MODE_PORT_NOT(_i)		(QCA8K_VTU_FUNC0_EG_MODE_NOT << QCA8K_VTU_FUNC0_EG_MODE_PORT_SHIFT(_i))
 #define QCA8K_REG_VTU_FUNC1				0x614
 #define   QCA8K_VTU_FUNC1_BUSY				BIT(31)
-#define   QCA8K_VTU_FUNC1_VID_S				16
+#define   QCA8K_VTU_FUNC1_VID_MASK			GENMASK(27, 16)
 #define   QCA8K_VTU_FUNC1_FULL				BIT(4)
 #define QCA8K_REG_GLOBAL_FW_CTRL0			0x620
 #define   QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN		BIT(10)
 #define QCA8K_REG_GLOBAL_FW_CTRL1			0x624
-#define   QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_S		24
-#define   QCA8K_GLOBAL_FW_CTRL1_BC_DP_S			16
-#define   QCA8K_GLOBAL_FW_CTRL1_MC_DP_S			8
-#define   QCA8K_GLOBAL_FW_CTRL1_UC_DP_S			0
+#define   QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_MASK		GENMASK(30, 24)
+#define   QCA8K_GLOBAL_FW_CTRL1_BC_DP_MASK		GENMASK(22, 16)
+#define   QCA8K_GLOBAL_FW_CTRL1_MC_DP_MASK		GENMASK(14, 8)
+#define   QCA8K_GLOBAL_FW_CTRL1_UC_DP_MASK		GENMASK(6, 0)
 #define QCA8K_PORT_LOOKUP_CTRL(_i)			(0x660 + (_i) * 0xc)
 #define   QCA8K_PORT_LOOKUP_MEMBER			GENMASK(6, 0)
-#define   QCA8K_PORT_LOOKUP_VLAN_MODE			GENMASK(9, 8)
-#define   QCA8K_PORT_LOOKUP_VLAN_MODE_NONE		(0 << 8)
-#define   QCA8K_PORT_LOOKUP_VLAN_MODE_FALLBACK		(1 << 8)
-#define   QCA8K_PORT_LOOKUP_VLAN_MODE_CHECK		(2 << 8)
-#define   QCA8K_PORT_LOOKUP_VLAN_MODE_SECURE		(3 << 8)
+#define   QCA8K_PORT_LOOKUP_VLAN_MODE_MASK		GENMASK(9, 8)
+#define   QCA8K_PORT_LOOKUP_VLAN_MODE(x)		FIELD_PREP(QCA8K_PORT_LOOKUP_VLAN_MODE_MASK, x)
+#define   QCA8K_PORT_LOOKUP_VLAN_MODE_NONE		QCA8K_PORT_LOOKUP_VLAN_MODE(0x0)
+#define   QCA8K_PORT_LOOKUP_VLAN_MODE_FALLBACK		QCA8K_PORT_LOOKUP_VLAN_MODE(0x1)
+#define   QCA8K_PORT_LOOKUP_VLAN_MODE_CHECK		QCA8K_PORT_LOOKUP_VLAN_MODE(0x2)
+#define   QCA8K_PORT_LOOKUP_VLAN_MODE_SECURE		QCA8K_PORT_LOOKUP_VLAN_MODE(0x3)
 #define   QCA8K_PORT_LOOKUP_STATE_MASK			GENMASK(18, 16)
-#define   QCA8K_PORT_LOOKUP_STATE_DISABLED		(0 << 16)
-#define   QCA8K_PORT_LOOKUP_STATE_BLOCKING		(1 << 16)
-#define   QCA8K_PORT_LOOKUP_STATE_LISTENING		(2 << 16)
-#define   QCA8K_PORT_LOOKUP_STATE_LEARNING		(3 << 16)
-#define   QCA8K_PORT_LOOKUP_STATE_FORWARD		(4 << 16)
-#define   QCA8K_PORT_LOOKUP_STATE			GENMASK(18, 16)
+#define   QCA8K_PORT_LOOKUP_STATE(x)			FIELD_PREP(QCA8K_PORT_LOOKUP_STATE_MASK, x)
+#define   QCA8K_PORT_LOOKUP_STATE_DISABLED		QCA8K_PORT_LOOKUP_STATE(0x0)
+#define   QCA8K_PORT_LOOKUP_STATE_BLOCKING		QCA8K_PORT_LOOKUP_STATE(0x1)
+#define   QCA8K_PORT_LOOKUP_STATE_LISTENING		QCA8K_PORT_LOOKUP_STATE(0x2)
+#define   QCA8K_PORT_LOOKUP_STATE_LEARNING		QCA8K_PORT_LOOKUP_STATE(0x3)
+#define   QCA8K_PORT_LOOKUP_STATE_FORWARD		QCA8K_PORT_LOOKUP_STATE(0x4)
 #define   QCA8K_PORT_LOOKUP_LEARN			BIT(20)
 
 #define QCA8K_REG_GLOBAL_FC_THRESH			0x800
-#define   QCA8K_GLOBAL_FC_GOL_XON_THRES(x)		((x) << 16)
-#define   QCA8K_GLOBAL_FC_GOL_XON_THRES_S		GENMASK(24, 16)
-#define   QCA8K_GLOBAL_FC_GOL_XOFF_THRES(x)		((x) << 0)
-#define   QCA8K_GLOBAL_FC_GOL_XOFF_THRES_S		GENMASK(8, 0)
+#define   QCA8K_GLOBAL_FC_GOL_XON_THRES_MASK		GENMASK(24, 16)
+#define   QCA8K_GLOBAL_FC_GOL_XON_THRES(x)		FIELD_PREP(QCA8K_GLOBAL_FC_GOL_XON_THRES_MASK, x)
+#define   QCA8K_GLOBAL_FC_GOL_XOFF_THRES_MASK		GENMASK(8, 0)
+#define   QCA8K_GLOBAL_FC_GOL_XOFF_THRES(x)		FIELD_PREP(QCA8K_GLOBAL_FC_GOL_XOFF_THRES_MASK, x)
 
 #define QCA8K_REG_PORT_HOL_CTRL0(_i)			(0x970 + (_i) * 0x8)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0_BUF		GENMASK(3, 0)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0(x)		((x) << 0)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1_BUF		GENMASK(7, 4)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1(x)		((x) << 4)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2_BUF		GENMASK(11, 8)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2(x)		((x) << 8)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3_BUF		GENMASK(15, 12)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3(x)		((x) << 12)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4_BUF		GENMASK(19, 16)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4(x)		((x) << 16)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5_BUF		GENMASK(23, 20)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5(x)		((x) << 20)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PORT_BUF		GENMASK(29, 24)
-#define   QCA8K_PORT_HOL_CTRL0_EG_PORT(x)		((x) << 24)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0_BUF_MASK		GENMASK(3, 0)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI0(x)		FIELD_PREP(QCA8K_PORT_HOL_CTRL0_EG_PRI0_BUF_MASK, x)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1_BUF_MASK		GENMASK(7, 4)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI1(x)		FIELD_PREP(QCA8K_PORT_HOL_CTRL0_EG_PRI1_BUF_MASK, x)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2_BUF_MASK		GENMASK(11, 8)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI2(x)		FIELD_PREP(QCA8K_PORT_HOL_CTRL0_EG_PRI2_BUF_MASK, x)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3_BUF_MASK		GENMASK(15, 12)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI3(x)		FIELD_PREP(QCA8K_PORT_HOL_CTRL0_EG_PRI3_BUF_MASK, x)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4_BUF_MASK		GENMASK(19, 16)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI4(x)		FIELD_PREP(QCA8K_PORT_HOL_CTRL0_EG_PRI4_BUF_MASK, x)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5_BUF_MASK		GENMASK(23, 20)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PRI5(x)		FIELD_PREP(QCA8K_PORT_HOL_CTRL0_EG_PRI5_BUF_MASK, x)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PORT_BUF_MASK		GENMASK(29, 24)
+#define   QCA8K_PORT_HOL_CTRL0_EG_PORT(x)		FIELD_PREP(QCA8K_PORT_HOL_CTRL0_EG_PORT_BUF_MASK, x)
 
 #define QCA8K_REG_PORT_HOL_CTRL1(_i)			(0x974 + (_i) * 0x8)
-#define   QCA8K_PORT_HOL_CTRL1_ING_BUF			GENMASK(3, 0)
-#define   QCA8K_PORT_HOL_CTRL1_ING(x)			((x) << 0)
+#define   QCA8K_PORT_HOL_CTRL1_ING_BUF_MASK		GENMASK(3, 0)
+#define   QCA8K_PORT_HOL_CTRL1_ING(x)			FIELD_PREP(QCA8K_PORT_HOL_CTRL1_ING_BUF_MASK, x)
 #define   QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN		BIT(6)
 #define   QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN		BIT(7)
 #define   QCA8K_PORT_HOL_CTRL1_WRED_EN			BIT(8)
 #define   QCA8K_PORT_HOL_CTRL1_EG_MIRROR_EN		BIT(16)
 
 /* Pkt edit registers */
+#define QCA8K_EGREES_VLAN_PORT_SHIFT(_i)		(16 * ((_i) % 2))
+#define QCA8K_EGREES_VLAN_PORT_MASK(_i)			(GENMASK(11, 0) << QCA8K_EGREES_VLAN_PORT_SHIFT(_i))
+#define QCA8K_EGREES_VLAN_PORT(_i, x)			((x) << QCA8K_EGREES_VLAN_PORT_SHIFT(_i))
 #define QCA8K_EGRESS_VLAN(x)				(0x0c70 + (4 * (x / 2)))
 
 /* L3 registers */
-- 
2.32.0

[net-next PATCH v2 3/9] net: dsa: qca8k: remove extra mutex_init in qca8k_setup

[Ansuel Smith]

Mutex is already init in sw_probe. Remove the extra init in qca8k_setup.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 6783a3c2620f..321d11dfcc2c 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1086,8 +1086,6 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
-	mutex_init(&priv->reg_mutex);
-
 	/* Start by setting up the register mapping */
 	priv->regmap = devm_regmap_init(ds->dev, NULL, priv,
 					&qca8k_regmap_config);
-- 
2.32.0

Re: [net-next PATCH v2 3/9] net: dsa: qca8k: remove extra mutex_init in qca8k_setup

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:03:07AM +0100, Ansuel Smith wrote:
> Mutex is already init in sw_probe. Remove the extra init in qca8k_setup.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

[net-next PATCH v2 4/9] net: dsa: qca8k: move regmap init in probe and set it mandatory

[Ansuel Smith]

In preparation for regmap conversion, move regmap init in the probe
function and make it mandatory as any read/write/rmw operation will be
converted to regmap API.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 321d11dfcc2c..52fca800e6f7 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1086,12 +1086,6 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
-	/* Start by setting up the register mapping */
-	priv->regmap = devm_regmap_init(ds->dev, NULL, priv,
-					&qca8k_regmap_config);
-	if (IS_ERR(priv->regmap))
-		dev_warn(priv->dev, "regmap initialization failed");
-
 	ret = qca8k_setup_mdio_bus(priv);
 	if (ret)
 		return ret;
@@ -2077,6 +2071,14 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
 		gpiod_set_value_cansleep(priv->reset_gpio, 0);
 	}
 
+	/* Start by setting up the register mapping */
+	priv->regmap = devm_regmap_init(&mdiodev->dev, NULL, priv,
+					&qca8k_regmap_config);
+	if (IS_ERR(priv->regmap)) {
+		dev_err(priv->dev, "regmap initialization failed");
+		return PTR_ERR(priv->regmap);
+	}
+
 	/* Check the detected switch id */
 	ret = qca8k_read_switch_id(priv);
 	if (ret)
-- 
2.32.0

Re: [net-next PATCH v2 4/9] net: dsa: qca8k: move regmap init in probe and set it mandatory

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:03:08AM +0100, Ansuel Smith wrote:
> In preparation for regmap conversion, move regmap init in the probe
> function and make it mandatory as any read/write/rmw operation will be
> converted to regmap API.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

[net-next PATCH v2 5/9] net: dsa: qca8k: convert qca8k to regmap helper

[Ansuel Smith]

Convert any qca8k read/write/rmw/set/clear/pool to regmap helper and add
missing config to regmap_config struct.
Ipq40xx SoC have the internal switch based on the qca8k regmap but use
mmio for read/write/rmw operation instead of mdio.
In preparation for the support of this internal switch, convert the
driver to regmap API to later split the driver to common and specific
code. The overhead introduced by the use of regamp API is marginal as the
internal mdio will bypass it by using its direct access and regmap will be
used only by configuration functions or fdb access.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 289 ++++++++++++++++++----------------------
 1 file changed, 131 insertions(+), 158 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 52fca800e6f7..159a1065e66b 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -10,6 +10,7 @@
 #include <linux/phy.h>
 #include <linux/netdevice.h>
 #include <linux/bitfield.h>
+#include <linux/regmap.h>
 #include <net/dsa.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
@@ -150,8 +151,9 @@ qca8k_set_page(struct mii_bus *bus, u16 page)
 }
 
 static int
-qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
+qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
 {
+	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
 	struct mii_bus *bus = priv->bus;
 	u16 r1, r2, page;
 	int ret;
@@ -172,8 +174,9 @@ qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
 }
 
 static int
-qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
+qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val)
 {
+	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
 	struct mii_bus *bus = priv->bus;
 	u16 r1, r2, page;
 	int ret;
@@ -194,8 +197,9 @@ qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
 }
 
 static int
-qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
+qca8k_regmap_update_bits(void *ctx, uint32_t reg, uint32_t mask, uint32_t write_val)
 {
+	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
 	struct mii_bus *bus = priv->bus;
 	u16 r1, r2, page;
 	u32 val;
@@ -223,34 +227,6 @@ qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
 	return ret;
 }
 
-static int
-qca8k_reg_set(struct qca8k_priv *priv, u32 reg, u32 val)
-{
-	return qca8k_rmw(priv, reg, 0, val);
-}
-
-static int
-qca8k_reg_clear(struct qca8k_priv *priv, u32 reg, u32 val)
-{
-	return qca8k_rmw(priv, reg, val, 0);
-}
-
-static int
-qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
-{
-	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
-
-	return qca8k_read(priv, reg, val);
-}
-
-static int
-qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val)
-{
-	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
-
-	return qca8k_write(priv, reg, val);
-}
-
 static const struct regmap_range qca8k_readable_ranges[] = {
 	regmap_reg_range(0x0000, 0x00e4), /* Global control */
 	regmap_reg_range(0x0100, 0x0168), /* EEE control */
@@ -282,26 +258,19 @@ static struct regmap_config qca8k_regmap_config = {
 	.max_register = 0x16ac, /* end MIB - Port6 range */
 	.reg_read = qca8k_regmap_read,
 	.reg_write = qca8k_regmap_write,
+	.reg_update_bits = qca8k_regmap_update_bits,
 	.rd_table = &qca8k_readable_table,
+	.disable_locking = true, /* Locking is handled by qca8k read/write */
+	.cache_type = REGCACHE_NONE, /* Explicitly disable CACHE */
 };
 
 static int
 qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask)
 {
-	int ret, ret1;
 	u32 val;
 
-	ret = read_poll_timeout(qca8k_read, ret1, !(val & mask),
-				0, QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC, false,
-				priv, reg, &val);
-
-	/* Check if qca8k_read has failed for a different reason
-	 * before returning -ETIMEDOUT
-	 */
-	if (ret < 0 && ret1 < 0)
-		return ret1;
-
-	return ret;
+	return regmap_read_poll_timeout(priv->regmap, reg, val, !(val & mask), 0,
+				       QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC);
 }
 
 static int
@@ -312,7 +281,7 @@ qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
 
 	/* load the ARL table into an array */
 	for (i = 0; i < 4; i++) {
-		ret = qca8k_read(priv, QCA8K_REG_ATU_DATA0 + (i * 4), &val);
+		ret = regmap_read(priv->regmap, QCA8K_REG_ATU_DATA0 + (i * 4), &val);
 		if (ret < 0)
 			return ret;
 
@@ -359,7 +328,7 @@ qca8k_fdb_write(struct qca8k_priv *priv, u16 vid, u8 port_mask, const u8 *mac,
 
 	/* load the array into the ARL table */
 	for (i = 0; i < 3; i++)
-		qca8k_write(priv, QCA8K_REG_ATU_DATA0 + (i * 4), reg[i]);
+		regmap_write(priv->regmap, QCA8K_REG_ATU_DATA0 + (i * 4), reg[i]);
 }
 
 static int
@@ -377,7 +346,7 @@ qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port)
 	}
 
 	/* Write the function register triggering the table access */
-	ret = qca8k_write(priv, QCA8K_REG_ATU_FUNC, reg);
+	ret = regmap_write(priv->regmap, QCA8K_REG_ATU_FUNC, reg);
 	if (ret)
 		return ret;
 
@@ -388,7 +357,7 @@ qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port)
 
 	/* Check for table full violation when adding an entry */
 	if (cmd == QCA8K_FDB_LOAD) {
-		ret = qca8k_read(priv, QCA8K_REG_ATU_FUNC, &reg);
+		ret = regmap_read(priv->regmap, QCA8K_REG_ATU_FUNC, &reg);
 		if (ret < 0)
 			return ret;
 		if (reg & QCA8K_ATU_FUNC_FULL)
@@ -458,7 +427,7 @@ qca8k_vlan_access(struct qca8k_priv *priv, enum qca8k_vlan_cmd cmd, u16 vid)
 	reg |= FIELD_PREP(QCA8K_VTU_FUNC1_VID_MASK, vid);
 
 	/* Write the function register triggering the table access */
-	ret = qca8k_write(priv, QCA8K_REG_VTU_FUNC1, reg);
+	ret = regmap_write(priv->regmap, QCA8K_REG_VTU_FUNC1, reg);
 	if (ret)
 		return ret;
 
@@ -469,7 +438,7 @@ qca8k_vlan_access(struct qca8k_priv *priv, enum qca8k_vlan_cmd cmd, u16 vid)
 
 	/* Check for table full violation when adding an entry */
 	if (cmd == QCA8K_VLAN_LOAD) {
-		ret = qca8k_read(priv, QCA8K_REG_VTU_FUNC1, &reg);
+		ret = regmap_read(priv->regmap, QCA8K_REG_VTU_FUNC1, &reg);
 		if (ret < 0)
 			return ret;
 		if (reg & QCA8K_VTU_FUNC1_FULL)
@@ -497,7 +466,7 @@ qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged)
 	if (ret < 0)
 		goto out;
 
-	ret = qca8k_read(priv, QCA8K_REG_VTU_FUNC0, &reg);
+	ret = regmap_read(priv->regmap, QCA8K_REG_VTU_FUNC0, &reg);
 	if (ret < 0)
 		goto out;
 	reg |= QCA8K_VTU_FUNC0_VALID | QCA8K_VTU_FUNC0_IVL_EN;
@@ -507,7 +476,7 @@ qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged)
 	else
 		reg |= QCA8K_VTU_FUNC0_EG_MODE_PORT_TAG(port);
 
-	ret = qca8k_write(priv, QCA8K_REG_VTU_FUNC0, reg);
+	ret = regmap_write(priv->regmap, QCA8K_REG_VTU_FUNC0, reg);
 	if (ret)
 		goto out;
 	ret = qca8k_vlan_access(priv, QCA8K_VLAN_LOAD, vid);
@@ -530,7 +499,7 @@ qca8k_vlan_del(struct qca8k_priv *priv, u8 port, u16 vid)
 	if (ret < 0)
 		goto out;
 
-	ret = qca8k_read(priv, QCA8K_REG_VTU_FUNC0, &reg);
+	ret = regmap_read(priv->regmap, QCA8K_REG_VTU_FUNC0, &reg);
 	if (ret < 0)
 		goto out;
 	reg &= ~QCA8K_VTU_FUNC0_EG_MODE_PORT_MASK(port);
@@ -550,7 +519,7 @@ qca8k_vlan_del(struct qca8k_priv *priv, u8 port, u16 vid)
 	if (del) {
 		ret = qca8k_vlan_access(priv, QCA8K_VLAN_PURGE, vid);
 	} else {
-		ret = qca8k_write(priv, QCA8K_REG_VTU_FUNC0, reg);
+		ret = regmap_write(priv->regmap, QCA8K_REG_VTU_FUNC0, reg);
 		if (ret)
 			goto out;
 		ret = qca8k_vlan_access(priv, QCA8K_VLAN_LOAD, vid);
@@ -568,7 +537,7 @@ qca8k_mib_init(struct qca8k_priv *priv)
 	int ret;
 
 	mutex_lock(&priv->reg_mutex);
-	ret = qca8k_reg_set(priv, QCA8K_REG_MIB, QCA8K_MIB_FLUSH | QCA8K_MIB_BUSY);
+	ret = regmap_set_bits(priv->regmap, QCA8K_REG_MIB, QCA8K_MIB_FLUSH | QCA8K_MIB_BUSY);
 	if (ret)
 		goto exit;
 
@@ -576,11 +545,11 @@ qca8k_mib_init(struct qca8k_priv *priv)
 	if (ret)
 		goto exit;
 
-	ret = qca8k_reg_set(priv, QCA8K_REG_MIB, QCA8K_MIB_CPU_KEEP);
+	ret = regmap_set_bits(priv->regmap, QCA8K_REG_MIB, QCA8K_MIB_CPU_KEEP);
 	if (ret)
 		goto exit;
 
-	ret = qca8k_write(priv, QCA8K_REG_MODULE_EN, QCA8K_MODULE_EN_MIB);
+	ret = regmap_write(priv->regmap, QCA8K_REG_MODULE_EN, QCA8K_MODULE_EN_MIB);
 
 exit:
 	mutex_unlock(&priv->reg_mutex);
@@ -597,9 +566,9 @@ qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
 		mask |= QCA8K_PORT_STATUS_LINK_AUTO;
 
 	if (enable)
-		qca8k_reg_set(priv, QCA8K_REG_PORT_STATUS(port), mask);
+		regmap_set_bits(priv->regmap, QCA8K_REG_PORT_STATUS(port), mask);
 	else
-		qca8k_reg_clear(priv, QCA8K_REG_PORT_STATUS(port), mask);
+		regmap_clear_bits(priv->regmap, QCA8K_REG_PORT_STATUS(port), mask);
 }
 
 static u32
@@ -861,8 +830,8 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
 		 * a dt-overlay and driver reload changed the configuration
 		 */
 
-		return qca8k_reg_clear(priv, QCA8K_MDIO_MASTER_CTRL,
-				       QCA8K_MDIO_MASTER_EN);
+		return regmap_clear_bits(priv->regmap, QCA8K_MDIO_MASTER_CTRL,
+					 QCA8K_MDIO_MASTER_EN);
 	}
 
 	/* Check if the devicetree declare the port:phy mapping */
@@ -903,10 +872,10 @@ qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
 		mask |= QCA8K_MAC_PWR_RGMII1_1_8V;
 
 	if (mask) {
-		ret = qca8k_rmw(priv, QCA8K_REG_MAC_PWR_SEL,
-				QCA8K_MAC_PWR_RGMII0_1_8V |
-				QCA8K_MAC_PWR_RGMII1_1_8V,
-				mask);
+		ret = regmap_update_bits(priv->regmap, QCA8K_REG_MAC_PWR_SEL,
+					 QCA8K_MAC_PWR_RGMII0_1_8V |
+					 QCA8K_MAC_PWR_RGMII1_1_8V,
+					 mask);
 	}
 
 	return ret;
@@ -947,8 +916,9 @@ qca8k_setup_of_pws_reg(struct qca8k_priv *priv)
 		if (data->reduced_package)
 			val |= QCA8327_PWS_PACKAGE148_EN;
 
-		ret = qca8k_rmw(priv, QCA8K_REG_PWS, QCA8327_PWS_PACKAGE148_EN,
-				val);
+		ret = regmap_update_bits(priv->regmap, QCA8K_REG_PWS,
+					 QCA8327_PWS_PACKAGE148_EN,
+					 val);
 		if (ret)
 			return ret;
 	}
@@ -965,9 +935,10 @@ qca8k_setup_of_pws_reg(struct qca8k_priv *priv)
 		val |= QCA8K_PWS_LED_OPEN_EN_CSR;
 	}
 
-	return qca8k_rmw(priv, QCA8K_REG_PWS,
-			QCA8K_PWS_LED_OPEN_EN_CSR | QCA8K_PWS_POWER_ON_SEL,
-			val);
+	return regmap_update_bits(priv->regmap, QCA8K_REG_PWS,
+				  QCA8K_PWS_LED_OPEN_EN_CSR |
+				  QCA8K_PWS_POWER_ON_SEL,
+				  val);
 }
 
 static int
@@ -1099,16 +1070,16 @@ qca8k_setup(struct dsa_switch *ds)
 		return ret;
 
 	/* Make sure MAC06 is disabled */
-	ret = qca8k_reg_clear(priv, QCA8K_REG_PORT0_PAD_CTRL,
-			      QCA8K_PORT0_PAD_MAC06_EXCHANGE_EN);
+	ret = regmap_clear_bits(priv->regmap, QCA8K_REG_PORT0_PAD_CTRL,
+				QCA8K_PORT0_PAD_MAC06_EXCHANGE_EN);
 	if (ret) {
 		dev_err(priv->dev, "failed disabling MAC06 exchange");
 		return ret;
 	}
 
 	/* Enable CPU Port */
-	ret = qca8k_reg_set(priv, QCA8K_REG_GLOBAL_FW_CTRL0,
-			    QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
+	ret = regmap_set_bits(priv->regmap, QCA8K_REG_GLOBAL_FW_CTRL0,
+			      QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
 	if (ret) {
 		dev_err(priv->dev, "failed enabling CPU port");
 		return ret;
@@ -1122,16 +1093,18 @@ qca8k_setup(struct dsa_switch *ds)
 	/* Initial setup of all ports */
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
 		/* Disable forwarding by default on all ports */
-		ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
-				QCA8K_PORT_LOOKUP_MEMBER, 0);
+		ret = regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(i),
+					 QCA8K_PORT_LOOKUP_MEMBER, 0);
 		if (ret)
 			return ret;
 
 		/* Enable QCA header mode on all cpu ports */
 		if (dsa_is_cpu_port(ds, i)) {
-			ret = qca8k_write(priv, QCA8K_REG_PORT_HDR_CTRL(i),
-					  FIELD_PREP(QCA8K_PORT_HDR_CTRL_TX_MASK, QCA8K_PORT_HDR_CTRL_ALL) |
-					  FIELD_PREP(QCA8K_PORT_HDR_CTRL_RX_MASK, QCA8K_PORT_HDR_CTRL_ALL));
+			ret = regmap_write(priv->regmap, QCA8K_REG_PORT_HDR_CTRL(i),
+					   FIELD_PREP(QCA8K_PORT_HDR_CTRL_TX_MASK,
+						      QCA8K_PORT_HDR_CTRL_ALL) |
+					   FIELD_PREP(QCA8K_PORT_HDR_CTRL_RX_MASK,
+						      QCA8K_PORT_HDR_CTRL_ALL));
 			if (ret) {
 				dev_err(priv->dev, "failed enabling QCA header mode");
 				return ret;
@@ -1147,11 +1120,11 @@ qca8k_setup(struct dsa_switch *ds)
 	 * Notice that in multi-cpu config only one port should be set
 	 * for igmp, unknown, multicast and broadcast packet
 	 */
-	ret = qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
-			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_MASK, BIT(cpu_port)) |
-			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_BC_DP_MASK, BIT(cpu_port)) |
-			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_MC_DP_MASK, BIT(cpu_port)) |
-			  FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_UC_DP_MASK, BIT(cpu_port)));
+	ret = regmap_write(priv->regmap, QCA8K_REG_GLOBAL_FW_CTRL1,
+			   FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_MASK, BIT(cpu_port)) |
+			   FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_BC_DP_MASK, BIT(cpu_port)) |
+			   FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_MC_DP_MASK, BIT(cpu_port)) |
+			   FIELD_PREP(QCA8K_GLOBAL_FW_CTRL1_UC_DP_MASK, BIT(cpu_port)));
 	if (ret)
 		return ret;
 
@@ -1161,38 +1134,38 @@ qca8k_setup(struct dsa_switch *ds)
 	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
 		/* CPU port gets connected to all user ports of the switch */
 		if (dsa_is_cpu_port(ds, i)) {
-			ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
-					QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
+			ret = regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(i),
+						 QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
 			if (ret)
 				return ret;
 		}
 
 		/* Individual user ports get connected to CPU port only */
 		if (dsa_is_user_port(ds, i)) {
-			ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
-					QCA8K_PORT_LOOKUP_MEMBER,
-					BIT(cpu_port));
+			ret = regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(i),
+						 QCA8K_PORT_LOOKUP_MEMBER,
+						 BIT(cpu_port));
 			if (ret)
 				return ret;
 
 			/* Enable ARP Auto-learning by default */
-			ret = qca8k_reg_set(priv, QCA8K_PORT_LOOKUP_CTRL(i),
-					    QCA8K_PORT_LOOKUP_LEARN);
+			ret = regmap_set_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(i),
+					      QCA8K_PORT_LOOKUP_LEARN);
 			if (ret)
 				return ret;
 
 			/* For port based vlans to work we need to set the
 			 * default egress vid
 			 */
-			ret = qca8k_rmw(priv, QCA8K_EGRESS_VLAN(i),
-					QCA8K_EGREES_VLAN_PORT_MASK(i),
-					QCA8K_EGREES_VLAN_PORT(i, QCA8K_PORT_VID_DEF));
+			ret = regmap_update_bits(priv->regmap, QCA8K_EGRESS_VLAN(i),
+						 QCA8K_EGREES_VLAN_PORT_MASK(i),
+						 QCA8K_EGREES_VLAN_PORT(i, QCA8K_PORT_VID_DEF));
 			if (ret)
 				return ret;
 
-			ret = qca8k_write(priv, QCA8K_REG_PORT_VLAN_CTRL0(i),
-					  QCA8K_PORT_VLAN_CVID(QCA8K_PORT_VID_DEF) |
-					  QCA8K_PORT_VLAN_SVID(QCA8K_PORT_VID_DEF));
+			ret = regmap_write(priv->regmap, QCA8K_REG_PORT_VLAN_CTRL0(i),
+					   QCA8K_PORT_VLAN_CVID(QCA8K_PORT_VID_DEF) |
+					   QCA8K_PORT_VLAN_SVID(QCA8K_PORT_VID_DEF));
 			if (ret)
 				return ret;
 		}
@@ -1226,18 +1199,18 @@ qca8k_setup(struct dsa_switch *ds)
 					QCA8K_PORT_HOL_CTRL0_EG_PRI3(0x8) |
 					QCA8K_PORT_HOL_CTRL0_EG_PORT(0x19);
 			}
-			qca8k_write(priv, QCA8K_REG_PORT_HOL_CTRL0(i), mask);
+			regmap_write(priv->regmap, QCA8K_REG_PORT_HOL_CTRL0(i), mask);
 
 			mask = QCA8K_PORT_HOL_CTRL1_ING(0x6) |
 			QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
 			QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
 			QCA8K_PORT_HOL_CTRL1_WRED_EN;
-			qca8k_rmw(priv, QCA8K_REG_PORT_HOL_CTRL1(i),
-				  QCA8K_PORT_HOL_CTRL1_ING_BUF_MASK |
-				  QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
-				  QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
-				  QCA8K_PORT_HOL_CTRL1_WRED_EN,
-				  mask);
+			regmap_update_bits(priv->regmap, QCA8K_REG_PORT_HOL_CTRL1(i),
+					   QCA8K_PORT_HOL_CTRL1_ING_BUF_MASK |
+					   QCA8K_PORT_HOL_CTRL1_EG_PRI_BUF_EN |
+					   QCA8K_PORT_HOL_CTRL1_EG_PORT_BUF_EN |
+					   QCA8K_PORT_HOL_CTRL1_WRED_EN,
+					   mask);
 		}
 
 		/* Set initial MTU for every port.
@@ -1255,14 +1228,14 @@ qca8k_setup(struct dsa_switch *ds)
 	if (priv->switch_id == QCA8K_ID_QCA8327) {
 		mask = QCA8K_GLOBAL_FC_GOL_XON_THRES(288) |
 		       QCA8K_GLOBAL_FC_GOL_XOFF_THRES(496);
-		qca8k_rmw(priv, QCA8K_REG_GLOBAL_FC_THRESH,
-			  QCA8K_GLOBAL_FC_GOL_XON_THRES_MASK |
-			  QCA8K_GLOBAL_FC_GOL_XOFF_THRES_MASK,
-			  mask);
+		regmap_update_bits(priv->regmap, QCA8K_REG_GLOBAL_FC_THRESH,
+				   QCA8K_GLOBAL_FC_GOL_XON_THRES_MASK |
+				   QCA8K_GLOBAL_FC_GOL_XOFF_THRES_MASK,
+				   mask);
 	}
 
 	/* Setup our port MTUs to match power on defaults */
-	ret = qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN);
+	ret = regmap_write(priv->regmap, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN);
 	if (ret)
 		dev_warn(priv->dev, "failed setting MTU settings");
 
@@ -1305,12 +1278,12 @@ qca8k_mac_config_setup_internal_delay(struct qca8k_priv *priv, int cpu_port_inde
 	}
 
 	/* Set RGMII delay based on the selected values */
-	ret = qca8k_rmw(priv, reg,
-			QCA8K_PORT_PAD_RGMII_TX_DELAY_MASK |
-			QCA8K_PORT_PAD_RGMII_RX_DELAY_MASK |
-			QCA8K_PORT_PAD_RGMII_TX_DELAY_EN |
-			QCA8K_PORT_PAD_RGMII_RX_DELAY_EN,
-			val);
+	ret = regmap_update_bits(priv->regmap, reg,
+				 QCA8K_PORT_PAD_RGMII_TX_DELAY_MASK |
+				 QCA8K_PORT_PAD_RGMII_RX_DELAY_MASK |
+				 QCA8K_PORT_PAD_RGMII_TX_DELAY_EN |
+				 QCA8K_PORT_PAD_RGMII_RX_DELAY_EN,
+				 val);
 	if (ret)
 		dev_err(priv->dev, "Failed to set internal delay for CPU port%d",
 			cpu_port_index == QCA8K_CPU_PORT0 ? 0 : 6);
@@ -1371,7 +1344,7 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
-		qca8k_write(priv, reg, QCA8K_PORT_PAD_RGMII_EN);
+		regmap_write(priv->regmap, reg, QCA8K_PORT_PAD_RGMII_EN);
 
 		/* Configure rgmii delay */
 		qca8k_mac_config_setup_internal_delay(priv, cpu_port_index, reg);
@@ -1381,26 +1354,26 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		 * rather than individual port registers.
 		 */
 		if (priv->switch_id == QCA8K_ID_QCA8337)
-			qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
-				    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
+			regmap_write(priv->regmap, QCA8K_REG_PORT5_PAD_CTRL,
+				     QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_1000BASEX:
 		/* Enable SGMII on the port */
-		qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
+		regmap_write(priv->regmap, reg, QCA8K_PORT_PAD_SGMII_EN);
 
 		/* Enable/disable SerDes auto-negotiation as necessary */
-		ret = qca8k_read(priv, QCA8K_REG_PWS, &val);
+		ret = regmap_read(priv->regmap, QCA8K_REG_PWS, &val);
 		if (ret)
 			return;
 		if (phylink_autoneg_inband(mode))
 			val &= ~QCA8K_PWS_SERDES_AEN_DIS;
 		else
 			val |= QCA8K_PWS_SERDES_AEN_DIS;
-		qca8k_write(priv, QCA8K_REG_PWS, val);
+		regmap_write(priv->regmap, QCA8K_REG_PWS, val);
 
 		/* Configure the SGMII parameters */
-		ret = qca8k_read(priv, QCA8K_REG_SGMII_CTRL, &val);
+		ret = regmap_read(priv->regmap, QCA8K_REG_SGMII_CTRL, &val);
 		if (ret)
 			return;
 
@@ -1422,7 +1395,7 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 			val |= QCA8K_SGMII_MODE_CTRL_BASEX;
 		}
 
-		qca8k_write(priv, QCA8K_REG_SGMII_CTRL, val);
+		regmap_write(priv->regmap, QCA8K_REG_SGMII_CTRL, val);
 
 		/* From original code is reported port instability as SGMII also
 		 * require delay set. Apply advised values here or take them from DT.
@@ -1447,10 +1420,10 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 			val |= QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE;
 
 		if (val)
-			ret = qca8k_rmw(priv, reg,
-					QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
-					QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
-					val);
+			ret = regmap_update_bits(priv->regmap, reg,
+						 QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
+						 QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
+						 val);
 
 		break;
 	default:
@@ -1531,7 +1504,7 @@ qca8k_phylink_mac_link_state(struct dsa_switch *ds, int port,
 	u32 reg;
 	int ret;
 
-	ret = qca8k_read(priv, QCA8K_REG_PORT_STATUS(port), &reg);
+	ret = regmap_read(priv->regmap, QCA8K_REG_PORT_STATUS(port), &reg);
 	if (ret < 0)
 		return ret;
 
@@ -1612,7 +1585,7 @@ qca8k_phylink_mac_link_up(struct dsa_switch *ds, int port, unsigned int mode,
 
 	reg |= QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
 
-	qca8k_write(priv, QCA8K_REG_PORT_STATUS(port), reg);
+	regmap_write(priv->regmap, QCA8K_REG_PORT_STATUS(port), reg);
 }
 
 static void
@@ -1642,12 +1615,12 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
 		mib = &ar8327_mib[i];
 		reg = QCA8K_PORT_MIB_COUNTER(port) + mib->offset;
 
-		ret = qca8k_read(priv, reg, &val);
+		ret = regmap_read(priv->regmap, reg, &val);
 		if (ret < 0)
 			continue;
 
 		if (mib->size == 2) {
-			ret = qca8k_read(priv, reg + 4, &hi);
+			ret = regmap_read(priv->regmap, reg + 4, &hi);
 			if (ret < 0)
 				continue;
 		}
@@ -1676,7 +1649,7 @@ qca8k_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *eee)
 	int ret;
 
 	mutex_lock(&priv->reg_mutex);
-	ret = qca8k_read(priv, QCA8K_REG_EEE_CTRL, &reg);
+	ret = regmap_read(priv->regmap, QCA8K_REG_EEE_CTRL, &reg);
 	if (ret < 0)
 		goto exit;
 
@@ -1684,7 +1657,7 @@ qca8k_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *eee)
 		reg |= lpi_en;
 	else
 		reg &= ~lpi_en;
-	ret = qca8k_write(priv, QCA8K_REG_EEE_CTRL, reg);
+	ret = regmap_write(priv->regmap, QCA8K_REG_EEE_CTRL, reg);
 
 exit:
 	mutex_unlock(&priv->reg_mutex);
@@ -1723,8 +1696,8 @@ qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 		break;
 	}
 
-	qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-		  QCA8K_PORT_LOOKUP_STATE_MASK, stp_state);
+	regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(port),
+			   QCA8K_PORT_LOOKUP_STATE_MASK, stp_state);
 }
 
 static int
@@ -1745,9 +1718,9 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
 		/* Add this port to the portvlan mask of the other ports
 		 * in the bridge
 		 */
-		ret = qca8k_reg_set(priv,
-				    QCA8K_PORT_LOOKUP_CTRL(i),
-				    BIT(port));
+		ret = regmap_set_bits(priv->regmap,
+				      QCA8K_PORT_LOOKUP_CTRL(i),
+				      BIT(port));
 		if (ret)
 			return ret;
 		if (i != port)
@@ -1755,8 +1728,8 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
 	}
 
 	/* Add all other ports to this ports portvlan mask */
-	ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-			QCA8K_PORT_LOOKUP_MEMBER, port_mask);
+	ret = regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(port),
+				 QCA8K_PORT_LOOKUP_MEMBER, port_mask);
 
 	return ret;
 }
@@ -1777,16 +1750,16 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
 		/* Remove this port to the portvlan mask of the other ports
 		 * in the bridge
 		 */
-		qca8k_reg_clear(priv,
-				QCA8K_PORT_LOOKUP_CTRL(i),
-				BIT(port));
+		regmap_clear_bits(priv->regmap,
+				  QCA8K_PORT_LOOKUP_CTRL(i),
+				  BIT(port));
 	}
 
 	/* Set the cpu port to be the only one in the portvlan mask of
 	 * this port
 	 */
-	qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-		  QCA8K_PORT_LOOKUP_MEMBER, BIT(cpu_port));
+	regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(port),
+			   QCA8K_PORT_LOOKUP_MEMBER, BIT(cpu_port));
 }
 
 static int
@@ -1826,7 +1799,7 @@ qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 			mtu = priv->port_mtu[i];
 
 	/* Include L2 header / FCS length */
-	return qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN);
+	return regmap_write(priv->regmap, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN);
 }
 
 static int
@@ -1902,13 +1875,13 @@ qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 	int ret;
 
 	if (vlan_filtering) {
-		ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-				QCA8K_PORT_LOOKUP_VLAN_MODE_MASK,
-				QCA8K_PORT_LOOKUP_VLAN_MODE_SECURE);
+		ret = regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(port),
+					 QCA8K_PORT_LOOKUP_VLAN_MODE_MASK,
+					 QCA8K_PORT_LOOKUP_VLAN_MODE_SECURE);
 	} else {
-		ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
-				QCA8K_PORT_LOOKUP_VLAN_MODE_MASK,
-				QCA8K_PORT_LOOKUP_VLAN_MODE_NONE);
+		ret = regmap_update_bits(priv->regmap, QCA8K_PORT_LOOKUP_CTRL(port),
+					 QCA8K_PORT_LOOKUP_VLAN_MODE_MASK,
+					 QCA8K_PORT_LOOKUP_VLAN_MODE_NONE);
 	}
 
 	return ret;
@@ -1931,15 +1904,15 @@ qca8k_port_vlan_add(struct dsa_switch *ds, int port,
 	}
 
 	if (pvid) {
-		ret = qca8k_rmw(priv, QCA8K_EGRESS_VLAN(port),
-				QCA8K_EGREES_VLAN_PORT_MASK(port),
-				QCA8K_EGREES_VLAN_PORT(port, vlan->vid));
+		ret = regmap_update_bits(priv->regmap, QCA8K_EGRESS_VLAN(port),
+					 QCA8K_EGREES_VLAN_PORT_MASK(port),
+					 QCA8K_EGREES_VLAN_PORT(port, vlan->vid));
 		if (ret)
 			return ret;
 
-		ret = qca8k_write(priv, QCA8K_REG_PORT_VLAN_CTRL0(port),
-				  QCA8K_PORT_VLAN_CVID(vlan->vid) |
-				  QCA8K_PORT_VLAN_SVID(vlan->vid));
+		ret = regmap_write(priv->regmap, QCA8K_REG_PORT_VLAN_CTRL0(port),
+				   QCA8K_PORT_VLAN_CVID(vlan->vid) |
+				   QCA8K_PORT_VLAN_SVID(vlan->vid));
 	}
 
 	return ret;
@@ -2023,7 +1996,7 @@ static int qca8k_read_switch_id(struct qca8k_priv *priv)
 	if (!data)
 		return -ENODEV;
 
-	ret = qca8k_read(priv, QCA8K_REG_MASK_CTRL, &val);
+	ret = regmap_read(priv->regmap, QCA8K_REG_MASK_CTRL, &val);
 	if (ret < 0)
 		return -ENODEV;
 
-- 
2.32.0

Re: [net-next PATCH v2 5/9] net: dsa: qca8k: convert qca8k to regmap helper

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:03:09AM +0100, Ansuel Smith wrote:
> Convert any qca8k read/write/rmw/set/clear/pool to regmap helper and add
> missing config to regmap_config struct.
> Ipq40xx SoC have the internal switch based on the qca8k regmap but use
> mmio for read/write/rmw operation instead of mdio.
> In preparation for the support of this internal switch, convert the
> driver to regmap API to later split the driver to common and specific
> code. The overhead introduced by the use of regamp API is marginal as the
> internal mdio will bypass it by using its direct access and regmap will be
> used only by configuration functions or fdb access.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/net/dsa/qca8k.c | 289 ++++++++++++++++++----------------------
>  1 file changed, 131 insertions(+), 158 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 52fca800e6f7..159a1065e66b 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -10,6 +10,7 @@
>  #include <linux/phy.h>
>  #include <linux/netdevice.h>
>  #include <linux/bitfield.h>
> +#include <linux/regmap.h>
>  #include <net/dsa.h>
>  #include <linux/of_net.h>
>  #include <linux/of_mdio.h>
> @@ -150,8 +151,9 @@ qca8k_set_page(struct mii_bus *bus, u16 page)
>  }
>  
>  static int
> -qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
> +qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
>  {
> +	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
>  	struct mii_bus *bus = priv->bus;
>  	u16 r1, r2, page;
>  	int ret;
> @@ -172,8 +174,9 @@ qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
>  }
>  
>  static int
> -qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
> +qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val)
>  {
> +	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
>  	struct mii_bus *bus = priv->bus;
>  	u16 r1, r2, page;
>  	int ret;
> @@ -194,8 +197,9 @@ qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
>  }
>  
>  static int
> -qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
> +qca8k_regmap_update_bits(void *ctx, uint32_t reg, uint32_t mask, uint32_t write_val)
>  {
> +	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
>  	struct mii_bus *bus = priv->bus;
>  	u16 r1, r2, page;
>  	u32 val;
> @@ -223,34 +227,6 @@ qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
>  	return ret;
>  }
>  
> -static int
> -qca8k_reg_set(struct qca8k_priv *priv, u32 reg, u32 val)
> -{
> -	return qca8k_rmw(priv, reg, 0, val);
> -}
> -
> -static int
> -qca8k_reg_clear(struct qca8k_priv *priv, u32 reg, u32 val)
> -{
> -	return qca8k_rmw(priv, reg, val, 0);
> -}
> -
> -static int
> -qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> -{
> -	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
> -
> -	return qca8k_read(priv, reg, val);
> -}
> -
> -static int
> -qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val)
> -{
> -	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
> -
> -	return qca8k_write(priv, reg, val);
> -}
> -
>  static const struct regmap_range qca8k_readable_ranges[] = {
>  	regmap_reg_range(0x0000, 0x00e4), /* Global control */
>  	regmap_reg_range(0x0100, 0x0168), /* EEE control */
> @@ -282,26 +258,19 @@ static struct regmap_config qca8k_regmap_config = {
>  	.max_register = 0x16ac, /* end MIB - Port6 range */
>  	.reg_read = qca8k_regmap_read,
>  	.reg_write = qca8k_regmap_write,
> +	.reg_update_bits = qca8k_regmap_update_bits,
>  	.rd_table = &qca8k_readable_table,
> +	.disable_locking = true, /* Locking is handled by qca8k read/write */
> +	.cache_type = REGCACHE_NONE, /* Explicitly disable CACHE */
>  };
>  
>  static int
>  qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask)
>  {
> -	int ret, ret1;
>  	u32 val;
>  
> -	ret = read_poll_timeout(qca8k_read, ret1, !(val & mask),
> -				0, QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC, false,
> -				priv, reg, &val);
> -
> -	/* Check if qca8k_read has failed for a different reason
> -	 * before returning -ETIMEDOUT
> -	 */
> -	if (ret < 0 && ret1 < 0)
> -		return ret1;
> -
> -	return ret;
> +	return regmap_read_poll_timeout(priv->regmap, reg, val, !(val & mask), 0,
> +				       QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC);
>  }
>  
>  static int
> @@ -312,7 +281,7 @@ qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
>  
>  	/* load the ARL table into an array */
>  	for (i = 0; i < 4; i++) {
> -		ret = qca8k_read(priv, QCA8K_REG_ATU_DATA0 + (i * 4), &val);
> +		ret = regmap_read(priv->regmap, QCA8K_REG_ATU_DATA0 + (i * 4), &val);

Maybe you could keep qca8k_read and qca8k_write and make them return
regmap_read(priv->regmap, ...), this could reduce the patch's delta,
make future bugfix patches conflict less with this change, etc etc.
What do you think?

Re: [net-next PATCH v2 5/9] net: dsa: qca8k: convert qca8k to regmap helper

[Ansuel Smith]

On Mon, Nov 22, 2021 at 03:38:53AM +0200, Vladimir Oltean wrote:
> On Mon, Nov 22, 2021 at 02:03:09AM +0100, Ansuel Smith wrote:
> > Convert any qca8k read/write/rmw/set/clear/pool to regmap helper and add
> > missing config to regmap_config struct.
> > Ipq40xx SoC have the internal switch based on the qca8k regmap but use
> > mmio for read/write/rmw operation instead of mdio.
> > In preparation for the support of this internal switch, convert the
> > driver to regmap API to later split the driver to common and specific
> > code. The overhead introduced by the use of regamp API is marginal as the
> > internal mdio will bypass it by using its direct access and regmap will be
> > used only by configuration functions or fdb access.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  drivers/net/dsa/qca8k.c | 289 ++++++++++++++++++----------------------
> >  1 file changed, 131 insertions(+), 158 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> > index 52fca800e6f7..159a1065e66b 100644
> > --- a/drivers/net/dsa/qca8k.c
> > +++ b/drivers/net/dsa/qca8k.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/phy.h>
> >  #include <linux/netdevice.h>
> >  #include <linux/bitfield.h>
> > +#include <linux/regmap.h>
> >  #include <net/dsa.h>
> >  #include <linux/of_net.h>
> >  #include <linux/of_mdio.h>
> > @@ -150,8 +151,9 @@ qca8k_set_page(struct mii_bus *bus, u16 page)
> >  }
> >  
> >  static int
> > -qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
> > +qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> >  {
> > +	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
> >  	struct mii_bus *bus = priv->bus;
> >  	u16 r1, r2, page;
> >  	int ret;
> > @@ -172,8 +174,9 @@ qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
> >  }
> >  
> >  static int
> > -qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
> > +qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val)
> >  {
> > +	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
> >  	struct mii_bus *bus = priv->bus;
> >  	u16 r1, r2, page;
> >  	int ret;
> > @@ -194,8 +197,9 @@ qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
> >  }
> >  
> >  static int
> > -qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
> > +qca8k_regmap_update_bits(void *ctx, uint32_t reg, uint32_t mask, uint32_t write_val)
> >  {
> > +	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
> >  	struct mii_bus *bus = priv->bus;
> >  	u16 r1, r2, page;
> >  	u32 val;
> > @@ -223,34 +227,6 @@ qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
> >  	return ret;
> >  }
> >  
> > -static int
> > -qca8k_reg_set(struct qca8k_priv *priv, u32 reg, u32 val)
> > -{
> > -	return qca8k_rmw(priv, reg, 0, val);
> > -}
> > -
> > -static int
> > -qca8k_reg_clear(struct qca8k_priv *priv, u32 reg, u32 val)
> > -{
> > -	return qca8k_rmw(priv, reg, val, 0);
> > -}
> > -
> > -static int
> > -qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> > -{
> > -	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
> > -
> > -	return qca8k_read(priv, reg, val);
> > -}
> > -
> > -static int
> > -qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val)
> > -{
> > -	struct qca8k_priv *priv = (struct qca8k_priv *)ctx;
> > -
> > -	return qca8k_write(priv, reg, val);
> > -}
> > -
> >  static const struct regmap_range qca8k_readable_ranges[] = {
> >  	regmap_reg_range(0x0000, 0x00e4), /* Global control */
> >  	regmap_reg_range(0x0100, 0x0168), /* EEE control */
> > @@ -282,26 +258,19 @@ static struct regmap_config qca8k_regmap_config = {
> >  	.max_register = 0x16ac, /* end MIB - Port6 range */
> >  	.reg_read = qca8k_regmap_read,
> >  	.reg_write = qca8k_regmap_write,
> > +	.reg_update_bits = qca8k_regmap_update_bits,
> >  	.rd_table = &qca8k_readable_table,
> > +	.disable_locking = true, /* Locking is handled by qca8k read/write */
> > +	.cache_type = REGCACHE_NONE, /* Explicitly disable CACHE */
> >  };
> >  
> >  static int
> >  qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask)
> >  {
> > -	int ret, ret1;
> >  	u32 val;
> >  
> > -	ret = read_poll_timeout(qca8k_read, ret1, !(val & mask),
> > -				0, QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC, false,
> > -				priv, reg, &val);
> > -
> > -	/* Check if qca8k_read has failed for a different reason
> > -	 * before returning -ETIMEDOUT
> > -	 */
> > -	if (ret < 0 && ret1 < 0)
> > -		return ret1;
> > -
> > -	return ret;
> > +	return regmap_read_poll_timeout(priv->regmap, reg, val, !(val & mask), 0,
> > +				       QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC);
> >  }
> >  
> >  static int
> > @@ -312,7 +281,7 @@ qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
> >  
> >  	/* load the ARL table into an array */
> >  	for (i = 0; i < 4; i++) {
> > -		ret = qca8k_read(priv, QCA8K_REG_ATU_DATA0 + (i * 4), &val);
> > +		ret = regmap_read(priv->regmap, QCA8K_REG_ATU_DATA0 + (i * 4), &val);
> 
> Maybe you could keep qca8k_read and qca8k_write and make them return
> regmap_read(priv->regmap, ...), this could reduce the patch's delta,
> make future bugfix patches conflict less with this change, etc etc.
> What do you think?

Problem is that many function will have to be moved to a separate file
(for the common stuff) and they won't have qca8k_read/write/rmw...
So converting everything to regmap would be handy as you drop the
extra functions.
But I see how reworking the read/write/rmw would massively reduce the
patch delta.

When we will have to split the code, we will have this problem again and
we will have to decide if continue using the qca8k_read/write... or drop
them and switch to regmap.

So... yes i'm stuck as if we want to save some conflicts we will have to
carry the extra function forver I think.
(Wonder if the conflict problem will just be """solved""" with the code
split as someone will have to rework the patch anyway as the function
will be located on a different file)

-- 
	Ansuel

Re: [net-next PATCH v2 5/9] net: dsa: qca8k: convert qca8k to regmap helper

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:56:58AM +0100, Ansuel Smith wrote:
> > Maybe you could keep qca8k_read and qca8k_write and make them return
> > regmap_read(priv->regmap, ...), this could reduce the patch's delta,
> > make future bugfix patches conflict less with this change, etc etc.
> > What do you think?
> 
> Problem is that many function will have to be moved to a separate file
> (for the common stuff) and they won't have qca8k_read/write/rmw...
> So converting everything to regmap would be handy as you drop the
> extra functions.
> But I see how reworking the read/write/rmw would massively reduce the
> patch delta.
> 
> When we will have to split the code, we will have this problem again and
> we will have to decide if continue using the qca8k_read/write... or drop
> them and switch to regmap.
> 
> So... yes i'm stuck as if we want to save some conflicts we will have to
> carry the extra function forver I think.
> (Wonder if the conflict problem will just be """solved""" with the code
> split as someone will have to rework the patch anyway as the function
> will be located on a different file)

Yeah, well, if you have to split then you have to split. It probably
won't be pretty, with a lot of code added for v5.16 and now for v5.17,
so it hasn't had time to reach to a larger pool of users and get cleaned
of bugs which you didn't notice. But what can you do. Other than wait
for a few months for the code to stabilize (which is counterproductive
in its own ways), probably nothing.

[net-next PATCH v2 6/9] net: dsa: qca8k: add additional MIB counter and make it dynamic

[Ansuel Smith]

We are currently missing 2 additionals MIB counter present in QCA833x
switch.
QC832x switch have 39 MIB counter and QCA833X have 41 MIB counter.
Add the additional MIB counter and rework the MIB function to print the
correct supported counter from the match_data struct.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
---
 drivers/net/dsa/qca8k.c | 23 ++++++++++++++++++++---
 drivers/net/dsa/qca8k.h |  4 ++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 159a1065e66b..57ba387a4aa1 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -70,6 +70,8 @@ static const struct qca8k_mib_desc ar8327_mib[] = {
 	MIB_DESC(1, 0x9c, "TxExcDefer"),
 	MIB_DESC(1, 0xa0, "TxDefer"),
 	MIB_DESC(1, 0xa4, "TxLateCol"),
+	MIB_DESC(1, 0xa8, "RXUnicast"),
+	MIB_DESC(1, 0xac, "TXUnicast"),
 };
 
 /* The 32bit switch registers are accessed indirectly. To achieve this we need
@@ -1591,12 +1593,16 @@ qca8k_phylink_mac_link_up(struct dsa_switch *ds, int port, unsigned int mode,
 static void
 qca8k_get_strings(struct dsa_switch *ds, int port, u32 stringset, uint8_t *data)
 {
+	const struct qca8k_match_data *match_data;
+	struct qca8k_priv *priv = ds->priv;
 	int i;
 
 	if (stringset != ETH_SS_STATS)
 		return;
 
-	for (i = 0; i < ARRAY_SIZE(ar8327_mib); i++)
+	match_data = of_device_get_match_data(priv->dev);
+
+	for (i = 0; i < match_data->mib_count; i++)
 		strncpy(data + i * ETH_GSTRING_LEN, ar8327_mib[i].name,
 			ETH_GSTRING_LEN);
 }
@@ -1606,12 +1612,15 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
 			uint64_t *data)
 {
 	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
+	const struct qca8k_match_data *match_data;
 	const struct qca8k_mib_desc *mib;
 	u32 reg, i, val;
 	u32 hi = 0;
 	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(ar8327_mib); i++) {
+	match_data = of_device_get_match_data(priv->dev);
+
+	for (i = 0; i < match_data->mib_count; i++) {
 		mib = &ar8327_mib[i];
 		reg = QCA8K_PORT_MIB_COUNTER(port) + mib->offset;
 
@@ -1634,10 +1643,15 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
 static int
 qca8k_get_sset_count(struct dsa_switch *ds, int port, int sset)
 {
+	const struct qca8k_match_data *match_data;
+	struct qca8k_priv *priv = ds->priv;
+
 	if (sset != ETH_SS_STATS)
 		return 0;
 
-	return ARRAY_SIZE(ar8327_mib);
+	match_data = of_device_get_match_data(priv->dev);
+
+	return match_data->mib_count;
 }
 
 static int
@@ -2140,14 +2154,17 @@ static SIMPLE_DEV_PM_OPS(qca8k_pm_ops,
 static const struct qca8k_match_data qca8327 = {
 	.id = QCA8K_ID_QCA8327,
 	.reduced_package = true,
+	.mib_count = QCA8K_QCA832X_MIB_COUNT,
 };
 
 static const struct qca8k_match_data qca8328 = {
 	.id = QCA8K_ID_QCA8327,
+	.mib_count = QCA8K_QCA832X_MIB_COUNT,
 };
 
 static const struct qca8k_match_data qca833x = {
 	.id = QCA8K_ID_QCA8337,
+	.mib_count = QCA8K_QCA833X_MIB_COUNT,
 };
 
 static const struct of_device_id qca8k_of_match[] = {
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 085885275398..91c94dfc9789 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -21,6 +21,9 @@
 #define PHY_ID_QCA8337					0x004dd036
 #define QCA8K_ID_QCA8337				0x13
 
+#define QCA8K_QCA832X_MIB_COUNT				39
+#define QCA8K_QCA833X_MIB_COUNT				41
+
 #define QCA8K_BUSY_WAIT_TIMEOUT				2000
 
 #define QCA8K_NUM_FDB_RECORDS				2048
@@ -279,6 +282,7 @@ struct ar8xxx_port_status {
 struct qca8k_match_data {
 	u8 id;
 	bool reduced_package;
+	u8 mib_count;
 };
 
 enum {
-- 
2.32.0

[net-next PATCH v2 7/9] net: dsa: qca8k: add support for port fast aging

[Ansuel Smith]

The switch supports fast aging by flushing any rule in the ARL
table for a specific port.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 11 +++++++++++
 drivers/net/dsa/qca8k.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 57ba387a4aa1..1ff951b6fe07 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1776,6 +1776,16 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
 			   QCA8K_PORT_LOOKUP_MEMBER, BIT(cpu_port));
 }
 
+static void
+qca8k_port_fast_age(struct dsa_switch *ds, int port)
+{
+	struct qca8k_priv *priv = ds->priv;
+
+	mutex_lock(&priv->reg_mutex);
+	qca8k_fdb_access(priv, QCA8K_FDB_FLUSH_PORT, port);
+	mutex_unlock(&priv->reg_mutex);
+}
+
 static int
 qca8k_port_enable(struct dsa_switch *ds, int port,
 		  struct phy_device *phy)
@@ -1984,6 +1994,7 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
 	.port_stp_state_set	= qca8k_port_stp_state_set,
 	.port_bridge_join	= qca8k_port_bridge_join,
 	.port_bridge_leave	= qca8k_port_bridge_leave,
+	.port_fast_age		= qca8k_port_fast_age,
 	.port_fdb_add		= qca8k_port_fdb_add,
 	.port_fdb_del		= qca8k_port_fdb_del,
 	.port_fdb_dump		= qca8k_port_fdb_dump,
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 91c94dfc9789..a533b8cf143b 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -262,6 +262,7 @@ enum qca8k_fdb_cmd {
 	QCA8K_FDB_FLUSH	= 1,
 	QCA8K_FDB_LOAD = 2,
 	QCA8K_FDB_PURGE = 3,
+	QCA8K_FDB_FLUSH_PORT = 5,
 	QCA8K_FDB_NEXT = 6,
 	QCA8K_FDB_SEARCH = 7,
 };
-- 
2.32.0

Re: [net-next PATCH v2 7/9] net: dsa: qca8k: add support for port fast aging

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:03:11AM +0100, Ansuel Smith wrote:
> The switch supports fast aging by flushing any rule in the ARL
> table for a specific port.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

[net-next PATCH v2 8/9] net: dsa: qca8k: add set_ageing_time support

[Ansuel Smith]

qca8k support setting ageing time in step of 7s. Add support for it and
set the max value accepted of 7645m.
Documentation talks about support for 10000m but that values doesn't
make sense as the value doesn't match the max value in the reg.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 25 +++++++++++++++++++++++++
 drivers/net/dsa/qca8k.h |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 1ff951b6fe07..21a7f1ed7a5c 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1247,6 +1247,10 @@ qca8k_setup(struct dsa_switch *ds)
 	/* We don't have interrupts for link changes, so we need to poll */
 	ds->pcs_poll = true;
 
+	/* Set min a max ageing value supported */
+	ds->ageing_time_min = 7000;
+	ds->ageing_time_max = 458745000;
+
 	return 0;
 }
 
@@ -1786,6 +1790,26 @@ qca8k_port_fast_age(struct dsa_switch *ds, int port)
 	mutex_unlock(&priv->reg_mutex);
 }
 
+static int
+qca8k_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
+{
+	struct qca8k_priv *priv = ds->priv;
+	unsigned int secs = msecs / 1000;
+	u32 val;
+
+	/* AGE_TIME reg is set in 7s step */
+	val = secs / 7;
+
+	/* Handle case with 0 as val to NOT disable
+	 * learning
+	 */
+	if (!val)
+		val = 1;
+
+	return regmap_update_bits(priv->regmap, QCA8K_REG_ATU_CTRL, QCA8K_ATU_AGE_TIME_MASK,
+				  QCA8K_ATU_AGE_TIME(val));
+}
+
 static int
 qca8k_port_enable(struct dsa_switch *ds, int port,
 		  struct phy_device *phy)
@@ -1985,6 +2009,7 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
 	.get_strings		= qca8k_get_strings,
 	.get_ethtool_stats	= qca8k_get_ethtool_stats,
 	.get_sset_count		= qca8k_get_sset_count,
+	.set_ageing_time	= qca8k_set_ageing_time,
 	.get_mac_eee		= qca8k_get_mac_eee,
 	.set_mac_eee		= qca8k_set_mac_eee,
 	.port_enable		= qca8k_port_enable,
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index a533b8cf143b..40ec8012622f 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -175,6 +175,9 @@
 #define   QCA8K_VTU_FUNC1_BUSY				BIT(31)
 #define   QCA8K_VTU_FUNC1_VID_MASK			GENMASK(27, 16)
 #define   QCA8K_VTU_FUNC1_FULL				BIT(4)
+#define QCA8K_REG_ATU_CTRL				0x618
+#define   QCA8K_ATU_AGE_TIME_MASK			GENMASK(15, 0)
+#define   QCA8K_ATU_AGE_TIME(x)				FIELD_PREP(QCA8K_ATU_AGE_TIME_MASK, (x))
 #define QCA8K_REG_GLOBAL_FW_CTRL0			0x620
 #define   QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN		BIT(10)
 #define QCA8K_REG_GLOBAL_FW_CTRL1			0x624
-- 
2.32.0

Re: [net-next PATCH v2 8/9] net: dsa: qca8k: add set_ageing_time support

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:03:12AM +0100, Ansuel Smith wrote:
> qca8k support setting ageing time in step of 7s. Add support for it and
> set the max value accepted of 7645m.
> Documentation talks about support for 10000m but that values doesn't
> make sense as the value doesn't match the max value in the reg.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

[net-next PATCH v2 9/9] net: dsa: qca8k: add support for mdb_add/del

[Ansuel Smith]

Add support for mdb add/del function. The ARL table is used to insert
the rule. The rule will be searched, deleted and reinserted with the
port mask updated. The function will check if the rule has to be updated
or insert directly with no deletion of the old rule.
If every port is removed from the port mask, the rule is removed.
The rule is set STATIC in the ARL table (aka it doesn't age) to not be
flushed by fast age function.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 97 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 21a7f1ed7a5c..e37528c8dbf2 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -417,6 +417,79 @@ qca8k_fdb_flush(struct qca8k_priv *priv)
 	mutex_unlock(&priv->reg_mutex);
 }
 
+static int
+qca8k_fdb_search_and_insert(struct qca8k_priv *priv, u8 port_mask, const u8 *mac, u16 vid)
+{
+	struct qca8k_fdb fdb = { 0 };
+	int ret;
+
+	mutex_lock(&priv->reg_mutex);
+
+	qca8k_fdb_write(priv, vid, 0, mac, 0);
+	ret = qca8k_fdb_access(priv, QCA8K_FDB_SEARCH, -1);
+	if (ret < 0)
+		goto exit;
+
+	ret = qca8k_fdb_read(priv, &fdb);
+	if (ret < 0)
+		goto exit;
+
+	/* Rule exist. Delete first */
+	if (!fdb.aging) {
+		ret = qca8k_fdb_access(priv, QCA8K_FDB_PURGE, -1);
+		if (ret)
+			goto exit;
+	}
+
+	/* Add port to fdb portmask */
+	fdb.port_mask |= port_mask;
+
+	qca8k_fdb_write(priv, vid, fdb.port_mask, mac, fdb.aging);
+	ret = qca8k_fdb_access(priv, QCA8K_FDB_LOAD, -1);
+
+exit:
+	mutex_unlock(&priv->reg_mutex);
+	return ret;
+}
+
+static int
+qca8k_fdb_search_and_del(struct qca8k_priv *priv, u8 port_mask, const u8 *mac, u16 vid)
+{
+	struct qca8k_fdb fdb = { 0 };
+	int ret;
+
+	mutex_lock(&priv->reg_mutex);
+
+	qca8k_fdb_write(priv, vid, 0, mac, 0);
+	ret = qca8k_fdb_access(priv, QCA8K_FDB_SEARCH, -1);
+	if (ret < 0)
+		goto exit;
+
+	/* Rule doesn't exist. Why delete? */
+	if (!fdb.aging) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	ret = qca8k_fdb_access(priv, QCA8K_FDB_PURGE, -1);
+	if (ret)
+		goto exit;
+
+	/* Only port in the rule is this port. Don't re insert */
+	if (fdb.port_mask == port_mask)
+		goto exit;
+
+	/* Remove port from port mask */
+	fdb.port_mask &= ~port_mask;
+
+	qca8k_fdb_write(priv, vid, fdb.port_mask, mac, fdb.aging);
+	ret = qca8k_fdb_access(priv, QCA8K_FDB_LOAD, -1);
+
+exit:
+	mutex_unlock(&priv->reg_mutex);
+	return ret;
+}
+
 static int
 qca8k_vlan_access(struct qca8k_priv *priv, enum qca8k_vlan_cmd cmd, u16 vid)
 {
@@ -1915,6 +1988,28 @@ qca8k_port_fdb_dump(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+qca8k_port_mdb_add(struct dsa_switch *ds, int port,
+		   const struct switchdev_obj_port_mdb *mdb)
+{
+	struct qca8k_priv *priv = ds->priv;
+	const u8 *addr = mdb->addr;
+	u16 vid = mdb->vid;
+
+	return qca8k_fdb_search_and_insert(priv, BIT(port), addr, vid);
+}
+
+static int
+qca8k_port_mdb_del(struct dsa_switch *ds, int port,
+		   const struct switchdev_obj_port_mdb *mdb)
+{
+	struct qca8k_priv *priv = ds->priv;
+	const u8 *addr = mdb->addr;
+	u16 vid = mdb->vid;
+
+	return qca8k_fdb_search_and_del(priv, BIT(port), addr, vid);
+}
+
 static int
 qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 			  struct netlink_ext_ack *extack)
@@ -2023,6 +2118,8 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
 	.port_fdb_add		= qca8k_port_fdb_add,
 	.port_fdb_del		= qca8k_port_fdb_del,
 	.port_fdb_dump		= qca8k_port_fdb_dump,
+	.port_mdb_add		= qca8k_port_mdb_add,
+	.port_mdb_del		= qca8k_port_mdb_del,
 	.port_vlan_filtering	= qca8k_port_vlan_filtering,
 	.port_vlan_add		= qca8k_port_vlan_add,
 	.port_vlan_del		= qca8k_port_vlan_del,
-- 
2.32.0

Re: [net-next PATCH v2 9/9] net: dsa: qca8k: add support for mdb_add/del

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:03:13AM +0100, Ansuel Smith wrote:
> Add support for mdb add/del function. The ARL table is used to insert
> the rule. The rule will be searched, deleted and reinserted with the
> port mask updated. The function will check if the rule has to be updated
> or insert directly with no deletion of the old rule.
> If every port is removed from the port mask, the rule is removed.
> The rule is set STATIC in the ARL table (aka it doesn't age) to not be
> flushed by fast age function.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/net/dsa/qca8k.c | 97 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 21a7f1ed7a5c..e37528c8dbf2 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -417,6 +417,79 @@ qca8k_fdb_flush(struct qca8k_priv *priv)
>  	mutex_unlock(&priv->reg_mutex);
>  }
>  
> +static int
> +qca8k_fdb_search_and_insert(struct qca8k_priv *priv, u8 port_mask, const u8 *mac, u16 vid)

FYI the networking tree is still sticking to its guns w.r.t. the 80
character per line limit. You can ignore the rule if you want to and it
makes sense, for example if you're printing a long string on nearby
lines (which shouldn't be split into multiple lines, because people grep
for error messages) and therefore it wouldn't look so out of place to
also have lines that are a bit longer. But in this case, the function
prototypes are sticking out like a sore thumb IMO, the nearby lines are
short otherwise.

And to be honest I don't understand your choice of where to split the
line either, this consumes two lines too, but doesn't exceed 80 characters:

static int qca8k_fdb_search_and_insert(struct qca8k_priv *priv, u8 port_mask,
				       const u8 *mac, u16 vid)

Otherwise:

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

> +{
> +	struct qca8k_fdb fdb = { 0 };
> +	int ret;
> +
> +	mutex_lock(&priv->reg_mutex);
> +
> +	qca8k_fdb_write(priv, vid, 0, mac, 0);
> +	ret = qca8k_fdb_access(priv, QCA8K_FDB_SEARCH, -1);
> +	if (ret < 0)
> +		goto exit;
> +
> +	ret = qca8k_fdb_read(priv, &fdb);
> +	if (ret < 0)
> +		goto exit;
> +
> +	/* Rule exist. Delete first */
> +	if (!fdb.aging) {
> +		ret = qca8k_fdb_access(priv, QCA8K_FDB_PURGE, -1);
> +		if (ret)
> +			goto exit;
> +	}
> +
> +	/* Add port to fdb portmask */
> +	fdb.port_mask |= port_mask;
> +
> +	qca8k_fdb_write(priv, vid, fdb.port_mask, mac, fdb.aging);
> +	ret = qca8k_fdb_access(priv, QCA8K_FDB_LOAD, -1);
> +
> +exit:
> +	mutex_unlock(&priv->reg_mutex);
> +	return ret;
> +}
> +
> +static int
> +qca8k_fdb_search_and_del(struct qca8k_priv *priv, u8 port_mask, const u8 *mac, u16 vid)
> +{
> +	struct qca8k_fdb fdb = { 0 };
> +	int ret;
> +
> +	mutex_lock(&priv->reg_mutex);
> +
> +	qca8k_fdb_write(priv, vid, 0, mac, 0);
> +	ret = qca8k_fdb_access(priv, QCA8K_FDB_SEARCH, -1);
> +	if (ret < 0)
> +		goto exit;
> +
> +	/* Rule doesn't exist. Why delete? */
> +	if (!fdb.aging) {
> +		ret = -EINVAL;
> +		goto exit;
> +	}
> +
> +	ret = qca8k_fdb_access(priv, QCA8K_FDB_PURGE, -1);
> +	if (ret)
> +		goto exit;
> +
> +	/* Only port in the rule is this port. Don't re insert */
> +	if (fdb.port_mask == port_mask)
> +		goto exit;
> +
> +	/* Remove port from port mask */
> +	fdb.port_mask &= ~port_mask;
> +
> +	qca8k_fdb_write(priv, vid, fdb.port_mask, mac, fdb.aging);
> +	ret = qca8k_fdb_access(priv, QCA8K_FDB_LOAD, -1);
> +
> +exit:
> +	mutex_unlock(&priv->reg_mutex);
> +	return ret;
> +}
> +
>  static int
>  qca8k_vlan_access(struct qca8k_priv *priv, enum qca8k_vlan_cmd cmd, u16 vid)
>  {
> @@ -1915,6 +1988,28 @@ qca8k_port_fdb_dump(struct dsa_switch *ds, int port,
>  	return 0;
>  }
>  
> +static int
> +qca8k_port_mdb_add(struct dsa_switch *ds, int port,
> +		   const struct switchdev_obj_port_mdb *mdb)
> +{
> +	struct qca8k_priv *priv = ds->priv;
> +	const u8 *addr = mdb->addr;
> +	u16 vid = mdb->vid;
> +
> +	return qca8k_fdb_search_and_insert(priv, BIT(port), addr, vid);
> +}
> +
> +static int
> +qca8k_port_mdb_del(struct dsa_switch *ds, int port,
> +		   const struct switchdev_obj_port_mdb *mdb)
> +{
> +	struct qca8k_priv *priv = ds->priv;
> +	const u8 *addr = mdb->addr;
> +	u16 vid = mdb->vid;
> +
> +	return qca8k_fdb_search_and_del(priv, BIT(port), addr, vid);
> +}
> +
>  static int
>  qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
>  			  struct netlink_ext_ack *extack)
> @@ -2023,6 +2118,8 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
>  	.port_fdb_add		= qca8k_port_fdb_add,
>  	.port_fdb_del		= qca8k_port_fdb_del,
>  	.port_fdb_dump		= qca8k_port_fdb_dump,
> +	.port_mdb_add		= qca8k_port_mdb_add,
> +	.port_mdb_del		= qca8k_port_mdb_del,
>  	.port_vlan_filtering	= qca8k_port_vlan_filtering,
>  	.port_vlan_add		= qca8k_port_vlan_add,
>  	.port_vlan_del		= qca8k_port_vlan_del,
> -- 
> 2.32.0
> 

Re: [net-next PATCH v2 0/9] Multiple cleanup and feature for qca8k

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:03:04AM +0100, Ansuel Smith wrote:
> This is a reduced version of the old massive series.
> Refer to the changelog to know what is removed from this.
> 
> THIS IS BASED ON net-next WITH THE 2 FIXES FROM net ALREADY REVIEWED
> net: dsa: qca8k: fix MTU calculation
> net: dsa: qca8k: fix internal delay applied to the wrong PAD config

Since patchwork has auto build hooks now, it doesn't detect dependencies
to other trees like "net" in this case, and your patches will fail to
apply without the other ones you've mentioned, which in turn will make
the builds fail. Patches without clean build reports aren't accepted, so
you'll have to resend either way. Your options are:
(a) wait until the bugfix patches get applied to "net", and Jakub and/or
    David send the networking pull request for v5.16-rc3 to Linus, then
    they'll merge the "net" tree into "net-next" quickly afterwards and
    your patches apply cleanly. Last two "net" pull requests were
    submitted on Nov 18th and 12th, if that is any indication as to when
    the next one is going to be.
(b) base your patches on "net-next" without the bug fixes, and let
    Jakub/David handle the merge conflict when the'll merge "net" into
    "net-next" next time. Please note that if you do this, there is a
    small chance that mistakes can be made, and you can't easily
    backport patches to a stable tree such as OpenWRT if that's what
    you're into, since part of the delta will be in a merge commit, and
    there isn't any simple way in which you can linearize that during
    cherry-pick time, if you're picking from divergent branches.

Re: [net-next PATCH v2 0/9] Multiple cleanup and feature for qca8k

[Ansuel Smith]

On Mon, Nov 22, 2021 at 03:29:10AM +0200, Vladimir Oltean wrote:
> On Mon, Nov 22, 2021 at 02:03:04AM +0100, Ansuel Smith wrote:
> > This is a reduced version of the old massive series.
> > Refer to the changelog to know what is removed from this.
> > 
> > THIS IS BASED ON net-next WITH THE 2 FIXES FROM net ALREADY REVIEWED
> > net: dsa: qca8k: fix MTU calculation
> > net: dsa: qca8k: fix internal delay applied to the wrong PAD config
> 
> Since patchwork has auto build hooks now, it doesn't detect dependencies
> to other trees like "net" in this case, and your patches will fail to
> apply without the other ones you've mentioned, which in turn will make
> the builds fail. Patches without clean build reports aren't accepted, so
> you'll have to resend either way. Your options are:
> (a) wait until the bugfix patches get applied to "net", and Jakub and/or
>     David send the networking pull request for v5.16-rc3 to Linus, then
>     they'll merge the "net" tree into "net-next" quickly afterwards and
>     your patches apply cleanly. Last two "net" pull requests were
>     submitted on Nov 18th and 12th, if that is any indication as to when
>     the next one is going to be.
> (b) base your patches on "net-next" without the bug fixes, and let
>     Jakub/David handle the merge conflict when the'll merge "net" into
>     "net-next" next time. Please note that if you do this, there is a
>     small chance that mistakes can be made, and you can't easily
>     backport patches to a stable tree such as OpenWRT if that's what
>     you're into, since part of the delta will be in a merge commit, and
>     there isn't any simple way in which you can linearize that during
>     cherry-pick time, if you're picking from divergent branches.

Mhhh I honestly think b option can be accepted here (due to the fact
that fixes patch are very small) but the backport part can be
problematic.
Think it's better to just wait and get the reviewed by tag.

Is it problematic to add stuff to this series while the fixes are
merged? (for example the LAGs or mirror part / the code split)
Or having big series is still problematic even if half of the patch are
already reviewed?
Just asking if there is a way to continue the review process while we
wait for the merge process.

-- 
	Ansuel

Re: [net-next PATCH v2 0/9] Multiple cleanup and feature for qca8k

[Vladimir Oltean]

On Mon, Nov 22, 2021 at 02:43:46AM +0100, Ansuel Smith wrote:
> On Mon, Nov 22, 2021 at 03:29:10AM +0200, Vladimir Oltean wrote:
> > On Mon, Nov 22, 2021 at 02:03:04AM +0100, Ansuel Smith wrote:
> > > This is a reduced version of the old massive series.
> > > Refer to the changelog to know what is removed from this.
> > > 
> > > THIS IS BASED ON net-next WITH THE 2 FIXES FROM net ALREADY REVIEWED
> > > net: dsa: qca8k: fix MTU calculation
> > > net: dsa: qca8k: fix internal delay applied to the wrong PAD config
> > 
> > Since patchwork has auto build hooks now, it doesn't detect dependencies
> > to other trees like "net" in this case, and your patches will fail to
> > apply without the other ones you've mentioned, which in turn will make
> > the builds fail. Patches without clean build reports aren't accepted, so
> > you'll have to resend either way. Your options are:
> > (a) wait until the bugfix patches get applied to "net", and Jakub and/or
> >     David send the networking pull request for v5.16-rc3 to Linus, then
> >     they'll merge the "net" tree into "net-next" quickly afterwards and
> >     your patches apply cleanly. Last two "net" pull requests were
> >     submitted on Nov 18th and 12th, if that is any indication as to when
> >     the next one is going to be.
> > (b) base your patches on "net-next" without the bug fixes, and let
> >     Jakub/David handle the merge conflict when the'll merge "net" into
> >     "net-next" next time. Please note that if you do this, there is a
> >     small chance that mistakes can be made, and you can't easily
> >     backport patches to a stable tree such as OpenWRT if that's what
> >     you're into, since part of the delta will be in a merge commit, and
> >     there isn't any simple way in which you can linearize that during
> >     cherry-pick time, if you're picking from divergent branches.
> 
> Mhhh I honestly think b option can be accepted here (due to the fact
> that fixes patch are very small) but the backport part can be
> problematic.
> Think it's better to just wait and get the reviewed by tag.

There is an option (c), which sometimes can be done and sometimes can't,
which is to write the patches in such a way that they don't conflict
with each other. I haven't checked what the exact conflicts are here,
but that regmap conversion thing is pretty noisy, you're renaming every
register read and write. Being more moderate about it can work to your
advantage.

> Is it problematic to add stuff to this series while the fixes are
> merged? (for example the LAGs or mirror part / the code split)
> Or having big series is still problematic even if half of the patch are
> already reviewed?
> Just asking if there is a way to continue the review process while we
> wait for the merge process.

You can always send RFC patches because for those, the build part isn't
so important, and there you can specifically ask for review tags and/or
point reviewers to other patch sets that they should apply first, were
they to review your submission by actually applying to a git tree and
not just from reading the patches in the email client.

I would still have multiple series of manageable sizes instead of a
monolithic one, just because it is conceptually easier to refer to them
("add qca8k support for X/Y/Z features" rather than "qca8k-misc-2021-11-22").

Re: [net-next PATCH v2 0/9] Multiple cleanup and feature for qca8k

[David Miller]

This series does not apply cleanly to net-next, please respin.

Thank you.