[+cc Andrew]

On Wed, Jul 29, 2015 at 07:44:18PM +0000, Gabriele Paoloni wrote:

quoted

-----Original Message-----
From: Bjorn Helgaas [mailto:bhelgaas@google.com]
Sent: Wednesday, July 29, 2015 6:21 PM
To: Gabriele Paoloni

quoted

On Mon, Jul 27, 2015 at 11:17:03PM +0800, Gabriele Paoloni wrote:

quoted

From: gabriele paoloni <redacted>

quoted

quoted

    This patch adds a new field in "struct of_pci_range" to store the
    pci bus start address; it fills the field in

of_pci_range_parser_one();

quoted

    in of_pci_get_host_bridge_resources() it retrieves the resource

entry

quoted

    after it is created and added to the resource list and uses
    entry->__res.start to store the pci controller address

struct of_pci_range is starting to get confusing to non-OF folks like
me.
It now contains:

  u32 pci_space;
  u64 pci_addr;
  u64 cpu_addr;
  u64 bus_addr;

Can you explain what all these things mean, and maybe even add one-line
comments to the structure?

quoted

pci_space: The only uses I see are to determine whether to print
"Prefetch".  I don't see any real functionality that uses this.

Looking at the code I agree. it's seems to be used only in powerpc 
and microblaze to print out.
However from my understanding pci_space is the phys.hi field of the 
ranges property: it defines the properties of the address space associated
to the PCI address. if you're curious you can find a nice and quick to read
"guide" in http://devicetree.org/MPC5200:PCI

I think pci_space should be removed and the users should test
"range.flags & IORESOURCE_PREFETCH" instead.  That's already set by
of_bus_pci_get_flags().  This is separate from your current patch, of
course.

29b635c00f3e ("of/pci: Provide support for parsing PCI DT ranges
property") added struct of_pci_range, and even at the time,
of_bus_pci_get_flags() set IORESOURCE_PREFETCH in of_pci_range.flags.

654837e8fe8d ("powerpc/pci: Use of_pci_range_parser helper in
pci_process_bridge_OF_ranges") converted powerpc to use
of_pci_range_parser() instead of parsing manually.  It converted other
references to look at struct of_pci_range.flags; I'm not sure why it
didn't do that for the prefetch bit.

I copied Andrew in case there's some subtlety here.

quoted

pci_addr: I assume this is a PCI bus address, like what you would see
if
you put an analyzer on the bus/link.  This address could go in a BAR.

Yes, this is the PCI start address of the range: phys.mid + phys.low in the
guide mentioned above

quoted

cpu_addr: I assume this is a CPU physical address, like what you would
see
in /proc/iomem and what you would pass to ioremap().

Yes correct

quoted

bus_addr: ?

According to the guide above, this is the address into which the pci_address 
get translated to and that is passed to the root complex. Between the root 
complex and the CPU there can be intermediate translation layers: 

I can't quite parse this, but I do understand how a host bridge can
translate CPU physical addresses to a different range of PCI bus
addresses.  What I don't understand is the difference between "pci_addr"
and the "bus_addr" you're adding.

see that to 
get pci_address we call "of_translate_address"; this will apply all the 
translation layers (ranges in the DT) that it finds till it comes to the root 
node of the DT (thus retrieving the CPU address).
Now said that, for designware we need the first translated PCI address, that we call
here bus_addr after Rob Herring suggested the name...honestly I cannot think of a 
different name

   

quoted

I'm trying to imagine how this might be expressed in ACPI.  A host
bridge
ACPI _CRS contains a CPU physical address and applying a _TRA
(translation
offset) to the CPU address gives you a PCI bus address.  I know this
code
is OF, not ACPI, but I assume that it should be possible to describe
your
hardware via ACPI as well as by OF.

quoted

quoted

diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index d88e81b..865f96e 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h

@@ -16,6 +16,7 @@ struct of_pci_range {
 	u32 pci_space;
 	u64 pci_addr;
 	u64 cpu_addr;
+	u64 bus_addr;
 	u64 size;
 	u32 flags;
 };