Skip to contentSkip to conversation

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

PATCH Superseded Saravana Kannan 4 participants 17 messages Feb 12, 2024 – Feb 21, 2024

Subsystems (6)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v2 cover Add post-init-supplier binding to improve suspend/resume stability

This patch series adds a "post-init-supplier" 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.

Patch 3 (the binding docunentation) provide a lot more details and examples.

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

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

 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 drivers/base/core.c                           |  14 ++-
 drivers/firmware/efi/sysfb_efi.c              |   2 +-
 drivers/of/property.c                         |  17 ++-
 include/linux/fwnode.h                        |   5 +-
 6 files changed, 138 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
Signature · 1 line
2.43.0.687.g38aa6559b0-goog
#1 lore raw

PATCH v2 1/4 driver core: Adds flags param to fwnode_link_add()

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

---
 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 14d46af40f9a..33055001e08e 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 39a3ee1dfb58..751c11a28f33 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.43.0.687.g38aa6559b0-goog
#2 lore raw

PATCH v2 2/4 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.

---
 drivers/base/core.c    | 9 ++++++++-
 include/linux/fwnode.h | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)
drivers/base/core.c +8 -1
index 33055001e08e..bd762d90dac0 100644
@@ -1010,7 +1010,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;
@@ -1960,6 +1961,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;
@@ -2033,6 +2037,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.43.0.687.g38aa6559b0-goog
#3 lore raw

PATCH v2 3/4 dt-bindings: Add post-init-supplier property

In reply to Saravana Kannan

The post-init-supplier property can be used to break a dependency cycle by marking some supplier(s) as a post device initialization supplier(s). This allows an OS to do a better job at ordering initialization and suspend/resume of the devices in a dependency cycle.

---
 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 2 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
Documentation/devicetree/bindings/post-init-supplier.yaml new file +101 -0
new file mode 100644
index 000000000000..aab75b667259
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020, Google LLC. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/post-init-supplier.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Post device initialization supplier
+
+maintainers:
+  - Saravana Kannan <[email protected]>
+
+description: |
+  This property is used to indicate that the device(s) pointed to by the
+  property are not needed for the initialization of the device that lists this
+  property. This property is meaningful only when pointing to direct suppliers
+  of a device that are pointed to by other properties in the device.
+
+  A device can list its suppliers in devicetree using one or more of the
+  standard devicetree bindings. By default, it would be safe to assume the
+  supplier device can be initialized before the consumer device is initialized.
+
+  However, that assumption cannot be made when there are cyclic dependencies
+  between devices. Since each device is a supplier (directly or indirectly) of
+  the others in the cycle, there is no guaranteed safe order for initializing
+  the devices in a cycle. We can try to initialize them in an arbitrary order
+  and eventually successfully initialize all of them, but that doesn't always
+  work well.
+
+  For example, say,
+  * The device tree has the following cyclic dependency X -> Y -> Z -> X (where
+    -> denotes "depends on").
+  * But X is not needed to fully initialize Z (X might be needed only when a
+    specific functionality is requested post initialization).
+
+  If all the other -> are mandatory initialization dependencies, then trying to
+  initialize the devices in a loop (or arbitrarily) will always eventually end
+  up with the devices being initialized in the order Z, Y and X.
+
+  However, if Y is an optional supplier for X (where X provides limited
+  functionality when Y is not initialized and providing its services), then
+  trying to initialize the devices in a loop (or arbitrarily) could end up with
+  the devices being initialized in the following order:
+
+  * Z, Y and X - All devices provide full functionality
+  * Z, X and Y - X provides partial functionality
+  * X, Z and Y - X provides partial functionality
+
+  However, we always want to initialize the devices in the order Z, Y and X
+  since that provides the full functionality without interruptions.
+
+  One alternate option that might be suggested is to have the driver for X
+  notice that Y became available at a later point and adjust the functionality
+  it provides. However, other userspace applications could have started using X
+  with the limited functionality before Y was available and it might not be
+  possible to transparently transition X or the users of X to full
+  functionality while X is in use.
+
+  Similarly, when it comes to suspend (resume) ordering, it's unclear which
+  device in a dependency cycle needs to be suspended/resumed first and trying
+  arbitrary orders can result in system crashes or instability.
+
+  Explicitly calling out which link in a cycle needs to be broken when
+  determining the order, simplifies things a lot, improves efficiency, makes
+  the behavior more deterministic and maximizes the functionality that can be
+  provided without interruption.
+
+  This property is used to provide this additional information between devices
+  in a cycle by telling which supplier(s) is not needed for initializing the
+  device that lists this property.
+
+  In the example above, Z would list X as a post-init-supplier and the
+  initialization dependency would become X -> Y -> Z -/-> X. So the best order
+  to initialize them become clear: Z, Y and then X.
+
+select: true
+properties:
+  post-init-supplier:
+    # One or more suppliers can be marked as post initialization supplier
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1
+
+examples:
+  - |
+    gcc: clock-controller@1000 {
+        compatible = "vendor,soc4-gcc", "vendor,soc1-gcc";
+        reg = <0x1000 0x80>;
+        clocks = <&dispcc 0x1>
+        #clock-cells = <1>;
+        post-init-supplier = <&dispcc>;
+    };
+    dispcc: clock-controller@2000 {
+        compatible = "vendor,soc4-dispcc", "vendor,soc1-dispcc";
+        reg = <0x2000 0x80>;
+        clocks = <&gcc 0xdd>
+        #clock-cells = <1>;
+    };
MAINTAINERS +7 -6
index 3dfe7ea25320..79719af714be 100644
@@ -6055,12 +6055,6 @@ S:	Maintained
 F:	drivers/base/devcoredump.c
 F:	include/linux/devcoredump.h
 
-DEVICE DEPENDENCY HELPER SCRIPT
-M:	Saravana Kannan <[email protected]>
-L:	[email protected]
-S:	Maintained
-F:	scripts/dev-needs.sh
-
 DEVICE DIRECT ACCESS (DAX)
 M:	Dan Williams <[email protected]>
 M:	Vishal Verma <[email protected]>
@@ -8295,6 +8289,13 @@ F:	include/linux/firewire.h
 F:	include/uapi/linux/firewire*.h
 F:	tools/firewire/
 
+FIRMWARE DEVICE LINK (fw_devlink)
+M:	Saravana Kannan <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/post-init-supplier.yaml
+F:	scripts/dev-needs.sh
+
 FIRMWARE FRAMEWORK FOR ARMV8-A
 M:	Sudeep Holla <[email protected]>
 L:	[email protected] (moderated for non-subscribers)
