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

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

[Yogesh Gaur]

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.

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;
+
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_80:
 		r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);
-- 
2.55.0.windows.5

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

[Heiner Kallweit]

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@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.

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

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@gmail.com> 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@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.

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);
>

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

[Heiner Kallweit]

On 10.09.2026 08:01, Yogesh Gaur wrote:
> On Wed, Sep 9, 2026 at 10:05 PM Heiner Kallweit <hkallweit1@gmail.com> 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@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
> 
>>>       switch (tp->mac_version) {
>>>       case RTL_GIGA_MAC_VER_80:
>>>               r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);
>>

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 1 · Low: 0

- [High] rtl_enable_ltr()
  (drivers/net/ethernet/realtek/r8169_main.c:3038-3048) gates all chip…
- [Medium] The new gate only skips *programming* the chip's LTR
  machinery; nothing ever programs it to the off state.

--

r8169: don't enable chip LTR when the platform has not enabled LTR

This adds a check in rtl_enable_ltr() that reads the endpoint's
PCI_EXP_DEVCTL2 register and returns early when PCI_EXP_DEVCTL2_LTR_EN is
clear, so the chip's LTR machinery is not programmed on hierarchies where
LTR was not enabled.

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2529752

> 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;

[Severity: High]
Is the endpoint's LTR Mechanism Enable bit really what the PCI core uses to
reflect an end-to-end LTR path?

Looking at pci_configure_ltr() in drivers/pci/pcie/aspm.c, when the bit is
already set (for example by firmware), the core leaves it alone and only sets
the path flag if the upstream bridge already has one:

	pcie_capability_read_dword(pdev, PCI_EXP_DEVCTL2, &ctl);
	if (ctl & PCI_EXP_DEVCTL2_LTR_EN) {
		if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) {
			pdev->ltr_path = 1;
			return;
		}

		bridge = pci_upstream_bridge(pdev);
		if (bridge && bridge->ltr_path)
			pdev->ltr_path = 1;

		return;
	}

So on a machine where firmware set LTR_EN on the r8169 function but the
hierarchy above it has no LTR path, the function returns with LTR_EN still set
and pdev->ltr_path left at 0.

The whole-path result appears to live in pci_dev::ltr_path in
include/linux/pci.h instead:

	unsigned int	ltr_path:1;	/* Latency Tolerance Reporting
					   supported from root to here */

and aspm.c uses that field, not DEVCTL2, when deciding L1.2 is unusable:

	if (!child->ltr_path)
		child_l1ss_cap &= ~PCI_L1SS_CAP_ASPM_L1_2;

If that reading is right, then on the platform class described in the changelog
the new test passes, rtl_hw_start() -> rtl_hw_aspm_clkreq_enable(tp, true) ->
rtl_enable_ltr() still programs ALDPS_LTR_EN, LTR_SNOOP_EN,
LINK_SPEED_CHANGE_EN and finally LTR_OBFF_LOCK_EN, and the chip is still let to
trigger L1.2. Does the link-flap case the changelog reports stay reachable
in that configuration?

There is also the CONFIG_PCIEASPM=n build, where pci_configure_ltr() is an
empty stub in drivers/pci/pci.h while pci_disable_link_state() is a stub
returning 0, so tp->aspm_manageable stays 1 and rtl_enable_ltr() still runs.
In that build no path discovery has happened at all - is testing the bit
meaningful there, or is it just leftover firmware state?

Alongside that, could the comment and the commit message be adjusted? The
comment states the core "reflects it in LTR Mechanism Enable", and the changelog
says the core "records the result by setting LTR Mechanism Enable in the
endpoint's Device Control 2 register", which does not hold for the
firmware-already-set case shown above.

[Severity: Medium]
This early return skips the programming, but does anything ever program the
chip's LTR bits to the off state?

Grepping r8169_main.c, ALDPS_LTR_EN, LTR_SNOOP_EN, LTR_OBFF_LOCK_EN,
LINK_SPEED_CHANGE_EN and COMBO_LTR_EXTEND_EN only ever appear in set
operations inside rtl_enable_ltr():

	r8168_mac_ocp_modify(tp, ALDPS_LTR, 0x0000, ALDPS_LTR_EN);
	RTL_W8(tp, COMBO_LTR_EXTEND, RTL_R8(tp, COMBO_LTR_EXTEND) | COMBO_LTR_EXTEND_EN);

The disable branch of rtl_hw_aspm_clkreq_enable() clears only 0xe092,
ClkReqEn/INT_CFG0_CLKREQEN and ASPM_en, so those LTR bits stay armed.

If they were armed earlier and rtl_enable_ltr() later takes the new early
return - say a pre-boot UEFI Realtek driver configured ASPM plus LTR, or an
earlier call ran while LTR_EN read as set and the bit was cleared afterwards,
with rtl_reset_work() -> rtl_hw_start() re-entering this path on the same
device - does the chip keep emitting LTR messages with LTR Mechanism Enable
clear and ALDPS still tied to them?

Would writing the off values for those bits when the platform has not enabled
LTR be more robust than skipping the writes, so the driver's view and the
chip's state cannot diverge?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909110554.1977-1-yogeshgaur.83%40gmail.com