From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Subject: Re: [RFC PATCH v4 3/5] PCI: Check platform specific ECAM quirks To: Duc Dang , Christopher Covington References: <1467100442-28078-1-git-send-email-tn@semihalf.com> <1467100442-28078-4-git-send-email-tn@semihalf.com> <577275EA.7010002@codeaurora.org> Cc: Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , Rafael Wysocki , Hanjun Guo , Lorenzo Pieralisi , Sinan Kaya , Jayachandran C , Robert Richter , Marcin Wojtas , Liviu Dudau , David Daney , Yijing Wang , Mark Salter , linux-pci@vger.kernel.org, linux-arm , linux-acpi@vger.kernel.org, Linux Kernel Mailing List , linaro-acpi@lists.linaro.org, Jon Masters , Andrea Gallo , jeremy.linton@arm.com, Dongdong Liu , Gabriele Paoloni , Jeffrey Hugo , Ard Biesheuvel From: Tomasz Nowicki Message-ID: <5773A773.8040700@semihalf.com> Date: Wed, 29 Jun 2016 12:48:19 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed List-ID: On 28.06.2016 18:12, Duc Dang wrote: > On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington > wrote: >> Hi Tomasz, >> >> On 06/28/2016 03:54 AM, Tomasz Nowicki wrote: >> >>> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c >>> new file mode 100644 >>> index 0000000..fb2b184 >>> --- /dev/null >>> +++ b/drivers/pci/host/mcfg-quirks.c >>> @@ -0,0 +1,88 @@ >> >>> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f, >>> + struct acpi_table_header *mcfg_header) >>> +{ >>> + int olen = min_t(u8, strlen(f->oem_id), ACPI_OEM_ID_SIZE); >>> + int tlen = min_t(u8, strlen(f->oem_table_id), ACPI_OEM_TABLE_ID_SIZE); >>> + >>> + return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) && >>> + !strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) && >>> + f->oem_revision == mcfg_header->oem_revision); >>> +} >> >> Ard's comments on v3 included: >> >> "... exact OEM table/rev id matches ..." >> "... substring match ... out of the question ..." >> >> I originally advocated the substring match approach because >> space-padding the input strings was unfamiliar. But given that some >> vendors have a "PLAT " then "PLAT2 " naming scheme, where the >> former needs quirks and the latter (hopefully) doesn't, I agree with Ard >> and think space-padded inputs is the better way to go. Sorry for the >> lack of foresight. > > I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the > next version of the SoC) > is common. So yes, matching full string is better as we can use "PLAT2 " > in MCFG table and not worry about the "PLAT" sub-string match causes the quirk > to be applied unintentionally. > Sorry, I forgot to address Ard's comment. Note that platforms already shipped where OEM string has no padding will have change the firmware or add 0 padding to our quirk array IDs. So how about: static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f, struct acpi_table_header *mcfg_header) { int olen = strnlen(mcfg_header->oem_id, ACPI_OEM_ID_SIZE); int tlen = strnlen(mcfg_header->oem_table_id, ACPI_OEM_TABLE_ID_SIZE); if (olen != strlen(f->oem_id) || tlen != strlen(f->oem_table_id)) return false; return (!strncmp(f->oem_id, mcfg_header->oem_id, olen) && !strncmp(f->oem_table_id, mcfg_header->oem_table_id, tlen) && f->oem_revision == mcfg_header->oem_revision); } This should work for all cases: 1. "PLAT" 2. "PLAT " padding 3. No need to change existing firmware Tomasz