irqchip: gic: killing gic_arch_extn, slowly

The gic_arch_extn hack that a number of platform use has been nagging me for too long. It is only there for the benefit of a few platform, and yet it impacts all GIC users. Moreover, it gives people the wrong idea ("let's use it to put some new custom hack in there"...).

But now that stacked irq domains have landed in -next, the time has come for gic_arch_extn to meet the Big Bit Bucket.

This patch series takes several steps towards the elimination of gic_arch_extn:

- moves Tegra's legacy interrupt controller support to drivers/irqchip, implementing a stacked domain on top of the standard GIC.

- OMAP, imx6 and exynos are also converted to stacked domains, but their implementation is left in place (the code is far too intricately mixed with other details of the platform for me to even try to move it).

- shmobile, ux500 and zynq are only slightly modified.

- The GIC itself is cleaned up, and some other bits and bobs are adjusted for a good measure.

It is worth realizing that:

- I haven't been able to test this as much as I would have wanted to (it's only been tested on tegra2 and omap5).

- I've created DT bindings when needed, updated existing ones, but I haven't created a binding for platforms that already used an undocumented one (imx6, I'm looking at you).

- I've relaxed quite a bit of the locking in the GIC code. I believe this is safe, but someone else should give it a long hard look.

- This actively *breaks* existing setups. Once you boot a new kernel with an old DT, suspend/resume *will* be broken. Old kernels on a new DT won't even boot! You've been warned. This really outline the necessity of actually describing the HW in device trees...

As for the patches, they are on top of 3.18-rc7 + tip/irq/irqdomain-arm.

I've pushed the code to:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/die-gic-arch-extn-die-die-die

Comments welcome,

	 M.

Marc Zyngier (16):
ARM: tegra: irq: nuke leftovers from non-DT support
irqchip: tegra: add DT-based support for legacy interrupt controller
ARM: tegra: skip gic_arch_extn setup if DT has a LIC node
ARM: tegra: update DTs to expose legacy interrupt controller
DT: tegra: add binding for the legacy interrupt controller
ARM: tegra: remove old LIC support
ARM: omap: convert wakeupgen to stacked domains
DT: omap4/5: add binding for the wake-up generator
ARM: imx6: convert wakeupgen to stacked domains
ARM: exynos4/5: convert pmu wakeup to stacked domains
DT: exynos: update PMU binding
irqchip: gic: add an entry point to set up irqchip flags
ARM: shmobile: remove use of gic_arch_extn.irq_set_wake
ARM: ux500: switch from gic_arch_extn to gic_set_irqchip_flags
ARM: zynq: switch from gic_arch_extn to gic_set_irqchip_flags
irqchip: gic: Drop support for gic_arch_extn

 .../devicetree/bindings/arm/samsung/pmu.txt        |  13 +
 .../interrupt-controller/nvidia,tegra-ictlr.txt    |  39 +++
 .../interrupt-controller/ti,omap4-wugen-mpu        |  32 ++
 arch/arm/boot/dts/am4372.dtsi                      |  11 +-
 arch/arm/boot/dts/dra7.dtsi                        |  12 +-
 arch/arm/boot/dts/exynos4.dtsi                     |   3 +
 arch/arm/boot/dts/exynos5250.dtsi                  |   3 +
 arch/arm/boot/dts/imx6qdl.dtsi                     |   6 +-
 arch/arm/boot/dts/imx6sl.dtsi                      |   5 +-
 arch/arm/boot/dts/imx6sx.dtsi                      |   5 +-
 arch/arm/boot/dts/omap4.dtsi                       |  12 +-
 arch/arm/boot/dts/omap5.dtsi                       |  12 +-
 arch/arm/boot/dts/tegra114.dtsi                    |  16 +-
 arch/arm/boot/dts/tegra124.dtsi                    |  16 +-
 arch/arm/boot/dts/tegra20.dtsi                     |  15 +-
 arch/arm/boot/dts/tegra30.dtsi                     |  16 +-
 arch/arm/mach-exynos/exynos.c                      |  12 +-
 arch/arm/mach-exynos/pm.c                          | 116 ++++++-
 arch/arm/mach-imx/common.h                         |   1 -
 arch/arm/mach-imx/gpc.c                            | 127 ++++++--
 arch/arm/mach-imx/mach-imx6q.c                     |   1 -
 arch/arm/mach-imx/mach-imx6sl.c                    |   1 -
 arch/arm/mach-imx/mach-imx6sx.c                    |   1 -
 arch/arm/mach-omap2/omap-wakeupgen.c               | 125 ++++++--
 arch/arm/mach-omap2/omap-wakeupgen.h               |   1 -
 arch/arm/mach-omap2/omap4-common.c                 |   1 -
 arch/arm/mach-shmobile/intc-sh73a0.c               |   7 +-
 arch/arm/mach-shmobile/setup-r8a7779.c             |   7 +-
 arch/arm/mach-tegra/iomap.h                        |  15 -
 arch/arm/mach-tegra/irq.c                          | 208 +------------
 arch/arm/mach-tegra/irq.h                          |   6 -
 arch/arm/mach-tegra/tegra.c                        |   1 -
 arch/arm/mach-ux500/cpu.c                          |   2 +-
 arch/arm/mach-zynq/common.c                        |   2 +-
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-gic.c                          |  59 +---
 drivers/irqchip/irq-tegra.c                        | 335 +++++++++++++++++++++
 include/linux/irqchip/arm-gic.h                    |   3 +-
 38 files changed, 869 insertions(+), 379 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu
 create mode 100644 drivers/irqchip/irq-tegra.c

2.1.3

The GIC is now always initialized from DT on tegra, and there is no point in keeping non-DT init code.

---
 arch/arm/mach-tegra/irq.c | 8 --------
 1 file changed, 8 deletions(-)

index ab95f53..7f87a50 100644
@@ -283,13 +283,5 @@ void __init tegra_init_irq(void)
 	gic_arch_extn.irq_set_wake = tegra_set_wake;
 	gic_arch_extn.flags = IRQCHIP_MASK_ON_SUSPEND;
 
-	/*
-	 * Check if there is a devicetree present, since the GIC will be
-	 * initialized elsewhere under DT.
-	 */
-	if (!of_have_populated_dt())
-		gic_init(0, 29, distbase,
-			IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x100));
-
 	tegra114_gic_cpu_pm_registration();
 }

