RE: [PATCH net-next 3/3] r8169: disable RTL8126 ZRX-DC timeout

Hau
2025-02-24T16:09:51Z

>
> 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:
> > Disable it due to it dose not meet ZRX-DC specification. If it is
> > enabled, device will exit L1 substate every 100ms. Disable it for
> > saving more power in L1 substate.
> >
> Is this compliant with the PCIe spec? Not being an expert on this topic, but
> when I read e.g. the following then my understanding is that this wakeup
> every 100ms is the expected behavior.
>
> https://lore.kernel.org/all/1610033323-10560-4-git-send-email-
> shradha.t@samsung.com/T/
>
ZRX-DC timeout is for L1.Idle, not for L1 Substate. And according to the spec below, because it will reduce power saving, it is not encouraged to implement.
So we disable it for saving power.

Next state is Recovery after a 100 ms timeout if the current data rate is 8.0 GT/s or higher and the Port’s
Receivers do not meet the ZRX-DC specification for 2.5 GT/s). All Ports are permitted, but not encouraged, to
implement the timeout and transition to Recovery when the data rate is 8.0 GT/s or higher.

> > Signed-off-by: ChunHao Lin <hau@realtek.com>
> > ---
> > drivers/net/ethernet/realtek/r8169_main.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/realtek/r8169_main.c
> > b/drivers/net/ethernet/realtek/r8169_main.c
> > index 9953eaa01c9d..7a5b99d54e12 100644
> > --- a/drivers/net/ethernet/realtek/r8169_main.c
> > +++ b/drivers/net/ethernet/realtek/r8169_main.c
> > @@ -2851,6 +2851,21 @@ static u32 rtl_csi_read(struct rtl8169_private *tp,
> int addr)
> > RTL_R32(tp, CSIDR) : ~0; }
> >
> > +static void rtl_disable_zrxdc_timeout(struct rtl8169_private *tp) {
> > + struct pci_dev *pdev = tp->pci_dev;
> > + u8 val;
> > +
> > + if (pdev->cfg_size > 0x0890 &&
> > + pci_read_config_byte(pdev, 0x0890, &val) == PCIBIOS_SUCCESSFUL
> &&
> > + pci_write_config_byte(pdev, 0x0890, val & ~BIT(0)) ==
> PCIBIOS_SUCCESSFUL)
> > + return;
> > +
> > + netdev_notice_once(tp->dev,
> > + "No native access to PCI extended config space, falling back to
> CSI\n");
> > + rtl_csi_write(tp, 0x0890, rtl_csi_read(tp, 0x0890) & ~BIT(0)); }
> > +
> > static void rtl_set_aspm_entry_latency(struct rtl8169_private *tp, u8
> > val) {
> > struct pci_dev *pdev = tp->pci_dev; @@ -3930,6 +3945,7 @@ static
> > void rtl_hw_start_8125d(struct rtl8169_private *tp)
> >
> > static void rtl_hw_start_8126a(struct rtl8169_private *tp) {
> > + rtl_disable_zrxdc_timeout(tp);
> > rtl_set_def_aspm_entry_latency(tp);
> > rtl_hw_start_8125_common(tp);
> > }