Signature · 1 line
2.43.0.687.g38aa6559b0-goog
maintainer
#4 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 5 lines · Saravana Kannan

The post-init-supplier property can be used to break a dependency cycle by marking some supplier(s) as a post device initialization supplier(s). This allows an OS to do a better job at ordering initialization and suspend/resume of the devices in a dependency cycle.

Quoted 7 lines
---
 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 2 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: [error] syntax error: mapping values are not allowed here (syntax)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/post-init-supplier.example.dts' Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: mapping values are not allowed in this context make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/post-init-supplier.example.dts] Error 1 make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: mapping values are not allowed in this context /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/post-init-supplier.yaml: ignoring, error parsing file make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1428: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

#5 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

In reply to Saravana Kannan
Quoted 35 lines · Saravana Kannan

The post-init-supplier property can be used to break a dependency cycle by marking some supplier(s) as a post device initialization supplier(s). This allows an OS to do a better job at ordering initialization and suspend/resume of the devices in a dependency cycle.

---
 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 2 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
Documentation/devicetree/bindings/post-init-supplier.yaml new file +16 -0
new file mode 100644
index 000000000000..aab75b667259
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020, Google LLC. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/post-init-supplier.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Post device initialization supplier
+
+maintainers:
+  - Saravana Kannan <[email protected]>
+
+description: |
+  This property is used to indicate that the device(s) pointed to by the
+  property are not needed for the initialization of the device that lists this
+  property.
Quoted 2 lines

This property is meaningful only when pointing to direct suppliers
+ of a device that are pointed to by other properties in the device.

I don't think this sentence makes sense, or at least it is not easy to parse. It implies that it can "point to" other properties too - but that's not the case. It is only valid to "point to" these suppliers. I'd drop this entirely.

Documentation/devicetree/bindings/post-init-supplier.yaml line 21

Quoted 4 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +4 -0
+
+  A device can list its suppliers in devicetree using one or more of the
+  standard devicetree bindings. By default, it would be safe to assume the
+  supplier device can be initialized before the consumer device is initialized.

"it would be safe to assume" seems odd wording to me - I feel like the default is stronger than "safe to assume". I'd just drop the "would be safe to assume and replace with "is assumed".

Quoted 43 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +43 -0
+
+  However, that assumption cannot be made when there are cyclic dependencies
+  between devices. Since each device is a supplier (directly or indirectly) of
+  the others in the cycle, there is no guaranteed safe order for initializing
+  the devices in a cycle. We can try to initialize them in an arbitrary order
+  and eventually successfully initialize all of them, but that doesn't always
+  work well.
+
+  For example, say,
+  * The device tree has the following cyclic dependency X -> Y -> Z -> X (where
+    -> denotes "depends on").
+  * But X is not needed to fully initialize Z (X might be needed only when a
+    specific functionality is requested post initialization).
+
+  If all the other -> are mandatory initialization dependencies, then trying to
+  initialize the devices in a loop (or arbitrarily) will always eventually end
+  up with the devices being initialized in the order Z, Y and X.
+
+  However, if Y is an optional supplier for X (where X provides limited
+  functionality when Y is not initialized and providing its services), then
+  trying to initialize the devices in a loop (or arbitrarily) could end up with
+  the devices being initialized in the following order:
+
+  * Z, Y and X - All devices provide full functionality
+  * Z, X and Y - X provides partial functionality
+  * X, Z and Y - X provides partial functionality
+
+  However, we always want to initialize the devices in the order Z, Y and X
+  since that provides the full functionality without interruptions.
+
+  One alternate option that might be suggested is to have the driver for X
+  notice that Y became available at a later point and adjust the functionality
+  it provides. However, other userspace applications could have started using X
+  with the limited functionality before Y was available and it might not be
+  possible to transparently transition X or the users of X to full
+  functionality while X is in use.
+
+  Similarly, when it comes to suspend (resume) ordering, it's unclear which
+  device in a dependency cycle needs to be suspended/resumed first and trying
+  arbitrary orders can result in system crashes or instability.
+
+  Explicitly calling out which link in a cycle needs to be broken when
+  determining the order, simplifies things a lot, improves efficiency, makes

Documentation/devicetree/bindings/post-init-supplier.yaml line 74

Quoted 10 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +10 -0
+  the behavior more deterministic and maximizes the functionality that can be
+  provided without interruption.
+
+  This property is used to provide this additional information between devices
+  in a cycle by telling which supplier(s) is not needed for initializing the
+  device that lists this property.
+
+  In the example above, Z would list X as a post-init-supplier and the
+  initialization dependency would become X -> Y -> Z -/-> X. So the best order
+  to initialize them become clear: Z, Y and then X.

Otherwise, I think this is a great description, describing the use case well :)

Documentation/devicetree/bindings/post-init-supplier.yaml line 85

Quoted 11 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +11 -0
+
+select: true
+properties:
+  post-init-supplier:
+    # One or more suppliers can be marked as post initialization supplier
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1

Rob's bot rightfully complains here about invalid syntax. What you actually want to enforce here is any number of device phandles, but these phandles all contain only the label and no indices etc, right?

Documentation/devicetree/bindings/post-init-supplier.yaml line 92

Quoted 7 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +7 -0
+
+examples:
+  - |
+    gcc: clock-controller@1000 {
+        compatible = "vendor,soc4-gcc", "vendor,soc1-gcc";
+        reg = <0x1000 0x80>;
+        clocks = <&dispcc 0x1>

This clearly was never tested, Rob's bot warnings aside. You're missing a ; at EOL here and with the other clock below.

Cheers,
Conor.

Quoted 9 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +9 -0
+        #clock-cells = <1>;
+        post-init-supplier = <&dispcc>;
+    };
+    dispcc: clock-controller@2000 {
+        compatible = "vendor,soc4-dispcc", "vendor,soc1-dispcc";
+        reg = <0x2000 0x80>;
+        clocks = <&gcc 0xdd>
+        #clock-cells = <1>;
+    };
#6 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 2 lines · Conor Dooley

Documentation/devicetree/bindings/post-init-supplier.yaml line 79

Quoted 2 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +2 -0
+  post-init-supplier:
+    # One or more suppliers can be marked as post initialization supplier

Also, this should likely be pluralised, to match "clocks" "resets" "interrupts" etc.

Quoted 6 lines
Quoted 6 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +6 -0
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1
#7 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

In reply to Conor Dooley

Hi Conon,

Quoted 37 lines · Conor Dooley
Quoted 34 lines · Saravana Kannan

