[net-next,v5,06/16] net: dsa: vsc73xx: add port_stp_state_set function

Message ID 20240223210049.3197486-7-paweldembicki@gmail.com
State New
Headers
Series [net-next,v5,01/16] net: dsa: vsc73xx: use read_poll_timeout instead delay loop |

Commit Message

Pawel Dembicki Feb. 23, 2024, 9 p.m. UTC
  This isn't a fully functional implementation of 802.1D, but
port_stp_state_set is required for a future tag8021q operations.

This implementation handles properly all states, but vsc73xx doesn't
forward STP packets.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
v5:
  - remove unneeded 'RECVMASK' operations
  - reorganise vsc73xx_refresh_fwd_map function
v4:
  - fully reworked port_stp_state_set
v3:
  - use 'VSC73XX_MAX_NUM_PORTS' define
  - add 'state == BR_STATE_DISABLED' condition
  - fix style issues
v2:
  - fix kdoc

 drivers/net/dsa/vitesse-vsc73xx-core.c | 99 +++++++++++++++++++++++---
 1 file changed, 88 insertions(+), 11 deletions(-)
  

Comments

Simon Horman Feb. 27, 2024, 7:27 p.m. UTC | #1
On Fri, Feb 23, 2024 at 10:00:36PM +0100, Pawel Dembicki wrote:
> This isn't a fully functional implementation of 802.1D, but
> port_stp_state_set is required for a future tag8021q operations.
> 
> This implementation handles properly all states, but vsc73xx doesn't
> forward STP packets.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

..

> diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c

..

> @@ -1036,6 +1029,89 @@ static void vsc73xx_phylink_get_caps(struct dsa_switch *dsa, int port,
>  	config->mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000;
>  }
>  
> +static void vsc73xx_refresh_fwd_map(struct dsa_switch *ds, int port, u8 state)
> +{
> +	struct dsa_port *other_dp, *dp = dsa_to_port(ds, port);
> +	struct vsc73xx *vsc = ds->priv;
> +	u16 mask;
> +
> +	if (state != BR_STATE_FORWARDING) {
> +		/* Ports that aren't in the forwarding state must not
> +		 * forward packets anywhere.
> +		 */
> +		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> +				    VSC73XX_SRCMASKS + port,
> +				    VSC73XX_SRCMASKS_PORTS_MASK, 0);
> +
> +		dsa_switch_for_each_available_port(other_dp, ds) {
> +			if (other_dp == dp)
> +				continue;
> +			vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
> +					    VSC73XX_SRCMASKS + other_dp->index,
> +					    BIT(port), 0);
> +		}
> +
> +	return;

Nit: the line above should be indented by one more tab.

> +	}

..
  
