2015-07-15 12:17:03

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 00/19] Per-device MSI domain & platform MSI

MSI-like interrupts are starting to creep out of the PCI world, and
can now be seen into a number of "platform"-type busses. The
introduction of the MSI domains feature in v3.19 recognised that fact,
and started providing a way to implement this.

Another step in this direction is Jiang Liu's msi_desc series:

https://github.com/jiangliu/linux.git msi_desc_v1

which moves the msi_list from being specific to PCI devices into the
generic device structure.

A problem we have to solve is to identify which MSI domain a device is
"connected" to. Currently, PCI gets away with a mixture of
arch-specific callbacks, and a msi_controller structure that can
optionally carry a pointer to an MSI domain. As we add new bus types
and start dealing with topologies that do not map to what PCI does,
this doesn't scale anymore.

This patch series tries to address some of it by providing a basic
link between 'struct device' and an MSI domain. It also adds :

- a way to "tag" domains according to the "type" of interrupt it
provides (PCI/MSI, platform MSI...), allowing a driver for a piece
of HW identified by its device_node to provide several IRQ domains

- (yet another) way for PCI to propagate the domain pointer through
the PCI device hierarchy, providing a method for OF to kick-start
the propagation process, and finally allowing the PCI/MSI layer to
use that information

- a similar way to hook an MSI domain to a platform device.

- a generic implementation of MSI for platform devices that can be
supported by interrupt controllers.

- support code for platform MSI for both GICv2m and GICv3 ITS.

This should make implementing MSI support in platform devices a
relatively trivial task.

Additionally, a few patches use most of the above to remove any trace
of the msi_controller structure from the three MSI controllers that
are currently in use on arm64, so that they solely rely on the above
infrastructure, leading to some interesting improvements (also known
as "gross hack prevention measures"). We also take this opportunity to
kill the domain pointer from the msi_controller structure.

My hope is to eventually kill msi_controller entirely, and only rely
on the msi_domain contained in the device structure (any help
welcomed, as I already have cleaned up Tegra, and Lorenzo has scrubbed
Designware).

This has been tested on arm64 with GICv2m (AMD Seattle) and GICv3 ITS
(FVP model), and use of platform MSI has been tested with the ARM
SMMUv3.

Patches are on top of 4.2-rc2 and Jiang Liu's series, and available
at:

git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/npci-msi-v2

As always, comments most welcome..

Marc Zyngier (19):
genirq: irqdomain: Allow irq domain aliasing
PCI: MSI: Register irq domain with specific token
device core: Introduce per-device MSI domain pointer
PCI/MSI: Add hooks to populate the msi_domain field
PCI/MSI: of: Add support for OF-provided msi_domain
PCI/MSI: of: Allow msi_domain lookup using the host bridge node
PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain
platform: of: Assign MSI domain to platform device
drivers: base: Add MSI domain support for non-PCI devices
genirq: Add DOMAIN_BUS_NEXUS irqdomain property
irqchip: gicv3-its: Split PCI/MSI code from the core ITS driver
irqchip: gicv3-its: Register irq domain with NEXUS token
irqchip: gicv3-its: Get rid of struct msi_controller
irqchip: gicv3-its: Make the PCI/MSI code standalone
irqchip: gicv3-its: Add platform MSI support
irqchip: GICv2m: Get rid of struct msi_controller
irqchip: GICv2m: Add platform MSI support
PCI/MSI: pci-xgene-msi: Get rid of struct msi_controller
PCI/MSI: Drop domain field from msi_controller

arch/powerpc/platforms/512x/mpc5121_ads_cpld.c | 3 +-
arch/powerpc/platforms/cell/interrupt.c | 3 +-
arch/powerpc/platforms/embedded6xx/flipper-pic.c | 3 +-
arch/powerpc/platforms/powermac/pic.c | 3 +-
arch/powerpc/platforms/powernv/opal-irqchip.c | 3 +-
arch/powerpc/platforms/ps3/interrupt.c | 3 +-
arch/powerpc/sysdev/ehv_pic.c | 3 +-
arch/powerpc/sysdev/i8259.c | 3 +-
arch/powerpc/sysdev/ipic.c | 3 +-
arch/powerpc/sysdev/mpic.c | 3 +-
arch/powerpc/sysdev/qe_lib/qe_ic.c | 3 +-
arch/powerpc/sysdev/xics/xics-common.c | 3 +-
drivers/base/Makefile | 1 +
drivers/base/platform-msi.c | 282 +++++++++++++++++++++++
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-gic-v2m.c | 52 +++--
drivers/irqchip/irq-gic-v3-its-pci-msi.c | 140 +++++++++++
drivers/irqchip/irq-gic-v3-its-platform-msi.c | 93 ++++++++
drivers/irqchip/irq-gic-v3-its.c | 140 ++++-------
drivers/of/irq.c | 16 ++
drivers/of/platform.c | 1 +
drivers/pci/host/pci-xgene-msi.c | 41 ++--
drivers/pci/msi.c | 11 +-
drivers/pci/of.c | 24 ++
drivers/pci/probe.c | 31 +++
include/linux/device.h | 20 ++
include/linux/irqchip/arm-gic-v3.h | 1 +
include/linux/irqdomain.h | 26 ++-
include/linux/msi.h | 18 +-
include/linux/of_irq.h | 1 +
include/linux/pci.h | 3 +
kernel/irq/irqdomain.c | 18 +-
32 files changed, 785 insertions(+), 172 deletions(-)
create mode 100644 drivers/base/platform-msi.c
create mode 100644 drivers/irqchip/irq-gic-v3-its-pci-msi.c
create mode 100644 drivers/irqchip/irq-gic-v3-its-platform-msi.c

--
2.1.4


2015-07-15 12:22:17

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 01/19] genirq: irqdomain: Allow irq domain aliasing

It is not uncommon (at least with the ARM stuff) to have a piece
of hardware that implements different flavours of "interrupts".
A typical example of this is the GICv3 ITS, which implements
standard PCI/MSI support, but also some form of "generic MSI".

So far, the PCI/MSI domain is registered using the ITS device_node,
so that irq_find_host can return it. On the contrary, the raw MSI
domain is not registered with an device_node, making it impossible
to be looked up by another subsystem (obviously, using the same
device_node twice would only result in confusion, as it is not
defined which one irq_find_host would return).

A solution to this is to "type" domains that may be aliasing, and
to be able to lookup an device_node that matches a given type.
For this, we introduce irq_find_matching_host() as a superset
of irq_find_host:

struct irq_domain *irq_find_matching_host(struct device_node *node,
enum irq_domain_bus_token bus_token);

where bus_token is the "type" we want to match the domain against
(so far, only DOMAIN_BUS_ANY is defined). This result in some
moderately invasive changes on the PPC side (which is the only
user of the .match method).

This has otherwise no functionnal change.

Signed-off-by: Marc Zyngier <[email protected]>
---
arch/powerpc/platforms/512x/mpc5121_ads_cpld.c | 3 ++-
arch/powerpc/platforms/cell/interrupt.c | 3 ++-
arch/powerpc/platforms/embedded6xx/flipper-pic.c | 3 ++-
arch/powerpc/platforms/powermac/pic.c | 3 ++-
arch/powerpc/platforms/powernv/opal-irqchip.c | 3 ++-
arch/powerpc/platforms/ps3/interrupt.c | 3 ++-
arch/powerpc/sysdev/ehv_pic.c | 3 ++-
arch/powerpc/sysdev/i8259.c | 3 ++-
arch/powerpc/sysdev/ipic.c | 3 ++-
arch/powerpc/sysdev/mpic.c | 3 ++-
arch/powerpc/sysdev/qe_lib/qe_ic.c | 3 ++-
arch/powerpc/sysdev/xics/xics-common.c | 3 ++-
include/linux/irqdomain.h | 23 +++++++++++++++++++++--
kernel/irq/irqdomain.c | 18 +++++++++++++-----
14 files changed, 58 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/platforms/512x/mpc5121_ads_cpld.c b/arch/powerpc/platforms/512x/mpc5121_ads_cpld.c
index ca3a062..11090ab 100644
--- a/arch/powerpc/platforms/512x/mpc5121_ads_cpld.c
+++ b/arch/powerpc/platforms/512x/mpc5121_ads_cpld.c
@@ -123,7 +123,8 @@ cpld_pic_cascade(unsigned int irq, struct irq_desc *desc)
}

static int
-cpld_pic_host_match(struct irq_domain *h, struct device_node *node)
+cpld_pic_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
return cpld_pic_node == node;
}
diff --git a/arch/powerpc/platforms/cell/interrupt.c b/arch/powerpc/platforms/cell/interrupt.c
index 3af8324..a15f1ef 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -222,7 +222,8 @@ void iic_request_IPIs(void)
#endif /* CONFIG_SMP */


-static int iic_host_match(struct irq_domain *h, struct device_node *node)
+static int iic_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
return of_device_is_compatible(node,
"IBM,CBEA-Internal-Interrupt-Controller");
diff --git a/arch/powerpc/platforms/embedded6xx/flipper-pic.c b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
index 4cde8e7..b7866e0 100644
--- a/arch/powerpc/platforms/embedded6xx/flipper-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
@@ -108,7 +108,8 @@ static int flipper_pic_map(struct irq_domain *h, unsigned int virq,
return 0;
}

-static int flipper_pic_match(struct irq_domain *h, struct device_node *np)
+static int flipper_pic_match(struct irq_domain *h, struct device_node *np,
+ enum irq_domain_bus_token bus_token)
{
return 1;
}
diff --git a/arch/powerpc/platforms/powermac/pic.c b/arch/powerpc/platforms/powermac/pic.c
index 59cfc9d..6f4f8b0 100644
--- a/arch/powerpc/platforms/powermac/pic.c
+++ b/arch/powerpc/platforms/powermac/pic.c
@@ -268,7 +268,8 @@ static struct irqaction gatwick_cascade_action = {
.name = "cascade",
};

-static int pmac_pic_host_match(struct irq_domain *h, struct device_node *node)
+static int pmac_pic_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
/* We match all, we don't always have a node anyway */
return 1;
diff --git a/arch/powerpc/platforms/powernv/opal-irqchip.c b/arch/powerpc/platforms/powernv/opal-irqchip.c
index e2e7d75..2c91ee7 100644
--- a/arch/powerpc/platforms/powernv/opal-irqchip.c
+++ b/arch/powerpc/platforms/powernv/opal-irqchip.c
@@ -134,7 +134,8 @@ static void opal_handle_irq_work(struct irq_work *work)
opal_handle_events(be64_to_cpu(last_outstanding_events));
}

-static int opal_event_match(struct irq_domain *h, struct device_node *node)
+static int opal_event_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
return h->of_node == node;
}
diff --git a/arch/powerpc/platforms/ps3/interrupt.c b/arch/powerpc/platforms/ps3/interrupt.c
index a6c42f3..638c406 100644
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -678,7 +678,8 @@ static int ps3_host_map(struct irq_domain *h, unsigned int virq,
return 0;
}

-static int ps3_host_match(struct irq_domain *h, struct device_node *np)
+static int ps3_host_match(struct irq_domain *h, struct device_node *np,
+ enum irq_domain_bus_token bus_token)
{
/* Match all */
return 1;
diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
index 2d20f10..eca0b00 100644
--- a/arch/powerpc/sysdev/ehv_pic.c
+++ b/arch/powerpc/sysdev/ehv_pic.c
@@ -177,7 +177,8 @@ unsigned int ehv_pic_get_irq(void)
return irq_linear_revmap(global_ehv_pic->irqhost, irq);
}

-static int ehv_pic_host_match(struct irq_domain *h, struct device_node *node)
+static int ehv_pic_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
/* Exact match, unless ehv_pic node is NULL */
return h->of_node == NULL || h->of_node == node;
diff --git a/arch/powerpc/sysdev/i8259.c b/arch/powerpc/sysdev/i8259.c
index 31c3347..e1a9c2c 100644
--- a/arch/powerpc/sysdev/i8259.c
+++ b/arch/powerpc/sysdev/i8259.c
@@ -162,7 +162,8 @@ static struct resource pic_edgectrl_iores = {
.flags = IORESOURCE_BUSY,
};

-static int i8259_host_match(struct irq_domain *h, struct device_node *node)
+static int i8259_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
return h->of_node == NULL || h->of_node == node;
}
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index d78f136..6b2b689 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -671,7 +671,8 @@ static struct irq_chip ipic_edge_irq_chip = {
.irq_set_type = ipic_set_irq_type,
};

