2014-07-09 23:05:14

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

From: Suravee Suthikulpanit <[email protected]>

This patch set introduces support for MSI(-X) in GICv2m specification,
which is implemented in some variation of GIC400.

This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
(https://lkml.org/lkml/2014/3/14/320).

Changes in V3:
* Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
(per Jason Cooper request)
* Misc fix/clean up per Mark Rutland comments
* Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
* Patch 4 is new to the series:
* Add ARM64-specific version arch_setup_msi_irqs() to allow support
for Multiple MSI.
* Add support for Multiple MSI for GICv2m.

Suravee Suthikulpanit (4):
irqchip: gic: Add binding probe for ARM GIC400
irqchip: gic: Restructuring ARM GIC code
irqchip: gic: Add supports for ARM GICv2m MSI(-X)
irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m

Documentation/devicetree/bindings/arm/gic.txt | 20 +-
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/msi.h | 15 ++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/msi.c | 57 +++++
drivers/irqchip/Kconfig | 7 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-gic-v2m.c | 331 ++++++++++++++++++++++++++
drivers/irqchip/irq-gic-v2m.h | 13 +
drivers/irqchip/irq-gic.c | 90 ++++---
drivers/irqchip/irq-gic.h | 67 ++++++
11 files changed, 562 insertions(+), 41 deletions(-)
create mode 100644 arch/arm64/include/asm/msi.h
create mode 100644 arch/arm64/kernel/msi.c
create mode 100644 drivers/irqchip/irq-gic-v2m.c
create mode 100644 drivers/irqchip/irq-gic-v2m.h
create mode 100644 drivers/irqchip/irq-gic.h

--
1.9.0


2014-07-09 23:05:20

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 1/4 V3] irqchip: gic: Add binding probe for ARM GIC400

From: Suravee Suthikulpanit <[email protected]>

Add new Irqchip declaration for GIC400. This was mentioned in
gic binding documentation, but there is not code to support it.

Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/irqchip/irq-gic.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 508b815..ac8f7ea 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1020,6 +1020,8 @@ gic_of_init(struct device_node *node, struct device_node *parent)
gic_cnt++;
return 0;
}
+
+IRQCHIP_DECLARE(arm_gic_400, "arm,gic-400", gic_of_init);
IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
--
1.9.0

2014-07-09 23:05:34

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 2/4 V3] irqchip: gic: Restructuring ARM GIC code

From: Suravee Suthikulpanit <[email protected]>

This patch restructures the code to prepare for future MSI support.
It moves the declaration of structures and functions into the header file,
and omit the static prefix.

Since we are planing to have different irq_chip for GICv2m, the patch adds
irq_chip pointer in the gic_chip_data which is initialized during probing phase.

This should not have any functional changes.

Cc: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/irqchip/irq-gic.c | 65 +++++++++++++++++++++--------------------------
drivers/irqchip/irq-gic.h | 52 +++++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+), 36 deletions(-)
create mode 100644 drivers/irqchip/irq-gic.h

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index ac8f7ea..966e1d5 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1,5 +1,5 @@
/*
- * linux/arch/arm/common/gic.c
+ * driver/irqchip/irq-gic.c
*
* Copyright (C) 2002 ARM Limited, All Rights Reserved.
*
@@ -47,30 +47,9 @@
#include <asm/smp_plat.h>

#include "irq-gic-common.h"
+#include "irq-gic.h"
#include "irqchip.h"

-union gic_base {
- void __iomem *common_base;
- void __percpu * __iomem *percpu_base;
-};
-
-struct gic_chip_data {
- union gic_base dist_base;
- union gic_base cpu_base;
-#ifdef CONFIG_CPU_PM
- u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
- u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
- u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
- u32 __percpu *saved_ppi_enable;
- u32 __percpu *saved_ppi_conf;
-#endif
- struct irq_domain *domain;
- unsigned int gic_irqs;
-#ifdef CONFIG_GIC_NON_BANKED
- void __iomem *(*get_base)(union gic_base *);
-#endif
-};
-
static DEFINE_RAW_SPINLOCK(irq_controller_lock);

/*
@@ -152,7 +131,7 @@ static inline unsigned int gic_irq(struct irq_data *d)
/*
* Routines to acknowledge, disable and enable interrupts
*/
-static void gic_mask_irq(struct irq_data *d)
+void gic_mask_irq(struct irq_data *d)
{
u32 mask = 1 << (gic_irq(d) % 32);

@@ -163,7 +142,7 @@ static void gic_mask_irq(struct irq_data *d)
raw_spin_unlock(&irq_controller_lock);
}

-static void gic_unmask_irq(struct irq_data *d)
+void gic_unmask_irq(struct irq_data *d)
{
u32 mask = 1 << (gic_irq(d) % 32);

@@ -174,7 +153,7 @@ static void gic_unmask_irq(struct irq_data *d)
raw_spin_unlock(&irq_controller_lock);
}

-static void gic_eoi_irq(struct irq_data *d)
+void gic_eoi_irq(struct irq_data *d)
{
if (gic_arch_extn.irq_eoi) {
raw_spin_lock(&irq_controller_lock);
@@ -185,7 +164,7 @@ static void gic_eoi_irq(struct irq_data *d)
writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
}

-static int gic_set_type(struct irq_data *d, unsigned int type)
+int gic_set_type(struct irq_data *d, unsigned int type)
{
void __iomem *base = gic_dist_base(d);
unsigned int gicirq = gic_irq(d);
@@ -209,7 +188,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
return 0;
}

-static int gic_retrigger(struct irq_data *d)
+int gic_retrigger(struct irq_data *d)
{
if (gic_arch_extn.irq_retrigger)
return gic_arch_extn.irq_retrigger(d);
@@ -219,8 +198,8 @@ static int gic_retrigger(struct irq_data *d)
}

#ifdef CONFIG_SMP
-static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
- bool force)
+int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
+ bool force)
{
void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
unsigned int cpu, shift = (gic_irq(d) % 4) * 8;
@@ -246,7 +225,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
#endif

#ifdef CONFIG_PM
-static int gic_set_wake(struct irq_data *d, unsigned int on)
+int gic_set_wake(struct irq_data *d, unsigned int on)
{
int ret = -ENXIO;

@@ -768,19 +747,21 @@ void __init gic_init_physaddr(struct device_node *node)
static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hw)
{
+ struct gic_chip_data *gic = d->host_data;
+
if (hw < 32) {
irq_set_percpu_devid(irq);
- irq_set_chip_and_handler(irq, &gic_chip,
+ irq_set_chip_and_handler(irq, gic->irq_chip,
handle_percpu_devid_irq);
set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
} else {
- irq_set_chip_and_handler(irq, &gic_chip,
+ irq_set_chip_and_handler(irq, gic->irq_chip,
handle_fasteoi_irq);
set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);

gic_routable_irq_domain_ops->map(d, irq, hw);
}
- irq_set_chip_data(irq, d->host_data);
+ irq_set_chip_data(irq, gic);
return 0;
}

@@ -989,8 +970,9 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
#ifdef CONFIG_OF
static int gic_cnt __initdata;

-static int __init
-gic_of_init(struct device_node *node, struct device_node *parent)
+int __init
+_gic_of_init(struct device_node *node, struct device_node *parent,
+ struct irq_chip *chip, struct gic_chip_data **gic)
{
void __iomem *cpu_base;
void __iomem *dist_base;
@@ -1009,6 +991,8 @@ gic_of_init(struct device_node *node, struct device_node *parent)
if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
percpu_offset = 0;

+ gic_data[gic_cnt].irq_chip = chip;
+
gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
if (!gic_cnt)
gic_init_physaddr(node);
@@ -1017,10 +1001,19 @@ gic_of_init(struct device_node *node, struct device_node *parent)
irq = irq_of_parse_and_map(node, 0);
gic_cascade_irq(gic_cnt, irq);
}
+
+ if (gic)
+ *gic = &gic_data[gic_cnt];
gic_cnt++;
return 0;
}

+static int __init
+gic_of_init(struct device_node *node, struct device_node *parent)
+{
+ return _gic_of_init(node, parent, &gic_chip, NULL);
+}
+
IRQCHIP_DECLARE(arm_gic_400, "arm,gic-400", gic_of_init);
IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
new file mode 100644
index 0000000..a4beb4a
--- /dev/null
+++ b/drivers/irqchip/irq-gic.h
@@ -0,0 +1,52 @@
+#ifndef _IRQ_GIC_H_
+#define _IRQ_GIC_H_
+
+#include <linux/msi.h>
+
+union gic_base {
+ void __iomem *common_base;
+ void __percpu * __iomem *percpu_base;
+};
+
+struct gic_chip_data {
+ union gic_base dist_base;
+ union gic_base cpu_base;
+#ifdef CONFIG_CPU_PM
+ u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
+ u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
+ u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
+ u32 __percpu *saved_ppi_enable;
+ u32 __percpu *saved_ppi_conf;
+#endif
+ struct irq_domain *domain;
+ unsigned int gic_irqs;
+ struct irq_chip *irq_chip;
+#ifdef CONFIG_GIC_NON_BANKED
+ void __iomem *(*get_base)(union gic_base *);
+#endif
+};
+
+void gic_mask_irq(struct irq_data *d);
+void gic_unmask_irq(struct irq_data *d);
+void gic_eoi_irq(struct irq_data *d);
+int gic_set_type(struct irq_data *d, unsigned int type);
+int gic_retrigger(struct irq_data *d);
+
+#ifdef CONFIG_SMP
+int gic_set_affinity(struct irq_data *d,
+ const struct cpumask *mask_val,
+ bool force);
+#endif
+
+#ifdef CONFIG_PM
+int gic_set_wake(struct irq_data *d, unsigned int on);
+#endif
+
+#ifdef CONFIG_OF
+int _gic_of_init(struct device_node *node,
+ struct device_node *parent,
+ struct irq_chip *chip,
+ struct gic_chip_data **gic) __init;
+#endif
+
+#endif /* _IRQ_GIC_H_ */
--
1.9.0

2014-07-09 23:05:42

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 3/4 V3] irqchip: gic: Add supports for ARM GICv2m MSI(-X)

From: Suravee Suthikulpanit <[email protected]>

ARM GICv2m specification extends GICv2 to support MSI(-X) with
a new set of register frames. This patch introduces support for
the non-secure GICv2m register frame.

The driver currently matchs "arm,gic-400-plus" in device tree binding,
which implements GICv2m.

The "msi-controller" keyword in ARM GIC devicetree binding is used to indentify
GIC driver that it should enable MSI(-X) support, The region of GICv2m MSI
register frame is specified using the register frame index 4 in the device tree.
MSI support is optional.

Each GIC maintains an "msi_chip" structure. To discover the msi_chip,
PCI host driver can do the following:

struct device_node *gic_node = of_irq_find_parent(pdev->dev.of_node);
pcie_bus->msi_chip = of_pci_find_msi_chip_by_node(gic_node);

Cc: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
Documentation/devicetree/bindings/arm/gic.txt | 20 +-
arch/arm64/Kconfig | 1 +
drivers/irqchip/Kconfig | 7 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-gic-v2m.c | 251 ++++++++++++++++++++++++++
drivers/irqchip/irq-gic-v2m.h | 13 ++
drivers/irqchip/irq-gic.c | 23 ++-
drivers/irqchip/irq-gic.h | 31 +++-
8 files changed, 334 insertions(+), 13 deletions(-)
create mode 100644 drivers/irqchip/irq-gic-v2m.c
create mode 100644 drivers/irqchip/irq-gic-v2m.h

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index 5573c08..d2eea0b 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -12,11 +12,14 @@ Main node required properties:

- compatible : should be one of:
"arm,gic-400"
+ "arm,gic-400-v2m"
"arm,cortex-a15-gic"
"arm,cortex-a9-gic"
"arm,cortex-a7-gic"
"arm,arm11mp-gic"
+
- interrupt-controller : Identifies the node as an interrupt controller
+
- #interrupt-cells : Specifies the number of cells needed to encode an
interrupt source. The type shall be a <u32> and the value shall be 3.

@@ -37,9 +40,16 @@ Main node required properties:
the 8 possible cpus attached to the GIC. A bit set to '1' indicated
the interrupt is wired to that CPU. Only valid for PPI interrupts.

-- reg : Specifies base physical address(s) and size of the GIC registers. The
- first region is the GIC distributor register base and size. The 2nd region is
- the GIC cpu interface register base and size.
+- reg : Specifies base physical address(s) and size of the GIC register frames.
+
+ Region | Description
+ Index |
+ -------------------------------------------------------------------
+ 0 | GIC distributor register base and size
+ 1 | GIC cpu interface register base and size
+ 2 | VGIC interface control register base and size (Optional)
+ 3 | VGIC CPU interface register base and size (Optional)
+ 4 | GICv2m MSI interface register base and size (Optional)

Optional
- interrupts : Interrupt source of the parent interrupt controller on
@@ -55,6 +65,10 @@ Optional
by a crossbar/multiplexer preceding the GIC. The GIC irq
input line is assigned dynamically when the corresponding
peripheral's crossbar line is mapped.
+
+- msi-controller : Identifies the node as an MSI controller.
+ (Required for GICv2m)
+
Example:

