Re: [PATCH net-next v2 2/4] driver core: factor sync-state-only link cleanup
Saravana Kannan <[email protected]>
| Newsgroups | dev.linux.lists.driver-core,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CACRMN=dWjKL48Ckee1nEVF02VG0ccDrE-GM7mqV+TqgraVveiw@mail.gmail.com> |
On Sun, Aug 16, 2026 at 5:00 PM James Hilliard <[email protected]> wrote: > > 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. See reply to cover letter. I don't think this patch is necessary. -Saravana > > Signed-off-by: James Hilliard <[email protected]> > --- > 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 >