On 8/15/19 6:50 PM, Saravana Kannan wrote:

On Wed, Aug 7, 2019 at 7:06 PM Frank Rowand [off-list ref] wrote:

quoted

On 7/23/19 5:10 PM, Saravana Kannan wrote:

quoted

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) >

quoted

/**
 * 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