[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Tomasz Nowicki]

Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
revision (the ones from standard header of MCFG table).

Static array is used to keep quirk entries. Each entry consists of
mentioned MCFG IDs along with custom pci_ops structure and initialization call.

As an example, the last patch presents quirk handling mechanism usage for
ThunderX PEM driver.

v4 -> v5
- rebase against v4.8-rc1
- rework to exact MCFG OEM ID, TABLE ID, rev match
  - use memcmp instead of strncmp
  - no substring match
- fix typos and dmesg message

Tomasz Nowicki (5):
  PCI: Embed pci_ecam_ops in pci_config_window structure
  PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
  PCI: Check platform specific ECAM quirks
  ARM64/PCI: Start using quirks handling for ACPI based PCI host
    controller
  PCI: thunder-pem: Support quirky configuration space access for ACPI
    based PCI host controller

 arch/arm64/kernel/pci.c            | 42 +----------------
 drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
 drivers/pci/ecam.c                 |  6 +--
 drivers/pci/host/Makefile          |  1 +
 drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
 drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
 drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
 include/linux/pci-acpi.h           |  5 ++
 include/linux/pci-ecam.h           |  2 +-
 9 files changed, 252 insertions(+), 57 deletions(-)
 create mode 100644 drivers/pci/host/mcfg-quirks.c
 create mode 100644 drivers/pci/host/mcfg-quirks.h

-- 
1.9.1

[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Tomasz Nowicki]

Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
revision (the ones from standard header of MCFG table).

Static array is used to keep quirk entries. Each entry consists of
mentioned MCFG IDs along with custom pci_ops structure and initialization call.

As an example, the last patch presents quirk handling mechanism usage for
ThunderX PEM driver.

v4 -> v5
- rebase against v4.8-rc1
- rework to exact MCFG OEM ID, TABLE ID, rev match
  - use memcmp instead of strncmp
  - no substring match
- fix typos and dmesg message

Tomasz Nowicki (5):
  PCI: Embed pci_ecam_ops in pci_config_window structure
  PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
  PCI: Check platform specific ECAM quirks
  ARM64/PCI: Start using quirks handling for ACPI based PCI host
    controller
  PCI: thunder-pem: Support quirky configuration space access for ACPI
    based PCI host controller

 arch/arm64/kernel/pci.c            | 42 +----------------
 drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
 drivers/pci/ecam.c                 |  6 +--
 drivers/pci/host/Makefile          |  1 +
 drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
 drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
 drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
 include/linux/pci-acpi.h           |  5 ++
 include/linux/pci-ecam.h           |  2 +-
 9 files changed, 252 insertions(+), 57 deletions(-)
 create mode 100644 drivers/pci/host/mcfg-quirks.c
 create mode 100644 drivers/pci/host/mcfg-quirks.h

-- 
1.9.1

[RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Tomasz Nowicki]

pci_config_window keeps pointer to pci_ecam_ops and every time
we want to deallocate pci_config_window (pci_ecam_free()) we need to make
sure to free pci_ecam_ops in case it was dynamically allocated prior to
pci_ecam_create() call.

To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
instead of just a pointer.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
To: Jayachandran C <jchandra@broadcom.com>
---
 arch/arm64/kernel/pci.c  | 2 +-
 drivers/pci/ecam.c       | 6 +++---
 include/linux/pci-ecam.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index acf3872..981e828 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 		return NULL;
 	}
 
-	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
+	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
 	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
 				   ri->cfg);
 	if (!bus)
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 43ed08d..24242f0 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	cfg->parent = dev;
-	cfg->ops = ops;
+	cfg->ops = *ops;
 	cfg->busr.start = busr->start;
 	cfg->busr.end = busr->end;
 	cfg->busr.flags = IORESOURCE_BUS;
@@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
 			       int where)
 {
 	struct pci_config_window *cfg = bus->sysdata;
-	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
+	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
 	unsigned int busn = bus->number;
 	void __iomem *base;
 
@@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
 	if (per_bus_mapping)
 		base = cfg->winp[busn];
 	else
-		base = cfg->win + (busn << cfg->ops->bus_shift);
+		base = cfg->win + (busn << cfg->ops.bus_shift);
 	return base + (devfn << devfn_shift) + where;
 }
 
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index 7adad20..0ce2920 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -39,7 +39,7 @@ struct pci_config_window {
 	struct resource			res;
 	struct resource			busr;
 	void				*priv;
-	struct pci_ecam_ops		*ops;
+	struct pci_ecam_ops		ops;
 	union {
 		void __iomem		*win;	/* 64-bit single mapping */
 		void __iomem		**winp; /* 32-bit per-bus mapping */
-- 
1.9.1

[RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Tomasz Nowicki]

pci_config_window keeps pointer to pci_ecam_ops and every time
we want to deallocate pci_config_window (pci_ecam_free()) we need to make
sure to free pci_ecam_ops in case it was dynamically allocated prior to
pci_ecam_create() call.

To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
instead of just a pointer.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
To: Jayachandran C <jchandra@broadcom.com>
---
 arch/arm64/kernel/pci.c  | 2 +-
 drivers/pci/ecam.c       | 6 +++---
 include/linux/pci-ecam.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index acf3872..981e828 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 		return NULL;
 	}
 
-	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
+	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
 	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
 				   ri->cfg);
 	if (!bus)
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 43ed08d..24242f0 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	cfg->parent = dev;
-	cfg->ops = ops;
+	cfg->ops = *ops;
 	cfg->busr.start = busr->start;
 	cfg->busr.end = busr->end;
 	cfg->busr.flags = IORESOURCE_BUS;
@@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
 			       int where)
 {
 	struct pci_config_window *cfg = bus->sysdata;
-	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
+	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
 	unsigned int busn = bus->number;
 	void __iomem *base;
 
@@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
 	if (per_bus_mapping)
 		base = cfg->winp[busn];
 	else
-		base = cfg->win + (busn << cfg->ops->bus_shift);
+		base = cfg->win + (busn << cfg->ops.bus_shift);
 	return base + (devfn << devfn_shift) + where;
 }
 
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index 7adad20..0ce2920 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -39,7 +39,7 @@ struct pci_config_window {
 	struct resource			res;
 	struct resource			busr;
 	void				*priv;
-	struct pci_ecam_ops		*ops;
+	struct pci_ecam_ops		ops;
 	union {
 		void __iomem		*win;	/* 64-bit single mapping */
 		void __iomem		**winp; /* 32-bit per-bus mapping */
-- 
1.9.1

[RFC PATCH V5 2/5] PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver

[Tomasz Nowicki]

pci_acpi_setup_ecam_mapping() is not really ARM64 specific so move it out
of arch/arm64/ directory. In preparation for adding MCFG quirk handling
extend pci_acpi_setup_ecam_mapping() functionality to accept custom
PCI config accessors (function's argument).

For ARM64 ACPI based PCI host controller we still use pci_generic_ecam_ops.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 arch/arm64/kernel/pci.c  | 41 ++---------------------------------------
 drivers/acpi/pci_mcfg.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |  3 +++
 3 files changed, 45 insertions(+), 39 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 981e828..2e7bed4 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -114,44 +114,6 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 	return 0;
 }
 
-/*
- * Lookup the bus range for the domain in MCFG, and set up config space
- * mapping.
- */
-static struct pci_config_window *
-pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
-{
-	struct resource *bus_res = &root->secondary;
-	u16 seg = root->segment;
-	struct pci_config_window *cfg;
-	struct resource cfgres;
-	unsigned int bsz;
-
-	/* Use address from _CBA if present, otherwise lookup MCFG */
-	if (!root->mcfg_addr)
-		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
-
-	if (!root->mcfg_addr) {
-		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
-			seg, bus_res);
-		return NULL;
-	}
-
-	bsz = 1 << pci_generic_ecam_ops.bus_shift;
-	cfgres.start = root->mcfg_addr + bus_res->start * bsz;
-	cfgres.end = cfgres.start + resource_size(bus_res) * bsz - 1;
-	cfgres.flags = IORESOURCE_MEM;
-	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res,
-			      &pci_generic_ecam_ops);
-	if (IS_ERR(cfg)) {
-		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
-			seg, bus_res, PTR_ERR(cfg));
-		return NULL;
-	}
-
-	return cfg;
-}
-
 /* release_info: free resources allocated by init_info */
 static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
 {
@@ -177,7 +139,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	if (!ri)
 		return NULL;
 
-	ri->cfg = pci_acpi_setup_ecam_mapping(root);
+	ri->cfg = pci_acpi_setup_ecam_mapping(root,
+					      &pci_generic_ecam_ops.pci_ops);
 	if (!ri->cfg) {
 		kfree(ri);
 		return NULL;
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index b5b376e..331b560 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
 
 /* Structure to hold entries from the MCFG table */
 struct mcfg_entry {
@@ -52,6 +53,45 @@ phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
 	return 0;
 }
 
+/*
+ * Lookup the bus range for the domain in MCFG, and set up config space
+ * mapping.
+ */
+struct pci_config_window *
+pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops)
+{
+	struct resource *bus_res = &root->secondary;
+	u16 seg = root->segment;
+	struct pci_config_window *cfg;
+	struct resource cfgres;
+	struct pci_ecam_ops ecam_ops = {
+			.bus_shift = 20,
+			.pci_ops = *ops,
+	};
+
+	/* Use address from _CBA if present, otherwise lookup MCFG */
+	if (!root->mcfg_addr)
+		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
+
+	if (!root->mcfg_addr) {
+		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
+			seg, bus_res);
+		return NULL;
+	}
+
+	cfgres.start = root->mcfg_addr + (bus_res->start << 20);
+	cfgres.end = cfgres.start + (resource_size(bus_res) << 20) - 1;
+	cfgres.flags = IORESOURCE_MEM;
+	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res, &ecam_ops);
+	if (IS_ERR(cfg)) {
+		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
+			seg, bus_res, PTR_ERR(cfg));
+		return NULL;
+	}
+
+	return cfg;
+}
+
 static __init int pci_mcfg_parse(struct acpi_table_header *header)
 {
 	struct acpi_table_mcfg *mcfg;
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..e9bfe00 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -26,6 +26,9 @@ 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_config_window *
+pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
+
 static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
 {
 	struct pci_bus *pbus = pdev->bus;
-- 
1.9.1

[RFC PATCH V5 2/5] PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver

[Tomasz Nowicki]

pci_acpi_setup_ecam_mapping() is not really ARM64 specific so move it out
of arch/arm64/ directory. In preparation for adding MCFG quirk handling
extend pci_acpi_setup_ecam_mapping() functionality to accept custom
PCI config accessors (function's argument).

For ARM64 ACPI based PCI host controller we still use pci_generic_ecam_ops.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 arch/arm64/kernel/pci.c  | 41 ++---------------------------------------
 drivers/acpi/pci_mcfg.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |  3 +++
 3 files changed, 45 insertions(+), 39 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 981e828..2e7bed4 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -114,44 +114,6 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 	return 0;
 }
 
-/*
- * Lookup the bus range for the domain in MCFG, and set up config space
- * mapping.
- */
-static struct pci_config_window *
-pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
-{
-	struct resource *bus_res = &root->secondary;
-	u16 seg = root->segment;
-	struct pci_config_window *cfg;
-	struct resource cfgres;
-	unsigned int bsz;
-
-	/* Use address from _CBA if present, otherwise lookup MCFG */
-	if (!root->mcfg_addr)
-		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
-
-	if (!root->mcfg_addr) {
-		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
-			seg, bus_res);
-		return NULL;
-	}
-
-	bsz = 1 << pci_generic_ecam_ops.bus_shift;
-	cfgres.start = root->mcfg_addr + bus_res->start * bsz;
-	cfgres.end = cfgres.start + resource_size(bus_res) * bsz - 1;
-	cfgres.flags = IORESOURCE_MEM;
-	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res,
-			      &pci_generic_ecam_ops);
-	if (IS_ERR(cfg)) {
-		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
-			seg, bus_res, PTR_ERR(cfg));
-		return NULL;
-	}
-
-	return cfg;
-}
-
 /* release_info: free resources allocated by init_info */
 static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
 {
@@ -177,7 +139,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	if (!ri)
 		return NULL;
 
-	ri->cfg = pci_acpi_setup_ecam_mapping(root);
+	ri->cfg = pci_acpi_setup_ecam_mapping(root,
+					      &pci_generic_ecam_ops.pci_ops);
 	if (!ri->cfg) {
 		kfree(ri);
 		return NULL;
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index b5b376e..331b560 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
 
 /* Structure to hold entries from the MCFG table */
 struct mcfg_entry {
@@ -52,6 +53,45 @@ phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
 	return 0;
 }
 
+/*
+ * Lookup the bus range for the domain in MCFG, and set up config space
+ * mapping.
+ */
+struct pci_config_window *
+pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops)
+{
+	struct resource *bus_res = &root->secondary;
+	u16 seg = root->segment;
+	struct pci_config_window *cfg;
+	struct resource cfgres;
+	struct pci_ecam_ops ecam_ops = {
+			.bus_shift = 20,
+			.pci_ops = *ops,
+	};
+
+	/* Use address from _CBA if present, otherwise lookup MCFG */
+	if (!root->mcfg_addr)
+		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
+
+	if (!root->mcfg_addr) {
+		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
+			seg, bus_res);
+		return NULL;
+	}
+
+	cfgres.start = root->mcfg_addr + (bus_res->start << 20);
+	cfgres.end = cfgres.start + (resource_size(bus_res) << 20) - 1;
+	cfgres.flags = IORESOURCE_MEM;
+	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res, &ecam_ops);
+	if (IS_ERR(cfg)) {
+		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
+			seg, bus_res, PTR_ERR(cfg));
+		return NULL;
+	}
+
+	return cfg;
+}
+
 static __init int pci_mcfg_parse(struct acpi_table_header *header)
 {
 	struct acpi_table_mcfg *mcfg;
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 7d63a66..e9bfe00 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -26,6 +26,9 @@ 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_config_window *
+pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
+
 static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
 {
 	struct pci_bus *pbus = pdev->bus;
-- 
1.9.1

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

Some platforms may not be fully compliant with generic set of PCI config
accessors. For these cases we implement the way to overwrite accessors
set. Algorithm traverses available quirk list (static array),
matches against <oem_id, oem_table_id, rev, domain, bus number range> and
returns pci_config_window structure with fancy PCI config ops.
oem_id, oem_table_id and rev come from MCFG table standard header.

It is possible to define custom init call which is responsible for
setting up PCI configuration access accordingly to quirk requirements.
If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().

pci_generic_ecam_ops will be used for platforms free from quirks.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 drivers/pci/host/Makefile      |  1 +
 drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
 include/linux/pci-acpi.h       |  2 +
 4 files changed, 109 insertions(+)
 create mode 100644 drivers/pci/host/mcfg-quirks.c
 create mode 100644 drivers/pci/host/mcfg-quirks.h

diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 8843410..500cf78 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
 obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
+obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
new file mode 100644
index 0000000..aa9907b
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.c
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2016 Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+
+#include "mcfg-quirks.h"
+
+struct pci_cfg_fixup {
+	char oem_id[ACPI_OEM_ID_SIZE + 1];
+	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+	u32 oem_revision;
+	struct resource domain_range;
+	struct resource bus_range;
+	struct pci_ops *ops;
+	struct pci_config_window *(*init)(struct acpi_pci_root *root,
+					  struct pci_ops *ops);
+};
+
+#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1), NULL, 0)
+#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
+#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1),	\
+						NULL, IORESOURCE_BUS)
+#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
+
+static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
+/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
+};
+
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
+				 struct acpi_table_header *mcfg_header)
+{
+	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
+		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
+			ACPI_OEM_TABLE_ID_SIZE) &&
+		f->oem_revision == mcfg_header->oem_revision);
+}
+
+struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
+{
+	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
+	struct resource *bus_res = &root->secondary;
+	struct pci_cfg_fixup *f = mcfg_quirks;
+	struct acpi_table_header *mcfg_header;
+	acpi_status status;
+	int i;
+
+	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
+	if (ACPI_FAILURE(status))
+		return NULL;
+
+	/*
+	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
+	 * table ID, and OEM revision from MCFG table standard header.
+	 */
+	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
+		if (resource_contains(&f->domain_range, &dom_res) &&
+		    resource_contains(&f->bus_range, bus_res) &&
+		    pci_mcfg_fixup_match(f, mcfg_header)) {
+			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
+				 f->oem_id, f->oem_table_id, f->oem_revision);
+			return f->init ? f->init(root, f->ops) :
+				pci_acpi_setup_ecam_mapping(root, f->ops);
+		}
+	}
+	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
+}
diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
new file mode 100644
index 0000000..45cbd16
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.h
@@ -0,0 +1,20 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#ifndef __MCFG_QUIRKS_H__
+#define __MCFG_QUIRKS_H__
+
+/* MCFG quirks initialize call list */
+
+#endif /* __MCFG_QUIRKS_H__ */
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index e9bfe00..28cdce4 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,8 @@ 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_config_window *
+pci_mcfg_match_quirks(struct acpi_pci_root *root);
 
 extern struct pci_config_window *
 pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
-- 
1.9.1

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

Some platforms may not be fully compliant with generic set of PCI config
accessors. For these cases we implement the way to overwrite accessors
set. Algorithm traverses available quirk list (static array),
matches against <oem_id, oem_table_id, rev, domain, bus number range> and
returns pci_config_window structure with fancy PCI config ops.
oem_id, oem_table_id and rev come from MCFG table standard header.

It is possible to define custom init call which is responsible for
setting up PCI configuration access accordingly to quirk requirements.
If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().

pci_generic_ecam_ops will be used for platforms free from quirks.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 drivers/pci/host/Makefile      |  1 +
 drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
 include/linux/pci-acpi.h       |  2 +
 4 files changed, 109 insertions(+)
 create mode 100644 drivers/pci/host/mcfg-quirks.c
 create mode 100644 drivers/pci/host/mcfg-quirks.h

diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 8843410..500cf78 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
 obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
+obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
new file mode 100644
index 0000000..aa9907b
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.c
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2016 Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+
+#include "mcfg-quirks.h"
+
+struct pci_cfg_fixup {
+	char oem_id[ACPI_OEM_ID_SIZE + 1];
+	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+	u32 oem_revision;
+	struct resource domain_range;
+	struct resource bus_range;
+	struct pci_ops *ops;
+	struct pci_config_window *(*init)(struct acpi_pci_root *root,
+					  struct pci_ops *ops);
+};
+
+#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1), NULL, 0)
+#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
+#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1),	\
+						NULL, IORESOURCE_BUS)
+#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
+
+static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
+/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
+};
+
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
+				 struct acpi_table_header *mcfg_header)
+{
+	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
+		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
+			ACPI_OEM_TABLE_ID_SIZE) &&
+		f->oem_revision == mcfg_header->oem_revision);
+}
+
+struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
+{
+	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
+	struct resource *bus_res = &root->secondary;
+	struct pci_cfg_fixup *f = mcfg_quirks;
+	struct acpi_table_header *mcfg_header;
+	acpi_status status;
+	int i;
+
+	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
+	if (ACPI_FAILURE(status))
+		return NULL;
+
+	/*
+	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
+	 * table ID, and OEM revision from MCFG table standard header.
+	 */
+	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
+		if (resource_contains(&f->domain_range, &dom_res) &&
+		    resource_contains(&f->bus_range, bus_res) &&
+		    pci_mcfg_fixup_match(f, mcfg_header)) {
+			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
+				 f->oem_id, f->oem_table_id, f->oem_revision);
+			return f->init ? f->init(root, f->ops) :
+				pci_acpi_setup_ecam_mapping(root, f->ops);
+		}
+	}
+	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
+}
diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
new file mode 100644
index 0000000..45cbd16
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.h
@@ -0,0 +1,20 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#ifndef __MCFG_QUIRKS_H__
+#define __MCFG_QUIRKS_H__
+
+/* MCFG quirks initialize call list */
+
+#endif /* __MCFG_QUIRKS_H__ */
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index e9bfe00..28cdce4 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,8 @@ 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_config_window *
+pci_mcfg_match_quirks(struct acpi_pci_root *root);
 
 extern struct pci_config_window *
 pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