The post-init-supplier property can be used to break a dependency cycle by marking some supplier(s) as a post device initialization supplier(s). This allows an OS to do a better job at ordering initialization and suspend/resume of the devices in a dependency cycle.

---
 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 2 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
Documentation/devicetree/bindings/post-init-supplier.yaml new file +16 -0
new file mode 100644
index 000000000000..aab75b667259
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020, Google LLC. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/post-init-supplier.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Post device initialization supplier
+
+maintainers:
+  - Saravana Kannan <[email protected]>
+
+description: |
+  This property is used to indicate that the device(s) pointed to by the
+  property are not needed for the initialization of the device that lists this
+  property.
Quoted 5 lines
Quoted 2 lines

This property is meaningful only when pointing to direct suppliers
+ of a device that are pointed to by other properties in the device.

I don't think this sentence makes sense, or at least it is not easy to parse. It implies that it can "point to" other properties too

I don't see how this sentence implies this. But open to suggestions on how to reword it. I don't want to drop this line entirely though because I'm trying to make it clear that this doesn't make a device (that's not previously a supplier) into a supplier. It only down grades an existing supplier to a post device initialization supplier.

Quoted 5 lines

- but
that's not the case. It is only valid to "point to" these suppliers. I'd drop this entirely.

Quoted 8 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 21

Quoted 4 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +4 -0
+
+  A device can list its suppliers in devicetree using one or more of the
+  standard devicetree bindings. By default, it would be safe to assume the
+  supplier device can be initialized before the consumer device is initialized.

"it would be safe to assume" seems odd wording to me - I feel like the default is stronger than "safe to assume". I'd just drop the "would be safe to assume and replace with "is assumed".

Sounds good.

Quoted 49 lines
Quoted 49 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +48 -0
+
+  However, that assumption cannot be made when there are cyclic dependencies
+  between devices. Since each device is a supplier (directly or indirectly) of
+  the others in the cycle, there is no guaranteed safe order for initializing
+  the devices in a cycle. We can try to initialize them in an arbitrary order
+  and eventually successfully initialize all of them, but that doesn't always
+  work well.
+
+  For example, say,
+  * The device tree has the following cyclic dependency X -> Y -> Z -> X (where
+    -> denotes "depends on").
+  * But X is not needed to fully initialize Z (X might be needed only when a
+    specific functionality is requested post initialization).
+
+  If all the other -> are mandatory initialization dependencies, then trying to
+  initialize the devices in a loop (or arbitrarily) will always eventually end
+  up with the devices being initialized in the order Z, Y and X.
+
+  However, if Y is an optional supplier for X (where X provides limited
+  functionality when Y is not initialized and providing its services), then
+  trying to initialize the devices in a loop (or arbitrarily) could end up with
+  the devices being initialized in the following order:
+
+  * Z, Y and X - All devices provide full functionality
+  * Z, X and Y - X provides partial functionality
+  * X, Z and Y - X provides partial functionality
+
+  However, we always want to initialize the devices in the order Z, Y and X
+  since that provides the full functionality without interruptions.
+
+  One alternate option that might be suggested is to have the driver for X
+  notice that Y became available at a later point and adjust the functionality
+  it provides. However, other userspace applications could have started using X
+  with the limited functionality before Y was available and it might not be
+  possible to transparently transition X or the users of X to full
+  functionality while X is in use.
+
+  Similarly, when it comes to suspend (resume) ordering, it's unclear which
+  device in a dependency cycle needs to be suspended/resumed first and trying
+  arbitrary orders can result in system crashes or instability.
+
+  Explicitly calling out which link in a cycle needs to be broken when
+  determining the order, simplifies things a lot, improves efficiency, makes
+  the behavior more deterministic and maximizes the functionality that can be
+  provided without interruption.
+
+  This property is used to provide this additional information between devices
+  in a cycle by telling which supplier(s) is not needed for initializing the
Quoted 8 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 74

Quoted 5 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +5 -0
+  device that lists this property.
+
+  In the example above, Z would list X as a post-init-supplier and the
+  initialization dependency would become X -> Y -> Z -/-> X. So the best order
+  to initialize them become clear: Z, Y and then X.

Otherwise, I think this is a great description, describing the use case well :)

Thanks! I always spend more time writing documentation and commit text than the time I spend writing code.

Quoted 5 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 78

Quoted 5 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +4 -0
+
+select: true
+properties:
+  post-init-supplier:

[Merging your other email here]

Quoted 2 lines

Also, this should likely be pluralised, to match "clocks" "resets" "interrupts" etc.

Good point. Done.

Quoted 9 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 85

Quoted 7 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +7 -0
+    # One or more suppliers can be marked as post initialization supplier
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1

Rob's bot rightfully complains here about invalid syntax.

I added these two lines based on Rob's feedback. Is the indentation that's wrong?

Yeah, I'm trying to run the dts checker, but I haven't be able to get it to work on my end. See my email to Rob on the v1 series about this.

$ make DT_CHECKER_FLAGS=-m dt_binding_check

The best I could get out of it is a bunch of error reports on other files and then:
...
<snip>/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

I also tried to use DT_SCHEMA_FILES so I can only test this one file, but that wasn't working either:

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/post-init-suppliers.yaml
or
$ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=<path to
the .patch file>

Results in this error early on in the output:
...
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA] [--list-files] [-f {parsable,standard,colored,github,auto}] [-s] [--no-warnings] [-v] [FILE_OR_DIR ...]
yamllint: error: one of the arguments FILE_OR_DIR - is required
...
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

Quoted 3 lines

What you
actually want to enforce here is any number of device phandles, but these phandles all contain only the label and no indices etc, right?

Correct.

Quoted 11 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 92

