[PATCH net-next v14 0/5] net: dsa: motorcomm: add support yt922x driver

[PATCH net-next v14 0/5] net: dsa: motorcomm: add support yt922x driver

[Kyle Switch]

This patchset mainly adds support for the yt922x series to the dsa driver.
Currently, only the yt9224 in the yt922x series is supported, and the
corresponding tag handling for yt922x is added.

At the same time, it optimizes the max ports boundary check in the
existing code, and initializes the dsa switch based on the chipid.

changes in v14:
1) Replace the goto-based skip_extmode_check path with an early
   conditional in yt921x_chip_detect().

changes in v13:
1) Fix a compilation error in patch1 only.
   The change is limited to the yt92xx_register_switch function:
   - Replace yt921x_priv with dsa_to_yt921x_priv
   - Replace to_device with yt921x_priv_to_device

changes in v12:
1) Rebase the code.
2) Delete origin patch4 and patch5 about MIB init and remove operation.
3) Optimize error message logging.
4) Add documents about yt922x tag.

changes in v11:
1) Initialize ctrl variable inside yt922x_port_up().
2) Update maitainer for new file.

changes in v10:
1) Rebase the code.

changes in v9:
patch 1: 1) Add a dev_err() when chipid mismatch.
patch 3: 1) Using GENMASK_U* to replace GENMASK

changes in v8:
patch 1: 1) Add a lock operation to acquire the chip ID during the probe()
         stage.
         2) Remove meaningless changes.
patch 3: Modify the changelog description.
patch 6: 1) Remove sds_init() in this patch.
         2) Code refactoring, including yt922x_sds_phyaddr_get() and
            yt921x_needs_extmode_check()
         3) In yt921x_intif_ext_write() and yt921x_intif_ext_read(),
            add a boundary check for phyaddr. The reason is that the
            phyaddr of the top ext register is 9, so max_ports should
            be used for the check to include it.
         4) In the yt922x driver, added initialization of yt921x_port
            for exporting priv.

changes in v7:
patch 1: Fix the series_lookup() redundancy issue.
         Fix the dsa->priv initialization and the code style issue.
patch 6: Move psc definition to yt921x_port
         Fix psc_get_state() and an_restart().
         Use the existing yt921x driver interface for the duplicated
         logic for chip_reset().
         Move the yt922x_cpu_port_set() to chip_setup_dsa() to align
         with yt921x function.


Kyle Switch (5):
  net: dsa: motorcomm: initialize dsa_switch based on chipid
  net: dsa: motorcomm: use max_ports in series for port bounds checking
  net: dsa: motorcomm: use fixed-width GENMASK_U*() helpers
  net: dsa: tag_yt922x: add support for Motorcomm YT922x tags
  net: dsa: motorcomm: Add support for Motorcomm YT922x

 MAINTAINERS                          |   2 +
 drivers/net/dsa/motorcomm/Kconfig    |   7 +-
 drivers/net/dsa/motorcomm/Makefile   |   1 +
 drivers/net/dsa/motorcomm/chip.c     | 526 ++++++++++++++++++++++++++-
 drivers/net/dsa/motorcomm/chip.h     | 113 ++++++
 drivers/net/dsa/motorcomm/mdio_bus.c |  47 ++-
 drivers/net/dsa/motorcomm/mdio_bus.h |   2 +
 drivers/net/dsa/motorcomm/pcs-922x.c | 173 +++++++++
 drivers/net/dsa/motorcomm/pcs.h      |   1 +
 include/net/dsa.h                    |   2 +
 net/dsa/Kconfig                      |   6 +
 net/dsa/Makefile                     |   1 +
 net/dsa/tag_yt922x.c                 | 122 +++++++
 13 files changed, 981 insertions(+), 22 deletions(-)
 create mode 100644 drivers/net/dsa/motorcomm/pcs-922x.c
 create mode 100644 net/dsa/tag_yt922x.c

-- 
2.25.1

[PATCH net-next v14 1/5] net: dsa: motorcomm: initialize dsa_switch based on chipid

[Kyle Switch]

Replace the hardcoded chip handling in yt92xx_register_switch() with a
data-driven loopup based on the chip ID.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/chip.c | 73 ++++++++++++++++++++++++++++----
 drivers/net/dsa/motorcomm/chip.h | 19 +++++++++
 2 files changed, 84 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index 91cc5647f2b6..d4c20658216a 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -4078,6 +4078,67 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
 	.setup			= yt921x_dsa_setup,
 };
 
+static const struct yt92xx_series yt92xx_series_table[] = {
+	[YT92XX_MODE_YT921X] = {
+		.name = "YT921x",
+		.max_ports = YT921X_PORT_NUM,
+		.num_lag_ids = YT921X_LAG_NUM,
+		.ageing_time_min = 1 * 5000,
+		.ageing_time_max = U16_MAX * 5000,
+		.dscp_prio_mapping_is_global = true,
+		.assisted_learning_on_cpu_port = true,
+		.switch_ops = &yt921x_dsa_switch_ops,
+		.mac_ops = &yt921x_phylink_mac_ops
+	},
+};
+
+static const struct yt92xx_series *yt92xx_series_lookup(u32 major)
+{
+	if (major == YT9215_MAJOR || major == YT9218_MAJOR)
+		return &yt92xx_series_table[YT92XX_MODE_YT921X];
+	else
+		return NULL;
+}
+
+static int yt92xx_register_switch(struct dsa_switch *ds)
+{
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(ds);
+	struct device *dev = yt921x_priv_to_device(priv);
+	const struct yt92xx_series *series;
+	u32 chipid;
+	u32 major;
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
+	mutex_unlock(&priv->reg_lock);
+	if (res) {
+		dev_err(dev, "Unexpected chipid 0x%x\n", chipid);
+		return res;
+	}
+
+	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
+	series = yt92xx_series_lookup(major);
+	if (!series) {
+		dev_err(dev, "Undefined series info for the corresponding major\n");
+		return -ENODEV;
+	}
+	priv->series = series;
+
+	ds->assisted_learning_on_cpu_port =
+		priv->series->assisted_learning_on_cpu_port;
+	ds->dscp_prio_mapping_is_global =
+		priv->series->dscp_prio_mapping_is_global;
+	ds->ageing_time_min = priv->series->ageing_time_min;
+	ds->ageing_time_max = priv->series->ageing_time_max;
+	ds->num_lag_ids = priv->series->num_lag_ids;
+	ds->num_ports = priv->series->max_ports;
+	ds->ops = priv->series->switch_ops;
+	ds->phylink_mac_ops = priv->series->mac_ops;
+
+	return 0;
+}
+
 static void yt921x_mdio_shutdown(struct mdio_device *mdiodev)
 {
 	struct yt921x_priv *priv = mdiodev_get_drvdata(mdiodev);
@@ -4120,6 +4181,7 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 	struct yt921x_reg_mdio *mdio;
 	struct yt921x_priv *priv;
 	struct dsa_switch *ds;
+	int res;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -4146,15 +4208,10 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 
 	ds = &priv->ds;
 	ds->dev = dev;
-	ds->assisted_learning_on_cpu_port = true;
-	ds->dscp_prio_mapping_is_global = true;
 	ds->priv = priv;
-	ds->ops = &yt921x_dsa_switch_ops;
-	ds->ageing_time_min = 1 * 5000;
-	ds->ageing_time_max = U16_MAX * 5000;
-	ds->phylink_mac_ops = &yt921x_phylink_mac_ops;
-	ds->num_lag_ids = YT921X_LAG_NUM;
-	ds->num_ports = YT921X_PORT_NUM;
+	res = yt92xx_register_switch(ds);
+	if (res)
+		return res;
 
 	mdiodev_set_drvdata(mdiodev, priv);
 
diff --git a/drivers/net/dsa/motorcomm/chip.h b/drivers/net/dsa/motorcomm/chip.h
index 25ff77054de3..85174a51b72d 100644
--- a/drivers/net/dsa/motorcomm/chip.h
+++ b/drivers/net/dsa/motorcomm/chip.h
@@ -841,9 +841,28 @@ static inline u16 yt921x_info_ports_mask(const struct yt921x_info *info)
 	return info->internal_mask | info->xmii_mask | info->serdes_mask;
 }
 
+enum yt92xx_mode {
+	YT92XX_MODE_YT921X,
+	YT92XX_MODE_YT922X,
+	YT92XX_MODE_MAX,
+};
+
+struct yt92xx_series {
+	const char *name;
+	unsigned int max_ports;
+	unsigned int num_lag_ids;
+	unsigned int ageing_time_min;
+	unsigned int ageing_time_max;
+	u32 dscp_prio_mapping_is_global;
+	u32 assisted_learning_on_cpu_port;
+	const struct dsa_switch_ops *switch_ops;
+	const struct phylink_mac_ops *mac_ops;
+};
+
 struct yt921x_priv {
 	struct dsa_switch ds;
 
+	const struct yt92xx_series *series;
 	const struct yt921x_info *info;
 	unsigned int meter_slot_ns;
 	unsigned int port_shape_slot_ns;
-- 
2.25.1

[PATCH net-next v14 2/5] net: dsa: motorcomm: use max_ports in series for port bounds checking

[Kyle Switch]

Replace the hardcoded YT921X_PORT_NUM macro with the per-series
max_ports field in port validation. This removes family-specific
constants from the common code path and simplifies adding new
switch families with different port counts.

No functional change for existing YT921X devices.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/mdio_bus.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/mdio_bus.c b/drivers/net/dsa/motorcomm/mdio_bus.c
index dd6c52f37bbf..83f0212551ff 100644
--- a/drivers/net/dsa/motorcomm/mdio_bus.c
+++ b/drivers/net/dsa/motorcomm/mdio_bus.c
@@ -113,7 +113,7 @@ static int yt921x_mbus_int_read(struct mii_bus *mbus, int port, int reg)
 	u16 val;
 	int res;
 
-	if (port >= YT921X_PORT_NUM)
+	if (port >= priv->series->max_ports)
 		return U16_MAX;
 
 	mutex_lock(&priv->reg_lock);
@@ -131,7 +131,7 @@ yt921x_mbus_int_write(struct mii_bus *mbus, int port, int reg, u16 data)
 	struct yt921x_priv *priv = mbus->priv;
 	int res;
 
-	if (port >= YT921X_PORT_NUM)
+	if (port >= priv->series->max_ports)
 		return -ENODEV;
 
 	mutex_lock(&priv->reg_lock);
@@ -145,6 +145,7 @@ int yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp)
 {
 	struct device *dev = yt921x_priv_to_device(priv);
 	struct mii_bus *mbus;
+	u32 max_ports;
 	int res;
 
 	mbus = devm_mdiobus_alloc(dev);
@@ -157,7 +158,8 @@ int yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp)
 	mbus->read = yt921x_mbus_int_read;
 	mbus->write = yt921x_mbus_int_write;
 	mbus->parent = dev;
-	mbus->phy_mask = (u32)~GENMASK(YT921X_PORT_NUM - 1, 0);
+	max_ports = priv->series->max_ports;
+	mbus->phy_mask = (u32)~GENMASK(max_ports - 1, 0);
 
 	res = devm_of_mdiobus_register(dev, mbus, mnp);
 	if (res)
