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:

quoted

On Monday, June 6, 2016 6:04:44 PM CEST Bjorn Helgaas wrote:

quoted

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

quoted

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