[PATCH v1 0/4] Add post-init-supplier binding to improve suspend/resume stability

[PATCH v1 0/4] Add post-init-supplier binding to improve suspend/resume stability

[Saravana Kannan]

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.

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          | 99 +++++++++++++++++++
 MAINTAINERS                                   |  3 +-
 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, 131 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml

-- 
2.43.0.687.g38aa6559b0-goog

[PATCH v1 1/4] driver core: Adds flags param to fwnode_link_add()

[Saravana Kannan]

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

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 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(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 14d46af40f9a..33055001e08e 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -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;
 }
diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
index 456d0e5eaf78..cc807ed35aed 100644
--- a/drivers/firmware/efi/sysfb_efi.c
+++ b/drivers/firmware/efi/sysfb_efi.c
@@ -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;
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 39a3ee1dfb58..751c11a28f33 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -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);
 }
 
 /**
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 2a72f55d26eb..c964749953e3 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -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);
 
-- 
2.43.0.687.g38aa6559b0-goog

[PATCH v1 2/4] driver core: Add FWLINK_FLAG_IGNORE to completely ignore a fwnode link

[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 <saravanak@google.com>
---
 drivers/base/core.c    | 9 ++++++++-
 include/linux/fwnode.h | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 33055001e08e..bd762d90dac0 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -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
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index c964749953e3..21699eee9641 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -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;
-- 
2.43.0.687.g38aa6559b0-goog

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

[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 the kernel to do a better job at ordering initialization and
suspend/resume of the devices in a dependency cycle.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 .../bindings/post-init-supplier.yaml          | 99 +++++++++++++++++++
 MAINTAINERS                                   |  3 +-
 2 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml

diff --git a/Documentation/devicetree/bindings/post-init-supplier.yaml b/Documentation/devicetree/bindings/post-init-supplier.yaml
new file mode 100644
index 000000000000..cf9071ecd06e
--- /dev/null
+++ b/Documentation/devicetree/bindings/post-init-supplier.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2018 Linaro Ltd.
+%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 <saravanak@google.com>
+
+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.
+
+  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 dependecies
+  between devices. Since each device is a supplier (directly or indirectly) of
+  the others in the cycle, there is no guaranteed safe order for initalizing
+  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 if 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.
+
+properties:
+  # A dictionary of DT properties for this binding schema
+  post-init-supplier:
+    # One or more suppliers can be marked as post initialization supplier
+    minItems: 1
+    description:
+      List of phandles to suppliers that are not needed for initializing or
+      resuming this device.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+examples:
+  - |
+    gcc: general-clock-controller@1000 {
+        compatible = "vendor,soc4-gcc", "vendor,soc1-gcc";
+        reg = <0x1000 0x80>;
+        clocks = <&dispcc 0x1>
+        #clock-cells = <1>;
+        post-init-supplier = <&dispcc>;
+    };
+    dispcc: display-clock-controller@2000 {
+        compatible = "vendor,soc4-dispcc", "vendor,soc1-dispcc";
+        reg = <0x2000 0x80>;
+        clocks = <&gcc 0xdd>
+        #clock-cells = <1>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3dfe7ea25320..40fd498543a5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6055,10 +6055,11 @@ S:	Maintained
 F:	drivers/base/devcoredump.c
 F:	include/linux/devcoredump.h
 
-DEVICE DEPENDENCY HELPER SCRIPT
+FIRMWARE DEVICE LINK (fw_devlink)
 M:	Saravana Kannan <saravanak@google.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/post-init-supplier.yaml
 F:	scripts/dev-needs.sh
 
 DEVICE DIRECT ACCESS (DAX)
-- 
2.43.0.687.g38aa6559b0-goog

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

[Rob Herring]

On Sat, Feb 10, 2024 at 3:06 AM Saravana Kannan <saravanak@google.com> wrote:
>
> 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 the kernel to do a better job at ordering initialization and

s/the kernel/an OS/

> suspend/resume of the devices in a dependency cycle.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  .../bindings/post-init-supplier.yaml          | 99 +++++++++++++++++++

This should probably go into dtschema instead, but fine here now for reviewing.

We have to consider if this property should be automatically allowed
on any node or nodes with specific suppliers, or if it should be
explicit for users. The former needs tool support. I'm leaning towards
the latter as I want to know when this is needed.

>  MAINTAINERS                                   |  3 +-
>  2 files changed, 101 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
>
> diff --git a/Documentation/devicetree/bindings/post-init-supplier.yaml b/Documentation/devicetree/bindings/post-init-supplier.yaml
> new file mode 100644
> index 000000000000..cf9071ecd06e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/post-init-supplier.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2018 Linaro Ltd.

?

> +%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 <saravanak@google.com>
> +
> +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.
> +
> +  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 dependecies

typo

> +  between devices. Since each device is a supplier (directly or indirectly) of
> +  the others in the cycle, there is no guaranteed safe order for initalizing

typo

> +  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 if 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

Otherwise, this is never applied.

> +properties:
> +  # A dictionary of DT properties for this binding schema

Drop

> +  post-init-supplier:
> +    # One or more suppliers can be marked as post initialization supplier
> +    minItems: 1

That's the default.

> +    description:
> +      List of phandles to suppliers that are not needed for initializing or
> +      resuming this device.
> +    $ref: /schemas/types.yaml#/definitions/phandle

Should be phandle-array plus:

items:
  maxItems: 1

(as each entry is a single phandle)

> +
> +examples:
> +  - |
> +    gcc: general-clock-controller@1000 {

clock-controller@1000

> +        compatible = "vendor,soc4-gcc", "vendor,soc1-gcc";
> +        reg = <0x1000 0x80>;> +        clocks = <&dispcc 0x1>
> +        #clock-cells = <1>;
> +        post-init-supplier = <&dispcc>;
> +    };
> +    dispcc: display-clock-controller@2000 {

clock-controller@2000

> +        compatible = "vendor,soc4-dispcc", "vendor,soc1-dispcc";
> +        reg = <0x2000 0x80>;
> +        clocks = <&gcc 0xdd>
> +        #clock-cells = <1>;
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3dfe7ea25320..40fd498543a5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6055,10 +6055,11 @@ S:      Maintained
>  F:     drivers/base/devcoredump.c
>  F:     include/linux/devcoredump.h
>
> -DEVICE DEPENDENCY HELPER SCRIPT
> +FIRMWARE DEVICE LINK (fw_devlink)
>  M:     Saravana Kannan <saravanak@google.com>
>  L:     linux-kernel@vger.kernel.org
>  S:     Maintained
> +F:     Documentation/devicetree/bindings/post-init-supplier.yaml
>  F:     scripts/dev-needs.sh
>
>  DEVICE DIRECT ACCESS (DAX)
> --
> 2.43.0.687.g38aa6559b0-goog
>

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

[Saravana Kannan]

On Sat, Feb 10, 2024 at 3:26 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Sat, Feb 10, 2024 at 3:06 AM Saravana Kannan <saravanak@google.com> wrote:
> >
> > 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 the kernel to do a better job at ordering initialization and
>
> s/the kernel/an OS/
>
> > suspend/resume of the devices in a dependency cycle.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> >  .../bindings/post-init-supplier.yaml          | 99 +++++++++++++++++++
>
> This should probably go into dtschema instead, but fine here now for reviewing.

How do I submit it for dtschema?

> We have to consider if this property should be automatically allowed
> on any node or nodes with specific suppliers, or if it should be
> explicit for users. The former needs tool support. I'm leaning towards
> the latter as I want to know when this is needed.

I think I understand your question. This property can be relevant for
any device -- it really depends on the device's consumers and
suppliers and if they form a cycle. Ideally, we can check if a device
is in a cycle and then allow it only for those, but that's going to be
too complicated I think. So, I think this should be allowed for any
device.

Since this property is defined as a "post-init-supplier" and only
applies to an existing direct supplier, there's not a lot of room for
misuse. Actually if you can add a check "are all the phandles listed
in this property also present in one of the other properties in this
device node", that should be sufficient.

I'll also edit the documentation to make it clear it's meaningful only
for an existing supplier.

>
> >  MAINTAINERS                                   |  3 +-
> >  2 files changed, 101 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/devicetree/bindings/post-init-supplier.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/post-init-supplier.yaml b/Documentation/devicetree/bindings/post-init-supplier.yaml
> > new file mode 100644
> > index 000000000000..cf9071ecd06e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/post-init-supplier.yaml
> > @@ -0,0 +1,99 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright 2018 Linaro Ltd.
>
> ?

Oops... didn't edit out all the lines in the example.

I'll address all your other comments in the next patch.

Btw, this is the command I used to validate this file (before I sent
v1), but it didn't spit out anything meaningful that I could
understand:

make dt_binding_check DT_CHECKER_FLAGS=-m
DT_SCHEMA_FILES=Documentation/devicetree/bindings/trivial-devices.yaml

Is that the right way to run it? I'm using yamllint 1.32. So it's not
terribly old either (can't install anything newer).

-Saravana

>
> > +%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 <saravanak@google.com>
> > +
> > +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.
> > +
> > +  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 dependecies
>
> typo
>
> > +  between devices. Since each device is a supplier (directly or indirectly) of
> > +  the others in the cycle, there is no guaranteed safe order for initalizing
>
> typo
>
> > +  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 if 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
>
> Otherwise, this is never applied.
>
> > +properties:
> > +  # A dictionary of DT properties for this binding schema
>
> Drop
>
> > +  post-init-supplier:
> > +    # One or more suppliers can be marked as post initialization supplier
> > +    minItems: 1
>
> That's the default.
>
> > +    description:
> > +      List of phandles to suppliers that are not needed for initializing or
> > +      resuming this device.
> > +    $ref: /schemas/types.yaml#/definitions/phandle
>
> Should be phandle-array plus:
>
> items:
>   maxItems: 1
>
> (as each entry is a single phandle)
>
> > +
> > +examples:
> > +  - |
> > +    gcc: general-clock-controller@1000 {
>
> clock-controller@1000
>
> > +        compatible = "vendor,soc4-gcc", "vendor,soc1-gcc";
> > +        reg = <0x1000 0x80>;> +        clocks = <&dispcc 0x1>
> > +        #clock-cells = <1>;
> > +        post-init-supplier = <&dispcc>;
> > +    };
> > +    dispcc: display-clock-controller@2000 {
>
> clock-controller@2000
>
> > +        compatible = "vendor,soc4-dispcc", "vendor,soc1-dispcc";
> > +        reg = <0x2000 0x80>;
> > +        clocks = <&gcc 0xdd>
> > +        #clock-cells = <1>;
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3dfe7ea25320..40fd498543a5 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6055,10 +6055,11 @@ S:      Maintained
> >  F:     drivers/base/devcoredump.c
> >  F:     include/linux/devcoredump.h
> >
> > -DEVICE DEPENDENCY HELPER SCRIPT
> > +FIRMWARE DEVICE LINK (fw_devlink)
> >  M:     Saravana Kannan <saravanak@google.com>
> >  L:     linux-kernel@vger.kernel.org
> >  S:     Maintained
> > +F:     Documentation/devicetree/bindings/post-init-supplier.yaml
> >  F:     scripts/dev-needs.sh
> >
> >  DEVICE DIRECT ACCESS (DAX)
> > --
> > 2.43.0.687.g38aa6559b0-goog
> >

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

[Rob Herring]

On Fri, 09 Feb 2024 19:05:46 -0800, Saravana Kannan wrote:
> 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 the kernel to do a better job at ordering initialization and
> suspend/resume of the devices in a dependency cycle.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  .../bindings/post-init-supplier.yaml          | 99 +++++++++++++++++++
>  MAINTAINERS                                   |  3 +-
>  2 files changed, 101 insertions(+), 1 deletion(-)
>  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:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/post-init-supplier.yaml: properties:post-init-supplier:minItems: False schema does not allow 1
	hint: Scalar properties should not have array keywords
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/post-init-supplier.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#
Error: Documentation/devicetree/bindings/post-init-supplier.example.dts:22.13-14 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/post-init-supplier.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
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/20240210030549.4048795-4-saravanak@google.com

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.

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

[Krzysztof Kozlowski]

On 10/02/2024 04:05, Saravana Kannan wrote:
> 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 the kernel to do a better job at ordering initialization and
> suspend/resume of the devices in a dependency cycle.

...

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3dfe7ea25320..40fd498543a5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6055,10 +6055,11 @@ S:	Maintained
>  F:	drivers/base/devcoredump.c
>  F:	include/linux/devcoredump.h
>  
> -DEVICE DEPENDENCY HELPER SCRIPT
> +FIRMWARE DEVICE LINK (fw_devlink)

This breaks ordering of MAINTAINERS...

Best regards,
Krzysztof

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

[Saravana Kannan]

On Sun, Feb 11, 2024 at 6:52 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 10/02/2024 04:05, Saravana Kannan wrote:
> > 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 the kernel to do a better job at ordering initialization and
> > suspend/resume of the devices in a dependency cycle.
>
> ...
>
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3dfe7ea25320..40fd498543a5 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6055,10 +6055,11 @@ S:    Maintained
> >  F:   drivers/base/devcoredump.c
> >  F:   include/linux/devcoredump.h
> >
> > -DEVICE DEPENDENCY HELPER SCRIPT
> > +FIRMWARE DEVICE LINK (fw_devlink)
>
> This breaks ordering of MAINTAINERS...

Oops. Thanks. Will fix it.

-Saravana

[PATCH v1 4/4] of: property: fw_devlink: Add support for "post-init-supplier" property

[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 <saravanak@google.com>
---
 drivers/of/property.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 751c11a28f33..dce451161c99 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -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);
 		}
-- 
2.43.0.687.g38aa6559b0-goog