[RFC PATCH net-next 00/11] net: dsa: microchip: lan937x: add switch cascade support

[RFC PATCH net-next 00/11] net: dsa: microchip: lan937x: add switch cascade support

[Rakesh Sankaranarayanan]

LAN937x switch series support cascade mode of operation, in which two
switch can be connected to work like a single switch having the
advantage of increased number of ports. Two switches can be connected
using SPI, and a dedicated port from each switch will be inter-connected 
forming a data path between two switches, known as cascaded port.

This patch series add support for cascade mode of operation using SPI
protocol and configures cascaded ports from each switches based on the
requirement.

Patch series tested on LAN9373 Dual Board, which is a custom board
with two LAN9373 switches connected in cascaded mode, and PORT 4
used as cascaded port from each switch.

Rakesh Sankaranarayanan (11):
  net: dsa: microchip: lan937x: add cascade tailtag
  net: dsa: microchip: lan937x: update SMI index
  net: dsa: microchip: lan937x: enable cascade port
  net: dsa: microchip: lan937x: update port number for LAN9373
  net: dsa: microchip: lan937x: add shared global interrupt
  net: dsa: microchip: lan937x: get cascade tag protocol
  net: dsa: microchip: lan937x: update switch register
  net: dsa: microchip: lan937x: avoid mib read for cascaded port
  net: dsa: microchip: lan937x: update port membership with dsa port
  net: dsa: microchip: lan937x: update vlan untag membership
  net: dsa: microchip: lan937x: update multicast table

 drivers/net/dsa/microchip/ksz9477.c      |  8 ++-
 drivers/net/dsa/microchip/ksz_common.c   | 47 ++++++++++----
 drivers/net/dsa/microchip/ksz_common.h   |  3 +
 drivers/net/dsa/microchip/lan937x.h      |  1 +
 drivers/net/dsa/microchip/lan937x_main.c | 33 +++++++++-
 drivers/net/dsa/microchip/lan937x_reg.h  |  3 +
 include/net/dsa.h                        | 17 +++++
 net/dsa/tag_ksz.c                        | 80 ++++++++++++++++++++++--
 8 files changed, 173 insertions(+), 19 deletions(-)

-- 
2.34.1

[RFC PATCH net-next 01/11] net: dsa: microchip: lan937x: add cascade tailtag

[Rakesh Sankaranarayanan]

cascade tailtag contains 3 bytes of information, it includes
additional bytes for accomodating port number in second switch.
Destination port bitmap on first switch is at bit position 7:0 and
of second switch is at bit position 15:8, add new tailtag xmit and
rcv functions for cascade with proper formatting. Add new tag protocol
for cascading and link with new xmit and rcv functions.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 include/net/dsa.h |  2 ++
 net/dsa/tag_ksz.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index a15f17a38eca..55651ad29193 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -56,6 +56,7 @@ struct phylink_link_state;
 #define DSA_TAG_PROTO_RTL8_4T_VALUE		25
 #define DSA_TAG_PROTO_RZN1_A5PSW_VALUE		26
 #define DSA_TAG_PROTO_LAN937X_VALUE		27
+#define DSA_TAG_PROTO_LAN937X_CASCADE_VALUE     28
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -86,6 +87,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_RTL8_4T		= DSA_TAG_PROTO_RTL8_4T_VALUE,
 	DSA_TAG_PROTO_RZN1_A5PSW	= DSA_TAG_PROTO_RZN1_A5PSW_VALUE,
 	DSA_TAG_PROTO_LAN937X		= DSA_TAG_PROTO_LAN937X_VALUE,
+	DSA_TAG_PROTO_LAN937X_CASCADE   = DSA_TAG_PROTO_LAN937X_CASCADE_VALUE,
 };
 
 struct dsa_switch;
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 0eb1c7784c3d..7ab2c7eaa4ca 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -16,6 +16,7 @@
 #define KSZ9477_NAME "ksz9477"
 #define KSZ9893_NAME "ksz9893"
 #define LAN937X_NAME "lan937x"
+#define LAN937X_CASCADE_NAME "lan937x_cascade"
 
 /* Typically only one byte is used for tail tag. */
 #define KSZ_PTP_TAG_LEN			4
@@ -24,6 +25,9 @@
 
 #define KSZ_HWTS_EN  0
 
+#define SWITCH_0       0
+#define SWITCH_1       1
+
 struct ksz_tagger_private {
 	struct ksz_tagger_data data; /* Must be first */
 	unsigned long state;
@@ -84,10 +88,10 @@ static int ksz_connect(struct dsa_switch *ds)
 }
 
 static struct sk_buff *ksz_common_rcv(struct sk_buff *skb,
-				      struct net_device *dev,
-				      unsigned int port, unsigned int len)
+				      struct net_device *dev, unsigned int port,
+				      unsigned int len, u8 device)
 {
-	skb->dev = dsa_master_find_slave(dev, 0, port);
+	skb->dev = dsa_master_find_slave(dev, device, port);
 	if (!skb->dev)
 		return NULL;
 
@@ -141,7 +145,7 @@ static struct sk_buff *ksz8795_rcv(struct sk_buff *skb, struct net_device *dev)
 {
 	u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
 
-	return ksz_common_rcv(skb, dev, tag[0] & 7, KSZ_EGRESS_TAG_LEN);
+	return ksz_common_rcv(skb, dev, tag[0] & 7, KSZ_EGRESS_TAG_LEN, SWITCH_0);
 }
 
 static const struct dsa_device_ops ksz8795_netdev_ops = {
@@ -177,6 +181,7 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8795, KSZ8795_NAME);
 #define KSZ9477_INGRESS_TAG_LEN		2
 #define KSZ9477_PTP_TAG_LEN		4
 #define KSZ9477_PTP_TAG_INDICATION	0x80
+#define LAN937X_CASCADE_CHIP		0x40
 
 #define KSZ9477_TAIL_TAG_PRIO		GENMASK(8, 7)
 #define KSZ9477_TAIL_TAG_OVERRIDE	BIT(9)
@@ -304,6 +309,7 @@ static struct sk_buff *ksz9477_rcv(struct sk_buff *skb, struct net_device *dev)
 	u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
 	unsigned int port = tag[0] & 7;
 	unsigned int len = KSZ_EGRESS_TAG_LEN;
+	u8 device = SWITCH_0;
 
 	/* Extra 4-bytes PTP timestamp */
 	if (tag[0] & KSZ9477_PTP_TAG_INDICATION) {
@@ -311,7 +317,10 @@ static struct sk_buff *ksz9477_rcv(struct sk_buff *skb, struct net_device *dev)
 		len += KSZ_PTP_TAG_LEN;
 	}
 
-	return ksz_common_rcv(skb, dev, port, len);
+	if (tag[0] & LAN937X_CASCADE_CHIP)
+		device = SWITCH_1;
+
+	return ksz_common_rcv(skb, dev, port, len, device);
 }
 
 static const struct dsa_device_ops ksz9477_netdev_ops = {
@@ -390,6 +399,7 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
  *	  (eg, 0x00=port1, 0x02=port3, 0x07=port8)
  */
 #define LAN937X_EGRESS_TAG_LEN		2
+#define LAN937X_CASCADE_TAG_LEN		3
 
 #define LAN937X_TAIL_TAG_BLOCKING_OVERRIDE	BIT(11)
 #define LAN937X_TAIL_TAG_LOOKUP			BIT(12)
@@ -442,11 +452,71 @@ static const struct dsa_device_ops lan937x_netdev_ops = {
 DSA_TAG_DRIVER(lan937x_netdev_ops);
 MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X, LAN937X_NAME);
 
+/* For xmit, 3/7 bytes are added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|tag1(1byte)|
+ * tag2(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * ts   : time stamp (Present only if PTP is enabled in the Hardware)
+ * tag0 : represents tag override, lookup and valid
+ * tag1 : each bit represents destination port map through switch 2
+ *	  (eg, 0x01=port1, 0x02=port2, 0x80=port8)
+ * tag2 : each bit represents destination port map through switch 1
+ *	  (eg, 0x01=port1, 0x02=port2, 0x80=port8)
+ *
+ * For rcv, 1/5 bytes is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * ts   : time stamp (Present only if bit 7 of tag0 is set)
+ * tag0 : zero-based value represents port
+ *	  (eg, 0x00=port1, 0x02=port3, 0x07=port8)
+ */
+static struct sk_buff *lan937x_cascade_xmit(struct sk_buff *skb,
+					    struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_slave_to_port(dev);
+	const struct ethhdr *hdr = eth_hdr(skb);
+	__be32 *tag;
+	u32 val;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
+		return NULL;
+
+	tag = skb_put(skb, LAN937X_CASCADE_TAG_LEN);
+
+	val |= BIT((dp->index + (8 * dp->ds->index)));
+
+	if (is_link_local_ether_addr(hdr->h_dest))
+		val |= (LAN937X_TAIL_TAG_BLOCKING_OVERRIDE << 8);
+
+	val |= (LAN937X_TAIL_TAG_VALID << 8);
+
+	put_unaligned_be24(val, tag);
+
+	return skb;
+}
+
+static const struct dsa_device_ops lan937x_cascade_netdev_ops = {
+	.name   = LAN937X_CASCADE_NAME,
+	.proto  = DSA_TAG_PROTO_LAN937X_CASCADE,
+	.xmit   = lan937x_cascade_xmit,
+	.rcv    = ksz9477_rcv,
+	.connect = ksz_connect,
+	.disconnect = ksz_disconnect,
+	.needed_tailroom = LAN937X_CASCADE_TAG_LEN,
+};
+
+DSA_TAG_DRIVER(lan937x_cascade_netdev_ops);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937x_CASCADE,
+			    LAN937X_CASCADE_NAME);
+
 static struct dsa_tag_driver *dsa_tag_driver_array[] = {
 	&DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
 	&DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
 	&DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),
 	&DSA_TAG_DRIVER_NAME(lan937x_netdev_ops),