intc: interrupt-controller@fff11000 {
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index be52492..0f9b11d 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -9,6 +9,7 @@ config ARM64
select ARM_AMBA
select ARM_ARCH_TIMER
select ARM_GIC
+ select ARM_GIC_V2M if (PCI && PCI_MSI)
select ARM_GIC_V3
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 7f0c2a3..274910d 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -7,6 +7,13 @@ config ARM_GIC
select IRQ_DOMAIN
select MULTI_IRQ_HANDLER

+config ARM_GIC_V2M
+ bool
+ select IRQ_DOMAIN
+ select MULTI_IRQ_HANDLER
+ depends on ARM_GIC
+ depends on PCI && PCI_MSI
+
config GIC_NON_BANKED
bool

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index c57e642..09c035a 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o
obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi-nmi.o
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_NVIC) += irq-nvic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
new file mode 100644
index 0000000..e54ca1d
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -0,0 +1,251 @@
+/*
+ * ARM GIC v2m MSI(-X) support
+ * Support for Message Signalelled Interrupts for systems that
+ * implement ARM Generic Interrupt Controller: GICv2m.
+ *
+ * Copyright (C) 2014 Advanced Micro Devices, Inc.
+ * Authors: Suravee Suthikulpanit <[email protected]>
+ * Harish Kasiviswanathan <[email protected]>
+ * Brandon Anderson <[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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pci.h>
+#include <linux/irq.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/bitmap.h>
+
+#include "irqchip.h"
+#include "irq-gic.h"
+
+/*
+* MSI_TYPER:
+* [31:26] Reserved
+* [25:16] lowest SPI assigned to MSI
+* [15:10] Reserved
+* [9:0] Numer of SPIs assigned to MSI
+*/
+#define V2M_MSI_TYPER 0x008
+#define V2M_MSI_TYPER_BASE_SHIFT (16)
+#define V2M_MSI_TYPER_BASE_MASK (0x3FF)
+#define V2M_MSI_TYPER_NUM_MASK (0x3FF)
+#define V2M_MSI_SETSPI_NS 0x040
+#define V2M_MIN_SPI 32
+#define V2M_MAX_SPI 1019
+
+#define GIC_OF_MSIV2M_RANGE_INDEX 4
+
+/*
+ * alloc_msi_irq - Allocate MSIs from avaialbe MSI bitmap.
+ * @data: Pointer to v2m_data
+ * @nvec: Number of interrupts to allocate
+ * @irq: Pointer to the allocated irq
+ *
+ * Allocates interrupts only if the contiguous range of MSIs
+ * with specified nvec are available. Otherwise return the number
+ * of available interrupts. If none are available, then returns -ENOENT.
+ */
+static int alloc_msi_irq(struct v2m_data *data, int nvec, int *irq)
+{
+ int size = data->nr_spis;
+ int next = size, i = nvec, ret;
+
+ /* We should never allocate more than available nr_spis */
+ if (i >= size)
+ i = size;
+
+ spin_lock(&data->msi_cnt_lock);
+
+ for (; i > 0; i--) {
+ next = bitmap_find_next_zero_area(data->bm,
+ size, 0, i, 0);
+ if (next < size)
+ break;
+ }
+
+ if (i != nvec) {
+ ret = i ? : -ENOENT;
+ } else {
+ bitmap_set(data->bm, next, nvec);
+ *irq = data->spi_start + next;
+ ret = 0;
+ }
+
+ spin_unlock(&data->msi_cnt_lock);
+
+ return ret;
+}
+
+static struct v2m_data *to_v2m_data(struct msi_chip *chip)
+{
+ struct gic_chip_data *gic = container_of(chip, struct gic_chip_data,
+ msi_chip);
+ return &gic->v2m_data;
+}
+
+static void gicv2m_teardown_msi_irq(struct msi_chip *chip, unsigned int irq)
+{
+ int pos;
+ struct v2m_data *data = to_v2m_data(chip);
+
+ spin_lock(&data->msi_cnt_lock);
+
+ pos = irq - data->spi_start;
+ if (pos >= 0 && pos < data->nr_spis)
+ bitmap_clear(data->bm, pos, 1);
+
+ spin_unlock(&data->msi_cnt_lock);
+}
+
+static int gicv2m_setup_msi_irq(struct msi_chip *chip, struct pci_dev *pdev,
+ struct msi_desc *desc)
+{
+ int avail, irq = 0;
+ struct msi_msg msg;
+ phys_addr_t addr;
+ struct v2m_data *data = to_v2m_data(chip);
+
+ if (!desc) {
+ dev_err(&pdev->dev,
+ "GICv2m: MSI setup failed. Invalid msi descriptor\n");
+ return -EINVAL;
+ }
+
+ avail = alloc_msi_irq(data, 1, &irq);
+ if (avail != 0) {
+ dev_err(&pdev->dev,
+ "GICv2m: MSI setup failed. Cannnot allocate IRQ\n");
+ return -ENOSPC;
+ }
+
+ irq_set_chip_data(irq, chip);
+ irq_set_msi_desc(irq, desc);
+ irq_set_irq_type(irq, IRQ_TYPE_EDGE_RISING);
+
+ addr = data->res.start + V2M_MSI_SETSPI_NS;
+
+ msg.address_hi = (u32)(addr >> 32);
+ msg.address_lo = (u32)(addr);
+ msg.data = irq;
+ write_msi_msg(irq, &msg);
+
+ return 0;
+}
+
+static int __init
+gicv2m_msi_init(struct device_node *node, struct v2m_data *v2m)
+{
+ unsigned int val;
+
+ if (of_address_to_resource(node, GIC_OF_MSIV2M_RANGE_INDEX,
+ &v2m->res)) {
+ pr_err("GICv2m: Failed locate GICv2m MSI register frame\n");
+ return -EINVAL;
+ }
+
+ v2m->base = of_iomap(node, GIC_OF_MSIV2M_RANGE_INDEX);
+ if (!v2m->base) {
+ pr_err("GICv2m: Failed to map GIC MSI registers\n");
+ return -EINVAL;
+ }
+
+ val = readl_relaxed(v2m->base + V2M_MSI_TYPER);
+ if (!val) {
+ pr_warn("GICv2m: Failed to read V2M_MSI_TYPER register\n");
+ return -EINVAL;
+ }
+
+ v2m->spi_start = (val >> V2M_MSI_TYPER_BASE_SHIFT) &
+ V2M_MSI_TYPER_BASE_MASK;
+ v2m->nr_spis = val & V2M_MSI_TYPER_NUM_MASK;
+ if ((v2m->spi_start < V2M_MIN_SPI) || (v2m->nr_spis >= V2M_MAX_SPI)) {
+ pr_err("GICv2m: Invalid MSI_TYPER (%#x)\n", val);
+ return -EINVAL;
+ }
+
+ v2m->bm = kzalloc(sizeof(long) * BITS_TO_LONGS(v2m->nr_spis),
+ GFP_KERNEL);
+ if (!v2m->bm) {
+ pr_err("GICv2m: Failed to allocate MSI bitmap\n");
+ return -ENOMEM;
+ }
+
+ spin_lock_init(&v2m->msi_cnt_lock);
+
+ pr_info("GICv2m: SPI range [%d:%d]\n",
+ v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
+
+ return 0;
+}
+
+static void gicv2m_mask_irq(struct irq_data *d)
+{
+ gic_mask_irq(d);
+ if (d->msi_desc)
+ mask_msi_irq(d);
+}
+
+static void gicv2m_unmask_irq(struct irq_data *d)
+{
+ gic_unmask_irq(d);
+ if (d->msi_desc)
+ unmask_msi_irq(d);
+}
+
+static struct irq_chip gicv2m_chip = {
+ .name = "GICv2m",
+ .irq_mask = gicv2m_mask_irq,
+ .irq_unmask = gicv2m_unmask_irq,
+ .irq_eoi = gic_eoi_irq,
+ .irq_set_type = gic_set_type,
+ .irq_retrigger = gic_retrigger,
+#ifdef CONFIG_SMP
+ .irq_set_affinity = gic_set_affinity,
+#endif
+#ifdef CONFIG_PM
+ .irq_set_wake = gic_set_wake,
+#endif
+};
+
+#ifdef CONFIG_OF
+static int __init
+gicv2m_of_init(struct device_node *node, struct device_node *parent)
+{
+ struct gic_chip_data *gic;
+ int ret;
+
+ ret = _gic_of_init(node, parent, &gicv2m_chip, &gic);
+ if (ret) {
+ pr_err("GICv2m: Failed to initialize GIC\n");
+ return ret;
+ }
+
+ gic->msi_chip.owner = THIS_MODULE;
+ gic->msi_chip.of_node = node;
+ gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;
+ gic->msi_chip.teardown_irq = gicv2m_teardown_msi_irq;
+ ret = of_pci_msi_chip_add(&gic->msi_chip);
+ if (ret) {
+ /* MSI is optional and not supported here */
+ pr_info("GICv2m: MSI is not supported.\n");
+ return 0;
+ }
+
+ ret = gicv2m_msi_init(node, &gic->v2m_data);
+ if (ret)
+ return ret;
+ return ret;
+}
+
+IRQCHIP_DECLARE(arm_gic_400_v2m, "arm,gic-400-v2m", gicv2m_of_init);
+
+#endif /* CONFIG_OF */
diff --git a/drivers/irqchip/irq-gic-v2m.h b/drivers/irqchip/irq-gic-v2m.h
new file mode 100644
index 0000000..2d93a87
--- /dev/null
+++ b/drivers/irqchip/irq-gic-v2m.h
@@ -0,0 +1,13 @@
+#ifndef _IRQ_GIC_V2M_H_
+#define _IRQ_GIC_V2M_H_
+
+struct v2m_data {
+ spinlock_t msi_cnt_lock;
+ struct resource res; /* GICv2m resource */
+ void __iomem *base; /* GICv2m virt address */
+ unsigned int spi_start; /* The SPI number that MSIs start */
+ unsigned int nr_spis; /* The number of SPIs for MSIs */
+ unsigned long *bm; /* MSI vector bitmap */
+};
+
+#endif /*_IRQ_GIC_V2M_H_*/
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 966e1d5..a054e0d 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -111,15 +111,34 @@ static inline void gic_set_base_accessor(struct gic_chip_data *data,
#define gic_set_base_accessor(d, f)
#endif

+static inline
+struct gic_chip_data *irq_data_get_gic_chip_data(struct irq_data *d)
+{
+ struct gic_chip_data *gic_data;
+ struct msi_chip *mchip;
+
+ /*
+ * For MSI, irq_data.chip_data points to struct msi_chip.
+ * For non-MSI, irq_data.chip_data points to struct gic_chip_data.
+ */
+ if (d->msi_desc) {
+ mchip = irq_data_get_irq_chip_data(d);
+ gic_data = container_of(mchip, struct gic_chip_data, msi_chip);
+ } else {
+ gic_data = irq_data_get_irq_chip_data(d);
+ }
+ return gic_data;
+}
+
static inline void __iomem *gic_dist_base(struct irq_data *d)
{
- struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+ struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
return gic_data_dist_base(gic_data);
}

static inline void __iomem *gic_cpu_base(struct irq_data *d)
{
- struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+ struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
return gic_data_cpu_base(gic_data);
}

diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
index a4beb4a..1c6547d 100644
--- a/drivers/irqchip/irq-gic.h
+++ b/drivers/irqchip/irq-gic.h
@@ -8,6 +8,17 @@ union gic_base {
void __percpu * __iomem *percpu_base;
};

+#ifdef CONFIG_ARM_GIC_V2M
+struct v2m_data {
+ spinlock_t msi_cnt_lock;
+ struct resource res; /* GICv2m resource */
+ void __iomem *base; /* GICv2m virt address */
+ unsigned int spi_start; /* The SPI number that MSIs start */
+ unsigned int nr_spis; /* The number of SPIs for MSIs */
+ unsigned long *bm; /* MSI vector bitmap */
+};
+#endif
+
struct gic_chip_data {
union gic_base dist_base;
union gic_base cpu_base;
@@ -20,12 +31,23 @@ struct gic_chip_data {
#endif
struct irq_domain *domain;
unsigned int gic_irqs;
- struct irq_chip *irq_chip;
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
+ struct irq_chip *irq_chip;
+ struct msi_chip msi_chip;
+#ifdef CONFIG_ARM_GIC_V2M
+ struct v2m_data v2m_data;
+#endif
};

+#ifdef CONFIG_OF
+int _gic_of_init(struct device_node *node,
+ struct device_node *parent,
+ struct irq_chip *chip,
+ struct gic_chip_data **gic) __init;
+#endif
+
void gic_mask_irq(struct irq_data *d);
void gic_unmask_irq(struct irq_data *d);
void gic_eoi_irq(struct irq_data *d);
@@ -42,11 +64,4 @@ int gic_set_affinity(struct irq_data *d,
int gic_set_wake(struct irq_data *d, unsigned int on);
#endif

-#ifdef CONFIG_OF
-int _gic_of_init(struct device_node *node,
- struct device_node *parent,
- struct irq_chip *chip,
- struct gic_chip_data **gic) __init;
-#endif
-
#endif /* _IRQ_GIC_H_ */
--
1.9.0

2014-07-09 23:06:14

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 4/4 V3] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m

From: Suravee Suthikulpanit <[email protected]>

This patch extend GICv2m MSI to support multiple MSI in ARM64.

This requires the common arch_setup_msi_irqs() to be overwriten
with ARM64 version which does not return 1 for PCI_CAP_ID_MSI and
nvec > 1.

Cc: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
arch/arm64/include/asm/msi.h | 15 ++++++++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/msi.c | 57 ++++++++++++++++++++++++++++++
drivers/irqchip/irq-gic-v2m.c | 80 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 153 insertions(+)
create mode 100644 arch/arm64/include/asm/msi.h
create mode 100644 arch/arm64/kernel/msi.c

diff --git a/arch/arm64/include/asm/msi.h b/arch/arm64/include/asm/msi.h
new file mode 100644
index 0000000..2a0944a
--- /dev/null
+++ b/arch/arm64/include/asm/msi.h
@@ -0,0 +1,15 @@
+#ifndef _ASM_ARM64_MSI_H_
+#define _ASM_ARM64_MSI_H_
+
+struct pci_dev;
+struct msi_desc;
+
+struct arm64_msi_ops {
+ int (*setup_msi_irqs)(struct pci_dev *dev, int nvec, int type);
+ void (*teardown_msi_irqs)(struct pci_dev *dev);
+};
+
+extern struct arm64_msi_ops arm64_msi;
+extern int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
+
+#endif /* _ASM_ARM64_MSI_H_ */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index cdaedad..0636e27 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -29,6 +29,7 @@ arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND) += sleep.o suspend.o
arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
arm64-obj-$(CONFIG_KGDB) += kgdb.o
arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o
+arm64-obj-$(CONFIG_PCI_MSI) += msi.o

obj-y += $(arm64-obj-y) vdso/
obj-m += $(arm64-obj-m)
diff --git a/arch/arm64/kernel/msi.c b/arch/arm64/kernel/msi.c
new file mode 100644
index 0000000..ed62397
--- /dev/null
+++ b/arch/arm64/kernel/msi.c
@@ -0,0 +1,57 @@
+/*
+ * ARM64 architectural MSI implemention
+ *
+ * Support for Message Signalelled Interrupts for systems that
+ * implement ARM Generic Interrupt Controller: GICv2m.
+ *
+ * Copyright (C) 2014 Advanced Micro Devices, Inc.
+ * Authors: Suravee Suthikulpanit <[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.
+ */
+
+#include <linux/irq.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+
+#include <asm/msi.h>
+
+/*
+ * ARM64 function for seting up MSI irqs.
+ * Copied from driver/pci/msi.c: arch_setup_msi_irqs().
+ */
+int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+ struct msi_desc *entry;
+ int ret;
+
+ if (type == PCI_CAP_ID_MSI && nvec > 1)
+ return 1;
+
+ list_for_each_entry(entry, &dev->msi_list, list) {
+ ret = arch_setup_msi_irq(dev, entry);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ return -ENOSPC;
+ }
+
+ return 0;
+}
+
+struct arm64_msi_ops arm64_msi = {
+ .setup_msi_irqs = arm64_setup_msi_irqs,
+ .teardown_msi_irqs = default_teardown_msi_irqs,
+};
+
+int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+ return arm64_msi.setup_msi_irqs(dev, nvec, type);
+}
+
+void arch_teardown_msi_irqs(struct pci_dev *dev)
+{
+ arm64_msi.teardown_msi_irqs(dev);
+}
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index e54ca1d..9d88ad9 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -24,6 +24,10 @@
#include <linux/of_pci.h>
#include <linux/bitmap.h>

+#ifdef CONFIG_ARM64
+#include <asm/msi.h>
+#endif
+
#include "irqchip.h"
#include "irq-gic.h"

@@ -216,6 +220,80 @@ static struct irq_chip gicv2m_chip = {
#endif
};

+
+/*
+ * _gicv2m_setup_msi_irqs - Setup MSI interrupts for the given PCI device.
+ * This overrides the weak definition in ./drivers/pci/msi.c.
+ * If nvec interrupts are irqable, then assign it to PCI device.
+ * Otherwise return error.
+ *
+ * @pdev: PCI device which is requesting to enable MSI
+ * @nvec: number of MSI vectors
+ */
+static int _gicv2m_setup_msi_irqs(struct pci_dev *pdev, int nvec)
+{
+ int irq = 0, nvec_pow2 = 0, avail;
+ int i = 0;
+ struct msi_msg msg;
+ phys_addr_t addr;
+ struct msi_desc *entry;
+ struct msi_chip *chip = pdev->bus->msi;
+ struct v2m_data *data = to_v2m_data(chip);
+
+ BUG_ON(list_empty(&pdev->msi_list));
+ WARN_ON(!list_is_singular(&pdev->msi_list));
+
+ entry = list_first_entry(&pdev->msi_list, struct msi_desc, list);
+ WARN_ON(entry->irq);
+ WARN_ON(entry->msi_attrib.multiple);
+ WARN_ON(entry->nvec_used);
+ WARN_ON(!entry->dev);
+
+ avail = alloc_msi_irq(data, nvec, &irq);
+ if (avail != 0) {
+ dev_err(&pdev->dev,
+ "GICv2m: Failed to allocate %d irqs.\n", nvec);
+ return avail;
+ }
+
+ /* Set lowest of the new interrupts assigned to the PCI device */
+ nvec_pow2 = __roundup_pow_of_two(nvec);
+ entry->nvec_used = nvec;
+ entry->msi_attrib.multiple = ilog2(nvec_pow2);
+
+ for (i = 0; i < nvec; i++) {
+ irq_set_chip_data(irq+i, chip);
+ if (irq_set_msi_desc_off(irq, i, entry)) {
+ dev_err(&pdev->dev,
+ "GICv2m: Failed to set up MSI irq %d\n",
+ (irq+i));
+ return -EINVAL;
+ }
+
+ irq_set_irq_type((irq+i), IRQ_TYPE_EDGE_RISING);
+ }
+
+ addr = data->res.start + V2M_MSI_SETSPI_NS;
+ msg.address_hi = (u32)(addr >> 32);
+ msg.address_lo = (u32)(addr);
+ msg.data = irq;
+ write_msi_msg(irq, &msg);
+
+ return 0;
+}
+
+static int
+gicv2m_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
+{
+ int ret;
+
+ if (type == PCI_CAP_ID_MSI)
+ ret = _gicv2m_setup_msi_irqs(pdev, nvec);
+ else
+ ret = arm64_setup_msi_irqs(pdev, nvec, type);
+ return ret;
+}
+
#ifdef CONFIG_OF
static int __init
gicv2m_of_init(struct device_node *node, struct device_node *parent)
@@ -229,6 +307,8 @@ gicv2m_of_init(struct device_node *node, struct device_node *parent)
return ret;
}

