[+cc Andrew] On Wed, Jul 29, 2015 at 07:44:18PM +0000, Gabriele Paoloni wrote: > > -----Original Message----- > > From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx] > > Sent: Wednesday, July 29, 2015 6:21 PM > > To: Gabriele Paoloni > > On Mon, Jul 27, 2015 at 11:17:03PM +0800, Gabriele Paoloni wrote: > > > From: gabriele paoloni <gabriele.paoloni@xxxxxxxxxx> > > > 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(); > > > in of_pci_get_host_bridge_resources() it retrieves the resource > > entry > > > 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? > > 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. > > 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 > > > 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 > > > 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 > > > > > 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. > > > 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; > > > }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
