Supplier bindings can map a property node to the device node which consumes the referenced resource. The remote-endpoint binding uses of_graph_get_port_parent(), which can return NULL for a malformed graph node without its expected parents. of_link_property() currently passes that NULL node through to fwnode_link_add(), which dereferences the consumer while adding the link. Only create the link when the binding resolved a consumer node. A malformed graph property then creates no dependency instead of crashing while fw_devlink parses the tree. Fixes: f7514a663016 ("of: property: fw_devlink: Add support for remote-endpoint") Signed-off-by: James Hilliard --- drivers/of/property.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/of/property.c b/drivers/of/property.c index 72cf12907de0..38c0c7dc428a 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1620,7 +1620,9 @@ static int of_link_property(struct device_node *con_np, const char *prop_name) matched = true; i++; - of_link_to_phandle(con_dev_np, phandle, s->fwlink_flags); + if (con_dev_np) + of_link_to_phandle(con_dev_np, phandle, + s->fwlink_flags); of_node_put(phandle); } s++; -- 2.53.0 device_links_driver_bound() drops managed sync-state-only links after a consumer successfully probes, then re-evaluates each affected supplier for a sync_state() callback. Frameworks which populate child devices below a driverless device need to complete the same lifecycle step, but cannot rely on a successful driver probe to trigger it. Factor the existing cleanup into a lock-held helper and reuse it from device_links_driver_bound(). Add a public locking wrapper so frameworks which populate children below driverless devices can retire temporary fw_devlink proxy links without affecting real dependency or runtime-PM links. Signed-off-by: James Hilliard --- drivers/base/core.c | 74 ++++++++++++++++++++++++++++++++++++++------------ include/linux/device.h | 1 + 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 4d026682944f..627fcfa274f0 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1305,6 +1305,57 @@ static void device_link_drop_managed(struct device_link *link) kref_put(&link->kref, __device_link_del); } +/* Caller must hold the device links write lock. */ +static void __device_links_drop_sync_state_only(struct device *dev, + struct list_head *sync_list) +{ + struct device_link *link, *ln; + + list_for_each_entry_safe(link, ln, &dev->links.suppliers, c_node) { + struct device *supplier; + + if (!device_link_test(link, DL_FLAG_MANAGED) || + !device_link_test(link, DL_FLAG_SYNC_STATE_ONLY)) + continue; + + /* + * DL_FLAG_SYNC_STATE_ONLY excludes all other managed link flags, + * so it is safe to drop the managed link completely. + */ + supplier = link->supplier; + device_link_drop_managed(link); + + if (defer_sync_state_count) + __device_links_supplier_defer_sync(supplier); + else + __device_links_queue_sync_state(supplier, sync_list); + } +} + +/** + * device_links_drop_sync_state_only - Drop managed sync-state-only links + * @dev: Consumer device whose proxy links should be dropped + * + * Frameworks which populate child devices below a driverless device can call + * this after adding all of the children. The children will have acquired their + * own links by then, so the temporary links which only prevent suppliers from + * receiving sync_state() callbacks are no longer needed. + * + * Re-evaluate each affected supplier for a sync_state() callback after its + * link is dropped. + */ +void device_links_drop_sync_state_only(struct device *dev) +{ + LIST_HEAD(sync_list); + + device_links_write_lock(); + __device_links_drop_sync_state_only(dev, &sync_list); + device_links_write_unlock(); + + device_links_flush_sync_list(&sync_list, NULL); +} +EXPORT_SYMBOL_GPL(device_links_drop_sync_state_only); + static ssize_t waiting_for_supplier_show(struct device *dev, const struct device_attribute *attr, char *buf) @@ -1418,6 +1469,8 @@ void device_links_driver_bound(struct device *dev) else __device_links_queue_sync_state(dev, &sync_list); + __device_links_drop_sync_state_only(dev, &sync_list); + list_for_each_entry_safe(link, ln, &dev->links.suppliers, c_node) { struct device *supplier; @@ -1425,17 +1478,10 @@ void device_links_driver_bound(struct device *dev) continue; supplier = link->supplier; - if (device_link_test(link, DL_FLAG_SYNC_STATE_ONLY)) { - /* - * When DL_FLAG_SYNC_STATE_ONLY is set, it means no - * other DL_MANAGED_LINK_FLAGS have been set. So, it's - * save to drop the managed link completely. - */ - device_link_drop_managed(link); - } else if (dev_is_best_effort(dev) && - device_link_test(link, DL_FLAG_INFERRED) && - link->status != DL_STATE_CONSUMER_PROBE && - !dev_can_match(link->supplier)) { + if (dev_is_best_effort(dev) && + device_link_test(link, DL_FLAG_INFERRED) && + link->status != DL_STATE_CONSUMER_PROBE && + !dev_can_match(link->supplier)) { /* * When dev_is_best_effort() is true, we ignore device * links to suppliers that don't have a driver. If the @@ -1449,12 +1495,6 @@ void device_links_driver_bound(struct device *dev) WRITE_ONCE(link->status, DL_STATE_ACTIVE); } - /* - * This needs to be done even for the deleted - * DL_FLAG_SYNC_STATE_ONLY device link in case it was the last - * device link that was preventing the supplier from getting a - * sync_state() call. - */ if (defer_sync_state_count) __device_links_supplier_defer_sync(supplier); else diff --git a/include/linux/device.h b/include/linux/device.h index aee79fd6b32b..8afb9f2b0d82 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1381,6 +1381,7 @@ struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags); void device_link_del(struct device_link *link); void device_link_remove(void *consumer, struct device *supplier); +void device_links_drop_sync_state_only(struct device *dev); void device_links_supplier_sync_state_pause(void); void device_links_supplier_sync_state_resume(void); void device_link_wait_removal(void); -- 2.53.0 Ethernet PHY package nodes describe shared resources for their member PHYs, but are not populated as struct devices. fw_devlink consequently represents the package dependencies with proxy links to the closest ancestor device. The MDIO bus is a class device without a driver, so such a proxy can remain available indefinitely and prevent a supplier from receiving its sync_state() callback even after every PHY has probed. Treat every enabled member PHY as a consumer of the common package suppliers. The links can then be converted to links for the real PHY devices and retired through their normal driver lifecycle. Fixes: 385ef48f4686 ("net: phy: add support for scanning PHY in PHY packages nodes") Signed-off-by: James Hilliard --- drivers/of/property.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/of/property.c b/drivers/of/property.c index 38c0c7dc428a..d59bcb1f29de 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1272,6 +1272,20 @@ static void of_link_to_phandle(struct device_node *con_np, tmp_np = of_get_next_parent(tmp_np); } + /* + * An Ethernet PHY package node describes resources shared by its member + * PHYs, but is not populated as a struct device. Link every enabled + * member PHY to those suppliers so fw_devlink can use the real consumer + * devices instead of leaving a proxy link on the MDIO bus indefinitely. + */ + if (of_node_name_eq(con_np, "ethernet-phy-package")) { + for_each_available_child_of_node_scoped(con_np, child) + fwnode_link_add(of_fwnode_handle(child), + of_fwnode_handle(sup_np), flags); + + return; + } + fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np), flags); } -- 2.53.0 fw_devlink creates sync-state-only links from the closest existing ancestor device to suppliers of child devices which have not been added yet. It normally drops those proxy links when the ancestor driver finishes probing. An MDIO bus is a driverless class device, so successful OF population never reaches that lifecycle point. Its proxy links can consequently remain forever and prevent suppliers from receiving sync_state() even after the real PHY devices have acquired and activated their own links. Call the driver-core cleanup helper after successful OF MDIO population. Only sync-state-only links are removed; the real PHY links continue to enforce dependency and runtime-PM ordering. Signed-off-by: James Hilliard --- drivers/net/mdio/of_mdio.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c index b8d298c04d3f..49f9b4062cf5 100644 --- a/drivers/net/mdio/of_mdio.c +++ b/drivers/net/mdio/of_mdio.c @@ -229,7 +229,7 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np, goto unregister; if (!scanphys) - return 0; + goto done; /* auto scan for PHYs with empty reg property */ for_each_available_child_of_node(np, child) { @@ -261,6 +261,9 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np, } } +done: + device_links_drop_sync_state_only(&mdio->dev); + return 0; put_unregister: -- 2.53.0