On Tue, Mar 12, 2013 at 09:57:49AM -0600, Jason Gunthorpe wrote:
> On Tue, Mar 12, 2013 at 08:08:52AM +0100, Thierry Reding wrote:
> 
> > So to recapitulate, we agree that configuration space can be translated
> > through the "ranges" property. That means the only missing link is a new
> > function to translate not only "assigned-addresses" but also the "reg"
> > property for PCI devices. Is that it?
> 
> No, the first conclusion is that placing a config space in ranges is
> against the language in the current spec (see section 12).
> 
> The second conclusion is that there is probably a way forward to
> update the spec in a backwards compatible way to model ECAM, but would
> require more analysis.
> 
> Finally, there was no objections to the approach in Thomas's patch,
> other than the note to fix the '@1,0' and the extra device_type="pci"

That's not what I deduced from Mitch's comments. He said explicitly that
he liked the idea to represent mapped configuration space using the
ranges property. Mitch, correct me if I misinterpret what you said.

I've also verified that things actually do work with the binding that I
proposed for Tegra, even if I add device_type = "pci" to the root port
nodes. The reason is that the OF core looks up the type of the *parent*
node to find a matching bus. This happens to be the pcie-controller node
which is not a PCI device and therefore the address translation works
properly.

So let me quote the latest binding that I'm using:

        pcie-controller {
                compatible = "nvidia,tegra20-pcie";
                reg = <0x80003000 0x00000800   /* PADS registers */
                       0x80003800 0x00000200   /* AFI registers */
                       0x90000000 0x10000000>; /* configuration space */
                reg-names = "pads", "afi", "cs";
                interrupts = <0 98 0x04   /* controller interrupt */
                              0 99 0x04>; /* MSI interrupt */
                interrupt-names = "intr", "msi";

                bus-range = <0x00 0xff>;
                #address-cells = <3>;
                #size-cells = <2>;

                ranges = <0x00000800 0 0 0x80000000 0 0x00001000   /* port 0 
configuration space */
                          0x00001000 0 0 0x80001000 0 0x00001000   /* port 1 
configuration space */
                          0x81000000 0 0 0x82000000 0 0x00010000   /* 
downstream I/O */
                          0x82000000 0 0 0xa0000000 0 0x10000000   /* 
non-prefetchable memory */
                          0xc2000000 0 0 0xb0000000 0 0x10000000>; /* 
prefetchable memory */

                clocks = <&tegra_car 70>, <&tegra_car 72>, <&tegra_car 74>,
                         <&tegra_car 118>;
                clock-names = "pex", "afi", "pcie_xclk", "pll_e";
                status = "disabled";

                pci@1,0 {
                        device_type = "pci";
                        reg = <0x000800 0 0 0 0x1000>;
                        status = "disabled";

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

                        nvidia,num-lanes = <2>;
                };

                pci@2,0 {
                        device_type = "pci";
                        reg = <0x001000 0 0 0 0x1000>;
                        status = "disabled";

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

                        nvidia,num-lanes = <2>;
                };
        };

I can't find anything wrong in the above. As far as I can tell, there's
nothing in it that's in conflict with the specification.

Thierry

Attachment: pgpkwOprP6_DC.pgp
Description: PGP signature

_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss

Reply via email to