Skip to content

Commit 3a2dbc5

Browse files
Saravana Kannangregkh
authored andcommitted
driver core: fw_devlink: Don't purge child fwnode's consumer links
When a device X is bound successfully to a driver, if it has a child firmware node Y that doesn't have a struct device created by then, we delete fwnode links where the child firmware node Y is the supplier. We did this to avoid blocking the consumers of the child firmware node Y from deferring probe indefinitely. While that a step in the right direction, it's better to make the consumers of the child firmware node Y to be consumers of the device X because device X is probably implementing whatever functionality is represented by child firmware node Y. By doing this, we capture the device dependencies more accurately and ensure better probe/suspend/resume ordering. Signed-off-by: Saravana Kannan <saravanak@google.com> Tested-by: Colin Foster <colin.foster@in-advantage.com> Tested-by: Sudeep Holla <sudeep.holla@arm.com> Tested-by: Douglas Anderson <dianders@chromium.org> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Tested-by: Luca Weiss <luca.weiss@fairphone.com> # qcom/sm7225-fairphone-fp4 Link: https://lore.kernel.org/r/20230207014207.1678715-2-saravanak@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 2bb3669 commit 3a2dbc5

File tree

1 file changed

+79
-18
lines changed

1 file changed

+79
-18
lines changed

drivers/base/core.c

Lines changed: 79 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,12 @@ static LIST_HEAD(deferred_sync);
5454
static unsigned int defer_sync_state_count = 1;
5555
static DEFINE_MUTEX(fwnode_link_lock);
5656
static bool fw_devlink_is_permissive(void);
57+
static void __fw_devlink_link_to_consumers(struct device *dev);
5758
static bool fw_devlink_drv_reg_done;
5859
static bool fw_devlink_best_effort;
5960

6061
/**
61-
* fwnode_link_add - Create a link between two fwnode_handles.
62+
* __fwnode_link_add - Create a link between two fwnode_handles.
6263
* @con: Consumer end of the link.
6364
* @sup: Supplier end of the link.
6465
*
@@ -74,22 +75,18 @@ static bool fw_devlink_best_effort;
7475
* Attempts to create duplicate links between the same pair of fwnode handles
7576
* are ignored and there is no reference counting.
7677
*/
77-
int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup)
78+
static int __fwnode_link_add(struct fwnode_handle *con,
79+
struct fwnode_handle *sup)
7880
{
7981
struct fwnode_link *link;
80-
int ret = 0;
81-
82-
mutex_lock(&fwnode_link_lock);
8382

8483
list_for_each_entry(link, &sup->consumers, s_hook)
8584
if (link->consumer == con)
86-
goto out;
85+
return 0;
8786

8887
link = kzalloc(sizeof(*link), GFP_KERNEL);
89-
if (!link) {
90-
ret = -ENOMEM;
91-
goto out;
92-
}
88+
if (!link)
89+
return -ENOMEM;
9390

9491
link->supplier = sup;
9592
INIT_LIST_HEAD(&link->s_hook);
@@ -100,9 +97,17 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup)
10097
list_add(&link->c_hook, &con->suppliers);
10198
pr_debug("%pfwP Linked as a fwnode consumer to %pfwP\n",
10299
con, sup);
103-
out:
104-
mutex_unlock(&fwnode_link_lock);
105100

101+
return 0;
102+
}
103+
104+
int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup)
105+
{
106+
int ret;
107+
108+
mutex_lock(&fwnode_link_lock);
109+
ret = __fwnode_link_add(con, sup);
110+
mutex_unlock(&fwnode_link_lock);
106111
return ret;
107112
}
108113

@@ -181,6 +186,51 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
181186
}
182187
EXPORT_SYMBOL_GPL(fw_devlink_purge_absent_suppliers);
183188

