Skip to contentSkip to conversation

irqchip: move away from gic_arch_extn.irq_set_wake

PATCH Superseded Marc Zyngier 3 participants 8 messages Mar 11, 2015 – Mar 18, 2015

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

v6 cover irqchip: move away from gic_arch_extn.irq_set_wake

This series is extracted from [4], which is trying to remove all traces of gic_arch_extn from the tree. As some maintainers are more responsive than others (understatement of the year...), I've decided to split it per sub-arch, and get it moving, at least partially.

This small set of patches slightly modifies some of the least offending platforms, by providing a much limited hook into the GIC driver.

Based on 4.0-rc1.

* From v5: [5]
- No change

* From v4: [4]
- Extracted from the full series
- Rebased on 4.0-rc1

* From v3 [3]:
- Rebased on top of the patch working around hardcoded IRQ on OMAP4/5 [4]
- Fixed more iMX6 DTs (Stephan)
- Fixed Exynos4/5 DTs

* From v2 [2]:
- Addressed numerous comments from Thierry
- Merged bug fixes from Nishanth
- Merged bug fix from Stefan

* From v1 [1]:
- Rebased on 3.19-rc3
- Fixed a number of additional platforms
- Added crossbar conversion to stacked domains
- Merged bug fixes from Nishanth

[5]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/325190.html
[4]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/317531.html
[3]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315385.html
[2]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/314041.html
[1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/307338.html

Marc Zyngier (4):
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

 arch/arm/mach-shmobile/intc-sh73a0.c   | 7 +------
 arch/arm/mach-shmobile/setup-r8a7779.c | 7 +------
 arch/arm/mach-ux500/cpu.c              | 2 +-
 arch/arm/mach-zynq/common.c            | 2 +-
 drivers/irqchip/irq-gic.c              | 5 +++++
 include/linux/irqchip/arm-gic.h        | 1 +
 6 files changed, 10 insertions(+), 14 deletions(-)
Signature · 1 line
2.1.4
#1 lore raw

PATCH v6 1/4 irqchip: gic: add an entry point to set up irqchip flags

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(+)
drivers/irqchip/irq-gic.c +5 -0
index e3ca6da..6004d84 100644
@@ -880,6 +880,11 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
 	.xlate = gic_irq_domain_xlate,
 };
 
+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)
include/linux/irqchip/arm-gic.h +1 -0
index 3978c5b..36ec4ae 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);
Signature · 1 line
2.1.4
#2 lore raw

PATCH v6 2/4 ARM: shmobile: remove use of gic_arch_extn.irq_set_wake

In reply to Marc Zyngier

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(-)
arch/arm/mach-shmobile/intc-sh73a0.c +1 -6
index 9e36180..fd63ae6 100644
@@ -252,11 +252,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)
@@ -318,8 +313,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);
arch/arm/mach-shmobile/setup-r8a7779.c +1 -6
index 27dceaf9..c03e562 100644
@@ -713,18 +713,13 @@ 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)
 {
 #ifdef CONFIG_ARCH_SHMOBILE_LEGACY
 	void __iomem *gic_dist_base = ioremap_nocache(0xf0001000, 0x1000);
 	void __iomem *gic_cpu_base = ioremap_nocache(0xf0000100, 0x1000);
 #endif
-	gic_arch_extn.irq_set_wake = r8a7779_set_wake;
+	gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE);
 
 #ifdef CONFIG_ARCH_SHMOBILE_LEGACY
 	gic_init(0, 29, gic_dist_base, gic_cpu_base);
Signature · 1 line
2.1.4
#3 lore raw

PATCH v6 3/4 ARM: ux500: switch from gic_arch_extn to gic_set_irqchip_flags

