[PATCH net-next v7 0/6] net: dsa: motorcomm: add support yt922x driver

[PATCH net-next v7 0/6] net: dsa: motorcomm: add support yt922x driver

[Kyle Switch]

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 |   1 +
 drivers/net/dsa/motorcomm/chip.c  | 979 +++++++++++++++++++++++++++++-
 drivers/net/dsa/motorcomm/chip.h  | 119 ++++
 include/net/dsa.h                 |   2 +
 net/dsa/Kconfig                   |   6 +
 net/dsa/Makefile                  |   1 +
 net/dsa/tag_yt922x.c              | 111 ++++
 7 files changed, 1191 insertions(+), 28 deletions(-)
 create mode 100644 net/dsa/tag_yt922x.c

-- 
2.25.1

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

[Kyle Switch]

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 | 71 +++++++++++++++++++++++++++-----
 drivers/net/dsa/motorcomm/chip.h | 20 +++++++++
 2 files changed, 81 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
index d663af010f43..1f9c0c67ad4a 100644
--- a/drivers/net/dsa/motorcomm/chip.c
+++ b/drivers/net/dsa/motorcomm/chip.c
@@ -4679,6 +4679,61 @@ 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;
+
+	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
+	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 +4782,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 +4810,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);
 
@@ -4770,8 +4821,8 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 }
 
 static const struct of_device_id yt921x_of_match[] = {
-	{ .compatible = "motorcomm,yt9215" },
-	{}
+	{ .compatible = "motorcomm,yt9215", },
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, yt921x_of_match);
 
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

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

[David Yang]

On Mon, Sep 14, 2026 at 6:30 PM Kyle Switch <kyle.switch@motor-comm.com> wrote:
>  static const struct of_device_id yt921x_of_match[] = {
> -       { .compatible = "motorcomm,yt9215" },
> -       {}
> +       { .compatible = "motorcomm,yt9215", },
> +       { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, yt921x_of_match);

Meaningless.

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

[netdev-bot+sashiko]

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

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

- [High] drivers/net/dsa/motorcomm/chip.c: the newly added
  yt92xx_register_switch() calls yt921x_reg_read(priv, YT921X_CHIP_ID,…

--

net: dsa: motorcomm: initialize dsa_switch based on chipid

This adds a yt92xx_series descriptor table and a new
yt92xx_register_switch() helper that reads the chip ID, looks up the
matching series and fills in the dsa_switch fields, replacing the
hardcoded assignments previously done in yt921x_mdio_probe().

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index d663af010f431..1f9c0c67ad4a6 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -4679,6 +4679,61 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
>  	.setup			= yt921x_dsa_setup,
>  };
>
> +static const struct yt92xx_series yt92xx_series_table[] = {

[ ... ]

> +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;
> +
> +	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
> +	if (res)
> +		return res;

[Severity: High]

Should this read be done with priv->reg_lock held?

yt921x_reg_read() asserts the lock in drivers/net/dsa/motorcomm/smi.c:

int yt921x_reg_read(struct yt921x_priv *priv, u32 reg, u32 *valp)
{
	lockdep_assert_held_once(&priv->reg_lock);

	return priv->reg_ops->read(priv->reg_ctx, reg, valp);
}

yt921x_mdio_probe() only calls mutex_init(&priv->reg_lock) before invoking
yt92xx_register_switch(), so the lock is never taken on this path.  With
CONFIG_PROVE_LOCKING enabled, doesn't that make every probe of this driver
emit a WARN_ON_ONCE splat (and panic when booted with panic_on_warn)?

Every other register access in the driver takes the mutex first, for
example yt921x_dsa_setup():

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

and the comment on the field in chip.h describes it as protecting the
register access:

	/* protect the access to the switch registers */
	struct mutex reg_lock;

Since probe context can sleep, would wrapping the YT921X_CHIP_ID read in
mutex_lock()/mutex_unlock(&priv->reg_lock) be the intended fix?

> +
> +	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
> +	series = yt92xx_series_lookup(major);
> +	if (!series)
> +		return -ENODEV;
> +	priv->series = series;

[ ... ]

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

[ ... ]

One more note on this: checking the tip of the series (the commit adding
support for Motorcomm YT922x), yt92xx_register_switch() still performs the
chip ID read without taking priv->reg_lock and is still called from probe,
so this does not appear to be addressed by a later patch.

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

[PATCH net-next v7 2/6] 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.

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 1f9c0c67ad4a..4479bb1c3c2b 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

Re: [PATCH net-next v7 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking

[netdev-bot+sashiko]

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

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

- [Medium] yt92xx_register_switch()
  (drivers/net/dsa/motorcomm/chip.c:4707-4737) calls…

--

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

This replaces the hardcoded YT921X_PORT_NUM in the internal MDIO bus port
checks and in the phy_mask computation with priv->series->max_ports, so the
common code no longer carries a family-specific port count.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 1f9c0c67ad4a6..4479bb1c3c2b1 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c

[ ... ]

> @@ -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: Medium]

These new users of priv->series depend on the chip-ID read that fills it in,
and that read looks like it now runs without priv->reg_lock held.

In yt92xx_register_switch():

	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
	if (res)
		return res;

	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
	series = yt92xx_series_lookup(major);

and in drivers/net/dsa/motorcomm/smi.c:

	int yt921x_reg_read(struct yt921x_priv *priv, u32 reg, u32 *valp)
	{
		lockdep_assert_held_once(&priv->reg_lock);

		return priv->reg_ops->read(priv->reg_ctx, reg, valp);
	}

Does this trip the lockdep assertion on every probe with
CONFIG_PROVE_LOCKING enabled?  The only caller is yt921x_mdio_probe(), which
does mutex_init(&priv->reg_lock) before yt92xx_register_switch(ds) but never
takes it, so the multi-transaction SMI access also runs outside the mutex
that chip.h describes as "protect the access to the switch registers".

Before this series the chip-ID read happened under the lock, via
yt921x_dsa_setup():

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

which reached yt921x_chip_detect().  The move into the unlocked probe path
came with "net: dsa: motorcomm: initialize dsa_switch based on chipid", and
the end of the series still has no mutex_lock() in
yt92xx_register_switch().

Would wrapping the yt921x_reg_read() in yt92xx_register_switch() with
mutex_lock(&priv->reg_lock)/mutex_unlock(&priv->reg_lock) be the intended
fix here?

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

[PATCH net-next v7 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup()

[Kyle Switch]

Move MIB polling start from probe() to dsa_setup().so that it can
be enabled based on the actual switch family support.

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 4479bb1c3c2b..49729db9993b 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);
@@ -4803,13 +4811,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

Re: [PATCH net-next v7 3/6] net: dsa: motorcomm: move mib start from probe() to dsa_setup()

[netdev-bot+sashiko]

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

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

- [Medium] Commit message does not describe the change: "Move MIB polling
  start from probe() to dsa_setup().so that it can be enabled based on…
- [Medium] Init/teardown asymmetry introduced by this patch: the only
  INIT_DELAYED_WORK(&pp->mib_read, ...) now runs in the DSA .setup…
- [Medium] Moving INIT_DELAYED_WORK(&pp->mib_read, ...) into
  yt921x_dsa_setup() places it on a path DSA can execute more than once…

--

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

This moves the per-port loop that sets pp->index and calls
INIT_DELAYED_WORK() for the MIB polling work out of yt921x_mdio_probe()
and into the DSA .setup callback yt921x_dsa_setup().

> Move MIB polling start from probe() to dsa_setup().so that it can
> be enabled based on the actual switch family support.

[Severity: Medium]
Does the changelog match what the patch does?

The hunk only relocates the delayed work initialization, and the new
comment even says so:

	/* mib polling init */
	for (size_t i = 0; i < ARRAY_SIZE(priv->ports); i++) {
		...
		INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);
	}

The place where polling actually starts is untouched in
yt921x_phylink_mac_link_up():

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

and the self re-arm in yt921x_poll_mib() is untouched as well:

	schedule_delayed_work(&pp->mib_read, delay);

There is also no new priv->series or priv->info test added anywhere by
this patch, so nothing is "enabled based on the actual switch family
support" yet.

Could the changelog instead describe the actual delta, namely that the
lifetime of the delayed work initialization changes from once per probe()
to once per DSA .setup()?

Also, "dsa_setup().so that" is missing a space after the period.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 4479bb1c3c2b1..49729db9993b7 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: Medium]
Can INIT_DELAYED_WORK() run here on a work item that is still armed?

