[PATCH net-next v4 0/3] net: dsa: qca8k: add a QCA8337 CPU PHY consumer

[PATCH net-next v4 0/3] net: dsa: qca8k: add a QCA8337 CPU PHY consumer

[Yongzhao Chen]

This series lets a QCA8337 use one of its internal PHYs (ports 1-5) as
the CPU port, with a PHY-to-PHY connection to the SoC's own PHY.

Patch 1 makes DSA pass get_phy_flags() to PHYs on CPU and DSA ports, so
that the internal CPU PHY receives the switch revision and gets the same
revision-specific initialization as the user ports. Patch 2 pauses
internal CPU ports as well as ports 0 and 6 during MTU changes and
serializes that sequence with reg_mutex. Patch 3 accepts an internal PHY
as the CPU port on QCA8337.

Changes since v3:
- Dropped v3 patches 4/5 and 5/5, which disabled SmartSpeed on internal
  CPU PHYs, and did not add the DT workaround property discussed in the
  v3 4/5 thread. The lost 1000BASE-T advertisement on the RA74 CPU link
  came from the IPQ5018 GE PHY's initialization order, not from the
  QCA8337. The IPQ5018 driver writes its analog settings (LDO, EEE, MSE,
  DAC) only in config_init() when the MAC attaches, about 38 s after
  PHY4 starts autonegotiating. Until then the IPQ5018 PHY negotiates with
  its reset defaults, 1000BASE-T does not come up, and SmartSpeed on
  both sides drops the 1000BASE-T advertisement. Nothing restores it on
  PHY4. On RA74, applying the settings at probe brought the CPU link up
  at 1 Gb/s with SmartSpeed left enabled. That fix is a separate net
  series:
    net: phy: qcom: at803x: Fix IPQ5018 short-cable DAC values
    net: phy: qcom: at803x: Apply IPQ5018 analog settings at probe
    <https://lore.kernel.org/netdev/20260928220717.939-1-yongzhao.derek@gmail.com/>
- Patch 1: removed the SmartSpeed reference from the commit message.
  The code is unchanged, so I kept Florian's Reviewed-by.
- Patch 3: the commit message no longer refers to an RFC. Internal CPU
  port selection is still limited to QCA8337; QCA8327 support has not
  been established. The code is unchanged.
- Patch 2 is unchanged.
- Posted as a regular patch series instead of an RFC.

Dependencies:
This series depends on the net series "[PATCH net v2 0/3] net: dsa:
qca8k: fix MDIO error handling" <https://lore.kernel.org/netdev/20260928220629.238-1-yongzhao.derek@gmail.com/>. It applies to
net-next without that series, but patch 2 aborts and restores the
paused ports only when register writes report MDIO failures, which
needs its first patch. Please take the net series first.

Brandon Mahdavi's multi-CPU RFC v2 [1] also replaces
qca8k_find_cpu_port() and restricts CPU ports to 0 and 6. It is not
merged. Whichever series lands second will need to handle the other's
CPU port selection.

Testing:
Each patch builds with W=1 for arm64 (drivers/net/dsa/qca, net/dsa,
drivers/net/phy/qcom, bcm_sf2, microchip) on the net-next base below
with the MDIO series, and checkpatch --strict is clean apart from
sign-offs.
The MTU/pause model test from v3 passes on patch 2 on top of the MDIO
series. A model test of patch 3 checks CPU port selection for every
CPU port mask on QCA8337 and QCA8327, and which ports count for the
MDIO bus choice. The code of all three patches is unchanged from v3.

On hardware, a Redmi AX5400 (IPQ5018 GE PHY to QCA8337 PHY4 as the CPU
port, plus port 6 as a second CPU port) was tested on OpenWrt's Linux
6.18.52 with a backport of all three patches, the MDIO series and the at803x
fixes, and no SmartSpeed change. OpenWrt's multi-CPU patches stay on
top and remove qca8k_find_cpu_port(), so on this board patch 3 only
contributes its MDIO scope change; its CPU port fallback is covered by
the model test only. The CPU link was verified at 1 Gb/s with SmartSpeed
enabled after a first boot, three reboots, a power-off cold boot, three
interface down/up cycles, three renegotiations and a network restart.
During a separate 10-minute observation, sampled link status remained
at 1 Gb/s with SmartSpeed enabled and no new CPU PHY link-down events
were logged. Raising a user port MTU to 1504 and back exercises patch
2's pause sequence. After each change, WAN HTTPS through CPU port 5
and SSH through CPU port 6 worked. Packet loss during the changes was
not measured. An MTU above the maximum was rejected. The switch
registers are not readable on that image, so MAX_FRAME_SIZE and the
MAC enable bits were not checked directly. There is no net-next boot
on this board, as it has no upstream DTS.

Thanks to Andrew Lunn for his detailed reviews of v3 4/5, and to Ziyang
Huang for the OpenWrt PHY-to-PHY CPU port code that patch 3 is based on
and for pointing me at the IPQ5018 DAC settings.

v3: https://lore.kernel.org/netdev/20260923215858.1653-1-yongzhao.derek@gmail.com/
v2: https://lore.kernel.org/netdev/20260922202653.1153-1-yongzhao.derek@gmail.com/
v1: https://lore.kernel.org/netdev/20260919085406.1395-1-yongzhao.derek@gmail.com/

[1] https://lore.kernel.org/netdev/20260802034701.3339052-1-brandon.mahdavi@mahcom.com/

Yongzhao Chen (2):
  net: dsa: pass PHY flags when connecting shared ports
  net: dsa: qca8k: serialize CPU MAC pause during MTU changes

Ziyang Huang (1):
  net: dsa: qca8k: support QCA8337 internal PHY CPU links

 Documentation/networking/dsa/dsa.rst |  2 +
 drivers/net/dsa/bcm_sf2.c            |  4 ++
 drivers/net/dsa/microchip/ksz8.c     |  4 ++
 drivers/net/dsa/qca/qca8k-8xxx.c     | 20 +++++--
 drivers/net/dsa/qca/qca8k-common.c   | 79 ++++++++++++++++++++++------
 net/dsa/port.c                       |  6 ++-
 6 files changed, 94 insertions(+), 21 deletions(-)


base-commit: 014d795c73837ea2339a4ea8e8f82c6e959b845d
prerequisite-patch-id: aad3a7fefa50cc4a8d29711256b182827e5b3bf2
prerequisite-patch-id: 92a623b4a20a1d2d516476558e3904d7a1c17b8f
prerequisite-patch-id: 2819bb7310717899a1fb577b9f4c71a6443a4340
-- 
2.43.0

