mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver
@ 2026-09-17  8:22 Kyle Switch
  2026-09-17  8:22 ` [PATCH net-next v8 1/6] net: dsa: motorcomm: initialize dsa_switch based on chipid Kyle Switch
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Kyle Switch @ 2026-09-17  8:22 UTC (permalink / raw)
  To: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel
  Cc: ming.xu, xiaolin.xu, jianmin.wang, wei.zhang, sijia.huang

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 (6):
  net: dsa: motorcomm: initialize dsa_switch based on chipid
  net: dsa: motorcomm: use max_ports in series for port bounds checking
  net: dsa: motorcomm: move mib start from probe() to dsa_setup()
  net: dsa: motorcomm: move mib stop from remove() to teardown()
  net: dsa: tag_yt922x: add support for Motorcomm YT922x tags
  net: dsa: motorcomm: Add support for Motorcomm YT922x

 drivers/net/dsa/motorcomm/Kconfig |   3 +-
 drivers/net/dsa/motorcomm/chip.c  | 789 ++++++++++++++++++++++++++++--
 drivers/net/dsa/motorcomm/chip.h  | 113 +++++
 include/net/dsa.h                 |   2 +
 net/dsa/Kconfig                   |   6 +
 net/dsa/Makefile                  |   1 +
 net/dsa/tag_yt922x.c              | 111 +++++
 7 files changed, 996 insertions(+), 29 deletions(-)
 create mode 100644 net/dsa/tag_yt922x.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v8 1/6] net: dsa: motorcomm: initialize dsa_switch based on chipid
  2026-09-17  8:22 [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver Kyle Switch
@ 2026-09-17  8:22 ` Kyle Switch
  2026-09-21  9:35   ` netdev-bot+sashiko
  2026-09-17  8:22 ` [PATCH net-next v8 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking Kyle Switch
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Kyle Switch @ 2026-09-17  8:22 UTC (permalink / raw)
  To: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel
  Cc: ming.xu, xiaolin.xu, jianmin.wang, wei.zhang, sijia.huang

Introduce yt92xx_series structure to hold private data
for different switch families, replacing hardcoded logic
in probe(). This makes the driver more extensible for
future switch support.

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/chip.c | 69 ++++++++++++++++++++++++++++----
 drivers/net/dsa/motorcomm/chip.h | 20 +++++++++
 2 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index d663af010f43..564e45ece22c 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -4679,6 +4679,63 @@ 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] = {
+		.mode = 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 = to_yt921x_priv(ds);
+	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)
+		return res;
+
+	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
+	series = yt92xx_series_lookup(major);
+	if (!series)
+		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);
@@ -4727,6 +4784,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)
@@ -4754,15 +4812,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 83cd454955dd..c446aea449ed 100644
--- a/drivers/net/dsa/motorcomm/chip.h
+++ b/drivers/net/dsa/motorcomm/chip.h
@@ -960,9 +960,29 @@ struct yt921x_reg_ops {
 	int (*write)(void *context, u32 reg, u32 val);
 };
 
