[PATCH v2] net: phy: fix duplicate eth_phy binding

[Marek Vasut]

On 4/16/26 9:12 AM, Vinay Tilak, Pranav wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Marek,
> 
>> -----Original Message-----
>> From: Marek Vasut <marek.vasut at mailbox.org>
>> Sent: Wednesday, April 8, 2026 10:02 PM
>> To: Vinay Tilak, Pranav <Pranav.VinayTilak at amd.com>; u-boot at lists.denx.de;
>> Simek, Michal <michal.simek at amd.com>
>> Cc: git (AMD-Xilinx) <git at amd.com>; Begari, Padmarao
>> <Padmarao.Begari at amd.com>; Jerome Forissier
>> <jerome.forissier at arm.com>; Tom Rini <trini at konsulko.com>; Peng Fan
>> <peng.fan at nxp.com>; Lucien.Jheng <lucienzx159 at gmail.com>; Yao Zi
>> <me at ziyao.cc>; SkyLake.Huang <skylake.huang at mediatek.com>; Siddharth
>> Vadapalli <s-vadapalli at ti.com>; Marek Vasut
>> <marek.vasut+renesas at mailbox.org>; Ramon Fried <rfried.dev at gmail.com>
>> Subject: Re: [PATCH v2] net: phy: fix duplicate eth_phy binding
>>
>> On 4/8/26 10:58 AM, Pranav Tilak wrote:
>>> When both CONFIG_PHY_ETHERNET_ID and CONFIG_DM_ETH_PHY are
>> enabled,
>>> eth_phy_binds_nodes() called from eth_post_bind() already binds the
>>> ethernet PHY node to eth_phy_generic_drv. However,
>>> phy_connect_phy_id() called via zynq_phy_init() -> phy_connect()
>>> cannot attach to the already bound PHY device and creates a second
>>> instance, resulting in duplicate entries in the dm tree:
>>>
>>> Fix this by guarding the phy_connect_phy_id() call in phy_connect()
>>> with uclass_get_device_by_phandle() to skip re-binding if the PHY node
>>> is already registered in UCLASS_ETH_PHY.
>>>
>>> Fixes: 68a4d1506109 ("net: phy: Bind ETH_PHY uclass driver to each new
>>> PHY")
>>> Signed-off-by: Pranav Tilak <pranav.vinaytilak at amd.com>
>>> ---
>>> Changes in v2:
>>> - Move duplicate binding check to caller phy_connect()
>>> - Update commit description
>>>
>>>    drivers/net/phy/phy.c | 9 +++++++--
>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index
>>> d7e0c4fe02d..cfca8328f1c 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -938,8 +938,13 @@ struct phy_device *phy_connect(struct mii_dev
>> *bus, int addr,
>>>    #endif
>>>
>>>    #ifdef CONFIG_PHY_ETHERNET_ID
>>> -   if (!phydev)
>>> -           phydev = phy_connect_phy_id(bus, dev, addr);
>>> +   if (!phydev) {
>>> +           struct udevice *phy_dev;
>>> +
>>> +           if (uclass_get_device_by_phandle(UCLASS_ETH_PHY, dev,
>>> +                                            "phy-handle", &phy_dev))
>>> +                   phydev = phy_connect_phy_id(bus, dev, addr);
>> Shouldn't this binding check be closer to the beginning of phy_connect() ?
>> Basically check whether the PHY driver is already bound at the beginning, and
>> if so, you are done. I don't think that test is specific to
>> CONFIG_PHY_ETHERNET_ID, is it ?
> 
> The check is placed inside phy_connect_phy_id() because that is the only code path that calls device_bind_driver_to_node() on the PHY node, which creates the duplicate.

The device_bind_driver_to_node() in phy_connect_phy_id() is called only 
under very specific conditions -- if IS_ENABLED(CONFIG_DM_ETH_PHY) and 
phy-handle property exists and points to a valid node. The DM_ETH_PHY 
test is not covered here.

But that is besides the point. I think phy_connect() should figure out 
whether any PHY is already connected, and if it is, simply exit early. 
This sounds to me like a more generic solution.

> The other paths (phy_connect_fixed, NCSI, phy_connect_gmii2rgmii) do not bind the PHY node and are not affected.
I worry this is only a matter of time, hence it would be good to have a 
generic solution which covers all the PHY options.