Add post-init-providers binding to improve suspend/resume stability

PATCH New Saravana Kannan 1 participant 4 messages Mar 5, 2024

Subsystems (5)

ACPI Driver core EFI Device tree +1 more subsystems

lore

Share & tools

lore

Saravana Kannan

author

2024-03-05 05:05 UTC #0 lore raw

v5 cover Add post-init-providers binding to improve suspend/resume stability

Greg,

Can you pull this in as part of your tree please?

Thanks, Saravana

This patch series adds a "post-init-providers" device tree binding that can be used to break dependency cycles in device tree and enforce a more determinstic probe/suspend/resume order. This will also improve the stability of global async probing and async suspend/resume and allow us to enable them more easily. Yet another step away from playing initcall chicken with probing and step towards fully async probing and suspend/resume.

dtschema update for post-init-providers will have a more substantial documentation of the examples and the need for this property.

v4->v5:
- As suggesed by reviewers, removed line wraps to go > 80 chars.
- Added Reviewed-by and Acked-by
- Dropped the DT schema documentation patch (will submit tha to dtschema)

v3->v4:
- Fixed MAINTAINERS file to go with the file rename.

v2->v3:
- Changes doc/code from "post-init-supplier" to "post-init-providers"
- Fixed some wording that was ambiguous for Conor.
- Fixed indentation, additionalProperies and white space issues in the yaml syntax.
- Fixed syntax errors in the example.

v1->v2:
- Addressed Documentation/commit text errors pointed out by Rob
- Reordered MAINTAINERS chunk as pointed out by Krzysztof

Saravana Kannan (3):
driver core: Adds flags param to fwnode_link_add()
driver core: Add FWLINK_FLAG_IGNORE to completely ignore a fwnode link
of: property: fw_devlink: Add support for "post-init-providers"
property

 drivers/base/core.c              | 14 +++++++++++---
 drivers/firmware/efi/sysfb_efi.c |  2 +-
 drivers/of/property.c            | 15 ++++++++++++---
 include/linux/fwnode.h           |  5 ++++-
 4 files changed, 28 insertions(+), 8 deletions(-)

Signature · 1 line

2.44.0.278.ge034bb2e1d-goog

Saravana Kannan

author

2024-03-05 05:05 UTC #1 lore raw

PATCH v5 1/3 driver core: Adds flags param to fwnode_link_add()

Allow the callers to set fwnode link flags when adding fwnode links.

Signed-off-by Saravana Kannan <[email protected]>

Acked-by Rafael J. Wysocki <[email protected]>

Reviewed-by Rob Herring <[email protected]>

---
 drivers/base/core.c              | 5 +++--
 drivers/firmware/efi/sysfb_efi.c | 2 +-
 drivers/of/property.c            | 2 +-
 include/linux/fwnode.h           | 3 ++-
 4 files changed, 7 insertions(+), 5 deletions(-)

drivers/base/core.c +3 -2

index 9828da9b933c..adeff041d472 100644
@@ -92,12 +92,13 @@ static int __fwnode_link_add(struct fwnode_handle *con,
 	return 0;
 }
 
-int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup)
+int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
+		    u8 flags)
 {
 	int ret;
 
 	mutex_lock(&fwnode_link_lock);
-	ret = __fwnode_link_add(con, sup, 0);
+	ret = __fwnode_link_add(con, sup, flags);
 	mutex_unlock(&fwnode_link_lock);
 	return ret;
 }

drivers/firmware/efi/sysfb_efi.c +1 -1

index 456d0e5eaf78..cc807ed35aed 100644
@@ -336,7 +336,7 @@ static int efifb_add_links(struct fwnode_handle *fwnode)
 	if (!sup_np)
 		return 0;
 
-	fwnode_link_add(fwnode, of_fwnode_handle(sup_np));
+	fwnode_link_add(fwnode, of_fwnode_handle(sup_np), 0);
 	of_node_put(sup_np);
 
 	return 0;

drivers/of/property.c +1 -1

index b71267c6667c..bce849f21ae2 100644
@@ -1085,7 +1085,7 @@ static void of_link_to_phandle(struct device_node *con_np,
 		tmp_np = of_get_next_parent(tmp_np);
 	}
 