+enum yt92xx_mode {
+	YT92XX_MODE_YT921X,
+	YT92XX_MODE_YT922X,
+	YT92XX_MODE_MAX,
+};
+
+struct yt92xx_series {
+	enum yt92xx_mode mode;
+	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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v8 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking
  2026-09-17  8:22 [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver Kyle Switch
  2026-09-17  8:22 ` [PATCH net-next v8 1/6] net: dsa: motorcomm: initialize dsa_switch based on chipid Kyle Switch
@ 2026-09-17  8:22 ` Kyle Switch
  2026-09-21  9:35   ` netdev-bot+sashiko
  2026-09-17  8:22 ` [PATCH net-next v8 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup() Kyle Switch
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Kyle Switch @ 2026-09-17  8:22 UTC (permalink / raw)
  To: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel
  Cc: ming.xu, xiaolin.xu, jianmin.wang, wei.zhang, sijia.huang

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.

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

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index 564e45ece22c..2156230a78aa 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -357,7 +357,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);
@@ -375,7 +375,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);
@@ -390,6 +390,7 @@ yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp)
 {
 	struct device *dev = to_device(priv);
 	struct mii_bus *mbus;
+	u32 max_ports;
 	int res;
 
 	mbus = devm_mdiobus_alloc(dev);
@@ -402,7 +403,8 @@ 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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v8 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup()
  2026-09-17  8:22 [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver Kyle Switch
  2026-09-17  8:22 ` [PATCH net-next v8 1/6] net: dsa: motorcomm: initialize dsa_switch based on chipid Kyle Switch
  2026-09-17  8:22 ` [PATCH net-next v8 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking Kyle Switch
@ 2026-09-17  8:22 ` Kyle Switch
  2026-09-21  9:35   ` netdev-bot+sashiko
  2026-09-17  8:22 ` [PATCH net-next v8 4/6] net: dsa: motorcomm: move mib stop from remove() to teardown() Kyle Switch
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Kyle Switch @ 2026-09-17  8:22 UTC (permalink / raw)
  To: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel
  Cc: ming.xu, xiaolin.xu, jianmin.wang, wei.zhang, sijia.huang

Move the delayed work initialization of the per-port MIB polling
from yt921x_mdio_probe() into the DSA .setup() callback, changing
its lifetime from once per probe() to once per DSA .setup().

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/chip.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index 2156230a78aa..167b20835771 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -4553,6 +4553,14 @@ static int yt921x_dsa_setup(struct dsa_switch *ds)
 	struct device_node *child;
 	int res;
 
+	/* mib polling init */
+	for (size_t i = 0; i < ARRAY_SIZE(priv->ports); i++) {
+		struct yt921x_port *pp = &priv->ports[i];
+
+		pp->index = i;
+		INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);
+	}
+
 	mutex_lock(&priv->reg_lock);
 	res = yt921x_chip_reset(priv);
 	mutex_unlock(&priv->reg_lock);
@@ -4805,13 +4813,6 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 	priv->reg_ops = &yt921x_reg_ops_mdio;
 	priv->reg_ctx = mdio;
 
-	for (size_t i = 0; i < ARRAY_SIZE(priv->ports); i++) {
-		struct yt921x_port *pp = &priv->ports[i];
-
-		pp->index = i;
-		INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);
-	}
-
 	ds = &priv->ds;
 	ds->dev = dev;
 	ds->priv = priv;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v8 4/6] net: dsa: motorcomm: move mib stop from remove() to teardown()
  2026-09-17  8:22 [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver Kyle Switch
                   ` (2 preceding siblings ...)
  2026-09-17  8:22 ` [PATCH net-next v8 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup() Kyle Switch
@ 2026-09-17  8:22 ` Kyle Switch
  2026-09-21  9:35   ` netdev-bot+sashiko
  2026-09-17  8:22 ` [PATCH net-next v8 5/6] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags Kyle Switch
  2026-09-17  8:22 ` [PATCH net-next v8 6/6] net: dsa: motorcomm: Add support for Motorcomm YT922x Kyle Switch
  5 siblings, 1 reply; 13+ messages in thread
From: Kyle Switch @ 2026-09-17  8:22 UTC (permalink / raw)
  To: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel
  Cc: ming.xu, xiaolin.xu, jianmin.wang, wei.zhang, sijia.huang

This change ensures symmetry with the MIB start operation,
which is already invoked in the setup(). Additionally, it
consolidates per-switch operation into the appropriate DSA ops,
improving code organization and maintainability.

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/chip.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index 167b20835771..2412fe2f696f 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -4233,6 +4233,11 @@ static void yt921x_dsa_teardown(struct dsa_switch *ds)
 #if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
 	yt921x_leds_remove(priv);
 #endif
+	for (size_t i = ARRAY_SIZE(priv->ports); i-- > 0; ) {
+		struct yt921x_port *pp = &priv->ports[i];
+
+		disable_delayed_work_sync(&pp->mib_read);
+	}
 }
 
 static int yt921x_chip_detect(struct yt921x_priv *priv)
@@ -4763,12 +4768,6 @@ static void yt921x_mdio_remove(struct mdio_device *mdiodev)
 	if (!priv)
 		return;
 
-	for (size_t i = ARRAY_SIZE(priv->ports); i-- > 0; ) {
-		struct yt921x_port *pp = &priv->ports[i];
-
-		disable_delayed_work_sync(&pp->mib_read);
-	}
-
 	dsa_unregister_switch(&priv->ds);
 
 	for (unsigned int i = 0; i < ARRAY_SIZE(priv->acl_blks); i++) {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v8 5/6] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags
  2026-09-17  8:22 [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver Kyle Switch
                   ` (3 preceding siblings ...)
  2026-09-17  8:22 ` [PATCH net-next v8 4/6] net: dsa: motorcomm: move mib stop from remove() to teardown() Kyle Switch
@ 2026-09-17  8:22 ` Kyle Switch
  2026-09-21  9:35   ` netdev-bot+sashiko
  2026-09-17  8:22 ` [PATCH net-next v8 6/6] net: dsa: motorcomm: Add support for Motorcomm YT922x Kyle Switch
  5 siblings, 1 reply; 13+ messages in thread
From: Kyle Switch @ 2026-09-17  8:22 UTC (permalink / raw)
  To: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel
  Cc: ming.xu, xiaolin.xu, jianmin.wang, wei.zhang, sijia.huang

Add support for Motorcomm YT922x tags with 8bytes. which includes
ethertype field (default to 0x9988).

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 include/net/dsa.h    |   2 +
 net/dsa/Kconfig      |   6 +++
 net/dsa/Makefile     |   1 +
 net/dsa/tag_yt922x.c | 111 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+)
 create mode 100644 net/dsa/tag_yt922x.c

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7507d632e7c6..0807a595aaad 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -61,6 +61,7 @@ struct tc_action;
 #define DSA_TAG_PROTO_NETC_VALUE		33
 #define DSA_TAG_PROTO_KSZ8463_VALUE		34
 #define DSA_TAG_PROTO_MT7628_VALUE		35
+#define DSA_TAG_PROTO_YT922X_VALUE		36
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -99,6 +100,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NETC		= DSA_TAG_PROTO_NETC_VALUE,
 	DSA_TAG_PROTO_KSZ8463		= DSA_TAG_PROTO_KSZ8463_VALUE,
 	DSA_TAG_PROTO_MT7628		= DSA_TAG_PROTO_MT7628_VALUE,
+	DSA_TAG_PROTO_YT922X		= DSA_TAG_PROTO_YT922X_VALUE,
 };
 
 struct dsa_switch;
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 23b4b74004ed..0b9f8a632cf2 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -227,4 +227,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 d15bcf5c68f0..0c53f4184bdb 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -44,6 +44,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..1ee9d1773598
--- /dev/null
+++ b/net/dsa/tag_yt922x.c
@@ -0,0 +1,111 @@
+// 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:
+ */
+#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)
+#define YT922X_TAG_RX_PRIO_M	GENMASK(15, 13)
+
+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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v8 6/6] net: dsa: motorcomm: Add support for Motorcomm YT922x
  2026-09-17  8:22 [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver Kyle Switch
                   ` (4 preceding siblings ...)
  2026-09-17  8:22 ` [PATCH net-next v8 5/6] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags Kyle Switch
@ 2026-09-17  8:22 ` Kyle Switch
  2026-09-21  9:35   ` netdev-bot+sashiko
  5 siblings, 1 reply; 13+ messages in thread
From: Kyle Switch @ 2026-09-17  8:22 UTC (permalink / raw)
  To: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel
  Cc: ming.xu, xiaolin.xu, jianmin.wang, wei.zhang, sijia.huang

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 |   3 +-
 drivers/net/dsa/motorcomm/chip.c  | 686 +++++++++++++++++++++++++++++-
 drivers/net/dsa/motorcomm/chip.h  |  93 ++++
 3 files changed, 777 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/Kconfig b/drivers/net/dsa/motorcomm/Kconfig
index 79cdd79a1fd2..ab2b548c216f 100644
--- a/drivers/net/dsa/motorcomm/Kconfig
+++ b/drivers/net/dsa/motorcomm/Kconfig
@@ -1,7 +1,8 @@
 # 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
diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index 2412fe2f696f..1cf5e859d8af 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.
@@ -38,6 +38,9 @@ struct yt921x_mib_desc {
 #define MIB_DESC(_size, _offset, _name) \
 	{_size, _offset, _name}
 
+#define pcs_to_yt921x_port(_pcs) container_of((_pcs), struct yt921x_port, pcs)
+#define yt921x_port_to_priv(pp) \
+	container_of_const((pp), struct yt921x_priv, ports[(pp)->index])
 /* Must agree with yt921x_mib
  *
  * Unstructured fields (name != NULL) will appear in get_ethtool_stats(),
@@ -112,6 +115,9 @@ struct yt921x_info {
 #define YT921X_PORT_MASK_INT0_n(n)	GENMASK((n) - 1, 0)
 #define YT921X_PORT_MASK_EXT0		BIT(8)
 #define YT921X_PORT_MASK_EXT1		BIT(9)
+#define YT922X_PORT_MASK_INTm_n(m, n)	GENMASK((n), (m))
+#define YT922X_PORT_MASK_EXT0		BIT(0)
+#define YT922X_PORT_MASK_EXT1		BIT(8)
 
 static const struct yt921x_info yt921x_infos[] = {
 	{
@@ -149,9 +155,17 @@ static const struct yt921x_info yt921x_infos[] = {
 		YT921X_PORT_MASK_INT0_n(8),
 		YT921X_PORT_MASK_EXT0 | YT921X_PORT_MASK_EXT1,
 	},
+	{
+		"YT9224", YT9224_MAJOR, 0, 0,
+		YT922X_PORT_MASK_INTm_n(4, 7),
+		YT922X_PORT_MASK_EXT0 | YT922X_PORT_MASK_EXT1,
+	},
 	{}
 };
 
+/* Define top ext addr */
+#define YT922X_COMMON_EXT_PHYADDR	9
+
 #define YT921X_VID_UNWARE	4095
 
 /* The interval should be small enough to avoid overflow of 32bit MIBs.
@@ -4240,6 +4254,11 @@ static void yt921x_dsa_teardown(struct dsa_switch *ds)
 	}
 }
 
+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 = to_device(priv);
@@ -4265,6 +4284,9 @@ static int yt921x_chip_detect(struct yt921x_priv *priv)
 		return -ENODEV;
 	}
 
+	if (!yt921x_needs_extmode_check(major))
+		goto skip_extmode_check;
+
 	res = yt921x_reg_read(priv, YT921X_CHIP_MODE, &mode);
 	if (res)
 		return res;
@@ -4304,6 +4326,15 @@ static int yt921x_chip_detect(struct yt921x_priv *priv)
 
 	priv->info = info;
 
+	return 0;
+
+skip_extmode_check:
+	dev_info(dev,
+		 "Motorcomm %s ethernet switch, chipid: 0x%x\n",
+		 info->name, chipid);
+
+	priv->info = info;
+
 	return 0;
 }
 
@@ -4694,6 +4725,637 @@ 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 = 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)
+{
+	u32 mask;
+	u32 ctrl;
+	int res;
+
+	switch (speed) {
+	case SPEED_10:
+		ctrl = YT921X_PORT_SPEED_10;
+		break;
+	case SPEED_100:
+		ctrl = YT921X_PORT_SPEED_100;
+		break;
+	case SPEED_1000:
+		ctrl = YT921X_PORT_SPEED_1000;
+		break;
+	case SPEED_2500:
+		ctrl = YT921X_PORT_SPEED_2500;
+		break;
+	case SPEED_5000:
+		ctrl = YT921X_PORT_SPEED_5000;
+		break;
+	case SPEED_10000:
+		ctrl = YT921X_PORT_SPEED_10000;
+		break;
+	default:
+		return -EINVAL;
+	}
+	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 = 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 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;
+}
+
+static 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;
+}
+
+static int yt922x_sds_phyaddr_get(int port,
+				  enum yt922x_phy_reg_type reg_type)
+{
+	/*
+	 * sds phyaddr mapping depend on reg_type
+	 */
+	if (!yt922x_port_is_internal_sds(port))
+		return -EOPNOTSUPP;
+	if (reg_type == YT922X_PHY_REG_TYPE_COMMON_EXT)
+		return YT922X_COMMON_EXT_PHYADDR;
+
+	return port;
+}
+
+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 = 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 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;
+	if (!yt922x_port_is_internal_sds(port))
+		return -EINVAL;
+
+	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;
+}
+
+static const struct phylink_pcs_ops yt922x_pcs_ops = {
+	.pcs_get_state = yt922x_pcs_get_state,
+	.pcs_config = yt922x_pcs_config,
+	.pcs_an_restart = yt922x_pcs_an_restart,
+};
+
+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 = 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->external_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 = 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(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 = to_yt921x_priv(ds);
+	const struct yt921x_info *info = priv->info;
+	unsigned long mask;
+	int port;
+
+	mask = info->external_mask;
+	for_each_set_bit(port, &mask, priv->series->max_ports) {
+		struct yt921x_port *pp = &priv->ports[port];
+
+		pp->pcs.ops = &yt922x_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 = to_yt921x_priv(ds);
+	struct device *dev = to_device(priv);
+	struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int res;
+
+	/* pp index init */
+	for (size_t i = 0; i < ARRAY_SIZE(priv->ports); i++) {
+		struct yt921x_port *pp = &priv->ports[i];
+
+		pp->index = i;
+	}
+
+	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] = {
 		.mode = YT92XX_MODE_YT921X,
@@ -4707,12 +5369,26 @@ static const struct yt92xx_series yt92xx_series_table[] = {
 		.switch_ops = &yt921x_dsa_switch_ops,
 		.mac_ops = &yt921x_phylink_mac_ops
 	},
+	[YT92XX_MODE_YT922X] = {
+		.mode = 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;
 }
@@ -4825,8 +5501,9 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 }
 
 static const struct of_device_id yt921x_of_match[] = {
-	{ .compatible = "motorcomm,yt9215" },
-	{}
+	{ .compatible = "motorcomm,yt9215", },
+	{ .compatible = "motorcomm,yt9224", },
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, yt921x_of_match);
 
@@ -4843,5 +5520,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 c446aea449ed..dc8ef6dd4bd5 100644
--- a/drivers/net/dsa/motorcomm/chip.h
+++ b/drivers/net/dsa/motorcomm/chip.h
@@ -111,6 +111,7 @@
 #define   YT921X_PORT_SPEED_1000			YT921X_PORT_SPEED(2)
 #define   YT921X_PORT_SPEED_10000			YT921X_PORT_SPEED(3)
 #define   YT921X_PORT_SPEED_2500			YT921X_PORT_SPEED(4)
+#define   YT921X_PORT_SPEED_5000			YT921X_PORT_SPEED(5)
 #define YT921X_PON_STRAP_FUNC		0x80320
 #define YT921X_PON_STRAP_VAL		0x80324
 #define YT921X_PON_STRAP_CAP		0x80328
@@ -837,6 +838,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
@@ -861,6 +863,96 @@ enum yt921x_fdb_entry_status {
 #define yt921x_port_is_internal(port) ((port) < 8)
 #define yt921x_port_is_external(port) ((port) == 8 || (port) == 9)
 
+/* 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 yt922x_port_is_internal_sds(port) ((port) == 0 || (port) == 8)
+
+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_mib {
 	u64 rx_broadcast;
 	u64 rx_pause;
@@ -953,6 +1045,7 @@ struct yt921x_port {
 
 	struct yt921x_led *leds[YT921X_LED_GROUP_NUM];
 #endif
+	struct phylink_pcs pcs;
 };
 
 struct yt921x_reg_ops {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v8 1/6] net: dsa: motorcomm: initialize dsa_switch based on chipid
  2026-09-17  8:22 ` [PATCH net-next v8 1/6] net: dsa: motorcomm: initialize dsa_switch based on chipid Kyle Switch
@ 2026-09-21  9:35   ` netdev-bot+sashiko
  0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-21  9:35 UTC (permalink / raw)
  To: kyle.switch
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel, ming.xu, xiaolin.xu, jianmin.wang,
	wei.zhang, sijia.huang

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] yt92xx_register_switch() (drivers/net/dsa/motorcomm/chip.c:4705)
  adds a live MDIO transaction and a new, completely silent probe abort…

--

net: dsa: motorcomm: initialize dsa_switch based on chipid

This adds a yt92xx_series table describing per-family dsa_switch
parameters, and a new yt92xx_register_switch() helper that reads the chip
ID and fills in ds fields from the matching table entry.  The previously
hardcoded assignments in yt921x_mdio_probe() are removed.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index d663af010f431..564e45ece22cb 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c

[ ... ]

> +static int yt92xx_register_switch(struct dsa_switch *ds)
> +{
> +	struct yt921x_priv *priv = to_yt921x_priv(ds);
> +	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)
> +		return res;
> +
> +	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
> +	series = yt92xx_series_lookup(major);
> +	if (!series)
> +		return -ENODEV;

[Severity: Low]
Should this path print something when the chip ID is not recognised?

Before this patch the first register access happened later, in
yt921x_dsa_setup() -> yt921x_chip_detect(), which logs the value it read
before giving up:

drivers/net/dsa/motorcomm/chip.c:yt921x_chip_detect() {
	...
	if (!info->name) {
		dev_err(dev, "Unexpected chipid 0x%x\n", chipid);
		return -ENODEV;
	}
	...
}

Here yt92xx_register_switch() returns a bare -ENODEV when
yt92xx_series_lookup() does not match YT9215_MAJOR or YT9218_MAJOR, and a
bare bus errno when yt921x_reg_read() of YT921X_CHIP_ID fails.  Since
really_probe() only pr_debug()s an -ENODEV/-ENXIO return, a board with an
unsupported or non-responding switch would end up with no message in dmesg
explaining why the DSA switch never appeared.  Would a dev_err() with the
chipid here keep the previous diagnostics?

Related: the commit message says the change is about

    "replacing hardcoded logic in probe()"

which reads as a pure data-driven refactor.  Could it also mention that
probe() now performs an MDIO read of YT921X_CHIP_ID and can fail early
because of it?  That is a behavioural change for probe(), which previously
had no hardware dependency.

> +	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;
> +}

[ ... ]

> @@ -4754,15 +4812,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;

[ ... ]

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v8 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking
  2026-09-17  8:22 ` [PATCH net-next v8 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking Kyle Switch
@ 2026-09-21  9:35   ` netdev-bot+sashiko
  0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-21  9:35 UTC (permalink / raw)
  To: kyle.switch
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel, ming.xu, xiaolin.xu, jianmin.wang,
	wei.zhang, sijia.huang

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] yt921x_mbus_int_read()/yt921x_mbus_int_write() (chip.c:360 and
  chip.c:378) now validate internal-MDIO addresses against…

Pre-existing issues:
- [Low] Claim of an inconsistency between the runtime per-series bound
  (ds->num_ports = priv->series->max_ports at chip.c:4734, and now the…

--

net: dsa: motorcomm: use max_ports in series for port bounds checking

This patch replaces YT921X_PORT_NUM with priv->series->max_ports in the
internal MDIO bus read/write address checks and in the phy_mask
computation in yt921x_mbus_int_init().  The commit message states there
is no functional change for existing YT921X devices.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 564e45ece22cb..2156230a78aad 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -357,7 +357,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);
> @@ -375,7 +375,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;

[Severity: Low]
Is max_ports the right quantity for these two checks?  The address passed
to yt921x_mbus_int_read()/yt921x_mbus_int_write() ends up in the
YT921X_INT_MBUS_CTRL PORT field, which selects one of the integral GbE
PHYs, whereas max_ports is the total DSA port count including the
external MAC ports and the MCU port.

The driver keeps those as separate quantities:

drivers/net/dsa/motorcomm/chip.h:
	/* 8 internal + 2 external + 1 mcu */
	#define YT921X_PORT_NUM			11
	...
	#define yt921x_port_is_internal(port) ((port) < 8)
	#define yt921x_port_is_external(port) ((port) == 8 || (port) == 9)

and struct yt921x_info carries:

	/* Ports with integral GbE PHYs, not including MCU Port 10 */
	u16 internal_mask;

With max_ports == 11 the guard now accepts internal-bus transactions for
ports 8 and 9 (external MACs) and port 10 (MCU), none of which have an
integral PHY.  For a series whose integral PHYs are not the lowest port
indices, the accepted set diverges further: the YT922x entry added later
in this series sets max_ports = 9 while its integral UTP PHYs are only
ports 4..7, so ports 0..3 and 8 would also be accepted.

The effect is bounded: these callbacks only see addresses phylib has
bound from the "mdio" child node, so a DT would have to list a PHY at
such an address.  When it does, the driver runs a real INT_MBUS cycle
against a nonexistent PHY index, and yt921x_intif_read() already has a
dev_info() for the resulting unexpected read data.

Would keying the check off the integral-PHY count or mask (the
internal_mask / yt921x_port_is_internal equivalent) rather than
max_ports, which DSA consumes as ds->num_ports, match the hardware
field being programmed?

> @@ -390,6 +390,7 @@ yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp)
>  {
>  	struct device *dev = to_device(priv);
>  	struct mii_bus *mbus;
> +	u32 max_ports;
>  	int res;
>  
>  	mbus = devm_mdiobus_alloc(dev);
> @@ -402,7 +403,8 @@ 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);

[Severity: Low]
This isn't a bug introduced by this patch, but now that the runtime bound
comes from priv->series->max_ports, is it worth tying that value to the
compile-time array size?

yt92xx_register_switch() already does:

	ds->num_ports = priv->series->max_ports;

while the per-port state stays fixed size in chip.h:

	struct yt921x_port ports[YT921X_PORT_NUM];

and several helpers still loop on the old constant, for example in
yt921x_chip_setup_dsa():

	for (int i = 0; i < YT921X_PORT_NUM; i++)
		ctrl |= YT921X_ACT_UNK_ACTn_TRAP(i);

Nothing is broken today: the only two series entries are max_ports == 11
(YT921x) and max_ports == 9 (YT922x), both within ARRAY_SIZE(priv->ports),
and the remaining YT921X_PORT_NUM loops live in helpers reachable only
through yt921x_dsa_switch_ops, i.e. only when max_ports == 11.  A
BUILD_BUG_ON() relating max_ports to ARRAY_SIZE(priv->ports), or
converting those loops to max_ports, would make the invariant explicit
for the next series added.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v8 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup()
  2026-09-17  8:22 ` [PATCH net-next v8 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup() Kyle Switch
@ 2026-09-21  9:35   ` netdev-bot+sashiko
  0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-21  9:35 UTC (permalink / raw)
  To: kyle.switch
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel, ming.xu, xiaolin.xu, jianmin.wang,
	wei.zhang, sijia.huang

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

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

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] yt921x_dsa_setup() now runs INIT_DELAYED_WORK(&pp->mib_read,
  yt921x_poll_mib) for every port on every DSA .setup() invocation…