+ arm64_msi.setup_msi_irqs = &gicv2m_setup_msi_irqs;
+
gic->msi_chip.owner = THIS_MODULE;
gic->msi_chip.of_node = node;
gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;
--
1.9.0

2014-07-13 23:01:37

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 3/4 V3] irqchip: gic: Add supports for ARM GICv2m MSI(-X)

Suravee,

On Wed, Jul 09, 2014 at 06:05:03PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> ARM GICv2m specification extends GICv2 to support MSI(-X) with
> a new set of register frames. This patch introduces support for
> the non-secure GICv2m register frame.
>
> The driver currently matchs "arm,gic-400-plus" in device tree binding,
> which implements GICv2m.

This commit message needs updated since you changed the compatible
string.

> The "msi-controller" keyword in ARM GIC devicetree binding is used to indentify

while-we're-here-nit: s/indentify/identify/

thx,

Jason.

> GIC driver that it should enable MSI(-X) support, The region of GICv2m MSI
> register frame is specified using the register frame index 4 in the device tree.
> MSI support is optional.
>
> Each GIC maintains an "msi_chip" structure. To discover the msi_chip,
> PCI host driver can do the following:
>
> struct device_node *gic_node = of_irq_find_parent(pdev->dev.of_node);
> pcie_bus->msi_chip = of_pci_find_msi_chip_by_node(gic_node);
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/gic.txt | 20 +-
> arch/arm64/Kconfig | 1 +
> drivers/irqchip/Kconfig | 7 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-gic-v2m.c | 251 ++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.h | 13 ++
> drivers/irqchip/irq-gic.c | 23 ++-
> drivers/irqchip/irq-gic.h | 31 +++-
> 8 files changed, 334 insertions(+), 13 deletions(-)
> create mode 100644 drivers/irqchip/irq-gic-v2m.c
> create mode 100644 drivers/irqchip/irq-gic-v2m.h
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08..d2eea0b 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -12,11 +12,14 @@ Main node required properties:
>
> - compatible : should be one of:
> "arm,gic-400"
> + "arm,gic-400-v2m"
> "arm,cortex-a15-gic"
> "arm,cortex-a9-gic"
> "arm,cortex-a7-gic"
> "arm,arm11mp-gic"
> +
> - interrupt-controller : Identifies the node as an interrupt controller
> +
> - #interrupt-cells : Specifies the number of cells needed to encode an
> interrupt source. The type shall be a <u32> and the value shall be 3.
>
> @@ -37,9 +40,16 @@ Main node required properties:
> the 8 possible cpus attached to the GIC. A bit set to '1' indicated
> the interrupt is wired to that CPU. Only valid for PPI interrupts.
>
> -- reg : Specifies base physical address(s) and size of the GIC registers. The
> - first region is the GIC distributor register base and size. The 2nd region is
> - the GIC cpu interface register base and size.
> +- reg : Specifies base physical address(s) and size of the GIC register frames.
> +
> + Region | Description
> + Index |
> + -------------------------------------------------------------------
> + 0 | GIC distributor register base and size
> + 1 | GIC cpu interface register base and size
> + 2 | VGIC interface control register base and size (Optional)
> + 3 | VGIC CPU interface register base and size (Optional)
> + 4 | GICv2m MSI interface register base and size (Optional)
>
> Optional
> - interrupts : Interrupt source of the parent interrupt controller on
> @@ -55,6 +65,10 @@ Optional
> by a crossbar/multiplexer preceding the GIC. The GIC irq
> input line is assigned dynamically when the corresponding
> peripheral's crossbar line is mapped.
> +
> +- msi-controller : Identifies the node as an MSI controller.
> + (Required for GICv2m)
> +
> Example:
>
> intc: interrupt-controller@fff11000 {
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index be52492..0f9b11d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -9,6 +9,7 @@ config ARM64
> select ARM_AMBA
> select ARM_ARCH_TIMER
> select ARM_GIC
> + select ARM_GIC_V2M if (PCI && PCI_MSI)
> select ARM_GIC_V3
> select BUILDTIME_EXTABLE_SORT
> select CLONE_BACKWARDS
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 7f0c2a3..274910d 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -7,6 +7,13 @@ config ARM_GIC
> select IRQ_DOMAIN
> select MULTI_IRQ_HANDLER
>
> +config ARM_GIC_V2M
> + bool
> + select IRQ_DOMAIN
> + select MULTI_IRQ_HANDLER
> + depends on ARM_GIC
> + depends on PCI && PCI_MSI
> +
> config GIC_NON_BANKED
> bool
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index c57e642..09c035a 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o
> obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi-nmi.o
> 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_NVIC) += irq-nvic.o
> obj-$(CONFIG_ARM_VIC) += irq-vic.o
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> new file mode 100644
> index 0000000..e54ca1d
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -0,0 +1,251 @@
> +/*
> + * ARM GIC v2m MSI(-X) support
> + * Support for Message Signalelled Interrupts for systems that
> + * implement ARM Generic Interrupt Controller: GICv2m.
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + * Authors: Suravee Suthikulpanit <[email protected]>
> + * Harish Kasiviswanathan <[email protected]>
> + * Brandon Anderson <[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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pci.h>
> +#include <linux/irq.h>
> +#include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/bitmap.h>
> +
> +#include "irqchip.h"
> +#include "irq-gic.h"
> +
> +/*
> +* MSI_TYPER:
> +* [31:26] Reserved
> +* [25:16] lowest SPI assigned to MSI
> +* [15:10] Reserved
> +* [9:0] Numer of SPIs assigned to MSI
> +*/
> +#define V2M_MSI_TYPER 0x008
> +#define V2M_MSI_TYPER_BASE_SHIFT (16)
> +#define V2M_MSI_TYPER_BASE_MASK (0x3FF)
> +#define V2M_MSI_TYPER_NUM_MASK (0x3FF)
> +#define V2M_MSI_SETSPI_NS 0x040
> +#define V2M_MIN_SPI 32
> +#define V2M_MAX_SPI 1019
> +
> +#define GIC_OF_MSIV2M_RANGE_INDEX 4
> +
> +/*
> + * alloc_msi_irq - Allocate MSIs from avaialbe MSI bitmap.
> + * @data: Pointer to v2m_data
> + * @nvec: Number of interrupts to allocate
> + * @irq: Pointer to the allocated irq
> + *
> + * Allocates interrupts only if the contiguous range of MSIs
> + * with specified nvec are available. Otherwise return the number
> + * of available interrupts. If none are available, then returns -ENOENT.
> + */
> +static int alloc_msi_irq(struct v2m_data *data, int nvec, int *irq)
> +{
> + int size = data->nr_spis;
> + int next = size, i = nvec, ret;
> +
> + /* We should never allocate more than available nr_spis */
> + if (i >= size)
> + i = size;
> +
> + spin_lock(&data->msi_cnt_lock);
> +
> + for (; i > 0; i--) {
> + next = bitmap_find_next_zero_area(data->bm,
> + size, 0, i, 0);
> + if (next < size)
> + break;
> + }
> +
> + if (i != nvec) {
> + ret = i ? : -ENOENT;
> + } else {
> + bitmap_set(data->bm, next, nvec);
> + *irq = data->spi_start + next;
> + ret = 0;
> + }
> +
> + spin_unlock(&data->msi_cnt_lock);
> +
> + return ret;
> +}
> +
> +static struct v2m_data *to_v2m_data(struct msi_chip *chip)
> +{
> + struct gic_chip_data *gic = container_of(chip, struct gic_chip_data,
> + msi_chip);
> + return &gic->v2m_data;
> +}
> +
> +static void gicv2m_teardown_msi_irq(struct msi_chip *chip, unsigned int irq)
> +{
> + int pos;
> + struct v2m_data *data = to_v2m_data(chip);
> +
> + spin_lock(&data->msi_cnt_lock);
> +
> + pos = irq - data->spi_start;
> + if (pos >= 0 && pos < data->nr_spis)
> + bitmap_clear(data->bm, pos, 1);
> +
> + spin_unlock(&data->msi_cnt_lock);
> +}
> +
> +static int gicv2m_setup_msi_irq(struct msi_chip *chip, struct pci_dev *pdev,
> + struct msi_desc *desc)
> +{
> + int avail, irq = 0;
> + struct msi_msg msg;
> + phys_addr_t addr;
> + struct v2m_data *data = to_v2m_data(chip);
> +
> + if (!desc) {
> + dev_err(&pdev->dev,
> + "GICv2m: MSI setup failed. Invalid msi descriptor\n");
> + return -EINVAL;
> + }
> +
> + avail = alloc_msi_irq(data, 1, &irq);
> + if (avail != 0) {
> + dev_err(&pdev->dev,
> + "GICv2m: MSI setup failed. Cannnot allocate IRQ\n");
> + return -ENOSPC;
> + }
> +
> + irq_set_chip_data(irq, chip);
> + irq_set_msi_desc(irq, desc);
> + irq_set_irq_type(irq, IRQ_TYPE_EDGE_RISING);
> +
> + addr = data->res.start + V2M_MSI_SETSPI_NS;
> +
> + msg.address_hi = (u32)(addr >> 32);
> + msg.address_lo = (u32)(addr);
> + msg.data = irq;
> + write_msi_msg(irq, &msg);
> +
> + return 0;
> +}
> +
> +static int __init
> +gicv2m_msi_init(struct device_node *node, struct v2m_data *v2m)
> +{
> + unsigned int val;
> +
> + if (of_address_to_resource(node, GIC_OF_MSIV2M_RANGE_INDEX,
> + &v2m->res)) {
> + pr_err("GICv2m: Failed locate GICv2m MSI register frame\n");
> + return -EINVAL;
> + }
> +
> + v2m->base = of_iomap(node, GIC_OF_MSIV2M_RANGE_INDEX);
> + if (!v2m->base) {
> + pr_err("GICv2m: Failed to map GIC MSI registers\n");
> + return -EINVAL;
> + }
> +
> + val = readl_relaxed(v2m->base + V2M_MSI_TYPER);
> + if (!val) {
> + pr_warn("GICv2m: Failed to read V2M_MSI_TYPER register\n");
> + return -EINVAL;
> + }
> +
> + v2m->spi_start = (val >> V2M_MSI_TYPER_BASE_SHIFT) &
> + V2M_MSI_TYPER_BASE_MASK;
> + v2m->nr_spis = val & V2M_MSI_TYPER_NUM_MASK;
> + if ((v2m->spi_start < V2M_MIN_SPI) || (v2m->nr_spis >= V2M_MAX_SPI)) {
> + pr_err("GICv2m: Invalid MSI_TYPER (%#x)\n", val);
> + return -EINVAL;
> + }
> +
> + v2m->bm = kzalloc(sizeof(long) * BITS_TO_LONGS(v2m->nr_spis),
> + GFP_KERNEL);
> + if (!v2m->bm) {
> + pr_err("GICv2m: Failed to allocate MSI bitmap\n");
> + return -ENOMEM;
> + }
> +
> + spin_lock_init(&v2m->msi_cnt_lock);
> +
> + pr_info("GICv2m: SPI range [%d:%d]\n",
> + v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
> +
> + return 0;
> +}
> +
> +static void gicv2m_mask_irq(struct irq_data *d)
> +{
> + gic_mask_irq(d);
> + if (d->msi_desc)
> + mask_msi_irq(d);
> +}
> +
> +static void gicv2m_unmask_irq(struct irq_data *d)
> +{
> + gic_unmask_irq(d);
> + if (d->msi_desc)
> + unmask_msi_irq(d);
> +}
> +
> +static struct irq_chip gicv2m_chip = {
> + .name = "GICv2m",
> + .irq_mask = gicv2m_mask_irq,
> + .irq_unmask = gicv2m_unmask_irq,
> + .irq_eoi = gic_eoi_irq,
> + .irq_set_type = gic_set_type,
> + .irq_retrigger = gic_retrigger,
> +#ifdef CONFIG_SMP
> + .irq_set_affinity = gic_set_affinity,
> +#endif
> +#ifdef CONFIG_PM
> + .irq_set_wake = gic_set_wake,
> +#endif
> +};
> +
> +#ifdef CONFIG_OF
> +static int __init
> +gicv2m_of_init(struct device_node *node, struct device_node *parent)
> +{
> + struct gic_chip_data *gic;
> + int ret;
> +
> + ret = _gic_of_init(node, parent, &gicv2m_chip, &gic);
> + if (ret) {
> + pr_err("GICv2m: Failed to initialize GIC\n");
> + return ret;
> + }
> +
> + gic->msi_chip.owner = THIS_MODULE;
> + gic->msi_chip.of_node = node;
> + gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;
> + gic->msi_chip.teardown_irq = gicv2m_teardown_msi_irq;
> + ret = of_pci_msi_chip_add(&gic->msi_chip);
> + if (ret) {
> + /* MSI is optional and not supported here */
> + pr_info("GICv2m: MSI is not supported.\n");
> + return 0;
> + }
> +
> + ret = gicv2m_msi_init(node, &gic->v2m_data);
> + if (ret)
> + return ret;
> + return ret;
> +}
> +
> +IRQCHIP_DECLARE(arm_gic_400_v2m, "arm,gic-400-v2m", gicv2m_of_init);
> +
> +#endif /* CONFIG_OF */
> diff --git a/drivers/irqchip/irq-gic-v2m.h b/drivers/irqchip/irq-gic-v2m.h
> new file mode 100644
> index 0000000..2d93a87
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v2m.h
> @@ -0,0 +1,13 @@
> +#ifndef _IRQ_GIC_V2M_H_
> +#define _IRQ_GIC_V2M_H_
> +
> +struct v2m_data {
> + spinlock_t msi_cnt_lock;
> + struct resource res; /* GICv2m resource */
> + void __iomem *base; /* GICv2m virt address */
> + unsigned int spi_start; /* The SPI number that MSIs start */
> + unsigned int nr_spis; /* The number of SPIs for MSIs */
> + unsigned long *bm; /* MSI vector bitmap */
> +};
> +
> +#endif /*_IRQ_GIC_V2M_H_*/
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 966e1d5..a054e0d 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -111,15 +111,34 @@ static inline void gic_set_base_accessor(struct gic_chip_data *data,
> #define gic_set_base_accessor(d, f)
> #endif
>
> +static inline
> +struct gic_chip_data *irq_data_get_gic_chip_data(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data;
> + struct msi_chip *mchip;
> +
> + /*
> + * For MSI, irq_data.chip_data points to struct msi_chip.
> + * For non-MSI, irq_data.chip_data points to struct gic_chip_data.
> + */
> + if (d->msi_desc) {
> + mchip = irq_data_get_irq_chip_data(d);
> + gic_data = container_of(mchip, struct gic_chip_data, msi_chip);
> + } else {
> + gic_data = irq_data_get_irq_chip_data(d);
> + }
> + return gic_data;
> +}
> +
> static inline void __iomem *gic_dist_base(struct irq_data *d)
> {
> - struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
> return gic_data_dist_base(gic_data);
> }
>
> static inline void __iomem *gic_cpu_base(struct irq_data *d)
> {
> - struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
> return gic_data_cpu_base(gic_data);
> }
>
> diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
> index a4beb4a..1c6547d 100644
> --- a/drivers/irqchip/irq-gic.h
> +++ b/drivers/irqchip/irq-gic.h
> @@ -8,6 +8,17 @@ union gic_base {
> void __percpu * __iomem *percpu_base;
> };
>
> +#ifdef CONFIG_ARM_GIC_V2M
> +struct v2m_data {
> + spinlock_t msi_cnt_lock;
> + struct resource res; /* GICv2m resource */
> + void __iomem *base; /* GICv2m virt address */
> + unsigned int spi_start; /* The SPI number that MSIs start */
> + unsigned int nr_spis; /* The number of SPIs for MSIs */
> + unsigned long *bm; /* MSI vector bitmap */
> +};
> +#endif
> +
> struct gic_chip_data {
> union gic_base dist_base;
> union gic_base cpu_base;
> @@ -20,12 +31,23 @@ struct gic_chip_data {
> #endif
> struct irq_domain *domain;
> unsigned int gic_irqs;
> - struct irq_chip *irq_chip;
> #ifdef CONFIG_GIC_NON_BANKED
> void __iomem *(*get_base)(union gic_base *);
> #endif
> + struct irq_chip *irq_chip;
> + struct msi_chip msi_chip;
> +#ifdef CONFIG_ARM_GIC_V2M
> + struct v2m_data v2m_data;
> +#endif
> };
>
> +#ifdef CONFIG_OF
> +int _gic_of_init(struct device_node *node,
> + struct device_node *parent,
> + struct irq_chip *chip,
> + struct gic_chip_data **gic) __init;
> +#endif
> +
> void gic_mask_irq(struct irq_data *d);
> void gic_unmask_irq(struct irq_data *d);
> void gic_eoi_irq(struct irq_data *d);
> @@ -42,11 +64,4 @@ int gic_set_affinity(struct irq_data *d,
> int gic_set_wake(struct irq_data *d, unsigned int on);
> #endif
>
> -#ifdef CONFIG_OF
> -int _gic_of_init(struct device_node *node,
> - struct device_node *parent,
> - struct irq_chip *chip,
> - struct gic_chip_data **gic) __init;
> -#endif
> -
> #endif /* _IRQ_GIC_H_ */
> --
> 1.9.0
>

2014-07-13 23:03:46

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 4/4 V3] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m

Suravee,

If you need to respin this series, please change the subject line to
"irqchip: gic-v2m: ..." If there are no other changes needed, It can be
fixed up when applied.

thx,

Jason.

On Wed, Jul 09, 2014 at 06:05:04PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> This patch extend GICv2m MSI to support multiple MSI in ARM64.
>
> This requires the common arch_setup_msi_irqs() to be overwriten
> with ARM64 version which does not return 1 for PCI_CAP_ID_MSI and
> nvec > 1.
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> arch/arm64/include/asm/msi.h | 15 ++++++++
> arch/arm64/kernel/Makefile | 1 +
> arch/arm64/kernel/msi.c | 57 ++++++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.c | 80 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 153 insertions(+)
> create mode 100644 arch/arm64/include/asm/msi.h
> create mode 100644 arch/arm64/kernel/msi.c
>
> diff --git a/arch/arm64/include/asm/msi.h b/arch/arm64/include/asm/msi.h
> new file mode 100644
> index 0000000..2a0944a
> --- /dev/null
> +++ b/arch/arm64/include/asm/msi.h
> @@ -0,0 +1,15 @@
> +#ifndef _ASM_ARM64_MSI_H_
> +#define _ASM_ARM64_MSI_H_
> +
> +struct pci_dev;
> +struct msi_desc;
> +
> +struct arm64_msi_ops {
> + int (*setup_msi_irqs)(struct pci_dev *dev, int nvec, int type);
> + void (*teardown_msi_irqs)(struct pci_dev *dev);
> +};
> +
> +extern struct arm64_msi_ops arm64_msi;
> +extern int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
> +
> +#endif /* _ASM_ARM64_MSI_H_ */
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index cdaedad..0636e27 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -29,6 +29,7 @@ arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND) += sleep.o suspend.o
> arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> arm64-obj-$(CONFIG_KGDB) += kgdb.o
> arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o
> +arm64-obj-$(CONFIG_PCI_MSI) += msi.o
>
> obj-y += $(arm64-obj-y) vdso/
> obj-m += $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/msi.c b/arch/arm64/kernel/msi.c
> new file mode 100644
> index 0000000..ed62397
> --- /dev/null
> +++ b/arch/arm64/kernel/msi.c
> @@ -0,0 +1,57 @@
> +/*
> + * ARM64 architectural MSI implemention
> + *
> + * Support for Message Signalelled Interrupts for systems that
> + * implement ARM Generic Interrupt Controller: GICv2m.
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + * Authors: Suravee Suthikulpanit <[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.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +
> +#include <asm/msi.h>
> +
> +/*
> + * ARM64 function for seting up MSI irqs.
> + * Copied from driver/pci/msi.c: arch_setup_msi_irqs().
> + */
> +int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> +{
> + struct msi_desc *entry;
> + int ret;
> +
> + if (type == PCI_CAP_ID_MSI && nvec > 1)
> + return 1;
> +
> + list_for_each_entry(entry, &dev->msi_list, list) {
> + ret = arch_setup_msi_irq(dev, entry);
> + if (ret < 0)
> + return ret;
> + if (ret > 0)
> + return -ENOSPC;
> + }
> +
> + return 0;
> +}
> +
> +struct arm64_msi_ops arm64_msi = {
> + .setup_msi_irqs = arm64_setup_msi_irqs,
> + .teardown_msi_irqs = default_teardown_msi_irqs,
> +};
> +
> +int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> +{
> + return arm64_msi.setup_msi_irqs(dev, nvec, type);
> +}
> +
> +void arch_teardown_msi_irqs(struct pci_dev *dev)
> +{
> + arm64_msi.teardown_msi_irqs(dev);
> +}
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> index e54ca1d..9d88ad9 100644
> --- a/drivers/irqchip/irq-gic-v2m.c
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -24,6 +24,10 @@
> #include <linux/of_pci.h>
> #include <linux/bitmap.h>
>
> +#ifdef CONFIG_ARM64
> +#include <asm/msi.h>
> +#endif
> +
> #include "irqchip.h"
> #include "irq-gic.h"
>
> @@ -216,6 +220,80 @@ static struct irq_chip gicv2m_chip = {
> #endif
> };
>
> +
> +/*
> + * _gicv2m_setup_msi_irqs - Setup MSI interrupts for the given PCI device.
> + * This overrides the weak definition in ./drivers/pci/msi.c.
> + * If nvec interrupts are irqable, then assign it to PCI device.
> + * Otherwise return error.
> + *
> + * @pdev: PCI device which is requesting to enable MSI
> + * @nvec: number of MSI vectors
> + */
> +static int _gicv2m_setup_msi_irqs(struct pci_dev *pdev, int nvec)
> +{
> + int irq = 0, nvec_pow2 = 0, avail;
> + int i = 0;
> + struct msi_msg msg;
> + phys_addr_t addr;
> + struct msi_desc *entry;
> + struct msi_chip *chip = pdev->bus->msi;
> + struct v2m_data *data = to_v2m_data(chip);
> +
> + BUG_ON(list_empty(&pdev->msi_list));
> + WARN_ON(!list_is_singular(&pdev->msi_list));
> +
> + entry = list_first_entry(&pdev->msi_list, struct msi_desc, list);
> + WARN_ON(entry->irq);
> + WARN_ON(entry->msi_attrib.multiple);
> + WARN_ON(entry->nvec_used);
> + WARN_ON(!entry->dev);
> +
> + avail = alloc_msi_irq(data, nvec, &irq);
> + if (avail != 0) {
> + dev_err(&pdev->dev,
> + "GICv2m: Failed to allocate %d irqs.\n", nvec);
> + return avail;
> + }
> +
> + /* Set lowest of the new interrupts assigned to the PCI device */
> + nvec_pow2 = __roundup_pow_of_two(nvec);
> + entry->nvec_used = nvec;
> + entry->msi_attrib.multiple = ilog2(nvec_pow2);
> +
> + for (i = 0; i < nvec; i++) {
> + irq_set_chip_data(irq+i, chip);
> + if (irq_set_msi_desc_off(irq, i, entry)) {
> + dev_err(&pdev->dev,
> + "GICv2m: Failed to set up MSI irq %d\n",
> + (irq+i));
> + return -EINVAL;
> + }
> +
> + irq_set_irq_type((irq+i), IRQ_TYPE_EDGE_RISING);
> + }
> +
> + addr = data->res.start + V2M_MSI_SETSPI_NS;
> + msg.address_hi = (u32)(addr >> 32);
> + msg.address_lo = (u32)(addr);
> + msg.data = irq;
> + write_msi_msg(irq, &msg);
> +
> + return 0;
> +}
> +
> +static int
> +gicv2m_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> +{
> + int ret;
> +
> + if (type == PCI_CAP_ID_MSI)
> + ret = _gicv2m_setup_msi_irqs(pdev, nvec);
> + else
> + ret = arm64_setup_msi_irqs(pdev, nvec, type);
> + return ret;
> +}
> +
> #ifdef CONFIG_OF
> static int __init
> gicv2m_of_init(struct device_node *node, struct device_node *parent)
> @@ -229,6 +307,8 @@ gicv2m_of_init(struct device_node *node, struct device_node *parent)
> return ret;
> }
>
> + arm64_msi.setup_msi_irqs = &gicv2m_setup_msi_irqs;
> +
> gic->msi_chip.owner = THIS_MODULE;
> gic->msi_chip.of_node = node;
> gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;
> --
> 1.9.0
>

2014-07-13 23:14:48

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

Suravee,

On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> This patch set introduces support for MSI(-X) in GICv2m specification,
> which is implemented in some variation of GIC400.
>
> This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> (https://lkml.org/lkml/2014/3/14/320).

Grrr. I mis-spoke against your v1 of this series. There are more
changes to irq-gic.c than I originally thought in this series.

Additionally, we have a lot of other significant changes to that driver
as well this cycle. It would be really helpful if I could take patches
1-3 through irqchip/gic. I can Ack #4 with the Subject change, and the
branch it lands in can depend on irqchip/gic, no problem there.

My main concern is your statement above and your answer to my inquiry
against v1.

Right now, I'm only concerned about breaking the build. Can I take 1-3?
Or, do we need to wait until aarch64 PCI lands in mainline?

If I can take them, I'll fix up the commit log of the one patch when I
apply them.

thx,

Jason.

> Changes in V3:
> * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> (per Jason Cooper request)
> * Misc fix/clean up per Mark Rutland comments
> * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> * Patch 4 is new to the series:
> * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> for Multiple MSI.
> * Add support for Multiple MSI for GICv2m.
>
> Suravee Suthikulpanit (4):
> irqchip: gic: Add binding probe for ARM GIC400
> irqchip: gic: Restructuring ARM GIC code
> irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
>
> Documentation/devicetree/bindings/arm/gic.txt | 20 +-
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/msi.h | 15 ++
> arch/arm64/kernel/Makefile | 1 +
> arch/arm64/kernel/msi.c | 57 +++++
> drivers/irqchip/Kconfig | 7 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-gic-v2m.c | 331 ++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.h | 13 +
> drivers/irqchip/irq-gic.c | 90 ++++---
> drivers/irqchip/irq-gic.h | 67 ++++++
> 11 files changed, 562 insertions(+), 41 deletions(-)
> create mode 100644 arch/arm64/include/asm/msi.h
> create mode 100644 arch/arm64/kernel/msi.c
> create mode 100644 drivers/irqchip/irq-gic-v2m.c
> create mode 100644 drivers/irqchip/irq-gic-v2m.h
> create mode 100644 drivers/irqchip/irq-gic.h
>
> --
> 1.9.0
>

2014-07-14 14:02:11

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 1/4 V3] irqchip: gic: Add binding probe for ARM GIC400

Am Mittwoch, 9. Juli 2014, 18:05:01 schrieb [email protected]:
> From: Suravee Suthikulpanit <[email protected]>
>
> Add new Irqchip declaration for GIC400. This was mentioned in
> gic binding documentation, but there is not code to support it.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>

The Rockchip RK3288 also contains a GIC400, so I would be very happy if
someone could pick up this one patch independently of the rest.

But in light of Matthias' cortex-a7-gic patch from last week, this one might
need an update.


> ---
> drivers/irqchip/irq-gic.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 508b815..ac8f7ea 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1020,6 +1020,8 @@ gic_of_init(struct device_node *node, struct
> device_node *parent) gic_cnt++;
> return 0;
> }
> +

