* [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT
@ 2020-06-10 1:19 Saravana Kannan
2020-06-10 1:19 ` [PATCH v2 1/2] driver core: Add device_is_dependent() to linux/device.h Saravana Kannan
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Saravana Kannan @ 2020-06-10 1:19 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand
Cc: Saravana Kannan, linux-kernel, devicetree, John Stultz, kernel-team
Patch 2/2 explain the series. Just using a cover letter to thread the
series and add CC's.
-Saravana
v1 -> v2:
Patch 2/2:
- Added more comments
- Fixed missing put_device()
- Fixed stupid fall through in the error case
Saravana Kannan (2):
driver core: Add device_is_dependent() to linux/device.h
of: property: Improve cycle detection when one of the devices is never
added
drivers/base/core.c | 2 +-
drivers/of/property.c | 62 ++++++++++++++++++++++++++++++++++++++----
include/linux/device.h | 1 +
3 files changed, 58 insertions(+), 7 deletions(-)
--
2.27.0.278.ge193c7cf3a9-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] driver core: Add device_is_dependent() to linux/device.h
2020-06-10 1:19 [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT Saravana Kannan
@ 2020-06-10 1:19 ` Saravana Kannan
2020-06-10 1:19 ` [PATCH v2 2/2] of: property: Improve cycle detection when one of the devices is never added Saravana Kannan
2020-06-10 4:47 ` [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT John Stultz
2 siblings, 0 replies; 6+ messages in thread
From: Saravana Kannan @ 2020-06-10 1:19 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand
Cc: Saravana Kannan, linux-kernel, devicetree, John Stultz, kernel-team
DT implementation of fw_devlink needs this function to detect cycles. So
make it available.
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
drivers/base/core.c | 2 +-
include/linux/device.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 139cdf7e7327..509b13610b56 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -118,7 +118,7 @@ int device_links_read_lock_held(void)
* Check if @target depends on @dev or any device dependent on it (its child or
* its consumer etc). Return 1 if that is the case or 0 otherwise.
*/
-static int device_is_dependent(struct device *dev, void *target)
+int device_is_dependent(struct device *dev, void *target)
{
struct device_link *link;
int ret;
diff --git a/include/linux/device.h b/include/linux/device.h
index ac8e37cd716a..5c618effc51e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -829,6 +829,7 @@ extern int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid);
extern const char *device_get_devnode(struct device *dev,
umode_t *mode, kuid_t *uid, kgid_t *gid,
const char **tmp);
+extern int device_is_dependent(struct device *dev, void *target);
static inline bool device_supports_offline(struct device *dev)
{
--
2.27.0.278.ge193c7cf3a9-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] of: property: Improve cycle detection when one of the devices is never added
2020-06-10 1:19 [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT Saravana Kannan
2020-06-10 1:19 ` [PATCH v2 1/2] driver core: Add device_is_dependent() to linux/device.h Saravana Kannan
@ 2020-06-10 1:19 ` Saravana Kannan
2020-06-17 22:12 ` Rob Herring
2020-06-10 4:47 ` [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT John Stultz
2 siblings, 1 reply; 6+ messages in thread
From: Saravana Kannan @ 2020-06-10 1:19 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand
Cc: Saravana Kannan, linux-kernel, devicetree, John Stultz, kernel-team
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 the
following 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@google.com>
---
drivers/of/property.c | 62 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 56 insertions(+), 6 deletions(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 1f2086f4e7ce..ef09e4372ce8 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1014,6 +1014,30 @@ static bool of_is_ancestor_of(struct device_node *test_ancestor,
return false;
}
+/**
+ * of_get_next_parent_dev - Add device link to supplier from supplier phandle
+ * @np: device tree node
+ *
+ * Given a device tree node (@np), this function finds its closest ancestor
+ * device tree node that has a corresponding struct device.
+ *
+ * The caller of this function is expected to call put_device() on the returned
+ * device when they are done.
+ */
+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 +1059,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 +1098,43 @@ 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;
+ /*
+ * DL_FLAG_SYNC_STATE_ONLY doesn't block probing and supports
+ * cycles. So cycle detection isn't necessary and shouldn't be
+ * done.
+ */
+ if (dl_flags & DL_FLAG_SYNC_STATE_ONLY) {
+ of_node_put(sup_np);
+ return -EAGAIN;
+ }
+
+ sup_par_dev = of_get_next_parent_dev(sup_np);
+
+ if (sup_par_dev && device_is_dependent(dev, sup_par_dev)) {
+ /* Cyclic dependency detected, don't try to link */
+ dev_dbg(dev, "Not linking to %pOFP - cycle detected\n",
+ sup_np);
+ ret = -EINVAL;
+ } else {
+ /*
+ * Can't check for cycles or no cycles. So let's try
+ * again later.
+ */
+ ret = -EAGAIN;
+ }
+
+ of_node_put(sup_np);
+ put_device(sup_par_dev);
+ return ret;
}
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT
2020-06-10 1:19 [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT Saravana Kannan
2020-06-10 1:19 ` [PATCH v2 1/2] driver core: Add device_is_dependent() to linux/device.h Saravana Kannan
2020-06-10 1:19 ` [PATCH v2 2/2] of: property: Improve cycle detection when one of the devices is never added Saravana Kannan
@ 2020-06-10 4:47 ` John Stultz
2 siblings, 0 replies; 6+ messages in thread
From: John Stultz @ 2020-06-10 4:47 UTC (permalink / raw)
To: Saravana Kannan
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand,
lkml, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Android Kernel Team
On Tue, Jun 9, 2020 at 6:19 PM Saravana Kannan <saravanak@google.com> wrote:
>
> Patch 2/2 explain the series. Just using a cover letter to thread the
> series and add CC's.
>
> -Saravana
>
> v1 -> v2:
> Patch 2/2:
> - Added more comments
> - Fixed missing put_device()
> - Fixed stupid fall through in the error case
>
> Saravana Kannan (2):
> driver core: Add device_is_dependent() to linux/device.h
> of: property: Improve cycle detection when one of the devices is never
> added
>
> drivers/base/core.c | 2 +-
> drivers/of/property.c | 62 ++++++++++++++++++++++++++++++++++++++----
> include/linux/device.h | 1 +
> 3 files changed, 58 insertions(+), 7 deletions(-)
With both patches, booting with fw_devlink=on (instead of
deferred_probe_timeout=30) this allows all modules to properly load on
the db845c, and without these patches, we fail to get display.
Tested-by: John Stultz <john.stultz@linaro.org>
thanks
-john
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] of: property: Improve cycle detection when one of the devices is never added
2020-06-10 1:19 ` [PATCH v2 2/2] of: property: Improve cycle detection when one of the devices is never added Saravana Kannan
@ 2020-06-17 22:12 ` Rob Herring
2020-06-18 1:13 ` Saravana Kannan
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2020-06-17 22:12 UTC (permalink / raw)
To: Saravana Kannan
Cc: Frank Rowand, Rob Herring, Rafael J. Wysocki, kernel-team,
devicetree, linux-kernel, Greg Kroah-Hartman, John Stultz
On Tue, 09 Jun 2020 18:19:34 -0700, Saravana Kannan wrote:
> 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 the
> following 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@google.com>
> ---
> drivers/of/property.c | 62 ++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 56 insertions(+), 6 deletions(-)
>
Both patches applied.
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] of: property: Improve cycle detection when one of the devices is never added
2020-06-17 22:12 ` Rob Herring
@ 2020-06-18 1:13 ` Saravana Kannan
0 siblings, 0 replies; 6+ messages in thread
From: Saravana Kannan @ 2020-06-18 1:13 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Rowand, Rob Herring, Rafael J. Wysocki,
Android Kernel Team,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
Greg Kroah-Hartman, John Stultz
On Wed, Jun 17, 2020 at 3:12 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, 09 Jun 2020 18:19:34 -0700, Saravana Kannan wrote:
> > 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 the
> > following 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@google.com>
> > ---
> > drivers/of/property.c | 62 ++++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 56 insertions(+), 6 deletions(-)
> >
>
> Both patches applied.
Thanks!
-Saravana
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-18 2:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-10 1:19 [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT Saravana Kannan
2020-06-10 1:19 ` [PATCH v2 1/2] driver core: Add device_is_dependent() to linux/device.h Saravana Kannan
2020-06-10 1:19 ` [PATCH v2 2/2] of: property: Improve cycle detection when one of the devices is never added Saravana Kannan
2020-06-17 22:12 ` Rob Herring
2020-06-18 1:13 ` Saravana Kannan
2020-06-10 4:47 ` [PATCH v2 0/2] fw_devlink: Improve cycle detection in DT John Stultz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).