- [Medium] After this patch the per-port delayed works are initialized
  only inside yt921x_dsa_setup() (chip.c:4556-4562), while…

Pre-existing issues:
- [Medium] If dsa_register_switch() fails after yt921x_dsa_setup()
  already ran and a port's mib_read work has been scheduled…
- [Medium] The self-re-arming per-port MIB poller (yt921x_poll_mib() ->
  yt921x_read_mib(), chip.c:609-626) is never stopped by…
- [Medium] The per-port init block moved into yt921x_dsa_setup()
  (chip.c:4556-4562) sets pp->index and initializes pp->mib_read but…

--

net: dsa: motorcomm: move mib start from probe() to dsa_setup()

This moves the per-port MIB delayed_work initialization out of
yt921x_mdio_probe() and into the DSA .setup() callback, so the works are
now initialized once per .setup() instead of once per probe().

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 2156230a78aad..167b208357710 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -4553,6 +4553,14 @@ static int yt921x_dsa_setup(struct dsa_switch *ds)
>  	struct device_node *child;
>  	int res;
>  
> +	/* mib polling init */
> +	for (size_t i = 0; i < ARRAY_SIZE(priv->ports); i++) {
> +		struct yt921x_port *pp = &priv->ports[i];
> +
> +		pp->index = i;
> +		INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);
> +	}
> +