also it looks like the omission of the blank line in the original file is
intentional, so I'd guess it shouldn't be added here.


> +IRQCHIP_DECLARE(arm_gic_400, "arm,gic-400", gic_of_init);
> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);


Thanks
Heiko

2014-07-14 16:00:39

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On 7/13/2014 6:14 PM, Jason Cooper wrote:
> Suravee,
>
> On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
>> From: Suravee Suthikulpanit <[email protected]>
>>
>> This patch set introduces support for MSI(-X) in GICv2m specification,
>> which is implemented in some variation of GIC400.
>>
>> This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
>> (https://lkml.org/lkml/2014/3/14/320).
>
> Grrr. I mis-spoke against your v1 of this series. There are more
> changes to irq-gic.c than I originally thought in this series.

I am not quite sure what your are referring to.

> Additionally, we have a lot of other significant changes to that driver
> as well this cycle. It would be really helpful if I could take patches
> 1-3 through irqchip/gic. I can Ack #4 with the Subject change, and the
> branch it lands in can depend on irqchip/gic, no problem there.

Patch 1-3 should be able to go through the irqchip/gic along with the
gicv3 from Marc, which I have rebased this patch against.

Patch 4 is arch64 architectural changes. Therefore, it might need to be
going through a different branch. Marc/Mark/Will/Catalin, do you have
any suggestions on which branch this should go to?

> My main concern is your statement above and your answer to my inquiry
> against v1.
>
> Right now, I'm only concerned about breaking the build. Can I take 1-3?
> Or, do we need to wait until aarch64 PCI lands in mainline?

1 and 2 should be trivial since there is no change functionally.

3 mostly adding new files which should not get built if ARCH64 PCI is
not supported based on the arch/arm64/Kconfig below.

+++ b/arch/arm64/Kconfig
@@ -9,6 +9,7 @@ config ARM64
select ARM_AMBA
select ARM_ARCH_TIMER
select ARM_GIC
+ select ARM_GIC_V2M if (PCI && PCI_MSI)
select ARM_GIC_V3
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS

The only thing is the change related to MSI in the irq-gic.c which
should not affect with the non-PCI system.

Thanks,

Suravee

2014-07-14 22:01:53

by Heiko Stuebner

[permalink] [raw]
Subject: [PATCH] irqchip: gic: Add binding probe for ARM GIC400

From: Suravee Suthikulpanit <[email protected]>

Commit 3ab72f9156bb "dt-bindings: add GIC-400 binding" added the
"arm,gic-400" compatible string, but the corresponding IRQCHIP_DECLARE
was never added to the gic driver.

Therefore add the missing irqchip declaration for it.

Signed-off-by: Suravee Suthikulpanit <[email protected]>

Removed additional empty line and adapted commit message to mark it
as fixing an issue.
Signed-off-by: Heiko Stuebner <[email protected]>
---
As I really need this, I took the liberty of adapting the patch accordingly
to make it apply on top of the current irqchip/for-next (or urgent) and
explicitly state the fixed issue. Hope that is ok

drivers/irqchip/irq-gic.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 9d26643..6ff28b4 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1020,6 +1020,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
gic_cnt++;
return 0;
}
+IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
--
1.9.0