[PATCH net-next v4 1/3] net: dsa: pass PHY flags when connecting shared ports

[Yongzhao Chen]

DSA calls get_phy_flags() for user ports, but passes zero when connecting
CPU or DSA port PHYs. Pass the callback result before PHY initialization
for shared ports too. Drivers without the callback still pass zero.

Keep bcm_sf2 and ksz88xx shared-port flags at zero, preserving their
existing behavior. Document the extended callback scope.

This lets qca8k pass its switch revision to an internal PHY used as a
CPU link, so the PHY driver applies the same revision-specific
initialization as on user ports. A later patch in this series adds
that user.

Assisted-by: LLM
Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 Documentation/networking/dsa/dsa.rst | 2 ++
 drivers/net/dsa/bcm_sf2.c            | 4 ++++
 drivers/net/dsa/microchip/ksz8.c     | 4 ++++
 net/dsa/port.c                       | 6 +++++-
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 7edfdd555f0..647f952e397 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -668,6 +668,8 @@ PHY devices and link management
   on its own (e.g.: coming from switch memory mapped registers), this function
   should return a 32-bit bitmask of "flags" that is private between the switch
   driver and the Ethernet PHY driver in ``drivers/net/phy/\*``.
+  It is called when connecting PHYs for user, CPU and DSA ports. Drivers
+  should return zero for ports that do not need switch-specific PHY flags.
 
 - ``phy_read``: Function invoked by the DSA user MDIO bus when attempting to read
   the switch port MDIO registers. If unavailable, return 0xffff for each read.
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 9e571301518..f516fc396c8 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -709,6 +709,10 @@ static u32 bcm_sf2_sw_get_phy_flags(struct dsa_switch *ds, int port)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 
+	/* Shared ports previously received no PHY flags. */
+	if (!dsa_is_user_port(ds, port))
+		return 0;
+
 	/* The BCM7xxx PHY driver expects to find the integrated PHY revision
 	 * in bits 15:8 and the patch level in bits 7:0 which is exactly what
 	 * the REG_PHY_REVISION register layout is.
diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
index d7498132064..be8861a7a76 100644
--- a/drivers/net/dsa/microchip/ksz8.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -3076,6 +3076,10 @@ static u32 ksz88xx_get_phy_flags(struct dsa_switch *ds, int port)
 {
 	struct ksz_device *dev = ds->priv;
 
+	/* Shared ports previously received no PHY flags. */
+	if (!dsa_is_user_port(ds, port))
+		return 0;
+
 	switch (dev->chip_id) {
 	case KSZ88X3_CHIP_ID:
 		/* Silicon Errata Sheet (DS80000830A):
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 1f5536c0dff..4db7e6f9ce5 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1666,6 +1666,7 @@ static int dsa_shared_port_phylink_register(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct device_node *port_dn = dp->dn;
+	u32 phy_flags = 0;
 	int err;
 
 	dp->pl_config.dev = ds->dev;
@@ -1675,7 +1676,10 @@ static int dsa_shared_port_phylink_register(struct dsa_port *dp)
 	if (err)
 		return err;
 
-	err = phylink_of_phy_connect(dp->pl, port_dn, 0);
+	if (ds->ops->get_phy_flags)
+		phy_flags = ds->ops->get_phy_flags(ds, dp->index);
+
+	err = phylink_of_phy_connect(dp->pl, port_dn, phy_flags);
 	if (err && err != -ENODEV) {
 		pr_err("could not attach to PHY: %d\n", err);
 		goto err_phy_connect;
-- 
2.43.0

[PATCH net-next v4 2/3] net: dsa: qca8k: serialize CPU MAC pause during MTU changes

[Yongzhao Chen]

The global maximum frame size must be updated with CPU MACs disabled.
The previous logic only paused ports 0 and 6, leaving an internal PHY
CPU port enabled while modifying the register.

Include enabled internal CPU ports in the pause sequence. Use the
existing reg_mutex to serialize the MTU update against port enable, port
disable, and phylink link-up and link-down transitions. Read and restore
each port's original TXMAC and RXMAC bits, ensuring ports that were down
remain down and preserving LINK_AUTO. Retain existing handling for ports
0 and 6.

Abort before updating the frame size if reading port status or pausing
the MAC fails. Attempt to restore all ports already modified, and report
any restoration failures even if an earlier error occurred.

The standalone qca8k MDIO error-propagation fix is a prerequisite for
this series; that error-handling bug predates this locking change.

Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
Assisted-by: LLM
---
 drivers/net/dsa/qca/qca8k-8xxx.c   |  2 +
 drivers/net/dsa/qca/qca8k-common.c | 79 ++++++++++++++++++++++++------
 2 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 89113d22d5d..7bd9d9abcef 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1495,7 +1495,9 @@ qca8k_phylink_mac_link_up(struct phylink_config *config,
 
 	reg |= QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
 
+	mutex_lock(&priv->reg_mutex);
 	qca8k_write(priv, QCA8K_REG_PORT_STATUS(port), reg);
+	mutex_unlock(&priv->reg_mutex);
 }
 
 static struct qca8k_pcs *pcs_to_qca8k_pcs(struct phylink_pcs *pcs)
diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
index 13005f10edb..6b32bdd75ea 100644
--- a/drivers/net/dsa/qca/qca8k-common.c
+++ b/drivers/net/dsa/qca/qca8k-common.c
@@ -463,7 +463,8 @@ int qca8k_mib_init(struct qca8k_priv *priv)
 	return ret;
 }
 
-void qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
+static void qca8k_port_set_status_locked(struct qca8k_priv *priv, int port,
+					 int enable)
 {
 	u32 mask = QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
 
@@ -477,6 +478,13 @@ void qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
 		regmap_clear_bits(priv->regmap, QCA8K_REG_PORT_STATUS(port), mask);
 }
 
+void qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
+{
+	mutex_lock(&priv->reg_mutex);
+	qca8k_port_set_status_locked(priv, port, enable);
+	mutex_unlock(&priv->reg_mutex);
+}
+
 void qca8k_get_strings(struct dsa_switch *ds, int port, u32 stringset,
 		       uint8_t *data)
 {
@@ -751,8 +759,10 @@ int qca8k_port_enable(struct dsa_switch *ds, int port,
 {
 	struct qca8k_priv *priv = ds->priv;
 
-	qca8k_port_set_status(priv, port, 1);
+	mutex_lock(&priv->reg_mutex);
+	qca8k_port_set_status_locked(priv, port, 1);
 	priv->port_enabled_map |= BIT(port);
+	mutex_unlock(&priv->reg_mutex);
 
 	if (dsa_is_user_port(ds, port))
 		phy_support_asym_pause(phy);
@@ -764,14 +774,20 @@ void qca8k_port_disable(struct dsa_switch *ds, int port)
 {
 	struct qca8k_priv *priv = ds->priv;
 
-	qca8k_port_set_status(priv, port, 0);
+	mutex_lock(&priv->reg_mutex);
+	qca8k_port_set_status_locked(priv, port, 0);
 	priv->port_enabled_map &= ~BIT(port);
+	mutex_unlock(&priv->reg_mutex);
 }
 
 int qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 {
+	u32 mask = QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
 	struct qca8k_priv *priv = ds->priv;
-	int ret;
+	u32 status[QCA8K_NUM_PORTS] = { 0 };
+	int ret, restore_ret, i;
+	u32 stopped = 0;
+	u32 ports;
 
 	/* We have only have a general MTU setting.
 	 * DSA always set the CPU port's MTU to the largest MTU of the user
@@ -784,25 +800,58 @@ int qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 
 	/* To change the MAX_FRAME_SIZE the cpu ports must be off or
 	 * the switch panics.
-	 * Turn off both cpu ports before applying the new value to prevent
-	 * this.
+	 * Include internal PHY CPU ports as well as the two MAC-only ports.
+	 * Toggle only MAC enables, preserving the phylink link-control mode.
 	 */
-	if (priv->port_enabled_map & BIT(0))
-		qca8k_port_set_status(priv, 0, 0);
+	ports = BIT(0) | BIT(6);
+	for (i = 1; i < 6; i++)
+		if (dsa_is_cpu_port(ds, i))
+			ports |= BIT(i);
 
-	if (priv->port_enabled_map & BIT(6))
-		qca8k_port_set_status(priv, 6, 0);
+	mutex_lock(&priv->reg_mutex);
+	ports &= priv->port_enabled_map;
+
+	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		if (!(ports & BIT(i)))
+			continue;
+
+		ret = regmap_read(priv->regmap, QCA8K_REG_PORT_STATUS(i),
+				  &status[i]);
+		if (ret)
+			goto unlock;
+	}
+
+	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
+		if (!(ports & BIT(i)) || !(status[i] & mask))
+			continue;
+
+		stopped |= BIT(i);
+		ret = regmap_clear_bits(priv->regmap, QCA8K_REG_PORT_STATUS(i),
+					mask);
+		if (ret)
+			goto restore;
+	}
 
 	/* Include L2 header / FCS length */
 	ret = qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, new_mtu +
 			  ETH_HLEN + ETH_FCS_LEN);
 