-- 
1.9.1

[RFC PATCH V5 4/5] ARM64/PCI: Start using quirks handling for ACPI based PCI host controller

[Tomasz Nowicki]

Since there are platforms which have non-compliant ECAM space we need to
override these accessors prior to PCI buses enumeration. In order to do
that we call pci_mcfg_match_quirks() to retrieve custom
pci_config_window structure. If no correlated quirk on list, use
fully ECAM compliant generic PCI config accessors.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 arch/arm64/kernel/pci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 2e7bed4..aa734bd 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -139,8 +139,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	if (!ri)
 		return NULL;
 
-	ri->cfg = pci_acpi_setup_ecam_mapping(root,
-					      &pci_generic_ecam_ops.pci_ops);
+	ri->cfg = pci_mcfg_match_quirks(root);
 	if (!ri->cfg) {
 		kfree(ri);
 		return NULL;
-- 
1.9.1

[RFC PATCH V5 4/5] ARM64/PCI: Start using quirks handling for ACPI based PCI host controller

[Tomasz Nowicki]

Since there are platforms which have non-compliant ECAM space we need to
override these accessors prior to PCI buses enumeration. In order to do
that we call pci_mcfg_match_quirks() to retrieve custom
pci_config_window structure. If no correlated quirk on list, use
fully ECAM compliant generic PCI config accessors.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 arch/arm64/kernel/pci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 2e7bed4..aa734bd 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -139,8 +139,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	if (!ri)
 		return NULL;
 
-	ri->cfg = pci_acpi_setup_ecam_mapping(root,
-					      &pci_generic_ecam_ops.pci_ops);
+	ri->cfg = pci_mcfg_match_quirks(root);
 	if (!ri->cfg) {
 		kfree(ri);
 		return NULL;
-- 
1.9.1

[RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Tomasz Nowicki]

Add infrastructure to support ThunderX PEM specific PCI configuration space
access for ACPI based PCI host controller. This involves:
1. New initialization call thunder_pem_cfg_init() to create configuration
space mapping based on hardcoded range addresses
2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
PEM specific register ranges
3. New quirk entry (for common quirk array) which identifies platform and
calls thunder_pem_cfg_init() from [1]

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/pci/host/mcfg-quirks.c     |  7 +++
 drivers/pci/host/mcfg-quirks.h     |  4 ++
 drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
 3 files changed, 94 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
index aa9907b..2993a72 100644
--- a/drivers/pci/host/mcfg-quirks.c
+++ b/drivers/pci/host/mcfg-quirks.c
@@ -44,6 +44,13 @@ struct pci_cfg_fixup {
 
 static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
 /*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
+#ifdef CONFIG_PCI_HOST_THUNDER_PEM
+	/* Pass2.0 */
+	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
+	  thunder_pem_cfg_init },
+	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
+	  thunder_pem_cfg_init },
+#endif
 };
 
 static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
index 45cbd16..411c667 100644
--- a/drivers/pci/host/mcfg-quirks.h
+++ b/drivers/pci/host/mcfg-quirks.h
@@ -16,5 +16,9 @@
 #define __MCFG_QUIRKS_H__
 
 /* MCFG quirks initialize call list */
+#ifdef CONFIG_PCI_HOST_THUNDER_PEM
+struct pci_config_window *
+thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops);
+#endif
 
 #endif /* __MCFG_QUIRKS_H__ */
diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
index 6abaf80..3f06e49 100644
--- a/drivers/pci/host/pci-thunder-pem.c
+++ b/drivers/pci/host/pci-thunder-pem.c
@@ -18,9 +18,12 @@
 #include <linux/init.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/pci-acpi.h>
 #include <linux/pci-ecam.h>
 #include <linux/platform_device.h>
 
+#include "mcfg-quirks.h"
+
 #define PEM_CFG_WR 0x28
 #define PEM_CFG_RD 0x30
 
@@ -284,6 +287,37 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
 	return pci_generic_config_write(bus, devfn, where, size, val);
 }
 
+#ifdef CONFIG_ACPI
+
+static struct resource thunder_pem_reg_res[] = {
+	[4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M),
+	[5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M),
+	[6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M),
+	[7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M),
+	[8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M),
+	[9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M),
+	[14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M),
+	[15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M),
+	[16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M),
+	[17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M),
+	[18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M),
+	[19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M),
+};
+
+static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
+{
+	struct acpi_device *adev = to_acpi_device(cfg->parent);
+	struct acpi_pci_root *root = acpi_driver_data(adev);
+
+	return &thunder_pem_reg_res[root->segment];
+}
+#else
+static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
+{
+	return NULL;
+}
+#endif
+
 static int thunder_pem_init(struct pci_config_window *cfg)
 {
 	struct device *dev = cfg->parent;
@@ -292,24 +326,24 @@ static int thunder_pem_init(struct pci_config_window *cfg)
 	struct thunder_pem_pci *pem_pci;
 	struct platform_device *pdev;
 
-	/* Only OF support for now */
-	if (!dev->of_node)
-		return -EINVAL;
-
 	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
 	if (!pem_pci)
 		return -ENOMEM;
 
-	pdev = to_platform_device(dev);
-
-	/*
-	 * The second register range is the PEM bridge to the PCIe
-	 * bus.  It has a different config access method than those
-	 * devices behind the bridge.
-	 */
-	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (acpi_disabled) {
+		pdev = to_platform_device(dev);
+
+		/*
+		 * The second register range is the PEM bridge to the PCIe
+		 * bus.  It has a different config access method than those
+		 * devices behind the bridge.
+		 */
+		res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	} else {
+		res_pem = thunder_pem_acpi_res(cfg);
+	}
 	if (!res_pem) {
-		dev_err(dev, "missing \"reg[1]\"property\n");
+		dev_err(dev, "missing configuration region\n");
 		return -EINVAL;
 	}
 
@@ -360,3 +394,39 @@ static struct platform_driver thunder_pem_driver = {
 	.probe = thunder_pem_probe,
 };
 builtin_platform_driver(thunder_pem_driver);
+
+#ifdef CONFIG_ACPI
+
+static struct resource thunder_pem_cfg_res[] = {
+	[4] = DEFINE_RES_MEM(0x88001f000000UL, 0x39 * SZ_16M),
+	[5] = DEFINE_RES_MEM(0x884057000000UL, 0x39 * SZ_16M),
+	[6] = DEFINE_RES_MEM(0x88808f000000UL, 0x39 * SZ_16M),
+	[7] = DEFINE_RES_MEM(0x89001f000000UL, 0x39 * SZ_16M),
+	[8] = DEFINE_RES_MEM(0x894057000000UL, 0x39 * SZ_16M),
+	[9] = DEFINE_RES_MEM(0x89808f000000UL, 0x39 * SZ_16M),
+	[14] = DEFINE_RES_MEM(0x98001f000000UL, 0x39 * SZ_16M),
+	[15] = DEFINE_RES_MEM(0x984057000000UL, 0x39 * SZ_16M),
+	[16] = DEFINE_RES_MEM(0x98808f000000UL, 0x39 * SZ_16M),
+	[17] = DEFINE_RES_MEM(0x99001f000000UL, 0x39 * SZ_16M),
+	[18] = DEFINE_RES_MEM(0x994057000000UL, 0x39 * SZ_16M),
+	[19] = DEFINE_RES_MEM(0x99808f000000UL, 0x39 * SZ_16M),
+};
+
+struct pci_config_window *
+thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops)
+{
+	struct resource *bus_res = &root->secondary;
+	u16 seg = root->segment;
+	struct pci_config_window *cfg;
+
+	cfg = pci_ecam_create(&root->device->dev, &thunder_pem_cfg_res[seg],
+			      bus_res, &pci_thunder_pem_ops);
+	if (IS_ERR(cfg)) {
+		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
+			seg, bus_res, PTR_ERR(cfg));
+		return NULL;
+	}
+
+	return cfg;
+}
+#endif
-- 
1.9.1

[RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Tomasz Nowicki]

Add infrastructure to support ThunderX PEM specific PCI configuration space
access for ACPI based PCI host controller. This involves:
1. New initialization call thunder_pem_cfg_init() to create configuration
space mapping based on hardcoded range addresses
2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
PEM specific register ranges
3. New quirk entry (for common quirk array) which identifies platform and
calls thunder_pem_cfg_init() from [1]

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/pci/host/mcfg-quirks.c     |  7 +++
 drivers/pci/host/mcfg-quirks.h     |  4 ++
 drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
 3 files changed, 94 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
index aa9907b..2993a72 100644
--- a/drivers/pci/host/mcfg-quirks.c
+++ b/drivers/pci/host/mcfg-quirks.c
@@ -44,6 +44,13 @@ struct pci_cfg_fixup {
 
 static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
 /*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
+#ifdef CONFIG_PCI_HOST_THUNDER_PEM
+	/* Pass2.0 */
+	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
+	  thunder_pem_cfg_init },
+	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
+	  thunder_pem_cfg_init },
+#endif
 };
 
 static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
index 45cbd16..411c667 100644
--- a/drivers/pci/host/mcfg-quirks.h
+++ b/drivers/pci/host/mcfg-quirks.h
@@ -16,5 +16,9 @@
 #define __MCFG_QUIRKS_H__
 
 /* MCFG quirks initialize call list */
+#ifdef CONFIG_PCI_HOST_THUNDER_PEM
+struct pci_config_window *
+thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops);
+#endif
 
 #endif /* __MCFG_QUIRKS_H__ */
diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
index 6abaf80..3f06e49 100644
--- a/drivers/pci/host/pci-thunder-pem.c
+++ b/drivers/pci/host/pci-thunder-pem.c
@@ -18,9 +18,12 @@
 #include <linux/init.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/pci-acpi.h>
 #include <linux/pci-ecam.h>
 #include <linux/platform_device.h>
 
+#include "mcfg-quirks.h"
+
 #define PEM_CFG_WR 0x28
 #define PEM_CFG_RD 0x30
 
@@ -284,6 +287,37 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
 	return pci_generic_config_write(bus, devfn, where, size, val);
 }
 
+#ifdef CONFIG_ACPI
+
+static struct resource thunder_pem_reg_res[] = {
+	[4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M),
+	[5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M),
+	[6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M),
+	[7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M),
+	[8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M),
+	[9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M),
+	[14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M),
+	[15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M),
+	[16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M),
+	[17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M),
+	[18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M),
+	[19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M),
+};
+
+static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
+{
+	struct acpi_device *adev = to_acpi_device(cfg->parent);
+	struct acpi_pci_root *root = acpi_driver_data(adev);
+
+	return &thunder_pem_reg_res[root->segment];
+}
+#else
+static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
+{
+	return NULL;
+}
+#endif
+
 static int thunder_pem_init(struct pci_config_window *cfg)
 {
 	struct device *dev = cfg->parent;
@@ -292,24 +326,24 @@ static int thunder_pem_init(struct pci_config_window *cfg)
 	struct thunder_pem_pci *pem_pci;
 	struct platform_device *pdev;
 
-	/* Only OF support for now */
-	if (!dev->of_node)
-		return -EINVAL;
-
 	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
 	if (!pem_pci)
 		return -ENOMEM;
 
-	pdev = to_platform_device(dev);
-
-	/*
-	 * The second register range is the PEM bridge to the PCIe
-	 * bus.  It has a different config access method than those
-	 * devices behind the bridge.
-	 */
-	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (acpi_disabled) {
+		pdev = to_platform_device(dev);
+
+		/*
+		 * The second register range is the PEM bridge to the PCIe
+		 * bus.  It has a different config access method than those
+		 * devices behind the bridge.
+		 */
+		res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	} else {
+		res_pem = thunder_pem_acpi_res(cfg);
+	}
 	if (!res_pem) {
-		dev_err(dev, "missing \"reg[1]\"property\n");
+		dev_err(dev, "missing configuration region\n");
 		return -EINVAL;
 	}
 
@@ -360,3 +394,39 @@ static struct platform_driver thunder_pem_driver = {
 	.probe = thunder_pem_probe,
 };
 builtin_platform_driver(thunder_pem_driver);
+
+#ifdef CONFIG_ACPI
+
+static struct resource thunder_pem_cfg_res[] = {
+	[4] = DEFINE_RES_MEM(0x88001f000000UL, 0x39 * SZ_16M),
+	[5] = DEFINE_RES_MEM(0x884057000000UL, 0x39 * SZ_16M),
+	[6] = DEFINE_RES_MEM(0x88808f000000UL, 0x39 * SZ_16M),
+	[7] = DEFINE_RES_MEM(0x89001f000000UL, 0x39 * SZ_16M),
+	[8] = DEFINE_RES_MEM(0x894057000000UL, 0x39 * SZ_16M),
+	[9] = DEFINE_RES_MEM(0x89808f000000UL, 0x39 * SZ_16M),
+	[14] = DEFINE_RES_MEM(0x98001f000000UL, 0x39 * SZ_16M),
+	[15] = DEFINE_RES_MEM(0x984057000000UL, 0x39 * SZ_16M),
+	[16] = DEFINE_RES_MEM(0x98808f000000UL, 0x39 * SZ_16M),
+	[17] = DEFINE_RES_MEM(0x99001f000000UL, 0x39 * SZ_16M),
+	[18] = DEFINE_RES_MEM(0x994057000000UL, 0x39 * SZ_16M),
+	[19] = DEFINE_RES_MEM(0x99808f000000UL, 0x39 * SZ_16M),
+};
+
+struct pci_config_window *
+thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops)
+{
+	struct resource *bus_res = &root->secondary;
+	u16 seg = root->segment;
+	struct pci_config_window *cfg;
+
+	cfg = pci_ecam_create(&root->device->dev, &thunder_pem_cfg_res[seg],
+			      bus_res, &pci_thunder_pem_ops);
+	if (IS_ERR(cfg)) {
+		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
+			seg, bus_res, PTR_ERR(cfg));
+		return NULL;
+	}
+
+	return cfg;
+}
+#endif
-- 
1.9.1

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Graeme Gregory]

On Mon, Aug 08, 2016 at 03:05:36PM +0200, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 

Series looks good to me.

Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>

> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
> 
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> 
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Graeme Gregory]

On Mon, Aug 08, 2016 at 03:05:36PM +0200, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 

Series looks good to me.

Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>

> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
> 
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> 
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Mark Salter]

On Mon, 2016-08-08 at 15:05 +0200, Tomasz Nowicki wrote:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite accessors
> set. Algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns pci_config_window structure with fancy PCI config ops.
> oem_id, oem_table_id and rev come from MCFG table standard header.
> 
> It is possible to define custom init call which is responsible for
> setting up PCI configuration access accordingly to quirk requirements.
> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> 
> pci_generic_ecam_ops will be used for platforms free from quirks.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  drivers/pci/host/Makefile      |  1 +
>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>  include/linux/pci-acpi.h       |  2 +
>  4 files changed, 109 insertions(+)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> 
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 8843410..500cf78 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> new file mode 100644
> index 0000000..aa9907b
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016 Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ioport.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "mcfg-quirks.h"
> +
> +struct pci_cfg_fixup {
> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	u32 oem_revision;
> +	struct resource domain_range;
> +	struct resource bus_range;
> +	struct pci_ops *ops;
> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
> +					  struct pci_ops *ops);
> +};
> +
> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1), NULL, 0)
> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1),	\
> +						NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
> +
> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
                                             ^^^^^^^^^^^^^^

I get section warnings because pci_cfg_fixup_match() is not
an init function.

WARNING: vmlinux.o(.text+0x3f6c74): Section mismatch in reference from the function pci_mcfg_match_quirks() to the variable .init.rodata:$d
The function pci_mcfg_match_quirks() references
the variable __initconst $d.
This is often because pci_mcfg_match_quirks lacks a __initconst 
annotation or the annotation of $d is wrong.

> +/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +};
> +
> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> +				 struct acpi_table_header *mcfg_header)
> +{
> +	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
> +		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
> +			ACPI_OEM_TABLE_ID_SIZE) &&
> +		f->oem_revision == mcfg_header->oem_revision);
> +}
> +
> +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
> +{
> +	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
> +	struct resource *bus_res = &root->secondary;
> +	struct pci_cfg_fixup *f = mcfg_quirks;
> +	struct acpi_table_header *mcfg_header;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
> +	if (ACPI_FAILURE(status))
> +		return NULL;
> +
> +	/*
> +	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
> +	 * table ID, and OEM revision from MCFG table standard header.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> +		if (resource_contains(&f->domain_range, &dom_res) &&
> +		    resource_contains(&f->bus_range, bus_res) &&
> +		    pci_mcfg_fixup_match(f, mcfg_header)) {
> +			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> +				 f->oem_id, f->oem_table_id, f->oem_revision);
> +			return f->init ? f->init(root, f->ops) :
> +				pci_acpi_setup_ecam_mapping(root, f->ops);
> +		}
> +	}
> +	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
> +}
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> new file mode 100644
> index 0000000..45cbd16
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -0,0 +1,20 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#ifndef __MCFG_QUIRKS_H__
> +#define __MCFG_QUIRKS_H__
> +
> +/* MCFG quirks initialize call list */
> +
> +#endif /* __MCFG_QUIRKS_H__ */
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index e9bfe00..28cdce4 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,8 @@ 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_config_window *
> +pci_mcfg_match_quirks(struct acpi_pci_root *root);
>  
>  extern struct pci_config_window *
>  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Mark Salter]

On Mon, 2016-08-08 at 15:05 +0200, Tomasz Nowicki wrote:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite accessors
> set. Algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns pci_config_window structure with fancy PCI config ops.
> oem_id, oem_table_id and rev come from MCFG table standard header.
> 
> It is possible to define custom init call which is responsible for
> setting up PCI configuration access accordingly to quirk requirements.
> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> 
> pci_generic_ecam_ops will be used for platforms free from quirks.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
> ?drivers/pci/host/Makefile??????|??1 +
> ?drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
> ?drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
> ?include/linux/pci-acpi.h???????|??2 +
> ?4 files changed, 109 insertions(+)
> ?create mode 100644 drivers/pci/host/mcfg-quirks.c
> ?create mode 100644 drivers/pci/host/mcfg-quirks.h
> 
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 8843410..500cf78 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
> ?obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
> ?obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> ?obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> new file mode 100644
> index 0000000..aa9907b
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016 Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.??See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ioport.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "mcfg-quirks.h"
> +
> +struct pci_cfg_fixup {
> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	u32 oem_revision;
> +	struct resource domain_range;
> +	struct resource bus_range;
> +	struct pci_ops *ops;
> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
> +					??struct pci_ops *ops);
> +};
> +
> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1), NULL, 0)
> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1),	\
> +						NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
> +
> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?^^^^^^^^^^^^^^