+	&DSA_TAG_DRIVER_NAME(lan937x_cascade_netdev_ops),
 };
 
 module_dsa_tag_drivers(dsa_tag_driver_array);
-- 
2.34.1

[RFC PATCH net-next 02/11] net: dsa: microchip: lan937x: update SMI index

[Rakesh Sankaranarayanan]

Current DSA driver register mdio interface for a port in the
format of SMI-switch_index:port_number, switch_index is derived
using variable ds->index. For a single switch ds->index will be
always zero, and for cascaded switch, ds->index should be one.
But it is found that ds->index is getting updated only after
mdio_register stage. Update mdio_register to use variable directly
from device tree using "dsa,member" identifier.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 6 +++++-
 drivers/net/dsa/microchip/ksz_common.h | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 46becc0382d6..d2ec5acd7b17 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1882,7 +1882,7 @@ static int ksz_mdio_register(struct ksz_device *dev)
 	bus->read = ksz_sw_mdio_read;
 	bus->write = ksz_sw_mdio_write;
 	bus->name = "ksz slave smi";
-	snprintf(bus->id, MII_BUS_ID_SIZE, "SMI-%d", ds->index);
+	snprintf(bus->id, MII_BUS_ID_SIZE, "SMI-%d", dev->smi_index);
 	bus->parent = ds->dev;
 	bus->phy_mask = ~ds->phys_mii_mask;
 
@@ -3136,6 +3136,7 @@ struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
 {
 	struct dsa_switch *ds;
 	struct ksz_device *swdev;
+	u32 sw_idx[2];
 
 	ds = devm_kzalloc(base, sizeof(*ds), GFP_KERNEL);
 	if (!ds)
@@ -3155,6 +3156,9 @@ struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
 	swdev->ds = ds;
 	swdev->priv = priv;
 
+	of_property_read_variable_u32_array(base->of_node, "dsa,member", sw_idx, 2, 2);
+	swdev->smi_index = sw_idx[1];
+
 	return swdev;
 }
 EXPORT_SYMBOL(ksz_switch_alloc);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index d2d5761d58e9..aab60f2587bf 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -147,6 +147,7 @@ struct ksz_device {
 	u32 chip_id;
 	u8 chip_rev;
 	int cpu_port;			/* port connected to CPU */
+	u32 smi_index;
 	int phy_port_cnt;
 	phy_interface_t compat_interface;
 	bool synclko_125;
-- 
2.34.1

[RFC PATCH net-next 03/11] net: dsa: microchip: lan937x: enable cascade port

[Rakesh Sankaranarayanan]

Get index of cascaded port (if any) from device tree and enable
the feature. These ports referenced as dev->dsa_port and will be
used for processing further based on cascaded connection.

For the second switch in cascaded connection, no dev->cpu_port will
be assigned, and same happens for dev->dsa_port variable for switches
without cascading. For the single switch design, there is no way
dev->cpu_port will be unassigned. But coming to cascaded connection,
it can be unassigned, and they will be having value zero. Keeping the
initial value as zero will create error in other features like port
forwarding since DSA will misunderstood these as port index zero. So
keep the default values as 0xFF which is of invalid value so that if
nothing assigned, taking bitmap of the cpu_port or dsa_port will not
cause any harm.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c   |  4 +++
 drivers/net/dsa/microchip/ksz_common.h   |  2 ++
 drivers/net/dsa/microchip/lan937x.h      |  1 +
 drivers/net/dsa/microchip/lan937x_main.c | 31 ++++++++++++++++++++++++
 drivers/net/dsa/microchip/lan937x_reg.h  |  3 +++
 5 files changed, 41 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index d2ec5acd7b17..ada673b6efc6 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -292,6 +292,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
 	.change_mtu = lan937x_change_mtu,
 	.phylink_mac_link_up = ksz9477_phylink_mac_link_up,
 	.config_cpu_port = lan937x_config_cpu_port,
+	.config_dsa_port = lan937x_config_dsa_port,
 	.tc_cbs_set_cinc = lan937x_tc_cbs_set_cinc,
 	.enable_stp_addr = ksz9477_enable_stp_addr,
 	.reset = lan937x_reset_switch,
@@ -2095,6 +2096,9 @@ static int ksz_setup(struct dsa_switch *ds)
 
 	dev->dev_ops->config_cpu_port(ds);
 
+	if (dev->dev_ops->config_dsa_port)
+		dev->dev_ops->config_dsa_port(ds);
+
 	dev->dev_ops->enable_stp_addr(dev);
 
 	ds->num_tx_queues = dev->info->num_tx_queues;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index aab60f2587bf..c3c3eee178f4 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -147,6 +147,7 @@ struct ksz_device {
 	u32 chip_id;
 	u8 chip_rev;
 	int cpu_port;			/* port connected to CPU */
+	int dsa_port;                   /* Port used as cascaded port */
 	u32 smi_index;
 	int phy_port_cnt;
 	phy_interface_t compat_interface;
@@ -358,6 +359,7 @@ struct ksz_dev_ops {
 	void (*setup_rgmii_delay)(struct ksz_device *dev, int port);
 	int (*tc_cbs_set_cinc)(struct ksz_device *dev, int port, u32 val);
 	void (*config_cpu_port)(struct dsa_switch *ds);
+	void (*config_dsa_port)(struct dsa_switch *ds);
 	int (*enable_stp_addr)(struct ksz_device *dev);
 	int (*reset)(struct ksz_device *dev);
 	int (*init)(struct ksz_device *dev);
diff --git a/drivers/net/dsa/microchip/lan937x.h b/drivers/net/dsa/microchip/lan937x.h
index 3388d91dbc44..ef84abc31556 100644
--- a/drivers/net/dsa/microchip/lan937x.h
+++ b/drivers/net/dsa/microchip/lan937x.h
@@ -11,6 +11,7 @@ int lan937x_setup(struct dsa_switch *ds);
 void lan937x_teardown(struct dsa_switch *ds);
 void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port);
 void lan937x_config_cpu_port(struct dsa_switch *ds);
+void lan937x_config_dsa_port(struct dsa_switch *ds);
 int lan937x_switch_init(struct ksz_device *dev);
 void lan937x_switch_exit(struct ksz_device *dev);
 int lan937x_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data);
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 399a3905e6ca..5108a3f4bf76 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -205,11 +205,42 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 	dev->dev_ops->cfg_port_member(dev, port, member);
 }
 