Quoted 8 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +7 -0
+
+examples:
+  - |
+    gcc: clock-controller@1000 {
+        compatible = "vendor,soc4-gcc", "vendor,soc1-gcc";
+        reg = <0x1000 0x80>;
+        clocks = <&dispcc 0x1>

This clearly was never tested, Rob's bot warnings aside. You're missing a ; at EOL here and with the other clock below.

Yup. I'm unable to get the test to run.

Thanks,
Saravana

#8 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 40 lines · Saravana Kannan

Hi Conon,

Quoted 37 lines · Conor Dooley
Quoted 34 lines · Saravana Kannan

The post-init-supplier property can be used to break a dependency cycle by marking some supplier(s) as a post device initialization supplier(s). This allows an OS to do a better job at ordering initialization and suspend/resume of the devices in a dependency cycle.

---
 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 2 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
Documentation/devicetree/bindings/post-init-supplier.yaml new file +16 -0
new file mode 100644
index 000000000000..aab75b667259
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020, Google LLC. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/post-init-supplier.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Post device initialization supplier
+
+maintainers:
+  - Saravana Kannan <[email protected]>
+
+description: |
+  This property is used to indicate that the device(s) pointed to by the
+  property are not needed for the initialization of the device that lists this
+  property.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

This property is meaningful only when pointing to direct suppliers
+ of a device that are pointed to by other properties in the device.

I don't think this sentence makes sense, or at least it is not easy to parse. It implies that it can "point to" other properties too

I don't see how this sentence implies this.

Because, to me, it reads as if you can put extra stuff in here that will be ignored if not "pointed to" by another property. The word "meaningful" is what implies that you can.

Quoted 5 lines

But open to suggestions on
how to reword it. I don't want to drop this line entirely though because I'm trying to make it clear that this doesn't make a device (that's not previously a supplier) into a supplier. It only down grades an existing supplier to a post device initialization supplier.

If you wanna keep it, I would just go for what you said in this response - that this property does not make devices into suppliers and is only to mark existing suppliers as post-init. I think that rules out putting other devices in there.

Quoted 98 lines
Quoted 5 lines

- but
that's not the case. It is only valid to "point to" these suppliers. I'd drop this entirely.

Quoted 8 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 21

Quoted 4 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +4 -0
+
+  A device can list its suppliers in devicetree using one or more of the
+  standard devicetree bindings. By default, it would be safe to assume the
+  supplier device can be initialized before the consumer device is initialized.

"it would be safe to assume" seems odd wording to me - I feel like the default is stronger than "safe to assume". I'd just drop the "would be safe to assume and replace with "is assumed".

Sounds good.

Quoted 49 lines
Quoted 49 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +48 -0
+
+  However, that assumption cannot be made when there are cyclic dependencies
+  between devices. Since each device is a supplier (directly or indirectly) of
+  the others in the cycle, there is no guaranteed safe order for initializing
+  the devices in a cycle. We can try to initialize them in an arbitrary order
+  and eventually successfully initialize all of them, but that doesn't always
+  work well.
+
+  For example, say,
+  * The device tree has the following cyclic dependency X -> Y -> Z -> X (where
+    -> denotes "depends on").
+  * But X is not needed to fully initialize Z (X might be needed only when a
+    specific functionality is requested post initialization).
+
+  If all the other -> are mandatory initialization dependencies, then trying to
+  initialize the devices in a loop (or arbitrarily) will always eventually end
+  up with the devices being initialized in the order Z, Y and X.
+
+  However, if Y is an optional supplier for X (where X provides limited
+  functionality when Y is not initialized and providing its services), then
+  trying to initialize the devices in a loop (or arbitrarily) could end up with
+  the devices being initialized in the following order:
+
+  * Z, Y and X - All devices provide full functionality
+  * Z, X and Y - X provides partial functionality
+  * X, Z and Y - X provides partial functionality
+
+  However, we always want to initialize the devices in the order Z, Y and X
+  since that provides the full functionality without interruptions.
+
+  One alternate option that might be suggested is to have the driver for X
+  notice that Y became available at a later point and adjust the functionality
+  it provides. However, other userspace applications could have started using X
+  with the limited functionality before Y was available and it might not be
+  possible to transparently transition X or the users of X to full
+  functionality while X is in use.
+
+  Similarly, when it comes to suspend (resume) ordering, it's unclear which
+  device in a dependency cycle needs to be suspended/resumed first and trying
+  arbitrary orders can result in system crashes or instability.
+
+  Explicitly calling out which link in a cycle needs to be broken when
+  determining the order, simplifies things a lot, improves efficiency, makes
+  the behavior more deterministic and maximizes the functionality that can be
+  provided without interruption.
+
+  This property is used to provide this additional information between devices
+  in a cycle by telling which supplier(s) is not needed for initializing the
Quoted 8 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 74

Quoted 5 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +5 -0
+  device that lists this property.
+
+  In the example above, Z would list X as a post-init-supplier and the
+  initialization dependency would become X -> Y -> Z -/-> X. So the best order
+  to initialize them become clear: Z, Y and then X.

Otherwise, I think this is a great description, describing the use case well :)

Thanks! I always spend more time writing documentation and commit text than the time I spend writing code.

Quoted 5 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 78

Quoted 5 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +4 -0
+
+select: true
+properties:
+  post-init-supplier:

[Merging your other email here]

Quoted 2 lines

Also, this should likely be pluralised, to match "clocks" "resets" "interrupts" etc.

Good point. Done.

Quoted 8 lines
Quoted 7 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +7 -0
+    # One or more suppliers can be marked as post initialization supplier
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1
Quoted 4 lines
Quoted 1 line

Rob's bot rightfully complains here about invalid syntax.

I added these two lines based on Rob's feedback. Is the indentation that's wrong?

Aye, both items: and maxItems: need to lose a level of indent. That said, its not actually restricting anything. I fixed it up locally and you can put as many elements as you like into each phandle and it does not care. Maybe Rob can tell what is going wrong there..

Quoted 4 lines

Yeah, I'm trying to run the dts checker, but I haven't be able to get it to work on my end. See my email to Rob on the v1 series about this.

Quoted 8 lines
$ make DT_CHECKER_FLAGS=-m dt_binding_check

The best I could get out of it is a bunch of error reports on other files and then:
...
<snip>/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

Yup, that is about right, although you snipped out the actual complaint.

Quoted 10 lines

I also tried to use DT_SCHEMA_FILES so I can only test this one file, but that wasn't working either:

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/post-init-suppliers.yaml
or
$ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=<path to
the .patch file>
Quoted 10 lines

Results in this error early on in the output:
...
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA] [--list-files] [-f {parsable,standard,colored,github,auto}] [-s] [--no-warnings] [-v] [FILE_OR_DIR ...]
yamllint: error: one of the arguments FILE_OR_DIR - is required
...
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

That is part of the actual complaint:

