Consider this example where -> means LHS device is a consumer of RHS
device and indentation represents "child of" of the previous device.
Device A -> Device C
Device B -> Device A
Device C
Without this commit:
1. Device A is added.
2. Device A is added to waiting for supplier list (Device C)
3. Device B is added
4. Device B is linked as a consumer to Device A
5. Device A doesn't probe because it's waiting for Device C to be added.
6. Device B doesn't probe because Device A hasn't probed.
7. Device C will never be added because it's parent hasn't probed.
So, Device A, B and C will be in a probe/add deadlock.
This commit detects this scenario and stops trying to create a device
link between Device A and Device C since doing so would create a cycle:
Device A -> Devic C -(parent)-> Device B -> Device A.
With this commit:
1. Device A is added.
3. Device B is added
4. Device B is linked as a consumer to Device A
5. Device A probes.
6. Device B probes because Device A has probed.
7. Device C is added and probed.
Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>
---
drivers/of/property.c | 44 +++++++++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 1f2086f4e7ce..7eebe21274a4 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1014,6 +1014,20 @@ static bool of_is_ancestor_of(struct device_node *test_ancestor,
return false;
}
+static struct device *of_get_next_parent_dev(struct device_node *np)
+{
+ struct device *dev = NULL;
+
+ of_node_get(np);
+ do {
+ np = of_get_next_parent(np);
+ if (np)
+ dev = get_dev_from_fwnode(&np->fwnode);
+ } while (np && !dev);
+ of_node_put(np);
+ return dev;
+}
+
/**
* of_link_to_phandle - Add device link to supplier from supplier phandle
* @dev: consumer device
@@ -1035,10 +1049,9 @@ static bool of_is_ancestor_of(struct device_node *test_ancestor,
static int of_link_to_phandle(struct device *dev, struct device_node *sup_np,
u32 dl_flags)
{
- struct device *sup_dev;
+ struct device *sup_dev, *sup_par_dev;
int ret = 0;
struct device_node *tmp_np = sup_np;
- int is_populated;
of_node_get(sup_np);
/*
@@ -1075,16 +1088,35 @@ static int of_link_to_phandle(struct device *dev, struct device_node *sup_np,
return -EINVAL;
}
sup_dev = get_dev_from_fwnode(&sup_np->fwnode);
- is_populated = of_node_check_flag(sup_np, OF_POPULATED);
- of_node_put(sup_np);
- if (!sup_dev && is_populated) {
+ if (!sup_dev && of_node_check_flag(sup_np, OF_POPULATED)) {
/* Early device without struct device. */
dev_dbg(dev, "Not linking to %pOFP - No struct device\n",
sup_np);
+ of_node_put(sup_np);
return -ENODEV;
} else if (!sup_dev) {
- return -EAGAIN;
+ sup_par_dev = of_get_next_parent_dev(sup_np);
+ of_node_put(sup_np);
+
+ /*
+ * DL_FLAG_SYNC_STATE_ONLY doesn't block probing, so cycle
+ * detection isn't necessary and shouldn't be done.
+ */
+ if (dl_flags & DL_FLAG_SYNC_STATE_ONLY)
+ return -EAGAIN;
+
+ /*
+ * If devices haven't been created for any of the ancestors, we
+ * can't check for cycles. So let's try again later.
+ */
+ if (!sup_par_dev)
+ return -EAGAIN;
+
+ /* Cyclic dependency detected, don't try to link */
+ if (device_is_dependent(dev, sup_par_dev))
+ return -EINVAL;
}
+ of_node_put(sup_np);
if (!device_link_add(dev, sup_dev, dl_flags))
ret = -EINVAL;
put_device(sup_dev);
--
2.27.0.278.ge193c7cf3a9-goog