+void lan937x_config_dsa_port(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct dsa_port *dp;
+
+	dev->dsa_port = 0xFF;
+
+	dsa_switch_for_each_port(dp, ds) {
+		if (dsa_is_dsa_port(ds, dp->index)) {
+			ksz_rmw32(dev, REG_SW_CASCADE_MODE_CTL,
+				  CASCADE_PORT_SEL, dp->index);
+			dev->dsa_port = dp->index;
+
+			/* Tail tag should be enabled for switch 0
+			 * in cascaded connection.
+			 */
+			if (dev->smi_index == 0) {
+				lan937x_port_cfg(dev, dp->index, REG_PORT_CTRL_0,
+						 PORT_TAIL_TAG_ENABLE, true);
+			}
+
+			/* Frame check length should be disabled for cascaded ports */
+			lan937x_port_cfg(dev, dp->index, REG_PORT_MAC_CTRL_0,
+					 PORT_CHECK_LENGTH, false);
+		}
+	}
+}
+
 void lan937x_config_cpu_port(struct dsa_switch *ds)
 {
 	struct ksz_device *dev = ds->priv;
 	struct dsa_port *dp;
 
+	/* Initializing cpu_port parameter into invalid value */
+	dev->cpu_port = 0xFF;
+
 	dsa_switch_for_each_cpu_port(dp, ds) {
 		if (dev->info->cpu_ports & (1 << dp->index)) {
 			dev->cpu_port = dp->index;
diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
index 45b606b6429f..4f30bc12f7a9 100644
--- a/drivers/net/dsa/microchip/lan937x_reg.h
+++ b/drivers/net/dsa/microchip/lan937x_reg.h
@@ -32,6 +32,9 @@
 #define REG_SW_PORT_INT_STATUS__4	0x0018
 #define REG_SW_PORT_INT_MASK__4		0x001C
 
+#define REG_SW_CASCADE_MODE_CTL         0x0030
+#define CASCADE_PORT_SEL                7
+
 /* 1 - Global */
 #define REG_SW_GLOBAL_OUTPUT_CTRL__1	0x0103
 #define SW_CLK125_ENB			BIT(1)
-- 
2.34.1

[RFC PATCH net-next 04/11] net: dsa: microchip: lan937x: update port number for LAN9373

[Rakesh Sankaranarayanan]

LAN9373 have total 8 physical ports. Update port_cnt member in
ksz_chip_data structure.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index ada673b6efc6..7062ce1749fb 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1490,7 +1490,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.num_alus = 1024,
 		.num_statics = 256,
 		.cpu_ports = 0x38,	/* can be configured as cpu port */
-		.port_cnt = 5,		/* total physical port count */
+		.port_cnt = 8,		/* total physical port count */
 		.port_nirqs = 6,
 		.num_tx_queues = 8,
 		.tc_cbs_supported = true,
-- 
2.34.1

[RFC PATCH net-next 05/11] net: dsa: microchip: lan937x: add shared global interrupt

[Rakesh Sankaranarayanan]

In cascade mode interrupt line is shared among both switches.
Update global interrupt flag for shared interrupt, otherwise second
switch probe will fail with busy status.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 7062ce1749fb..adf8391dd29f 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2009,7 +2009,8 @@ static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)
 	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
 }
 
-static int ksz_irq_common_setup(struct ksz_device *dev, struct ksz_irq *kirq)
+static int ksz_irq_common_setup(struct ksz_device *dev, struct ksz_irq *kirq,
+				unsigned long flag)
 {
 	int ret, n;
 
@@ -2025,7 +2026,7 @@ static int ksz_irq_common_setup(struct ksz_device *dev, struct ksz_irq *kirq)
 		irq_create_mapping(kirq->domain, n);
 
 	ret = request_threaded_irq(kirq->irq_num, NULL, ksz_irq_thread_fn,
-				   IRQF_ONESHOT, kirq->name, kirq);
+				   flag, kirq->name, kirq);
 	if (ret)
 		goto out;
 
@@ -2048,7 +2049,7 @@ static int ksz_girq_setup(struct ksz_device *dev)
 
 	girq->irq_num = dev->irq;
 
-	return ksz_irq_common_setup(dev, girq);
+	return ksz_irq_common_setup(dev, girq, (IRQF_ONESHOT | IRQF_SHARED));
 }
 
 static int ksz_pirq_setup(struct ksz_device *dev, u8 p)
@@ -2064,7 +2065,7 @@ static int ksz_pirq_setup(struct ksz_device *dev, u8 p)
 	if (pirq->irq_num < 0)
 		return pirq->irq_num;
 
-	return ksz_irq_common_setup(dev, pirq);
+	return ksz_irq_common_setup(dev, pirq, IRQF_ONESHOT);
 }
 
 static int ksz_setup(struct dsa_switch *ds)
-- 
2.34.1

[RFC PATCH net-next 06/11] net: dsa: microchip: lan937x: get cascade tag protocol

[Rakesh Sankaranarayanan]

Update ksz_get_tag_protocol to return separate tag protocol if
switch is connected in cascade mode. Variable ds->dst->last_switch
will contain total number of switches registered. For cascaded
connection alone, this will be more than zero.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index adf8391dd29f..2160a3e61a5a 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2567,9 +2567,13 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
 	    dev->chip_id == KSZ9567_CHIP_ID)
 		proto = DSA_TAG_PROTO_KSZ9477;
 
-	if (is_lan937x(dev))
+	if (is_lan937x(dev)) {
 		proto = DSA_TAG_PROTO_LAN937X_VALUE;
 
+		if (ds->dst->last_switch)
+			proto = DSA_TAG_PROTO_LAN937X_CASCADE_VALUE;
+	}
+
 	return proto;
 }
 
-- 
2.34.1

[RFC PATCH net-next 07/11] net: dsa: microchip: lan937x: update switch register

[Rakesh Sankaranarayanan]

Second switch in cascaded connection doesn't have port with macb
interface. dsa_switch_register returns error if macb interface is
not up. Due to this reason, second switch in cascaded connection will
not report error during dsa_switch_register and mib thread work will be
invoked even if actual switch register is not done. This will lead to
kernel warning and it can be avoided by checking device tree setup
status. This will return true only after actual switch register is done.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 2160a3e61a5a..0df71156a540 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3213,6 +3213,7 @@ int ksz_switch_register(struct ksz_device *dev)
 {
 	const struct ksz_chip_data *info;
 	struct device_node *port, *ports;
+	struct dsa_switch_tree *dst;
 	phy_interface_t interface;
 	unsigned int port_num;
 	int ret;
@@ -3330,6 +3331,15 @@ int ksz_switch_register(struct ksz_device *dev)
 		return ret;
 	}
 
