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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Mon, 30 Mar 2015 15:27:27 +0200
, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
 wrote:
> Hi Pantelis,
> 
> On Tue, Mar 24, 2015 at 6:56 PM, Pantelis Antoniou
> <pantelis.antoniou@xxxxxxxxxxxx> wrote:
> >> On Mar 24, 2015, at 07:50 , Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> >> On Fri, Mar 20, 2015 at 12:39 PM, Pantelis Antoniou
> >> <pantelis.antoniou@xxxxxxxxxxxx> wrote:
> >>>> On Mar 19, 2015, at 21:18 , Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
> >>>> On Tue, 16 Dec 2014 14:11:31 +0200
> >>>> , Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
> >>>> wrote:
> >>>>> 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.
> >
> >> 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:

I talked to Pantelis about this while we were at ELC. Even if it did
correctly reorder the registration and solved your problem, it won't
help at all for most devices registered by the of_platform_populate()
call.

The core device model is oportunistic about when drivers get bound to
devices. Both drivers and devices can be registered at any time. When a
device is registered, it kernel tries to bind it against all available
drivers, and similarly when a driver is registered the kernel tries to
bind it with all unbound devices.

In the normal case, of_platform_populate() is called at arch_initcall
(level 3).  Most drivers are registered at device_initcall (level 6).
The only reason reordering works for you is because both the renesas pfc
irqc drivers use postcore_initcall (level 2). Therefore, both drivers
get registered before of_platform_populate() which makes device
registration order matter. If the devices were registered before the
drivers, then it would be kernel link order which determines the
behaviour.

There are two ways to fix this so that .dtb order doesn't matter. The
dirty hack is to change the pfc driver to use subsys_initcall (level 4)
or later so that it happens after the devices are registered. The second
solution is to make the pfc drivers able to return -EPROBE_DEFER, but
that also requires fixing deferred probe to start retrying devices
before late_initcall time. Right now there is a holdoff flag that
prevents any retries before late_initcall(). It was done that way
because the defer workqueue conflicted with some of the initcalls in a
bad way.

We could add a manual walk of the queue between each initcall level
without using the workqueue. That would solve the problem.

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux