On Sun, Mar 10, 2013 at 07:46:04PM -1000, Mitch Bradley wrote: > So it seems that we are faced with two requirements that are somewhat at > odds with one another. > > 1) Some of the root port bridge registers have to be accessed via config > space access functions so common PCI enumeration code will work. To > represent this with the usual DT structure, the top root-complex needs > to define a 3/2 address space so its children > can have standard PCI reg properties. Presumably, if those registers > are being programmed by common code, Marvell-specific code would > restrict its role to setting up config-space access functions, leaving > the actual touching of the registers to the common code. > > 2) Marvell chips have additional non-standard per-root-port registers > that generic PCI code would not understand. These registers would be > touched only by Marvell-specific code. > > The two kinds of registers are adjacent in MMIO space. However, unless > I am misunderstanding this MV78230 manual, the highest "config header" > register index is 0x134, well below the 0x1000 size limit of a PCIe > config header. Some of the extra registers are at 0x8xx, and others are > above 0x1800. The register block you are looking at is for the cores 'target end port mode', and the MMIO version is for internal use only, to allow the CPU to configure RO bits, and other tasks. The target core will allow access to that space via config cycles, either internally (through the indirection register) or externally (from the off-chip root complex) generated. 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). > For requirement (2), the top node has a reg property listing the > portions of the address space that are consumed by the driver at the top > level instead of being passed through to the PCI addressing domain. E.g. > > reg = <0xd0040800 0x1800>, <0xd0080800 0x1800>; Okay, so this is agreeing with what Thomas already has: http://www.spinics.net/lists/arm-kernel/msg228749.html With your two notes: - Correct the pcie@x,y to match the OF spec - Add a 'device_type = "pci"' to the pcie-controller node Is that right? > I said that it was bogus to use size=0x2000 for a config space header. > That was based on an interpretation - which I now dislike - of the > meaning of 3-cell config addresses. By that old interpretation, > size=0x800 would also be bogus, because bits 10-8 of the config address > are for the function number. Hum, I thought the spec was pretty clear on this point: 00 denotes Configuration Space, in which case: [..] bbbbbbbb,ddddd,fff,rrrrrrrrr is the Configuration Space address hh...hh,ll...ll must be zero And also the text at 2.1.4.4.. So you get an 8 bit register offset, placed in the highest DW.. Can you read things another way? Is there an updated spec that supports PCI-E extended config space? > Consider the following question, which I have never previously > considered, at least not explicitly: > > Q: What would be the 3-cell representation of the Command/Status > register address (offset 4) in device 1, function 1? Well, see section 11.1.2, where it provides an example for the 'Expansion ROM base address register (0x30)' as being: 02xxxx30 00000000 00000000 Also the f-code bindings say: The data type 'config-addr' refers to the 'phys.hi' cell of the numerical representation of a Configuration Space address. So there is an strong convention to use the 'r' bits as the offset.. 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? > > Two things bothered me > > - Describing a CPU MMIO mapping with a config address space seems > > wonky > > I agree that mapping config space is sort of a jarring concept, but I > think that's because PCs have polluted the mindspace, not because > there I'm well aware of MMIO config space acces, that isn't so jarring. What is so jarring is the idea that the OF translation would work like: <0x00000900 0 0> -> 0xd4004000 <0x00000901 0 0> -> 0xd4004001 Which is completely unlike any ranges translation I've ever seen. Basically, I look at how the register offset is encoded in the higest dword plus the statement in section 12, and and conclude the there wasn't an intention to model a memory map'd config space through OF. What am I missing here? Regards, Jason -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html