+	/* Do not proceed further if device tree setup is not done.
+	 * dsa_register_switch() will not report error in case of
+	 * cascaded switch. This will lead to scheduling mib read
+	 * work and kernel warning.
+	 */
+	dst = dev->ds->dst;
+	if (!dst->setup)
+		return 0;
+
 	/* Read MIB counters every 30 seconds to avoid overflow. */
 	dev->mib_read_interval = msecs_to_jiffies(5000);
 
-- 
2.34.1

[RFC PATCH net-next 08/11] net: dsa: microchip: lan937x: avoid mib read for cascaded port

[Rakesh Sankaranarayanan]

Cascaded port need not be involved in mib read process. Unlike cpu port,
mib read function will be called for all other ports. Add check to skip
function if port is of type DSA_PORT_TYPE_DSA.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 0df71156a540..913296c5dd50 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2230,7 +2230,8 @@ static void ksz_mib_read_work(struct work_struct *work)
 	int i;
 
 	for (i = 0; i < dev->info->port_cnt; i++) {
-		if (dsa_is_unused_port(dev->ds, i))
+		if (dsa_is_unused_port(dev->ds, i) ||
+		    dsa_is_dsa_port(dev->ds, i))
 			continue;
 
 		p = &dev->ports[i];
-- 
2.34.1

[RFC PATCH net-next 09/11] net: dsa: microchip: lan937x: update port membership with dsa port

[Rakesh Sankaranarayanan]

Like cpu port, cascaded port will act as host port in second switch. And
all ports from both switches should be able to forward packets to cascaded
ports. Add cascaded port (dev->dsa_port) to each port membership.

Current design add bit map of user ports as cpu port membership. Include
cascaded port index as well to this group.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c   |  7 ++++---
 drivers/net/dsa/microchip/lan937x_main.c |  2 +-
 include/net/dsa.h                        | 15 +++++++++++++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 913296c5dd50..b8b7b5b7b52d 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1748,9 +1748,9 @@ static void ksz_get_strings(struct dsa_switch *ds, int port,
 
 static void ksz_update_port_member(struct ksz_device *dev, int port)
 {
+	u8 port_member = 0, cpu_port, dsa_port;
 	struct ksz_port *p = &dev->ports[port];
 	struct dsa_switch *ds = dev->ds;
-	u8 port_member = 0, cpu_port;
 	const struct dsa_port *dp;
 	int i, j;
 
@@ -1759,6 +1759,7 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
 
 	dp = dsa_to_port(ds, port);
 	cpu_port = BIT(dsa_upstream_port(ds, port));
+	dsa_port = BIT(dev->dsa_port);
 
 	for (i = 0; i < ds->num_ports; i++) {
 		const struct dsa_port *other_dp = dsa_to_port(ds, i);
@@ -1798,10 +1799,10 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
 				val |= BIT(j);
 		}
 
-		dev->dev_ops->cfg_port_member(dev, i, val | cpu_port);
+		dev->dev_ops->cfg_port_member(dev, i, val | cpu_port | dsa_port);
 	}
 
-	dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port);
+	dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port | dsa_port);
 }
 
 static int ksz_sw_mdio_read(struct mii_bus *bus, int addr, int regnum)
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 5108a3f4bf76..b17bb1ea2a4a 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -198,7 +198,7 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 				 true);
 
 	if (cpu_port)
-		member = dsa_user_ports(ds);
+		member = dsa_user_ports(ds) | dsa_dsa_ports(ds);
 	else
 		member = BIT(dsa_upstream_port(ds, port));
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 55651ad29193..939aa6ff1a38 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -591,6 +591,10 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
 	dsa_switch_for_each_port((_dp), (_ds)) \
 		if (dsa_port_is_cpu((_dp)))
 
+#define dsa_switch_for_each_dsa_port(_dp, _ds) \
+	dsa_switch_for_each_port((_dp), (_ds)) \
+		if (dsa_port_is_dsa((_dp)))
+
 #define dsa_switch_for_each_cpu_port_continue_reverse(_dp, _ds) \
 	dsa_switch_for_each_port_continue_reverse((_dp), (_ds)) \
 		if (dsa_port_is_cpu((_dp)))
@@ -617,6 +621,17 @@ static inline u32 dsa_cpu_ports(struct dsa_switch *ds)
 	return mask;
 }
 