I get section warnings because pci_cfg_fixup_match() is not
an init function.

WARNING: vmlinux.o(.text+0x3f6c74): Section mismatch in reference from the function pci_mcfg_match_quirks() to the variable .init.rodata:$d
The function pci_mcfg_match_quirks() references
the variable __initconst $d.
This is often because pci_mcfg_match_quirks lacks a __initconst?
annotation or the annotation of $d is wrong.

> +/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +};
> +
> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> +				?struct acpi_table_header *mcfg_header)
> +{
> +	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
> +		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
> +			ACPI_OEM_TABLE_ID_SIZE) &&
> +		f->oem_revision == mcfg_header->oem_revision);
> +}
> +
> +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
> +{
> +	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
> +	struct resource *bus_res = &root->secondary;
> +	struct pci_cfg_fixup *f = mcfg_quirks;
> +	struct acpi_table_header *mcfg_header;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
> +	if (ACPI_FAILURE(status))
> +		return NULL;
> +
> +	/*
> +	?* First match against PCI topology <domain:bus> then use OEM ID, OEM
> +	?* table ID, and OEM revision from MCFG table standard header.
> +	?*/
> +	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> +		if (resource_contains(&f->domain_range, &dom_res) &&
> +		????resource_contains(&f->bus_range, bus_res) &&
> +		????pci_mcfg_fixup_match(f, mcfg_header)) {
> +			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> +				?f->oem_id, f->oem_table_id, f->oem_revision);
> +			return f->init ? f->init(root, f->ops) :
> +				pci_acpi_setup_ecam_mapping(root, f->ops);
> +		}
> +	}
> +	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
> +}
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> new file mode 100644
> index 0000000..45cbd16
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -0,0 +1,20 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.??See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#ifndef __MCFG_QUIRKS_H__
> +#define __MCFG_QUIRKS_H__
> +
> +/* MCFG quirks initialize call list */
> +
> +#endif /* __MCFG_QUIRKS_H__ */
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index e9bfe00..28cdce4 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,8 @@ 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_config_window *
> +pci_mcfg_match_quirks(struct acpi_pci_root *root);
> ?
> ?extern struct pci_config_window *
> ?pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Mark Salter]

On Mon, 2016-08-08 at 15:05 +0200, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
> 
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> 

Tested-by: Mark Salter <msalter@redhat.com>

[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Mark Salter]

On Mon, 2016-08-08 at 15:05 +0200, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
> ? - use memcmp instead of strncmp
> ? - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
> ? PCI: Embed pci_ecam_ops in pci_config_window structure
> ? PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
> ? PCI: Check platform specific ECAM quirks
> ? ARM64/PCI: Start using quirks handling for ACPI based PCI host
> ????controller
> ? PCI: thunder-pem: Support quirky configuration space access for ACPI
> ????based PCI host controller
> 
> ?arch/arm64/kernel/pci.c????????????| 42 +----------------
> ?drivers/acpi/pci_mcfg.c????????????| 40 ++++++++++++++++
> ?drivers/pci/ecam.c?????????????????|??6 +--
> ?drivers/pci/host/Makefile??????????|??1 +
> ?drivers/pci/host/mcfg-quirks.c?????| 93 ++++++++++++++++++++++++++++++++++++
> ?drivers/pci/host/mcfg-quirks.h?????| 24 ++++++++++
> ?drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
> ?include/linux/pci-acpi.h???????????|??5 ++
> ?include/linux/pci-ecam.h???????????|??2 +-
> ?9 files changed, 252 insertions(+), 57 deletions(-)
> ?create mode 100644 drivers/pci/host/mcfg-quirks.c
> ?create mode 100644 drivers/pci/host/mcfg-quirks.h
> 

Tested-by: Mark Salter <msalter@redhat.com>