2014-07-15 08:01:45

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] irqchip: gic: Add binding probe for ARM GIC400

On Mon, Jul 14, 2014 at 11:03:03PM +0100, Heiko St?bner wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> Commit 3ab72f9156bb "dt-bindings: add GIC-400 binding" added the
> "arm,gic-400" compatible string, but the corresponding IRQCHIP_DECLARE
> was never added to the gic driver.
>
> Therefore add the missing irqchip declaration for it.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>
> Removed additional empty line and adapted commit message to mark it
> as fixing an issue.
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> As I really need this, I took the liberty of adapting the patch accordingly
> to make it apply on top of the current irqchip/for-next (or urgent) and
> explicitly state the fixed issue. Hope that is ok

Acked-by: Will Deacon <[email protected]>

Will

> drivers/irqchip/irq-gic.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 9d26643..6ff28b4 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1020,6 +1020,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> gic_cnt++;
> return 0;
> }
> +IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
> --
> 1.9.0
>
>
>
>

2014-07-17 12:48:27

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH] irqchip: gic: Add binding probe for ARM GIC400

On Tue, Jul 15, 2014 at 12:03:03AM +0200, Heiko St?bner wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> Commit 3ab72f9156bb "dt-bindings: add GIC-400 binding" added the
> "arm,gic-400" compatible string, but the corresponding IRQCHIP_DECLARE
> was never added to the gic driver.
>
> Therefore add the missing irqchip declaration for it.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>
> Removed additional empty line and adapted commit message to mark it
> as fixing an issue.
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> As I really need this, I took the liberty of adapting the patch accordingly
> to make it apply on top of the current irqchip/for-next (or urgent) and
> explicitly state the fixed issue. Hope that is ok
>
> drivers/irqchip/irq-gic.c | 1 +
> 1 file changed, 1 insertion(+)

Applied to irqchip/urgent with Will's Ack and Cc'd to stable for v3.14+

thx,

Jason.

2014-07-17 12:51:43

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On Mon, Jul 14, 2014 at 10:59:57AM -0500, Suravee Suthikulanit wrote:
> On 7/13/2014 6:14 PM, Jason Cooper wrote:
> >Suravee,
> >
> >On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> >>From: Suravee Suthikulpanit <[email protected]>
> >>
> >>This patch set introduces support for MSI(-X) in GICv2m specification,
> >>which is implemented in some variation of GIC400.
> >>
> >>This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> >>(https://lkml.org/lkml/2014/3/14/320).
> >
> >Grrr. I mis-spoke against your v1 of this series. There are more
> >changes to irq-gic.c than I originally thought in this series.
>
> I am not quite sure what your are referring to.
>

No problem, it was an old comment. If you weren't depending on it then it
doesn't matter. ;-)

> >Additionally, we have a lot of other significant changes to that driver
> >as well this cycle. It would be really helpful if I could take patches
> >1-3 through irqchip/gic. I can Ack #4 with the Subject change, and the
> >branch it lands in can depend on irqchip/gic, no problem there.
>
> Patch 1-3 should be able to go through the irqchip/gic along with
> the gicv3 from Marc, which I have rebased this patch against.

Perfect!

> Patch 4 is arch64 architectural changes. Therefore, it might need
> to be going through a different branch. Marc/Mark/Will/Catalin, do
> you have any suggestions on which branch this should go to?

Yep, I'll Ack it for arm64 to take, conflicts should be minimal.

thx,

Jason.

2014-07-17 12:53:22

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 4/4 V3] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m

Suravee,

On Wed, Jul 09, 2014 at 06:05:04PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> This patch extend GICv2m MSI to support multiple MSI in ARM64.
>
> This requires the common arch_setup_msi_irqs() to be overwriten
> with ARM64 version which does not return 1 for PCI_CAP_ID_MSI and
> nvec > 1.
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> arch/arm64/include/asm/msi.h | 15 ++++++++
> arch/arm64/kernel/Makefile | 1 +
> arch/arm64/kernel/msi.c | 57 ++++++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.c | 80 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 153 insertions(+)
> create mode 100644 arch/arm64/include/asm/msi.h
> create mode 100644 arch/arm64/kernel/msi.c

With the Subject line nit fixed ("irqchip: gic-v2m: ..."),

Acked-by: Jason Cooper <[email protected]>

Please route as you see fit.

thx,

Jason.

2014-07-17 13:12:56

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 2/4 V3] irqchip: gic: Restructuring ARM GIC code

On Wed, Jul 09, 2014 at 06:05:02PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> This patch restructures the code to prepare for future MSI support.
> It moves the declaration of structures and functions into the header file,
> and omit the static prefix.
>
> Since we are planing to have different irq_chip for GICv2m, the patch adds
> irq_chip pointer in the gic_chip_data which is initialized during probing phase.
>
> This should not have any functional changes.
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> drivers/irqchip/irq-gic.c | 65 +++++++++++++++++++++--------------------------
> drivers/irqchip/irq-gic.h | 52 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 81 insertions(+), 36 deletions(-)
> create mode 100644 drivers/irqchip/irq-gic.h

Applied to irqchip/gic. With a dependency on irqchip/urgent for

144cb08864ed irqchip: gic: Add binding probe for ARM GIC400

As a result, I had to hand-jam the last hunk for irq-gic.c. Please
double-check my work.

thx,

Jason.

2014-07-17 13:13:38

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 3/4 V3] irqchip: gic: Add supports for ARM GICv2m MSI(-X)

On Wed, Jul 09, 2014 at 06:05:03PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> ARM GICv2m specification extends GICv2 to support MSI(-X) with
> a new set of register frames. This patch introduces support for
> the non-secure GICv2m register frame.
>
> The driver currently matchs "arm,gic-400-plus" in device tree binding,
> which implements GICv2m.
>
> The "msi-controller" keyword in ARM GIC devicetree binding is used to indentify
> GIC driver that it should enable MSI(-X) support, The region of GICv2m MSI
> register frame is specified using the register frame index 4 in the device tree.
> MSI support is optional.
>
> Each GIC maintains an "msi_chip" structure. To discover the msi_chip,
> PCI host driver can do the following:
>
> struct device_node *gic_node = of_irq_find_parent(pdev->dev.of_node);
> pcie_bus->msi_chip = of_pci_find_msi_chip_by_node(gic_node);
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/gic.txt | 20 +-
> arch/arm64/Kconfig | 1 +
> drivers/irqchip/Kconfig | 7 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-gic-v2m.c | 251 ++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.h | 13 ++
> drivers/irqchip/irq-gic.c | 23 ++-
> drivers/irqchip/irq-gic.h | 31 +++-
> 8 files changed, 334 insertions(+), 13 deletions(-)
> create mode 100644 drivers/irqchip/irq-gic-v2m.c
> create mode 100644 drivers/irqchip/irq-gic-v2m.h

Applied to irqchip/gic with some minor typos fixed in the commit
message.

thx,

Jason.

2014-07-17 13:18:49

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 3/4 V3] irqchip: gic: Add supports for ARM GICv2m MSI(-X)

Hi Suravee,

Apologies for the late reply on this one. I was hoping that Marc would
be able to take another look at this, but he's away at present.

On Thu, Jul 10, 2014 at 12:05:03AM +0100, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> ARM GICv2m specification extends GICv2 to support MSI(-X) with
> a new set of register frames. This patch introduces support for
> the non-secure GICv2m register frame.
>
> The driver currently matchs "arm,gic-400-plus" in device tree binding,
> which implements GICv2m.
>
> The "msi-controller" keyword in ARM GIC devicetree binding is used to indentify
> GIC driver that it should enable MSI(-X) support, The region of GICv2m MSI
> register frame is specified using the register frame index 4 in the device tree.
> MSI support is optional.
>
> Each GIC maintains an "msi_chip" structure. To discover the msi_chip,
> PCI host driver can do the following:
>
> struct device_node *gic_node = of_irq_find_parent(pdev->dev.of_node);
> pcie_bus->msi_chip = of_pci_find_msi_chip_by_node(gic_node);
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/gic.txt | 20 +-
> arch/arm64/Kconfig | 1 +
> drivers/irqchip/Kconfig | 7 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-gic-v2m.c | 251 ++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.h | 13 ++
> drivers/irqchip/irq-gic.c | 23 ++-
> drivers/irqchip/irq-gic.h | 31 +++-
> 8 files changed, 334 insertions(+), 13 deletions(-)
> create mode 100644 drivers/irqchip/irq-gic-v2m.c
> create mode 100644 drivers/irqchip/irq-gic-v2m.h
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08..d2eea0b 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -12,11 +12,14 @@ Main node required properties:
>
> - compatible : should be one of:
> "arm,gic-400"
> + "arm,gic-400-v2m"

I'm still not entirely comfortable about this, as I was under the
impression that the MSI frame was a block on the side of the GIC rather
than being a composite entity with the rest of the GIC.

Which means that it would be entirely possible to attach multiple copies
of that block, which this binding doesn't cover.

> "arm,cortex-a15-gic"
> "arm,cortex-a9-gic"
> "arm,cortex-a7-gic"
> "arm,arm11mp-gic"
> +
> - interrupt-controller : Identifies the node as an interrupt controller
> +
> - #interrupt-cells : Specifies the number of cells needed to encode an
> interrupt source. The type shall be a <u32> and the value shall be 3.
>
> @@ -37,9 +40,16 @@ Main node required properties:
> the 8 possible cpus attached to the GIC. A bit set to '1' indicated
> the interrupt is wired to that CPU. Only valid for PPI interrupts.
>
> -- reg : Specifies base physical address(s) and size of the GIC registers. The
> - first region is the GIC distributor register base and size. The 2nd region is
> - the GIC cpu interface register base and size.
> +- reg : Specifies base physical address(s) and size of the GIC register frames.
> +
> + Region | Description
> + Index |
> + -------------------------------------------------------------------
> + 0 | GIC distributor register base and size
> + 1 | GIC cpu interface register base and size
> + 2 | VGIC interface control register base and size (Optional)
> + 3 | VGIC CPU interface register base and size (Optional)
> + 4 | GICv2m MSI interface register base and size (Optional)

And describing it as a separate (but related) component would get around
the issue of orthogonality with the GICV and GICH registers.

Nit: can we use the architected prefixes here please? GICC, GICD, GICH,
and GICV respectively for indexes 0-3.

Cheers,
Mark.

2014-07-17 13:19:09

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> This patch set introduces support for MSI(-X) in GICv2m specification,
> which is implemented in some variation of GIC400.
>
> This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> (https://lkml.org/lkml/2014/3/14/320).
>
> Changes in V3:
> * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> (per Jason Cooper request)
> * Misc fix/clean up per Mark Rutland comments
> * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> * Patch 4 is new to the series:
> * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> for Multiple MSI.
> * Add support for Multiple MSI for GICv2m.
>
> Suravee Suthikulpanit (4):
> irqchip: gic: Add binding probe for ARM GIC400
> irqchip: gic: Restructuring ARM GIC code
> irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m

Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
irqchip/gic with irqchip/urgent merged in. To facilitate
testing/merging, I've prepared an unsigned tag for you on the
irqchip/gic branch:

git://git.infradead.org/users/jcooper/linux.git tags/deps-irqchip-gic-3.17-2

Unless you tell me I broke something horribly, this tag and irqchip/gic
up to that point are stable.

thx,

Jason.

2014-07-17 13:31:20

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH] irqchip: gic: Add binding probe for ARM GIC400

On Mon, Jul 14, 2014 at 11:03:03PM +0100, Heiko Stübner wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> Commit 3ab72f9156bb "dt-bindings: add GIC-400 binding" added the
> "arm,gic-400" compatible string, but the corresponding IRQCHIP_DECLARE
> was never added to the gic driver.
>
> Therefore add the missing irqchip declaration for it.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>
> Removed additional empty line and adapted commit message to mark it
> as fixing an issue.
> Signed-off-by: Heiko Stuebner <[email protected]>
> ---
> As I really need this, I took the liberty of adapting the patch accordingly
> to make it apply on top of the current irqchip/for-next (or urgent) and
> explicitly state the fixed issue. Hope that is ok

While this is a fix that we should have, I'm not certain I see why this
is urgent. A dt can already have:

compatible = "arm,gic-400", "arm,cortex-a15-gic";

Which will work _today_, before this patch. If we need to treat the A15
GIC string differently, we'll have to ensure we support the GIC 400
string first.

Thanks,
Mark.

>
> drivers/irqchip/irq-gic.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 9d26643..6ff28b4 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1020,6 +1020,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> gic_cnt++;
> return 0;
> }
> +IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
> --
> 1.9.0
>
>
>
>

2014-07-17 13:56:19

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

Hi Jason,

