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

[Javen]

>On 13.03.2026 08:25, Javen wrote:
>>> 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?
>>
>Based on the bit numbers this seems to be the BMSR register. Can all clause
>22 registers be accessed in a similar way so that a MII bus can be implemented
>for accessing the SerDes?

We checked our chip documentation and confirmed that the SerDes register file maps
all clause 22 registers(e.g. WR 0xeb10<reg_addr>, RD 0xeb14. reg_addr 0x40 is for BMCR,
0x41 for BMSR, 0x42 for PHYSID1, 0x43 for PHYSID2, etc.)
As you suggested, I wonder whether the proper approach is to implement a virtual mii_bus
and map the standard serdes reg in the new map.

Thanks,
Javen Xu

>In general it's hard to provide a statement w/o knowing the internal layout of
>the IP blocks.
>Can you provide any chip documentation / datasheet?
>For proper support of such a component chain between MAC and SFP port
>the driver would have to be converted to use phylink, what requires access to
>the involved components, incl. the SFP I2C bus.
>
>
>> 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);
>>