[Severity: High]
Can this re-initialize a delayed_work that is still armed or queued?

.setup() can run a second time for the same struct yt921x_priv without a
new probe().  dsa_switch_teardown() clears ds->setup, so when a peer
switch of a multi-switch tree is unbound and re-bound, the whole tree is
torn down and set up again and ds->ops->setup() is invoked once more.

The counterpart yt921x_dsa_teardown() does not touch the works at all:

	static void yt921x_dsa_teardown(struct dsa_switch *ds)
	{
		struct yt921x_priv *priv = to_yt921x_priv(ds);

	#if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
		yt921x_leds_remove(priv);
	#endif
	}

The only per-port cancellation before device removal is the
non-synchronous cancel in yt921x_phylink_mac_link_down():

	/* No need to sync; port control block is hold until device remove */
	cancel_delayed_work(&priv->ports[port].mib_read);

and the worker re-arms itself unconditionally:

	yt921x_poll_mib()
		...
		schedule_delayed_work(&pp->mib_read, delay);

So if yt921x_poll_mib() is executing when the port goes down, the cancel
is a no-op and the work is armed again.  A subsequent .setup() then runs
INIT_DELAYED_WORK() on that live work, overwriting work->data,
work->entry and dwork->timer while they are linked into the timer base or
a pool worklist.  Does that risk list corruption, lost work, and
debug-objects/lockdep splats?