-- 
2.25.1

[PATCH net-next v14 3/5] net: dsa: motorcomm: use fixed-width GENMASK_U*() helpers

[Kyle Switch]

Some masks assigned to u16 and u32 fields are defined with
GENMASK(),which is an unsigned long constant and wider than
the field type on 64-bit builds. Convert the affected
internal_mask entries in chip.c and the YT921X_MBUS_CTRL_*
masks used by mdio_bus.c to fixed-width helpers.

This is a type cleanup only, with no functional change.  Other
GENMASK() uses in the driver are intentionally left untouched.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/chip.c     | 12 ++++++------
 drivers/net/dsa/motorcomm/mdio_bus.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index d4c20658216a..ba988e121d68 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -36,19 +36,19 @@
 static const struct yt921x_info yt921x_infos[] = {
 	{
 		"YT9215SC", YT9215_MAJOR, 1, 0,
-		GENMASK(4, 0),
+		GENMASK_U16(4, 0),
 		BIT(9),
 		BIT(8) | BIT(9),
 	},
 	{
 		"YT9215S", YT9215_MAJOR, 2, 0,
-		GENMASK(4, 0),
+		GENMASK_U16(4, 0),
 		BIT(9),
 		BIT(8),
 	},
 	{
 		"YT9215RB", YT9215_MAJOR, 3, 0,
-		GENMASK(4, 0),
+		GENMASK_U16(4, 0),
 		BIT(8) | BIT(9),
 		0,
 	},
@@ -66,13 +66,13 @@ static const struct yt921x_info yt921x_infos[] = {
 	},
 	{
 		"YT9218N", YT9218_MAJOR, 0, 0,
-		GENMASK(7, 0),
+		GENMASK_U16(7, 0),
 		0,
 		0,
 	},
 	{
 		"YT9218MB", YT9218_MAJOR, 1, 0,
-		GENMASK(7, 0),
+		GENMASK_U16(7, 0),
 		BIT(8) | BIT(9),
 		BIT(8) | BIT(9),
 	},
@@ -3741,7 +3741,7 @@ static int yt921x_chip_setup_dsa(struct yt921x_priv *priv)
 	if (res)
 		return res;
 
-	ctrl = GENMASK(10, 0);
+	ctrl = GENMASK_U32(10, 0);
 	res = yt921x_reg_write(priv, YT921X_FILTER_UNK_UCAST, ctrl);
 	if (res)
 		return res;
diff --git a/drivers/net/dsa/motorcomm/mdio_bus.c b/drivers/net/dsa/motorcomm/mdio_bus.c
index 83f0212551ff..3aa1111efaa4 100644
--- a/drivers/net/dsa/motorcomm/mdio_bus.c
+++ b/drivers/net/dsa/motorcomm/mdio_bus.c
@@ -159,7 +159,7 @@ int yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp)
 	mbus->write = yt921x_mbus_int_write;
 	mbus->parent = dev;
 	max_ports = priv->series->max_ports;
-	mbus->phy_mask = (u32)~GENMASK(max_ports - 1, 0);
+	mbus->phy_mask = ~GENMASK_U32(max_ports - 1, 0);
 
 	res = devm_of_mdiobus_register(dev, mbus, mnp);
 	if (res)
-- 
2.25.1

[PATCH net-next v14 4/5] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags

[Kyle Switch]

Add DSA tag support for Motorcomm YT922x switches.  YT922x uses the
same ETH_P_YT921X EtherType and the same 8-byte tag length as YT921x,
but the tag layout differs, so a separate tag driver is needed instead
of extending tag_yt921x.c.

The new driver documents both the to-CPU and from-CPU tag layouts.

Update the co-maintainer for the Motorcomm DSA driver.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 MAINTAINERS          |   2 +
 include/net/dsa.h    |   2 +
 net/dsa/Kconfig      |   6 +++
 net/dsa/Makefile     |   1 +
 net/dsa/tag_yt922x.c | 122 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 133 insertions(+)
 create mode 100644 net/dsa/tag_yt922x.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 509b35b5a0e7..434cd217011b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18434,11 +18434,13 @@ F:	drivers/net/phy/motorcomm.c
 
 MOTORCOMM YT921X ETHERNET SWITCH DRIVER
 M:	David Yang <mmyangfl@gmail.com>
+M:	Kyle Switch <kyle.switch@motor-comm.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/net/dsa/motorcomm,yt921x.yaml
 F:	drivers/net/dsa/motorcomm/
 F:	net/dsa/tag_yt921x.c
+F:	net/dsa/tag_yt922x.c
 
 MOXA SMARTIO/INDUSTIO/INTELLIO SERIAL CARD
 M:	Jiri Slaby <jirislaby@kernel.org>
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 5d12191b6f6f..5a173fe2bb87 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -62,6 +62,7 @@ struct tc_action;
 #define DSA_TAG_PROTO_KSZ8463_VALUE		34
 #define DSA_TAG_PROTO_MT7628_VALUE		35
 #define DSA_TAG_PROTO_KS8995_VALUE		36
+#define DSA_TAG_PROTO_YT922X_VALUE		37
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -101,6 +102,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_KSZ8463		= DSA_TAG_PROTO_KSZ8463_VALUE,
 	DSA_TAG_PROTO_MT7628		= DSA_TAG_PROTO_MT7628_VALUE,
 	DSA_TAG_PROTO_KS8995		= DSA_TAG_PROTO_KS8995_VALUE,
+	DSA_TAG_PROTO_YT922X		= DSA_TAG_PROTO_YT922X_VALUE,
 };
 
 struct dsa_switch;
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 4f44bf3ede23..06d8d6526054 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -233,4 +233,10 @@ config NET_DSA_TAG_YT921X
 	  Say Y or M if you want to enable support for tagging frames for
 	  Motorcomm YT921x switches.
 
+config NET_DSA_TAG_YT922X
+	tristate "Tag driver for Motorcomm YT922x switches"
+	help
+	  Say Y or M if you want to enable support for tagging frames for
+	  Motorcomm YT922x switches.
+
 endif
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 1f9cc30e9988..0ef6dfce3b92 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
 obj-$(CONFIG_NET_DSA_TAG_VSC73XX_8021Q) += tag_vsc73xx_8021q.o
 obj-$(CONFIG_NET_DSA_TAG_XRS700X) += tag_xrs700x.o
 obj-$(CONFIG_NET_DSA_TAG_YT921X) += tag_yt921x.o
+obj-$(CONFIG_NET_DSA_TAG_YT922X) += tag_yt922x.o
 
 # for tracing framework to find trace.h
 CFLAGS_trace.o := -I$(src)
diff --git a/net/dsa/tag_yt922x.c b/net/dsa/tag_yt922x.c
new file mode 100644
index 000000000000..b6146a9af98b
--- /dev/null
+++ b/net/dsa/tag_yt922x.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Motorcomm YT922x Switch Extended CPU Port Tagging
+ *
+ * Copyright (c) 2026 Kyle switch <kyle.switch@motor-comm.com>
+ *
+ */
+
+#include <linux/etherdevice.h>
+
+#include "tag.h"
+
+#define YT922X_TAG_LEN 8
+
+/*
+ * To define the from cpu tag format 8 bytes.
+ *
+ * To-CPU (RX) tag format, 8 bytes:
+ *
+ *   tag[0]: TPID (default: 0x9988)
+ *   tag[1]: VID lookup
+ *   tag[2]: bit 5:2  - RX source port
+ *
+ * From-CPU (TX) tag format, 8 bytes:
+ *
+ *   tag[0]: TPID (default: 0x9988)
+ *   tag[1]: bit 8:0 - Dst port mask
+ *   tag[2]: bit 15  - Dst port 0
+ *
+ */
+#define YT922X_TAG_NAME		"yt922x"
+#define YT922X_TAG_PORTMASK_0	BIT(15)
+#define YT922X_TAG_PORTMASK_M	GENMASK(8, 0)
+#define  YT922X_TAG_PORTS(x)		FIELD_PREP(YT922X_TAG_PORTMASK_M, (x))
+#define YT922X_TAG_FORCE_DST	BIT(9)
+#define YT922X_TAG_PRIO_M	GENMASK(12, 10)
+#define YT922X_TAG_PRIO_EN	BIT(13)
+#define  YT922X_TAG_PRIO(x)	(FIELD_PREP(YT922X_TAG_PRIO_M, (x)) | YT922X_TAG_PRIO_EN)
+#define YT922X_TAG_RX_PORT_M	GENMASK(5, 2)
+
+static struct sk_buff *
+yt922x_tag_xmit(struct sk_buff *skb, struct net_device *netdev)
+{
+	unsigned long ports;
+	__be16 *tag;
+	u16 ctrl;
+
+	skb_push(skb, YT922X_TAG_LEN);
+	dsa_alloc_etype_header(skb, YT922X_TAG_LEN);
+	tag = dsa_etype_header_pos_tx(skb);
+
+	tag[0] = htons(ETH_P_YT921X);
+	ports = dsa_xmit_port_mask(skb, netdev);
+	/*To fill in the case where the port index is not 0 */
+	ctrl = YT922X_TAG_PRIO(skb->priority) | YT922X_TAG_FORCE_DST |
+	       YT922X_TAG_PORTS(ports >> 1);
+	tag[1] = htons(ctrl);
+	if (ports & BIT(0)) {
+		/* To fill in the case where the port index is 0 */
+		ctrl = YT922X_TAG_PORTMASK_0;
+		tag[2] = htons(ctrl);
+	} else {
+		tag[2] = 0;
+	}
+	tag[3] = 0;
+
+	return skb;
+}
+
+static struct sk_buff *
+yt922x_tag_rcv(struct sk_buff *skb, struct net_device *netdev)
+{
+	unsigned int port;
+	__be16 *tag;
+	u16 rx;
+
+	if (unlikely(!pskb_may_pull(skb, YT922X_TAG_LEN))) {
+		kfree_skb(skb);
+		return NULL;
+	}
+
+	tag = dsa_etype_header_pos_rx(skb);
+
+	if (unlikely(tag[0] != htons(ETH_P_YT921X))) {
+		dev_warn_ratelimited(&netdev->dev,
+				     "Unexpected EtherType 0x%04x\n",
+				     ntohs(tag[0]));
+		kfree_skb(skb);
+		return NULL;
+	}
+
+	/* Locate which port this is coming from */
+	rx = ntohs(tag[2]);
+	port = FIELD_GET(YT922X_TAG_RX_PORT_M, rx);
+	skb->dev = dsa_conduit_find_user(netdev, 0, port);
+	if (unlikely(!skb->dev)) {
+		dev_warn_ratelimited(&netdev->dev,
+				     "Couldn't decode source port %u\n", port);
+		kfree_skb(skb);
+		return NULL;
+	}
+
+	/* Remove tag and update checksum */
+	skb_pull_rcsum(skb, YT922X_TAG_LEN);
+	dsa_strip_etype_header(skb, YT922X_TAG_LEN);
+
+	return skb;
+}
+
+static const struct dsa_device_ops yt922x_netdev_ops = {
+	.name = YT922X_TAG_NAME,
+	.proto = DSA_TAG_PROTO_YT922X,
+	.xmit = yt922x_tag_xmit,
+	.rcv = yt922x_tag_rcv,
+	.needed_headroom = YT922X_TAG_LEN,
+};
+
+MODULE_DESCRIPTION("DSA tag driver for Motorcomm YT922x switches");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_YT922X, YT922X_TAG_NAME);
+
+module_dsa_tag_driver(yt922x_netdev_ops);
-- 
2.25.1