2.1.3

Tegra's LIC (Legacy Interrupt Controller) has been so far only supported as a weird extension of the GIC, which is not exactly pretty.

The stacked irq domain framework fits this pretty well, and allows the LIC code to be turned into a standalone irqchip. In the process, make the driver DT aware, something that was sorely missing from the mach-tegra implementation.

---
 drivers/irqchip/Makefile    |   1 +
 drivers/irqchip/irq-tegra.c | 335 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 336 insertions(+)
 create mode 100644 drivers/irqchip/irq-tegra.c

index 983634d..0fd9c7d 100644
@@ -6,6 +6,7 @@ obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
 obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o
 obj-$(CONFIG_METAG)			+= irq-metag-ext.o

2.1.3

If we detect that our DT has a LIC node, don't setup gic_arch_extn, and skip tegra_legacy_irq_syscore_init as well.

This is only a temporary measure until that code is removed for good.

---
 arch/arm/mach-tegra/irq.c   | 11 +++++++++++
 arch/arm/mach-tegra/tegra.c |  1 -
 2 files changed, 11 insertions(+), 1 deletion(-)

index 7f87a50..b37141d 100644
@@ -255,11 +255,21 @@ static void tegra114_gic_cpu_pm_registration(void)
 static void tegra114_gic_cpu_pm_registration(void) { }
 #endif
 
+static const struct of_device_id tegra_ictlr_match[] __initconst = {
+	{ .compatible = "nvidia,tegra-ictlr" },
+	{ }
+};
+
 void __init tegra_init_irq(void)
 {
 	int i;
 	void __iomem *distbase;
 
+	if (of_find_matching_node(NULL, tegra_ictlr_match))
+		goto skip_extn_setup;
+
+	tegra_legacy_irq_syscore_init();
+
 	distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
 	num_ictlrs = readl_relaxed(distbase + GIC_DIST_CTR) & 0x1f;
 

@@ -283,5 +293,6 @@ void __init tegra_init_irq(void)
 	gic_arch_extn.irq_set_wake = tegra_set_wake;
 	gic_arch_extn.flags = IRQCHIP_MASK_ON_SUSPEND;
 
+skip_extn_setup:
 	tegra114_gic_cpu_pm_registration();
 }

index ef016af..c33fba7 100644
@@ -82,7 +82,6 @@ static void __init tegra_dt_init_irq(void)
 {
 	tegra_init_irq();
 	irqchip_init();
-	tegra_legacy_irq_syscore_init();
 }
 
 static void __init tegra_dt_init(void)

2.1.3

Describe the legacy interrupt controller in every tegra DTSI files, and make it the parent of most interrupts.

---
 arch/arm/boot/dts/tegra114.dtsi | 16 +++++++++++++++-
 arch/arm/boot/dts/tegra124.dtsi | 16 +++++++++++++++-
 arch/arm/boot/dts/tegra20.dtsi  | 15 ++++++++++++++-
 arch/arm/boot/dts/tegra30.dtsi  | 16 +++++++++++++++-
 4 files changed, 59 insertions(+), 4 deletions(-)

index 222f3b3..b89454d 100644
@@ -7,7 +7,7 @@
 
 / {
 	compatible = "nvidia,tegra114";
-	interrupt-parent = <&gic>;
+	interrupt-parent = <&ictlr>;
 
 	host1x at 50000000 {
 		compatible = "nvidia,tegra114-host1x", "simple-bus";

@@ -129,6 +129,19 @@
 		      <0x50046000 0x2000>;
 		interrupts = <GIC_PPI 9
 			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+		interrupt-parent = <&gic>;
+	};
+
+	ictlr: interrupt-controller at 60004000 {
+		compatible = "nvidia,tegra114-ictlr", "nvidia,tegra-ictlr";
+		reg = <0x60004000 64>,
+		      <0x60004100 64>,
+		      <0x60004200 64>,
+		      <0x60004300 64>,
+		      <0x60004400 64>;
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&gic>;
 	};
 
 	timer at 60005000 {

@@ -761,5 +774,6 @@
 				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 			<GIC_PPI 10
 				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+		interrupt-parent = <&gic>;
 	};
 };

index df2b06b..e428655 100644
@@ -8,7 +8,7 @@
 
 / {
 	compatible = "nvidia,tegra124";
-	interrupt-parent = <&gic>;
+	interrupt-parent = <&ictlr>;
 	#address-cells = <2>;
 	#size-cells = <2>;
 

@@ -167,6 +167,7 @@
 		      <0x0 0x50046000 0x0 0x2000>;
 		interrupts = <GIC_PPI 9
 			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+		interrupt-parent = <&gic>;
 	};
 
 	gpu at 0,57000000 {

@@ -184,6 +185,18 @@
 		status = "disabled";
 	};
 
+	ictlr: interrupt-controller at 60004000 {
+		compatible = "nvidia,tegra124-ictlr", "nvidia,tegra-ictlr";
+		reg = <0x0 0x60004000 0x0 0x40>,
+		      <0x0 0x60004100 0x0 0x40>,
+		      <0x0 0x60004200 0x0 0x40>,
+		      <0x0 0x60004300 0x0 0x40>,
+		      <0x0 0x60004400 0x0 0x40>;
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&gic>;
+	};
+
 	timer at 0,60005000 {
 		compatible = "nvidia,tegra124-timer", "nvidia,tegra20-timer";
 		reg = <0x0 0x60005000 0x0 0x400>;

@@ -891,5 +904,6 @@
 				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
 			     <GIC_PPI 10
 				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+		interrupt-parent = <&gic>;
 	};
 };

