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

[Thierry Reding]

On Mon, Mar 11, 2013 at 01:38:38PM -1000, Mitch Bradley wrote:
> On 3/11/2013 1:25 PM, Jason Gunthorpe wrote:
> > On Mon, Mar 11, 2013 at 11:50:19AM -1000, Mitch Bradley wrote:
> > 
> >>> However - the driver runs the core in a 'root port bridge mode' where
> >>> the config header register block you are looking at is inhibited. The
> >>> Marvell IP block requires software support to run in bridge mode. So
> >>> Marvell really has only (2), while Tegra has only (1).
> >>
> >> Interesting.
> >>
> >> For Marvell, if Marvell has only (2), does that imply that standard PCI
> >> discovery and enumeration code cannot find the root port bridges, and
> >> also there is no way to auto-configure the bridges with common pci-pci
> >> bridge code?
> > 
> > The driver is required to take care of all of this. From the common
> > code's perspective there is a bridge config header, and writing to it
> > controls the device, as the PCI spec intends. But the driver doesn't
> > copy those read/writes 1:1 to any HW register block, it distributes
> > them around the device's register space as necessary to create the
> > PCI bridge config header.
> > 
> > The DT is still modeling the HW, there really are root port bridge(s),
> > that are completely conformant at the TLP level, but the IP designers
> > choose to leave the detail of the bridge config space up to a SW
> > implementation.
> 
> Oh, ouch, deep spoofing.  I had to do that in system management mode
> firmware for a Geode chip.  What a pain that was.
> 
> > 
> >> At this point I am confused again.  There was talk of the need to
> >> use standard PCI enumeration code to deal with the bridges, thus the
> >> need for 3/2 to describe the interface between root-complex and the
> >> child root-port nodes.
> > 
> > I think your confusion is coming from your expectation that the PCI-E
> > IP block would be an entirely self contained root port, rather than
> > 'tools to build a root port, with the right driver SW'.
> > 
> >>> Further, review section 12 about how ranges should be treated - it
> >>> specifically says that the b,d,f bits in ranges should be 0, and the
> >>> child address should have those bits masked prior to searching the
> >>> ranges.
> >>>
> >>> Section 12 would seem to forbid this:
> >>>
> >>>       ranges = <0x00000800 0 0          0xd4004000 0 0x00000800   /* Root port config header */
> >>>                 0x00001000 0 0          0xd4008000 0 0x00000800   /* Root port config header */
> >>>
> >>> Are you reading that differently?
> >>
> >> I wasn't reading it at all, until you pointed it out to me :-)  I was
> >> just reasoning based on my mental model.
> >>
> >> Again, I agree with your reading of the spec, but I can't think of any
> >> reason why relaxing that restriction to permit config space mappings
> >> would be a problem.
> > 
> > Okay, just wanted to confirm that, I think I follow you now. :)
> > 
> > So you are basically proposing a small update to Section 12 that would
> > read something like:
> > 
> > 'When matching a config space address, the b,d,f bits are valid in the
> >  ranges and left unmasked in the child. The r bits in the ranges
> >  should be 0, and the r bits from the child are copied in the last DW
> >  and masked off in the first DW.'
> > 
> > Which, as you note, would not conflict with any existing usage..
> > 
> > Though, I can already see that the above language is not good enough,
> > it doesn't elegantly represent standard ECAM, for instance.. How would
> > you even represent ECAM with ranges??
> > 
> > Another wrinkle is that tegra not only has the per-bridge memory
> > mapped config space, bit it also has 4 'all port' ECAM-like memory
> > mapped config spaces. The rules around them are sufficiently
> > specialized that I'm not sure a ranges mapping would ever be
> > practical.. So right out of the gate we'd fail to capture memory
> > mapped config access via ranges on at least one SOC.
> > 
> > So, to me, this seems like overkill. Only ECAM has a hope of having a
> > common implementation, everything else will be host driver specific,
> > so why go to the trouble to revise the OF PCI specification for config
> > space?
> > 
> > Particularly since there are patches waiting on this question being
> > resolved :)
> 
> Okay, so I'll just bow out.  Do what you have to do.

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?

And for Marvell the non-configuration-space per-root-port registers need
to be mapped separately from the configuration space windows?

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.infradead.org/pipermail/linux-arm-kernel/attachments/20130312/1908ebed/attachment-0001.sig>