Commit 67cad5c6 authored by Saravana Kannan's avatar Saravana Kannan Committed by Greg Kroah-Hartman

driver core: fw_devlink: Add DL_FLAG_CYCLE support to device links

fw_devlink uses DL_FLAG_SYNC_STATE_ONLY device link flag for two
purposes:

1. To allow a parent device to proxy its child device's dependency on a
   supplier so that the supplier doesn't get its sync_state() callback
   before the child device/consumer can be added and probed. In this
   usage scenario, we need to ignore cycles for ensure correctness of
   sync_state() callbacks.

2. When there are dependency cycles in firmware, we don't know which of
   those dependencies are valid. So, we have to ignore them all wrt
   probe ordering while still making sure the sync_state() callbacks
   come correctly.

However, when detecting dependency cycles, there can be multiple
dependency cycles between two devices that we need to detect. For
example:

A -> B -> A and A -> C -> B -> A.

To detect multiple cycles correct, we need to be able to differentiate
DL_FLAG_SYNC_STATE_ONLY device links used for (1) vs (2) above.

To allow this differentiation, add a DL_FLAG_CYCLE that can be use to
mark use case (2). We can then use the DL_FLAG_CYCLE to decide which
DL_FLAG_SYNC_STATE_ONLY device links to follow when looking for
dependency cycles.