index 8acf5d8..ab2f004 100644
@@ -7,7 +7,7 @@
 
 / {
 	compatible = "nvidia,tegra20";
-	interrupt-parent = <&intc>;
+	interrupt-parent = <&ictlr>;
 
 	host1x at 50000000 {
 		compatible = "nvidia,tegra20-host1x", "simple-bus";

@@ -142,6 +142,7 @@
 
 	timer at 50004600 {
 		compatible = "arm,cortex-a9-twd-timer";
+		interrupt-parent = <&intc>;
 		reg = <0x50040600 0x20>;
 		interrupts = <GIC_PPI 13
 			(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;

@@ -154,6 +155,7 @@
 		       0x50040100 0x0100>;
 		interrupt-controller;
 		#interrupt-cells = <3>;
+		interrupt-parent = <&intc>;
 	};
 
 	cache-controller at 50043000 {

@@ -165,6 +167,17 @@
 		cache-level = <2>;
 	};
 
+	ictlr: interrupt-controller at 60004000 {
+		compatible = "nvidia,tegra20-ictlr", "nvidia,tegra-ictlr";
+		reg = <0x60004000 64>,
+		      <0x60004100 64>,
+		      <0x60004200 64>,
+		      <0x60004300 64>;
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&intc>;
+	};
+
 	timer at 60005000 {
 		compatible = "nvidia,tegra20-timer";
 		reg = <0x60005000 0x60>;

index b270b9e..4490f8c 100644
@@ -7,7 +7,7 @@
 
 / {
 	compatible = "nvidia,tegra30";
-	interrupt-parent = <&intc>;
+	interrupt-parent = <&ictlr>;
 
 	pcie-controller at 00003000 {
 		compatible = "nvidia,tegra30-pcie";

@@ -223,6 +223,7 @@
 	timer at 50004600 {
 		compatible = "arm,cortex-a9-twd-timer";
 		reg = <0x50040600 0x20>;
+		interrupt-parent = <&intc>;
 		interrupts = <GIC_PPI 13
 			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		clocks = <&tegra_car TEGRA30_CLK_TWD>;

@@ -234,6 +235,7 @@
 		       0x50040100 0x0100>;
 		interrupt-controller;
 		#interrupt-cells = <3>;
+		interrupt-parent = <&intc>;
 	};
 
 	cache-controller at 50043000 {

@@ -245,6 +247,18 @@
 		cache-level = <2>;
 	};
 
+	ictlr: interrupt-controller at 60004000 {
+		compatible = "nvidia,tegra30-ictlr", "nvidia,tegra-ictlr";
+		reg = <0x60004000 64>,
+		      <0x60004100 64>,
+		      <0x60004200 64>,
+		      <0x60004300 64>,
+		      <0x60004400 64>;
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&intc>;
+	};
+
 	timer at 60005000 {
 		compatible = "nvidia,tegra30-timer", "nvidia,tegra20-timer";
 		reg = <0x60005000 0x400>;

2.1.3

---
 .../interrupt-controller/nvidia,tegra-ictlr.txt    | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/nvidia,tegra-ictlr.txt

new file mode 100644
index 0000000..44fd873
@@ -0,0 +1,39 @@
+NVIDIA Legacy Interrupt Controller
+
+All Tegra SoCs contain a legacy interrupt controller that routes
+interrupts to the GIC, and also serves as a wakeup source. It is also
+refered to as "ictlr", hence the name of the binding.
+
+The HW block exposes a number of frames, each implementing a set of 32
+interrupts.
+
+Reguired properties:
+
+- compatible : should contain at least "nvidia,tegra-ictlr".
+- reg : Specifies base physical address and size of the registers.
+  Each frame must be described separately.
+- interrupt-controller : Identifies the node as an interrupt controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source. The value must be 3.
+- interrupt-parent : a phandle to the GIC these interrupts are routed
+  to.
+
+Notes:
+
+- Because this HW ultimately routes interrupts to the GIC, the
+  interrupt specifier must be that of the GIC.
+- Only SPIs can use the ictlr as an interrupt parent. SGIs and PPIs
+  are explicitely forbiden.
+
+Example:
+
+	ictlr: interrupt-controller at 60004000 {
+		compatible = "nvidia,tegra20-ictlr", "nvidia,tegra-ictlr";
+		reg = <0x60004000 64>,
+		      <0x60004100 64>,
+		      <0x60004200 64>,
+		      <0x60004300 64>;
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&intc>;
+	};

2.1.3

Now that all DTs have been updated, entierely drop support for the non-DT code.

This is likely to break platforms that do not update their DT, so print a warning at boot time.

---
 arch/arm/mach-tegra/iomap.h |  15 ----
 arch/arm/mach-tegra/irq.c   | 201 +-------------------------------------------
 arch/arm/mach-tegra/irq.h   |   6 --
 3 files changed, 2 insertions(+), 220 deletions(-)

index ee79808..81dc950 100644
@@ -31,21 +31,6 @@
 #define TEGRA_ARM_INT_DIST_BASE		0x50041000
 #define TEGRA_ARM_INT_DIST_SIZE		SZ_4K
 
-#define TEGRA_PRIMARY_ICTLR_BASE	0x60004000
-#define TEGRA_PRIMARY_ICTLR_SIZE	SZ_64
-
-#define TEGRA_SECONDARY_ICTLR_BASE	0x60004100
-#define TEGRA_SECONDARY_ICTLR_SIZE	SZ_64
-
-#define TEGRA_TERTIARY_ICTLR_BASE	0x60004200
-#define TEGRA_TERTIARY_ICTLR_SIZE	SZ_64
-
-#define TEGRA_QUATERNARY_ICTLR_BASE	0x60004300
-#define TEGRA_QUATERNARY_ICTLR_SIZE	SZ_64
-
-#define TEGRA_QUINARY_ICTLR_BASE	0x60004400
-#define TEGRA_QUINARY_ICTLR_SIZE	SZ_64
-
 #define TEGRA_TMR1_BASE			0x60005000
 #define TEGRA_TMR1_SIZE			SZ_8
 

index bc05ce5..5142649 100644
@@ -19,10 +19,4 @@
 
 bool tegra_pending_sgi(void);
 
-#ifdef CONFIG_PM_SLEEP
-int tegra_legacy_irq_syscore_init(void);
-#else
-static inline int tegra_legacy_irq_syscore_init(void) { return 0; }
-#endif
-
 #endif

2.1.3

OMAP4/5 has been (ab)using the gic_arch_extn to provide wakeup from suspend, and it makes a lot of sense to convert this code to use stacked domains instead.

This patch does just this, updating the DT files to actually reflect what the HW provides.

BIG FAT WARNING: because the DTs were so far lying by not exposing the WUGEN HW block, kernels with this patch applied won't have any suspend-resume facility when booted with old DTs, and old kernels with updated DTs won't even boot.

On a platform with this patch applied, the system looks like this:

root at bacon-fat:~# cat /proc/interrupts
CPU0 CPU1

 16:          0          0     WUGEN  37  gp_timer
 19:       3049       1764       GIC  27  arch_timer
 44:          0          0  4ae10000.gpio  13  DMA
294:          0          0     WUGEN  20  gpmc
297:          0          0     WUGEN  56  48070000.i2c
298:          0          0     WUGEN  57  48072000.i2c
299:          0          0     WUGEN  61  48060000.i2c
300:          0          0     WUGEN  62  4807a000.i2c
301:          0          0     WUGEN  60  4807c000.i2c
308:       1561          0       GIC 106  OMAP UART2
350:          0          0      PRCM  pinctrl, pinctrl
406:       5422         45       GIC 109  ehci_hcd:usb1
IPI0:          0          1  CPU wakeup interrupts
IPI1:          0          0  Timer broadcast interrupts
IPI2:       2243       4122  Rescheduling interrupts
IPI3:          0          0  Function call interrupts
IPI4:        522        611  Single function call interrupts
IPI5:          0          0  CPU stop interrupts
IPI6:          0          0  IRQ work interrupts
IPI7:          0          0  completion interrupts
Err:          0

---
 arch/arm/boot/dts/am4372.dtsi        |  11 ++-
 arch/arm/boot/dts/dra7.dtsi          |  12 +++-
 arch/arm/boot/dts/omap4.dtsi         |  12 +++-
 arch/arm/boot/dts/omap5.dtsi         |  12 +++-
 arch/arm/mach-omap2/omap-wakeupgen.c | 125 ++++++++++++++++++++++++++---------
 arch/arm/mach-omap2/omap-wakeupgen.h |   1 -
 arch/arm/mach-omap2/omap4-common.c   |   1 -
 7 files changed, 138 insertions(+), 36 deletions(-)

index 46660ff..5a0ea3c 100644
@@ -15,7 +15,7 @@
 
 / {
 	compatible = "ti,am4372", "ti,am43";
-	interrupt-parent = <&gic>;
+	interrupt-parent = <&wakeupgen>;
 
 
 	aliases {

@@ -48,6 +48,15 @@
 		#interrupt-cells = <3>;
 		reg = <0x48241000 0x1000>,
 		      <0x48240100 0x0100>;
+		interrupt-parent = <&gic>;
+	};
+
+	wakeupgen: interrupt-controller at 48281000 {
+		compatible = "ti,omap4-wugen-mpu";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		reg = <0x48281000 0x1000>;
+		interrupt-parent = <&gic>;
 	};
 
 	l2-cache-controller at 48242000 {

index 9cc9843..49ad4b3 100644
@@ -20,7 +20,7 @@
 	#size-cells = <1>;
 
 	compatible = "ti,dra7xx";
-	interrupt-parent = <&gic>;
+	interrupt-parent = <&wakeupgen>;
 
 	aliases {
 		i2c0 = &i2c1;

@@ -42,6 +42,7 @@
 			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
 			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
 			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>;
+		interrupt-parent = <&gic>;
 	};
 
 	gic: interrupt-controller at 48211000 {

@@ -54,6 +55,15 @@
 		      <0x48214000 0x2000>,
 		      <0x48216000 0x2000>;
 		interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
+		interrupt-parent = <&gic>;
+	};
+
+	wakeupgen: interrupt-controller at 48281000 {
+		compatible = "ti,omap5-wugen-mpu", "ti,omap4-wugen-mpu";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		reg = <0x48281000 0x1000>;
+		interrupt-parent = <&gic>;
 	};
 
 	/*

index 878c979..4c01a19 100644
@@ -14,7 +14,7 @@
 
 / {
 	compatible = "ti,omap4430", "ti,omap4";
-	interrupt-parent = <&gic>;
+	interrupt-parent = <&wakeupgen>;
 
 	aliases {
 		i2c0 = &i2c1;

@@ -56,6 +56,7 @@
 		#interrupt-cells = <3>;
 		reg = <0x48241000 0x1000>,
 		      <0x48240100 0x0100>;
+		interrupt-parent = <&gic>;
 	};
 
 	L2: l2-cache-controller at 48242000 {

@@ -70,6 +71,15 @@
 		clocks = <&mpu_periphclk>;
 		reg = <0x48240600 0x20>;
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_HIGH)>;
+		interrupt-parent = <&gic>;
+	};
+
+	wakeupgen: interrupt-controller at 48281000 {
+		compatible = "ti,omap4-wugen-mpu";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		reg = <0x48281000 0x1000>;
+		interrupt-parent = <&gic>;
 	};
 
 	/*

index 256b7f6..ae4612a 100644
@@ -18,7 +18,7 @@
 	#size-cells = <1>;
 
 	compatible = "ti,omap5";
-	interrupt-parent = <&gic>;
+	interrupt-parent = <&wakeupgen>;
 
 	aliases {
 		i2c0 = &i2c1;

@@ -79,6 +79,7 @@
 			     <GIC_PPI 14 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
 			     <GIC_PPI 11 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>,
 			     <GIC_PPI 10 (GIC_CPU_MASK_RAW(3) | IRQ_TYPE_LEVEL_LOW)>;
+		interrupt-parent = <&gic>;
 	};
 
 	pmu {

@@ -95,6 +96,15 @@
 		      <0x48212000 0x1000>,
 		      <0x48214000 0x2000>,
 		      <0x48216000 0x2000>;
+		interrupt-parent = <&gic>;
+	};
+
+	wakeupgen: interrupt-controller at 48281000 {
+		compatible = "ti,omap5-wugen-mpu", "ti,omap4-wugen-mpu";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		reg = <0x48281000 0x1000>;
+		interrupt-parent = <&gic>;
 	};
 
 	/*

index b3c8ecc..a3491ad 100644
@@ -33,7 +33,6 @@
 #define OMAP_TIMESTAMPCYCLELO			0xc08
 #define OMAP_TIMESTAMPCYCLEHI			0xc0c
 
-extern int __init omap_wakeupgen_init(void);
 extern void __iomem *omap_get_wakeupgen_base(void);
 extern int omap_secure_apis_support(void);
 #endif

index 16b20ce..0465448 100644
@@ -274,7 +274,6 @@ void __init omap_gic_of_init(void)
 	WARN_ON(!twd_base);
 
 skip_errata_init:
-	omap_wakeupgen_init();
 #ifdef CONFIG_IRQ_CROSSBAR
 	irqcrossbar_init();
 #endif

2.1.3

---
 .../interrupt-controller/ti,omap4-wugen-mpu        | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,omap4-wugen-mpu

new file mode 100644
index 0000000..16149d9
@@ -0,0 +1,32 @@
+TI OMAP4 Wake-up Generator
+
+All TI OMAP4/5 (and their derivatives) an interrupt controllerthat
+routes interrupts to the GIC, and also serves as a wakeup source. It
+is also refered to as "WUGEN-MPU", hence the name of the binding.
+
+Reguired properties:
+
+- compatible : should contain at least "ti,omap4-wugen-mpu"
+- reg : Specifies base physical address and size of the registers.
+- interrupt-controller : Identifies the node as an interrupt controller.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source. The value must be 3.
+- interrupt-parent : a phandle to the GIC these interrupts are routed
+  to.
+
+Notes:
+
+- Because this HW ultimately routes interrupts to the GIC, the
+  interrupt specifier must be that of the GIC.
+- Only SPIs can use the ictlr as an interrupt parent. SGIs and PPIs
+  are explicitely forbiden.
+
+Example:
+
+       wakeupgen: interrupt-controller at 48281000 {
+               compatible = "ti,omap5-wugen-mpu", "ti,omap4-wugen-mpu";
+               interrupt-controller;
+               #interrupt-cells = <3>;
+               reg = <0x48281000 0x1000>;
+               interrupt-parent = <&gic>;
+       };

2.1.3

IMX6 has been (ab)using the gic_arch_extn to provide wakeup from suspend, and it makes a lot of sense to convert this code to use stacked domains instead.

This patch does just this, updating the DT files to actually reflect what the HW provides.

BIG FAT WARNING: because the DTs were so far lying by not exposing the fact that the GPC block is actually the first interrupt controller in the chain, kernels with this patch applied wont have any suspend-resume facility when booted with old DTs, and old kernels with updated DTs won't even boot.

---
 arch/arm/boot/dts/imx6qdl.dtsi  |   6 +-
 arch/arm/boot/dts/imx6sl.dtsi   |   5 +-
 arch/arm/boot/dts/imx6sx.dtsi   |   5 +-
 arch/arm/mach-imx/common.h      |   1 -
 arch/arm/mach-imx/gpc.c         | 127 ++++++++++++++++++++++++++++++++--------
 arch/arm/mach-imx/mach-imx6q.c  |   1 -
 arch/arm/mach-imx/mach-imx6sl.c |   1 -
 arch/arm/mach-imx/mach-imx6sx.c |   1 -
 8 files changed, 116 insertions(+), 31 deletions(-)

index 9596ed5..b7b4ee8 100644
@@ -53,6 +53,7 @@
 		interrupt-controller;
 		reg = <0x00a01000 0x1000>,
 		      <0x00a00100 0x100>;
+		interrupt-parent = <&intc>;
 	};
 
 	clocks {

@@ -82,7 +83,7 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		compatible = "simple-bus";
-		interrupt-parent = <&intc>;
+		interrupt-parent = <&gpc>;
 		ranges;
 
 		dma_apbh: dma-apbh at 00110000 {

@@ -122,6 +123,7 @@
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = <0x00a00600 0x20>;
 			interrupts = <1 13 0xf01>;
+			interrupt-parent = <&intc>;
 			clocks = <&clks IMX6QDL_CLK_TWD>;
 		};
 

@@ -680,8 +682,10 @@
 			gpc: gpc at 020dc000 {
 				compatible = "fsl,imx6q-gpc";
 				reg = <0x020dc000 0x4000>;
+				interrupt-controller;
 				interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 90 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-parent = <&intc>;
 			};
 
 			gpr: iomuxc-gpr at 020e0000 {

index dfd83e6..802507e 100644
@@ -72,6 +72,7 @@
 		interrupt-controller;
 		reg = <0x00a01000 0x1000>,
 		      <0x00a00100 0x100>;
+		interrupt-parent = <&intc>;
 	};
 
 	clocks {

@@ -95,7 +96,7 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		compatible = "simple-bus";
-		interrupt-parent = <&intc>;
+		interrupt-parent = <&gpc>;
 		ranges;
 
 		ocram: sram at 00900000 {

@@ -597,7 +598,9 @@
 			gpc: gpc at 020dc000 {
 				compatible = "fsl,imx6sl-gpc", "fsl,imx6q-gpc";
 				reg = <0x020dc000 0x4000>;
+				interrupt-controller;
 				interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-parent = <&intc>;
 			};
 
 			gpr: iomuxc-gpr at 020e0000 {

index f3e88c0..75ca610 100644
@@ -88,6 +88,7 @@
 		interrupt-controller;
 		reg = <0x00a01000 0x1000>,
 		      <0x00a00100 0x100>;
+		interrupt-parent = <&intc>;
 	};
 
 	clocks {

@@ -131,7 +132,7 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		compatible = "simple-bus";
-		interrupt-parent = <&intc>;
+		interrupt-parent = <&gpc>;
 		ranges;
 
 		pmu {

@@ -694,7 +695,9 @@
 			gpc: gpc at 020dc000 {
 				compatible = "fsl,imx6sx-gpc", "fsl,imx6q-gpc";
 				reg = <0x020dc000 0x4000>;
+				interrupt-controller;
 				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-parent = <&intc>;
 			};
 
 			iomuxc: iomuxc at 020e0000 {

index 66662a1..ac65dd5 100644
@@ -103,7 +103,6 @@ static inline void imx_scu_map_io(void) {}
 static inline void imx_smp_prepare(void) {}
 #endif
 void imx_src_init(void);
-void imx_gpc_init(void);
 void imx_gpc_pre_suspend(bool arm_power_off);
 void imx_gpc_post_resume(void);
 void imx_gpc_mask_all(void);

index d51c6e9..5242a9c 100644
@@ -392,7 +392,6 @@ static void __init imx6q_init_irq(void)
 	imx_init_revision_from_anatop();
 	imx_init_l2cache();
 	imx_src_init();
-	imx_gpc_init();
 	irqchip_init();
 }
 

index ed263a2..49c4e22 100644
@@ -66,7 +66,6 @@ static void __init imx6sl_init_irq(void)
 	imx_init_revision_from_anatop();
 	imx_init_l2cache();
 	imx_src_init();
-	imx_gpc_init();
 	irqchip_init();
 }
 

index 3de3b73..dc0df73 100644
@@ -35,7 +35,6 @@ static void __init imx6sx_init_irq(void)
 	imx_init_revision_from_anatop();
 	imx_init_l2cache();
 	imx_src_init();
-	imx_gpc_init();
 	irqchip_init();
 }
 

2.1.3

Not sure this is entirely true. I don't think that GPC block is really in the chain of the interrupts for normal interrupt delivery. This is tested to be _not_ true for Vybird (which has also a GPC block and seems to be quite similar). I can mask an interrupt in GPC_IMR2 for instance and it still gets delivered to the GIC. However, the registers need to be unmasked to deliver wake-up events.

In the GPC_IMRx register description of Vybrid this sentence actually states what I observe: "This register is used to mask certain interrupts if they are not desired to be a source of
wake up during STOP mode.". However, this sentence is not there in the i.MX6 RM. But the drawing in Chapter 27.1 (GPC, Overview) suggests to me, that it is implemented like I see it in Vybrid. So I'm not completely sure about this, and I don't have hardware handy to test this...

I'm aware that the code did something different before. Not sure whether this refactoring would look differently if my suspicion holds true. I guess even if it is only wake-up capabilities, we would use stacked IRQ domain and just implement the wake-up part...?

--
Stefan

Hi Stephan,

Well, if it has an influence on the delivery of interrupts in the wake-up case, we need to take care of it.

My copy of the TRM has the following wording: "GPC determines wake-up irq for exiting STOP mode", and the diagram just below has some indication of the interrupts being used to generate a wake-up signal.

Very much so. My (limited) understanding of the HW:
- GPC is placed before the GIC, and derives a wake-up signal based on a mask and an enable signal (most probably the STOP state).
- It always forwards interrupts to the GIC.
- When STOP is asserted, the GIC is offline

So we can see this as either
- two parallel interrupt controllers
- two cascaded interrupt controllers

The first case cannot be represented in DT (an interrupt can only have a single interrupt parent). so we're left with the second option, which is a good enough approximation IMHO. Better than not describing it, anyway.

Thanks,

	M.

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

Exynos has been (ab)using the gic_arch_extn to provide wakeup from suspend, and it makes a lot of sense to convert this code to use stacked domains instead.

This patch does just this, updating the DT files to actually reflect what the HW provides.

BIG FAT WARNING: because the DTs were so far lying by not exposing the fact that the PMU block is actually the first interrupt controller in the chain for RTC, kernels with this patch applied wont have any suspend-resume facility when booted with old DTs, and old kernels with updated DTs may not even boot.

Also, I stronly suspect that there is more than two wake-up interrupts on these platforms, but I leave it to the maintainers to fix their mess.

---
 arch/arm/boot/dts/exynos4.dtsi    |   3 +
 arch/arm/boot/dts/exynos5250.dtsi |   3 +
 arch/arm/mach-exynos/exynos.c     |  12 ++--
 arch/arm/mach-exynos/pm.c         | 116 +++++++++++++++++++++++++++++++++++---
 4 files changed, 119 insertions(+), 15 deletions(-)

index e0278ec..04decb9 100644
@@ -141,6 +141,8 @@
 	pmu_system_controller: system-controller at 10020000 {
 		compatible = "samsung,exynos4210-pmu", "syscon";
 		reg = <0x10020000 0x4000>;
+		interrupt-controller;
+		interrupt-parent = <&gic>;
 	};
 
 	dsi_0: dsi at 11C80000 {

@@ -253,6 +255,7 @@
 	rtc at 10070000 {
 		compatible = "samsung,s3c6410-rtc";
 		reg = <0x10070000 0x100>;
+		interrupt-parent = <&pmu_system_controller>;
 		interrupts = <0 44 0>, <0 45 0>;
 		clocks = <&clock CLK_RTC>;
 		clock-names = "rtc";

index d55c1a2..b937e7b 100644
@@ -194,6 +194,8 @@
 		clock-names = "clkout16";
 		clocks = <&clock CLK_FIN_PLL>;
 		#clock-cells = <1>;
+		interrupt-controller;
+		interrupt-parent = <&gic>;
 	};
 
 	sysreg_system_controller: syscon at 10050000 {

@@ -230,6 +232,7 @@
 	rtc at 101E0000 {
 		clocks = <&clock CLK_RTC>;
 		clock-names = "rtc";
+		interrupt-parent = <&pmu_system_controller>;
 		status = "disabled";
 	};
 

index 6b283eb..a4d2f65 100644
@@ -247,12 +247,13 @@ static void __init exynos_init_io(void)
 	exynos_map_io();
 }
 
+/*
+ * Apparently, these SoCs are not able to wake-up from suspend using
+ * the PMU. Too bad. Should they suddenly become capable of such a
+ * feat, the matches below should be moved to pm.c.
+ */
 static const struct of_device_id exynos_dt_pmu_match[] = {
 	{ .compatible = "samsung,exynos3250-pmu" },
-	{ .compatible = "samsung,exynos4210-pmu" },
-	{ .compatible = "samsung,exynos4212-pmu" },
-	{ .compatible = "samsung,exynos4412-pmu" },
-	{ .compatible = "samsung,exynos5250-pmu" },
 	{ .compatible = "samsung,exynos5260-pmu" },
 	{ .compatible = "samsung,exynos5410-pmu" },
 	{ .compatible = "samsung,exynos5420-pmu" },

@@ -266,9 +267,6 @@ static void exynos_map_pmu(void)
 	np = of_find_matching_node(NULL, exynos_dt_pmu_match);
 	if (np)
 		pmu_base_addr = of_iomap(np, 0);
-
-	if (!pmu_base_addr)
-		panic("failed to find exynos pmu register\n");
 }
 
 static void __init exynos_init_irq(void)

2.1.3

Document the fact that some Exynos PMUs are capable of acting as an interrupt controller.

---
 Documentation/devicetree/bindings/arm/samsung/pmu.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

index 1e1979b..d698e74 100644
@@ -28,10 +28,23 @@ Properties:
  - clocks : list of phandles and specifiers to all input clocks listed in
 		clock-names property.
 
+Optional properties:
+
+Some PMUs are capable of behaving as an interrupt controller (mostly
+to wake up a suspended PMU). In which case, they can have the
+following properties:
+
+- interrupt-controller: indicate that said PMU is an interrupt controller
+
+- interrupt-parent: a phandle indicating which interrupt controller
+  this PMU signals interrupts to.
+
 Example :
 pmu_system_controller: system-controller at 10040000 {
 	compatible = "samsung,exynos5250-pmu", "syscon";
 	reg = <0x10040000 0x5000>;
+	interrupt-controller;
+	interrupt-parent = <&gic>;
 	#clock-cells = <1>;
 	clock-names = "clkout0", "clkout1", "clkout2", "clkout3",
 			"clkout4", "clkout8", "clkout9";

2.1.3

A common use of gic_arch_extn is to set up additional flags to the GIC irqchip. It looks like a benign enough hack that doesn't really require the users of that feature to be converted to stacked domains.

Add a gic_set_irqchip_flags() function that platform code can call instead of using the dreaded gic_arch_extn.

---
 drivers/irqchip/irq-gic.c       | 5 +++++
 include/linux/irqchip/arm-gic.h | 1 +
 2 files changed, 6 insertions(+)

index 5a71be7..91de85d 100644
@@ -919,6 +919,11 @@ static const struct irq_domain_ops gic_default_routable_irq_domain_ops = {
 const struct irq_domain_ops *gic_routable_irq_domain_ops =
 					&gic_default_routable_irq_domain_ops;
 
+void gic_set_irqchip_flags(unsigned long flags)
+{
+	gic_chip.flags |= flags;
+}
+
 void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 			   void __iomem *dist_base, void __iomem *cpu_base,
 			   u32 percpu_offset, struct device_node *node)

index 71d706d..3bca864 100644
@@ -97,6 +97,7 @@ struct device_node;
 
 extern struct irq_chip gic_arch_extn;
 
+void gic_set_irqchip_flags(unsigned long flags);
 void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
 		    u32 offset, struct device_node *);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);

2.1.3

shmobile only uses gic_arch_extn.irq_set_wake to prevent the GIC from returning -ENXIO when receiving a wake-up configuration request.

It is a lot simpler to tell the irq layer that we don't need any configuration by using the IRQCHIP_SKIP_SET_WAKE, thanks to the new gic_set_irqchip_flags function.

---
 arch/arm/mach-shmobile/intc-sh73a0.c   | 7 +------
 arch/arm/mach-shmobile/setup-r8a7779.c | 7 +------
 2 files changed, 2 insertions(+), 12 deletions(-)

index 44457a9..b5af6ad 100644
@@ -256,11 +256,6 @@ static irqreturn_t sh73a0_intcs_demux(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int sh73a0_set_wake(struct irq_data *data, unsigned int on)
-{
-	return 0; /* always allow wakeup */
-}
-
 #define PINTER0_PHYS 0xe69000a0
 #define PINTER1_PHYS 0xe69000a4
 #define PINTER0_VIRT IOMEM(0xe69000a0)

@@ -322,8 +317,8 @@ void __init sh73a0_init_irq(void)
 	void __iomem *gic_cpu_base = IOMEM(0xf0000100);
 	void __iomem *intevtsa = ioremap_nocache(0xffd20100, PAGE_SIZE);
 
+	gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE);
 	gic_init(0, 29, gic_dist_base, gic_cpu_base);
-	gic_arch_extn.irq_set_wake = sh73a0_set_wake;
 
 	register_intc_controller(&intcs_desc);
 	register_intc_controller(&intc_pint0_desc);

index 136078a..8cde457 100644
@@ -716,14 +716,9 @@ void __init r8a7779_init_late(void)
 }
 
 #ifdef CONFIG_USE_OF
-static int r8a7779_set_wake(struct irq_data *data, unsigned int on)
-{
-	return 0; /* always allow wakeup */
-}
-
 void __init r8a7779_init_irq_dt(void)
 {
-	gic_arch_extn.irq_set_wake = r8a7779_set_wake;
+	gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE);
 
 	irqchip_init();
 

2.1.3

Hi Marc,

this looks reasonable to me. Should I take it through my tree (in v3.20) or did you have another path in mind?

Hi Simon,

As you can see, there is a dependency on the GIC itself, and other platforms are affected to. So I'd think that an arm-soc branch containing all the required patches would be more appropriate, assuming that you're happy to Ack this patch.

Thanks,

	M.

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

Sure, that is fine by me.

Instead of directly touching gic_arch_extn, which is about to be removed, use gic_set_irqchip_flags instead.

---
 arch/arm/mach-ux500/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index dbb2970..6ced0f6 100644
@@ -52,7 +52,7 @@ void ux500_restart(enum reboot_mode mode, const char *cmd)
 */
 void __init ux500_init_irq(void)
 {
-	gic_arch_extn.flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND;
+	gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND);
 	irqchip_init();
 
 	/*

2.1.3

Yours,
Linus Walleij

Instead of directly touching gic_arch_extn, which is about to be removed, use gic_set_irqchip_flags instead.

---
 arch/arm/mach-zynq/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 26f92c2..82734d5 100644
@@ -188,7 +188,7 @@ static void __init zynq_map_io(void)
 
 static void __init zynq_irq_init(void)
 {
-	gic_arch_extn.flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND;
+	gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND);
 	irqchip_init();
 }
 

2.1.3

Now that the users of gic_arch_extn have been fixed, drop the "feature" for good. This leads to the removal of some now useless locking.

---
 drivers/irqchip/irq-gic.c       | 54 -----------------------------------------
 include/linux/irqchip/arm-gic.h |  2 --
 2 files changed, 56 deletions(-)

index 91de85d..540f23c 100644
@@ -80,19 +80,6 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 #define NR_GIC_CPU_IF 8
 static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
 
-/*
- * Supported arch specific GIC irq extension.
- * Default make them NULL.
- */
-struct irq_chip gic_arch_extn = {
-	.irq_eoi	= NULL,
-	.irq_mask	= NULL,
-	.irq_unmask	= NULL,
-	.irq_retrigger	= NULL,
-	.irq_set_type	= NULL,
-	.irq_set_wake	= NULL,
-};
-
 #ifndef MAX_GIC_NR
 #define MAX_GIC_NR	1
 #endif

@@ -155,32 +142,18 @@ static void gic_mask_irq(struct irq_data *d)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
 
-	raw_spin_lock(&irq_controller_lock);
 	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_CLEAR + (gic_irq(d) / 32) * 4);
-	if (gic_arch_extn.irq_mask)
-		gic_arch_extn.irq_mask(d);
-	raw_spin_unlock(&irq_controller_lock);
 }
 
 static void gic_unmask_irq(struct irq_data *d)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
 
-	raw_spin_lock(&irq_controller_lock);
-	if (gic_arch_extn.irq_unmask)
-		gic_arch_extn.irq_unmask(d);
 	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_SET + (gic_irq(d) / 32) * 4);
-	raw_spin_unlock(&irq_controller_lock);
 }
 
 static void gic_eoi_irq(struct irq_data *d)
 {
-	if (gic_arch_extn.irq_eoi) {
-		raw_spin_lock(&irq_controller_lock);
-		gic_arch_extn.irq_eoi(d);
-		raw_spin_unlock(&irq_controller_lock);
-	}
-
 	writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
 }
 

@@ -196,23 +169,13 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
 	if (type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING)
 		return -EINVAL;
 
-	raw_spin_lock(&irq_controller_lock);
-
-	if (gic_arch_extn.irq_set_type)
-		gic_arch_extn.irq_set_type(d, type);
-
 	gic_configure_irq(gicirq, type, base, NULL);
 
-	raw_spin_unlock(&irq_controller_lock);
-
 	return 0;
 }
 
 static int gic_retrigger(struct irq_data *d)
 {
-	if (gic_arch_extn.irq_retrigger)
-		return gic_arch_extn.irq_retrigger(d);
-
 	/* the genirq layer expects 0 if we can't retrigger in hardware */
 	return 0;
 }

@@ -244,21 +207,6 @@ 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 ret = -ENXIO;
-
-	if (gic_arch_extn.irq_set_wake)
-		ret = gic_arch_extn.irq_set_wake(d, on);
-
-	return ret;
-}
-
-#else
-#define gic_set_wake	NULL
-#endif
-
 static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
 {
 	u32 irqstat, irqnr;

@@ -321,7 +269,6 @@ static struct irq_chip gic_chip = {
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= gic_set_affinity,
 #endif
-	.irq_set_wake		= gic_set_wake,
 };
 
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)

@@ -1034,7 +981,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 		set_handle_irq(gic_handle_irq);
 	}
 