Re: [RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

On 08.08.2016 17:34, Mark Salter wrote:
> On Mon, 2016-08-08 at 15:05 +0200, Tomasz Nowicki wrote:
>> Some platforms may not be fully compliant with generic set of PCI config
>> accessors. For these cases we implement the way to overwrite accessors
>> set. Algorithm traverses available quirk list (static array),
>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>> returns pci_config_window structure with fancy PCI config ops.
>> oem_id, oem_table_id and rev come from MCFG table standard header.
>>
>> It is possible to define custom init call which is responsible for
>> setting up PCI configuration access accordingly to quirk requirements.
>> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
>>
>> pci_generic_ecam_ops will be used for platforms free from quirks.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>  drivers/pci/host/Makefile      |  1 +
>>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>>  include/linux/pci-acpi.h       |  2 +
>>  4 files changed, 109 insertions(+)
>>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>>  create mode 100644 drivers/pci/host/mcfg-quirks.h
>>
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 8843410..500cf78 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
>> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
>> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
>> new file mode 100644
>> index 0000000..aa9907b
>> --- /dev/null
>> +++ b/drivers/pci/host/mcfg-quirks.c
>> @@ -0,0 +1,86 @@
>> +/*
>> + * Copyright (C) 2016 Semihalf
>> + *	Author: Tomasz Nowicki <tn@semihalf.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License, version 2, as
>> + * published by the Free Software Foundation (the "GPL").
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License version 2 (GPLv2) for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * version 2 (GPLv2) along with this source code.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/ioport.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci-acpi.h>
>> +#include <linux/pci-ecam.h>
>> +
>> +#include "mcfg-quirks.h"
>> +
>> +struct pci_cfg_fixup {
>> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
>> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
>> +	u32 oem_revision;
>> +	struct resource domain_range;
>> +	struct resource bus_range;
>> +	struct pci_ops *ops;
>> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
>> +					  struct pci_ops *ops);
>> +};
>> +
>> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
>> +						((end) - (start) + 1), NULL, 0)
>> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
>> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
>> +						((end) - (start) + 1),	\
>> +						NULL, IORESOURCE_BUS)
>> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
>> +
>> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
>                                              ^^^^^^^^^^^^^^
>
> I get section warnings because pci_cfg_fixup_match() is not
> an init function.
>
> WARNING: vmlinux.o(.text+0x3f6c74): Section mismatch in reference from the function pci_mcfg_match_quirks() to the variable .init.rodata:$d
> The function pci_mcfg_match_quirks() references
> the variable __initconst $d.
> This is often because pci_mcfg_match_quirks lacks a __initconst
> annotation or the annotation of $d is wrong.

Thanks Mark. I will fix it.

Tomasz

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

On 08.08.2016 17:34, Mark Salter wrote:
> On Mon, 2016-08-08 at 15:05 +0200, Tomasz Nowicki wrote:
>> Some platforms may not be fully compliant with generic set of PCI config
>> accessors. For these cases we implement the way to overwrite accessors
>> set. Algorithm traverses available quirk list (static array),
>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>> returns pci_config_window structure with fancy PCI config ops.
>> oem_id, oem_table_id and rev come from MCFG table standard header.
>>
>> It is possible to define custom init call which is responsible for
>> setting up PCI configuration access accordingly to quirk requirements.
>> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
>>
>> pci_generic_ecam_ops will be used for platforms free from quirks.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>  drivers/pci/host/Makefile      |  1 +
>>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>>  include/linux/pci-acpi.h       |  2 +
>>  4 files changed, 109 insertions(+)
>>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>>  create mode 100644 drivers/pci/host/mcfg-quirks.h
>>
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 8843410..500cf78 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
>> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
>> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
>> new file mode 100644
>> index 0000000..aa9907b
>> --- /dev/null
>> +++ b/drivers/pci/host/mcfg-quirks.c
>> @@ -0,0 +1,86 @@
>> +/*
>> + * Copyright (C) 2016 Semihalf
>> + *	Author: Tomasz Nowicki <tn@semihalf.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License, version 2, as
>> + * published by the Free Software Foundation (the "GPL").
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * General Public License version 2 (GPLv2) for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * version 2 (GPLv2) along with this source code.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/ioport.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci-acpi.h>
>> +#include <linux/pci-ecam.h>
>> +
>> +#include "mcfg-quirks.h"
>> +
>> +struct pci_cfg_fixup {
>> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
>> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
>> +	u32 oem_revision;
>> +	struct resource domain_range;
>> +	struct resource bus_range;
>> +	struct pci_ops *ops;
>> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
>> +					  struct pci_ops *ops);
>> +};
>> +
>> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
>> +						((end) - (start) + 1), NULL, 0)
>> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
>> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
>> +						((end) - (start) + 1),	\
>> +						NULL, IORESOURCE_BUS)
>> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
>> +
>> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
>                                              ^^^^^^^^^^^^^^
>
> I get section warnings because pci_cfg_fixup_match() is not
> an init function.
>
> WARNING: vmlinux.o(.text+0x3f6c74): Section mismatch in reference from the function pci_mcfg_match_quirks() to the variable .init.rodata:$d
> The function pci_mcfg_match_quirks() references
> the variable __initconst $d.
> This is often because pci_mcfg_match_quirks lacks a __initconst
> annotation or the annotation of $d is wrong.

Thanks Mark. I will fix it.

Tomasz

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Dongdong Liu]

在 2016/8/8 21:05, Tomasz Nowicki 写道:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
>
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
>
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
>
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>    - use memcmp instead of strncmp
>    - no substring match
> - fix typos and dmesg message
>
> Tomasz Nowicki (5):
>    PCI: Embed pci_ecam_ops in pci_config_window structure
>    PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>    PCI: Check platform specific ECAM quirks
>    ARM64/PCI: Start using quirks handling for ACPI based PCI host
>      controller
>    PCI: thunder-pem: Support quirky configuration space access for ACPI
>      based PCI host controller
>
>   arch/arm64/kernel/pci.c            | 42 +----------------
>   drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>   drivers/pci/ecam.c                 |  6 +--
>   drivers/pci/host/Makefile          |  1 +
>   drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>   drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>   drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>   include/linux/pci-acpi.h           |  5 ++
>   include/linux/pci-ecam.h           |  2 +-
>   9 files changed, 252 insertions(+), 57 deletions(-)
>   create mode 100644 drivers/pci/host/mcfg-quirks.c
>   create mode 100644 drivers/pci/host/mcfg-quirks.h
>

Based on the patchset, tested on HiSilicon D03 board with intel 82599 net card. It worked OK.

Tested-by: Dongdong Liu <liudongdong3@huawei.com>

Thanks
Dongdong

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Dongdong Liu]

在 2016/8/8 21:05, Tomasz Nowicki 写道:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
>
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
>
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
>
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>    - use memcmp instead of strncmp
>    - no substring match
> - fix typos and dmesg message
>
> Tomasz Nowicki (5):
>    PCI: Embed pci_ecam_ops in pci_config_window structure
>    PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>    PCI: Check platform specific ECAM quirks
>    ARM64/PCI: Start using quirks handling for ACPI based PCI host
>      controller
>    PCI: thunder-pem: Support quirky configuration space access for ACPI
>      based PCI host controller
>
>   arch/arm64/kernel/pci.c            | 42 +----------------
>   drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>   drivers/pci/ecam.c                 |  6 +--
>   drivers/pci/host/Makefile          |  1 +
>   drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>   drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>   drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>   include/linux/pci-acpi.h           |  5 ++
>   include/linux/pci-ecam.h           |  2 +-
>   9 files changed, 252 insertions(+), 57 deletions(-)
>   create mode 100644 drivers/pci/host/mcfg-quirks.c
>   create mode 100644 drivers/pci/host/mcfg-quirks.h
>

Based on the patchset, tested on HiSilicon D03 board with intel 82599 net card. It worked OK.

Tested-by: Dongdong Liu <liudongdong3@huawei.com>

Thanks
Dongdong

[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Dongdong Liu]

? 2016/8/8 21:05, Tomasz Nowicki ??:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
>
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
>
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
>
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>    - use memcmp instead of strncmp
>    - no substring match
> - fix typos and dmesg message
>
> Tomasz Nowicki (5):
>    PCI: Embed pci_ecam_ops in pci_config_window structure
>    PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>    PCI: Check platform specific ECAM quirks
>    ARM64/PCI: Start using quirks handling for ACPI based PCI host
>      controller
>    PCI: thunder-pem: Support quirky configuration space access for ACPI
>      based PCI host controller
>
>   arch/arm64/kernel/pci.c            | 42 +----------------
>   drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>   drivers/pci/ecam.c                 |  6 +--
>   drivers/pci/host/Makefile          |  1 +
>   drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>   drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>   drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>   include/linux/pci-acpi.h           |  5 ++
>   include/linux/pci-ecam.h           |  2 +-
>   9 files changed, 252 insertions(+), 57 deletions(-)
>   create mode 100644 drivers/pci/host/mcfg-quirks.c
>   create mode 100644 drivers/pci/host/mcfg-quirks.h
>

Based on the patchset, tested on HiSilicon D03 board with intel 82599 net card. It worked OK.

Tested-by: Dongdong Liu <liudongdong3@huawei.com>

Thanks
Dongdong

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Robert Richter]

On 08.08.16 15:05:36, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
> 
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h

For the whole series

Reviewed-by: Robert Richter <rrichter@cavium.com>
Tested-by: Robert Richter <rrichter@cavium.com>

Test included:

 [PATCH V7 0/8] Introduce ACPI world to ITS irqchip

Thanks Tomasz,

-Robert

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Robert Richter]

On 08.08.16 15:05:36, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
> 
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h

For the whole series

Reviewed-by: Robert Richter <rrichter@cavium.com>
Tested-by: Robert Richter <rrichter@cavium.com>

Test included:

 [PATCH V7 0/8] Introduce ACPI world to ITS irqchip

Thanks Tomasz,

-Robert

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Robert Richter]

On 08.08.16 15:05:36, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
> 
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h

For the whole series

Reviewed-by: Robert Richter <rrichter@cavium.com>
Tested-by: Robert Richter <rrichter@cavium.com>

Test included:

 [PATCH V7 0/8] Introduce ACPI world to ITS irqchip

Thanks Tomasz,

-Robert

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Robert Richter]

On 08.08.16 15:05:36, Tomasz Nowicki wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
> 
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
> 
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.
> 
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
> 
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
> 
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h

For the whole series

Reviewed-by: Robert Richter <rrichter@cavium.com>
Tested-by: Robert Richter <rrichter@cavium.com>

Test included:

 [PATCH V7 0/8] Introduce ACPI world to ITS irqchip

Thanks Tomasz,

-Robert

Re: [RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Robert Richter]

On 08.08.16 15:05:41, Tomasz Nowicki wrote:
> Add infrastructure to support ThunderX PEM specific PCI configuration space
> access for ACPI based PCI host controller. This involves:
> 1. New initialization call thunder_pem_cfg_init() to create configuration
> space mapping based on hardcoded range addresses
> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
> PEM specific register ranges
> 3. New quirk entry (for common quirk array) which identifies platform and
> calls thunder_pem_cfg_init() from [1]
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>

Acked-by: Robert Richter <rrichter@cavium.com>

> ---
>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  3 files changed, 94 insertions(+), 13 deletions(-)

Re: [RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Robert Richter]

On 08.08.16 15:05:41, Tomasz Nowicki wrote:
> Add infrastructure to support ThunderX PEM specific PCI configuration space
> access for ACPI based PCI host controller. This involves:
> 1. New initialization call thunder_pem_cfg_init() to create configuration
> space mapping based on hardcoded range addresses
> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
> PEM specific register ranges
> 3. New quirk entry (for common quirk array) which identifies platform and
> calls thunder_pem_cfg_init() from [1]
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>

Acked-by: Robert Richter <rrichter@cavium.com>

> ---
>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  3 files changed, 94 insertions(+), 13 deletions(-)

Re: [RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Robert Richter]

On 08.08.16 15:05:41, Tomasz Nowicki wrote:
> Add infrastructure to support ThunderX PEM specific PCI configuration space
> access for ACPI based PCI host controller. This involves:
> 1. New initialization call thunder_pem_cfg_init() to create configuration
> space mapping based on hardcoded range addresses
> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
> PEM specific register ranges
> 3. New quirk entry (for common quirk array) which identifies platform and
> calls thunder_pem_cfg_init() from [1]
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>

Acked-by: Robert Richter <rrichter@cavium.com>

> ---
>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  3 files changed, 94 insertions(+), 13 deletions(-)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Robert Richter]

On 08.08.16 15:05:41, Tomasz Nowicki wrote:
> Add infrastructure to support ThunderX PEM specific PCI configuration space
> access for ACPI based PCI host controller. This involves:
> 1. New initialization call thunder_pem_cfg_init() to create configuration
> space mapping based on hardcoded range addresses
> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
> PEM specific register ranges
> 3. New quirk entry (for common quirk array) which identifies platform and
> calls thunder_pem_cfg_init() from [1]
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>

Acked-by: Robert Richter <rrichter@cavium.com>

> ---
>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  3 files changed, 94 insertions(+), 13 deletions(-)

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Duc Dang]

On Mon, Aug 8, 2016 at 6:05 AM, Tomasz Nowicki <tn@semihalf.com> wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
>
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
>
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.

Tested on X-Gene with X-Gene Ecam quirk and the PCIe ports work fine
with this patch set.

>
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
>
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
>
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
>
> --
> 1.9.1
>
Regards,
Duc Dang.

Re: [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Duc Dang]

On Mon, Aug 8, 2016 at 6:05 AM, Tomasz Nowicki <tn@semihalf.com> wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
>
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
>
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.

Tested on X-Gene with X-Gene Ecam quirk and the PCIe ports work fine
with this patch set.

>
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
>
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
>
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
>
> --
> 1.9.1
>
Regards,
Duc Dang.

[RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

[Duc Dang]

On Mon, Aug 8, 2016 at 6:05 AM, Tomasz Nowicki <tn@semihalf.com> wrote:
> Quirk handling relies on an idea of matching MCFG OEM ID, TABLE ID and
> revision (the ones from standard header of MCFG table).
>
> Static array is used to keep quirk entries. Each entry consists of
> mentioned MCFG IDs along with custom pci_ops structure and initialization call.
>
> As an example, the last patch presents quirk handling mechanism usage for
> ThunderX PEM driver.

Tested on X-Gene with X-Gene Ecam quirk and the PCIe ports work fine
with this patch set.

>
> v4 -> v5
> - rebase against v4.8-rc1
> - rework to exact MCFG OEM ID, TABLE ID, rev match
>   - use memcmp instead of strncmp
>   - no substring match
> - fix typos and dmesg message
>
> Tomasz Nowicki (5):
>   PCI: Embed pci_ecam_ops in pci_config_window structure
>   PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver
>   PCI: Check platform specific ECAM quirks
>   ARM64/PCI: Start using quirks handling for ACPI based PCI host
>     controller
>   PCI: thunder-pem: Support quirky configuration space access for ACPI
>     based PCI host controller
>
>  arch/arm64/kernel/pci.c            | 42 +----------------
>  drivers/acpi/pci_mcfg.c            | 40 ++++++++++++++++
>  drivers/pci/ecam.c                 |  6 +--
>  drivers/pci/host/Makefile          |  1 +
>  drivers/pci/host/mcfg-quirks.c     | 93 ++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h     | 24 ++++++++++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  include/linux/pci-acpi.h           |  5 ++
>  include/linux/pci-ecam.h           |  2 +-
>  9 files changed, 252 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
>
> --
> 1.9.1
>
Regards,
Duc Dang.

Re: [RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> pci_config_window keeps pointer to pci_ecam_ops and every time
> we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> sure to free pci_ecam_ops in case it was dynamically allocated prior to
> pci_ecam_create() call.

I think this is a theoretical problem, right?  All the current callers
pass a pointer to a static pci_ecam_ops struct that does not need to
be deallocated.

I see that the next patch uses a pci_ecam_ops struct on the stack,
which would definitely cause issues without this patch, so I agree we
need something like this; I'm just trying to make the changelog make
more sense.

> To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
> instead of just a pointer.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> To: Jayachandran C <jchandra@broadcom.com>

This looks like an editing mistake?

> ---
>  arch/arm64/kernel/pci.c  | 2 +-
>  drivers/pci/ecam.c       | 6 +++---
>  include/linux/pci-ecam.h | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index acf3872..981e828 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  		return NULL;
>  	}
>  
> -	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
> +	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
>  	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
>  				   ri->cfg);
>  	if (!bus)
> diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> index 43ed08d..24242f0 100644
> --- a/drivers/pci/ecam.c
> +++ b/drivers/pci/ecam.c
> @@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
>  		return ERR_PTR(-ENOMEM);
>  
>  	cfg->parent = dev;
> -	cfg->ops = ops;
> +	cfg->ops = *ops;
>  	cfg->busr.start = busr->start;
>  	cfg->busr.end = busr->end;
>  	cfg->busr.flags = IORESOURCE_BUS;
> @@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  			       int where)
>  {
>  	struct pci_config_window *cfg = bus->sysdata;
> -	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
> +	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
>  	unsigned int busn = bus->number;
>  	void __iomem *base;
>  
> @@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  	if (per_bus_mapping)
>  		base = cfg->winp[busn];
>  	else
> -		base = cfg->win + (busn << cfg->ops->bus_shift);
> +		base = cfg->win + (busn << cfg->ops.bus_shift);
>  	return base + (devfn << devfn_shift) + where;
>  }
>  
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 7adad20..0ce2920 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -39,7 +39,7 @@ struct pci_config_window {
>  	struct resource			res;
>  	struct resource			busr;
>  	void				*priv;
> -	struct pci_ecam_ops		*ops;
> +	struct pci_ecam_ops		ops;
>  	union {
>  		void __iomem		*win;	/* 64-bit single mapping */
>  		void __iomem		**winp; /* 32-bit per-bus mapping */
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> pci_config_window keeps pointer to pci_ecam_ops and every time
> we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> sure to free pci_ecam_ops in case it was dynamically allocated prior to
> pci_ecam_create() call.

I think this is a theoretical problem, right?  All the current callers
pass a pointer to a static pci_ecam_ops struct that does not need to
be deallocated.

I see that the next patch uses a pci_ecam_ops struct on the stack,
which would definitely cause issues without this patch, so I agree we
need something like this; I'm just trying to make the changelog make
more sense.

> To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
> instead of just a pointer.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> To: Jayachandran C <jchandra@broadcom.com>

This looks like an editing mistake?

> ---
>  arch/arm64/kernel/pci.c  | 2 +-
>  drivers/pci/ecam.c       | 6 +++---
>  include/linux/pci-ecam.h | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index acf3872..981e828 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  		return NULL;
>  	}
>  
> -	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
> +	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
>  	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
>  				   ri->cfg);
>  	if (!bus)
> diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> index 43ed08d..24242f0 100644
> --- a/drivers/pci/ecam.c
> +++ b/drivers/pci/ecam.c
> @@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
>  		return ERR_PTR(-ENOMEM);
>  
>  	cfg->parent = dev;
> -	cfg->ops = ops;
> +	cfg->ops = *ops;
>  	cfg->busr.start = busr->start;
>  	cfg->busr.end = busr->end;
>  	cfg->busr.flags = IORESOURCE_BUS;
> @@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  			       int where)
>  {
>  	struct pci_config_window *cfg = bus->sysdata;
> -	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
> +	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
>  	unsigned int busn = bus->number;
>  	void __iomem *base;
>  
> @@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  	if (per_bus_mapping)
>  		base = cfg->winp[busn];
>  	else
> -		base = cfg->win + (busn << cfg->ops->bus_shift);
> +		base = cfg->win + (busn << cfg->ops.bus_shift);
>  	return base + (devfn << devfn_shift) + where;
>  }
>  
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 7adad20..0ce2920 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -39,7 +39,7 @@ struct pci_config_window {
>  	struct resource			res;
>  	struct resource			busr;
>  	void				*priv;
> -	struct pci_ecam_ops		*ops;
> +	struct pci_ecam_ops		ops;
>  	union {
>  		void __iomem		*win;	/* 64-bit single mapping */
>  		void __iomem		**winp; /* 32-bit per-bus mapping */
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> pci_config_window keeps pointer to pci_ecam_ops and every time
> we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> sure to free pci_ecam_ops in case it was dynamically allocated prior to
> pci_ecam_create() call.

I think this is a theoretical problem, right?  All the current callers
pass a pointer to a static pci_ecam_ops struct that does not need to
be deallocated.

I see that the next patch uses a pci_ecam_ops struct on the stack,
which would definitely cause issues without this patch, so I agree we
need something like this; I'm just trying to make the changelog make
more sense.

> To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
> instead of just a pointer.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> To: Jayachandran C <jchandra@broadcom.com>

This looks like an editing mistake?

> ---
>  arch/arm64/kernel/pci.c  | 2 +-
>  drivers/pci/ecam.c       | 6 +++---
>  include/linux/pci-ecam.h | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index acf3872..981e828 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  		return NULL;
>  	}
>  
> -	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
> +	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
>  	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
>  				   ri->cfg);
>  	if (!bus)
> diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> index 43ed08d..24242f0 100644
> --- a/drivers/pci/ecam.c
> +++ b/drivers/pci/ecam.c
> @@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
>  		return ERR_PTR(-ENOMEM);
>  
>  	cfg->parent = dev;
> -	cfg->ops = ops;
> +	cfg->ops = *ops;
>  	cfg->busr.start = busr->start;
>  	cfg->busr.end = busr->end;
>  	cfg->busr.flags = IORESOURCE_BUS;
> @@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  			       int where)
>  {
>  	struct pci_config_window *cfg = bus->sysdata;
> -	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
> +	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
>  	unsigned int busn = bus->number;
>  	void __iomem *base;
>  
> @@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  	if (per_bus_mapping)
>  		base = cfg->winp[busn];
>  	else
> -		base = cfg->win + (busn << cfg->ops->bus_shift);
> +		base = cfg->win + (busn << cfg->ops.bus_shift);
>  	return base + (devfn << devfn_shift) + where;
>  }
>  
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 7adad20..0ce2920 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -39,7 +39,7 @@ struct pci_config_window {
>  	struct resource			res;
>  	struct resource			busr;
>  	void				*priv;
> -	struct pci_ecam_ops		*ops;
> +	struct pci_ecam_ops		ops;
>  	union {
>  		void __iomem		*win;	/* 64-bit single mapping */
>  		void __iomem		**winp; /* 32-bit per-bus mapping */
> -- 
> 1.9.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> pci_config_window keeps pointer to pci_ecam_ops and every time
> we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> sure to free pci_ecam_ops in case it was dynamically allocated prior to
> pci_ecam_create() call.

I think this is a theoretical problem, right?  All the current callers
pass a pointer to a static pci_ecam_ops struct that does not need to
be deallocated.

I see that the next patch uses a pci_ecam_ops struct on the stack,
which would definitely cause issues without this patch, so I agree we
need something like this; I'm just trying to make the changelog make
more sense.

> To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
> instead of just a pointer.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> To: Jayachandran C <jchandra@broadcom.com>

This looks like an editing mistake?

> ---
>  arch/arm64/kernel/pci.c  | 2 +-
>  drivers/pci/ecam.c       | 6 +++---
>  include/linux/pci-ecam.h | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index acf3872..981e828 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  		return NULL;
>  	}
>  
> -	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
> +	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
>  	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
>  				   ri->cfg);
>  	if (!bus)
> diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> index 43ed08d..24242f0 100644
> --- a/drivers/pci/ecam.c
> +++ b/drivers/pci/ecam.c
> @@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
>  		return ERR_PTR(-ENOMEM);
>  
>  	cfg->parent = dev;
> -	cfg->ops = ops;
> +	cfg->ops = *ops;
>  	cfg->busr.start = busr->start;
>  	cfg->busr.end = busr->end;
>  	cfg->busr.flags = IORESOURCE_BUS;
> @@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  			       int where)
>  {
>  	struct pci_config_window *cfg = bus->sysdata;
> -	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
> +	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
>  	unsigned int busn = bus->number;
>  	void __iomem *base;
>  
> @@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  	if (per_bus_mapping)
>  		base = cfg->winp[busn];
>  	else
> -		base = cfg->win + (busn << cfg->ops->bus_shift);
> +		base = cfg->win + (busn << cfg->ops.bus_shift);
>  	return base + (devfn << devfn_shift) + where;
>  }
>  
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 7adad20..0ce2920 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -39,7 +39,7 @@ struct pci_config_window {
>  	struct resource			res;
>  	struct resource			busr;
>  	void				*priv;
> -	struct pci_ecam_ops		*ops;
> +	struct pci_ecam_ops		ops;
>  	union {
>  		void __iomem		*win;	/* 64-bit single mapping */
>  		void __iomem		**winp; /* 32-bit per-bus mapping */
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Lorenzo Pieralisi]

On Thu, Sep 01, 2016 at 01:23:45PM -0500, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> > pci_config_window keeps pointer to pci_ecam_ops and every time
> > we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> > sure to free pci_ecam_ops in case it was dynamically allocated prior to
> > pci_ecam_create() call.
> 
> I think this is a theoretical problem, right?  All the current callers
> pass a pointer to a static pci_ecam_ops struct that does not need to
> be deallocated.
> 
> I see that the next patch uses a pci_ecam_ops struct on the stack,

I asked Tomasz why we need to have pci_ecam_ops on the stack in the
first place since I do not think it is needed, or nicer (actually I
think it is not nice at all, what's the problem in making it static ?).

Anyway, that's part of this thread but I think Tomasz was looking
more for feedback/agreement on the whole quirks mechanism to check
if it is fine, more below.

> which would definitely cause issues without this patch, so I agree we
> need something like this; I'm just trying to make the changelog make
> more sense.
> 
> > To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
> > instead of just a pointer.
> > 
> > Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> > To: Jayachandran C <jchandra@broadcom.com>
> 
> This looks like an editing mistake?

Yes it is.

I think the most important bit about this series is to agree on
whether it is ok to keep quirks in drivers/pci/host/mcfg-quirks.c
or we need to find a different dir for it, and of course to make
sure the quirks mechanism is fine with everyone.

Thanks !
Lorenzo

> >  arch/arm64/kernel/pci.c  | 2 +-
> >  drivers/pci/ecam.c       | 6 +++---
> >  include/linux/pci-ecam.h | 2 +-
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index acf3872..981e828 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> >  		return NULL;
> >  	}
> >  
> > -	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
> > +	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
> >  	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
> >  				   ri->cfg);
> >  	if (!bus)
> > diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> > index 43ed08d..24242f0 100644
> > --- a/drivers/pci/ecam.c
> > +++ b/drivers/pci/ecam.c
> > @@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
> >  		return ERR_PTR(-ENOMEM);
> >  
> >  	cfg->parent = dev;
> > -	cfg->ops = ops;
> > +	cfg->ops = *ops;
> >  	cfg->busr.start = busr->start;
> >  	cfg->busr.end = busr->end;
> >  	cfg->busr.flags = IORESOURCE_BUS;
> > @@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
> >  			       int where)
> >  {
> >  	struct pci_config_window *cfg = bus->sysdata;
> > -	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
> > +	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
> >  	unsigned int busn = bus->number;
> >  	void __iomem *base;
> >  
> > @@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
> >  	if (per_bus_mapping)
> >  		base = cfg->winp[busn];
> >  	else
> > -		base = cfg->win + (busn << cfg->ops->bus_shift);
> > +		base = cfg->win + (busn << cfg->ops.bus_shift);
> >  	return base + (devfn << devfn_shift) + where;
> >  }
> >  
> > diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> > index 7adad20..0ce2920 100644
> > --- a/include/linux/pci-ecam.h
> > +++ b/include/linux/pci-ecam.h
> > @@ -39,7 +39,7 @@ struct pci_config_window {
> >  	struct resource			res;
> >  	struct resource			busr;
> >  	void				*priv;
> > -	struct pci_ecam_ops		*ops;
> > +	struct pci_ecam_ops		ops;
> >  	union {
> >  		void __iomem		*win;	/* 64-bit single mapping */
> >  		void __iomem		**winp; /* 32-bit per-bus mapping */
> > -- 
> > 1.9.1
> > 
> 

[RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Lorenzo Pieralisi]

On Thu, Sep 01, 2016 at 01:23:45PM -0500, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> > pci_config_window keeps pointer to pci_ecam_ops and every time
> > we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> > sure to free pci_ecam_ops in case it was dynamically allocated prior to
> > pci_ecam_create() call.
> 
> I think this is a theoretical problem, right?  All the current callers
> pass a pointer to a static pci_ecam_ops struct that does not need to
> be deallocated.
> 
> I see that the next patch uses a pci_ecam_ops struct on the stack,

I asked Tomasz why we need to have pci_ecam_ops on the stack in the
first place since I do not think it is needed, or nicer (actually I
think it is not nice at all, what's the problem in making it static ?).

Anyway, that's part of this thread but I think Tomasz was looking
more for feedback/agreement on the whole quirks mechanism to check
if it is fine, more below.

> which would definitely cause issues without this patch, so I agree we
> need something like this; I'm just trying to make the changelog make
> more sense.
> 
> > To avoid that extra effort, embed pci_ecam_ops in pci_config_window,
> > instead of just a pointer.
> > 
> > Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> > To: Jayachandran C <jchandra@broadcom.com>
> 
> This looks like an editing mistake?

Yes it is.

I think the most important bit about this series is to agree on
whether it is ok to keep quirks in drivers/pci/host/mcfg-quirks.c
or we need to find a different dir for it, and of course to make
sure the quirks mechanism is fine with everyone.

Thanks !
Lorenzo

> >  arch/arm64/kernel/pci.c  | 2 +-
> >  drivers/pci/ecam.c       | 6 +++---
> >  include/linux/pci-ecam.h | 2 +-
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index acf3872..981e828 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -183,7 +183,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> >  		return NULL;
> >  	}
> >  
> > -	acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
> > +	acpi_pci_root_ops.pci_ops = &ri->cfg->ops.pci_ops;
> >  	bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
> >  				   ri->cfg);
> >  	if (!bus)
> > diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> > index 43ed08d..24242f0 100644
> > --- a/drivers/pci/ecam.c
> > +++ b/drivers/pci/ecam.c
> > @@ -52,7 +52,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
> >  		return ERR_PTR(-ENOMEM);
> >  
> >  	cfg->parent = dev;
> > -	cfg->ops = ops;
> > +	cfg->ops = *ops;
> >  	cfg->busr.start = busr->start;
> >  	cfg->busr.end = busr->end;
> >  	cfg->busr.flags = IORESOURCE_BUS;
> > @@ -138,7 +138,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
> >  			       int where)
> >  {
> >  	struct pci_config_window *cfg = bus->sysdata;
> > -	unsigned int devfn_shift = cfg->ops->bus_shift - 8;
> > +	unsigned int devfn_shift = cfg->ops.bus_shift - 8;
> >  	unsigned int busn = bus->number;
> >  	void __iomem *base;
> >  
> > @@ -149,7 +149,7 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
> >  	if (per_bus_mapping)
> >  		base = cfg->winp[busn];
> >  	else
> > -		base = cfg->win + (busn << cfg->ops->bus_shift);
> > +		base = cfg->win + (busn << cfg->ops.bus_shift);
> >  	return base + (devfn << devfn_shift) + where;
> >  }
> >  
> > diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> > index 7adad20..0ce2920 100644
> > --- a/include/linux/pci-ecam.h
> > +++ b/include/linux/pci-ecam.h
> > @@ -39,7 +39,7 @@ struct pci_config_window {
> >  	struct resource			res;
> >  	struct resource			busr;
> >  	void				*priv;
> > -	struct pci_ecam_ops		*ops;
> > +	struct pci_ecam_ops		ops;
> >  	union {
> >  		void __iomem		*win;	/* 64-bit single mapping */
> >  		void __iomem		**winp; /* 32-bit per-bus mapping */
> > -- 
> > 1.9.1
> > 
> 

Re: [RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Bjorn Helgaas]

On Fri, Sep 02, 2016 at 04:38:45PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Sep 01, 2016 at 01:23:45PM -0500, Bjorn Helgaas wrote:
> > On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> > > pci_config_window keeps pointer to pci_ecam_ops and every time
> > > we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> > > sure to free pci_ecam_ops in case it was dynamically allocated prior to
> > > pci_ecam_create() call.
> > 
> > I think this is a theoretical problem, right?  All the current callers
> > pass a pointer to a static pci_ecam_ops struct that does not need to
> > be deallocated.
> > 
> > I see that the next patch uses a pci_ecam_ops struct on the stack,
> 
> I asked Tomasz why we need to have pci_ecam_ops on the stack in the
> first place since I do not think it is needed, or nicer (actually I
> think it is not nice at all, what's the problem in making it static ?).

If it's useful to copy the struct instead of saving the pointer, that's
fine.  I just want the changelog to match the diff.

Bjorn

[RFC PATCH V5 1/5] PCI: Embed pci_ecam_ops in pci_config_window structure

[Bjorn Helgaas]

On Fri, Sep 02, 2016 at 04:38:45PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Sep 01, 2016 at 01:23:45PM -0500, Bjorn Helgaas wrote:
> > On Mon, Aug 08, 2016 at 03:05:37PM +0200, Tomasz Nowicki wrote:
> > > pci_config_window keeps pointer to pci_ecam_ops and every time
> > > we want to deallocate pci_config_window (pci_ecam_free()) we need to make
> > > sure to free pci_ecam_ops in case it was dynamically allocated prior to
> > > pci_ecam_create() call.
> > 
> > I think this is a theoretical problem, right?  All the current callers
> > pass a pointer to a static pci_ecam_ops struct that does not need to
> > be deallocated.
> > 
> > I see that the next patch uses a pci_ecam_ops struct on the stack,
> 
> I asked Tomasz why we need to have pci_ecam_ops on the stack in the
> first place since I do not think it is needed, or nicer (actually I
> think it is not nice at all, what's the problem in making it static ?).

If it's useful to copy the struct instead of saving the pointer, that's
fine.  I just want the changelog to match the diff.

Bjorn

Re: [RFC PATCH V5 2/5] PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:38PM +0200, Tomasz Nowicki wrote:
> pci_acpi_setup_ecam_mapping() is not really ARM64 specific so move it out
> of arch/arm64/ directory. In preparation for adding MCFG quirk handling
> extend pci_acpi_setup_ecam_mapping() functionality to accept custom
> PCI config accessors (function's argument).
> 
> For ARM64 ACPI based PCI host controller we still use pci_generic_ecam_ops.

I'm not sure we gain much by moving pci_acpi_setup_ecam_mapping() from
arm64 code to generic code, since nobody else uses it yet.  But if you
do want to move it, can you do the move (with no other change at all)
in one patch, and add the new "ops" argument in a second patch?  I
just don't want the "ops" change to get lost in the noise of the move.

> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  arch/arm64/kernel/pci.c  | 41 ++---------------------------------------
>  drivers/acpi/pci_mcfg.c  | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-acpi.h |  3 +++
>  3 files changed, 45 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 981e828..2e7bed4 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -114,44 +114,6 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  	return 0;
>  }
>  
> -/*
> - * Lookup the bus range for the domain in MCFG, and set up config space
> - * mapping.
> - */
> -static struct pci_config_window *
> -pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
> -{
> -	struct resource *bus_res = &root->secondary;
> -	u16 seg = root->segment;
> -	struct pci_config_window *cfg;
> -	struct resource cfgres;
> -	unsigned int bsz;
> -
> -	/* Use address from _CBA if present, otherwise lookup MCFG */
> -	if (!root->mcfg_addr)
> -		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
> -
> -	if (!root->mcfg_addr) {
> -		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
> -			seg, bus_res);
> -		return NULL;
> -	}
> -
> -	bsz = 1 << pci_generic_ecam_ops.bus_shift;
> -	cfgres.start = root->mcfg_addr + bus_res->start * bsz;
> -	cfgres.end = cfgres.start + resource_size(bus_res) * bsz - 1;
> -	cfgres.flags = IORESOURCE_MEM;
> -	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res,
> -			      &pci_generic_ecam_ops);
> -	if (IS_ERR(cfg)) {
> -		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
> -			seg, bus_res, PTR_ERR(cfg));
> -		return NULL;
> -	}
> -
> -	return cfg;
> -}
> -
>  /* release_info: free resources allocated by init_info */
>  static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
>  {
> @@ -177,7 +139,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	if (!ri)
>  		return NULL;
>  
> -	ri->cfg = pci_acpi_setup_ecam_mapping(root);
> +	ri->cfg = pci_acpi_setup_ecam_mapping(root,
> +					      &pci_generic_ecam_ops.pci_ops);
>  	if (!ri->cfg) {
>  		kfree(ri);
>  		return NULL;
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index b5b376e..331b560 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/pci.h>
>  #include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
>  
>  /* Structure to hold entries from the MCFG table */
>  struct mcfg_entry {
> @@ -52,6 +53,45 @@ phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
>  	return 0;
>  }
>  
> +/*
> + * Lookup the bus range for the domain in MCFG, and set up config space
> + * mapping.
> + */
> +struct pci_config_window *
> +pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops)
> +{
> +	struct resource *bus_res = &root->secondary;
> +	u16 seg = root->segment;
> +	struct pci_config_window *cfg;
> +	struct resource cfgres;
> +	struct pci_ecam_ops ecam_ops = {
> +			.bus_shift = 20,
> +			.pci_ops = *ops,
> +	};
> +
> +	/* Use address from _CBA if present, otherwise lookup MCFG */
> +	if (!root->mcfg_addr)
> +		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
> +
> +	if (!root->mcfg_addr) {
> +		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
> +			seg, bus_res);
> +		return NULL;
> +	}
> +
> +	cfgres.start = root->mcfg_addr + (bus_res->start << 20);
> +	cfgres.end = cfgres.start + (resource_size(bus_res) << 20) - 1;
> +	cfgres.flags = IORESOURCE_MEM;
> +	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res, &ecam_ops);
> +	if (IS_ERR(cfg)) {
> +		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
> +			seg, bus_res, PTR_ERR(cfg));
> +		return NULL;
> +	}
> +
> +	return cfg;
> +}
> +
>  static __init int pci_mcfg_parse(struct acpi_table_header *header)
>  {
>  	struct acpi_table_mcfg *mcfg;
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 7d63a66..e9bfe00 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -26,6 +26,9 @@ 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_config_window *
> +pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
> +
>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>  {
>  	struct pci_bus *pbus = pdev->bus;
> -- 
> 1.9.1
> 

[RFC PATCH V5 2/5] PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:38PM +0200, Tomasz Nowicki wrote:
> pci_acpi_setup_ecam_mapping() is not really ARM64 specific so move it out
> of arch/arm64/ directory. In preparation for adding MCFG quirk handling
> extend pci_acpi_setup_ecam_mapping() functionality to accept custom
> PCI config accessors (function's argument).
> 
> For ARM64 ACPI based PCI host controller we still use pci_generic_ecam_ops.

I'm not sure we gain much by moving pci_acpi_setup_ecam_mapping() from
arm64 code to generic code, since nobody else uses it yet.  But if you
do want to move it, can you do the move (with no other change at all)
in one patch, and add the new "ops" argument in a second patch?  I
just don't want the "ops" change to get lost in the noise of the move.

> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  arch/arm64/kernel/pci.c  | 41 ++---------------------------------------
>  drivers/acpi/pci_mcfg.c  | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-acpi.h |  3 +++
>  3 files changed, 45 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 981e828..2e7bed4 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -114,44 +114,6 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  	return 0;
>  }
>  
> -/*
> - * Lookup the bus range for the domain in MCFG, and set up config space
> - * mapping.
> - */
> -static struct pci_config_window *
> -pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
> -{
> -	struct resource *bus_res = &root->secondary;
> -	u16 seg = root->segment;
> -	struct pci_config_window *cfg;
> -	struct resource cfgres;
> -	unsigned int bsz;
> -
> -	/* Use address from _CBA if present, otherwise lookup MCFG */
> -	if (!root->mcfg_addr)
> -		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
> -
> -	if (!root->mcfg_addr) {
> -		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
> -			seg, bus_res);
> -		return NULL;
> -	}
> -
> -	bsz = 1 << pci_generic_ecam_ops.bus_shift;
> -	cfgres.start = root->mcfg_addr + bus_res->start * bsz;
> -	cfgres.end = cfgres.start + resource_size(bus_res) * bsz - 1;
> -	cfgres.flags = IORESOURCE_MEM;
> -	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res,
> -			      &pci_generic_ecam_ops);
> -	if (IS_ERR(cfg)) {
> -		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
> -			seg, bus_res, PTR_ERR(cfg));
> -		return NULL;
> -	}
> -
> -	return cfg;
> -}
> -
>  /* release_info: free resources allocated by init_info */
>  static void pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
>  {
> @@ -177,7 +139,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	if (!ri)
>  		return NULL;
>  
> -	ri->cfg = pci_acpi_setup_ecam_mapping(root);
> +	ri->cfg = pci_acpi_setup_ecam_mapping(root,
> +					      &pci_generic_ecam_ops.pci_ops);
>  	if (!ri->cfg) {
>  		kfree(ri);
>  		return NULL;
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index b5b376e..331b560 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/pci.h>
>  #include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
>  
>  /* Structure to hold entries from the MCFG table */
>  struct mcfg_entry {
> @@ -52,6 +53,45 @@ phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
>  	return 0;
>  }
>  
> +/*
> + * Lookup the bus range for the domain in MCFG, and set up config space
> + * mapping.
> + */
> +struct pci_config_window *
> +pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops)
> +{
> +	struct resource *bus_res = &root->secondary;
> +	u16 seg = root->segment;
> +	struct pci_config_window *cfg;
> +	struct resource cfgres;
> +	struct pci_ecam_ops ecam_ops = {
> +			.bus_shift = 20,
> +			.pci_ops = *ops,
> +	};
> +
> +	/* Use address from _CBA if present, otherwise lookup MCFG */
> +	if (!root->mcfg_addr)
> +		root->mcfg_addr = pci_mcfg_lookup(seg, bus_res);
> +
> +	if (!root->mcfg_addr) {
> +		dev_err(&root->device->dev, "%04x:%pR ECAM region not found\n",
> +			seg, bus_res);
> +		return NULL;
> +	}
> +
> +	cfgres.start = root->mcfg_addr + (bus_res->start << 20);
> +	cfgres.end = cfgres.start + (resource_size(bus_res) << 20) - 1;
> +	cfgres.flags = IORESOURCE_MEM;
> +	cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res, &ecam_ops);
> +	if (IS_ERR(cfg)) {
> +		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
> +			seg, bus_res, PTR_ERR(cfg));
> +		return NULL;
> +	}
> +
> +	return cfg;
> +}
> +
>  static __init int pci_mcfg_parse(struct acpi_table_header *header)
>  {
>  	struct acpi_table_mcfg *mcfg;
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 7d63a66..e9bfe00 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -26,6 +26,9 @@ 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_config_window *
> +pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
> +
>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>  {
>  	struct pci_bus *pbus = pdev->bus;
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite accessors
> set. Algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns pci_config_window structure with fancy PCI config ops.
> oem_id, oem_table_id and rev come from MCFG table standard header.
> 
> It is possible to define custom init call which is responsible for
> setting up PCI configuration access accordingly to quirk requirements.
> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> 
> pci_generic_ecam_ops will be used for platforms free from quirks.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  drivers/pci/host/Makefile      |  1 +
>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>  include/linux/pci-acpi.h       |  2 +
>  4 files changed, 109 insertions(+)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h

If the object is to work around defects in the ACPI MCFG table, I
think I'd put the quirks closer to drivers/acpi/pci_mcfg.c, where we
parse that table.  What if we actually put them directly *in*
drivers/acpi/pci_mcfg.c?

> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 8843410..500cf78 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> new file mode 100644
> index 0000000..aa9907b
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016 Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ioport.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "mcfg-quirks.h"
> +
> +struct pci_cfg_fixup {
> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	u32 oem_revision;
> +	struct resource domain_range;
> +	struct resource bus_range;
> +	struct pci_ops *ops;
> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
> +					  struct pci_ops *ops);
> +};
> +
> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1), NULL, 0)
> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1),	\
> +						NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
> +
> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
> +/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +};
> +
> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> +				 struct acpi_table_header *mcfg_header)
> +{
> +	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
> +		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
> +			ACPI_OEM_TABLE_ID_SIZE) &&
> +		f->oem_revision == mcfg_header->oem_revision);
> +}
> +
> +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
> +{
> +	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
> +	struct resource *bus_res = &root->secondary;
> +	struct pci_cfg_fixup *f = mcfg_quirks;
> +	struct acpi_table_header *mcfg_header;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
> +	if (ACPI_FAILURE(status))
> +		return NULL;
> +
> +	/*
> +	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
> +	 * table ID, and OEM revision from MCFG table standard header.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> +		if (resource_contains(&f->domain_range, &dom_res) &&
> +		    resource_contains(&f->bus_range, bus_res) &&
> +		    pci_mcfg_fixup_match(f, mcfg_header)) {

I think I'd put all the quirk matching tests (domain, bus, OEM ID,
table ID, etc) in the same place.

> +			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> +				 f->oem_id, f->oem_table_id, f->oem_revision);
> +			return f->init ? f->init(root, f->ops) :
> +				pci_acpi_setup_ecam_mapping(root, f->ops);
> +		}
> +	}
> +	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
> +}
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> new file mode 100644
> index 0000000..45cbd16
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -0,0 +1,20 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#ifndef __MCFG_QUIRKS_H__
> +#define __MCFG_QUIRKS_H__
> +
> +/* MCFG quirks initialize call list */
> +
> +#endif /* __MCFG_QUIRKS_H__ */
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index e9bfe00..28cdce4 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,8 @@ 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_config_window *
> +pci_mcfg_match_quirks(struct acpi_pci_root *root);
>  
>  extern struct pci_config_window *
>  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
> -- 
> 1.9.1
> 

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite accessors
> set. Algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns pci_config_window structure with fancy PCI config ops.
> oem_id, oem_table_id and rev come from MCFG table standard header.
> 
> It is possible to define custom init call which is responsible for
> setting up PCI configuration access accordingly to quirk requirements.
> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> 
> pci_generic_ecam_ops will be used for platforms free from quirks.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  drivers/pci/host/Makefile      |  1 +
>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>  include/linux/pci-acpi.h       |  2 +
>  4 files changed, 109 insertions(+)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h

If the object is to work around defects in the ACPI MCFG table, I
think I'd put the quirks closer to drivers/acpi/pci_mcfg.c, where we
parse that table.  What if we actually put them directly *in*
drivers/acpi/pci_mcfg.c?

> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 8843410..500cf78 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> new file mode 100644
> index 0000000..aa9907b
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016 Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ioport.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "mcfg-quirks.h"
> +
> +struct pci_cfg_fixup {
> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	u32 oem_revision;
> +	struct resource domain_range;
> +	struct resource bus_range;
> +	struct pci_ops *ops;
> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
> +					  struct pci_ops *ops);
> +};
> +
> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1), NULL, 0)
> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1),	\
> +						NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
> +
> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
> +/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +};
> +
> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> +				 struct acpi_table_header *mcfg_header)
> +{
> +	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
> +		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
> +			ACPI_OEM_TABLE_ID_SIZE) &&
> +		f->oem_revision == mcfg_header->oem_revision);
> +}
> +
> +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
> +{
> +	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
> +	struct resource *bus_res = &root->secondary;
> +	struct pci_cfg_fixup *f = mcfg_quirks;
> +	struct acpi_table_header *mcfg_header;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
> +	if (ACPI_FAILURE(status))
> +		return NULL;
> +
> +	/*
> +	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
> +	 * table ID, and OEM revision from MCFG table standard header.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> +		if (resource_contains(&f->domain_range, &dom_res) &&
> +		    resource_contains(&f->bus_range, bus_res) &&
> +		    pci_mcfg_fixup_match(f, mcfg_header)) {

I think I'd put all the quirk matching tests (domain, bus, OEM ID,
table ID, etc) in the same place.

> +			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> +				 f->oem_id, f->oem_table_id, f->oem_revision);
> +			return f->init ? f->init(root, f->ops) :
> +				pci_acpi_setup_ecam_mapping(root, f->ops);
> +		}
> +	}
> +	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
> +}
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> new file mode 100644
> index 0000000..45cbd16
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -0,0 +1,20 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#ifndef __MCFG_QUIRKS_H__
> +#define __MCFG_QUIRKS_H__
> +
> +/* MCFG quirks initialize call list */
> +
> +#endif /* __MCFG_QUIRKS_H__ */
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index e9bfe00..28cdce4 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,8 @@ 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_config_window *
> +pci_mcfg_match_quirks(struct acpi_pci_root *root);
>  
>  extern struct pci_config_window *
>  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 4/5] ARM64/PCI: Start using quirks handling for ACPI based PCI host controller

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:40PM +0200, Tomasz Nowicki wrote:
> Since there are platforms which have non-compliant ECAM space we need to
> override these accessors prior to PCI buses enumeration. In order to do
> that we call pci_mcfg_match_quirks() to retrieve custom
> pci_config_window structure. If no correlated quirk on list, use
> fully ECAM compliant generic PCI config accessors.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  arch/arm64/kernel/pci.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 2e7bed4..aa734bd 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -139,8 +139,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	if (!ri)
>  		return NULL;
>  
> -	ri->cfg = pci_acpi_setup_ecam_mapping(root,
> -					      &pci_generic_ecam_ops.pci_ops);
> +	ri->cfg = pci_mcfg_match_quirks(root);