-	if (priv->port_enabled_map & BIT(0))
-		qca8k_port_set_status(priv, 0, 1);
-
-	if (priv->port_enabled_map & BIT(6))
-		qca8k_port_set_status(priv, 6, 1);
+restore:
+	for (i = 0; i < QCA8K_NUM_PORTS; i++)
+		if (stopped & BIT(i)) {
+			restore_ret = regmap_update_bits(priv->regmap,
+							 QCA8K_REG_PORT_STATUS(i),
+							 mask, status[i] & mask);
+			if (restore_ret) {
+				dev_err(priv->dev, "failed to restore MAC state on port %d: %d\n",
+					i, restore_ret);
+				if (!ret)
+					ret = restore_ret;
+			}
+		}
 
+unlock:
+	mutex_unlock(&priv->reg_mutex);
 	return ret;
 }
 
-- 
2.43.0

[PATCH net-next v4 3/3] net: dsa: qca8k: support QCA8337 internal PHY CPU links

[Yongzhao Chen]

From: Ziyang Huang <hzyitc@outlook.com>

A PHY-to-PHY CPU link connects the SoC PHY directly to an internal
switch PHY. The QCA8337 supports header mode on these ports, and
existing phylink callbacks already handle their internal interfaces.

Allow QCA8337 CPU port selection to fall back to ports 1 through 5 after
checking the dedicated MAC-only ports. Preserve the preference for CPU
ports 0 and 6 across all switch models, limiting the internal-port
fallback to QCA8337. Support for QCA8327 internal CPU links has not
been established and is not enabled. Include internal CPU PHYs when
selecting the PHY access method, without altering the handling of
MAC-only or external user ports.

This enables a single internal CPU PHY configured with an explicit
phy-handle and phy-mode = "internal". The conduit interface uses its own
PHY on the opposite side of the MDI connection. Existing single-CPU DSA
forwarding uses the selected port for default flooding and membership
without extra routing changes.

Adapted from the OpenWrt PHY-to-PHY CPU link patch, narrowing the MDIO
filter adjustments to preserve handling for external user ports.

[yongzhao: preserve external user-port handling on port 6 and limit
 internal CPU PHY support to QCA8337]

Assisted-by: LLM
Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 7bd9d9abcef..cd36efd1c4f 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1026,7 +1026,8 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
 			return ret;
 		}
 
-		if (!dsa_is_user_port(priv->ds, reg))
+		if (!dsa_is_user_port(priv->ds, reg) &&
+		    !(reg > 0 && reg < 6 && dsa_is_cpu_port(priv->ds, reg)))
 			continue;
 
 		of_get_phy_mode(port, &mode);
@@ -1102,16 +1103,23 @@ qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
 static int qca8k_find_cpu_port(struct dsa_switch *ds)
 {
 	struct qca8k_priv *priv = ds->priv;
+	int port;
 
-	/* Find the connected cpu port. Valid port are 0 or 6 */
 	if (dsa_is_cpu_port(ds, 0))
 		return 0;
 
-	dev_dbg(priv->dev, "port 0 is not the CPU port. Checking port 6");
-
 	if (dsa_is_cpu_port(ds, 6))
 		return 6;
 
+	/* Internal PHY CPU port selection is currently enabled for QCA8337. */
+	if (priv->switch_id != QCA8K_ID_QCA8337)
+		return -EINVAL;
+
+	/* An internal PHY can provide a PHY-to-PHY CPU link. */
+	for (port = 1; port < 6; port++)
+		if (dsa_is_cpu_port(ds, port))
+			return port;
+
 	return -EINVAL;
 }
 
@@ -1863,7 +1871,7 @@ qca8k_setup(struct dsa_switch *ds)
 
 	cpu_port = qca8k_find_cpu_port(ds);
 	if (cpu_port < 0) {
-		dev_err(priv->dev, "No cpu port configured in both cpu port0 and port6");
+		dev_err(priv->dev, "No CPU port configured");
 		return cpu_port;
 	}
 
