* [PATCH V9 01/11] PCI/ECAM: Move ecam.h to linux/include/pci-ecam.h
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 02/11] PCI/ECAM: Add parent device field to pci_config_window Tomasz Nowicki
` (10 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov
From: Jayachandran C <jchandra@broadcom.com>
This header will be used from arch/arm64 for ACPI PCI implementation
so it needs to be moved out of drivers/pci.
Update users of the header file to use the new name. No functional
changes.
Signed-off-by: Jayachandran C <jchandra@broadcom.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
drivers/pci/ecam.c | 3 +-
drivers/pci/ecam.h | 67 -------------------------------------
drivers/pci/host/pci-host-common.c | 3 +-
drivers/pci/host/pci-host-generic.c | 3 +-
drivers/pci/host/pci-thunder-ecam.c | 3 +-
drivers/pci/host/pci-thunder-pem.c | 3 +-
include/linux/pci-ecam.h | 67 +++++++++++++++++++++++++++++++++++++
7 files changed, 72 insertions(+), 77 deletions(-)
delete mode 100644 drivers/pci/ecam.h
create mode 100644 include/linux/pci-ecam.h
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index f9832ad..820e26b 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -19,10 +19,9 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/pci-ecam.h>
#include <linux/slab.h>
-#include "ecam.h"
-
/*
* On 64-bit systems, we do a single ioremap for the whole config space
* since we have enough virtual address range available. On 32-bit, we
diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h
deleted file mode 100644
index 9878beb..0000000
--- a/drivers/pci/ecam.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*
- * Copyright 2016 Broadcom
- *
- * 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 DRIVERS_PCI_ECAM_H
-#define DRIVERS_PCI_ECAM_H
-
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-
-/*
- * struct to hold pci ops and bus shift of the config window
- * for a PCI controller.
- */
-struct pci_config_window;
-struct pci_ecam_ops {
- unsigned int bus_shift;
- struct pci_ops pci_ops;
- int (*init)(struct device *,
- struct pci_config_window *);
-};
-
-/*
- * struct to hold the mappings of a config space window. This
- * is expected to be used as sysdata for PCI controllers that
- * use ECAM.
- */
-struct pci_config_window {
- struct resource res;
- struct resource busr;
- void *priv;
- struct pci_ecam_ops *ops;
- union {
- void __iomem *win; /* 64-bit single mapping */
- void __iomem **winp; /* 32-bit per-bus mapping */
- };
-};
-
-/* create and free pci_config_window */
-struct pci_config_window *pci_ecam_create(struct device *dev,
- struct resource *cfgres, struct resource *busr,
- struct pci_ecam_ops *ops);
-void pci_ecam_free(struct pci_config_window *cfg);
-
-/* map_bus when ->sysdata is an instance of pci_config_window */
-void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
- int where);
-/* default ECAM ops */
-extern struct pci_ecam_ops pci_generic_ecam_ops;
-
-#ifdef CONFIG_PCI_HOST_GENERIC
-/* for DT-based PCI controllers that support ECAM */
-int pci_host_common_probe(struct platform_device *pdev,
- struct pci_ecam_ops *ops);
-#endif
-#endif
diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index 8cba7ab..c18b9e3 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -20,10 +20,9 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
+#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
-#include "../ecam.h"
-
static int gen_pci_parse_request_of_pci_ranges(struct device *dev,
struct list_head *resources, struct resource **bus_range)
{
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 6eaceab..f0ca6de 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -23,10 +23,9 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
+#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
-#include "../ecam.h"
-
static struct pci_ecam_ops gen_pci_cfg_cam_bus_ops = {
.bus_shift = 16,
.pci_ops = {
diff --git a/drivers/pci/host/pci-thunder-ecam.c b/drivers/pci/host/pci-thunder-ecam.c
index 540d030..a9fc1c9 100644
--- a/drivers/pci/host/pci-thunder-ecam.c
+++ b/drivers/pci/host/pci-thunder-ecam.c
@@ -11,10 +11,9 @@
#include <linux/ioport.h>
#include <linux/of_pci.h>
#include <linux/of.h>
+#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
-#include "../ecam.h"
-
static void set_val(u32 v, int where, int size, u32 *val)
{
int shift = (where & 3) * 8;
diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
index 9b8ab94..5020d3d 100644
--- a/drivers/pci/host/pci-thunder-pem.c
+++ b/drivers/pci/host/pci-thunder-pem.c
@@ -18,10 +18,9 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
+#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
-#include "../ecam.h"
-
#define PEM_CFG_WR 0x28
#define PEM_CFG_RD 0x30
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
new file mode 100644
index 0000000..9878beb
--- /dev/null
+++ b/include/linux/pci-ecam.h
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2016 Broadcom
+ *
+ * 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 DRIVERS_PCI_ECAM_H
+#define DRIVERS_PCI_ECAM_H
+
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+
+/*
+ * struct to hold pci ops and bus shift of the config window
+ * for a PCI controller.
+ */
+struct pci_config_window;
+struct pci_ecam_ops {
+ unsigned int bus_shift;
+ struct pci_ops pci_ops;
+ int (*init)(struct device *,
+ struct pci_config_window *);
+};
+
+/*
+ * struct to hold the mappings of a config space window. This
+ * is expected to be used as sysdata for PCI controllers that
+ * use ECAM.
+ */
+struct pci_config_window {
+ struct resource res;
+ struct resource busr;
+ void *priv;
+ struct pci_ecam_ops *ops;
+ union {
+ void __iomem *win; /* 64-bit single mapping */
+ void __iomem **winp; /* 32-bit per-bus mapping */
+ };
+};
+
+/* create and free pci_config_window */
+struct pci_config_window *pci_ecam_create(struct device *dev,
+ struct resource *cfgres, struct resource *busr,
+ struct pci_ecam_ops *ops);
+void pci_ecam_free(struct pci_config_window *cfg);
+
+/* map_bus when ->sysdata is an instance of pci_config_window */
+void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
+ int where);
+/* default ECAM ops */
+extern struct pci_ecam_ops pci_generic_ecam_ops;
+
+#ifdef CONFIG_PCI_HOST_GENERIC
+/* for DT-based PCI controllers that support ECAM */
+int pci_host_common_probe(struct platform_device *pdev,
+ struct pci_ecam_ops *ops);
+#endif
+#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH V9 02/11] PCI/ECAM: Add parent device field to pci_config_window
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 01/11] PCI/ECAM: Move ecam.h to linux/include/pci-ecam.h Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 03/11] PCI: Add new function to unmap IO resources Tomasz Nowicki
` (9 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov
From: Jayachandran C <jchandra@broadcom.com>
Add a parent device field to struct pci_config_window. The parent
is not saved now, but will be useful to save it in some cases.
Specifically in case of ACPI for ARM64, it can be used to setup
ACPI companion and domain.
Since the parent dev is in struct pci_config_window now, we need
not pass it to the init function as a separate argument.
Signed-off-by: Jayachandran C <jchandra@broadcom.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
drivers/pci/ecam.c | 3 ++-
drivers/pci/host/pci-thunder-pem.c | 3 ++-
include/linux/pci-ecam.h | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
index 820e26b..66e0d71 100644
--- a/drivers/pci/ecam.c
+++ b/drivers/pci/ecam.c
@@ -51,6 +51,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
if (!cfg)
return ERR_PTR(-ENOMEM);
+ cfg->parent = dev;
cfg->ops = ops;
cfg->busr.start = busr->start;
cfg->busr.end = busr->end;
@@ -94,7 +95,7 @@ struct pci_config_window *pci_ecam_create(struct device *dev,
}
if (ops->init) {
- err = ops->init(dev, cfg);
+ err = ops->init(cfg);
if (err)
goto err_exit;
}
diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
index 5020d3d..91f6fc6 100644
--- a/drivers/pci/host/pci-thunder-pem.c
+++ b/drivers/pci/host/pci-thunder-pem.c
@@ -284,8 +284,9 @@ static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
return pci_generic_config_write(bus, devfn, where, size, val);
}
-static int thunder_pem_init(struct device *dev, struct pci_config_window *cfg)
+static int thunder_pem_init(struct pci_config_window *cfg)
{
+ struct device *dev = cfg->parent;
resource_size_t bar4_start;
struct resource *res_pem;
struct thunder_pem_pci *pem_pci;
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index 9878beb..7adad20 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -27,8 +27,7 @@ struct pci_config_window;
struct pci_ecam_ops {
unsigned int bus_shift;
struct pci_ops pci_ops;
- int (*init)(struct device *,
- struct pci_config_window *);
+ int (*init)(struct pci_config_window *);
};
/*
@@ -45,6 +44,7 @@ struct pci_config_window {
void __iomem *win; /* 64-bit single mapping */
void __iomem **winp; /* 32-bit per-bus mapping */
};
+ struct device *parent;/* ECAM res was from this dev */
};
/* create and free pci_config_window */
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH V9 03/11] PCI: Add new function to unmap IO resources
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 01/11] PCI/ECAM: Move ecam.h to linux/include/pci-ecam.h Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 02/11] PCI/ECAM: Add parent device field to pci_config_window Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 04/11] ACPI/PCI: Support IO resources when parsing PCI host bridge resources Tomasz Nowicki
` (8 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
From: Sinan Kaya <okaya@codeaurora.org>
We need to release I/O resources so that the same I/O resources
can be allocated again in pci_remap_iospace(), like in PCI hotplug removal
scenario. Therefore implement new pci_unmap_iospace() call which
unmaps I/O space as the symmetry to pci_remap_iospace().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
drivers/pci/pci.c | 18 ++++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c8b4dbd..eb431b5 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -25,6 +25,7 @@
#include <linux/device.h>
#include <linux/pm_runtime.h>
#include <linux/pci_hotplug.h>
+#include <linux/vmalloc.h>
#include <asm/setup.h>
#include <linux/aer.h>
#include "pci.h"
@@ -3165,6 +3166,23 @@ int __weak pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
#endif
}
+/**
+ * pci_unmap_iospace - Unmap the memory mapped I/O space
+ * @res: resource to be unmapped
+ *
+ * Unmap the CPU virtual address @res from virtual address space.
+ * Only architectures that have memory mapped IO functions defined
+ * (and the PCI_IOBASE value defined) should call this function.
+ */
+void pci_unmap_iospace(struct resource *res)
+{
+#if defined(PCI_IOBASE) && defined(CONFIG_MMU)
+ unsigned long vaddr = (unsigned long)PCI_IOBASE + res->start;
+
+ unmap_kernel_range(vaddr, resource_size(res));
+#endif
+}
+
static void __pci_set_master(struct pci_dev *dev, bool enable)
{
u16 old_cmd, cmd;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b67e4df..12349de 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1167,6 +1167,7 @@ int pci_register_io_range(phys_addr_t addr, resource_size_t size);
unsigned long pci_address_to_pio(phys_addr_t addr);
phys_addr_t pci_pio_to_address(unsigned long pio);
int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
+void pci_unmap_iospace(struct resource *res);
static inline pci_bus_addr_t pci_bus_address(struct pci_dev *pdev, int bar)
{
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH V9 04/11] ACPI/PCI: Support IO resources when parsing PCI host bridge resources
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (2 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 03/11] PCI: Add new function to unmap IO resources Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 05/11] ACPI/PCI: Add generic MCFG table handling Tomasz Nowicki
` (7 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
From: Jayachandran C <jchandra@broadcom.com>
Platforms that have memory mapped IO port (such as ARM64) need special
handling for PCI I/O resources. For host bridge's resource probing case
these resources need to be fixed up with
pci_register_io_range()/pci_remap_iospace() etc.
The same I/O resources need to be released after hotplug
removal so that it can be re-added back by the pci_remap_iospace()
function during insertion. As a consequence unmap I/O resources
with pci_unmap_iospace() when we release host bridge resources.
Signed-off-by: Jayachandran C <jchandra@broadcom.com>
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
[ Tomasz: merged in Sinan's patch to unmap IO resources properly, updated changelog]
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
drivers/acpi/pci_root.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index ae3fe4e..9a26dd1 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -720,6 +720,40 @@ next:
}
}
+#ifdef PCI_IOBASE
+static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
+{
+ struct resource *res = entry->res;
+ resource_size_t cpu_addr = res->start;
+ resource_size_t pci_addr = cpu_addr - entry->offset;
+ resource_size_t length = resource_size(res);
+ unsigned long port;
+
+ if (pci_register_io_range(cpu_addr, length))
+ goto err;
+
+ port = pci_address_to_pio(cpu_addr);
+ if (port == (unsigned long)-1)
+ goto err;
+
+ res->start = port;
+ res->end = port + length - 1;
+ entry->offset = port - pci_addr;
+
+ if (pci_remap_iospace(res, cpu_addr) < 0)
+ goto err;
+
+ pr_info("Remapped I/O %pa to %pR\n", &cpu_addr, res);
+ return;
+err:
+ res->flags |= IORESOURCE_DISABLED;
+}
+#else
+static inline void acpi_pci_root_remap_iospace(struct resource_entry *entry)
+{
+}
+#endif
+
int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
{
int ret;
@@ -740,6 +774,9 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
"no IO and memory resources present in _CRS\n");
else {
resource_list_for_each_entry_safe(entry, tmp, list) {
+ if (entry->res->flags & IORESOURCE_IO)
+ acpi_pci_root_remap_iospace(entry);
+
if (entry->res->flags & IORESOURCE_DISABLED)
resource_list_destroy_entry(entry);
else
@@ -811,6 +848,8 @@ static void acpi_pci_root_release_info(struct pci_host_bridge *bridge)
resource_list_for_each_entry(entry, &bridge->windows) {
res = entry->res;
+ if (res->flags & IORESOURCE_IO)
+ pci_unmap_iospace(res);
if (res->parent &&
(res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
release_resource(res);
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH V9 05/11] ACPI/PCI: Add generic MCFG table handling
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (3 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 04/11] ACPI/PCI: Support IO resources when parsing PCI host bridge resources Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 23:25 ` Bjorn Helgaas
2016-06-10 19:55 ` [PATCH V9 06/11] PCI: Refactor generic bus domain assignment Tomasz Nowicki
` (6 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
According to PCI firmware specifications, on systems booting with ACPI,
PCI configuration for a host bridge must be set-up through the MCFG table
regions for non-hotpluggable bridges and _CBA method for hotpluggable ones.
Current MCFG table handling code, as implemented for x86, cannot be
easily generalized owing to x86 specific quirks handling and related
code, which makes it hard to reuse on other architectures.
In order to implement MCFG PCI configuration handling for new platforms
booting with ACPI (eg ARM64) this patch re-implements MCFG handling from
scratch in a streamlined fashion and provides (through a generic
interface available to all arches):
- Simplified MCFG table parsing (executed through the pci_mmcfg_late_init()
hook as in current x86)
- MCFG regions look-up interface through domain:bus_start:bus_end tuple
The new MCFG regions handling interface is added to generic ACPI code
so that existing architectures (eg x86) can be moved over to it and
architectures relying on MCFG for ACPI PCI config space can rely on it
without having to resort to arch specific implementations.
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Jayachandran C <jchandra@broadcom.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
drivers/acpi/Kconfig | 3 ++
drivers/acpi/Makefile | 1 +
drivers/acpi/pci_mcfg.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci-acpi.h | 2 ++
include/linux/pci.h | 2 +-
5 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 drivers/acpi/pci_mcfg.c
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b7e2e77..f98c328 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -217,6 +217,9 @@ config ACPI_PROCESSOR_IDLE
bool
select CPU_IDLE
+config ACPI_MCFG
+ bool
+
config ACPI_CPPC_LIB
bool
depends on ACPI_PROCESSOR
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 251ce85..632e81f 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -40,6 +40,7 @@ acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
acpi-y += ec.o
acpi-$(CONFIG_ACPI_DOCK) += dock.o
acpi-y += pci_root.o pci_link.o pci_irq.o
+obj-$(CONFIG_ACPI_MCFG) += pci_mcfg.o
acpi-y += acpi_lpss.o acpi_apd.o
acpi-y += acpi_platform.o
acpi-y += acpi_pnp.o
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
new file mode 100644
index 0000000..d3c3e85
--- /dev/null
+++ b/drivers/acpi/pci_mcfg.c
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ * Author: Jayachandran C <jchandra@broadcom.com>
+ * 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.
+ */
+
+#define pr_fmt(fmt) "ACPI: " fmt
+
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+
+/* Structure to hold entries from the MCFG table */
+struct mcfg_entry {
+ struct list_head list;
+ phys_addr_t addr;
+ u16 segment;
+ u8 bus_start;
+ u8 bus_end;
+};
+
+/* List to save mcfg entries */
+static LIST_HEAD(pci_mcfg_list);
+
+phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
+{
+ struct mcfg_entry *e;
+
+ /*
+ * We expect exact match, unless MCFG entry end bus covers more than
+ * specified by caller.
+ */
+ list_for_each_entry(e, &pci_mcfg_list, list) {
+ if (e->segment == seg && e->bus_start == bus_res->start &&
+ e->bus_end >= bus_res->end)
+ return e->addr;
+ }
+
+ return 0;
+}
+
+static __init int pci_mcfg_parse(struct acpi_table_header *header)
+{
+ struct acpi_table_mcfg *mcfg;
+ struct acpi_mcfg_allocation *mptr;
+ struct mcfg_entry *e, *arr;
+ int i, n;
+
+ if (header->length < sizeof(struct acpi_table_mcfg))
+ return -EINVAL;
+
+ n = (header->length - sizeof(struct acpi_table_mcfg)) /
+ sizeof(struct acpi_mcfg_allocation);
+ mcfg = (struct acpi_table_mcfg *)header;
+ mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
+
+ arr = kcalloc(n, sizeof(*arr), GFP_KERNEL);
+ if (!arr)
+ return -ENOMEM;
+
+ for (i = 0, e = arr; i < n; i++, mptr++, e++) {
+ e->segment = mptr->pci_segment;
+ e->addr = mptr->address;
+ e->bus_start = mptr->start_bus_number;
+ e->bus_end = mptr->end_bus_number;
+ list_add(&e->list, &pci_mcfg_list);
+ }
+
+ pr_info("MCFG table detected, %d entries\n", n);
+ return 0;
+}
+
+/* Interface called by ACPI - parse and save MCFG table */
+void __init pci_mmcfg_late_init(void)
+{
+ int err = acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
+ if (err)
+ pr_err("Failed to parse MCFG (%d)\n", err);
+}
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 89ab057..7d63a66 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -24,6 +24,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);
+
static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
{
struct pci_bus *pbus = pdev->bus;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 12349de..ce03d65 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1723,7 +1723,7 @@ void pcibios_free_irq(struct pci_dev *dev);
extern struct dev_pm_ops pcibios_pm_ops;
#endif
-#ifdef CONFIG_PCI_MMCONFIG
+#if defined(CONFIG_PCI_MMCONFIG) || defined(CONFIG_ACPI_MCFG)
void __init pci_mmcfg_early_init(void);
void __init pci_mmcfg_late_init(void);
#else
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH V9 05/11] ACPI/PCI: Add generic MCFG table handling
2016-06-10 19:55 ` [PATCH V9 05/11] ACPI/PCI: Add generic MCFG table handling Tomasz Nowicki
@ 2016-06-10 23:25 ` Bjorn Helgaas
0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Helgaas @ 2016-06-10 23:25 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On Fri, Jun 10, 2016 at 09:55:13PM +0200, Tomasz Nowicki wrote:
> According to PCI firmware specifications, on systems booting with ACPI,
> PCI configuration for a host bridge must be set-up through the MCFG table
> regions for non-hotpluggable bridges and _CBA method for hotpluggable ones.
>
> Current MCFG table handling code, as implemented for x86, cannot be
> easily generalized owing to x86 specific quirks handling and related
> code, which makes it hard to reuse on other architectures.
>
> In order to implement MCFG PCI configuration handling for new platforms
> booting with ACPI (eg ARM64) this patch re-implements MCFG handling from
> scratch in a streamlined fashion and provides (through a generic
> interface available to all arches):
>
> - Simplified MCFG table parsing (executed through the pci_mmcfg_late_init()
> hook as in current x86)
> - MCFG regions look-up interface through domain:bus_start:bus_end tuple
>
> The new MCFG regions handling interface is added to generic ACPI code
> so that existing architectures (eg x86) can be moved over to it and
> architectures relying on MCFG for ACPI PCI config space can rely on it
> without having to resort to arch specific implementations.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
> drivers/acpi/Kconfig | 3 ++
> drivers/acpi/Makefile | 1 +
> drivers/acpi/pci_mcfg.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pci-acpi.h | 2 ++
> include/linux/pci.h | 2 +-
> 5 files changed, 99 insertions(+), 1 deletion(-)
> create mode 100644 drivers/acpi/pci_mcfg.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b7e2e77..f98c328 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -217,6 +217,9 @@ config ACPI_PROCESSOR_IDLE
> bool
> select CPU_IDLE
>
> +config ACPI_MCFG
> + bool
> +
> config ACPI_CPPC_LIB
> bool
> depends on ACPI_PROCESSOR
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 251ce85..632e81f 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -40,6 +40,7 @@ acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
> acpi-y += ec.o
> acpi-$(CONFIG_ACPI_DOCK) += dock.o
> acpi-y += pci_root.o pci_link.o pci_irq.o
> +obj-$(CONFIG_ACPI_MCFG) += pci_mcfg.o
> acpi-y += acpi_lpss.o acpi_apd.o
> acpi-y += acpi_platform.o
> acpi-y += acpi_pnp.o
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> new file mode 100644
> index 0000000..d3c3e85
> --- /dev/null
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -0,0 +1,92 @@
> +/*
> + * Copyright (C) 2016 Broadcom
> + * Author: Jayachandran C <jchandra@broadcom.com>
> + * 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.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +
> +/* Structure to hold entries from the MCFG table */
> +struct mcfg_entry {
> + struct list_head list;
> + phys_addr_t addr;
> + u16 segment;
> + u8 bus_start;
> + u8 bus_end;
> +};
> +
> +/* List to save mcfg entries */
> +static LIST_HEAD(pci_mcfg_list);
> +
> +phys_addr_t pci_mcfg_lookup(u16 seg, struct resource *bus_res)
> +{
> + struct mcfg_entry *e;
> +
> + /*
> + * We expect exact match, unless MCFG entry end bus covers more than
> + * specified by caller.
> + */
> + list_for_each_entry(e, &pci_mcfg_list, list) {
> + if (e->segment == seg && e->bus_start == bus_res->start &&
> + e->bus_end >= bus_res->end)
> + return e->addr;
I think this is more strict than necessary. Here's a common situation
on x86:
MCFG for domain 0000 [bus 00-ff] at [mem 0xe0000000-0xefffffff]
ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-7f])
ACPI: PCI Root Bridge [PCI1] (domain 0000 [bus 80-ff])
We would fail on PCI1 because the start doesn't match. But what you
have is fine for now, and we can loosen this up later if necessary.
> + }
> +
> + return 0;
> +}
> +
> +static __init int pci_mcfg_parse(struct acpi_table_header *header)
> +{
> + struct acpi_table_mcfg *mcfg;
> + struct acpi_mcfg_allocation *mptr;
> + struct mcfg_entry *e, *arr;
> + int i, n;
> +
> + if (header->length < sizeof(struct acpi_table_mcfg))
> + return -EINVAL;
> +
> + n = (header->length - sizeof(struct acpi_table_mcfg)) /
> + sizeof(struct acpi_mcfg_allocation);
> + mcfg = (struct acpi_table_mcfg *)header;
> + mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
> +
> + arr = kcalloc(n, sizeof(*arr), GFP_KERNEL);
> + if (!arr)
> + return -ENOMEM;
> +
> + for (i = 0, e = arr; i < n; i++, mptr++, e++) {
> + e->segment = mptr->pci_segment;
> + e->addr = mptr->address;
> + e->bus_start = mptr->start_bus_number;
> + e->bus_end = mptr->end_bus_number;
> + list_add(&e->list, &pci_mcfg_list);
> + }
> +
> + pr_info("MCFG table detected, %d entries\n", n);
> + return 0;
> +}
> +
> +/* Interface called by ACPI - parse and save MCFG table */
> +void __init pci_mmcfg_late_init(void)
> +{
> + int err = acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
> + if (err)
> + pr_err("Failed to parse MCFG (%d)\n", err);
> +}
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 89ab057..7d63a66 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -24,6 +24,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);
> +
> static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
> {
> struct pci_bus *pbus = pdev->bus;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 12349de..ce03d65 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1723,7 +1723,7 @@ void pcibios_free_irq(struct pci_dev *dev);
> extern struct dev_pm_ops pcibios_pm_ops;
> #endif
>
> -#ifdef CONFIG_PCI_MMCONFIG
> +#if defined(CONFIG_PCI_MMCONFIG) || defined(CONFIG_ACPI_MCFG)
> void __init pci_mmcfg_early_init(void);
> void __init pci_mmcfg_late_init(void);
> #else
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH V9 06/11] PCI: Refactor generic bus domain assignment
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (4 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 05/11] ACPI/PCI: Add generic MCFG table handling Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 20:50 ` Lorenzo Pieralisi
2016-06-10 19:55 ` [PATCH V9 07/11] PCI: Factor DT specific pci_bus_find_domain_nr() code out Tomasz Nowicki
` (5 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
Change the way PCI bus domain number is assigned and improve function
name to reflect what function does. No functional changes.
Instead of assigning bus domain number inside of pci_bus_assign_domain_nr()
simply return domain number and let pci_create_root_bus() do assignment.
This way pci_create_root_bus() setups bus structure data in the consistent
way. Since pci_bus_assign_domain_nr() now does not assign but retrieves
domain number instead, rename it to pci_bus_find_domain_nr().
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
drivers/pci/pci.c | 4 ++--
drivers/pci/probe.c | 4 +++-
include/linux/pci.h | 7 +------
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index eb431b5..b9a7833 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4941,7 +4941,7 @@ int pci_get_new_domain_nr(void)
}
#ifdef CONFIG_PCI_DOMAINS_GENERIC
-void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
+int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
{
static int use_dt_domains = -1;
int domain = -1;
@@ -4985,7 +4985,7 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
domain = -1;
}
- bus->domain_nr = domain;
+ return domain;
}
#endif
#endif
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8e3ef72..380d46d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2127,7 +2127,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
b->sysdata = sysdata;
b->ops = ops;
b->number = b->busn_res.start = bus;
- pci_bus_assign_domain_nr(b, parent);
+#ifdef CONFIG_PCI_DOMAINS_GENERIC
+ b->domain_nr = pci_bus_find_domain_nr(b, parent);
+#endif
b2 = pci_find_bus(pci_domain_nr(b), bus);
if (b2) {
/* If we already got to this bus through a different bridge, ignore it */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ce03d65..48839e8 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1390,12 +1390,7 @@ static inline int pci_domain_nr(struct pci_bus *bus)
{
return bus->domain_nr;
}
-void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent);
-#else
-static inline void pci_bus_assign_domain_nr(struct pci_bus *bus,
- struct device *parent)
-{
-}
+int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent);
#endif
/* some architectures require additional setup to direct VGA traffic */
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH V9 06/11] PCI: Refactor generic bus domain assignment
2016-06-10 19:55 ` [PATCH V9 06/11] PCI: Refactor generic bus domain assignment Tomasz Nowicki
@ 2016-06-10 20:50 ` Lorenzo Pieralisi
0 siblings, 0 replies; 30+ messages in thread
From: Lorenzo Pieralisi @ 2016-06-10 20:50 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
okaya, jchandra, robert.richter, mw, Liviu.Dudau, ddaney,
wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi, jcm,
andrea.gallo, dhdang, jeremy.linton, liudongdong3, cov
On Fri, Jun 10, 2016 at 09:55:14PM +0200, Tomasz Nowicki wrote:
> Change the way PCI bus domain number is assigned and improve function
> name to reflect what function does. No functional changes.
>
> Instead of assigning bus domain number inside of pci_bus_assign_domain_nr()
> simply return domain number and let pci_create_root_bus() do assignment.
> This way pci_create_root_bus() setups bus structure data in the consistent
> way. Since pci_bus_assign_domain_nr() now does not assign but retrieves
> domain number instead, rename it to pci_bus_find_domain_nr().
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
> drivers/pci/pci.c | 4 ++--
> drivers/pci/probe.c | 4 +++-
> include/linux/pci.h | 7 +------
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index eb431b5..b9a7833 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4941,7 +4941,7 @@ int pci_get_new_domain_nr(void)
> }
>
> #ifdef CONFIG_PCI_DOMAINS_GENERIC
> -void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
> +int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
> {
> static int use_dt_domains = -1;
> int domain = -1;
> @@ -4985,7 +4985,7 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
> domain = -1;
> }
>
> - bus->domain_nr = domain;
> + return domain;
> }
> #endif
> #endif
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8e3ef72..380d46d 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2127,7 +2127,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> b->sysdata = sysdata;
> b->ops = ops;
> b->number = b->busn_res.start = bus;
> - pci_bus_assign_domain_nr(b, parent);
> +#ifdef CONFIG_PCI_DOMAINS_GENERIC
> + b->domain_nr = pci_bus_find_domain_nr(b, parent);
> +#endif
> b2 = pci_find_bus(pci_domain_nr(b), bus);
> if (b2) {
> /* If we already got to this bus through a different bridge, ignore it */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index ce03d65..48839e8 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1390,12 +1390,7 @@ static inline int pci_domain_nr(struct pci_bus *bus)
> {
> return bus->domain_nr;
> }
> -void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent);
> -#else
> -static inline void pci_bus_assign_domain_nr(struct pci_bus *bus,
> - struct device *parent)
> -{
> -}
> +int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent);
> #endif
>
> /* some architectures require additional setup to direct VGA traffic */
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH V9 07/11] PCI: Factor DT specific pci_bus_find_domain_nr() code out
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (5 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 06/11] PCI: Refactor generic bus domain assignment Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 20:51 ` Lorenzo Pieralisi
2016-06-10 19:55 ` [PATCH V9 08/11] ARM64/PCI: Add ACPI hook to assign domain number Tomasz Nowicki
` (4 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
pci_bus_find_domain_nr() retrieves the host bridge domain number in a DT
specific way. Factor our pci_bus_find_domain_nr() in a separate DT
function (ie of_pci_bus_find_domain_nr()) so that DT code is self
contained, paving the way for retrieving domain number in
pci_bus_find_domain_nr() with additional firmware methods (ie ACPI).
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
drivers/pci/pci.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b9a7833..327828d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4941,7 +4941,7 @@ int pci_get_new_domain_nr(void)
}
#ifdef CONFIG_PCI_DOMAINS_GENERIC
-int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
+static int of_pci_bus_find_domain_nr(struct device *parent)
{
static int use_dt_domains = -1;
int domain = -1;
@@ -4987,6 +4987,11 @@ int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
return domain;
}
+
+int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
+{
+ return of_pci_bus_find_domain_nr(parent);
+}
#endif
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH V9 07/11] PCI: Factor DT specific pci_bus_find_domain_nr() code out
2016-06-10 19:55 ` [PATCH V9 07/11] PCI: Factor DT specific pci_bus_find_domain_nr() code out Tomasz Nowicki
@ 2016-06-10 20:51 ` Lorenzo Pieralisi
0 siblings, 0 replies; 30+ messages in thread
From: Lorenzo Pieralisi @ 2016-06-10 20:51 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
okaya, jchandra, robert.richter, mw, Liviu.Dudau, ddaney,
wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi, jcm,
andrea.gallo, dhdang, jeremy.linton, liudongdong3, cov
On Fri, Jun 10, 2016 at 09:55:15PM +0200, Tomasz Nowicki wrote:
> pci_bus_find_domain_nr() retrieves the host bridge domain number in a DT
> specific way. Factor our pci_bus_find_domain_nr() in a separate DT
s/our/out
> function (ie of_pci_bus_find_domain_nr()) so that DT code is self
> contained, paving the way for retrieving domain number in
> pci_bus_find_domain_nr() with additional firmware methods (ie ACPI).
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
> drivers/pci/pci.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b9a7833..327828d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4941,7 +4941,7 @@ int pci_get_new_domain_nr(void)
> }
>
> #ifdef CONFIG_PCI_DOMAINS_GENERIC
> -int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
> +static int of_pci_bus_find_domain_nr(struct device *parent)
> {
> static int use_dt_domains = -1;
> int domain = -1;
> @@ -4987,6 +4987,11 @@ int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
>
> return domain;
> }
> +
> +int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
> +{
> + return of_pci_bus_find_domain_nr(parent);
> +}
> #endif
> #endif
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH V9 08/11] ARM64/PCI: Add ACPI hook to assign domain number
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (6 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 07/11] PCI: Factor DT specific pci_bus_find_domain_nr() code out Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 19:55 ` [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code Tomasz Nowicki
` (3 subsequent siblings)
11 siblings, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki,
Lorenzo Pieralisi
PCI core code provides a config option (CONFIG_PCI_DOMAINS_GENERIC)
that allows assigning the PCI bus domain number generically by
relying on device tree bindings, and falling back to a simple counter
when the respective DT properties (ie "linux,pci-domain") are not
specified in the host bridge device tree node.
In a similar way, when a system is booted through ACPI, architectures
that are selecting CONFIG_PCI_DOMAINS_GENERIC (ie ARM64) require kernel
hooks to retrieve the domain number so that the PCI bus domain number
set-up can be handled seamlessly with DT and ACPI in generic core code
when CONFIG_PCI_DOMAINS_GENERIC is selected.
Since currently it is not possible to retrieve a pointer to the PCI
host bridge ACPI device backing the host bridge from core PCI code
(which would allow retrieving the domain number in an arch agnostic
way through the ACPI _SEG method), an arch specific ACPI hook has to
be declared and implemented by all arches that rely on
CONFIG_PCI_DOMAINS_GENERIC to retrieve the domain number and set it
up in core PCI code.
For the aforementioned reasons, introduce acpi_pci_bus_find_domain_nr()
hook to retrieve the domain number on a per-arch basis when the system
boots through ACPI. ARM64 dummy implementation of the same is provided
in first place in preparation for ARM64 ACPI based PCI host controller
driver.
acpi_pci_bus_find_domain_nr() is called from generic
pci_bus_find_domain_nr() as an ACPI option to DT domain assignment.
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
arch/arm64/kernel/pci.c | 7 +++++++
drivers/pci/pci.c | 4 +++-
include/linux/pci.h | 7 +++++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 3c4e308..d5d3d26 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -17,6 +17,7 @@
#include <linux/mm.h>
#include <linux/of_pci.h>
#include <linux/of_platform.h>
+#include <linux/pci.h>
#include <linux/slab.h>
/*
@@ -85,6 +86,12 @@ EXPORT_SYMBOL(pcibus_to_node);
#endif
#ifdef CONFIG_ACPI
+
+int acpi_pci_bus_find_domain_nr(struct pci_bus *bus)
+{
+ return 0;
+}
+
/* Root bridge scanning */
struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
{
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 327828d..4834cee 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -7,6 +7,7 @@
* Copyright 1997 -- 2000 Martin Mares <mj@ucw.cz>
*/
+#include <linux/acpi.h>
#include <linux/kernel.h>
#include <linux/delay.h>
#include <linux/init.h>
@@ -4990,7 +4991,8 @@ static int of_pci_bus_find_domain_nr(struct device *parent)
int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent)
{
- return of_pci_bus_find_domain_nr(parent) ;
+ return acpi_disabled ? of_pci_bus_find_domain_nr(parent) :
+ acpi_pci_bus_find_domain_nr(bus);
}
#endif
#endif
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 48839e8..49ba8af 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1390,6 +1390,13 @@ static inline int pci_domain_nr(struct pci_bus *bus)
{
return bus->domain_nr;
}
+/* Arch specific ACPI hook to set-up domain number */
+#ifdef CONFIG_ACPI
+int acpi_pci_bus_find_domain_nr(struct pci_bus *bus);
+#else
+static inline int acpi_pci_bus_find_domain_nr(struct pci_bus *bus)
+{ return 0; }
+#endif
int pci_bus_find_domain_nr(struct pci_bus *bus, struct device *parent);
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (7 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 08/11] ARM64/PCI: Add ACPI hook to assign domain number Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 23:36 ` Bjorn Helgaas
2016-06-10 19:55 ` [PATCH V9 10/11] ARM64/PCI: Implement ACPI low-level calls to access PCI_Config region from AML Tomasz Nowicki
` (2 subsequent siblings)
11 siblings, 1 reply; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
To enable PCI legacy IRQs on platforms booting with ACPI, arch code
should include ACPI specific callbacks that parse and set-up the
device IRQ number, equivalent to the DT boot path. Owing to the current
ACPI core scan handlers implementation, ACPI PCI legacy IRQs bindings
cannot be parsed at device add time, since that would trigger ACPI scan
handlers ordering issues depending on how the ACPI tables are defined.
To solve this problem and consolidate FW PCI legacy IRQs parsing in
one single pcibios callback (pending final removal), this patch moves
DT PCI IRQ parsing to the pcibios_alloc_irq() callback (called by
PCI core code at device probe time) and adds ACPI PCI legacy IRQs
parsing to the same callback too, so that FW PCI legacy IRQs parsing
is confined in one single arch callback that can be easily removed
when code parsing PCI legacy IRQs is consolidated and moved to core
PCI code.
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
arch/arm64/kernel/pci.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index d5d3d26..b3b8a2c 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
}
/*
- * Try to assign the IRQ number from DT when adding a new device
+ * Try to assign the IRQ number when probing a new device
*/
-int pcibios_add_device(struct pci_dev *dev)
+int pcibios_alloc_irq(struct pci_dev *dev)
{
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
+ if (acpi_disabled)
+ dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
+#ifdef CONFIG_ACPI
+ else
+ return acpi_pci_irq_enable(dev);
+#endif
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
2016-06-10 19:55 ` [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code Tomasz Nowicki
@ 2016-06-10 23:36 ` Bjorn Helgaas
2016-06-13 10:00 ` Tomasz Nowicki
2016-06-13 10:40 ` Lorenzo Pieralisi
0 siblings, 2 replies; 30+ messages in thread
From: Bjorn Helgaas @ 2016-06-10 23:36 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On Fri, Jun 10, 2016 at 09:55:17PM +0200, Tomasz Nowicki wrote:
> To enable PCI legacy IRQs on platforms booting with ACPI, arch code
> should include ACPI specific callbacks that parse and set-up the
> device IRQ number, equivalent to the DT boot path. Owing to the current
> ACPI core scan handlers implementation, ACPI PCI legacy IRQs bindings
> cannot be parsed at device add time, since that would trigger ACPI scan
> handlers ordering issues depending on how the ACPI tables are defined.
Uh, OK :) I can't figure out exactly what the problem is here -- I
don't know where to look if I wanted to fix the scan handler ordering
issues, and I don't know how I could tell if it would ever be safe to
move this from driver probe-time back to device add-time.
I also notice that x86 and ia64 call acpi_pci_irq_enable() even later,
when the driver *enables* the device. Is there a reason you didn't do
it at the same time as x86 and ia64? This is another of those pcibios
hooks that really don't do anything arch-specific, so I can imagine
refactoring this somehow, someday.
Did we have this conversation before? It seems vaguely familiar, so I
apologize if you already explained this once.
> To solve this problem and consolidate FW PCI legacy IRQs parsing in
> one single pcibios callback (pending final removal), this patch moves
> DT PCI IRQ parsing to the pcibios_alloc_irq() callback (called by
> PCI core code at device probe time) and adds ACPI PCI legacy IRQs
> parsing to the same callback too, so that FW PCI legacy IRQs parsing
> is confined in one single arch callback that can be easily removed
> when code parsing PCI legacy IRQs is consolidated and moved to core
> PCI code.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
> arch/arm64/kernel/pci.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index d5d3d26..b3b8a2c 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> }
>
> /*
> - * Try to assign the IRQ number from DT when adding a new device
> + * Try to assign the IRQ number when probing a new device
> */
> -int pcibios_add_device(struct pci_dev *dev)
> +int pcibios_alloc_irq(struct pci_dev *dev)
> {
> - dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> + if (acpi_disabled)
> + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> +#ifdef CONFIG_ACPI
> + else
> + return acpi_pci_irq_enable(dev);
> +#endif
>
> return 0;
> }
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
2016-06-10 23:36 ` Bjorn Helgaas
@ 2016-06-13 10:00 ` Tomasz Nowicki
2016-06-13 10:40 ` Lorenzo Pieralisi
1 sibling, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-13 10:00 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On 11.06.2016 01:36, Bjorn Helgaas wrote:
> On Fri, Jun 10, 2016 at 09:55:17PM +0200, Tomasz Nowicki wrote:
>> To enable PCI legacy IRQs on platforms booting with ACPI, arch code
>> should include ACPI specific callbacks that parse and set-up the
>> device IRQ number, equivalent to the DT boot path. Owing to the current
>> ACPI core scan handlers implementation, ACPI PCI legacy IRQs bindings
>> cannot be parsed at device add time, since that would trigger ACPI scan
>> handlers ordering issues depending on how the ACPI tables are defined.
>
> Uh, OK :) I can't figure out exactly what the problem is here -- I
> don't know where to look if I wanted to fix the scan handler ordering
> issues, and I don't know how I could tell if it would ever be safe to
> move this from driver probe-time back to device add-time.
>
> I also notice that x86 and ia64 call acpi_pci_irq_enable() even later,
> when the driver *enables* the device. Is there a reason you didn't do
> it at the same time as x86 and ia64? This is another of those pcibios
> hooks that really don't do anything arch-specific, so I can imagine
> refactoring this somehow, someday.
>
> Did we have this conversation before? It seems vaguely familiar, so I
> apologize if you already explained this once.
It stared with a scan handler ordering issue, please see:
https://lists.linaro.org/pipermail/linaro-acpi/2015-October/005944.html
hence we decided to postpone ACPI legacy IRQs parsing. We also decided
to keep ACPI and DT legacy IRQs parsing all together as the candidate
for further PCI legacy IRQs consolidation, please see [1].
This will allow us to remove it from here (as you pointed out is is not
arch-specific at all).
I share Lorenzo's option. We could put this to pcibios_enable_device
instead of pcibios_alloc_irq but since ACPI and DT code is in one call
we wanted to avoid regressions on PCI host controllers that do not call
pci_fixup_irqs(), but rely on the legacy IRQ routing to be done in arm64
pcibios_add_device().
[1] http://www.spinics.net/lists/linux-pci/msg45950.html
Thanks,
Tomasz
>
>> To solve this problem and consolidate FW PCI legacy IRQs parsing in
>> one single pcibios callback (pending final removal), this patch moves
>> DT PCI IRQ parsing to the pcibios_alloc_irq() callback (called by
>> PCI core code at device probe time) and adds ACPI PCI legacy IRQs
>> parsing to the same callback too, so that FW PCI legacy IRQs parsing
>> is confined in one single arch callback that can be easily removed
>> when code parsing PCI legacy IRQs is consolidated and moved to core
>> PCI code.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> ---
>> arch/arm64/kernel/pci.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
>> index d5d3d26..b3b8a2c 100644
>> --- a/arch/arm64/kernel/pci.c
>> +++ b/arch/arm64/kernel/pci.c
>> @@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>> }
>>
>> /*
>> - * Try to assign the IRQ number from DT when adding a new device
>> + * Try to assign the IRQ number when probing a new device
>> */
>> -int pcibios_add_device(struct pci_dev *dev)
>> +int pcibios_alloc_irq(struct pci_dev *dev)
>> {
>> - dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
>> + if (acpi_disabled)
>> + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
>> +#ifdef CONFIG_ACPI
>> + else
>> + return acpi_pci_irq_enable(dev);
>> +#endif
>>
>> return 0;
>> }
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
2016-06-10 23:36 ` Bjorn Helgaas
2016-06-13 10:00 ` Tomasz Nowicki
@ 2016-06-13 10:40 ` Lorenzo Pieralisi
2016-06-13 15:56 ` Liviu.Dudau
2016-06-13 20:01 ` Duc Dang
1 sibling, 2 replies; 30+ messages in thread
From: Lorenzo Pieralisi @ 2016-06-13 10:40 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tomasz Nowicki, arnd, will.deacon, catalin.marinas, rafael,
hanjun.guo, okaya, jchandra, robert.richter, mw, Liviu.Dudau,
ddaney, wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi, jcm,
andrea.gallo, dhdang, jeremy.linton, liudongdong3, cov
On Fri, Jun 10, 2016 at 06:36:12PM -0500, Bjorn Helgaas wrote:
> On Fri, Jun 10, 2016 at 09:55:17PM +0200, Tomasz Nowicki wrote:
> > To enable PCI legacy IRQs on platforms booting with ACPI, arch code
> > should include ACPI specific callbacks that parse and set-up the
> > device IRQ number, equivalent to the DT boot path. Owing to the current
> > ACPI core scan handlers implementation, ACPI PCI legacy IRQs bindings
> > cannot be parsed at device add time, since that would trigger ACPI scan
> > handlers ordering issues depending on how the ACPI tables are defined.
>
> Uh, OK :) I can't figure out exactly what the problem is here -- I
> don't know where to look if I wanted to fix the scan handler ordering
> issues, and I don't know how I could tell if it would ever be safe to
> move this from driver probe-time back to device add-time.
Right, the commit log could have been more informative.
pcibios_add_device() was added in:
commit d1e6dc91b532 ("arm64: Add architectural support for PCI")
whose commit log does not specify why legacy IRQ parsing should
be done at pcibios_add_device() either, so honestly we had to
do with the information we have at hand.
> I also notice that x86 and ia64 call acpi_pci_irq_enable() even later,
> when the driver *enables* the device. Is there a reason you didn't do
> it at the same time as x86 and ia64? This is another of those pcibios
> hooks that really don't do anything arch-specific, so I can imagine
> refactoring this somehow, someday.
Yes, with [1], that was the goal, that stopped because [1] does not
work on x86.
Only DT platform(s) affected by this change are all platforms relying on
drivers/pci/host/pci-xgene.c (others rely on pci_fixup_irqs() that
should be removed too), if on those platforms probing IRQs at device
enable time works ok I can update this patch (it can be done through [1]
once we figure out what to do with it on x86) and move the IRQ set-up at
pcibios_enable_device() time.
@Duc: any feedback on this ?
Thanks,
Lorenzo
[1] http://www.spinics.net/lists/linux-pci/msg45950.html
> Did we have this conversation before? It seems vaguely familiar, so I
> apologize if you already explained this once.
>
> > To solve this problem and consolidate FW PCI legacy IRQs parsing in
> > one single pcibios callback (pending final removal), this patch moves
> > DT PCI IRQ parsing to the pcibios_alloc_irq() callback (called by
> > PCI core code at device probe time) and adds ACPI PCI legacy IRQs
> > parsing to the same callback too, so that FW PCI legacy IRQs parsing
> > is confined in one single arch callback that can be easily removed
> > when code parsing PCI legacy IRQs is consolidated and moved to core
> > PCI code.
> >
> > Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> > Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > ---
> > arch/arm64/kernel/pci.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index d5d3d26..b3b8a2c 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> > }
> >
> > /*
> > - * Try to assign the IRQ number from DT when adding a new device
> > + * Try to assign the IRQ number when probing a new device
> > */
> > -int pcibios_add_device(struct pci_dev *dev)
> > +int pcibios_alloc_irq(struct pci_dev *dev)
> > {
> > - dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > + if (acpi_disabled)
> > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > +#ifdef CONFIG_ACPI
> > + else
> > + return acpi_pci_irq_enable(dev);
> > +#endif
> >
> > return 0;
> > }
> > --
> > 1.9.1
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
2016-06-13 10:40 ` Lorenzo Pieralisi
@ 2016-06-13 15:56 ` Liviu.Dudau
2016-06-13 20:01 ` Duc Dang
1 sibling, 0 replies; 30+ messages in thread
From: Liviu.Dudau @ 2016-06-13 15:56 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Tomasz Nowicki, arnd, will.deacon,
catalin.marinas, rafael, hanjun.guo, okaya, jchandra,
robert.richter, mw, ddaney, wangyijing, Suravee.Suthikulpanit,
msalter, linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On Mon, Jun 13, 2016 at 11:40:05AM +0100, Lorenzo Pieralisi wrote:
> On Fri, Jun 10, 2016 at 06:36:12PM -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 10, 2016 at 09:55:17PM +0200, Tomasz Nowicki wrote:
> > > To enable PCI legacy IRQs on platforms booting with ACPI, arch code
> > > should include ACPI specific callbacks that parse and set-up the
> > > device IRQ number, equivalent to the DT boot path. Owing to the current
> > > ACPI core scan handlers implementation, ACPI PCI legacy IRQs bindings
> > > cannot be parsed at device add time, since that would trigger ACPI scan
> > > handlers ordering issues depending on how the ACPI tables are defined.
> >
> > Uh, OK :) I can't figure out exactly what the problem is here -- I
> > don't know where to look if I wanted to fix the scan handler ordering
> > issues, and I don't know how I could tell if it would ever be safe to
> > move this from driver probe-time back to device add-time.
>
> Right, the commit log could have been more informative.
>
> pcibios_add_device() was added in:
>
> commit d1e6dc91b532 ("arm64: Add architectural support for PCI")
>
> whose commit log does not specify why legacy IRQ parsing should
> be done at pcibios_add_device() either, so honestly we had to
> do with the information we have at hand.
Because at that time (September 2014) there was no pcibios_alloc_irq() (added
June 2015 by commit 890e4847587fcf) and after that nobody bothered to update
the arm64 code to make use of the new API.
Best regards,
Liviu
>
> > I also notice that x86 and ia64 call acpi_pci_irq_enable() even later,
> > when the driver *enables* the device. Is there a reason you didn't do
> > it at the same time as x86 and ia64? This is another of those pcibios
> > hooks that really don't do anything arch-specific, so I can imagine
> > refactoring this somehow, someday.
>
> Yes, with [1], that was the goal, that stopped because [1] does not
> work on x86.
>
> Only DT platform(s) affected by this change are all platforms relying on
> drivers/pci/host/pci-xgene.c (others rely on pci_fixup_irqs() that
> should be removed too), if on those platforms probing IRQs at device
> enable time works ok I can update this patch (it can be done through [1]
> once we figure out what to do with it on x86) and move the IRQ set-up at
> pcibios_enable_device() time.
>
> @Duc: any feedback on this ?
>
> Thanks,
> Lorenzo
>
> [1] http://www.spinics.net/lists/linux-pci/msg45950.html
>
> > Did we have this conversation before? It seems vaguely familiar, so I
> > apologize if you already explained this once.
> >
> > > To solve this problem and consolidate FW PCI legacy IRQs parsing in
> > > one single pcibios callback (pending final removal), this patch moves
> > > DT PCI IRQ parsing to the pcibios_alloc_irq() callback (called by
> > > PCI core code at device probe time) and adds ACPI PCI legacy IRQs
> > > parsing to the same callback too, so that FW PCI legacy IRQs parsing
> > > is confined in one single arch callback that can be easily removed
> > > when code parsing PCI legacy IRQs is consolidated and moved to core
> > > PCI code.
> > >
> > > Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> > > Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > ---
> > > arch/arm64/kernel/pci.c | 11 ++++++++---
> > > 1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > > index d5d3d26..b3b8a2c 100644
> > > --- a/arch/arm64/kernel/pci.c
> > > +++ b/arch/arm64/kernel/pci.c
> > > @@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> > > }
> > >
> > > /*
> > > - * Try to assign the IRQ number from DT when adding a new device
> > > + * Try to assign the IRQ number when probing a new device
> > > */
> > > -int pcibios_add_device(struct pci_dev *dev)
> > > +int pcibios_alloc_irq(struct pci_dev *dev)
> > > {
> > > - dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > > + if (acpi_disabled)
> > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > > +#ifdef CONFIG_ACPI
> > > + else
> > > + return acpi_pci_irq_enable(dev);
> > > +#endif
> > >
> > > return 0;
> > > }
> > > --
> > > 1.9.1
> > >
> >
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
2016-06-13 10:40 ` Lorenzo Pieralisi
2016-06-13 15:56 ` Liviu.Dudau
@ 2016-06-13 20:01 ` Duc Dang
2016-06-14 9:30 ` Lorenzo Pieralisi
1 sibling, 1 reply; 30+ messages in thread
From: Duc Dang @ 2016-06-13 20:01 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Bjorn Helgaas, Tomasz Nowicki, Arnd Bergmann, Will Deacon,
Catalin Marinas, Rafael Wysocki, Hanjun Guo, okaya,
Jayachandran C, robert.richter, Marcin Wojtas, Liviu Dudau,
David Daney, Yijing Wang, Suravee Suthikulpanit, Mark Salter,
linux-pci, linux-arm, linux-acpi, Linux Kernel Mailing List,
linaro-acpi, Jon Masters, Andrea Gallo, jeremy.linton,
liudongdong3, cov
On Mon, Jun 13, 2016 at 3:40 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Fri, Jun 10, 2016 at 06:36:12PM -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 10, 2016 at 09:55:17PM +0200, Tomasz Nowicki wrote:
> > > To enable PCI legacy IRQs on platforms booting with ACPI, arch code
> > > should include ACPI specific callbacks that parse and set-up the
> > > device IRQ number, equivalent to the DT boot path. Owing to the current
> > > ACPI core scan handlers implementation, ACPI PCI legacy IRQs bindings
> > > cannot be parsed at device add time, since that would trigger ACPI scan
> > > handlers ordering issues depending on how the ACPI tables are defined.
> >
> > Uh, OK :) I can't figure out exactly what the problem is here -- I
> > don't know where to look if I wanted to fix the scan handler ordering
> > issues, and I don't know how I could tell if it would ever be safe to
> > move this from driver probe-time back to device add-time.
>
> Right, the commit log could have been more informative.
>
> pcibios_add_device() was added in:
>
> commit d1e6dc91b532 ("arm64: Add architectural support for PCI")
>
> whose commit log does not specify why legacy IRQ parsing should
> be done at pcibios_add_device() either, so honestly we had to
> do with the information we have at hand.
>
> > I also notice that x86 and ia64 call acpi_pci_irq_enable() even later,
> > when the driver *enables* the device. Is there a reason you didn't do
> > it at the same time as x86 and ia64? This is another of those pcibios
> > hooks that really don't do anything arch-specific, so I can imagine
> > refactoring this somehow, someday.
>
> Yes, with [1], that was the goal, that stopped because [1] does not
> work on x86.
>
> Only DT platform(s) affected by this change are all platforms relying on
> drivers/pci/host/pci-xgene.c (others rely on pci_fixup_irqs() that
> should be removed too), if on those platforms probing IRQs at device
> enable time works ok I can update this patch (it can be done through [1]
> once we figure out what to do with it on x86) and move the IRQ set-up at
> pcibios_enable_device() time.
>
> @Duc: any feedback on this ?
Hi Lorenzo,
The changes to add pcibios_alloc_irq works fine on X-Gene PCIe
I also tried to remove pcibios_alloc_irq and move its code into
pcibios_enable_device
after pci_enable_resource call and legacy IRQ also works.
Can you also point me to the discussion thread or some info. about the
issue on x86 with [1]?
I want to check if there is any more test case I need to verify.
Regards,
Duc Dang.
>
> Thanks,
> Lorenzo
>
> [1] http://www.spinics.net/lists/linux-pci/msg45950.html
>
> > Did we have this conversation before? It seems vaguely familiar, so I
> > apologize if you already explained this once.
> >
> > > To solve this problem and consolidate FW PCI legacy IRQs parsing in
> > > one single pcibios callback (pending final removal), this patch moves
> > > DT PCI IRQ parsing to the pcibios_alloc_irq() callback (called by
> > > PCI core code at device probe time) and adds ACPI PCI legacy IRQs
> > > parsing to the same callback too, so that FW PCI legacy IRQs parsing
> > > is confined in one single arch callback that can be easily removed
> > > when code parsing PCI legacy IRQs is consolidated and moved to core
> > > PCI code.
> > >
> > > Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> > > Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > ---
> > > arch/arm64/kernel/pci.c | 11 ++++++++---
> > > 1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > > index d5d3d26..b3b8a2c 100644
> > > --- a/arch/arm64/kernel/pci.c
> > > +++ b/arch/arm64/kernel/pci.c
> > > @@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> > > }
> > >
> > > /*
> > > - * Try to assign the IRQ number from DT when adding a new device
> > > + * Try to assign the IRQ number when probing a new device
> > > */
> > > -int pcibios_add_device(struct pci_dev *dev)
> > > +int pcibios_alloc_irq(struct pci_dev *dev)
> > > {
> > > - dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > > + if (acpi_disabled)
> > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > > +#ifdef CONFIG_ACPI
> > > + else
> > > + return acpi_pci_irq_enable(dev);
> > > +#endif
> > >
> > > return 0;
> > > }
> > > --
> > > 1.9.1
> > >
> >
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code
2016-06-13 20:01 ` Duc Dang
@ 2016-06-14 9:30 ` Lorenzo Pieralisi
0 siblings, 0 replies; 30+ messages in thread
From: Lorenzo Pieralisi @ 2016-06-14 9:30 UTC (permalink / raw)
To: Duc Dang
Cc: Bjorn Helgaas, Tomasz Nowicki, Arnd Bergmann, Will Deacon,
Catalin Marinas, Rafael Wysocki, Hanjun Guo, okaya,
Jayachandran C, robert.richter, Marcin Wojtas, Liviu Dudau,
David Daney, Yijing Wang, Suravee Suthikulpanit, Mark Salter,
linux-pci, linux-arm, linux-acpi, Linux Kernel Mailing List,
linaro-acpi, Jon Masters, Andrea Gallo, jeremy.linton,
liudongdong3, cov
On Mon, Jun 13, 2016 at 01:01:35PM -0700, Duc Dang wrote:
> On Mon, Jun 13, 2016 at 3:40 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> >
> > On Fri, Jun 10, 2016 at 06:36:12PM -0500, Bjorn Helgaas wrote:
> > > On Fri, Jun 10, 2016 at 09:55:17PM +0200, Tomasz Nowicki wrote:
> > > > To enable PCI legacy IRQs on platforms booting with ACPI, arch code
> > > > should include ACPI specific callbacks that parse and set-up the
> > > > device IRQ number, equivalent to the DT boot path. Owing to the current
> > > > ACPI core scan handlers implementation, ACPI PCI legacy IRQs bindings
> > > > cannot be parsed at device add time, since that would trigger ACPI scan
> > > > handlers ordering issues depending on how the ACPI tables are defined.
> > >
> > > Uh, OK :) I can't figure out exactly what the problem is here -- I
> > > don't know where to look if I wanted to fix the scan handler ordering
> > > issues, and I don't know how I could tell if it would ever be safe to
> > > move this from driver probe-time back to device add-time.
> >
> > Right, the commit log could have been more informative.
> >
> > pcibios_add_device() was added in:
> >
> > commit d1e6dc91b532 ("arm64: Add architectural support for PCI")
> >
> > whose commit log does not specify why legacy IRQ parsing should
> > be done at pcibios_add_device() either, so honestly we had to
> > do with the information we have at hand.
> >
> > > I also notice that x86 and ia64 call acpi_pci_irq_enable() even later,
> > > when the driver *enables* the device. Is there a reason you didn't do
> > > it at the same time as x86 and ia64? This is another of those pcibios
> > > hooks that really don't do anything arch-specific, so I can imagine
> > > refactoring this somehow, someday.
> >
> > Yes, with [1], that was the goal, that stopped because [1] does not
> > work on x86.
> >
> > Only DT platform(s) affected by this change are all platforms relying on
> > drivers/pci/host/pci-xgene.c (others rely on pci_fixup_irqs() that
> > should be removed too), if on those platforms probing IRQs at device
> > enable time works ok I can update this patch (it can be done through [1]
> > once we figure out what to do with it on x86) and move the IRQ set-up at
> > pcibios_enable_device() time.
> >
> > @Duc: any feedback on this ?
>
> Hi Lorenzo,
>
> The changes to add pcibios_alloc_irq works fine on X-Gene PCIe
>
> I also tried to remove pcibios_alloc_irq and move its code into
> pcibios_enable_device
> after pci_enable_resource call and legacy IRQ also works.
Thank you, that was important to check.
> Can you also point me to the discussion thread or some info. about the
> issue on x86 with [1]?
> I want to check if there is any more test case I need to verify.
http://marc.info/?l=linux-pci&m=145091187918297&w=2
I do not think there is much we can do, unless we convert pci_fixup_irqs
as per [1] but keep x86 code as-is (for now), there is a way to do it
it is just a matter of taking Matt's series and refactor it, there are
potential problems (not on xgene, but on other ARM platforms
where the IRQ legacy routing is done through pci_fixup_irqs()).
Lorenzo
> Regards,
> Duc Dang.
>
> >
> > Thanks,
> > Lorenzo
> >
> > [1] http://www.spinics.net/lists/linux-pci/msg45950.html
> >
> > > Did we have this conversation before? It seems vaguely familiar, so I
> > > apologize if you already explained this once.
> > >
> > > > To solve this problem and consolidate FW PCI legacy IRQs parsing in
> > > > one single pcibios callback (pending final removal), this patch moves
> > > > DT PCI IRQ parsing to the pcibios_alloc_irq() callback (called by
> > > > PCI core code at device probe time) and adds ACPI PCI legacy IRQs
> > > > parsing to the same callback too, so that FW PCI legacy IRQs parsing
> > > > is confined in one single arch callback that can be easily removed
> > > > when code parsing PCI legacy IRQs is consolidated and moved to core
> > > > PCI code.
> > > >
> > > > Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> > > > Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > ---
> > > > arch/arm64/kernel/pci.c | 11 ++++++++---
> > > > 1 file changed, 8 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > > > index d5d3d26..b3b8a2c 100644
> > > > --- a/arch/arm64/kernel/pci.c
> > > > +++ b/arch/arm64/kernel/pci.c
> > > > @@ -51,11 +51,16 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
> > > > }
> > > >
> > > > /*
> > > > - * Try to assign the IRQ number from DT when adding a new device
> > > > + * Try to assign the IRQ number when probing a new device
> > > > */
> > > > -int pcibios_add_device(struct pci_dev *dev)
> > > > +int pcibios_alloc_irq(struct pci_dev *dev)
> > > > {
> > > > - dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > > > + if (acpi_disabled)
> > > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
> > > > +#ifdef CONFIG_ACPI
> > > > + else
> > > > + return acpi_pci_irq_enable(dev);
> > > > +#endif
> > > >
> > > > return 0;
> > > > }
> > > > --
> > > > 1.9.1
> > > >
> > >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH V9 10/11] ARM64/PCI: Implement ACPI low-level calls to access PCI_Config region from AML
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (8 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 09/11] ARM64/PCI: ACPI support for legacy IRQs parsing and consolidation with DT code Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-06-10 20:54 ` Lorenzo Pieralisi
2016-06-10 19:55 ` [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller Tomasz Nowicki
2016-06-10 23:41 ` [PATCH V9 00/11] Support for ARM64 " Bjorn Helgaas
11 siblings, 1 reply; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
ACPI spec6.1 - chapter: 5.5.2.4 defines OperationRegion (Declare Operation
Region). Following the spec: " [...] An Operation Region is a specific
region of operation within an address space that is declared as a subset
of the entire address space using a starting address (offset) and a length.
Control methods must have exclusive access to any address accessed via
fields declared in Operation Regions. [...]".
OperationRegion allows to declare various of operation region address space
identifiers including PCI_Config. PCI_Config is meant to access PCI
configuration space from the ASL. So every time ASL opcode operates
on PCI_Config space region, ASL interpreter dispatches accesses to OS
low-level calls - raw_pci_write() and raw_pci_read() for Linux - so-called
ACPI RAW accessors.
In order to support PCI_Config operation region, implement mentioned
raw_pci_write() and raw_pci_read() calls so they find associated bus
and call read/write ops.
Waiting for clarification in the ACPI specifications in relation
to PCI_Config space handling before PCI bus enumeration is completed,
current code does not support PCI_Config region accesses before PCI bus
enumeration whilst providing full AML PCI_Config access availability
when the PCI bus enumeration is completed by the kernel so that
RAW accessors can look-up PCI operations through the struct pci_bus
associated with a PCI bus.
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Jayachandran C <jchandra@broadcom.com>
---
arch/arm64/kernel/pci.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index b3b8a2c..328f857 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -71,13 +71,21 @@ int pcibios_alloc_irq(struct pci_dev *dev)
int raw_pci_read(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 *val)
{
- return -ENXIO;
+ struct pci_bus *b = pci_find_bus(domain, bus);
+
+ if (!b)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ return b->ops->read(b, devfn, reg, len, val);
}
int raw_pci_write(unsigned int domain, unsigned int bus,
unsigned int devfn, int reg, int len, u32 val)
{
- return -ENXIO;
+ struct pci_bus *b = pci_find_bus(domain, bus);
+
+ if (!b)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ return b->ops->write(b, devfn, reg, len, val);
}
#ifdef CONFIG_NUMA
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH V9 10/11] ARM64/PCI: Implement ACPI low-level calls to access PCI_Config region from AML
2016-06-10 19:55 ` [PATCH V9 10/11] ARM64/PCI: Implement ACPI low-level calls to access PCI_Config region from AML Tomasz Nowicki
@ 2016-06-10 20:54 ` Lorenzo Pieralisi
0 siblings, 0 replies; 30+ messages in thread
From: Lorenzo Pieralisi @ 2016-06-10 20:54 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
okaya, jchandra, robert.richter, mw, Liviu.Dudau, ddaney,
wangyijing, Suravee.Suthikulpanit, msalter, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, linaro-acpi, jcm,
andrea.gallo, dhdang, jeremy.linton, liudongdong3, cov
On Fri, Jun 10, 2016 at 09:55:18PM +0200, Tomasz Nowicki wrote:
> ACPI spec6.1 - chapter: 5.5.2.4 defines OperationRegion (Declare Operation
> Region). Following the spec: " [...] An Operation Region is a specific
> region of operation within an address space that is declared as a subset
> of the entire address space using a starting address (offset) and a length.
> Control methods must have exclusive access to any address accessed via
> fields declared in Operation Regions. [...]".
>
> OperationRegion allows to declare various of operation region address space
s/of//
> identifiers including PCI_Config. PCI_Config is meant to access PCI
> configuration space from the ASL. So every time ASL opcode operates
> on PCI_Config space region, ASL interpreter dispatches accesses to OS
> low-level calls - raw_pci_write() and raw_pci_read() for Linux - so-called
> ACPI RAW accessors.
>
> In order to support PCI_Config operation region, implement mentioned
> raw_pci_write() and raw_pci_read() calls so they find associated bus
> and call read/write ops.
>
> Waiting for clarification in the ACPI specifications in relation
> to PCI_Config space handling before PCI bus enumeration is completed,
> current code does not support PCI_Config region accesses before PCI bus
> enumeration whilst providing full AML PCI_Config access availability
> when the PCI bus enumeration is completed by the kernel so that
> RAW accessors can look-up PCI operations through the struct pci_bus
> associated with a PCI bus.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Jayachandran C <jchandra@broadcom.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
> arch/arm64/kernel/pci.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index b3b8a2c..328f857 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -71,13 +71,21 @@ int pcibios_alloc_irq(struct pci_dev *dev)
> int raw_pci_read(unsigned int domain, unsigned int bus,
> unsigned int devfn, int reg, int len, u32 *val)
> {
> - return -ENXIO;
> + struct pci_bus *b = pci_find_bus(domain, bus);
> +
> + if (!b)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + return b->ops->read(b, devfn, reg, len, val);
> }
>
> int raw_pci_write(unsigned int domain, unsigned int bus,
> unsigned int devfn, int reg, int len, u32 val)
> {
> - return -ENXIO;
> + struct pci_bus *b = pci_find_bus(domain, bus);
> +
> + if (!b)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + return b->ops->write(b, devfn, reg, len, val);
> }
>
> #ifdef CONFIG_NUMA
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (9 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 10/11] ARM64/PCI: Implement ACPI low-level calls to access PCI_Config region from AML Tomasz Nowicki
@ 2016-06-10 19:55 ` Tomasz Nowicki
2016-11-22 23:13 ` Bjorn Helgaas
2016-06-10 23:41 ` [PATCH V9 00/11] Support for ARM64 " Bjorn Helgaas
11 siblings, 1 reply; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-10 19:55 UTC (permalink / raw)
To: helgaas, arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra
Cc: robert.richter, mw, Liviu.Dudau, ddaney, wangyijing,
Suravee.Suthikulpanit, msalter, linux-pci, linux-arm-kernel,
linux-acpi, linux-kernel, linaro-acpi, jcm, andrea.gallo, dhdang,
jeremy.linton, liudongdong3, cov, Tomasz Nowicki
Implement pci_acpi_scan_root and other arch-specific call so that ARM64
can start using ACPI to setup and enumerate PCI buses.
Prior to buses enumeration the pci_acpi_scan_root() implementation looks
for configuration space start address (obtained through ACPI _CBA method or
MCFG interface). If succeed, it uses ECAM library to create new mapping.
Then it attaches generic ECAM ops (pci_generic_ecam_ops) which are used
for accessing configuration space later on.
On ARM64, we need to use generic domains (CONFIG_PCI_DOMAINS_GENERIC).
In order to achieve that for ACPI case implement
acpi_pci_bus_find_domain_nr() body so that it retrieves pci_config_window
structure from bus sysdata and eventually gets domain number from
acpi_pci_root structure.
ACPI requires to run acpi_pci_{add|remove}_bus while new PCI bus is created.
This allows to do some ACPI-specific additional configuration, like
PCI hotplug slot enumeration. In order to fulfill these requirements,
we implement arch-specific pcibios_{add|remove}_bus calls
and call acpi_pci_{add|remove}_bus from there.
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Signed-off-by: Jayachandran C <jchandra@broadcom.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
arch/arm64/Kconfig | 2 +
arch/arm64/kernel/pci.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5a0a691..4806cde 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -3,6 +3,7 @@ config ARM64
select ACPI_CCA_REQUIRED if ACPI
select ACPI_GENERIC_GSI if ACPI
select ACPI_REDUCED_HARDWARE_ONLY if ACPI
+ select ACPI_MCFG if ACPI
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_HAS_ELF_RANDOMIZE
@@ -96,6 +97,7 @@ config ARM64
select OF_EARLY_FLATTREE
select OF_NUMA if NUMA && OF
select OF_RESERVED_MEM
+ select PCI_ECAM if ACPI
select PERF_USE_VMALLOC
select POWER_RESET
select POWER_SUPPLY
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 328f857..94cd43c 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -18,6 +18,8 @@
#include <linux/of_pci.h>
#include <linux/of_platform.h>
#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
#include <linux/slab.h>
/*
@@ -100,15 +102,123 @@ EXPORT_SYMBOL(pcibus_to_node);
#ifdef CONFIG_ACPI
+struct acpi_pci_generic_root_info {
+ struct acpi_pci_root_info common;
+ struct pci_config_window *cfg; /* config space mapping */
+};
+
int acpi_pci_bus_find_domain_nr(struct pci_bus *bus)
{
+ struct pci_config_window *cfg = bus->sysdata;
+ struct acpi_device *adev = to_acpi_device(cfg->parent);
+ struct acpi_pci_root *root = acpi_driver_data(adev);
+
+ return root->segment;
+}
+
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+ if (!acpi_disabled) {
+ struct pci_config_window *cfg = bridge->bus->sysdata;
+ struct acpi_device *adev = to_acpi_device(cfg->parent);
+ ACPI_COMPANION_SET(&bridge->dev, adev);
+ }
+
return 0;
}
-/* Root bridge scanning */
+/*
+ * 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)
+{
+ struct acpi_pci_generic_root_info *ri;
+
+ ri = container_of(ci, struct acpi_pci_generic_root_info, common);
+ pci_ecam_free(ri->cfg);
+ kfree(ri);
+}
+
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
+ .release_info = pci_acpi_generic_release_info,
+};
+
+/* Interface called from ACPI code to setup PCI host controller */
struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
{
- /* TODO: Should be revisited when implementing PCI on ACPI */
- return NULL;
+ int node = acpi_get_node(root->device->handle);
+ struct acpi_pci_generic_root_info *ri;
+ struct pci_bus *bus, *child;
+
+ ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
+ if (!ri)
+ return NULL;
+
+ ri->cfg = pci_acpi_setup_ecam_mapping(root);
+ if (!ri->cfg) {
+ kfree(ri);
+ return NULL;
+ }
+
+ 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)
+ return NULL;
+
+ pci_bus_size_bridges(bus);
+ pci_bus_assign_resources(bus);
+
+ list_for_each_entry(child, &bus->children, node)
+ pcie_bus_configure_settings(child);
+
+ return bus;
+}
+
+void pcibios_add_bus(struct pci_bus *bus)
+{
+ acpi_pci_add_bus(bus);
}
+
+void pcibios_remove_bus(struct pci_bus *bus)
+{
+ acpi_pci_remove_bus(bus);
+}
+
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller
2016-06-10 19:55 ` [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller Tomasz Nowicki
@ 2016-11-22 23:13 ` Bjorn Helgaas
2016-11-23 11:21 ` Tomasz Nowicki
0 siblings, 1 reply; 30+ messages in thread
From: Bjorn Helgaas @ 2016-11-22 23:13 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
Hi Tomasz,
On Fri, Jun 10, 2016 at 09:55:19PM +0200, Tomasz Nowicki wrote:
> Implement pci_acpi_scan_root and other arch-specific call so that ARM64
> can start using ACPI to setup and enumerate PCI buses.
>
> Prior to buses enumeration the pci_acpi_scan_root() implementation looks
> for configuration space start address (obtained through ACPI _CBA method or
> MCFG interface). If succeed, it uses ECAM library to create new mapping.
> Then it attaches generic ECAM ops (pci_generic_ecam_ops) which are used
> for accessing configuration space later on.
> ...
> +static struct acpi_pci_root_ops acpi_pci_root_ops = {
> + .release_info = pci_acpi_generic_release_info,
> +};
> +
> +/* Interface called from ACPI code to setup PCI host controller */
> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> {
> - /* TODO: Should be revisited when implementing PCI on ACPI */
> - return NULL;
> + int node = acpi_get_node(root->device->handle);
> + struct acpi_pci_generic_root_info *ri;
> + struct pci_bus *bus, *child;
> +
> + ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
> + if (!ri)
> + return NULL;
> +
> + ri->cfg = pci_acpi_setup_ecam_mapping(root);
> + if (!ri->cfg) {
> + kfree(ri);
> + return NULL;
> + }
> +
> + acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
This has already been merged, but this isn't right, is it? We're
writing a host controller-specific pointer into the single system-wide
acpi_pci_root_ops, then passing it on to acpi_pci_root_create().
Today, I think ri->cfg->ops->pci_ops is always &pci_generic_ecam_ops,
from this path:
ri->cfg = pci_acpi_setup_ecam_mapping
cfg = pci_ecam_create(..., &pci_generic_ecam_ops)
cfg = kzalloc(...)
cfg->ops = ops # &pci_generic_ecam_ops
But we're about to merge the ECAM quirks series, which will mean it
may not be &pci_generic_ecam_ops. Even apart from the ECAM quirks, we
should avoid this pattern of putting device-specific info in a single
shared structure because it's too difficult to verify that it's
correct.
> + bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
> + ri->cfg);
Bjorn
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller
2016-11-22 23:13 ` Bjorn Helgaas
@ 2016-11-23 11:21 ` Tomasz Nowicki
2016-11-23 18:22 ` Bjorn Helgaas
0 siblings, 1 reply; 30+ messages in thread
From: Tomasz Nowicki @ 2016-11-23 11:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
Hi Bjorn,
On 23.11.2016 00:13, Bjorn Helgaas wrote:
> Hi Tomasz,
>
> On Fri, Jun 10, 2016 at 09:55:19PM +0200, Tomasz Nowicki wrote:
>> Implement pci_acpi_scan_root and other arch-specific call so that ARM64
>> can start using ACPI to setup and enumerate PCI buses.
>>
>> Prior to buses enumeration the pci_acpi_scan_root() implementation looks
>> for configuration space start address (obtained through ACPI _CBA method or
>> MCFG interface). If succeed, it uses ECAM library to create new mapping.
>> Then it attaches generic ECAM ops (pci_generic_ecam_ops) which are used
>> for accessing configuration space later on.
>> ...
>
>> +static struct acpi_pci_root_ops acpi_pci_root_ops = {
>> + .release_info = pci_acpi_generic_release_info,
>> +};
>> +
>> +/* Interface called from ACPI code to setup PCI host controller */
>> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>> {
>> - /* TODO: Should be revisited when implementing PCI on ACPI */
>> - return NULL;
>> + int node = acpi_get_node(root->device->handle);
>> + struct acpi_pci_generic_root_info *ri;
>> + struct pci_bus *bus, *child;
>> +
>> + ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>> + if (!ri)
>> + return NULL;
>> +
>> + ri->cfg = pci_acpi_setup_ecam_mapping(root);
>> + if (!ri->cfg) {
>> + kfree(ri);
>> + return NULL;
>> + }
>> +
>> + acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
>
> This has already been merged, but this isn't right, is it? We're
> writing a host controller-specific pointer into the single system-wide
> acpi_pci_root_ops, then passing it on to acpi_pci_root_create().
>
> Today, I think ri->cfg->ops->pci_ops is always &pci_generic_ecam_ops,
> from this path:
>
> ri->cfg = pci_acpi_setup_ecam_mapping
> cfg = pci_ecam_create(..., &pci_generic_ecam_ops)
> cfg = kzalloc(...)
> cfg->ops = ops # &pci_generic_ecam_ops
>
> But we're about to merge the ECAM quirks series, which will mean it
> may not be &pci_generic_ecam_ops. Even apart from the ECAM quirks, we
> should avoid this pattern of putting device-specific info in a single
> shared structure because it's too difficult to verify that it's
> correct.
>
Well spotted. I agree, we need to fix this. How about this:
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index fb439c7..31c0e1c 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -152,33 +152,35 @@ static void pci_acpi_generic_release_info(struct
acpi_pci_root_info *ci)
ri = container_of(ci, struct acpi_pci_generic_root_info, common);
pci_ecam_free(ri->cfg);
+ kfree(ci->ops);
kfree(ri);
}
-static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .release_info = pci_acpi_generic_release_info,
-};
-
/* Interface called from ACPI code to setup PCI host controller */
struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
{
int node = acpi_get_node(root->device->handle);
struct acpi_pci_generic_root_info *ri;
struct pci_bus *bus, *child;
+ struct acpi_pci_root_ops *root_ops;
ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
if (!ri)
return NULL;
+ root_ops = kzalloc_node(sizeof(*root_ops), GFP_KERNEL, node);
+ if (!root_ops)
+ return NULL;
+
ri->cfg = pci_acpi_setup_ecam_mapping(root);
if (!ri->cfg) {
kfree(ri);
+ kfree(root_ops);
return NULL;
}
- 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);
+ root_ops->release_info = pci_acpi_generic_release_info;
+ root_ops->pci_ops = &ri->cfg->ops->pci_ops;
+ bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
if (!bus)
return NULL;
Of course, this should be the part of ECAM quirks core patches.
The other option we have is to remove "struct pci_ops *pci_ops;" from
acpi_pci_root_ops structure and pass struct pci_ops as an extra argument
to acpi_pci_root_create(). What do you think?
Thanks,
Tomasz
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller
2016-11-23 11:21 ` Tomasz Nowicki
@ 2016-11-23 18:22 ` Bjorn Helgaas
2016-11-24 11:10 ` Tomasz Nowicki
0 siblings, 1 reply; 30+ messages in thread
From: Bjorn Helgaas @ 2016-11-23 18:22 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On Wed, Nov 23, 2016 at 12:21:03PM +0100, Tomasz Nowicki wrote:
> Hi Bjorn,
>
> On 23.11.2016 00:13, Bjorn Helgaas wrote:
> >Hi Tomasz,
> >
> >On Fri, Jun 10, 2016 at 09:55:19PM +0200, Tomasz Nowicki wrote:
> >>Implement pci_acpi_scan_root and other arch-specific call so that ARM64
> >>can start using ACPI to setup and enumerate PCI buses.
> >>
> >>Prior to buses enumeration the pci_acpi_scan_root() implementation looks
> >>for configuration space start address (obtained through ACPI _CBA method or
> >>MCFG interface). If succeed, it uses ECAM library to create new mapping.
> >>Then it attaches generic ECAM ops (pci_generic_ecam_ops) which are used
> >>for accessing configuration space later on.
> >>...
> >
> >>+static struct acpi_pci_root_ops acpi_pci_root_ops = {
> >>+ .release_info = pci_acpi_generic_release_info,
> >>+};
> >>+
> >>+/* Interface called from ACPI code to setup PCI host controller */
> >> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> >> {
> >>- /* TODO: Should be revisited when implementing PCI on ACPI */
> >>- return NULL;
> >>+ int node = acpi_get_node(root->device->handle);
> >>+ struct acpi_pci_generic_root_info *ri;
> >>+ struct pci_bus *bus, *child;
> >>+
> >>+ ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
> >>+ if (!ri)
> >>+ return NULL;
> >>+
> >>+ ri->cfg = pci_acpi_setup_ecam_mapping(root);
> >>+ if (!ri->cfg) {
> >>+ kfree(ri);
> >>+ return NULL;
> >>+ }
> >>+
> >>+ acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
> >
> >This has already been merged, but this isn't right, is it? We're
> >writing a host controller-specific pointer into the single system-wide
> >acpi_pci_root_ops, then passing it on to acpi_pci_root_create().
> >
> >Today, I think ri->cfg->ops->pci_ops is always &pci_generic_ecam_ops,
> >from this path:
> >
> > ri->cfg = pci_acpi_setup_ecam_mapping
> > cfg = pci_ecam_create(..., &pci_generic_ecam_ops)
> > cfg = kzalloc(...)
> > cfg->ops = ops # &pci_generic_ecam_ops
> >
> >But we're about to merge the ECAM quirks series, which will mean it
> >may not be &pci_generic_ecam_ops. Even apart from the ECAM quirks, we
> >should avoid this pattern of putting device-specific info in a single
> >shared structure because it's too difficult to verify that it's
> >correct.
> >
>
> Well spotted. I agree, we need to fix this. How about this:
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index fb439c7..31c0e1c 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -152,33 +152,35 @@ static void
> pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
>
> ri = container_of(ci, struct acpi_pci_generic_root_info, common);
> pci_ecam_free(ri->cfg);
> + kfree(ci->ops);
> kfree(ri);
> }
>
> -static struct acpi_pci_root_ops acpi_pci_root_ops = {
> - .release_info = pci_acpi_generic_release_info,
> -};
> -
> /* Interface called from ACPI code to setup PCI host controller */
> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> {
> int node = acpi_get_node(root->device->handle);
> struct acpi_pci_generic_root_info *ri;
> struct pci_bus *bus, *child;
> + struct acpi_pci_root_ops *root_ops;
>
> ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
> if (!ri)
> return NULL;
>
> + root_ops = kzalloc_node(sizeof(*root_ops), GFP_KERNEL, node);
> + if (!root_ops)
> + return NULL;
> +
> ri->cfg = pci_acpi_setup_ecam_mapping(root);
> if (!ri->cfg) {
> kfree(ri);
> + kfree(root_ops);
> return NULL;
> }
>
> - 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);
> + root_ops->release_info = pci_acpi_generic_release_info;
> + root_ops->pci_ops = &ri->cfg->ops->pci_ops;
> + bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
> if (!bus)
> return NULL;
>
> Of course, this should be the part of ECAM quirks core patches.
>
> The other option we have is to remove "struct pci_ops *pci_ops;"
> from acpi_pci_root_ops structure and pass struct pci_ops as an extra
> argument to acpi_pci_root_create(). What do you think?
I think your patch above is fine and avoids the need to change the x86 and
ia64 code. Would you mind packaging this up with a changelog and
signed-off-by? I can take care of putting it in the ECAM series.
Thanks,
Bjorn
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller
2016-11-23 18:22 ` Bjorn Helgaas
@ 2016-11-24 11:10 ` Tomasz Nowicki
0 siblings, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-11-24 11:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On 23.11.2016 19:22, Bjorn Helgaas wrote:
> On Wed, Nov 23, 2016 at 12:21:03PM +0100, Tomasz Nowicki wrote:
>> Hi Bjorn,
>>
>> On 23.11.2016 00:13, Bjorn Helgaas wrote:
>>> Hi Tomasz,
>>>
>>> On Fri, Jun 10, 2016 at 09:55:19PM +0200, Tomasz Nowicki wrote:
>>>> Implement pci_acpi_scan_root and other arch-specific call so that ARM64
>>>> can start using ACPI to setup and enumerate PCI buses.
>>>>
>>>> Prior to buses enumeration the pci_acpi_scan_root() implementation looks
>>>> for configuration space start address (obtained through ACPI _CBA method or
>>>> MCFG interface). If succeed, it uses ECAM library to create new mapping.
>>>> Then it attaches generic ECAM ops (pci_generic_ecam_ops) which are used
>>>> for accessing configuration space later on.
>>>> ...
>>>
>>>> +static struct acpi_pci_root_ops acpi_pci_root_ops = {
>>>> + .release_info = pci_acpi_generic_release_info,
>>>> +};
>>>> +
>>>> +/* Interface called from ACPI code to setup PCI host controller */
>>>> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>>>> {
>>>> - /* TODO: Should be revisited when implementing PCI on ACPI */
>>>> - return NULL;
>>>> + int node = acpi_get_node(root->device->handle);
>>>> + struct acpi_pci_generic_root_info *ri;
>>>> + struct pci_bus *bus, *child;
>>>> +
>>>> + ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>>>> + if (!ri)
>>>> + return NULL;
>>>> +
>>>> + ri->cfg = pci_acpi_setup_ecam_mapping(root);
>>>> + if (!ri->cfg) {
>>>> + kfree(ri);
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
>>>
>>> This has already been merged, but this isn't right, is it? We're
>>> writing a host controller-specific pointer into the single system-wide
>>> acpi_pci_root_ops, then passing it on to acpi_pci_root_create().
>>>
>>> Today, I think ri->cfg->ops->pci_ops is always &pci_generic_ecam_ops,
>> >from this path:
>>>
>>> ri->cfg = pci_acpi_setup_ecam_mapping
>>> cfg = pci_ecam_create(..., &pci_generic_ecam_ops)
>>> cfg = kzalloc(...)
>>> cfg->ops = ops # &pci_generic_ecam_ops
>>>
>>> But we're about to merge the ECAM quirks series, which will mean it
>>> may not be &pci_generic_ecam_ops. Even apart from the ECAM quirks, we
>>> should avoid this pattern of putting device-specific info in a single
>>> shared structure because it's too difficult to verify that it's
>>> correct.
>>>
>>
>> Well spotted. I agree, we need to fix this. How about this:
>> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
>> index fb439c7..31c0e1c 100644
>> --- a/arch/arm64/kernel/pci.c
>> +++ b/arch/arm64/kernel/pci.c
>> @@ -152,33 +152,35 @@ static void
>> pci_acpi_generic_release_info(struct acpi_pci_root_info *ci)
>>
>> ri = container_of(ci, struct acpi_pci_generic_root_info, common);
>> pci_ecam_free(ri->cfg);
>> + kfree(ci->ops);
>> kfree(ri);
>> }
>>
>> -static struct acpi_pci_root_ops acpi_pci_root_ops = {
>> - .release_info = pci_acpi_generic_release_info,
>> -};
>> -
>> /* Interface called from ACPI code to setup PCI host controller */
>> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>> {
>> int node = acpi_get_node(root->device->handle);
>> struct acpi_pci_generic_root_info *ri;
>> struct pci_bus *bus, *child;
>> + struct acpi_pci_root_ops *root_ops;
>>
>> ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>> if (!ri)
>> return NULL;
>>
>> + root_ops = kzalloc_node(sizeof(*root_ops), GFP_KERNEL, node);
>> + if (!root_ops)
>> + return NULL;
>> +
>> ri->cfg = pci_acpi_setup_ecam_mapping(root);
>> if (!ri->cfg) {
>> kfree(ri);
>> + kfree(root_ops);
>> return NULL;
>> }
>>
>> - 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);
>> + root_ops->release_info = pci_acpi_generic_release_info;
>> + root_ops->pci_ops = &ri->cfg->ops->pci_ops;
>> + bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
>> if (!bus)
>> return NULL;
>>
>> Of course, this should be the part of ECAM quirks core patches.
>>
>> The other option we have is to remove "struct pci_ops *pci_ops;"
>> from acpi_pci_root_ops structure and pass struct pci_ops as an extra
>> argument to acpi_pci_root_create(). What do you think?
>
> I think your patch above is fine and avoids the need to change the x86 and
> ia64 code. Would you mind packaging this up with a changelog and
> signed-off-by? I can take care of putting it in the ECAM series.
>
Sure, I have just sent the patch in replay to ECAM quirks V6 patch set.
Let us know when you update your branch so we base our quirks on it.
Thanks,
Tomasz
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller
2016-06-10 19:55 [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller Tomasz Nowicki
` (10 preceding siblings ...)
2016-06-10 19:55 ` [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller Tomasz Nowicki
@ 2016-06-10 23:41 ` Bjorn Helgaas
2016-06-10 23:50 ` Jon Masters
` (2 more replies)
11 siblings, 3 replies; 30+ messages in thread
From: Bjorn Helgaas @ 2016-06-10 23:41 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On Fri, Jun 10, 2016 at 09:55:08PM +0200, Tomasz Nowicki wrote:
> From the functionality point of view this series may be split into the
> following logic parts:
> 1. Export ECAM API and add parent device to pci_config_window
> 2. Add IO resources handling to PCI core code
> 3. New MCFG driver
> 4. Cleanups and support for generic domain assignment based on ACPI
> 5. Implement ARM64 ACPI based PCI host controller driver under arch/arm64/
This is beautiful! It really turned out well.
I applied all these to pci/arm64-acpi for v4.8.
Bjorn
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller
2016-06-10 23:41 ` [PATCH V9 00/11] Support for ARM64 " Bjorn Helgaas
@ 2016-06-10 23:50 ` Jon Masters
2016-06-10 23:58 ` [Linaro-acpi] " Jon Masters
2016-06-11 9:51 ` Tomasz Nowicki
2 siblings, 0 replies; 30+ messages in thread
From: Jon Masters @ 2016-06-10 23:50 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Tomasz Nowicki, arnd, will.deacon, catalin.marinas, rafael,
hanjun.guo, Lorenzo.Pieralisi, okaya, jchandra, robert.richter,
mw, Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit,
msalter, linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, andrea.gallo, dhdang, jeremy.linton, liudongdong3,
cov
Thank you Bjorn :)
Next time we are in the same place (anyone involved in this series), beer is on me.
--
Computer Architect | Sent from my 64-bit #ARM Powered phone
> On Jun 10, 2016, at 18:41, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
>> On Fri, Jun 10, 2016 at 09:55:08PM +0200, Tomasz Nowicki wrote:
>> From the functionality point of view this series may be split into the
>> following logic parts:
>> 1. Export ECAM API and add parent device to pci_config_window
>> 2. Add IO resources handling to PCI core code
>> 3. New MCFG driver
>> 4. Cleanups and support for generic domain assignment based on ACPI
>> 5. Implement ARM64 ACPI based PCI host controller driver under arch/arm64/
>
> This is beautiful! It really turned out well.
>
> I applied all these to pci/arm64-acpi for v4.8.
>
> Bjorn
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Linaro-acpi] [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller
2016-06-10 23:41 ` [PATCH V9 00/11] Support for ARM64 " Bjorn Helgaas
2016-06-10 23:50 ` Jon Masters
@ 2016-06-10 23:58 ` Jon Masters
2016-06-11 9:51 ` Tomasz Nowicki
2 siblings, 0 replies; 30+ messages in thread
From: Jon Masters @ 2016-06-10 23:58 UTC (permalink / raw)
To: Bjorn Helgaas, Tomasz Nowicki
Cc: rafael, linux-pci, will.deacon, okaya, wangyijing, andrea.gallo,
linaro-acpi, ddaney, linux-acpi, robert.richter, catalin.marinas,
Liviu.Dudau, arnd, jcm, cov, mw, linux-arm-kernel, jchandra,
dhdang, linux-kernel, jeremy.linton
On 06/10/2016 07:41 PM, Bjorn Helgaas wrote:
> On Fri, Jun 10, 2016 at 09:55:08PM +0200, Tomasz Nowicki wrote:
>> From the functionality point of view this series may be split into the
>> following logic parts:
>> 1. Export ECAM API and add parent device to pci_config_window
>> 2. Add IO resources handling to PCI core code
>> 3. New MCFG driver
>> 4. Cleanups and support for generic domain assignment based on ACPI
>> 5. Implement ARM64 ACPI based PCI host controller driver under arch/arm64/
>
> This is beautiful! It really turned out well.
>
> I applied all these to pci/arm64-acpi for v4.8.
Quick followup - I've forwarded your mail to the Linaro Enterprise Group
members and started nudging the vendors to prep any remaining pieces for
post-merge. That includes quirks, and then any drivers that anyone has
been sitting on until this was upstream.
Looking forward to F25 Server booting out of the box, and finally
getting to a point where Fedora is once again upstream of RHEL(SA).
Jon.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH V9 00/11] Support for ARM64 ACPI based PCI host controller
2016-06-10 23:41 ` [PATCH V9 00/11] Support for ARM64 " Bjorn Helgaas
2016-06-10 23:50 ` Jon Masters
2016-06-10 23:58 ` [Linaro-acpi] " Jon Masters
@ 2016-06-11 9:51 ` Tomasz Nowicki
2 siblings, 0 replies; 30+ messages in thread
From: Tomasz Nowicki @ 2016-06-11 9:51 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: arnd, will.deacon, catalin.marinas, rafael, hanjun.guo,
Lorenzo.Pieralisi, okaya, jchandra, robert.richter, mw,
Liviu.Dudau, ddaney, wangyijing, Suravee.Suthikulpanit, msalter,
linux-pci, linux-arm-kernel, linux-acpi, linux-kernel,
linaro-acpi, jcm, andrea.gallo, dhdang, jeremy.linton,
liudongdong3, cov
On 11.06.2016 01:41, Bjorn Helgaas wrote:
> On Fri, Jun 10, 2016 at 09:55:08PM +0200, Tomasz Nowicki wrote:
>> From the functionality point of view this series may be split into the
>> following logic parts:
>> 1. Export ECAM API and add parent device to pci_config_window
>> 2. Add IO resources handling to PCI core code
>> 3. New MCFG driver
>> 4. Cleanups and support for generic domain assignment based on ACPI
>> 5. Implement ARM64 ACPI based PCI host controller driver under arch/arm64/
>
> This is beautiful! It really turned out well.
>
> I applied all these to pci/arm64-acpi for v4.8.
>
Thanks Bjorn! This is great news.
Tomasz
^ permalink raw reply [flat|nested] 30+ messages in thread