mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net 0/2] stmmac: fix ETHQOS SerDes interface handling
@ 2026-09-16 10:39 Thomas Karpiniec
  2026-09-16 10:39 ` [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors Thomas Karpiniec
  2026-09-16 10:39 ` [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces Thomas Karpiniec
  0 siblings, 2 replies; 10+ messages in thread
From: Thomas Karpiniec @ 2026-09-16 10:39 UTC (permalink / raw)
  To: netdev
  Cc: Maxime Chevallier, Mohd Ayaan Anwar, Russell King, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Choong Yong Liang, Bartosz Golaszewski, linux-arm-msm,
	linux-kernel

This series fixes gigabit connectivity with ETHQOS and a QCA8081 PHY on
an IQ-8275 EVK, along with an error-propagation bug found while reviewing
the same configuration path. The first patch passes platform mac_finish()
errors to phylink. The second advertises the supported SerDes interfaces
so that phylink retains the PHY's lower-speed copper modes.

This revision was built into a WendyOS test image and booted on the
IQ-8275 EVK. Tests covered boot at 1Gbps and 2.5Gbps, physical cable swaps
in both directions, stable carrier, and traffic at 1Gbps. Injecting
-ETIMEDOUT at the platform callback held carrier down at both speeds;
restarting the interface after disabling injection recovered it. This
was a synthetic callback error, not a physical SerDes failure.

The tests used regulator_ignore_unused: without it, the upstream board
tree repeatedly lost PCS link. The vendor tree has a REFGEN always-on
workaround absent from this tree. No extra kernel or device-tree patches
were applied for these tests.

Sparse reported no findings in the changed code.

Patch 2 uses SerDes APIs introduced in v7.1. Older affected kernels
need an adapted backport rather than a straight cherry-pick.

Thomas Karpiniec (2):
  net: stmmac: propagate platform mac_finish errors
  net: stmmac: qcom-ethqos: advertise supported SerDes interfaces

 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 22 ++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  4 ++--
 2 files changed, 24 insertions(+), 2 deletions(-)


base-commit: ceac0de741bfb47ca255eee075257b3bb31f0651
-- 
2.50.1 (Apple Git-155)

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

* [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors
  2026-09-16 10:39 [PATCH net 0/2] stmmac: fix ETHQOS SerDes interface handling Thomas Karpiniec
@ 2026-09-16 10:39 ` Thomas Karpiniec
  2026-09-16 12:03   ` Mohd Ayaan Anwar
  2026-09-16 13:02   ` Maxime Chevallier
  2026-09-16 10:39 ` [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces Thomas Karpiniec
  1 sibling, 2 replies; 10+ messages in thread
From: Thomas Karpiniec @ 2026-09-16 10:39 UTC (permalink / raw)
  To: netdev
  Cc: Maxime Chevallier, Mohd Ayaan Anwar, Russell King, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Choong Yong Liang, Bartosz Golaszewski, linux-arm-msm,
	linux-kernel

From: Thomas Karpiniec <tom@wendy.sh>

stmmac_mac_finish() discards the platform callback's return value and
always reports success to phylink. For example, the Qualcomm ETHQOS
callback can return an error from phy_set_mode_ext() if SerDes
configuration fails. Phylink then treats the interface as successfully
configured and can report carrier even though the SerDes is not ready.

Return the platform callback's result so that phylink can report the
failure and keep the link down until a subsequent configuration succeeds.
Keep returning zero when no platform callback is installed.

Fixes: e654cfc718d4 ("net: stmmac: configure SerDes on mac_finish")
Cc: stable@vger.kernel.org
Assisted-by: LLM sparse
Signed-off-by: Thomas Karpiniec <tom@wendy.sh>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1fb5f804ea23..1350e92943ac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1048,8 +1048,8 @@ static int stmmac_mac_finish(struct phylink_config *config, unsigned int mode,
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
 	if (priv->plat->mac_finish)
-		priv->plat->mac_finish(ndev, priv->plat->bsp_priv, mode,
-				       interface);
+		return priv->plat->mac_finish(ndev, priv->plat->bsp_priv, mode,
+					     interface);
 
 	return 0;
 }
-- 
2.50.1 (Apple Git-155)


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

* [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces
  2026-09-16 10:39 [PATCH net 0/2] stmmac: fix ETHQOS SerDes interface handling Thomas Karpiniec
  2026-09-16 10:39 ` [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors Thomas Karpiniec
@ 2026-09-16 10:39 ` Thomas Karpiniec
  2026-09-16 12:17   ` Mohd Ayaan Anwar
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Karpiniec @ 2026-09-16 10:39 UTC (permalink / raw)
  To: netdev
  Cc: Maxime Chevallier, Mohd Ayaan Anwar, Russell King, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Choong Yong Liang, Bartosz Golaszewski, linux-arm-msm,
	linux-kernel

From: Thomas Karpiniec <tom@wendy.sh>

With a QCA8081 PHY and phy-mode = "2500base-x", stmmac reports only
2500BASE-X in phylink's supported_interfaces. Phylink consequently removes
10/100/1000BASE-T modes from the PHY's advertisement, preventing a link
with a gigabit switch.

The QCA8081 uses SGMII for lower copper speeds and 2500BASE-X for 2.5Gbps.
ETHQOS already reconfigures the SerDes in ethqos_mac_finish_serdes() when
the PHY changes interface, but does not advertise this capability.

Provide a get_interfaces() callback for serial interfaces. Add SGMII and
2500BASE-X when the SerDes validates them, allowing phylink to retain the
corresponding copper link modes. Leave the existing stmmac fallback to
the firmware interface in place when no interfaces are reported. RGMII
configurations are unchanged.

Fixes: 61e9be0efbe8 ("net: stmmac: qcom-ethqos: add support for 2.5G BASEX mode")
Cc: stable@vger.kernel.org # 7.1+
Assisted-by: LLM sparse
Signed-off-by: Thomas Karpiniec <tom@wendy.sh>
---
 .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index ac7d6d3e205a..3493e090f1a8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2018-19, Linaro Limited
 
+#include <linux/bitops.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_net.h>
@@ -564,6 +565,26 @@ static void ethqos_pcs_set_inband(struct qcom_ethqos *ethqos, bool enable)
 	stmmac_pcs_ctrl_ane(priv, enable, 0);
 }
 
+static void ethqos_get_interfaces_serdes(struct stmmac_priv *priv, void *bsp_priv,
+					 unsigned long *interfaces)
+{
+	struct qcom_ethqos *ethqos = bsp_priv;
+
+	if (!ethqos->serdes_phy)
+		return;
+
+	/* PHYs such as QCA8081 switch between SGMII and 2500BASE-X with
+	 * the negotiated copper speed. mac_finish reconfigures the SerDes
+	 * accordingly; let phylink validate all modes that path supports.
+	 */
+	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
+			  PHY_INTERFACE_MODE_SGMII, NULL))
+		__set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
+	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
+			  PHY_INTERFACE_MODE_2500BASEX, NULL))
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
+}
+
 /* On interface toggle MAC registers gets reset.
  * Configure MAC block for SGMII on ethernet phy link up
  */
@@ -718,6 +739,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 		break;
 	case PHY_INTERFACE_MODE_2500BASEX:
 	case PHY_INTERFACE_MODE_SGMII:
+		plat_dat->get_interfaces = ethqos_get_interfaces_serdes;
 		plat_dat->fix_mac_speed = ethqos_fix_mac_speed_sgmii;
 		plat_dat->mac_finish = ethqos_mac_finish_serdes;
 		break;
-- 
2.50.1 (Apple Git-155)


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

* Re: [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors
  2026-09-16 10:39 ` [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors Thomas Karpiniec
@ 2026-09-16 12:03   ` Mohd Ayaan Anwar
  2026-09-17  4:01     ` Thomas Karpiniec
  2026-09-16 13:02   ` Maxime Chevallier
  1 sibling, 1 reply; 10+ messages in thread
From: Mohd Ayaan Anwar @ 2026-09-16 12:03 UTC (permalink / raw)
  To: Thomas Karpiniec
  Cc: netdev, Maxime Chevallier, Russell King, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Choong Yong Liang, Bartosz Golaszewski, linux-arm-msm,
	linux-kernel

On Wed, Sep 16, 2026 at 08:39:49PM +1000, Thomas Karpiniec wrote:
> From: Thomas Karpiniec <tom@wendy.sh>
> 
> stmmac_mac_finish() discards the platform callback's return value and
> always reports success to phylink. For example, the Qualcomm ETHQOS
> callback can return an error from phy_set_mode_ext() if SerDes
> configuration fails. Phylink then treats the interface as successfully
> configured and can report carrier even though the SerDes is not ready.
> 
> Return the platform callback's result so that phylink can report the
> failure and keep the link down until a subsequent configuration succeeds.
> Keep returning zero when no platform callback is installed.
> 
> Fixes: e654cfc718d4 ("net: stmmac: configure SerDes on mac_finish")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM sparse
> Signed-off-by: Thomas Karpiniec <tom@wendy.sh>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 1fb5f804ea23..1350e92943ac 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1048,8 +1048,8 @@ static int stmmac_mac_finish(struct phylink_config *config, unsigned int mode,
>  	struct stmmac_priv *priv = netdev_priv(ndev);
>  
>  	if (priv->plat->mac_finish)
> -		priv->plat->mac_finish(ndev, priv->plat->bsp_priv, mode,
> -				       interface);
> +		return priv->plat->mac_finish(ndev, priv->plat->bsp_priv, mode,
> +					     interface);
>  
>  	return 0;
>  }

Just curious, are you trying to resolve the issue where an interface
gets created depite:
  qcom-dwmac-sgmii-phy 8909000.phy: QSERDES_COM_C_READY_STATUS timed-out
  qcom-ethqos 23040000.ethernet eth0: __stmmac_open: Serdes powerup failed
in dmesg?

Anyways, the change looks reasonable to me:

Acked-by: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

	Ayaan


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

* Re: [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces
  2026-09-16 10:39 ` [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces Thomas Karpiniec
@ 2026-09-16 12:17   ` Mohd Ayaan Anwar
  2026-09-16 12:58     ` Maxime Chevallier
  0 siblings, 1 reply; 10+ messages in thread
From: Mohd Ayaan Anwar @ 2026-09-16 12:17 UTC (permalink / raw)
  To: Thomas Karpiniec
  Cc: netdev, Maxime Chevallier, Russell King, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Choong Yong Liang, Bartosz Golaszewski, Russell King,
	linux-arm-msm, linux-kernel

On Wed, Sep 16, 2026 at 08:39:50PM +1000, Thomas Karpiniec wrote:
> From: Thomas Karpiniec <tom@wendy.sh>
> 
> With a QCA8081 PHY and phy-mode = "2500base-x", stmmac reports only
> 2500BASE-X in phylink's supported_interfaces. Phylink consequently removes
> 10/100/1000BASE-T modes from the PHY's advertisement, preventing a link
> with a gigabit switch.
> 
> The QCA8081 uses SGMII for lower copper speeds and 2500BASE-X for 2.5Gbps.
> ETHQOS already reconfigures the SerDes in ethqos_mac_finish_serdes() when
> the PHY changes interface, but does not advertise this capability.
> 
> Provide a get_interfaces() callback for serial interfaces. Add SGMII and
> 2500BASE-X when the SerDes validates them, allowing phylink to retain the
> corresponding copper link modes. Leave the existing stmmac fallback to
> the firmware interface in place when no interfaces are reported. RGMII
> configurations are unchanged.
> 
> Fixes: 61e9be0efbe8 ("net: stmmac: qcom-ethqos: add support for 2.5G BASEX mode")
> Cc: stable@vger.kernel.org # 7.1+
> Assisted-by: LLM sparse
> Signed-off-by: Thomas Karpiniec <tom@wendy.sh>
> ---
>  .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index ac7d6d3e205a..3493e090f1a8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2018-19, Linaro Limited
>  
> +#include <linux/bitops.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_net.h>
> @@ -564,6 +565,26 @@ static void ethqos_pcs_set_inband(struct qcom_ethqos *ethqos, bool enable)
>  	stmmac_pcs_ctrl_ane(priv, enable, 0);
>  }
>  
> +static void ethqos_get_interfaces_serdes(struct stmmac_priv *priv, void *bsp_priv,
> +					 unsigned long *interfaces)
> +{
> +	struct qcom_ethqos *ethqos = bsp_priv;
> +
> +	if (!ethqos->serdes_phy)
> +		return;
> +
> +	/* PHYs such as QCA8081 switch between SGMII and 2500BASE-X with
> +	 * the negotiated copper speed. mac_finish reconfigures the SerDes
> +	 * accordingly; let phylink validate all modes that path supports.
> +	 */
> +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
> +			  PHY_INTERFACE_MODE_SGMII, NULL))
> +		__set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
> +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
> +			  PHY_INTERFACE_MODE_2500BASEX, NULL))
> +		__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
> +}
> +

So, I had proposed this exact change a while back:
https://lore.kernel.org/netdev/aMgootkPQ%2FGcdiXX@oss.qualcomm.com/

As Russell noted, this behaviour is due to the messed-up integrated
PCS support in STMMAC/QCOM-ETHQOS.

I would defer to him for this if he's around.

	Ayaan

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

* Re: [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces
  2026-09-16 12:17   ` Mohd Ayaan Anwar
@ 2026-09-16 12:58     ` Maxime Chevallier
  2026-09-17  4:29       ` Thomas Karpiniec
  0 siblings, 1 reply; 10+ messages in thread
From: Maxime Chevallier @ 2026-09-16 12:58 UTC (permalink / raw)
  To: Mohd Ayaan Anwar, Thomas Karpiniec
  Cc: netdev, Russell King, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Choong Yong Liang,
	Bartosz Golaszewski, Russell King, linux-arm-msm, linux-kernel

Hi,

On 9/16/26 14:17, Mohd Ayaan Anwar wrote:
> On Wed, Sep 16, 2026 at 08:39:50PM +1000, Thomas Karpiniec wrote:
>> From: Thomas Karpiniec <tom@wendy.sh>
>>
>> With a QCA8081 PHY and phy-mode = "2500base-x", stmmac reports only
>> 2500BASE-X in phylink's supported_interfaces. Phylink consequently removes
>> 10/100/1000BASE-T modes from the PHY's advertisement, preventing a link
>> with a gigabit switch.
>>
>> The QCA8081 uses SGMII for lower copper speeds and 2500BASE-X for 2.5Gbps.
>> ETHQOS already reconfigures the SerDes in ethqos_mac_finish_serdes() when
>> the PHY changes interface, but does not advertise this capability.
>>
>> Provide a get_interfaces() callback for serial interfaces. Add SGMII and
>> 2500BASE-X when the SerDes validates them, allowing phylink to retain the
>> corresponding copper link modes. Leave the existing stmmac fallback to
>> the firmware interface in place when no interfaces are reported. RGMII
>> configurations are unchanged.
>>
>> Fixes: 61e9be0efbe8 ("net: stmmac: qcom-ethqos: add support for 2.5G BASEX mode")
>> Cc: stable@vger.kernel.org # 7.1+
>> Assisted-by: LLM sparse
>> Signed-off-by: Thomas Karpiniec <tom@wendy.sh>
>> ---
>>  .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 22 +++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
>> index ac7d6d3e205a..3493e090f1a8 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
>> @@ -1,6 +1,7 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  // Copyright (c) 2018-19, Linaro Limited
>>  
>> +#include <linux/bitops.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_net.h>
>> @@ -564,6 +565,26 @@ static void ethqos_pcs_set_inband(struct qcom_ethqos *ethqos, bool enable)
>>  	stmmac_pcs_ctrl_ane(priv, enable, 0);
>>  }
>>  
>> +static void ethqos_get_interfaces_serdes(struct stmmac_priv *priv, void *bsp_priv,
>> +					 unsigned long *interfaces)
>> +{
>> +	struct qcom_ethqos *ethqos = bsp_priv;
>> +
>> +	if (!ethqos->serdes_phy)
>> +		return;
>> +
>> +	/* PHYs such as QCA8081 switch between SGMII and 2500BASE-X with
>> +	 * the negotiated copper speed. mac_finish reconfigures the SerDes
>> +	 * accordingly; let phylink validate all modes that path supports.
>> +	 */
>> +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
>> +			  PHY_INTERFACE_MODE_SGMII, NULL))
>> +		__set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
>> +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
>> +			  PHY_INTERFACE_MODE_2500BASEX, NULL))
>> +		__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
>> +}
>> +
> 
> So, I had proposed this exact change a while back:
> https://lore.kernel.org/netdev/aMgootkPQ%2FGcdiXX@oss.qualcomm.com/
> 
> As Russell noted, this behaviour is due to the messed-up integrated
> PCS support in STMMAC/QCOM-ETHQOS.
> 
> I would defer to him for this if he's around.

Indeed, but we haven't heard from him in a while, let's see if we can
figure this out. 

Does this IP have an integrated PCS ? (i.e. dma_cap.pcs is set)
you can check that in debugfs :

mount -t debugfs none /sys/kernel/debug
cat /sys/kernel/debug/stmmaceth/eth0/dma_cap | grep PCS

if it's Y, then you have the integrated one.

Russell worked hard on that, it looks like a lot of the plumbing went
through.

I think the right approach here is to have the PCS itself report the
list of supported interfaces, instead of relying on the glue. looking
at stmmac_pcs.c :

220 int stmmac_integrated_pcs_init(struct stmmac_priv *priv,                                
221                                const struct stmmac_pcs_info *pcs_info)                  
222 {                                                                                       
223         struct stmmac_pcs *spcs;                                                        
224                                                                                         
225         spcs = devm_kzalloc(priv->device, sizeof(*spcs), GFP_KERNEL);                   
226         if (!spcs)                                                                      
227                 return -ENOMEM;                                                         

[...]
                                                             
246         /* Only allow 2500BASE-X if the SerDes has support. */                          
247         if (priv->plat->flags & STMMAC_FLAG_SERDES_SUPPORTS_2500M)                      
248                 __set_bit(PHY_INTERFACE_MODE_2500BASEX,                                 
249                           spcs->pcs.supported_interfaces);                              
250                                                                                         
251         priv->integrated_pcs = spcs;                                                    
252                                                                                         
253         return 0;                                                                       
254 }        

Can you test settung the STMMAC_FLAG_SERDES_SUPPORTS_2500M flag in
dwmac-qcom-ethqos ?

Maxime


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

* Re: [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors
  2026-09-16 10:39 ` [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors Thomas Karpiniec
  2026-09-16 12:03   ` Mohd Ayaan Anwar
@ 2026-09-16 13:02   ` Maxime Chevallier
  1 sibling, 0 replies; 10+ messages in thread
From: Maxime Chevallier @ 2026-09-16 13:02 UTC (permalink / raw)
  To: Thomas Karpiniec, netdev
  Cc: Mohd Ayaan Anwar, Russell King, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Choong Yong Liang,
	Bartosz Golaszewski, linux-arm-msm, linux-kernel

Hi,

On 9/16/26 12:39, Thomas Karpiniec wrote:
> From: Thomas Karpiniec <tom@wendy.sh>
> 
> stmmac_mac_finish() discards the platform callback's return value and
> always reports success to phylink. For example, the Qualcomm ETHQOS
> callback can return an error from phy_set_mode_ext() if SerDes
> configuration fails. Phylink then treats the interface as successfully
> configured and can report carrier even though the SerDes is not ready.
> 
> Return the platform callback's result so that phylink can report the
> failure and keep the link down until a subsequent configuration succeeds.
> Keep returning zero when no platform callback is installed.
> 
> Fixes: e654cfc718d4 ("net: stmmac: configure SerDes on mac_finish")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM sparse
> Signed-off-by: Thomas Karpiniec <tom@wendy.sh>

Let's see if this uncovers other dirt under the stmmac phylink carpet :)

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime


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

* Re: [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors
  2026-09-16 12:03   ` Mohd Ayaan Anwar
@ 2026-09-17  4:01     ` Thomas Karpiniec
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Karpiniec @ 2026-09-17  4:01 UTC (permalink / raw)
  To: Mohd Ayaan Anwar
  Cc: netdev, Maxime Chevallier, Russell King, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Choong Yong Liang, Bartosz Golaszewski, linux-arm-msm,
	linux-kernel

On 16/09/2026 10:03 pm, Mohd Ayaan Anwar wrote:
> On Wed, Sep 16, 2026 at 08:39:49PM +1000, Thomas Karpiniec wrote:
>> From: Thomas Karpiniec <tom@wendy.sh>
>>
>> stmmac_mac_finish() discards the platform callback's return value and
>> always reports success to phylink. For example, the Qualcomm ETHQOS
>> callback can return an error from phy_set_mode_ext() if SerDes
>> configuration fails. Phylink then treats the interface as successfully
>> configured and can report carrier even though the SerDes is not ready.
>>
>> Return the platform callback's result so that phylink can report the
>> failure and keep the link down until a subsequent configuration succeeds.
>> Keep returning zero when no platform callback is installed.
>>
>> Fixes: e654cfc718d4 ("net: stmmac: configure SerDes on mac_finish")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM sparse
>> Signed-off-by: Thomas Karpiniec <tom@wendy.sh>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 1fb5f804ea23..1350e92943ac 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -1048,8 +1048,8 @@ static int stmmac_mac_finish(struct phylink_config *config, unsigned int mode,
>>   	struct stmmac_priv *priv = netdev_priv(ndev);
>>   
>>   	if (priv->plat->mac_finish)
>> -		priv->plat->mac_finish(ndev, priv->plat->bsp_priv, mode,
>> -				       interface);
>> +		return priv->plat->mac_finish(ndev, priv->plat->bsp_priv, mode,
>> +					     interface);
>>   
>>   	return 0;
>>   }
> 
> Just curious, are you trying to resolve the issue where an interface
> gets created depite:
>    qcom-dwmac-sgmii-phy 8909000.phy: QSERDES_COM_C_READY_STATUS timed-out
>    qcom-ethqos 23040000.ethernet eth0: __stmmac_open: Serdes powerup failed
> in dmesg?
No, I haven't seen that error or any specific SerDes faults. The error 
message you mention appears to be a failure in the serdes_powerup 
callback whereas this is propagating errors from mac_finish.

I just happened to notice the discarded return value while working in 
the area so I don't think it's related.

Tom


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

* Re: [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces
  2026-09-16 12:58     ` Maxime Chevallier
@ 2026-09-17  4:29       ` Thomas Karpiniec
  2026-09-17  5:36         ` Maxime Chevallier
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Karpiniec @ 2026-09-17  4:29 UTC (permalink / raw)
  To: Maxime Chevallier, Mohd Ayaan Anwar
  Cc: netdev, Russell King, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Choong Yong Liang,
	Bartosz Golaszewski, Russell King, linux-arm-msm, linux-kernel

On 16/09/2026 10:58 pm, Maxime Chevallier wrote:
> On 9/16/26 14:17, Mohd Ayaan Anwar wrote:
>> On Wed, Sep 16, 2026 at 08:39:50PM +1000, Thomas Karpiniec wrote:
>>> +static void ethqos_get_interfaces_serdes(struct stmmac_priv *priv, void *bsp_priv,
>>> +					 unsigned long *interfaces)
>>> +{
>>> +	struct qcom_ethqos *ethqos = bsp_priv;
>>> +
>>> +	if (!ethqos->serdes_phy)
>>> +		return;
>>> +
>>> +	/* PHYs such as QCA8081 switch between SGMII and 2500BASE-X with
>>> +	 * the negotiated copper speed. mac_finish reconfigures the SerDes
>>> +	 * accordingly; let phylink validate all modes that path supports.
>>> +	 */
>>> +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
>>> +			  PHY_INTERFACE_MODE_SGMII, NULL))
>>> +		__set_bit(PHY_INTERFACE_MODE_SGMII, interfaces);
>>> +	if (!phy_validate(ethqos->serdes_phy, PHY_MODE_ETHERNET,
>>> +			  PHY_INTERFACE_MODE_2500BASEX, NULL))
>>> +		__set_bit(PHY_INTERFACE_MODE_2500BASEX, interfaces);
>>> +}
>>> +
>>
>> So, I had proposed this exact change a while back:
>> https://lore.kernel.org/netdev/aMgootkPQ%2FGcdiXX@oss.qualcomm.com/
>>
>> As Russell noted, this behaviour is due to the messed-up integrated
>> PCS support in STMMAC/QCOM-ETHQOS.
>>
>> I would defer to him for this if he's around.
> 
> Indeed, but we haven't heard from him in a while, let's see if we can
> figure this out.
> 
> Does this IP have an integrated PCS ? (i.e. dma_cap.pcs is set)
> you can check that in debugfs :
> 
> mount -t debugfs none /sys/kernel/debug
> cat /sys/kernel/debug/stmmaceth/eth0/dma_cap | grep PCS
> 
> if it's Y, then you have the integrated one.
> 
> Russell worked hard on that, it looks like a lot of the plumbing went
> through.
> 
> I think the right approach here is to have the PCS itself report the
> list of supported interfaces, instead of relying on the glue. looking
> at stmmac_pcs.c :
> 
> 220 int stmmac_integrated_pcs_init(struct stmmac_priv *priv,
> 221                                const struct stmmac_pcs_info *pcs_info)
> 222 {
> 223         struct stmmac_pcs *spcs;
> 224
> 225         spcs = devm_kzalloc(priv->device, sizeof(*spcs), GFP_KERNEL);
> 226         if (!spcs)
> 227                 return -ENOMEM;
> 
> [...]
>                                                               
> 246         /* Only allow 2500BASE-X if the SerDes has support. */
> 247         if (priv->plat->flags & STMMAC_FLAG_SERDES_SUPPORTS_2500M)
> 248                 __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> 249                           spcs->pcs.supported_interfaces);
> 250
> 251         priv->integrated_pcs = spcs;
> 252
> 253         return 0;
> 254 }
> 
> Can you test settung the STMMAC_FLAG_SERDES_SUPPORTS_2500M flag in
> dwmac-qcom-ethqos ?

I can confirm that dma_cap.pcs = Y.

I tested setting STMMAC_FLAG_SERDES_SUPPORTS_2500M on hardware and 
unfortunately the flag alone doesn't solve the problem.

I think there are a couple of things missing. stmmac_phylink_setup 
doesn't use the integrated PCS bitmap to populate 
config->supported_interfaces, so SGMII is still not advertised to 
phylink as a usable host interface. Also stmmac_integrated_pcs_init is 
adding 1000BASE-X unconditionally, which wouldn't be appropriate here.

I did a proof-of-concept fixing those things and the end result looks a 
little messy: config->supported_interfaces could come from either 
get_interfaces _or_ the integrated PCS with unclear (to me) responsibility.

This POC did show that STMMAC_FLAG_SERDES_SUPPORTS_2500M enabled phylink 
to configure the integrated PCS for 2500BASE-X and a 2.5 Gbps link came 
up with no apparent issues.

Do you have any thoughts what the clean end-state should be?

Tom

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

* Re: [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces
  2026-09-17  4:29       ` Thomas Karpiniec
@ 2026-09-17  5:36         ` Maxime Chevallier
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Chevallier @ 2026-09-17  5:36 UTC (permalink / raw)
  To: Thomas Karpiniec, Mohd Ayaan Anwar, Coia Prant
  Cc: netdev, Russell King, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Choong Yong Liang,
	Bartosz Golaszewski, Russell King, linux-arm-msm, linux-kernel

Hi Thomas,

+Coia

On 9/17/26 06:29, Thomas Karpiniec wrote:

>> Can you test settung the STMMAC_FLAG_SERDES_SUPPORTS_2500M flag in
>> dwmac-qcom-ethqos ?
> 
> I can confirm that dma_cap.pcs = Y.

Thanks for looking up :) This is a good start

> 
> I tested setting STMMAC_FLAG_SERDES_SUPPORTS_2500M on hardware and unfortunately the flag alone doesn't solve the problem.

Yeah, but it's still the right first move. Russell planned to do that as well :

https://lore.kernel.org/netdev/E1vvDJi-0000000ArhH-3ipf@rmk-PC.armlinux.org.uk/#t

> 
> I think there are a couple of things missing. stmmac_phylink_setup doesn't use the integrated PCS bitmap to populate config->supported_interfaces, so SGMII is still not advertised to phylink as a usable host interface. Also stmmac_integrated_pcs_init is adding 1000BASE-X unconditionally, which wouldn't be appropriate here.

Ok so, the missing bit seems to be that priv->integrated_pcs isn't taken
into account in stmmac_phylink_setup(). This needs to be added then :)

Looks like there's also the inband support that needs addressing, that was
part of his RFC here :

https://lore.kernel.org/netdev/E1vvDJi-0000000ArhH-3ipf@rmk-PC.armlinux.org.uk/#t

> I did a proof-of-concept fixing those things and the end result looks a little messy: config->supported_interfaces could come from either get_interfaces _or_ the integrated PCS with unclear (to me) responsibility.

There's only dwmac-intel, dwmac-spacemit and dwmac-rk that use the
.get_interfaces() API. Let me add Coia in CC, as they're working
on better PCS support specifically for Rockchip :

https://lore.kernel.org/netdev/20260915123802.1561724-1-coiaprant@gmail.com/

For spacemit, looks like it's not really a problem as there's no PCS
involved. For intel though, heh... Intel platforms with dwmac AND PCS
are hard to come by, but I think at that point let's roll with the
bitwise or'int of get_interfaces + pcs interfaces.

> This POC did show that STMMAC_FLAG_SERDES_SUPPORTS_2500M enabled phylink to configure the integrated PCS for 2500BASE-X and a 2.5 Gbps link came up with no apparent issues.

Maybe the blind spot is inband aneg. Mohd, you were quite involved in
this, do you remember where we stand on that front ?

Thanks,

Maxime



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

end of thread, other threads:[~2026-09-17  5:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 10:39 [PATCH net 0/2] stmmac: fix ETHQOS SerDes interface handling Thomas Karpiniec
2026-09-16 10:39 ` [PATCH net 1/2] net: stmmac: propagate platform mac_finish errors Thomas Karpiniec
2026-09-16 12:03   ` Mohd Ayaan Anwar
2026-09-17  4:01     ` Thomas Karpiniec
2026-09-16 13:02   ` Maxime Chevallier
2026-09-16 10:39 ` [PATCH net 2/2] net: stmmac: qcom-ethqos: advertise supported SerDes interfaces Thomas Karpiniec
2026-09-16 12:17   ` Mohd Ayaan Anwar
2026-09-16 12:58     ` Maxime Chevallier
2026-09-17  4:29       ` Thomas Karpiniec
2026-09-17  5:36         ` Maxime Chevallier

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

all inboxes | Powered by JetHome®