I don't really like this part: previously it was obvious that we're
setting up ECAM mappings, and now it's not.  I think the quirk
functionality should be buried somewhere inside; it shouldn't be
something this generic code has to worry about.

>  	if (!ri->cfg) {
>  		kfree(ri);
>  		return NULL;
> -- 
> 1.9.1
> 

[RFC PATCH V5 4/5] ARM64/PCI: Start using quirks handling for ACPI based PCI host controller

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:40PM +0200, Tomasz Nowicki wrote:
> Since there are platforms which have non-compliant ECAM space we need to
> override these accessors prior to PCI buses enumeration. In order to do
> that we call pci_mcfg_match_quirks() to retrieve custom
> pci_config_window structure. If no correlated quirk on list, use
> fully ECAM compliant generic PCI config accessors.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  arch/arm64/kernel/pci.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 2e7bed4..aa734bd 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -139,8 +139,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>  	if (!ri)
>  		return NULL;
>  
> -	ri->cfg = pci_acpi_setup_ecam_mapping(root,
> -					      &pci_generic_ecam_ops.pci_ops);
> +	ri->cfg = pci_mcfg_match_quirks(root);

I don't really like this part: previously it was obvious that we're
setting up ECAM mappings, and now it's not.  I think the quirk
functionality should be buried somewhere inside; it shouldn't be
something this generic code has to worry about.

>  	if (!ri->cfg) {
>  		kfree(ri);
>  		return NULL;
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:41PM +0200, Tomasz Nowicki wrote:
> Add infrastructure to support ThunderX PEM specific PCI configuration space
> access for ACPI based PCI host controller. This involves:
> 1. New initialization call thunder_pem_cfg_init() to create configuration
> space mapping based on hardcoded range addresses
> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
> PEM specific register ranges
> 3. New quirk entry (for common quirk array) which identifies platform and
> calls thunder_pem_cfg_init() from [1]

Is it possible to split this up a little bit?  I *hope* most quirks
aren't as complicated as all this.  It'd be nice to make the actual
quirk patches as small as possible so they can be easily copied and
adapted.

Another question below...

> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  3 files changed, 94 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> index aa9907b..2993a72 100644
> --- a/drivers/pci/host/mcfg-quirks.c
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -44,6 +44,13 @@ struct pci_cfg_fixup {
>  
>  static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
>  /*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
> +	/* Pass2.0 */
> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
> +	  thunder_pem_cfg_init },
> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
> +	  thunder_pem_cfg_init },
> +#endif
>  };
>  
>  static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> index 45cbd16..411c667 100644
> --- a/drivers/pci/host/mcfg-quirks.h
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -16,5 +16,9 @@
>  #define __MCFG_QUIRKS_H__
>  
>  /* MCFG quirks initialize call list */
> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
> +struct pci_config_window *
> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops);
> +#endif
>  
>  #endif /* __MCFG_QUIRKS_H__ */
> diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
> index 6abaf80..3f06e49 100644
> --- a/drivers/pci/host/pci-thunder-pem.c
> +++ b/drivers/pci/host/pci-thunder-pem.c
> @@ -18,9 +18,12 @@
>  #include <linux/init.h>
>  #include <linux/of_address.h>
>  #include <linux/of_pci.h>
> +#include <linux/pci-acpi.h>
>  #include <linux/pci-ecam.h>
>  #include <linux/platform_device.h>
>  
> +#include "mcfg-quirks.h"
> +
>  #define PEM_CFG_WR 0x28
>  #define PEM_CFG_RD 0x30
>  
> @@ -284,6 +287,37 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
>  	return pci_generic_config_write(bus, devfn, where, size, val);
>  }
>  
> +#ifdef CONFIG_ACPI
> +
> +static struct resource thunder_pem_reg_res[] = {
> +	[4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M),
> +	[5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M),
> +	[6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M),
> +	[7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M),
> +	[8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M),
> +	[9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M),
> +	[14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M),
> +	[15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M),
> +	[16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M),
> +	[17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M),
> +	[18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M),
> +	[19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M),
> +};
> +
> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
> +{
> +	struct acpi_device *adev = to_acpi_device(cfg->parent);
> +	struct acpi_pci_root *root = acpi_driver_data(adev);
> +
> +	return &thunder_pem_reg_res[root->segment];
> +}
> +#else
> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  static int thunder_pem_init(struct pci_config_window *cfg)
>  {
>  	struct device *dev = cfg->parent;
> @@ -292,24 +326,24 @@ static int thunder_pem_init(struct pci_config_window *cfg)
>  	struct thunder_pem_pci *pem_pci;
>  	struct platform_device *pdev;
>  
> -	/* Only OF support for now */
> -	if (!dev->of_node)
> -		return -EINVAL;
> -
>  	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
>  	if (!pem_pci)
>  		return -ENOMEM;
>  
> -	pdev = to_platform_device(dev);
> -
> -	/*
> -	 * The second register range is the PEM bridge to the PCIe
> -	 * bus.  It has a different config access method than those
> -	 * devices behind the bridge.
> -	 */
> -	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (acpi_disabled) {
> +		pdev = to_platform_device(dev);
> +
> +		/*
> +		 * The second register range is the PEM bridge to the PCIe
> +		 * bus.  It has a different config access method than those
> +		 * devices behind the bridge.
> +		 */
> +		res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	} else {
> +		res_pem = thunder_pem_acpi_res(cfg);
> +	}
>  	if (!res_pem) {
> -		dev_err(dev, "missing \"reg[1]\"property\n");
> +		dev_err(dev, "missing configuration region\n");
>  		return -EINVAL;
>  	}
>  
> @@ -360,3 +394,39 @@ static struct platform_driver thunder_pem_driver = {
>  	.probe = thunder_pem_probe,
>  };
>  builtin_platform_driver(thunder_pem_driver);
> +
> +#ifdef CONFIG_ACPI
> +
> +static struct resource thunder_pem_cfg_res[] = {
> +	[4] = DEFINE_RES_MEM(0x88001f000000UL, 0x39 * SZ_16M),
> +	[5] = DEFINE_RES_MEM(0x884057000000UL, 0x39 * SZ_16M),
> +	[6] = DEFINE_RES_MEM(0x88808f000000UL, 0x39 * SZ_16M),
> +	[7] = DEFINE_RES_MEM(0x89001f000000UL, 0x39 * SZ_16M),
> +	[8] = DEFINE_RES_MEM(0x894057000000UL, 0x39 * SZ_16M),
> +	[9] = DEFINE_RES_MEM(0x89808f000000UL, 0x39 * SZ_16M),
> +	[14] = DEFINE_RES_MEM(0x98001f000000UL, 0x39 * SZ_16M),
> +	[15] = DEFINE_RES_MEM(0x984057000000UL, 0x39 * SZ_16M),
> +	[16] = DEFINE_RES_MEM(0x98808f000000UL, 0x39 * SZ_16M),
> +	[17] = DEFINE_RES_MEM(0x99001f000000UL, 0x39 * SZ_16M),
> +	[18] = DEFINE_RES_MEM(0x994057000000UL, 0x39 * SZ_16M),
> +	[19] = DEFINE_RES_MEM(0x99808f000000UL, 0x39 * SZ_16M),
> +};
> +
> +struct pci_config_window *
> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops)
> +{
> +	struct resource *bus_res = &root->secondary;
> +	u16 seg = root->segment;
> +	struct pci_config_window *cfg;
> +
> +	cfg = pci_ecam_create(&root->device->dev, &thunder_pem_cfg_res[seg],
> +			      bus_res, &pci_thunder_pem_ops);

What happens when root->segment is not one of the values defined in
thunder_pem_cfg_res[], say "0"?  I *think* pci_ecam_create() will see
cfgres as all zeros, and the request_resource_conflict() will probably
fail.  That doesn't seem like a very graceful way to report a firmware
error.

> +	if (IS_ERR(cfg)) {
> +		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
> +			seg, bus_res, PTR_ERR(cfg));
> +		return NULL;
> +	}
> +
> +	return cfg;
> +}
> +#endif
> -- 
> 1.9.1
> 

[RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:41PM +0200, Tomasz Nowicki wrote:
> Add infrastructure to support ThunderX PEM specific PCI configuration space
> access for ACPI based PCI host controller. This involves:
> 1. New initialization call thunder_pem_cfg_init() to create configuration
> space mapping based on hardcoded range addresses
> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
> PEM specific register ranges
> 3. New quirk entry (for common quirk array) which identifies platform and
> calls thunder_pem_cfg_init() from [1]

Is it possible to split this up a little bit?  I *hope* most quirks
aren't as complicated as all this.  It'd be nice to make the actual
quirk patches as small as possible so they can be easily copied and
adapted.

Another question below...

> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>  3 files changed, 94 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> index aa9907b..2993a72 100644
> --- a/drivers/pci/host/mcfg-quirks.c
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -44,6 +44,13 @@ struct pci_cfg_fixup {
>  
>  static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
>  /*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
> +	/* Pass2.0 */
> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
> +	  thunder_pem_cfg_init },
> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
> +	  thunder_pem_cfg_init },
> +#endif
>  };
>  
>  static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> index 45cbd16..411c667 100644
> --- a/drivers/pci/host/mcfg-quirks.h
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -16,5 +16,9 @@
>  #define __MCFG_QUIRKS_H__
>  
>  /* MCFG quirks initialize call list */
> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
> +struct pci_config_window *
> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops);
> +#endif
>  
>  #endif /* __MCFG_QUIRKS_H__ */
> diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
> index 6abaf80..3f06e49 100644
> --- a/drivers/pci/host/pci-thunder-pem.c
> +++ b/drivers/pci/host/pci-thunder-pem.c
> @@ -18,9 +18,12 @@
>  #include <linux/init.h>
>  #include <linux/of_address.h>
>  #include <linux/of_pci.h>
> +#include <linux/pci-acpi.h>
>  #include <linux/pci-ecam.h>
>  #include <linux/platform_device.h>
>  
> +#include "mcfg-quirks.h"
> +
>  #define PEM_CFG_WR 0x28
>  #define PEM_CFG_RD 0x30
>  
> @@ -284,6 +287,37 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
>  	return pci_generic_config_write(bus, devfn, where, size, val);
>  }
>  
> +#ifdef CONFIG_ACPI
> +
> +static struct resource thunder_pem_reg_res[] = {
> +	[4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M),
> +	[5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M),
> +	[6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M),
> +	[7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M),
> +	[8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M),
> +	[9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M),
> +	[14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M),
> +	[15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M),
> +	[16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M),
> +	[17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M),
> +	[18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M),
> +	[19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M),
> +};
> +
> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
> +{
> +	struct acpi_device *adev = to_acpi_device(cfg->parent);
> +	struct acpi_pci_root *root = acpi_driver_data(adev);
> +
> +	return &thunder_pem_reg_res[root->segment];
> +}
> +#else
> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  static int thunder_pem_init(struct pci_config_window *cfg)
>  {
>  	struct device *dev = cfg->parent;
> @@ -292,24 +326,24 @@ static int thunder_pem_init(struct pci_config_window *cfg)
>  	struct thunder_pem_pci *pem_pci;
>  	struct platform_device *pdev;
>  
> -	/* Only OF support for now */
> -	if (!dev->of_node)
> -		return -EINVAL;
> -
>  	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
>  	if (!pem_pci)
>  		return -ENOMEM;
>  
> -	pdev = to_platform_device(dev);
> -
> -	/*
> -	 * The second register range is the PEM bridge to the PCIe
> -	 * bus.  It has a different config access method than those
> -	 * devices behind the bridge.
> -	 */
> -	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (acpi_disabled) {
> +		pdev = to_platform_device(dev);
> +
> +		/*
> +		 * The second register range is the PEM bridge to the PCIe
> +		 * bus.  It has a different config access method than those
> +		 * devices behind the bridge.
> +		 */
> +		res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	} else {
> +		res_pem = thunder_pem_acpi_res(cfg);
> +	}
>  	if (!res_pem) {
> -		dev_err(dev, "missing \"reg[1]\"property\n");
> +		dev_err(dev, "missing configuration region\n");
>  		return -EINVAL;
>  	}
>  
> @@ -360,3 +394,39 @@ static struct platform_driver thunder_pem_driver = {
>  	.probe = thunder_pem_probe,
>  };
>  builtin_platform_driver(thunder_pem_driver);
> +
> +#ifdef CONFIG_ACPI
> +
> +static struct resource thunder_pem_cfg_res[] = {
> +	[4] = DEFINE_RES_MEM(0x88001f000000UL, 0x39 * SZ_16M),
> +	[5] = DEFINE_RES_MEM(0x884057000000UL, 0x39 * SZ_16M),
> +	[6] = DEFINE_RES_MEM(0x88808f000000UL, 0x39 * SZ_16M),
> +	[7] = DEFINE_RES_MEM(0x89001f000000UL, 0x39 * SZ_16M),
> +	[8] = DEFINE_RES_MEM(0x894057000000UL, 0x39 * SZ_16M),
> +	[9] = DEFINE_RES_MEM(0x89808f000000UL, 0x39 * SZ_16M),
> +	[14] = DEFINE_RES_MEM(0x98001f000000UL, 0x39 * SZ_16M),
> +	[15] = DEFINE_RES_MEM(0x984057000000UL, 0x39 * SZ_16M),
> +	[16] = DEFINE_RES_MEM(0x98808f000000UL, 0x39 * SZ_16M),
> +	[17] = DEFINE_RES_MEM(0x99001f000000UL, 0x39 * SZ_16M),
> +	[18] = DEFINE_RES_MEM(0x994057000000UL, 0x39 * SZ_16M),
> +	[19] = DEFINE_RES_MEM(0x99808f000000UL, 0x39 * SZ_16M),
> +};
> +
> +struct pci_config_window *
> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops)
> +{
> +	struct resource *bus_res = &root->secondary;
> +	u16 seg = root->segment;
> +	struct pci_config_window *cfg;
> +
> +	cfg = pci_ecam_create(&root->device->dev, &thunder_pem_cfg_res[seg],
> +			      bus_res, &pci_thunder_pem_ops);

What happens when root->segment is not one of the values defined in
thunder_pem_cfg_res[], say "0"?  I *think* pci_ecam_create() will see
cfgres as all zeros, and the request_resource_conflict() will probably
fail.  That doesn't seem like a very graceful way to report a firmware
error.

> +	if (IS_ERR(cfg)) {
> +		dev_err(&root->device->dev, "%04x:%pR error %ld mapping ECAM\n",
> +			seg, bus_res, PTR_ERR(cfg));
> +		return NULL;
> +	}
> +
> +	return cfg;
> +}
> +#endif
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite accessors
> set. Algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns pci_config_window structure with fancy PCI config ops.
> oem_id, oem_table_id and rev come from MCFG table standard header.
> 
> It is possible to define custom init call which is responsible for
> setting up PCI configuration access accordingly to quirk requirements.
> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> 
> pci_generic_ecam_ops will be used for platforms free from quirks.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  drivers/pci/host/Makefile      |  1 +
>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>  include/linux/pci-acpi.h       |  2 +
>  4 files changed, 109 insertions(+)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> 
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 8843410..500cf78 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> new file mode 100644
> index 0000000..aa9907b
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016 Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ioport.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "mcfg-quirks.h"
> +
> +struct pci_cfg_fixup {
> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	u32 oem_revision;
> +	struct resource domain_range;
> +	struct resource bus_range;
> +	struct pci_ops *ops;
> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
> +					  struct pci_ops *ops);
> +};
> +
> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1), NULL, 0)
> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1),	\
> +						NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
> +
> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
> +/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +};
> +
> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> +				 struct acpi_table_header *mcfg_header)
> +{
> +	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
> +		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
> +			ACPI_OEM_TABLE_ID_SIZE) &&
> +		f->oem_revision == mcfg_header->oem_revision);
> +}
> +
> +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
> +{
> +	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
> +	struct resource *bus_res = &root->secondary;
> +	struct pci_cfg_fixup *f = mcfg_quirks;
> +	struct acpi_table_header *mcfg_header;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
> +	if (ACPI_FAILURE(status))
> +		return NULL;

I remember working out why pci_mcfg_parse() has to cache the data from
MCFG, but I don't remember the actual reason...  a comment there would
have been helpful.  Anyway, now I wonder why it's OK to call
acpi_get_table(ACPI_SIG_MCFG) again here.  I guess I'd be inclined to
just cache the OEM info along with the rest of the data so you don't
have to get the table again here.

> +	/*
> +	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
> +	 * table ID, and OEM revision from MCFG table standard header.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> +		if (resource_contains(&f->domain_range, &dom_res) &&
> +		    resource_contains(&f->bus_range, bus_res) &&
> +		    pci_mcfg_fixup_match(f, mcfg_header)) {
> +			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> +				 f->oem_id, f->oem_table_id, f->oem_revision);
> +			return f->init ? f->init(root, f->ops) :
> +				pci_acpi_setup_ecam_mapping(root, f->ops);
> +		}
> +	}
> +	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);

I think the scope of these MCFG quirks is to:

  1) Override (or even fabricate) a MEM resource to be used as ECAM
     space, and

  2) Supply a pci_ecam_ops structure if we need something other than
     pci_generic_ecam_ops

Today, pci_mcfg_lookup() merely returns a physical address.  I didn't
chase down where (or whether) that is turned into a struct resource
that we put in the iomem tree.  I wonder if pci_mcfg_lookup() could be
extended to return a resource and a pci_ecam_ops pointer, and handle
the quirk application internally, e.g.,

  int pci_mcfg_lookup(u16 seg, struct resource *bus_res,
                      struct resource *cfgres, struct pci_ecam_ops **ecam_ops)
  {
    struct resource res;
    struct pci_ecam_ops *ops;
    struct mcfg_entry *e;

    memset(&res, 0, sizeof(res));
    res.flags = IORESOURCE_MEM;

    e = pci_mcfg_list_search(seg, bus_res);
    if (e) {
      res.start = e->addr;
      res.end = res.start + ...;
    }

    ops = &pci_generic_ecam_ops;
    ret = pci_mcfg_match_quirks(seg, bus_res, &res, &ops);
    if (ret)
      return ret;

    *cfgres = res;
    *ecam_ops = ops;
    return 0;
  }

Then the quirks would see any mem resource we discovered via MCFG, and
they would see the default pci_ecam_ops, and they could override
whatever they needed to.

That would make pci_acpi_setup_ecam_mapping() look something like this:

  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
  {
    struct resource cfgres;
    struct pci_ecam_ops *ecam_ops;

    ret = pci_mcfg_lookup(root->segment, &root->secondary, &cfgres, &ecam_ops);
    if (ret) {
      dev_err(..., "ECAM region not found\n");
      return ret;
    }

    cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res,
                          ecam_ops);
  }

> +}
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> new file mode 100644
> index 0000000..45cbd16
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -0,0 +1,20 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#ifndef __MCFG_QUIRKS_H__
> +#define __MCFG_QUIRKS_H__
> +
> +/* MCFG quirks initialize call list */
> +
> +#endif /* __MCFG_QUIRKS_H__ */
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index e9bfe00..28cdce4 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,8 @@ 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_config_window *
> +pci_mcfg_match_quirks(struct acpi_pci_root *root);
>  
>  extern struct pci_config_window *
>  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite accessors
> set. Algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns pci_config_window structure with fancy PCI config ops.
> oem_id, oem_table_id and rev come from MCFG table standard header.
> 
> It is possible to define custom init call which is responsible for
> setting up PCI configuration access accordingly to quirk requirements.
> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> 
> pci_generic_ecam_ops will be used for platforms free from quirks.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  drivers/pci/host/Makefile      |  1 +
>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>  include/linux/pci-acpi.h       |  2 +
>  4 files changed, 109 insertions(+)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> 
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 8843410..500cf78 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> new file mode 100644
> index 0000000..aa9907b
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016 Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ioport.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "mcfg-quirks.h"
> +
> +struct pci_cfg_fixup {
> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	u32 oem_revision;
> +	struct resource domain_range;
> +	struct resource bus_range;
> +	struct pci_ops *ops;
> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
> +					  struct pci_ops *ops);
> +};
> +
> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1), NULL, 0)
> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1),	\
> +						NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
> +
> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
> +/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +};
> +
> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> +				 struct acpi_table_header *mcfg_header)
> +{
> +	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
> +		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
> +			ACPI_OEM_TABLE_ID_SIZE) &&
> +		f->oem_revision == mcfg_header->oem_revision);
> +}
> +
> +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
> +{
> +	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
> +	struct resource *bus_res = &root->secondary;
> +	struct pci_cfg_fixup *f = mcfg_quirks;
> +	struct acpi_table_header *mcfg_header;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
> +	if (ACPI_FAILURE(status))
> +		return NULL;

I remember working out why pci_mcfg_parse() has to cache the data from
MCFG, but I don't remember the actual reason...  a comment there would
have been helpful.  Anyway, now I wonder why it's OK to call
acpi_get_table(ACPI_SIG_MCFG) again here.  I guess I'd be inclined to
just cache the OEM info along with the rest of the data so you don't
have to get the table again here.

> +	/*
> +	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
> +	 * table ID, and OEM revision from MCFG table standard header.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> +		if (resource_contains(&f->domain_range, &dom_res) &&
> +		    resource_contains(&f->bus_range, bus_res) &&
> +		    pci_mcfg_fixup_match(f, mcfg_header)) {
> +			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> +				 f->oem_id, f->oem_table_id, f->oem_revision);
> +			return f->init ? f->init(root, f->ops) :
> +				pci_acpi_setup_ecam_mapping(root, f->ops);
> +		}
> +	}
> +	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);

I think the scope of these MCFG quirks is to:

  1) Override (or even fabricate) a MEM resource to be used as ECAM
     space, and

  2) Supply a pci_ecam_ops structure if we need something other than
     pci_generic_ecam_ops

Today, pci_mcfg_lookup() merely returns a physical address.  I didn't
chase down where (or whether) that is turned into a struct resource
that we put in the iomem tree.  I wonder if pci_mcfg_lookup() could be
extended to return a resource and a pci_ecam_ops pointer, and handle
the quirk application internally, e.g.,

  int pci_mcfg_lookup(u16 seg, struct resource *bus_res,
                      struct resource *cfgres, struct pci_ecam_ops **ecam_ops)
  {
    struct resource res;
    struct pci_ecam_ops *ops;
    struct mcfg_entry *e;

    memset(&res, 0, sizeof(res));
    res.flags = IORESOURCE_MEM;

    e = pci_mcfg_list_search(seg, bus_res);
    if (e) {
      res.start = e->addr;
      res.end = res.start + ...;
    }

    ops = &pci_generic_ecam_ops;
    ret = pci_mcfg_match_quirks(seg, bus_res, &res, &ops);
    if (ret)
      return ret;

    *cfgres = res;
    *ecam_ops = ops;
    return 0;
  }

Then the quirks would see any mem resource we discovered via MCFG, and
they would see the default pci_ecam_ops, and they could override
whatever they needed to.

That would make pci_acpi_setup_ecam_mapping() look something like this:

  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
  {
    struct resource cfgres;
    struct pci_ecam_ops *ecam_ops;

    ret = pci_mcfg_lookup(root->segment, &root->secondary, &cfgres, &ecam_ops);
    if (ret) {
      dev_err(..., "ECAM region not found\n");
      return ret;
    }

    cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res,
                          ecam_ops);
  }

> +}
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> new file mode 100644
> index 0000000..45cbd16
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -0,0 +1,20 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#ifndef __MCFG_QUIRKS_H__
> +#define __MCFG_QUIRKS_H__
> +
> +/* MCFG quirks initialize call list */
> +
> +#endif /* __MCFG_QUIRKS_H__ */
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index e9bfe00..28cdce4 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,8 @@ 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_config_window *
> +pci_mcfg_match_quirks(struct acpi_pci_root *root);
>  
>  extern struct pci_config_window *
>  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
> -- 
> 1.9.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Bjorn Helgaas]