[PATCH net-next v14 5/5] net: dsa: motorcomm: Add support for Motorcomm YT922x

[Kyle Switch]

Add support for Motorcomm YT922X, which is series of
ethernet switches developed by Motorcomm Electronic
Technology, includes YT9224 and YT9228.

This patch only adds support for the YT9224 variant.
YT9228 is not supported yet.

This patch adds basic support for a working DSA switch,
includes .port_setup, .setup, .phylink_get_caps,
.get_tag_protocol.

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/Kconfig    |   7 +-
 drivers/net/dsa/motorcomm/Makefile   |   1 +
 drivers/net/dsa/motorcomm/chip.c     | 449 ++++++++++++++++++++++++++-
 drivers/net/dsa/motorcomm/chip.h     |  94 ++++++
 drivers/net/dsa/motorcomm/mdio_bus.c |  39 +++
 drivers/net/dsa/motorcomm/mdio_bus.h |   2 +
 drivers/net/dsa/motorcomm/pcs-922x.c | 173 +++++++++++
 drivers/net/dsa/motorcomm/pcs.h      |   1 +
 8 files changed, 757 insertions(+), 9 deletions(-)
 create mode 100644 drivers/net/dsa/motorcomm/pcs-922x.c

diff --git a/drivers/net/dsa/motorcomm/Kconfig b/drivers/net/dsa/motorcomm/Kconfig
index 79cdd79a1fd2..648c49d6c527 100644
--- a/drivers/net/dsa/motorcomm/Kconfig
+++ b/drivers/net/dsa/motorcomm/Kconfig
@@ -1,11 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config NET_DSA_YT921X
-	tristate "Motorcomm YT9215 ethernet switch chip support"
+	tristate "Motorcomm YT9215 and YT9224 ethernet switch chip support"
 	select NET_DSA_TAG_YT921X
+	select NET_DSA_TAG_YT922X
 	select NET_IEEE8021Q_HELPERS if DCB
 	help
-	  This enables support for the Motorcomm YT9215 ethernet switch
-	  chip.
+	  This enables support for the Motorcomm YT9215 and YT9224
+	  ethernet switch chip.
 
 config NET_DSA_YT921X_LEDS
 	bool "LED support for Motorcomm YT9215"
diff --git a/drivers/net/dsa/motorcomm/Makefile b/drivers/net/dsa/motorcomm/Makefile
index 9c1e3d538d21..22e6da0c10e1 100644
--- a/drivers/net/dsa/motorcomm/Makefile
+++ b/drivers/net/dsa/motorcomm/Makefile
@@ -4,5 +4,6 @@ yt921x-objs := chip.o
 yt921x-$(CONFIG_NET_DSA_YT921X_LEDS) += leds.o
 yt921x-objs += mdio_bus.o
 yt921x-objs += mib.o
+yt921x-objs += pcs-922x.o
 yt921x-objs += pcs-921x.o
 yt921x-objs += smi.o
diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index ba988e121d68..f0488fac9097 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Driver for Motorcomm YT921x Switch
+ * Driver for Motorcomm YT921x and YT922x Switch
  *
  * Should work on YT9213/YT9214/YT9215/YT9218, but only tested on YT9215+SGMII,
  * be sure to do your own checks before porting to another chip.
@@ -76,6 +76,18 @@ static const struct yt921x_info yt921x_infos[] = {
 		BIT(8) | BIT(9),
 		BIT(8) | BIT(9),
 	},
+	{
+		"YT9224", YT9224_MAJOR, 0, 0,
+		GENMASK_U16(7, 4),
+		0,
+		BIT(8) | BIT(0),
+	},
+	{
+		"YT9228", YT9224_MAJOR, 0, 0,
+		GENMASK_U16(7, 4),
+		0,
+		BIT(8) | GENMASK_U16(3, 0),
+	},
 	{}
 };
 
@@ -3606,6 +3618,11 @@ static void yt921x_dsa_teardown(struct dsa_switch *ds)
 #endif
 }
 
+static bool yt921x_needs_extmode_check(u32 major)
+{
+	return (major == YT9224_MAJOR) ? false : true;
+}
+
 static int yt921x_chip_detect(struct yt921x_priv *priv)
 {
 	struct device *dev = yt921x_priv_to_device(priv);
@@ -3631,6 +3648,14 @@ static int yt921x_chip_detect(struct yt921x_priv *priv)
 		return -ENODEV;
 	}
 
+	if (!yt921x_needs_extmode_check(major)) {
+		dev_info(dev,
+			 "Motorcomm %s ethernet switch, chipid: 0x%x\n",
+			 info->name, chipid);
+		priv->info = info;
+		return 0;
+	}
+
 	res = yt921x_reg_read(priv, YT921X_CHIP_MODE, &mode);
 	if (res)
 		return res;
@@ -4078,6 +4103,405 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
 	.setup			= yt921x_dsa_setup,
 };
 
