[PATCH v6 0/4] irqchip: move away from gic_arch_extn.irq_set_wake

[PATCH v6 0/4] irqchip: move away from gic_arch_extn.irq_set_wake

[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

 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(-)

-- 
2.1.4

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

[Marc Zyngier]

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.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c       | 5 +++++
 include/linux/irqchip/arm-gic.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index e3ca6da..6004d84 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -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)
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 3978c5b..36ec4ae 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -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.4

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

[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.

Acked-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-shmobile/intc-sh73a0.c   | 7 +------
 arch/arm/mach-shmobile/setup-r8a7779.c | 7 +------
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-shmobile/intc-sh73a0.c b/arch/arm/mach-shmobile/intc-sh73a0.c
index 9e36180..fd63ae6 100644
--- a/arch/arm/mach-shmobile/intc-sh73a0.c
+++ b/arch/arm/mach-shmobile/intc-sh73a0.c
@@ -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);
diff --git a/arch/arm/mach-shmobile/setup-r8a7779.c b/arch/arm/mach-shmobile/setup-r8a7779.c
index 27dceaf9..c03e562 100644
--- a/arch/arm/mach-shmobile/setup-r8a7779.c
+++ b/arch/arm/mach-shmobile/setup-r8a7779.c
@@ -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);
-- 
2.1.4

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

[Marc Zyngier]

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

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-ux500/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-ux500/cpu.c b/arch/arm/mach-ux500/cpu.c
index dbb2970..6ced0f6 100644
--- a/arch/arm/mach-ux500/cpu.c
+++ b/arch/arm/mach-ux500/cpu.c
@@ -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.4

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

[Marc Zyngier]

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

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/mach-zynq/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index c887196..58ef2a7 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -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();
 }
 
-- 
2.1.4

[PATCH v6 0/4] irqchip: move away from gic_arch_extn.irq_set_wake

[Jason Cooper]

Marc,

On Wed, Mar 11, 2015 at 03:45:33PM +0000, Marc Zyngier wrote:
> 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(-)

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

thx,

Jason.

[PATCH v6 0/4] irqchip: move away from gic_arch_extn.irq_set_wake

[Geert Uytterhoeven]

Hi Marc,

On Wed, Mar 11, 2015 at 4:45 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 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.

> 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

[PATCH v6 0/4] irqchip: move away from gic_arch_extn.irq_set_wake

[Marc Zyngier]

On 18/03/15 15:30, Geert Uytterhoeven wrote:
> Hi Marc,
> 
> On Wed, Mar 11, 2015 at 4:45 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> 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.
> 
>> 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.

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.

> 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?

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.

> 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
> 


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