[no subject]

> Without this patch the range translation chain is broken, like this:

> pcie@120000: <0x2000000 0x00 0x00    0x1f 0x00                0x00 0xfffffffc>;
> ~~~ chain breaks here ~~~
> pci@0      : <0x82000000 0x1f 0x00   0x82000000 0x1f 0x00     0x00 0x600000>;
> dev@0,0    : <0x01 0x00 0x00         0x82010000 0x1f 0x00     0x00 0x400000>;
> rp1@0      : <0xc0 0x40000000        0x01 0x00 0x00           0x00 0x400000>;

The cover letter said "RP1 is an MFD chipset that acts as a
south-bridge PCIe endpoint .. the RP1 as an endpoint itself is
discoverable via usual PCI enumeration".

I assume pcie@120000 is the PCI host bridge and is already in the
original DT describing the platform.  I assume pci@0 is a Root Port
and dev@0,0 is the RP1 Endpoint, and the existing code already adds
them as they are enumerated when pci_bus_add_device() calls
of_pci_make_dev_node(), and I think this series adds the rp1@0
description.

And the "ranges" properties are built when of_pci_make_dev_node()
eventually calls of_pci_prop_ranges().  With reference to sec 2.2.1.1
of https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
and
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#ranges,
I *think* your example says:

pcie@120000 has:
  child phys.hi	      0x02000000    n=0 p=0 t=0 ss=10b
  child phys.mid,lo   0x00000000_00000000
  parent phys.hi,lo   0x0000001f_00000000
  length hi,lo        0x00000000_fffffffc

which would make it a bridge where the child (PCI) address space is
relocatable non-prefetchable 32-bit memory space at
0x00000000-0xfffffffc, and the corresponding parent address space is
0x1f_00000000-0x1f_fffffffc.  That means the host bridge applies an
address translation of "child_addr = parent_addr - 0x1f_00000000".

pci@0 has:
  child phys.hi	      0x82000000    n=1 p=0 t=0 ss=10b
  child phys.mid,lo   0x0000001f_00000000
  parent phys.hi      0x82000000    n=1 p=0 t=0 ss=10b
  parent phys.mid,lo  0x0000001f_00000000
  length hi,lo        0x00000000_00600000

which would make it a PCI-to-PCI bridge (I assume a PCIe Root Port),
where the child (secondary bus) address space is the non-relocatable
non-prefetchable 32-bit memory space 0x1f_00000000-0x1f_005fffff and
the parent (primary bus) address space is also non-relocatable
non-prefetchable 32-bit memory space at 0x1f_00000000-0x1f_005fffff.

This looks wrong to me because the pci@0 parent address space
(0x1f_00000000-0x1f_005fffff) should be inside the pcie@120000 child
address space (0x00000000-0xfffffffc), but it's not.

IIUC, this patch clears the upper 32 bits in the pci@0 parent address
space.  That would make things work correctly in this case because
that happens to be the exact translation of pcie@120000, so it results
in pci@0 parent address space of 0x00000000-0x005fffff.

But I don't think it works in general because there's no requirement
that the host bridge address translation be that simple.  For example,
if we have two host bridges, and we want each to have 2GB of 32-bit
PCI address space starting at 0x0, it might look like this:

  0x00000002_00000000 -> PCI 0x00000000 (subtract 0x00000002_00000000)
  0x00000002_80000000 -> PCI 0x00000000 (subtract 0x00000002_80000000)

In this case simply ignoring the high 32 bits of the CPU address isn't
the correct translation for the second host bridge.  I think we should
look at each host bridge's "ranges", find the difference between its
parent and child addresses, and apply the same difference to
everything below that bridge.

> while with the patch applied the chain correctly become:

> pcie@120000: <0x2000000 0x00 0x00    0x1f 0x00                0x00 0xfffffffc>;
> pci@0      : <0x82000000 0x00 0x00   0x82000000 0x00 0x00     0x00 0x600000>;
> dev@0,0    : <0x01 0x00 0x00         0x82010000 0x00 0x00     0x00 0x400000>;
> rp1@0      : <0xc0 0x40000000        0x01 0x00 0x00           0x00 0x400000>;

> > > > Maybe my expectation of this being described in DT is mistaken.
> > > 
> > > Not sure what you mean here, the address being translated are coming from
> > > DT, in fact they are described by "ranges" properties.
> > 
> > Right, for my own future reference since I couldn't find a generic
> > description of "ranges" in Documentation/devicetree/:
> > 
> > [1] https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#ranges