+static inline u32 dsa_dsa_ports(struct dsa_switch *ds)
+{
+	struct dsa_port *dsa_dp;
+	u32 mask = 0;
+
+	dsa_switch_for_each_dsa_port(dsa_dp, ds)
+		mask |= BIT(dsa_dp->index);
+
+	return mask;
+}
+
 /* Return the local port used to reach an arbitrary switch device */
 static inline unsigned int dsa_routing_port(struct dsa_switch *ds, int device)
 {
-- 
2.34.1

[RFC PATCH net-next 10/11] net: dsa: microchip: lan937x: update vlan untag membership

[Rakesh Sankaranarayanan]

Exclude cascaded port from vlan untag membership table since it will be
the host port for second switch. Here setting 1 means, port will be
capable of receiving tagged frames and 0 means, port can not receive
tagged frames.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index bf13d47c26cf..4c12131098b1 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -399,7 +399,7 @@ int ksz9477_port_vlan_add(struct ksz_device *dev, int port,
 		vlan_table[1] |= BIT(port);
 	else
 		vlan_table[1] &= ~BIT(port);
-	vlan_table[1] &= ~(BIT(dev->cpu_port));
+	vlan_table[1] &= ~(BIT(dev->cpu_port) | BIT(dev->dsa_port));
 
 	vlan_table[2] |= BIT(port) | BIT(dev->cpu_port);
 
-- 
2.34.1

[RFC PATCH net-next 11/11] net: dsa: microchip: lan937x: update multicast table

[Rakesh Sankaranarayanan]

Program multicast table for cascaded port in second switch with
default port forward value since it is the host port for second switch.
Current driver program the same for cpu port in first switch.

Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 4c12131098b1..521d8c2e1540 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1116,18 +1116,22 @@ void ksz9477_config_cpu_port(struct dsa_switch *ds)
 
 int ksz9477_enable_stp_addr(struct ksz_device *dev)
 {
+	u32 fwd_port = BIT(dev->cpu_port);
 	const u32 *masks;
 	u32 data;
 	int ret;
 
 	masks = dev->info->masks;
 
+	if (dev->ds->index == 1)
+		fwd_port = BIT(dev->dsa_port);
+
 	/* Enable Reserved multicast table */
 	ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_RESV_MCAST_ENABLE, true);
 
 	/* Set the Override bit for forwarding BPDU packet to CPU */
 	ret = ksz_write32(dev, REG_SW_ALU_VAL_B,
-			  ALU_V_OVERRIDE | BIT(dev->cpu_port));
+			  ALU_V_OVERRIDE | fwd_port);
 	if (ret < 0)
 		return ret;
 
-- 
2.34.1

Re: [RFC PATCH net-next 04/11] net: dsa: microchip: lan937x: update port number for LAN9373

[Andrew Lunn]

On Thu, Feb 02, 2023 at 06:29:23PM +0530, Rakesh Sankaranarayanan wrote:
> LAN9373 have total 8 physical ports. Update port_cnt member in
> ksz_chip_data structure.

This seems more like a fix. Should it be applied to net, not net-next,
and have Fixes: tag?

    Andrew

Re: [RFC PATCH net-next 05/11] net: dsa: microchip: lan937x: add shared global interrupt

[Andrew Lunn]

On Thu, Feb 02, 2023 at 06:29:24PM +0530, Rakesh Sankaranarayanan wrote:
> In cascade mode interrupt line is shared among both switches.

I assume this is specific to the board you are using. Other boards
could have two interrupts. It should not cause a problem marking it a
shared, but please update the commit message to indicate that the
interrupts don't need to be shared.

	Andrew

Re: [RFC PATCH net-next 07/11] net: dsa: microchip: lan937x: update switch register

[Andrew Lunn]

On Thu, Feb 02, 2023 at 06:29:26PM +0530, Rakesh Sankaranarayanan wrote:
> Second switch in cascaded connection doesn't have port with macb
> interface. dsa_switch_register returns error if macb interface is
> not up. Due to this reason, second switch in cascaded connection will
> not report error during dsa_switch_register and mib thread work will be
> invoked even if actual switch register is not done. This will lead to
> kernel warning and it can be avoided by checking device tree setup
> status. This will return true only after actual switch register is done.

What i think you need to do is move the code into ksz_setup().

With a D in DSA setup, dsa_switch_register() adds the switch to the
list of switches, and then a check is performed to see if all switches
in the cluster have been registered. If not, it just returns. If all
switches have been registered, it then iterates over all the switches
can calls dsa_switch_ops.setup().

By moving the start of the MIB counter into setup(), it will only be
started once all the switches are present, and it means you don't need
to look at DSA core internal state.

	Andrew

Re: [RFC PATCH net-next 08/11] net: dsa: microchip: lan937x: avoid mib read for cascaded port

[Andrew Lunn]

On Thu, Feb 02, 2023 at 06:29:27PM +0530, Rakesh Sankaranarayanan wrote:
> Cascaded port need not be involved in mib read process. Unlike cpu port,
> mib read function will be called for all other ports. Add check to skip
> function if port is of type DSA_PORT_TYPE_DSA.

I would actually read the statistics. Having debugged D in DSA
systems, it is useful to know if packets are making it from one switch
to the other, etc.

The problem is getting the information out of the kernel. For
mv88e6xxx we have had an out of tree patch which exposes this
information in debugfs.

	Andrew

Re: [RFC PATCH net-next 04/11] net: dsa: microchip: lan937x: update port number for LAN9373

[Rakesh.Sankaranarayanan]

Hi Andrew,

On Thu, 2023-02-02 at 16:19 +0100, Andrew Lunn wrote:
> > LAN9373 have total 8 physical ports. Update port_cnt member in
> > ksz_chip_data structure.
> 
> This seems more like a fix. Should it be applied to net, not net-
> next,
> and have Fixes: tag?
> 
>     Andrew

Yes, I will update and send it as separate net patch with fixes tag.

Rakesh S.

Re: [RFC PATCH net-next 07/11] net: dsa: microchip: lan937x: update switch register

[Rakesh.Sankaranarayanan]

Hi Andrew,

Thanks for the comment, I will change and test the code as you
explained and update the patch in next revision.

Thanks,
Rakesh S.

On Thu, 2023-02-02 at 16:40 +0100, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Feb 02, 2023 at 06:29:26PM +0530, Rakesh Sankaranarayanan
> wrote:
> > Second switch in cascaded connection doesn't have port with macb
> > interface. dsa_switch_register returns error if macb interface is
> > not up. Due to this reason, second switch in cascaded connection
> > will
> > not report error during dsa_switch_register and mib thread work
> > will be
> > invoked even if actual switch register is not done. This will lead
> > to
> > kernel warning and it can be avoided by checking device tree setup
> > status. This will return true only after actual switch register is
> > done.
> 
> What i think you need to do is move the code into ksz_setup().
> 
> With a D in DSA setup, dsa_switch_register() adds the switch to the
> list of switches, and then a check is performed to see if all
> switches
> in the cluster have been registered. If not, it just returns. If all
> switches have been registered, it then iterates over all the switches
> can calls dsa_switch_ops.setup().
> 
> By moving the start of the MIB counter into setup(), it will only be
> started once all the switches are present, and it means you don't
> need
> to look at DSA core internal state.
> 
>         Andrew

Re: [RFC PATCH net-next 01/11] net: dsa: microchip: lan937x: add cascade tailtag

[Vladimir Oltean]

On Thu, Feb 02, 2023 at 06:29:20PM +0530, Rakesh Sankaranarayanan wrote:
> cascade tailtag contains 3 bytes of information, it includes
> additional bytes for accomodating port number in second switch.
> Destination port bitmap on first switch is at bit position 7:0 and
> of second switch is at bit position 15:8, add new tailtag xmit and
> rcv functions for cascade with proper formatting. Add new tag protocol
> for cascading and link with new xmit and rcv functions.
> 
> Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
> ---
>  include/net/dsa.h |  2 ++
>  net/dsa/tag_ksz.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 77 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index a15f17a38eca..55651ad29193 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -390,6 +399,7 @@ MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
>   *	  (eg, 0x00=port1, 0x02=port3, 0x07=port8)
>   */
>  #define LAN937X_EGRESS_TAG_LEN		2
> +#define LAN937X_CASCADE_TAG_LEN		3
>  
>  #define LAN937X_TAIL_TAG_BLOCKING_OVERRIDE	BIT(11)
>  #define LAN937X_TAIL_TAG_LOOKUP			BIT(12)
> @@ -442,11 +452,71 @@ static const struct dsa_device_ops lan937x_netdev_ops = {
>  DSA_TAG_DRIVER(lan937x_netdev_ops);
>  MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X, LAN937X_NAME);
>  
> +/* For xmit, 3/7 bytes are added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|tag1(1byte)|
> + * tag2(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * ts   : time stamp (Present only if PTP is enabled in the Hardware)
> + * tag0 : represents tag override, lookup and valid
> + * tag1 : each bit represents destination port map through switch 2
> + *	  (eg, 0x01=port1, 0x02=port2, 0x80=port8)
> + * tag2 : each bit represents destination port map through switch 1
> + *	  (eg, 0x01=port1, 0x02=port2, 0x80=port8)
> + *
> + * For rcv, 1/5 bytes is added before FCS.

Plural is only used for more than 5 bytes?
"5 bytes is added" vs "7 bytes are added"

> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * ts   : time stamp (Present only if bit 7 of tag0 is set)
> + * tag0 : zero-based value represents port
> + *	  (eg, 0x00=port1, 0x02=port3, 0x07=port8)
> + */
> +static struct sk_buff *lan937x_cascade_xmit(struct sk_buff *skb,
> +					    struct net_device *dev)
> +{
> +	struct dsa_port *dp = dsa_slave_to_port(dev);
> +	const struct ethhdr *hdr = eth_hdr(skb);
> +	__be32 *tag;
> +	u32 val;
> +
> +	if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
> +		return NULL;
> +
> +	tag = skb_put(skb, LAN937X_CASCADE_TAG_LEN);
> +
> +	val |= BIT((dp->index + (8 * dp->ds->index)));

This only works because cascade port is port 0, right?

> +
> +	if (is_link_local_ether_addr(hdr->h_dest))
> +		val |= (LAN937X_TAIL_TAG_BLOCKING_OVERRIDE << 8);
> +
> +	val |= (LAN937X_TAIL_TAG_VALID << 8);
> +
> +	put_unaligned_be24(val, tag);
> +
> +	return skb;
> +}
> +
> +static const struct dsa_device_ops lan937x_cascade_netdev_ops = {
> +	.name   = LAN937X_CASCADE_NAME,
> +	.proto  = DSA_TAG_PROTO_LAN937X_CASCADE,
> +	.xmit   = lan937x_cascade_xmit,
> +	.rcv    = ksz9477_rcv,
> +	.connect = ksz_connect,
> +	.disconnect = ksz_disconnect,
> +	.needed_tailroom = LAN937X_CASCADE_TAG_LEN,
> +};

Nope, no new lan937x_cascade tagging protocol.

If you read Documentation/networking/dsa/dsa.rst, it says:

| From the perspective of the network stack, all switches within the same DSA
| switch tree use the same tagging protocol.

Unless you are prepared to remove this assumption from the DSA framework,
you need to fold the cascade tag handling into the regular lan937x
tagging protocol (and declare the larger needed_tailroom to cover the
tail tag case).

You can look at dp->ds->index when figuring out the length of the tail
tag that should be inserted.

There's a lot of consolidation that could (and should) be done first
between lan937x_xmit() and ksz9477_xmit(), prior to adding the support
for cascade tagging.

> +
> +DSA_TAG_DRIVER(lan937x_cascade_netdev_ops);
> +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937x_CASCADE,
> +			    LAN937X_CASCADE_NAME);
> +
>  static struct dsa_tag_driver *dsa_tag_driver_array[] = {
>  	&DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
>  	&DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
>  	&DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),
>  	&DSA_TAG_DRIVER_NAME(lan937x_netdev_ops),
> +	&DSA_TAG_DRIVER_NAME(lan937x_cascade_netdev_ops),
>  };
>  
>  module_dsa_tag_drivers(dsa_tag_driver_array);
> -- 
> 2.34.1
> 

