Re: [PATCH v1] schemas: Add schema for post-init-providers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hi Rob,

On Thu, 7 Mar 2024 at 11:23, Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Wed, Mar 6, 2024 at 1:17 PM Simon Glass <sjg@xxxxxxxxxxxx> wrote:
> >
> > Hi Saravana,
> >
> > On Wed, 6 Mar 2024 at 13:30, Saravana Kannan <saravanak@xxxxxxxxxx> 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
> >
> > Please can you add hyphens to avoid confusion? I believe this should
> > be 'post-device-initialization' throughout. There is no 'post' device.
>
> There is no 'post-device-initialization' thing either. Shrug.

'post-device-initialization' is intended to be an adjective and the
hyphens help to make that clear.

>
> > > 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@xxxxxxxxxx>
> > > ---
> > >  dtschema/schemas/post-init-providers.yaml | 105 ++++++++++++++++++++++
> > >  1 file changed, 105 insertions(+)
> > >  create mode 100644 dtschema/schemas/post-init-providers.yaml
> > >
> > > diff --git a/dtschema/schemas/post-init-providers.yaml b/dtschema/schemas/post-init-providers.yaml
> > > new file mode 100644
> > > index 0000000..92eb9a0
> > > --- /dev/null
> > > +++ b/dtschema/schemas/post-init-providers.yaml
> > > @@ -0,0 +1,105 @@
> > > +# 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-providers.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Post device initialization providers
> > > +
> > > +maintainers:
> > > +  - Saravana Kannan <saravanak@xxxxxxxxxx>
> > > +
> > > +description: |
> > > +  This property is used to indicate that the device(s) pointed to by the
> >
> > Should this be 'device node' instead of 'device'?
> >
> > > +  property are not needed for the initialization of the device that lists this
> > > +  property. This property does not make a device (that's previously not a
> > > +  provider) into a provider. It simply downgrades an existing provider to a
> > > +  post device initialization provider.
> > > +
> > > +  A device can list its providers in devicetree using one or more of the
> > > +  standard devicetree bindings. By default, it is assumed that the provider
> > > +  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 provider (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).
> >
> > How about 'is requested after initialization of Z'
> >
> > > +
> > > +  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 provider 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.
> >
> > This seems strange to me. It seems like something that could be
> > implemented. That said, I understand that such an implementation could
> > become painful.
>
> It has been implemented. And cycle detection has been implemented. And
> yet we are still here needing something more.

Sure, I understand that.

>
>
> > > +  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 provider(s) is not needed for initializing the
> > > +  device that lists this property.
> > > +
> > > +  In the example above, Z would list X as a post-init-providers and the
> > > +  initialization dependency would become X -> Y -> Z -/-> X. So the best order
> > > +  to initialize them becomes clear: Z, Y and then X.
> > > +
> > > +select: true
> > > +
> > > +properties:
> > > +  post-init-providers:
> >
> > What is 'init'? It seems to mean when Linux probes the device. In
> > U-Boot, devices all are bound at the start, but only probed (lazilly)
> > when used. For some device types there is then an 'init' step which
> > actually starts using the device, before which no hardware is
> > accessed. So the use of the word 'init' seems a bit vague to me.
> >
> > In general this binding seems liable to be specific to the OS being
> > used. It also seems to be a hint, rather than something that must be
> > parsed and used. However I suppose adding the suffix '-hint' would
> > just confuse things.
> >
> > How about 'secondary-providers' or 'minor-providers' or
> > 'delayed-providers' or 'deferred-providers'?
> >
> > Separately/alternatively I wonder if the target node is always going
> > to be inited later, for any device that uses it. If so, perhaps a
> > property in the target node would be better, something like:
> >
> > dispcc: clock-controller@2000 {
> >    ...
> >    minor-provider;
>
> That doesn't work if a node is 2 or more providers and only some of
> them are optional/post-init/deferred.

That's right. It would be nice though, if the situation you describe
does not actually happen in practice currently.

Regards,
Simon





[Index of Archives]     [Device Tree]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Photos]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]

  Powered by Linux