[PATCH next-next] net: dsa: qca8k: Add support for force mode for fixed link topology

[PATCH next-next] net: dsa: qca8k: Add support for force mode for fixed link topology

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

A fixed link topology is commonly used to connect this switch (on port
0 or 6) to a SoC's MAC over SGMII. When inband negotiation is not used,
the switch needs to be configured to operate in force mode. As such,
enable support for force mode.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 6 +++++-
 drivers/net/dsa/qca/qca8k.h      | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index a36b8b07030e..3dbd18ab8638 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1611,10 +1611,14 @@ static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 	if (priv->ports_config.sgmii_tx_clk_falling_edge)
 		val |= QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE;
 
+	if (neg_mode == PHYLINK_PCS_NEG_OUTBAND)
+		val |= QCA8K_PORT_PAD_SGMII_FORCE_MODE;
+
 	if (val)
 		ret = qca8k_rmw(priv, reg,
 				QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
-				QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
+				QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE |
+				QCA8K_PORT_PAD_SGMII_FORCE_MODE,
 				val);
 
 	return 0;
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 1a00e2f62fef..956338893032 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -58,6 +58,7 @@
 #define	  QCA8K_PORT_PAD_RGMII_TX_DELAY_EN		BIT(25)
 #define   QCA8K_PORT_PAD_RGMII_RX_DELAY_EN		BIT(24)
 #define   QCA8K_PORT_PAD_SGMII_EN			BIT(7)
+#define   QCA8K_PORT_PAD_SGMII_FORCE_MODE		BIT(3)
 #define QCA8K_REG_PWS					0x010
 #define   QCA8K_PWS_POWER_ON_SEL			BIT(31)
 /* This reg is only valid for QCA832x and toggle the package

---
base-commit: b7bee4ca5688e30ca50fbc87b1b8f7eed7006c17
change-id: 20260603-qca8337-force-mode-fbcba5296446

Best regards,
-- 
George Moussalem <george.moussalem@outlook.com>

Re: [PATCH next-next] net: dsa: qca8k: Add support for force mode for fixed link topology

[Andrew Lunn]

On Wed, Jun 03, 2026 at 11:57:39AM +0400, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> A fixed link topology is commonly used to connect this switch (on port
> 0 or 6) to a SoC's MAC over SGMII. When inband negotiation is not used,
> the switch needs to be configured to operate in force mode. As such,
> enable support for force mode.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH next-next] net: dsa: qca8k: Add support for force mode for fixed link topology

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: qca8k: Add support for force mode for fixed link topology
    
A fixed link topology is commonly used to connect this switch (on port
0 or 6) to a SoC's MAC over SGMII. When inband negotiation is not used,
the switch needs to be configured to operate in force mode. As such,
enable support for force mode.

> diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
> index a36b8b07030e3..3dbd18ab86380 100644
> --- a/drivers/net/dsa/qca/qca8k-8xxx.c
> +++ b/drivers/net/dsa/qca/qca8k-8xxx.c
[ ... ]
> @@ -1611,10 +1611,14 @@ static int qca8k_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
>  	if (priv->ports_config.sgmii_tx_clk_falling_edge)
>  		val |= QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE;
>  
> +	if (neg_mode == PHYLINK_PCS_NEG_OUTBAND)
> +		val |= QCA8K_PORT_PAD_SGMII_FORCE_MODE;
> +

[Severity: High]
Could there be an issue with configuring port 6 on QCA8327 and QCA8337
hardware?

Earlier in qca8k_pcs_config(), reg is overridden for these models:

	if (priv->switch_id == QCA8K_ID_QCA8327 ||
	    priv->switch_id == QCA8K_ID_QCA8337)
		reg = QCA8K_REG_PORT0_PAD_CTRL;

Since the SGMII force mode configuration uses this same reg variable,
won't this cause the FORCE_MODE bit for port 6 to be incorrectly written
to port 0's PAD control register on these specific switches?

>  	if (val)
>  		ret = qca8k_rmw(priv, reg,
>  				QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
> -				QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
> +				QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE |
> +				QCA8K_PORT_PAD_SGMII_FORCE_MODE,
>  				val);

[Severity: High]
Is FORCE_MODE properly cleared if it was previously set by a bootloader,
but we don't use outband negotiation and no clock falling edge properties
are configured?

If neg_mode != PHYLINK_PCS_NEG_OUTBAND and the clock phase bits are not set,
val will be 0. This causes the if (val) check to fail, skipping the
qca8k_rmw() call entirely, leaving FORCE_MODE unchanged in hardware.

However, if clock phases are configured in the device tree, val will be
non-zero, and qca8k_rmw() will clear FORCE_MODE. Does this create a
scenario where clearing this mode unpredictably depends on unrelated clock
configurations?

>  
>  	return 0;
-- 
pw-bot: cr

Re: [PATCH next-next] net: dsa: qca8k: Add support for force mode for fixed link topology

[George Moussalem]

On 6/5/26 05:24, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
...
> 
> Earlier in qca8k_pcs_config(), reg is overridden for these models:
> 
> 	if (priv->switch_id == QCA8K_ID_QCA8327 ||
> 	    priv->switch_id == QCA8K_ID_QCA8337)
> 		reg = QCA8K_REG_PORT0_PAD_CTRL;
> 
> Since the SGMII force mode configuration uses this same reg variable,
> won't this cause the FORCE_MODE bit for port 6 to be incorrectly written
> to port 0's PAD control register on these specific switches?

I've taken another look at the vendor SDK and this property is always
set on PORT0_PAD_CTRL regardless of whether it's physically wired on
port 0 or 6. I did see it is only set on the QCA8337 version, so I'll
check for that in v2.

> 
>>  	if (val)
>>  		ret = qca8k_rmw(priv, reg,
>>  				QCA8K_PORT0_PAD_SGMII_RXCLK_FALLING_EDGE |
>> -				QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
>> +				QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE |
>> +				QCA8K_PORT_PAD_SGMII_FORCE_MODE,
>>  				val);
> 
> [Severity: High]
> Is FORCE_MODE properly cleared if it was previously set by a bootloader,
> but we don't use outband negotiation and no clock falling edge properties
> are configured?
> 
> If neg_mode != PHYLINK_PCS_NEG_OUTBAND and the clock phase bits are not set,
> val will be 0. This causes the if (val) check to fail, skipping the
> qca8k_rmw() call entirely, leaving FORCE_MODE unchanged in hardware.
> 
> However, if clock phases are configured in the device tree, val will be
> non-zero, and qca8k_rmw() will clear FORCE_MODE. Does this create a
> scenario where clearing this mode unpredictably depends on unrelated clock
> configurations?

No, the default value, as for all documented bits on PORT0_PAD_CTRL, is
0 (disabled) after a reset. So it will be disabled by default and only
enabled if a fixed link is defined in the port node in the DT.

> 
>>  
>>  	return 0;

Best regards,
George