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: <CAGETcx_pSnC_2D7ufLRyfE3b8uRc814XEf8zu+SpNtT7_Z8NLg@mail.gmail.com>
 <CAL_JsqKWcGSzCF_ZyEo6bbuayoYks51A-JAMp_oLR1RyTUzNUA@mail.gmail.com>
 <CAGETcx_RL4hHHA2MFTVyV1ivgghaBZePROXpnC-UUJ7tcH4kSQ@mail.gmail.com>
 <CAL_JsqJB+41Sjxi-udYzw8sAq0myrcnxjSyzrxeEsoctZX6pbw@mail.gmail.com>
 <CAGETcx9T_3GKgAj=3jANb=JAa5b5hP+r4CLVm9a2LYf2CQiH9Q@mail.gmail.com> <CAGETcx-_Mewt-ZND1WkjtdvLZ9iXTZBEdSPU6kO3G_L28mCHdQ@mail.gmail.com>
In-Reply-To: <CAGETcx-_Mewt-ZND1WkjtdvLZ9iXTZBEdSPU6kO3G_L28mCHdQ@mail.gmail.com>
From:   Saravana Kannan <saravanak@google.com>
Date:   Wed, 19 Feb 2020 23:03:01 -0800
Message-ID: <CAGETcx_2vdjSWc3BBN-N2WrtJP90ZnH-2vE=2iVuHuaE1YmMWQ@mail.gmail.com>
Subject: Re: Adding depends-on DT binding to break cyclic dependencies
To:     Rob Herring <robh+dt@kernel.org>
Cc:     Mark Rutland <mark.rutland@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Frank Rowand <frowand.list@gmail.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, LKML <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, Aug 30, 2019 at 10:01 PM Saravana Kannan <saravanak@google.com> wrote:
>
> So we can take our time trying to solve this in a generic fashion (new
> DT property/binding, edit_links(), letting devices probe, etc). In the
> meantime, maybe we'll run into more cycle issues that'll give us a
> better idea of which solution would be better as a generic solution.

Mainly reviving an old thread to say this to Rob and Frank: Thanks for
pushing back on "depends-on" and asking me to use the existing
bindings instead. Saved a whole bunch of time when I actually tried to
use of_devlink. Didn't have to add stupid "depends-on" for all the
existing dependencies.

But then I've also been meaning to send an RFC for this following, so
rolling it into the same email.

Thanks for also pushing back on all the earlier "meh" solutions for
solving the cyclic dependency issue. I think I have a pretty good
proposal now.

While trying to solve the "dependencies of child nodes need to be
proxied by the parents till the child devices are created" problem, I
ended up having to add a "SYNC_STATE_ONLY" device link flag that
treats those dependencies as "optional for probing". It also allows
cycles (because it only affects sync state behavior). Also,
dependencies of child nodes (whether they are actually devices or not)
are always treated as "optional for probe" dependencies by of_devlink.

So, how does this affect cyclic dependencies? Obviously, when two
devices have cyclic dependencies, they don't have cyclic probe
dependencies. Then they'd never probe even if of_devlink is not in the
picture. At least one of the dependencies is only relevant for some
"post-probe" functionality.

So let's take a simple example:

dev_a: device-a@xxxx {
   compatible = "fizzbuzz";
}

dev_b: device-b@yyyy {
   compatible = "fizzbazz";
   supplier-property-1 = <&dev_a>;
   supplier-property-2 = <&dev_c>;
}

dev_c: device-c@zzzz {
   compatible = "fizzfizz";
   supplier-property-1 = <&dev_a>;
   supplier-property-3 = <&dev_b>;
}

Let's say dev_c only doesn't depend on dev_b for probing but needs it
only for some functionality "foo" (Eg: thermal management, secure
video playback, etc. Nothing OS specific). If the DT nodes are written
as above, then there'll be a cycle with of_devlink and neither dev_b
or dev_c will probe.

However, if we can write dev_c DT as:

dev_c: device-c@zzzz {
   compatible = "fizzfizz";
   supplier-property-1 = <&dev_a>;
   foo {
      /* No compatible property */
      supplier-property-2 = <&dev_b>;
   }
}

Then of_devlink will automatically treat dev_b as an optional
requirement for dev_c. I think this is also nice from a DT perspective
because it gives a clear representation of the dependency without
really breaking or adding any DT rules. If you need some DT bindings
only for a subset functionality, just list them under a child node
with a meaningful name for that functionality.

For this to work, the framework that supports "supplier-property-2"
will have to add APIs to "get" the supplier by passing a DT node
(instead of just struct device), but:
1) That is already supported by quite a few frameworks.
2) That shouldn't be too hard to add where necessary.

And the driver needs to handle the child node explicitly (kinda obvious).

Thoughts? Like the proposal?

-Saravana