[net-next RESEND PATCH 1/2] net: dsa: qca8k: tidy for loop in setup and add cpu port check

[net-next RESEND PATCH 1/2] net: dsa: qca8k: tidy for loop in setup and add cpu port check

[Ansuel Smith]

Tidy and organize qca8k setup function from multiple for loop.
Change for loop in bridge leave/join to scan all port and skip cpu port.
No functional change intended.

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

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 2b0aadb0114c..ba0411d4c5ae 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -1122,28 +1122,34 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		dev_warn(priv->dev, "mib init failed");
 
-	/* Enable QCA header mode on the cpu port */
-	ret = qca8k_write(priv, QCA8K_REG_PORT_HDR_CTRL(cpu_port),
-			  QCA8K_PORT_HDR_CTRL_ALL << QCA8K_PORT_HDR_CTRL_TX_S |
-			  QCA8K_PORT_HDR_CTRL_ALL << QCA8K_PORT_HDR_CTRL_RX_S);
-	if (ret) {
-		dev_err(priv->dev, "failed enabling QCA header mode");
-		return ret;
-	}
-
-	/* Disable forwarding by default on all ports */
+	/* 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);
 		if (ret)
 			return ret;
-	}
 
-	/* Disable MAC by default on all ports */
-	for (i = 1; i < QCA8K_NUM_PORTS; i++)
-		qca8k_port_set_status(priv, i, 0);
+		/* 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);
+			if (ret) {
+				dev_err(priv->dev, "failed enabling QCA header mode");
+				return ret;
+			}
+		}
 
-	/* Forward all unknown frames to CPU port for Linux processing */
+		/* Disable MAC by default on all user ports */
+		if (dsa_is_user_port(ds, i))
+			qca8k_port_set_status(priv, i, 0);
+	}
+
+	/* Forward all unknown frames to CPU port for Linux processing
+	 * 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,
 			  BIT(cpu_port) << QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_S |
 			  BIT(cpu_port) << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
@@ -1152,11 +1158,13 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
-	/* Setup connection between CPU port & user ports */
+	/* Setup connection between CPU port & user ports
+	 * Configure specific switch configuration for ports
+	 */
 	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(cpu_port),
+			ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i),
 					QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds));
 			if (ret)
 				return ret;
@@ -1193,16 +1201,14 @@ qca8k_setup(struct dsa_switch *ds)
 			if (ret)
 				return ret;
 		}
-	}
 
-	/* The port 5 of the qca8337 have some problem in flood condition. The
-	 * original legacy driver had some specific buffer and priority settings
-	 * for the different port suggested by the QCA switch team. Add this
-	 * missing settings to improve switch stability under load condition.
-	 * This problem is limited to qca8337 and other qca8k switch are not affected.
-	 */
-	if (priv->switch_id == QCA8K_ID_QCA8337) {
-		for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		/* The port 5 of the qca8337 have some problem in flood condition. The
+		 * original legacy driver had some specific buffer and priority settings
+		 * for the different port suggested by the QCA switch team. Add this
+		 * missing settings to improve switch stability under load condition.
+		 * This problem is limited to qca8337 and other qca8k switch are not affected.
+		 */
+		if (priv->switch_id == QCA8K_ID_QCA8337) {
 			switch (i) {
 			/* The 2 CPU port and port 5 requires some different
 			 * priority than any other ports.
@@ -1238,6 +1244,12 @@ qca8k_setup(struct dsa_switch *ds)
 				  QCA8K_PORT_HOL_CTRL1_WRED_EN,
 				  mask);
 		}
+
+		/* Set initial MTU for every port.
+		 * We have only have a general MTU setting. So track
+		 * every port and set the max across all port.
+		 */
+		priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN;
 	}
 
 	/* Special GLOBAL_FC_THRESH value are needed for ar8327 switch */
@@ -1251,8 +1263,6 @@ qca8k_setup(struct dsa_switch *ds)
 	}
 
 	/* Setup our port MTUs to match power on defaults */
-	for (i = 0; i < QCA8K_NUM_PORTS; i++)
-		priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN;
 	ret = qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN);
 	if (ret)
 		dev_warn(priv->dev, "failed setting MTU settings");
@@ -1728,7 +1738,9 @@ qca8k_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br)
 	cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
 	port_mask = BIT(cpu_port);
 
