Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190524010117.225219-1-saravanak@google.com> <20190524010117.225219-4-saravanak@google.com>
 <20190524150135.GD15566@lakrids.cambridge.arm.com>
In-Reply-To: <20190524150135.GD15566@lakrids.cambridge.arm.com>
From:   Saravana Kannan <saravanak@google.com>
Date:   Fri, 24 May 2019 15:09:28 -0700
Message-ID: <CAGETcx-DgM2M2iA7x=2nsGu4LVcFAN+o=Wgg+tP+fOUv+D-vLQ@mail.gmail.com>
Subject: Re: [PATCH v1 3/5] dt-bindings: Add depends-on property
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        Android Kernel Team <kernel-team@android.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, May 24, 2019 at 8:01 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, May 23, 2019 at 06:01:14PM -0700, Saravana Kannan wrote:
> > The depends-on property is used to list the mandatory functional
> > dependencies of a consumer device on zero or more supplier devices.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> >  .../devicetree/bindings/depends-on.txt        | 26 +++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/depends-on.txt
> >
> > diff --git a/Documentation/devicetree/bindings/depends-on.txt b/Documentation/devicetree/bindings/depends-on.txt
> > new file mode 100644
> > index 000000000000..1cbddd11cf17
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/depends-on.txt
> > @@ -0,0 +1,26 @@
> > +Functional dependency linking
> > +=============================
> > +
> > +Apart from parent-child relationships, devices (consumers) often have
> > +functional dependencies on other devices (suppliers). Common examples of
> > +suppliers are clock, regulators, pinctrl, etc. However not all of them are
> > +dependencies with well defined devicetree bindings.
>
> For clocks, regualtors, and pinctrl, that dependency is already implicit
> in the consumer node's properties. We should be able to derive those
> dependencies within the kernel.
>
> Can you give an example of where a dependency is not implicit in an
> existing binding?

I already gave the IRQ example. But if that's not good I replied to
other emails with a clock providers example that's based on real
hardware.

> > Also, not all functional
> > +dependencies are mandatory as the device might be able to operate in a limited
> > +mode without some of the dependencies.
>
> Whether something is a mandatory dependency will depend on the driver
> and dynamic runtime details more than it will depend on the hardware.
>
> For example, assume I have an IP block that functions as both a
> clocksource and a watchdog that can reset the system, with those two
> functions derived from separate input clocks.
>
> I could use the device as just a clocksource, or as just a watchdog, and
> neither feature in isolation is necessarily mandatory for the device to
> be somewhat useful to the OS.

Aren't you talking about the supplier here? I don't see any issues so far.

You could have consumers that try to use both the features of this IP
you mention (although I'm hard pressed to see how a watchdog is a
mandatory dependency but let's assume it is). So lets say consumer A
adds depends-on to your IP block for the clock and consumer B adds
depends-on your IP block for the watchdog.

If your driver is incomplete and provides only the watchdog feature,
then consumer A and B will attempt to probe once your device probes,
but consumer A will never probe successfully because it'll keep
getting -EPROBE_DEFER or an error. Your driver will never get a
sync_state() callback and that's fine because you don't know how to
use or turn off the clock source.

If the situation is reversed and your driver provides only the clock
feature, then consumer B will never probe because it'll never be able
to "get()" or whatever it tries to do with the watchdog feature. And
your driver will never get a sync_state() callback and you can never
turn off your clock. But that's the best you'll get till you send a
patch to add the watchdog support to your driver :)

> We need better ways of dynamically providing and managing this
> information. For example, if a driver could register its dynamic
> dependencies at probe (or some new pre-probe callback), we'd be able to
> notify it immediately when its dependencies are available.

We can't depend on the drivers to notify the core framework because
that doesn't work on a system with modules. The example I gave in the
commit text for the last patch is a good one. If your driver is
supplying power to the screen backlight and the backlight module is
never loaded, you can never turn off the supply to the backlight ever.
That use case has to work and it won't work if you depend on the
backlight driver to tell you what it depends on.

-Saravana