[PATCH net] net: stmmac: intel: don't reconfigure SerDes on unchanged mode

[PATCH net] net: stmmac: intel: don't reconfigure SerDes on unchanged mode

[Markus Breitenberger]

From: Markus Breitenberger <bre@keba.com>

intel_mac_finish() is registered as the phylink mac_finish() callback
for the Elkhart Lake SGMII ports. phylink calls mac_finish() at the end
of every major link reconfiguration, including the initial one during
probe, before any interface mode has actually changed.

The callback reprograms the shared ModPHY LCPLL through the PMC IPC and
then power-cycles the SerDes. On Elkhart Lake that ModPHY is also used
by the on-die AHCI SATA PHY. Running the reconfiguration during the
initial boot-time link-up disturbs the shared analog block while it is
still driving SATA, so the SATA link fails to train:

  ata1: SATA link down (SStatus 1 SControl 300)

The disk carrying the root filesystem is never detected and the system
hangs at rootwait. Ethernet itself comes up normally, which makes the
failure look unrelated to the network driver.

Firmware already programs the ModPHY for the configured interface, so
the reconfiguration is redundant unless the interface mode really
changes. Return early when the requested mode equals the current one.
This avoids touching the shared ModPHY (and the SATA PHY) during boot
while preserving runtime SGMII to 2500BASE-X switching, which still
sees a genuine mode change and reconfigures as before.

