Re: [PATCH net-next v7 4/7] r8169: add support for RTL8116af

On Thu, Jul 23, 2026 at 03:21:26AM +0000, Javen wrote:

quoted

quoted

 static void rtl8169_init_phy(struct rtl8169_private *tp)  {
+     phy_init_hw(tp->phydev);
+     phy_resume(tp->phydev);
      r8169_hw_phy_config(tp, tp->phydev, tp->mac_version);

      if (tp->mac_version <= RTL_GIGA_MAC_VER_06) { @@ -2562,7 +2607,7

@@ static void rtl8169_init_phy(struct rtl8169_private *tp)
          tp->pci_dev->subsystem_device == 0xe000)
              phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b);

-     if (tp->sfp_mode)
+     if (tp->sfp_mode == RTL_SFP_8127_ATF)
              rtl_sfp_init(tp);

This could be diff getting me confused....

You unconditionally call phy_init_hw() and phy_resume(). And then if this
device should have an SFP you do something else. How can a device with an
SFP also have a PHY?

I totally agree that the SFP initialization logic does not belong in a function meant for PHYs. But rtl_sfp_init() call was already located inside rtl8169_init_phy() before my series. And it works well. This mainly because the patch which add support for RTL8127atf(commit id: fef0f545511f72223) assume that RTL8127atf has a dummy phy. So tp->phydev will not be NULL.

[Goes and looks at what rtl_sfp_init() actually does]

So it does not touch phydev making this function at least safe.

And in patch 5/7 of this series which add phylink support for RTL8127atf, I remove it.

Yes, i did notice the dummy PHY getting deleted. I will look at that
patch in more detail.

      Thanks
	Andrew