kernel test robot March 1, 2024, 11:35 a.m. UTC | #2
Hi Pawel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Pawel-Dembicki/net-dsa-vsc73xx-use-read_poll_timeout-instead-delay-loop/20240224-050950
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240223210049.3197486-7-paweldembicki%40gmail.com
patch subject: [PATCH net-next v5 06/16] net: dsa: vsc73xx: add port_stp_state_set function
config: x86_64-randconfig-161-20240301 (https://download.01.org/0day-ci/archive/20240301/202403011918.VO71zUCH-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403011918.VO71zUCH-lkp@intel.com/

smatch warnings:
drivers/net/dsa/vitesse-vsc73xx-core.c:1054 vsc73xx_refresh_fwd_map() warn: inconsistent indenting

vim +1054 drivers/net/dsa/vitesse-vsc73xx-core.c

  1031	
  1032	static void vsc73xx_refresh_fwd_map(struct dsa_switch *ds, int port, u8 state)
  1033	{
  1034		struct dsa_port *other_dp, *dp = dsa_to_port(ds, port);
  1035		struct vsc73xx *vsc = ds->priv;
  1036		u16 mask;
  1037	
  1038		if (state != BR_STATE_FORWARDING) {
  1039			/* Ports that aren't in the forwarding state must not
  1040			 * forward packets anywhere.
  1041			 */
  1042			vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
  1043					    VSC73XX_SRCMASKS + port,
  1044					    VSC73XX_SRCMASKS_PORTS_MASK, 0);
  1045	
  1046			dsa_switch_for_each_available_port(other_dp, ds) {
  1047				if (other_dp == dp)
  1048					continue;
  1049				vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
  1050						    VSC73XX_SRCMASKS + other_dp->index,
  1051						    BIT(port), 0);
  1052			}
  1053	
> 1054		return;
  1055		}
  1056	
  1057		/* Forwarding ports must forward to the CPU and to other ports
  1058		 * in the same bridge
  1059		 */
  1060		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
  1061				    VSC73XX_SRCMASKS + CPU_PORT, BIT(port), BIT(port));
  1062	
  1063		mask = BIT(CPU_PORT);
  1064	
  1065		if (dp->bridge) {
  1066			dsa_switch_for_each_user_port(other_dp, ds) {
  1067				if (other_dp->bridge == dp->bridge &&
  1068				    other_dp->index != port &&
  1069				    other_dp->stp_state == BR_STATE_FORWARDING) {
  1070					int other_port = other_dp->index;
  1071	
  1072					mask |= BIT(other_port);
  1073					vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER,
  1074							    0,
  1075							    VSC73XX_SRCMASKS +
  1076							    other_port,
  1077							    BIT(port), BIT(port));
  1078				}
  1079			}
  1080		}
  1081	
  1082		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
  1083				    VSC73XX_SRCMASKS + port,
  1084				    VSC73XX_SRCMASKS_PORTS_MASK, mask);
  1085	}
  1086
  

Patch

diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 425999d7bf41..c3ef4c22f687 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -164,6 +164,10 @@ 
 #define VSC73XX_AGENCTRL	0xf0
 #define VSC73XX_CAPRST		0xff
 
+#define VSC73XX_SRCMASKS_CPU_COPY		BIT(27)
+#define VSC73XX_SRCMASKS_MIRROR			BIT(26)
+#define VSC73XX_SRCMASKS_PORTS_MASK		GENMASK(7, 0)
+
 #define VSC73XX_MACACCESS_CPU_COPY		BIT(14)
 #define VSC73XX_MACACCESS_FWD_KILL		BIT(13)
 #define VSC73XX_MACACCESS_IGNORE_VLAN		BIT(12)
@@ -623,9 +627,6 @@  static int vsc73xx_setup(struct dsa_switch *ds)
 	vsc73xx_write(vsc, VSC73XX_BLOCK_SYSTEM, 0, VSC73XX_GMIIDELAY,
 		      VSC73XX_GMIIDELAY_GMII0_GTXDELAY_2_0_NS |
 		      VSC73XX_GMIIDELAY_GMII0_RXDELAY_2_0_NS);
-	/* Enable reception of frames on all ports */
-	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_RECVMASK,
-		      0x5f);
 	/* IP multicast flood mask (table 144) */
 	vsc73xx_write(vsc, VSC73XX_BLOCK_ANALYZER, 0, VSC73XX_IFLODMSK,
 		      0xff);
@@ -785,10 +786,6 @@  static void vsc73xx_phylink_mac_link_down(struct dsa_switch *ds, int port,
 	/* Allow backward dropping of frames from this port */
 	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
 			    VSC73XX_SBACKWDROP, BIT(port), BIT(port));
-
-	/* Receive mask (disable forwarding) */
-	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
-			    VSC73XX_RECVMASK, BIT(port), 0);
 }
 
 static void vsc73xx_phylink_mac_link_up(struct dsa_switch *ds, int port,
@@ -841,10 +838,6 @@  static void vsc73xx_phylink_mac_link_up(struct dsa_switch *ds, int port,
 	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
 			    VSC73XX_ARBDISC, BIT(port), 0);
 
-	/* Enable port (forwarding) in the receieve mask */
-	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
-			    VSC73XX_RECVMASK, BIT(port), BIT(port));
-
 	/* Disallow backward dropping of frames from this port */
 	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
 			    VSC73XX_SBACKWDROP, BIT(port), 0);