In reply to Marc Zyngier

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(-)
arch/arm/mach-ux500/cpu.c +1 -1
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();
 
 	/*
Signature · 1 line
2.1.4
#4 lore raw

PATCH v6 4/4 ARM: zynq: switch from gic_arch_extn to gic_set_irqchip_flags

In reply to Marc Zyngier

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(-)
arch/arm/mach-zynq/common.c +1 -1
index c887196..58ef2a7 100644
@@ -186,7 +186,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();
 }
 
Signature · 1 line
2.1.4
Applied
#5 lore raw

v6 cover irqchip: move away from gic_arch_extn.irq_set_wake

In reply to Marc Zyngier

Marc,

Quoted 46 lines · Marc Zyngier

This series is extracted from [4], which is trying to remove all traces of gic_arch_extn from the tree. As some maintainers are more responsive than others (understatement of the year...), I've decided to split it per sub-arch, and get it moving, at least partially.

This small set of patches slightly modifies some of the least offending platforms, by providing a much limited hook into the GIC driver.

Based on 4.0-rc1.

* From v5: [5]
- No change

* From v4: [4]
- Extracted from the full series
- Rebased on 4.0-rc1

* From v3 [3]:
- Rebased on top of the patch working around hardcoded IRQ on OMAP4/5 [4]
- Fixed more iMX6 DTs (Stephan)
- Fixed Exynos4/5 DTs

* From v2 [2]:
- Addressed numerous comments from Thierry
- Merged bug fixes from Nishanth
- Merged bug fix from Stefan

* From v1 [1]:
- Rebased on 3.19-rc3
- Fixed a number of additional platforms
- Added crossbar conversion to stacked domains
- Merged bug fixes from Nishanth

[5]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/325190.html
[4]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/317531.html
[3]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/315385.html
[2]: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/314041.html
[1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/307338.html

Marc Zyngier (4):
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

Quoted 7 lines
 arch/arm/mach-shmobile/intc-sh73a0.c   | 7 +------
 arch/arm/mach-shmobile/setup-r8a7779.c | 7 +------
 arch/arm/mach-ux500/cpu.c              | 2 +-
 arch/arm/mach-zynq/common.c            | 2 +-
 drivers/irqchip/irq-gic.c              | 5 +++++
 include/linux/irqchip/arm-gic.h        | 1 +
 6 files changed, 10 insertions(+), 14 deletions(-)

Applied to irqchip/stacked-irq_set_wake. I know, not the best name... :)

thx,

Jason.

#6 lore raw

v6 cover irqchip: move away from gic_arch_extn.irq_set_wake

In reply to Marc Zyngier

Hi Marc,

Quoted 9 lines · Marc Zyngier

This series is extracted from [4], which is trying to remove all traces of gic_arch_extn from the tree. As some maintainers are more responsive than others (understatement of the year...), I've decided to split it per sub-arch, and get it moving, at least partially.

This small set of patches slightly modifies some of the least offending platforms, by providing a much limited hook into the GIC driver.

Quoted 5 lines

Marc Zyngier (4):
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

As I'm feeling the need to add gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE) for more shmobile SoCs (which implies populating mach_desc.init_irq() again, and also calling irqchip_init() explicitly), I'm wondering if it wouldn't be better to just initialize "gic_chip.flags = IRQCHIP_SKIP_SET_WAKE" unconditionally?
Currently the GIC driver doesn't have to do anything special to support wake-up, but support for that may be added one day.

Every driver that wants to implements wake-up properly should call irq_set_irq_wake() or {en,dis}able_irq_wake() to inform the upstream interrupt controller. However, with GIC, that gives (except on sh73a0, r8a7779, ux500, and zynq) annoying warning messages during resume:

    WARNING: CPU: 1 PID: 1341 at kernel/irq/manage.c:540
irq_set_irq_wake+0x9c/0xf8()
    Unbalanced IRQ 26 wake disable

I guess I could just leave out the call to irq_set_irq_wake() in my GPIO driver, as it works fine without, but that doesn't look correct to me.

What do you think?

Thanks!

See also "[PATCH resend 2/2] [RFC] genirq: Set IRQCHIP_SKIP_SET_WAKE for no_irq_chip and dummy_irq_chip" (https://lkml.org/lkml/2015/1/12/506).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#7 lore raw

v6 cover irqchip: move away from gic_arch_extn.irq_set_wake

Quoted 18 lines · Geert Uytterhoeven

Hi Marc,

Quoted 9 lines · Marc Zyngier

This series is extracted from [4], which is trying to remove all traces of gic_arch_extn from the tree. As some maintainers are more responsive than others (understatement of the year...), I've decided to split it per sub-arch, and get it moving, at least partially.

This small set of patches slightly modifies some of the least offending platforms, by providing a much limited hook into the GIC driver.

Quoted 5 lines

Marc Zyngier (4):
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

Quoted 7 lines

As I'm feeling the need to add gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE) for more shmobile SoCs (which implies populating mach_desc.init_irq() again, and also calling irqchip_init() explicitly), I'm wondering if it wouldn't be better to just initialize "gic_chip.flags = IRQCHIP_SKIP_SET_WAKE" unconditionally?
Currently the GIC driver doesn't have to do anything special to support wake-up, but support for that may be added one day.

It is unlikely we'll ever add this support, because the GIC doesn't not have that capability in HW (wake-up is not even part of the architecture). This is why wake up is always left to some other interrupt controller.

Quoted 5 lines

Every driver that wants to implements wake-up properly should call irq_set_irq_wake() or {en,dis}able_irq_wake() to inform the upstream interrupt controller. However, with GIC, that gives (except on sh73a0, r8a7779, ux500, and zynq) annoying warning messages during resume:

Quoted 8 lines
    WARNING: CPU: 1 PID: 1341 at kernel/irq/manage.c:540
irq_set_irq_wake+0x9c/0xf8()
    Unbalanced IRQ 26 wake disable

I guess I could just leave out the call to irq_set_irq_wake() in my GPIO driver, as it works fine without, but that doesn't look correct to me.

What do you think?

At the moment, your approach will work, but only because we have another bug, as described here:

http://lkml.iu.edu/hypermail/linux/kernel/1501.1/04757.html

Fixing this bug means we will propagate the flag to be visible at the top level of the controller hierarchy, which will cause any irq_set_irq_wake() request to be ignored. Probably not what people have mind.

Only the platform code knows the topology of the system, unfortunately.

When it comes to the message you describe above, I'd blame the driver:
irq_set_irq_wake() does return -ENXIO when there is no irq_set_wake method. The driver should definitely test this, avoid doing a wake disable if the enable has failed.

Thanks,

	M.
Quoted 16 lines

Thanks!

See also "[PATCH resend 2/2] [RFC] genirq: Set IRQCHIP_SKIP_SET_WAKE for no_irq_chip and dummy_irq_chip" (https://lkml.org/lkml/2015/1/12/506).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Signature · 1 line
Jazz is not dead. It just smells funny...