-	gic_chip.flags |= gic_arch_extn.flags;
 	gic_dist_init(gic);
 	gic_cpu_init(gic);
 	gic_pm_init(gic);

index 3bca864..4637213 100644
@@ -95,8 +95,6 @@
 
 struct device_node;
 
-extern struct irq_chip gic_arch_extn;
-
 void gic_set_irqchip_flags(unsigned long flags);
 void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
 		    u32 offset, struct device_node *);

2.1.3

I wonder if we should take this as the trigger to come up with a better way of handling incompatible binding changes. The machine_desc has a .dt_fixup callback that we could use to modify the DT passed from a boot loader and warn about it, but no platform does this at the moment.

with a new kernel is to add a particular node and flip a few interrupt-parent properties, which seems doable as a quirk if people agree that it's a good idea.

Arnd

Hi Arnd,

That's probably the only way to avoid the breakage introduced by this kind of changes. A few questions though:
- Where do we stop? Eventually, don't we end-up with a full DT in there?
- When do we retire such fixup? Or do we keep them forever?

Thanks,

	M.

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

Excellent questions. My first reaction to both would be to leave it up to the individual platform maintainer, but I'd also limit it to working around regressions and never to add device nodes in order to activate features that didn't already work with an older kernel.

Also, it would probably be good to control these using a compile-time option that easily allows building a kernel with no such hacks, possibly even using a versioning system to annotate how far back you want to support, though I would not implement that at first.

Arnd