+static int yt922x_port_down(struct yt921x_priv *priv, int port)
+{
+	u32 mask;
+	int res;
+
+	/* mac force down */
+	mask = YT922X_PORT_LINK | YT922X_PORT_RX_MAC_EN |
+	YT922X_PORT_TX_MAC_EN | YT922X_PORT_LINK_AN;
+	res = yt921x_reg_clear_bits(priv, YT922X_PORTn_CTRL(port), mask);
+	if (res)
+		return res;
+	/* Need force op to make soft configuration effective */
+	mask = YT922X_PORT_FORCE_OP;
+	res = yt921x_reg_set_bits(priv, YT922X_PORTn_CTRL(port), mask);
+	if (res)
+		return res;
+
+	/* disable en_phy */
+	res = yt921x_reg_clear_bits(priv, YT922X_EN_PHY_VALUE, BIT(port));
+	if (res)
+		return res;
+	res = yt921x_reg_set_bits(priv, YT922X_EN_PHY_OVERWRITE, BIT(port));
+	if (res)
+		return res;
+
+	return 0;
+}
+
+static void
+yt922x_phylink_mac_link_down(struct phylink_config *config, unsigned int mode,
+			     phy_interface_t interface)
+{
+	struct dsa_port *dp = dsa_phylink_to_port(config);
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(dp->ds);
+	int port = dp->index;
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+	res = yt922x_port_down(priv, port);
+	mutex_unlock(&priv->reg_lock);
+
+	if (res)
+		dev_err(dp->ds->dev, "Failed to %s port %d: %i\n", "bring down",
+			port, res);
+}
+
+static int
+yt922x_port_up(struct yt921x_priv *priv, int port, unsigned int mode,
+	       phy_interface_t interface, int speed, int duplex,
+	       bool tx_pause, bool rx_pause)
+{
+	int ps = ethtool_speed_to_yt921x(speed);
+	u32 ctrl = 0;
+	u32 mask;
+	int res;
+
+	if (ps == YT921X_SPEED_INVALID)
+		return -EINVAL;
+	ctrl |= ps;
+
+	if (duplex == DUPLEX_FULL)
+		ctrl |= YT922X_PORT_DUPLEX_FULL;
+	if (tx_pause)
+		ctrl |= YT922X_PORT_TX_PAUSE;
+	if (rx_pause)
+		ctrl |= YT922X_PORT_RX_PAUSE;
+	ctrl |= YT922X_PORT_RX_MAC_EN | YT922X_PORT_TX_MAC_EN |
+			YT922X_PORT_CFG_TX_EN | YT922X_PORT_LINK |
+			YT922X_PORT_CFG_RX_EN;
+	ctrl &= ~(YT922X_PORT_FC_AN | YT922X_PORT_LINK_AN);
+	res = yt921x_reg_write(priv, YT922X_PORTn_CTRL(port), ctrl);
+	if (res)
+		return res;
+
+	/* force op */
+	mask = YT922X_PORT_FORCE_OP;
+	res = yt921x_reg_set_bits(priv, YT922X_PORTn_CTRL(port), mask);
+	if (res)
+		return res;
+
+	/* enable en_phy */
+	res = yt921x_reg_set_bits(priv, YT922X_EN_PHY_VALUE, BIT(port));
+	if (res)
+		return res;
+	res = yt921x_reg_set_bits(priv, YT922X_EN_PHY_OVERWRITE, BIT(port));
+	if (res)
+		return res;
+
+	return 0;
+}
+
+static void
+yt922x_phylink_mac_link_up(struct phylink_config *config,
+			   struct phy_device *phydev, unsigned int mode,
+			   phy_interface_t interface, int speed, int duplex,
+			   bool tx_pause, bool rx_pause)
+{
+	struct dsa_port *dp = dsa_phylink_to_port(config);
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(dp->ds);
+	int port = dp->index;
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+	res = yt922x_port_up(priv, port, mode, interface, speed, duplex,
+			     tx_pause, rx_pause);
+	mutex_unlock(&priv->reg_lock);
+
+	if (res)
+		dev_err(dp->ds->dev, "Failed to %s port %d: %i\n", "bring up",
+			port, res);
+}
+
+static void
+yt922x_phylink_mac_config(struct phylink_config *config, unsigned int mode,
+			  const struct phylink_link_state *state)
+{
+}
+
+static struct phylink_pcs *
+yt922x_phylink_mac_select_pcs(struct phylink_config *config,
+			      phy_interface_t interface)
+{
+	struct dsa_port *dp = dsa_phylink_to_port(config);
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(dp->ds);
+
+	switch (interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_USXGMII:
+		return &priv->ports[dp->index].pcs;
+
+	default:
+		return NULL;
+	}
+}
+
+static const struct phylink_mac_ops yt922x_phylink_mac_ops = {
+	.mac_select_pcs = yt922x_phylink_mac_select_pcs,
+	.mac_link_down = yt922x_phylink_mac_link_down,
+	.mac_link_up = yt922x_phylink_mac_link_up,
+	.mac_config = yt922x_phylink_mac_config,
+};
+
+static enum dsa_tag_protocol
+yt922x_dsa_get_tag_protocol(struct dsa_switch *ds, int port,
+			    enum dsa_tag_protocol m)
+{
+	return DSA_TAG_PROTO_YT922X;
+}
+
+static void
+yt922x_dsa_phylink_get_caps(struct dsa_switch *ds, int port,
+			    struct phylink_config *config)
+{
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(ds);
+	const struct yt921x_info *info = priv->info;
+
+	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
+				   MAC_10 | MAC_100 | MAC_1000;
+
+	if (info->internal_mask & BIT(port)) {
+		/* port 4 to port 7, internal utp */
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_2500FD;
+	}
+	if (info->serdes_mask & BIT(port)) {
+		/* serdes */
+		__set_bit(PHY_INTERFACE_MODE_SGMII,
+			  config->supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_1000BASEX,
+			  config->supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_2500FD;
+		__set_bit(PHY_INTERFACE_MODE_USXGMII,
+			  config->supported_interfaces);
+		config->mac_capabilities |= MAC_5000FD;
+		config->mac_capabilities |= MAC_10000FD;
+	}
+}
+
+static int yt922x_port_setup(struct yt921x_priv *priv, int port)
+{
+	struct dsa_switch *ds = &priv->ds;
+	u32 mask;
+	u32 ctrl;
+	int res;
+
+	/* enable user port isolation and disable fdb learning */
+	ctrl = ~priv->cpu_ports_mask;
+	res = yt921x_reg_write(priv, YT922X_PORTn_ISOLATION(port), ctrl);
+	if (res)
+		return res;
+
+	mask = YT922X_PORT_LEARN_DIS;
+	res = yt921x_reg_set_bits(priv, YT922X_PORTn_LEARN(port), mask);
+	if (res)
+		return res;
+
+	if (dsa_is_cpu_port(ds, port)) {
+		ctrl = ~(u32)0;
+		res = yt921x_reg_write(priv, YT922X_PORTn_ISOLATION(port),
+				       ctrl);
+		if (res)
+			return res;
+	}
+
+	return 0;
+}
+
+static int yt922x_dsa_port_setup(struct dsa_switch *ds, int port)
+{
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(ds);
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+	res = yt922x_port_setup(priv, port);
+	mutex_unlock(&priv->reg_lock);
+
+	return res;
+}
+
+static int yt922x_cpu_tag_mode_set_8b(struct yt921x_priv *priv)
+{
+	u32 val;
+	u32 val1;
+	int res;
+
+	/* cpu tag mode set to 8b */
+	res = yt921x_reg_read(priv, YT922X_CPU_TAG_RX_CTRL, &val);
+	if (res)
+		return res;
+	res = yt921x_reg_read(priv, YT922X_CPU_TAG_TX_CTRL, &val1);
+	if (res)
+		return res;
+	val &= ~YT922X_CPU_TAG_RX_MODE;
+	val1 &= ~YT922X_CPU_TAG_TX_MODE;
+	val1 &= ~YT922X_CPU_TAG_TX_TYPE;
+	res = yt921x_reg_write(priv, YT922X_CPU_TAG_RX_CTRL, val);
+	if (res)
+		return res;
+	res = yt921x_reg_write(priv, YT922X_CPU_TAG_TX_CTRL, val1);
+	if (res)
+		return res;
+
+	return 0;
+}
+
+static int yt922x_cpu_port_set(struct yt921x_priv *priv)
+{
+	struct dsa_switch *ds = &priv->ds;
+	u32 ctrl;
+	int res;
+
+	/* cpu tag mode */
+	res = yt922x_cpu_tag_mode_set_8b(priv);
+	if (res)
+		return res;
+
+	/* Enable DSA */
+	priv->cpu_ports_mask = dsa_cpu_ports(ds);
+	ctrl = YT921X_EXT_CPU_PORT_TAG_EN | YT921X_EXT_CPU_PORT_PORT_EN |
+	       YT921X_EXT_CPU_PORT_PORT(__ffs(priv->cpu_ports_mask));
+	res = yt921x_reg_write(priv, YT921X_EXT_CPU_PORT, ctrl);
+	if (res)
+		return res;
+
+	/* Setup software switch */
+	ctrl = YT922X_CPU_COPY_TO_EXT_CPU;
+	res = yt921x_reg_write(priv, YT922X_CPU_COPY, ctrl);
+	if (res)
+		return res;
+
+	return res;
+}
+
+static int yt922x_chip_setup_dsa(struct yt921x_priv *priv)
+{
+	unsigned long cpu_ports_mask;
+	u32 ctrl;
+	int port;
+	int res;
+
+	/* cpu port set */
+	res = yt922x_cpu_port_set(priv);
+	if (res)
+		return res;
+
+	ctrl = GENMASK_U32(8, 0);
+	res = yt921x_reg_write(priv, YT922X_FILTER_UNK_UCAST, ctrl);
+	if (res)
+		return res;
+
+	ctrl = 0;
+	for (int i = 0; i < priv->series->max_ports; i++)
+		ctrl |= YT922X_ACT_UNK_ACTn_TRAP(i);
+	cpu_ports_mask = priv->cpu_ports_mask;
+	for_each_set_bit(port, &cpu_ports_mask, priv->series->max_ports) {
+		ctrl &= ~YT922X_ACT_UNK_ACTn_M(port);
+		ctrl |= YT922X_ACT_UNK_ACTn_DROP(port);
+	}
+	res = yt921x_reg_write(priv, YT922X_ACT_UNK_UCAST, ctrl);
+	if (res)
+		return res;
+	res = yt921x_reg_write(priv, YT922X_ACT_UNK_MCAST, ctrl);
+	if (res)
+		return res;
+
+	return 0;
+}
+
+static int yt922x_chip_setup(struct yt921x_priv *priv)
+{
+	u32 ctrl;
+	int res;
+
+	ctrl = YT922X_FUNC_MIB | YT922X_FUNC_ACL;
+	res = yt921x_reg_set_bits(priv, YT921X_FUNC, ctrl);
+	if (res)
+		return res;
+
+	res = yt922x_chip_setup_dsa(priv);
+	if (res)
+		return res;
+
+	return 0;
+}
+
+static void yt922x_pcs_setup(struct dsa_switch *ds)
+{
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(ds);
+	const struct yt921x_info *info = priv->info;
+	unsigned long mask;
+	int port;
+
+	mask = info->serdes_mask;
+	for_each_set_bit(port, &mask, priv->series->max_ports) {
+		struct yt921x_port *pp = &priv->ports[port];
+
+		pp->pcs.ops = &yt922x_phylink_pcs_ops;
+		pp->pcs.poll = true;
+
+		__set_bit(PHY_INTERFACE_MODE_SGMII,
+			  pp->pcs.supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_1000BASEX,
+			  pp->pcs.supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
+			  pp->pcs.supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_USXGMII,
+			  pp->pcs.supported_interfaces);
+	}
+}
+
+static int yt922x_dsa_setup(struct dsa_switch *ds)
+{
+	struct yt921x_priv *priv = dsa_to_yt921x_priv(ds);
+	struct device *dev = yt921x_priv_to_device(priv);
+	struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+	res = yt921x_chip_reset(priv);
+	mutex_unlock(&priv->reg_lock);
+	if (res)
+		return res;
+
+	/* Register the internal mdio bus. */
+	child = of_get_child_by_name(np, "mdio");
+	if (child) {
+		res = yt921x_mbus_int_init(priv, child);
+		of_node_put(child);
+		if (res)
+			return res;
+	}
+
+	mutex_lock(&priv->reg_lock);
+	res = yt922x_chip_setup(priv);
+	mutex_unlock(&priv->reg_lock);
+	if (res)
+		return res;
+
+	/* switch sds pcs setup */
+	yt922x_pcs_setup(ds);
+
+	return 0;
+}
+
+static const struct dsa_switch_ops yt922x_dsa_switch_ops = {
+	/* port */
+	.get_tag_protocol = yt922x_dsa_get_tag_protocol,
+	.phylink_get_caps = yt922x_dsa_phylink_get_caps,
+	.port_setup  = yt922x_dsa_port_setup,
+	/* chip */
+	.setup   = yt922x_dsa_setup,
+};
+
 static const struct yt92xx_series yt92xx_series_table[] = {
 	[YT92XX_MODE_YT921X] = {
 		.name = "YT921x",
@@ -4090,12 +4514,25 @@ static const struct yt92xx_series yt92xx_series_table[] = {
 		.switch_ops = &yt921x_dsa_switch_ops,
 		.mac_ops = &yt921x_phylink_mac_ops
 	},
+	[YT92XX_MODE_YT922X] = {
+		.name = "YT922x",
+		.max_ports = YT922X_PORT_NUM,
+		.num_lag_ids = YT922X_LAG_NUM,
+		.ageing_time_min = 1 * 6000,
+		.ageing_time_max = U16_MAX * 6000,
+		.dscp_prio_mapping_is_global = true,
+		.assisted_learning_on_cpu_port = true,
+		.switch_ops = &yt922x_dsa_switch_ops,
+		.mac_ops = &yt922x_phylink_mac_ops,
+	},
 };
 
 static const struct yt92xx_series *yt92xx_series_lookup(u32 major)
 {
 	if (major == YT9215_MAJOR || major == YT9218_MAJOR)
 		return &yt92xx_series_table[YT92XX_MODE_YT921X];
+	else if (major == YT9224_MAJOR)
+		return &yt92xx_series_table[YT92XX_MODE_YT922X];
 	else
 		return NULL;
 }
@@ -4112,15 +4549,13 @@ static int yt92xx_register_switch(struct dsa_switch *ds)
 	mutex_lock(&priv->reg_lock);
 	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
 	mutex_unlock(&priv->reg_lock);
-	if (res) {
-		dev_err(dev, "Unexpected chipid 0x%x\n", chipid);
+	if (res)
 		return res;
-	}
 
 	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
 	series = yt92xx_series_lookup(major);
 	if (!series) {
-		dev_err(dev, "Undefined series info for the corresponding major\n");
+		dev_err(dev, "Chip ID: 0x%x, and Unknown major.\n", chipid);
 		return -ENODEV;
 	}
 	priv->series = series;
@@ -4220,6 +4655,7 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 
 static const struct of_device_id yt921x_of_match[] = {
 	{ .compatible = "motorcomm,yt9215" },
+	{ .compatible = "motorcomm,yt9224" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, yt921x_of_match);
@@ -4237,5 +4673,6 @@ static struct mdio_driver yt921x_mdio_driver = {
 mdio_module_driver(yt921x_mdio_driver);
 
 MODULE_AUTHOR("David Yang <mmyangfl@gmail.com>");
-MODULE_DESCRIPTION("Driver for Motorcomm YT921x Switch");
+MODULE_AUTHOR("Kyle Switch <kyle.switch@motor-comm.com>");
+MODULE_DESCRIPTION("Driver for Motorcomm YT921x and YT922x Switch");
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/motorcomm/chip.h b/drivers/net/dsa/motorcomm/chip.h
index 85174a51b72d..f14747278d1d 100644
--- a/drivers/net/dsa/motorcomm/chip.h
+++ b/drivers/net/dsa/motorcomm/chip.h
@@ -119,6 +119,7 @@ enum yt921x_speed {
 	YT921X_SPEED_1000,
 	YT921X_SPEED_10000,
 	YT921X_SPEED_2500,
+	YT921X_SPEED_5000,
 	YT921X_SPEED_INVALID
 };
 
@@ -749,6 +750,7 @@ enum yt921x_fdb_entry_status {
 
 #define YT9215_MAJOR	0x9002
 #define YT9218_MAJOR	0x9001
+#define YT9224_MAJOR	0x9004
 
 /* required for a hard reset */
 #define YT921X_RST_DELAY_US	10000
@@ -770,6 +772,96 @@ enum yt921x_fdb_entry_status {
 
 #define YT921X_NAME	"yt921x"
 
+/* yt922x register lists */
+#define YT92XX_PAGE_SELECT	0x1e
+#define YT92XX_PAGE		0x1f
+#define YT922X_PORTn_STATUS(port)	(0x80200 + 4 * (port))
+#define  YT922X_PORT_LINK_STATE			BIT(8)
+#define  YT922X_PORT_LINK_DUPLEX		BIT(7)
+#define  YT922X_PORT_RX_FC_EN			BIT(6)
+#define  YT922X_PORT_TX_FC_EN			BIT(5)
+#define YT922X_PORT_SPEED_10		0
+#define YT922X_PORT_SPEED_100		1
+#define YT922X_PORT_SPEED_1000		2
+#define YT922X_PORT_SPEED_10000		3
+#define YT922X_PORT_SPEED_2500		4
+#define YT922X_PORT_SPEED_5000		5
+#define YT922X_EN_PHY_OVERWRITE		(0x80040)
+#define YT922X_EN_PHY_VALUE		(0x8003c)
+/* CTRL: force op to make soft configuration effective */
+#define YT922X_PORTn_CTRL(port)		(0x80080 + 4 * (port))
+#define  YT922X_PORT_FORCE_OP			BIT(14)
+#define  YT922X_PORT_CFG_TX_EN			BIT(13)
+#define  YT922X_PORT_CFG_RX_EN			BIT(12)
+#define  YT922X_PORT_FC_AN			BIT(11)
+#define  YT922X_PORT_LINK_AN			BIT(10)  /* CTRL: auto negotiation */
+#define  YT922X_PORT_LINK			BIT(9)  /* CTRL: link status */
+#define  YT922X_PORT_HALF_PAUSE			BIT(8)  /* Half-duplex back pressure mode */
+#define  YT922X_PORT_DUPLEX_FULL		BIT(7)
+#define  YT922X_PORT_RX_PAUSE			BIT(6)
+#define  YT922X_PORT_TX_PAUSE			BIT(5)
+#define  YT922X_PORT_RX_MAC_EN			BIT(4)
+#define  YT922X_PORT_TX_MAC_EN			BIT(3)
+#define  YT922X_PORT_SPEED_M			GENMASK(2, 0)
+#define YT922X_PORT_SDSn		0x400
+#define  YT922X_SERDES_MODE_M			GENMASK(6, 4)
+#define   YT922X_SERDES_MODE(x)				FIELD_PREP(YT922X_SERDES_MODE_M, (x))
+#define YT922X_SERDES_MODE_SGMII	YT922X_SERDES_MODE(0)
+#define YT922X_SERDES_MODE_REVSGMII	YT922X_SERDES_MODE(1)
+#define YT922X_SERDES_MODE_1000BASEX	YT922X_SERDES_MODE(2)
+#define YT922X_SERDES_MODE_100BASEX	YT922X_SERDES_MODE(3)
+#define YT922X_SERDES_MODE_2500BASEX	YT922X_SERDES_MODE(4)
+#define YT922X_SERDES_MODE_USXGMII	YT922X_SERDES_MODE(6)
+#define YT922X_PORT_NUM			9
+#define YT922X_PCS_LINK_CTRL		0x11
+#define YT922X_PCS_LINK_STATUS		BIT(10)
+#define YT922X_PCS_AN_COMPLETE		BIT(11)
+
+/* LAG */
+#define YT922X_LAG_NUM			4
+/* ISO */
+#define YT922X_PORTn_ISOLATION(port)	(0x4 * (port) + 0x180d80)
+/* FDB */
+#define YT922X_PORTn_LEARN(port)	(0x180300 + 4 * (port))
+#define  YT922X_PORT_LEARN_DIS			BIT(18)
+/* GLOBAL CTRL */
+#define  YT922X_FUNC_ACL			BIT(5)
+#define  YT922X_FUNC_MIB			BIT(4)
+/* CTRL PKT */
+#define YT922X_FILTER_UNK_UCAST		0x180ec8
+#define YT922X_ACT_UNK_UCAST		0x180ed8
+#define YT922X_ACT_UNK_MCAST		0x180ee0
+#define  YT922X_ACT_UNK_MCAST_BYPASS_DROP_PIM	BIT(22)
+#define  YT922X_ACT_UNK_MCAST_BYPASS_DROP_MLD	BIT(21)
+#define  YT922X_ACT_UNK_MCAST_BYPASS_DROP_IGMP	BIT(20)
+#define  YT922X_ACT_UNK_ACTn_M(port)			GENMASK(2 * (port) + 1, 2 * (port))
+#define  YT922X_ACT_UNK_ACTn(port, x)			((x) << (2 * (port)))
+#define  YT922X_ACT_UNK_ACTn_FORWARD(port)		YT922X_ACT_UNK_ACTn(port, 0)  /* flood */
+#define  YT922X_ACT_UNK_ACTn_DROP(port)			YT922X_ACT_UNK_ACTn(port, 1)  /* discard */
+#define  YT922X_ACT_UNK_ACTn_TRAP(port)			YT922X_ACT_UNK_ACTn(port, 3)  /* steer to CPU */
+
+/* CPU PORT */
+#define YT922X_CPU_COPY			0x181100
+#define  YT922X_CPU_COPY_TO_INT_CPU		BIT(1)
+#define  YT922X_CPU_COPY_TO_EXT_CPU		BIT(0)
+#define YT922X_CPU_TAG_RX_CTRL		0x80504
+#define  YT922X_CPU_TAG_RX_MODE			BIT(0)
+#define YT922X_CPU_TAG_TX_CTRL		0x100710
+#define  YT922X_CPU_TAG_TX_TYPE		BIT(0)
+#define  YT922X_CPU_TAG_TX_MODE		BIT(1)
+#define  YT922X_CPU_TAG_TX_CTAG_OP	BIT(2)
+#define  YT922X_CPU_TAG_TX_STAG_OP	BIT(3)
+/* Define top ext addr for yt922x */
+#define YT922X_COMMON_EXT_PHYADDR 9
+
+enum yt922x_phy_reg_type {
+	YT922X_PHY_REG_TYPE_COMMON_EXT,
+	YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
+	YT922X_PHY_REG_TYPE_MII,
+	YT922X_PHY_REG_TYPE_EXT,
+	YT922X_PHY_REG_TYPE_MAX
+};
+
 struct yt921x_acl_entry {
 	u32 key[2];
 	u32 mask[2];
@@ -907,6 +999,8 @@ static inline int ethtool_speed_to_yt921x(int speed)
 		return YT921X_SPEED_2500;
 	case SPEED_10000:
 		return YT921X_SPEED_10000;
+	case SPEED_5000:
+		return YT921X_SPEED_5000;
 	default:
 		return YT921X_SPEED_INVALID;
 	}
diff --git a/drivers/net/dsa/motorcomm/mdio_bus.c b/drivers/net/dsa/motorcomm/mdio_bus.c
index 3aa1111efaa4..a4cb74d0fd90 100644
--- a/drivers/net/dsa/motorcomm/mdio_bus.c
+++ b/drivers/net/dsa/motorcomm/mdio_bus.c
@@ -299,3 +299,42 @@ int yt921x_mbus_ext_init(struct yt921x_priv *priv, struct device_node *mnp)
 
 	return 0;
 }
+
+/* intif extend register read/write api */
+int
+yt921x_intif_ext_write(struct yt921x_priv *priv, int port, int reg, u16 val)
+{
+	int res;
+
+	if (port > priv->series->max_ports)
+		return -ENODEV;
+
+	res = yt921x_intif_write(priv, port, YT92XX_PAGE_SELECT, reg);
+	if (res)
+		return res;
+
+	res = yt921x_intif_write(priv, port, YT92XX_PAGE, val);
+	if (res)
+		return res;
+
+	return 0;
+}
+
+int
+yt921x_intif_ext_read(struct yt921x_priv *priv, int port, int reg, u16 *valp)
+{
+	int res;
+
+	if (port > priv->series->max_ports)
+		return -ENODEV;
+
+	res = yt921x_intif_write(priv, port, YT92XX_PAGE_SELECT, reg);
+	if (res)
+		return res;
+
+	res = yt921x_intif_read(priv, port, YT92XX_PAGE, valp);
+	if (res)
+		return res;
+
+	return 0;
+}
diff --git a/drivers/net/dsa/motorcomm/mdio_bus.h b/drivers/net/dsa/motorcomm/mdio_bus.h
index e79b725d435b..513a8d2d7618 100644
--- a/drivers/net/dsa/motorcomm/mdio_bus.h
+++ b/drivers/net/dsa/motorcomm/mdio_bus.h
@@ -50,5 +50,7 @@ yt921x_intif_modify(struct yt921x_priv *priv, int port, int reg, u16 mask,
 
 int yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp);
 int yt921x_mbus_ext_init(struct yt921x_priv *priv, struct device_node *mnp);
+int yt921x_intif_ext_write(struct yt921x_priv *priv, int port, int reg, u16 val);
+int yt921x_intif_ext_read(struct yt921x_priv *priv, int port, int reg, u16 *valp);
 
 #endif
diff --git a/drivers/net/dsa/motorcomm/pcs-922x.c b/drivers/net/dsa/motorcomm/pcs-922x.c
new file mode 100644
index 000000000000..98d403d01ba3
--- /dev/null
+++ b/drivers/net/dsa/motorcomm/pcs-922x.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2026 Kyle Switch
+ */
+
+#include "chip.h"
+#include "mdio_bus.h"
+#include "pcs.h"
+#include "smi.h"
+
+#define to_device(priv) ((priv)->ds.dev)
+
+static int yt922x_sds_phyaddr_get(int port,
+				  enum yt922x_phy_reg_type reg_type)
+{
+	if (reg_type == YT922X_PHY_REG_TYPE_COMMON_EXT)
+		return YT922X_COMMON_EXT_PHYADDR;
+
+	return port;
+}
+
+static void yt922x_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
+				 struct phylink_link_state *state)
+{
+	struct yt921x_port *pp = pcs_to_yt921x_port(pcs);
+	struct yt921x_priv *priv = yt921x_port_to_priv(pp);
+	int port = pp->index;
+	int res = 0;
+	u16 data;
+	int addr;
+	u16 lp;
+
+	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII);
+	if (addr < 0) {
+		state->link = false;
+		return;
+	}
+
+	mutex_lock(&priv->reg_lock);
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		res = yt921x_intif_read(priv, addr, MII_BMSR, &data);
+		if (res)
+			goto err;
+		res = yt921x_intif_read(priv, addr, MII_LPA, &lp);
+		if (res)
+			goto err;
+		phylink_mii_c22_pcs_decode_state(state, neg_mode, data, lp);
+		break;
+	case PHY_INTERFACE_MODE_USXGMII:
+		res = yt921x_intif_read(priv, addr, YT922X_PCS_LINK_CTRL,
+					&data);
+		if (res)
+			goto err;
+		state->link =  FIELD_GET(YT922X_PCS_LINK_STATUS, data);
+		state->an_complete = FIELD_GET(YT922X_PCS_AN_COMPLETE, data);
+		res = yt921x_intif_read(priv, addr, MII_LPA, &lp);
+		if (res)
+			goto err;
+		if (state->link)
+			phylink_decode_usxgmii_word(state, lp);
+		break;
+	default:
+		state->link = false;
+		break;
+	}
+	mutex_unlock(&priv->reg_lock);
+	return;
+
+err:
+	mutex_unlock(&priv->reg_lock);
+	state->link = false;
+}
+
+static void yt922x_pcs_an_restart(struct phylink_pcs *pcs)
+{
+	struct yt921x_port *pp = pcs_to_yt921x_port(pcs);
+	struct yt921x_priv *priv = yt921x_port_to_priv(pp);
+	struct device *dev = to_device(priv);
+	int port = pp->index;
+	u16 data;
+	int addr;
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII);
+	if (addr < 0) {
+		res = addr;
+		goto err;
+	}
+	res = yt921x_intif_read(priv, addr, MII_BMCR, &data);
+	if (res)
+		goto err;
+	data |= BMCR_ANRESTART;
+	res = yt921x_intif_write(priv, addr, MII_BMCR, data);
+	if (res)
+		goto err;
+	mutex_unlock(&priv->reg_lock);
+	return;
+
+err:
+	mutex_unlock(&priv->reg_lock);
+	if (res)
+		dev_err(dev, "Failed to %s PCS port %d: %i\n", "an restart",
+			port, res);
+}
+
+static int yt922x_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
+			     phy_interface_t interface,
+			     const unsigned long *advertising,
+			     bool permit_pause_to_mac)
+{
+	struct yt921x_port *pp = pcs_to_yt921x_port(pcs);
+	struct yt921x_priv *priv = yt921x_port_to_priv(pp);
+	int res, port;
+	u16 data;
+	u16 ctrl;
+	int addr;
+
+	port = pp->index;
+	mutex_lock(&priv->reg_lock);
+	addr = yt922x_sds_phyaddr_get
+		(port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT);
+	if (addr < 0) {
+		res = addr;
+		goto err;
+	}
+	/* write protect */
+	res = yt921x_intif_ext_write(priv, addr, 0x4be, 0xd);
+	if (res)
+		goto err;
+	switch (interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+		ctrl = YT922X_SERDES_MODE_SGMII;
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		ctrl = YT922X_SERDES_MODE_1000BASEX;
+		break;
+	case PHY_INTERFACE_MODE_2500BASEX:
+		ctrl = YT922X_SERDES_MODE_2500BASEX;
+		break;
+	case PHY_INTERFACE_MODE_USXGMII:
+		ctrl = YT922X_SERDES_MODE_USXGMII;
+		break;
+	default:
+		res = -EINVAL;
+		goto err;
+	}
+	res = yt921x_intif_ext_read(priv, addr, YT922X_PORT_SDSn, &data);
+	if (res)
+		goto err;
+	data &= ~YT922X_SERDES_MODE_M;
+	data |= ctrl;
+	res = yt921x_intif_ext_write(priv, addr, YT922X_PORT_SDSn, data);
+	if (res)
+		goto err;
+	mutex_unlock(&priv->reg_lock);
+
+	return res;
+
+err:
+	mutex_unlock(&priv->reg_lock);
+
+	return res;
+}
+
+const struct phylink_pcs_ops yt922x_phylink_pcs_ops = {
+	.pcs_get_state = yt922x_pcs_get_state,
+	.pcs_config = yt922x_pcs_config,
+	.pcs_an_restart = yt922x_pcs_an_restart,
+};
diff --git a/drivers/net/dsa/motorcomm/pcs.h b/drivers/net/dsa/motorcomm/pcs.h
index 42426558086a..c85caa7bf212 100644
--- a/drivers/net/dsa/motorcomm/pcs.h
+++ b/drivers/net/dsa/motorcomm/pcs.h
@@ -9,5 +9,6 @@
 #include <linux/phylink.h>
 
 extern const struct phylink_pcs_ops yt921x_phylink_pcs_ops;
