Hi Bjorn, Hi Thomas,
On 18.06.2016 01:46, Bjorn Helgaas wrote:
On Fri, Jun 03, 2016 at 07:57:34AM +0200, Stefan Roese wrote:
Hi Thomas,
On 02.06.2016 15:18, Thomas Petazzoni wrote:
On Thu, 2 Jun 2016 14:52:47 +0200, Stefan Roese wrote:
Add a check to mvebu_pcie_add_windows() to detect, if an MBus window is
already configured. If this is the case (base address, size, target and
attribute are identical), then this window is not created. This fixes
a problem I'm currently seeing on a custom Armada XP based board, which
generates this error upon PCI rescanning (in this case via sysfs):
$ echo 1 > /sys/bus/pci/rescan
mvebu_mbus: cannot add window '4:e8', conflicts with another window
mvebu-pcie soc:pcie-controller: Could not create MBus window at [mem 0x9e000000-0x9e0fffff]: -22
Thanks for the patch. I am not familiar with what happens during a PCI
rescan, but shouldn't the MBus windows be deleted and then re-added?
It does not happen in this rescan case.
The pci-mvebu driver deletes the MBus windows for a given PCI device
when invalid memory base/limits are set in the emulated PCI bridge.
Shouldn't this happen over a rescan?
I'm not sure what *should* happen upon rescan. But testing shows, that
no MBus window is removed upon rescan. I just tested what happens,
if the PCI devices are "removed" via sysfs. And this also does not
delete any of the MBus windows.
If it doesn't, then the other question is whether the check you're
adding should be done in the PCI driver or in the MBus driver.
In commit b566e782be32145664d96ada3e389f17d32742e5, we already relaxed
the checks done by the MBus driver, and since this commit we allow
different windows to have the same target/attribute.
Should the MBus driver also allow re-creating a window that
already exists, if all its properties are the same?
This sounds like a good idea to me. To move this detection / decision
one layer up. So that it will work for other drivers using this
MBus interface as well.
I don't know how you want to solve it, but it definitely should be
safe to write the bridge window registers multiple times, either with
the same values, a new invalid base/limit pair, or a new valid
base/limit pair.
A complete emulation should handle all those, including disposing of
an old valid window and replacing it with a new, different, valid
window.
And of course, it should handle individual 16-bit writes for the
base/limit of the 32-bit windows.
So this patch under discussion allows the re-creation of the identical
MBus window (a Marvell special internal bus). Which is a result of a
PCI re-scan with unchanged PCIe devices.
For a "complete emulation" as mentioned above, we would need to
remove all MBus windows matching the target and attribute for this
PCIe port and lane. This is usually one window - I can't currently
think of a situation with multiple MBus windows with the same
properties here.
Thomas, should I go this way and rework this patch to first remove
the MBus window(s) and then create the new one? Or what is your
preferred solution here?
Thanks,
Stefan
--
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
