Re: pci and pcie device-tree binding - range No cells

[Grant Likely <grant.likely@secretlab.ca>]

On Mon, 10 Dec 2012 09:21:51 -0600, Rob Herring <robherring2@gmail.com> wrote:
> On 12/10/2012 09:05 AM, Michal Simek wrote:
> > On 12/10/2012 03:26 PM, Rob Herring wrote:
> >> On 12/10/2012 06:20 AM, Michal Simek wrote:
> >>> Hi Grant and others,
> >>>
> >>> I have a question regarding number of cells in ranges property
> >>> for pci and pcie nodes.
> >>>
> >>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
> >>> sequoia.dts, etc)
> >>> but also 6 cells format too (mpc832x_mds.dts)
> >>>
> >>> Here is shown 6 cells ranges format and describe
> >>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
> >>>
> >>> And also in documentation in the linux
> >>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
> >>>
> >>> Both format uses:
> >>> #size-cells = <2>;
> >>> #address-cells = <3>;
> >>>
> >>> What is valid format?
> >>
> >> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
> >> are valid when the host bus is 32-bit. The ranges property is <<child
> >> address> <parent address> <size>>. The parent address #address-cells is
> >> taken from the parent node.
> > 
> > Ok. Got it.
> > 
> > Here is what we use on zynq and microblaze - both 32bit which should be
> > fine.
> > 
> >     ps7_axi_interconnect_0: axi@0 {
> >         #address-cells = <1>;
> >         #size-cells = <1>;
> >         axi_pcie_0: axi-pcie@50000000 {
> >             #address-cells = <3>;
> >             #size-cells = <2>;
> >             compatible = "xlnx,axi-pcie-1.05.a";
> >             ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
> >             ...
> >         }
> >     }
> > 
> > What I am wondering is pci_process_bridge_OF_ranges() at
> > arch/powerpc/kernel/pci-common.c
> > where there are used some hardcoded values which should be probably
> > loaded from device-tree.
> > 
> > For example:
> > 683         int np = pna + 5;
> > ...
> > 702                 pci_addr = of_read_number(ranges + 1, 2);
> > 703                 cpu_addr = of_translate_address(dev, ranges + 3);
> > 704                 size = of_read_number(ranges + pna + 3, 2);
> 
> These would always be correct whether you have 6 or 7 cells. pna is the
> parent bus address cells size. The pci address is fixed at 3 cells.
> 
> > 
> > 
> > Unfortunately we have copied it to microblaze.
> 
> I look at the PCI DT code in powerpc and see a whole bunch of code that
> seems like it should be common. The different per arch pci structs
> complicates that. No one has really gotten to looking at PCI DT on ARM
> yet except you and Thierry for Tegra. We definitely don't want to create
> a 3rd copy. Starting the process of moving it to something like
> drivers/pci/pci-of.c would be great.

A lot of it should be common. The microblaze code is a copy of the
powerpc version. I'll strongly nack any attempt to add a third!  :-)

drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
which might actually be a good idea in the short term so that it gets
appropriate supervision while being generalized before being moved into
the pci directory.

g.