On 5/15/26 1:13 PM, Vinay Tilak, Pranav wrote:
[...]
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;
Please pick a variable name which is not phy_dev , there is already a
phydev variable in this function and the naming is VERY confusing.
+ 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;
You already check if (!ret) here ...
#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.
... so I think the goto jump target needs to be adjusted ?
In fact, I do not understand why does this code call phy_find_by_mask()
if the PHY udevice was already found by uclass_find_device_by_phandle()
? Can you not convert that already found PHY udevice into phydev ?
!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.
If uclass_find_device_by_phandle() does locate PHY udevice , it will
return 0, and the !ret conditional will evaluate to true , so
phydev->node = dev_ofnode(phy_dev); will run 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);
[...]