The comment quoted above ("port control block is hold until device
remove") also seems to no longer hold once the init lifetime becomes
per-.setup(); should it be updated together with this change?

Note the follow-up patch in this series, "net: dsa: motorcomm: move mib
stop from remove() to teardown()", adds the disable_delayed_work_sync()
loop to yt921x_dsa_teardown(), and with that in place the works are
drained before any re-initialization.  Would it make sense to fold the
two changes together so this intermediate state does not exist for
bisect?

[Severity: Medium]
This is a pre-existing issue, but the new placement of the init loop
makes it worth asking about: can the poller read the MIB block while it
is reset?

The init block now sits immediately before:

	mutex_lock(&priv->reg_lock);
	res = yt921x_chip_reset(priv);
	mutex_unlock(&priv->reg_lock);

Since yt921x_dsa_teardown() does not stop the poller and
yt921x_phylink_mac_link_down() only uses cancel_delayed_work() against a
worker that re-arms itself, a poll can still run during a second
.setup(), in the window between yt921x_chip_reset() and
yt921x_chip_setup() re-enabling YT921X_FUNC_MIB plus writing
YT921X_MIB_CTRL_CLEAN.  priv->reg_lock serializes individual register
accesses but does not keep the poller out of that window.

yt921x_read_mib() treats any decrease of the low 32 bits as a wrap:

		val = (old_val & ~(u64)U32_MAX) | val0;
		if (val < old_val)
			val += 1ull << 32;

so one read of a reset or disabled MIB block permanently shifts the
shadow counter by 2^32 rather than producing a single bad sample.  The
follow-up patch adding disable_delayed_work_sync() to
yt921x_dsa_teardown() closes this window.

[Severity: Medium]
This is a pre-existing divergence, but this hunk moves the per-port init
onto exactly the path that clears the hardware counters: should the
software MIB shadow be reset here too?

The block sets pp->index and initializes pp->mib_read, but leaves
pp->mib, pp->rx_frames and pp->tx_frames untouched, while the same
.setup() path clears the hardware counters in yt921x_chip_setup():

	/* Clear MIB */
	ctrl = YT921X_MIB_CTRL_CLEAN | YT921X_MIB_CTRL_ALL_PORT;
	res = yt921x_reg_write(priv, YT921X_MIB_CTRL, ctrl);

yt921x_read_mib() documents the precondition:

	/* Read and handle overflow of 32bit MIBs. MIB buffer must be zeroed before. */

pp->mib is only ever zeroed by the devm_kzalloc() in yt921x_mdio_probe().
On a second .setup() for the same priv the hardware counters restart at
zero while the shadow keeps its old values, so the wrap heuristic adds
2^32 to every 32-bit counter and ethtool -S / rtnl statistics jump by
roughly 4.29e9 per counter.  This one is still present at the end of the
series.

>  	mutex_lock(&priv->reg_lock);
>  	res = yt921x_chip_reset(priv);
>  	mutex_unlock(&priv->reg_lock);
> @@ -4805,13 +4813,6 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
>  	priv->reg_ops = &yt921x_reg_ops_mdio;
>  	priv->reg_ctx = mdio;
>  
> -	for (size_t i = 0; i < ARRAY_SIZE(priv->ports); i++) {
> -		struct yt921x_port *pp = &priv->ports[i];
> -
> -		pp->index = i;
> -		INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);
> -	}
> -

