From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751351AbcFNFwp (ORCPT ); Tue, 14 Jun 2016 01:52:45 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:31807 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750806AbcFNFwm (ORCPT ); Tue, 14 Jun 2016 01:52:42 -0400 Subject: Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks To: Duc Dang , Christopher Covington References: <1465822923-33599-1-git-send-email-liudongdong3@huawei.com> <1465822923-33599-2-git-send-email-liudongdong3@huawei.com> <575ED57F.6050503@codeaurora.org> CC: Bjorn Helgaas , Arnd Bergmann , "Will Deacon" , Catalin Marinas , Rafael Wysocki , Hanjun Guo , Lorenzo Pieralisi , , Jayachandran C , Tomasz Nowicki , , Marcin Wojtas , "Liviu Dudau" , David Daney , "Yijing Wang" , Suravee Suthikulpanit , Mark Salter , , linux-arm , , "Linux Kernel Mailing List" , , Jon Masters , Andrea Gallo , , Gabriele Paoloni , , From: Dongdong Liu Message-ID: <575F9B4D.8080501@huawei.com> Date: Tue, 14 Jun 2016 13:51:09 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.61.21.156] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090205.575F9B5F.005E,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 9c0407a4edbcca5c4c977a95b732aebe Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Duc 在 2016/6/14 4:57, Duc Dang 写道: > On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington > wrote: >> Hi Dongdong, >> >> On 06/13/2016 09:02 AM, Dongdong Liu wrote: >>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c >>> index d3c3e85..49612b3 100644 >>> --- a/drivers/acpi/pci_mcfg.c >>> +++ b/drivers/acpi/pci_mcfg.c >>> @@ -22,6 +22,10 @@ >>> #include >>> #include >>> #include >>> +#include >>> + >>> +/* Root pointer to the mapped MCFG table */ >>> +static struct acpi_table_mcfg *mcfg_table; >>> >>> /* Structure to hold entries from the MCFG table */ >>> struct mcfg_entry { >>> @@ -35,6 +39,38 @@ struct mcfg_entry { >>> /* List to save mcfg entries */ >>> static LIST_HEAD(pci_mcfg_list); >>> >>> +extern struct pci_cfg_fixup __start_acpi_mcfg_fixups[]; >>> +extern struct pci_cfg_fixup __end_acpi_mcfg_fixups[]; >>> + >>> +struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root) >>> +{ >>> + int bus_num = root->secondary.start; >>> + int domain = root->segment; >>> + struct pci_cfg_fixup *f; >>> + >>> + if (!mcfg_table) >>> + return &pci_generic_ecam_ops; >>> + >>> + /* >>> + * Match against platform specific quirks and return corresponding >>> + * CAM ops. >>> + * >>> + * First match against PCI topology then use OEM ID and >>> + * OEM revision from MCFG table standard header. >>> + */ >>> + 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) && >>> + (!strncmp(f->oem_id, mcfg_table->header.oem_id, >>> + ACPI_OEM_ID_SIZE)) && >>> + (!strncmp(f->oem_table_id, mcfg_table->header.oem_table_id, >>> + ACPI_OEM_TABLE_ID_SIZE))) >> >> This would just be a small convenience, but if the character count used here were >> >> min(strlen(f->oem_id), ACPI_OEM_ID_SIZE) >> >> then the parameters to DECLARE_ACPI_MCFG_FIXUP macro could be substrings and >> wouldn't need to be padded out to the full length. >> >>> + return f->ops; >>> + } >>> + /* No quirks, use ECAM */ >>> + return &pci_generic_ecam_ops; >>> +} >> >>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >>> index 7d63a66..088a1da 100644 >>> --- a/include/linux/pci-acpi.h >>> +++ b/include/linux/pci-acpi.h >>> @@ -25,6 +25,7 @@ static inline acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev) >>> extern phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle); >>> >>> extern phys_addr_t pci_mcfg_lookup(u16 domain, struct resource *bus_res); >>> +extern struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root); >>> >>> static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev) >>> { >>> @@ -72,6 +73,25 @@ struct acpi_pci_root_ops { >>> int (*prepare_resources)(struct acpi_pci_root_info *info); >>> }; >>> >>> +struct pci_cfg_fixup { >>> + struct pci_ecam_ops *ops; >>> + char *oem_id; >>> + char *oem_table_id; >>> + int domain; >>> + int bus_num; >>> +}; >>> + >>> +#define PCI_MCFG_DOMAIN_ANY -1 >>> +#define PCI_MCFG_BUS_ANY -1 >>> + >>> +/* Designate a routine to fix up buggy MCFG */ >>> +#define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, oem_table_id, dom, bus) \ >>> + static const struct pci_cfg_fixup \ >>> + __mcfg_fixup_##oem_id##oem_table_id##dom##bus \ >> >> I'm not entirely sure that this is the right fix--I'm pretty blindly >> following a GCC documentation suggestion [1]--but removing the first two >> preprocessor concatenation operators "##" solved the following build error >> for me. >> >> include/linux/pci-acpi.h:90:2: error: pasting "__mcfg_fixup_" and ""QCOM"" does not give a valid preprocessing token >> __mcfg_fixup_##oem_id##oem_table_id##dom##bus \ > > I think the problem is gcc is not happy with quoted string when > processing these tokens > (""QCOM"", the extra "" are added by gcc). So should we not concat > string tokens and > use the fixup definition in v1 of this RFC: > /* Designate a routine to fix up buggy MCFG */ > #define DECLARE_ACPI_MCFG_FIXUP(ops, oem_id, rev, dom, bus) \ > static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\ > __used __attribute__((__section__(".acpi_fixup_mcfg"), \ > aligned((sizeof(void *))))) = \ > { ops, oem_id, rev, dom, bus }; V1 fixup exist the redefinition error when compiling mutiple DECLARE_ACPI_MCFG_FIXUP with the same PCI_MCFG_DOMAIN_ANY and PCI_MCFG_BUS_ANY. #define EFI_ACPI_HISI_OEM_ID "HISI" #define EFI_ACPI_HISI_D02_OEM_TABLE_ID "HISI-D02" #define EFI_ACPI_HISI_D03_OEM_TABLE_ID "HISI-D03" DECLARE_ACPI_MCFG_FIXUP(&hisi_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID, EFI_ACPI_HISI_D02_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); DECLARE_ACPI_MCFG_FIXUP(&hisi_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID, EFI_ACPI_HISI_D03_OEM_TABLE_ID, PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); In file included from drivers/pci/host/pcie-hisi-acpi.c:15:0: include/linux/pci-acpi.h:98:43: error: redefinition of '__mcfg_fixup_systemPCI_MCFG_DOMAIN_ANYPCI_MCFG_BUS_ANY' static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\ ^ drivers/pci/host/pcie-hisi-acpi.c:215:1: note: in expansion of macro 'DECLARE_ACPI_MCFG_FIXUP' DECLARE_ACPI_MCFG_FIXUP(&hisi_pcie_ecam_ops, EFI_ACPI_HISI_OEM_ID, ^ include/linux/pci-acpi.h:98:43: note: previous definition of '__mcfg_fixup_systemPCI_MCFG_DOMAIN_ANYPCI_MCFG_BUS_ANY' was here static const struct pci_cfg_fixup __mcfg_fixup_##system##dom##bus\ ^ drivers/pci/host/pcie-hisi-acpi.c:212:1: note: in expansion of macro 'DECLARE_ACPI_MCFG_FIXUP' V2 fixup can resolve the redefinition error, but need to use macro. We can see that the name of macro is not replace with it's value in "__mcfg_fixup_EFI_ACPI_HISI_OEM_IDEFI_ACPI_HISI_D03_OEM_TABLE_IDPCI_MCFG_DOMAIN_ANYPCI_MCFG_BUS_ANY". Any good idea is appreciated. Thanks Dongdong > > Regards, > Duc Dang. > > >> ^ >> arch/arm64/kernel/pci.c:225:1: note: in expansion of macro ‘DECLARE_ACPI_MCFG_FIXUP’ >> DECLARE_ACPI_MCFG_FIXUP(&pci_32b_ecam_ops, "QCOM", "QDF2432", PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); >> ^ >> arch/arm64/kernel/pci.c:225:44: error: pasting ""QCOM"" and ""QDF2432"" does not give a valid preprocessing token >> DECLARE_ACPI_MCFG_FIXUP(&pci_32b_ecam_ops, "QCOM", "QDF2432", PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); >> ^ >> include/linux/pci-acpi.h:90:17: note: in definition of macro ‘DECLARE_ACPI_MCFG_FIXUP’ >> __mcfg_fixup_##oem_id##oem_table_id##dom##bus \ >> ^ >> arch/arm64/kernel/pci.c:225:52: error: pasting ""QDF2432"" and "PCI_MCFG_DOMAIN_ANY" does not give a valid preprocessi >> ng token >> DECLARE_ACPI_MCFG_FIXUP(&pci_32b_ecam_ops, "QCOM", "QDF2432", PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); >> ^ >> include/linux/pci-acpi.h:90:25: note: in definition of macro ‘DECLARE_ACPI_MCFG_FIXUP’ >> __mcfg_fixup_##oem_id##oem_table_id##dom##bus \ >> ^ >> arch/arm64/kernel/pci.c:225:44: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before string constant >> DECLARE_ACPI_MCFG_FIXUP(&pci_32b_ecam_ops, "QCOM", "QDF2432", PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY); >> ^ >> include/linux/pci-acpi.h:90:17: note: in definition of macro ‘DECLARE_ACPI_MCFG_FIXUP’ >> __mcfg_fixup_##oem_id##oem_table_id##dom##bus \ >> ^ >> make[1]: *** [arch/arm64/kernel/pci.o] Error 1 >> make: *** [arch/arm64/kernel] Error 2 >> >> 1. https://gcc.gnu.org/onlinedocs/cpp/Concatenation.html#Concatenation >> >>> + __used __attribute__((__section__(".acpi_fixup_mcfg"), \ >>> + aligned((sizeof(void *))))) = \ >>> + { ops, oem_id, oem_table_id, dom, bus }; >>> + >>> extern int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info); >>> extern struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, >>> struct acpi_pci_root_ops *ops, >>> >> >> Thanks, >> Cov >> >> -- >> Qualcomm Innovation Center, Inc. >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, >> a Linux Foundation Collaborative Project > > . >