Re: [PATCH] [RFC] OF: probe order dependency aware of_platform_populate

Hi Geert,

On Mar 30, 2015, at 16:27 , Geert Uytterhoeven [off-list ref] wrote:

Hi Pantelis,

On Tue, Mar 24, 2015 at 6:56 PM, Pantelis Antoniou
[off-list ref] wrote:

quoted

quoted

On Mar 24, 2015, at 07:50 , Geert Uytterhoeven [off-list ref] wrote:
On Fri, Mar 20, 2015 at 12:39 PM, Pantelis Antoniou
[off-list ref] wrote:

quoted

quoted

On Mar 19, 2015, at 21:18 , Grant Likely [off-list ref] wrote:
On Tue, 16 Dec 2014 14:11:31 +0200
, Pantelis Antoniou [off-list ref]
wrote:

quoted

A nice side-effect of the changes in DTC for supporting overlays
is that it is now possible to do dependency tracking of platform
devices automatically.

This patch implements dependency aware probe order for users
of of_platform_populate.

There are no changes in the syntax of the DTS bindings, the
dependency is generated automatically by the use of phandle
references.

Do you have measurements showing improvement? Conceptually, I don't have
a problem with having a small scale solution like this, but I want proof
that it actively makes things better, and is worth the extra complexity.
It's not an easy block of code to understand.

I will be the first to admit that the code it’s a bit hard to follow, but
that’s the nature of trees and recursion.

FWIW I’ve been booting with this applied for a month with no adverse effects,
besides the fact that there dependency cycles which I would file as a bug.

IIUC, this would fix the issue I worked around in "ARM: shmobile: r8a73a4:
Move pfc node to work around probe ordering bug"?
https://git.kernel.org/cgit/linux/kernel/git/horms/renesas.git/commit/?h=r8a73a4-ccf-and-multiplatform-for-v4.1&id=e4ba0a9bddff3ba52cec100414d2f178440efc91

A victim^Wtester! Yeah, it’s supposed to do that, but no guarantees, this is an RFC.

I would be happy to know if it solves your problem or not, please keep me in the loop.

quoted

I'll give it a try when I'm back from ELC...

And so I did. I
 1. Moved the pfc node in arch/arm/boot/dts/r8a73a4.dtsi back between dmac
    and i2c5.
 2. Applied your patch,
 3. Disabled the check for "/__local_fixups__" in
    __of_platform_populate_scan().
 4. Added support for "interrupts-extended" (gmail-whitespace-damaged patch
    at the end of this email),
 5. Enabled DEBUG,
 6. Uncommented all your debug pr_info() calls (except the one for
    "interrupts", as it references non-existing variables!).

But it didn't make any difference. The debug log below shows that it detects
the dependency of pfc@e6050000 on irqc0 (interrupt-controller@e61c0000)
irqc1 (interrupt-controller@e61c0200), but it doesn't reorder the nodes,
so pfc is still initialized first:

Interesting. I guess this are indeed more complicated.

It is good that it detects the dependency first, and registers
the devices in the correct order.

The problem is that this is not enough. The probe is initiated when the
drivers are registered afterwards, but not in any particular order.

So we need to do deferred matching for this to work reliably.

Thank you so much for taking the time to send this debug log.
It does help considerably.

Regards

— Pantelis

[snip]

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html