From: Frank Rowand <frowand.list@gmail.com>
To: Saravana Kannan <saravanak@google.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
David Collins <collinsd@codeaurora.org>,
Android Kernel Team <kernel-team@android.com>,
Linux Doc Mailing List <linux-doc@vger.kernel.org>
Subject: Re: [PATCH v7 3/7] of/platform: Add functional dependency link from DT bindings
Date: Mon, 19 Aug 2019 10:16:25 -0700 [thread overview]
Message-ID: <19c99a6e-51c3-68d7-d1d6-640aae754c14@gmail.com> (raw)
In-Reply-To: <CAGETcx-dVnLCRA+1CX47gtZgtwTcrN5KefpjMzh9OJB-BEnqyg@mail.gmail.com>
On 8/15/19 6:50 PM, Saravana Kannan wrote:
> On Wed, Aug 7, 2019 at 7:06 PM Frank Rowand <frowand.list@gmail.com> wrote:
>>
>> On 7/23/19 5:10 PM, Saravana Kannan wrote:
>>> Add device-links after the devices are created (but before they are
>>> probed) by looking at common DT bindings like clocks and
>>> interconnects.
< very big snip (lots of comments that deserve answers) >
>>
>> /**
>> * of_link_property - TODO:
>> * dev:
>> * con_np:
>> * prop:
>> *
>> * TODO...
>> *
>> * Any failed attempt to create a link will NOT result in an immediate return.
>> * of_link_property() must create all possible links even when one of more
>> * attempts to create a link fail.
>>
>> Why? isn't one failure enough to prevent probing this device?
>> Continuing to scan just results in extra work... which will be
>> repeated every time device_link_check_waiting_consumers() is called
>
> Context:
> As I said in the cover letter, avoiding unnecessary probes is just one
> of the reasons for this patch. The other (arguably more important)
Agree that it is more important.
> reason for this patch is to make sure suppliers know that they have
> consumers that are yet to be probed. That way, suppliers can leave
> their resource on AND in the right state if they were left on by the
> bootloader. For example, if a clock was left on and at 200 MHz, the
> clock provider needs to keep that clock ON and at 200 MHz till all the
> consumers are probed.
>
> Answer: Let's say a consumer device Z has suppliers A, B and C. If the
> linking fails at A and you return immediately, then B and C could
> probe and then figure that they have no more consumers (they don't see
> a link to Z) and turn off their resources. And Z could fail
> catastrophically.
Then I think that this approach is fatally flawed in the current implementation.
A device can be added by a module that is loaded. In that case the device
was not present at late boot when the suppliers may turn off their resources.
(I am assuming the details since I have not reviewed the patches later in
the series that implement this part.)
Am I missing something?
If I am wrong, then I'll have more comments for your review replies for
patches 2 and 3.
>
< another snip >
> Thanks,
> Saravana
>
-Frank
next prev parent reply other threads:[~2019-08-19 17:16 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-24 0:10 [PATCH v7 0/7] Solve postboot supplier cleanup and optimize probe ordering Saravana Kannan
2019-07-24 0:10 ` [PATCH v7 1/7] driver core: Add support for linking devices during device addition Saravana Kannan
2019-08-08 2:04 ` Frank Rowand
2019-08-16 1:50 ` Saravana Kannan
2019-08-19 3:38 ` Frank Rowand
2019-08-20 0:00 ` Saravana Kannan
2019-08-20 4:25 ` Frank Rowand
2019-08-20 22:10 ` Saravana Kannan
2019-08-21 1:06 ` Frank Rowand
2019-08-21 1:56 ` Greg Kroah-Hartman
2019-08-21 2:04 ` Saravana Kannan
[not found] ` <CAGETcx8eVfJ0mn08E6pTDytjVE+RFcj_gOt_enide4E6G1NAWw@mail.gmail.com>
2019-08-21 4:24 ` Frank Rowand
2019-08-21 2:22 ` Saravana Kannan
2019-08-21 15:36 ` Frank Rowand
2019-07-24 0:10 ` [PATCH v7 2/7] driver core: Add edit_links() callback for drivers Saravana Kannan
2019-08-08 2:05 ` Frank Rowand
2019-08-16 1:50 ` Saravana Kannan
2019-07-24 0:10 ` [PATCH v7 3/7] of/platform: Add functional dependency link from DT bindings Saravana Kannan
2019-08-08 2:06 ` Frank Rowand
2019-08-16 1:50 ` Saravana Kannan
2019-08-19 17:16 ` Frank Rowand [this message]
2019-08-19 20:49 ` Saravana Kannan
2019-08-19 21:30 ` Frank Rowand
2019-08-20 0:09 ` Saravana Kannan
2019-08-20 4:26 ` Frank Rowand
2019-08-20 22:09 ` Saravana Kannan
2019-08-21 16:39 ` Frank Rowand
2019-07-24 0:10 ` [PATCH v7 4/7] driver core: Add sync_state driver/bus callback Saravana Kannan
2019-07-24 0:10 ` [PATCH v7 5/7] of/platform: Pause/resume sync state during init and of_platform_populate() Saravana Kannan
2019-07-24 0:10 ` [PATCH v7 6/7] of/platform: Create device links for all child-supplier depencencies Saravana Kannan
2019-07-24 0:11 ` [PATCH v7 7/7] of/platform: Don't create device links for default busses Saravana Kannan
2019-07-25 13:42 ` [PATCH v7 0/7] Solve postboot supplier cleanup and optimize probe ordering Greg Kroah-Hartman
2019-07-25 21:04 ` Frank Rowand
2019-07-26 14:32 ` Greg Kroah-Hartman
2019-07-31 2:22 ` Frank Rowand
2019-08-08 2:02 ` Frank Rowand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=19c99a6e-51c3-68d7-d1d6-640aae754c14@gmail.com \
--to=frowand.list@gmail.com \
--cc=collinsd@codeaurora.org \
--cc=corbet@lwn.net \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=kernel-team@android.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=rafael@kernel.org \
--cc=robh+dt@kernel.org \
--cc=saravanak@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).