+extern const struct phylink_pcs_ops yt922x_phylink_pcs_ops;
 
 #endif
-- 
2.25.1

Re: [PATCH net-next v14 5/5] net: dsa: motorcomm: Add support for Motorcomm YT922x

[Andrew Lunn]

> +++ b/drivers/net/dsa/motorcomm/mdio_bus.c
> @@ -299,3 +299,42 @@ int yt921x_mbus_ext_init(struct yt921x_priv *priv, struct device_node *mnp)
>  
>  	return 0;
>  }
> +
> +/* intif extend register read/write api */
> +int
> +yt921x_intif_ext_write(struct yt921x_priv *priv, int port, int reg, u16 val)
> +{
> +	int res;
> +
> +	if (port > priv->series->max_ports)
> +		return -ENODEV;
> +
> +	res = yt921x_intif_write(priv, port, YT92XX_PAGE_SELECT, reg);
> +	if (res)
> +		return res;
> +
> +	res = yt921x_intif_write(priv, port, YT92XX_PAGE, val);
> +	if (res)
> +		return res;
> +
> +	return 0;
> +}
> +
> +int
> +yt921x_intif_ext_read(struct yt921x_priv *priv, int port, int reg, u16 *valp)
> +{
> +	int res;
> +
> +	if (port > priv->series->max_ports)
> +		return -ENODEV;
> +
> +	res = yt921x_intif_write(priv, port, YT92XX_PAGE_SELECT, reg);
> +	if (res)
> +		return res;
> +
> +	res = yt921x_intif_read(priv, port, YT92XX_PAGE, valp);
> +	if (res)
> +		return res;
> +
> +	return 0;
> +}

