phy_dev_ops() returns dev->driver->ops without checking it, and every generic_phy_*() helper dereferences the result unconditionally. A PHY driver registered without an operations table therefore faults on the first PHY access.
Add a uclass post_bind hook rejecting such drivers, as the pinctrl uclass already does, instead of open-coding a NULL check in each helper. Return -ENODEV so that lists_bind_fdt() only skips the offending device instead of failing the whole device tree scan. Signed-off-by: Julien Stephan <[email protected]> --- While developing a new PHY driver, I forgot to add the ops table, and my U-Boot crashed: phy_dev_ops() hands out dev->driver->ops without checking it, and the generic_phy_*() helpers dereference the result unconditionally. No in-tree driver is affected today. Other uclasses, such as pinctrl and remoteproc, do this kind of check either in post_bind (pinctrl) or in post_probe (remoteproc). Add a post_bind callback in the PHY uclass to add this check. A note on the error code: the equivalent check in the pinctrl uclass returns -EINVAL. This series deliberately returns -ENODEV instead: lists_bind_fdt() treats -ENODEV as "Driver refuses to bind" but does not break the boot, while any other error code interrupts the boot. IMHO, a PHY driver not binding should not break the build. Maybe pinctrl should move to -ENODEV too, but that is left out of this series. One note: I checked all 9 members of struct phy_ops, and every one is individually guarded (of_xlate, init, exit, reset, power_on, power_off, configure, set_mode, set_speed) and has a fallback return value. This implies that none of the ops callbacks is mandatory. So maybe a better fix is to add a default empty ops {}? I would be happy to switch to this if necessary. --- drivers/phy/phy-uclass.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c index f8d4fb3b41b..6ba4cc593df 100644 --- a/drivers/phy/phy-uclass.c +++ b/drivers/phy/phy-uclass.c @@ -85,6 +85,20 @@ static int phy_alloc_counts(struct phy *phy, struct udevice *supply) return 0; } +static int phy_uclass_post_bind(struct udevice *dev) +{ + /* + * The uclass dereferences the driver operations unconditionally, + * so refuse to bind a driver which does not provide any. + */ + if (!phy_dev_ops(dev)) { + dev_err(dev, "PHY driver has no ops\n"); + return -ENODEV; + } + + return 0; +} + static int phy_uclass_pre_probe(struct udevice *dev) { struct list_head *uc_priv = dev_get_uclass_priv(dev); @@ -552,6 +566,7 @@ int generic_shutdown_phy(struct phy *phy) UCLASS_DRIVER(phy) = { .id = UCLASS_PHY, .name = "phy", + .post_bind = phy_uclass_post_bind, .pre_probe = phy_uclass_pre_probe, .pre_remove = phy_uclass_pre_remove, .per_device_auto = sizeof(struct list_head), --- base-commit: 5437f0d7943e352ebe126f66275edf4f8206b545 change-id: 20260914-phy-reject-driver-without-ops-446d841d74d9 Best regards, -- Julien Stephan <[email protected]>
