Re: genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)

[Geert Uytterhoeven]

Hi Marc,

On Fri, Sep 16, 2016 at 9:51 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> On 16/09/16 00:02, Alban Browaeys wrote:
>> Le mercredi 14 septembre 2016 Ã 21:25 +0200, Geert Uytterhoeven a
>> Ãcrit :
>>> JFYI, with v4.8-rc6 I'm seeing
>>>
>>>     genirq: Setting trigger mode 0 for irq 11 failed
>>> (txx9_irq_set_type+0x0/0xb8)
>>>
>>> on rbtx4927. This did not happen with v4.8-rc3.
>>
>> txx9_irq_set_type receives a type IRQ_TYPE_NONE from the call to
>> __irq_set_trigger added in:
>> 1e12c4a939 ("genirq: Correctly configure the trigger on chained interrupts")

Yep, that's the commit that introduced the issue.

>> This patch is a regression fix for :
>>
>> Desc: irqdomain: Don't set type when mapping an IRQ breaks nexus7 gpio buttons
>> Repo: 2016-07-30 https://marc.info/?l=linux-kernel&m=146985356305280&w=2
>>
>> I am seeing this on arm odroid u2 devicetree :
>> genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x64)
>
> Passing IRQ_TYPE_NONE to a cascading interrupt is risky at best...
> Can you point me to the various DTs and their failing interrupts?

Rbtx4927 does not use DT. The issue is triggered during:

    irq_set_chained_handler(RBTX4927_IRQ_IOCINT, handle_simple_irq);

in arch/mips/txx9/rbtx4927/irq.c:toshiba_rbtx4927_irq_ioc_init(),
which is inlined into rbtx4927_irq_setup().

> Also, can you please give the following patch a go and let me know
> if that fixes the issue (I'm interested in the potential warning here).
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6373890..8422779 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,8 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>         desc->name = name;
>
>         if (handle != handle_bad_irq && is_chained) {
> +               unsigned int type = irqd_get_trigger_type(&desc->irq_data);
> +
>                 /*
>                  * We're about to start this interrupt immediately,
>                  * hence the need to set the trigger configuration.
> @@ -828,8 +830,10 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>                  * chained interrupt. Reset it immediately because we
>                  * do know better.
>                  */
> -               __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data));
> -               desc->handle_irq = handle;
> +               if (!(WARN_ON(type == IRQ_TYPE_NONE))) {
> +                       __irq_set_trigger(desc, type);
> +                       desc->handle_irq = handle;
> +               }
>
>                 irq_settings_set_noprobe(desc);
>                 irq_settings_set_norequest(desc);

This indeed makes the issue go away:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x144/0x1b4
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted
4.8.0-rc6-rbtx4927-01162-g1596ef0280a363ac-dirty #50
Stack : 00000000 10000000 0000000b 00000004 80453f47 804542d8 80429ff8 00000000
         804a36c8 00000341 80789384 000001e0 803c70b0 8014a19c 80460ac8 80460acc
         804314ec 00000000 8042f408 80451d94 80789384 80173624 803c70b0 8014a19c
         00000007 000009e0 00000000 00000000 00000000 00000000 00000000 0000000

         00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
         ...
Call Trace:
[<8010abf0>] show_stack+0x50/0x84
[<8012100c>] __warn+0xe4/0x118
[<80121098>] warn_slowpath_null+0x1c/0x28
[<8014f7a0>] __irq_do_set_handler+0x144/0x1b4
[<8014f860>] __irq_set_handler+0x50/0x7c
[<804740ac>] tx4927_irq_init+0x34/0xa8
[<80475e9c>] rbtx4927_irq_setup+0x30/0xd0
[<8046e9c0>] start_kernel+0x288/0x450

---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x144/0x1b4
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Tainted: G        W
4.8.0-rc6-rbtx4927-01162-g1596ef0280a363ac-dirty #50
Stack : 00000000 10000400 00000009 00000004 80453f47 804542d8 80429ff8 00000000
         804a36c8 00000341 80789384 000001e0 803c70b0 8014a19c 80460ac8 80460acc
         804314ec 00000000 8042f408 80451dac 80789384 80173624 803c70b0 8014a19c
         00000000 00000f00 00000000 00000000 00000000 00000000 00000000 00000000
         00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
         ...
Call Trace:
[<8010abf0>] show_stack+0x50/0x84
[<8012100c>] __warn+0xe4/0x118
[<80121098>] warn_slowpath_null+0x1c/0x28
[<8014f7a0>] __irq_do_set_handler+0x144/0x1b4
[<8014f860>] __irq_set_handler+0x50/0x7c
[<80475f18>] rbtx4927_irq_setup+0xac/0xd0
[<8046e9c0>] start_kernel+0x288/0x450

---[ end trace f68728a0d3053b52 ]---

/proc/interrupts says:

               CPU0
      7:      11977      MIPS   7  timer
     13:      30586      TXX9  RBHMA4X00-RTL8019
     16:        293      TXX9  serial_txx9
     30:          0      TXX9  PCI error
    ERR:          3

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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