Hi Lorenzo > -----Original Message----- > From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@xxxxxxx] > Sent: 23 May 2016 11:57 > To: Ard Biesheuvel > Cc: Gabriele Paoloni; Jon Masters; Tomasz Nowicki; helgaas@xxxxxxxxxx; > arnd@xxxxxxxx; will.deacon@xxxxxxx; catalin.marinas@xxxxxxx; > rafael@xxxxxxxxxx; hanjun.guo@xxxxxxxxxx; okaya@xxxxxxxxxxxxxx; > jchandra@xxxxxxxxxxxx; linaro-acpi@xxxxxxxxxxxxxxxx; linux- > pci@xxxxxxxxxxxxxxx; dhdang@xxxxxxx; Liviu.Dudau@xxxxxxx; > ddaney@xxxxxxxxxxxxxxxxxx; jeremy.linton@xxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; > robert.richter@xxxxxxxxxxxxxxxxxx; Suravee.Suthikulpanit@xxxxxxx; > msalter@xxxxxxxxxx; Wangyijing; mw@xxxxxxxxxxxx; > andrea.gallo@xxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH V7 00/11] Support for generic ACPI based PCI host > controller > > On Fri, May 20, 2016 at 11:14:03AM +0200, Ard Biesheuvel wrote: > > On 20 May 2016 at 10:40, Gabriele Paoloni > <gabriele.paoloni@xxxxxxxxxx> wrote: > > > Hi Ard > > > > > >> -----Original Message----- > > >> From: Ard Biesheuvel [mailto:ard.biesheuvel@xxxxxxxxxx] > > [...] > > >> > > >> Is the PCIe root complex so special that you cannot simply > describe an > > >> implementation that is not PNP0408 compatible as something else, > under > > >> its own unique HID? If everybody is onboard with using ACPI, how > is > > >> this any different from describing other parts of the platform > > >> topology? Even if the SBSA mandates generic PCI, they already > deviated > > >> from that when they built the hardware, so pretending that it is a > > >> PNP0408 with quirks really does not buy us anything. > > > > > > From my understanding we want to avoid this as this would allow > each > > > vendor to come up with his own code and it would be much more > effort > > > for the PCI maintainer to rework the PCI framework to accommodate > X86 > > > and "all" ARM64 Host Controllers... > > > > > > I guess this approach is too risky and we want to avoid this. > Through > > > standardization we can more easily maintain the code and scale it > to > > > multiple SoCs... > > > > > > So this is my understanding; maybe Jon, Tomasz or Lorenzo can give > > > a bit more explanation... > > > > > > > OK, so that boils down to recommending to vendors to represent known > > non-compliant hardware as compliant, just so that we don't have to > > change the code to support additional flavors of ECAM ? It's fine to > > be pragmatic, but that sucks. > > > > We keep confusing the x86 case with the ARM case here: for x86, they > > needed to deal with broken hardware *after* the fact, and all they > > could do is find /some/ distinguishing feature in order to guess > which > > exact hardware they might be running on. For arm64, it is the > opposite > > case. We are currently in a position where we can demand vendors to > > comply with the standards they endorsed themselves, and (ab)using > ACPI > > + DMI as a de facto platform description rather than plain ACPI makes > > me think the DT crowd were actually right from the beginning. It > > *directly* violates the standardization principle, since it requires > a > > priori knowledge inside the OS that a certain 'generic' device must > be > > driven in a special way. > > > > So can anyone comment on the feasibility of adding support for > devices > > with vendor specific HIDs (and no generic CIDs) to the current ACPI > > ECAM driver in Linux? > > Host bridges in ACPI are handled through PNP0A08/PNP0A03 ids, and > most of the arch specific code is handled in the respective arch > directories (X86 and IA64, even though IA64 does not rely on ECAM/MCFG > for > PCI ops), it is not a driver per-se, PNP0A08/PNP0A03 are detected > through > ACPI scan handlers and the respective arch code (ie pci_acpi_scan_root) > sets-up resources AND config space on an arch specific basis. > > X86 deals with that with code in arch/x86 that sets-up the pci_raw_ops > on a platform specific basis (and it is not nice, but it works because > as you all know the number of platforms in X86 world is contained). > > Will this happen for ARM64 in arch/arm64 based on vendor specific > HIDs ? > > No. > > So given the current state of play (we were requested to move the > arch/arm64 specific ACPI PCI bits to arch/arm64), we would end up > with arch/arm64 code requiring code in /drivers to set-up pci_ops > in a platform specific way, it is horrible, if feasible at all. > > The only way this can be implemented is by pretending that the > ACPI/PCI arch/arm64 implementation is generic code (that's what this > series does), move it to /drivers (where it is in this series), and > implement _DSD vendor specific bindings (per HID) to set-up the pci > operations; whether this solution should go upstream, given that it > is just a short-term solution for early platforms bugs, it is another > story and my personal answer is no. I think it shouldn't be too bad to move quirk handling mechanism to arch/arm64. Effectively we would not move platform specific code into arch/arm64 but just the mechanism checking if there is any quirk that is defined. i.e.: extern struct pci_cfg_fixup __start_acpi_mcfg_fixups[]; extern struct pci_cfg_fixup __end_acpi_mcfg_fixups[]; static struct pci_ecam_ops *pci_acpi_get_ops(struct acpi_pci_root *root) { int bus_num = root->secondary.start; int domain = root->segment; struct pci_cfg_fixup *f; /* * Match against platform specific quirks and return corresponding * CAM ops. * * First match against PCI topology <domain:bus> then use DMI or * custom match handler. */ for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups; f++) { if ((f->domain == domain || f->domain == PCI_MCFG_DOMAIN_ANY) && (f->bus_num == bus_num || f->bus_num == PCI_MCFG_BUS_ANY) && (f->system ? dmi_check_system(f->system) : 1) && (f->match ? f->match(f, root) : 1)) return f->ops; } /* No quirks, use ECAM */ return &pci_generic_ecam_ops; } Such quirks will be defined anyway in drivers/pci/host/ in the vendor specific quirk implementations. e.g. in HiSilicon case we would have DECLARE_ACPI_MCFG_FIXUP(NULL, hisi_pcie_match, &hisi_pcie_ecam_ops, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); in "drivers/pci/host/pcie-hisi-acpi.c " Thanks Gab > > Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html