-- 
2.43.0

Re: [PATCH net-next v4 2/3] net: dsa: qca8k: serialize CPU MAC pause during MTU changes

[Christian Marangi]

On Tue, Sep 29, 2026 at 12:08:10AM +0200, Yongzhao Chen wrote:
> The global maximum frame size must be updated with CPU MACs disabled.
> The previous logic only paused ports 0 and 6, leaving an internal PHY
> CPU port enabled while modifying the register.
> 
> Include enabled internal CPU ports in the pause sequence. Use the
> existing reg_mutex to serialize the MTU update against port enable, port
> disable, and phylink link-up and link-down transitions. Read and restore
> each port's original TXMAC and RXMAC bits, ensuring ports that were down
> remain down and preserving LINK_AUTO. Retain existing handling for ports
> 0 and 6.
> 
> Abort before updating the frame size if reading port status or pausing
> the MAC fails. Attempt to restore all ports already modified, and report
> any restoration failures even if an earlier error occurred.
> 
> The standalone qca8k MDIO error-propagation fix is a prerequisite for
> this series; that error-handling bug predates this locking change.
> 
> Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
> Assisted-by: LLM
> ---
>  drivers/net/dsa/qca/qca8k-8xxx.c   |  2 +
>  drivers/net/dsa/qca/qca8k-common.c | 79 ++++++++++++++++++++++++------
>  2 files changed, 66 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
> index 89113d22d5d..7bd9d9abcef 100644
> --- a/drivers/net/dsa/qca/qca8k-8xxx.c
> +++ b/drivers/net/dsa/qca/qca8k-8xxx.c
> @@ -1495,7 +1495,9 @@ qca8k_phylink_mac_link_up(struct phylink_config *config,
>  
>  	reg |= QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
>  
> +	mutex_lock(&priv->reg_mutex);
>  	qca8k_write(priv, QCA8K_REG_PORT_STATUS(port), reg);
> +	mutex_unlock(&priv->reg_mutex);
>  }

I'm not entirely sure we need to use the reg mutex here since each port
have their own register and is independent... a dedicated mutex should be
considered for the task...

>  
>  static struct qca8k_pcs *pcs_to_qca8k_pcs(struct phylink_pcs *pcs)
> diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
> index 13005f10edb..6b32bdd75ea 100644
> --- a/drivers/net/dsa/qca/qca8k-common.c
> +++ b/drivers/net/dsa/qca/qca8k-common.c
> @@ -463,7 +463,8 @@ int qca8k_mib_init(struct qca8k_priv *priv)
>  	return ret;
>  }
>  
> -void qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
> +static void qca8k_port_set_status_locked(struct qca8k_priv *priv, int port,
> +					 int enable)

Personal taste but I always feel this might be confusing...
_locked may imply that the function will lock, not that you should lock
before calling... I know it's already pattern in the kernel, your choice to
use this or __ variant.
I would use __qca8k_port_set.. and add tag to enforce that the mutex should be
locked here.