Fixes: 2de9d8e0 ("driver core: fw_devlink: Improve handling of cyclic dependencies")
Signed-off-by: default avatarSaravana Kannan <saravanak@google.com>
Tested-by: default avatarColin Foster <colin.foster@in-advantage.com>
Tested-by: default avatarSudeep Holla <sudeep.holla@arm.com>
Tested-by: default avatarDouglas Anderson <dianders@chromium.org>
Tested-by: default avatarGeert 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-6-saravanak@google.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 38dfa56b
...@@ -322,6 +322,12 @@ static bool device_is_ancestor(struct device *dev, struct device *target) ...@@ -322,6 +322,12 @@ static bool device_is_ancestor(struct device *dev, struct device *target)
return false; return false;
} }
static inline bool device_link_flag_is_sync_state_only(u32 flags)
{
return (flags & ~(DL_FLAG_INFERRED | DL_FLAG_CYCLE)) ==
(DL_FLAG_SYNC_STATE_ONLY | DL_FLAG_MANAGED);
}
/** /**
* device_is_dependent - Check if one device depends on another one * device_is_dependent - Check if one device depends on another one
* @dev: Device to check dependencies for. * @dev: Device to check dependencies for.
...@@ -348,8 +354,7 @@ int device_is_dependent(struct device *dev, void *target) ...@@ -348,8 +354,7 @@ int device_is_dependent(struct device *dev, void *target)
return ret; return ret;
list_for_each_entry(link, &dev->links.consumers, s_node) { list_for_each_entry(link, &dev->links.consumers, s_node) {
if ((link->flags & ~DL_FLAG_INFERRED) == if (device_link_flag_is_sync_state_only(link->flags))
(DL_FLAG_SYNC_STATE_ONLY | DL_FLAG_MANAGED))
continue; continue;
if (link->consumer == target) if (link->consumer == target)
...@@ -422,8 +427,7 @@ static int device_reorder_to_tail(struct device *dev, void *not_used) ...@@ -422,8 +427,7 @@ static int device_reorder_to_tail(struct device *dev, void *not_used)
device_for_each_child(dev, NULL, device_reorder_to_tail); device_for_each_child(dev, NULL, device_reorder_to_tail);
list_for_each_entry(link, &dev->links.consumers, s_node) { list_for_each_entry(link, &dev->links.consumers, s_node) {
if ((link->flags & ~DL_FLAG_INFERRED) == if (device_link_flag_is_sync_state_only(link->flags))
(DL_FLAG_SYNC_STATE_ONLY | DL_FLAG_MANAGED))
continue; continue;
device_reorder_to_tail(link->consumer, NULL); device_reorder_to_tail(link->consumer, NULL);
} }
...@@ -684,7 +688,8 @@ postcore_initcall(devlink_class_init); ...@@ -684,7 +688,8 @@ postcore_initcall(devlink_class_init);
DL_FLAG_AUTOREMOVE_SUPPLIER | \ DL_FLAG_AUTOREMOVE_SUPPLIER | \
DL_FLAG_AUTOPROBE_CONSUMER | \ DL_FLAG_AUTOPROBE_CONSUMER | \
DL_FLAG_SYNC_STATE_ONLY | \ DL_FLAG_SYNC_STATE_ONLY | \
DL_FLAG_INFERRED) DL_FLAG_INFERRED | \
DL_FLAG_CYCLE)
#define DL_ADD_VALID_FLAGS (DL_MANAGED_LINK_FLAGS | DL_FLAG_STATELESS | \ #define DL_ADD_VALID_FLAGS (DL_MANAGED_LINK_FLAGS | DL_FLAG_STATELESS | \
DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE) DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE)
...@@ -753,8 +758,6 @@ struct device_link *device_link_add(struct device *consumer, ...@@ -753,8 +758,6 @@ struct device_link *device_link_add(struct device *consumer,
if (!consumer || !supplier || consumer == supplier || if (!consumer || !supplier || consumer == supplier ||
flags & ~DL_ADD_VALID_FLAGS || flags & ~DL_ADD_VALID_FLAGS ||
(flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) || (flags & DL_FLAG_STATELESS && flags & DL_MANAGED_LINK_FLAGS) ||
(flags & DL_FLAG_SYNC_STATE_ONLY &&
(flags & ~DL_FLAG_INFERRED) != DL_FLAG_SYNC_STATE_ONLY) ||
(flags & DL_FLAG_AUTOPROBE_CONSUMER && (flags & DL_FLAG_AUTOPROBE_CONSUMER &&
flags & (DL_FLAG_AUTOREMOVE_CONSUMER | flags & (DL_FLAG_AUTOREMOVE_CONSUMER |
DL_FLAG_AUTOREMOVE_SUPPLIER))) DL_FLAG_AUTOREMOVE_SUPPLIER)))
...@@ -770,6 +773,10 @@ struct device_link *device_link_add(struct device *consumer, ...@@ -770,6 +773,10 @@ struct device_link *device_link_add(struct device *consumer,
if (!(flags & DL_FLAG_STATELESS)) if (!(flags & DL_FLAG_STATELESS))
flags |= DL_FLAG_MANAGED; flags |= DL_FLAG_MANAGED;
if (flags & DL_FLAG_SYNC_STATE_ONLY &&
!device_link_flag_is_sync_state_only(flags))
return NULL;
device_links_write_lock(); device_links_write_lock();
device_pm_lock(); device_pm_lock();
...@@ -1729,7 +1736,7 @@ static void fw_devlink_relax_link(struct device_link *link) ...@@ -1729,7 +1736,7 @@ static void fw_devlink_relax_link(struct device_link *link)
if (!(link->flags & DL_FLAG_INFERRED)) if (!(link->flags & DL_FLAG_INFERRED))
return; return;
if (link->flags == (DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE)) if (device_link_flag_is_sync_state_only(link->flags))
return; return;
pm_runtime_drop_link(link); pm_runtime_drop_link(link);
...@@ -1853,8 +1860,8 @@ static int fw_devlink_relax_cycle(struct device *con, void *sup) ...@@ -1853,8 +1860,8 @@ static int fw_devlink_relax_cycle(struct device *con, void *sup)
return ret; return ret;
list_for_each_entry(link, &con->links.consumers, s_node) { list_for_each_entry(link, &con->links.consumers, s_node) {
if ((link->flags & ~DL_FLAG_INFERRED) == if (!(link->flags & DL_FLAG_CYCLE) &&
(DL_FLAG_SYNC_STATE_ONLY | DL_FLAG_MANAGED)) device_link_flag_is_sync_state_only(link->flags))
continue; continue;
if (!fw_devlink_relax_cycle(link->consumer, sup)) if (!fw_devlink_relax_cycle(link->consumer, sup))
...@@ -1863,6 +1870,7 @@ static int fw_devlink_relax_cycle(struct device *con, void *sup) ...@@ -1863,6 +1870,7 @@ static int fw_devlink_relax_cycle(struct device *con, void *sup)
ret = 1; ret = 1;
fw_devlink_relax_link(link); fw_devlink_relax_link(link);
link->flags |= DL_FLAG_CYCLE;
} }
return ret; return ret;
} }
......
...@@ -328,6 +328,7 @@ enum device_link_state { ...@@ -328,6 +328,7 @@ enum device_link_state {
#define DL_FLAG_MANAGED BIT(6) #define DL_FLAG_MANAGED BIT(6)
#define DL_FLAG_SYNC_STATE_ONLY BIT(7) #define DL_FLAG_SYNC_STATE_ONLY BIT(7)
#define DL_FLAG_INFERRED BIT(8) #define DL_FLAG_INFERRED BIT(8)
#define DL_FLAG_CYCLE BIT(9)
/** /**
* enum dl_dev_state - Device driver presence tracking information. * enum dl_dev_state - Device driver presence tracking information.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment