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. Bjorn
