mbox series

[v3,0/4] Add post-init-providers binding to improve suspend/resume stability

Message ID 20240221233026.2915061-1-saravanak@google.com
Headers show
Series Add post-init-providers binding to improve suspend/resume stability | expand

Message

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

Patch 3 (the binding documentation) provides a lot more details and
examples.

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

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

Saravana Kannan (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-providers property
  of: property: fw_devlink: Add support for "post-init-providers"
    property

 .../bindings/post-init-providers.yaml         | 105 ++++++++++++++++++
 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, 142 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/post-init-providers.yaml

Comments

Rob Herring Feb. 22, 2024, 12:23 a.m. UTC | #1
On Wed, 21 Feb 2024 15:30:23 -0800, Saravana Kannan wrote:
> The post-init-providers property can be used to break a dependency cycle by
> marking some provider(s) as a post device initialization provider(s). This
> allows an OS 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-providers.yaml         | 105 ++++++++++++++++++
>  MAINTAINERS                                   |  13 ++-
>  2 files changed, 112 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/post-init-providers.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:
Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@1000: failed to match any schema with compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@1000: failed to match any schema with compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@2000: failed to match any schema with compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']
Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@2000: failed to match any schema with compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']

doc reference errors (make refcheckdocs):
Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/post-init-supplier.yaml
MAINTAINERS: Documentation/devicetree/bindings/post-init-supplier.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240221233026.2915061-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.
Saravana Kannan Feb. 22, 2024, 3:41 a.m. UTC | #2
On Wed, Feb 21, 2024 at 4:23 PM Rob Herring <robh@kernel.org> wrote:
>
>
> On Wed, 21 Feb 2024 15:30:23 -0800, Saravana Kannan wrote:
> > The post-init-providers property can be used to break a dependency cycle by
> > marking some provider(s) as a post device initialization provider(s). This
> > allows an OS 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-providers.yaml         | 105 ++++++++++++++++++
> >  MAINTAINERS                                   |  13 ++-
> >  2 files changed, 112 insertions(+), 6 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/post-init-providers.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:
> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@1000: failed to match any schema with compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@1000: failed to match any schema with compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@2000: failed to match any schema with compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']
> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@2000: failed to match any schema with compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']

I'm assuming it's okay to ignore these warnings about made up
compatible string names.

> doc reference errors (make refcheckdocs):
> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/post-init-supplier.yaml
> MAINTAINERS: Documentation/devicetree/bindings/post-init-supplier.yaml

Will fix this and send out v4. Ignore the v3 series please.

-Saravana

>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240221233026.2915061-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.
>
Krzysztof Kozlowski Feb. 22, 2024, 9:07 a.m. UTC | #3
On 22/02/2024 04:41, Saravana Kannan wrote:
> On Wed, Feb 21, 2024 at 4:23 PM Rob Herring <robh@kernel.org> wrote:
>>
>>
>> On Wed, 21 Feb 2024 15:30:23 -0800, Saravana Kannan wrote:
>>> The post-init-providers property can be used to break a dependency cycle by
>>> marking some provider(s) as a post device initialization provider(s). This
>>> allows an OS 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-providers.yaml         | 105 ++++++++++++++++++
>>>  MAINTAINERS                                   |  13 ++-
>>>  2 files changed, 112 insertions(+), 6 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/post-init-providers.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:
>> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@1000: failed to match any schema with compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
>> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@1000: failed to match any schema with compatible: ['vendor,soc4-gcc', 'vendor,soc1-gcc']
>> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@2000: failed to match any schema with compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']
>> Documentation/devicetree/bindings/post-init-providers.example.dtb: /example-0/clock-controller@2000: failed to match any schema with compatible: ['vendor,soc4-dispcc', 'vendor,soc1-dispcc']
> 
> I'm assuming it's okay to ignore these warnings about made up
> compatible string names.

No, unfortunately not. I think you need to come with a real example or
just drop compatibles.

BTW, I still don't see any users of this binding.

Best regards,
Krzysztof
Andy Shevchenko Feb. 22, 2024, 1:34 p.m. UTC | #4
On Wed, Feb 21, 2024 at 03:30:20PM -0800, Saravana Kannan wrote:
> This patch series adds a "post-init-providers" device tree binding that
> can be used to break dependency cycles in device tree and enforce a more
> determinstic probe/suspend/resume order. This will also improve the
> stability of global async probing and async suspend/resume and allow us
> to enable them more easily. Yet another step away from playing initcall
> chicken with probing and step towards fully async probing and
> suspend/resume.

Do you know what is the state of affairs in ACPI? Is there any (similar)
issue even possible?
Saravana Kannan Feb. 23, 2024, 12:02 a.m. UTC | #5
On Thu, Feb 22, 2024 at 5:34 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Feb 21, 2024 at 03:30:20PM -0800, Saravana Kannan wrote:
> > This patch series adds a "post-init-providers" device tree binding that
> > can be used to break dependency cycles in device tree and enforce a more
> > determinstic probe/suspend/resume order. This will also improve the
> > stability of global async probing and async suspend/resume and allow us
> > to enable them more easily. Yet another step away from playing initcall
> > chicken with probing and step towards fully async probing and
> > suspend/resume.
>
> Do you know what is the state of affairs in ACPI? Is there any (similar)
> issue even possible?

I'm not very familiar with ACPI, but I wouldn't be surprised if ACPI
devices have cyclic dependencies. But then ACPI on a PC doesn't
typically have as many devices/drivers and ACPI might be hiding the
dependencies from the kernel. So maybe the possibility of a cycle
visible to the kernel might be low.

I would really like to see fw_devlink extended to ACPI (it's written
in a way to make that possible), but don't have enough knowledge to do
it.

-Saravana
Rafael J. Wysocki Feb. 29, 2024, 6:01 p.m. UTC | #6
On Fri, Feb 23, 2024 at 1:03 AM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Feb 22, 2024 at 5:34 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Wed, Feb 21, 2024 at 03:30:20PM -0800, Saravana Kannan wrote:
> > > This patch series adds a "post-init-providers" device tree binding that
> > > can be used to break dependency cycles in device tree and enforce a more
> > > determinstic probe/suspend/resume order. This will also improve the
> > > stability of global async probing and async suspend/resume and allow us
> > > to enable them more easily. Yet another step away from playing initcall
> > > chicken with probing and step towards fully async probing and
> > > suspend/resume.
> >
> > Do you know what is the state of affairs in ACPI? Is there any (similar)
> > issue even possible?
>
> I'm not very familiar with ACPI, but I wouldn't be surprised if ACPI
> devices have cyclic dependencies. But then ACPI on a PC doesn't
> typically have as many devices/drivers and ACPI might be hiding the
> dependencies from the kernel. So maybe the possibility of a cycle
> visible to the kernel might be low.
>
> I would really like to see fw_devlink extended to ACPI (it's written
> in a way to make that possible), but don't have enough knowledge to do
> it.

This might happen one day, for example in the _DEP handling context
(for now it is very limited, but I'm not actually sure how much more
capable it needs to be).

I don't think that ACPI will ever need device links between parents
and children, though.

On a related note, RISC-V people seem to want to use it on ACPI
systems for interrupt controller dependency tracking.