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

[Hau <hau@realtek.com>]

> 
> External mail : This email originated from outside the organization. Do not
> reply, click links, or open attachments unless you recognize the sender and
> know the content is safe.
> 
> 
> 
> 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?
> 
LTR is for L1,2. But L1 will be disabled when rtl_aspm_is_safe() return false. So LTR needs rtl_aspm_is_safe()
to return true.

> > 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?
> 
It is for specific platform. On that platform driver needs to do this to let LTR works.

> +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?
> 
It is for device compatibility, I will check internally to see if we can remove it.


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