DSA can call ds->ops->setup() more than once for the same priv.
dsa_switch_setup() returns early on ds->setup, but dsa_switch_teardown()
clears it:

net/dsa/dsa.c:dsa_switch_teardown() {
	...
	ds->setup = false;
}

So on a multi-switch tree, unbinding and re-binding a peer member tears
down and re-sets up the surviving switch.

In this patch yt921x_dsa_teardown() has no counterpart cancel:

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
}

and the only per-port stop is non-synchronous 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);

while yt921x_poll_mib() unconditionally re-arms itself. Re-running
INIT_DELAYED_WORK() (and thus timer_setup()) on a timer still linked in a
timer base, and resetting work->data/work->entry, would give a
debugobjects "init active timer" report, a lost cancellation, and a
polling chain touching registers of a switch that yt921x_chip_reset()
just reset a few lines below.

The following patch in the series, "net: dsa: motorcomm: move mib stop
from remove() to teardown()", adds the disable_delayed_work_sync() loop to
yt921x_dsa_teardown(), which closes this window at the end of the series.
Would swapping the order of the two patches avoid the intermediate state?

Also, does the comment in yt921x_phylink_mac_link_down() saying the port
control block is "hold until device remove" still hold once the work is
initialized per .setup() rather than per probe()?

>  	mutex_lock(&priv->reg_lock);
>  	res = yt921x_chip_reset(priv);
>  	mutex_unlock(&priv->reg_lock);
> @@ -4803,13 +4811,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 initialization gone from probe(), can yt921x_mdio_remove() call
disable_delayed_work_sync() on a work item that was never initialized?

yt921x_mdio_remove() still does this 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);
	}

priv comes from devm_kzalloc(), so if ds->ops->setup() never ran, mib_read
is all zeroes, including work->func and the timer.

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 is the multi-switch dsa,member case where the peer has not probed
yet. Unbinding that switch then reaches:

	disable_delayed_work_sync() -> __cancel_work_sync() -> __flush_work()

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

so a WARN splat on unbind, fatal under panic_on_warn, preceded by
__cancel_work()/work_grab_pending() operating on a timer that was never
passed to timer_setup().

The next patch in the series moves this loop into yt921x_dsa_teardown(),
where dsa_switch_teardown() returns early on !ds->setup, so the final
state of the series is fine. Would reordering the two patches keep each
one bisectable?

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

[PATCH net-next v7 4/6] net: dsa: motorcomm: move mib stop from remove() to teardown()

[Kyle Switch]

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 49729db9993b..3f56d60da8ea 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)
@@ -4761,12 +4766,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

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

[Kyle Switch]

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

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 0 · Low: 4

