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

On 10.09.2026 08:01, Yogesh Gaur wrote:

On Wed, Sep 9, 2026 at 10:05 PM Heiner Kallweit [off-list ref] wrote:

quoted

On 09.09.2026 13:05, Yogesh Gaur wrote:

quoted

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 determines that in pci_configure_ltr() and
records the result by setting LTR Mechanism Enable in the endpoint's
Device Control 2 register; per PCIe r6.0 sec 7.5.3.16 a function must not
issue LTR messages while that bit is clear.

So on a platform whose hierarchy has no LTR path, the driver now 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.

Read the endpoint's LTR Mechanism Enable bit and leave the chip's LTR
machinery alone when the platform did not enable it.
pcie_capability_read_word() zeroes its output on error, so an unreadable
capability takes the same safe path.

Thanks for the fix!

quoted

Fixes: 9ab94a32af70 ("r8169: enable LTR support")
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2529752
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ec4fc21fa21f..c1ff4e898570 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c

@@ -3037,6 +3037,16 @@ static void rtl_disable_exit_l1(struct rtl8169_private *tp)

 static void rtl_enable_ltr(struct rtl8169_private *tp)
 {
+     u16 ctl2;
+
+     /* The chip must not issue LTR messages unless the platform enabled
+      * LTR on the whole path up to the root port. The PCI core discovers
+      * that in pci_configure_ltr() and reflects it in LTR Mechanism Enable.
+      */
+     pcie_capability_read_word(tp->pci_dev, PCI_EXP_DEVCTL2, &ctl2);
+     if (!(ctl2 & PCI_EXP_DEVCTL2_LTR_EN))
+             return;
+

Can't you simply query tp->pci_dev->ltr_path instead of doing this low-level
PCI register read? When reading through pci_configure_ltr(), I think this
should do the trick.

Thats was actually my first version, but it does not build: struct
pci_dev::ltr_path
is inside #ifdef CONFIG_PCIEASPM (include/linux/pci.h), and r8169 can be built
with CONFIG_PCIEASPM=n.

However, w/o support for PCIe NIC's you don't need rtl_enable_ltr() at all.
So the complete function could be conditionally compiled.

I think we should keep DEVCTL2 read. It is what other drivers with this need do
- rtw89(rtw89_pci_dev_ltr_enabled()), iwlwifi (pcie/gen1_2/trans.c),
qed_rdma.c,
rtsx_pcr.c all read PCI_EXP_DEVCTL2 and test PCI_ECP_DEVCTL2_LTR_EN.

I'd not consider these old drivers as role models. As far as possible
I'd like to leave dealing with low-level PCI registers to the PCI subsystem.

Please suggest.

Yogesh

quoted

quoted

      switch (tp->mac_version) {
      case RTL_GIGA_MAC_VER_80:
              r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);