diff mbox series

[net-next] r8169: avoid link-up interrupt issue on RTL8106e if user enables ASPM

Message ID 7060a8ba-720f-904f-a6c6-c873559d8dbe@gmail.com (mailing list archive)
State Accepted
Commit 1ee8856de82faec9bc8bd0f2308a7f27e30ba207
Delegated to: Netdev Maintainers
Headers show
Series [net-next] r8169: avoid link-up interrupt issue on RTL8106e if user enables ASPM | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Heiner Kallweit June 10, 2021, 8:56 p.m. UTC
It has been reported that on RTL8106e the link-up interrupt may be
significantly delayed if the user enables ASPM L1. Per default ASPM
is disabled. The change leaves L1 enabled on the PCIe link (thus still
allowing to reach higher package power saving states), but the
NIC won't actively trigger it.

Reported-by: Koba Ko <koba.ko@canonical.com>
Tested-by: Koba Ko <koba.ko@canonical.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Koba Ko June 11, 2021, 8:04 a.m. UTC | #1
On Fri, Jun 11, 2021 at 4:57 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> It has been reported that on RTL8106e the link-up interrupt may be
> significantly delayed if the user enables ASPM L1. Per default ASPM
> is disabled. The change leaves L1 enabled on the PCIe link (thus still
> allowing to reach higher package power saving states), but the
> NIC won't actively trigger it.
>
> Reported-by: Koba Ko <koba.ko@canonical.com>
> Tested-by: Koba Ko <koba.ko@canonical.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 64f94a3fe..6a9fe9f7e 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -3508,7 +3508,6 @@ static void rtl_hw_start_8106(struct rtl8169_private *tp)
>         rtl_eri_write(tp, 0x1b0, ERIAR_MASK_0011, 0x0000);
>
>         rtl_pcie_state_l2l3_disable(tp);
> -       rtl_hw_aspm_clkreq_enable(tp, true);
>  }

As per 0866cd15029b, this also affects the intel soc idle state.
Even the result is positive currently, I think this modification would
have higher risk.

>
>  DECLARE_RTL_COND(rtl_mac_ocp_e00e_cond)
> --
> 2.32.0
>
Heiner Kallweit June 11, 2021, 9:14 a.m. UTC | #2
On 11.06.2021 10:04, Koba Ko wrote:
> On Fri, Jun 11, 2021 at 4:57 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> It has been reported that on RTL8106e the link-up interrupt may be
>> significantly delayed if the user enables ASPM L1. Per default ASPM
>> is disabled. The change leaves L1 enabled on the PCIe link (thus still
>> allowing to reach higher package power saving states), but the
>> NIC won't actively trigger it.
>>
>> Reported-by: Koba Ko <koba.ko@canonical.com>
>> Tested-by: Koba Ko <koba.ko@canonical.com>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/net/ethernet/realtek/r8169_main.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index 64f94a3fe..6a9fe9f7e 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -3508,7 +3508,6 @@ static void rtl_hw_start_8106(struct rtl8169_private *tp)
>>         rtl_eri_write(tp, 0x1b0, ERIAR_MASK_0011, 0x0000);
>>
>>         rtl_pcie_state_l2l3_disable(tp);
>> -       rtl_hw_aspm_clkreq_enable(tp, true);
>>  }
> 
> As per 0866cd15029b, this also affects the intel soc idle state.
> Even the result is positive currently, I think this modification would
> have higher risk.
> 
At the time of 0866cd15029b ASPM was enabled in r8169. Interesting that
after 0866cd15029b nobody reported the link-up issue you're facing.
A few months later (with b75bb8a5b755) we had to disable ASPM again
because of several problem reports. Since then 0866cd15029b has no
practical relevance in mainline.

>>
>>  DECLARE_RTL_COND(rtl_mac_ocp_e00e_cond)
>> --
>> 2.32.0
>>
patchwork-bot+netdevbpf@kernel.org June 11, 2021, 8 p.m. UTC | #3
Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Thu, 10 Jun 2021 22:56:59 +0200 you wrote:
> It has been reported that on RTL8106e the link-up interrupt may be
> significantly delayed if the user enables ASPM L1. Per default ASPM
> is disabled. The change leaves L1 enabled on the PCIe link (thus still
> allowing to reach higher package power saving states), but the
> NIC won't actively trigger it.
> 
> Reported-by: Koba Ko <koba.ko@canonical.com>
> Tested-by: Koba Ko <koba.ko@canonical.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> [...]

Here is the summary with links:
  - [net-next] r8169: avoid link-up interrupt issue on RTL8106e if user enables ASPM
    https://git.kernel.org/netdev/net-next/c/1ee8856de82f

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 64f94a3fe..6a9fe9f7e 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -3508,7 +3508,6 @@  static void rtl_hw_start_8106(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0x1b0, ERIAR_MASK_0011, 0x0000);
 
 	rtl_pcie_state_l2l3_disable(tp);
-	rtl_hw_aspm_clkreq_enable(tp, true);
 }
 
 DECLARE_RTL_COND(rtl_mac_ocp_e00e_cond)