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

[Heiner Kallweit]

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?
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);
>