Fixes: a42f6b3f1cc1 ("net: stmmac: configure SerDes according to the interface mode")
Cc: stable@vger.kernel.org
Assisted-by: GitHub-Copilot:claude-opus-4.8
Signed-off-by: Markus Breitenberger <bre@keba.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index b8d467ba6d72..9a162831ca40 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -536,6 +536,15 @@ static int intel_mac_finish(struct net_device *ndev,
 	int max_regs = 0;
 	int ret = 0;
 
+	/* mac_finish() runs at the end of every major link reconfiguration,
+	 * including the initial one at probe, where the interface mode has
+	 * not actually changed. Reprogramming and power-cycling the SerDes is
+	 * only needed on a real mode change and is otherwise needlessly
+	 * disruptive, so skip it when the mode is unchanged.
+	 */
+	if (priv->plat->phy_interface == interface)
+		return 0;
+
 	ret = intel_tsn_lane_is_available(ndev, intel_priv);
 	if (ret < 0) {
 		netdev_info(priv->dev, "No TSN lane available to set the registers.\n");
-- 
2.55.0

Re: [PATCH net] net: stmmac: intel: don't reconfigure SerDes on unchanged mode

[Maxime Chevallier]

Hi Markus,

On 7/6/26 08:19, Markus Breitenberger wrote:
> From: Markus Breitenberger <bre@keba.com>
> 
> intel_mac_finish() is registered as the phylink mac_finish() callback
> for the Elkhart Lake SGMII ports. phylink calls mac_finish() at the end
> of every major link reconfiguration, including the initial one during
> probe, before any interface mode has actually changed.
> 
> The callback reprograms the shared ModPHY LCPLL through the PMC IPC and
> then power-cycles the SerDes. On Elkhart Lake that ModPHY is also used
> by the on-die AHCI SATA PHY. Running the reconfiguration during the
> initial boot-time link-up disturbs the shared analog block while it is
> still driving SATA, so the SATA link fails to train:
> 
>   ata1: SATA link down (SStatus 1 SControl 300)
> 
> The disk carrying the root filesystem is never detected and the system
> hangs at rootwait. Ethernet itself comes up normally, which makes the
> failure look unrelated to the network driver.
> 
> Firmware already programs the ModPHY for the configured interface, so
> the reconfiguration is redundant unless the interface mode really
> changes. Return early when the requested mode equals the current one.
> This avoids touching the shared ModPHY (and the SATA PHY) during boot
> while preserving runtime SGMII to 2500BASE-X switching, which still
> sees a genuine mode change and reconfigures as before.

One thing is that now we 'blindly' rely on the bootloader / fw having
correctly configured the initial interface.

From what I see the only configuration that's done is regarding the serdes
rate. Maybe instead the serdes interaction logic can be reworked so that you
query the serdes rate, see if you need to adjust it based on the selected
interface, and if so you re-configure it ?

Maxime

Re: [PATCH net] net: stmmac: intel: don't reconfigure SerDes on unchanged mode

[Andrew Lunn]

On Mon, Jul 06, 2026 at 08:19:54AM +0200, Markus Breitenberger wrote:
> From: Markus Breitenberger <bre@keba.com>
> 
> intel_mac_finish() is registered as the phylink mac_finish() callback
> for the Elkhart Lake SGMII ports. phylink calls mac_finish() at the end
> of every major link reconfiguration, including the initial one during
> probe, before any interface mode has actually changed.
> 
> The callback reprograms the shared ModPHY LCPLL through the PMC IPC and
> then power-cycles the SerDes. On Elkhart Lake that ModPHY is also used
> by the on-die AHCI SATA PHY. Running the reconfiguration during the
> initial boot-time link-up disturbs the shared analog block while it is
> still driving SATA, so the SATA link fails to train:
> 
>   ata1: SATA link down (SStatus 1 SControl 300)
> 
> The disk carrying the root filesystem is never detected and the system
> hangs at rootwait. Ethernet itself comes up normally, which makes the
> failure look unrelated to the network driver.
> 
> Firmware already programs the ModPHY for the configured interface, so
> the reconfiguration is redundant unless the interface mode really
> changes. Return early when the requested mode equals the current one.
> This avoids touching the shared ModPHY (and the SATA PHY) during boot
> while preserving runtime SGMII to 2500BASE-X switching, which still
> sees a genuine mode change and reconfigures as before.

What happens to the disk at runtime, rather than boot time, if it is
necessary to reconfigure the ModPHY?

I think i would prefer the machine fails to boot, rather than corrupt
its disk when i plug it into a different network switch.

	Andrew

Re: [PATCH net] net: stmmac: intel: don't reconfigure SerDes on unchanged mode

[Markus Breitenberger]

Hi Maxime,

Thanks for the review, and sorry - my commit message was wrong and
sent you down the wrong path.

> One thing is that now we 'blindly' rely on the bootloader / fw
> having correctly configured the initial interface.

That impression came from my commit message, where I wrote that
"firmware already programs the ModPHY for the configured interface".
That was incorrect: the kernel programs the SerDes rate itself, in
intel_serdes_powerup(), from priv->plat->phy_interface. I'll drop that
claim in v2.

> Maybe instead the serdes interaction logic can be reworked so that
> you query the serdes rate, see if you need to adjust it based on the
> selected interface, and if so you re-configure it ?

That's a good suggestion, and I'll do exactly that in v2. Instead of
comparing the cached priv->plat->phy_interface, it will read the
current lane rate back from SERDES_GCR0 and only run the disruptive PMC
reconfiguration when the rate actually differs from what the selected
interface needs:

  cur_rate = (data & SERDES_RATE_MASK) >> SERDES_RATE_PCIE_SHIFT;
  want_rate = interface == PHY_INTERFACE_MODE_2500BASEX ?
                  SERDES_RATE_PCIE_GEN2 : SERDES_RATE_PCIE_GEN1;
  return cur_rate != want_rate;

The callback selects between the 1G and 2.5G ModPHY programming tables
from the requested interface, and the SerDes lane rate is the
observable state that tells us whether that programming is already
active. intel_mac_finish() applies the selected table to the shared
ModPHY LCPLL through the PMC IPC (intel_set_reg_access()) and then
power-cycles the SerDes, and that power-cycle is what disturbs the
on-die AHCI SATA PHY sharing the ModPHY on Elkhart Lake. Gating on the
actual rate keeps that reprogramming out of the boot path when the
SerDes is already configured correctly, while still handling a genuine
SGMII to 2500BASE-X change at runtime. If the read cannot be completed,
the helper returns true so the reconfiguration runs as before.

One caveat: the read-back covers the SerDes rate bits, which is
the setting relevant to this regression; it does not read back the
full LCPLL DWORD state. I'm keying on the rate because that is what the
initial mac_finish() would re-apply on this path, but if you'd rather
key the decision off something more specific I'm happy to adjust.

Thanks again for the pointer - reading the hardware is clearly the
more robust check.

Markus

Re: [PATCH net] net: stmmac: intel: don't reconfigure SerDes on unchanged mode

[Markus Breitenberger]

Hi Andrew,

Thanks for looking at this, and you're right - the runtime case is the
more dangerous one. If a genuine interface change (SGMII <-> 2500BASE-X)
happened at runtime while the disk was live, reprogramming the shared
ModPHY LCPLL would disturb the SATA PHY under an active filesystem, and
a failed boot would be preferable to that.

Two points of clarification:

- A plain switch change does not reprogram the ModPHY on my fixed-PHY
  setup. mac_finish() only runs a real reconfiguration when the
  MAC-side interface mode changes (e.g. a multi-rate SFP moving between
  SGMII and 2500BASE-X). On a fixed copper PHY the interface mode does
  not change, so changing the link partner / switch does not trigger
  the reconfiguration.

- The runtime reconfiguration path is not introduced by this patch. It
  came in with the Fixes: commit a42f6b3f1cc1 ("net: stmmac: configure
  SerDes according to the interface mode"), which added
  intel_mac_finish()/intel_set_reg_access() and the PMC LCPLL
  reprogramming. v2 will only read the current SerDes rate back from
  SERDES_GCR0 and skip the reconfiguration when it already matches the
  selected interface. At boot that suppresses the redundant reprogram
  that breaks SATA; for a real rate change, v2 leaves the
  reconfiguration unchanged from mainline.

So for the runtime case you are worried about - a real ModPHY rate
change while SATA is live - this patch does not make things safer or
more dangerous; it only removes the spurious boot-time reprogramming.
The broader question of protecting a live SATA disk against a real
runtime ModPHY change is pre-existing, and I don't have a board that
combines a multi-rate SFP with SATA on the same ModPHY, so I can't
exercise or safely test a guard for that topology.

Given that, I'd like to keep this patch scoped to the boot regression
and leave the pre-existing shared-ModPHY-with-live-SATA question to the
maintainers, who have the hardware knowledge to decide whether a
stronger guard is warranted.

Thanks,
Markus

Re: [PATCH net] net: stmmac: intel: don't reconfigure SerDes on unchanged mode

[Andrew Lunn]

On Wed, Jul 08, 2026 at 12:08:14AM +0200, Markus Breitenberger wrote:
> Hi Andrew,
> 
> Thanks for looking at this, and you're right - the runtime case is the
> more dangerous one. If a genuine interface change (SGMII <-> 2500BASE-X)
> happened at runtime while the disk was live, reprogramming the shared
> ModPHY LCPLL would disturb the SATA PHY under an active filesystem, and
> a failed boot would be preferable to that.
> 
> Two points of clarification:
> 
> - A plain switch change does not reprogram the ModPHY on my fixed-PHY
>   setup. mac_finish() only runs a real reconfiguration when the
>   MAC-side interface mode changes (e.g. a multi-rate SFP moving between
>   SGMII and 2500BASE-X).

A fibre SFP is unlikely to use SGMII. It will swap between 2500BaseX
and 1000BaseX, if the SFP module is ejected and a different one
plugged in.

> On a fixed copper PHY the interface mode does
>   not change, so changing the link partner / switch does not trigger
>   the reconfiguration.

That depends on the PHY. Some change there host side interface to
match the line side. So they use 25000BaseX for 2.5G, but SGMII for
10/100/1G. Other use 'rate-adaptation'. They run the host side at the
fastest speed, 25000BaseX, and then insert pause frames to slow down
the MAC when the line side is running at 10/100/1G.

> Given that, I'd like to keep this patch scoped to the boot regression
> and leave the pre-existing shared-ModPHY-with-live-SATA question to the
> maintainers, who have the hardware knowledge to decide whether a
> stronger guard is warranted.

a42f6b3f1cc1 is from Intel, so i assume they thought about what
happens to the SATA controller, and are happy to take the risk of
destroying filesystems. So, yes, lets leave it as is for the moment.

	   Andrew