Dear Bjorn Helgaas,

On Wed, 30 Jan 2013 11:52:15 -0700, Bjorn Helgaas wrote:

I'm most concerned about the stuff in drivers/pci.  I hesitate to
merge drivers/pci/sw-pci-pci-bridge.c as-is because it's a model
that's not connected to hardware and only works in a completely static
situation, and the rest of the PCI core can't really deal with that.

But I don't think supporting hotplug should be a show-stopper at this
point, either.  It sounds like we might be heading towards hooking
this up more directly to the Marvell hardware, which will make it more
arch-dependent.  Something like that could either go in arch/arm, or
in some not-quite-so-generic spot under drivers/pci.

If you really don't want sw-pci-pci-bridge.c in drivers/pci, then I can
make it a part of the drivers/pci/host/pci-mvebu.c driver itself. I
initially followed the idea started by Thierry Redding for the emulated
host bridge, but if you feel that this emulated PCI-to-PCI bridge is
too specific to this driver, then I'm fine with keeping it inside the
driver itself.

quoted

quoted

I also forgot about the bus number munging in mvebu_pcie_rd_conf().
The PCI core can update the bridge secondary/subordinate registers.
It looks like you don't support writing to them, and the read path
(pci_sw_pci_bridge_read()) looks like it doesn't do any translation
between the hardware and Linux bus numbers.  I don't understand the
system well enough to know if this is an issue.

Right. Could you explain a little bit for what reasons the PCI core
could update the secondary/subordinate registers, and to what
values it sets them?

The secondary/subordinate registers effectively define a bus number
aperture that tells the bridge which transactions to claim and forward
downstream.  When enumerating devices, we may update the subordinate
bus number to widen the aperture so we can enumerate an arbitrary tree
behind the bridge.  When we're finished, we'll probably narrow it by
updating the subordinate again, so the unused bus number space can be
used for other bridges.  I don't know the exact details of the
algorithm, and they're likely to change anyway, but pci_scan_bridge()
is where most of it happens.

It looks like your current system doesn't support trees below the
bridges, but hopefully we can make it so the generic enumeration
algorithms still work.

In practice, in our situation, there isn't a tree below the bridge.
There is one single device. I'd prefer to not implement features that I
cannot effectively test, and let the implementation of those additional
features to whoever will need them, and therefore be able to test them.

I guess that if I integrate the PCI-to-PCI bridge emulation code within
the Marvell driver, then I can keep it fairly limited to whatever the
Marvell PCI driver requires, no?

quoted

For now, I statically assign the secondary bus register value to be
X+1, where X is the number of the PCIe interface, since X=0 is
reserved for the root bus (which has the host bridge and the
PCI-to-PCI bridges).

That makes sense but limits you to a single bus (and really, a single
device since this is PCIe) below the bridge.

Which is exactly what is happening here.

quoted

Also, could you detail what kind of translation I should be doing
when reading the hardware and Linux bus numbers?

I'm hoping that the register Jason mentioned is enough to avoid the
need for translation.  If it's not, we can explore this a bit more.

Ok.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com