Why are these in mdio_bus? They don't have anything to do with MDIO.

Since they build on yt921x_intif_read they seem to fit in yt921x.c
better? Or am i missing something?

> +static void yt922x_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
> +				 struct phylink_link_state *state)
> +{
> +	struct yt921x_port *pp = pcs_to_yt921x_port(pcs);
> +	struct yt921x_priv *priv = yt921x_port_to_priv(pp);
> +	int port = pp->index;
> +	int res = 0;
> +	u16 data;
> +	int addr;
> +	u16 lp;
> +
> +	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII);
> +	if (addr < 0) {
> +		state->link = false;
> +		return;
> +	}
> +
> +	mutex_lock(&priv->reg_lock);
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		res = yt921x_intif_read(priv, addr, MII_BMSR, &data);
> +		if (res)
> +			goto err;
> +		res = yt921x_intif_read(priv, addr, MII_LPA, &lp);
> +		if (res)
> +			goto err;
> +		phylink_mii_c22_pcs_decode_state(state, neg_mode, data, lp);


Rather than data, call it bmsr. It then fits:

void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
				      unsigned int neg_mode, u16 bmsr, u16 lpa)

> +static void yt922x_pcs_an_restart(struct phylink_pcs *pcs)
> +{
> +	struct yt921x_port *pp = pcs_to_yt921x_port(pcs);
> +	struct yt921x_priv *priv = yt921x_port_to_priv(pp);
> +	struct device *dev = to_device(priv);
> +	int port = pp->index;
> +	u16 data;
> +	int addr;
> +	int res;
> +
> +	mutex_lock(&priv->reg_lock);
> +	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII);
> +	if (addr < 0) {
> +		res = addr;
> +		goto err;
> +	}
> +	res = yt921x_intif_read(priv, addr, MII_BMCR, &data);
> +	if (res)
> +		goto err;
> +	data |= BMCR_ANRESTART;
> +	res = yt921x_intif_write(priv, addr, MII_BMCR, data);

Again, bmcr. It then looks a lot like
phylink_mii_c22_pcs_an_restart().

> +	if (res)
> +		goto err;
> +	mutex_unlock(&priv->reg_lock);
> +	return;
> +
> +err:
> +	mutex_unlock(&priv->reg_lock);
> +	if (res)
> +		dev_err(dev, "Failed to %s PCS port %d: %i\n", "an restart",
> +			port, res);
> +}
> +
> +static int yt922x_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> +			     phy_interface_t interface,
> +			     const unsigned long *advertising,
> +			     bool permit_pause_to_mac)
> +{

advertising does not appear to be used here. Does this pcs not support
it? That would be odd, because you looked at lpa in earlier functions.

	Andrew

Re: [PATCH net-next v14 1/5] net: dsa: motorcomm: initialize dsa_switch based on chipid

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In yt92xx_register_switch() (drivers/net/dsa/motorcomm/chip.c),
  when yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid) fails, the error…
- [Low] The commit message presents the change as a pure refactor
  ('Replace the hardcoded chip handling ... with a data-driven loopup').
- [Low] yt92xx_series_lookup() is an open-coded if-chain (`major ==
  YT9215_MAJOR || major == YT9218_MAJOR`), not a table lookup.
- [Low] Naming and typing nits in the new series abstraction.
- [Low] The commit message says it replaces 'the hardcoded chip handling
  in yt92xx_register_switch()'.
- [Low] At this commit, enum yt92xx_mode defines YT92XX_MODE_YT922X and
  YT92XX_MODE_MAX, but yt92xx_series_table[] is unsized and has only the…

--

net: dsa: motorcomm: initialize dsa_switch based on chipid

This patch moves the dsa_switch field setup out of yt921x_mdio_probe()
and into a new helper, yt92xx_register_switch(). The helper reads
YT921X_CHIP_ID at probe time, maps the major number to an entry in a
new struct yt92xx_series table, and copies that entry's ops and limits
into the dsa_switch.

> Replace the hardcoded chip handling in yt92xx_register_switch() with a
> data-driven loopup based on the chip ID.

[Severity: Low]
This isn't a bug, but yt92xx_register_switch() doesn't exist before
this patch. This patch adds it.

The hardcoded assignments being replaced were in yt921x_mdio_probe().
Should the message name yt921x_mdio_probe() instead?

There's also a typo: "loopup" should be "lookup".

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 91cc5647f2b66..d4c20658216a6 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -4078,6 +4078,67 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
>  	.setup			= yt921x_dsa_setup,
>  };
>  
> +static const struct yt92xx_series yt92xx_series_table[] = {
> +	[YT92XX_MODE_YT921X] = {
> +		.name = "YT921x",
> +		.max_ports = YT921X_PORT_NUM,
> +		.num_lag_ids = YT921X_LAG_NUM,
> +		.ageing_time_min = 1 * 5000,
> +		.ageing_time_max = U16_MAX * 5000,
> +		.dscp_prio_mapping_is_global = true,
> +		.assisted_learning_on_cpu_port = true,
> +		.switch_ops = &yt921x_dsa_switch_ops,
> +		.mac_ops = &yt921x_phylink_mac_ops
> +	},
> +};

[Severity: Low]
This isn't a bug, but yt92xx_series_table[] has no explicit size and
only one entry, YT92XX_MODE_YT921X. enum yt92xx_mode already defines
YT92XX_MODE_YT922X and YT92XX_MODE_MAX, so indexing the table with
YT92XX_MODE_YT922X would read past the end.