>  {
>  	u32 mask = QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
>  
> @@ -477,6 +478,13 @@ void qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
>  		regmap_clear_bits(priv->regmap, QCA8K_REG_PORT_STATUS(port), mask);
>  }
>  
> +void qca8k_port_set_status(struct qca8k_priv *priv, int port, int enable)
> +{
> +	mutex_lock(&priv->reg_mutex);
> +	qca8k_port_set_status_locked(priv, port, enable);
> +	mutex_unlock(&priv->reg_mutex);
> +}
> +
>  void qca8k_get_strings(struct dsa_switch *ds, int port, u32 stringset,
>  		       uint8_t *data)
>  {
> @@ -751,8 +759,10 @@ int qca8k_port_enable(struct dsa_switch *ds, int port,
>  {
>  	struct qca8k_priv *priv = ds->priv;
>  
> -	qca8k_port_set_status(priv, port, 1);
> +	mutex_lock(&priv->reg_mutex);
> +	qca8k_port_set_status_locked(priv, port, 1);
>  	priv->port_enabled_map |= BIT(port);
> +	mutex_unlock(&priv->reg_mutex);
>  

Can port be enabled concurrently and corrupt the port enable map? Can you
check with AI if this case is possible? If yes then this might be a good
idea to make a separate prereq patch introducing a dedicated mutex for
port status and protect it accordingly. (might also be worth for net)

>  	if (dsa_is_user_port(ds, port))
>  		phy_support_asym_pause(phy);
> @@ -764,14 +774,20 @@ void qca8k_port_disable(struct dsa_switch *ds, int port)
>  {
>  	struct qca8k_priv *priv = ds->priv;
>  
> -	qca8k_port_set_status(priv, port, 0);
> +	mutex_lock(&priv->reg_mutex);
> +	qca8k_port_set_status_locked(priv, port, 0);
>  	priv->port_enabled_map &= ~BIT(port);
> +	mutex_unlock(&priv->reg_mutex);
>  }

ditto.

>  
>  int qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
>  {
> +	u32 mask = QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
>  	struct qca8k_priv *priv = ds->priv;
> -	int ret;
> +	u32 status[QCA8K_NUM_PORTS] = { 0 };

nit. Reverse tree.

> +	int ret, restore_ret, i;
> +	u32 stopped = 0;
> +	u32 ports;
>  
>  	/* We have only have a general MTU setting.
>  	 * DSA always set the CPU port's MTU to the largest MTU of the user
> @@ -784,25 +800,58 @@ int qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
>  
>  	/* To change the MAX_FRAME_SIZE the cpu ports must be off or
>  	 * the switch panics.
> -	 * Turn off both cpu ports before applying the new value to prevent
> -	 * this.
> +	 * Include internal PHY CPU ports as well as the two MAC-only ports.
> +	 * Toggle only MAC enables, preserving the phylink link-control mode.
>  	 */
> -	if (priv->port_enabled_map & BIT(0))
> -		qca8k_port_set_status(priv, 0, 0);
> +	ports = BIT(0) | BIT(6);
> +	for (i = 1; i < 6; i++)
> +		if (dsa_is_cpu_port(ds, i))
> +			ports |= BIT(i);

In the context of internal PHY CPU port port 0 and port 6 won't be
connected... Should we check that and create a mask of the cpu port right
from the start?

>  
> -	if (priv->port_enabled_map & BIT(6))
> -		qca8k_port_set_status(priv, 6, 0);
> +	mutex_lock(&priv->reg_mutex);
> +	ports &= priv->port_enabled_map;
> +
> +	for (i = 0; i < QCA8K_NUM_PORTS; i++) {

for_each_set_bit might be better?

> +		if (!(ports & BIT(i)))
> +			continue;
> +
> +		ret = regmap_read(priv->regmap, QCA8K_REG_PORT_STATUS(i),
> +				  &status[i]);
> +		if (ret)
> +			goto unlock;
> +	}
> +
> +	for (i = 0; i < QCA8K_NUM_PORTS; i++) {

ditto.

> +		if (!(ports & BIT(i)) || !(status[i] & mask))
> +			continue;
> +
> +		stopped |= BIT(i);
> +		ret = regmap_clear_bits(priv->regmap, QCA8K_REG_PORT_STATUS(i),
> +					mask);
> +		if (ret)
> +			goto restore;
> +	}
>  
>  	/* Include L2 header / FCS length */
>  	ret = qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, new_mtu +
>  			  ETH_HLEN + ETH_FCS_LEN);
>  
> -	if (priv->port_enabled_map & BIT(0))
> -		qca8k_port_set_status(priv, 0, 1);
> -
> -	if (priv->port_enabled_map & BIT(6))
> -		qca8k_port_set_status(priv, 6, 1);
> +restore:
> +	for (i = 0; i < QCA8K_NUM_PORTS; i++)
> +		if (stopped & BIT(i)) {
> +			restore_ret = regmap_update_bits(priv->regmap,
> +							 QCA8K_REG_PORT_STATUS(i),
> +							 mask, status[i] & mask);
> +			if (restore_ret) {
> +				dev_err(priv->dev, "failed to restore MAC state on port %d: %d\n",
> +					i, restore_ret);
> +				if (!ret)
> +					ret = restore_ret;
> +			}
> +		}
>  
> +unlock:
> +	mutex_unlock(&priv->reg_mutex);
>  	return ret;
>  }
>  
> -- 
> 2.43.0
> 

-- 
	Ansuel

Re: [PATCH net-next v4 3/3] net: dsa: qca8k: support QCA8337 internal PHY CPU links

[Christian Marangi]

On Tue, Sep 29, 2026 at 12:08:11AM +0200, Yongzhao Chen wrote:
> From: Ziyang Huang <hzyitc@outlook.com>
> 
> A PHY-to-PHY CPU link connects the SoC PHY directly to an internal
> switch PHY. The QCA8337 supports header mode on these ports, and
> existing phylink callbacks already handle their internal interfaces.
> 
> Allow QCA8337 CPU port selection to fall back to ports 1 through 5 after
> checking the dedicated MAC-only ports. Preserve the preference for CPU
> ports 0 and 6 across all switch models, limiting the internal-port
> fallback to QCA8337. Support for QCA8327 internal CPU links has not
> been established and is not enabled. Include internal CPU PHYs when
> selecting the PHY access method, without altering the handling of
> MAC-only or external user ports.
> 
> This enables a single internal CPU PHY configured with an explicit
> phy-handle and phy-mode = "internal". The conduit interface uses its own
> PHY on the opposite side of the MDI connection. Existing single-CPU DSA
> forwarding uses the selected port for default flooding and membership
> without extra routing changes.
> 
> Adapted from the OpenWrt PHY-to-PHY CPU link patch, narrowing the MDIO
> filter adjustments to preserve handling for external user ports.
>

Can you put an example DT for this? Also no additional register are needed
to this special mode?
 
> [yongzhao: preserve external user-port handling on port 6 and limit
>  internal CPU PHY support to QCA8337]
> 
> Assisted-by: LLM
> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> Signed-off-by: Yongzhao Chen <yongzhao.derek@gmail.com>
> ---
>  drivers/net/dsa/qca/qca8k-8xxx.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
> index 7bd9d9abcef..cd36efd1c4f 100644
> --- a/drivers/net/dsa/qca/qca8k-8xxx.c
> +++ b/drivers/net/dsa/qca/qca8k-8xxx.c
> @@ -1026,7 +1026,8 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
>  			return ret;
>  		}
>  
> -		if (!dsa_is_user_port(priv->ds, reg))
> +		if (!dsa_is_user_port(priv->ds, reg) &&
> +		    !(reg > 0 && reg < 6 && dsa_is_cpu_port(priv->ds, reg)))
>  			continue;
>  
>  		of_get_phy_mode(port, &mode);
> @@ -1102,16 +1103,23 @@ qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
>  static int qca8k_find_cpu_port(struct dsa_switch *ds)
>  {
>  	struct qca8k_priv *priv = ds->priv;
> +	int port;
>  
> -	/* Find the connected cpu port. Valid port are 0 or 6 */
>  	if (dsa_is_cpu_port(ds, 0))
>  		return 0;
>  
> -	dev_dbg(priv->dev, "port 0 is not the CPU port. Checking port 6");
> -
>  	if (dsa_is_cpu_port(ds, 6))
>  		return 6;
>  
> +	/* Internal PHY CPU port selection is currently enabled for QCA8337. */
> +	if (priv->switch_id != QCA8K_ID_QCA8337)
> +		return -EINVAL;
> +
> +	/* An internal PHY can provide a PHY-to-PHY CPU link. */
> +	for (port = 1; port < 6; port++)
> +		if (dsa_is_cpu_port(ds, port))
> +			return port;
> +
>  	return -EINVAL;
>  }
>  
> @@ -1863,7 +1871,7 @@ qca8k_setup(struct dsa_switch *ds)
>  
>  	cpu_port = qca8k_find_cpu_port(ds);
>  	if (cpu_port < 0) {
> -		dev_err(priv->dev, "No cpu port configured in both cpu port0 and port6");
> +		dev_err(priv->dev, "No CPU port configured");
>  		return cpu_port;
>  	}
>  
> -- 
> 2.43.0
> 

-- 
	Ansuel

Re: [PATCH net-next v4 2/3] net: dsa: qca8k: serialize CPU MAC pause during MTU changes

[Yongzhao Chen]

Hi Christian,

Thanks for the detailed review.

> I'm not entirely sure we need to use the reg mutex here since each port
> have their own register and is independent... a dedicated mutex should be
> considered for the task...

As you suggested, the next revision adds a dedicated per-switch
port_status_lock instead of reusing reg_mutex, which stays with the
FDB/VLAN operations. It is held across the whole MTU sequence and by all
PORT_STATUS writers.

The lock is needed because the MTU sequence can interleave with
phylink's MAC link-up/down callbacks, which run from the phylink resolve
work without RTNL. Per-access register locking cannot protect the whole
read/pause/change/restore sequence.

> I would use __qca8k_port_set.. and add tag to enforce that the mutex should be
> locked here.

Done: the helper is now __qca8k_port_set_status() with
lockdep_assert_held().

> Can port be enabled concurrently and corrupt the port enable map? Can you
> check with AI if this case is possible? If yes then this might be a good
> idea to make a separate prereq patch introducing a dedicated mutex for
> port status and protect it accordingly. (might also be worth for net)

The DSA core calls port_enable() and port_disable() under RTNL, so the
updates to port_enabled_map are already serialized. I did not find a
path where two updates can race, so I don't think a separate net fix is
needed.

> In the context of internal PHY CPU port port 0 and port 6 won't be
> connected... Should we check that and create a mask of the cpu port right
> from the start?

The mask is now (BIT(0) | BIT(6) | dsa_cpu_ports(ds)), intersected with
port_enabled_map. I kept pausing ports 0 and 6 whenever they are
enabled, as the current code does. With an internal CPU port they can
still be in use (in my port 5 CPU test, port 6 was a fixed-link user
port), and I have no evidence that changing the frame size is safe with
their MACs running. Is there hardware guidance confirming that enabled
non-CPU ports 0/6 can remain running during the MTU update?

I have also fixed the reverse xmas tree ordering, and the loops now use
for_each_set_bit() on an unsigned long mask.

Deterministic tests built from the extracted kernel functions cover the
MTU/MAC-callback interleavings and fail when the relevant locking is
removed. On a Redmi AX5400 running an OpenWrt Linux 6.18.52 backport
(wired only, lockdep enabled), MTU changes during repeated renegotiation
passed the functional checks but never hit
a stably down link. In a separate test with the port 5 PHY powered down,
the MTU changes succeeded and the port 5 MAC stayed off in every stable
link-down sample.

Thanks,
Yongzhao Chen

Re: [PATCH net-next v4 3/3] net: dsa: qca8k: support QCA8337 internal PHY CPU links

[Yongzhao Chen]

Hi Christian,

Thanks for the review.

> Can you put an example DT for this? Also no additional register are needed
> to this special mode?

Here is an example using the switch's internal MDIO bus, with the other
user ports omitted. The SoC MAC has its own PHY at the other end of the
PHY-to-PHY connection; its phy-mode must follow that MAC's binding. The
full example passed dtc and the qca8k binding check.

    switch@10 {
        compatible = "qca,qca8337";
        reg = <0x10>;

        ports {
            #address-cells = <1>;
            #size-cells = <0>;

            port@1 {
                reg = <1>;
                label = "lan1";
                phy-mode = "internal";
                phy-handle = <&switch_phy0>;
            };

            port@5 {
                reg = <5>;
                ethernet = <&soc_mac>;
                phy-mode = "internal";
                phy-handle = <&switch_phy4>;
            };
        };

        mdio {
            #address-cells = <1>;
            #size-cells = <0>;
            switch_phy0: ethernet-phy@0 { reg = <0>; };
            switch_phy4: ethernet-phy@4 { reg = <4>; };
        };
    };

I did not add any register writes for this mode: qca8k_setup() already
programs header mode for the CPU port, the four GLOBAL_FW_CTRL1
destination masks, and the CPU/user membership masks using the selected
port. CPU_PORT_EN remains set by the existing setup code; I have not
tested clearing it.

I tested port 5 as the only CPU port on a Redmi AX5400 (RA74) with an
OpenWrt Linux 6.18.52 backport. Readback showed header mode only on
port 5 and all four destination masks selecting port 5. BPDU, LLDP,
EAPOL-Start and broadcast ARP frames arrived intact in both directions,
and unknown unicast/multicast flooding toward the CPU, DHCP, MTU
changes, renegotiation and ping also passed.

That test used the external MDIO bus with wireless disabled, and needed
two workarounds that are not in the posted series: a dummy phy-handle on
port 6 (a fixed-link user port) for MDIO classification, and a NULL-PHY
guard in qca8k_port_enable(). So it does not validate the internal-MDIO
example above on hardware. I will reword the commit message in the next
revision to claim only what was tested.

Would you prefer the fixed-link NULL-PHY handling to be addressed in a
separate prerequisite patch? The guard skips phy_support_asym_pause()
when phy is NULL, as it is for a fixed-link user port. For port 5, are
there other CPU-port registers or traffic paths you would want checked,
or is testing the internal-MDIO configuration on hardware the main gap?

Thanks,
Yongzhao Chen

Re: [PATCH net-next v4 2/3] net: dsa: qca8k: serialize CPU MAC pause during MTU changes

[netdev-bot+sashiko]

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

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

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

- [Medium] The commit message says the new error handling works: it will
  "abort before updating the frame size if ... pausing the MAC fails"…
- [Medium] For internal-PHY CPU ports (1-5), which this patch adds to the
  pause set, qca8k_port_change_mtu() clears only TXMAC|RXMAC (`mask`,…
- [Low] The concern: the commit message treats a CPU port on internal-PHY
  ports 1-5 as a live configuration, though the driver in this tree only…

--

net: dsa: qca8k: serialize CPU MAC pause during MTU changes

This commit adds internal PHY CPU ports 1-5 to the pause sequence that
runs in qca8k_port_change_mtu() before QCA8K_MAX_FRAME_SIZE is written.
It takes reg_mutex to serialize that sequence against port enable, port
disable and phylink link transitions. It also saves each port's
TXMAC/RXMAC bits and restores them afterwards, with new error handling
that aborts and restores on failure.

> diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
> index 60f2a615a5cc2..dfe18b6e936bf 100644
> --- a/drivers/net/dsa/qca/qca8k-8xxx.c
> +++ b/drivers/net/dsa/qca/qca8k-8xxx.c
> @@ -1483,7 +1483,9 @@ qca8k_phylink_mac_link_up(struct phylink_config *config,
>  
>  	reg |= QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
>  
> +	mutex_lock(&priv->reg_mutex);
>  	qca8k_write(priv, QCA8K_REG_PORT_STATUS(port), reg);
> +	mutex_unlock(&priv->reg_mutex);
>  }

[Severity: Low]
This isn't a bug, but should part of this change carry a Fixes: tag?

qca8k_phylink_mac_link_up() and the link-down callback run from the
phylink resolve work without rtnl. qca8k_port_change_mtu() runs under
rtnl.

Before this patch, a concurrent link-up could set TXMAC/RXMAC on port 0
or 6 after the pause but before the QCA8K_MAX_FRAME_SIZE write. The same
goes for restoring ports 0 and 6 to their previous state instead of
always turning them back on. Both look like fixes for existing behavior.

The commit message also treats a CPU port on internal PHY ports 1-5 as a
configuration that exists today. In this tree, qca8k_find_cpu_port()
only accepts ports 0 or 6. That support arrives later in the series in
"net: dsa: qca8k: support QCA8337 internal PHY CPU links". Could the
message say this part is preparation for that commit?

> diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
> index 13005f10edb7d..6b32bdd75eabe 100644
> --- a/drivers/net/dsa/qca/qca8k-common.c
> +++ b/drivers/net/dsa/qca/qca8k-common.c

[ ... ]

> @@ -764,14 +774,20 @@ void qca8k_port_disable(struct dsa_switch *ds, int port)

[ ... ]

>  int qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
>  {
> +	u32 mask = QCA8K_PORT_STATUS_TXMAC | QCA8K_PORT_STATUS_RXMAC;
>  	struct qca8k_priv *priv = ds->priv;

[ ... ]

> @@ -784,25 +800,58 @@ int qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
>  
>  	/* To change the MAX_FRAME_SIZE the cpu ports must be off or
>  	 * the switch panics.
> -	 * Turn off both cpu ports before applying the new value to prevent
> -	 * this.
> +	 * Include internal PHY CPU ports as well as the two MAC-only ports.
> +	 * Toggle only MAC enables, preserving the phylink link-control mode.
>  	 */
> -	if (priv->port_enabled_map & BIT(0))
> -		qca8k_port_set_status(priv, 0, 0);
> +	ports = BIT(0) | BIT(6);
> +	for (i = 1; i < 6; i++)
> +		if (dsa_is_cpu_port(ds, i))
> +			ports |= BIT(i);

[Severity: Medium]
For internal PHY CPU ports 1-5, which are now in the pause set, does
clearing only TXMAC and RXMAC actually stop the MAC?

The driver's existing disable path also clears LINK_AUTO for those
ports:

qca8k_port_set_status_locked() {
	...
	/* Port 0 and 6 have no internal PHY */
	if (port > 0 && port < 6)
		mask |= QCA8K_PORT_STATUS_LINK_AUTO;
	...
}

If LINK_AUTO (LINK_EN) stays set, the MAC may keep following the
internal PHY's link state. It could then keep passing traffic while
QCA8K_MAX_FRAME_SIZE is rewritten.

One case: qca8k_port_enable() sets TXMAC|RXMAC|LINK_AUTO on the CPU port
during setup. Then the MTU is changed while the user port is created,
possibly before phylink's mac_link_up has rewritten the register:

dsa_user_create()
  dsa_user_change_mtu(user_dev, ETH_DATA_LEN)
    dsa_port_mtu_change(cpu_dp)
      qca8k_port_change_mtu()

qca8k_phylink_mac_link_up() also writes LINK_AUTO in in-band mode and in
the default branch for unknown speeds.

Whether LINK_EN overrides cleared TXMAC/RXMAC is hardware behavior that
can't be confirmed from the code. Since status[] already saves the
original bits, could LINK_AUTO be added to the pause mask for ports 1-5
and restored afterwards?

[ ... ]

> +	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
> +		if (!(ports & BIT(i)) || !(status[i] & mask))
> +			continue;
> +
> +		stopped |= BIT(i);
> +		ret = regmap_clear_bits(priv->regmap, QCA8K_REG_PORT_STATUS(i),
> +					mask);
> +		if (ret)
> +			goto restore;
> +	}

[Severity: Medium]
The commit message says:

  Abort before updating the frame size if reading port status or pausing
  the MAC fails. Attempt to restore all ports already modified, and report
  any restoration failures even if an earlier error occurred.

Can this goto restore fire on the MDIO register path?
qca8k_mii_write32() in qca8k-8xxx.c returns void:

static void
qca8k_mii_write32(struct mii_bus *bus, int phy_id, u32 regnum, u32 val)
{
	if (qca8k_mii_write_lo(bus, phy_id, regnum, val) < 0)
		return;

	qca8k_mii_write_hi(bus, phy_id, regnum + 1, val);
}

qca8k_regmap_update_bits_mii() returns the read result and ignores the
write:

	val &= ~mask;
	val |= write_val;
	qca8k_mii_write32(bus, 0x10 | r2, r1, val);

exit:
	mutex_unlock(&bus->mdio_lock);

	return ret;

qca8k_write_mii() only returns the qca8k_set_page() result.

Without mgmt_conduit, or when the Ethernet management path falls back to
MDIO, three calls report success even if the bus write fails:
regmap_clear_bits() here, qca8k_write() of QCA8K_MAX_FRAME_SIZE, and
regmap_update_bits() in the restore loop.

Wouldn't that let MAX_FRAME_SIZE be written while a CPU MAC is still
enabled? It would also mean the "failed to restore MAC state" message
never prints for a failed MDIO write.

The commit message also says:

  The standalone qca8k MDIO error-propagation fix is a prerequisite for
  this series; that error-handling bug predates this locking change.

That fix is not in the tree or in this series. The MDIO write helpers
are still unchanged at the end of the series.

Could the message name the prerequisite commit or link to it, so
maintainers and stable backporters know what must be applied first?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928220811.1880-1-yongzhao.derek%40gmail.com

Re: [PATCH net-next v4 3/3] net: dsa: qca8k: support QCA8337 internal PHY CPU links

[netdev-bot+sashiko]

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

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

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

- [Medium] The new MDIO-filter clause in qca8k_setup_mdio_bus()
  (drivers/net/dsa/qca/qca8k-8xxx.c:1017-1018) puts any CPU port on 1..5…
- [Low] qca8k_phylink_mac_link_up()
  (drivers/net/dsa/qca/qca8k-8xxx.c:1485-1489) sets…
- [Low] The patch removes the dev_dbg("port 0 is not the CPU port.
- [Low] Concern: with an internal CPU port written as phy-mode =
  "internal" and user ports reached through the external MDC pass-…

Pre-existing issues:
- [Medium] On QCA8337, a DT that declares CPU ports on an internal port
  (1..5) and on port 6 makes qca8k_find_cpu_port() pick port 6.

--

net: dsa: qca8k: support QCA8337 internal PHY CPU links

On QCA8337, this patch lets qca8k_find_cpu_port() fall back to internal
PHY ports 1-5 when neither port 0 nor port 6 is a CPU port. It adds CPU
ports 1-5 to the internal/external MDIO classification in
qca8k_setup_mdio_bus(). It also rewords the error that qca8k_setup()
prints when no CPU port is found.

> diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
> index dfe18b6e936bf..c30913a3c2784 100644
> --- a/drivers/net/dsa/qca/qca8k-8xxx.c
> +++ b/drivers/net/dsa/qca/qca8k-8xxx.c
> @@ -1014,7 +1014,8 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
>  			return ret;
>  		}
>  
> -		if (!dsa_is_user_port(priv->ds, reg))
> +		if (!dsa_is_user_port(priv->ds, reg) &&
> +		    !(reg > 0 && reg < 6 && dsa_is_cpu_port(priv->ds, reg)))
>  			continue;

[Severity: Medium]
This clause lets any CPU port on 1..5 into the MDIO classification. It
does so on every switch model, and also when port 0 or 6 is a CPU port.

The QCA8337-only check exists only in qca8k_find_cpu_port(), after the
early returns for ports 0 and 6. So the two functions can disagree about
which ports count as internal CPU links.

Before this patch, a secondary CPU port on 1..5 is not a user port, so it
hit the continue. Now it gets classified. For example, it sets
internal_mdio_mask if it has no phy-handle (or uses a fixed-link), or if
it has phy-mode = "internal".

The user ports may use the external MDIO pass-through (phy-handle and no
phy-mode, as in the binding's external MDIO example). In that case they
also set external_mdio_mask, and then:

	if (!!external_mdio_mask && !!internal_mdio_mask) {
		dev_err(priv->dev, "either internal or external mdio bus configuration is supported.\n");
		return -EINVAL;
	}

Could qca8k_setup() now fail with -EINVAL on a DT that probed before
this patch?

The commit message says QCA8327 internal CPU links are "not enabled" and
that the change is made "without altering the handling of MAC-only or
external user ports". Should this clause be limited to QCA8337, and to
the port that qca8k_find_cpu_port() actually selected?

[Severity: Low]
Related to this, the commit message describes an internal CPU PHY
"configured with an explicit phy-handle and phy-mode = "internal"".

On boards whose user ports use the external MDC pass-through, such a CPU
port goes into internal_mdio_mask while the user ports go into
external_mdio_mask. Setup then fails with the existing mixed-configuration
error.

The binding says this mixed configuration is not supported by the
hardware. A CPU port described the same way as the user ports
(phy-handle, no phy-mode) is classified as external and works.

This isn't a bug, but could the commit message say that on external MDIO
boards the CPU port should be described the same way as the user ports,
rather than with phy-mode = "internal"?

>  
>  		of_get_phy_mode(port, &mode);
> @@ -1090,16 +1091,23 @@ qca8k_setup_mac_pwr_sel(struct qca8k_priv *priv)
>  static int qca8k_find_cpu_port(struct dsa_switch *ds)
>  {
>  	struct qca8k_priv *priv = ds->priv;
> +	int port;
>  
> -	/* Find the connected cpu port. Valid port are 0 or 6 */
>  	if (dsa_is_cpu_port(ds, 0))
>  		return 0;
>  
> -	dev_dbg(priv->dev, "port 0 is not the CPU port. Checking port 6");
> -
>  	if (dsa_is_cpu_port(ds, 6))
>  		return 6;

[Severity: Medium]
This isn't a bug introduced by this patch, but consider a QCA8337 DT that
declares CPU ports on both an internal port (1..5) and port 6. Here
qca8k_find_cpu_port() returns 6.

However, dsa_tree_setup_cpu_ports() in net/dsa/dsa.c sets dp->cpu_dp for
every user port to the lowest-index CPU port, because qca8k does not
provide .preferred_default_local_cpu_port.

qca8k_setup() programs GLOBAL_FW_CTRL1 flooding and the LOOKUP_MEMBER
masks from cpu_port = 6. qca8k_update_port_member() in qca8k-common.c
later uses:

	u32 port_mask = BIT(dp->cpu_dp->index);

That is the internal port. Setup cleared its own LOOKUP_MEMBER and
nothing sets it again. Can hardware forwarding and DSA disagree about
which CPU port is in use here?

The baseline code also returned 6 for this DT. The new fallback loop only
runs when neither 0 nor 6 is a CPU port, so this patch does not change
that behaviour.

>  
> +	/* Internal PHY CPU port selection is currently enabled for QCA8337. */
> +	if (priv->switch_id != QCA8K_ID_QCA8337)
> +		return -EINVAL;
> +
> +	/* An internal PHY can provide a PHY-to-PHY CPU link. */
> +	for (port = 1; port < 6; port++)
> +		if (dsa_is_cpu_port(ds, port))
> +			return port;

[Severity: Low]
With this fallback, the CPU port on QCA8337 can be an autonegotiated
internal PHY port. qca8k_phylink_mac_link_up() turns flow control on for
every CPU port, whatever pause state phylink resolved:

	if (rx_pause || dsa_port_is_cpu(dp))
		reg |= QCA8K_PORT_STATUS_RXFLOW;

	if (tx_pause || dsa_port_is_cpu(dp))
		reg |= QCA8K_PORT_STATUS_TXFLOW;

Before this patch, the CPU port could only be port 0 or 6 on a fixed
link. On a PHY-to-PHY CPU link, tx_pause and rx_pause hold the negotiated
result. Will PORT_STATUS enable PAUSE even when negotiation settled on no
pause?

The commit message says the existing phylink callbacks "already handle"
these internal interfaces. Should the dsa_port_is_cpu() override apply
only to ports 0 and 6?

> +
>  	return -EINVAL;
>  }
>  
> @@ -1851,7 +1859,7 @@ qca8k_setup(struct dsa_switch *ds)
>  
>  	cpu_port = qca8k_find_cpu_port(ds);
>  	if (cpu_port < 0) {
> -		dev_err(priv->dev, "No cpu port configured in both cpu port0 and port6");
> +		dev_err(priv->dev, "No CPU port configured");

[Severity: Low]
This isn't a bug, but the commit message does not mention removing the
dev_dbg() in qca8k_find_cpu_port() or rewording this error.

The new text is also misleading for the rejection path this patch adds.
On QCA8327, or any model other than QCA8337, with a CPU port on 1..5, the
switch_id check in qca8k_find_cpu_port() returns -EINVAL. This message
then says "No CPU port configured" even though one is configured.

Could the message list the valid ports, or say that internal CPU ports
are only accepted on QCA8337?

>  		return cpu_port;
>  	}
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928220811.1880-1-yongzhao.derek%40gmail.com