On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
> On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> > From: Suravee Suthikulpanit <[email protected]>
> >
> > This patch set introduces support for MSI(-X) in GICv2m specification,
> > which is implemented in some variation of GIC400.
> >
> > This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> > (https://lkml.org/lkml/2014/3/14/320).
> >
> > Changes in V3:
> > * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> > (per Jason Cooper request)
> > * Misc fix/clean up per Mark Rutland comments
> > * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> > * Patch 4 is new to the series:
> > * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> > for Multiple MSI.
> > * Add support for Multiple MSI for GICv2m.
> >
> > Suravee Suthikulpanit (4):
> > irqchip: gic: Add binding probe for ARM GIC400
> > irqchip: gic: Restructuring ARM GIC code
> > irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> > irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
>
> Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
> irqchip/gic with irqchip/urgent merged in. To facilitate
> testing/merging, I've prepared an unsigned tag for you on the
> irqchip/gic branch:

I'm a little concerned that this is all going through for v3.17 without
a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.

While his comments on v1 have been addressed, he has not had a chance to
acknowledge the solutions. I appreciate Marc's holiday is unfortunately
timed.

I also have an open concern with the binding with regard to the
orthogonality of GICV GICH and the MSI registers.

Suravee, do you need this urgently for v3.17? I was under the impression
that we wouldn't have full PCIe support by then.

Cheers,
Mark.

2014-07-17 14:12:53

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On Thu, Jul 17, 2014 at 02:55:34PM +0100, Mark Rutland wrote:
> Hi Jason,
>
> On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
> > On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> > > From: Suravee Suthikulpanit <[email protected]>
> > >
> > > This patch set introduces support for MSI(-X) in GICv2m specification,
> > > which is implemented in some variation of GIC400.
> > >
> > > This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> > > (https://lkml.org/lkml/2014/3/14/320).
> > >
> > > Changes in V3:
> > > * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> > > (per Jason Cooper request)
> > > * Misc fix/clean up per Mark Rutland comments
> > > * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> > > * Patch 4 is new to the series:
> > > * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> > > for Multiple MSI.
> > > * Add support for Multiple MSI for GICv2m.
> > >
> > > Suravee Suthikulpanit (4):
> > > irqchip: gic: Add binding probe for ARM GIC400
> > > irqchip: gic: Restructuring ARM GIC code
> > > irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> > > irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
> >
> > Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
> > irqchip/gic with irqchip/urgent merged in. To facilitate
> > testing/merging, I've prepared an unsigned tag for you on the
> > irqchip/gic branch:
>
> I'm a little concerned that this is all going through for v3.17 without
> a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.

Well, that's why it's not in irqchip/core yet. ;-) It can be undone if
needed.

> While his comments on v1 have been addressed, he has not had a chance to
> acknowledge the solutions. I appreciate Marc's holiday is unfortunately
> timed.
>
> I also have an open concern with the binding with regard to the
> orthogonality of GICV GICH and the MSI registers.
>
> Suravee, do you need this urgently for v3.17? I was under the impression
> that we wouldn't have full PCIe support by then.

If Suravee is ok with it, I can drop them for now and he can resend for
v3.18. Since we've worked out a way to merge it all in one window, I
don't think it would hurt anything to wait.

I'll leave these in irqchip/for-next until I hear from Suravee, then
I'll drop the lot till we hear from Marc and look at the timing.

thx,

Jason.

2014-07-17 14:13:58

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH] irqchip: gic: Add binding probe for ARM GIC400

On 7/17/2014 7:48 AM, Jason Cooper wrote:
> On Tue, Jul 15, 2014 at 12:03:03AM +0200, Heiko St?bner wrote:
>> From: Suravee Suthikulpanit <[email protected]>
>>
>> Commit 3ab72f9156bb "dt-bindings: add GIC-400 binding" added the
>> "arm,gic-400" compatible string, but the corresponding IRQCHIP_DECLARE
>> was never added to the gic driver.
>>
>> Therefore add the missing irqchip declaration for it.
>>
>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>>
>> Removed additional empty line and adapted commit message to mark it
>> as fixing an issue.
>> Signed-off-by: Heiko Stuebner <[email protected]>
>> ---
>> As I really need this, I took the liberty of adapting the patch accordingly
>> to make it apply on top of the current irqchip/for-next (or urgent) and
>> explicitly state the fixed issue. Hope that is ok
>>
>> drivers/irqchip/irq-gic.c | 1 +
>> 1 file changed, 1 insertion(+)
>
> Applied to irqchip/urgent with Will's Ack and Cc'd to stable for v3.14+
>
> thx,
>
> Jason.
>
Thank you,

Suravee

2014-07-17 14:51:01

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On 7/17/2014 8:55 AM, Mark Rutland wrote:
> Hi Jason,
>
> On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
>> On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
>>> From: Suravee Suthikulpanit <[email protected]>
>>>
>>> This patch set introduces support for MSI(-X) in GICv2m specification,
>>> which is implemented in some variation of GIC400.
>>>
>>> This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
>>> (https://lkml.org/lkml/2014/3/14/320).
>>>
>>> Changes in V3:
>>> * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
>>> (per Jason Cooper request)
>>> * Misc fix/clean up per Mark Rutland comments
>>> * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
>>> * Patch 4 is new to the series:
>>> * Add ARM64-specific version arch_setup_msi_irqs() to allow support
>>> for Multiple MSI.
>>> * Add support for Multiple MSI for GICv2m.
>>>
>>> Suravee Suthikulpanit (4):
>>> irqchip: gic: Add binding probe for ARM GIC400
>>> irqchip: gic: Restructuring ARM GIC code
>>> irqchip: gic: Add supports for ARM GICv2m MSI(-X)
>>> irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
>>
>> Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
>> irqchip/gic with irqchip/urgent merged in. To facilitate
>> testing/merging, I've prepared an unsigned tag for you on the
>> irqchip/gic branch:
>
> I'm a little concerned that this is all going through for v3.17 without
> a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.

> While his comments on v1 have been addressed, he has not had a chance to
> acknowledge the solutions. I appreciate Marc's holiday is unfortunately
> timed.
>
> I also have an open concern with the binding with regard to the
> orthogonality of GICV GICH and the MSI registers.

The MSI part is normally enabled from the optional "msi-controller"
keyword. It should not really matter which compatible ID it uses.

Ooops. I noticed that was accidentally dropped the check for
"msi-controller" in the gicv2m_of_init() function. I'll send a follow
up patch to fix this.

> Suravee, do you need this urgently for v3.17? I was under the impression
> that we wouldn't have full PCIe support by then.
>

PCI is the dependency for this patch to function. So, it should be
aligned with upstreaming of PCI patches.

Suravee

2014-07-18 09:03:06

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On Thu, Jul 17, 2014 at 03:12:35PM +0100, Jason Cooper wrote:
> On Thu, Jul 17, 2014 at 02:55:34PM +0100, Mark Rutland wrote:
> > Hi Jason,
> >
> > On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
> > > On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> > > > From: Suravee Suthikulpanit <[email protected]>
> > > >
> > > > This patch set introduces support for MSI(-X) in GICv2m specification,
> > > > which is implemented in some variation of GIC400.
> > > >
> > > > This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> > > > (https://lkml.org/lkml/2014/3/14/320).
> > > >
> > > > Changes in V3:
> > > > * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> > > > (per Jason Cooper request)
> > > > * Misc fix/clean up per Mark Rutland comments
> > > > * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> > > > * Patch 4 is new to the series:
> > > > * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> > > > for Multiple MSI.
> > > > * Add support for Multiple MSI for GICv2m.
> > > >
> > > > Suravee Suthikulpanit (4):
> > > > irqchip: gic: Add binding probe for ARM GIC400
> > > > irqchip: gic: Restructuring ARM GIC code
> > > > irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> > > > irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
> > >
> > > Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
> > > irqchip/gic with irqchip/urgent merged in. To facilitate
> > > testing/merging, I've prepared an unsigned tag for you on the
> > > irqchip/gic branch:
> >
> > I'm a little concerned that this is all going through for v3.17 without
> > a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.
>
> Well, that's why it's not in irqchip/core yet. ;-) It can be undone if
> needed.

Ah, perhaps I was a litte premature. :)

> > While his comments on v1 have been addressed, he has not had a chance to
> > acknowledge the solutions. I appreciate Marc's holiday is unfortunately
> > timed.
> >
> > I also have an open concern with the binding with regard to the
> > orthogonality of GICV GICH and the MSI registers.
> >
> > Suravee, do you need this urgently for v3.17? I was under the impression
> > that we wouldn't have full PCIe support by then.
>
> If Suravee is ok with it, I can drop them for now and he can resend for
> v3.18. Since we've worked out a way to merge it all in one window, I
> don't think it would hurt anything to wait.

Ok.

> I'll leave these in irqchip/for-next until I hear from Suravee, then
> I'll drop the lot till we hear from Marc and look at the timing.

Keeping it in for-next for testing sounds like a good idea, but until we
hear from Marc I wouldn't want to see this queued for mainline. So the
above sounds fine to me.

Cheers,
Mark.

2014-07-18 09:05:28

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On Thu, Jul 17, 2014 at 03:48:29PM +0100, Suravee Suthikulanit wrote:
> On 7/17/2014 8:55 AM, Mark Rutland wrote:
> > Hi Jason,
> >
> > On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
> >> On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> >>> From: Suravee Suthikulpanit <[email protected]>
> >>>
> >>> This patch set introduces support for MSI(-X) in GICv2m specification,
> >>> which is implemented in some variation of GIC400.
> >>>
> >>> This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> >>> (https://lkml.org/lkml/2014/3/14/320).
> >>>
> >>> Changes in V3:
> >>> * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> >>> (per Jason Cooper request)
> >>> * Misc fix/clean up per Mark Rutland comments
> >>> * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> >>> * Patch 4 is new to the series:
> >>> * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> >>> for Multiple MSI.
> >>> * Add support for Multiple MSI for GICv2m.
> >>>
> >>> Suravee Suthikulpanit (4):
> >>> irqchip: gic: Add binding probe for ARM GIC400
> >>> irqchip: gic: Restructuring ARM GIC code
> >>> irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> >>> irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
> >>
> >> Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
> >> irqchip/gic with irqchip/urgent merged in. To facilitate
> >> testing/merging, I've prepared an unsigned tag for you on the
> >> irqchip/gic branch:
> >
> > I'm a little concerned that this is all going through for v3.17 without
> > a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.
>
> > While his comments on v1 have been addressed, he has not had a chance to
> > acknowledge the solutions. I appreciate Marc's holiday is unfortunately
> > timed.
> >
> > I also have an open concern with the binding with regard to the
> > orthogonality of GICV GICH and the MSI registers.
>
> The MSI part is normally enabled from the optional "msi-controller"
> keyword. It should not really matter which compatible ID it uses.

I meant the fact that the MSI registers being described in reg[4],
rather than how the driver determines MSI support.

> Ooops. I noticed that was accidentally dropped the check for
> "msi-controller" in the gicv2m_of_init() function. I'll send a follow
> up patch to fix this.

Sure. Whatever happens we should have both the msi-controller property
and the registers for the MSI block before we enable MSI support.

> > Suravee, do you need this urgently for v3.17? I was under the impression
> > that we wouldn't have full PCIe support by then.
> >
>
> PCI is the dependency for this patch to function. So, it should be
> aligned with upstreaming of PCI patches.

Ok, so it sounds like this can wait for the moment (but we should
definitely ensure this gets some testing before then).

Cheers,
Mark

2014-07-18 12:31:56

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On Fri, Jul 18, 2014 at 10:02:05AM +0100, Mark Rutland wrote:
> On Thu, Jul 17, 2014 at 03:12:35PM +0100, Jason Cooper wrote:
> > On Thu, Jul 17, 2014 at 02:55:34PM +0100, Mark Rutland wrote:
> > > Hi Jason,
> > >
> > > On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
> > > > On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> > > > > From: Suravee Suthikulpanit <[email protected]>
> > > > >
> > > > > This patch set introduces support for MSI(-X) in GICv2m specification,
> > > > > which is implemented in some variation of GIC400.
> > > > >
> > > > > This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> > > > > (https://lkml.org/lkml/2014/3/14/320).
> > > > >
> > > > > Changes in V3:
> > > > > * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> > > > > (per Jason Cooper request)
> > > > > * Misc fix/clean up per Mark Rutland comments
> > > > > * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> > > > > * Patch 4 is new to the series:
> > > > > * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> > > > > for Multiple MSI.
> > > > > * Add support for Multiple MSI for GICv2m.
> > > > >
> > > > > Suravee Suthikulpanit (4):
> > > > > irqchip: gic: Add binding probe for ARM GIC400
> > > > > irqchip: gic: Restructuring ARM GIC code
> > > > > irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> > > > > irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
> > > >
> > > > Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
> > > > irqchip/gic with irqchip/urgent merged in. To facilitate
> > > > testing/merging, I've prepared an unsigned tag for you on the
> > > > irqchip/gic branch:
> > >
> > > I'm a little concerned that this is all going through for v3.17 without
> > > a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.
> >
> > Well, that's why it's not in irqchip/core yet. ;-) It can be undone if
> > needed.
>
> Ah, perhaps I was a litte premature. :)
>
> > > While his comments on v1 have been addressed, he has not had a chance to
> > > acknowledge the solutions. I appreciate Marc's holiday is unfortunately
> > > timed.
> > >
> > > I also have an open concern with the binding with regard to the
> > > orthogonality of GICV GICH and the MSI registers.
> > >
> > > Suravee, do you need this urgently for v3.17? I was under the impression
> > > that we wouldn't have full PCIe support by then.
> >
> > If Suravee is ok with it, I can drop them for now and he can resend for
> > v3.18. Since we've worked out a way to merge it all in one window, I
> > don't think it would hurt anything to wait.
>
> Ok.
>
> > I'll leave these in irqchip/for-next until I hear from Suravee, then
> > I'll drop the lot till we hear from Marc and look at the timing.
>
> Keeping it in for-next for testing sounds like a good idea, but until we
> hear from Marc I wouldn't want to see this queued for mainline. So the
> above sounds fine to me.

Ok, here's what I did:

[jason@triton] $ git ldo --graph v3.16-rc1^..irqchip/gic-v2m
* 3e44358c12cc (HEAD, irqchip/gic-v2m) irqchip: gic: Add support for ARM GICv2m MSI(-X)
* fe7ac63fe539 irqchip: gic: Restructuring ARM GIC code
* 6bf0be3f077e Merge branch 'irqchip/urgent' into irqchip/gic
|\
| * 144cb08864ed (irqchip/urgent) irqchip: gic: Add binding probe for ARM GIC400
| * a97e8027b1d2 irqchip: gic: Add support for cortex a7 compatible string
| * 4f4366033945 (tag: irqchip-urgent-3.16) irqchip: spear_shirq: Fix interrupt offset
| * 00ac20279174 irqchip: brcmstb-l2: Level-2 interrupts are edge sensitive
| * b73842b75646 irqchip: armada-370-xp: Mask all interrupts during initialization.
* | 021f653791ad (tag: deps-irqchip-gic-3.17, irqchip/gic) irqchip: gic-v3: Initial support for GICv3
* | d51d0af43b30 irqchip: gic: Move some bits of GICv2 to a library-type file
|/
* 7171511eaec5 (tag: v3.16-rc1) Linux 3.16-rc1

remote branches removed for clarity.

Basically, I reset irqchip/gic back to tags/deps-irqchip-gic-3.17 (The
stable tag for GICv3 support), and made the MSI series irqchip/gic-v2m.

I've also removed the dep tag for gic-v2m (tags/dep-irqchip-gic-3.17-2).
Please consider irqchip/gic-v2m to be completely unstable and drop-able.
It'll be in -next, but until I hear from Marc, it's not going anywhere.

No commit IDs have changed during this process. The only problem that
may occur is if the guys depending on GICv3 pulled the branch *and*
updated it yesterday. If they pulled the tag as I provided, then
they're fine.

Sorry for the confusion, I hope I've made it clear above.

thx,

Jason.
>
> Cheers,
> Mark.

2014-07-18 12:41:15

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 0/4 V3] irqchip: gic: Introduce ARM GICv2m MSI(-X) support

