On Fri, Aug 17, 2018 at 5:25 PM Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> wrote: > > On Thu, Aug 16, 2018 at 11:04:53PM +0200, Arnd Bergmann wrote: > > On Thu, Aug 16, 2018 at 10:59 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > > On Wed, Aug 15, 2018 at 02:52:28PM -0500, Bjorn Helgaas wrote: > > > > On Thu, Aug 02, 2018 at 05:54:30PM +0100, Lorenzo Pieralisi wrote: > > > > > > This patch seems OK to me. > > > > > > > > I don't really care about the prototype. There's only one > > > > pcibios_add_device() implementation (x86) that returns anything other > > > > than 0, and that's a pretty obscure error case related to f9a37be0f02a > > > > ("x86: Use PCI setup data"), which lets us use ROM data from boot > > > > services. Even then the only thing that happens is a WARN_ON(). A > > > > more descriptive printk would be a lot more useful. > > > > > > Thinking about this some more, I'm not so sure about the connection > > > with removing pcibios_add_device(). This host_bridge->add_dev() hook > > > would be for host bridge-specific things, while pcibios_add_device() > > > is for arch-specific things. > > > > > > I'd still love to get rid of pcibios_add_device() (especially the > > > non-arch-specific things like the pci_claim_resource() in s390); I'm > > > just not sure yet whether this particular patch is the vehicle. > > > > I think most of the arch-specific pcibios_* calls are actually > > host bridge specific after all, it just so happens that they are > > implemented on architectures that only have one specific > > host bridge implementation, or that they are used on an > > architecture that does something odd in one place and needs > > to do something else in another place. > > > > For pci_claim_resource() we seem to be doing this in a number > > of different places, but there isn't strictly a reason for that. > > pci_claim_resource() is needed if either arch code or the host > controller driver does not trigger a resources assignment (which claims > them while at it); in theory that's arch agnostic but it turned out to > be very arch/platform specific - aka if we move s390 code to core code > we will notice :) so pci_claim_resource() in a pcibios call is > unfortunately legitimate - whether it can be moved out of it to > generic code that's a very complicated problem. The generic pci_host_probe() already does either pci_bus_claim_resources() or pci_bus_size_bridges()/pci_bus_assign_resources()/pcie_bus_configure_settings() I would hope that we can move a lot of implementations over to just call this function, with the appropriate PCI_PROBE_ONLY setting. Arnd