quoted

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

Do we know what these magic numbers mean?

0xdc00 is a package-detect field. 0xd006 is internal HW id. RTL8116AF shares the same RTL_GIGA_MAC_VER_52 mac_version with other variants. 

quoted

 static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private
*tp)  {
      switch (tp->mac_version) {

@@ -2397,7 +2431,7 @@ static int rtl8169_set_link_ksettings(struct

net_device *ndev,

quoted

      int duplex = cmd->base.duplex;
      int speed = cmd->base.speed;

-     if (!tp->sfp_mode)
+     if (tp->sfp_mode != RTL_SFP_8127_ATF)
              return phylink_ethtool_ksettings_set(tp->phylink, cmd);

Is this even needed? phylink should be able to handle sfp and copper in the
same way.

I will try to handle this.

quoted

@@ -2509,9 +2543,10 @@ void r8169_apply_firmware(struct

rtl8169_private *tp)

quoted

              tp->ocp_base = OCP_STD_PHY_BASE;

              /* PHY soft reset may still be in progress */
-             phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
-                                   !(val & BMCR_RESET),
-                                   50000, 600000, true);
+             if (tp->phydev)
+                     phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
+                                           !(val & BMCR_RESET),
+                                           50000, 600000, true);

Maybe this all needs to move into the PHY driver?

This is after firmware application. And PHY_MDIO_CHG opcode switches the access callbacks between PHY and MAC accessors.
data == 0: phy_read/phy_write
data != 0: mac_mcu_read/mac_mcu_write
So the firmware may contain mixed PHY and MAC.

quoted

-     rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
-     if (rg_saw_cnt > 0) {
-             u16 sw_cnt_1ms_ini;
+     if (tp->phydev) {
+             rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
+             if (rg_saw_cnt > 0) {
+                     u16 sw_cnt_1ms_ini;

-             sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
-             r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
+                     sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
+                     r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
+             }

Can this move into the PHY driver?

It reads a counter from PHY, but the calculated value is programmed into MAC OCP register via r8168_mac_ocp_modify, which accesses r8169 through tp->mmio_addr. So I think this can not be moved.

quoted

@@ -5017,9 +5054,11 @@ static void rtl8169_up(struct rtl8169_private *tp)
              rtl8168_driver_start(tp);

      pci_set_master(tp->pci_dev);
-     phy_init_hw(tp->phydev);
-     phy_resume(tp->phydev);
-     rtl8169_init_phy(tp);
+     if (tp->phydev) {
+             phy_init_hw(tp->phydev);
+             phy_resume(tp->phydev);
+             rtl8169_init_phy(tp);
+     }

Why is this needed?

I will try to remove this.

Thanks for your review.

BRs,
Javen