Re: [PATCH net-next 2/3] r8169: enable RTL8168H/RTL8168EP/RTL8168FP/RTL8125/RTL8126 LTR support

Heiner Kallweit
2025-02-21T19:39:05Z

On 21.02.2025 08:18, ChunHao Lin wrote:
> This patch will enable RTL8168H/RTL8168EP/RTL8168FP/RTL8125/RTL8126 LTR
> support on the platforms that have tested with LTR enabled.
>

Where in the code is the check whether platform has been tested with LTR?

> Signed-off-by: ChunHao Lin <hau@realtek.com>
> ---
> drivers/net/ethernet/realtek/r8169_main.c | 108 ++++++++++++++++++++++
> 1 file changed, 108 insertions(+)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 731302361989..9953eaa01c9d 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -2955,6 +2955,111 @@ static void rtl_disable_exit_l1(struct rtl8169_private *tp)
> }
> }
>
> +static void rtl_set_ltr_latency(struct rtl8169_private *tp)
> +{
> + switch (tp->mac_version) {
> + case RTL_GIGA_MAC_VER_70:
> + case RTL_GIGA_MAC_VER_71:
> + r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdd2, 0x8c09);
> + r8168_mac_ocp_write(tp, 0xcdd8, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdd4, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdda, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdd6, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcddc, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcde8, 0x887a);
> + r8168_mac_ocp_write(tp, 0xcdea, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdec, 0x8c09);
> + r8168_mac_ocp_write(tp, 0xcdee, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdf0, 0x8a62);
> + r8168_mac_ocp_write(tp, 0xcdf2, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdf4, 0x883e);
> + r8168_mac_ocp_write(tp, 0xcdf6, 0x9003);
> + break;
> + case RTL_GIGA_MAC_VER_61 ... RTL_GIGA_MAC_VER_66:
> + r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdd2, 0x889c);
> + r8168_mac_ocp_write(tp, 0xcdd8, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdd4, 0x8c30);
> + r8168_mac_ocp_write(tp, 0xcdda, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdd6, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcddc, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcde8, 0x883e);
> + r8168_mac_ocp_write(tp, 0xcdea, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdec, 0x889c);
> + r8168_mac_ocp_write(tp, 0xcdee, 0x9003);
> + r8168_mac_ocp_write(tp, 0xcdf0, 0x8C09);
> + r8168_mac_ocp_write(tp, 0xcdf2, 0x9003);
> + break;
> + case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_53:
> + 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);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void rtl_reset_pci_ltr(struct rtl8169_private *tp)
> +{
> + struct pci_dev *pdev = tp->pci_dev;
> + u16 cap;
> +
> + pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &cap);
> + if (cap & PCI_EXP_DEVCTL2_LTR_EN) {
> + pcie_capability_clear_word(pdev, PCI_EXP_DEVCTL2,
> + PCI_EXP_DEVCTL2_LTR_EN);
> + pcie_capability_set_word(pdev, PCI_EXP_DEVCTL2,
> + PCI_EXP_DEVCTL2_LTR_EN);

I'd prefer that only PCI core deals with these registers (functions like
pci_configure_ltr()). Any specific reason for this reset? Is it something
which could be applicable for other devices too, so that the PCI core
should be extended?

+Bjorn and PCI list, to get an opinion from the PCI folks.

> + }
> +}
> +
> +static void rtl_enable_ltr(struct rtl8169_private *tp)
> +{
> + switch (tp->mac_version) {
> + case RTL_GIGA_MAC_VER_61 ... RTL_GIGA_MAC_VER_71:
> + r8168_mac_ocp_modify(tp, 0xe034, 0x0000, 0xc000);
> + r8168_mac_ocp_modify(tp, 0xe0a2, 0x0000, BIT(0));
> + r8168_mac_ocp_modify(tp, 0xe032, 0x0000, BIT(14));
> + break;
> + case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
> + case RTL_GIGA_MAC_VER_52 ... RTL_GIGA_MAC_VER_53:
> + r8168_mac_ocp_modify(tp, 0xe0a2, 0x0000, BIT(0));
> + RTL_W8(tp, 0xb6, RTL_R8(tp, 0xb6) | BIT(0));
> + fallthrough;
> + case RTL_GIGA_MAC_VER_51:
> + r8168_mac_ocp_modify(tp, 0xe034, 0x0000, 0xc000);
> + r8168_mac_ocp_write(tp, 0xe02c, 0x1880);
> + r8168_mac_ocp_write(tp, 0xe02e, 0x4880);
> + break;
> + default:
> + return;
> + }
> +
> + rtl_set_ltr_latency(tp);
> +
> + /* chip can trigger LTR */
> + r8168_mac_ocp_modify(tp, 0xe032, 0x0003, BIT(0));
> +
> + /* reset LTR to notify host */
> + rtl_reset_pci_ltr(tp);
> +}
> +
> +static void rtl_disable_ltr(struct rtl8169_private *tp)
> +{
> + switch (tp->mac_version) {
> + case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_71:
> + r8168_mac_ocp_modify(tp, 0xe032, 0x0003, 0);
> + break;
> + default:
> + break;
> + }
> +}
> +
> static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
> {
> u8 val8;
> @@ -2971,6 +3076,8 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
> tp->mac_version == RTL_GIGA_MAC_VER_43)
> return;
>
> + rtl_enable_ltr(tp);
> +
> rtl_mod_config5(tp, 0, ASPM_en);
> switch (tp->mac_version) {
> case RTL_GIGA_MAC_VER_70:
> @@ -4821,6 +4928,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
>
> rtl8169_cleanup(tp);
> rtl_disable_exit_l1(tp);
> + rtl_disable_ltr(tp);

Any specific reason why LTR isn't configured just once, on driver load?

> rtl_prepare_power_down(tp);
>
> if (tp->dash_type != RTL_DASH_NONE)