[Severity: Medium]
With the init now conditional on .setup() having run, can
yt921x_mdio_remove() operate on never-initialized works?

yt921x_mdio_remove() still disables all of them unconditionally:

	for (size_t i = ARRAY_SIZE(priv->ports); i-- > 0; ) {
		struct yt921x_port *pp = &priv->ports[i];

		disable_delayed_work_sync(&pp->mib_read);
	}

dsa_register_switch() can return 0 without ever calling ds->ops->setup():

	net/dsa/dsa.c:dsa_tree_setup() {
		...
		complete = dsa_tree_setup_routing_table(dst);
		if (!complete)
			return 0;
		...
	}

That happens when a DT-declared dsa-link peer switch has not probed yet,
or never probes.  priv is devm_kzalloc()'d and drvdata is set before
dsa_register_switch(), so a later unbind or module unload reaches
yt921x_mdio_remove() with zero-filled delayed_work objects.

disable_delayed_work_sync() -> __cancel_work_sync() then calls
timer_delete() on an uninitialized timer, which trips the debug-objects
assert under CONFIG_DEBUG_OBJECTS_TIMERS, and unconditionally calls
__flush_work(work, true):

	kernel/workqueue.c:__flush_work() {
		if (WARN_ON(!work->func))
			return false;
		...
	}

so one warning per port.  The follow-up patch moves this loop into
yt921x_dsa_teardown(), which DSA only invokes when setup actually ran
(dsa_switch_teardown() returns early on !ds->setup), so this only exists
at this bisect point.

[Severity: Medium]
This isn't a bug introduced by this patch, but the probe error path looks
unprotected: does it leave a scheduled work pointing at freed memory?

yt921x_mdio_probe() ends with:

	mdiodev_set_drvdata(mdiodev, priv);

	return dsa_register_switch(ds);

If dsa_register_switch() fails after yt921x_dsa_setup() already ran and a
port's work has been armed by
yt921x_phylink_mac_link_up() -> schedule_delayed_work(&priv->ports[port].mib_read, 0),
probe returns the error, the mdio core does not call yt921x_mdio_remove(),
and devm releases the priv that embeds the delayed_work.  Port teardown
on the unwind path only calls cancel_delayed_work(), and
yt921x_poll_mib() re-arms itself, so the work is not guaranteed to be
drained and can run against freed priv/pp memory.

Placing disable_delayed_work_sync() in yt921x_dsa_teardown(), as the
follow-up patch does, also covers this, since DSA calls .teardown() on
the tree-setup error paths via dsa_tree_teardown_switches().

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v8 4/6] net: dsa: motorcomm: move mib stop from remove() to teardown()
  2026-09-17  8:22 ` [PATCH net-next v8 4/6] net: dsa: motorcomm: move mib stop from remove() to teardown() Kyle Switch
@ 2026-09-21  9:35   ` netdev-bot+sashiko
  0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-21  9:35 UTC (permalink / raw)
  To: kyle.switch
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel, ming.xu, xiaolin.xu, jianmin.wang,
	wei.zhang, sijia.huang

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] Commit message inaccuracy plus fragile use of a counting-disable
  API.

--

net: dsa: motorcomm: move mib stop from remove() to teardown()

The per-port MIB polling work is now disabled in yt921x_dsa_teardown()
instead of yt921x_mdio_remove(). The loop calling
disable_delayed_work_sync() on priv->ports[i].mib_read is moved verbatim
from the mdio remove path into the DSA teardown op.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 167b208357710..2412fe2f696f8 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -4233,6 +4233,11 @@ static void yt921x_dsa_teardown(struct dsa_switch *ds)
>  #if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
>  	yt921x_leds_remove(priv);
>  #endif
> +	for (size_t i = ARRAY_SIZE(priv->ports); i-- > 0; ) {
> +		struct yt921x_port *pp = &priv->ports[i];
> +
> +		disable_delayed_work_sync(&pp->mib_read);
> +	}
>  }

[Severity: Low]
The changelog says this change "ensures symmetry with the MIB start
operation, which is already invoked in the setup()".  Is that accurate?
The only MIB work operation in yt921x_dsa_setup() is the initialization:

	INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);

The polling is actually started from yt921x_phylink_mac_link_up():

	schedule_delayed_work(&priv->ports[port].mib_read, 0);

and re-armed by yt921x_poll_mib() itself:

	schedule_delayed_work(&pp->mib_read, delay);

So what the new teardown call pairs with looks like the work
initialization rather than a start.  Would it be worth rewording that
sentence?

A second question about the API choice here.  disable_delayed_work_sync()
is a counting disable:

kernel/workqueue.c:disable_delayed_work_sync() {
	return __cancel_work_sync(&dwork->work,
				  WORK_CANCEL_DELAYED | WORK_CANCEL_DISABLE);
}

The disable depth it leaves in work->data is only cleared by
enable_delayed_work(), and there is no enable_delayed_work() call anywhere
under drivers/net/dsa/motorcomm/.  ds->ops->teardown is repeatable —
dsa_switch_teardown() runs it on unregister and the dsa_switch_setup()
error path runs it too — and a later ->setup() can run on the same priv.

Today this still works only because yt921x_dsa_setup() re-runs
INIT_DELAYED_WORK() for every port, which resets work->data and silently
drops the accumulated disable count.  If that one-shot init is ever moved
back to probe(), which is the more usual DSA pattern, would the
schedule_delayed_work() in yt921x_phylink_mac_link_up() become a no-op
after the first teardown/setup cycle, freezing the per-port MIB counters?

Would cancel_delayed_work_sync() in teardown (or an explicit
enable_delayed_work() in setup) make the pairing symmetric without
depending on the re-init side effect?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v8 5/6] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags
  2026-09-17  8:22 ` [PATCH net-next v8 5/6] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags Kyle Switch
@ 2026-09-21  9:35   ` netdev-bot+sashiko
  0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-21  9:35 UTC (permalink / raw)
  To: kyle.switch
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel, ming.xu, xiaolin.xu, jianmin.wang,
	wei.zhang, sijia.huang

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] Claim that the new tagger has no in-tree user (dead code, wasted
  DSA_TAG_PROTO value).