-	for (i = 1; i < QCA8K_NUM_PORTS; i++) {
+	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		if (dsa_is_cpu_port(ds, i))
+			continue;
 		if (dsa_to_port(ds, i)->bridge_dev != br)
 			continue;
 		/* Add this port to the portvlan mask of the other ports
@@ -1758,7 +1770,9 @@ qca8k_port_bridge_leave(struct dsa_switch *ds, int port, struct net_device *br)
 
 	cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
 
-	for (i = 1; i < QCA8K_NUM_PORTS; i++) {
+	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		if (dsa_is_cpu_port(ds, i))
+			continue;
 		if (dsa_to_port(ds, i)->bridge_dev != br)
 			continue;
 		/* Remove this port to the portvlan mask of the other ports
-- 
2.32.0

[net-next RESEND PATCH 2/2] net: dsa: qca8k: fix delay applied to wrong cpu in parse_port_config

[Ansuel Smith]

Fix delay settings applied to wrong cpu in parse_port_config. The delay
values is set to the wrong index as the cpu_port_index is incremented
too early. Start the cpu_port_index to -1 so the correct value is
applied to address also the case with invalid phy mode and not available
port.

Signed-off-by: Ansuel Smith <ansuelsmth@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 ba0411d4c5ae..ee51186720d2 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -976,7 +976,7 @@ qca8k_setup_of_pws_reg(struct qca8k_priv *priv)
 static int
 qca8k_parse_port_config(struct qca8k_priv *priv)
 {
-	int port, cpu_port_index = 0, ret;
+	int port, cpu_port_index = -1, ret;
 	struct device_node *port_dn;
 	phy_interface_t mode;
 	struct dsa_port *dp;
-- 
2.32.0

Re: [net-next RESEND PATCH 1/2] net: dsa: qca8k: tidy for loop in setup and add cpu port check

[Jakub Kicinski]

On Sun, 17 Oct 2021 16:56:45 +0200 Ansuel Smith wrote:
> Tidy and organize qca8k setup function from multiple for loop.
> Change for loop in bridge leave/join to scan all port and skip cpu port.
> No functional change intended.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

There's some confusion in patchwork. I think previous posting got
applied, but patch 1 of this series git marked as applied.. while 
it was patch 2 that corresponds to previous posting..?

Please make sure you mark new postings as v2 v3 etc. It's not a problem
to post a vN+1 and say "no changes" in the change log, while it may be 
a problem if patchwork bot gets confused and doesn't mark series as
superseded appropriately.

I'm dropping the remainder of this series from patchwork, please rebase
and resend what's missing in net-next.

Thanks!

Re: [net-next RESEND PATCH 1/2] net: dsa: qca8k: tidy for loop in setup and add cpu port check

[Ansuel Smith]

>
> On Sun, 17 Oct 2021 16:56:45 +0200 Ansuel Smith wrote:
> > Tidy and organize qca8k setup function from multiple for loop.
> > Change for loop in bridge leave/join to scan all port and skip cpu port.
> > No functional change intended.
> >
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
>
> There's some confusion in patchwork. I think previous posting got
> applied, but patch 1 of this series git marked as applied.. while
> it was patch 2 that corresponds to previous posting..?
>
> Please make sure you mark new postings as v2 v3 etc. It's not a problem
> to post a vN+1 and say "no changes" in the change log, while it may be
> a problem if patchwork bot gets confused and doesn't mark series as
> superseded appropriately.
>
> I'm dropping the remainder of this series from patchwork, please rebase
> and resend what's missing in net-next.
>
> Thanks!

Sorry for the mess. I think I got confused.
I resent these 2 patch (in one go) as i didn't add the net-next tag
and i thought they got ignored as the target was wrong.
I didn't receive any review or ack so i thought it was a good idea to
resend them in one go with the correct tag.
Hope it's not a stupid question but can you point me where should
i check to prevent this kind of error?
So anyway i both send these 2 patch as a dedicated patch with the
absent tag.

Re: [net-next RESEND PATCH 1/2] net: dsa: qca8k: tidy for loop in setup and add cpu port check

[Jakub Kicinski]

On Tue, 19 Oct 2021 00:54:17 +0200 Ansuel Smith wrote:
> > > Tidy and organize qca8k setup function from multiple for loop.
> > > Change for loop in bridge leave/join to scan all port and skip cpu port.
> > > No functional change intended.
> > >
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>  
> >
> > There's some confusion in patchwork. I think previous posting got
> > applied, but patch 1 of this series git marked as applied.. while
> > it was patch 2 that corresponds to previous posting..?
> >
> > Please make sure you mark new postings as v2 v3 etc. It's not a problem
> > to post a vN+1 and say "no changes" in the change log, while it may be
> > a problem if patchwork bot gets confused and doesn't mark series as
> > superseded appropriately.
> >
> > I'm dropping the remainder of this series from patchwork, please rebase
> > and resend what's missing in net-next.
> >
> > Thanks!  
> 
> Sorry for the mess. I think I got confused.
> I resent these 2 patch (in one go) as i didn't add the net-next tag
> and i thought they got ignored as the target was wrong.
> I didn't receive any review or ack so i thought it was a good idea to
> resend them in one go with the correct tag.
> Hope it's not a stupid question but can you point me where should
> i check to prevent this kind of error?

You can check in patchwork if your submission was indeed ignored.

All the "active" patches are here:

https://patchwork.kernel.org/project/netdevbpf/list/

You can also look up particular patch by using it's message ID:

https://patchwork.kernel.org/project/netdevbpf/patch/<msg-id>/

E.g.

https://patchwork.kernel.org/project/netdevbpf/patch/20211017145646.56-1-ansuelsmth@gmail.com/

If the patch is in New, Under review or Needs ACK state then there's 
no need to resend.

> So anyway i both send these 2 patch as a dedicated patch with the
> absent tag.

Ah! I see the first posting of both now, looks like patchwork realized
it's a repost of patch 1 so it marked that as superseded.

Re: [net-next RESEND PATCH 1/2] net: dsa: qca8k: tidy for loop in setup and add cpu port check

[Ansuel Smith]

>
> On Tue, 19 Oct 2021 00:54:17 +0200 Ansuel Smith wrote:
> > > > Tidy and organize qca8k setup function from multiple for loop.
> > > > Change for loop in bridge leave/join to scan all port and skip cpu port.
> > > > No functional change intended.
> > > >
> > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > >
> > > There's some confusion in patchwork. I think previous posting got
> > > applied, but patch 1 of this series git marked as applied.. while
> > > it was patch 2 that corresponds to previous posting..?
> > >
> > > Please make sure you mark new postings as v2 v3 etc. It's not a problem
> > > to post a vN+1 and say "no changes" in the change log, while it may be
> > > a problem if patchwork bot gets confused and doesn't mark series as
> > > superseded appropriately.
> > >
> > > I'm dropping the remainder of this series from patchwork, please rebase
> > > and resend what's missing in net-next.
> > >
> > > Thanks!
> >
> > Sorry for the mess. I think I got confused.
> > I resent these 2 patch (in one go) as i didn't add the net-next tag
> > and i thought they got ignored as the target was wrong.
> > I didn't receive any review or ack so i thought it was a good idea to
> > resend them in one go with the correct tag.
> > Hope it's not a stupid question but can you point me where should
> > i check to prevent this kind of error?
>
> You can check in patchwork if your submission was indeed ignored.
>
> All the "active" patches are here:
>
> https://patchwork.kernel.org/project/netdevbpf/list/
>
> You can also look up particular patch by using it's message ID:
>
> https://patchwork.kernel.org/project/netdevbpf/patch/<msg-id>/
>
> E.g.
>
> https://patchwork.kernel.org/project/netdevbpf/patch/20211017145646.56-1-ansuelsmth@gmail.com/
>
> If the patch is in New, Under review or Needs ACK state then there's
> no need to resend.
>
> > So anyway i both send these 2 patch as a dedicated patch with the
> > absent tag.
>
> Ah! I see the first posting of both now, looks like patchwork realized
> it's a repost of patch 1 so it marked that as superseded.

Should I resend just that with the correct tag?
Thx a lot for the hint about patchwork

Re: [net-next RESEND PATCH 1/2] net: dsa: qca8k: tidy for loop in setup and add cpu port check

[Jakub Kicinski]

On Tue, 19 Oct 2021 01:14:30 +0200 Ansuel Smith wrote:
> > > So anyway i both send these 2 patch as a dedicated patch with the
> > > absent tag.  
> >
> > Ah! I see the first posting of both now, looks like patchwork realized
> > it's a repost of patch 1 so it marked that as superseded.  
> 
> Should I resend just that with the correct tag?

Please fetch the latest net-next/master, and rebase your branch with
these changes. git will figure out what's already applied. Resend the
rest as [PATCH net-next v2].