On Tue, Jun 07, 2016 at 08:11:05AM -0500, Bjorn Helgaas wrote: > On Tue, Jun 07, 2016 at 10:21:36AM +0200, Arnd Bergmann wrote: > > On Monday, June 6, 2016 6:04:44 PM CEST Bjorn Helgaas wrote: > > > Several host bridge drivers (designware and all derivatives, iproc, > > > xgene, xilinx, and xilinx-nwl) don't request the MMIO and I/O port > > > windows they forward downstream to the PCI bus. > > > > > > That means the PCI core can't request resources for PCI bridge > > > windows and PCI BARs. > > > > > > Several other drivers (altera, generic, mvebu, rcar, tegra) do request > > > the windows, but use some duplicated code to do it. > > > > > > This adds a new devm_request_pci_bus_resources() interface and changes > > > these drivers to use it. It also fixes several error paths where we failed > > > to free the resource list allocated by of_pci_get_host_bridge_resources(). > > > > > > Tegra guys, please take a look at "PCI: tegra: Remove top-level resource > > > from hierarchy" in particular. Removing the top-level resource definitely > > > makes /proc/iomem look uglier (although it will look more like that of > > > other drivers). A short-term fix could be to include device information in > > > the resource name. I think a better long-term fix would be to make the DT > > > or platform device core request all the resources from the DT. > > > > > > Comments welcome. I expect we'll trip over something here, so I marked > > > this "v1" and I don't plan to put it into -next for a while. > > > > > > This is on my pci/host-request-windows branch, which you can pull or view > > > at https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-request-windows > > > > This looks very nice. There is one related aspect that I have been > > grumbling about for a while, but I don't know what the driver is > > actually supposed to do there: > > > > For the IORESOURCE_IO resources, some drivers request the MMIO address > > that the window is mapped into, some drivers request the PIO range, and > > some of them request both. I also believe the resource that gets put > > into the bridge resources list is not always the same one (or maybe > > that got fixed by now). > > > > What do you think is the correct behavior here, should the driver only > > request the PIO range with parent=ioport_resource, or should it also > > request the MMIO window for the I/O ports with parent=iomem_resource? > > In the latter case, any idea how that can be generalized? > > I think it should request both because I think iomem_resource should > contain everything in the memory map. This would be required if we ever > did any significant reassignment of top-level devices, e.g., ACPI devices. > For example, on ia64, we do this: > > /proc/ioports: > 00000000-00003fff : PCI Bus 0000:00 > 00004000-00009fff : PCI Bus 0000:80 > 0000a000-0000bfff : PCI Bus 0000:a0 > 0000c000-0000ffff : PCI Bus 0000:c0 > > /proc/iomem: > 80000000-9fffffff : PCI Bus 0000:00 > a0000000-cfffffff : PCI Bus 0000:80 > d0000000-dfffffff : PCI Bus 0000:a0 > e0000000-fdffffff : PCI Bus 0000:c0 > 80004000000-80103fffffe : PCI Bus 0000:00 > c0004000000-c0103fffffe : PCI Bus 0000:80 > d0004000000-d0103fffffe : PCI Bus 0000:a0 > e0004000000-e0103fffffe : PCI Bus 0000:c0 > 3fffffc000000-3fffffcffffff : PCI Bus 0000:00 I/O Ports 00000000-00003fff > 3fffffd000000-3fffffe7fffff : PCI Bus 0000:80 I/O Ports 00004000-00009fff > 3fffffe800000-3fffffeffffff : PCI Bus 0000:a0 I/O Ports 0000a000-0000bfff > 3ffffff000000-3ffffffffffff : PCI Bus 0000:c0 I/O Ports 0000c000-0000ffff > > > Another aspect is that we already have the > > gen_pci_parse_request_of_pci_ranges() function that does the same as your > > new devm_request_pci_bus_resources() and then a few other things. I > > have been wondering whether we could move that function into common > > code convert drivers to use that wherever possible, but I guess we can > > always do that as a follow-up after this series. > > Oh, I didn't notice that; thanks for pointing it out. That should be > consolidated somehow. It also checks to be sure there is a > non-prefetchable memory resource. A few other drivers also do that, but > most don't. I suppose that will mostly catch DT errors. Coming back to this, I did actually change gen_pci_parse_request_of_pci_ranges() to call my new function in [16/25] "PCI: generic: Request host bridge window resources with core function". The gen_pci_parse_request_of_pci_ranges() is still there and it still contains the loop to deal with the I/O port space and to validate that a non-prefetchable memory window exists. Both of those could probably be made more generic later. Bjorn
