Re: [PATCH] powerpc/pci: Improve device hotplug initialization

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Sat, 2013-06-01 at 06:58 -0700, Guenter Roeck wrote:
> the comment was actuially directed towards Yuanquan.
> 
> No problem, take your time. I did my best to test it, but I agree that this is a
> critical area of the code, and it would be desirable to get additional scrutiny
> and test feedback.
> 
> The code has been running in our system (P2020 and P5040) for several months.
> I was preparing a patch for upstream submission when I noticed commit 37f02195b.
> After testing ithis commit, I noticed the problems with it and wrote this patch,
> which aligns the code with our initial patch. I tested it as good as I could on
> our systems as well as with a P5040 evaluation board and an Intel GE PCIe
> card.

Ok, so I like this very much. So much that I was considering still sneaking it
into 3.10, until I hit a snag...

[ Basically, the previous patch that moved the setup to pcibios_enable_device()
always made me nervous. It did regress at least one platform (mac stuff) due
to missed IRQ fixup, which I worked around later on, and I'm still not terribly
happy about it. Your approach is much cleaner. ]

I suppose that when I wrote the original setup stuff there wasn't an add
hook or I didn't see it...

In fact I would go further and completely remove pcibios_setup_bus_devices()
which is now empty since it's only called by the powerpc code, it's not
a generic hook.

However, here's the snag. Unless I missed something, we now setup the devices
DMA before we call pcibios_fixup_bus(). And *that* is going to break some
pseries.

We have an assumption in there that the bus fixup is done first, because in
some cases, the DMA windows are established at the bus level, and the "dev"
setup just picks up the bits.

Now looking at that code, it's not unfixable but it won't make 3.10. Maybe
we need a new pre-scan hook for busses... we can use the pcibios_add_device()
hook of the bridge itself for P2P but that won't do for the root bus and I
don't like having two different path here...

Cheers,
Ben.