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

[Heiner Kallweit]

On 01.04.2026 03:43, Javen wrote:
>> On 17.03.2026 04:14, Javen wrote:
>>>> 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.
>>>
>> We'd need to know how the internal PHY and the SerDes play together.
>> Whether the usual internal PHY plays any role in communication between
>> MAC and SerDes.
>> How interface modes are controlled on MAC and SerDes side, etc.
>> If Realtek doesn't implement proper layering in hw, and doesn't have enough
>> knowledge about mainline network driver structures, and on the other hand
>> doesn't want to provide chip documentation, then the situation somewhat
>> deadlocks.
>>
> 
> Hi, Heiner
> For RTL8116af, the datapath is structured as MAC->PCS->SerDes.
> However, PCS block does not have a standard MDIO interface. Instead, we have to 
> access the standard reg via MAC OCP.These standard registers are exactly the standard 
> IEEE 802.3 Clause 22 PHY registers.
> Given hardware information above, I would like to ask for your advice on the recommended 
> approach to support RTL8116af. Or any other information do I need to provide?
> 

The described scenario MAC->PCS->SerDes is a standard scenario supported
by phylink, see e.g. phylink_pcs_ops and the drivers under drivers/net/pcs.
So you best get familiar with phylink, and start thinking about what it
would take to drive your IP blocks using phylink.

And still: Proper advice needs documentation as input. So if Realtek asks
for advice, they should also be willing to share datasheets etc.

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