Nothing does that at this commit, because yt92xx_series_lookup() only
returns index 0. The last patch in the series also adds the
YT92XX_MODE_YT922X entry.

Would sizing the table as [YT92XX_MODE_MAX] make this clearer?

> +
> +static const struct yt92xx_series *yt92xx_series_lookup(u32 major)
> +{
> +	if (major == YT9215_MAJOR || major == YT9218_MAJOR)
> +		return &yt92xx_series_table[YT92XX_MODE_YT921X];
> +	else
> +		return NULL;
> +}

[Severity: Low]
This isn't a bug, but this if-chain classifies the major separately
from yt921x_infos[]. Those entries already carry .major, and
yt921x_chip_detect() walks them after reading YT921X_CHIP_ID a second
time:

drivers/net/dsa/motorcomm/chip.c:yt921x_chip_detect() {
    ...
	for (info = yt921x_infos; info->name; info++)
		if (info->major == major)
			break;
    ...
}

Every new chip variant has to be added in both places:

- If only yt921x_infos[] is updated, probe rejects the chip with
  -ENODEV.
- If only this if-chain is updated, probe accepts the chip and setup
  rejects it later.

The last patch in the series, "net: dsa: motorcomm: Add support for
Motorcomm YT922x", adds an "else if (major == YT9224_MAJOR)" here
instead of linking the series to the info table.

Could the series come from yt921x_infos[] instead, to match the
"data-driven lookup" in the commit message?

> +
> +static int yt92xx_register_switch(struct dsa_switch *ds)
> +{
> +	struct yt921x_priv *priv = dsa_to_yt921x_priv(ds);
> +	struct device *dev = yt921x_priv_to_device(priv);
> +	const struct yt92xx_series *series;
> +	u32 chipid;
> +	u32 major;
> +	int res;
> +
> +	mutex_lock(&priv->reg_lock);
> +	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
> +	mutex_unlock(&priv->reg_lock);
> +	if (res) {
> +		dev_err(dev, "Unexpected chipid 0x%x\n", chipid);
> +		return res;
> +	}
> +
> +	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
> +	series = yt92xx_series_lookup(major);
> +	if (!series) {
> +		dev_err(dev, "Undefined series info for the corresponding major\n");
> +		return -ENODEV;
> +	}

[Severity: Low]
Is chipid initialized when the first dev_err() runs?
yt921x_reg_mdio_read() only writes *valp after all four MDIO accesses
succeed:

drivers/net/dsa/motorcomm/chip.c:yt921x_reg_mdio_read() {
    ...
	res = __mdiobus_read(bus, addr, reg_data);
	if (res < 0)
		goto end;
	val = (val << 16) | (u16)res;

	*valp = val;
	res = 0;

end:
    ...
}

On an MDIO error, the log would show a stale stack value and describe
a bus failure as an unexpected chip ID.

The !series branch, where chipid is valid, doesn't print the ID at
all. Should the two messages be swapped, the way yt921x_chip_detect()
handles it?

The last patch in the series, "net: dsa: motorcomm: Add support for
Motorcomm YT922x", fixes this. The read-failure branch becomes a plain
return res, and the !series branch prints the chip ID. This
intermediate commit could still be hit during a bisect, so could the
fix be folded into this patch?

[ ... ]

> @@ -4146,15 +4208,10 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
>  
>  	ds = &priv->ds;
>  	ds->dev = dev;
> -	ds->assisted_learning_on_cpu_port = true;
> -	ds->dscp_prio_mapping_is_global = true;
>  	ds->priv = priv;
> -	ds->ops = &yt921x_dsa_switch_ops;
> -	ds->ageing_time_min = 1 * 5000;
> -	ds->ageing_time_max = U16_MAX * 5000;
> -	ds->phylink_mac_ops = &yt921x_phylink_mac_ops;
> -	ds->num_lag_ids = YT921X_LAG_NUM;
> -	ds->num_ports = YT921X_PORT_NUM;
> +	res = yt92xx_register_switch(ds);
> +	if (res)
> +		return res;

[Severity: Low]
The commit message describes this as a pure refactor. However,
yt921x_mdio_probe() now takes priv->reg_lock and does an MDIO read of
YT921X_CHIP_ID before dsa_register_switch().

Before this patch, probe only filled in static dsa_switch fields. The
first register access was yt921x_chip_detect(), reached via
yt921x_dsa_setup()->yt921x_chip_reset().

Now an MDIO error or an unknown major fails probe directly, rather
than failing in the .setup callback. The chip ID is also read twice:
once here and again in yt921x_chip_detect().

Could the commit message mention this change in behaviour?

> diff --git a/drivers/net/dsa/motorcomm/chip.h b/drivers/net/dsa/motorcomm/chip.h
> index 25ff77054de30..85174a51b72d6 100644
> --- a/drivers/net/dsa/motorcomm/chip.h
> +++ b/drivers/net/dsa/motorcomm/chip.h
> @@ -841,9 +841,28 @@ static inline u16 yt921x_info_ports_mask(const struct yt921x_info *info)
>  	return info->internal_mask | info->xmii_mask | info->serdes_mask;
>  }
>  
> +enum yt92xx_mode {
> +	YT92XX_MODE_YT921X,
> +	YT92XX_MODE_YT922X,
> +	YT92XX_MODE_MAX,
> +};
> +
> +struct yt92xx_series {
> +	const char *name;
> +	unsigned int max_ports;
> +	unsigned int num_lag_ids;
> +	unsigned int ageing_time_min;
> +	unsigned int ageing_time_max;
> +	u32 dscp_prio_mapping_is_global;
> +	u32 assisted_learning_on_cpu_port;
> +	const struct dsa_switch_ops *switch_ops;
> +	const struct phylink_mac_ops *mac_ops;
> +};

[Severity: Low]
This isn't a bug, but a few small things about the new series
abstraction:

dscp_prio_mapping_is_global and assisted_learning_on_cpu_port are
boolean flags that get copied into bool fields of struct dsa_switch.
Should they be bool here as well?

The name field and YT92XX_MODE_MAX don't appear to be used anywhere,
either at this commit or at the end of the series.

yt92xx_register_switch() in chip.c doesn't register anything. It
reads the chip ID and fills in dsa_switch fields, and the actual
registration is the later dsa_register_switch() call in
yt921x_mdio_probe(). Would a name such as yt92xx_init_switch() fit
better?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013011122.1889766-1-kyle.switch%40motor-comm.com

Re: [PATCH net-next v14 3/5] net: dsa: motorcomm: use fixed-width GENMASK_U*() helpers

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message for "net: dsa: motorcomm: use fixed-width
  GENMASK_U*() helpers" does not describe the diff correctly.

--

net: dsa: motorcomm: use fixed-width GENMASK_U*() helpers

This patch replaces several GENMASK() uses in the motorcomm DSA driver
with GENMASK_U16() and GENMASK_U32(). It touches the internal_mask
entries in yt921x_infos[], the ctrl value in yt921x_chip_setup_dsa()
and the phy_mask computation in yt921x_mbus_int_init(). It is described
as a type cleanup with no functional change.

> Some masks assigned to u16 and u32 fields are defined with
> GENMASK(),which is an unsigned long constant and wider than
> the field type on 64-bit builds. Convert the affected
> internal_mask entries in chip.c and the YT921X_MBUS_CTRL_*
> masks used by mdio_bus.c to fixed-width helpers.

[Severity: Low]
Does this description match the diff?

The YT921X_MBUS_CTRL_* masks are defined in
drivers/net/dsa/motorcomm/mdio_bus.h. This patch does not touch that
file, and the masks still use plain GENMASK() after the patch:

#define  YT921X_MBUS_CTRL_PORT_M		GENMASK(25, 21)
#define  YT921X_MBUS_CTRL_REG_M			GENMASK(20, 16)
#define  YT921X_MBUS_CTRL_TYPE_M		GENMASK(11, 8)  /* wild guess */
#define  YT921X_MBUS_CTRL_OP_M			GENMASK(3, 2)  /* wild guess */

The only change in mdio_bus.c is the mii_bus phy_mask in
yt921x_mbus_int_init(). That mask is computed from a runtime value and
is not one of the YT921X_MBUS_CTRL_* masks:

-	mbus->phy_mask = (u32)~GENMASK(max_ports - 1, 0);
+	mbus->phy_mask = ~GENMASK_U32(max_ports - 1, 0);

In chip.c, the changes also go beyond the internal_mask entries.
yt921x_chip_setup_dsa() converts the local register value written to
YT921X_FILTER_UNK_UCAST and YT921X_FILTER_UNK_MCAST:

-	ctrl = GENMASK(10, 0);
+	ctrl = GENMASK_U32(10, 0);

Could the message be reworded to describe the phy_mask and ctrl
changes? Alternatively, should the mdio_bus.h conversion be added as
the message describes?

The description and the diff disagree, so was the Reviewed-by tag
perhaps given on an earlier revision with different content?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013011122.1889766-1-kyle.switch%40motor-comm.com

Re: [PATCH net-next v14 4/5] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The tag-layout comment in net/dsa/tag_yt922x.c does not match
  what yt922x_tag_xmit() writes.
- [Low] Concern: yt922x_tag_rcv() never calls
  dsa_default_offload_fwd_mark(), so the software bridge might forward…

--

net: dsa: tag_yt922x: add support for Motorcomm YT922x tags

This adds a new DSA tag driver for Motorcomm YT922x switches. YT922x uses
the same ETH_P_YT921X EtherType and 8-byte tag length as YT921x, but the
tag layout is different. The patch also adds a co-maintainer to the
Motorcomm DSA MAINTAINERS entry.

> diff --git a/net/dsa/tag_yt922x.c b/net/dsa/tag_yt922x.c
> new file mode 100644
> index 0000000000000..b6146a9af98b3
> --- /dev/null
> +++ b/net/dsa/tag_yt922x.c
> @@ -0,0 +1,122 @@

[ ... ]

> +/*
> + * To define the from cpu tag format 8 bytes.
> + *
> + * To-CPU (RX) tag format, 8 bytes:
> + *
> + *   tag[0]: TPID (default: 0x9988)
> + *   tag[1]: VID lookup
> + *   tag[2]: bit 5:2  - RX source port
> + *
> + * From-CPU (TX) tag format, 8 bytes:
> + *
> + *   tag[0]: TPID (default: 0x9988)
> + *   tag[1]: bit 8:0 - Dst port mask
> + *   tag[2]: bit 15  - Dst port 0
> + *
> + */

[ ... ]

> +	tag[0] = htons(ETH_P_YT921X);
> +	ports = dsa_xmit_port_mask(skb, netdev);
> +	/*To fill in the case where the port index is not 0 */
> +	ctrl = YT922X_TAG_PRIO(skb->priority) | YT922X_TAG_FORCE_DST |
> +	       YT922X_TAG_PORTS(ports >> 1);
> +	tag[1] = htons(ctrl);