-static int ipic_host_match(struct irq_domain *h, struct device_node *node)
+static int ipic_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
/* Exact match, unless ipic node is NULL */
return h->of_node == NULL || h->of_node == node;
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index c8e7333..97a8ae8 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1007,7 +1007,8 @@ static struct irq_chip mpic_irq_ht_chip = {
#endif /* CONFIG_MPIC_U3_HT_IRQS */


-static int mpic_host_match(struct irq_domain *h, struct device_node *node)
+static int mpic_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
/* Exact match, unless mpic node is NULL */
return h->of_node == NULL || h->of_node == node;
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 6512cd8..47b352e 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -244,7 +244,8 @@ static struct irq_chip qe_ic_irq_chip = {
.irq_mask_ack = qe_ic_mask_irq,
};

-static int qe_ic_host_match(struct irq_domain *h, struct device_node *node)
+static int qe_ic_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
/* Exact match, unless qe_ic node is NULL */
return h->of_node == NULL || h->of_node == node;
diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
index 08c248e..47e43b7 100644
--- a/arch/powerpc/sysdev/xics/xics-common.c
+++ b/arch/powerpc/sysdev/xics/xics-common.c
@@ -298,7 +298,8 @@ int xics_get_irq_server(unsigned int virq, const struct cpumask *cpumask,
}
#endif /* CONFIG_SMP */

-static int xics_host_match(struct irq_domain *h, struct device_node *node)
+static int xics_host_match(struct irq_domain *h, struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
struct ics *ics;

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 744ac0e..91a83ad 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -45,6 +45,17 @@ struct irq_data;
/* Number of irqs reserved for a legacy isa controller */
#define NUM_ISA_INTERRUPTS 16

+/*
+ * Should several domains have the same device node, but serve
+ * different purposes (for example one domain is for PCI/MSI, and the
+ * other for wired IRQs), they can be distinguished using a
+ * bus-specific token. Most domains are expected to only carry
+ * DOMAIN_BUS_ANY.
+ */
+enum irq_domain_bus_token {
+ DOMAIN_BUS_ANY = 0,
+};
+
/**
* struct irq_domain_ops - Methods for irq_domain objects
* @match: Match an interrupt controller device node to a host, returns
@@ -61,7 +72,8 @@ struct irq_data;
* to setup the irq_desc when returning from map().
*/
struct irq_domain_ops {
- int (*match)(struct irq_domain *d, struct device_node *node);
+ int (*match)(struct irq_domain *d, struct device_node *node,
+ enum irq_domain_bus_token bus_token);
int (*map)(struct irq_domain *d, unsigned int virq, irq_hw_number_t hw);
void (*unmap)(struct irq_domain *d, unsigned int virq);
int (*xlate)(struct irq_domain *d, struct device_node *node,
@@ -116,6 +128,7 @@ struct irq_domain {

/* Optional data */
struct device_node *of_node;
+ enum irq_domain_bus_token bus_token;
struct irq_domain_chip_generic *gc;
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
struct irq_domain *parent;
@@ -161,9 +174,15 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
irq_hw_number_t first_hwirq,
const struct irq_domain_ops *ops,
void *host_data);
-extern struct irq_domain *irq_find_host(struct device_node *node);
+extern struct irq_domain *irq_find_matching_host(struct device_node *node,
+ enum irq_domain_bus_token bus_token);
extern void irq_set_default_host(struct irq_domain *host);

+static inline struct irq_domain *irq_find_host(struct device_node *node)
+{
+ return irq_find_matching_host(node, DOMAIN_BUS_ANY);
+}
+
/**
* irq_domain_add_linear() - Allocate and register a linear revmap irq_domain.
* @of_node: pointer to interrupt controller's device tree node.
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 8c3577f..995d217 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -187,10 +187,12 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
EXPORT_SYMBOL_GPL(irq_domain_add_legacy);

/**
- * irq_find_host() - Locates a domain for a given device node
+ * irq_find_matching_host() - Locates a domain for a given device node
* @node: device-tree node of the interrupt controller
+ * @data: domain-specific data
*/
-struct irq_domain *irq_find_host(struct device_node *node)
+struct irq_domain *irq_find_matching_host(struct device_node *node,
+ enum irq_domain_bus_token bus_token)
{
struct irq_domain *h, *found = NULL;
int rc;
@@ -199,13 +201,19 @@ struct irq_domain *irq_find_host(struct device_node *node)
* it might potentially be set to match all interrupts in
* the absence of a device node. This isn't a problem so far
* yet though...
+ *
+ * bus_token == DOMAIN_BUS_ANY matches any domain, any other
+ * values must generate an exact match for the domain to be
+ * selected.
*/
mutex_lock(&irq_domain_mutex);
list_for_each_entry(h, &irq_domain_list, link) {
if (h->ops->match)
- rc = h->ops->match(h, node);
+ rc = h->ops->match(h, node, bus_token);
else
- rc = (h->of_node != NULL) && (h->of_node == node);
+ rc = ((h->of_node != NULL) && (h->of_node == node) &&
+ ((bus_token == DOMAIN_BUS_ANY) ||
+ (h->bus_token == bus_token)));

if (rc) {
found = h;
@@ -215,7 +223,7 @@ struct irq_domain *irq_find_host(struct device_node *node)
mutex_unlock(&irq_domain_mutex);
return found;
}
-EXPORT_SYMBOL_GPL(irq_find_host);
+EXPORT_SYMBOL_GPL(irq_find_matching_host);

/**
* irq_set_default_host() - Set a "default" irq domain
--
2.1.4

2015-07-15 12:17:06

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 02/19] PCI: MSI: Register irq domain with specific token

When creating a PCI/MSI domain, tag it with DOMAIN_BUS_PCI_MSI so
that it can be looked-up using irq_find_matching_host().

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/pci/msi.c | 8 +++++++-
include/linux/irqdomain.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 373d96e..ef4ec6e 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1273,12 +1273,18 @@ struct irq_domain *pci_msi_create_irq_domain(struct device_node *node,
struct msi_domain_info *info,
struct irq_domain *parent)
{
+ struct irq_domain *domain;
+
if (info->flags & MSI_FLAG_USE_DEF_DOM_OPS)
pci_msi_domain_update_dom_ops(info);
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
pci_msi_domain_update_chip_ops(info);

- return msi_create_irq_domain(node, info, parent);
+ domain = msi_create_irq_domain(node, info, parent);
+ if (domain)
+ domain->bus_token = DOMAIN_BUS_PCI_MSI;
+
+ return domain;
}

/**
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 91a83ad..25e9e66 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -54,6 +54,7 @@ struct irq_data;
*/
enum irq_domain_bus_token {
DOMAIN_BUS_ANY = 0,
+ DOMAIN_BUS_PCI_MSI,
};

/**
--
2.1.4

2015-07-15 12:17:10

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 03/19] device core: Introduce per-device MSI domain pointer

As MSI-type features are creeping into non-PCI devices, it is
starting to make sense to give our struct device some form of
support for this, by allowing a pointer to an MSI irq domain to
be set/retrieved.

Signed-off-by: Marc Zyngier <[email protected]>
---
include/linux/device.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 22227e7..7c49b48 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -714,6 +714,7 @@ struct device_dma_parameters {
* @pins: For device pin management.
* See Documentation/pinctrl.txt for details.
* @msi_list: Hosts MSI descriptors
+ * @msi_domain: The generic MSI domain this device is using.
* @numa_node: NUMA node this device is close to.
* @dma_mask: Dma mask (if dma'ble device).
* @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all
@@ -774,6 +775,9 @@ struct device {
struct dev_pm_info power;
struct dev_pm_domain *pm_domain;

+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+ struct irq_domain *msi_domain; /* MSI domain device uses */
+#endif
#ifdef CONFIG_PINCTRL
struct dev_pin_info *pins;
#endif
@@ -864,6 +868,22 @@ static inline void set_dev_node(struct device *dev, int node)
}
#endif

+static inline struct irq_domain *dev_get_msi_domain(const struct device *dev)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+ return dev->msi_domain;
+#else
+ return NULL;
+#endif
+}
+
+static inline void dev_set_msi_domain(struct device *dev, struct irq_domain *d)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+ dev->msi_domain = d;
+#endif
+}
+
static inline void *dev_get_drvdata(const struct device *dev)
{
return dev->driver_data;
--
2.1.4

2015-07-15 12:21:25

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

In order to be able to populate the device msi_domain field,
add the necesary hooks to propagate the host bridge msi_domain
across secondary busses to devices.

So far, nobody populates the initial msi_domain.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 31 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index cefd636..376f6fa 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
}
}

+void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
+{
+}
+
+static void pci_set_bus_msi_domain(struct pci_bus *bus)
+{
+ struct pci_dev *bridge = bus->self;
+
+ if (!bridge)
+ pcibios_set_host_bridge_msi_domain(bus);
+ else
+ dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
+}
+
static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
struct pci_dev *bridge, int busnr)
{
@@ -714,6 +728,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
bridge->subordinate = child;

add_dev:
+ pci_set_bus_msi_domain(child);
ret = device_register(&child->dev);
WARN_ON(ret < 0);

@@ -1544,6 +1559,17 @@ static void pci_init_capabilities(struct pci_dev *dev)
pci_enable_acs(dev);
}

+static void pci_set_msi_domain(struct pci_dev *dev)
+{
+ /*
+ * If no domain has been set through the pcibios callback,
+ * inherit the default from the bus device.
+ */
+ if (!dev_get_msi_domain(&dev->dev))
+ dev_set_msi_domain(&dev->dev,
+ dev_get_msi_domain(&dev->bus->dev));
+}
+
void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
{
int ret;
@@ -1585,6 +1611,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
ret = pcibios_add_device(dev);
WARN_ON(ret < 0);

+ /* Setup MSI irq domain */
+ pci_set_msi_domain(dev);
+
/* Notifier could use PCI capabilities */
dev->match_driver = false;
ret = device_add(&dev->dev);
@@ -1975,6 +2004,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
b->bridge = get_device(&bridge->dev);
device_enable_async_suspend(b->bridge);
pci_set_bus_of_node(b);
+ pci_set_bus_msi_domain(b);

if (!parent)
set_dev_node(b->bridge, pcibus_to_node(b));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fbf245f..fe0825f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1644,6 +1644,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
int pcibios_add_device(struct pci_dev *dev);
void pcibios_release_device(struct pci_dev *dev);
void pcibios_penalize_isa_irq(int irq, int active);
+void pcibios_set_host_bridge_msi_domain(struct pci_bus *bus);

#ifdef CONFIG_HIBERNATE_CALLBACKS
extern struct dev_pm_ops pcibios_pm_ops;
--
2.1.4

2015-07-15 12:17:15

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 05/19] PCI/MSI: of: Add support for OF-provided msi_domain

In order to populate the PHB msi_domain, use the "msi-parent"
attribute to lookup a corresponding irq domain. If found,
this is our MSI domain.

This gets plugged into the core PCI code.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/pci/of.c | 19 +++++++++++++++++++
drivers/pci/probe.c | 1 +
include/linux/pci.h | 2 ++
3 files changed, 22 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index f092993..09df465 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -9,6 +9,7 @@
* 2 of the License, or (at your option) any later version.
*/

+#include <linux/irqdomain.h>
#include <linux/kernel.h>
#include <linux/pci.h>
#include <linux/of.h>
@@ -59,3 +60,21 @@ struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
return of_node_get(bus->bridge->parent->of_node);
return NULL;
}
+
+void pci_set_phb_of_msi_domain(struct pci_bus *bus)
+{
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+ struct device_node *np;
+ struct irq_domain *d;
+
+ if (!bus->dev.of_node)
+ return;
+ np = of_parse_phandle(bus->dev.of_node, "msi-parent", 0);
+ if (!np)
+ return;
+ d = irq_find_matching_host(np, DOMAIN_BUS_PCI_MSI);
+ if (!d)
+ d = irq_find_host(np);
+ dev_set_msi_domain(&bus->dev, d);
+#endif
+}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 376f6fa..c80626f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -663,6 +663,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)

void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
{
+ pci_set_phb_of_msi_domain(bus);
}