make dt_binding_check W=1 -j 30 DT_SCHEMA_FILES=post-init-supplier.yaml
LINT Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/post-init-supplier.example.dts
Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: mapping values are not allowed here
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/post-init-supplier.example.dts] Error 1 make[2]: *** Deleting file 'Documentation/devicetree/bindings/post-init-supplier.example.dts'
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: [error] syntax error: mapping values are not allowed here (syntax)
CHKDT Documentation/devicetree/bindings/processed-schema.json
./Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: mapping values are not allowed here
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/stuff/linux-dt/Documentation/devicetree/bindings/post-init-supplier.yaml: ignoring, error parsing file
make[1]: *** [/stuff/linux-dt/Makefile:1432: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

#9 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 60 lines · Conor Dooley
Quoted 40 lines · Saravana Kannan

Hi Conon,

Quoted 37 lines · Conor Dooley
Quoted 34 lines · Saravana Kannan

The post-init-supplier property can be used to break a dependency cycle by marking some supplier(s) as a post device initialization supplier(s). This allows an OS to do a better job at ordering initialization and suspend/resume of the devices in a dependency cycle.

---
 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 2 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
Documentation/devicetree/bindings/post-init-supplier.yaml new file +16 -0
new file mode 100644
index 000000000000..aab75b667259
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020, Google LLC. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/post-init-supplier.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Post device initialization supplier
+
+maintainers:
+  - Saravana Kannan <[email protected]>
+
+description: |
+  This property is used to indicate that the device(s) pointed to by the
+  property are not needed for the initialization of the device that lists this
+  property.
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

This property is meaningful only when pointing to direct suppliers
+ of a device that are pointed to by other properties in the device.

I don't think this sentence makes sense, or at least it is not easy to parse. It implies that it can "point to" other properties too

I don't see how this sentence implies this.

Because, to me, it reads as if you can put extra stuff in here that will be ignored if not "pointed to" by another property. The word "meaningful" is what implies that you can.

Quoted 5 lines

But open to suggestions on
how to reword it. I don't want to drop this line entirely though because I'm trying to make it clear that this doesn't make a device (that's not previously a supplier) into a supplier. It only down grades an existing supplier to a post device initialization supplier.

Quoted 4 lines

If you wanna keep it, I would just go for what you said in this response - that this property does not make devices into suppliers and is only to mark existing suppliers as post-init. I think that rules out putting other devices in there.

Sounds good.

Quoted 100 lines
Quoted 100 lines
Quoted 5 lines

- but
that's not the case. It is only valid to "point to" these suppliers. I'd drop this entirely.

Quoted 8 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 21

Quoted 4 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +4 -0
+
+  A device can list its suppliers in devicetree using one or more of the
+  standard devicetree bindings. By default, it would be safe to assume the
+  supplier device can be initialized before the consumer device is initialized.

"it would be safe to assume" seems odd wording to me - I feel like the default is stronger than "safe to assume". I'd just drop the "would be safe to assume and replace with "is assumed".

Sounds good.

Quoted 49 lines
Quoted 49 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +48 -0
+
+  However, that assumption cannot be made when there are cyclic dependencies
+  between devices. Since each device is a supplier (directly or indirectly) of
+  the others in the cycle, there is no guaranteed safe order for initializing
+  the devices in a cycle. We can try to initialize them in an arbitrary order
+  and eventually successfully initialize all of them, but that doesn't always
+  work well.
+
+  For example, say,
+  * The device tree has the following cyclic dependency X -> Y -> Z -> X (where
+    -> denotes "depends on").
+  * But X is not needed to fully initialize Z (X might be needed only when a
+    specific functionality is requested post initialization).
+
+  If all the other -> are mandatory initialization dependencies, then trying to
+  initialize the devices in a loop (or arbitrarily) will always eventually end
+  up with the devices being initialized in the order Z, Y and X.
+
+  However, if Y is an optional supplier for X (where X provides limited
+  functionality when Y is not initialized and providing its services), then
+  trying to initialize the devices in a loop (or arbitrarily) could end up with
+  the devices being initialized in the following order:
+
+  * Z, Y and X - All devices provide full functionality
+  * Z, X and Y - X provides partial functionality
+  * X, Z and Y - X provides partial functionality
+
+  However, we always want to initialize the devices in the order Z, Y and X
+  since that provides the full functionality without interruptions.
+
+  One alternate option that might be suggested is to have the driver for X
+  notice that Y became available at a later point and adjust the functionality
+  it provides. However, other userspace applications could have started using X
+  with the limited functionality before Y was available and it might not be
+  possible to transparently transition X or the users of X to full
+  functionality while X is in use.
+
+  Similarly, when it comes to suspend (resume) ordering, it's unclear which
+  device in a dependency cycle needs to be suspended/resumed first and trying
+  arbitrary orders can result in system crashes or instability.
+
+  Explicitly calling out which link in a cycle needs to be broken when
+  determining the order, simplifies things a lot, improves efficiency, makes
+  the behavior more deterministic and maximizes the functionality that can be
+  provided without interruption.
+
+  This property is used to provide this additional information between devices
+  in a cycle by telling which supplier(s) is not needed for initializing the
Quoted 8 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 74

Quoted 5 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +5 -0
+  device that lists this property.
+
+  In the example above, Z would list X as a post-init-supplier and the
+  initialization dependency would become X -> Y -> Z -/-> X. So the best order
+  to initialize them become clear: Z, Y and then X.

Otherwise, I think this is a great description, describing the use case well :)

Thanks! I always spend more time writing documentation and commit text than the time I spend writing code.

Quoted 5 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 78

Quoted 5 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +4 -0
+
+select: true
+properties:
+  post-init-supplier:

[Merging your other email here]

Quoted 2 lines

Also, this should likely be pluralised, to match "clocks" "resets" "interrupts" etc.

Good point. Done.

Quoted 9 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 85

Quoted 7 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +7 -0
+    # One or more suppliers can be marked as post initialization supplier
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1

Rob's bot rightfully complains here about invalid syntax.

Quoted 7 lines
Quoted 2 lines

I added these two lines based on Rob's feedback. Is the indentation that's wrong?

Aye, both items: and maxItems: need to lose a level of indent. That said, its not actually restricting anything. I fixed it up locally and you can put as many elements as you like into each phandle and it does not care. Maybe Rob can tell what is going wrong there..

I made that fix and now I'm getting this:

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/post-init-suppliers.yaml
  DTEX    Documentation/devicetree/bindings/post-init-suppliers.example.dts
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
'oneOf' conditional failed, one must be fixed:
        'unevaluatedProperties' is a required property
        'additionalProperties' is a required property
        hint: Either unevaluatedProperties or additionalProperties
must be present
        from schema $id: http://devicetree.org/meta-schemas/core.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/mnt/android/linus-tree/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml:
