Public Hi Marek,
> -----Original Message----- > From: Marek Vasut <[email protected]> > Sent: Wednesday, May 13, 2026 7:09 PM > To: Vinay Tilak, Pranav <[email protected]>; [email protected]; > Simek, Michal <[email protected]> > Cc: git (AMD-Xilinx) <[email protected]>; Begari, Padmarao > <[email protected]>; Jerome Forissier > <[email protected]>; Tom Rini <[email protected]>; Peng Fan > <[email protected]>; Patrice Chotard <[email protected]>; > Lucien.Jheng <[email protected]>; Siddharth Vadapalli <s- > [email protected]>; SkyLake.Huang <[email protected]>; Marek > Vasut <[email protected]>; Ramon Fried > <[email protected]> > Subject: Re: [PATCH v5] net: phy: fix duplicate eth_phy binding > > On 5/13/26 3:33 PM, 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 phy_connect() also binds the same PHY > > node, resulting in duplicate entries in the DM tree. > > > > Fix this by checking at the beginning of phy_connect() whether the PHY > > is already bound via uclass_find_device_by_phandle(). If so, skip all > > generic binding methods and fall through to phy_find_by_mask(). Also > > assign phydev->node from the already bound DM device. > > > > Fixes: 68a4d1506109 ("net: phy: Bind ETH_PHY uclass driver to each new > > PHY") > > Signed-off-by: Pranav Tilak <[email protected]> > > --- > > Changes in v5: > > - Use int ret instead of bool phy_bound and goto out > > > > Changes in v4: > > - Removed IS_ENABLED(CONFIG_DM_ETH_PHY), use runtime check via > > uclass_find_device_by_phandle() > > - Fix indentation of binding methods block > > - Assign phydev->node from the bound DM device's ofnode > > > > Changes in v3: > > - Moved duplicate binding check to beginning of phy_connect() > > > > Changes in v2: > > - Move duplicate binding check to caller phy_connect() > > - Update commit description > > --- > > drivers/net/phy/phy.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index > > d7e0c4fe02d..7d698f00c09 100644 > > --- a/drivers/net/phy/phy.c > > +++ b/drivers/net/phy/phy.c > > @@ -24,6 +24,7 @@ > > #include <linux/delay.h> > > #include <linux/err.h> > > #include <linux/compiler.h> > > +#include <dm/uclass-internal.h> > > > > /* Generic PHY support and helper functions */ > > > > @@ -927,6 +928,14 @@ struct phy_device *phy_connect(struct mii_dev > *bus, int addr, > > { > > struct phy_device *phydev = NULL; > > uint mask = (addr >= 0) ? (1 << addr) : 0xffffffff; > > + struct udevice *phy_dev; > > + int ret; > > + > > + /* Skip binding if PHY already bound by eth_phy_binds_nodes(). */ > > + ret = uclass_find_device_by_phandle(UCLASS_ETH_PHY, dev, > > + "phy-handle", &phy_dev); > > + if (!ret) > > + goto out; > > > > #ifdef CONFIG_PHY_FIXED > > phydev = phy_connect_fixed(bus, dev); @@ -947,9 +956,13 @@ > struct > > phy_device *phy_connect(struct mii_dev *bus, int addr, > > phydev = phy_connect_gmii2rgmii(bus, dev); > > #endif > > > > +out: > > if (!phydev) > > phydev = phy_find_by_mask(bus, mask); > > > > + if (phydev && !ret && !ofnode_valid(phydev->node)) > > + phydev->node = dev_ofnode(phy_dev); > Is the !ret test necessary in this conditional ? Yes i checked, !ret is necessary as uclass_find_device_by_phandle() always sets phy_dev= NULL on entry, when it fails to find an already bound PHY phy_dev is NULL. Removing !ret would call dev_ofnode(NULL) which dereferences a NULL pointer and crashes. !ret also serves as a guard for the assignment of phydev->node it should only happen when the PHY is already DM bound. When binding methods run (ret != 0), they already set phydev->node correctly so no need to again set phydev->node in that case. The !ofnode_valid(phydev->node) check is redundant and can be dropped. In the ret == 0 path, only phy_find_by_mask() runs and it never sets phydev->node, so !ofnode_valid(phydev->node) is always true. Hence condition can be simplified to: if (phydev && !ret) phydev->node = dev_ofnode(phy_dev); Thanks & regards, Pranav Tilak
