On 22-07-05 16:45:02, Abel Vesa wrote: > In case of a cyclic dependency, if the supplier is not yet available, > the parent of the supplier is checked for dependency. But if there are > more than one suppliers with the same parent, the first check returns > true while the next checks skip that specific link entirely. So add a > flag that marks the link for future checks and bail early if it is > already marked with that flag. > > Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx> > --- > > For more details about this issue, have a look at this thread: > https://lore.kernel.org/all/CAGETcx8F0wP+RA0KpjOJeZfc=DVG-MbM_=SkRHD4UhD2ReL7Kw@xxxxxxxxxxxxxx/ > > drivers/base/core.c | 14 +++++++++++--- > include/linux/device.h | 2 ++ > 2 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index ccdd5b4295de..38cb478ae400 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -297,12 +297,18 @@ int device_is_dependent(struct device *dev, void *target) > return ret; > > list_for_each_entry(link, &dev->links.consumers, s_node) { > + /* if already marked before as dependent, bail early */ > + if (link->flags & DL_FLAG_DEVICE_IS_DEPENDENT) > + return 1; On a second thought, this is wrong since it might be a different consumer link. > + > if ((link->flags & ~DL_FLAG_INFERRED) == > (DL_FLAG_SYNC_STATE_ONLY | DL_FLAG_MANAGED)) > continue; > > - if (link->consumer == target) > + if (link->consumer == target) { > + link->flags |= DL_FLAG_DEVICE_IS_DEPENDENT; > return 1; > + } > > ret = device_is_dependent(link->consumer, target); > if (ret) > @@ -1660,11 +1666,13 @@ static void fw_devlink_relax_link(struct device_link *link) > if (!(link->flags & DL_FLAG_INFERRED)) > return; > > - if (link->flags == (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE)) > + if ((link->flags & (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE)) == > + (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE)) > return; > > pm_runtime_drop_link(link); > - link->flags = DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE; > + link->flags &= DL_FLAG_DEVICE_IS_DEPENDENT; > + link->flags |= DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE; > dev_dbg(link->consumer, "Relaxing link with %s\n", > dev_name(link->supplier)); > } > diff --git a/include/linux/device.h b/include/linux/device.h > index 424b55df0272..3b0c4b777a60 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -317,6 +317,7 @@ enum device_link_state { > * MANAGED: The core tracks presence of supplier/consumer drivers (internal). > * SYNC_STATE_ONLY: Link only affects sync_state() behavior. > * INFERRED: Inferred from data (eg: firmware) and not from driver actions. > + * DEVICE_IS_DEPENDENT: The consumer is dependent on the supplier > */ > #define DL_FLAG_STATELESS BIT(0) > #define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1) > @@ -327,6 +328,7 @@ enum device_link_state { > #define DL_FLAG_MANAGED BIT(6) > #define DL_FLAG_SYNC_STATE_ONLY BIT(7) > #define DL_FLAG_INFERRED BIT(8) > +#define DL_FLAG_DEVICE_IS_DEPENDENT BIT(9) > > /** > * enum dl_dev_state - Device driver presence tracking information. > -- > 2.34.3 >
