On Tue, 2019-03-26 at 12:17 +0000, Lorenzo Pieralisi wrote: > This code is basically identical to (apart from the string matching > the DBI resource) > > drivers/pci/controller/pcie-hisi.c > > because, as you said, that's a DW quirk that is really not > platform specific AFAICS. > > Not that I am really fond of the idea but in practice we can > create a quirk that applies to all host controllers DW based, > in case they want to boot with ACPI, this patch is basically > code duplication. Having chatted to Jonny in a little more detail... this isn't quite duplicate code. On the Annapurna implementation we have fixed the alignment constraints so we can just use pci_generic_config_read() directly instead of forcing alignment. We only need to filter out the "ghost" devices on bus zero. There might eventually be scope for some form of consolidation, but it doesn't really seem clear at this point that it would be worth it. There are three separate quirks needed for different versions of the DW controller. One is that the config space of the controller itself shows up multiple times in all slots of bus zero. The second is that bus zero cannot be accessed through ECAM and must be accessed through a separate MMIO region. The third is the requirement for 32-bit alignment of the host controller's config space (through the special MMIO region). Some vendors have managed to work around some of these issues, for example Annapurna fixing the alignment one. It looks like the three DT matches in pci-host-generic.c which use pci_dw_ecam_bus_ops are assuming the hardware suffers only issue #1 from the list above, and not the other two. The Annapurna hardware gives us a new combination, and that's why it isn't quite a duplicate. Again, there may be scope for consolidation in the future but it's not clear what it should look like. Especially as when exposed through DT, both the hisi and al versions seem to do things quite differently and need their own specific code there anyway. There will be a DT variant of the AL driver coming in the near future, but when it's exposed via ACPI in the "as close to ECAM compliant as we could make it in this iteration of the hardware" mode, it's relatively simple so we did that patch first.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
