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

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

>
>>  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.

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

BRs,
Javen

>
>        Andrew