static void pci_set_bus_msi_domain(struct pci_bus *bus)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fe0825f..b3ad3d4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1846,6 +1846,7 @@ void pci_set_of_node(struct pci_dev *dev);
void pci_release_of_node(struct pci_dev *dev);
void pci_set_bus_of_node(struct pci_bus *bus);
void pci_release_bus_of_node(struct pci_bus *bus);
+void pci_set_phb_of_msi_domain(struct pci_bus *bus);

/* Arch may override this (weak) */
struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus);
@@ -1868,6 +1869,7 @@ static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
static inline struct device_node *
pci_device_to_OF_node(const struct pci_dev *pdev) { return NULL; }
+static inline void pci_set_phb_of_msi_domain(struct pci_bus *bus) { }
#endif /* CONFIG_OF */

#ifdef CONFIG_EEH
--
2.1.4

2015-07-15 12:20:59

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 06/19] PCI/MSI: of: Allow msi_domain lookup using the host bridge node

A number of platforms do not need to use the msi-parent property,
as the host bridge itself provides the MSI controller.

Allow this configuration by performing an irq domain lookup based
on the host bridge node if it doesn't have a valid msi-parent property.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/pci/of.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 09df465..9d61cef 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -69,9 +69,14 @@ void pci_set_phb_of_msi_domain(struct pci_bus *bus)

if (!bus->dev.of_node)
return;
+ /* Start looking for a phandle to an MSI controller. */
np = of_parse_phandle(bus->dev.of_node, "msi-parent", 0);
+ /*
+ * If we don't have an msi-parent property, look for a domain
+ * directly attached to the host bridge.
+ */
if (!np)
- return;
+ np = bus->dev.of_node;
d = irq_find_matching_host(np, DOMAIN_BUS_PCI_MSI);
if (!d)
d = irq_find_host(np);
--
2.1.4

2015-07-15 12:17:18

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 07/19] PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain

Now that we can easily find which MSI domain a PCI device is
using, use dev_get_msi_domain as a way to retrieve the information.

The original code is still used as a fallback.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/pci/msi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index ef4ec6e..c77fdaf 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -41,7 +41,8 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev)
{
struct irq_domain *domain = NULL;

- if (dev->bus->msi)
+ domain = dev_get_msi_domain(&dev->dev);
+ if (!domain && dev->bus->msi)
domain = dev->bus->msi->domain;
if (!domain)
domain = arch_get_pci_msi_domain(dev);
--
2.1.4

2015-07-15 12:17:22

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 08/19] platform: of: Assign MSI domain to platform device

As for PCI, we're able to populate the msi_domain field at probe time,
provided that the device tree has an "msi-parent" property.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/of/irq.c | 16 ++++++++++++++++
drivers/of/platform.c | 1 +
include/linux/irqdomain.h | 1 +
include/linux/of_irq.h | 1 +
4 files changed, 19 insertions(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 3cf7a01..64392a6 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -18,6 +18,7 @@
* driver.
*/

+#include <linux/device.h>
#include <linux/errno.h>
#include <linux/list.h>
#include <linux/module.h>
@@ -576,3 +577,18 @@ err:
kfree(desc);
}
}
+
+void of_msi_configure(struct device *dev, struct device_node *np)
+{
+ struct device_node *msi_np;
+ struct irq_domain *d;
+
+ msi_np = of_parse_phandle(np, "msi-parent", 0);
+ if (!msi_np)
+ return;
+
+ d = irq_find_matching_host(msi_np, DOMAIN_BUS_PLATFORM_MSI);
+ if (!d)
+ d = irq_find_host(msi_np);
+ dev_set_msi_domain(dev, d);
+}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index ddf8e42..8a002d6 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -184,6 +184,7 @@ static struct platform_device *of_platform_device_create_pdata(
dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
of_dma_configure(&dev->dev, dev->dev.of_node);
+ of_msi_configure(&dev->dev, dev->dev.of_node);

if (of_device_add(dev) != 0) {
of_dma_deconfigure(&dev->dev);
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 25e9e66..b4a74f7 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -55,6 +55,7 @@ struct irq_data;
enum irq_domain_bus_token {
DOMAIN_BUS_ANY = 0,
DOMAIN_BUS_PCI_MSI,
+ DOMAIN_BUS_PLATFORM_MSI,
};

/**
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index d884929..4bcbd58 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -74,6 +74,7 @@ static inline int of_irq_to_resource_table(struct device_node *dev,
*/
extern unsigned int irq_of_parse_and_map(struct device_node *node, int index);
extern struct device_node *of_irq_find_parent(struct device_node *child);
+extern void of_msi_configure(struct device *dev, struct device_node *np);

#else /* !CONFIG_OF */
static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
--
2.1.4

2015-07-15 12:17:24

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 09/19] drivers: base: Add MSI domain support for non-PCI devices

With the msi_list and the msi_domain properties now being at the
generic device level, it is starting to be relatively easy to offer
a generic way of providing non-PCI MSIs.

The two major hurdles with this idea are:

- Lack of global ID that identifies a device: this is worked around by
having a global ID allocator for each device that gets enrolled in
the platform MSI subsystem

- Lack of standard way to write the message in the generating device.
This is solved by mandating driver code to provide a write_msg
callback, so that everyone can have their own square wheel

Apart from that, the API is fairly straightforward:

- platform_msi_create_irq_domain creates an MSI domain that gets
tagged with DOMAIN_BUS_PLATFORM_MSI

- platform_msi_domain_alloc_irqs allocate MSIs for a given device,
populating the msi_list

- platform_msi_domain_free_irqs does what is written on the tin

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/base/Makefile | 1 +
drivers/base/platform-msi.c | 282 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/msi.h | 15 +++
3 files changed, 298 insertions(+)
create mode 100644 drivers/base/platform-msi.c

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 527d291..6b2a84e 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_REGMAP) += regmap/
obj-$(CONFIG_SOC_BUS) += soc.o
obj-$(CONFIG_PINCTRL) += pinctrl.o
obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
+obj-$(CONFIG_GENERIC_MSI_IRQ_DOMAIN) += platform-msi.o

ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
new file mode 100644
index 0000000..e35be99
--- /dev/null
+++ b/drivers/base/platform-msi.c
@@ -0,0 +1,282 @@
+/*
+ * MSI framework for platform devices
+ *
+ * Copyright (C) 2015 ARM Limited, All Rights Reserved.
+ * Author: Marc Zyngier <[email protected]>
+ *
+ * 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.
+ *
+ * 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 for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/device.h>
+#include <linux/idr.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/msi.h>
+#include <linux/slab.h>
+
+#define DEV_ID_SHIFT 24
+
+/*
+ * Internal data structure containing a (made up, but unique) devid
+ * and the callback to write the MSI message.
+ */
+struct platform_msi_priv_data {
+ irq_write_msi_msg_t write_msg;
+ int devid;
+};
+
+/* The devid allocator */
+static DEFINE_IDA(platform_msi_devid_ida);
+
+#ifdef GENERIC_MSI_DOMAIN_OPS
+/*
+ * Convert an msi_desc to a globaly unique identifier (per-device
+ * devid + msi_desc position in the msi_list).
+ */
+static irq_hw_number_t platform_msi_calc_hwirq(struct msi_desc *desc)
+{
+ u32 devid;
+
+ devid = desc->platform_msi_priv_data->devid;
+
+ return (devid << (32 - DEV_ID_SHIFT)) | desc->msi_index;
+}
+
+static void platform_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
+{
+ arg->desc = desc;
+ arg->hwirq = platform_msi_calc_hwirq(desc);
+}
+
+static int platform_msi_init(struct irq_domain *domain,
+ struct msi_domain_info *info,
+ unsigned int virq, irq_hw_number_t hwirq,
+ msi_alloc_info_t *arg)
+{
+ struct irq_data *data;
+
+ irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+ info->chip, info->chip_data);
+
+ /*
+ * Save the MSI descriptor in handler_data so that the
+ * irq_write_msi_msg callback can retrieve it (and the
+ * associated device).
+ */
+ data = irq_domain_get_irq_data(domain, virq);
+ data->handler_data = arg->desc;
+
+ return 0;
+}
+#else
+#define platform_msi_set_desc NULL
+#define platform_msi_init NULL
+#endif
+
+static void platform_msi_update_dom_ops(struct msi_domain_info *info)
+{
+ struct msi_domain_ops *ops = info->ops;
+
+ BUG_ON(!ops);
+
+ if (ops->msi_init == NULL)
+ ops->msi_init = platform_msi_init;
+ if (ops->set_desc == NULL)
+ ops->set_desc = platform_msi_set_desc;
+}
+
+static void platform_msi_write_msg(struct irq_data *data, struct msi_msg *msg)
+{
+ struct msi_desc *desc = irq_data_get_irq_handler_data(data);
+ struct platform_msi_priv_data *priv_data;
+
+ priv_data = desc->platform_msi_priv_data;
+
+ priv_data->write_msg(desc, msg);
+}
+
+static void platform_msi_update_chip_ops(struct msi_domain_info *info)
+{
+ struct irq_chip *chip = info->chip;
+
+ BUG_ON(!chip);
+ if (!chip->irq_mask)
+ chip->irq_mask = irq_chip_mask_parent;
+ if (!chip->irq_unmask)
+ chip->irq_unmask = irq_chip_unmask_parent;
+ if (!chip->irq_eoi)
+ chip->irq_eoi = irq_chip_eoi_parent;
+ if (!chip->irq_set_affinity)
+ chip->irq_set_affinity = msi_domain_set_affinity;
+ if (!chip->irq_write_msi_msg)
+ chip->irq_write_msi_msg = platform_msi_write_msg;
+}
+
+static void platform_msi_free_descs(struct device *dev)
+{
+ struct msi_desc *desc, *tmp;
+
+ list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
+ list_del(&desc->list);
+ free_msi_entry(desc);
+ }
+}
+
+static int platform_msi_alloc_descs(struct device *dev, int nvec,
+ struct platform_msi_priv_data *data)
+
+{
+ int i;
+
+ for (i = 0; i < nvec; i++) {
+ struct msi_desc *desc;
+
+ desc = alloc_msi_entry(dev);
+ if (!desc)
+ break;
+
+ desc->platform_msi_priv_data = data;
+ desc->msi_index = i;
+ desc->nvec_used = 1;
+
+ list_add_tail(&desc->list, dev_to_msi_list(dev));
+ }
+
+ if (i != nvec) {
+ /* Clean up the mess */
+ platform_msi_free_descs(dev);
+
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+/**
+ * platform_msi_create_irq_domain - Create a platform MSI interrupt domain
+ * @np: Optional device-tree node of the interrupt controller
+ * @info: MSI domain info
+ * @parent: Parent irq domain
+ *
+ * Updates the domain and chip ops and creates a platform MSI
+ * interrupt domain.
+ *
+ * Returns:
+ * A domain pointer or NULL in case of failure.
+ */
+struct irq_domain *platform_msi_create_irq_domain(struct device_node *np,
+ struct msi_domain_info *info,
+ struct irq_domain *parent)
+{
+ struct irq_domain *domain;
+
+ if (info->flags & MSI_FLAG_USE_DEF_DOM_OPS)
+ platform_msi_update_dom_ops(info);
+ if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
+ platform_msi_update_chip_ops(info);
+
+ domain = msi_create_irq_domain(np, info, parent);
+ if (domain)
+ domain->bus_token = DOMAIN_BUS_PLATFORM_MSI;
+
+ return domain;
+}
+
+/**
+ * platform_msi_domain_alloc_irqs - Allocate MSI interrupts for @dev
+ * @dev: The device for which to allocate interrupts
+ * @nvec: The number of interrupts to allocate
+ * @write_msi_msg: Callback to write an interrupt message for @dev
+ *
+ * Returns:
+ * Zero for success, or an error code in case of failure
+ */
+int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
+ irq_write_msi_msg_t write_msi_msg)
+{
+ struct platform_msi_priv_data *priv_data;
+ int err;
+
+ /*
+ * Limit the number of interrupts to 256 per device. Should we
+ * need to bump this up, DEV_ID_SHIFT should be adjusted
+ * accordingly (which would impact the max number of MSI
+ * capable devices).
+ */
+ if (!dev->msi_domain || !write_msi_msg || !nvec ||
+ nvec > (1 << (32 - DEV_ID_SHIFT)))
+ return -EINVAL;
+
+ if (dev->msi_domain->bus_token != DOMAIN_BUS_PLATFORM_MSI) {
+ dev_err(dev, "Incompatible msi_domain, giving up\n");
+ return -EINVAL;
+ }
+
+ /* Already had a helping of MSI? Greed... */
+ if (!list_empty(dev_to_msi_list(dev)))
+ return -EBUSY;
+
+ priv_data = kzalloc(sizeof(*priv_data), GFP_KERNEL);
+ if (!priv_data)
+ return -ENOMEM;
+
+ priv_data->devid = ida_simple_get(&platform_msi_devid_ida,
+ 0, 1 << DEV_ID_SHIFT, GFP_KERNEL);
+ if (priv_data->devid < 0) {
+ err = priv_data->devid;
+ goto out_free_data;
+ }
+
+ priv_data->write_msg = write_msi_msg;
+
+ err = platform_msi_alloc_descs(dev, nvec, priv_data);
+ if (err)
+ goto out_free_id;
+
+ err = msi_domain_alloc_irqs(dev->msi_domain, dev, nvec);
+ if (err)
+ goto out_free_desc;
+
+ return 0;
+
+out_free_desc:
+ platform_msi_free_descs(dev);
+out_free_id:
+ ida_simple_remove(&platform_msi_devid_ida, priv_data->devid);
+out_free_data:
+ kfree(priv_data);
+
+ return err;
+}
+
+/**
+ * platform_msi_domain_free_irqs - Free MSI interrupts for @dev
+ * @dev: The device for which to free interrupts
+ */
+void platform_msi_domain_free_irqs(struct device *dev)
+{
+ struct msi_desc *desc;
+
+ desc = first_msi_entry(dev);
+ if (desc) {
+ struct platform_msi_priv_data *data;
+
+ data = desc->platform_msi_priv_data;
+
+ ida_simple_remove(&platform_msi_devid_ida, data->devid);
+ kfree(data);
+ }
+
+ msi_domain_free_irqs(dev->msi_domain, dev);
+ platform_msi_free_descs(dev);
+}
diff --git a/include/linux/msi.h b/include/linux/msi.h
index c10ec56..b55cf63 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -18,6 +18,11 @@ struct pci_dev;
void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);