On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite accessors
> set. Algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns pci_config_window structure with fancy PCI config ops.
> oem_id, oem_table_id and rev come from MCFG table standard header.
> 
> It is possible to define custom init call which is responsible for
> setting up PCI configuration access accordingly to quirk requirements.
> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> 
> pci_generic_ecam_ops will be used for platforms free from quirks.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  drivers/pci/host/Makefile      |  1 +
>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>  include/linux/pci-acpi.h       |  2 +
>  4 files changed, 109 insertions(+)
>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> 
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 8843410..500cf78 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
> new file mode 100644
> index 0000000..aa9907b
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016 Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/ioport.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "mcfg-quirks.h"
> +
> +struct pci_cfg_fixup {
> +	char oem_id[ACPI_OEM_ID_SIZE + 1];
> +	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> +	u32 oem_revision;
> +	struct resource domain_range;
> +	struct resource bus_range;
> +	struct pci_ops *ops;
> +	struct pci_config_window *(*init)(struct acpi_pci_root *root,
> +					  struct pci_ops *ops);
> +};
> +
> +#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1), NULL, 0)
> +#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
> +#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
> +						((end) - (start) + 1),	\
> +						NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
> +
> +static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
> +/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
> +};
> +
> +static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
> +				 struct acpi_table_header *mcfg_header)
> +{
> +	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
> +		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
> +			ACPI_OEM_TABLE_ID_SIZE) &&
> +		f->oem_revision == mcfg_header->oem_revision);
> +}
> +
> +struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
> +{
> +	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
> +	struct resource *bus_res = &root->secondary;
> +	struct pci_cfg_fixup *f = mcfg_quirks;
> +	struct acpi_table_header *mcfg_header;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
> +	if (ACPI_FAILURE(status))
> +		return NULL;

I remember working out why pci_mcfg_parse() has to cache the data from
MCFG, but I don't remember the actual reason...  a comment there would
have been helpful.  Anyway, now I wonder why it's OK to call
acpi_get_table(ACPI_SIG_MCFG) again here.  I guess I'd be inclined to
just cache the OEM info along with the rest of the data so you don't
have to get the table again here.

> +	/*
> +	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
> +	 * table ID, and OEM revision from MCFG table standard header.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> +		if (resource_contains(&f->domain_range, &dom_res) &&
> +		    resource_contains(&f->bus_range, bus_res) &&
> +		    pci_mcfg_fixup_match(f, mcfg_header)) {
> +			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> +				 f->oem_id, f->oem_table_id, f->oem_revision);
> +			return f->init ? f->init(root, f->ops) :
> +				pci_acpi_setup_ecam_mapping(root, f->ops);
> +		}
> +	}
> +	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);

I think the scope of these MCFG quirks is to:

  1) Override (or even fabricate) a MEM resource to be used as ECAM
     space, and

  2) Supply a pci_ecam_ops structure if we need something other than
     pci_generic_ecam_ops

Today, pci_mcfg_lookup() merely returns a physical address.  I didn't
chase down where (or whether) that is turned into a struct resource
that we put in the iomem tree.  I wonder if pci_mcfg_lookup() could be
extended to return a resource and a pci_ecam_ops pointer, and handle
the quirk application internally, e.g.,

  int pci_mcfg_lookup(u16 seg, struct resource *bus_res,
                      struct resource *cfgres, struct pci_ecam_ops **ecam_ops)
  {
    struct resource res;
    struct pci_ecam_ops *ops;
    struct mcfg_entry *e;

    memset(&res, 0, sizeof(res));
    res.flags = IORESOURCE_MEM;

    e = pci_mcfg_list_search(seg, bus_res);
    if (e) {
      res.start = e->addr;
      res.end = res.start + ...;
    }

    ops = &pci_generic_ecam_ops;
    ret = pci_mcfg_match_quirks(seg, bus_res, &res, &ops);
    if (ret)
      return ret;

    *cfgres = res;
    *ecam_ops = ops;
    return 0;
  }

Then the quirks would see any mem resource we discovered via MCFG, and
they would see the default pci_ecam_ops, and they could override
whatever they needed to.

That would make pci_acpi_setup_ecam_mapping() look something like this:

  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root)
  {
    struct resource cfgres;
    struct pci_ecam_ops *ecam_ops;

    ret = pci_mcfg_lookup(root->segment, &root->secondary, &cfgres, &ecam_ops);
    if (ret) {
      dev_err(..., "ECAM region not found\n");
      return ret;
    }

    cfg = pci_ecam_create(&root->device->dev, &cfgres, bus_res,
                          ecam_ops);
  }

> +}
> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
> new file mode 100644
> index 0000000..45cbd16
> --- /dev/null
> +++ b/drivers/pci/host/mcfg-quirks.h
> @@ -0,0 +1,20 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation (the "GPL").
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License version 2 (GPLv2) for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 (GPLv2) along with this source code.
> + */
> +
> +#ifndef __MCFG_QUIRKS_H__
> +#define __MCFG_QUIRKS_H__
> +
> +/* MCFG quirks initialize call list */
> +
> +#endif /* __MCFG_QUIRKS_H__ */
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index e9bfe00..28cdce4 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -25,6 +25,8 @@ 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_config_window *
> +pci_mcfg_match_quirks(struct acpi_pci_root *root);
>  
>  extern struct pci_config_window *
>  pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
> -- 
> 1.9.1
> 

Re: [RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

On 05.09.2016 04:25, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
>> Some platforms may not be fully compliant with generic set of PCI config
>> accessors. For these cases we implement the way to overwrite accessors
>> set. Algorithm traverses available quirk list (static array),
>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>> returns pci_config_window structure with fancy PCI config ops.
>> oem_id, oem_table_id and rev come from MCFG table standard header.
>>
>> It is possible to define custom init call which is responsible for
>> setting up PCI configuration access accordingly to quirk requirements.
>> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
>>
>> pci_generic_ecam_ops will be used for platforms free from quirks.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>  drivers/pci/host/Makefile      |  1 +
>>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>>  include/linux/pci-acpi.h       |  2 +
>>  4 files changed, 109 insertions(+)
>>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>>  create mode 100644 drivers/pci/host/mcfg-quirks.h
>
> If the object is to work around defects in the ACPI MCFG table, I
> think I'd put the quirks closer to drivers/acpi/pci_mcfg.c, where we
> parse that table.  What if we actually put them directly *in*
> drivers/acpi/pci_mcfg.c?

Then we need to export quirk init calls or ecam ops from 
drivers/pci/host/* directory.

Currently we keep mcfg quirk handling code and related drivers in one 
place drivers/pci/host/

Tomasz

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

On 05.09.2016 04:25, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
>> Some platforms may not be fully compliant with generic set of PCI config
>> accessors. For these cases we implement the way to overwrite accessors
>> set. Algorithm traverses available quirk list (static array),
>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>> returns pci_config_window structure with fancy PCI config ops.
>> oem_id, oem_table_id and rev come from MCFG table standard header.
>>
>> It is possible to define custom init call which is responsible for
>> setting up PCI configuration access accordingly to quirk requirements.
>> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
>>
>> pci_generic_ecam_ops will be used for platforms free from quirks.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>  drivers/pci/host/Makefile      |  1 +
>>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
>>  include/linux/pci-acpi.h       |  2 +
>>  4 files changed, 109 insertions(+)
>>  create mode 100644 drivers/pci/host/mcfg-quirks.c
>>  create mode 100644 drivers/pci/host/mcfg-quirks.h
>
> If the object is to work around defects in the ACPI MCFG table, I
> think I'd put the quirks closer to drivers/acpi/pci_mcfg.c, where we
> parse that table.  What if we actually put them directly *in*
> drivers/acpi/pci_mcfg.c?

Then we need to export quirk init calls or ecam ops from 
drivers/pci/host/* directory.

Currently we keep mcfg quirk handling code and related drivers in one 
place drivers/pci/host/

Tomasz

Re: [RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Tomasz Nowicki]

On 05.09.2016 04:34, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:41PM +0200, Tomasz Nowicki wrote:
>> Add infrastructure to support ThunderX PEM specific PCI configuration space
>> access for ACPI based PCI host controller. This involves:
>> 1. New initialization call thunder_pem_cfg_init() to create configuration
>> space mapping based on hardcoded range addresses
>> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
>> PEM specific register ranges
>> 3. New quirk entry (for common quirk array) which identifies platform and
>> calls thunder_pem_cfg_init() from [1]
>
> Is it possible to split this up a little bit?  I *hope* most quirks
> aren't as complicated as all this.  It'd be nice to make the actual
> quirk patches as small as possible so they can be easily copied and
> adapted.

Yes this is a complicated quirk example. Next time I will include a 
simple one as well.

Also, I will split this patch as you suggested.

>
> Another question below...
>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>>  3 files changed, 94 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
>> index aa9907b..2993a72 100644
>> --- a/drivers/pci/host/mcfg-quirks.c
>> +++ b/drivers/pci/host/mcfg-quirks.c
>> @@ -44,6 +44,13 @@ struct pci_cfg_fixup {
>>
>>  static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
>>  /*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
>> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
>> +	/* Pass2.0 */
>> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
>> +	  thunder_pem_cfg_init },
>> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
>> +	  thunder_pem_cfg_init },
>> +#endif
>>  };
>>
>>  static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
>> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
>> index 45cbd16..411c667 100644
>> --- a/drivers/pci/host/mcfg-quirks.h
>> +++ b/drivers/pci/host/mcfg-quirks.h
>> @@ -16,5 +16,9 @@
>>  #define __MCFG_QUIRKS_H__
>>
>>  /* MCFG quirks initialize call list */
>> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
>> +struct pci_config_window *
>> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops);
>> +#endif
>>
>>  #endif /* __MCFG_QUIRKS_H__ */
>> diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
>> index 6abaf80..3f06e49 100644
>> --- a/drivers/pci/host/pci-thunder-pem.c
>> +++ b/drivers/pci/host/pci-thunder-pem.c
>> @@ -18,9 +18,12 @@
>>  #include <linux/init.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_pci.h>
>> +#include <linux/pci-acpi.h>
>>  #include <linux/pci-ecam.h>
>>  #include <linux/platform_device.h>
>>
>> +#include "mcfg-quirks.h"
>> +
>>  #define PEM_CFG_WR 0x28
>>  #define PEM_CFG_RD 0x30
>>
>> @@ -284,6 +287,37 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
>>  	return pci_generic_config_write(bus, devfn, where, size, val);
>>  }
>>
>> +#ifdef CONFIG_ACPI
>> +
>> +static struct resource thunder_pem_reg_res[] = {
>> +	[4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M),
>> +	[5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M),
>> +	[6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M),
>> +	[7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M),
>> +	[8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M),
>> +	[9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M),
>> +	[14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M),
>> +	[15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M),
>> +	[16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M),
>> +	[17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M),
>> +	[18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M),
>> +	[19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M),
>> +};
>> +
>> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
>> +{
>> +	struct acpi_device *adev = to_acpi_device(cfg->parent);
>> +	struct acpi_pci_root *root = acpi_driver_data(adev);
>> +
>> +	return &thunder_pem_reg_res[root->segment];
>> +}
>> +#else
>> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
>> +{
>> +	return NULL;
>> +}
>> +#endif
>> +
>>  static int thunder_pem_init(struct pci_config_window *cfg)
>>  {
>>  	struct device *dev = cfg->parent;
>> @@ -292,24 +326,24 @@ static int thunder_pem_init(struct pci_config_window *cfg)
>>  	struct thunder_pem_pci *pem_pci;
>>  	struct platform_device *pdev;
>>
>> -	/* Only OF support for now */
>> -	if (!dev->of_node)
>> -		return -EINVAL;
>> -
>>  	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
>>  	if (!pem_pci)
>>  		return -ENOMEM;
>>
>> -	pdev = to_platform_device(dev);
>> -
>> -	/*
>> -	 * The second register range is the PEM bridge to the PCIe
>> -	 * bus.  It has a different config access method than those
>> -	 * devices behind the bridge.
>> -	 */
>> -	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	if (acpi_disabled) {
>> +		pdev = to_platform_device(dev);
>> +
>> +		/*
>> +		 * The second register range is the PEM bridge to the PCIe
>> +		 * bus.  It has a different config access method than those
>> +		 * devices behind the bridge.
>> +		 */
>> +		res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	} else {
>> +		res_pem = thunder_pem_acpi_res(cfg);
>> +	}
>>  	if (!res_pem) {
>> -		dev_err(dev, "missing \"reg[1]\"property\n");
>> +		dev_err(dev, "missing configuration region\n");
>>  		return -EINVAL;
>>  	}
>>
>> @@ -360,3 +394,39 @@ static struct platform_driver thunder_pem_driver = {
>>  	.probe = thunder_pem_probe,
>>  };
>>  builtin_platform_driver(thunder_pem_driver);
>> +
>> +#ifdef CONFIG_ACPI
>> +
>> +static struct resource thunder_pem_cfg_res[] = {
>> +	[4] = DEFINE_RES_MEM(0x88001f000000UL, 0x39 * SZ_16M),
>> +	[5] = DEFINE_RES_MEM(0x884057000000UL, 0x39 * SZ_16M),
>> +	[6] = DEFINE_RES_MEM(0x88808f000000UL, 0x39 * SZ_16M),
>> +	[7] = DEFINE_RES_MEM(0x89001f000000UL, 0x39 * SZ_16M),
>> +	[8] = DEFINE_RES_MEM(0x894057000000UL, 0x39 * SZ_16M),
>> +	[9] = DEFINE_RES_MEM(0x89808f000000UL, 0x39 * SZ_16M),
>> +	[14] = DEFINE_RES_MEM(0x98001f000000UL, 0x39 * SZ_16M),
>> +	[15] = DEFINE_RES_MEM(0x984057000000UL, 0x39 * SZ_16M),
>> +	[16] = DEFINE_RES_MEM(0x98808f000000UL, 0x39 * SZ_16M),
>> +	[17] = DEFINE_RES_MEM(0x99001f000000UL, 0x39 * SZ_16M),
>> +	[18] = DEFINE_RES_MEM(0x994057000000UL, 0x39 * SZ_16M),
>> +	[19] = DEFINE_RES_MEM(0x99808f000000UL, 0x39 * SZ_16M),
>> +};
>> +
>> +struct pci_config_window *
>> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops)
>> +{
>> +	struct resource *bus_res = &root->secondary;
>> +	u16 seg = root->segment;
>> +	struct pci_config_window *cfg;
>> +
>> +	cfg = pci_ecam_create(&root->device->dev, &thunder_pem_cfg_res[seg],
>> +			      bus_res, &pci_thunder_pem_ops);
>
> What happens when root->segment is not one of the values defined in
> thunder_pem_cfg_res[], say "0"?  I *think* pci_ecam_create() will see
> cfgres as all zeros, and the request_resource_conflict() will probably
> fail.  That doesn't seem like a very graceful way to report a firmware
> error.

Right, I will fix it. Thanks.

Tomasz

[RFC PATCH V5 5/5] PCI: thunder-pem: Support quirky configuration space access for ACPI based PCI host controller

[Tomasz Nowicki]

On 05.09.2016 04:34, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:41PM +0200, Tomasz Nowicki wrote:
>> Add infrastructure to support ThunderX PEM specific PCI configuration space
>> access for ACPI based PCI host controller. This involves:
>> 1. New initialization call thunder_pem_cfg_init() to create configuration
>> space mapping based on hardcoded range addresses
>> 2. thunder_pem_init() ACPI extension to obtain hardcoded addresses for
>> PEM specific register ranges
>> 3. New quirk entry (for common quirk array) which identifies platform and
>> calls thunder_pem_cfg_init() from [1]
>
> Is it possible to split this up a little bit?  I *hope* most quirks
> aren't as complicated as all this.  It'd be nice to make the actual
> quirk patches as small as possible so they can be easily copied and
> adapted.

Yes this is a complicated quirk example. Next time I will include a 
simple one as well.

Also, I will split this patch as you suggested.

>
> Another question below...
>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>  drivers/pci/host/mcfg-quirks.c     |  7 +++
>>  drivers/pci/host/mcfg-quirks.h     |  4 ++
>>  drivers/pci/host/pci-thunder-pem.c | 96 ++++++++++++++++++++++++++++++++------
>>  3 files changed, 94 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
>> index aa9907b..2993a72 100644
>> --- a/drivers/pci/host/mcfg-quirks.c
>> +++ b/drivers/pci/host/mcfg-quirks.c
>> @@ -44,6 +44,13 @@ struct pci_cfg_fixup {
>>
>>  static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
>>  /*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
>> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
>> +	/* Pass2.0 */
>> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
>> +	  thunder_pem_cfg_init },
>> +	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
>> +	  thunder_pem_cfg_init },
>> +#endif
>>  };
>>
>>  static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
>> diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
>> index 45cbd16..411c667 100644
>> --- a/drivers/pci/host/mcfg-quirks.h
>> +++ b/drivers/pci/host/mcfg-quirks.h
>> @@ -16,5 +16,9 @@
>>  #define __MCFG_QUIRKS_H__
>>
>>  /* MCFG quirks initialize call list */
>> +#ifdef CONFIG_PCI_HOST_THUNDER_PEM
>> +struct pci_config_window *
>> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops);
>> +#endif
>>
>>  #endif /* __MCFG_QUIRKS_H__ */
>> diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
>> index 6abaf80..3f06e49 100644
>> --- a/drivers/pci/host/pci-thunder-pem.c
>> +++ b/drivers/pci/host/pci-thunder-pem.c
>> @@ -18,9 +18,12 @@
>>  #include <linux/init.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_pci.h>
>> +#include <linux/pci-acpi.h>
>>  #include <linux/pci-ecam.h>
>>  #include <linux/platform_device.h>
>>
>> +#include "mcfg-quirks.h"
>> +
>>  #define PEM_CFG_WR 0x28
>>  #define PEM_CFG_RD 0x30
>>
>> @@ -284,6 +287,37 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
>>  	return pci_generic_config_write(bus, devfn, where, size, val);
>>  }
>>
>> +#ifdef CONFIG_ACPI
>> +
>> +static struct resource thunder_pem_reg_res[] = {
>> +	[4] = DEFINE_RES_MEM(0x87e0c0000000UL, SZ_16M),
>> +	[5] = DEFINE_RES_MEM(0x87e0c1000000UL, SZ_16M),
>> +	[6] = DEFINE_RES_MEM(0x87e0c2000000UL, SZ_16M),
>> +	[7] = DEFINE_RES_MEM(0x87e0c3000000UL, SZ_16M),
>> +	[8] = DEFINE_RES_MEM(0x87e0c4000000UL, SZ_16M),
>> +	[9] = DEFINE_RES_MEM(0x87e0c5000000UL, SZ_16M),
>> +	[14] = DEFINE_RES_MEM(0x97e0c0000000UL, SZ_16M),
>> +	[15] = DEFINE_RES_MEM(0x97e0c1000000UL, SZ_16M),
>> +	[16] = DEFINE_RES_MEM(0x97e0c2000000UL, SZ_16M),
>> +	[17] = DEFINE_RES_MEM(0x97e0c3000000UL, SZ_16M),
>> +	[18] = DEFINE_RES_MEM(0x97e0c4000000UL, SZ_16M),
>> +	[19] = DEFINE_RES_MEM(0x97e0c5000000UL, SZ_16M),
>> +};
>> +
>> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
>> +{
>> +	struct acpi_device *adev = to_acpi_device(cfg->parent);
>> +	struct acpi_pci_root *root = acpi_driver_data(adev);
>> +
>> +	return &thunder_pem_reg_res[root->segment];
>> +}
>> +#else
>> +static struct resource *thunder_pem_acpi_res(struct pci_config_window *cfg)
>> +{
>> +	return NULL;
>> +}
>> +#endif
>> +
>>  static int thunder_pem_init(struct pci_config_window *cfg)
>>  {
>>  	struct device *dev = cfg->parent;
>> @@ -292,24 +326,24 @@ static int thunder_pem_init(struct pci_config_window *cfg)
>>  	struct thunder_pem_pci *pem_pci;
>>  	struct platform_device *pdev;
>>
>> -	/* Only OF support for now */
>> -	if (!dev->of_node)
>> -		return -EINVAL;
>> -
>>  	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
>>  	if (!pem_pci)
>>  		return -ENOMEM;
>>
>> -	pdev = to_platform_device(dev);
>> -
>> -	/*
>> -	 * The second register range is the PEM bridge to the PCIe
>> -	 * bus.  It has a different config access method than those
>> -	 * devices behind the bridge.
>> -	 */
>> -	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	if (acpi_disabled) {
>> +		pdev = to_platform_device(dev);
>> +
>> +		/*
>> +		 * The second register range is the PEM bridge to the PCIe
>> +		 * bus.  It has a different config access method than those
>> +		 * devices behind the bridge.
>> +		 */
>> +		res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +	} else {
>> +		res_pem = thunder_pem_acpi_res(cfg);
>> +	}
>>  	if (!res_pem) {
>> -		dev_err(dev, "missing \"reg[1]\"property\n");
>> +		dev_err(dev, "missing configuration region\n");
>>  		return -EINVAL;
>>  	}
>>
>> @@ -360,3 +394,39 @@ static struct platform_driver thunder_pem_driver = {
>>  	.probe = thunder_pem_probe,
>>  };
>>  builtin_platform_driver(thunder_pem_driver);
>> +
>> +#ifdef CONFIG_ACPI
>> +
>> +static struct resource thunder_pem_cfg_res[] = {
>> +	[4] = DEFINE_RES_MEM(0x88001f000000UL, 0x39 * SZ_16M),
>> +	[5] = DEFINE_RES_MEM(0x884057000000UL, 0x39 * SZ_16M),
>> +	[6] = DEFINE_RES_MEM(0x88808f000000UL, 0x39 * SZ_16M),
>> +	[7] = DEFINE_RES_MEM(0x89001f000000UL, 0x39 * SZ_16M),
>> +	[8] = DEFINE_RES_MEM(0x894057000000UL, 0x39 * SZ_16M),
>> +	[9] = DEFINE_RES_MEM(0x89808f000000UL, 0x39 * SZ_16M),
>> +	[14] = DEFINE_RES_MEM(0x98001f000000UL, 0x39 * SZ_16M),
>> +	[15] = DEFINE_RES_MEM(0x984057000000UL, 0x39 * SZ_16M),
>> +	[16] = DEFINE_RES_MEM(0x98808f000000UL, 0x39 * SZ_16M),
>> +	[17] = DEFINE_RES_MEM(0x99001f000000UL, 0x39 * SZ_16M),
>> +	[18] = DEFINE_RES_MEM(0x994057000000UL, 0x39 * SZ_16M),
>> +	[19] = DEFINE_RES_MEM(0x99808f000000UL, 0x39 * SZ_16M),
>> +};
>> +
>> +struct pci_config_window *
>> +thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops)
>> +{
>> +	struct resource *bus_res = &root->secondary;
>> +	u16 seg = root->segment;
>> +	struct pci_config_window *cfg;
>> +
>> +	cfg = pci_ecam_create(&root->device->dev, &thunder_pem_cfg_res[seg],
>> +			      bus_res, &pci_thunder_pem_ops);
>
> What happens when root->segment is not one of the values defined in
> thunder_pem_cfg_res[], say "0"?  I *think* pci_ecam_create() will see
> cfgres as all zeros, and the request_resource_conflict() will probably
> fail.  That doesn't seem like a very graceful way to report a firmware
> error.

Right, I will fix it. Thanks.

Tomasz

Re: [RFC PATCH V5 2/5] PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver

[Tomasz Nowicki]

On 05.09.2016 04:22, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:38PM +0200, Tomasz Nowicki wrote:
>> pci_acpi_setup_ecam_mapping() is not really ARM64 specific so move it out
>> of arch/arm64/ directory. In preparation for adding MCFG quirk handling
>> extend pci_acpi_setup_ecam_mapping() functionality to accept custom
>> PCI config accessors (function's argument).
>>
>> For ARM64 ACPI based PCI host controller we still use pci_generic_ecam_ops.
>
> I'm not sure we gain much by moving pci_acpi_setup_ecam_mapping() from
> arm64 code to generic code, since nobody else uses it yet.  But if you
> do want to move it, can you do the move (with no other change at all)
> in one patch, and add the new "ops" argument in a second patch?  I
> just don't want the "ops" change to get lost in the noise of the move.

Yes, will do.

Tomasz

[RFC PATCH V5 2/5] PCI/ACPI: Move ACPI ECAM mapping to generic MCFG driver

[Tomasz Nowicki]

On 05.09.2016 04:22, Bjorn Helgaas wrote:
> On Mon, Aug 08, 2016 at 03:05:38PM +0200, Tomasz Nowicki wrote:
>> pci_acpi_setup_ecam_mapping() is not really ARM64 specific so move it out
>> of arch/arm64/ directory. In preparation for adding MCFG quirk handling
>> extend pci_acpi_setup_ecam_mapping() functionality to accept custom
>> PCI config accessors (function's argument).
>>
>> For ARM64 ACPI based PCI host controller we still use pci_generic_ecam_ops.
>
> I'm not sure we gain much by moving pci_acpi_setup_ecam_mapping() from
> arm64 code to generic code, since nobody else uses it yet.  But if you
> do want to move it, can you do the move (with no other change at all)
> in one patch, and add the new "ops" argument in a second patch?  I
> just don't want the "ops" change to get lost in the noise of the move.

Yes, will do.

Tomasz

Re: [RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Arnd Bergmann]

On Tuesday, September 6, 2016 7:49:49 PM CEST Tomasz Nowicki wrote:
> On 05.09.2016 04:25, Bjorn Helgaas wrote:
> > On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
> >> Some platforms may not be fully compliant with generic set of PCI config
> >> accessors. For these cases we implement the way to overwrite accessors
> >> set. Algorithm traverses available quirk list (static array),
> >> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> >> returns pci_config_window structure with fancy PCI config ops.
> >> oem_id, oem_table_id and rev come from MCFG table standard header.
> >>
> >> It is possible to define custom init call which is responsible for
> >> setting up PCI configuration access accordingly to quirk requirements.
> >> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> >>
> >> pci_generic_ecam_ops will be used for platforms free from quirks.
> >>
> >> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> >> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> >> ---
> >>  drivers/pci/host/Makefile      |  1 +
> >>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
> >>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
> >>  include/linux/pci-acpi.h       |  2 +
> >>  4 files changed, 109 insertions(+)
> >>  create mode 100644 drivers/pci/host/mcfg-quirks.c
> >>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> >
> > If the object is to work around defects in the ACPI MCFG table, I
> > think I'd put the quirks closer to drivers/acpi/pci_mcfg.c, where we
> > parse that table.  What if we actually put them directly *in*
> > drivers/acpi/pci_mcfg.c?
> 
> Then we need to export quirk init calls or ecam ops from 
> drivers/pci/host/* directory.
> 
> Currently we keep mcfg quirk handling code and related drivers in one 
> place drivers/pci/host/

I think that's the wrong place to have it, it just leads to people
trying to share code with the host drivers in that directory, which
hasn't really worked out so far, it just adds complexity.

	Arnd

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Arnd Bergmann]

On Tuesday, September 6, 2016 7:49:49 PM CEST Tomasz Nowicki wrote:
> On 05.09.2016 04:25, Bjorn Helgaas wrote:
> > On Mon, Aug 08, 2016 at 03:05:39PM +0200, Tomasz Nowicki wrote:
> >> Some platforms may not be fully compliant with generic set of PCI config
> >> accessors. For these cases we implement the way to overwrite accessors
> >> set. Algorithm traverses available quirk list (static array),
> >> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> >> returns pci_config_window structure with fancy PCI config ops.
> >> oem_id, oem_table_id and rev come from MCFG table standard header.
> >>
> >> It is possible to define custom init call which is responsible for
> >> setting up PCI configuration access accordingly to quirk requirements.
> >> If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().
> >>
> >> pci_generic_ecam_ops will be used for platforms free from quirks.
> >>
> >> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> >> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> >> ---
> >>  drivers/pci/host/Makefile      |  1 +
> >>  drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
> >>  drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
> >>  include/linux/pci-acpi.h       |  2 +
> >>  4 files changed, 109 insertions(+)
> >>  create mode 100644 drivers/pci/host/mcfg-quirks.c
> >>  create mode 100644 drivers/pci/host/mcfg-quirks.h
> >
> > If the object is to work around defects in the ACPI MCFG table, I
> > think I'd put the quirks closer to drivers/acpi/pci_mcfg.c, where we
> > parse that table.  What if we actually put them directly *in*
> > drivers/acpi/pci_mcfg.c?
> 
> Then we need to export quirk init calls or ecam ops from 
> drivers/pci/host/* directory.
> 
> Currently we keep mcfg quirk handling code and related drivers in one 
> place drivers/pci/host/

I think that's the wrong place to have it, it just leads to people
trying to share code with the host drivers in that directory, which
hasn't really worked out so far, it just adds complexity.

	Arnd

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

Some platforms may not be fully compliant with generic set of PCI config
accessors. For these cases we implement the way to overwrite accessors
set. Algorithm traverses available quirk list (static array),
matches against <oem_id, oem_table_id, rev, domain, bus number range> and
returns pci_config_window structure with fancy PCI config ops.
oem_id, oem_table_id and rev come from MCFG table standard header.

It is possible to define custom init call which is responsible for
setting up PCI configuration access accordingly to quirk requirements.
If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().

pci_generic_ecam_ops will be used for platforms free from quirks.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 drivers/pci/host/Makefile      |  1 +
 drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
 include/linux/pci-acpi.h       |  2 +
 4 files changed, 109 insertions(+)
 create mode 100644 drivers/pci/host/mcfg-quirks.c
 create mode 100644 drivers/pci/host/mcfg-quirks.h

diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 8843410..500cf78 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
 obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
+obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
new file mode 100644
index 0000000..aa9907b
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.c
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2016 Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+
+#include "mcfg-quirks.h"
+
+struct pci_cfg_fixup {
+	char oem_id[ACPI_OEM_ID_SIZE + 1];
+	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+	u32 oem_revision;
+	struct resource domain_range;
+	struct resource bus_range;
+	struct pci_ops *ops;
+	struct pci_config_window *(*init)(struct acpi_pci_root *root,
+					  struct pci_ops *ops);
+};
+
+#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1), NULL, 0)
+#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
+#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1),	\
+						NULL, IORESOURCE_BUS)
+#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
+
+static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
+/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
+};
+
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
+				 struct acpi_table_header *mcfg_header)
+{
+	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
+		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
+			ACPI_OEM_TABLE_ID_SIZE) &&
+		f->oem_revision == mcfg_header->oem_revision);
+}
+
+struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
+{
+	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
+	struct resource *bus_res = &root->secondary;
+	struct pci_cfg_fixup *f = mcfg_quirks;
+	struct acpi_table_header *mcfg_header;
+	acpi_status status;
+	int i;
+
+	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
+	if (ACPI_FAILURE(status))
+		return NULL;
+
+	/*
+	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
+	 * table ID, and OEM revision from MCFG table standard header.
+	 */
+	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
+		if (resource_contains(&f->domain_range, &dom_res) &&
+		    resource_contains(&f->bus_range, bus_res) &&
+		    pci_mcfg_fixup_match(f, mcfg_header)) {
+			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
+				 f->oem_id, f->oem_table_id, f->oem_revision);
+			return f->init ? f->init(root, f->ops) :
+				pci_acpi_setup_ecam_mapping(root, f->ops);
+		}
+	}
+	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
+}
diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
new file mode 100644
index 0000000..45cbd16
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.h
@@ -0,0 +1,20 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#ifndef __MCFG_QUIRKS_H__
+#define __MCFG_QUIRKS_H__
+
+/* MCFG quirks initialize call list */
+
+#endif /* __MCFG_QUIRKS_H__ */
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index e9bfe00..28cdce4 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,8 @@ 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_config_window *
+pci_mcfg_match_quirks(struct acpi_pci_root *root);
 
 extern struct pci_config_window *
 pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
