[PATCH v4 0/7] net: dsa: mt7530: support EcoNet EN751221

[PATCH v4 0/7] net: dsa: mt7530: support EcoNet EN751221

[Caleb James DeLisle]

The EcoNet EN751221 has a unique switch configuration. Every chip made
with an EN751221 die has at least one on-die MT7530 that is MMIO based.
This switch has 3x 10/100 ports and 1x gigabit port.

Some popular chip packages such as the EN7513G and EN7526G have another
MT7530 implemented as a multi-chip-module that connects to port 5 of
the on-die switch and is controlled via the MDIO controller integrated
in the on-die MT7530.

In this configurations, the upstream switch is put into a "passthrough"
mode in which all traffic is forwarded between port 5 and port 6
without altering its DSA tags.

Furthermore, since the DSA core does not call mt753x_setup until all
switches are present, and the downstream switch only becomes available
when the upstream switch has configured its MDIO controller, the MDIO
controller setup on MMIO implementations must be done before
mt753x_setup, or else it will never complete.

Finally, the on-die switch and the MCM switch connect via a TRGMII link
which requires calibration. In order to calibrate this, PLL adjustment
must be possible on the on-die switch, necessitating changes to the
core_(set|clear|write|rmw) functions to support MMIO implementations.

Solve all of these issues, add the TRGMII calibration, and add support
for both switches.

Since this patchset will probably be subject to some discussion, please
consider accepting any patches that are non-controversial since it will
make future re-submissions easier.

Thanks!

Changes since v3:
* Fix "econet,en751221-gswitch" not correctly used in driver
* "support MDIO switch downstream of MMIO switch" - clearer comments
* mt7530_port_change_mtu prefer moved function to forward declaration
* dt binding "passthrough mode" documentation, clarify it is a
restricted case of switch hierarchy
* Use different dsa_switch_ops for passthrough rather than short
circuiting in each function.
v3: https://lore.kernel.org/netdev/20261028011317.1557238-1-cjd@cjdns.fr/

Changes since v2:
* The MCM switch previously identified as econet,en751221, similarly to
mediatek,mt7621, however since econet,en751221 is already used for the
SoC compatible, is it changed to econet,en751221-gswitch. The name
"gswitch" is chosen as there's apparently no standard and for EN751221
this is intuitive because it is the gigabit switch used in
implementations with a "G" in the chip name.

* Do not use a special mt7530_switch_ops for passthrough mode, instead
detect passthrough mode in each function and exit early so as to avoid
errors when trying to make a configuration switch-tree wide.

v2: https://lore.kernel.org/netdev/20260915174643.253660-1-cjd@cjdns.fr/

Changes since v1:
* Move mt7530_setup_mdio_irq out of mt7530_setup_mdio because
mt7530_setup_mdio may be called before mt753x_setup

* Do not return error in mt7988_probe if mt7530_setup_mdio fails because
doing so leaks the DSA switch registration, and in any case
mt7530_setup_mdio will be called again.

* default_taps_ondie not being used en751221_trgmii_cal_ok unused args

* Use clk_drvp / clk_drvp instead of drvp / drvn (typo)

* en751221_set_tx_drive ext values too large for fields - values taken
from reference implementation, reduced from 16 to 15.

v1: https://lore.kernel.org/netdev/20260909140346.2861572-1-cjd@cjdns.fr/

Caleb James DeLisle (7):
  net: dsa: mt7530: get ctrl phy addr using a function
  dt-bindings: net: dsa: mediatek,mt7530: describe 'passthrough'
    hierarchy
  net: dsa: mediatek: add support for passthrough mode
  net: dsa: mediatek: support PLL setup on MMIO MT7530
  net: dsa: mediatek: support MDIO switch downstream of MMIO switch
  dt-bindings: net: dsa: mediatek,mt7530: add EcoNet EN751221 switches
  net: dsa: mediatek: support EN751221 switch

 .../bindings/net/dsa/mediatek,mt7530.yaml     |  48 +-
 drivers/net/dsa/mt7530-mdio.c                 |   1 +
 drivers/net/dsa/mt7530-mmio.c                 |  17 +-
 drivers/net/dsa/mt7530.c                      | 800 ++++++++++++++++--
 drivers/net/dsa/mt7530.h                      |  46 +-
 5 files changed, 824 insertions(+), 88 deletions(-)


base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b
-- 
2.39.5

[PATCH v4 1/7] net: dsa: mt7530: get ctrl phy addr using a function

[Caleb James DeLisle]

The MDIO MT7530 has 5 integrated PHYs, of these, the first PHY has
the additional registers that are used for such things as configuring
the clock configuration for the switch. The switch is typically on
MDIO address 31 and the PHYs are 0, 1, 2, 3, and 4 with 0 being the
special one. So taking the address of mdiodev (the switch) add 1 and
modulo 31 finds the first PHY.

However there are a number of MMIO based implementations of MT7530
and on these, mdiodev is NULL though they often still have a control
PHY for the purpose. Move MT753X_CTRL_PHY_ADDR to a function which
handles the NULL condition somewhat more gracefully and allows a
place for MMIO implementations to add special case handling to
provide their control PHY address.

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/net/dsa/mt7530.c | 54 +++++++++++++++++++++-------------------
 drivers/net/dsa/mt7530.h |  2 --
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 7781a63b4e6f..d10194488bde 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -46,6 +46,16 @@ static const struct mt7530_mib_desc mt7530_mib[] = {
 	MIB_DESC(1, MT7530_PORT_MIB_RX_ARL_DROP, "RxArlDrop"),
 };
 
+static int
+mt753x_ctrl_phy_addr(struct mt7530_priv *priv)
+{
+	if (WARN_ON_ONCE(!priv->mdiodev))
+		return 0;
+
+	/* Default is 1st PHY */
+	return (priv->mdiodev->addr + 1) & (PHY_MAX_ADDR - 1);
+}
+
 static void
 mt7530_mutex_lock(struct mt7530_priv *priv)
 {
@@ -63,32 +73,30 @@ mt7530_mutex_unlock(struct mt7530_priv *priv)
 static void
 core_write(struct mt7530_priv *priv, u32 reg, u32 val)
 {
+	int ctl_phy = mt753x_ctrl_phy_addr(priv);
 	struct mii_bus *bus = priv->bus;
 	int ret;
 
 	mt7530_mutex_lock(priv);
 
 	/* Write the desired MMD Devad */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-			 MII_MMD_CTRL, MDIO_MMD_VEND2);
+	ret = bus->write(bus, ctl_phy, MII_MMD_CTRL, MDIO_MMD_VEND2);
 	if (ret < 0)
 		goto err;
 
 	/* Write the desired MMD register address */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-			 MII_MMD_DATA, reg);
+	ret = bus->write(bus, ctl_phy, MII_MMD_DATA, reg);
 	if (ret < 0)
 		goto err;
 
 	/* Select the Function : DATA with no post increment */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
+	ret = bus->write(bus, ctl_phy,
 			 MII_MMD_CTRL, MDIO_MMD_VEND2 | MII_MMD_CTRL_NOINCR);
 	if (ret < 0)
 		goto err;
 
 	/* Write the data into MMD's selected register */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-			 MII_MMD_DATA, val);
+	ret = bus->write(bus, ctl_phy, MII_MMD_DATA, val);
 err:
 	if (ret < 0)
 		dev_err(&bus->dev, "failed to write mmd register\n");
@@ -99,6 +107,7 @@ core_write(struct mt7530_priv *priv, u32 reg, u32 val)
 static void
 core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set)
 {
+	int ctl_phy = mt753x_ctrl_phy_addr(priv);
 	struct mii_bus *bus = priv->bus;
 	u32 val;
 	int ret;
@@ -106,26 +115,23 @@ core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set)
 	mt7530_mutex_lock(priv);
 
 	/* Write the desired MMD Devad */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-			 MII_MMD_CTRL, MDIO_MMD_VEND2);
+	ret = bus->write(bus, ctl_phy, MII_MMD_CTRL, MDIO_MMD_VEND2);
 	if (ret < 0)
 		goto err;
 
 	/* Write the desired MMD register address */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-			 MII_MMD_DATA, reg);
+	ret = bus->write(bus, ctl_phy, MII_MMD_DATA, reg);
 	if (ret < 0)
 		goto err;
 
 	/* Select the Function : DATA with no post increment */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
+	ret = bus->write(bus, ctl_phy,
 			 MII_MMD_CTRL, MDIO_MMD_VEND2 | MII_MMD_CTRL_NOINCR);
 	if (ret < 0)
 		goto err;
 
 	/* Read the content of the MMD's selected register */
-	ret = bus->read(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-			MII_MMD_DATA);
+	ret = bus->read(bus, ctl_phy, MII_MMD_DATA);
 	if (ret < 0)
 		goto err;
 	val = ret;
@@ -133,8 +139,7 @@ core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set)
 	val &= ~mask;
 	val |= set;
 	/* Write the data into MMD's selected register */
-	ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-			 MII_MMD_DATA, val);
+	ret = bus->write(bus, ctl_phy, MII_MMD_DATA, val);
 err:
 	if (ret < 0)
 		dev_err(&bus->dev, "failed to write mmd register\n");
@@ -2668,8 +2673,11 @@ mt7531_setup(struct dsa_switch *ds)
 {
 	struct mt7530_priv *priv = ds->priv;
 	u32 val, id;
+	int ctl_phy;
 	int ret, i;
 
+	ctl_phy = mt753x_ctrl_phy_addr(priv);
+
 	/* Reset whole chip through gpio pin or memory-mapped registers for
 	 * different type of hardware
 	 */
@@ -2737,25 +2745,21 @@ mt7531_setup(struct dsa_switch *ds)
 	 * phy_[read,write]_mmd_indirect is called, we provide our own
 	 * mt7531_ind_mmd_phy_[read,write] to complete this function.
 	 */
-	ret = mt7531_ind_c45_phy_read(priv,
-				      MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-				      MDIO_MMD_VEND2, CORE_PLL_GROUP4);
+	ret = mt7531_ind_c45_phy_read(priv, ctl_phy, MDIO_MMD_VEND2,
+				      CORE_PLL_GROUP4);
 	if (ret < 0)
 		return ret;
 
 	val = ret;
 	val |= MT7531_RG_SYSPLL_DMY2 | MT7531_PHY_PLL_BYPASS_MODE;
 	val &= ~MT7531_PHY_PLL_OFF;
-	ret = mt7531_ind_c45_phy_write(priv,
-				       MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr),
-				       MDIO_MMD_VEND2, CORE_PLL_GROUP4, val);
+	ret = mt7531_ind_c45_phy_write(priv, ctl_phy, MDIO_MMD_VEND2,
+				       CORE_PLL_GROUP4, val);
 	if (ret < 0)
 		return ret;
 
 	/* Disable EEE advertisement on the switch PHYs. */
-	for (i = MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr);
-	     i < MT753X_CTRL_PHY_ADDR(priv->mdiodev->addr) + MT7530_NUM_PHYS;
-	     i++) {
+	for (i = ctl_phy; i < ctl_phy + MT7530_NUM_PHYS; i++) {
 		mt7531_ind_c45_phy_write(priv, i, MDIO_MMD_AN, MDIO_AN_EEE_ADV,
 					 0);
 	}
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 2bbbe617b52e..3dabbc99fbbc 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -747,8 +747,6 @@ enum mt7531_xtal_fsel {
 #define  MT7531_PHY_PLL_OFF		BIT(5)
 #define  MT7531_PHY_PLL_BYPASS_MODE	BIT(4)
 
-#define MT753X_CTRL_PHY_ADDR(addr)	(((addr) + 1) & (PHY_MAX_ADDR - 1))
-
 #define CORE_PLL_GROUP5			0x404
 #define  RG_LCDDS_PCW_NCPO1_MASK	GENMASK(15, 0)
 #define  RG_LCDDS_PCW_NCPO1(x)		FIELD_PREP(RG_LCDDS_PCW_NCPO1_MASK, x)
-- 
2.39.5

[PATCH v4 2/7] dt-bindings: net: dsa: mediatek,mt7530: describe 'passthrough' hierarchy

[Caleb James DeLisle]

Due to hardware limitations, full switch hierarchies are not supported
with DSA based on the mt7530 DSA tag, however it does support a simplified
hierarchy in which one or more switches act as passthrough conduits with
only one CPU port and one DSA port active. Document this capability.

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml         | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 90b3582b7619..f98b9113ad12 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -73,6 +73,11 @@ description: |
 
       Check out example 7.
 
+  The MT7530 and MT7531 switches offer limited support for switch hierarchies,
+  particularly in the case where one or more switches have only one upstream
+  and one downstream port, and can act as blind pass-throughs to a fully
+  featured switch downstream.
+
 properties:
   compatible:
     oneOf:
-- 
2.39.5

[PATCH v4 3/7] net: dsa: mediatek: add support for passthrough mode

[Caleb James DeLisle]

For reasons outlined below, full switch hierarchies are not supported
by the MT7530, however in some cases, hardware integrators add a
second MT7530 switch to a port of the CPU-connected MT7530 and
configure the upstream switch to pass all traffic to and from the
downstream switch. Particularly, without altering the MediaTek DSA tag.
The typical reason for this is because the downstream switch is more
capable, but the upstream switch is an irremovable part of the SoC.

To enable passthrough, the main switch has its CPU port, and the port
to the second switch, both configured as MT7530_VLAN_EG_DISABLED and
MT7530_VLAN_TRANSPARENT and it uses the port matrix to control
forwarding.

Support this restricted form of switch hierarchy which is detected by
reading the DT. More complex switch hierarchies are still unsupported
but now provide a more useful error log.

As a point of information:

There exist features in the hardware which appear to enable full
two-switch hierarchies (i.e. ports on the upstream switch are also
available for use). The way that this is supposed to work is though a
PASSTHROUGH flag in the DSA tag on BIT(7). The downstream switch sets
a flag called "PT_OPTION" on the PVC register of its "CPU" port (the
port linking to the upstream switch). This causes the downstream switch
to set the PASSTHROUGH flag on all traffic exiting toward the upstream
switch. The CPU then sets the PASSTHROUGH flag on egress traffic meant
for the downstream switch.

In this configuration, the upstream switch sets MT7530_VLAN_USER, and
PORT_SPEC_TAG on the (real) CPU port (as usual), but also sets it on
the port leading to the downstream switch. The PORT_SPEC_TAG flag on
the downstream link port causes incoming DSA tags to be copied over
rather than being replaced by tags identifying the traffic as having
come from the link port. Finally, an ACL is installed which causes the
upstream switch to forward all egress traffic with the PASSTHROUGH bit
to the link port.

However for reasons unknown, the PASSTHROUGH bit is cleared by the
upstream switch as it receives ingress traffic from the downstream
switch. So by the time the traffic reaches the CPU, it no longer bears
the PASSTHROUGH bit and its origin cannot be known.

Reference code defaults to making the upstream switch a
blind-passthrough as is done in the present patch. There is however
support in the reference implementation for hierarchies where ports on
both switches are used, providing no two ports with the same number are
both active. This allows the tag protocol to guess which switch a
packet comes from. This implementation does not implement that mode
because EcoNet EN751221 hardware does not require it.

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/net/dsa/mt7530.c | 381 +++++++++++++++++++++++++++++++++------
 drivers/net/dsa/mt7530.h |   6 +
 2 files changed, 336 insertions(+), 51 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index d10194488bde..c5b3b00acb95 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -193,6 +193,11 @@ mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 	return 0;
 }
 
+static void mt7530_passthrough_port_fast_age(struct dsa_switch *ds,
+					     int port)
+{
+}
+
 static void mt7530_port_fast_age(struct dsa_switch *ds, int port)
 {
 	struct mt7530_priv *priv = ds->priv;
@@ -960,6 +965,12 @@ static void mt7530_get_eth_ctrl_stats(struct dsa_switch *ds, int port,
 			       &ctrl_stats->MACControlFramesReceived);
 }
 
+static int
+mt7530_passthrough_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
+{
+	return 0;
+}
+
 static int
 mt7530_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
 {
@@ -1286,13 +1297,81 @@ mt753x_trap_frames(struct mt7530_priv *priv)
 				   TO_CPU_FW_CPU_ONLY);
 }
 