Re: [RFC PATCH net-next 02/11] net: dsa: microchip: lan937x: update SMI index

[Vladimir Oltean]

On Thu, Feb 02, 2023 at 06:29:21PM +0530, Rakesh Sankaranarayanan wrote:
> Current DSA driver register mdio interface for a port in the
> format of SMI-switch_index:port_number, switch_index is derived
> using variable ds->index. For a single switch ds->index will be
> always zero, and for cascaded switch, ds->index should be one.
> But it is found that ds->index is getting updated only after
> mdio_register stage. Update mdio_register to use variable directly
> from device tree using "dsa,member" identifier.
> 
> Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
> ---

Impossible, check again.

The call path is:

ksz_switch_register()
-> dsa_register_switch()
   -> dsa_switch_probe()
      -> dsa_switch_parse_of()
         -> dsa_switch_parse_member_of()
            -> sets ds->index
      -> dsa_tree_setup()
         -> dsa_tree_setup_switches()
            -> dsa_switch_setup()
               -> ksz_setup()
                  -> ksz_mdio_register()
                     -> you claim ds->index isn't set

You don't even need to be an expert on the code path, you can grep for
"ds->index = ", put a dump_stack() where it's set and one where you need
it, and compare in the stack trace which functions are common and where
they diverge.

Re: [RFC PATCH net-next 03/11] net: dsa: microchip: lan937x: enable cascade port

[Vladimir Oltean]

