Thread (9 messages) flat view 9 messages, 3 authors, 17h ago
HOTtoday

[PATCH net v3] r8169: don't enable chip LTR when the platform has not enabled LTR

From: Yogesh Gaur <yogeshgaur.83@gmail.com>
Date: 2026-09-11 13:14:09
Also in: lkml
Subsystem: 8169 10/100/1000 gigabit ethernet driver, networking drivers, the rest · Maintainers: Heiner Kallweit, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

rtl_enable_ltr() programs the MAC to generate LTR messages - ALDPS_LTR_EN,
LTR_SNOOP_EN, LTR_OBFF_LOCK_EN, plus LINK_SPEED_CHANGE_EN on
RTL8125/RTL8126/RTL8127 - and rtl_hw_aspm_clkreq_enable() calls it on
every ASPM enable, then goes on to let the chip trigger L1.2.

The only gate is tp->aspm_manageable, which records that the OS is allowed
to control ASPM. It says nothing about LTR. LTR is a separate PCIe
capability that only works if every device on the path to the root port
supports it. The PCI core works that out in pci_configure_ltr() and
records the result in pci_dev->ltr_path; per PCIe r6.0 sec 7.5.3.16 a
function must not issue LTR messages while LTR Mechanism Enable is clear.

So on a platform whose hierarchy has no LTR path, the driver tells the
chip to start sending LTR messages nothing will honour, and ties ALDPS -
the PHY's link-down power saving - to them. A report against RTL8125B
(rev 05, firmware rtl8125b-2_0.0.2) in a mini PC shows the effect: 291
link down/up transitions in one eight-hour boot, with repeated downshifts
to 100Mbps, against four transitions at boot and then a stable link on
the kernel before the LTR change.

Gate the chip's LTR programming on pci_dev->ltr_path. That field exists
only when CONFIG_PCIEASPM is enabled, so the test needs a preprocessor
guard rather than IS_ENABLED(). With CONFIG_PCIEASPM=n the PCI core's
pci_configure_ltr() is a stub and LTR is enabled nowhere, so the chip
must not issue LTR messages in that configuration either.

Note this does change CONFIG_PCIEASPM=n builds, which used to program
chip LTR unconditionally: pci_disable_link_state() is a stub that returns
success there, so tp->aspm_manageable ends up set.

Fixes: 9ab94a32af70 ("r8169: enable LTR support")
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2529752
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
v3:
 - Drop the #ifdef/#else pair of rtl_enable_ltr() definitions and keep a
   single function with the guard inline, so there is no stub function.
   Suggested by Heiner Kallweit.
 - IS_ENABLED(CONFIG_PCIEASPM) cannot replace the #ifdef here: ltr_path is
   declared inside #ifdef CONFIG_PCIEASPM in include/linux/pci.h, so the
   member reference still has to compile in a CONFIG_PCIEASPM=n build:
     error: 'struct pci_dev' has no member named 'ltr_path'
   (x86_64 defconfig + EXPERT=y + PCIEASPM=n + R8169=m). Happy to send a
   PCI patch first that either moves the field out of the #ifdef or adds a
   pcie_ltr_path() accessor, and then drop the #ifdef from here - please
   say if you would prefer that, it is a different tree.
 - Added testing disclosure below.

v2: https://lore.kernel.org/all/20260910130140.910-1-yogeshgaur.83@gmail.com/ (local)
v1: https://lore.kernel.org/all/20260909110554.1977-1-yogeshgaur.83@gmail.com/ (local)

NOT VERIFIED ON HARDWARE. I have no RTL8125 here. The causal claim above is
reasoned from the register writes.

Compile-tested only: drivers/net/ethernet/realtek/r8169_main.o, x86_64
defconfig + R8169=m, built W=1 clean with both CONFIG_PCIEASPM=y and
CONFIG_PCIEASPM=n.

 drivers/net/ethernet/realtek/r8169_main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ec4fc21fa21f..c305a8f19551 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -3037,6 +3037,18 @@ static void rtl_disable_exit_l1(struct rtl8169_private *tp)
 
 static void rtl_enable_ltr(struct rtl8169_private *tp)
 {
+	/* The chip must not issue LTR messages unless LTR is enabled for the
+	 * whole path up to the root port. pci_configure_ltr() works that out
+	 * and records the result in pci_dev->ltr_path, which exists only with
+	 * CONFIG_PCIEASPM; without it the PCI core never enables LTR anywhere.
+	 */
+#ifdef CONFIG_PCIEASPM
+	if (!tp->pci_dev->ltr_path)
+		return;
+#else
+	return;
+#endif
+
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_80:
 		r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);
-- 
2.55.0.windows.5
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help