-	fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np));
+	fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np), 0);
 }
 
 /**

include/linux/fwnode.h +2 -1

index 2a72f55d26eb..c964749953e3 100644
@@ -210,7 +210,8 @@ static inline void fwnode_dev_initialized(struct fwnode_handle *fwnode,
 }
 
 extern bool fw_devlink_is_strict(void);
-int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup);
+int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,
+		    u8 flags);
 void fwnode_links_purge(struct fwnode_handle *fwnode);
 void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);

Signature · 1 line

2.44.0.278.ge034bb2e1d-goog

Saravana Kannan

author

2024-03-05 05:05 UTC #2 lore raw

PATCH v5 2/3 driver core: Add FWLINK_FLAG_IGNORE to completely ignore a fwnode link

In reply to Saravana Kannan

A fwnode link between specific supplier-consumer fwnodes can be added multiple times for multiple reasons. If that dependency doesn't exist, deleting the fwnode link once doesn't guarantee that it won't get created again.

So, add FWLINK_FLAG_IGNORE flag to mark a fwnode link as one that needs to be completely ignored. Since a fwnode link's flags is an OR of all the flags passed to all the fwnode_link_add() calls to create that specific fwnode link, the FWLINK_FLAG_IGNORE flag is preserved and can be used to mark a fwnode link as on that need to be completely ignored until it is deleted.

Signed-off-by Saravana Kannan <[email protected]>

Acked-by Rafael J. Wysocki <[email protected]>

Reviewed-by Rob Herring <[email protected]>

---
 drivers/base/core.c    | 9 ++++++++-
 include/linux/fwnode.h | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

drivers/base/core.c +8 -1

index adeff041d472..fac017657d25 100644
@@ -1012,7 +1012,8 @@ static struct fwnode_handle *fwnode_links_check_suppliers(
 		return NULL;
 
 	list_for_each_entry(link, &fwnode->suppliers, c_hook)
-		if (!(link->flags & FWLINK_FLAG_CYCLE))
+		if (!(link->flags &
+		      (FWLINK_FLAG_CYCLE | FWLINK_FLAG_IGNORE)))
 			return link->supplier;
 
 	return NULL;

@@ -1963,6 +1964,9 @@ static bool __fw_devlink_relax_cycles(struct device *con,
 	}
 
 	list_for_each_entry(link, &sup_handle->suppliers, c_hook) {
+		if (link->flags & FWLINK_FLAG_IGNORE)
+			continue;
+
 		if (__fw_devlink_relax_cycles(con, link->supplier)) {
 			__fwnode_link_cycle(link);
 			ret = true;

@@ -2041,6 +2045,9 @@ static int fw_devlink_create_devlink(struct device *con,
 	int ret = 0;
 	u32 flags;
 
+	if (link->flags & FWLINK_FLAG_IGNORE)
+		return 0;
+
 	if (con->fwnode == link->consumer)
 		flags = fw_devlink_get_flags(link->flags);
 	else

include/linux/fwnode.h +2 -0

index c964749953e3..21699eee9641 100644
@@ -53,8 +53,10 @@ struct fwnode_handle {
  * fwnode link flags
  *
  * CYCLE:	The fwnode link is part of a cycle. Don't defer probe.
+ * IGNORE:	Completely ignore this link, even during cycle detection.
  */
 #define FWLINK_FLAG_CYCLE			BIT(0)
+#define FWLINK_FLAG_IGNORE			BIT(1)
 
 struct fwnode_link {
 	struct fwnode_handle *supplier;

Signature · 1 line

2.44.0.278.ge034bb2e1d-goog

Saravana Kannan

author

2024-03-05 05:05 UTC #3 lore raw

PATCH v5 3/3 of: property: fw_devlink: Add support for "post-init-providers" property

In reply to Saravana Kannan

Add support for this property so that dependency cycles can be broken and fw_devlink can do better probe/suspend/resume ordering between devices in a dependency cycle.

Signed-off-by Saravana Kannan <[email protected]>

Acked-by Rafael J. Wysocki <[email protected]>

Reviewed-by Rob Herring <[email protected]>

---
 drivers/of/property.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

drivers/of/property.c +12 -3

index bce849f21ae2..b517a92dabca 100644
@@ -1066,7 +1066,8 @@ of_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 }
 
 static void of_link_to_phandle(struct device_node *con_np,
-			      struct device_node *sup_np)
+			      struct device_node *sup_np,
+			      u8 flags)
 {
 	struct device_node *tmp_np = of_node_get(sup_np);

@@ -1085,7 +1086,7 @@ static void of_link_to_phandle(struct device_node *con_np,
 		tmp_np = of_get_next_parent(tmp_np);
 	}
 
-	fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np), 0);
+	fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np), flags);
 }
 
 /**

@@ -1198,6 +1199,8 @@ static struct device_node *parse_##fname(struct device_node *np,	     \
  *		 to a struct device, implement this ops so fw_devlink can use it
  *		 to find the true consumer.
  * @optional: Describes whether a supplier is mandatory or not
+ * @fwlink_flags: Optional fwnode link flags to use when creating a fwnode link
+ *		  for this property.
  *
  * Returns:
  * parse_prop() return values are

@@ -1210,6 +1213,7 @@ struct supplier_bindings {
 					  const char *prop_name, int index);
 	struct device_node *(*get_con_dev)(struct device_node *np);
 	bool optional;
+	u8 fwlink_flags;
 };
 
 DEFINE_SIMPLE_PROP(clocks, "clocks", "#clock-cells")

@@ -1240,6 +1244,7 @@ DEFINE_SIMPLE_PROP(leds, "leds", NULL)
 DEFINE_SIMPLE_PROP(backlight, "backlight", NULL)
 DEFINE_SIMPLE_PROP(panel, "panel", NULL)
 DEFINE_SIMPLE_PROP(msi_parent, "msi-parent", "#msi-cells")
+DEFINE_SIMPLE_PROP(post_init_providers, "post-init-providers", NULL)
 DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
 DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")

@@ -1349,6 +1354,10 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_regulators, },
 	{ .parse_prop = parse_gpio, },
 	{ .parse_prop = parse_gpios, },
+	{
+		.parse_prop = parse_post_init_providers,
+		.fwlink_flags = FWLINK_FLAG_IGNORE,
+	},
 	{}
 };

@@ -1393,7 +1402,7 @@ static int of_link_property(struct device_node *con_np, const char *prop_name)
 					: of_node_get(con_np);
 			matched = true;
 			i++;
-			of_link_to_phandle(con_dev_np, phandle);
+			of_link_to_phandle(con_dev_np, phandle, s->fwlink_flags);
 			of_node_put(phandle);
 			of_node_put(con_dev_np);
 		}

Signature · 1 line

2.44.0.278.ge034bb2e1d-goog