On Thu, Feb 02, 2023 at 06:29:22PM +0530, Rakesh Sankaranarayanan wrote:
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index aab60f2587bf..c3c3eee178f4 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -147,6 +147,7 @@ struct ksz_device {
>  	u32 chip_id;
>  	u8 chip_rev;
>  	int cpu_port;			/* port connected to CPU */
> +	int dsa_port;                   /* Port used as cascaded port */

nitpick: since "dsa_port" is typically a keyword that I use for the DSA
framework's generic port structure (struct dsa_port *dp), I would appreciate
if you could name this in some other way, like "cascade_port". I don't
believe the explanatory comment would even be necessary in that case.
And btw, the comment is inconsistent in alignment (tabs vs spaces) with
the line immediately above it.

>  	u32 smi_index;
>  	int phy_port_cnt;
>  	phy_interface_t compat_interface;
> @@ -358,6 +359,7 @@ struct ksz_dev_ops {
>  	void (*setup_rgmii_delay)(struct ksz_device *dev, int port);
>  	int (*tc_cbs_set_cinc)(struct ksz_device *dev, int port, u32 val);
>  	void (*config_cpu_port)(struct dsa_switch *ds);
> +	void (*config_dsa_port)(struct dsa_switch *ds);
>  	int (*enable_stp_addr)(struct ksz_device *dev);
>  	int (*reset)(struct ksz_device *dev);
>  	int (*init)(struct ksz_device *dev);
> diff --git a/drivers/net/dsa/microchip/lan937x.h b/drivers/net/dsa/microchip/lan937x.h
> index 3388d91dbc44..ef84abc31556 100644
> --- a/drivers/net/dsa/microchip/lan937x.h
> +++ b/drivers/net/dsa/microchip/lan937x.h
> @@ -11,6 +11,7 @@ int lan937x_setup(struct dsa_switch *ds);
>  void lan937x_teardown(struct dsa_switch *ds);
>  void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port);
>  void lan937x_config_cpu_port(struct dsa_switch *ds);
> +void lan937x_config_dsa_port(struct dsa_switch *ds);
>  int lan937x_switch_init(struct ksz_device *dev);
>  void lan937x_switch_exit(struct ksz_device *dev);
>  int lan937x_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data);
> diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
> index 399a3905e6ca..5108a3f4bf76 100644
> --- a/drivers/net/dsa/microchip/lan937x_main.c
> +++ b/drivers/net/dsa/microchip/lan937x_main.c
> @@ -205,11 +205,42 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
>  	dev->dev_ops->cfg_port_member(dev, port, member);
>  }
>  
> +void lan937x_config_dsa_port(struct dsa_switch *ds)
> +{
> +	struct ksz_device *dev = ds->priv;
> +	struct dsa_port *dp;
> +
> +	dev->dsa_port = 0xFF;
> +
> +	dsa_switch_for_each_port(dp, ds) {
> +		if (dsa_is_dsa_port(ds, dp->index)) {

Would be good if you introduced dsa_switch_for_each_dsa_port() as a
separate patch, and used it here first.

Not sure if you realize this, but dsa_is_dsa_port() contains a list
iteration hidden in it, in dsa_to_port(). So dsa_switch_for_each_port()
-> dsa_is_dsa_port() effectively does an O(n^2) list walk (apart from
uselessly increasing the code indentation).

> +			ksz_rmw32(dev, REG_SW_CASCADE_MODE_CTL,
> +				  CASCADE_PORT_SEL, dp->index);
> +			dev->dsa_port = dp->index;
> +
> +			/* Tail tag should be enabled for switch 0
> +			 * in cascaded connection.
> +			 */
> +			if (dev->smi_index == 0) {
> +				lan937x_port_cfg(dev, dp->index, REG_PORT_CTRL_0,
> +						 PORT_TAIL_TAG_ENABLE, true);
> +			}
> +
> +			/* Frame check length should be disabled for cascaded ports */
> +			lan937x_port_cfg(dev, dp->index, REG_PORT_MAC_CTRL_0,
> +					 PORT_CHECK_LENGTH, false);

break?

> +		}
> +	}
> +}
> +
>  void lan937x_config_cpu_port(struct dsa_switch *ds)
>  {
>  	struct ksz_device *dev = ds->priv;
>  	struct dsa_port *dp;
>  
> +	/* Initializing cpu_port parameter into invalid value */
> +	dev->cpu_port = 0xFF;
> +
>  	dsa_switch_for_each_cpu_port(dp, ds) {
>  		if (dev->info->cpu_ports & (1 << dp->index)) {
>  			dev->cpu_port = dp->index;
> diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h
> index 45b606b6429f..4f30bc12f7a9 100644
> --- a/drivers/net/dsa/microchip/lan937x_reg.h
> +++ b/drivers/net/dsa/microchip/lan937x_reg.h
> @@ -32,6 +32,9 @@
>  #define REG_SW_PORT_INT_STATUS__4	0x0018
>  #define REG_SW_PORT_INT_MASK__4		0x001C
>  
> +#define REG_SW_CASCADE_MODE_CTL         0x0030
> +#define CASCADE_PORT_SEL                7
> +
>  /* 1 - Global */
>  #define REG_SW_GLOBAL_OUTPUT_CTRL__1	0x0103
>  #define SW_CLK125_ENB			BIT(1)
> -- 
> 2.34.1
> 

Re: [RFC PATCH net-next 04/11] net: dsa: microchip: lan937x: update port number for LAN9373

[Vladimir Oltean]

On Fri, Feb 03, 2023 at 10:43:40AM +0000, Rakesh.Sankaranarayanan@microchip.com wrote:
> Hi Andrew,
> 
> On Thu, 2023-02-02 at 16:19 +0100, Andrew Lunn wrote:
> > > LAN9373 have total 8 physical ports. Update port_cnt member in
> > > ksz_chip_data structure.
> > 
> > This seems more like a fix. Should it be applied to net, not net-
> > next,
> > and have Fixes: tag?
> > 
> >     Andrew
> 
> Yes, I will update and send it as separate net patch with fixes tag.

What's the story here? Arun must have surely known this isn't a 5 port switch?

Re: [RFC PATCH net-next 06/11] net: dsa: microchip: lan937x: get cascade tag protocol

[Vladimir Oltean]

On Thu, Feb 02, 2023 at 06:29:25PM +0530, Rakesh Sankaranarayanan wrote:
> Update ksz_get_tag_protocol to return separate tag protocol if
> switch is connected in cascade mode. Variable ds->dst->last_switch
> will contain total number of switches registered. For cascaded
> connection alone, this will be more than zero.

Nope, last_switch does not contain the total number of switches
registered, but the index of the last switch in this tree. DSA does not
assume that the indices are consecutive.

If you make any assumption in the driver regarding switch numbering in a
cascade setup, it is an assumption that a device tree writer who is not
you needs to know about. So you must document it in
Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml.

> 
> Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
> ---
>  drivers/net/dsa/microchip/ksz_common.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index adf8391dd29f..2160a3e61a5a 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -2567,9 +2567,13 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds,
>  	    dev->chip_id == KSZ9567_CHIP_ID)
>  		proto = DSA_TAG_PROTO_KSZ9477;
>  
> -	if (is_lan937x(dev))
> +	if (is_lan937x(dev)) {
>  		proto = DSA_TAG_PROTO_LAN937X_VALUE;
>  
> +		if (ds->dst->last_switch)
> +			proto = DSA_TAG_PROTO_LAN937X_CASCADE_VALUE;
> +	}

Also nope, see the comment on patch 1.

> +
>  	return proto;
>  }
>  
> -- 
> 2.34.1
> 

Re: [RFC PATCH net-next 07/11] net: dsa: microchip: lan937x: update switch register

[Vladimir Oltean]

On Thu, Feb 02, 2023 at 04:40:36PM +0100, Andrew Lunn wrote:
> On Thu, Feb 02, 2023 at 06:29:26PM +0530, Rakesh Sankaranarayanan wrote:
> > Second switch in cascaded connection doesn't have port with macb
> > interface. dsa_switch_register returns error if macb interface is
> > not up. Due to this reason, second switch in cascaded connection will
> > not report error during dsa_switch_register and mib thread work will be
> > invoked even if actual switch register is not done. This will lead to
> > kernel warning and it can be avoided by checking device tree setup
> > status. This will return true only after actual switch register is done.
> 
> What i think you need to do is move the code into ksz_setup().
> 
> With a D in DSA setup, dsa_switch_register() adds the switch to the
> list of switches, and then a check is performed to see if all switches
> in the cluster have been registered. If not, it just returns. If all
> switches have been registered, it then iterates over all the switches
> can calls dsa_switch_ops.setup().
> 
> By moving the start of the MIB counter into setup(), it will only be
> started once all the switches are present, and it means you don't need
> to look at DSA core internal state.

+1

Also there's a bug in its own right in ksz_mib_read_work(), here:

			if (!netif_carrier_ok(dp->slave))
				mib->cnt_ptr = dev->info->reg_mib_cnt;

The code accesses dp->slave, so naturally it kicks the bucket for
cascade ports.

It doesn't crash with CPU ports because dp->slave is in a union with
dp->master, which is also a struct net_device * with its own carrier:

struct dsa_port {
	/* A CPU port is physically connected to a master device.
	 * A user port exposed to userspace has a slave device.
	 */
	union {
		struct net_device *master;
		struct net_device *slave;
	};

This needs to be fixed, since accessing the DSA master through a
dp->slave pointer is dangerous and unintended.

Easiest thing to do would be to only check link state if (dsa_port_is_user(dp)).
For other ports always read all MIB counters.

Re: [RFC PATCH net-next 09/11] net: dsa: microchip: lan937x: update port membership with dsa port

[Vladimir Oltean]

On Thu, Feb 02, 2023 at 06:29:28PM +0530, Rakesh Sankaranarayanan wrote:
> Like cpu port, cascaded port will act as host port in second switch. And
> all ports from both switches should be able to forward packets to cascaded
> ports. Add cascaded port (dev->dsa_port) to each port membership.
> 
> Current design add bit map of user ports as cpu port membership. Include
> cascaded port index as well to this group.
> 
> Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
> ---
>  drivers/net/dsa/microchip/ksz_common.c   |  7 ++++---
>  drivers/net/dsa/microchip/lan937x_main.c |  2 +-
>  include/net/dsa.h                        | 15 +++++++++++++++
>  3 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 913296c5dd50..b8b7b5b7b52d 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1748,9 +1748,9 @@ static void ksz_get_strings(struct dsa_switch *ds, int port,
>  
>  static void ksz_update_port_member(struct ksz_device *dev, int port)
>  {
> +	u8 port_member = 0, cpu_port, dsa_port;

Don't need these unimaginative names ("cpu_port", "dsa_port" for what is
actually a port *mask*). You can bitwise-OR the upstream port and the
downstream cascade port directly into "port_member", and the code in
ksz_update_port_member() would tolerate it just fine.

>  	struct ksz_port *p = &dev->ports[port];
>  	struct dsa_switch *ds = dev->ds;
> -	u8 port_member = 0, cpu_port;
>  	const struct dsa_port *dp;
>  	int i, j;
>  
> @@ -1759,6 +1759,7 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
>  
>  	dp = dsa_to_port(ds, port);
>  	cpu_port = BIT(dsa_upstream_port(ds, port));
> +	dsa_port = BIT(dev->dsa_port);

If dev->dsa_port is 0xff, what is BIT(0xff)?

>  
>  	for (i = 0; i < ds->num_ports; i++) {
>  		const struct dsa_port *other_dp = dsa_to_port(ds, i);
> @@ -1798,10 +1799,10 @@ static void ksz_update_port_member(struct ksz_device *dev, int port)
>  				val |= BIT(j);
>  		}
>  
> -		dev->dev_ops->cfg_port_member(dev, i, val | cpu_port);
> +		dev->dev_ops->cfg_port_member(dev, i, val | cpu_port | dsa_port);
>  	}
>  
> -	dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port);
> +	dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port | dsa_port);
>  }
>  
>  static int ksz_sw_mdio_read(struct mii_bus *bus, int addr, int regnum)
> diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
> index 5108a3f4bf76..b17bb1ea2a4a 100644
> --- a/drivers/net/dsa/microchip/lan937x_main.c
> +++ b/drivers/net/dsa/microchip/lan937x_main.c
> @@ -198,7 +198,7 @@ void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
>  				 true);
>  
>  	if (cpu_port)
> -		member = dsa_user_ports(ds);
> +		member = dsa_user_ports(ds) | dsa_dsa_ports(ds);