[Severity: Low]
This isn't a bug, but does the tag layout comment match what
yt922x_tag_xmit() writes? The commit message says:

    The new driver documents both the to-CPU and from-CPU tag layouts.

The from-CPU part of the comment describes tag[1] bits 8:0 as the
destination port mask. yt922x_tag_xmit() puts ports >> 1 into that field,
though, so bit 0 there is port 1, not port 0.

tag[1] also always carries YT922X_TAG_FORCE_DST (bit 9), YT922X_TAG_PRIO_M
(bits 12:10) and YT922X_TAG_PRIO_EN (bit 13), and tag[3] is written as 0.
The comment doesn't mention any of these.

Separately, the comment block opens with "To define the from cpu tag format
8 bytes." right above the To-CPU (RX) description. Is that line a leftover?

[ ... ]

> +	/* Remove tag and update checksum */
> +	skb_pull_rcsum(skb, YT922X_TAG_LEN);
> +	dsa_strip_etype_header(skb, YT922X_TAG_LEN);
> +
> +	return skb;
> +}

[Severity: Low]
Should yt922x_tag_rcv() call dsa_default_offload_fwd_mark() before it
returns? tag_yt921x.c and most of the other tag drivers do.

As far as I can tell, leaving it out has no effect with the switch driver
later in the series. yt922x_dsa_switch_ops has no .port_bridge_join, so
dp->bridge stays NULL and the bridge falls back to software bridging. In
that case the helper would set the mark to 0 anyway:

net/dsa/tag.h:dsa_default_offload_fwd_mark() {
    ...
	skb->offload_fwd_mark = !!(dp->bridge);
}

The port isolation and trap-to-CPU setup in the switch driver also stop the
hardware from forwarding between user ports.

If bridge offload is added later, would the software bridge forward frames
again that the switch has already forwarded, unless the mark is set here?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013011122.1889766-1-kyle.switch%40motor-comm.com

Re: [PATCH net-next v14 5/5] net: dsa: motorcomm: Add support for Motorcomm YT922x

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 3 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] net: dsa: motorcomm: port control register clobbered in
  yt922x_port_up due to missing read
- [Medium] The new "YT9228" entry in yt921x_infos can never be selected.
- [Medium] The patch adds { .compatible = "motorcomm,yt9224" } to
  yt921x_of_match, but the only binding for this driver,…
- [Medium] yt922x_pcs_config() accepts neg_mode, advertising and
  permit_pause_to_mac but uses none of them.
- [Low] yt922x_port_down() and yt922x_port_up() each run several separate
  register transactions: the PORTn_CTRL MAC/link bits, then FORCE_OP,…

--

net: dsa: motorcomm: Add support for Motorcomm YT922x

This commit adds YT922x series support to the existing YT921x DSA driver.
It adds a new series table entry, phylink MAC ops, a SerDes PCS
implementation in pcs-922x.c and a "motorcomm,yt9224" compatible. The
commit message says only the YT9224 variant is supported for now.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index ba988e121d68e..f0488fac90976 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c

[ ... ]

> @@ -76,6 +76,18 @@ static const struct yt921x_info yt921x_infos[] = {
>  		BIT(8) | BIT(9),
>  		BIT(8) | BIT(9),
>  	},
> +	{
> +		"YT9224", YT9224_MAJOR, 0, 0,
> +		GENMASK_U16(7, 4),
> +		0,
> +		BIT(8) | BIT(0),
> +	},
> +	{
> +		"YT9228", YT9224_MAJOR, 0, 0,
> +		GENMASK_U16(7, 4),
> +		0,
> +		BIT(8) | GENMASK_U16(3, 0),
> +	},
>  	{}
>  };

[ ... ]

> @@ -3631,6 +3648,14 @@ static int yt921x_chip_detect(struct yt921x_priv *priv)
>  		return -ENODEV;
>  	}
>  
> +	if (!yt921x_needs_extmode_check(major)) {
> +		dev_info(dev,
> +			 "Motorcomm %s ethernet switch, chipid: 0x%x\n",
> +			 info->name, chipid);
> +		priv->info = info;
> +		return 0;
> +	}

[Severity: Medium]
Can the YT9228 entry in yt921x_infos[] ever be selected?

Both new entries use YT9224_MAJOR with mode 0 and extmode 0. The lookup
loop in yt921x_chip_detect() stops at the first entry whose major matches:

	for (info = yt921x_infos; info->name; info++)
		if (info->major == major)
			break;

For major 0x9004 that is always "YT9224". The new early return then uses
that entry without checking mode or extmode, and the two entries have the
same values for those anyway.

The commit message says:

    YT9228 is not supported yet.

If a YT9228 reports this major, as the table says it does, wouldn't it
bind instead of failing with -ENODEV? yt92xx_series_lookup() also sends
every 0x9004 chip to the YT922x series, and the chip would be logged as
"Motorcomm YT9224 ...".

It would also get the YT9224 serdes_mask. In that case,
yt922x_dsa_phylink_get_caps() and yt922x_pcs_setup() leave ports 1-3 with
no supported interfaces and no PCS. phylink_create() would then fail for
those ports, and DSA would quietly treat them as unused. Is that intended?

[ ... ]

> +static int
> +yt922x_port_up(struct yt921x_priv *priv, int port, unsigned int mode,
> +	       phy_interface_t interface, int speed, int duplex,
> +	       bool tx_pause, bool rx_pause)
> +{

[ ... ]

> +	res = yt921x_reg_write(priv, YT922X_PORTn_CTRL(port), ctrl);
> +	if (res)
> +		return res;
> +
> +	/* force op */
> +	mask = YT922X_PORT_FORCE_OP;
> +	res = yt921x_reg_set_bits(priv, YT922X_PORTn_CTRL(port), mask);
> +	if (res)
> +		return res;
> +
> +	/* enable en_phy */
> +	res = yt921x_reg_set_bits(priv, YT922X_EN_PHY_VALUE, BIT(port));
> +	if (res)
> +		return res;
> +	res = yt921x_reg_set_bits(priv, YT922X_EN_PHY_OVERWRITE, BIT(port));
> +	if (res)
> +		return res;
> +
> +	return 0;
> +}

[Severity: High]
Is this meant to be a read-modify-write of YT922X_PORTn_CTRL?

ctrl starts at 0. Only the speed, duplex, pause and MAC/CFG enable
bits are ORed into it, so clearing YT922X_PORT_FC_AN and
YT922X_PORT_LINK_AN afterwards does nothing. That clear only makes
sense if ctrl was supposed to hold the current register contents
first.

As written, yt921x_reg_write() replaces the whole register on every
link up. Any field not set here is forced to zero each time phylink
calls yt922x_phylink_mac_link_up(). That includes
YT922X_PORT_HALF_PAUSE and any reserved or vendor-default bits above
YT922X_PORT_FORCE_OP. Is that safe on this hardware?

If a full write is really intended, the dead clear could be dropped
and the register layout documented. Otherwise, should this use
yt921x_reg_update_bits() with a mask covering only the fields being
programmed? That would mean speed, duplex, pause, the MAC/CFG enables
and the AN bits, in the same way that yt922x_port_down() only touches
the bits it cares about via yt921x_reg_clear_bits().

[Severity: Low]
This is minor, but what state is the port left in if the
YT922X_EN_PHY_VALUE update fails after FORCE_OP has already applied the
forced MAC link-up?

The MAC would stay forced up while the PHY-enable override from the
previous yt922x_port_down() stays forced low. yt922x_phylink_mac_link_up()
only logs the error. yt922x_port_down() has the same structure, with no
rollback of earlier steps.

This copies the existing yt921x_port_up() and yt921x_port_down() pattern.
It can only happen once SMI/MDIO access is already failing.

[ ... ]

> @@ -4220,6 +4655,7 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
>  
>  static const struct of_device_id yt921x_of_match[] = {
>  	{ .compatible = "motorcomm,yt9215" },
> +	{ .compatible = "motorcomm,yt9224" },
>  	{}
>  };

[Severity: Medium]
Should this come with a binding update? The binding in
Documentation/devicetree/bindings/net/dsa/motorcomm,yt921x.yaml still
only has:

  compatible:
    const: motorcomm,yt9215

With that binding, any DTS using "motorcomm,yt9224" would fail dt-schema
validation, and checkpatch would report the compatible as undocumented.

The binding text also covers YT921x port layouts only. Its mdio
description says:

      Internal MDIO bus for the internal GbE PHYs. PHY 0-7 are used for Port
      0-7 respectively.

The new info table gives YT9224 internal PHYs only on ports 4-7, and
SerDes on ports 0 and 8.

[ ... ]

> diff --git a/drivers/net/dsa/motorcomm/pcs-922x.c b/drivers/net/dsa/motorcomm/pcs-922x.c
> new file mode 100644
> index 0000000000000..98d403d01ba3c
> --- /dev/null
> +++ b/drivers/net/dsa/motorcomm/pcs-922x.c

[ ... ]

> +static int yt922x_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
> +			     phy_interface_t interface,
> +			     const unsigned long *advertising,
> +			     bool permit_pause_to_mac)
> +{

[ ... ]

> +	res = yt921x_intif_ext_read(priv, addr, YT922X_PORT_SDSn, &data);
> +	if (res)
> +		goto err;
> +	data &= ~YT922X_SERDES_MODE_M;
> +	data |= ctrl;
> +	res = yt921x_intif_ext_write(priv, addr, YT922X_PORT_SDSn, data);
> +	if (res)
> +		goto err;
> +	mutex_unlock(&priv->reg_lock);
> +
> +	return res;

[Severity: Medium]
yt922x_pcs_config() never uses neg_mode, advertising or
permit_pause_to_mac. Apart from the 0xd write to ext reg 0x4be, it only
rewrites the SerDes mode field. Is that enough to meet the pcs_config
contract?

The pcs_config kernel-doc in include/linux/phylink.h says in-band should
be enabled in MLO_AN_INBAND and disabled otherwise. It also says the
1000BASE-X advertisement should be programmed into the PCS. pcs-921x.c
does this with phylink_mii_c22_pcs_encode_advertisement() and
BMCR_ANENABLE handling.

Here, in-band-enabled and in-band-disabled requests produce the same
register writes. Depending on the reset default, won't one of them leave
the hardware AN state wrong? For example, a fixed-link 2500BASE-X CPU port
could be left with AN enabled, or an in-band SGMII PHY link with AN
disabled.

phylink_change_inband_advert() also relies on pcs_config() to apply
advertisement and pause changes:

	ret = phylink_pcs_config(pl->pcs, pl->pcs_neg_mode, &pl->link_config,
				 !!(pl->link_config.pause & MLO_PAUSE_AN));
	if (ret < 0)
		return ret;

	if (ret > 0)
		phylink_pcs_an_restart(pl);

Since this always returns 0 and never writes MII_ADVERTISE, wouldn't
ethtool advertisement or pause changes be silently dropped?

yt922x_pcs_get_state() also decodes BMSR/LPA as if in-band AN had been
configured. yt922x_pcs_an_restart() sets BMCR_ANRESTART even though AN was
never configured.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013011122.1889766-1-kyle.switch%40motor-comm.com