Re: [RFC PATCH V2 1/2] ACPI/PCI: Match PCI config space accessors against platfrom specific ECAM quirks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Duc

在 2016/6/14 4:57, Duc Dang 写道:
On Mon, Jun 13, 2016 at 8:47 AM, Christopher Covington
<cov@xxxxxxxxxxxxxx> 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 <linux/kernel.h>
  #include <linux/pci.h>
  #include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+
+/* 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 <domain:bus> 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

.


--
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



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux