Received-SPF: pass (google.com: domain of linux-kernel+bounces-60372-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240210030549.4048795-1-saravanak@google.com> <20240210030549.4048795-4-saravanak@google.com>
In-Reply-To: <20240210030549.4048795-4-saravanak@google.com>
From: Rob Herring <robh+dt@kernel.org>
Date: Sat, 10 Feb 2024 11:26:13 +0000
Message-ID: <CAL_Jsq+tMK6myLtvD2EYEE2juev+wMvWoVMnYpT8JPy2eD9hKA@mail.gmail.com>
Subject: Re: [PATCH v1 3/4] dt-bindings: Add post-init-supplier property
To: Saravana Kannan <saravanak@google.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>, "Rafael J. Wysocki" <rafael@kernel.org>, 
	Ard Biesheuvel <ardb@kernel.org>, Frank Rowand <frowand.list@gmail.com>, 
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Daniel Scally <djrscally@gmail.com>, 
	Heikki Krogerus <heikki.krogerus@linux.intel.com>, 
	Sakari Ailus <sakari.ailus@linux.intel.com>, Len Brown <lenb@kernel.org>, kernel-team@android.com, 
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, 
	linux-efi@vger.kernel.org, linux-acpi@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Feb 10, 2024 at 3:06=E2=80=AFAM Saravana Kannan <saravanak@google.c=
om> wrote:
>
> The post-init-supplier property can be used to break a dependency cycle b=
y
> marking some supplier(s) as a post device initialization supplier(s). Thi=
s
> 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 review=
ing.

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 list=
s 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 t=
he
> +  supplier device can be initialized before the consumer device is initi=
alized.
> +
> +  However, that assumption cannot be made when there are cyclic dependec=
ies

typo

> +  between devices. Since each device is a supplier (directly or indirect=
ly) of
> +  the others in the cycle, there is no guaranteed safe order for initali=
zing

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 a=
lways
> +  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 wh=
en a
> +    specific functionality if requested post initialization).
> +
> +  If all the other -> are mandatory initialization dependencies, then tr=
ying to
> +  initialize the devices in a loop (or arbitrarily) will always eventual=
ly 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), t=
hen
> +  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 an=
d 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 functio=
nality
> +  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 wh=
ich
> +  device in a dependency cycle needs to be suspended/resumed first and t=
rying
> +  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, m=
akes
> +  the behavior more deterministic and maximizes the functionality that c=
an be
> +  provided without interruption.
> +
> +  This property is used to provide this additional information between d=
evices
> +  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 supplie=
r
> +    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 =3D "vendor,soc4-gcc", "vendor,soc1-gcc";
> +        reg =3D <0x1000 0x80>;> +        clocks =3D <&dispcc 0x1>
> +        #clock-cells =3D <1>;
> +        post-init-supplier =3D <&dispcc>;
> +    };
> +    dispcc: display-clock-controller@2000 {

clock-controller@2000

> +        compatible =3D "vendor,soc4-dispcc", "vendor,soc1-dispcc";
> +        reg =3D <0x2000 0x80>;
> +        clocks =3D <&gcc 0xdd>
> +        #clock-cells =3D <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
>