From mboxrd@z Thu Jan 1 00:00:00 1970 From: thierry.reding@avionic-design.de (Thierry Reding) Date: Thu, 7 Mar 2013 21:47:26 +0100 Subject: [PATCH 24/32] pci: PCIe driver for Marvell Armada 370/XP systems In-Reply-To: <20130307200235.GB20695@obsidianresearch.com> References: <1360686546-24277-1-git-send-email-thomas.petazzoni@free-electrons.com> <1360686546-24277-25-git-send-email-thomas.petazzoni@free-electrons.com> <20130212223511.GB31555@obsidianresearch.com> <20130306105441.4d24033e@skate> <20130306121118.GA17079@avionic-0098.mockup.avionic-design.de> <20130306180946.GA2433@obsidianresearch.com> <20130307080832.GD3451@avionic-0098.mockup.avionic-design.de> <20130307174955.GC20840@obsidianresearch.com> <20130307194830.GA1811@avionic-0098.mockup.avionic-design.de> <20130307200235.GB20695@obsidianresearch.com> Message-ID: <20130307204726.GB1811@avionic-0098.mockup.avionic-design.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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: