diff mbox series

[net-next,v1,8/9] net: dsa: qca: ar9331: add STP support

Message ID 20210403114848.30528-9-o.rempel@pengutronix.de (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series ar9331: mainline some parts of switch functionality | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 2 maintainers not CCed: rmk+kernel@armlinux.org.uk linux@rempel-privat.de
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: networking block comments don't use an empty /* line, use /* Comment...
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Oleksij Rempel April 3, 2021, 11:48 a.m. UTC
According to the datasheet, this switch has configurable STP port
states. Suddenly LISTENING and BLOCKING states didn't forwarded packets
to the CPU and linux bridge continuously re enabled ports even if a  loop
was detected. To make it work, I reused bridge functionality to isolate
port in LISTENING and BLOCKING states.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/qca/ar9331.c | 69 ++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)
diff mbox series

Patch

diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index bf9588574205..83b59e771a5f 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -327,6 +327,7 @@  struct ar9331_sw_priv {
 	struct reset_control *sw_reset;
 	struct ar9331_sw_port port[AR9331_SW_PORTS];
 	int cpu_port;
+	u32 isolated_ports;
 };
 
 static struct ar9331_sw_priv *ar9331_sw_port_to_priv(struct ar9331_sw_port *port)
@@ -1151,6 +1152,10 @@  static int ar9331_sw_port_bridge_join(struct dsa_switch *ds, int port,
 		if (!dsa_is_user_port(ds, port))
 			continue;
 
+		/* part of the bridge but should be isolated for now */
+		if (priv->isolated_ports & BIT(i))
+			continue;
+
 		val = FIELD_PREP(AR9331_SW_PORT_VLAN_PORT_VID_MEMBER, BIT(port));
 		ret = regmap_set_bits(regmap, AR9331_SW_REG_PORT_VLAN(i), val);
 		if (ret)
@@ -1205,6 +1210,69 @@  static void ar9331_sw_port_bridge_leave(struct dsa_switch *ds, int port,
 	dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);
 }
 
+static void ar9331_sw_port_stp_state_set(struct dsa_switch *ds, int port,
+					 u8 state)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	struct net_device *br = dsa_to_port(ds, port)->bridge_dev;
+	struct regmap *regmap = priv->regmap;
+	u32 port_ctrl = 0, port_state = 0;
+	bool join = false;
+	int ret;
+
+	/*
+	 * STP hw support is buggy or I didn't understood it. So, it seems to
+	 * be easier to make hand crafted implementation by using bridge
+	 * functionality. Similar implementation can be found on ksz9477 switch
+	 * and may be we need some generic code to so for all related devices
+	 */
+	switch (state) {
+	case BR_STATE_FORWARDING:
+		join = true;
+		fallthrough;
+	case BR_STATE_LEARNING:
+		port_ctrl = AR9331_SW_PORT_CTRL_LEARN_EN;
+		fallthrough;
+	case BR_STATE_LISTENING:
+	case BR_STATE_BLOCKING:
+		port_state = AR9331_SW_PORT_CTRL_PORT_STATE_FORWARD;
+		break;
+	case BR_STATE_DISABLED:
+	default:
+		port_state = AR9331_SW_PORT_CTRL_PORT_STATE_DISABLED;
+		break;
+	}
+
+	port_ctrl |= FIELD_PREP(AR9331_SW_PORT_CTRL_PORT_STATE, port_state);
+
+	ret = regmap_update_bits(regmap, AR9331_SW_REG_PORT_CTRL(port),
+				 AR9331_SW_PORT_CTRL_LEARN_EN |
+				 AR9331_SW_PORT_CTRL_PORT_STATE, port_ctrl);
+	if (ret)
+		goto error;
+
+	if (!dsa_is_user_port(ds, port))
+		return;
+
+	/*
+	 * here we care only about user ports. CPU port do not need this
+	 * configuration
+	 */
+	if (join) {
+		priv->isolated_ports &= ~BIT(port);
+		if (br)
+			ar9331_sw_port_bridge_join(ds, port, br);
+	} else {
+		priv->isolated_ports |= BIT(port);
+		if (br)
+			ar9331_sw_port_bridge_leave(ds, port, br);
+	}
+
+	return;
+error:
+	dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);
+}
+
 static const struct dsa_switch_ops ar9331_sw_ops = {
 	.get_tag_protocol	= ar9331_sw_get_tag_protocol,
 	.setup			= ar9331_sw_setup,
@@ -1223,6 +1291,7 @@  static const struct dsa_switch_ops ar9331_sw_ops = {
 	.set_ageing_time	= ar9331_sw_set_ageing_time,
 	.port_bridge_join	= ar9331_sw_port_bridge_join,
 	.port_bridge_leave	= ar9331_sw_port_bridge_leave,
+	.port_stp_state_set	= ar9331_sw_port_stp_state_set,
 };
 
 static irqreturn_t ar9331_sw_irq(int irq, void *data)