[PATCH 24/32] pci: PCIe driver for Marvell Armada 370/XP systems

[Thierry Reding]

On Thu, Mar 07, 2013 at 01:02:35PM -0700, Jason Gunthorpe wrote:
> On Thu, Mar 07, 2013 at 08:48:30PM +0100, Thierry Reding wrote:
> > On Thu, Mar 07, 2013 at 10:49:55AM -0700, Jason Gunthorpe wrote:
> > > On Thu, Mar 07, 2013 at 09:08:32AM +0100, Thierry Reding wrote:
> > [...]
> > > > > Both have various problems, but I think I prefer the first one as it
> > > > > doesn't conflate the contoller registers and host apertures in a
> > > > > single ranges..
> > > > 
> > > > I think a better alternative would be (and this matches what Thomas has
> > > > said elsewhere) to use something like the first alternative but move the
> > > > regs property into the pcie at 0,X nodes. That would save us from having to
> > > > index a property in the parent. At least from a DT point of view I find
> > > > that to be a more consistent representation.
> > > 
> > > You are thinking a new property 'host-controller-regs' or the like?
> > 
> > Well, something shorter would be nice, but that's the general idea, yes.
> > As I mentioned before, for Tegra these registers aren't actually any
> > controller specific registers but rather a window to access the PCI
> > configuration space for the root ports.
> 
> Yes, I understand - but in this DT model configuration space access is
> a host controller function, not a PCI-device function. Anyhow I was
> also thinking that by the choice of the name it could do translation
> from the host-controller scope, not from the bridge scope - so the
> extra elements in ranges could be avoided as well. Hence the name..
> 
> > I don't think assigned-addresses is a good fit either. The PCI binding
> > document is equally specific about it as it is about the reg property.
> > So in my opinion a separate property would be a better choice. The only
> > big obstacle is that it needs to be somehow hooked up with the OF core
> > so that proper address translation can be performed.
> 
> Yes, agreed.
> 
> My suggestion is to get the OF experts like GKL/Rob H/etc to weigh in
> on a preferred approach to this problem with the goal of standardizing
> across all PCI host drivers. Seems like there are 2 main options
> (outside regs + regnames/etc or new 'regs' in the bridge) and 1 hacky
> one (assigned addresses)

Arnd is already Cc'ed on this thread, adding Grant, Rob and the
devicetree-discuss ML.

In a nutshell (since some of the context isn't quoted anymore) the
problem that we're trying to solve is that some of the embedded SoCs
require per-root-port registers for configuration. The PCI DT
specification doesn't make any provisions for this. A few alternatives
have been discussed so far:

	1) Use a "regs" property outside of the root port nodes with
	   some mechanism to index into them from within the root port
	   nodes. Conceptually somewhat like this:

		pcie-controller {
			...
			regs = <0x80000000 0x00001000
			        0x80001000 0x00001000>;

			pci at 0,1 {
				...
				port-index = <0>;
			};

			pci at 0,2 {
				...
				port-index = <1>;
			};
		};

	2) Use a "regs" property inside of the root part nodes, along
	   the following lines:

		pcie-controller {
			...
			pci at 0,1 {
				...
				reg = <0x00000800 0 0 0 0>;

				regs = <0x80000000 0x00001000>;
			};

			pci at 0,2 {
				...
				reg = <0x00001000 0 0 0 0>;

				regs = <0x80001000 0x00001000>;
			};
		};

	3) Repurpose the "assigned-addresses" property to achieve the
	   same. This should work out-of-the-box but isn't a good fit
	   because it conflicts with the OF PCI specification which
	   defines this property to contain the addresses assigned to
	   the base address registers.

Options 1 and 2 above require changes to the OF core to allow proper
address translation, but the changes shouldn't be very big.

> > One possible solution that wouldn't be too hard to implement is to
> > provide a new function (say of_get_named_address()) similar to
> > of_get_address() which doesn't get the name of the register property
> > from the struct of_bus but from a parameter and call that function from
> > another new function similar to of_address_to_resource() that also gets
> > the property name from a parameter. I can't think of a better name for
> > the latter than of_named_address_to_resource(), which is rather long.
> 
> Seems like a reasonable API, maybe pass in a be32*/length pointer
> instead of a name to be more flexible?

There's already __of_address_to_resource() which takes a be32 * and size
but I thought it might be easier to wrap that to make it easier on the
drivers to use the API.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/devicetree-discuss/attachments/20130307/2df62bd0/attachment-0001.sig>