- [Low] yt922x_tag_rcv() (net/dsa/tag_yt922x.c:58-96) never calls
  dsa_default_offload_fwd_mark(), unlike the sibling tagger…
- [Low] net/dsa/tag_yt922x.c declares an ingress-priority field
  `YT922X_TAG_RX_PRIO_M GENMASK(15, 13)` (line 27) that no code…

--

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

Adds a new DSA tagger for Motorcomm YT922x switches using an 8 byte
EtherType-based tag, allocating DSA_TAG_PROTO_YT922X_VALUE 36, plus the
Kconfig and Makefile entries.  A few questions below, none of them
serious.

> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 23b4b74004ed5..0b9f8a632cf22 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -227,4 +227,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

[Severity: Low]
This isn't a bug, just a note for anyone reading the patch on its own:
as of this commit nothing in the tree selects NET_DSA_TAG_YT922X and
nothing returns DSA_TAG_PROTO_YT922X.

The next patch in the series ("net: dsa: motorcomm: Add support for
Motorcomm YT922x") adds yt922x_dsa_get_tag_protocol() returning
DSA_TAG_PROTO_YT922X and adds select NET_DSA_TAG_YT922X to
drivers/net/dsa/motorcomm/Kconfig, so the tagger does get a user by
the end of the series.

[ ... ]

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

[ ... ]

> +#define YT922X_TAG_LEN	8
> +
> +/*
> + * To define the from cpu tag format 8 bytes:
> + */
> +#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)
> +#define YT922X_TAG_RX_PRIO_M	GENMASK(15, 13)

[Severity: Low]
Is YT922X_TAG_RX_PRIO_M meant to be used somewhere?  It is defined here
and never referenced anywhere else in the file, so the hardware ingress
traffic class is dropped.

yt922x_tag_rcv() reads the word that contains it but only extracts the
source port:

	rx = ntohs(tag[2]);
	port = FIELD_GET(YT922X_TAG_RX_PORT_M, rx);

The sibling tagger does propagate it in yt921x_tag_rcv():

	skb->priority = FIELD_GET(YT921X_TAG_PRIO_M, rx);

Should yt922x_tag_rcv() set skb->priority the same way, or should the
unused macro be dropped?

Related to that, the only comment describing the layout says "To define
the from cpu tag format 8 bytes", but the block mixes from-CPU and
to-CPU fields.  In particular tag[2] bit 15 is the port 0 destination
bit on transmit (YT922X_TAG_PORTMASK_0, used in yt922x_tag_xmit()) while
YT922X_TAG_RX_PRIO_M declares bits 15:13 of the same word as receive
priority.  Could the comment be extended to spell out the from-CPU and
to-CPU words separately, so it is clear which fields belong to which
direction?

[ ... ]

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

[Severity: Low]
yt922x_tag_rcv() returns without calling dsa_default_offload_fwd_mark(),
unlike yt921x_tag_rcv() which does:

	/* Already forwarded by hardware */
	dsa_default_offload_fwd_mark(skb);

This has no effect today, since dsa_default_offload_fwd_mark() computes:

	skb->offload_fwd_mark = !!(dp->bridge);

and yt922x_dsa_switch_ops in the following patch provides only
get_tag_protocol, phylink_get_caps, port_setup and setup, so
dsa_switch_bridge_join() returns -EOPNOTSUPP, dp->bridge stays NULL and
DSA falls back to software bridging.

The observation is more about the RX tag decode: it does not extract any
forward/trap ("code") field, so there is currently no way to tell a
hardware-forwarded frame from a CPU-only trap.  Is that field absent from
the YT922x to-CPU tag, or is the decode intentionally left minimal until
bridge offload ops are added?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v8 6/6] net: dsa: motorcomm: Add support for Motorcomm YT922x
  2026-09-17  8:22 ` [PATCH net-next v8 6/6] net: dsa: motorcomm: Add support for Motorcomm YT922x Kyle Switch
@ 2026-09-21  9:35   ` netdev-bot+sashiko
  0 siblings, 0 replies; 13+ messages in thread
From: netdev-bot+sashiko @ 2026-09-21  9:35 UTC (permalink / raw)
  To: kyle.switch
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, mmyangfl, horms,
	linux, netdev, linux-kernel, ming.xu, xiaolin.xu, jianmin.wang,
	wei.zhang, sijia.huang

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

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

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] yt922x_pcs_config() violates the documented phylink PCS
  contract: it ignores neg_mode, advertising and permit_pause_to_mac and…
- [High] net: devm_ leak and sysfs crash in DSA .setup callback
- [Medium] The driver advertises a new device-tree compatible
  "motorcomm,yt9224" in yt921x_of_match[], but no DT binding documents…
- [Low] drivers/net/dsa/motorcomm/Kconfig: the NET_DSA_YT921X prompt is
  updated to "Motorcomm YT9215 and YT9224 ethernet switch chip support"…
- [Low] net/dsa/tag_yt922x.c defines YT922X_TAG_RX_PRIO_M (bits 15:13 of
  the received control word) but yt922x_tag_rcv() reads that same word…

--

net: dsa: motorcomm: Add support for Motorcomm YT922x

This adds YT922x (YT9224 only for now) support to the existing Motorcomm
YT921x DSA driver: a new chip info entry, a new yt92xx series entry with
its own dsa_switch_ops and phylink_mac_ops, a PCS implementation for the
internal SerDes ports, and a new "motorcomm,yt9224" OF compatible.

A few questions below.

> diff --git a/drivers/net/dsa/motorcomm/Kconfig b/drivers/net/dsa/motorcomm/Kconfig
> index 79cdd79a1fd25..ab2b548c216f3 100644
> --- a/drivers/net/dsa/motorcomm/Kconfig
> +++ b/drivers/net/dsa/motorcomm/Kconfig
> @@ -1,7 +1,8 @@
>  # 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

[Severity: Low]
This isn't a bug, but the prompt now mentions YT9224 while the help text
right below it still says:

	  This enables support for the Motorcomm YT9215 ethernet switch
	  chip.

Should the help text mention YT9224 too?

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 2412fe2f696f8..1cf5e859d8af0 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c

[ ... ]