- [Low] Claim: the new DSA tag protocol DSA_TAG_PROTO_YT922X /
  CONFIG_NET_DSA_TAG_YT922X has no consumer in the tree (dead code).
- [Low] The YT922x tag layout is effectively undocumented:
  net/dsa/tag_yt922x.c:15-17 contains only the placeholder comment "/*…
- [Low] The RX priority field declared by the new tag format is never
  decoded.
- [Low] The new tagger emits and validates the uAPI constant ETH_P_YT921X
  for a YT922x tag: net/dsa/tag_yt922x.c:40 (`tag[0] =…

--

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

This adds a new DSA tagging driver for Motorcomm YT922x switches with an
8-byte tag preceded by an EtherType of 0x9988, plus the new
DSA_TAG_PROTO_YT922X protocol value, Kconfig symbol and Makefile entry.

A few questions below.

> 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:
> + */

[Severity: Low]
This isn't a bug, but could the tag layout be documented here?  The
comment above currently describes none of the fields, while the sibling
tagger for the same vendor, net/dsa/tag_yt921x.c, carries a full frame
diagram and a per-bit description of every 16-bit tag word:

net/dsa/tag_yt921x.c:
 * +----+----+-------+-----+----+---------
 * | DA | SA | TagET | Tag | ET | Payload ...
 * +----+----+-------+-----+----+---------
 *   6    6      2      6    2       N
 ...
 *   2:
 *     15b: Tx Port(s) Valid
 *     10b-0b: Tx Port(s) Mask

Without that, the reuse of tag[2] in opposite directions (transmit writes
YT922X_TAG_PORTMASK_0 = BIT(15), receive reads YT922X_TAG_RX_PORT_M =
GENMASK(5, 2)) and the split of the destination port mask across tag[1]
bits 8:0 and tag[2] bit 15 cannot be checked against the hardware by a
reviewer.

> +#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?  It is defined here but never
referenced anywhere in the file, and yt922x_tag_rcv() only extracts the
source port from tag[2] and never assigns skb->priority, so the ingress
priority reported by the switch is dropped.

The transmit path does encode skb->priority into the tag:

	ctrl = YT922X_TAG_PRIO(skb->priority) | YT922X_TAG_FORCE_DST |
	       YT922X_TAG_PORTS(ports >> 1);

and comparable taggers decode it on receive, for example
net/dsa/tag_yt921x.c:yt921x_tag_rcv():

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

Should yt922x_tag_rcv() do the same with YT922X_TAG_RX_PRIO_M?

> +
> +static struct sk_buff *
> +yt922x_tag_xmit(struct sk_buff *skb, struct net_device *netdev)
> +{

[ ... ]

> +	tag[0] = htons(ETH_P_YT921X);

[Severity: Low]
This isn't a bug, but the YT922x tagger emits and validates a uAPI
constant named for YT921x, both here and in yt922x_tag_rcv():

	if (unlikely(tag[0] != htons(ETH_P_YT921X))) {

include/uapi/linux/if_ether.h currently scopes that value to one family:

#define ETH_P_YT921X	0x9988		/* Motorcomm YT921x DSA [ NOT AN OFFICIALLY REGISTERED ID ] */

Should the header comment be broadened (something like "Motorcomm YT92xx
DSA") or a YT922x alias added, so the uAPI still describes all in-tree
users of 0x9988?  It would also help if the commit message stated that the
two chip families intentionally share the tag EtherType while using
different tag bodies.

> +	ports = dsa_xmit_port_mask(skb, netdev);
> +	/*To fill in the case where the port index is not 0 */

[ ... ]

> +static const struct dsa_device_ops yt922x_netdev_ops = {
> +	.name = YT922X_TAG_NAME,
> +	.proto = DSA_TAG_PROTO_YT922X,

[Severity: Low]
This isn't a bug, but could the commit message mention the companion
patch that consumes this protocol?

At this commit nothing requests DSA_TAG_PROTO_YT922X, and the consumer
only appears in the last patch of the series, "net: dsa: motorcomm: Add
support for Motorcomm YT922x", which 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 there is no dead code in the merged series, and the ordering is the
usual DSA practice; a note in the commit message pointing at the driver
patch would make that clear on its own.

> +	.xmit = yt922x_tag_xmit,
> +	.rcv = yt922x_tag_rcv,
> +	.needed_headroom = YT922X_TAG_LEN,
> +};

Also, in the commit message, "Add support for Motorcomm YT922x tags with
8bytes. which includes ethertype field (default to 0x9988)." reads a bit
oddly: "8bytes" is missing a space, and the sentence break before "which"
should probably be a comma.

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

[PATCH net-next v7 6/6] 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 adds basic support for a working DSA switch.

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/dsa/motorcomm/Kconfig |   1 +
 drivers/net/dsa/motorcomm/chip.c  | 874 +++++++++++++++++++++++++++++-
 drivers/net/dsa/motorcomm/chip.h  |  99 ++++
 3 files changed, 972 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/Kconfig b/drivers/net/dsa/motorcomm/Kconfig
index 79cdd79a1fd2..f690de4c7a7c 100644
--- a/drivers/net/dsa/motorcomm/Kconfig
+++ b/drivers/net/dsa/motorcomm/Kconfig
@@ -2,6 +2,7 @@
 config NET_DSA_YT921X
 	tristate "Motorcomm YT9215 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 3f56d60da8ea..d8218f9f93aa 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,17 @@ static void yt921x_dsa_teardown(struct dsa_switch *ds)
 	}
 }
 
+static bool yt921x_needs_extmode_check(u32 major)
+{
+	switch (major) {
+	case YT9215_MAJOR:
+	case YT9218_MAJOR:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int yt921x_chip_detect(struct yt921x_priv *priv)
 {
 	struct device *dev = to_device(priv);
@@ -4265,6 +4290,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 +4332,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 +4731,823 @@ 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,
+				  enum yt922x_phy_reg_space reg_space)
+{
+	int res = port;
+
+	/*
+	 * sds phyaddr mapping depend on reg_type and reg_space
+	 */
+	if (!yt922x_port_is_internal_sds(port))
+		return -EOPNOTSUPP;
+	if (reg_type == YT922X_PHY_REG_TYPE_COMMON_EXT) {
+		res = YT922X_COMMON_EXT_PHYADDR;
+		return res;
+	}
+
+	return res;
+}
+
+/*
+ * Initialize serdes configuration based on interface mode.
+ */
+static int yt922x_sds_init(struct yt921x_priv *priv, int port,
+			   phy_interface_t interface)
+{
+	int addr;
+	u16 data;
+	int res;
+
+	addr = yt922x_sds_phyaddr_get
+		(port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
+		 YT922X_PHY_REG_SPACE_SGMII);
+	if (addr < 0)
+		return -EINVAL;
+	/* write protect */
+	res = yt921x_intif_ext_write(priv, addr, 0x4be, 0xd);
+	if (res)
+		return res;
+	/* CDR */
+	if (interface == PHY_INTERFACE_MODE_100BASEX) {
+		res = yt921x_intif_ext_write(priv, addr, 0x406, 0x0);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x416, 0x3458);
+		if (res)
+			return res;
+	} else {
+		res = yt921x_intif_ext_write(priv, addr, 0x406, 0x800);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x416, 0x4558);
+		if (res)
+			return res;
+	}
+	/* PLL */
+	if (interface == PHY_INTERFACE_MODE_USXGMII) {
+		res = yt921x_intif_ext_write(priv, addr, 0x43a, 0x1006);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x43f, 0x3029);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x42a, 0xf070);
+		if (res)
+			return res;
+	} else {
+		res = yt921x_intif_ext_write(priv, addr, 0x43d, 0x207d);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x43c, 0x207d);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x43f, 0x3032);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x43a, 0x6);
+		if (res)
+			return res;
+		res = yt921x_intif_ext_write(priv, addr, 0x42a, 0xf070);
+		if (res)
+			return res;
+	}
+	/* VCO */
+	res = yt921x_intif_ext_write(priv, addr, 0x439, 0xC0);
+	if (res)
+		return res;
+	/* Vdac */
+	res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7f7f);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x491, 0x7f);
+	if (res)
+		return res;
+	/* Eye */
+	res = yt921x_intif_ext_write(priv, addr, 0x454, 0xf14);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x497, 0xa44);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x4cd, 0x0);
+	if (res)
+		return res;
+
+	res = yt921x_intif_ext_write(priv, addr, 0x4af, 0x45e3);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x48a, 0xfff);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x408, 0x7c00);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x4d6, 0x7f);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x44f, 0xff08);
+	if (res)
+		return res;
+	/* FFE */
+	res = yt921x_intif_ext_write(priv, addr, 0x48e, 0x7d00);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0xd, 0x60f);
+	if (res)
+		return res;
+	/* CTLE */
+	res = yt921x_intif_ext_write(priv, addr, 0x4b0, 0x804);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x4b1, 0x7774);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x4af, 0x45e7);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x3, 0x5603);
+	if (res)
+		return res;
+
+	msleep(20);
+	res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7fff);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7f7f);
+	if (res)
+		return res;
+	/* CTLE */
+	res = yt921x_intif_ext_write(priv, addr, 0x2000, 0x40);
+	if (res)
+		return res;
+	res = yt921x_intif_ext_write(priv, addr, 0x2000, 0x0);
+	if (res)
+		return res;
+
+	if (interface == PHY_INTERFACE_MODE_SGMII) {
+		res = yt921x_intif_ext_write(priv, addr, 0x1042, 0x48c);
+		if (res)
+			return res;
+	}
+	/* soft reset */
+	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
+				      YT922X_PHY_REG_SPACE_SGMII);
+	if (addr < 0)
+		return res;
+	res = yt921x_intif_read(priv, addr, 0x0, &data);
+	if (res)
+		return res;
+	data &= ~(1 << 15);
+	res = yt921x_intif_write(priv, addr, 0x0, data);
+	if (res)
+		return res;
+	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
+				      YT922X_PHY_REG_SPACE_USXGMII);
+	if (addr < 0)
+		return res;
+	res = yt921x_intif_read(priv, addr, 0x0, &data);
+	if (res)
+		return res;
+	data |= 1 << 15;
+	res = yt921x_intif_write(priv, addr, 0x0, data);
+	if (res)
+		return res;
+
+	return 0;
+}
+
+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_100BASEX:
+	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;
+
+	mutex_lock(&priv->reg_lock);
+	switch (state->interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_100BASEX:
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		addr = yt922x_sds_phyaddr_get
+			(port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
+			 YT922X_PHY_REG_SPACE_SGMII);
+		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:
+		addr = yt922x_sds_phyaddr_get
+			(port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
+			 YT922X_PHY_REG_SPACE_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,
+		 YT922X_PHY_REG_SPACE_SGMII);
+	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;
+	addr = yt922x_sds_phyaddr_get
+		(port, YT922X_PHY_REG_TYPE_MII,
+		 YT922X_PHY_REG_SPACE_USXGMII);
+	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 (port != 0 && port != 8)
+		return -EINVAL;
+
+	mutex_lock(&priv->reg_lock);
+	switch (interface) {
+	case PHY_INTERFACE_MODE_SGMII:
+		ctrl = YT92XX_SERDES_MODE_SGMII;
+		break;
+	case PHY_INTERFACE_MODE_100BASEX:
+		ctrl = YT92XX_SERDES_MODE_100BASEX;
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		ctrl = YT92XX_SERDES_MODE_1000BASEX;
+		break;
+	case PHY_INTERFACE_MODE_2500BASEX:
+		ctrl = YT92XX_SERDES_MODE_2500BASEX;
+		break;
+	case PHY_INTERFACE_MODE_USXGMII:
+		ctrl = YT92XX_SERDES_MODE_USXGMII;
+		break;
+	default:
+		res = -EINVAL;
+		goto err;
+	}
+	addr = yt922x_sds_phyaddr_get
+		(port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
+		 YT922X_PHY_REG_SPACE_SGMII);
+	if (addr < 0) {
+		res = addr;
+		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;
+	/* SERDES init and interface configuration */
+	res = yt922x_sds_init(priv, port, interface);
+	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_100BASEX,
+			  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 */
+	mutex_lock(&priv->reg_lock);
+	res = yt922x_cpu_port_set(priv);
+	mutex_unlock(&priv->reg_lock);
+	if (res)
+		return res;
+
+	ctrl = GENMASK(9, 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_100BASEX,
+			  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;
+
+	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 +5561,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;
 }
@@ -4824,6 +5692,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", },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, yt921x_of_match);
@@ -4841,5 +5710,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..3a8ce801aabd 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,102 @@ 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 YT92XX_SERDES_MODE_SGMII	YT922X_SERDES_MODE(0)
+#define YT92XX_SERDES_MODE_REVSGMII	YT921X_SERDES_MODE(1)
+#define YT92XX_SERDES_MODE_1000BASEX	YT921X_SERDES_MODE(2)
+#define YT92XX_SERDES_MODE_100BASEX	YT921X_SERDES_MODE(3)
+#define YT92XX_SERDES_MODE_2500BASEX	YT921X_SERDES_MODE(4)
+#define YT92XX_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
+};
+
+enum yt922x_phy_reg_space {
+	YT922X_PHY_REG_SPACE_SGMII,
+	YT922X_PHY_REG_SPACE_USXGMII,
+	YT922X_PHY_REG_SPACE_MAX
+};
+
 struct yt921x_mib {
 	u64 rx_broadcast;
 	u64 rx_pause;
@@ -953,6 +1051,7 @@ struct yt921x_port {
 
 	struct yt921x_led *leds[YT921X_LED_GROUP_NUM];
 #endif
+	struct phylink_pcs pcs;
 };
 
 struct yt921x_reg_ops {
-- 
2.25.1

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

[David Yang]

On Mon, Sep 14, 2026 at 6:30 PM Kyle Switch <kyle.switch@motor-comm.com> wrote:
> +/*
> + * Initialize serdes configuration based on interface mode.
> + */
> +static int yt922x_sds_init(struct yt921x_priv *priv, int port,
> +                          phy_interface_t interface)
> +{
> +       int addr;
> +       u16 data;
> +       int res;
> +
> +       addr = yt922x_sds_phyaddr_get
> +               (port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
> +                YT922X_PHY_REG_SPACE_SGMII);
> +       if (addr < 0)
> +               return -EINVAL;
> +       /* write protect */
> +       res = yt921x_intif_ext_write(priv, addr, 0x4be, 0xd);
> +       if (res)
> +               return res;
> +       /* CDR */
> +       if (interface == PHY_INTERFACE_MODE_100BASEX) {
> +               res = yt921x_intif_ext_write(priv, addr, 0x406, 0x0);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x416, 0x3458);
> +               if (res)
> +                       return res;
> +       } else {
> +               res = yt921x_intif_ext_write(priv, addr, 0x406, 0x800);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x416, 0x4558);
> +               if (res)
> +                       return res;
> +       }
> +       /* PLL */
> +       if (interface == PHY_INTERFACE_MODE_USXGMII) {
> +               res = yt921x_intif_ext_write(priv, addr, 0x43a, 0x1006);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43f, 0x3029);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x42a, 0xf070);
> +               if (res)
> +                       return res;
> +       } else {
> +               res = yt921x_intif_ext_write(priv, addr, 0x43d, 0x207d);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43c, 0x207d);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43f, 0x3032);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43a, 0x6);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x42a, 0xf070);
> +               if (res)
> +                       return res;
> +       }
> +       /* VCO */
> +       res = yt921x_intif_ext_write(priv, addr, 0x439, 0xC0);
> +       if (res)
> +               return res;
> +       /* Vdac */
> +       res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7f7f);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x491, 0x7f);
> +       if (res)
> +               return res;
> +       /* Eye */
> +       res = yt921x_intif_ext_write(priv, addr, 0x454, 0xf14);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x497, 0xa44);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4cd, 0x0);
> +       if (res)
> +               return res;
> +
> +       res = yt921x_intif_ext_write(priv, addr, 0x4af, 0x45e3);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x48a, 0xfff);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x408, 0x7c00);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4d6, 0x7f);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x44f, 0xff08);
> +       if (res)
> +               return res;
> +       /* FFE */
> +       res = yt921x_intif_ext_write(priv, addr, 0x48e, 0x7d00);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0xd, 0x60f);
> +       if (res)
> +               return res;
> +       /* CTLE */
> +       res = yt921x_intif_ext_write(priv, addr, 0x4b0, 0x804);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4b1, 0x7774);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4af, 0x45e7);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x3, 0x5603);
> +       if (res)
> +               return res;
> +
> +       msleep(20);
> +       res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7fff);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7f7f);
> +       if (res)
> +               return res;
> +       /* CTLE */
> +       res = yt921x_intif_ext_write(priv, addr, 0x2000, 0x40);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x2000, 0x0);
> +       if (res)
> +               return res;
> +
> +       if (interface == PHY_INTERFACE_MODE_SGMII) {
> +               res = yt921x_intif_ext_write(priv, addr, 0x1042, 0x48c);
> +               if (res)
> +                       return res;
> +       }
> +       /* soft reset */
> +       addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
> +                                     YT922X_PHY_REG_SPACE_SGMII);
> +       if (addr < 0)
> +               return res;
> +       res = yt921x_intif_read(priv, addr, 0x0, &data);
> +       if (res)
> +               return res;
> +       data &= ~(1 << 15);
> +       res = yt921x_intif_write(priv, addr, 0x0, data);
> +       if (res)
> +               return res;
> +       addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
> +                                     YT922X_PHY_REG_SPACE_USXGMII);
> +       if (addr < 0)
> +               return res;
> +       res = yt921x_intif_read(priv, addr, 0x0, &data);
> +       if (res)
> +               return res;
> +       data |= 1 << 15;
> +       res = yt921x_intif_write(priv, addr, 0x0, data);
> +       if (res)
> +               return res;
> +
> +       return 0;
> +}

You didn't read my previous review. Remove this from this series.

> +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)) {

Worth an early return or else if here, but still OK.

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

[netdev-bot+sashiko]

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

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

- [Critical] YT922x PCS callbacks recover their private structure from an
  uninitialized `pp->index`.
- [High] Four of the six YT922x serdes-mode constants are encoded into
  the wrong register field.
- [High] `yt922x_sds_phyaddr_get()` (chip.c:4901-4918) accepts an `enum
  yt922x_phy_reg_space` argument and its own comment says "sds phyaddr…
- [High] `yt922x_dsa_setup()` (chip.c:5507) takes `priv->reg_lock` at
  chip.c:5530 and calls `yt922x_chip_setup()` (chip.c:5463) with the…
- [Medium] The driver advertises `PHY_INTERFACE_MODE_100BASEX` for YT922x
  external ports (in `yt922x_dsa_phylink_get_caps()` at chip.c:5317 and…
- [Medium] `yt922x_pcs_config()` (chip.c:5217) ignores `neg_mode`,
  `advertising` and `permit_pause_to_mac`.
- [Medium] The driver adds the OF compatible "motorcomm,yt9224" to
  `yt921x_of_match[]` (chip.c:5695) but no device-tree binding documents…
- [Low] The commit message states the YT922X series "includes YT9224 and
  YT9228", but only YT9224 is implemented: a single `yt921x_infos` entry…
- [Low] `NET_DSA_YT921X` keeps the prompt "Motorcomm YT9215 ethernet
  switch chip support" and YT9215-only help text while the same patch…
- [Low] `yt922x_chip_setup_dsa()` writes a hardcoded `GENMASK(9, 0)` (10
  port bits) to `YT922X_FILTER_UNK_UCAST` at chip.c:5440, while the two…
- [Low] `yt922x_pcs_config()` commits the new serdes mode to
  `YT922X_PORT_SDSn` (chip.c:~5262) and then runs the ~40-write…

--

net: dsa: motorcomm: Add support for Motorcomm YT922x

This adds a YT922x variant to the Motorcomm DSA driver: a new yt92xx_series
entry with its own dsa_switch_ops and phylink_mac_ops, a YT9224 chip info
entry, a large block of YT922x register definitions, and a per-port embedded
phylink PCS for the serdes ports. It also registers the motorcomm,yt9224 OF
compatible and selects the YT922x tagger.

A few questions on the commit message and the new code below.

>     Add support for Motorcomm YT922X, which is series of
>     ethernet switches developed by Motorcomm Electronic
>     Technology, includes YT9224 and YT9228.
>     
>     This patch adds basic support for a working DSA switch.

[Severity: Low]
Is YT9228 actually supported here?  Only YT9224 appears in the code: one
yt921x_infos entry for YT9224_MAJOR (0x9004), one motorcomm,yt9224
compatible, and yt92xx_series_lookup() matching only YT9224_MAJOR.  A grep
of the tree finds no YT9228 identifier, so a YT9228 would be rejected in
yt921x_chip_detect() with "Unexpected chipid" or by
yt92xx_series_lookup() returning NULL.

Could the second sentence also state what "basic" covers?
yt922x_dsa_switch_ops provides only get_tag_protocol, phylink_get_caps,
port_setup and setup, and yt922x_port_setup() sets YT922X_PORT_LEARN_DIS on
every port, so bridging, VLAN, FDB and statistics are not available.

> diff --git a/drivers/net/dsa/motorcomm/Kconfig b/drivers/net/dsa/motorcomm/Kconfig
> index 79cdd79a1fd25..f690de4c7a7c5 100644
> --- a/drivers/net/dsa/motorcomm/Kconfig
> +++ b/drivers/net/dsa/motorcomm/Kconfig
> @@ -2,6 +2,7 @@
>  config NET_DSA_YT921X
>  	tristate "Motorcomm YT9215 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]
Should the prompt and help text mention YT922x too?  The same patch makes
this symbol bind motorcomm,yt9224, select NET_DSA_TAG_YT922X and set
MODULE_DESCRIPTION("Driver for Motorcomm YT921x and YT922x Switch"), but
the only user-visible description still says YT9215 only.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 3f56d60da8eab..d8218f9f93aa9 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c

[ ... ]

> @@ -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

[Severity: Critical]
yt921x_port_to_priv() recovers priv by subtracting
offsetof(struct yt921x_priv, ports[pp->index]), so it is only correct when
pp->index equals the slot the yt921x_port actually occupies.

Where is pp->index assigned on the YT922x path?  The only assignment in the
driver is in yt921x_dsa_setup():

	pp->index = i;
	INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);

yt922x_dsa_setup() has no equivalent loop, and priv is zero-allocated, so
every priv->ports[i].index stays 0 on YT922x.

yt922x_pcs_setup() then installs pp->pcs.ops for each bit of
info->external_mask, which for YT9224 is BIT(0) | BIT(8), and
yt922x_phylink_mac_select_pcs() returns &priv->ports[dp->index].pcs.  So
phylink calls the PCS ops with pp == &priv->ports[8] while pp->index == 0,
and yt921x_port_to_priv() yields (char *)priv + 8 * sizeof(struct
yt921x_port) instead of priv.

Does that mean yt922x_pcs_get_state() runs
mutex_lock(&priv->reg_lock) on bytes that are really part of
priv->ports[] (MIB counters and a delayed_work), and then calls
priv->reg_ops->read() through a function pointer loaded from that same
memory?

A second effect of the uninitialised index: int port = pp->index is 0 for
port 8 in all three PCS callbacks, so the

	if (port != 0 && port != 8)
		return -EINVAL;

check in yt922x_pcs_config() passes with the wrong port and port 0's serdes
registers are programmed.

> @@ -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,
> +	},
>  	{}
>  };

[ ... ]

> @@ -4694,18 +4731,847 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {

[ ... ]

> +static int yt922x_sds_phyaddr_get(int port,
> +				  enum yt922x_phy_reg_type reg_type,
> +				  enum yt922x_phy_reg_space reg_space)
> +{
> +	int res = port;
> +
> +	/*
> +	 * sds phyaddr mapping depend on reg_type and reg_space
> +	 */
> +	if (!yt922x_port_is_internal_sds(port))
> +		return -EOPNOTSUPP;
> +	if (reg_type == YT922X_PHY_REG_TYPE_COMMON_EXT) {
> +		res = YT922X_COMMON_EXT_PHYADDR;
> +		return res;
> +	}
> +
> +	return res;
> +}

[Severity: High]
The comment says the mapping depends on reg_type and reg_space, but
reg_space is never read: for every reg_type other than
YT922X_PHY_REG_TYPE_COMMON_EXT the function returns port.  Do the SGMII and
USXGMII register spaces then alias to the same internal MDIO address?

The closing soft reset in yt922x_sds_init() looks like it becomes
self-cancelling:

	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
				      YT922X_PHY_REG_SPACE_SGMII);
	...
	data &= ~(1 << 15);
	res = yt921x_intif_write(priv, addr, 0x0, data);
	...
	addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
				      YT922X_PHY_REG_SPACE_USXGMII);
	...
	data |= 1 << 15;
	res = yt921x_intif_write(priv, addr, 0x0, data);

Both lookups return the same addr, so bit 15 is cleared and then set on one
register and the USXGMII PCS is never touched.  The same aliasing makes
yt922x_pcs_get_state() read YT922X_PCS_LINK_CTRL/MII_LPA from the SGMII
address in the USXGMII case, and makes yt922x_pcs_an_restart() perform the
identical BMCR read-modify-write twice.

Two smaller points in the same area:

	if (addr < 0)
		return res;

res is 0 at both of these sites in yt922x_sds_init(), so a failed address
lookup reports success while silently skipping the soft reset.  Should
these return addr?

And yt922x_pcs_get_state() and yt922x_pcs_an_restart() pass addr straight
to yt921x_intif_read() without checking for the -EOPNOTSUPP that
yt922x_sds_phyaddr_get() returns for non-serdes ports.

[ ... ]

> +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;
> +
> +	mutex_lock(&priv->reg_lock);
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_100BASEX:
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		addr = yt922x_sds_phyaddr_get
> +			(port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
> +			 YT922X_PHY_REG_SPACE_SGMII);
> +		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;

[Severity: Medium]
Can a 100BASE-X link ever be reported up through this path?
phylink_mii_c22_pcs_decode_state() in drivers/net/phy/phylink.c has cases
only for 1000BASE-X, 2500BASE-X, SGMII/PSGMII/QSGMII and QUSGMII:

	default:
		state->link = false;
		break;

100BASE-X is advertised as supported by yt922x_dsa_phylink_get_caps() and
yt922x_pcs_setup(), and yt922x_phylink_mac_select_pcs() returns this PCS
for it, so the mode looks reachable but always decodes as link down.

[ ... ]

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

[Severity: Medium]
neg_mode, advertising and permit_pause_to_mac appear only in the parameter
list; the body only rewrites the serdes mode field and runs
yt922x_sds_init().  Should this callback enable or disable in-band
negotiation according to neg_mode and program the local advertisement for
the Base-X modes?

As written, yt922x_pcs_get_state() hands neg_mode to
phylink_mii_c22_pcs_decode_state(), so the read path assumes a negotiation
setup that the write path never applied.

> +	port = pp->index;
> +	if (port != 0 && port != 8)
> +		return -EINVAL;
> +
> +	mutex_lock(&priv->reg_lock);
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +		ctrl = YT92XX_SERDES_MODE_SGMII;
> +		break;
> +	case PHY_INTERFACE_MODE_100BASEX:
> +		ctrl = YT92XX_SERDES_MODE_100BASEX;
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +		ctrl = YT92XX_SERDES_MODE_1000BASEX;
> +		break;
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		ctrl = YT92XX_SERDES_MODE_2500BASEX;
> +		break;
> +	case PHY_INTERFACE_MODE_USXGMII:
> +		ctrl = YT92XX_SERDES_MODE_USXGMII;
> +		break;
> +	default:
> +		res = -EINVAL;
> +		goto err;
> +	}
> +	addr = yt922x_sds_phyaddr_get
> +		(port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
> +		 YT922X_PHY_REG_SPACE_SGMII);
> +	if (addr < 0) {
> +		res = addr;
> +		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;
> +	/* SERDES init and interface configuration */
> +	res = yt922x_sds_init(priv, port, interface);
> +	if (res)
> +		goto err;
> +	mutex_unlock(&priv->reg_lock);

[Severity: Low]
The new mode is committed to YT922X_PORT_SDSn before the roughly forty
writes of yt922x_sds_init() run.  If one of those indirect MDIO
transactions fails, the error path only unlocks and returns, leaving the
new mode selected with a mix of old and new CDR/PLL/VCO/eye/CTLE settings
and no closing PCS reset.  Is a rollback wanted here, or is the intent to
rely on the next successful pcs_config?  The YT921x paths such as
yt921x_port_up() have the same shape today.

[ ... ]

> +static int yt922x_chip_setup_dsa(struct yt921x_priv *priv)
> +{
> +	unsigned long cpu_ports_mask;
> +	u32 ctrl;
> +	int port;
> +	int res;
> +
> +	/* cpu port set */
> +	mutex_lock(&priv->reg_lock);
> +	res = yt922x_cpu_port_set(priv);
> +	mutex_unlock(&priv->reg_lock);
> +	if (res)
> +		return res;

[Severity: High]
Can this deadlock the probing thread?  yt922x_dsa_setup() already holds
priv->reg_lock across the call:

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

and yt922x_chip_setup() calls yt922x_chip_setup_dsa() directly, so this
mutex_lock() re-acquires a mutex the same task already owns.  struct mutex
is not recursive, so this looks like it hangs on the first YT9224 probe
inside dsa_register_switch(), with lockdep reporting possible recursive
locking.

The register accesses further down in this function need the outer lock
anyway (yt921x_reg_read()/yt921x_reg_write() contain
lockdep_assert_held_once(&priv->reg_lock)), so removing this inner
lock/unlock pair rather than moving the outer one seems to be what is
wanted.

> +
> +	ctrl = GENMASK(9, 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);

[Severity: Low]
The loops here size their masks with priv->series->max_ports, which is
YT922X_PORT_NUM == 9, but GENMASK(9, 0) covers ten port bits.  The YT921x
equivalent writes GENMASK(10, 0) with YT921X_PORT_NUM == 11, i.e. exactly
max_ports bits.  Is bit 9 intentional here, or should this be derived from
YT922X_PORT_NUM?

[ ... ]

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

[ ... ]

> @@ -4824,6 +5692,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", },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, yt921x_of_match);

[Severity: Medium]
Is a binding update missing for this compatible?
Documentation/devicetree/bindings/net/dsa/motorcomm,yt921x.yaml still has:

  compatible:
    const: motorcomm,yt9215

so a DTS using motorcomm,yt9224 fails dtbs_check with "'motorcomm,yt9224'
is not one of ['motorcomm,yt9215']".

The same binding also describes the port map as internal GbE PHYs on Port
0-7 with the GMACs on Port 8-9, which does not match the new YT9224 entry
(internal_mask GENMASK(7, 4), external_mask BIT(0) | BIT(8),
YT922X_PORT_NUM 9).  Should the mdio/port description be extended for
YT922x as well?

> diff --git a/drivers/net/dsa/motorcomm/chip.h b/drivers/net/dsa/motorcomm/chip.h
> index c446aea449ed5..3a8ce801aabdc 100644
> --- a/drivers/net/dsa/motorcomm/chip.h
> +++ b/drivers/net/dsa/motorcomm/chip.h

[ ... ]

> @@ -861,6 +863,102 @@ enum yt921x_fdb_entry_status {

[ ... ]

> +#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 YT92XX_SERDES_MODE_SGMII	YT922X_SERDES_MODE(0)
> +#define YT92XX_SERDES_MODE_REVSGMII	YT921X_SERDES_MODE(1)
> +#define YT92XX_SERDES_MODE_1000BASEX	YT921X_SERDES_MODE(2)
> +#define YT92XX_SERDES_MODE_100BASEX	YT921X_SERDES_MODE(3)
> +#define YT92XX_SERDES_MODE_2500BASEX	YT921X_SERDES_MODE(4)
> +#define YT92XX_SERDES_MODE_USXGMII	YT922X_SERDES_MODE(6)

[Severity: High]
Should the four middle constants use YT922X_SERDES_MODE() rather than
YT921X_SERDES_MODE()?  YT921X_SERDES_MODE(x) is
FIELD_PREP(GENMASK(9, 7), x), while the YT922x field is GENMASK(6, 4).

yt922x_pcs_config() does:

	data &= ~YT922X_SERDES_MODE_M;
	data |= ctrl;

so for 100BASE-X, 1000BASE-X and 2500BASE-X the mode field in bits 6:4 is
cleared to 0, which is SGMII, and the value ends up in bits 9:7 of
YT922X_PORT_SDSn instead.  Only SGMII (value 0) and USXGMII use the YT922x
constructor.

[ ... ]

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