+typedef void (*irq_write_msi_msg_t)(struct msi_desc *desc,
+ struct msi_msg *msg);
+
+struct platform_msi_priv_data;
+
struct msi_desc {
struct list_head list;
unsigned int irq;
@@ -42,6 +47,10 @@ struct msi_desc {
void __iomem *mask_base;
};
};
+ struct {
+ struct platform_msi_priv_data *platform_msi_priv_data;
+ u16 msi_index;
+ };
};
};

@@ -228,6 +237,12 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev);
struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);

+struct irq_domain *platform_msi_create_irq_domain(struct device_node *np,
+ struct msi_domain_info *info,
+ struct irq_domain *parent);
+int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
+ irq_write_msi_msg_t write_msi_msg);
+void platform_msi_domain_free_irqs(struct device *dev);
#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */

#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
--
2.1.4

2015-07-15 12:20:11

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 10/19] genirq: Add DOMAIN_BUS_NEXUS irqdomain property

Some IRQ domains are not designed to directly provide interrupts
to devices, but strictly to be used by other domains. An example
of this is the GICv3 ITS, which is completely bus agnostic, and
on which it is possible to device a PCI/MSI domain.

Just introduce the irq_domain_bus_token property for now.

Signed-off-by: Marc Zyngier <[email protected]>
---
include/linux/irqdomain.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index b4a74f7..d3ca792 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -56,6 +56,7 @@ enum irq_domain_bus_token {
DOMAIN_BUS_ANY = 0,
DOMAIN_BUS_PCI_MSI,
DOMAIN_BUS_PLATFORM_MSI,
+ DOMAIN_BUS_NEXUS,
};

/**
--
2.1.4

2015-07-15 12:17:27

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 11/19] irqchip: gicv3-its: Split PCI/MSI code from the core ITS driver

It is becoming obvious that having the PCI/MSI code in the same
file as the the core ITS code is giving people implementing non-PCI
MSI support the wrong kind of idea.

In order to make things a bit clearer, let's move the PCI/MSI code
out to its own file. Hopefully it will make it clear that whoever
thinks of hooking into the core ITS better have a very strong point.

We use a temporary entry point that will get removed in a subsequent
patch, once the proper infrastructure is added.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-gic-v3-its-pci-msi.c | 105 +++++++++++++++++++++++++++++++
drivers/irqchip/irq-gic-v3-its.c | 93 ++++-----------------------
include/linux/irqchip/arm-gic-v3.h | 6 ++
4 files changed, 123 insertions(+), 83 deletions(-)
create mode 100644 drivers/irqchip/irq-gic-v3-its-pci-msi.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b8d4e96..0d5f2a9 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -22,7 +22,7 @@ obj-$(CONFIG_ARCH_SPEAR3XX) += spear-shirq.o
obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o
obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o
obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o
-obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o
+obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o
obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
obj-$(CONFIG_ATMEL_AIC_IRQ) += irq-atmel-aic-common.o irq-atmel-aic.o
diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
new file mode 100644
index 0000000..7614721
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2013-2015 ARM Limited, All Rights Reserved.
+ * Author: Marc Zyngier <[email protected]>
+ *
+ * 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.
+ *
+ * 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 for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/msi.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+
+#include <linux/irqchip/arm-gic-v3.h>
+
+static void its_mask_msi_irq(struct irq_data *d)
+{
+ pci_msi_mask_irq(d);
+ irq_chip_mask_parent(d);
+}
+
+static void its_unmask_msi_irq(struct irq_data *d)
+{
+ pci_msi_unmask_irq(d);
+ irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip its_msi_irq_chip = {
+ .name = "ITS-MSI",
+ .irq_unmask = its_unmask_msi_irq,
+ .irq_mask = its_mask_msi_irq,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_write_msi_msg = pci_msi_domain_write_msg,
+};
+
+struct its_pci_alias {
+ struct pci_dev *pdev;
+ u32 dev_id;
+ u32 count;
+};
+
+static int its_pci_msi_vec_count(struct pci_dev *pdev)
+{
+ int msi, msix;
+
+ msi = max(pci_msi_vec_count(pdev), 0);
+ msix = max(pci_msix_vec_count(pdev), 0);
+
+ return max(msi, msix);
+}
+
+static int its_get_pci_alias(struct pci_dev *pdev, u16 alias, void *data)
+{
+ struct its_pci_alias *dev_alias = data;
+
+ dev_alias->dev_id = alias;
+ if (pdev != dev_alias->pdev)
+ dev_alias->count += its_pci_msi_vec_count(dev_alias->pdev);
+
+ return 0;
+}
+
+static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *info)
+{
+ struct pci_dev *pdev;
+ struct its_pci_alias dev_alias;
+
+ if (!dev_is_pci(dev))
+ return -EINVAL;
+
+ pdev = to_pci_dev(dev);
+ dev_alias.pdev = pdev;
+ dev_alias.count = nvec;
+
+ pci_for_each_dma_alias(pdev, its_get_pci_alias, &dev_alias);
+
+ return its_msi_prepare(domain, dev_alias.dev_id, dev_alias.count, info);
+}
+
+static struct msi_domain_ops its_pci_msi_ops = {
+ .msi_prepare = its_pci_msi_prepare,
+};
+
+static struct msi_domain_info its_pci_msi_domain_info = {
+ .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+ MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
+ .ops = &its_pci_msi_ops,
+ .chip = &its_msi_irq_chip,
+};
+
+struct irq_domain *its_pci_msi_alloc_domain(struct device_node *np,
+ struct irq_domain *parent)
+{
+ return pci_msi_create_irq_domain(np, &its_pci_msi_domain_info, parent);
+}
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b7e155..7c01fe4 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -611,26 +611,6 @@ static struct irq_chip its_irq_chip = {
.irq_compose_msi_msg = its_irq_compose_msi_msg,
};

-static void its_mask_msi_irq(struct irq_data *d)
-{
- pci_msi_mask_irq(d);
- irq_chip_mask_parent(d);
-}
-
-static void its_unmask_msi_irq(struct irq_data *d)
-{
- pci_msi_unmask_irq(d);
- irq_chip_unmask_parent(d);
-}
-
-static struct irq_chip its_msi_irq_chip = {
- .name = "ITS-MSI",
- .irq_unmask = its_unmask_msi_irq,
- .irq_mask = its_mask_msi_irq,
- .irq_eoi = irq_chip_eoi_parent,
- .irq_write_msi_msg = pci_msi_domain_write_msg,
-};
-
/*
* How we allocate LPIs:
*
@@ -1173,85 +1153,34 @@ static int its_alloc_device_irq(struct its_device *dev, irq_hw_number_t *hwirq)
return 0;
}

-struct its_pci_alias {
- struct pci_dev *pdev;
- u32 dev_id;
- u32 count;
-};
-
-static int its_pci_msi_vec_count(struct pci_dev *pdev)
-{
- int msi, msix;
-
- msi = max(pci_msi_vec_count(pdev), 0);
- msix = max(pci_msix_vec_count(pdev), 0);
-
- return max(msi, msix);
-}
-
-static int its_get_pci_alias(struct pci_dev *pdev, u16 alias, void *data)
+int its_msi_prepare(struct irq_domain *domain, u32 dev_id,
+ int nvec, msi_alloc_info_t *info)
{
- struct its_pci_alias *dev_alias = data;
-
- dev_alias->dev_id = alias;
- if (pdev != dev_alias->pdev)
- dev_alias->count += its_pci_msi_vec_count(dev_alias->pdev);
-
- return 0;
-}
-
-static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
- int nvec, msi_alloc_info_t *info)
-{
- struct pci_dev *pdev;
struct its_node *its;
struct its_device *its_dev;
- struct its_pci_alias dev_alias;
-
- if (!dev_is_pci(dev))
- return -EINVAL;
-
- pdev = to_pci_dev(dev);
- dev_alias.pdev = pdev;
- dev_alias.count = nvec;

- pci_for_each_dma_alias(pdev, its_get_pci_alias, &dev_alias);
its = domain->parent->host_data;
-
- its_dev = its_find_device(its, dev_alias.dev_id);
+ its_dev = its_find_device(its, dev_id);
if (its_dev) {
/*
* We already have seen this ID, probably through
* another alias (PCI bridge of some sort). No need to
* create the device.
*/
- dev_dbg(dev, "Reusing ITT for devID %x\n", dev_alias.dev_id);
+ pr_debug("Reusing ITT for devID %x\n", dev_id);
goto out;
}

- its_dev = its_create_device(its, dev_alias.dev_id, dev_alias.count);
+ its_dev = its_create_device(its, dev_id, nvec);
if (!its_dev)
return -ENOMEM;

- dev_dbg(&pdev->dev, "ITT %d entries, %d bits\n",
- dev_alias.count, ilog2(dev_alias.count));
+ pr_debug("ITT %d entries, %d bits\n", nvec, ilog2(nvec));
out:
info->scratchpad[0].ptr = its_dev;
- info->scratchpad[1].ptr = dev;
return 0;
}

-static struct msi_domain_ops its_pci_msi_ops = {
- .msi_prepare = its_msi_prepare,
-};
-
-static struct msi_domain_info its_pci_msi_domain_info = {
- .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
- MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
- .ops = &its_pci_msi_ops,
- .chip = &its_msi_irq_chip,
-};
-
static int its_irq_gic_domain_alloc(struct irq_domain *domain,
unsigned int virq,
irq_hw_number_t hwirq)
@@ -1287,8 +1216,9 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,

irq_domain_set_hwirq_and_chip(domain, virq + i,
hwirq, &its_irq_chip, its_dev);
- dev_dbg(info->scratchpad[1].ptr, "ID:%d pID:%d vID:%d\n",
- (int)(hwirq - its_dev->lpi_base), (int)hwirq, virq + i);
+ pr_debug("ID:%d pID:%d vID:%d\n",
+ (int)(hwirq - its_dev->lpi_base),
+ (int)hwirq, virq + i);
}

return 0;
@@ -1485,9 +1415,8 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)

its->domain->parent = parent;

