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

[Marc Zyngier]

On 18/09/16 11:26, Geert Uytterhoeven wrote:
> 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().

Right. Vintage stuff ;-). It is odd that there is not handling of
interrupt trigger at all, but maybe the HW doesn't support it
(everything looks level).

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

OK. I'll turn this a proper patch (without the WARN_ON) and post it for
Thomas to pick ASAP.

Thanks,

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