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

[Javen <javen_xu@realsil.com.cn>]

Hi Andrew,
>
>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

Just a gentle ping on this thread.

I am currently preparing for the next version. However, since you mentioned earlier that you were planning to look at patch 5/7 in more detail, I want to ask if you prefer me to send the next version or should I hold off and wait for your feedback on patch 5/7 so I can address everything in next patch.

Thanks,
Javen