RE: [PATCH net-next v1 2/6] r8169: add support for phylink

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

>
>Hi,
>
>On 6/5/26 12:39, javen wrote:
>> From: Javen Xu <javen_xu@realsil.com.cn>
>>
>> Transfer old framework to phylink. Phylink can support fiber mode card
>> which can not get link status or link speed from standard phy registers.
>
>This is a good start, but you need to go deeper than that. Looking at the end
>result, you still access tp->phydev a lot in this driver :
>
>Looking at r8169_mdio_register() for example :
>
> -> don't configure the PHY eee support with phy_support_eee() and
>    phy_disable_eee_mode(), let phylink to that for you
>
>All over the driver, there's still a lot of manual control of the PHY with phylib,
>look at the calls for phy_init_hw(), phy_resume(), and so on, you need to
>assume that with phylink you may not have a PHY.
>
>The problem then is that there are places in the code where PHY registers are
>directly accessed :
>
>static void rtl8169_init_phy(struct rtl8169_private *tp) { [...]
>
>        if (tp->mac_version == RTL_GIGA_MAC_VER_05 &&
>            tp->pci_dev->subsystem_vendor == PCI_VENDOR_ID_GIGABYTE &&
>            tp->pci_dev->subsystem_device == 0xe000)
>                phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b); [...]
>        genphy_soft_reset(tp->phydev);
>}
>
>In the end, you shouldn't even need to use tp->phydev at all.
>
>It's hard to know what this is all about, but it seems like something a PHY
>driver should have to do, not a MAC driver :(
>
>Also, I'd merge the next commit with this one to have one single commit doing
>the phylink conversion.
>
>Maxime
>
Hi Maxime,

I have one question about the existing link-change interrupt model.

For most r8169 chips, the PHY exists and the current driver uses the MAC LinkChg interrupt to notify phylib:
        if (status & LinkChg)
                phy_mac_interrupt(tp->phydev);

After converting the driver to phylink, my understanding is that the MAC driver should avoid keeping a private tp->phydev pointer. The PHY can still be found during setup and passed to phylink_connect_phy(), but then I am not sure how the MAC-routed PHY interrupt should be handled.

Would switching the internal PHY to PHY_POLL be acceptable , or would you prefer to keep the existing interrupt-driven behaviour? If the latter, what would be the preferred way to notify phylib without storing tp->phydev in the driver?

I also have a related question about the existing firmware handling and sw_cnt_1ms_ini logic. Some of this code touches MAC MMIO registers, so it does not seem suitable to move it entirely into the Realtek PHY driver. The current code uses tp->phydev in this area.

What would be the preferred phylink-compatible way to handle this? Should the driver still keep the tp->phydev, or is there another recommended approach?

Link: https://lore.kernel.org/netdev/b54b5f3a6703498fbc17e64548550ba0@realsil.com.cn/

Thanks,
BRs,
Javen