RE: [PATCH net-next v1 2/4] r8169: add sfp mode for RTL8116af

[Javen]

>On 02.03.2026 07:32, javen wrote:
>> From: Javen Xu <javen_xu@xxxxxxxxxxxxxx>
>>
>> RTL8116af is a variation of RTL8168fp. It uses SerDes instead of PHY.
>> But SerDes status will not relect to PHY. So it needs to add sfp mode
>> for quirk to help reflect SerDes status during PHY read.
>>
>
>Is there any mass market device using this chip version? As far as possible I'd
>like to avoid adding support for chip versions that never make it to the mass
>market. This just makes driver maintenance harder.
>
>The patch includes support for 100Mbps fiber mode. Is there any use case for
>such legacy modes?
>
>
>> Signed-off-by: Javen Xu <javen_xu@xxxxxxxxxxxxxx>
>> ---
>>  drivers/net/ethernet/realtek/r8169_main.c | 71
>> ++++++++++++++++++++---
>>  1 file changed, 62 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c
>> b/drivers/net/ethernet/realtek/r8169_main.c
>> index fb2247a20c36..a5c0d3995328 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -726,6 +726,12 @@ enum rtl_dash_type {
>>       RTL_DASH_25_BP,
>>  };
>>
>> +enum rtl_sfp_mode {
>> +     RTL_SFP_NONE,
>> +     RTL_SFP_8168_AF,
>> +     RTL_SFP_8127_ATF,
>> +};
>> +
>>  struct rtl8169_private {
>>       void __iomem *mmio_addr;        /* memory map physical address */
>>       struct pci_dev *pci_dev;
>> @@ -734,6 +740,7 @@ struct rtl8169_private {
>>       struct napi_struct napi;
>>       enum mac_version mac_version;
>>       enum rtl_dash_type dash_type;
>> +     enum rtl_sfp_mode sfp_mode;
>>       u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
>>       u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
>>       u32 dirty_tx;
>> @@ -760,7 +767,6 @@ struct rtl8169_private {
>>       unsigned supports_gmii:1;
>>       unsigned aspm_manageable:1;
>>       unsigned dash_enabled:1;
>> -     bool sfp_mode:1;
>>       dma_addr_t counters_phys_addr;
>>       struct rtl8169_counters *counters;
>>       struct rtl8169_tc_offsets tc_offset; @@ -1126,7 +1132,7 @@
>> static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
>>               return 0;
>>
>>       /* Return dummy MII_PHYSID2 in SFP mode to match SFP PHY driver */
>> -     if (tp->sfp_mode && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
>> +     if (tp->sfp_mode == RTL_SFP_8127_ATF && reg ==
>(OCP_STD_PHY_BASE
>> + + 2 * MII_PHYSID2))
>>               return PHY_ID_RTL_DUMMY_SFP & 0xffff;
>>
>>       RTL_W32(tp, GPHY_OCP, reg << 15); @@ -1270,6 +1276,34 @@ static
>> int r8168g_mdio_read(struct rtl8169_private *tp, int reg)
>>       return r8168_phy_ocp_read(tp, tp->ocp_base + reg * 2);  }
>>
>> +/* The quirk reflects RTL8116af SerDes status. */ static int
>> +r8116af_mdio_read_quirk(struct rtl8169_private *tp, int reg) {
>> +     u8 phyStatus = RTL_R8(tp, PHYstatus);
>> +
>> +     if (!(phyStatus & LinkStatus))
>> +             return 0;
>> +
>> +     /* BMSR */
>> +     if (tp->ocp_base == OCP_STD_PHY_BASE && reg == MII_BMSR)
>> +             return BMSR_ANEGCOMPLETE | BMSR_LSTATUS;
>> +
>> +     /* PHYSR */
>> +     if (tp->ocp_base == 0xa430 && reg == 0x12) {
>> +             if (phyStatus & _1000bpsF)
>> +                     return 0x0028;
>> +             else if (phyStatus & _100bps)
>> +                     return 0x0018;
>
>This is a complete hack. Any means to access the SerDes directly?

Hi, Heiner
Now we find a indirect access to serdes reg. The serdes reg is designed according to IEEE 802.3.
BIT 2 means Link Status and BIT 5 means ANEGCOMPLETE. We can access it through read mac ocp
0xeb14, I wonder whether this indirect access can be accepted?

Thanks,
Javen Xu

>
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int r8116af_mdio_read(struct rtl8169_private *tp, int reg) {
>> +     return r8168g_mdio_read(tp, reg) | r8116af_mdio_read_quirk(tp,
>> +reg); }
>> +
>>  static void mac_mcu_write(struct rtl8169_private *tp, int reg, int
>> value)  {
>>       if (reg == 0x1f) {
>> @@ -1280,6 +1314,13 @@ static void mac_mcu_write(struct rtl8169_private
>*tp, int reg, int value)
>>       r8168_mac_ocp_write(tp, tp->ocp_base + reg, value);  }
>>
>> +static bool rtl_is_8116af(struct rtl8169_private *tp) {
>> +     return tp->mac_version == RTL_GIGA_MAC_VER_52 &&
>> +             (r8168_mac_ocp_read(tp, 0xdc00) & 0x0078) == 0x0030 &&
>> +             (r8168_mac_ocp_read(tp, 0xd006) & 0x00ff) == 0x0000; }
>> +
>>  static int mac_mcu_read(struct rtl8169_private *tp, int reg)  {
>>       return r8168_mac_ocp_read(tp, tp->ocp_base + reg); @@ -1386,7
>> +1427,10 @@ static int rtl_readphy(struct rtl8169_private *tp, int location)
>>       case RTL_GIGA_MAC_VER_31:
>>               return r8168dp_2_mdio_read(tp, location);
>>       case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_LAST:
>> -             return r8168g_mdio_read(tp, location);
>> +             if (tp->sfp_mode == RTL_SFP_8168_AF)
>> +                     return r8116af_mdio_read(tp, location);
>> +             else
>> +                     return r8168g_mdio_read(tp, location);
>>       default:
>>               return r8169_mdio_read(tp, location);
>>       }
>> @@ -1575,6 +1619,20 @@ static bool rtl_dash_is_enabled(struct
>rtl8169_private *tp)
>>       }
>>  }
>>
>> +static enum rtl_sfp_mode rtl_get_sfp_mode(struct rtl8169_private *tp)
>> +{
>> +     if (rtl_is_8125(tp)) {
>> +             u16 data = r8168_mac_ocp_read(tp, 0xd006);
>> +
>> +             if ((data & 0xff) == 0x07)
>> +                     return RTL_SFP_8127_ATF;
>> +     } else if (rtl_is_8116af(tp)) {
>> +             return RTL_SFP_8168_AF;
>> +     }
>> +
>> +     return RTL_SFP_NONE;
>> +}
>> +
>>  static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private
>> *tp)  {
>>       switch (tp->mac_version) {
>> @@ -5693,12 +5751,7 @@ static int rtl_init_one(struct pci_dev *pdev, const
>struct pci_device_id *ent)
>>       }
>>       tp->aspm_manageable = !rc;
>>
>> -     if (rtl_is_8125(tp)) {
>> -             u16 data = r8168_mac_ocp_read(tp, 0xd006);
>> -
>> -             if ((data & 0xff) == 0x07)
>> -                     tp->sfp_mode = true;
>> -     }
>> +     tp->sfp_mode = rtl_get_sfp_mode(tp);
>>
>>       tp->dash_type = rtl_get_dash_type(tp);
>>       tp->dash_enabled = rtl_dash_is_enabled(tp);