| Message ID | 20230127001141.407071-7-saravanak@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fw_devlink improvements | expand |
On Thu, Jan 26, 2023 at 04:11:33PM -0800, Saravana Kannan wrote: > To improve detection and handling of dependency cycles, we need to be > able to mark fwnode links as being part of cycles. fwnode links marked > as being part of a cycle should not block their consumers from probing. ... > + list_for_each_entry(link, &fwnode->suppliers, c_hook) { > + if (link->flags & FWLINK_FLAG_CYCLE) > + continue; > + return link->supplier; Hmm... if (!(link->flags & FWLINK_FLAG_CYCLE)) return link->supplier; ? > + } > + > + return NULL; ... > - if (dev->fwnode && !list_empty(&dev->fwnode->suppliers) && > - !fw_devlink_is_permissive()) { > - sup_fw = list_first_entry(&dev->fwnode->suppliers, > - struct fwnode_link, > - c_hook)->supplier; > + sup_fw = fwnode_links_check_suppliers(dev->fwnode); dev_fwnode() ? ... > - val = !list_empty(&dev->fwnode->suppliers); > + mutex_lock(&fwnode_link_lock); > + val = !!fwnode_links_check_suppliers(dev->fwnode); Ditto? > + mutex_unlock(&fwnode_link_lock);
On Fri, Jan 27, 2023 at 1:33 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jan 26, 2023 at 04:11:33PM -0800, Saravana Kannan wrote: > > To improve detection and handling of dependency cycles, we need to be > > able to mark fwnode links as being part of cycles. fwnode links marked > > as being part of a cycle should not block their consumers from probing. > > ... > > > + list_for_each_entry(link, &fwnode->suppliers, c_hook) { > > + if (link->flags & FWLINK_FLAG_CYCLE) > > + continue; > > + return link->supplier; > > Hmm... Thanks! > > if (!(link->flags & FWLINK_FLAG_CYCLE)) > return link->supplier; > > ? > > > + } > > + > > + return NULL; > > ... > > > - if (dev->fwnode && !list_empty(&dev->fwnode->suppliers) && > > - !fw_devlink_is_permissive()) { > > - sup_fw = list_first_entry(&dev->fwnode->suppliers, > > - struct fwnode_link, > > - c_hook)->supplier; > > + sup_fw = fwnode_links_check_suppliers(dev->fwnode); > > dev_fwnode() ? > > ... > > > - val = !list_empty(&dev->fwnode->suppliers); > > + mutex_lock(&fwnode_link_lock); > > + val = !!fwnode_links_check_suppliers(dev->fwnode); > > Ditto? Similar response as Patch 1 and Patch 4. -Saravana
On Fri, Jan 27, 2023 at 11:34:19PM -0800, Saravana Kannan wrote: > On Fri, Jan 27, 2023 at 1:33 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jan 26, 2023 at 04:11:33PM -0800, Saravana Kannan wrote: ... > > > - if (dev->fwnode && !list_empty(&dev->fwnode->suppliers) && > > > - !fw_devlink_is_permissive()) { > > > - sup_fw = list_first_entry(&dev->fwnode->suppliers, > > > - struct fwnode_link, > > > - c_hook)->supplier; > > > + sup_fw = fwnode_links_check_suppliers(dev->fwnode); > > > > dev_fwnode() ? > > > > ... > > > > > - val = !list_empty(&dev->fwnode->suppliers); > > > + mutex_lock(&fwnode_link_lock); > > > + val = !!fwnode_links_check_suppliers(dev->fwnode); > > > > Ditto? > > Similar response as Patch 1 and Patch 4. Same.
diff --git a/drivers/base/core.c b/drivers/base/core.c index e5390b09a02f..82b29e9070bf 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -126,6 +126,19 @@ static void __fwnode_link_del(struct fwnode_link *link) kfree(link); } +/** + * __fwnode_link_cycle - Mark a fwnode link as being part of a cycle. + * @link: the fwnode_link to be marked + * + * The fwnode_link_lock needs to be held when this function is called. + */ +static void __fwnode_link_cycle(struct fwnode_link *link) +{ + pr_debug("%pfwf: Relaxing link with %pfwf\n", + link->consumer, link->supplier); + link->flags |= FWLINK_FLAG_CYCLE; +} + /** * fwnode_links_purge_suppliers - Delete all supplier links of fwnode_handle. * @fwnode: fwnode whose supplier links need to be deleted @@ -1041,6 +1054,23 @@ static bool dev_is_best_effort(struct device *dev) (dev->fwnode && (dev->fwnode->flags & FWNODE_FLAG_BEST_EFFORT)); } +static struct fwnode_handle *fwnode_links_check_suppliers( + struct fwnode_handle *fwnode) +{ + struct fwnode_link *link; + + if (!fwnode || fw_devlink_is_permissive()) + return NULL; + + list_for_each_entry(link, &fwnode->suppliers, c_hook) { + if (link->flags & FWLINK_FLAG_CYCLE) + continue; + return link->supplier; + } + + return NULL; +} + /** * device_links_check_suppliers - Check presence of supplier drivers. * @dev: Consumer device. @@ -1068,11 +1098,8 @@ int device_links_check_suppliers(struct device *dev) * probe. */ mutex_lock(&fwnode_link_lock); - if (dev->fwnode && !list_empty(&dev->fwnode->suppliers) && - !fw_devlink_is_permissive()) { - sup_fw = list_first_entry(&dev->fwnode->suppliers, - struct fwnode_link, - c_hook)->supplier; + sup_fw = fwnode_links_check_suppliers(dev->fwnode); + if (sup_fw) { if (!dev_is_best_effort(dev)) { fwnode_ret = -EPROBE_DEFER; dev_err_probe(dev, -EPROBE_DEFER, @@ -1261,7 +1288,9 @@ static ssize_t waiting_for_supplier_show(struct device *dev, bool val; device_lock(dev); - val = !list_empty(&dev->fwnode->suppliers); + mutex_lock(&fwnode_link_lock); + val = !!fwnode_links_check_suppliers(dev->fwnode); + mutex_unlock(&fwnode_link_lock); device_unlock(dev); return sysfs_emit(buf, "%u\n", val); } diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h index 89b9bdfca925..fdf2ee0285b7 100644 --- a/include/linux/fwnode.h +++ b/include/linux/fwnode.h @@ -18,7 +18,7 @@ struct fwnode_operations; struct device; /* - * fwnode link flags + * fwnode flags * * LINKS_ADDED: The fwnode has already be parsed to add fwnode links. * NOT_DEVICE: The fwnode will never be populated as a struct device. @@ -36,6 +36,7 @@ struct device; #define FWNODE_FLAG_INITIALIZED BIT(2) #define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD BIT(3) #define FWNODE_FLAG_BEST_EFFORT BIT(4) +#define FWNODE_FLAG_VISITED BIT(5) struct fwnode_handle { struct fwnode_handle *secondary; @@ -46,11 +47,19 @@ struct fwnode_handle { u8 flags; }; +/* + * fwnode link flags + * + * CYCLE: The fwnode link is part of a cycle. Don't defer probe. + */ +#define FWLINK_FLAG_CYCLE BIT(0) + struct fwnode_link { struct fwnode_handle *supplier; struct list_head s_hook; struct fwnode_handle *consumer; struct list_head c_hook; + u8 flags; }; /**
To improve detection and handling of dependency cycles, we need to be able to mark fwnode links as being part of cycles. fwnode links marked as being part of a cycle should not block their consumers from probing. Fixes: 2de9d8e0d2fe ("driver core: fw_devlink: Improve handling of cyclic dependencies") Signed-off-by: Saravana Kannan <saravanak@google.com> --- drivers/base/core.c | 41 +++++++++++++++++++++++++++++++++++------ include/linux/fwnode.h | 11 ++++++++++- 2 files changed, 45 insertions(+), 7 deletions(-)