On Fri, Jul 18, 2014 at 01:31:40PM +0100, Jason Cooper wrote:
> On Fri, Jul 18, 2014 at 10:02:05AM +0100, Mark Rutland wrote:
> > On Thu, Jul 17, 2014 at 03:12:35PM +0100, Jason Cooper wrote:
> > > On Thu, Jul 17, 2014 at 02:55:34PM +0100, Mark Rutland wrote:
> > > > Hi Jason,
> > > >
> > > > On Thu, Jul 17, 2014 at 02:18:54PM +0100, Jason Cooper wrote:
> > > > > On Wed, Jul 09, 2014 at 06:05:00PM -0500, [email protected] wrote:
> > > > > > From: Suravee Suthikulpanit <[email protected]>
> > > > > >
> > > > > > This patch set introduces support for MSI(-X) in GICv2m specification,
> > > > > > which is implemented in some variation of GIC400.
> > > > > >
> > > > > > This depends on and has been tested with the V7 of"Add support for PCI in AArch64"
> > > > > > (https://lkml.org/lkml/2014/3/14/320).
> > > > > >
> > > > > > Changes in V3:
> > > > > > * Rebase to git://git.infradead.org/users/jcooper/linux.git irqchip/gic
> > > > > > (per Jason Cooper request)
> > > > > > * Misc fix/clean up per Mark Rutland comments
> > > > > > * Minor Clean up in the driver/irqchip/irq-gic-v2m.c: alloc_msi_irqs()
> > > > > > * Patch 4 is new to the series:
> > > > > > * Add ARM64-specific version arch_setup_msi_irqs() to allow support
> > > > > > for Multiple MSI.
> > > > > > * Add support for Multiple MSI for GICv2m.
> > > > > >
> > > > > > Suravee Suthikulpanit (4):
> > > > > > irqchip: gic: Add binding probe for ARM GIC400
> > > > > > irqchip: gic: Restructuring ARM GIC code
> > > > > > irqchip: gic: Add supports for ARM GICv2m MSI(-X)
> > > > > > irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m
> > > > >
> > > > > Ok, patch #1 applied to irqchip/urgent. Patches 2 and 3 applied to
> > > > > irqchip/gic with irqchip/urgent merged in. To facilitate
> > > > > testing/merging, I've prepared an unsigned tag for you on the
> > > > > irqchip/gic branch:
> > > >
> > > > I'm a little concerned that this is all going through for v3.17 without
> > > > a {Reviewed,Acked}-by from Marc or anyone working with GIC{,v2m}.
> > >
> > > Well, that's why it's not in irqchip/core yet. ;-) It can be undone if
> > > needed.
> >
> > Ah, perhaps I was a litte premature. :)
> >
> > > > While his comments on v1 have been addressed, he has not had a chance to
> > > > acknowledge the solutions. I appreciate Marc's holiday is unfortunately
> > > > timed.
> > > >
> > > > I also have an open concern with the binding with regard to the
> > > > orthogonality of GICV GICH and the MSI registers.
> > > >
> > > > Suravee, do you need this urgently for v3.17? I was under the impression
> > > > that we wouldn't have full PCIe support by then.
> > >
> > > If Suravee is ok with it, I can drop them for now and he can resend for
> > > v3.18. Since we've worked out a way to merge it all in one window, I
> > > don't think it would hurt anything to wait.
> >
> > Ok.
> >
> > > I'll leave these in irqchip/for-next until I hear from Suravee, then
> > > I'll drop the lot till we hear from Marc and look at the timing.
> >
> > Keeping it in for-next for testing sounds like a good idea, but until we
> > hear from Marc I wouldn't want to see this queued for mainline. So the
> > above sounds fine to me.
>
> Ok, here's what I did:
>
> [jason@triton] $ git ldo --graph v3.16-rc1^..irqchip/gic-v2m
> * 3e44358c12cc (HEAD, irqchip/gic-v2m) irqchip: gic: Add support for ARM GICv2m MSI(-X)
> * fe7ac63fe539 irqchip: gic: Restructuring ARM GIC code
> * 6bf0be3f077e Merge branch 'irqchip/urgent' into irqchip/gic
> |\
> | * 144cb08864ed (irqchip/urgent) irqchip: gic: Add binding probe for ARM GIC400
> | * a97e8027b1d2 irqchip: gic: Add support for cortex a7 compatible string
> | * 4f4366033945 (tag: irqchip-urgent-3.16) irqchip: spear_shirq: Fix interrupt offset
> | * 00ac20279174 irqchip: brcmstb-l2: Level-2 interrupts are edge sensitive
> | * b73842b75646 irqchip: armada-370-xp: Mask all interrupts during initialization.
> * | 021f653791ad (tag: deps-irqchip-gic-3.17, irqchip/gic) irqchip: gic-v3: Initial support for GICv3
> * | d51d0af43b30 irqchip: gic: Move some bits of GICv2 to a library-type file
> |/
> * 7171511eaec5 (tag: v3.16-rc1) Linux 3.16-rc1
>
> remote branches removed for clarity.
>
> Basically, I reset irqchip/gic back to tags/deps-irqchip-gic-3.17 (The
> stable tag for GICv3 support), and made the MSI series irqchip/gic-v2m.
>
> I've also removed the dep tag for gic-v2m (tags/dep-irqchip-gic-3.17-2).
> Please consider irqchip/gic-v2m to be completely unstable and drop-able.
> It'll be in -next, but until I hear from Marc, it's not going anywhere.
>
> No commit IDs have changed during this process. The only problem that
> may occur is if the guys depending on GICv3 pulled the branch *and*
> updated it yesterday. If they pulled the tag as I provided, then
> they're fine.

Ok, let's hope that's the case for now. I'll keep an eye out just in
case.

> Sorry for the confusion, I hope I've made it clear above.

Nothing to worry about, thanks for dealing with this quickly and
effectively. I think we're on the same page now.

Cheers,
Mark.

2014-07-30 14:58:14

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 3/4 V3] irqchip: gic: Add supports for ARM GICv2m MSI(-X)

On Thu, Jul 10 2014 at 12:05:03 am BST, "[email protected]" <[email protected]> wrote:

Hi Suravee,

> From: Suravee Suthikulpanit <[email protected]>
>
> ARM GICv2m specification extends GICv2 to support MSI(-X) with
> a new set of register frames. This patch introduces support for
> the non-secure GICv2m register frame.
>
> The driver currently matchs "arm,gic-400-plus" in device tree binding,
> which implements GICv2m.
>
> The "msi-controller" keyword in ARM GIC devicetree binding is used to indentify
> GIC driver that it should enable MSI(-X) support, The region of GICv2m MSI
> register frame is specified using the register frame index 4 in the device tree.
> MSI support is optional.
>
> Each GIC maintains an "msi_chip" structure. To discover the msi_chip,
> PCI host driver can do the following:
>
> struct device_node *gic_node = of_irq_find_parent(pdev->dev.of_node);
> pcie_bus->msi_chip = of_pci_find_msi_chip_by_node(gic_node);
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/gic.txt | 20 +-
> arch/arm64/Kconfig | 1 +
> drivers/irqchip/Kconfig | 7 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-gic-v2m.c | 251 ++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.h | 13 ++
> drivers/irqchip/irq-gic.c | 23 ++-
> drivers/irqchip/irq-gic.h | 31 +++-
> 8 files changed, 334 insertions(+), 13 deletions(-)
> create mode 100644 drivers/irqchip/irq-gic-v2m.c
> create mode 100644 drivers/irqchip/irq-gic-v2m.h
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08..d2eea0b 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -12,11 +12,14 @@ Main node required properties:
>
> - compatible : should be one of:
> "arm,gic-400"
> + "arm,gic-400-v2m"
> "arm,cortex-a15-gic"
> "arm,cortex-a9-gic"
> "arm,cortex-a7-gic"
> "arm,arm11mp-gic"
> +
> - interrupt-controller : Identifies the node as an interrupt controller
> +
> - #interrupt-cells : Specifies the number of cells needed to encode an
> interrupt source. The type shall be a <u32> and the value shall be 3.
>
> @@ -37,9 +40,16 @@ Main node required properties:
> the 8 possible cpus attached to the GIC. A bit set to '1' indicated
> the interrupt is wired to that CPU. Only valid for PPI interrupts.
>
> -- reg : Specifies base physical address(s) and size of the GIC registers. The
> - first region is the GIC distributor register base and size. The 2nd region is
> - the GIC cpu interface register base and size.
> +- reg : Specifies base physical address(s) and size of the GIC register frames.
> +
> + Region | Description
> + Index |
> + -------------------------------------------------------------------
> + 0 | GIC distributor register base and size
> + 1 | GIC cpu interface register base and size
> + 2 | VGIC interface control register base and size (Optional)
> + 3 | VGIC CPU interface register base and size (Optional)
> + 4 | GICv2m MSI interface register base and size (Optional)

Given that the v2m block is somehow bolted on the side of a standard
GICv2, I'd rather see it as a subnode of the main GIC.

Then you could directly call into the v2m layer to initialize it,
instead of the odd "reverse probing" you're using here...

> Optional
> - interrupts : Interrupt source of the parent interrupt controller on
> @@ -55,6 +65,10 @@ Optional
> by a crossbar/multiplexer preceding the GIC. The GIC irq
> input line is assigned dynamically when the corresponding
> peripheral's crossbar line is mapped.
> +
> +- msi-controller : Identifies the node as an MSI controller.
> + (Required for GICv2m)
> +
> Example:
>
> intc: interrupt-controller@fff11000 {
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index be52492..0f9b11d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -9,6 +9,7 @@ config ARM64
> select ARM_AMBA
> select ARM_ARCH_TIMER
> select ARM_GIC
> + select ARM_GIC_V2M if (PCI && PCI_MSI)
> select ARM_GIC_V3
> select BUILDTIME_EXTABLE_SORT
> select CLONE_BACKWARDS
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 7f0c2a3..274910d 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -7,6 +7,13 @@ config ARM_GIC
> select IRQ_DOMAIN
> select MULTI_IRQ_HANDLER
>
> +config ARM_GIC_V2M
> + bool
> + select IRQ_DOMAIN
> + select MULTI_IRQ_HANDLER
> + depends on ARM_GIC
> + depends on PCI && PCI_MSI
> +
> config GIC_NON_BANKED
> bool
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index c57e642..09c035a 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o
> obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi-nmi.o
> 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_NVIC) += irq-nvic.o
> obj-$(CONFIG_ARM_VIC) += irq-vic.o
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> new file mode 100644
> index 0000000..e54ca1d
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -0,0 +1,251 @@
> +/*
> + * ARM GIC v2m MSI(-X) support
> + * Support for Message Signalelled Interrupts for systems that
> + * implement ARM Generic Interrupt Controller: GICv2m.
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + * Authors: Suravee Suthikulpanit <[email protected]>
> + * Harish Kasiviswanathan <[email protected]>
> + * Brandon Anderson <[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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pci.h>
> +#include <linux/irq.h>
> +#include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/bitmap.h>
> +
> +#include "irqchip.h"
> +#include "irq-gic.h"
> +
> +/*
> +* MSI_TYPER:
> +* [31:26] Reserved
> +* [25:16] lowest SPI assigned to MSI
> +* [15:10] Reserved
> +* [9:0] Numer of SPIs assigned to MSI
> +*/
> +#define V2M_MSI_TYPER 0x008
> +#define V2M_MSI_TYPER_BASE_SHIFT (16)
> +#define V2M_MSI_TYPER_BASE_MASK (0x3FF)
> +#define V2M_MSI_TYPER_NUM_MASK (0x3FF)
> +#define V2M_MSI_SETSPI_NS 0x040
> +#define V2M_MIN_SPI 32
> +#define V2M_MAX_SPI 1019
> +
> +#define GIC_OF_MSIV2M_RANGE_INDEX 4
> +
> +/*
> + * alloc_msi_irq - Allocate MSIs from avaialbe MSI bitmap.
> + * @data: Pointer to v2m_data
> + * @nvec: Number of interrupts to allocate
> + * @irq: Pointer to the allocated irq
> + *
> + * Allocates interrupts only if the contiguous range of MSIs
> + * with specified nvec are available. Otherwise return the number
> + * of available interrupts. If none are available, then returns -ENOENT.
> + */
> +static int alloc_msi_irq(struct v2m_data *data, int nvec, int *irq)
> +{
> + int size = data->nr_spis;
> + int next = size, i = nvec, ret;
> +
> + /* We should never allocate more than available nr_spis */
> + if (i >= size)
> + i = size;
> +
> + spin_lock(&data->msi_cnt_lock);
> +
> + for (; i > 0; i--) {
> + next = bitmap_find_next_zero_area(data->bm,
> + size, 0, i, 0);
> + if (next < size)
> + break;
> + }
> +
> + if (i != nvec) {
> + ret = i ? : -ENOENT;
> + } else {
> + bitmap_set(data->bm, next, nvec);
> + *irq = data->spi_start + next;
> + ret = 0;
> + }
> +
> + spin_unlock(&data->msi_cnt_lock);
> +
> + return ret;
> +}
> +
> +static struct v2m_data *to_v2m_data(struct msi_chip *chip)
> +{
> + struct gic_chip_data *gic = container_of(chip, struct gic_chip_data,
> + msi_chip);
> + return &gic->v2m_data;
> +}
> +
> +static void gicv2m_teardown_msi_irq(struct msi_chip *chip, unsigned int irq)
> +{
> + int pos;
> + struct v2m_data *data = to_v2m_data(chip);
> +
> + spin_lock(&data->msi_cnt_lock);
> +
> + pos = irq - data->spi_start;
> + if (pos >= 0 && pos < data->nr_spis)
> + bitmap_clear(data->bm, pos, 1);
> +
> + spin_unlock(&data->msi_cnt_lock);
> +}
> +
> +static int gicv2m_setup_msi_irq(struct msi_chip *chip, struct pci_dev *pdev,
> + struct msi_desc *desc)
> +{
> + int avail, irq = 0;
> + struct msi_msg msg;
> + phys_addr_t addr;
> + struct v2m_data *data = to_v2m_data(chip);
> +
> + if (!desc) {
> + dev_err(&pdev->dev,
> + "GICv2m: MSI setup failed. Invalid msi descriptor\n");
> + return -EINVAL;
> + }
> +
> + avail = alloc_msi_irq(data, 1, &irq);
> + if (avail != 0) {
> + dev_err(&pdev->dev,
> + "GICv2m: MSI setup failed. Cannnot allocate IRQ\n");
> + return -ENOSPC;
> + }
> +
> + irq_set_chip_data(irq, chip);
> + irq_set_msi_desc(irq, desc);
> + irq_set_irq_type(irq, IRQ_TYPE_EDGE_RISING);
> +
> + addr = data->res.start + V2M_MSI_SETSPI_NS;
> +
> + msg.address_hi = (u32)(addr >> 32);
> + msg.address_lo = (u32)(addr);
> + msg.data = irq;
> + write_msi_msg(irq, &msg);
> +
> + return 0;
> +}
> +
> +static int __init
> +gicv2m_msi_init(struct device_node *node, struct v2m_data *v2m)
> +{
> + unsigned int val;
> +
> + if (of_address_to_resource(node, GIC_OF_MSIV2M_RANGE_INDEX,
> + &v2m->res)) {
> + pr_err("GICv2m: Failed locate GICv2m MSI register frame\n");
> + return -EINVAL;
> + }
> +
> + v2m->base = of_iomap(node, GIC_OF_MSIV2M_RANGE_INDEX);
> + if (!v2m->base) {
> + pr_err("GICv2m: Failed to map GIC MSI registers\n");
> + return -EINVAL;
> + }
> +
> + val = readl_relaxed(v2m->base + V2M_MSI_TYPER);
> + if (!val) {
> + pr_warn("GICv2m: Failed to read V2M_MSI_TYPER register\n");
> + return -EINVAL;
> + }
> +
> + v2m->spi_start = (val >> V2M_MSI_TYPER_BASE_SHIFT) &
> + V2M_MSI_TYPER_BASE_MASK;
> + v2m->nr_spis = val & V2M_MSI_TYPER_NUM_MASK;
> + if ((v2m->spi_start < V2M_MIN_SPI) || (v2m->nr_spis >= V2M_MAX_SPI)) {
> + pr_err("GICv2m: Invalid MSI_TYPER (%#x)\n", val);
> + return -EINVAL;
> + }
> +
> + v2m->bm = kzalloc(sizeof(long) * BITS_TO_LONGS(v2m->nr_spis),
> + GFP_KERNEL);
> + if (!v2m->bm) {
> + pr_err("GICv2m: Failed to allocate MSI bitmap\n");
> + return -ENOMEM;
> + }
> +
> + spin_lock_init(&v2m->msi_cnt_lock);
> +
> + pr_info("GICv2m: SPI range [%d:%d]\n",
> + v2m->spi_start, (v2m->spi_start + v2m->nr_spis));
> +
> + return 0;
> +}