- its->msi_chip.domain = pci_msi_create_irq_domain(node,
- &its_pci_msi_domain_info,
- its->domain);
+ its->msi_chip.domain = its_pci_msi_alloc_domain(node,
+ its->domain);
if (!its->msi_chip.domain) {
err = -ENOMEM;
goto out_free_domains;
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index ffbc034..d6149ba 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -360,6 +360,7 @@
#ifndef __ASSEMBLY__

#include <linux/stringify.h>
+#include <asm/msi.h>

/*
* We need a value to serve as a irq-type for LPIs. Choose one that will
@@ -388,6 +389,11 @@ struct irq_domain;
int its_cpu_init(void);
int its_init(struct device_node *node, struct rdists *rdists,
struct irq_domain *domain);
+int its_msi_prepare(struct irq_domain *domain, u32 dev_id,
+ int nvec, msi_alloc_info_t *info);
+
+struct irq_domain *its_pci_msi_alloc_domain(struct device_node *node,
+ struct irq_domain *parent);

#endif

--
2.1.4

2015-07-15 12:19:33

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 12/19] irqchip: gicv3-its: Register irq domain with NEXUS token

Now that we can distinguish between multiple domains carrying the
same device_node, tag the raw ITS domain with DOMAIN_BUS_NEXUS.
This will allow MSI providers built on top of the raw ITS domain
to identify it.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 7c01fe4..3f95826 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1407,13 +1407,14 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);

if (of_property_read_bool(its->msi_chip.of_node, "msi-controller")) {
- its->domain = irq_domain_add_tree(NULL, &its_domain_ops, its);
+ its->domain = irq_domain_add_tree(node, &its_domain_ops, its);
if (!its->domain) {
err = -ENOMEM;
goto out_free_tables;
}

its->domain->parent = parent;
+ its->domain->bus_token = DOMAIN_BUS_NEXUS_MSI;

its->msi_chip.domain = its_pci_msi_alloc_domain(node,
its->domain);
--
2.1.4

2015-07-15 12:17:34

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 13/19] irqchip: gicv3-its: Get rid of struct msi_controller

The GICv3 ITS only uses the msi_controller structure as a way
to match the PHB with its MSI HW, and thus the msi_domain.
But now that we can directly associate an msi_domain with a device,
there is no use keeping this msi_controller around.

Just remove all traces of msi_controller from the driver.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 38 ++++++++++++++++----------------------
1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 3f95826..20fe0dc 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -54,13 +54,12 @@ struct its_collection {

/*
* The ITS structure - contains most of the infrastructure, with the
- * msi_controller, the command queue, the collections, and the list of
- * devices writing to it.
+ * top-level MSI domain, the command queue, the collections, and the
+ * list of devices writing to it.
*/
struct its_node {
raw_spinlock_t lock;
struct list_head entry;
- struct msi_controller msi_chip;
struct irq_domain *domain;
void __iomem *base;
unsigned long phys_base;
@@ -776,7 +775,7 @@ static void its_free_tables(struct its_node *its)
}
}

-static int its_alloc_tables(struct its_node *its)
+static int its_alloc_tables(const char *node_name, struct its_node *its)
{
int err;
int i;
@@ -819,7 +818,7 @@ static int its_alloc_tables(struct its_node *its)
if (order >= MAX_ORDER) {
order = MAX_ORDER - 1;
pr_warn("%s: Device Table too large, reduce its page order to %u\n",
- its->msi_chip.of_node->full_name, order);
+ node_name, order);
}
}

@@ -889,7 +888,7 @@ retry_baser:

if (val != tmp) {
pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n",
- its->msi_chip.of_node->full_name, i,
+ node_name, i,
(unsigned long) val, (unsigned long) tmp);
err = -ENXIO;
goto out_free;
@@ -1317,6 +1316,7 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
struct resource res;
struct its_node *its;
void __iomem *its_base;
+ struct irq_domain *inner_domain = NULL;
u32 val;
u64 baser, tmp;
int err;
@@ -1360,7 +1360,6 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
INIT_LIST_HEAD(&its->its_device_list);
its->base = its_base;
its->phys_base = res.start;
- its->msi_chip.of_node = node;
its->ite_size = ((readl_relaxed(its_base + GITS_TYPER) >> 4) & 0xf) + 1;

its->cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL);
@@ -1370,7 +1369,7 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
}
its->cmd_write = its->cmd_base;

- err = its_alloc_tables(its);
+ err = its_alloc_tables(node->full_name, its);
if (err)
goto out_free_cmd;

@@ -1406,26 +1405,21 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
writeq_relaxed(0, its->base + GITS_CWRITER);
writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);

