Re: [PATCH v2 07/10] ARM: tegra: pcie: Add device tree support

[Thierry Reding <thierry.reding@avionic-design.de>]

[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]

On Tue, Jun 12, 2012 at 10:36:55PM -1000, Mitch Bradley wrote:
> On 6/12/2012 10:19 PM, Thierry Reding wrote:
> >On Tue, Jun 12, 2012 at 10:05:35PM -1000, Mitch Bradley wrote:
> >>On 6/12/2012 9:52 PM, Thierry Reding wrote:
> >>>I think the configuration spaces and downstream I/O ranges need to be in the
> >>>pcie-controller's reg property because they are remapped and used by the
> >>>controller driver, not by the individual ports.
> >>>
> >>>That's probably not really necessary but rather a result of how the driver
> >>>was written. Perhaps the driver should handle them differently instead,
> >>>listing the regions in the ranges property of the parent and listing the
> >>>corresponding partitions in the ranges properties of the pci child nodes.
> >>>
> >>>Like in the following, where the ranges property of the ports partition the
> >>>ranges passed from the parent evenly:
> >>>
> >>>	pcie-controller {
> >>>		compatible = "nvidia,tegra20-pcie";
> >>>		reg =<0x80003000 0x00000800   /* PADS registers */
> >>>		       0x80003800 0x00000200>; /* AFI registers */
> >>>		interrupts =<0 98 0x04   /* controller interrupt */
> >>>			      0 99 0x04>; /* MSI interrupt */
> >>>		status = "disabled";
> >>>
> >>>		ranges =<0x80000000 0x80000000 0x00002000   /* 2 root ports */
> >>>			  0x80004000 0x80004000 0x00100000   /* configuration space */
> >>>			  0x80104000 0x80100000 0x00100000   /* extended configuration space */
> >>>			  0x80400000 0x80400000 0x00010000   /* downstream I/O */
> >>>			  0x90000000 0x90000000 0x10000000   /* non-prefetchable memory */
> >>>			  0xa0000000 0xa0000000 0x10000000>; /* prefetchable memory */
> >>>
> >>>		#address-cells =<1>;
> >>>		#size-cells =<1>;
> >>>
> >>>		pci@80000000 {
> >>>			reg =<0x80000000 0x00001000>;
> >>>			status = "disabled";
> >>>
> >>>			#address-cells =<3>;
> >>>			#size-cells =<2>;
> >>>
> >>>			ranges =<0x80400000 0x80400000 0x00008000   /* I/O */
> >>>				  0x90000000 0x90000000 0x08000000   /* non-prefetchable memory */
> >>>				  0xa0000000 0xa0000000 0x08000000>; /* prefetchable memory */
> >>
> >>You are on the right track here, but the format of the child-address
> >>portion of the above ranges property is incorrect.  Since the child
> >>address space is the PCI address space, the child-address portion
> >>needs to be 3 cells.  It's not a linear address but rather a triple.
> >>The first cell identifies the address type (config, I/O, memory..)
> >>and the second and third cells are offsets within that subspace.
> >>The second and third cells will typically be 0.  The PCI binding has
> >>details.
> 
> Also, the size field in ranges is specified according to the child
> address space, so there must be 2 size cells in the ranges at this
> level.  Each ranges entry at this level is:
> 
> <child_address_space, child_address_high, child_address_low,
> parent_address, child_size_high, child_size_low>
> 
> The above should be:
> 
> 			ranges =<0x81000000 0 0  0x80400000  0 0x00008000   /* I/O */
> 				 0x82000000 0 0  0x90000000  0 0x08000000   /* non-prefetchable memory */
> 				 0xc2000000 0 0  0xa0000000  0 0x08000000>; /* prefetchable memory */

Okay, I think I'm beginning to understand. Thanks.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]