ignoring, error in schema: properties
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error in schema:
/mnt/android/linus-tree/Documentation/devicetree/bindings/soc/tegra/nvidia,tegra20-pmc.yaml:
ignoring, error in schema: allOf: 0: then: properties: pinmux
/mnt/android/linus-tree/Documentation/devicetree/bindings/net/lantiq,pef2256.yaml:
ignoring, error in schema: properties: lantiq,data-rate-bps
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-supplier.yaml:
ignoring, error in schema:
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml:
ignoring, error in schema: properties: honeywell,pmax-pascal
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml:
ignoring, error in schema: properties: honeywell,pmax-pascal
  DTC_CHK Documentation/devicetree/bindings/post-init-suppliers.example.dtb
Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@1000: failed to match any schema with
compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@1000: failed to match any schema with
compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@2000: failed to match any schema with
compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']
Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@2000: failed to match any schema with
compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']

But I guess the "oneOf" error is because the yaml is being treated as a description of a DT node and not a schema?

Rob,

Can you let me know how to move ahead with this? I'll do the fixes that Conor suggested in v3.

-Saravana

Quoted 53 lines
Quoted 5 lines

Yeah, I'm trying to run the dts checker, but I haven't be able to get it to work on my end. See my email to Rob on the v1 series about this.

Quoted 8 lines
$ make DT_CHECKER_FLAGS=-m dt_binding_check

The best I could get out of it is a bunch of error reports on other files and then:
...
<snip>/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

Yup, that is about right, although you snipped out the actual complaint.

Quoted 10 lines

I also tried to use DT_SCHEMA_FILES so I can only test this one file, but that wasn't working either:

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/post-init-suppliers.yaml
or
$ make DT_CHECKER_FLAGS=-m dt_binding_check DT_SCHEMA_FILES=<path to
the .patch file>
Quoted 10 lines

Results in this error early on in the output:
...
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA] [--list-files] [-f {parsable,standard,colored,github,auto}] [-s] [--no-warnings] [-v] [FILE_OR_DIR ...]
yamllint: error: one of the arguments FILE_OR_DIR - is required
...
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

That is part of the actual complaint:

