[PATCH 1/3] ARM: pci: make bus I/O resources optional

[Arnd Bergmann]

On Fri, Mar 26, 2021 at 1:18 PM Russell King <rmk+kernel at armlinux.org.uk> wrote:
>
> Adding a bus I/O resource that extends from pcibios_min_io to the top
> of PCI space breaks Footbridge based platforms, which may have ISA
> southbridges and IDE controllers that are in legacy mode.
>
> The PCI I/O space on these machines really does cover port addresses
> from zero upwards, even when pcibios_min_io is non-zero.
>
> Fix this by making Rob's changes optional - Rob's change is probably
> based on a misunderstanding of what pcibios_min_io is - it is the
> minimum IO port address that we wish to start allocating bus resources
> which may not be the same as the minimum IO port address for the bus.

I think Rob's original change is needed for platforms with multiple PCI
root bridges, since they need non-overlapping I/O resources, and each
if they can't all be the root ioport_resource, they have to be children of that.

Leaving out the lower 0x1000 port numbers in turn is required so
legacy ISA drivers acquire the ports with request_region(). The reason
it broke on your machine was apparently that pci_claim_resource()
failed to assign the low port numbers to the PCI bus after that has
explicitly set the minimum.

Using ioport_resource as the PCI host bridge resource as you do
is the easiest way to make it work for both ISA drivers and PCI drivers
using low I/O ports.

> @@ -434,19 +435,25 @@ static int pcibios_init_resource(int busnr, struct pci_sys_data *sys,
>                 if (resource_type(window->res) == IORESOURCE_IO)
>                         return 0;
>
> -       sys->io_res.start = (busnr * SZ_64K) ?  : pcibios_min_io;
> -       sys->io_res.end = (busnr + 1) * SZ_64K - 1;
> -       sys->io_res.flags = IORESOURCE_IO;
> -       sys->io_res.name = sys->io_res_name;
> -       sprintf(sys->io_res_name, "PCI%d I/O", busnr);
> +       if (bus_ioport_resource) {
> +               sys->io_res.start = (busnr * SZ_64K) ?  : pcibios_min_io;
> +               sys->io_res.end = (busnr + 1) * SZ_64K - 1;
> +               sys->io_res.flags = IORESOURCE_IO;


After rereading this function, I see that the function already provides
a way to do what you want here: If the hw->setup() function inserts
ioport_resource into sys->resources along with  the memory
resources, it will skip the extra one, and you get the same effect.

This is what ixp4xx and sa1100/nanoengine apparently do. I
see that cns3xxx also adds an I/O resource, but this one seems
to get it wrong (it refers to the physical address of the I/O window,
not the port numbers), which would be an unrelated bug.

         Arnd