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

[Yogesh Gaur]

On Wed, Sep 9, 2026 at 10:05 PM Heiner Kallweit <hkallweit1@xxxxxxxxx> wrote:
>
> On 09.09.2026 13:05, Yogesh Gaur wrote:
> > 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!
>
> > Fixes: 9ab94a32af70 ("r8169: enable LTR support")
> > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2529752
> > Signed-off-by: Yogesh Gaur <yogeshgaur.83@xxxxxxxxx>
> > ---
> >  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.

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.

Please suggest.

Yogesh

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