This patch series has been forked from the following patch series since
it no longer depends on the rest of the patches.
[PATCH v4 00/10] ACPI GIC Self-probing, GICv2m and GICv3 support
https://lkml.org/lkml/2015/7/29/234
It has been ported to use the newly introduced device fwnode_handle
for ACPI irqdmain introduced by Marc in the following patch series:
[PATCH v2 00/17] Divorcing irqdomain and device_node
http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git irq/irq-domain-fwnode-v2
The following git branch contains the submitted patches along with
the pre-requsite patches (mainly for ARM64 PCI support for ACPI).
https://github.com/ssuthiku/linux.git v2m-multiframe-v5
This has been tested on AMD Seattle (Overdrive) RevB system.
NOTE: I have not tested ACPI GICv2m multiframe support since
I don't have access to such system. Any helps are appreciated.
Thanks,
Suravee
Changes from V4: (https://lkml.org/lkml/2015/12/08/613)
- Fix build error when not specifying CONFIG_IRQ_DOMAIN.
Changes from V3: (https://lkml.org/lkml/2015/10/21/691)
- Merged patch 2 into 1, and got rid off pci_msi_get_fwnode()
since only ACPI will likely use this. (per Marc suggestion)
- Rebased to 4.4.0-rc4
Changes from V2: (https://lkml.org/lkml/2015/10/14/1010)
- Minor clean up from Tomasz review comment in patch 6/6.
Changes from V1: (https://lkml.org/lkml/2015/10/13/859)
- Rebase on top of Marc's patch to addng support for multiple MSI frames
(https://lkml.org/lkml/2015/10/14/271)
- Adding fwnode convenient functions (patch 3 and 4)
Suravee Suthikulpanit (5):
acpi: pci: Setup MSI domain for ACPI based pci devices
irqdomain: introduce is_fwnode_irqchip helper
irqdomain: Introduce irq_domain_get_irqchip_fwnode_name helper
function
gicv2m: Refactor to prepare for ACPI support
gicv2m: acpi: Introducing GICv2m ACPI support
drivers/irqchip/irq-gic-v2m.c | 152 ++++++++++++++++++++++++++++++++++------
drivers/irqchip/irq-gic.c | 5 +-
drivers/pci/pci-acpi.c | 32 +++++++++
drivers/pci/probe.c | 2 +
include/linux/irqchip/arm-gic.h | 4 ++
include/linux/irqdomain.h | 11 +++
include/linux/pci.h | 10 +++
kernel/irq/irqdomain.c | 20 +++++-
8 files changed, 213 insertions(+), 23 deletions(-)
--
2.1.0
This patch introduces pci_msi_register_fwnode_provider() for irqchip
to register a callback, to provide a way to determine appropriate MSI
domain for a pci device.
It also introduces pci_host_bridge_acpi_msi_domain(), which returns
the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
bus token. Then, it is assigned to pci device.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/pci/pci-acpi.c | 32 ++++++++++++++++++++++++++++++++
drivers/pci/probe.c | 2 ++
include/linux/irqdomain.h | 5 +++++
include/linux/pci.h | 10 ++++++++++
4 files changed, 49 insertions(+)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index a32ba75..a555d7e 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -9,7 +9,9 @@
#include <linux/delay.h>
#include <linux/init.h>
+#include <linux/irqdomain.h>
#include <linux/pci.h>
+#include <linux/msi.h>
#include <linux/pci_hotplug.h>
#include <linux/module.h>
#include <linux/pci-aspm.h>
@@ -689,6 +691,36 @@ static struct acpi_bus_type acpi_pci_bus = {
.cleanup = pci_acpi_cleanup,
};
+
+static struct fwnode_handle *(*pci_msi_get_fwnode_cb)(struct device *dev);
+
+/**
+ * pci_msi_register_fwnode_provider - Register callback to retrieve fwnode
+ * @fn: The interrupt domain to retrieve
+ *
+ * This should be called by irqchip driver, which is the parent of
+ * the MSI domain to provide callback interface to query fwnode.
+ */
+void
+pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *))
+{
+ pci_msi_get_fwnode_cb = fn;
+}
+
+struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
+{
+ struct irq_domain *dom = NULL;
+ struct fwnode_handle *fwnode = NULL;
+
+ if (pci_msi_get_fwnode_cb)
+ fwnode = pci_msi_get_fwnode_cb(&bus->dev);
+
+ if (fwnode)
+ dom = irq_find_matching_fwnode(fwnode,
+ DOMAIN_BUS_PCI_MSI);
+ return dom;
+}
+
static int __init acpi_pci_init(void)
{
int ret;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index edb1984..553a029 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -672,6 +672,8 @@ static struct irq_domain *pci_host_bridge_msi_domain(struct pci_bus *bus)
* should be called from here.
*/
d = pci_host_bridge_of_msi_domain(bus);
+ if (!d)
+ d = pci_host_bridge_acpi_msi_domain(bus);
return d;
}
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index d5e5c5b..a06feda 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -410,6 +410,11 @@ static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
static inline void irq_dispose_mapping(unsigned int virq) { }
static inline void irq_domain_activate_irq(struct irq_data *data) { }
static inline void irq_domain_deactivate_irq(struct irq_data *data) { }
+static inline struct irq_domain *irq_find_matching_fwnode(
+ struct fwnode_handle *fwnode, enum irq_domain_bus_token bus_token)
+{
+ return NULL;
+}
#endif /* !CONFIG_IRQ_DOMAIN */
#endif /* _LINUX_IRQDOMAIN_H */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 6ae25aa..d86378c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1946,6 +1946,16 @@ static inline struct irq_domain *
pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
#endif /* CONFIG_OF */
+#ifdef CONFIG_ACPI
+struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus);
+
+void
+pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *));
+#else
+static inline struct irq_domain *
+pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
+#endif
+
#ifdef CONFIG_EEH
static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
{
--
2.1.0
Since there will be several places checking if fwnode.type
is equal FWNODE_IRQCHIP, this patch adds a convenient function
for this purpose.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/irqchip/irq-gic.c | 2 +-
include/linux/irqdomain.h | 5 +++++
kernel/irq/irqdomain.c | 2 +-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index abf2ffa..fcd327f 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -972,7 +972,7 @@ static int gic_irq_domain_translate(struct irq_domain *d,
return 0;
}
- if (fwspec->fwnode->type == FWNODE_IRQCHIP) {
+ if (is_fwnode_irqchip(fwspec->fwnode)) {
if(fwspec->param_count != 2)
return -EINVAL;
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index a06feda..d72fabc 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -211,6 +211,11 @@ static inline struct fwnode_handle *of_node_to_fwnode(struct device_node *node)
return node ? &node->fwnode : NULL;
}
+static inline bool is_fwnode_irqchip(struct fwnode_handle *fwnode)
+{
+ return fwnode && fwnode->type == FWNODE_IRQCHIP;
+}
+
static inline struct irq_domain *irq_find_matching_host(struct device_node *node,
enum irq_domain_bus_token bus_token)
{
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 22aa961..7f34d98 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -70,7 +70,7 @@ void irq_domain_free_fwnode(struct fwnode_handle *fwnode)
{
struct irqchip_fwid *fwid;
- if (WARN_ON(fwnode->type != FWNODE_IRQCHIP))
+ if (WARN_ON(!is_fwnode_irqchip(fwnode)))
return;
fwid = container_of(fwnode, struct irqchip_fwid, fwnode);
--
2.1.0
This patch adds an accessor function to retrieve struct irqchip_fwid.name.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
include/linux/irqdomain.h | 1 +
kernel/irq/irqdomain.c | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index d72fabc..c0a95a9 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -187,6 +187,7 @@ static inline struct device_node *irq_domain_get_of_node(struct irq_domain *d)
#ifdef CONFIG_IRQ_DOMAIN
struct fwnode_handle *irq_domain_alloc_fwnode(void *data);
void irq_domain_free_fwnode(struct fwnode_handle *fwnode);
+const char *irq_domain_get_irqchip_fwnode_name(struct fwnode_handle *fwnode);
struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
irq_hw_number_t hwirq_max, int direct_max,
const struct irq_domain_ops *ops,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 7f34d98..a8c1cf6 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -79,6 +79,24 @@ void irq_domain_free_fwnode(struct fwnode_handle *fwnode)
}
/**
+ * irq_domain_get_irqchip_fwnode_name - Retrieve associated name of
+ * specified irqchip fwnode
+ * @fwnode: Specified fwnode_handle
+ *
+ * Returns associated name of the specified fwnode, or NULL on failure.
+ */
+const char *irq_domain_get_irqchip_fwnode_name(struct fwnode_handle *fwnode)
+{
+ struct irqchip_fwid *fwid;
+
+ if (!is_fwnode_irqchip(fwnode))
+ return NULL;
+
+ fwid = container_of(fwnode, struct irqchip_fwid, fwnode);
+ return fwid->name;
+}
+
+/**
* __irq_domain_add() - Allocate a new irq_domain data structure
* @of_node: optional device-tree node of the interrupt controller
* @size: Size of linear map; 0 for radix mapping only
--
2.1.0
This patch replaces the struct device_node with struct fwnode_handle
since this structure is common between DT and ACPI.
It also refactors gicv2m_init_one() to prepare for ACPI support.
There should be no functional changes.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/irqchip/irq-gic-v2m.c | 57 +++++++++++++++++++++++++++----------------
1 file changed, 36 insertions(+), 21 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 87f8d10..7e60f7e 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -55,7 +55,7 @@ static DEFINE_SPINLOCK(v2m_lock);
struct v2m_data {
struct list_head entry;
- struct device_node *node;
+ struct fwnode_handle *fwnode;
struct resource res; /* GICv2m resource */
void __iomem *base; /* GICv2m virt address */
u32 spi_start; /* The SPI number that MSIs start */
@@ -254,7 +254,7 @@ static void gicv2m_teardown(void)
list_del(&v2m->entry);
kfree(v2m->bm);
iounmap(v2m->base);
- of_node_put(v2m->node);
+ of_node_put(to_of_node(v2m->fwnode));
kfree(v2m);
}
}
@@ -268,7 +268,7 @@ static int gicv2m_allocate_domains(struct irq_domain *parent)
if (!v2m)
return 0;
- inner_domain = irq_domain_create_tree(of_node_to_fwnode(v2m->node),
+ inner_domain = irq_domain_create_tree(v2m->fwnode,
&gicv2m_domain_ops, v2m);
if (!inner_domain) {
pr_err("Failed to create GICv2m domain\n");
@@ -277,10 +277,10 @@ static int gicv2m_allocate_domains(struct irq_domain *parent)
inner_domain->bus_token = DOMAIN_BUS_NEXUS;
inner_domain->parent = parent;
- pci_domain = pci_msi_create_irq_domain(of_node_to_fwnode(v2m->node),
+ pci_domain = pci_msi_create_irq_domain(v2m->fwnode,
&gicv2m_msi_domain_info,
inner_domain);
- plat_domain = platform_msi_create_irq_domain(of_node_to_fwnode(v2m->node),
+ plat_domain = platform_msi_create_irq_domain(v2m->fwnode,
&gicv2m_pmsi_domain_info,
inner_domain);
if (!pci_domain || !plat_domain) {
@@ -296,11 +296,13 @@ static int gicv2m_allocate_domains(struct irq_domain *parent)
return 0;
}
-static int __init gicv2m_init_one(struct device_node *node,
- struct irq_domain *parent)
+static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
+ u32 spi_start, u32 nr_spis,
+ struct resource *res)
{
int ret;
struct v2m_data *v2m;
+ const char *name = NULL;
v2m = kzalloc(sizeof(struct v2m_data), GFP_KERNEL);
if (!v2m) {
@@ -309,13 +311,9 @@ static int __init gicv2m_init_one(struct device_node *node,
}
INIT_LIST_HEAD(&v2m->entry);
- v2m->node = node;
+ v2m->fwnode = fwnode;
- ret = of_address_to_resource(node, 0, &v2m->res);
- if (ret) {
- pr_err("Failed to allocate v2m resource.\n");
- goto err_free_v2m;
- }
+ memcpy(&v2m->res, res, sizeof(struct resource));
v2m->base = ioremap(v2m->res.start, resource_size(&v2m->res));
if (!v2m->base) {
@@ -324,10 +322,9 @@ static int __init gicv2m_init_one(struct device_node *node,
goto err_free_v2m;
}
- if (!of_property_read_u32(node, "arm,msi-base-spi", &v2m->spi_start) &&
- !of_property_read_u32(node, "arm,msi-num-spis", &v2m->nr_spis)) {
- pr_info("Overriding V2M MSI_TYPER (base:%u, num:%u)\n",
- v2m->spi_start, v2m->nr_spis);
+ if (spi_start && nr_spis) {
+ v2m->spi_start = spi_start;
+ v2m->nr_spis = nr_spis;
} else {
u32 typer = readl_relaxed(v2m->base + V2M_MSI_TYPER);
@@ -359,10 +356,13 @@ static int __init gicv2m_init_one(struct device_node *node,
}
list_add_tail(&v2m->entry, &v2m_nodes);
- pr_info("Node %s: range[%#lx:%#lx], SPI[%d:%d]\n", node->name,
- (unsigned long)v2m->res.start, (unsigned long)v2m->res.end,
- v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
+ if (to_of_node(fwnode))
+ name = to_of_node(fwnode)->name;
+
+ pr_info("Frame %s: range[%#lx:%#lx], SPI[%d:%d]\n", name,
+ (unsigned long)res->start, (unsigned long)res->end,
+ v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
return 0;
err_iounmap:
@@ -384,10 +384,25 @@ int __init gicv2m_of_init(struct device_node *node, struct irq_domain *parent)
for (child = of_find_matching_node(node, gicv2m_device_id); child;
child = of_find_matching_node(child, gicv2m_device_id)) {
+ u32 spi_start = 0, nr_spis = 0;
+ struct resource res;
+
if (!of_find_property(child, "msi-controller", NULL))
continue;
- ret = gicv2m_init_one(child, parent);
+ ret = of_address_to_resource(child, 0, &res);
+ if (ret) {
+ pr_err("Failed to allocate v2m resource.\n");
+ break;
+ }
+
+ if (!of_property_read_u32(child, "arm,msi-base-spi",
+ &spi_start) &&
+ !of_property_read_u32(child, "arm,msi-num-spis", &nr_spis))
+ pr_info("DT overriding V2M MSI_TYPER (base:%u, num:%u)\n",
+ spi_start, nr_spis);
+
+ ret = gicv2m_init_one(&child->fwnode, spi_start, nr_spis, &res);
if (ret) {
of_node_put(node);
break;
--
2.1.0
This patch introduces gicv2m_acpi_init(), which uses information
in MADT GIC MSI frames structure to initialize GICv2m driver.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
Signed-off-by: Hanjun Guo <[email protected]>
---
drivers/irqchip/irq-gic-v2m.c | 95 +++++++++++++++++++++++++++++++++++++++++
drivers/irqchip/irq-gic.c | 3 ++
include/linux/irqchip/arm-gic.h | 4 ++
3 files changed, 102 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 7e60f7e..4f52e9a 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -15,9 +15,11 @@
#define pr_fmt(fmt) "GICv2m: " fmt
+#include <linux/acpi.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
#include <linux/kernel.h>
+#include <linux/msi.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
#include <linux/slab.h>
@@ -138,6 +140,11 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
fwspec.param[0] = 0;
fwspec.param[1] = hwirq - 32;
fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+ } else if (is_fwnode_irqchip(domain->parent->fwnode)) {
+ fwspec.fwnode = domain->parent->fwnode;
+ fwspec.param_count = 2;
+ fwspec.param[0] = hwirq;
+ fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
} else {
return -EINVAL;
}
@@ -255,6 +262,8 @@ static void gicv2m_teardown(void)
kfree(v2m->bm);
iounmap(v2m->base);
of_node_put(to_of_node(v2m->fwnode));
+ if (is_fwnode_irqchip(v2m->fwnode))
+ irq_domain_free_fwnode(v2m->fwnode);
kfree(v2m);
}
}
@@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
if (to_of_node(fwnode))
name = to_of_node(fwnode)->name;
+ else
+ name = irq_domain_get_irqchip_fwnode_name(fwnode);
pr_info("Frame %s: range[%#lx:%#lx], SPI[%d:%d]\n", name,
(unsigned long)res->start, (unsigned long)res->end,
@@ -415,3 +426,87 @@ int __init gicv2m_of_init(struct device_node *node, struct irq_domain *parent)
gicv2m_teardown();
return ret;
}
+
+#ifdef CONFIG_ACPI
+static int acpi_num_msi;
+
+static struct fwnode_handle *gicv2m_get_fwnode(struct device *dev)
+{
+ struct v2m_data *data;
+
+ if (WARN_ON(acpi_num_msi <= 0))
+ return NULL;
+
+ /* We only return the fwnode of the first MSI frame. */
+ data = list_first_entry_or_null(&v2m_nodes, struct v2m_data, entry);
+ if (!data)
+ return NULL;
+
+ return data->fwnode;
+}
+
+static int __init
+acpi_parse_madt_msi(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+ int ret;
+ struct resource res;
+ u32 spi_start = 0, nr_spis = 0;
+ struct acpi_madt_generic_msi_frame *m;
+ struct fwnode_handle *fwnode = NULL;
+
+ m = (struct acpi_madt_generic_msi_frame *)header;
+ if (BAD_MADT_ENTRY(m, end))
+ return -EINVAL;
+
+ res.start = m->base_address;
+ res.end = m->base_address + 0x1000;
+
+ if (m->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) {
+ spi_start = m->spi_base;
+ nr_spis = m->spi_count;
+
+ pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n",
+ spi_start, nr_spis);
+ }
+
+ fwnode = irq_domain_alloc_fwnode((void *)m->base_address);
+ if (!fwnode) {
+ pr_err("Unable to allocate GICv2m domain token\n");
+ return -EINVAL;
+ }
+
+ ret = gicv2m_init_one(fwnode, spi_start, nr_spis, &res);
+ if (ret)
+ irq_domain_free_fwnode(fwnode);
+
+ return ret;
+}
+
+int __init gicv2m_acpi_init(struct irq_domain *parent)
+{
+ int ret;
+
+ if (acpi_num_msi > 0)
+ return 0;
+
+ acpi_num_msi = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_MSI_FRAME,
+ acpi_parse_madt_msi, 0);
+
+ if (acpi_num_msi <= 0)
+ goto err_out;
+
+ ret = gicv2m_allocate_domains(parent);
+ if (ret)
+ goto err_out;
+
+ pci_msi_register_fwnode_provider(&gicv2m_get_fwnode);
+
+ return 0;
+
+err_out:
+ gicv2m_teardown();
+ return -EINVAL;
+}
+
+#endif /* CONFIG_ACPI */
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index fcd327f..e4463f7 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1358,6 +1358,9 @@ static int __init gic_v2_acpi_init(struct acpi_subtable_header *header,
__gic_init_bases(0, -1, dist_base, cpu_base, 0, domain_handle);
+ if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
+ gicv2m_acpi_init(gic_data[0].domain);
+
acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
return 0;
}
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index bae69e5..30b2ccb 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -108,6 +108,10 @@ void gic_init(unsigned int nr, int start,
int gicv2m_of_init(struct device_node *node, struct irq_domain *parent);
+#ifdef CONFIG_ACPI
+int gicv2m_acpi_init(struct irq_domain *parent);
+#endif
+
void gic_send_sgi(unsigned int cpu_id, unsigned int irq);
int gic_get_cpu_id(unsigned int cpu);
void gic_migrate_target(unsigned int new_cpu_id);
--
2.1.0
On Tue, 8 Dec 2015 17:48:02 -0800
Suravee Suthikulpanit <[email protected]> wrote:
> This patch introduces pci_msi_register_fwnode_provider() for irqchip
> to register a callback, to provide a way to determine appropriate MSI
> domain for a pci device.
>
> It also introduces pci_host_bridge_acpi_msi_domain(), which returns
> the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
> bus token. Then, it is assigned to pci device.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> drivers/pci/pci-acpi.c | 32 ++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 2 ++
> include/linux/irqdomain.h | 5 +++++
> include/linux/pci.h | 10 ++++++++++
> 4 files changed, 49 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a32ba75..a555d7e 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -9,7 +9,9 @@
>
> #include <linux/delay.h>
> #include <linux/init.h>
> +#include <linux/irqdomain.h>
> #include <linux/pci.h>
> +#include <linux/msi.h>
> #include <linux/pci_hotplug.h>
> #include <linux/module.h>
> #include <linux/pci-aspm.h>
> @@ -689,6 +691,36 @@ static struct acpi_bus_type acpi_pci_bus = {
> .cleanup = pci_acpi_cleanup,
> };
>
> +
> +static struct fwnode_handle *(*pci_msi_get_fwnode_cb)(struct device *dev);
> +
> +/**
> + * pci_msi_register_fwnode_provider - Register callback to retrieve fwnode
> + * @fn: The interrupt domain to retrieve
This is the wrong description. Maybe something like@
* @fn: Callback matching a device to a fwnode that identifies a PCI
MSI domain
Or something along those lines.
> + *
> + * This should be called by irqchip driver, which is the parent of
> + * the MSI domain to provide callback interface to query fwnode.
> + */
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *))
> +{
> + pci_msi_get_fwnode_cb = fn;
> +}
> +
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
> +{
Documentation?
> + struct irq_domain *dom = NULL;
> + struct fwnode_handle *fwnode = NULL;
> +
> + if (pci_msi_get_fwnode_cb)
> + fwnode = pci_msi_get_fwnode_cb(&bus->dev);
> +
> + if (fwnode)
> + dom = irq_find_matching_fwnode(fwnode,
> + DOMAIN_BUS_PCI_MSI);
> + return dom;
> +}
> +
Suggestion:
struct fwnode_handle *fwnode;
if (!pci_msi_get_fwnode_cb)
return NULL;
fwnode = pci_msi_get_fwnode_cb(&bus->dev);
if (!fwnode)
return NULL;
return irq_find_matching_fwnode(...);
My own personal taste, though.
> static int __init acpi_pci_init(void)
> {
> int ret;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index edb1984..553a029 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -672,6 +672,8 @@ static struct irq_domain *pci_host_bridge_msi_domain(struct pci_bus *bus)
> * should be called from here.
> */
> d = pci_host_bridge_of_msi_domain(bus);
> + if (!d)
> + d = pci_host_bridge_acpi_msi_domain(bus);
>
> return d;
> }
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index d5e5c5b..a06feda 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -410,6 +410,11 @@ static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
> static inline void irq_dispose_mapping(unsigned int virq) { }
> static inline void irq_domain_activate_irq(struct irq_data *data) { }
> static inline void irq_domain_deactivate_irq(struct irq_data *data) { }
> +static inline struct irq_domain *irq_find_matching_fwnode(
> + struct fwnode_handle *fwnode, enum irq_domain_bus_token bus_token)
> +{
> + return NULL;
> +}
> #endif /* !CONFIG_IRQ_DOMAIN */
>
> #endif /* _LINUX_IRQDOMAIN_H */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6ae25aa..d86378c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1946,6 +1946,16 @@ static inline struct irq_domain *
> pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
> #endif /* CONFIG_OF */
>
> +#ifdef CONFIG_ACPI
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus);
> +
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *));
> +#else
> +static inline struct irq_domain *
> +pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
> +#endif
> +
> #ifdef CONFIG_EEH
> static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> {
Other than the couple of nits above:
Reviewed-by: Marc Zyngier <[email protected]>
M.
--
Without deviation from the norm, progress is not possible.
On Tue, 8 Dec 2015 17:48:03 -0800
Suravee Suthikulpanit <[email protected]> wrote:
> Since there will be several places checking if fwnode.type
> is equal FWNODE_IRQCHIP, this patch adds a convenient function
> for this purpose.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
M.
--
Without deviation from the norm, progress is not possible.
On Tue, 8 Dec 2015 17:48:05 -0800
Suravee Suthikulpanit <[email protected]> wrote:
> This patch replaces the struct device_node with struct fwnode_handle
> since this structure is common between DT and ACPI.
>
> It also refactors gicv2m_init_one() to prepare for ACPI support.
> There should be no functional changes.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
M.
--
Without deviation from the norm, progress is not possible.
On Tue, 8 Dec 2015 17:48:06 -0800
Suravee Suthikulpanit <[email protected]> wrote:
> This patch introduces gicv2m_acpi_init(), which uses information
> in MADT GIC MSI frames structure to initialize GICv2m driver.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Signed-off-by: Hanjun Guo <[email protected]>
> ---
> drivers/irqchip/irq-gic-v2m.c | 95 +++++++++++++++++++++++++++++++++++++++++
> drivers/irqchip/irq-gic.c | 3 ++
> include/linux/irqchip/arm-gic.h | 4 ++
> 3 files changed, 102 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> index 7e60f7e..4f52e9a 100644
> --- a/drivers/irqchip/irq-gic-v2m.c
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -15,9 +15,11 @@
>
> #define pr_fmt(fmt) "GICv2m: " fmt
>
> +#include <linux/acpi.h>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
> +#include <linux/msi.h>
> #include <linux/of_address.h>
> #include <linux/of_pci.h>
> #include <linux/slab.h>
> @@ -138,6 +140,11 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
> fwspec.param[0] = 0;
> fwspec.param[1] = hwirq - 32;
> fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> + } else if (is_fwnode_irqchip(domain->parent->fwnode)) {
> + fwspec.fwnode = domain->parent->fwnode;
> + fwspec.param_count = 2;
> + fwspec.param[0] = hwirq;
> + fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> } else {
> return -EINVAL;
> }
> @@ -255,6 +262,8 @@ static void gicv2m_teardown(void)
> kfree(v2m->bm);
> iounmap(v2m->base);
> of_node_put(to_of_node(v2m->fwnode));
> + if (is_fwnode_irqchip(v2m->fwnode))
> + irq_domain_free_fwnode(v2m->fwnode);
> kfree(v2m);
> }
> }
> @@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
>
> if (to_of_node(fwnode))
> name = to_of_node(fwnode)->name;
> + else
> + name = irq_domain_get_irqchip_fwnode_name(fwnode);
Don't bother with that, the name associated with the domain is
absolutely meaningless. You are already printing the frame address,
which is enough to identify it, should someone need to debug this.
Drop the name from the previous patch as well, and that will make one
less difference to care about. Patch #3 can die as well.
>
> pr_info("Frame %s: range[%#lx:%#lx], SPI[%d:%d]\n", name,
> (unsigned long)res->start, (unsigned long)res->end,
> @@ -415,3 +426,87 @@ int __init gicv2m_of_init(struct device_node *node, struct irq_domain *parent)
> gicv2m_teardown();
> return ret;
> }
> +
> +#ifdef CONFIG_ACPI
> +static int acpi_num_msi;
> +
> +static struct fwnode_handle *gicv2m_get_fwnode(struct device *dev)
> +{
> + struct v2m_data *data;
> +
> + if (WARN_ON(acpi_num_msi <= 0))
> + return NULL;
> +
> + /* We only return the fwnode of the first MSI frame. */
> + data = list_first_entry_or_null(&v2m_nodes, struct v2m_data, entry);
> + if (!data)
> + return NULL;
> +
> + return data->fwnode;
> +}
> +
> +static int __init
> +acpi_parse_madt_msi(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + int ret;
> + struct resource res;
> + u32 spi_start = 0, nr_spis = 0;
> + struct acpi_madt_generic_msi_frame *m;
> + struct fwnode_handle *fwnode = NULL;
You shouldn't need to initialize this to NULL.
> +
> + m = (struct acpi_madt_generic_msi_frame *)header;
> + if (BAD_MADT_ENTRY(m, end))
> + return -EINVAL;
> +
> + res.start = m->base_address;
> + res.end = m->base_address + 0x1000;
SZ_4K?
> +
> + if (m->flags & ACPI_MADT_OVERRIDE_SPI_VALUES) {
> + spi_start = m->spi_base;
> + nr_spis = m->spi_count;
> +
> + pr_info("ACPI overriding V2M MSI_TYPER (base:%u, num:%u)\n",
> + spi_start, nr_spis);
> + }
> +
> + fwnode = irq_domain_alloc_fwnode((void *)m->base_address);
> + if (!fwnode) {
> + pr_err("Unable to allocate GICv2m domain token\n");
> + return -EINVAL;
> + }
> +
> + ret = gicv2m_init_one(fwnode, spi_start, nr_spis, &res);
> + if (ret)
> + irq_domain_free_fwnode(fwnode);
> +
> + return ret;
> +}
> +
> +int __init gicv2m_acpi_init(struct irq_domain *parent)
> +{
> + int ret;
> +
> + if (acpi_num_msi > 0)
> + return 0;
> +
> + acpi_num_msi = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_MSI_FRAME,
> + acpi_parse_madt_msi, 0);
> +
> + if (acpi_num_msi <= 0)
> + goto err_out;
> +
> + ret = gicv2m_allocate_domains(parent);
> + if (ret)
> + goto err_out;
> +
> + pci_msi_register_fwnode_provider(&gicv2m_get_fwnode);
> +
> + return 0;
> +
> +err_out:
> + gicv2m_teardown();
> + return -EINVAL;
> +}
> +
> +#endif /* CONFIG_ACPI */
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index fcd327f..e4463f7 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1358,6 +1358,9 @@ static int __init gic_v2_acpi_init(struct acpi_subtable_header *header,
>
> __gic_init_bases(0, -1, dist_base, cpu_base, 0, domain_handle);
>
> + if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
> + gicv2m_acpi_init(gic_data[0].domain);
> +
Please move this after the next line. It makes more sense to finish the
initialization of the basic GIC before starting to probe other things.
> acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
> return 0;
> }
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index bae69e5..30b2ccb 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -108,6 +108,10 @@ void gic_init(unsigned int nr, int start,
>
> int gicv2m_of_init(struct device_node *node, struct irq_domain *parent);
>
> +#ifdef CONFIG_ACPI
> +int gicv2m_acpi_init(struct irq_domain *parent);
> +#endif
> +
How about having a single:
int gicv2m_init(struct fwnode_handle *parent_handle,
struct irq_domain *parent_domain);
which in turn calls either gicv2m_of_init or gicv2m_acpi_init? Saves
some #ifdef, and avoids another entry point.
> void gic_send_sgi(unsigned int cpu_id, unsigned int irq);
> int gic_get_cpu_id(unsigned int cpu);
> void gic_migrate_target(unsigned int new_cpu_id);
Otherwise, this looks good.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On 12/9/2015 4:13 AM, Marc Zyngier wrote:
> On Tue, 8 Dec 2015 17:48:02 -0800
> Suravee Suthikulpanit <[email protected]> wrote:
>
>> This patch introduces pci_msi_register_fwnode_provider() for irqchip
>> to register a callback, to provide a way to determine appropriate MSI
>> domain for a pci device.
>>
>> It also introduces pci_host_bridge_acpi_msi_domain(), which returns
>> the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
>> bus token. Then, it is assigned to pci device.
>>
>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>> ---
>> drivers/pci/pci-acpi.c | 32 ++++++++++++++++++++++++++++++++
>> drivers/pci/probe.c | 2 ++
>> include/linux/irqdomain.h | 5 +++++
>> include/linux/pci.h | 10 ++++++++++
>> 4 files changed, 49 insertions(+)
>>
> [...]
>
> Other than the couple of nits above:
>
> Reviewed-by: Marc Zyngier <[email protected]>
>
> M.
>
Thanks. I'll take care of them and send out V6.
Suravee
Hi Marc,
On 12/9/2015 4:38 AM, Marc Zyngier wrote:
> On Tue, 8 Dec 2015 17:48:06 -0800
> Suravee Suthikulpanit <[email protected]> wrote:
>
>> This patch introduces gicv2m_acpi_init(), which uses information
>> in MADT GIC MSI frames structure to initialize GICv2m driver.
>>
>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>> Signed-off-by: Hanjun Guo <[email protected]>
>> ---
>> drivers/irqchip/irq-gic-v2m.c | 95 +++++++++++++++++++++++++++++++++++++++++
>> drivers/irqchip/irq-gic.c | 3 ++
>> include/linux/irqchip/arm-gic.h | 4 ++
>> 3 files changed, 102 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
>> [...]
>> @@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
>>
>> if (to_of_node(fwnode))
>> name = to_of_node(fwnode)->name;
>> + else
>> + name = irq_domain_get_irqchip_fwnode_name(fwnode);
>
> Don't bother with that, the name associated with the domain is
> absolutely meaningless. You are already printing the frame address,
> which is enough to identify it, should someone need to debug this.
>
> Drop the name from the previous patch as well, and that will make one
> less difference to care about. Patch #3 can die as well.
>
Ok. I'll just leave them blank (i.e. const char *name ="")
>> [...]
>> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
>> index bae69e5..30b2ccb 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -108,6 +108,10 @@ void gic_init(unsigned int nr, int start,
>>
>> int gicv2m_of_init(struct device_node *node, struct irq_domain *parent);
>>
>> +#ifdef CONFIG_ACPI
>> +int gicv2m_acpi_init(struct irq_domain *parent);
>> +#endif
>> +
>
> How about having a single:
>
> int gicv2m_init(struct fwnode_handle *parent_handle,
> struct irq_domain *parent_domain);
>
> which in turn calls either gicv2m_of_init or gicv2m_acpi_init? Saves
> some #ifdef, and avoids another entry point.
Sounds good. I'll take care of this.
>> void gic_send_sgi(unsigned int cpu_id, unsigned int irq);
>> int gic_get_cpu_id(unsigned int cpu);
>> void gic_migrate_target(unsigned int new_cpu_id);
>
> Otherwise, this looks good.
>
> Thanks,
>
> M.
>
Thanks. Sending out v6 in a little bit.
Suravee
On 09/12/15 18:02, Suravee Suthikulanit wrote:
> Hi Marc,
>
> On 12/9/2015 4:38 AM, Marc Zyngier wrote:
>> On Tue, 8 Dec 2015 17:48:06 -0800
>> Suravee Suthikulpanit <[email protected]> wrote:
>>
>>> This patch introduces gicv2m_acpi_init(), which uses information
>>> in MADT GIC MSI frames structure to initialize GICv2m driver.
>>>
>>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>>> Signed-off-by: Hanjun Guo <[email protected]>
>>> ---
>>> drivers/irqchip/irq-gic-v2m.c | 95 +++++++++++++++++++++++++++++++++++++++++
>>> drivers/irqchip/irq-gic.c | 3 ++
>>> include/linux/irqchip/arm-gic.h | 4 ++
>>> 3 files changed, 102 insertions(+)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
>>> [...]
>>> @@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
>>>
>>> if (to_of_node(fwnode))
>>> name = to_of_node(fwnode)->name;
>>> + else
>>> + name = irq_domain_get_irqchip_fwnode_name(fwnode);
>>
>> Don't bother with that, the name associated with the domain is
>> absolutely meaningless. You are already printing the frame address,
>> which is enough to identify it, should someone need to debug this.
>>
>> Drop the name from the previous patch as well, and that will make one
>> less difference to care about. Patch #3 can die as well.
>>
>
> Ok. I'll just leave them blank (i.e. const char *name ="")
No, just remove name altogether. Nobody reads that anyway, and if they
want to find out, there is the address that's clear enough.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 12/9/2015 12:16 PM, Marc Zyngier wrote:
>>>> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
>>>> >>>[...]
>>>> >>>@@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
>>>> >>>
>>>> >>> if (to_of_node(fwnode))
>>>> >>> name = to_of_node(fwnode)->name;
>>>> >>>+ else
>>>> >>>+ name = irq_domain_get_irqchip_fwnode_name(fwnode);
>>> >>
>>> >>Don't bother with that, the name associated with the domain is
>>> >>absolutely meaningless. You are already printing the frame address,
>>> >>which is enough to identify it, should someone need to debug this.
>>> >>
>>> >>Drop the name from the previous patch as well, and that will make one
>>> >>less difference to care about. Patch #3 can die as well.
>>> >>
>> >
>> >Ok. I'll just leave them blank (i.e. const char *name ="")
> No, just remove name altogether. Nobody reads that anyway, and if they
> want to find out, there is the address that's clear enough.
>
> Thanks,
>
> M.
Ok.
Suravee