- if (of_property_read_bool(its->msi_chip.of_node, "msi-controller")) {
- its->domain = irq_domain_add_tree(node, &its_domain_ops, its);
- if (!its->domain) {
+ if (of_property_read_bool(node, "msi-controller")) {
+ inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
+ if (!inner_domain) {
err = -ENOMEM;
goto out_free_tables;
}

- its->domain->parent = parent;
- its->domain->bus_token = DOMAIN_BUS_NEXUS_MSI;
+ inner_domain->parent = parent;
+ inner_domain->bus_token = DOMAIN_BUS_NEXUS_MSI;

- its->msi_chip.domain = its_pci_msi_alloc_domain(node,
- its->domain);
- if (!its->msi_chip.domain) {
+ its->domain = its_pci_msi_alloc_domain(node, inner_domain);
+ if (!its->domain) {
err = -ENOMEM;
goto out_free_domains;
}
-
- err = of_pci_msi_chip_add(&its->msi_chip);
- if (err)
- goto out_free_domains;
}

spin_lock(&its_lock);
@@ -1435,10 +1429,10 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
return 0;

out_free_domains:
- if (its->msi_chip.domain)
- irq_domain_remove(its->msi_chip.domain);
if (its->domain)
irq_domain_remove(its->domain);
+ if (inner_domain)
+ irq_domain_remove(inner_domain);
out_free_tables:
its_free_tables(its);
out_free_cmd:
--
2.1.4

2015-07-15 12:19:05

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 14/19] irqchip: gicv3-its: Make the PCI/MSI code standalone

We can now lookup the base ITS domain, making it possible to
initialize the PCI/MSI code independently from the main ITS
subsystem.

This allows us to remove all the previously add hooks.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v3-its-pci-msi.c | 47 ++++++++++++++++++++++++++----
drivers/irqchip/irq-gic-v3-its.c | 50 +++++++++++++++++++++-----------
include/linux/irqchip/arm-gic-v3.h | 5 ----
3 files changed, 74 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index 7614721..cf351c6 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -20,8 +20,6 @@
#include <linux/of_irq.h>
#include <linux/of_pci.h>

-#include <linux/irqchip/arm-gic-v3.h>
-
static void its_mask_msi_irq(struct irq_data *d)
{
pci_msi_mask_irq(d);
@@ -74,17 +72,24 @@ static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
{
struct pci_dev *pdev;
struct its_pci_alias dev_alias;
+ struct msi_domain_info *msi_info;

if (!dev_is_pci(dev))
return -EINVAL;

+ msi_info = msi_get_domain_info(domain->parent);
+
pdev = to_pci_dev(dev);
dev_alias.pdev = pdev;
dev_alias.count = nvec;

pci_for_each_dma_alias(pdev, its_get_pci_alias, &dev_alias);

- return its_msi_prepare(domain, dev_alias.dev_id, dev_alias.count, info);
+ /* ITS specific DeviceID, as the core ITS ignores dev. */
+ info->scratchpad[0].ul = dev_alias.dev_id;
+
+ return msi_info->ops->msi_prepare(domain->parent,
+ dev, dev_alias.count, info);
}

static struct msi_domain_ops its_pci_msi_ops = {
@@ -98,8 +103,38 @@ static struct msi_domain_info its_pci_msi_domain_info = {
.chip = &its_msi_irq_chip,
};

-struct irq_domain *its_pci_msi_alloc_domain(struct device_node *np,
- struct irq_domain *parent)
+static struct of_device_id its_device_id[] = {
+ { .compatible = "arm,gic-v3-its", },
+ {},
+};
+
+static int __init its_pci_msi_init(void)
{
- return pci_msi_create_irq_domain(np, &its_pci_msi_domain_info, parent);
+ struct device_node *np;
+ struct irq_domain *parent;
+
+ for (np = of_find_matching_node(NULL, its_device_id); np;
+ np = of_find_matching_node(np, its_device_id)) {
+ if (!of_property_read_bool(np, "msi-controller"))
+ continue;
+
+ parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
+ if (!parent || !msi_get_domain_info(parent)) {
+ pr_err("%s: unable to locate ITS domain\n",
+ np->full_name);
+ continue;
+ }
+
+ if (!pci_msi_create_irq_domain(np, &its_pci_msi_domain_info,
+ parent)) {
+ pr_err("%s: unable to create PCI domain\n",
+ np->full_name);
+ continue;
+ }
+
+ pr_info("PCI/MSI: %s domain created\n", np->full_name);
+ }
+
+ return 0;
}
+early_initcall(its_pci_msi_init);
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 20fe0dc..21b002f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -60,7 +60,6 @@ struct its_collection {
struct its_node {
raw_spinlock_t lock;
struct list_head entry;
- struct irq_domain *domain;
void __iomem *base;
unsigned long phys_base;
struct its_cmd_block *cmd_base;
@@ -1152,13 +1151,25 @@ static int its_alloc_device_irq(struct its_device *dev, irq_hw_number_t *hwirq)
return 0;
}

-int its_msi_prepare(struct irq_domain *domain, u32 dev_id,
- int nvec, msi_alloc_info_t *info)
+static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *info)
{
struct its_node *its;
struct its_device *its_dev;
+ struct msi_domain_info *msi_info;
+ u32 dev_id;
+
+ /*
+ * We ignore "dev" entierely, and rely on the dev_id that has
+ * been passed via the scratchpad. This limits this domain's
+ * usefulness to upper layers that definitely know that they
+ * are built on top of the ITS.
+ */
+ dev_id = info->scratchpad[0].ul;
+
+ msi_info = msi_get_domain_info(domain);
+ its = msi_info->data;

- its = domain->parent->host_data;
its_dev = its_find_device(its, dev_id);
if (its_dev) {
/*
@@ -1180,6 +1191,10 @@ out:
return 0;
}

+static struct msi_domain_ops its_msi_domain_ops = {
+ .msi_prepare = its_msi_prepare,
+};
+
static int its_irq_gic_domain_alloc(struct irq_domain *domain,
unsigned int virq,
irq_hw_number_t hwirq)
@@ -1316,7 +1331,7 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
struct resource res;
struct its_node *its;
void __iomem *its_base;
- struct irq_domain *inner_domain = NULL;
+ struct irq_domain *inner_domain;
u32 val;
u64 baser, tmp;
int err;
@@ -1406,20 +1421,26 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);

if (of_property_read_bool(node, "msi-controller")) {
+ struct msi_domain_info *info;
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info) {
+ err = -ENOMEM;
+ goto out_free_tables;
+ }
+
inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
if (!inner_domain) {
err = -ENOMEM;
+ kfree(info);
goto out_free_tables;
}

inner_domain->parent = parent;
- inner_domain->bus_token = DOMAIN_BUS_NEXUS_MSI;
-
- its->domain = its_pci_msi_alloc_domain(node, inner_domain);
- if (!its->domain) {
- err = -ENOMEM;
- goto out_free_domains;
- }
+ inner_domain->bus_token = DOMAIN_BUS_NEXUS;
+ info->ops = &its_msi_domain_ops;
+ info->data = its;
+ inner_domain->host_data = info;
}

spin_lock(&its_lock);
@@ -1428,11 +1449,6 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)

return 0;

-out_free_domains:
- if (its->domain)
- irq_domain_remove(its->domain);
- if (inner_domain)
- irq_domain_remove(inner_domain);
out_free_tables:
its_free_tables(its);
out_free_cmd:
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index d6149ba..bf982e0 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -389,11 +389,6 @@ struct irq_domain;
int its_cpu_init(void);
int its_init(struct device_node *node, struct rdists *rdists,
struct irq_domain *domain);
-int its_msi_prepare(struct irq_domain *domain, u32 dev_id,
- int nvec, msi_alloc_info_t *info);
-
-struct irq_domain *its_pci_msi_alloc_domain(struct device_node *node,
- struct irq_domain *parent);

#endif

--
2.1.4

2015-07-15 12:18:43

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 15/19] irqchip: gicv3-its: Add platform MSI support

In order to support non-PCI MSI with the GICv3 ITS, add the minimal
required entry points for the MSI domain (an msi_prepare implementation).

The rest is only boilerplate code to find the raw ITS domain.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-gic-v3-its-platform-msi.c | 93 +++++++++++++++++++++++++++
2 files changed, 94 insertions(+), 1 deletion(-)
create mode 100644 drivers/irqchip/irq-gic-v3-its-platform-msi.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 0d5f2a9..11d08c9 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -22,7 +22,7 @@ obj-$(CONFIG_ARCH_SPEAR3XX) += spear-shirq.o
obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o
obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o
obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o
-obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o
+obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
obj-$(CONFIG_ARM_NVIC) += irq-nvic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
obj-$(CONFIG_ATMEL_AIC_IRQ) += irq-atmel-aic-common.o irq-atmel-aic.o
diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
new file mode 100644
index 0000000..a865505
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2013-2015 ARM Limited, All Rights Reserved.
+ * Author: Marc Zyngier <[email protected]>
+ *
+ * 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.
+ *
+ * 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 for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/device.h>
+#include <linux/msi.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+
+static struct irq_chip its_pmsi_irq_chip = {
+ .name = "ITS-pMSI",
+};
+
+static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
+ int nvec, msi_alloc_info_t *info)
+{
+ struct msi_domain_info *msi_info;
+ u32 dev_id;
+ int ret;
+
+ msi_info = msi_get_domain_info(domain->parent);
+
+ /* Suck the DeviceID out of the msi-parent property */
+ ret = of_property_read_u32_index(dev->of_node, "msi-parent",
+ 1, &dev_id);
+ if (ret)
+ return ret;
+
+ /* ITS specific DeviceID, as the core ITS ignores dev. */
+ info->scratchpad[0].ul = dev_id;
+
+ return msi_info->ops->msi_prepare(domain->parent,
+ dev, nvec, info);
+}
+
+static struct msi_domain_ops its_pmsi_ops = {
+ .msi_prepare = its_pmsi_prepare,
+};
+
+static struct msi_domain_info its_pmsi_domain_info = {
+ .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
+ .ops = &its_pmsi_ops,
+ .chip = &its_pmsi_irq_chip,
+};
+
+static struct of_device_id its_device_id[] = {
+ { .compatible = "arm,gic-v3-its", },
+ {},
+};
+
+static int __init its_pmsi_init(void)
+{
+ struct device_node *np;
+ struct irq_domain *parent;
+
+ for (np = of_find_matching_node(NULL, its_device_id); np;
+ np = of_find_matching_node(np, its_device_id)) {
+ if (!of_property_read_bool(np, "msi-controller"))
+ continue;
+
+ parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
+ if (!parent || !msi_get_domain_info(parent)) {
+ pr_err("%s: unable to locate ITS domain\n",
+ np->full_name);
+ continue;
+ }
+
+ if (!platform_msi_create_irq_domain(np, &its_pmsi_domain_info,
+ parent)) {
+ pr_err("%s: unable to create platform domain\n",
+ np->full_name);
+ continue;
+ }
+
+ pr_info("Platform MSI: %s domain created\n", np->full_name);
+ }
+
+ return 0;
+}
+early_initcall(its_pmsi_init);
--
2.1.4

2015-07-15 12:17:38

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 16/19] irqchip: GICv2m: Get rid of struct msi_controller

GICv2m only uses the msi_controller structure as a way to match
the host bridge with its MSI HW, and thus the msi_domain.

But now that we can directly associate an msi_domain with a device,
there is no use keeping this msi_controller around.

Just remove all traces of msi_controller from the driver. Also
tag the inner (non-PCI) domain with DOMAIN_BUS_NEXUS.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v2m.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index fdf7065..ec9c376 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -45,7 +45,6 @@

struct v2m_data {
spinlock_t msi_cnt_lock;
- struct msi_controller mchip;
struct resource res; /* GICv2m resource */
void __iomem *base; /* GICv2m virt address */
u32 spi_start; /* The SPI number that MSIs start */
@@ -218,6 +217,7 @@ static int __init gicv2m_init_one(struct device_node *node,
{
int ret;
struct v2m_data *v2m;
+ struct irq_domain *inner_domain;

v2m = kzalloc(sizeof(struct v2m_data), GFP_KERNEL);
if (!v2m) {
@@ -261,19 +261,18 @@ static int __init gicv2m_init_one(struct device_node *node,
goto err_iounmap;
}

- v2m->domain = irq_domain_add_tree(NULL, &gicv2m_domain_ops, v2m);
- if (!v2m->domain) {
+ inner_domain = irq_domain_add_tree(node, &gicv2m_domain_ops, v2m);
+ if (!inner_domain) {
pr_err("Failed to create GICv2m domain\n");
ret = -ENOMEM;
goto err_free_bm;
}

- v2m->domain->parent = parent;
- v2m->mchip.of_node = node;
- v2m->mchip.domain = pci_msi_create_irq_domain(node,
- &gicv2m_msi_domain_info,
- v2m->domain);
- if (!v2m->mchip.domain) {
+ inner_domain->bus_token = DOMAIN_BUS_NEXUS;
+ inner_domain->parent = parent;
+ v2m->domain = pci_msi_create_irq_domain(node, &gicv2m_msi_domain_info,
+ inner_domain);
+ if (!v2m->domain) {
pr_err("Failed to create MSI domain\n");
ret = -ENOMEM;
goto err_free_domains;
@@ -281,12 +280,6 @@ static int __init gicv2m_init_one(struct device_node *node,

spin_lock_init(&v2m->msi_cnt_lock);

- ret = of_pci_msi_chip_add(&v2m->mchip);
- if (ret) {
- pr_err("Failed to add msi_chip.\n");
- goto err_free_domains;
- }
-
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));
@@ -294,10 +287,10 @@ static int __init gicv2m_init_one(struct device_node *node,
return 0;

err_free_domains:
- if (v2m->mchip.domain)
- irq_domain_remove(v2m->mchip.domain);
if (v2m->domain)
irq_domain_remove(v2m->domain);
+ if (inner_domain)
+ irq_domain_remove(inner_domain);
err_free_bm:
kfree(v2m->bm);
err_iounmap:
--
2.1.4

2015-07-15 12:18:19

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 17/19] irqchip: GICv2m: Add platform MSI support

In order to support non-PCI MSI with GICv2m, add the minimal
required entry points for the MSI domain, which is actually almost
nothing (we just use the defaults provided by the core code).

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v2m.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index ec9c376..db04fc1 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -50,7 +50,6 @@ struct v2m_data {
u32 spi_start; /* The SPI number that MSIs start */
u32 nr_spis; /* The number of SPIs for MSIs */
unsigned long *bm; /* MSI vector bitmap */
- struct irq_domain *domain;
};

static void gicv2m_mask_msi_irq(struct irq_data *d)
@@ -212,12 +211,25 @@ static bool is_msi_spi_valid(u32 base, u32 num)
return true;
}

+static struct irq_chip gicv2m_pmsi_irq_chip = {
+ .name = "pMSI",
+};
+
+static struct msi_domain_ops gicv2m_pmsi_ops = {
+};
+
+static struct msi_domain_info gicv2m_pmsi_domain_info = {
+ .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS),
+ .ops = &gicv2m_pmsi_ops,
+ .chip = &gicv2m_pmsi_irq_chip,
+};
+
static int __init gicv2m_init_one(struct device_node *node,
struct irq_domain *parent)
{
int ret;
struct v2m_data *v2m;
- struct irq_domain *inner_domain;
+ struct irq_domain *inner_domain, *pci_domain, *plat_domain;

v2m = kzalloc(sizeof(struct v2m_data), GFP_KERNEL);
if (!v2m) {
@@ -270,10 +282,13 @@ static int __init gicv2m_init_one(struct device_node *node,

inner_domain->bus_token = DOMAIN_BUS_NEXUS;
inner_domain->parent = parent;
- v2m->domain = pci_msi_create_irq_domain(node, &gicv2m_msi_domain_info,
- inner_domain);
- if (!v2m->domain) {
- pr_err("Failed to create MSI domain\n");
+ pci_domain = pci_msi_create_irq_domain(node, &gicv2m_msi_domain_info,
+ inner_domain);
+ plat_domain = platform_msi_create_irq_domain(node,
+ &gicv2m_pmsi_domain_info,
+ inner_domain);
+ if (!pci_domain || !plat_domain) {
+ pr_err("Failed to create MSI domains\n");
ret = -ENOMEM;
goto err_free_domains;
}
@@ -287,8 +302,10 @@ static int __init gicv2m_init_one(struct device_node *node,
return 0;

err_free_domains:
- if (v2m->domain)
- irq_domain_remove(v2m->domain);
+ if (plat_domain)
+ irq_domain_remove(plat_domain);
+ if (pci_domain)
+ irq_domain_remove(pci_domain);
if (inner_domain)
irq_domain_remove(inner_domain);
err_free_bm:
--
2.1.4

2015-07-15 12:17:44

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 18/19] PCI/MSI: pci-xgene-msi: Get rid of struct msi_controller

The XGene MSI driver only uses the msi_controller structure as
a way to match the host bridge with its MSI HW, and thus the
msi_domain.

But now that we can directly associate an msi_domain with a device,
there is no use keeping this msi_controller around.

Just remove all traces of msi_controller from the driver.

Tested-by: Duc Dang <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/pci/host/pci-xgene-msi.c | 41 ++++++++++++++++------------------------
1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
index 2d31d4d..ec5a14b 100644
--- a/drivers/pci/host/pci-xgene-msi.c
+++ b/drivers/pci/host/pci-xgene-msi.c
@@ -40,8 +40,8 @@ struct xgene_msi_group {

struct xgene_msi {
struct device_node *node;
- struct msi_controller mchip;
- struct irq_domain *domain;
+ struct irq_domain *inner_domain;
+ struct irq_domain *msi_domain;
u64 msi_addr;
void __iomem *msi_regs;
unsigned long *bitmap;
@@ -252,17 +252,17 @@ static const struct irq_domain_ops msi_domain_ops = {

static int xgene_allocate_domains(struct xgene_msi *msi)
{
- msi->domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
- &msi_domain_ops, msi);
- if (!msi->domain)
+ msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
+ &msi_domain_ops, msi);
+ if (!msi->inner_domain)
return -ENOMEM;

- msi->mchip.domain = pci_msi_create_irq_domain(msi->mchip.of_node,
- &xgene_msi_domain_info,
- msi->domain);
+ msi->msi_domain = pci_msi_create_irq_domain(msi->node,
+ &xgene_msi_domain_info,
+ msi->inner_domain);

- if (!msi->mchip.domain) {
- irq_domain_remove(msi->domain);
+ if (!msi->msi_domain) {
+ irq_domain_remove(msi->inner_domain);
return -ENOMEM;
}

@@ -271,10 +271,10 @@ static int xgene_allocate_domains(struct xgene_msi *msi)

static void xgene_free_domains(struct xgene_msi *msi)
{
- if (msi->mchip.domain)
- irq_domain_remove(msi->mchip.domain);
- if (msi->domain)
- irq_domain_remove(msi->domain);
+ if (msi->msi_domain)
+ irq_domain_remove(msi->msi_domain);
+ if (msi->inner_domain)
+ irq_domain_remove(msi->inner_domain);
}

static int xgene_msi_init_allocator(struct xgene_msi *xgene_msi)
@@ -340,7 +340,7 @@ static void xgene_msi_isr(unsigned int irq, struct irq_desc *desc)
* CPU0
*/
hw_irq = hwirq_to_canonical_hwirq(hw_irq);
- virq = irq_find_mapping(xgene_msi->domain, hw_irq);
+ virq = irq_find_mapping(xgene_msi->inner_domain, hw_irq);
WARN_ON(!virq);
if (virq != 0)
generic_handle_irq(virq);
@@ -497,7 +497,7 @@ static int xgene_msi_probe(struct platform_device *pdev)
goto error;
}
xgene_msi->msi_addr = res->start;
-
+ xgene_msi->node = pdev->dev.of_node;
xgene_msi->num_cpus = num_possible_cpus();

rc = xgene_msi_init_allocator(xgene_msi);
@@ -561,19 +561,10 @@ static int xgene_msi_probe(struct platform_device *pdev)

cpu_notifier_register_done();

- xgene_msi->mchip.of_node = pdev->dev.of_node;
- rc = of_pci_msi_chip_add(&xgene_msi->mchip);
- if (rc) {
- dev_err(&pdev->dev, "failed to add MSI controller chip\n");
- goto error_notifier;
- }
-
dev_info(&pdev->dev, "APM X-Gene PCIe MSI driver loaded\n");

return 0;

-error_notifier:
- unregister_hotcpu_notifier(&xgene_msi_cpu_notifier);
error:
xgene_msi_remove(pdev);
return rc;
--
2.1.4

2015-07-15 12:17:56

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH v4 19/19] PCI/MSI: Drop domain field from msi_controller

The only three users of that field are not using the msi_controller
structure anymore, so drop it altogether.

Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/pci/msi.c | 2 --
include/linux/msi.h | 3 ---
2 files changed, 5 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index c77fdaf..31bae50 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -42,8 +42,6 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev)
struct irq_domain *domain = NULL;

domain = dev_get_msi_domain(&dev->dev);
- if (!domain && dev->bus->msi)
- domain = dev->bus->msi->domain;
if (!domain)
domain = arch_get_pci_msi_domain(dev);

diff --git a/include/linux/msi.h b/include/linux/msi.h
index b55cf63..e29c31f 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -124,9 +124,6 @@ struct msi_controller {
struct device *dev;
struct device_node *of_node;
struct list_head list;
-#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
- struct irq_domain *domain;
-#endif

int (*setup_irq)(struct msi_controller *chip, struct pci_dev *dev,
struct msi_desc *desc);
--
2.1.4

2015-07-20 02:37:30

by majun (Euler7)

[permalink] [raw]
Subject: Re: [PATCH v4 09/19] drivers: base: Add MSI domain support for non-PCI devices



?? 2015/7/15 20:16, Marc Zyngier д??:
> With the msi_list and the msi_domain properties now being at the
> generic device level, it is starting to be relatively easy to offer
> a generic way of providing non-PCI MSIs.
>
[...]
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index c10ec56..b55cf63 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -18,6 +18,11 @@ struct pci_dev;
> void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
>
> +typedef void (*irq_write_msi_msg_t)(struct msi_desc *desc,
> + struct msi_msg *msg);
> +
> +struct platform_msi_priv_data;
> +
> struct msi_desc {
> struct list_head list;
> unsigned int irq;
> @@ -42,6 +47,10 @@ struct msi_desc {
> void __iomem *mask_base;
> };
> };
> + struct {
> + struct platform_msi_priv_data *platform_msi_priv_data;
> + u16 msi_index;
> + };
> };
> };
>
When I add this patch in linux 4.2.rc2, this part is rejected. So I added this part
myself. But there is compiling errors.

drivers/base/platform-msi.c: In function 'platform_msi_free_descs':
drivers/base/platform-msi.c:129:2: error: implicit declaration of function 'to_pci_dev' [-Werror=implicit-function-declaration]
list_for_each_entry_safe(desc, tmp, dev_to_msi_list(dev), list) {
^
In file included from include/linux/list.h:8:0,
from include/linux/kobject.h:20,
from include/linux/device.h:17,
from drivers/base/platform-msi.c:20:
include/linux/msi.h:57:50: error: invalid type argument of '->' (have 'int')
#define dev_to_msi_list(dev) (&to_pci_dev((dev))->msi_list)
^
include/linux/kernel.h:810:49: note: in definition of macro 'container_of'
const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^

> @@ -228,6 +237,12 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
> void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev);
> struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain);
>
> +struct irq_domain *platform_msi_create_irq_domain(struct device_node *np,
> + struct msi_domain_info *info,
> + struct irq_domain *parent);
> +int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
> + irq_write_msi_msg_t write_msi_msg);
> +void platform_msi_domain_free_irqs(struct device *dev);
> #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
>
> #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
>

2015-07-20 07:54:35

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v4 09/19] drivers: base: Add MSI domain support for non-PCI devices

On 20/07/15 03:33, majun (F) wrote:
>
>
> ?? 2015/7/15 20:16, Marc Zyngier д??:
>> With the msi_list and the msi_domain properties now being at the
>> generic device level, it is starting to be relatively easy to offer
>> a generic way of providing non-PCI MSIs.
>>
> [...]
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index c10ec56..b55cf63 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -18,6 +18,11 @@ struct pci_dev;
>> void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
>> void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
>>
>> +typedef void (*irq_write_msi_msg_t)(struct msi_desc *desc,
>> + struct msi_msg *msg);
>> +
>> +struct platform_msi_priv_data;
>> +
>> struct msi_desc {
>> struct list_head list;
>> unsigned int irq;
>> @@ -42,6 +47,10 @@ struct msi_desc {
>> void __iomem *mask_base;
>> };
>> };
>> + struct {
>> + struct platform_msi_priv_data *platform_msi_priv_data;
>> + u16 msi_index;
>> + };
>> };
>> };
>>
> When I add this patch in linux 4.2.rc2, this part is rejected. So I added this part
> myself. But there is compiling errors.

[...]

Did you read this crucial part of the cover letter:

<quote>
[...]
Patches are on top of 4.2-rc2 and Jiang Liu's series, and available at:

git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
irq/npci-msi-v2
</quote>

Without Jiang Liu's series as a prerequisite, it is not surprising
nothing applies, let alone compile...

M.
--
Jazz is not dead. It just smells funny...

2015-07-21 21:17:39

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 07/19] PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain

Hi Marc,

On Wed, Jul 15, 2015 at 01:16:41PM +0100, Marc Zyngier wrote:
> Now that we can easily find which MSI domain a PCI device is
> using, use dev_get_msi_domain as a way to retrieve the information.
>
> The original code is still used as a fallback.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> drivers/pci/msi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index ef4ec6e..c77fdaf 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -41,7 +41,8 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev)
> {
> struct irq_domain *domain = NULL;
>
> - if (dev->bus->msi)
> + domain = dev_get_msi_domain(&dev->dev);
> + if (!domain && dev->bus->msi)
> domain = dev->bus->msi->domain;
> if (!domain)
> domain = arch_get_pci_msi_domain(dev);

I think this would be slightly easier to read as:

struct irq_domain *domain;

domain = dev_get_msi_domain(&dev->dev);
if (domain)
return domain;

if (dev->bus->msi && (domain = dev->bus->msi->domain))
return domain;

return arch_get_pci_msi_domain(dev);

I'm not a huge fan of assignments inside "if" conditions, and checkpatch
might even complain about it, but it exposes the fallback order pretty well
here. I guess we could also just repeat the dev->bus->msi->domain
expression:

if (dev->bus->msi && dev->bus->msi->domain)
return dev->bus->msi->domain;

We can at least get rid of the superfluous initialization of domain to
NULL.

Bjorn

2015-07-21 21:19:55

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

On Wed, Jul 15, 2015 at 01:16:38PM +0100, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the host bridge msi_domain

s/necesary/necessary/

> across secondary busses to devices.
>
> So far, nobody populates the initial msi_domain.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
> include/linux/pci.h | 1 +
> 2 files changed, 31 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index cefd636..376f6fa 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
> }
> }
>
> +void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
> +{
> +}
> +
> +static void pci_set_bus_msi_domain(struct pci_bus *bus)
> +{
> + struct pci_dev *bridge = bus->self;
> +
> + if (!bridge)
> + pcibios_set_host_bridge_msi_domain(bus);

I would use pci_is_root_bus() here instead of testing "!bridge".

> + else
> + dev_set_msi_domain(&bus->dev, dev_get_msi_domain(&bridge->dev));
> +}
> +
> static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> struct pci_dev *bridge, int busnr)
> {
> @@ -714,6 +728,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
> bridge->subordinate = child;
>
> add_dev:
> + pci_set_bus_msi_domain(child);
> ret = device_register(&child->dev);
> WARN_ON(ret < 0);
>
> @@ -1544,6 +1559,17 @@ static void pci_init_capabilities(struct pci_dev *dev)
> pci_enable_acs(dev);
> }
>
> +static void pci_set_msi_domain(struct pci_dev *dev)
> +{
> + /*
> + * If no domain has been set through the pcibios callback,
> + * inherit the default from the bus device.
> + */
> + if (!dev_get_msi_domain(&dev->dev))
> + dev_set_msi_domain(&dev->dev,
> + dev_get_msi_domain(&dev->bus->dev));
> +}
> +
> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> {
> int ret;
> @@ -1585,6 +1611,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> ret = pcibios_add_device(dev);
> WARN_ON(ret < 0);
>
> + /* Setup MSI irq domain */
> + pci_set_msi_domain(dev);
> +
> /* Notifier could use PCI capabilities */
> dev->match_driver = false;
> ret = device_add(&dev->dev);
> @@ -1975,6 +2004,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> b->bridge = get_device(&bridge->dev);
> device_enable_async_suspend(b->bridge);
> pci_set_bus_of_node(b);
> + pci_set_bus_msi_domain(b);
>
> if (!parent)
> set_dev_node(b->bridge, pcibus_to_node(b));
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fbf245f..fe0825f 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1644,6 +1644,7 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev,
> int pcibios_add_device(struct pci_dev *dev);
> void pcibios_release_device(struct pci_dev *dev);
> void pcibios_penalize_isa_irq(int irq, int active);
> +void pcibios_set_host_bridge_msi_domain(struct pci_bus *bus);
>
> #ifdef CONFIG_HIBERNATE_CALLBACKS
> extern struct dev_pm_ops pcibios_pm_ops;
> --
> 2.1.4
>

2015-07-21 21:26:51

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

On Wed, Jul 15, 2015 at 01:16:38PM +0100, Marc Zyngier wrote:
> In order to be able to populate the device msi_domain field,
> add the necesary hooks to propagate the host bridge msi_domain
> across secondary busses to devices.
>
> So far, nobody populates the initial msi_domain.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
> include/linux/pci.h | 1 +
> 2 files changed, 31 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index cefd636..376f6fa 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
> }
> }
>
> +void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
> +{
> +}

I don't think there's anything in this series that requires this to be a
weak function, is there? This is the only definition I see.

2015-07-21 21:30:39

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 05/19] PCI/MSI: of: Add support for OF-provided msi_domain

