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>
This commit is contained in:
Saravana Kannan 2023-02-06 17:41:53 -08:00 committed by Greg Kroah-Hartman
parent 2bb3669f57
commit 3a2dbc510c
1 changed files with 79 additions and 18 deletions

View File

@ -54,11 +54,12 @@ static LIST_HEAD(deferred_sync);
static unsigned int defer_sync_state_count = 1;
static DEFINE_MUTEX(fwnode_link_lock);
static bool fw_devlink_is_permissive(void);
static void __fw_devlink_link_to_consumers(struct device *dev);
static bool fw_devlink_drv_reg_done;
static bool fw_devlink_best_effort;
/**
* fwnode_link_add - Create a link between two fwnode_handles.
* __fwnode_link_add - Create a link between two fwnode_handles.
* @con: Consumer end of the link.
* @sup: Supplier end of the link.
*
@ -74,22 +75,18 @@ static bool fw_devlink_best_effort;
* Attempts to create duplicate links between the same pair of fwnode handles
* are ignored and there is no reference counting.
*/
int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup)
static int __fwnode_link_add(struct fwnode_handle *con,
struct fwnode_handle *sup)
{
struct fwnode_link *link;
int ret = 0;
mutex_lock(&fwnode_link_lock);
list_for_each_entry(link, &sup->consumers, s_hook)
if (link->consumer == con)
goto out;
return 0;
link = kzalloc(sizeof(*link), GFP_KERNEL);
if (!link) {
ret = -ENOMEM;
goto out;
}
if (!link)
return -ENOMEM;
link->supplier = sup;
INIT_LIST_HEAD(&link->s_hook);
@ -100,9 +97,17 @@ int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup)
list_add(&link->c_hook, &con->suppliers);
pr_debug("%pfwP Linked as a fwnode consumer to %pfwP\n",
con, sup);
out:
mutex_unlock(&fwnode_link_lock);
return 0;
}
int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup)
{
int ret;
mutex_lock(&fwnode_link_lock);
ret = __fwnode_link_add(con, sup);
mutex_unlock(&fwnode_link_lock);
return ret;
}
@ -181,6 +186,51 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
}
EXPORT_SYMBOL_GPL(fw_devlink_purge_absent_suppliers);
/**
* __fwnode_links_move_consumers - Move consumer from @from to @to fwnode_handle
* @from: move consumers away from this fwnode
* @to: move consumers to this fwnode
*
* Move all consumer links from @from fwnode to @to fwnode.
*/
static void __fwnode_links_move_consumers(struct fwnode_handle *from,
struct fwnode_handle *to)
{
struct fwnode_link *link, *tmp;
list_for_each_entry_safe(link, tmp, &from->consumers, s_hook) {
__fwnode_link_add(link->consumer, to);
__fwnode_link_del(link);
}
}
/**
* __fw_devlink_pickup_dangling_consumers - Pick up dangling consumers
* @fwnode: fwnode from which to pick up dangling consumers
* @new_sup: fwnode of new supplier
*
* If the @fwnode has a corresponding struct device and the device supports
* probing (that is, added to a bus), then we want to let fw_devlink create
* MANAGED device links to this device, so leave @fwnode and its descendant's
* fwnode links alone.
*
* Otherwise, move its consumers to the new supplier @new_sup.
*/
static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
struct fwnode_handle *new_sup)
{
struct fwnode_handle *child;
if (fwnode->dev && fwnode->dev->bus)
return;
fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
__fwnode_links_move_consumers(fwnode, new_sup);
fwnode_for_each_available_child_node(fwnode, child)
__fw_devlink_pickup_dangling_consumers(child, new_sup);
}
#ifdef CONFIG_SRCU
static DEFINE_MUTEX(device_links_lock);
DEFINE_STATIC_SRCU(device_links_srcu);
@ -1267,16 +1317,23 @@ void device_links_driver_bound(struct device *dev)
* them. So, fw_devlink no longer needs to create device links to any
* of the device's suppliers.
*
* Also, if a child firmware node of this bound device is not added as
* a device by now, assume it is never going to be added and make sure
* other devices don't defer probe indefinitely by waiting for such a
* child device.
* Also, if a child firmware node of this bound device is not added as a
* device by now, assume it is never going to be added. Make this bound
* device the fallback supplier to the dangling consumers of the child
* firmware node because this bound device is probably implementing the
* child firmware node functionality and we don't want the dangling
* consumers to defer probe indefinitely waiting for a device for the
* child firmware node.
*/
if (dev->fwnode && dev->fwnode->dev == dev) {
struct fwnode_handle *child;
fwnode_links_purge_suppliers(dev->fwnode);
mutex_lock(&fwnode_link_lock);
fwnode_for_each_available_child_node(dev->fwnode, child)
fw_devlink_purge_absent_suppliers(child);
__fw_devlink_pickup_dangling_consumers(child,
dev->fwnode);
__fw_devlink_link_to_consumers(dev);
mutex_unlock(&fwnode_link_lock);
}
device_remove_file(dev, &dev_attr_waiting_for_supplier);
@ -1855,7 +1912,11 @@ static int fw_devlink_create_devlink(struct device *con,
fwnode_is_ancestor_of(sup_handle, con->fwnode))
return -EINVAL;
sup_dev = get_dev_from_fwnode(sup_handle);
if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE)
sup_dev = fwnode_get_next_parent_dev(sup_handle);
else
sup_dev = get_dev_from_fwnode(sup_handle);
if (sup_dev) {
/*
* If it's one of those drivers that don't actually bind to