> @@ -4694,6 +4725,637 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {

[ ... ]

> +static void
> +yt922x_phylink_mac_config(struct phylink_config *config, unsigned int mode,
> +			  const struct phylink_link_state *state)
> +{
> +}

[ ... ]

> +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;
> +	if (!yt922x_port_is_internal_sds(port))
> +		return -EINVAL;
> +
> +	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;

[Severity: High]
Is this safe on a second .setup() call?

DSA .setup() is not a once-per-device callback: the tree is torn
down and set up again whenever the conduit goes away and comes back
(dsa_tree_teardown() / dsa_tree_setup()), while the mdio_device that
owns the devm allocations stays bound the whole time. So the devm
resources taken in yt921x_mbus_int_init() (devm_mdiobus_alloc() plus
devm_of_mdiobus_register()) are only released when the mdiodev
itself unbinds, not when the switch is torn down.

That means on the second yt922x_dsa_setup() the first internal MDIO
bus is still registered, priv->mbus_int gets overwritten (leaking
the first allocation), and the of_mdiobus_register() for the new bus
hits a duplicate sysfs name for the same bus id, which fails the
re-setup and leaves the switch half-configured.

Making it worse, yt922x_dsa_switch_ops has no .teardown at all,
unlike yt921x_dsa_switch_ops which supplies yt921x_dsa_teardown(),
so there is nothing on the yt922x path that can unwind anything
allocated or registered from .setup().

How do you want to fix this? Either register the internal bus with
the plain mdiobus_alloc()/of_mdiobus_register() and drop it from a
new yt922x .teardown callback, or move the bus registration out of
.setup() into yt921x_mdio_probe(), where the devm lifetime actually
matches the device. If the yt921x path has the same issue today,
please say so and fix both consistently rather than copying the
pattern into the new ops.

[Severity: High]
Can the SerDes ports negotiate a link with this pcs_config()?

The callback only rewrites the 3-bit SERDES_MODE field of the ext register
YT922X_PORT_SDSn. neg_mode, advertising and permit_pause_to_mac are unused,
so MII_BMCR.BMCR_ANENABLE is never set or cleared and MII_ADVERTISE is
never programmed.

The kerneldoc for pcs_config in include/linux/phylink.h states:

 * When operating in %MLO_AN_INBAND, inband should always be enabled,
 * otherwise inband should be disabled.
 ...
 * For 1000BASE-X, the advertisement should be programmed into the PCS.
 ...
 * When an autonegotiation restart is required for 802.3z modes, .pcs_config()
 * should return a positive non-zero integer (e.g. 1)

Here res is 0 on the success path, so phylink never triggers the restart:

drivers/net/phy/phylink.c:phylink_change_inband_advert() {
	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);
}

which means an ethtool advertisement change never reaches
yt922x_pcs_an_restart(), and yt922x_pcs_an_restart() itself only sets
BMCR_ANRESTART, so it cannot enable in-band AN either.

Meanwhile yt922x_pcs_get_state() hands MII_LPA to
phylink_mii_c22_pcs_decode_state() (and phylink_decode_usxgmii_word() for
USXGMII) as if it were a completed negotiation word, and
yt922x_phylink_mac_link_up() -> yt922x_port_up() then force-writes that
speed/duplex/pause into YT922X_PORTn_CTRL. Since
yt922x_phylink_mac_config() is empty, nothing else applies the interface
change either.

Is there also anything that resets or re-times the SerDes when the mode
changes between rates (SGMII 1.25G, 2500BASEX 3.125G, USXGMII 10.3125G)?

For YT9224 external_mask is BIT(0) | BIT(8), so this affects the ports that
normally carry the CPU link.

[ ... ]

> +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;
> +}

[Severity: Low]
On the tagger added earlier in this series (net/dsa/tag_yt922x.c), is
YT922X_TAG_RX_PRIO_M used anywhere? yt922x_tag_rcv() reads the same control
word only for the source port:

	rx = ntohs(tag[2]);
	port = FIELD_GET(YT922X_TAG_RX_PORT_M, rx);
	skb->dev = dsa_conduit_find_user(netdev, 0, port);

while the sibling yt921x_tag_rcv() does:

	skb->priority = FIELD_GET(YT921X_TAG_RX_PRIO_M, rx);

Was the skb->priority assignment meant to be there as well, or should the
unused macro be dropped?

[ ... ]

> @@ -4825,8 +5501,9 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
>  }
>  
>  static const struct of_device_id yt921x_of_match[] = {
> -	{ .compatible = "motorcomm,yt9215" },
> -	{}
> +	{ .compatible = "motorcomm,yt9215", },
> +	{ .compatible = "motorcomm,yt9224", },
> +	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, yt921x_of_match);

[Severity: Medium]
Should this series also update the binding? The only Motorcomm DSA schema in
the tree, Documentation/devicetree/bindings/net/dsa/motorcomm,yt921x.yaml,
still has:

properties:
  compatible:
    const: motorcomm,yt9215

so a node with compatible = "motorcomm,yt9224" is matched by the driver but
rejected by dt_binding_check/dtbs_check.

The binding prose also disagrees with the topology added here. The schema
describes "up to 8 internal GbE PHYs and up to 2 GMACs" with "PHY 0-7 are
used for Port 0-7 respectively", while the new chip info entry is:

	{
		"YT9224", YT9224_MAJOR, 0, 0,
		YT922X_PORT_MASK_INTm_n(4, 7),
		YT922X_PORT_MASK_EXT0 | YT922X_PORT_MASK_EXT1,
	},

i.e. internal PHYs on ports 4-7 and SerDes on ports 0 and 8, with
YT922X_PORT_NUM = 9 (no port 9). Following the current binding text, would a
device tree author end up describing ports the driver cannot drive?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-09-21  9:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17  8:22 [PATCH net-next v8 0/6] net: dsa: motorcomm: add support yt922x driver Kyle Switch
2026-09-17  8:22 ` [PATCH net-next v8 1/6] net: dsa: motorcomm: initialize dsa_switch based on chipid Kyle Switch
2026-09-21  9:35   ` netdev-bot+sashiko
2026-09-17  8:22 ` [PATCH net-next v8 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking Kyle Switch
2026-09-21  9:35   ` netdev-bot+sashiko
2026-09-17  8:22 ` [PATCH net-next v8 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup() Kyle Switch
2026-09-21  9:35   ` netdev-bot+sashiko
2026-09-17  8:22 ` [PATCH net-next v8 4/6] net: dsa: motorcomm: move mib stop from remove() to teardown() Kyle Switch
2026-09-21  9:35   ` netdev-bot+sashiko
2026-09-17  8:22 ` [PATCH net-next v8 5/6] net: dsa: tag_yt922x: add support for Motorcomm YT922x tags Kyle Switch
2026-09-21  9:35   ` netdev-bot+sashiko
2026-09-17  8:22 ` [PATCH net-next v8 6/6] net: dsa: motorcomm: Add support for Motorcomm YT922x Kyle Switch
2026-09-21  9:35   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®