@@ -1036,6 +1029,89 @@  static void vsc73xx_phylink_get_caps(struct dsa_switch *dsa, int port,
 	config->mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000;
 }
 
+static void vsc73xx_refresh_fwd_map(struct dsa_switch *ds, int port, u8 state)
+{
+	struct dsa_port *other_dp, *dp = dsa_to_port(ds, port);
+	struct vsc73xx *vsc = ds->priv;
+	u16 mask;
+
+	if (state != BR_STATE_FORWARDING) {
+		/* Ports that aren't in the forwarding state must not
+		 * forward packets anywhere.
+		 */
+		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+				    VSC73XX_SRCMASKS + port,
+				    VSC73XX_SRCMASKS_PORTS_MASK, 0);
+
+		dsa_switch_for_each_available_port(other_dp, ds) {
+			if (other_dp == dp)
+				continue;
+			vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+					    VSC73XX_SRCMASKS + other_dp->index,
+					    BIT(port), 0);
+		}
+
+	return;
+	}
+
+	/* Forwarding ports must forward to the CPU and to other ports
+	 * in the same bridge
+	 */
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+			    VSC73XX_SRCMASKS + CPU_PORT, BIT(port), BIT(port));
+
+	mask = BIT(CPU_PORT);
+
+	if (dp->bridge) {
+		dsa_switch_for_each_user_port(other_dp, ds) {
+			if (other_dp->bridge == dp->bridge &&
+			    other_dp->index != port &&
+			    other_dp->stp_state == BR_STATE_FORWARDING) {
+				int other_port = other_dp->index;
+
+				mask |= BIT(other_port);
+				vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER,
+						    0,
+						    VSC73XX_SRCMASKS +
+						    other_port,
+						    BIT(port), BIT(port));
+			}
+		}
+	}
+
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+			    VSC73XX_SRCMASKS + port,
+			    VSC73XX_SRCMASKS_PORTS_MASK, mask);
+}
+
+/* FIXME: STP frames aren't forwarded at this moment. BPDU frames are
+ * forwarded only from and to PI/SI interface. For more info see chapter
+ * 2.7.1 (CPU Forwarding) in datasheet.
+ * This function is required for tag_8021q operations.
+ */
+static void vsc73xx_port_stp_state_set(struct dsa_switch *ds, int port,
+				       u8 state)
+{
+	struct vsc73xx *vsc = ds->priv;
+	u32 val;
+
+	val = (state == BR_STATE_BLOCKING || state == BR_STATE_DISABLED) ?
+	      0 : BIT(port);
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+			    VSC73XX_RECVMASK, BIT(port), val);
+
+	val = (state == BR_STATE_LEARNING || state == BR_STATE_FORWARDING) ?
+	      BIT(port) : 0;
+	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,
+			    VSC73XX_LEARNMASK, BIT(port), val);
+
+	/* CPU Port should always forward packets when user ports are forwarding
+	 * so let's configure it from other ports only.
+	 */
+	if (port != CPU_PORT)
+		vsc73xx_refresh_fwd_map(ds, port, state);
+}
+
 static const struct dsa_switch_ops vsc73xx_ds_ops = {
 	.get_tag_protocol = vsc73xx_get_tag_protocol,
 	.setup = vsc73xx_setup,
@@ -1051,6 +1127,7 @@  static const struct dsa_switch_ops vsc73xx_ds_ops = {
 	.port_disable = vsc73xx_port_disable,
 	.port_change_mtu = vsc73xx_change_mtu,
 	.port_max_mtu = vsc73xx_get_max_mtu,
+	.port_stp_state_set = vsc73xx_port_stp_state_set,
 	.phylink_get_caps = vsc73xx_phylink_get_caps,
 };