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

[Andrew Lunn <andrew@lunn.ch>]

On Thu, Jul 23, 2026 at 03:21:26AM +0000, Javen wrote:
> >
> >>  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