On Wed, Jul 15, 2015 at 01:16:39PM +0100, Marc Zyngier wrote:
> In order to populate the PHB msi_domain, use the "msi-parent"
> attribute to lookup a corresponding irq domain. If found,
> this is our MSI domain.
>
> This gets plugged into the core PCI code.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> drivers/pci/of.c | 19 +++++++++++++++++++
> drivers/pci/probe.c | 1 +
> include/linux/pci.h | 2 ++
> 3 files changed, 22 insertions(+)
>
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index f092993..09df465 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -9,6 +9,7 @@
> * 2 of the License, or (at your option) any later version.
> */
>
> +#include <linux/irqdomain.h>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> #include <linux/of.h>
> @@ -59,3 +60,21 @@ struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
> return of_node_get(bus->bridge->parent->of_node);
> return NULL;
> }
> +
> +void pci_set_phb_of_msi_domain(struct pci_bus *bus)
> +{
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> + struct device_node *np;
> + struct irq_domain *d;
> +
> + if (!bus->dev.of_node)
> + return;
> + np = of_parse_phandle(bus->dev.of_node, "msi-parent", 0);
> + if (!np)
> + return;
> + d = irq_find_matching_host(np, DOMAIN_BUS_PCI_MSI);
> + if (!d)
> + d = irq_find_host(np);
> + dev_set_msi_domain(&bus->dev, d);
> +#endif
> +}
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 376f6fa..c80626f 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -663,6 +663,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>
> void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
> {
> + pci_set_phb_of_msi_domain(bus);

The code itself is OK with me, but "phb" is an abbreviation that we haven't
really been using in the PCI core, so I'd prefer something else. Maybe
"pci_set_host_of_msi_domain()"? Or even (for symmetry with the pcibios
function), "pci_set_host_bridge_of_msi_domain()", although that is getting
pretty wordy?

2015-07-21 21:37:12

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 19/19] PCI/MSI: Drop domain field from msi_controller

