[PATCH v2 3/3] PCI: ARM: add support for generic PCI host controller

On Thu, Feb 13, 2014 at 06:06:16PM +0000, Jason Gunthorpe wrote:

On Thu, Feb 13, 2014 at 11:07:21AM +0000, Will Deacon wrote:

quoted

Not in this case! kvmtool generates the following:

Well, the example is nice so someone can review it..
 

quoted

	pci {
		#address-cells = <0x3>;
		#size-cells = <0x2>;
		#interrupt-cells = <0x1>;
		compatible = "arm,pci-cam-generic";
		reg = <0x0 0x40000000>;
		ranges = <0x1000000 0x0 0x0 0x0 0x0 0x0 0x10000 0x2000000 0x0 0x41000000 0x0 0x41000000 0x0 0x3f000000>;
		interrupt-map = <0x0 0x0 0x0 0x1 0x1 0x0 0x4 0x1 0x800 0x0 0x0 0x1 0x1 0x0 0x5 0x1 0x1000 0x0 0x0 0x1 0x1 0x0 0x6 0x1 0x1800 0x0 0x0 0x1 0x1 0x0 0x7 0x1>;
		interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
	};

Looks like there are a few misses in the above. How about this:

pci {
    compatible = "arm,pci-cam-generic"
    device_type = "pci";
    ** ^^^^^^^^^^^^^^^^^
    ** MANDATORY for the host bridge node

Ok, I'll add that.

    #address-cells = <3>;
    #size-cells = <2>;

    // BUS_ADDRESS(3)  CPU_PHYSICAL(2)  SIZE(2)
    ranges = <0x1000000 0x0 0x00000000  0x0 0x00000000  0x0 0x00010000>,
                                        ^^^^^^^^^^^^^^
            ** ?? Is this why you had problems with the offset? Should
	    ** be the cpu physical address of the start of the IO window.
             <0x2000000 0x0 0x41000000  0x0 0x41000000  0x0 0x3f000000>;

   
    #interrupt-cells = <0x1>;
    // PCI_DEVICE(3) INT#(1)  CONTROLLER(PHANDLE)  CONTROLLER_DATA(3)
    interrupt-map = <  0x0 0x0 0x0 0x1  0x1  0x0 0x4 0x1
                     0x800 0x0 0x0 0x1  0x1  0x0 0x5 0x1 
                    0x1000 0x0 0x0 0x1  0x1  0x0 0x6 0x1
                    0x1800 0x0 0x0 0x1  0x1  0x0 0x7 0x1>;
                 **                    ^^^^^^
                 ** This should be a phandle to the interrupt controller

That is a phandle, libfdt/kvmtool just ends up dealing with the number
directly. I'll fix in the example.

    // PCI_DEVICE(3) INT#1()
    interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
}

I keep thinking of making a pcie-dt.h file with helper macros for
all this. :|

If it's not being generated automatically, that could be useful.

FWWI, I like to put a double space between the logically distinct
things in the lists.

Makes sense, I may well do the same.

Thanks,

Will