+static int
+mt7530_passthrough_port_change_mtu(struct dsa_switch *ds, int port,
+				   int new_mtu)
+{
+	return 0;
+}
+
+static int
+mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+	struct mt7530_priv *priv = ds->priv;
+	int length;
+	u32 val;
+
+	/* When a new MTU is set, DSA always set the CPU port's MTU to the
+	 * largest MTU of the user ports. Because the switch only has a global
+	 * RX length register, only allowing CPU port here is enough.
+	 */
+	if (!dsa_is_upstream_port(ds, port))
+		return 0;
+
+	regmap_read(priv->regmap, MT7530_GMACCR, &val);
+	val &= ~MAX_RX_PKT_LEN_MASK;
+
+	/* RX length also includes Ethernet header, MTK tag, and FCS length */
+	length = new_mtu + ETH_HLEN + MTK_HDR_LEN + ETH_FCS_LEN;
+	if (length <= 1522) {
+		val |= MAX_RX_PKT_LEN_1522;
+	} else if (length <= 1536) {
+		val |= MAX_RX_PKT_LEN_1536;
+	} else if (length <= 1552) {
+		val |= MAX_RX_PKT_LEN_1552;
+	} else {
+		val &= ~MAX_RX_JUMBO_MASK;
+		val |= MAX_RX_JUMBO(DIV_ROUND_UP(length, 1024));
+		val |= MAX_RX_PKT_LEN_JUMBO;
+	}
+
+	regmap_write(priv->regmap, MT7530_GMACCR, val);
+
+	return 0;
+}
+
+/* If this switch is downstream of another switch that is in passthrough mode,
+ * the "CPU" port is actually a DSA port.
+ */
 static void
 mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
 {
 	struct mt7530_priv *priv = ds->priv;
 
-	/* Enable Mediatek header mode on the cpu port */
-	regmap_write(priv->regmap, MT7530_PVC_P(port), PORT_SPEC_TAG);
+	if (priv->is_passthrough) {
+		/* Disable parsing of the DSA tag, it will be forwarded blindly
+		 * to the downstream switch.
+		 */
+		regmap_write(priv->regmap, MT7530_PVC_P(port),
+			     VLAN_ATTR(MT7530_VLAN_TRANSPARENT) |
+			     PVC_EG_TAG(MT7530_VLAN_EG_DISABLED));
+
+		/* The port is not configured to parse DSA tags, so they are
+		 * liable to be confused for length fields, so length check is
+		 * disabled.
+		 */
+		regmap_clear_bits(priv->regmap, MT753X_AGC, AGC_L2LEN_CHK);
+
+		/* In passthrough mode, MTU is only enforced downstream */
+		mt7530_port_change_mtu(ds, port, MT7530_MAX_MTU);
+
+		/* Loop detection has no value in passthrough mode */
+		regmap_set_bits(priv->regmap, MT753X_MTRAP,
+				MT7530_LOOP_DET_DISABLE);
+	} else {
+		/* Not passthrough, enable DSA tag handling on CPU port. */
+		regmap_write(priv->regmap, MT7530_PVC_P(port), PORT_SPEC_TAG);
+	}
 
 	/* Enable flooding on the CPU port */
 	regmap_set_bits(priv->regmap, MT753X_MFC,
@@ -1322,6 +1401,7 @@ static int
 mt7530_port_enable(struct dsa_switch *ds, int port,
 		   struct phy_device *phy)
 {
+	int upstream_pt = dsa_switch_upstream_port(ds);
 	struct dsa_port *dp = dsa_to_port(ds, port);
 	struct mt7530_priv *priv = ds->priv;
 
@@ -1332,13 +1412,38 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
 	 * bridge.
 	 */
 	if (dsa_port_is_user(dp)) {
-		struct dsa_port *cpu_dp = dp->cpu_dp;
+		priv->ports[port].pm |= PCR_MATRIX(BIT(upstream_pt));
 
-		priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));
+	} else if (dsa_port_is_dsa(dp) && dp->index != upstream_pt) {
+		priv->ports[port].pm |= PCR_MATRIX(BIT(upstream_pt));
+
+		/* Should not happen */
+		WARN_ON_ONCE(!priv->is_passthrough);
+
+		/* We are passing through to a downstream switch so we set both
+		 * CPU and downstream link to pass traffic untouched so that
+		 * the STAG from the downstream switch will pass to the upstream.
+		 */
+		regmap_write(priv->regmap, MT7530_PVC_P(port),
+			     VLAN_ATTR(MT7530_VLAN_TRANSPARENT) |
+			     PVC_EG_TAG(MT7530_VLAN_EG_DISABLED));
+
+		/* We let the downstream switch flood */
+		regmap_set_bits(priv->regmap, MT753X_MFC, BC_FFP(BIT(port)) |
+				UNM_FFP(BIT(port)) | UNU_FFP(BIT(port)));
+
+		/* Make the upstream port able to connect to the DSA port.
+		 * This must be explicit because PORT_SPEC_TAG is unset.
+		 */
+		regmap_write(priv->regmap, MT7530_PCR_P(upstream_pt),
+			     PCR_MATRIX(BIT(port)));
 	}
 	priv->ports[port].enable = true;
-	regmap_update_bits(priv->regmap, MT7530_PCR_P(port), PCR_MATRIX_MASK,
-			   priv->ports[port].pm);
+
+	/* In passthrough mode, CPU port mask is set above. */
+	if (!(priv->is_passthrough && dp->index == upstream_pt))
+		regmap_update_bits(priv->regmap, MT7530_PCR_P(port),
+				   PCR_MATRIX_MASK, priv->ports[port].pm);
 
 	mutex_unlock(&priv->reg_mutex);
 
@@ -1380,45 +1485,14 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
 }
 
 static int
-mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+mt7530_port_max_mtu(struct dsa_switch *ds, int port)
 {
-	struct mt7530_priv *priv = ds->priv;
-	int length;
-	u32 val;
-
-	/* When a new MTU is set, DSA always set the CPU port's MTU to the
-	 * largest MTU of the user ports. Because the switch only has a global
-	 * RX length register, only allowing CPU port here is enough.
-	 */
-	if (!dsa_is_cpu_port(ds, port))
-		return 0;
-
-	regmap_read(priv->regmap, MT7530_GMACCR, &val);
-	val &= ~MAX_RX_PKT_LEN_MASK;
-
-	/* RX length also includes Ethernet header, MTK tag, and FCS length */
-	length = new_mtu + ETH_HLEN + MTK_HDR_LEN + ETH_FCS_LEN;
-	if (length <= 1522) {
-		val |= MAX_RX_PKT_LEN_1522;
-	} else if (length <= 1536) {
-		val |= MAX_RX_PKT_LEN_1536;
-	} else if (length <= 1552) {
-		val |= MAX_RX_PKT_LEN_1552;
-	} else {
-		val &= ~MAX_RX_JUMBO_MASK;
-		val |= MAX_RX_JUMBO(DIV_ROUND_UP(length, 1024));
-		val |= MAX_RX_PKT_LEN_JUMBO;
-	}
-
-	regmap_write(priv->regmap, MT7530_GMACCR, val);
-
-	return 0;
+	return MT7530_MAX_MTU;
 }
 
-static int
-mt7530_port_max_mtu(struct dsa_switch *ds, int port)
+static void
+mt7530_passthrough_stp_state_set(struct dsa_switch *ds, int port, u8 state)
 {
-	return MT7530_MAX_MTU;
 }
 
 static void
@@ -1514,6 +1588,14 @@ mt7530_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt7530_passthrough_port_bridge_flags(struct dsa_switch *ds, int port,
+				     struct switchdev_brport_flags flags,
+				     struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
 static int
 mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
 			 struct switchdev_brport_flags flags,
@@ -1554,6 +1636,15 @@ mt7530_port_bridge_flags(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt7530_passthrough_port_bridge_join(struct dsa_switch *ds, int port,
+				    struct dsa_bridge bridge,
+				    bool *tx_fwd_offload,
+				    struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
 static int
 mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 			struct dsa_bridge bridge, bool *tx_fwd_offload,
@@ -1708,6 +1799,12 @@ mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
 	}
 }
 
+static void
+mt7530_passthrough_port_bridge_leave(struct dsa_switch *ds, int port,
+				     struct dsa_bridge bridge)
+{
+}
+
 static void
 mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 			 struct dsa_bridge bridge)
@@ -1728,6 +1825,14 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 	mutex_unlock(&priv->reg_mutex);
 }
 