On Wed, Jul 15, 2015 at 01:16:53PM +0100, Marc Zyngier wrote:
> The only three users of that field are not using the msi_controller
> structure anymore, so drop it altogether.
>
> Signed-off-by: Marc Zyngier <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> drivers/pci/msi.c | 2 --
> include/linux/msi.h | 3 ---
> 2 files changed, 5 deletions(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index c77fdaf..31bae50 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -42,8 +42,6 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev)
> struct irq_domain *domain = NULL;
>
> domain = dev_get_msi_domain(&dev->dev);
> - if (!domain && dev->bus->msi)
> - domain = dev->bus->msi->domain;
> if (!domain)
> domain = arch_get_pci_msi_domain(dev);

I guess this takes care of the "assignment inside if" question :)

> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index b55cf63..e29c31f 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -124,9 +124,6 @@ struct msi_controller {
> struct device *dev;
> struct device_node *of_node;
> struct list_head list;
> -#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> - struct irq_domain *domain;
> -#endif
>
> int (*setup_irq)(struct msi_controller *chip, struct pci_dev *dev,
> struct msi_desc *desc);
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-07-22 14:43:10

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

Hi Bjorn,

On 21/07/15 22:26, Bjorn Helgaas wrote:
> On Wed, Jul 15, 2015 at 01:16:38PM +0100, Marc Zyngier wrote:
>> In order to be able to populate the device msi_domain field,
>> add the necesary hooks to propagate the host bridge msi_domain
>> across secondary busses to devices.
>>
>> So far, nobody populates the initial msi_domain.
>>
>> Signed-off-by: Marc Zyngier <[email protected]>
>> ---
>> drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
>> include/linux/pci.h | 1 +
>> 2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index cefd636..376f6fa 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>> }
>> }
>>
>> +void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
>> +{
>> +}
>
> I don't think there's anything in this series that requires this to be a
> weak function, is there? This is the only definition I see.

It looks like all the pcibios_* functions so far have a weak attribute,
and I've added it as a matter of consistency.

I don't mind dropping it though.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2015-07-22 14:48:28

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

On Wed, Jul 22, 2015 at 9:42 AM, Marc Zyngier <[email protected]> wrote:
> Hi Bjorn,
>
> On 21/07/15 22:26, Bjorn Helgaas wrote:
>> On Wed, Jul 15, 2015 at 01:16:38PM +0100, Marc Zyngier wrote:
>>> In order to be able to populate the device msi_domain field,
>>> add the necesary hooks to propagate the host bridge msi_domain
>>> across secondary busses to devices.
>>>
>>> So far, nobody populates the initial msi_domain.
>>>
>>> Signed-off-by: Marc Zyngier <[email protected]>
>>> ---
>>> drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
>>> include/linux/pci.h | 1 +
>>> 2 files changed, 31 insertions(+)
>>>
>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>> index cefd636..376f6fa 100644
>>> --- a/drivers/pci/probe.c
>>> +++ b/drivers/pci/probe.c
>>> @@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>>> }
>>> }
>>>
>>> +void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
>>> +{
>>> +}
>>
>> I don't think there's anything in this series that requires this to be a
>> weak function, is there? This is the only definition I see.
>
> It looks like all the pcibios_* functions so far have a weak attribute,
> and I've added it as a matter of consistency.

We've used pcibios_* names where we might need an arch-specific
implementation. I'm not sure that's the case here -- do you envision
an implementation under arch/* someday? If not, maybe it should just
be a pci_* function instead of pcibios_*.

Bjorn

2015-07-22 14:54:20

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

On 22/07/15 15:48, Bjorn Helgaas wrote:
> On Wed, Jul 22, 2015 at 9:42 AM, Marc Zyngier <[email protected]> wrote:
>> Hi Bjorn,
>>
>> On 21/07/15 22:26, Bjorn Helgaas wrote:
>>> On Wed, Jul 15, 2015 at 01:16:38PM +0100, Marc Zyngier wrote:
>>>> In order to be able to populate the device msi_domain field,
>>>> add the necesary hooks to propagate the host bridge msi_domain
>>>> across secondary busses to devices.
>>>>
>>>> So far, nobody populates the initial msi_domain.
>>>>
>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>> ---
>>>> drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
>>>> include/linux/pci.h | 1 +
>>>> 2 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>>> index cefd636..376f6fa 100644
>>>> --- a/drivers/pci/probe.c
>>>> +++ b/drivers/pci/probe.c
>>>> @@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>>>> }
>>>> }
>>>>
>>>> +void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
>>>> +{
>>>> +}
>>>
>>> I don't think there's anything in this series that requires this to be a
>>> weak function, is there? This is the only definition I see.
>>
>> It looks like all the pcibios_* functions so far have a weak attribute,
>> and I've added it as a matter of consistency.
>
> We've used pcibios_* names where we might need an arch-specific
> implementation. I'm not sure that's the case here -- do you envision
> an implementation under arch/* someday? If not, maybe it should just
> be a pci_* function instead of pcibios_*.

I could definitely see non-OF driven architectures wanting to override this.

Or maybe we should turn it the other way around and make it call the
various firmware interfaces (OF, ACPI...) until one of them succeeds. In
which case your suggestion of making it a pci_* function makes a lot of
sense.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2015-07-22 15:19:52

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v4 07/19] PCI/MSI: Let pci_msi_get_domain use struct device's msi_domain

On 21/07/15 22:17, Bjorn Helgaas wrote:
> Hi Marc,
>
> On Wed, Jul 15, 2015 at 01:16:41PM +0100, Marc Zyngier wrote:
>> Now that we can easily find which MSI domain a PCI device is
>> using, use dev_get_msi_domain as a way to retrieve the information.
>>
>> The original code is still used as a fallback.
>>
>> Signed-off-by: Marc Zyngier <[email protected]>
>> ---
>> drivers/pci/msi.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>> index ef4ec6e..c77fdaf 100644
>> --- a/drivers/pci/msi.c
>> +++ b/drivers/pci/msi.c
>> @@ -41,7 +41,8 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev)
>> {
>> struct irq_domain *domain = NULL;
>>
>> - if (dev->bus->msi)
>> + domain = dev_get_msi_domain(&dev->dev);
>> + if (!domain && dev->bus->msi)
>> domain = dev->bus->msi->domain;
>> if (!domain)
>> domain = arch_get_pci_msi_domain(dev);
>
> I think this would be slightly easier to read as:
>
> struct irq_domain *domain;
>
> domain = dev_get_msi_domain(&dev->dev);
> if (domain)
> return domain;
>
> if (dev->bus->msi && (domain = dev->bus->msi->domain))
> return domain;
>
> return arch_get_pci_msi_domain(dev);
>
> I'm not a huge fan of assignments inside "if" conditions, and checkpatch
> might even complain about it, but it exposes the fallback order pretty well
> here. I guess we could also just repeat the dev->bus->msi->domain
> expression:
>
> if (dev->bus->msi && dev->bus->msi->domain)
> return dev->bus->msi->domain;
>
> We can at least get rid of the superfluous initialization of domain to
> NULL.

Yeah, that's better, considering that (as you've noticed) the ugly
assignment is removed in the last patch.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2015-07-22 16:53:25

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

On Wed, Jul 22, 2015 at 03:54:14PM +0100, Marc Zyngier wrote:
> On 22/07/15 15:48, Bjorn Helgaas wrote:
> > On Wed, Jul 22, 2015 at 9:42 AM, Marc Zyngier <[email protected]> wrote:
> >> Hi Bjorn,
> >>
> >> On 21/07/15 22:26, Bjorn Helgaas wrote:
> >>> On Wed, Jul 15, 2015 at 01:16:38PM +0100, Marc Zyngier wrote:
> >>>> In order to be able to populate the device msi_domain field,
> >>>> add the necesary hooks to propagate the host bridge msi_domain
> >>>> across secondary busses to devices.
> >>>>
> >>>> So far, nobody populates the initial msi_domain.
> >>>>
> >>>> Signed-off-by: Marc Zyngier <[email protected]>
> >>>> ---
> >>>> drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
> >>>> include/linux/pci.h | 1 +
> >>>> 2 files changed, 31 insertions(+)
> >>>>
> >>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> >>>> index cefd636..376f6fa 100644
> >>>> --- a/drivers/pci/probe.c
> >>>> +++ b/drivers/pci/probe.c
> >>>> @@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
> >>>> }
> >>>> }
> >>>>
> >>>> +void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
> >>>> +{
> >>>> +}
> >>>
> >>> I don't think there's anything in this series that requires this to be a
> >>> weak function, is there? This is the only definition I see.
> >>
> >> It looks like all the pcibios_* functions so far have a weak attribute,
> >> and I've added it as a matter of consistency.
> >
> > We've used pcibios_* names where we might need an arch-specific
> > implementation. I'm not sure that's the case here -- do you envision
> > an implementation under arch/* someday? If not, maybe it should just
> > be a pci_* function instead of pcibios_*.
>
> I could definitely see non-OF driven architectures wanting to override this.
>
> Or maybe we should turn it the other way around and make it call the
> various firmware interfaces (OF, ACPI...) until one of them succeeds. In
> which case your suggestion of making it a pci_* function makes a lot of
> sense.

Here's my advice, FWIW:

- remove __weak

- rename pcibios_set_host_bridge_msi_domain() to
pci_host_bridge_msi_domain() and have it return a struct irq_domain *

- pci_host_bridge_msi_domain() can call whatever firmware- or
arch-specific code you need to look up the irq_domain

- move the dev_set_msi_domain() call from pci_set_phb_of_msi_domain() to
pci_set_bus_msi_domain()

2015-07-22 17:42:18

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v4 04/19] PCI/MSI: Add hooks to populate the msi_domain field

On 22/07/15 17:53, Bjorn Helgaas wrote:
> On Wed, Jul 22, 2015 at 03:54:14PM +0100, Marc Zyngier wrote:
>> On 22/07/15 15:48, Bjorn Helgaas wrote:
>>> On Wed, Jul 22, 2015 at 9:42 AM, Marc Zyngier <[email protected]> wrote:
>>>> Hi Bjorn,
>>>>
>>>> On 21/07/15 22:26, Bjorn Helgaas wrote:
>>>>> On Wed, Jul 15, 2015 at 01:16:38PM +0100, Marc Zyngier wrote:
>>>>>> In order to be able to populate the device msi_domain field,
>>>>>> add the necesary hooks to propagate the host bridge msi_domain
>>>>>> across secondary busses to devices.
>>>>>>
>>>>>> So far, nobody populates the initial msi_domain.
>>>>>>
>>>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>>>> ---
>>>>>> drivers/pci/probe.c | 30 ++++++++++++++++++++++++++++++
>>>>>> include/linux/pci.h | 1 +
>>>>>> 2 files changed, 31 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>>>>> index cefd636..376f6fa 100644
>>>>>> --- a/drivers/pci/probe.c
>>>>>> +++ b/drivers/pci/probe.c
>>>>>> @@ -661,6 +661,20 @@ static void pci_set_bus_speed(struct pci_bus *bus)
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> +void __weak pcibios_set_host_bridge_msi_domain(struct pci_bus *bus)
>>>>>> +{
>>>>>> +}
>>>>>
>>>>> I don't think there's anything in this series that requires this to be a
>>>>> weak function, is there? This is the only definition I see.
>>>>
>>>> It looks like all the pcibios_* functions so far have a weak attribute,
>>>> and I've added it as a matter of consistency.
>>>
>>> We've used pcibios_* names where we might need an arch-specific
>>> implementation. I'm not sure that's the case here -- do you envision
>>> an implementation under arch/* someday? If not, maybe it should just
>>> be a pci_* function instead of pcibios_*.
>>
>> I could definitely see non-OF driven architectures wanting to override this.
>>
>> Or maybe we should turn it the other way around and make it call the
>> various firmware interfaces (OF, ACPI...) until one of them succeeds. In
>> which case your suggestion of making it a pci_* function makes a lot of
>> sense.
>
> Here's my advice, FWIW:
>
> - remove __weak
>
> - rename pcibios_set_host_bridge_msi_domain() to
> pci_host_bridge_msi_domain() and have it return a struct irq_domain *
>
> - pci_host_bridge_msi_domain() can call whatever firmware- or
> arch-specific code you need to look up the irq_domain
>
> - move the dev_set_msi_domain() call from pci_set_phb_of_msi_domain() to
> pci_set_bus_msi_domain()
>

Yes, seems to make sense. I end-up with something that looks much more
readable (pci_set_bus_msi_domain looks fairly neat now).

I'll post a new version shortly.

Thanks,

M.
--
Jazz is not dead. It just smells funny...