make dt_binding_check W=1 -j 30 DT_SCHEMA_FILES=post-init-supplier.yaml
LINT Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/post-init-supplier.example.dts
Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: mapping values are not allowed here
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/post-init-supplier.example.dts] Error 1 make[2]: *** Deleting file 'Documentation/devicetree/bindings/post-init-supplier.example.dts'
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: [error] syntax error: mapping values are not allowed here (syntax)
CHKDT Documentation/devicetree/bindings/processed-schema.json
./Documentation/devicetree/bindings/post-init-supplier.yaml:84:12: mapping values are not allowed here
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/stuff/linux-dt/Documentation/devicetree/bindings/post-init-supplier.yaml: ignoring, error parsing file
make[1]: *** [/stuff/linux-dt/Makefile:1432: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

#10 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 29 lines · Saravana Kannan

I made that fix and now I'm getting this:

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/post-init-suppliers.yaml
  DTEX    Documentation/devicetree/bindings/post-init-suppliers.example.dts
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
'oneOf' conditional failed, one must be fixed:
        'unevaluatedProperties' is a required property
        'additionalProperties' is a required property
        hint: Either unevaluatedProperties or additionalProperties
must be present
        from schema $id: http://devicetree.org/meta-schemas/core.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/mnt/android/linus-tree/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml:
ignoring, error in schema: properties
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error in schema:
/mnt/android/linus-tree/Documentation/devicetree/bindings/soc/tegra/nvidia,tegra20-pmc.yaml:
ignoring, error in schema: allOf: 0: then: properties: pinmux
/mnt/android/linus-tree/Documentation/devicetree/bindings/net/lantiq,pef2256.yaml:
ignoring, error in schema: properties: lantiq,data-rate-bps
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-supplier.yaml:
ignoring, error in schema:
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml:
ignoring, error in schema: properties: honeywell,pmax-pascal
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml:
ignoring, error in schema: properties: honeywell,pmax-pascal
Quoted 13 lines

DTC_CHK Documentation/devicetree/bindings/post-init-suppliers.example.dtb
Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@1000: failed to match any schema with
compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc'] Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@1000: failed to match any schema with
compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc'] Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@2000: failed to match any schema with
compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc'] Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@2000: failed to match any schema with
compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']

FWIW, I don't see these or the other errors you see above. You really need to get yourself a newer version of dt-schema, or else avoid working on this using whatever castrated system google provides you with!

Quoted 2 lines

But I guess the "oneOf" error is because the yaml is being treated as a description of a DT node and not a schema?

The oneOf is due to missing "additionalProperties: true" - As far as I understand you need that regardless of whether this is going into dt-schema or the kernel.

#11 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 45 lines · Conor Dooley
Quoted 29 lines · Saravana Kannan

I made that fix and now I'm getting this:

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/post-init-suppliers.yaml
  DTEX    Documentation/devicetree/bindings/post-init-suppliers.example.dts
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
'oneOf' conditional failed, one must be fixed:
        'unevaluatedProperties' is a required property
        'additionalProperties' is a required property
        hint: Either unevaluatedProperties or additionalProperties
must be present
        from schema $id: http://devicetree.org/meta-schemas/core.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/mnt/android/linus-tree/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml:
ignoring, error in schema: properties
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error in schema:
/mnt/android/linus-tree/Documentation/devicetree/bindings/soc/tegra/nvidia,tegra20-pmc.yaml:
ignoring, error in schema: allOf: 0: then: properties: pinmux
/mnt/android/linus-tree/Documentation/devicetree/bindings/net/lantiq,pef2256.yaml:
ignoring, error in schema: properties: lantiq,data-rate-bps
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-supplier.yaml:
ignoring, error in schema:
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml:
ignoring, error in schema: properties: honeywell,pmax-pascal
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml:
ignoring, error in schema: properties: honeywell,pmax-pascal
Quoted 13 lines

DTC_CHK Documentation/devicetree/bindings/post-init-suppliers.example.dtb
Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@1000: failed to match any schema with
compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc'] Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@1000: failed to match any schema with
compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc'] Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@2000: failed to match any schema with
compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc'] Documentation/devicetree/bindings/post-init-suppliers.example.dtb:0:0:
/example-0/clock-controller@2000: failed to match any schema with
compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']

Quoted 3 lines

FWIW, I don't see these or the other errors you see above. You really need to get yourself a newer version of dt-schema, or else avoid working on this using whatever castrated system google provides you with!

Ok, finally found the workaround to updating these packages and the output is a lot cleaner now.

Quoted 6 lines
Quoted 2 lines

But I guess the "oneOf" error is because the yaml is being treated as a description of a DT node and not a schema?

The oneOf is due to missing "additionalProperties: true" - As far as I understand you need that regardless of whether this is going into dt-schema or the kernel.

Ok, I added that and the errors go away. I'll send out a v3 and hopefully Rob can pick it up.

-Saravana

#12 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

In reply to Saravana Kannan
Quoted 28 lines · Saravana Kannan

Good point. Done.

Quoted 9 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 85

Quoted 7 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +7 -0
+    # One or more suppliers can be marked as post initialization supplier
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1

Rob's bot rightfully complains here about invalid syntax.

I added these two lines based on Rob's feedback. Is the indentation that's wrong?

Yeah, I'm trying to run the dts checker, but I haven't be able to get it to work on my end. See my email to Rob on the v1 series about this.

$ make DT_CHECKER_FLAGS=-m dt_binding_check

The best I could get out of it is a bunch of error reports on other files and then:
...
<snip>/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

Quoted 2 lines

I also tried to use DT_SCHEMA_FILES so I can only test this one file, but that wasn't working either:

I see the errors immediately during testing, no special arguments needed:

crosc64_dt_binding_check post-init-supplier.yaml
make[1]: Entering directory '/home/krzk/dev/linux/linux/out'
LINT Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/post-init-supplier.example.dts
../Documentation/devicetree/bindings/post-init-supplier.yaml:84:12:
[error] syntax error: mapping values are not allowed here (syntax)
CHKDT Documentation/devicetree/bindings/processed-schema.json
../Documentation/devicetree/bindings/post-init-supplier.yaml:84:12:
mapping values are not allowed in this context
make[3]: *** [../Documentation/devicetree/bindings/Makefile:26:
Documentation/devicetree/bindings/post-init-supplier.example.dts] Error 1 make[3]: *** Deleting file 'Documentation/devicetree/bindings/post-init-supplier.example.dts' make[3]: *** Waiting for unfinished jobs.... ../Documentation/devicetree/bindings/post-init-supplier.yaml:84:12:
mapping values are not allowed in this context
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/home/krzk/dev/linux/linux/Documentation/devicetree/bindings/post-init-supplier.yaml:
ignoring, error parsing file
make[2]: *** [/home/krzk/dev/linux/linux/Makefile:1424:
dt_binding_check] Error 2
make[1]: *** [/home/krzk/dev/linux/linux/Makefile:240: __sub-make] Error 2 make[1]: Leaving directory '/home/krzk/dev/linux/linux/out'
make: *** [Makefile:240: __sub-make] Error 2

https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/

I assume you develop on some older trees, because both next and v6.8-rc1 work... or standard issues: old dtschema, old yamllint.

I am afraid you do it for some old Android kernel... :(

Best regards,
Krzysztof

#13 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 51 lines · Krzysztof Kozlowski
Quoted 28 lines · Saravana Kannan

Good point. Done.

Quoted 9 lines

Documentation/devicetree/bindings/post-init-supplier.yaml line 85

Quoted 7 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +7 -0
+    # One or more suppliers can be marked as post initialization supplier
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+      items:
+        maxItems: 1

Rob's bot rightfully complains here about invalid syntax.

I added these two lines based on Rob's feedback. Is the indentation that's wrong?

Yeah, I'm trying to run the dts checker, but I haven't be able to get it to work on my end. See my email to Rob on the v1 series about this.

$ make DT_CHECKER_FLAGS=-m dt_binding_check

The best I could get out of it is a bunch of error reports on other files and then:
...
<snip>/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
...

Quoted 2 lines

I also tried to use DT_SCHEMA_FILES so I can only test this one file, but that wasn't working either:

I see the errors immediately during testing, no special arguments needed:

crosc64_dt_binding_check post-init-supplier.yaml
make[1]: Entering directory '/home/krzk/dev/linux/linux/out'
LINT Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/post-init-supplier.example.dts
../Documentation/devicetree/bindings/post-init-supplier.yaml:84:12:
[error] syntax error: mapping values are not allowed here (syntax)
CHKDT Documentation/devicetree/bindings/processed-schema.json
../Documentation/devicetree/bindings/post-init-supplier.yaml:84:12:
mapping values are not allowed in this context
make[3]: *** [../Documentation/devicetree/bindings/Makefile:26:
Documentation/devicetree/bindings/post-init-supplier.example.dts] Error 1 make[3]: *** Deleting file 'Documentation/devicetree/bindings/post-init-supplier.example.dts' make[3]: *** Waiting for unfinished jobs.... ../Documentation/devicetree/bindings/post-init-supplier.yaml:84:12:
mapping values are not allowed in this context

Quoted 8 lines

SCHEMA Documentation/devicetree/bindings/processed-schema.json
/home/krzk/dev/linux/linux/Documentation/devicetree/bindings/post-init-supplier.yaml:
ignoring, error parsing file
make[2]: *** [/home/krzk/dev/linux/linux/Makefile:1424:
dt_binding_check] Error 2
make[1]: *** [/home/krzk/dev/linux/linux/Makefile:240: __sub-make] Error 2 make[1]: Leaving directory '/home/krzk/dev/linux/linux/out'
make: *** [Makefile:240: __sub-make] Error 2

I think I was just getting overwhelmed with the sea of error logs I saw (for unrelated files). If I don't use the flags it's way too noisy and it's not always the first thing that's reported.

This is what I see now and I think I now understand what to look for.

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/post-init-suppliers.yaml
  LINT    Documentation/devicetree/bindings
./Documentation/devicetree/bindings/post-init-suppliers.yaml:84:12:
[error] syntax error: mapping values are not allowed here (syntax)
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
./Documentation/devicetree/bindings/post-init-suppliers.yaml:84:12:
mapping values are not allowed in this context
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/mnt/android/linus-tree/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml:
ignoring, error in schema: properties
/mnt/android/linus-tree/Documentation/devicetree/bindings/post-init-suppliers.yaml:
ignoring, error parsing file
/mnt/android/linus-tree/Documentation/devicetree/bindings/soc/tegra/nvidia,tegra20-pmc.yaml:
ignoring, error in schema: allOf: 0: then: properties: pinmux
/mnt/android/linus-tree/Documentation/devicetree/bindings/net/lantiq,pef2256.yaml:
ignoring, error in schema: properties: lantiq,data-rate-bps
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml:
ignoring, error in schema: properties: honeywell,pmin-pascal
/mnt/android/linus-tree/Documentation/devicetree/bindings/iio/pressure/honeywell,hsc030pa.yaml:
ignoring, error in schema: properties: honeywell,pmax-pascal
  DTEX    Documentation/devicetree/bindings/post-init-suppliers.example.dts
Documentation/devicetree/bindings/post-init-suppliers.yaml:84:12:
mapping values are not allowed in this context
make[2]: *** [Documentation/devicetree/bindings/Makefile:26:
Documentation/devicetree/bindings/post-init-suppliers.example.dts]
Error 1
make[2]: *** Deleting file
'Documentation/devicetree/bindings/post-init-suppliers.example.dts'
make[1]: *** [/mnt/android/linus-tree/Makefile:1432: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2
Quoted 8 lines

https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/

I assume you develop on some older trees, because both next and v6.8-rc1 work... or standard issues: old dtschema, old yamllint.

I am afraid you do it for some old Android kernel... :(

No, I always develop on Linus's tree and test it on an android kernel that's behind Linus's tree by a month or so.

My yamllint version is 1.32.0, but until 2 weeks ago the latest yamllint version was 1.33.0.

And dt-schema is 2022.08.2-5 and I had to revert this from Linus's tree to get it to work:
b32dcf23a03e dt-bindings: Drop kernel copy of common reserved-memory bindings

Unfortunately, AFAIK, I don't have permissions to change the package repo, so can't really install a newer version.

Thanks for the tips.

-Saravana

-Saravana

#14 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

Quoted 14 lines · Saravana Kannan
Quoted 7 lines

https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/

I assume you develop on some older trees, because both next and v6.8-rc1 work... or standard issues: old dtschema, old yamllint.

I am afraid you do it for some old Android kernel... :(

No, I always develop on Linus's tree and test it on an android kernel that's behind Linus's tree by a month or so.

My yamllint version is 1.32.0, but until 2 weeks ago the latest yamllint version was 1.33.0.

Quoted 6 lines

And dt-schema is 2022.08.2-5 and I had to revert this from Linus's tree to get it to work:
b32dcf23a03e dt-bindings: Drop kernel copy of common reserved-memory bindings

Unfortunately, AFAIK, I don't have permissions to change the package repo, so can't really install a newer version.

pip packages are by default per user, so why you cannot install updated dtschema?

Best regards,
Krzysztof

maintainer
#15 lore raw

Re: [PATCH v2 3/4] dt-bindings: Add post-init-supplier property

In reply to Saravana Kannan
Quoted 84 lines · Saravana Kannan

The post-init-supplier property can be used to break a dependency cycle by marking some supplier(s) as a post device initialization supplier(s). This allows an OS to do a better job at ordering initialization and suspend/resume of the devices in a dependency cycle.

---
 .../bindings/post-init-supplier.yaml          | 101 ++++++++++++++++++
 MAINTAINERS                                   |  13 +--
 2 files changed, 108 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
Documentation/devicetree/bindings/post-init-supplier.yaml new file +66 -0
new file mode 100644
index 000000000000..aab75b667259
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020, Google LLC. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/post-init-supplier.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Post device initialization supplier
+
+maintainers:
+  - Saravana Kannan <[email protected]>
+
+description: |
+  This property is used to indicate that the device(s) pointed to by the
+  property are not needed for the initialization of the device that lists this
+  property. This property is meaningful only when pointing to direct suppliers
+  of a device that are pointed to by other properties in the device.
+
+  A device can list its suppliers in devicetree using one or more of the
+  standard devicetree bindings. By default, it would be safe to assume the
+  supplier device can be initialized before the consumer device is initialized.
+
+  However, that assumption cannot be made when there are cyclic dependencies
+  between devices. Since each device is a supplier (directly or indirectly) of
+  the others in the cycle, there is no guaranteed safe order for initializing
+  the devices in a cycle. We can try to initialize them in an arbitrary order
+  and eventually successfully initialize all of them, but that doesn't always
+  work well.
+
+  For example, say,
+  * The device tree has the following cyclic dependency X -> Y -> Z -> X (where
+    -> denotes "depends on").
+  * But X is not needed to fully initialize Z (X might be needed only when a
+    specific functionality is requested post initialization).
+
+  If all the other -> are mandatory initialization dependencies, then trying to
+  initialize the devices in a loop (or arbitrarily) will always eventually end
+  up with the devices being initialized in the order Z, Y and X.
+
+  However, if Y is an optional supplier for X (where X provides limited
+  functionality when Y is not initialized and providing its services), then
+  trying to initialize the devices in a loop (or arbitrarily) could end up with
+  the devices being initialized in the following order:
+
+  * Z, Y and X - All devices provide full functionality
+  * Z, X and Y - X provides partial functionality
+  * X, Z and Y - X provides partial functionality
+
+  However, we always want to initialize the devices in the order Z, Y and X
+  since that provides the full functionality without interruptions.
+
+  One alternate option that might be suggested is to have the driver for X
+  notice that Y became available at a later point and adjust the functionality
+  it provides. However, other userspace applications could have started using X
+  with the limited functionality before Y was available and it might not be
+  possible to transparently transition X or the users of X to full
+  functionality while X is in use.
+
+  Similarly, when it comes to suspend (resume) ordering, it's unclear which
+  device in a dependency cycle needs to be suspended/resumed first and trying
+  arbitrary orders can result in system crashes or instability.
+
+  Explicitly calling out which link in a cycle needs to be broken when
+  determining the order, simplifies things a lot, improves efficiency, makes
+  the behavior more deterministic and maximizes the functionality that can be
+  provided without interruption.

Documentation/devicetree/bindings/post-init-supplier.yaml line 76

Quoted 10 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +10 -0
+
+  This property is used to provide this additional information between devices
+  in a cycle by telling which supplier(s) is not needed for initializing the
+  device that lists this property.
+
+  In the example above, Z would list X as a post-init-supplier and the
+  initialization dependency would become X -> Y -> Z -/-> X. So the best order
+  to initialize them become clear: Z, Y and then X.
+
+select: true

blank line

Documentation/devicetree/bindings/post-init-supplier.yaml line 78

Quoted 2 lines
Documentation/devicetree/bindings/post-init-supplier.yaml new file +2 -0
+properties:
+  post-init-supplier:

'supply' is already used for regulators. Let's make it 'post-init-providers'.

Rob

#16 lore raw

PATCH v2 4/4 of: property: fw_devlink: Add support for "post-init-supplier" 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.

---
 drivers/of/property.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
drivers/of/property.c +14 -3
index 751c11a28f33..dce451161c99 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,8 @@ 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 +1200,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 +1214,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 +1245,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_supplier, "post-init-supplier", NULL)
 DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
 DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
 
@@ -1349,6 +1355,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_supplier,
+		.fwlink_flags = FWLINK_FLAG_IGNORE,
+	},
 	{}
 };
 
@@ -1393,7 +1403,8 @@ 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.43.0.687.g38aa6559b0-goog