I'm wondering if a single dsa_switch_for_each_port() list traversal plus
an "if ()" wouldn't be more efficient here.

>  	else
>  		member = BIT(dsa_upstream_port(ds, port));
>  
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 55651ad29193..939aa6ff1a38 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -591,6 +591,10 @@ static inline bool dsa_is_user_port(struct dsa_switch *ds, int p)
>  	dsa_switch_for_each_port((_dp), (_ds)) \
>  		if (dsa_port_is_cpu((_dp)))
>  
> +#define dsa_switch_for_each_dsa_port(_dp, _ds) \
> +	dsa_switch_for_each_port((_dp), (_ds)) \
> +		if (dsa_port_is_dsa((_dp)))
> +
>  #define dsa_switch_for_each_cpu_port_continue_reverse(_dp, _ds) \
>  	dsa_switch_for_each_port_continue_reverse((_dp), (_ds)) \
>  		if (dsa_port_is_cpu((_dp)))
> @@ -617,6 +621,17 @@ static inline u32 dsa_cpu_ports(struct dsa_switch *ds)
>  	return mask;
>  }
>  
> +static inline u32 dsa_dsa_ports(struct dsa_switch *ds)
> +{
> +	struct dsa_port *dsa_dp;

can name this just "dp"

> +	u32 mask = 0;
> +
> +	dsa_switch_for_each_dsa_port(dsa_dp, ds)
> +		mask |= BIT(dsa_dp->index);
> +
> +	return mask;
> +}
> +
>  /* Return the local port used to reach an arbitrary switch device */
>  static inline unsigned int dsa_routing_port(struct dsa_switch *ds, int device)
>  {
> -- 
> 2.34.1
> 

Re: [RFC PATCH net-next 10/11] net: dsa: microchip: lan937x: update vlan untag membership

[Vladimir Oltean]

On Thu, Feb 02, 2023 at 06:29:29PM +0530, Rakesh Sankaranarayanan wrote:
> Exclude cascaded port from vlan untag membership table since it will be
> the host port for second switch. Here setting 1 means, port will be
> capable of receiving tagged frames and 0 means, port can not receive
> tagged frames.
> 
> Signed-off-by: Rakesh Sankaranarayanan <rakesh.sankaranarayanan@microchip.com>
> ---
>  drivers/net/dsa/microchip/ksz9477.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index bf13d47c26cf..4c12131098b1 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -399,7 +399,7 @@ int ksz9477_port_vlan_add(struct ksz_device *dev, int port,
>  		vlan_table[1] |= BIT(port);
>  	else
>  		vlan_table[1] &= ~BIT(port);
> -	vlan_table[1] &= ~(BIT(dev->cpu_port));
> +	vlan_table[1] &= ~(BIT(dev->cpu_port) | BIT(dev->dsa_port));

same comment, what if dev->dsa_port is 0xff?

>  
>  	vlan_table[2] |= BIT(port) | BIT(dev->cpu_port);
>  
> -- 
> 2.34.1
> 

Re: [RFC PATCH net-next 01/11] net: dsa: microchip: lan937x: add cascade tailtag

[kernel test robot]

Hi Rakesh,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Rakesh-Sankaranarayanan/net-dsa-microchip-lan937x-add-cascade-tailtag/20230202-210316
patch link:    https://lore.kernel.org/r/20230202125930.271740-2-rakesh.sankaranarayanan%40microchip.com
patch subject: [RFC PATCH net-next 01/11] net: dsa: microchip: lan937x: add cascade tailtag
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20230204/202302041634.D4xvmQxf-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/d70ada5150dd09e1036c9de8c81198b9489430b3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Rakesh-Sankaranarayanan/net-dsa-microchip-lan937x-add-cascade-tailtag/20230202-210316
        git checkout d70ada5150dd09e1036c9de8c81198b9489430b3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash net/dsa/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/dsa/tag_ksz.c:488:2: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
           val |= BIT((dp->index + (8 * dp->ds->index)));
           ^~~
   net/dsa/tag_ksz.c:481:9: note: initialize the variable 'val' to silence this warning
           u32 val;
                  ^
                   = 0
   1 warning generated.


vim +/val +488 net/dsa/tag_ksz.c

   454	
   455	/* For xmit, 3/7 bytes are added before FCS.
   456	 * ---------------------------------------------------------------------------
   457	 * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|tag1(1byte)|
   458	 * tag2(1byte)|FCS(4bytes)
   459	 * ---------------------------------------------------------------------------
   460	 * ts   : time stamp (Present only if PTP is enabled in the Hardware)
   461	 * tag0 : represents tag override, lookup and valid
   462	 * tag1 : each bit represents destination port map through switch 2
   463	 *	  (eg, 0x01=port1, 0x02=port2, 0x80=port8)
   464	 * tag2 : each bit represents destination port map through switch 1
   465	 *	  (eg, 0x01=port1, 0x02=port2, 0x80=port8)
   466	 *
   467	 * For rcv, 1/5 bytes is added before FCS.
   468	 * ---------------------------------------------------------------------------
   469	 * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|ts(4bytes)|tag0(1byte)|FCS(4bytes)
   470	 * ---------------------------------------------------------------------------
   471	 * ts   : time stamp (Present only if bit 7 of tag0 is set)
   472	 * tag0 : zero-based value represents port
   473	 *	  (eg, 0x00=port1, 0x02=port3, 0x07=port8)
   474	 */
   475	static struct sk_buff *lan937x_cascade_xmit(struct sk_buff *skb,
   476						    struct net_device *dev)
   477	{
   478		struct dsa_port *dp = dsa_slave_to_port(dev);
   479		const struct ethhdr *hdr = eth_hdr(skb);
   480		__be32 *tag;
   481		u32 val;
   482	
   483		if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
   484			return NULL;
   485	
   486		tag = skb_put(skb, LAN937X_CASCADE_TAG_LEN);
   487	
 > 488		val |= BIT((dp->index + (8 * dp->ds->index)));
   489	
   490		if (is_link_local_ether_addr(hdr->h_dest))
   491			val |= (LAN937X_TAIL_TAG_BLOCKING_OVERRIDE << 8);
   492	
   493		val |= (LAN937X_TAIL_TAG_VALID << 8);
   494	
   495		put_unaligned_be24(val, tag);
   496	
   497		return skb;
   498	}
   499	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [RFC PATCH net-next 04/11] net: dsa: microchip: lan937x: update port number for LAN9373

[Arun.Ramadoss]

Hi Vladimir,
On Sat, 2023-02-04 at 01:26 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Fri, Feb 03, 2023 at 10:43:40AM +0000, 
> Rakesh.Sankaranarayanan@microchip.com wrote:
> > Hi Andrew,
> > 
> > On Thu, 2023-02-02 at 16:19 +0100, Andrew Lunn wrote:
> > > > LAN9373 have total 8 physical ports. Update port_cnt member in
> > > > ksz_chip_data structure.
> > > 
> > > This seems more like a fix. Should it be applied to net, not net-
> > > next,
> > > and have Fixes: tag?
> > > 
> > >     Andrew
> > 
> > Yes, I will update and send it as separate net patch with fixes
> > tag.
> 
> What's the story here? Arun must have surely known this isn't a 5
> port switch?

It was my mistake during replicating the structure for LAN9370 and
LAN9373. I tested the basic switch functionality on LAN9370 and LAN9374
but not LAN9373. LAN9373 Evaluation board available in cascading setup.
When Rakesh brought up the board for cascading, he found out there is
bug. I should have double checked all the structure member before
submitting the patch but I overlooked it.