+static int
+mt7530_passthrough_port_fdb_add(struct dsa_switch *ds, int port,
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db)
+{
+	return 0;
+}
+
 static int
 mt7530_port_fdb_add(struct dsa_switch *ds, int port,
 		    const unsigned char *addr, u16 vid,
@@ -1745,6 +1850,14 @@ mt7530_port_fdb_add(struct dsa_switch *ds, int port,
 	return ret;
 }
 
+static int
+mt7530_passthrough_port_fdb_del(struct dsa_switch *ds, int port,
+				const unsigned char *addr, u16 vid,
+				struct dsa_db db)
+{
+	return 0;
+}
+
 static int
 mt7530_port_fdb_del(struct dsa_switch *ds, int port,
 		    const unsigned char *addr, u16 vid,
@@ -1797,6 +1910,14 @@ mt7530_port_fdb_dump(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt7530_passthrough_port_mdb_add(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db)
+{
+	return 0;
+}
+
 static int
 mt7530_port_mdb_add(struct dsa_switch *ds, int port,
 		    const struct switchdev_obj_port_mdb *mdb,
@@ -1826,6 +1947,14 @@ mt7530_port_mdb_add(struct dsa_switch *ds, int port,
 	return ret;
 }
 
+static int
+mt7530_passthrough_port_mdb_del(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_mdb *mdb,
+				struct dsa_db db)
+{
+	return 0;
+}
+
 static int
 mt7530_port_mdb_del(struct dsa_switch *ds, int port,
 		    const struct switchdev_obj_port_mdb *mdb,
@@ -1856,6 +1985,14 @@ mt7530_port_mdb_del(struct dsa_switch *ds, int port,
 	return ret;
 }
 
+static int
+mt7530_passthrough_port_vlan_filtering(struct dsa_switch *ds, int port,
+				       bool vlan_filtering,
+				       struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
 static int
 mt7530_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
 			   struct netlink_ext_ack *extack)
@@ -1960,6 +2097,14 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid,
 	mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, vid);
 }
 
+static int
+mt7530_passthrough_port_vlan_add(struct dsa_switch *ds, int port,
+				 const struct switchdev_obj_port_vlan *vlan,
+				 struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
 static int
 mt7530_port_vlan_add(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan,
@@ -2015,6 +2160,13 @@ mt7530_port_vlan_add(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt7530_passthrough_port_vlan_del(struct dsa_switch *ds, int port,
+				 const struct switchdev_obj_port_vlan *vlan)
+{
+	return 0;
+}
+
 static int
 mt7530_port_vlan_del(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan)
@@ -2055,6 +2207,14 @@ mt7530_port_vlan_del(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt753x_passthrough_port_mirror_add(struct dsa_switch *ds, int port,
+				   struct dsa_mall_mirror_tc_entry *mirror,
+				   bool ingress, struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
 static int mt753x_port_mirror_add(struct dsa_switch *ds, int port,
 				  struct dsa_mall_mirror_tc_entry *mirror,
 				  bool ingress, struct netlink_ext_ack *extack)
@@ -2093,6 +2253,11 @@ static int mt753x_port_mirror_add(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static void mt753x_passthrough_port_mirror_del(struct dsa_switch *ds, int port,
+					       struct dsa_mall_mirror_tc_entry *mirror)
+{
+}
+
 static void mt753x_port_mirror_del(struct dsa_switch *ds, int port,
 				   struct dsa_mall_mirror_tc_entry *mirror)
 {
@@ -2405,11 +2570,6 @@ mt7530_setup(struct dsa_switch *ds)
 		break;
 	}
 
-	if (!dn) {
-		dev_err(ds->dev, "parent OF node of DSA conduit not found");
-		return -EINVAL;
-	}
-
 	ds->assisted_learning_on_cpu_port = true;
 	ds->untag_vlan_aware_bridge_pvid = true;
 	ds->mtu_enforcement_ingress = true;
@@ -2511,7 +2671,7 @@ mt7530_setup(struct dsa_switch *ds)
 		/* Disable learning by default on all ports */
 		regmap_set_bits(priv->regmap, MT7530_PSC_P(i), SA_DIS);
 
-		if (dsa_is_cpu_port(ds, i)) {
+		if (dsa_is_upstream_port(ds, i)) {
 			mt753x_cpu_port_enable(ds, i);
 		} else {
 			mt7530_port_disable(ds, i);
@@ -2535,7 +2695,7 @@ mt7530_setup(struct dsa_switch *ds)
 		return ret;
 
 	/* Check for PHY muxing on port 5 */
-	if (dsa_is_unused_port(ds, 5)) {
+	if (dn && dsa_is_unused_port(ds, 5)) {
 		/* Scan the ethernet nodes. Look for GMAC1, lookup the used PHY.
 		 * Set priv->p5_mode to the appropriate value if PHY muxing is
 		 * detected.
@@ -2635,7 +2795,7 @@ mt7531_setup_common(struct dsa_switch *ds)
 		regmap_set_bits(priv->regmap, MT7531_DBG_CNT(i),
 				MT7531_DIS_CLR);
 
-		if (dsa_is_cpu_port(ds, i)) {
+		if (dsa_is_upstream_port(ds, i)) {
 			mt753x_cpu_port_enable(ds, i);
 		} else {
 			mt7530_port_disable(ds, i);
@@ -3255,6 +3415,14 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt753x_passthrough_port_change_conduit(struct dsa_switch *ds, int port,
+				       struct net_device *conduit,
+				       struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
 static int
 mt753x_port_change_conduit(struct dsa_switch *ds, int port,
 			   struct net_device *conduit,
@@ -3304,7 +3472,7 @@ mt753x_conduit_state_change(struct dsa_switch *ds,
 
 	/* Set the CPU port to trap frames to for MT7530. Trapped frames will be
 	 * forwarded to the numerically smallest CPU port whose conduit
-	 * interface is up.
+	 * interface is up. NOTE: "CPU port" can also mean an upstream DSA link.
 	 */
 	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
 	    priv->id != ID_EN7528)
@@ -3362,6 +3530,13 @@ static int mt753x_tc_setup_qdisc_tbf(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int mt753x_passthrough_setup_tc(struct dsa_switch *ds, int port,
+				       enum tc_setup_type type,
+				       void *type_data)
+{
+	return 0;
+}
+
 static int mt753x_setup_tc(struct dsa_switch *ds, int port,
 			   enum tc_setup_type type, void *type_data)
 {
@@ -3399,6 +3574,99 @@ static int mt7988_setup(struct dsa_switch *ds)
 	return mt7531_setup_common(ds);
 }
 
+/* 1 if passthrough, negative if error. */
+static int mt753x_check_passthrough(struct device *dev)
+{
+	struct device_node *ports, *port;
+	int passthrough_ports = 0;
+	int enabled_ports = 0;
+
+	ports = of_get_child_by_name(dev->of_node, "ports");
+	if (!ports)
+		ports = of_get_child_by_name(dev->of_node, "ethernet-ports");
+
+	if (!ports) {
+		dev_err(dev, "no ports child node found\n");
+		return -EINVAL;
+	}
+
+	for_each_available_child_of_node(ports, port) {
+		struct device_node *link;
+
+		enabled_ports++;
+
+		link = of_parse_phandle(port, "ethernet", 0);
+		if (!link)
+			link = of_parse_phandle(port, "link", 0);
+
+		if (!link)
+			continue;
+
+		of_node_put(link);
+
+		passthrough_ports++;
+	}
+
+	of_node_put(ports);
+
+	/*
+	 * A switch is considered passthrough if exactly two available
+	 * ports have an "ethernet" or "link" phandle.
+	 */
+	if (passthrough_ports > 2 ||
+	    (passthrough_ports == 2 && enabled_ports != 2)
+	) {
+		dev_err(dev, "Only two ports allowed in passthrough mode\n");
+		return -EINVAL;
+	}
+
+	return passthrough_ports == 2;
+}
+
+/* No manipulation of forwarding rules allowed in passthrough mode */
+static const struct dsa_switch_ops mt7530_passthrough_switch_ops = {
+	.get_tag_protocol       = mtk_get_tag_protocol,
+	.setup                  = mt753x_setup,
+	.teardown               = mt753x_teardown,
+	.preferred_default_local_cpu_port = mt753x_preferred_default_local_cpu_port,
+	.port_change_conduit	= mt753x_passthrough_port_change_conduit,
+	.get_strings            = mt7530_get_strings,
+	.get_ethtool_stats      = mt7530_get_ethtool_stats,
+	.get_sset_count         = mt7530_get_sset_count,
+	.get_eth_mac_stats      = mt7530_get_eth_mac_stats,
+	.get_rmon_stats         = mt7530_get_rmon_stats,
+	.get_eth_ctrl_stats     = mt7530_get_eth_ctrl_stats,
+	.get_stats64            = mt7530_get_stats64,
+	.set_ageing_time	= mt7530_passthrough_set_ageing_time,
+	.port_enable            = mt7530_port_enable,
+	.port_disable           = mt7530_port_disable,
+	.port_change_mtu	= mt7530_passthrough_port_change_mtu,
+	.port_max_mtu		= mt7530_port_max_mtu,
+	.port_stp_state_set	= mt7530_passthrough_stp_state_set,
+	.port_pre_bridge_flags	= mt7530_port_pre_bridge_flags,
+	.port_bridge_flags	= mt7530_passthrough_port_bridge_flags,
+	.port_bridge_join	= mt7530_passthrough_port_bridge_join,
+	.port_bridge_leave	= mt7530_passthrough_port_bridge_leave,
+	.port_fast_age		= mt7530_passthrough_port_fast_age,
+	.port_fdb_add		= mt7530_passthrough_port_fdb_add,
+	.port_fdb_del		= mt7530_passthrough_port_fdb_del,
+	.port_fdb_dump		= mt7530_port_fdb_dump,
+	.port_mdb_add		= mt7530_passthrough_port_mdb_add,
+	.port_mdb_del		= mt7530_passthrough_port_mdb_del,
+	.port_vlan_filtering	= mt7530_passthrough_port_vlan_filtering,
+	.port_vlan_add		= mt7530_passthrough_port_vlan_add,
+	.port_vlan_del		= mt7530_passthrough_port_vlan_del,
+	.port_mirror_add	= mt753x_passthrough_port_mirror_add,
+	.port_mirror_del	= mt753x_passthrough_port_mirror_del,
+	.phylink_get_caps       = mt753x_phylink_get_caps,
+	.support_eee            = dsa_supports_eee,
+	.set_mac_eee            = mt753x_set_mac_eee,
+	.conduit_state_change   = mt753x_conduit_state_change,
+	.port_setup_tc		= mt753x_passthrough_setup_tc,
+	.port_hsr_join		= dsa_port_simple_hsr_join,
+	.port_hsr_leave		= dsa_port_simple_hsr_leave,
+};
+
 static const struct dsa_switch_ops mt7530_switch_ops = {
 	.get_tag_protocol	= mtk_get_tag_protocol,
 	.setup			= mt753x_setup,
@@ -3531,8 +3799,12 @@ EXPORT_SYMBOL_GPL(mt753x_table);
 int
 mt7530_probe_common(struct mt7530_priv *priv)
 {
+	int passthrough = mt753x_check_passthrough(priv->dev);
 	struct device *dev = priv->dev;
 
+	if (passthrough < 0)
+		return passthrough;
+
 	priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
 	if (!priv->ds)
 		return -ENOMEM;
@@ -3550,7 +3822,14 @@ mt7530_probe_common(struct mt7530_priv *priv)
 	priv->id = priv->info->id;
 	priv->dev = dev;
 	priv->ds->priv = priv;
-	priv->ds->ops = &mt7530_switch_ops;
+
+	priv->is_passthrough = passthrough;
+
+	if (passthrough)
+		priv->ds->ops = &mt7530_passthrough_switch_ops;
+	else
+		priv->ds->ops = &mt7530_switch_ops;
+
 	priv->ds->phylink_mac_ops = &mt753x_phylink_mac_ops;
 	mutex_init(&priv->reg_mutex);
 	spin_lock_init(&priv->stats_lock);
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 3dabbc99fbbc..108c831dd012 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -40,6 +40,7 @@ enum mt753x_id {
 /* Register for ARL global control */
 #define MT753X_AGC			0xc
 #define  LOCAL_EN			BIT(7)
+#define  AGC_L2LEN_CHK			BIT(4)
 
 /* Register for MAC forward control */
 #define MT753X_MFC			0x10
@@ -583,6 +584,7 @@ enum mt7531_clk_skew {
 #define MT753X_MTRAP			0x7804
 #define  MT7530_P5_PHY0_SEL		BIT(20)
 #define  MT7530_CHG_TRAP		BIT(16)
+#define  MT7530_LOOP_DET_DISABLE	BIT(14)
 #define  MT7530_P5_MAC_SEL		BIT(13)
 #define  MT7530_P6_DIS			BIT(8)
 #define  MT7530_P5_RGMII_MODE		BIT(7)
@@ -897,6 +899,9 @@ struct mt753x_info {
  * @p5_mode:		Holding the current mode of port 5 of the MT7530 switch
  * @p5_sgmii:		Flag for distinguishing if port 5 of the MT7531 switch
  *			has got SGMII
+ * @is_passthrough:	If this switch is to pass traffic between the upstream
+ *			(CPU) port and a second downstream switch, without
+ *			altering the DSA tag.
  * @irq_domain:		IRQ domain of the switch irq_chip
  * @create_sgmii:	Pointer to function creating SGMII PCS instance(s)
  * @active_cpu_ports:	Holding the active CPU ports
@@ -921,6 +926,7 @@ struct mt7530_priv {
 	bool			p5_sgmii;
 	u8			mirror_rx;
 	u8			mirror_tx;
+	bool			is_passthrough;
 	struct mt7530_port	ports[MT7530_NUM_PORTS];
 	struct mt753x_pcs	pcs[MT7530_NUM_PORTS];
 	/* protect among processes for registers access*/
-- 
2.39.5

[PATCH v4 4/7] net: dsa: mediatek: support PLL setup on MMIO MT7530

[Caleb James DeLisle]

On MT7530 and MT7531, PHYs are an integral part of the switch. However,
on MT7530, certain configuration such as PLL setup is done via special
registers on one of the PHYs.

In an MMIO implementation, the MDIO bus is part of the switch itself.
This bus is already setup for PHY calibration so begin using it for
core switch PLL setup.

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/net/dsa/mt7530.c | 15 +++++++++++++++
 drivers/net/dsa/mt7530.h |  4 ++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c5b3b00acb95..d45388e2efc8 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -77,6 +77,12 @@ core_write(struct mt7530_priv *priv, u32 reg, u32 val)
 	struct mii_bus *bus = priv->bus;
 	int ret;
 
+	if (!bus)
+		bus = priv->child_bus;
+
+	if (WARN_ON_ONCE(!bus))
+		return;
+
 	mt7530_mutex_lock(priv);
 
 	/* Write the desired MMD Devad */
@@ -112,6 +118,12 @@ core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set)
 	u32 val;
 	int ret;
 
+	if (!bus)
+		bus = priv->child_bus;
+
+	if (WARN_ON_ONCE(!bus))
+		return;
+
 	mt7530_mutex_lock(priv);
 
 	/* Write the desired MMD Devad */
@@ -2534,8 +2546,11 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
 	if (priv->irq_domain && !mnp)
 		mt7530_setup_mdio_irq(priv);
 
+	priv->child_bus = bus;
+
 	ret = devm_of_mdiobus_register(dev, bus, mnp);
 	if (ret) {
+		priv->child_bus = NULL;
 		dev_err(dev, "failed to register MDIO bus: %d\n", ret);
 		if (priv->irq_domain && !mnp)
 			mt7530_free_mdio_irq(priv);
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 108c831dd012..5b7394f6383c 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -886,6 +886,9 @@ struct mt753x_info {
  * @dev:		The device pointer
  * @ds:			The pointer to the dsa core structure
  * @bus:		The bus used for the device and built-in PHY
+ * @child_bus:		The bus created by the device, on MDIO implementations
+ *			this is bus indirectly accesses `bus`, on MMIO
+ *			implementations this bus is part of the switch.
  * @regmap:		The regmap instance representing all switch registers
  * @rstc:		The pointer to reset control used by MCM
  * @core_pwr:		The power supplied into the core
@@ -914,6 +917,7 @@ struct mt7530_priv {
 	struct device		*dev;
 	struct dsa_switch	*ds;
 	struct mii_bus		*bus;
+	struct mii_bus		*child_bus;
 	struct regmap		*regmap;
 	struct reset_control	*rstc;
 	struct regulator	*core_pwr;
-- 
2.39.5

[PATCH v4 5/7] net: dsa: mediatek: support MDIO switch downstream of MMIO switch

[Caleb James DeLisle]

The MT7530 / MT7531 can connect via either MDIO or MMIO. In MMIO mode
the switch provides its own MDIO bus which connects to its integrated
PHYs, but also can be connected to other things, such as another
switch.

If the MDIO bus contains another switch, then the upstream switch
will register but it will not setup the DSA tree because the
downstream switch is missing. Without the complete DSA tree,
mt753x_setup() will not be called on the switch and therefore
mt7530_setup_mdio() is never called so the downstream switch never
registers.

However, the call to mt7530_setup_mdio() from mt753x_setup() must
also be kept because all PHYs are expected to be operational after
mt753x_setup() returns.

Update mt7530_setup_mdio() so that it can be called multiple times,
export the symbol, and call it again after dsa_register_switch()
from mt7988_probe().

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/net/dsa/mt7530-mmio.c | 16 +++++++++++++++-
 drivers/net/dsa/mt7530.c      | 17 +++++++++++++----
 drivers/net/dsa/mt7530.h      | 10 ++++++++++
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/mt7530-mmio.c b/drivers/net/dsa/mt7530-mmio.c
index cf3bdb23a2d5..746a0a462249 100644
--- a/drivers/net/dsa/mt7530-mmio.c
+++ b/drivers/net/dsa/mt7530-mmio.c
@@ -61,7 +61,21 @@ mt7988_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	return dsa_register_switch(priv->ds);
+	ret = dsa_register_switch(priv->ds);
+
+	if (ret)
+		return ret;
+
+	/*
+	 * If not all switches in the hierarchy have been probed,
+	 * dsa_register_switch() returns 0 without calling mt753x_setup().
+	 * Call mt7530_setup_mdio() because there's a chance that the missing
+	 * switch is on the MDIO bus. Do not call this before
+	 * dsa_register_switch() as it  may exit with an error.
+	 */
+	mt7530_setup_mdio(priv);
+
+	return 0;
 }
 
 static void mt7988_remove(struct platform_device *pdev)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index d45388e2efc8..24e4485624d3 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2509,7 +2509,7 @@ mt7530_free_mdio_irq(struct mt7530_priv *priv)
 	}
 }
 
-static int
+int
 mt7530_setup_mdio(struct mt7530_priv *priv)
 {
 	struct device_node *mnp, *np = priv->dev->of_node;
@@ -2519,6 +2519,10 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
 	static int idx;
 	int ret = 0;
 
+	/* Already done */
+	if (priv->child_bus)
+		return 0;
+
 	mnp = of_get_child_by_name(np, "mdio");
 
 	if (mnp && !of_device_is_available(mnp))
@@ -2543,9 +2547,6 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
 	bus->parent = dev;
 	bus->phy_mask = ~ds->phys_mii_mask;
 
-	if (priv->irq_domain && !mnp)
-		mt7530_setup_mdio_irq(priv);
-
 	priv->child_bus = bus;
 
 	ret = devm_of_mdiobus_register(dev, bus, mnp);
@@ -2560,6 +2561,7 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
 	of_node_put(mnp);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(mt7530_setup_mdio);
 
 static int
 mt7530_setup(struct dsa_switch *ds)
@@ -3386,10 +3388,17 @@ mt753x_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	/*
+	 * This must be called before mt753x_setup() returns because the DSA
+	 * core synchronously sets up ports and expects PHYs to be operational.
+	 */
 	ret = mt7530_setup_mdio(priv);
 	if (ret)
 		return ret;
 
+	if (priv->irq_domain && ds->user_mii_bus)
+		mt7530_setup_mdio_irq(priv);
+
 	/* Initialise the PCS devices */
 	for (i = 0; i < priv->ds->num_ports; i++) {
 		priv->pcs[i].pcs.ops = priv->info->pcs_ops;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 5b7394f6383c..c86bc4bca29b 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -969,6 +969,16 @@ struct mt7530_hw_stats {
 int mt7530_probe_common(struct mt7530_priv *priv);
 void mt7530_remove_common(struct mt7530_priv *priv);
 
+/*
+ * When an MMIO based switch has an MDIO bus which contains a downstream MDIO
+ * switch, the MDIO bus is not normally setup until mt753x_setup() has been
+ * called. But mt753x_setup() is not called until DSA sees that every switch in
+ * the tree has been probed. Calling mt7530_setup_mdio() after at the end of
+ * the MMIO probe function allows the downstream switch to register, completing
+ * the tree.
+ */
+int mt7530_setup_mdio(struct mt7530_priv *priv);
+
 extern const struct mt753x_info mt753x_table[];
 
 #endif /* __MT7530_H */
-- 
2.39.5

[PATCH v4 6/7] dt-bindings: net: dsa: mediatek,mt7530: add EcoNet EN751221 switches

[Caleb James DeLisle]

The EcoNet EN751221 MIPS SoC integrates an MT7530 switch memory-mapped
in the SoC register space. It is similar to the econet,en7528-switch
except that ports 0..3 are 10/100 only, port 4 has a gigabit PHY, and
port 5 has no PHY, but is in some implementations connected to a
downstream MT7530 multi-chip module via a TRGMII link.

The downstream MT7530 is similar to the MT7621 multi-chip module and
has all gigabit ports.

Add the econet,en751221-switch compatible with the same constraints as
the other built-in switches and also add the econet,en751221-gswitch
multi-chip-module switch whose port 6 is configured as a DSA link port.

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 .../bindings/net/dsa/mediatek,mt7530.yaml     | 43 ++++++++++++++++++-
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index f98b9113ad12..011c94ef9554 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -29,6 +29,12 @@ description: |
   and the switch registers are directly mapped into SoC's memory map rather than
   using MDIO. The DSA driver currently doesn't support MT7620 variants.
 
+  EcoNet EN751221 SoCs have a memory mapped MT7530 with 3x 10/100 PHYs and 1x
+  gigabit PHY. Some EN751221 chips such as EN7513G and EN7526G have a multi-chip
+  module implementation of the MT7530 as well as the on-die switch. In these
+  packages, the multi-chip module is connected to port 5 of the on-die switch
+  via a TRGMII link and allows for 5 gigabit ports.
+
   There is only the standalone version of MT7531.
 
   Port 5 on MT7530 supports various configurations:
@@ -109,6 +115,14 @@ properties:
           Built-in switch of the EcoNet EN7528 SoC
         const: econet,en7528-switch
 
+      - description:
+          Built-in switch of EcoNet EN751221 SoC
+        const: econet,en751221-switch
+
+      - description:
+          Multi-chip module gigabit MT7530 in EN751221 SoC
+        const: econet,en751221-gswitch
+
   reg:
     maxItems: 1
 
@@ -158,8 +172,8 @@ properties:
   mediatek,mcm:
     type: boolean
     description:
-      Used for MT7621AT, MT7621DAT, MT7621ST and MT7623AI SoCs which the MT7530
-      switch is a part of the multi-chip module.
+      Used for MT7621AT, MT7621DAT, MT7621ST, MT7623AI and EN751221 SoCs which
+      the MT7530 switch is a part of the multi-chip module.
 
   reset-gpios:
     description: |
@@ -221,6 +235,20 @@ $defs:
                 phy-mode:
                   const: internal
 
+  mt7530-passthrough-dsa-port:
+    patternProperties:
+      "^(ethernet-)?ports$":
+        patternProperties:
+          "^(ethernet-)?port@[0-6]$":
+            if:
+              required: [ link ]
+            then:
+              properties:
+                reg:
+                  const: 6
+                phy-mode:
+                  const: trgmii
+
   mt7530-dsa-port:
     patternProperties:
       "^(ethernet-)?ports$":
@@ -328,6 +356,7 @@ allOf:
             - airoha,en7581-switch
             - airoha,an7583-switch
             - econet,en7528-switch
+            - econet,en751221-switch
     then:
       $ref: "#/$defs/builtin-dsa-port"
       properties:
@@ -335,6 +364,16 @@ allOf:
         mediatek,mcm: false
         reset-names: false
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - econet,en751221-gswitch
+    then:
+      $ref: "#/$defs/mt7530-passthrough-dsa-port"
+      required:
+        - mediatek,mcm
+
 unevaluatedProperties: false
 
 examples:
-- 
2.39.5

[PATCH v4 7/7] net: dsa: mediatek: support EN751221 switch

[Caleb James DeLisle]

The EcoNet EN751221 has either one, or two, onboard MT7530 switches.
Every implementation of this SoC has one switch on die with the CPU,
but some chips - notably those with a "G" in the name, have a second
MT7530 as an MCM module.

The on-die MT7530 has 4 FE ports and 1 GE port. The MCM switch has all
gigabit ports.

The MCM switch connects to the SoC switch via a TRGMII link from port
5 on the SoC switch to port 6 on the MCM switch. This link undergoes
calibration on startup.

All known devices with the MCM present do not make any use of the ports
on the on-die switch, it is put into "passthrough" mode in which all
traffic is shuttled between port 6 and port 5 without alteration of the
DSA tags.

Add support for both the on-die and the MCM switch, and calibration of
the TRGMII link between them.

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/net/dsa/mt7530-mdio.c |   1 +
 drivers/net/dsa/mt7530-mmio.c |   1 +
 drivers/net/dsa/mt7530.c      | 335 +++++++++++++++++++++++++++++++++-
 drivers/net/dsa/mt7530.h      |  24 +++
 4 files changed, 357 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
index 0be107dc6759..6dd099466dc3 100644
--- a/drivers/net/dsa/mt7530-mdio.c
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -139,6 +139,7 @@ mt7531_create_sgmii(struct mt7530_priv *priv)
 }
 
 static const struct of_device_id mt7530_of_match[] = {
+	{ .compatible = "econet,en751221-gswitch", .data = &mt753x_table[ID_EN751221_EXT], },
 	{ .compatible = "mediatek,mt7621", .data = &mt753x_table[ID_MT7621], },
 	{ .compatible = "mediatek,mt7530", .data = &mt753x_table[ID_MT7530], },
 	{ .compatible = "mediatek,mt7531", .data = &mt753x_table[ID_MT7531], },
diff --git a/drivers/net/dsa/mt7530-mmio.c b/drivers/net/dsa/mt7530-mmio.c
index 746a0a462249..6da7a1787ffe 100644
--- a/drivers/net/dsa/mt7530-mmio.c
+++ b/drivers/net/dsa/mt7530-mmio.c
@@ -12,6 +12,7 @@
 static const struct of_device_id mt7988_of_match[] = {
 	{ .compatible = "airoha,an7583-switch", .data = &mt753x_table[ID_AN7583], },
 	{ .compatible = "airoha,en7581-switch", .data = &mt753x_table[ID_EN7581], },
+	{ .compatible = "econet,en751221-switch", .data = &mt753x_table[ID_EN751221], },
 	{ .compatible = "econet,en7528-switch", .data = &mt753x_table[ID_EN7528], },
 	{ .compatible = "mediatek,mt7988-switch", .data = &mt753x_table[ID_MT7988], },
 	{ /* sentinel */ }
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 24e4485624d3..447c80455460 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -49,6 +49,9 @@ static const struct mt7530_mib_desc mt7530_mib[] = {
 static int
 mt753x_ctrl_phy_addr(struct mt7530_priv *priv)
 {
+	if (priv->id == ID_EN751221)
+		return 12;
+
 	if (WARN_ON_ONCE(!priv->mdiodev))
 		return 0;
 
@@ -349,12 +352,19 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
 	regmap_read(priv->regmap, MT753X_MTRAP, &xtal);
 	xtal &= MT7530_XTAL_MASK;
 
+	/* EN751221 on-die does not report clock speed in MTRAP */
+	if (priv->id == ID_EN751221)
+		xtal = MT7530_XTAL_25MHZ;
+
 	if (xtal == MT7530_XTAL_25MHZ)
 		ssc_delta = 0x57;
 	else
 		ssc_delta = 0x87;
 
-	if (priv->id == ID_MT7621) {
+	if (priv->id == ID_EN751221_EXT || priv->id == ID_EN751221) {
+		/* PLL frequency: 362.5Mhz */
+		ncpo1 = 0x1d00;
+	} else if (priv->id == ID_MT7621) {
 		/* PLL frequency: 125MHz: 1.0GBit */
 		if (xtal == MT7530_XTAL_40MHZ)
 			ncpo1 = 0x0640;
@@ -471,6 +481,251 @@ mt7531_pll_setup(struct mt7530_priv *priv)
 	usleep_range(25, 35);
 }
 
+static bool
+en751221_trgmii_cal_ok(struct mt7530_priv *rx, u32 reg)
+{
+	u32 val = 0;
+
+	regmap_read(rx->regmap, reg, &val);
+	regmap_write(rx->regmap, reg, val | EDGE_CHK);
+	regmap_write(rx->regmap, reg, val & ~EDGE_CHK);
+	regmap_read(rx->regmap, reg, &val);
+
+	return FIELD_GET(RD_VALUE_MASK, val) == TGMII_TD_PAT &&
+	       !FIELD_GET(RD_ERR_MASK, val);
+}
+
+static void
+en751221_trgmii_calibrate_direction(struct mt7530_priv *tx,
+				    struct mt7530_priv *rx,
+				    u8 *default_taps)
+{
+	int channel;
+
+	regmap_set_bits(tx->regmap, MT7530_TRGMII_TXCTRL, TRAIN_TXEN);
+
+	/* Put fail pattern on every channel */
+	for (channel = 0; channel < NUM_TRGMII_CTRL; channel++)
+		regmap_update_bits(tx->regmap, EN7530_TRGMII_TD_CTRL(channel),
+				   TGMII_TD_PAT_MASK,
+				   FIELD_PREP(TGMII_TD_PAT_MASK,
+					      TGMII_TD_FAIL_PAT));
+
+	for (channel = 0; channel < NUM_TRGMII_CTRL; channel++) {
+		u32 rx_reg = MT7530_TRGMII_RD(channel);
+		int best_tap = -1, best_range = -1;
+		int first = -1;
+		u8 dac;
+		char log[TD_TAP_MAX + 1];
+
+		/* Put test pattern on correct channel */
+		regmap_update_bits(tx->regmap, EN7530_TRGMII_TD_CTRL(channel),
+				   TGMII_TD_PAT_MASK,
+				   FIELD_PREP(TGMII_TD_PAT_MASK, TGMII_TD_PAT));
+
+		for (dac = 0; dac < TD_TAP_MAX; dac++) {
+			/* Set TAP */
+			regmap_update_bits(rx->regmap, rx_reg, RD_TAP_MASK,
+					   RD_TAP(dac));
+
+			/* Check if we can get a good read */
+			if (en751221_trgmii_cal_ok(rx, rx_reg)) {
+				log[dac] = '+';
+
+				if (first < 0)
+					first = dac;
+
+				if ((dac - first) > best_range) {
+					best_tap = (first + dac) / 2;
+					best_range = dac - first;
+				}
+			} else {
+				log[dac] = '.';
+				first = -1;
+			}
+		}
+
+		log[TD_TAP_MAX] = '\0';
+
+		if (best_tap > -1) {
+			regmap_update_bits(rx->regmap, rx_reg, RD_TAP_MASK,
+					   RD_TAP(best_tap));
+
+			log[best_tap] = 'X';
+
+			dev_info(rx->dev,
+				 "TRGMII lane %d: %s tap %d old %d\n",
+				 channel, log, best_tap, default_taps[channel]);
+		} else {
+			regmap_update_bits(rx->regmap, rx_reg, RD_TAP_MASK,
+					   RD_TAP(default_taps[channel]));
+
+			dev_warn(rx->dev,
+				 "TRGMII lane %d: %s calibration failed, preserving tap %u\n",
+				 channel, log, default_taps[channel]);
+		}
+
+		/* Return channel to failure pattern */
+		regmap_update_bits(tx->regmap, EN7530_TRGMII_TD_CTRL(channel),
+				   TGMII_TD_PAT_MASK,
+				   FIELD_PREP(TGMII_TD_PAT_MASK,
+					      TGMII_TD_FAIL_PAT));
+	}
+
+	regmap_clear_bits(tx->regmap, MT7530_TRGMII_TXCTRL, TRAIN_TXEN);
+}
+
+static void
+en751221_set_tx_drive(struct mt7530_priv *priv, u8 drvp, u8 drvn, u8 clk_drvp,
+		      u8 clk_drvn)
+{
+	int channel;
+
+	/* Tx driving for TRGMII path on SoC */
+	for (channel = 0; channel < NUM_TRGMII_CTRL; channel++)
+		regmap_write(priv->regmap, MT7530_TRGMII_TD_ODT(channel),
+			     TD_DM_DRVP(drvp) | TD_DM_DRVN(drvn));
+
+	/* Undocumented 6th channel, probably drive strength for clock line. */
+	regmap_write(priv->regmap, MT7530_TRGMII_TD_ODT(5), TD_DM_DRVP(clk_drvp) |
+		     TD_DM_DRVN(clk_drvn));
+}
+
+static void
+en751221_trgmii_pair_setup(struct mt7530_priv *ext, int ext_port,
+			   struct mt7530_priv *ondie, int ondie_port)
+{
+	u8 default_taps_ondie[NUM_TRGMII_CTRL];
+	u8 default_taps_ext[NUM_TRGMII_CTRL];
+	u32 mcr_down, mcr_up;
+	int channel;
+	int reg_val;
+
+	/* BOTH: Put interfaces in a downed state */
+	mcr_down = PMCR_IFG_XMIT(PMCR_IFG_XMIT_64) | PMCR_MAC_MODE |
+		   MT7530_FORCE_MODE | PMCR_MAC_RX_EN | PMCR_BACKOFF_EN |
+		   PMCR_BACKPR_EN | PMCR_FORCE_SPEED_1000 | PMCR_FORCE_FDX;
+	regmap_write(ondie->regmap,  MT753X_PMCR_P(ondie_port), mcr_down);
+	regmap_write(ext->regmap, MT753X_PMCR_P(ext_port), mcr_down);
+	usleep_range(5000, 6000);
+
+	/* BOTH: Reset TX */
+	regmap_set_bits(ext->regmap, MT7530_TRGMII_TXCTRL, TX_RST);
+	regmap_set_bits(ondie->regmap, MT7530_TRGMII_TXCTRL, TX_RST);
+	usleep_range(5000, 6000);
+	regmap_clear_bits(ext->regmap, MT7530_TRGMII_TXCTRL, TX_RST);
+	regmap_clear_bits(ondie->regmap, MT7530_TRGMII_TXCTRL, TX_RST);
+
+	/* BOTH: Reset RX controllers down */
+	regmap_set_bits(ext->regmap, MT7530_TRGMII_RCK_CTRL, RX_RST);
+	regmap_set_bits(ondie->regmap, MT7530_TRGMII_RCK_CTRL, RX_RST);
+
+	/* BOTH: Set TX drive strength, MCM uses more */
+	en751221_set_tx_drive(ondie, 8, 8, 7, 7);
+	en751221_set_tx_drive(ext, 11, 11, 15, 15);
+
+	/* BOTH: Reset RX controllers up */
+	regmap_clear_bits(ext->regmap, MT7530_TRGMII_RCK_CTRL, RX_RST);
+	regmap_clear_bits(ondie->regmap, MT7530_TRGMII_RCK_CTRL, RX_RST);
+
+	/* BOTH: Ports up */
+	mcr_up = mcr_down | PMCR_MAC_TX_EN | PMCR_FORCE_LNK;
+	regmap_write(ondie->regmap, MT753X_PMCR_P(ondie_port), mcr_up);
+	regmap_write(ext->regmap, MT753X_PMCR_P(ext_port), mcr_up);
+
+	/* MCM: Set Ext->SoC TX delay to 0 */
+	for (channel = 0; channel < NUM_TRGMII_CTRL; channel++)
+		regmap_clear_bits(ext->regmap, EN7530_TRGMII_TD_CTRL(channel),
+				  TGMII_TD_TAP_MASK);
+
+	/* SOC: ODT */
+	regmap_set_bits(ondie->regmap, MT7530_TRGMII_RCK_RTT,
+			DQS1_GATE | DQS0_GATE | EN751221_B17);
+
+	/* SOC: Undocumented */
+	for (channel = 0; channel < NUM_TRGMII_CTRL; channel++)
+		regmap_write(ondie->regmap,  (0x7a14 + channel * 8),
+			     0x3227700);
+
+	/* MCM: Spread spectrum clock*/
+	core_clear(ext, CORE_PLL_GROUP8, RG_LCDDS_SSC_EN);
+
+	/* BOTH: Zero clock delay */
+	regmap_clear_bits(ext->regmap, MT7530_TRGMII_RCK_CTRL,
+			  DQSI0_TAP_MASK);
+	regmap_clear_bits(ondie->regmap, MT7530_TRGMII_RCK_CTRL,
+			  DQSI0_TAP_MASK);
+
+	/* BOTH: Collect and then zero every RX TAP */
+	for (channel = 0; channel < NUM_TRGMII_CTRL; channel++) {
+		reg_val = 0;
+		regmap_read(ext->regmap, MT7530_TRGMII_RD(channel), &reg_val);
+		default_taps_ext[channel] = FIELD_GET(RD_TAP_MASK, reg_val);
+		regmap_clear_bits(ext->regmap, MT7530_TRGMII_RD(channel),
+				  RD_TAP_MASK);
+
+		reg_val = 0;
+		regmap_read(ondie->regmap, MT7530_TRGMII_RD(channel), &reg_val);
+		default_taps_ondie[channel] = FIELD_GET(RD_TAP_MASK, reg_val);
+		regmap_clear_bits(ondie->regmap, MT7530_TRGMII_RD(channel),
+				  RD_TAP_MASK);
+	}
+
+	en751221_trgmii_calibrate_direction(ondie, ext, default_taps_ondie);
+	en751221_trgmii_calibrate_direction(ext, ondie, default_taps_ext);
+
+	dev_info(ondie->dev, "TRGMII inter-switch link initialized\n");
+}
+
+static void
+mt7530_cal_interswitch_trgmii(struct dsa_port *dp)
+{
+	struct dsa_switch *ds = dp->ds;
+	struct dsa_switch *peer_ds;
+	struct dsa_port *peer_dp;
+	struct mt7530_priv *peer;
+	struct mt7530_priv *priv;
+	struct dsa_link *dl;
+	u32 pmsr = 0;
+
+	priv = ds->priv;
+
+	if (priv->id != ID_EN751221_EXT)
+		return;
+
+	if (dp->type != DSA_PORT_TYPE_DSA)
+		return;
+
+	regmap_read(priv->regmap, MT7530_PMSR_P(dp->index), &pmsr);
+
+	if (!(pmsr & PMSR_LINK))
+		return;
+
+	list_for_each_entry(dl, &ds->dst->rtable, list) {
+		if (dl->dp != dp)
+			continue;
+
+		peer_dp = dl->link_dp;
+
+		if (peer_dp->type != DSA_PORT_TYPE_DSA)
+			continue;
+
+		peer_ds = dl->link_dp->ds;
+
+		if (!of_device_is_compatible(peer_ds->dev->of_node,
+					     "econet,en751221-switch"))
+			continue;
+
+		peer = peer_ds->priv;
+
+		if (peer->id != ID_EN751221)
+			continue;
+
+		en751221_trgmii_pair_setup(priv, dp->index, peer,
+					   peer_dp->index);
+	}
+}
+
 static void
 mt7530_mib_reset(struct dsa_switch *ds)
 {
@@ -1459,7 +1714,8 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
 
 	mutex_unlock(&priv->reg_mutex);
 
-	if (priv->id != ID_MT7530 && priv->id != ID_MT7621)
+	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
+	    priv->id != ID_EN751221 && priv->id != ID_EN751221_EXT)
 		return 0;
 
 	if (port == 5)
@@ -1486,7 +1742,8 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
 
 	mutex_unlock(&priv->reg_mutex);
 
-	if (priv->id != ID_MT7530 && priv->id != ID_MT7621)
+	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
+	    priv->id != ID_EN751221 && priv->id != ID_EN751221_EXT)
 		return;
 
 	/* Do not set MT7530_P5_DIS when port 5 is being used for PHY muxing. */
@@ -2665,6 +2922,12 @@ mt7530_setup(struct dsa_switch *ds)
 			   MT7530_CHG_TRAP | MT7530_PHY_INDIRECT_ACCESS,
 			   MT7530_CHG_TRAP);
 
+	/* EN751221 MCM starts on the wrong frequency */
+	if (priv->id == ID_EN751221_EXT)
+		regmap_update_bits(priv->regmap, MT753X_MTRAP,
+				   MT7530_CHG_TRAP | MT7530_CK_SEL,
+				   MT7530_CHG_TRAP);
+
 	if ((val & MT7530_XTAL_MASK) == MT7530_XTAL_40MHZ)
 		mt7530_pll_setup(priv);
 
@@ -3085,6 +3348,44 @@ static void en7528_mac_port_get_caps(struct dsa_switch *ds, int port,
 	}
 }
 
+static void en751221_mac_port_get_caps(struct dsa_switch *ds, int port,
+				       struct phylink_config *config)
+{
+	switch (port) {
+	/* EN751221 ports 0..3 are connected to the integrated FE PHYs. */
+	case 0 ... 3:
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  config->supported_interfaces);
+
+		config->mac_capabilities |= MAC_10 | MAC_100;
+		break;
+
+	/* Port 4 is connected to the standalone EN7512/EN7521 GPHY. */
+	case 4:
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  config->supported_interfaces);
+
+		config->mac_capabilities |= MAC_10 | MAC_100 | MAC_1000FD;
+		break;
+
+	/* Port 5 is the 1 Gbit/s TRGMII cascade to the external MT7530. */
+	case 5:
+		__set_bit(PHY_INTERFACE_MODE_TRGMII,
+			  config->supported_interfaces);
+
+		config->mac_capabilities |= MAC_1000FD;
+		break;
+
+	/* Port 6 is the 1 Gbit/s CPU link to GDM1. */
+	case 6:
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  config->supported_interfaces);
+
+		config->mac_capabilities |= MAC_1000FD;
+		break;
+	}
+}
+
 static void
 mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
 		  phy_interface_t interface)
@@ -3234,6 +3535,9 @@ static void mt753x_phylink_mac_link_up(struct phylink_config *config,
 	}
 
 	regmap_set_bits(priv->regmap, MT753X_PMCR_P(dp->index), mcr);
+
+	if (interface == PHY_INTERFACE_MODE_TRGMII)
+		mt7530_cal_interswitch_trgmii(dp);
 }
 
 static void mt753x_phylink_mac_disable_tx_lpi(struct phylink_config *config)
@@ -3499,7 +3803,8 @@ mt753x_conduit_state_change(struct dsa_switch *ds,
 	 * interface is up. NOTE: "CPU port" can also mean an upstream DSA link.
 	 */
 	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
-	    priv->id != ID_EN7528)
+	    priv->id != ID_EN7528 && priv->id != ID_EN751221 &&
+	    priv->id != ID_EN751221_EXT)
 		return;
 
 	mask = BIT(cpu_dp->index);
@@ -3817,6 +4122,28 @@ const struct mt753x_info mt753x_table[] = {
 		.phy_write_c45 = mt7531_ind_c45_phy_write,
 		.mac_port_get_caps = en7528_mac_port_get_caps,
 	},
+	[ID_EN751221] = {
+		.id = ID_EN751221,
+		.pcs_ops = &mt7530_pcs_ops,
+		.sw_setup = mt7988_setup,
+		.phy_read_c22 = mt7531_ind_c22_phy_read,
+		.phy_write_c22 = mt7531_ind_c22_phy_write,
+		.phy_read_c45 = mt7531_ind_c45_phy_read,
+		.phy_write_c45 = mt7531_ind_c45_phy_write,
+		.mac_port_get_caps = en751221_mac_port_get_caps,
+		.mac_port_config = mt7530_mac_config,
+	},
+	[ID_EN751221_EXT] = {
+		.id = ID_EN751221_EXT,
+		.pcs_ops = &mt7530_pcs_ops,
+		.sw_setup = mt7530_setup,
+		.phy_read_c22 = mt7530_phy_read_c22,
+		.phy_write_c22 = mt7530_phy_write_c22,
+		.phy_read_c45 = mt7530_phy_read_c45,
+		.phy_write_c45 = mt7530_phy_write_c45,
+		.mac_port_get_caps = mt7530_mac_port_get_caps,
+		.mac_port_config = mt7530_mac_config,
+	},
 };
 EXPORT_SYMBOL_GPL(mt753x_table);
 
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index c86bc4bca29b..1a4329749c0f 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -24,6 +24,8 @@ enum mt753x_id {
 	ID_EN7581 = 4,
 	ID_AN7583 = 5,
 	ID_EN7528 = 6,
+	ID_EN751221 = 7,
+	ID_EN751221_EXT = 8,
 };
 
 #define	NUM_TRGMII_CTRL			5
@@ -356,6 +358,9 @@ enum mt7530_vlan_port_acc_frm {
 #define MT753X_PMCR_P(x)		(0x3000 + ((x) * 0x100))
 #define  PMCR_IFG_XMIT_MASK		GENMASK(19, 18)
 #define  PMCR_IFG_XMIT(x)		FIELD_PREP(PMCR_IFG_XMIT_MASK, x)
+#define    PMCR_IFG_XMIT_96		0
+#define    PMCR_IFG_XMIT_RAND		1
+#define    PMCR_IFG_XMIT_64		2
 #define  PMCR_EXT_PHY			BIT(17)
 #define  PMCR_MAC_MODE			BIT(16)
 #define  MT7530_FORCE_MODE		BIT(15)
@@ -584,6 +589,7 @@ enum mt7531_clk_skew {
 #define MT753X_MTRAP			0x7804
 #define  MT7530_P5_PHY0_SEL		BIT(20)
 #define  MT7530_CHG_TRAP		BIT(16)
+#define  MT7530_CK_SEL			BIT(15)
 #define  MT7530_LOOP_DET_DISABLE	BIT(14)
 #define  MT7530_P5_MAC_SEL		BIT(13)
 #define  MT7530_P6_DIS			BIT(8)
@@ -601,6 +607,8 @@ enum mt7531_xtal_fsel {
 /* Register for TOP signal control */
 #define MT7530_TOP_SIG_CTRL		0x7808
 #define  TOP_SIG_CTRL_NORMAL		(BIT(17) | BIT(16))
+/* Undocumented */
+#define  TOP_SIG_CTRL_B0		BIT(0)
 
 #define MT7531_TOP_SIG_SR		0x780c
 #define  PAD_DUAL_SGMII_EN		BIT(1)
@@ -647,18 +655,31 @@ enum mt7531_xtal_fsel {
 #define MT7530_TRGMII_RCK_RTT		0x7a04
 #define  DQS1_GATE			BIT(31)
 #define  DQS0_GATE			BIT(30)
+/* Undocumented */
+#define  EN751221_B17			BIT(17)
 
 #define MT7530_TRGMII_RD(x)		(0x7a10 + (x) * 8)
 #define  BSLIP_EN			BIT(31)
 #define  EDGE_CHK			BIT(30)
+#define  RD_VALUE_MASK			GENMASK(23, 16)
+#define  RD_ERR_MASK			GENMASK(11, 8)
 #define  RD_TAP_MASK			GENMASK(6, 0)
 #define  RD_TAP(x)			FIELD_PREP(RD_TAP_MASK, x)
+/* Training does not try anything beyond this */
+#define  TD_TAP_MAX			64
 
 #define MT7530_TRGMII_TXCTRL		0x7a40
 #define  TRAIN_TXEN			BIT(31)
 #define  TXC_INV			BIT(30)
 #define  TX_RST				BIT(28)
 
+#define EN7530_TRGMII_TD_CTRL(x)	(0x7a50 + (x) * 8)
+#define   TGMII_TD_TAP_MASK		GENMASK(11, 8)
+#define   TGMII_TD_PAT_MASK		GENMASK(7, 0)
+/* Use a 01010101 bit pattern */
+#define   TGMII_TD_PAT			0x55
+#define   TGMII_TD_FAIL_PAT		0xaa
+
 #define MT7530_TRGMII_TD_ODT(i)		(0x7a54 + 8 * (i))
 #define  TD_DM_DRVP_MASK		GENMASK(3, 0)
 #define  TD_DM_DRVP(x)			FIELD_PREP(TD_DM_DRVP_MASK, x)
@@ -764,6 +785,9 @@ enum mt7531_xtal_fsel {
 #define  RG_LCCDS_C(x)			FIELD_PREP(RG_LCCDS_C_MASK, x)
 #define  RG_LCDDS_PCW_NCPO_CHG		BIT(3)
 
+#define CORE_PLL_GROUP8			0x407
+#define  RG_LCDDS_SSC_EN		BIT(10)
+
 #define CORE_PLL_GROUP10		0x409
 #define  RG_LCDDS_SSC_DELTA_MASK	GENMASK(11, 0)
 #define  RG_LCDDS_SSC_DELTA(x)		FIELD_PREP(RG_LCDDS_SSC_DELTA_MASK, x)
-- 
2.39.5

Re: [PATCH v4 3/7] net: dsa: mediatek: add support for passthrough mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] net: dsa: mt7530: mt753x_check_passthrough() breaks dual-CPU-port boards
- [High] net: dsa: mt7530: mt753x_trap_frames() blocks CPU control frames in passthrough
- [High] net: dsa: mt7530: downstream switch callbacks assume local CPU port
- [Medium] net: dsa: mt7530: setup overwrites PVC_EG_TAG on passthrough CPU port
- [Medium] net: dsa: mt7530: mt7530_port_enable() ignores per-port CPU conduit
--

commit 89ff16dda9d512576bf6153ac6b119283ed62bea
Author: Caleb James DeLisle <cjd@cjdns.fr>

net: dsa: mediatek: add support for passthrough mode

This commit adds passthrough mode support for MT7530 switches so that an
upstream SoC-integrated switch can pass traffic to and from a downstream
MT7530 switch without altering the MediaTek DSA tag.

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index d10194488bdee..c5b3b00acb956 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c

[ ... ]

> @@ -1286,13 +1297,81 @@ mt753x_trap_frames(struct mt7530_priv *priv)
>  				   TO_CPU_FW_CPU_ONLY);
>  }

[ ... ]

> +/* If this switch is downstream of another switch that is in passthrough mode,
> + * the "CPU" port is actually a DSA port.
> + */
>  static void
>  mt753x_cpu_port_enable(struct dsa_switch *ds, int port)
>  {
>  	struct mt7530_priv *priv = ds->priv;
>  
> -	/* Enable Mediatek header mode on the cpu port */
> -	regmap_write(priv->regmap, MT7530_PVC_P(port), PORT_SPEC_TAG);
> +	if (priv->is_passthrough) {
> +		/* Disable parsing of the DSA tag, it will be forwarded blindly
> +		 * to the downstream switch.
> +		 */
> +		regmap_write(priv->regmap, MT7530_PVC_P(port),
> +			     VLAN_ATTR(MT7530_VLAN_TRANSPARENT) |
> +			     PVC_EG_TAG(MT7530_VLAN_EG_DISABLED));
> +
> +		/* The port is not configured to parse DSA tags, so they are
> +		 * liable to be confused for length fields, so length check is
> +		 * disabled.
> +		 */
> +		regmap_clear_bits(priv->regmap, MT753X_AGC, AGC_L2LEN_CHK);
> +
> +		/* In passthrough mode, MTU is only enforced downstream */
> +		mt7530_port_change_mtu(ds, port, MT7530_MAX_MTU);
> +
> +		/* Loop detection has no value in passthrough mode */
> +		regmap_set_bits(priv->regmap, MT753X_MTRAP,
> +				MT7530_LOOP_DET_DISABLE);
> +	} else {
> +		/* Not passthrough, enable DSA tag handling on CPU port. */
> +		regmap_write(priv->regmap, MT7530_PVC_P(port), PORT_SPEC_TAG);
> +	}
>  
>  	/* Enable flooding on the CPU port */
>  	regmap_set_bits(priv->regmap, MT753X_MFC,
> @@ -1322,6 +1401,7 @@ static int
>  mt7530_port_enable(struct dsa_switch *ds, int port,
>  		   struct phy_device *phy)
>  {
> +	int upstream_pt = dsa_switch_upstream_port(ds);
>  	struct dsa_port *dp = dsa_to_port(ds, port);
>  	struct mt7530_priv *priv = ds->priv;
>  
> @@ -1332,13 +1412,38 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
>  	 * bridge.
>  	 */
>  	if (dsa_port_is_user(dp)) {
> -		struct dsa_port *cpu_dp = dp->cpu_dp;
> +		priv->ports[port].pm |= PCR_MATRIX(BIT(upstream_pt));
>  
> -		priv->ports[port].pm |= PCR_MATRIX(BIT(cpu_dp->index));

[Severity: Medium]
On switches with multiple CPU ports (such as MT7531 where per-port conduit
assignment is supported via mt753x_port_change_conduit()), does replacing
dp->cpu_dp->index with dsa_switch_upstream_port(ds) in mt7530_port_enable()
connect a user port to the wrong CPU port when its assigned conduit differs
from that of the first available port?

dsa_switch_upstream_port(ds) returns dsa_upstream_port(ds, dp->index) for the
first available port on the switch rather than dsa_upstream_port(ds, port)
for the port being enabled. When a user port assigned to the secondary CPU
conduit is brought up via dsa_user_open() -> mt7530_port_enable(), shouldn't
this use dsa_upstream_port(ds, port) instead?

[ ... ]

> @@ -2511,7 +2671,7 @@ mt7530_setup(struct dsa_switch *ds)
>  		/* Disable learning by default on all ports */
>  		regmap_set_bits(priv->regmap, MT7530_PSC_P(i), SA_DIS);
>  
> -		if (dsa_is_cpu_port(ds, i)) {
> +		if (dsa_is_upstream_port(ds, i)) {
>  			mt753x_cpu_port_enable(ds, i);
>  		} else {
>  			mt7530_port_disable(ds, i);

[Severity: Medium]
In both mt7530_setup() and mt7531_setup_common(), mt753x_cpu_port_enable()
sets MT7530_PVC_P(port) with PVC_EG_TAG(MT7530_VLAN_EG_DISABLED) when
priv->is_passthrough is true, but right after the if/else block in the loop:

drivers/net/dsa/mt7530.c:mt7530_setup() {
    ...
		if (dsa_is_upstream_port(ds, i)) {
			mt753x_cpu_port_enable(ds, i);
		} else {
			mt7530_port_disable(ds, i);

			/* Set default PVID to 0 on all user ports */
			regmap_update_bits(priv->regmap, MT7530_PPBV1_P(i),
					   G0_PORT_VID_MASK, G0_PORT_VID_DEF);
		}
		/* Enable consistent egress tag */
		regmap_update_bits(priv->regmap, MT7530_PVC_P(i),
				   PVC_EG_TAG_MASK,
				   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
    ...
}

Doesn't this unconditionally overwrite PVC_EG_TAG_MASK on the passthrough
upstream CPU port with MT7530_VLAN_EG_CONSISTENT, since mt7530_port_enable()
only rewrites MT7530_PVC_P(port) for the downstream DSA link port?

[Severity: High]
Also in mt7530_setup() and mt7531_setup_common(), mt753x_trap_frames(priv)
is still called unconditionally even when priv->is_passthrough is true:

drivers/net/dsa/mt7530.c:mt753x_trap_frames() {
	regmap_update_bits(priv->regmap, MT753X_BPC,
			   PAE_BPDU_FR | PAE_EG_TAG_MASK | PAE_PORT_FW_MASK |
				   BPDU_EG_TAG_MASK | BPDU_PORT_FW_MASK,
			   PAE_BPDU_FR | PAE_EG_TAG(MT7530_VLAN_EG_DISABLED) |
				   PAE_PORT_FW(TO_CPU_FW_CPU_ONLY) |
				   BPDU_EG_TAG(MT7530_VLAN_EG_DISABLED) |
				   TO_CPU_FW_CPU_ONLY);
    ...
}

On a normal switch, PORT_SPEC_TAG is enabled on the CPU port so frames sent
by the CPU are directed by the MediaTek special tag. On a passthrough switch,
mt753x_cpu_port_enable() leaves PORT_SPEC_TAG unset on the CPU port so frames
are forwarded via the port matrix.

When the CPU transmits STP BPDUs, 802.1X PAE, LACP, PAUSE, or LLDP frames
(matching 01:80:c2:00:00:00/01/02/03/0e in BPC, RGAC1, and RGAC2) towards a
downstream switch port, won't those frames ingressing the passthrough
switch's CPU port match the trap rules and be restricted to
TO_CPU_FW_CPU_ONLY (the passthrough switch's own CPU port) instead of being
forwarded to the downstream DSA link port?

[ ... ]

> @@ -3304,7 +3472,7 @@ mt753x_conduit_state_change(struct dsa_switch *ds,
>  
>  	/* Set the CPU port to trap frames to for MT7530. Trapped frames will be
>  	 * forwarded to the numerically smallest CPU port whose conduit
> -	 * interface is up.
> +	 * interface is up. NOTE: "CPU port" can also mean an upstream DSA link.
>  	 */
>  	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
>  	    priv->id != ID_EN7528)

[Severity: High]
On the downstream switch behind a passthrough switch, the upstream port is a
DSA link port (DSA_PORT_TYPE_DSA) rather than a CPU port (DSA_PORT_TYPE_CPU),
and dp->cpu_dp / conduit->dsa_ptr point to the CPU port on the upstream
switch. How are the following paths handled on the downstream switch?

First, in mt753x_conduit_state_change(), mt7530_update_port_member(), and
mt7530_port_vlan_filtering(), the code uses cpu_dp->index directly:

drivers/net/dsa/mt7530.c:mt753x_conduit_state_change() {
	struct dsa_port *cpu_dp = conduit->dsa_ptr;
    ...
	mask = BIT(cpu_dp->index);
}

drivers/net/dsa/mt7530.c:mt7530_update_port_member() {
	struct dsa_port *dp = dsa_to_port(priv->ds, port), *other_dp;
	struct mt7530_port *p = &priv->ports[port], *other_p;
	struct dsa_port *cpu_dp = dp->cpu_dp;
	u32 port_bitmap = BIT(cpu_dp->index);
    ...
}

drivers/net/dsa/mt7530.c:mt7530_port_vlan_filtering() {
	struct dsa_port *dp = dsa_to_port(ds, port);
	struct dsa_port *cpu_dp = dp->cpu_dp;

	if (vlan_filtering) {
		mt7530_port_set_vlan_aware(ds, port);
		mt7530_port_set_vlan_aware(ds, cpu_dp->index);
    ...
}

If the upstream switch's CPU port index differs from the downstream switch's
upstream DSA link port index (for example, port 6 on the upstream switch vs
port 5 on the downstream switch), won't these program the wrong port on the
downstream switch?

Second, when a VLAN is added or deleted on the downstream switch, DSA calls
mt7530_port_vlan_add() and mt7530_port_vlan_del() on the downstream switch's
upstream DSA link port (passing the user port's untagged and pvid flags). In
mt7530_hw_vlan_add():

drivers/net/dsa/mt7530.c:mt7530_hw_vlan_add() {
    ...
	if (dsa_port_is_cpu(dp))
		val = MT7530_VLAN_EGRESS_STACK;
	else if (entry->untagged)
		val = MT7530_VLAN_EGRESS_UNTAG;
	else
		val = MT7530_VLAN_EGRESS_TAG;
	regmap_update_bits(priv->regmap, MT7530_VAWD2,
			   ETAG_CTRL_P_MASK(entry->port),
			   ETAG_CTRL_P(entry->port, val));
}

Because dsa_port_is_cpu(dp) is false for the downstream switch's upstream DSA
link port, won't mt7530_hw_vlan_add() fail to set MT7530_VLAN_EGRESS_STACK
and instead set UNTAG or TAG, while mt7530_port_vlan_add() and
mt7530_port_vlan_del() also overwrite PVID and ACC_FRM_MASK on the upstream
port?

[ ... ]

> @@ -3399,6 +3574,99 @@ static int mt7988_setup(struct dsa_switch *ds)
>  	return mt7531_setup_common(ds);
>  }
>  
> +/* 1 if passthrough, negative if error. */
> +static int mt753x_check_passthrough(struct device *dev)
> +{
> +	struct device_node *ports, *port;
> +	int passthrough_ports = 0;
> +	int enabled_ports = 0;
> +
> +	ports = of_get_child_by_name(dev->of_node, "ports");
> +	if (!ports)
> +		ports = of_get_child_by_name(dev->of_node, "ethernet-ports");
> +
> +	if (!ports) {
> +		dev_err(dev, "no ports child node found\n");
> +		return -EINVAL;
> +	}
> +
> +	for_each_available_child_of_node(ports, port) {
> +		struct device_node *link;
> +
> +		enabled_ports++;
> +
> +		link = of_parse_phandle(port, "ethernet", 0);
> +		if (!link)
> +			link = of_parse_phandle(port, "link", 0);
> +
> +		if (!link)
> +			continue;
> +
> +		of_node_put(link);
> +
> +		passthrough_ports++;
> +	}
> +
> +	of_node_put(ports);
> +
> +	/*
> +	 * A switch is considered passthrough if exactly two available
> +	 * ports have an "ethernet" or "link" phandle.
> +	 */
> +	if (passthrough_ports > 2 ||
> +	    (passthrough_ports == 2 && enabled_ports != 2)
> +	) {
> +		dev_err(dev, "Only two ports allowed in passthrough mode\n");
> +		return -EINVAL;
> +	}
> +
> +	return passthrough_ports == 2;
> +}

[Severity: High]
What happens in mt7530_probe_common() -> mt753x_check_passthrough() on
standalone dual-CPU-port boards where two ports (such as port@5 and port@6)
both have "ethernet" phandles and no port has a "link" phandle?

Because mt753x_check_passthrough() counts both "ethernet" and "link" phandles
into passthrough_ports without checking whether any "link" phandle is
present, passthrough_ports is 2 whenever two CPU ports are enabled. If any
user ports are also enabled (enabled_ports > 2, as on mt7621, mt7622,
mt7623n, and mt7986 dual-CPU-port boards), doesn't mt753x_check_passthrough()
return -EINVAL and abort the switch probe? And if all user ports happen to be
disabled (enabled_ports == 2), wouldn't it falsely enable passthrough mode on
a switch with two CPU ports and no downstream switch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029184202.1859843-1-cjd@cjdns.fr?part=3

Re: [PATCH v4 4/7] net: dsa: mediatek: support PLL setup on MMIO MT7530

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: dsa: mt7530: missing child_bus mdio_lock in core_write() and core_rmw()
--

commit 3f6621fc167b9cbb00feef9399db3022a584cb8e
Author: Caleb James DeLisle <cjd@cjdns.fr>

net: dsa: mediatek: support PLL setup on MMIO MT7530

This commit records the switch's internal MDIO bus in priv->child_bus during
mt7530_setup_mdio() and updates core_write() and core_rmw() to fall back to
priv->child_bus when priv->bus is NULL so MMIO MT7530 instances can perform
PLL and core register configuration via the internal PHY MDIO bus.

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index c5b3b00acb956..d45388e2efc83 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -77,6 +77,12 @@ core_write(struct mt7530_priv *priv, u32 reg, u32 val)
>  	struct mii_bus *bus = priv->bus;
>  	int ret;
>  
> +	if (!bus)
> +		bus = priv->child_bus;
> +
> +	if (WARN_ON_ONCE(!bus))
> +		return;
> +
>  	mt7530_mutex_lock(priv);
>  
>  	/* Write the desired MMD Devad */
> @@ -112,6 +118,12 @@ core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set)
>  	u32 val;
>  	int ret;
>  
> +	if (!bus)
> +		bus = priv->child_bus;
> +
> +	if (WARN_ON_ONCE(!bus))
> +		return;
> +
>  	mt7530_mutex_lock(priv);
>  
>  	/* Write the desired MMD Devad */

[Severity: Medium]
When priv->bus is NULL and core_write() or core_rmw() falls back to
priv->child_bus, does mt7530_mutex_lock() leave bus->mdio_lock unlocked?

Looking at mt7530_mutex_lock() and mt7530_mutex_unlock() in
drivers/net/dsa/mt7530.c:

static void
mt7530_mutex_lock(struct mt7530_priv *priv)
{
	if (priv->bus)
		mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
}

static void
mt7530_mutex_unlock(struct mt7530_priv *priv)
{
	if (priv->bus)
		mutex_unlock(&priv->bus->mdio_lock);
}

On MMIO switches where priv->bus is NULL, mt7530_mutex_lock() becomes a
no-op. However, core_write() and core_rmw() perform multi-step Clause 22
MMD indirect sequences across MII_MMD_CTRL and MII_MMD_DATA using raw
bus->write() and bus->read() calls.

Even though the underlying indirect PHY accessors acquire priv->reg_mutex for
each individual Clause 22 register transfer, priv->reg_mutex is released
between each step of the MII_MMD_CTRL and MII_MMD_DATA sequence. Once
mt7530_setup_mdio() registers priv->child_bus, runtime callers such as
mt753x_phylink_mac_config() -> mt7530_mac_config() -> mt7530_setup_port6()
call core_clear(), core_write(), and core_set() (which use core_rmw() and
core_write()). Could concurrent phylib or MDIO accesses on priv->child_bus
(which serialize on priv->child_bus->mdio_lock) interleave between those
steps and corrupt the MMD register accesses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029184202.1859843-1-cjd@cjdns.fr?part=4

Re: [PATCH v4 5/7] net: dsa: mediatek: support MDIO switch downstream of MMIO switch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] net: dsa: mt7530: broken PHY IRQs due to late mt7530_setup_mdio_irq()
- [High] net: dsa: mt7530: premature and re-entrant mt7530_setup_mdio() in probe
- [High] net: dsa: mt7530: unchecked mt7530_setup_mdio() error in mt7988_probe()
- [High] net: dsa: mt7530: reg_mutex lockdep recursion with downstream switch
--

commit 51caf2efd42e68935e207fbaf1614c12f27f5efa
Author: Caleb James DeLisle <cjd@cjdns.fr>

net: dsa: mediatek: support MDIO switch downstream of MMIO switch

This commit exports mt7530_setup_mdio(), makes it idempotent via a check on
priv->child_bus, moves mt7530_setup_mdio_irq() to mt753x_setup(), and calls
mt7530_setup_mdio() from mt7988_probe() after dsa_register_switch() to allow a
downstream MDIO switch to probe and complete the DSA switch tree.

> diff --git a/drivers/net/dsa/mt7530-mmio.c b/drivers/net/dsa/mt7530-mmio.c
> index cf3bdb23a2d54..746a0a4622497 100644
> --- a/drivers/net/dsa/mt7530-mmio.c
> +++ b/drivers/net/dsa/mt7530-mmio.c
> @@ -61,7 +61,21 @@ mt7988_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->regmap))
>  		return PTR_ERR(priv->regmap);
>  
> -	return dsa_register_switch(priv->ds);
> +	ret = dsa_register_switch(priv->ds);
> +
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * If not all switches in the hierarchy have been probed,
> +	 * dsa_register_switch() returns 0 without calling mt753x_setup().
> +	 * Call mt7530_setup_mdio() because there's a chance that the missing
> +	 * switch is on the MDIO bus. Do not call this before
> +	 * dsa_register_switch() as it  may exit with an error.
> +	 */
> +	mt7530_setup_mdio(priv);
> +
> +	return 0;

[Severity: High]
What happens if mt7530_setup_mdio(priv) fails here in mt7988_probe(), for
example with -ENOMEM from devm_mdiobus_alloc() or -EPROBE_DEFER from
devm_of_mdiobus_register()?

Because the return value of mt7530_setup_mdio(priv) is ignored and
mt7988_probe() returns 0, the driver core will treat probe as successful and
never retry deferred probing, leaving the downstream switch unprobed, the DSA
tree permanently incomplete, and priv->ds registered in the switch tree
holding allocated dsa_port instances and conduit net_device references without
unwinding via mt7530_remove_common(priv) / dsa_unregister_switch(priv->ds).
Should mt7988_probe() check the return value of mt7530_setup_mdio(priv) and
unregister the switch on error?

[Severity: High]
Also, when a downstream MDIO switch is registered on the MMIO switch's MDIO
bus (upstream_priv->child_bus), won't register accesses on the downstream
switch trigger a lockdep recursive locking warning on priv->reg_mutex?

Both the upstream MMIO switch and the downstream MDIO switch initialize
priv->reg_mutex at the same callsite in mt7530_probe_common():

drivers/net/dsa/mt7530.c:mt7530_probe_common() {
	...
	mutex_init(&priv->reg_mutex);
	...
}

This assigns both mutexes to the same static lockdep class. When any operation
on the downstream switch holds mutex_lock(&downstream_priv->reg_mutex) at
subclass 0 and performs a regmap access, such as in mt7530_port_disable():

drivers/net/dsa/mt7530.c:mt7530_port_disable() {
	mutex_lock(&priv->reg_mutex);
	priv->ports[port].enable = false;
	regmap_update_bits(priv->regmap, MT7530_PCR_P(port), PCR_MATRIX_MASK,
			   PCR_MATRIX_CLR);
	mutex_unlock(&priv->reg_mutex);
}

the underlying MDIO transfer goes through mt7530_regmap_read() /
mt7530_regmap_write() on downstream_priv->bus (which is
upstream_priv->child_bus) and calls mt7531_ind_c22_phy_read() /
mt7531_ind_c22_phy_write(), which acquires mutex_lock(&upstream_priv->reg_mutex)
at subclass 0 of the same lockdep class:

drivers/net/dsa/mt7530.c:mt7531_ind_c22_phy_write() {
	mutex_lock(&priv->reg_mutex);
	...
}

Does priv->reg_mutex need a separate lockdep key or nesting annotation when
switches are stacked this way?

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index d45388e2efc83..24e4485624d39 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2509,7 +2509,7 @@ mt7530_free_mdio_irq(struct mt7530_priv *priv)
>  	}
>  }
>  
> -static int
> +int
>  mt7530_setup_mdio(struct mt7530_priv *priv)
>  {
>  	struct device_node *mnp, *np = priv->dev->of_node;
> @@ -2519,6 +2519,10 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
>  	static int idx;
>  	int ret = 0;
>  
> +	/* Already done */
> +	if (priv->child_bus)
> +		return 0;
> +
>  	mnp = of_get_child_by_name(np, "mdio");
>  
>  	if (mnp && !of_device_is_available(mnp))
> @@ -2543,9 +2547,6 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
>  	bus->parent = dev;
>  	bus->phy_mask = ~ds->phys_mii_mask;
>  
> -	if (priv->irq_domain && !mnp)
> -		mt7530_setup_mdio_irq(priv);
> -
>  	priv->child_bus = bus;

[Severity: High]
When mt7988_probe() calls mt7530_setup_mdio(priv) after dsa_register_switch()
returns 0 on an incomplete DSA tree, neither dsa_switch_setup() nor
mt753x_setup() has executed yet. Can calling mt7530_setup_mdio() at that point
cause four ordering and re-entrancy regressions?

1. Hardware reset and PHY clock setup in mt7988_setup() have not run yet when
mt7988_probe() calls mt7530_setup_mdio(priv). In mt7988_setup():

drivers/net/dsa/mt7530.c:mt7988_setup() {
	/* Reset the switch */
	reset_control_assert(priv->rstc);
	usleep_range(20, 50);
	reset_control_deassert(priv->rstc);
	usleep_range(20, 50);

	/* AN7583 require additional tweak to CONN_CFG */
	if (priv->id == ID_AN7583)
		regmap_update_bits(priv->regmap, AN7583_GEPHY_CONN_CFG, ...);

	/* Reset the switch PHYs */
	regmap_write(priv->regmap, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
}

MDIO accesses in mt7530_setup_mdio() happen before priv->rstc is deasserted
and before AN7583_GEPHY_CONN_CFG is configured. Then, when the downstream
switch registers and completes the DSA tree, dsa_tree_setup() invokes
mt753x_setup() -> mt7988_setup() on the upstream switch, asserting priv->rstc
and writing SYS_CTRL_PHY_RST after MDIO devices and PHYs on the bus have
already been probed.

2. mt7530_setup_irq(priv) is only called from mt753x_setup(), so
priv->irq_domain is still NULL when mt7988_probe() calls
mt7530_setup_mdio(priv). Any child PHY or switch node under the mdio DT node
whose interrupt-parent is the upstream switch will fail fwnode_irq_get() with
-EPROBE_DEFER during devm_of_mdiobus_register().

3. ds->phys_mii_mask is only populated by dsa_switch_setup():

net/dsa/dsa.c:dsa_switch_setup() {
	...
	ds->phys_mii_mask |= dsa_user_ports(ds);
	...
}

When mt7988_probe() calls mt7530_setup_mdio(priv) on an incomplete tree,
ds->phys_mii_mask is still 0, so bus->phy_mask = ~ds->phys_mii_mask is
0xffffffff. If there is no mdio child DT node (mnp == NULL),
devm_of_mdiobus_register(dev, bus, NULL) scans no PHYs and sets
priv->child_bus = bus. When the tree later completes and mt753x_setup() calls
mt7530_setup_mdio(priv), it returns 0 immediately because priv->child_bus is
already set, leaving ds->user_mii_bus with no PHYs.

4. Because priv->child_bus = bus is set before devm_of_mdiobus_register(dev,
bus, mnp) finishes iterating all child nodes of mnp in
__of_mdiobus_parse_phys(), probing the downstream switch child node
synchronously calls:

mt7530_probe()
  -> dsa_register_switch()
    -> dsa_tree_setup()
      -> dsa_tree_setup_switches()
        -> mt753x_setup()
          -> mt7530_setup_mdio()

That re-entrant mt7530_setup_mdio() call sees priv->child_bus != NULL and
returns 0 while the outer __of_mdiobus_parse_phys() loop is still mid-iteration
and has not yet registered any PHY child nodes listed after the downstream
switch node in mnp. dsa_tree_setup_ports() then runs immediately and fails
phylink_of_phy_connect() with -ENODEV for those upstream user ports.

[ ... ]

> @@ -3386,10 +3388,17 @@ mt753x_setup(struct dsa_switch *ds)
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * This must be called before mt753x_setup() returns because the DSA
> +	 * core synchronously sets up ports and expects PHYs to be operational.
> +	 */
>  	ret = mt7530_setup_mdio(priv);
>  	if (ret)
>  		return ret;
>  
> +	if (priv->irq_domain && ds->user_mii_bus)
> +		mt7530_setup_mdio_irq(priv);

[Severity: High]
Doesn't moving mt7530_setup_mdio_irq(priv) after mt7530_setup_mdio(priv) break
PHY interrupts for all switches using ds->user_mii_bus?

In mt7530_setup_mdio(), devm_mdiobus_alloc() initializes all bus->irq[i]
entries to PHY_POLL. When mnp is NULL, devm_of_mdiobus_register(dev, bus, NULL)
synchronously scans the bus and creates the phy_device instances via:

devm_of_mdiobus_register()
  -> __of_mdiobus_register()
    -> __mdiobus_register()
      -> mdiobus_scan_bus_c22()
        -> mdiobus_scan_c22()
          -> get_phy_device()
            -> phy_device_create()

During phy_device_create(), phylib copies bus->irq[addr] into each struct
phy_device:

drivers/net/phy/phy_device.c:phy_device_create() {
	...
	dev->irq = bus->irq[addr];
	...
}

Because mt7530_setup_mdio_irq(priv) now runs after mt7530_setup_mdio(priv) has
already registered the bus and created the PHY devices, bus->irq[addr] is still
PHY_POLL at creation time, so every PHY is created with phydev->irq == PHY_POLL
and populating ds->user_mii_bus->irq[p] afterwards has no effect on the
already-created PHY devices (while leaking the mapped IRQ descriptors).

In addition, the error path in mt7530_setup_mdio() still calls
mt7530_free_mdio_irq(priv) when devm_of_mdiobus_register() fails:

drivers/net/dsa/mt7530.c:mt7530_setup_mdio() {
	...
	ret = devm_of_mdiobus_register(dev, bus, mnp);
	if (ret) {
		priv->child_bus = NULL;
		dev_err(dev, "failed to register MDIO bus: %d\n", ret);
		if (priv->irq_domain && !mnp)
			mt7530_free_mdio_irq(priv);
	}
}

even though mt7530_setup_mdio_irq(priv) was not called before
devm_of_mdiobus_register().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029184202.1859843-1-cjd@cjdns.fr?part=5

Re: [PATCH v4 7/7] net: dsa: mediatek: support EN751221 switch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] net: dsa: mt7530: missing MT7530_CHG_TRAP setup for ID_EN751221
- [High] net: dsa: mt7530: recursive reg_mutex lockdep class in MMIO-MDIO hierarchy
- [Medium] net: dsa: mt7530: swapped default_taps in en751221_trgmii_pair_setup()
- [Medium] net: dsa: mt7530: missing child_bus mdio_lock in core_write() and core_rmw()
- [Medium] net: dsa: mt7530: race with peer phylink in mt7530_cal_interswitch_trgmii()
--

commit fcbb4fdaccb60b7ebd8ef125ae65f04744031511
Author: Caleb James DeLisle <cjd@cjdns.fr>

net: dsa: mediatek: support EN751221 switch

This patch adds support for the EcoNet EN751221 on-die MT7530 switch and the
optional external MCM MT7530 switch, along with startup calibration for the
TRGMII link connecting them.

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 24e4485624d39..447c80455460e 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -49,6 +49,9 @@ static const struct mt7530_mib_desc mt7530_mib[] = {
>  static int
>  mt753x_ctrl_phy_addr(struct mt7530_priv *priv)
>  {
> +	if (priv->id == ID_EN751221)
> +		return 12;
> +
>  	if (WARN_ON_ONCE(!priv->mdiodev))
>  		return 0;
>  
> @@ -349,12 +352,19 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
>  	regmap_read(priv->regmap, MT753X_MTRAP, &xtal);
>  	xtal &= MT7530_XTAL_MASK;
>  
> +	/* EN751221 on-die does not report clock speed in MTRAP */
> +	if (priv->id == ID_EN751221)
> +		xtal = MT7530_XTAL_25MHZ;
> +
>  	if (xtal == MT7530_XTAL_25MHZ)
>  		ssc_delta = 0x57;
>  	else
>  		ssc_delta = 0x87;
>  
> -	if (priv->id == ID_MT7621) {
> +	if (priv->id == ID_EN751221_EXT || priv->id == ID_EN751221) {
> +		/* PLL frequency: 362.5Mhz */
> +		ncpo1 = 0x1d00;
> +	} else if (priv->id == ID_MT7621) {

[Severity: Medium]
When mt7530_mac_config() calls mt7530_setup_port6() on ID_EN751221, it invokes
core_clear(), core_write(), and core_set() on control PHY address 12.

On the MMIO-based ID_EN751221 switch, priv->bus is NULL and core_write() and
core_rmw() fall back to priv->child_bus:

drivers/net/dsa/mt7530.c:mt7530_mutex_lock() {
	if (priv->bus)
		mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
}

drivers/net/dsa/mt7530.c:core_write() {
	...
	if (!bus)
		bus = priv->child_bus;

	if (WARN_ON_ONCE(!bus))
		return;

	mt7530_mutex_lock(priv);

	/* Write the desired MMD Devad */
	ret = bus->write(bus, ctl_phy, MII_MMD_CTRL, MDIO_MMD_VEND2);
	...
}

Because mt7530_mutex_lock() and mt7530_mutex_unlock() only check priv->bus,
priv->child_bus->mdio_lock is not acquired when priv->bus is NULL. While
mt7531_ind_c22_phy_read() and mt7531_ind_c22_phy_write() acquire
priv->reg_mutex for each individual Clause 22 transfer, can the multi-step
MII_MMD_CTRL / MII_MMD_DATA sequence and read-modify-write in core_write() and
core_rmw() interleave with concurrent MDIO transactions on priv->child_bus
(such as phylib polling or regmap accesses to the downstream ID_EN751221_EXT
switch)?

[ ... ]

> @@ -471,6 +481,251 @@ mt7531_pll_setup(struct mt7530_priv *priv)
>  	usleep_range(25, 35);
>  }
>  
> +static bool
> +en751221_trgmii_cal_ok(struct mt7530_priv *rx, u32 reg)
> +{
> +	u32 val = 0;
> +
> +	regmap_read(rx->regmap, reg, &val);
> +	regmap_write(rx->regmap, reg, val | EDGE_CHK);
> +	regmap_write(rx->regmap, reg, val & ~EDGE_CHK);
> +	regmap_read(rx->regmap, reg, &val);
> +
> +	return FIELD_GET(RD_VALUE_MASK, val) == TGMII_TD_PAT &&
> +	       !FIELD_GET(RD_ERR_MASK, val);
> +}
> +
> +static void
> +en751221_trgmii_calibrate_direction(struct mt7530_priv *tx,
> +				    struct mt7530_priv *rx,
> +				    u8 *default_taps)
> +{

[ ... ]

> +		if (best_tap > -1) {
> +			regmap_update_bits(rx->regmap, rx_reg, RD_TAP_MASK,
> +					   RD_TAP(best_tap));
> +
> +			log[best_tap] = 'X';
> +
> +			dev_info(rx->dev,
> +				 "TRGMII lane %d: %s tap %d old %d\n",
> +				 channel, log, best_tap, default_taps[channel]);
> +		} else {
> +			regmap_update_bits(rx->regmap, rx_reg, RD_TAP_MASK,
> +					   RD_TAP(default_taps[channel]));
> +
> +			dev_warn(rx->dev,
> +				 "TRGMII lane %d: %s calibration failed, preserving tap %u\n",
> +				 channel, log, default_taps[channel]);
> +		}

[ ... ]

> +static void
> +en751221_trgmii_pair_setup(struct mt7530_priv *ext, int ext_port,
> +			   struct mt7530_priv *ondie, int ondie_port)
> +{

[ ... ]

> +	/* BOTH: Collect and then zero every RX TAP */
> +	for (channel = 0; channel < NUM_TRGMII_CTRL; channel++) {
> +		reg_val = 0;
> +		regmap_read(ext->regmap, MT7530_TRGMII_RD(channel), &reg_val);
> +		default_taps_ext[channel] = FIELD_GET(RD_TAP_MASK, reg_val);
> +		regmap_clear_bits(ext->regmap, MT7530_TRGMII_RD(channel),
> +				  RD_TAP_MASK);
> +
> +		reg_val = 0;
> +		regmap_read(ondie->regmap, MT7530_TRGMII_RD(channel), &reg_val);
> +		default_taps_ondie[channel] = FIELD_GET(RD_TAP_MASK, reg_val);
> +		regmap_clear_bits(ondie->regmap, MT7530_TRGMII_RD(channel),
> +				  RD_TAP_MASK);
> +	}
> +
> +	en751221_trgmii_calibrate_direction(ondie, ext, default_taps_ondie);
> +	en751221_trgmii_calibrate_direction(ext, ondie, default_taps_ext);

[Severity: Medium]
Are default_taps_ondie and default_taps_ext swapped in these two calls to
en751221_trgmii_calibrate_direction()?

In en751221_trgmii_calibrate_direction(tx, rx, default_taps), default_taps is
used to log the previous RX tap value on rx->dev and to restore rx->regmap's
MT7530_TRGMII_RD(channel) register if calibration fails. Passing
default_taps_ondie when rx is ext (and default_taps_ext when rx is ondie) uses
the peer switch's saved RX taps instead of rx's own saved taps.

> +
> +	dev_info(ondie->dev, "TRGMII inter-switch link initialized\n");
> +}
> +
> +static void
> +mt7530_cal_interswitch_trgmii(struct dsa_port *dp)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +	struct dsa_switch *peer_ds;
> +	struct dsa_port *peer_dp;
> +	struct mt7530_priv *peer;
> +	struct mt7530_priv *priv;
> +	struct dsa_link *dl;
> +	u32 pmsr = 0;
> +
> +	priv = ds->priv;
> +
> +	if (priv->id != ID_EN751221_EXT)
> +		return;
> +
> +	if (dp->type != DSA_PORT_TYPE_DSA)
> +		return;
> +
> +	regmap_read(priv->regmap, MT7530_PMSR_P(dp->index), &pmsr);
> +
> +	if (!(pmsr & PMSR_LINK))
> +		return;
> +
> +	list_for_each_entry(dl, &ds->dst->rtable, list) {
> +		if (dl->dp != dp)
> +			continue;
> +
> +		peer_dp = dl->link_dp;
> +
> +		if (peer_dp->type != DSA_PORT_TYPE_DSA)
> +			continue;
> +
> +		peer_ds = dl->link_dp->ds;
> +
> +		if (!of_device_is_compatible(peer_ds->dev->of_node,
> +					     "econet,en751221-switch"))
> +			continue;
> +
> +		peer = peer_ds->priv;
> +
> +		if (peer->id != ID_EN751221)
> +			continue;
> +
> +		en751221_trgmii_pair_setup(priv, dp->index, peer,
> +					   peer_dp->index);
> +	}
> +}

[Severity: Medium]
Can mt7530_cal_interswitch_trgmii() race with the peer port's phylink_resolve()
work item?

When dsa_tree_setup_ports() enables the DSA ports on both switches, each port
schedules its own phylink_resolve() work item holding only its own
dp->pl->state_mutex. In mt753x_phylink_mac_link_up(), priv->regmap's
MT753X_PMCR_P(dp->index) has just been written with PMCR_FORCE_LNK right
before calling mt7530_cal_interswitch_trgmii(dp), so checking PMSR_LINK on
priv->regmap checks the local port rather than peer->regmap's
MT7530_PMSR_P(peer_dp->index).

While en751221_trgmii_pair_setup() forces MT753X_PMCR_P(ondie_port) down and
sleeps in usleep_range(5000, 6000) twice during TX/RX reset, could peer_dp's
phylink_resolve() run concurrently on another worker and call
mt753x_phylink_mac_link_up() on ondie_port mid-reset? Also, when
en751221_trgmii_pair_setup() writes mcr_up to MT753X_PMCR_P on both ports,
does it overwrite PMCR_FORCE_TX_FC_EN and PMCR_FORCE_RX_FC_EN configured by
mt753x_phylink_mac_link_up()?

> @@ -1459,7 +1714,8 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
>  
>  	mutex_unlock(&priv->reg_mutex);
>  
> -	if (priv->id != ID_MT7530 && priv->id != ID_MT7621)
> +	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
> +	    priv->id != ID_EN751221 && priv->id != ID_EN751221_EXT)
>  		return 0;
>  
>  	if (port == 5)
> @@ -1486,7 +1742,8 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
>  
>  	mutex_unlock(&priv->reg_mutex);
>  
> -	if (priv->id != ID_MT7530 && priv->id != ID_MT7621)
> +	if (priv->id != ID_MT7530 && priv->id != ID_MT7621 &&
> +	    priv->id != ID_EN751221 && priv->id != ID_EN751221_EXT)
>  		return;

[Severity: High]
Do the updates to MT753X_MTRAP for ID_EN751221 in mt7530_port_enable(),
mt7530_port_disable(), mt7530_setup_port5(), and mt7531_setup_common() take
effect without MT7530_CHG_TRAP being set?

In mt753x_table, ID_EN751221 uses mt7988_setup() -> mt7531_setup_common().
Unlike mt7530_setup(), which sets MT7530_CHG_TRAP in MT753X_MTRAP after
resetting the switch so that trap bits can be modified by software,
mt7988_setup() asserts and deasserts priv->rstc and then calls
mt7531_setup_common() without setting MT7530_CHG_TRAP in MT753X_MTRAP.

[ ... ]

> @@ -3817,6 +4122,28 @@ const struct mt753x_info mt753x_table[] = {
>  		.phy_write_c45 = mt7531_ind_c45_phy_write,
>  		.mac_port_get_caps = en7528_mac_port_get_caps,
>  	},
> +	[ID_EN751221] = {
> +		.id = ID_EN751221,
> +		.pcs_ops = &mt7530_pcs_ops,
> +		.sw_setup = mt7988_setup,
> +		.phy_read_c22 = mt7531_ind_c22_phy_read,
> +		.phy_write_c22 = mt7531_ind_c22_phy_write,
> +		.phy_read_c45 = mt7531_ind_c45_phy_read,
> +		.phy_write_c45 = mt7531_ind_c45_phy_write,
> +		.mac_port_get_caps = en751221_mac_port_get_caps,
> +		.mac_port_config = mt7530_mac_config,
> +	},
> +	[ID_EN751221_EXT] = {
> +		.id = ID_EN751221_EXT,
> +		.pcs_ops = &mt7530_pcs_ops,
> +		.sw_setup = mt7530_setup,
> +		.phy_read_c22 = mt7530_phy_read_c22,
> +		.phy_write_c22 = mt7530_phy_write_c22,
> +		.phy_read_c45 = mt7530_phy_read_c45,
> +		.phy_write_c45 = mt7530_phy_write_c45,
> +		.mac_port_get_caps = mt7530_mac_port_get_caps,
> +		.mac_port_config = mt7530_mac_config,
> +	},

[Severity: High]
When ID_EN751221_EXT is registered as an MDIO device on ID_EN751221's internal
MDIO bus (ondie->child_bus), both switches initialize priv->reg_mutex in
mt7530_probe_common() with the same lockdep class key.

When an operation on the downstream ID_EN751221_EXT switch holds
ext->reg_mutex and accesses ext->regmap, the regmap MDIO callbacks invoke the
parent switch's child_bus operations, which acquire ondie->reg_mutex:

mt7530_setup() (on ext)
  mt7530_port_disable()
    mutex_lock(&ext->reg_mutex)
    regmap_update_bits(ext->regmap, ...)
      mt7530_regmap_write()
        bus->write() (on ondie->child_bus)
          mt753x_phy_write_c22()
            mt7531_ind_c22_phy_write()
              mutex_lock(&ondie->reg_mutex)

Will this nested acquisition of ondie->reg_mutex while ext->reg_mutex is held
trigger a lockdep recursive locking warning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029184202.1859843-1-cjd@cjdns.fr?part=7