Re: [PATCH net-next v2 5/6] r8169: add ltr support for RTL8116af
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-06-13 22:06:03
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- r8169: add ltr support for RTL8116af This patch adds ltr support for RTL8116af, enables RTL8116af enter l1.2 state. This makes sense for the system to enter c10 state.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index a73c0215b2407..a49e7a8d8f4da 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3153,8 +3155,22 @@ static void rtl_enable_ltr(struct rtl8169_private *tp) r8168_mac_ocp_write(tp, 0xcdf2, 0x9003); r8168_mac_ocp_modify(tp, LTR_OBFF_LOCK, 0x0000, LINK_SPEED_CHANGE_EN); break; - case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48: case RTL_GIGA_MAC_VER_52: + r8168_mac_ocp_write(tp, 0xcdd0, 0x9003); + r8168_mac_ocp_modify(tp, LTR_SNOOP, 0x0000, LTR_SNOOP_EN); + r8168_mac_ocp_write(tp, 0xe02c, 0x1880); + r8168_mac_ocp_write(tp, 0xe02e, 0x4880);
[Severity: High] Does this completely redefine the LTR enable sequence for all devices using RTL_GIGA_MAC_VER_52? Since older devices like RTL8117 also use this MAC version, could this cause regressions for them by replacing the previous fallthrough behavior? Should there be a check to ensure this only runs for the newly added RTL8116af?
quoted hunk ↗ jump to hunk
+ r8168_mac_ocp_modify(tp, ALDPS_LTR, 0x0000, ALDPS_LTR_EN); + r8168_mac_ocp_write(tp, 0xcdd8, 0x9003); + r8168_mac_ocp_write(tp, 0xcdda, 0x9003); + r8168_mac_ocp_write(tp, 0xcddc, 0x9003); + r8168_mac_ocp_write(tp, 0xcdd2, 0x883c); + r8168_mac_ocp_write(tp, 0xcdd4, 0x8c12); + r8168_mac_ocp_write(tp, 0xcdd6, 0x9003); + r8168_mac_ocp_write(tp, 0xe0a6, 0x9003); + r8168_mac_ocp_write(tp, 0xe0a8, 0x9003); + break; + case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48: 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); fallthrough;@@ -3174,6 +3190,7 @@ static void rtl_enable_ltr(struct rtl8169_private *tp) } /* chip can trigger LTR */ r8168_mac_ocp_modify(tp, LTR_OBFF_LOCK, 0x0003, LTR_OBFF_LOCK_EN); + r8168_mac_ocp_modify(tp, SEND_LTR_MSG, 0x0000, LTR_MSG_EN);
[Severity: High] This unconditionally modifies SEND_LTR_MSG at the end of rtl_enable_ltr(), which is called for multiple MAC versions. Was it intended to apply globally to all chips, or should this change also be restricted to RTL8116af?
} static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3732,7 +3751,9 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp) rtl_eri_set_bits(tp, 0xd4, 0x0010); - rtl_eri_write(tp, 0x5f0, ERIAR_MASK_0011, 0x4f87); + rtl_eri_write(tp, 0x5f0, ERIAR_MASK_0011, 0x4000); + + r8168_mac_ocp_write(tp, 0xe098, 0xc302);
[Severity: High] Since older devices also use rtl_hw_start_8117(), will applying these register changes unconditionally to all RTL_GIGA_MAC_VER_52 devices affect the hardware initialization of existing devices? Could we add a check to restrict these tweaks specifically to RTL8116af?
quoted hunk ↗ jump to hunk
rtl_disable_rxdvgate(tp);@@ -3759,9 +3780,9 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp) } r8168_mac_ocp_modify(tp, 0xe056, 0x00f0, 0x0000); - r8168_mac_ocp_write(tp, 0xea80, 0x0003); - r8168_mac_ocp_modify(tp, 0xe052, 0x0000, 0x0009); - r8168_mac_ocp_modify(tp, 0xd420, 0x0fff, 0x047f); + r8168_mac_ocp_write(tp, 0xea80, 0x0000); + r8168_mac_ocp_modify(tp, 0xe052, 0x0009, 0x0000); + r8168_mac_ocp_modify(tp, 0xd420, 0x0fff, 0x045f);
[Severity: High] Like the changes above in rtl_hw_start_8117(), these magic register updates will be applied to RTL8117 as well. Can this break link stability or ASPM for existing devices by replacing their expected register values?
r8168_mac_ocp_write(tp, 0xe63e, 0x0001); r8168_mac_ocp_write(tp, 0xe63e, 0x0000);