Re: [PATCH v3] PCI: of: Avoid pci_remap_iospace() when PCI_IOBASE not defined

[Arnd Bergmann <arnd@arndb.de>]

On Thu, Sep 23, 2021 at 10:33 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
> On Thu, Sep 23, 2021 at 7:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Sep 23, 2021 at 4:55 PM Sergio Paracuellos
> >
> > Ok, got it. So the memory space uses a normal zero offset with cpu address
> > equal to bus address, but the I/O space has a rather usual mapping of
> > bus address equal to the window into the mmio space, with an offset of
> > 0x1e160000.
> >
> > The normal way to do this would be map the PCI port range 0-0x10000
> > into CPU address 0x1e160000, like:
> >
> > ranges = <0x02000000 0 0x60000000 0x60000000 0 0x10000000>,  /* pci memory */
> >                <0x01000000 0 0x1e160000 0 0 0x00010000>;
>
> This change as it is got into the same behaviour:
>
> mt7621-pci 1e140000.pcie: host bridge /pcie@1e140000 ranges:
> mt7621-pci 1e140000.pcie:   No bus range found for /pcie@1e140000,
> using [bus 00-ff]
> mt7621-pci 1e140000.pcie:      MEM 0x0060000000..0x006fffffff -> 0x0060000000
> mt7621-pci 1e140000.pcie:       IO 0x0000000000..0x000000ffff -> 0x001e160000
>                                                    ^^^^^^
>                                                     This is the only
> change I appreciate in all the trace with the range change.

Oops, my mistake, I mixed up the CPU address and the PCI address.

The correct notation should be

<0x01000000 0 0  0 0x1e160000 0x00010000>;

i.e. bus address 0 to cpu address 0x1e160000, rather than the other
way around as I wrote it.

> pci 0000:00:02.0: PCI bridge to [bus 03-ff]
> pci 0000:00:02.0:   bridge window [io  0x0000-0x0fff]
> pci 0000:00:02.0:   bridge window [mem 0x00000000-0x000fffff]
> pci 0000:00:02.0:   bridge window [mem 0x00000000-0x000fffff pref]
...
> pci 0000:00:02.0: PCI bridge to [bus 03]
> pci 0000:00:02.0:   bridge window [io  0x2000-0x2fff]
> pci 0000:00:02.0:   bridge window [mem 0x60400000-0x604fffff]
> pci 0000:00:02.0:   bridge window [mem 0x60500000-0x605fffff pref
>
> # cat /proc/ioports
> 00000000-0000ffff : pcie@1e140000
>   00000000-00000fff : PCI Bus 0000:01
>     00000000-0000000f : 0000:01:00.0
>       00000000-0000000f : ahci
>     00000010-00000017 : 0000:01:00.0
...

These are all what I would expect, but that should just be
based on the PCI_IOBASE value, not the mapping behind that.

> > Do you know where that mapping is set up? Is this a hardware setting,
> > or is there a way to change the inbound I/O port ranges to the
> > normal mapping?
>
> The only thing related is the IOBASE register which is the base
> address for the IO space window. Driver code is setting this up to pci
> IO resource start address [0].

So this line:
      pcie_write(pcie, entry->res->start, RALINK_PCI_IOBASE);

That does sound like it is the bus address you are writing, not
the CPU address. Some host bridges allow you to configure both,
some only one of them, but the sensible assumption here would
be that this is the bus address if only one is configurable.

If my assumption is correct here, then you must write the value
that you have read from the DT property, which would be
0x001e160000 or 0 in the two versions of the DT property we
have listed, but in theory any (properly aligned) number ought
to work here, as long as the BAR values, the RALINK_PCI_IOBASE
and the io_offset all agree what it is.

The line just before this is

pcie_write(pcie, 0xffffffff, RALINK_PCI_MEMBASE)

Can you clarify what this does? I would have expected an
identity-map for the memory space, which would mean writing
the third cell from the ranges property (0x60000000)
into this. Is -1 a special value for identity mapping, or does
this register do something else?

          Arnd