189+
/**
190+
* __fwnode_links_move_consumers - Move consumer from @from to @to fwnode_handle
191+
* @from: move consumers away from this fwnode
192+
* @to: move consumers to this fwnode
193+
*
194+
* Move all consumer links from @from fwnode to @to fwnode.
195+
*/
196+
static void __fwnode_links_move_consumers(struct fwnode_handle *from,
197+
struct fwnode_handle *to)
198+
{
199+
struct fwnode_link *link, *tmp;
200+
201+
list_for_each_entry_safe(link, tmp, &from->consumers, s_hook) {
202+
__fwnode_link_add(link->consumer, to);
203+
__fwnode_link_del(link);
204+
}
205+
}
206+
207+
/**
208+
* __fw_devlink_pickup_dangling_consumers - Pick up dangling consumers
209+
* @fwnode: fwnode from which to pick up dangling consumers
210+
* @new_sup: fwnode of new supplier
211+
*
212+
* If the @fwnode has a corresponding struct device and the device supports
213+
* probing (that is, added to a bus), then we want to let fw_devlink create
214+
* MANAGED device links to this device, so leave @fwnode and its descendant's
215+
* fwnode links alone.
216+
*
217+
* Otherwise, move its consumers to the new supplier @new_sup.
218+
*/
219+
static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
220+
struct fwnode_handle *new_sup)
221+
{
222+
struct fwnode_handle *child;
223+
224+
if (fwnode->dev && fwnode->dev->bus)
225+
return;
226+
227+
fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
228+
__fwnode_links_move_consumers(fwnode, new_sup);
229+
230+
fwnode_for_each_available_child_node(fwnode, child)
231+
__fw_devlink_pickup_dangling_consumers(child, new_sup);
232+
}
233+
184234
#ifdef CONFIG_SRCU
185235
static DEFINE_MUTEX(device_links_lock);
186236
DEFINE_STATIC_SRCU(device_links_srcu);
@@ -1267,16 +1317,23 @@ void device_links_driver_bound(struct device *dev)
12671317
* them. So, fw_devlink no longer needs to create device links to any
12681318
* of the device's suppliers.
12691319
*
1270-
* Also, if a child firmware node of this bound device is not added as
1271-
* a device by now, assume it is never going to be added and make sure
1272-
* other devices don't defer probe indefinitely by waiting for such a
1273-
* child device.
1320+
* Also, if a child firmware node of this bound device is not added as a
1321+
* device by now, assume it is never going to be added. Make this bound
1322+
* device the fallback supplier to the dangling consumers of the child
1323+
* firmware node because this bound device is probably implementing the
1324+
* child firmware node functionality and we don't want the dangling
1325+
* consumers to defer probe indefinitely waiting for a device for the
1326+
* child firmware node.
12741327
*/
12751328
if (dev->fwnode && dev->fwnode->dev == dev) {
12761329
struct fwnode_handle *child;
12771330
fwnode_links_purge_suppliers(dev->fwnode);
1331+
mutex_lock(&fwnode_link_lock);
12781332
fwnode_for_each_available_child_node(dev->fwnode, child)
1279-
fw_devlink_purge_absent_suppliers(child);
1333+
__fw_devlink_pickup_dangling_consumers(child,
1334+
dev->fwnode);
1335+
__fw_devlink_link_to_consumers(dev);
1336+
mutex_unlock(&fwnode_link_lock);
12801337
}
12811338
device_remove_file(dev, &dev_attr_waiting_for_supplier);
12821339

@@ -1855,7 +1912,11 @@ static int fw_devlink_create_devlink(struct device *con,
18551912
fwnode_is_ancestor_of(sup_handle, con->fwnode))
18561913
return -EINVAL;
18571914

1858-
sup_dev = get_dev_from_fwnode(sup_handle);
1915+
if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE)
1916+
sup_dev = fwnode_get_next_parent_dev(sup_handle);
1917+
else
1918+
sup_dev = get_dev_from_fwnode(sup_handle);
1919+
18591920
if (sup_dev) {
18601921
/*
18611922
* If it's one of those drivers that don't actually bind to

0 commit comments

Comments
 (0)