On Thursday, September 28, 2017 1:49 PM, Bjorn Helgaas wrote: > On Thu, Sep 28, 2017 at 08:51:43AM -0700, Ard Biesheuvel wrote: > > On 26 September 2017 at 10:32, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > > On Mon, Aug 28, 2017 at 07:04:36PM +0100, Ard Biesheuvel wrote: > > >> Some implementations of the Synopsys Designware PCIe controller > implement > > >> a so-called ECAM shift mode, which allows a static memory window to > be > > >> configured that covers the configuration space of the entire bus > range. > > > >> Note that, unlike most drivers for this IP, this driver does not > expose > > >> a fake bridge device at B/D/F 00:00.0. There is no point in doing so, > > >> given that this is not a true bridge, and does not require any > windows > > >> to be configured in order for the downstream device to operate > correctly. > > >> Omitting it also prevents the PCI resource allocation routines from > > >> handing out BAR space to it unnecessarily. > > > > > > This is a tangent, but does this mean the other drivers do not need to > > > expose a fake 00:00.0 device either? > > > > To be honest, I am not so sure anymore. I am seeing some issues in > > ASPM code making the assumption that any device which is not a root > > port has a parent. If this is mandated by the spec, I guess there > > isn't a whole lot we can do except expose a fake root port on b/d/f > > 0/0/0. This used to work fine, though, and I have to confirm whether > > the issues I am seeing currently are due to different hardware or > > changes in the software. > > I agree that our ASPM code had some assumptions that any non-root port > device should have a parent. I think the spec also makes that > assumption, but I haven't found an explicit mandate. And there *are* > systems lacking root ports: > > http://lkml.kernel.org/r/1439808478-23253-1-git-send-email- > wangyijing@xxxxxxxxxx > > We fixed one such assumption in that thread, but I wouldn't be > surprised if more remain. If there are, I think we should fix the > code to remove the assumption. > > > > This really doesn't have any DesignWare specifics in it, and it seems > > > more related to drivers/pci/host/pci-host-generic.c than to anything > > > in drivers/pci/dwc. Maybe it should be > > > drivers/pci/host/pci-host-generic-quirks.c or something? That's > > > unwieldy, I admit. > > > > I don't care where we put it, and I am fine with owning it if you prefer. > > I think Will's idea of teaching pci-host-generic to deal with this is > perfect. I agree. I cannot find any reason to create new dwc-specific file. Reusing 'pci-host-generic.c' looks better. Maybe 'pci-host-generic.c with quirks' will be good. Best regards, Jingoo Han > > > >> +config PCIE_DW_HOST_ECAM > > >> + bool "Synopsys DesignWare PCIe controller in ECAM mode" > > >> + depends on OF && PCI > > >> + select PCI_HOST_COMMON > > >> + select IRQ_DOMAIN > > >> + help > > >> + Add support for Synopsys DesignWare PCIe controllers > configured > > >> + by the firmware into ECAM shift mode. In some cases, these are > > >> + fully ECAM compliant, in which case the pci-host-generic > driver > > >> + may be used instead. > > > > > > This doesn't quite read right. It sounds like a controller in ECAM > > > shift mode might be fully ECAM compliant, but I don't think that's > > > what you intended. > > > > Yes, that is what I mean. ECAM shift mode results in a fully compliant > > ECAM config space iff the IP was synthesized with a 32 KB granularity > > for the iATU windows. The default is 64 KB, though, in which case you > > need this driver. > > OK. I'm trying to figure out how I as a user would know whether to > select this option. Maybe the config option will go away if you add > the smarts to pci-host-generic? > > Bjorn