-- 
1.9.1

[RFC PATCH V5 3/5] PCI: Check platform specific ECAM quirks

[Tomasz Nowicki]

Some platforms may not be fully compliant with generic set of PCI config
accessors. For these cases we implement the way to overwrite accessors
set. Algorithm traverses available quirk list (static array),
matches against <oem_id, oem_table_id, rev, domain, bus number range> and
returns pci_config_window structure with fancy PCI config ops.
oem_id, oem_table_id and rev come from MCFG table standard header.

It is possible to define custom init call which is responsible for
setting up PCI configuration access accordingly to quirk requirements.
If custom init call is not defined, use standard pci_acpi_setup_ecam_mapping().

pci_generic_ecam_ops will be used for platforms free from quirks.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 drivers/pci/host/Makefile      |  1 +
 drivers/pci/host/mcfg-quirks.c | 86 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/mcfg-quirks.h | 20 ++++++++++
 include/linux/pci-acpi.h       |  2 +
 4 files changed, 109 insertions(+)
 create mode 100644 drivers/pci/host/mcfg-quirks.c
 create mode 100644 drivers/pci/host/mcfg-quirks.h

diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 8843410..500cf78 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
 obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
+obj-$(CONFIG_ACPI_MCFG) += mcfg-quirks.o
diff --git a/drivers/pci/host/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
new file mode 100644
index 0000000..aa9907b
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.c
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2016 Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#include <linux/kernel.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+
+#include "mcfg-quirks.h"
+
+struct pci_cfg_fixup {
+	char oem_id[ACPI_OEM_ID_SIZE + 1];
+	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
+	u32 oem_revision;
+	struct resource domain_range;
+	struct resource bus_range;
+	struct pci_ops *ops;
+	struct pci_config_window *(*init)(struct acpi_pci_root *root,
+					  struct pci_ops *ops);
+};
+
+#define MCFG_DOM_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1), NULL, 0)
+#define MCFG_DOM_ANY			MCFG_DOM_RANGE(0x0, 0xffff)
+#define MCFG_BUS_RANGE(start, end)	DEFINE_RES_NAMED((start),	\
+						((end) - (start) + 1),	\
+						NULL, IORESOURCE_BUS)
+#define MCFG_BUS_ANY			MCFG_BUS_RANGE(0x0, 0xff)
+
+static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
+/*	{ OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
+};
+
+static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
+				 struct acpi_table_header *mcfg_header)
+{
+	return (!memcmp(f->oem_id, mcfg_header->oem_id, ACPI_OEM_ID_SIZE) &&
+		!memcmp(f->oem_table_id, mcfg_header->oem_table_id,
+			ACPI_OEM_TABLE_ID_SIZE) &&
+		f->oem_revision == mcfg_header->oem_revision);
+}
+
+struct pci_config_window *pci_mcfg_match_quirks(struct acpi_pci_root *root)
+{
+	struct resource dom_res = MCFG_DOM_RANGE(root->segment, root->segment);
+	struct resource *bus_res = &root->secondary;
+	struct pci_cfg_fixup *f = mcfg_quirks;
+	struct acpi_table_header *mcfg_header;
+	acpi_status status;
+	int i;
+
+	status = acpi_get_table(ACPI_SIG_MCFG, 0, &mcfg_header);
+	if (ACPI_FAILURE(status))
+		return NULL;
+
+	/*
+	 * First match against PCI topology <domain:bus> then use OEM ID, OEM
+	 * table ID, and OEM revision from MCFG table standard header.
+	 */
+	for (i = 0; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
+		if (resource_contains(&f->domain_range, &dom_res) &&
+		    resource_contains(&f->bus_range, bus_res) &&
+		    pci_mcfg_fixup_match(f, mcfg_header)) {
+			dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
+				 f->oem_id, f->oem_table_id, f->oem_revision);
+			return f->init ? f->init(root, f->ops) :
+				pci_acpi_setup_ecam_mapping(root, f->ops);
+		}
+	}
+	return pci_acpi_setup_ecam_mapping(root, &pci_generic_ecam_ops.pci_ops);
+}
diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
new file mode 100644
index 0000000..45cbd16
--- /dev/null
+++ b/drivers/pci/host/mcfg-quirks.h
@@ -0,0 +1,20 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation (the "GPL").
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 (GPLv2) for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 (GPLv2) along with this source code.
+ */
+
+#ifndef __MCFG_QUIRKS_H__
+#define __MCFG_QUIRKS_H__
+
+/* MCFG quirks initialize call list */
+
+#endif /* __MCFG_QUIRKS_H__ */
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index e9bfe00..28cdce4 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -25,6 +25,8 @@ 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_config_window *
+pci_mcfg_match_quirks(struct acpi_pci_root *root);
 
 extern struct pci_config_window *
 pci_acpi_setup_ecam_mapping(struct acpi_pci_root *root, struct pci_ops *ops);
-- 
1.9.1