This function is assuming that you will only see one single MSI frame
here. Is there any chance that there would be more than one in a system?

> +
> +static void gicv2m_mask_irq(struct irq_data *d)
> +{
> + gic_mask_irq(d);
> + if (d->msi_desc)
> + mask_msi_irq(d);
> +}
> +
> +static void gicv2m_unmask_irq(struct irq_data *d)
> +{
> + gic_unmask_irq(d);
> + if (d->msi_desc)
> + unmask_msi_irq(d);
> +}
> +
> +static struct irq_chip gicv2m_chip = {
> + .name = "GICv2m",
> + .irq_mask = gicv2m_mask_irq,
> + .irq_unmask = gicv2m_unmask_irq,
> + .irq_eoi = gic_eoi_irq,
> + .irq_set_type = gic_set_type,
> + .irq_retrigger = gic_retrigger,

I think you can loose the retrigger here.

> +#ifdef CONFIG_SMP
> + .irq_set_affinity = gic_set_affinity,
> +#endif
> +#ifdef CONFIG_PM
> + .irq_set_wake = gic_set_wake,
> +#endif
> +};
> +
> +#ifdef CONFIG_OF
> +static int __init
> +gicv2m_of_init(struct device_node *node, struct device_node *parent)
> +{
> + struct gic_chip_data *gic;
> + int ret;
> +
> + ret = _gic_of_init(node, parent, &gicv2m_chip, &gic);
> + if (ret) {
> + pr_err("GICv2m: Failed to initialize GIC\n");
> + return ret;
> + }
> +
> + gic->msi_chip.owner = THIS_MODULE;
> + gic->msi_chip.of_node = node;
> + gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;
> + gic->msi_chip.teardown_irq = gicv2m_teardown_msi_irq;
> + ret = of_pci_msi_chip_add(&gic->msi_chip);
> + if (ret) {
> + /* MSI is optional and not supported here */
> + pr_info("GICv2m: MSI is not supported.\n");
> + return 0;
> + }
> +
> + ret = gicv2m_msi_init(node, &gic->v2m_data);
> + if (ret)
> + return ret;
> + return ret;
> +}
> +
> +IRQCHIP_DECLARE(arm_gic_400_v2m, "arm,gic-400-v2m", gicv2m_of_init);

So if you follow my advise of reversing your probing and call into the
v2m init from the main GIC driver, you could take a irq_chip as a
parameter, and use it to populate the v2m irq_chip, only overriding the
two methods that actually differ.

This would have the net effect of completely dropping patch #2, which
becomes effectively useless.

> +#endif /* CONFIG_OF */
> diff --git a/drivers/irqchip/irq-gic-v2m.h b/drivers/irqchip/irq-gic-v2m.h
> new file mode 100644
> index 0000000..2d93a87
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v2m.h
> @@ -0,0 +1,13 @@
> +#ifndef _IRQ_GIC_V2M_H_
> +#define _IRQ_GIC_V2M_H_
> +
> +struct v2m_data {
> + spinlock_t msi_cnt_lock;
> + struct resource res; /* GICv2m resource */
> + void __iomem *base; /* GICv2m virt address */
> + unsigned int spi_start; /* The SPI number that MSIs start */
> + unsigned int nr_spis; /* The number of SPIs for MSIs */
> + unsigned long *bm; /* MSI vector bitmap */
> +};
> +
> +#endif /*_IRQ_GIC_V2M_H_*/
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 966e1d5..a054e0d 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -111,15 +111,34 @@ static inline void gic_set_base_accessor(struct gic_chip_data *data,
> #define gic_set_base_accessor(d, f)
> #endif
>
> +static inline
> +struct gic_chip_data *irq_data_get_gic_chip_data(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data;
> + struct msi_chip *mchip;
> +
> + /*
> + * For MSI, irq_data.chip_data points to struct msi_chip.
> + * For non-MSI, irq_data.chip_data points to struct gic_chip_data.
> + */
> + if (d->msi_desc) {
> + mchip = irq_data_get_irq_chip_data(d);
> + gic_data = container_of(mchip, struct gic_chip_data, msi_chip);
> + } else {
> + gic_data = irq_data_get_irq_chip_data(d);
> + }
> + return gic_data;
> +}
> +
> static inline void __iomem *gic_dist_base(struct irq_data *d)
> {
> - struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
> return gic_data_dist_base(gic_data);
> }
>
> static inline void __iomem *gic_cpu_base(struct irq_data *d)
> {
> - struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + struct gic_chip_data *gic_data = irq_data_get_gic_chip_data(d);
> return gic_data_cpu_base(gic_data);
> }
>
> diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
> index a4beb4a..1c6547d 100644
> --- a/drivers/irqchip/irq-gic.h
> +++ b/drivers/irqchip/irq-gic.h
> @@ -8,6 +8,17 @@ union gic_base {
> void __percpu * __iomem *percpu_base;
> };
>
> +#ifdef CONFIG_ARM_GIC_V2M
> +struct v2m_data {
> + spinlock_t msi_cnt_lock;
> + struct resource res; /* GICv2m resource */
> + void __iomem *base; /* GICv2m virt address */
> + unsigned int spi_start; /* The SPI number that MSIs start */
> + unsigned int nr_spis; /* The number of SPIs for MSIs */
> + unsigned long *bm; /* MSI vector bitmap */
> +};
> +#endif
> +
> struct gic_chip_data {
> union gic_base dist_base;
> union gic_base cpu_base;
> @@ -20,12 +31,23 @@ struct gic_chip_data {
> #endif
> struct irq_domain *domain;
> unsigned int gic_irqs;
> - struct irq_chip *irq_chip;
> #ifdef CONFIG_GIC_NON_BANKED
> void __iomem *(*get_base)(union gic_base *);
> #endif
> + struct irq_chip *irq_chip;
> + struct msi_chip msi_chip;
> +#ifdef CONFIG_ARM_GIC_V2M
> + struct v2m_data v2m_data;
> +#endif

Why isn't msi_chip directly part of v2m_data? I think that would make
some of the code slightly clearer, and avoid polluting unsuspecting
architectures...

> };
>
> +#ifdef CONFIG_OF
> +int _gic_of_init(struct device_node *node,
> + struct device_node *parent,
> + struct irq_chip *chip,
> + struct gic_chip_data **gic) __init;
> +#endif
> +
> void gic_mask_irq(struct irq_data *d);
> void gic_unmask_irq(struct irq_data *d);
> void gic_eoi_irq(struct irq_data *d);
> @@ -42,11 +64,4 @@ int gic_set_affinity(struct irq_data *d,
> int gic_set_wake(struct irq_data *d, unsigned int on);
> #endif
>
> -#ifdef CONFIG_OF
> -int _gic_of_init(struct device_node *node,
> - struct device_node *parent,
> - struct irq_chip *chip,
> - struct gic_chip_data **gic) __init;
> -#endif
> -
> #endif /* _IRQ_GIC_H_ */

What is the purpose of this move?

Thanks,

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

2014-07-30 15:16:33

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 4/4 V3] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m

On Thu, Jul 10 2014 at 12:05:04 am BST, "[email protected]" <[email protected]> wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> This patch extend GICv2m MSI to support multiple MSI in ARM64.
>
> This requires the common arch_setup_msi_irqs() to be overwriten
> with ARM64 version which does not return 1 for PCI_CAP_ID_MSI and
> nvec > 1.
>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> arch/arm64/include/asm/msi.h | 15 ++++++++
> arch/arm64/kernel/Makefile | 1 +
> arch/arm64/kernel/msi.c | 57 ++++++++++++++++++++++++++++++
> drivers/irqchip/irq-gic-v2m.c | 80 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 153 insertions(+)
> create mode 100644 arch/arm64/include/asm/msi.h
> create mode 100644 arch/arm64/kernel/msi.c
>
> diff --git a/arch/arm64/include/asm/msi.h b/arch/arm64/include/asm/msi.h
> new file mode 100644
> index 0000000..2a0944a
> --- /dev/null
> +++ b/arch/arm64/include/asm/msi.h
> @@ -0,0 +1,15 @@
> +#ifndef _ASM_ARM64_MSI_H_
> +#define _ASM_ARM64_MSI_H_
> +
> +struct pci_dev;
> +struct msi_desc;
> +
> +struct arm64_msi_ops {
> + int (*setup_msi_irqs)(struct pci_dev *dev, int nvec, int type);
> + void (*teardown_msi_irqs)(struct pci_dev *dev);
> +};
> +
> +extern struct arm64_msi_ops arm64_msi;
> +extern int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
> +
> +#endif /* _ASM_ARM64_MSI_H_ */
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index cdaedad..0636e27 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -29,6 +29,7 @@ arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND) += sleep.o suspend.o
> arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> arm64-obj-$(CONFIG_KGDB) += kgdb.o
> arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o
> +arm64-obj-$(CONFIG_PCI_MSI) += msi.o
>
> obj-y += $(arm64-obj-y) vdso/
> obj-m += $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/msi.c b/arch/arm64/kernel/msi.c
> new file mode 100644
> index 0000000..ed62397
> --- /dev/null
> +++ b/arch/arm64/kernel/msi.c
> @@ -0,0 +1,57 @@
> +/*
> + * ARM64 architectural MSI implemention
> + *
> + * Support for Message Signalelled Interrupts for systems that
> + * implement ARM Generic Interrupt Controller: GICv2m.
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + * Authors: Suravee Suthikulpanit <[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.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +
> +#include <asm/msi.h>
> +
> +/*
> + * ARM64 function for seting up MSI irqs.
> + * Copied from driver/pci/msi.c: arch_setup_msi_irqs().
> + */
> +int arm64_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> +{
> + struct msi_desc *entry;
> + int ret;
> +
> + if (type == PCI_CAP_ID_MSI && nvec > 1)
> + return 1;
> +
> + list_for_each_entry(entry, &dev->msi_list, list) {
> + ret = arch_setup_msi_irq(dev, entry);
> + if (ret < 0)
> + return ret;
> + if (ret > 0)
> + return -ENOSPC;
> + }
> +
> + return 0;
> +}
> +
> +struct arm64_msi_ops arm64_msi = {
> + .setup_msi_irqs = arm64_setup_msi_irqs,
> + .teardown_msi_irqs = default_teardown_msi_irqs,
> +};
> +
> +int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
> +{
> + return arm64_msi.setup_msi_irqs(dev, nvec, type);
> +}
> +
> +void arch_teardown_msi_irqs(struct pci_dev *dev)
> +{
> + arm64_msi.teardown_msi_irqs(dev);
> +}
> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
> index e54ca1d..9d88ad9 100644
> --- a/drivers/irqchip/irq-gic-v2m.c
> +++ b/drivers/irqchip/irq-gic-v2m.c
> @@ -24,6 +24,10 @@
> #include <linux/of_pci.h>
> #include <linux/bitmap.h>
>
> +#ifdef CONFIG_ARM64
> +#include <asm/msi.h>
> +#endif
> +
> #include "irqchip.h"
> #include "irq-gic.h"
>
> @@ -216,6 +220,80 @@ static struct irq_chip gicv2m_chip = {
> #endif
> };
>
> +
> +/*
> + * _gicv2m_setup_msi_irqs - Setup MSI interrupts for the given PCI device.
> + * This overrides the weak definition in ./drivers/pci/msi.c.
> + * If nvec interrupts are irqable, then assign it to PCI device.
> + * Otherwise return error.
> + *
> + * @pdev: PCI device which is requesting to enable MSI
> + * @nvec: number of MSI vectors
> + */
> +static int _gicv2m_setup_msi_irqs(struct pci_dev *pdev, int nvec)
> +{
> + int irq = 0, nvec_pow2 = 0, avail;
> + int i = 0;
> + struct msi_msg msg;
> + phys_addr_t addr;
> + struct msi_desc *entry;
> + struct msi_chip *chip = pdev->bus->msi;
> + struct v2m_data *data = to_v2m_data(chip);
> +
> + BUG_ON(list_empty(&pdev->msi_list));
> + WARN_ON(!list_is_singular(&pdev->msi_list));
> +
> + entry = list_first_entry(&pdev->msi_list, struct msi_desc, list);
> + WARN_ON(entry->irq);
> + WARN_ON(entry->msi_attrib.multiple);
> + WARN_ON(entry->nvec_used);
> + WARN_ON(!entry->dev);
> +
> + avail = alloc_msi_irq(data, nvec, &irq);
> + if (avail != 0) {
> + dev_err(&pdev->dev,
> + "GICv2m: Failed to allocate %d irqs.\n", nvec);
> + return avail;
> + }
> +
> + /* Set lowest of the new interrupts assigned to the PCI device */
> + nvec_pow2 = __roundup_pow_of_two(nvec);
> + entry->nvec_used = nvec;
> + entry->msi_attrib.multiple = ilog2(nvec_pow2);
> +
> + for (i = 0; i < nvec; i++) {
> + irq_set_chip_data(irq+i, chip);
> + if (irq_set_msi_desc_off(irq, i, entry)) {
> + dev_err(&pdev->dev,
> + "GICv2m: Failed to set up MSI irq %d\n",
> + (irq+i));
> + return -EINVAL;
> + }
> +
> + irq_set_irq_type((irq+i), IRQ_TYPE_EDGE_RISING);
> + }
> +
> + addr = data->res.start + V2M_MSI_SETSPI_NS;
> + msg.address_hi = (u32)(addr >> 32);
> + msg.address_lo = (u32)(addr);
> + msg.data = irq;
> + write_msi_msg(irq, &msg);
> +
> + return 0;
> +}
> +
> +static int
> +gicv2m_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
> +{
> + int ret;
> +
> + if (type == PCI_CAP_ID_MSI)
> + ret = _gicv2m_setup_msi_irqs(pdev, nvec);
> + else
> + ret = arm64_setup_msi_irqs(pdev, nvec, type);
> + return ret;
> +}
> +
> #ifdef CONFIG_OF
> static int __init
> gicv2m_of_init(struct device_node *node, struct device_node *parent)
> @@ -229,6 +307,8 @@ gicv2m_of_init(struct device_node *node, struct device_node *parent)
> return ret;
> }
>
> + arm64_msi.setup_msi_irqs = &gicv2m_setup_msi_irqs;
> +
> gic->msi_chip.owner = THIS_MODULE;
> gic->msi_chip.of_node = node;
> gic->msi_chip.setup_irq = gicv2m_setup_msi_irq;

Why do we need this complexity at all? Is there any case where we'd want
to limit ourselves to a single vector for MSI? arm64 is a new enough
architecture so that we can expect all interrupt controllers to cope
with that.

I think that would allow you to turn gicv2m_setup_msi_irqs and
gicv2m_setup_msi_irq into the same function, wouldn't it?

Thanks,

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