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

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

[Geert Uytterhoeven]

Hi Nemoto-san,

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.

No idea yet if this has any ill effects...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

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

[Alban Browaeys]

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


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)


Alban

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

[Marc Zyngier]

Hi Alban,

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

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

Thanks,

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

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

[Alban Browaeys]

Le vendredi 16 septembre 2016 à 08:51 +0100, Marc Zyngier a écrit :
> Hi Alban,
> 
> On 16/09/16 00:02, Alban Browaeys wrote:
> > 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?

mine is:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts

I got a report of this issue to another odroid :
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts



they both get their settings from :
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi

relevant in the chain are:
- combiner modified:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460
- gic:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi#n576
- gic and combiner initial settings:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4.dtsi#n134



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

1st batch of warnings is :

------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc6-debug+ #30
Hardware name: ODROID-U2/U3
[<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
[<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
[<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
[<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
[<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
[<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
[<c017d450>] (irq_set_chained_handler_and_data) from [<c0a15878>] (combiner_of_init+0x1a0/0x1c4)
[<c0a15878>] (combiner_of_init) from [<c0a1ead4>] (of_irq_init+0x194/0x2e8)
[<c0a1ead4>] (of_irq_init) from [<c0a07450>] (exynos_init_irq+0x8/0x3c)
[<c0a07450>] (exynos_init_irq) from [<c0a0190c>] (init_IRQ+0x2c/0x88)
[<c0a0190c>] (init_IRQ) from [<c0a00b78>] (start_kernel+0x284/0x388)
[<c0a00b78>] (start_kernel) from [<40008078>] (0x40008078)
---[ end trace f68728a0d3053b52 ]---

2nd batch is :

------------[ cut here ]------------
WARNING: CPU: 1 PID: 1 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G        W       4.8.0-rc6-debug+ #30
Hardware name: ODROID-U2/U3
[<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
[<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
[<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
[<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
[<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
[<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
[<c017d450>] (irq_set_chained_handler_and_data) from [<c038e340>] (exynos_eint_wkup_init+0x188/0x2dc)
[<c038e340>] (exynos_eint_wkup_init) from [<c038d668>] (samsung_pinctrl_probe+0x874/0xa18)
[<c038d668>] (samsung_pinctrl_probe) from [<c04342c8>] (platform_drv_probe+0x4c/0xb0)
[<c04342c8>] (platform_drv_probe) from [<c043267c>] (driver_probe_device+0x24c/0x440)
[<c043267c>] (driver_probe_device) from [<c0430658>] (bus_for_each_drv+0x64/0x98)
[<c0430658>] (bus_for_each_drv) from [<c04322e8>] (__device_attach+0xb4/0x144)
[<c04322e8>] (__device_attach) from [<c04316f4>] (bus_probe_device+0x88/0x90)
[<c04316f4>] (bus_probe_device) from [<c042f850>] (device_add+0x428/0x5c8)
[<c042f850>] (device_add) from [<c054c3f8>] (of_platform_device_create_pdata+0x84/0xb8)
[<c054c3f8>] (of_platform_device_create_pdata) from [<c054c59c>] (of_platform_bus_create+0x164/0x440)
[<c054c59c>] (of_platform_bus_create) from [<c054ca20>] (of_platform_populate+0x80/0x114)
[<c054ca20>] (of_platform_populate) from [<c0a1d458>] (of_platform_default_populate_init+0x6c/0x80)
[<c0a1d458>] (of_platform_default_populate_init) from [<c01018d4>] (do_one_initcall+0x50/0x198)
[<c01018d4>] (do_one_initcall) from [<c0a00ecc>] (kernel_init_freeable+0x250/0x2f0)
[<c0a00ecc>] (kernel_init_freeable) from [<c06c1064>] (kernel_init+0x8/0x114)
[<c06c1064>] (kernel_init) from [<c0108710>] (ret_from_fork+0x14/0x24)
---[ end trace f68728a0d3053b66 ]---



Best regards,
Alban

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

[Marc Zyngier]

+Krzystof, Kukjin,

On 16/09/16 12:03, Alban Browaeys wrote:
> Le vendredi 16 septembre 2016 à 08:51 +0100, Marc Zyngier a écrit :
>> Hi Alban,
>>
>> On 16/09/16 00:02, Alban Browaeys wrote:
>>> 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?
> 
> mine is:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts
> 
> I got a report of this issue to another odroid :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts
> 
> 
> 
> they both get their settings from :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi
> 
> relevant in the chain are:
> - combiner modified:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460

How wonderful. This section is an utter pile of crap. Really.
Having 0 as the trigger is illegal, and the valid values are fully
documented in the GIC binding. No wonder things start breaking.

> - gic:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi#n576
> - gic and combiner initial settings:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4.dtsi#n134
> 
> 
> 
>> 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).
> 
> 1st batch of warnings is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c0a15878>] (combiner_of_init+0x1a0/0x1c4)
> [<c0a15878>] (combiner_of_init) from [<c0a1ead4>] (of_irq_init+0x194/0x2e8)
> [<c0a1ead4>] (of_irq_init) from [<c0a07450>] (exynos_init_irq+0x8/0x3c)
> [<c0a07450>] (exynos_init_irq) from [<c0a0190c>] (init_IRQ+0x2c/0x88)
> [<c0a0190c>] (init_IRQ) from [<c0a00b78>] (start_kernel+0x284/0x388)
> [<c0a00b78>] (start_kernel) from [<40008078>] (0x40008078)
> ---[ end trace f68728a0d3053b52 ]---

That's our above friend the combiner.

> 
> 2nd batch is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Tainted: G        W       4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c038e340>] (exynos_eint_wkup_init+0x188/0x2dc)
> [<c038e340>] (exynos_eint_wkup_init) from [<c038d668>] (samsung_pinctrl_probe+0x874/0xa18)
> [<c038d668>] (samsung_pinctrl_probe) from [<c04342c8>] (platform_drv_probe+0x4c/0xb0)
> [<c04342c8>] (platform_drv_probe) from [<c043267c>] (driver_probe_device+0x24c/0x440)
> [<c043267c>] (driver_probe_device) from [<c0430658>] (bus_for_each_drv+0x64/0x98)
> [<c0430658>] (bus_for_each_drv) from [<c04322e8>] (__device_attach+0xb4/0x144)
> [<c04322e8>] (__device_attach) from [<c04316f4>] (bus_probe_device+0x88/0x90)
> [<c04316f4>] (bus_probe_device) from [<c042f850>] (device_add+0x428/0x5c8)
> [<c042f850>] (device_add) from [<c054c3f8>] (of_platform_device_create_pdata+0x84/0xb8)
> [<c054c3f8>] (of_platform_device_create_pdata) from [<c054c59c>] (of_platform_bus_create+0x164/0x440)
> [<c054c59c>] (of_platform_bus_create) from [<c054ca20>] (of_platform_populate+0x80/0x114)
> [<c054ca20>] (of_platform_populate) from [<c0a1d458>] (of_platform_default_populate_init+0x6c/0x80)
> [<c0a1d458>] (of_platform_default_populate_init) from [<c01018d4>] (do_one_initcall+0x50/0x198)
> [<c01018d4>] (do_one_initcall) from [<c0a00ecc>] (kernel_init_freeable+0x250/0x2f0)
> [<c0a00ecc>] (kernel_init_freeable) from [<c06c1064>] (kernel_init+0x8/0x114)
> [<c06c1064>] (kernel_init) from [<c0108710>] (ret_from_fork+0x14/0x24)
> ---[ end trace f68728a0d3053b66 ]---

And that's from the following stuff:

	&pinctrl_0 {
        	compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11400000 0x1000>;
	        interrupts = <0 47 0>;
	};

	&pinctrl_1 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11000000 0x1000>;
	        interrupts = <0 46 0>;

	        wakup_eint: wakeup-interrupt-controller {
	                compatible = "samsung,exynos4210-wakeup-eint";
	                interrupt-parent = <&gic>;
	                interrupts = <0 32 0>;
	        };
	};

	[...]

	&pinctrl_3 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x106E0000 0x1000>;
	        interrupts = <0 72 0>;
	};

which perpetuates this fine tradition...

At that stage, I'm not sure I should care. Does the workaround make your
platform usable? The Samsung maintainers should really try and fix their
DT, because it is a miracle this has made it that far.

I'm also interested in hearing from Geert, whose platform doesn't seem
to be DT driven.

Thanks,

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

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

[Marc Zyngier]

+Krzystof, Kukjin,

On 16/09/16 12:03, Alban Browaeys wrote:
> Le vendredi 16 septembre 2016 ? 08:51 +0100, Marc Zyngier a ?crit :
>> Hi Alban,
>>
>> On 16/09/16 00:02, Alban Browaeys wrote:
>>> 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?
> 
> mine is:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts
> 
> I got a report of this issue to another odroid :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts
> 
> 
> 
> they both get their settings from :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi
> 
> relevant in the chain are:
> - combiner modified:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460

How wonderful. This section is an utter pile of crap. Really.
Having 0 as the trigger is illegal, and the valid values are fully
documented in the GIC binding. No wonder things start breaking.

> - gic:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi#n576
> - gic and combiner initial settings:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4.dtsi#n134
> 
> 
> 
>> 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).
> 
> 1st batch of warnings is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c0a15878>] (combiner_of_init+0x1a0/0x1c4)
> [<c0a15878>] (combiner_of_init) from [<c0a1ead4>] (of_irq_init+0x194/0x2e8)
> [<c0a1ead4>] (of_irq_init) from [<c0a07450>] (exynos_init_irq+0x8/0x3c)
> [<c0a07450>] (exynos_init_irq) from [<c0a0190c>] (init_IRQ+0x2c/0x88)
> [<c0a0190c>] (init_IRQ) from [<c0a00b78>] (start_kernel+0x284/0x388)
> [<c0a00b78>] (start_kernel) from [<40008078>] (0x40008078)
> ---[ end trace f68728a0d3053b52 ]---

That's our above friend the combiner.

> 
> 2nd batch is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Tainted: G        W       4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c038e340>] (exynos_eint_wkup_init+0x188/0x2dc)
> [<c038e340>] (exynos_eint_wkup_init) from [<c038d668>] (samsung_pinctrl_probe+0x874/0xa18)
> [<c038d668>] (samsung_pinctrl_probe) from [<c04342c8>] (platform_drv_probe+0x4c/0xb0)
> [<c04342c8>] (platform_drv_probe) from [<c043267c>] (driver_probe_device+0x24c/0x440)
> [<c043267c>] (driver_probe_device) from [<c0430658>] (bus_for_each_drv+0x64/0x98)
> [<c0430658>] (bus_for_each_drv) from [<c04322e8>] (__device_attach+0xb4/0x144)
> [<c04322e8>] (__device_attach) from [<c04316f4>] (bus_probe_device+0x88/0x90)
> [<c04316f4>] (bus_probe_device) from [<c042f850>] (device_add+0x428/0x5c8)
> [<c042f850>] (device_add) from [<c054c3f8>] (of_platform_device_create_pdata+0x84/0xb8)
> [<c054c3f8>] (of_platform_device_create_pdata) from [<c054c59c>] (of_platform_bus_create+0x164/0x440)
> [<c054c59c>] (of_platform_bus_create) from [<c054ca20>] (of_platform_populate+0x80/0x114)
> [<c054ca20>] (of_platform_populate) from [<c0a1d458>] (of_platform_default_populate_init+0x6c/0x80)
> [<c0a1d458>] (of_platform_default_populate_init) from [<c01018d4>] (do_one_initcall+0x50/0x198)
> [<c01018d4>] (do_one_initcall) from [<c0a00ecc>] (kernel_init_freeable+0x250/0x2f0)
> [<c0a00ecc>] (kernel_init_freeable) from [<c06c1064>] (kernel_init+0x8/0x114)
> [<c06c1064>] (kernel_init) from [<c0108710>] (ret_from_fork+0x14/0x24)
> ---[ end trace f68728a0d3053b66 ]---

And that's from the following stuff:

	&pinctrl_0 {
        	compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11400000 0x1000>;
	        interrupts = <0 47 0>;
	};

	&pinctrl_1 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11000000 0x1000>;
	        interrupts = <0 46 0>;

	        wakup_eint: wakeup-interrupt-controller {
	                compatible = "samsung,exynos4210-wakeup-eint";
	                interrupt-parent = <&gic>;
	                interrupts = <0 32 0>;
	        };
	};

	[...]

	&pinctrl_3 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x106E0000 0x1000>;
	        interrupts = <0 72 0>;
	};

which perpetuates this fine tradition...

At that stage, I'm not sure I should care. Does the workaround make your
platform usable? The Samsung maintainers should really try and fix their
DT, because it is a miracle this has made it that far.

I'm also interested in hearing from Geert, whose platform doesn't seem
to be DT driven.

Thanks,

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

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

[Marc Zyngier]

+Krzystof, Kukjin,

On 16/09/16 12:03, Alban Browaeys wrote:
> Le vendredi 16 septembre 2016 à 08:51 +0100, Marc Zyngier a écrit :
>> Hi Alban,
>>
>> On 16/09/16 00:02, Alban Browaeys wrote:
>>> 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?
> 
> mine is:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts
> 
> I got a report of this issue to another odroid :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts
> 
> 
> 
> they both get their settings from :
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi
> 
> relevant in the chain are:
> - combiner modified:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460

How wonderful. This section is an utter pile of crap. Really.
Having 0 as the trigger is illegal, and the valid values are fully
documented in the GIC binding. No wonder things start breaking.

> - gic:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi#n576
> - gic and combiner initial settings:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4.dtsi#n134
> 
> 
> 
>> 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).
> 
> 1st batch of warnings is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c0a15878>] (combiner_of_init+0x1a0/0x1c4)
> [<c0a15878>] (combiner_of_init) from [<c0a1ead4>] (of_irq_init+0x194/0x2e8)
> [<c0a1ead4>] (of_irq_init) from [<c0a07450>] (exynos_init_irq+0x8/0x3c)
> [<c0a07450>] (exynos_init_irq) from [<c0a0190c>] (init_IRQ+0x2c/0x88)
> [<c0a0190c>] (init_IRQ) from [<c0a00b78>] (start_kernel+0x284/0x388)
> [<c0a00b78>] (start_kernel) from [<40008078>] (0x40008078)
> ---[ end trace f68728a0d3053b52 ]---

That's our above friend the combiner.

> 
> 2nd batch is :
> 
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Tainted: G        W       4.8.0-rc6-debug+ #30
> Hardware name: ODROID-U2/U3
> [<c010fc74>] (unwind_backtrace) from [<c010c9a0>] (show_stack+0x10/0x14)
> [<c010c9a0>] (show_stack) from [<c035cafc>] (dump_stack+0xa8/0xd4)
> [<c035cafc>] (dump_stack) from [<c01214dc>] (__warn+0xe8/0x100)
> [<c01214dc>] (__warn) from [<c01215a4>] (warn_slowpath_null+0x20/0x28)
> [<c01215a4>] (warn_slowpath_null) from [<c017d394>] (__irq_do_set_handler+0x1c0/0x1c4)
> [<c017d394>] (__irq_do_set_handler) from [<c017d450>] (irq_set_chained_handler_and_data+0x38/0x54)
> [<c017d450>] (irq_set_chained_handler_and_data) from [<c038e340>] (exynos_eint_wkup_init+0x188/0x2dc)
> [<c038e340>] (exynos_eint_wkup_init) from [<c038d668>] (samsung_pinctrl_probe+0x874/0xa18)
> [<c038d668>] (samsung_pinctrl_probe) from [<c04342c8>] (platform_drv_probe+0x4c/0xb0)
> [<c04342c8>] (platform_drv_probe) from [<c043267c>] (driver_probe_device+0x24c/0x440)
> [<c043267c>] (driver_probe_device) from [<c0430658>] (bus_for_each_drv+0x64/0x98)
> [<c0430658>] (bus_for_each_drv) from [<c04322e8>] (__device_attach+0xb4/0x144)
> [<c04322e8>] (__device_attach) from [<c04316f4>] (bus_probe_device+0x88/0x90)
> [<c04316f4>] (bus_probe_device) from [<c042f850>] (device_add+0x428/0x5c8)
> [<c042f850>] (device_add) from [<c054c3f8>] (of_platform_device_create_pdata+0x84/0xb8)
> [<c054c3f8>] (of_platform_device_create_pdata) from [<c054c59c>] (of_platform_bus_create+0x164/0x440)
> [<c054c59c>] (of_platform_bus_create) from [<c054ca20>] (of_platform_populate+0x80/0x114)
> [<c054ca20>] (of_platform_populate) from [<c0a1d458>] (of_platform_default_populate_init+0x6c/0x80)
> [<c0a1d458>] (of_platform_default_populate_init) from [<c01018d4>] (do_one_initcall+0x50/0x198)
> [<c01018d4>] (do_one_initcall) from [<c0a00ecc>] (kernel_init_freeable+0x250/0x2f0)
> [<c0a00ecc>] (kernel_init_freeable) from [<c06c1064>] (kernel_init+0x8/0x114)
> [<c06c1064>] (kernel_init) from [<c0108710>] (ret_from_fork+0x14/0x24)
> ---[ end trace f68728a0d3053b66 ]---

And that's from the following stuff:

	&pinctrl_0 {
        	compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11400000 0x1000>;
	        interrupts = <0 47 0>;
	};

	&pinctrl_1 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x11000000 0x1000>;
	        interrupts = <0 46 0>;

	        wakup_eint: wakeup-interrupt-controller {
	                compatible = "samsung,exynos4210-wakeup-eint";
	                interrupt-parent = <&gic>;
	                interrupts = <0 32 0>;
	        };
	};

	[...]

	&pinctrl_3 {
	        compatible = "samsung,exynos4x12-pinctrl";
	        reg = <0x106E0000 0x1000>;
	        interrupts = <0 72 0>;
	};

which perpetuates this fine tradition...

At that stage, I'm not sure I should care. Does the workaround make your
platform usable? The Samsung maintainers should really try and fix their
DT, because it is a miracle this has made it that far.

I'm also interested in hearing from Geert, whose platform doesn't seem
to be DT driven.

Thanks,

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

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

[Krzysztof Kozlowski]

On Fri, Sep 16, 2016 at 2:55 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> +Krzystof, Kukjin,
>
> On 16/09/16 12:03, Alban Browaeys wrote:
>> Le vendredi 16 septembre 2016 à 08:51 +0100, Marc Zyngier a écrit :
>>> Hi Alban,
>>>
>>> On 16/09/16 00:02, Alban Browaeys wrote:
>>>> 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?
>>
>> mine is:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts
>>
>> I got a report of this issue to another odroid :
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts
>>
>>
>>
>> they both get their settings from :
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi
>>
>> relevant in the chain are:
>> - combiner modified:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460
>
> How wonderful. This section is an utter pile of crap. Really.
> Having 0 as the trigger is illegal, and the valid values are fully
> documented in the GIC binding. No wonder things start breaking.


+CC Marek Szyprowski,

Yes, that is interesting pile. Lately we stomped into it as well... I
started fixing this today but didn't finish it yet.

> And that's from the following stuff:
>
>         &pinctrl_0 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x11400000 0x1000>;
>                 interrupts = <0 47 0>;
>         };
>
>         &pinctrl_1 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x11000000 0x1000>;
>                 interrupts = <0 46 0>;
>
>                 wakup_eint: wakeup-interrupt-controller {
>                         compatible = "samsung,exynos4210-wakeup-eint";
>                         interrupt-parent = <&gic>;
>                         interrupts = <0 32 0>;
>                 };
>         };
>
>         [...]
>
>         &pinctrl_3 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x106E0000 0x1000>;
>                 interrupts = <0 72 0>;
>         };
>
> which perpetuates this fine tradition...
>
> At that stage, I'm not sure I should care. Does the workaround make your
> platform usable? The Samsung maintainers should really try and fix their
> DT, because it is a miracle this has made it that far.

Miracle or coincidence. :)

Thanks Geert and Alban for bringing this up, I'll add also yours reported-by.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Fri, Sep 16, 2016 at 2:55 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> +Krzystof, Kukjin,
>
> On 16/09/16 12:03, Alban Browaeys wrote:
>> Le vendredi 16 septembre 2016 ? 08:51 +0100, Marc Zyngier a ?crit :
>>> Hi Alban,
>>>
>>> On 16/09/16 00:02, Alban Browaeys wrote:
>>>> 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?
>>
>> mine is:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts
>>
>> I got a report of this issue to another odroid :
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts
>>
>>
>>
>> they both get their settings from :
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi
>>
>> relevant in the chain are:
>> - combiner modified:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460
>
> How wonderful. This section is an utter pile of crap. Really.
> Having 0 as the trigger is illegal, and the valid values are fully
> documented in the GIC binding. No wonder things start breaking.


+CC Marek Szyprowski,

Yes, that is interesting pile. Lately we stomped into it as well... I
started fixing this today but didn't finish it yet.

> And that's from the following stuff:
>
>         &pinctrl_0 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x11400000 0x1000>;
>                 interrupts = <0 47 0>;
>         };
>
>         &pinctrl_1 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x11000000 0x1000>;
>                 interrupts = <0 46 0>;
>
>                 wakup_eint: wakeup-interrupt-controller {
>                         compatible = "samsung,exynos4210-wakeup-eint";
>                         interrupt-parent = <&gic>;
>                         interrupts = <0 32 0>;
>                 };
>         };
>
>         [...]
>
>         &pinctrl_3 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x106E0000 0x1000>;
>                 interrupts = <0 72 0>;
>         };
>
> which perpetuates this fine tradition...
>
> At that stage, I'm not sure I should care. Does the workaround make your
> platform usable? The Samsung maintainers should really try and fix their
> DT, because it is a miracle this has made it that far.

Miracle or coincidence. :)

Thanks Geert and Alban for bringing this up, I'll add also yours reported-by.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Fri, Sep 16, 2016 at 2:55 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> +Krzystof, Kukjin,
>
> On 16/09/16 12:03, Alban Browaeys wrote:
>> Le vendredi 16 septembre 2016 à 08:51 +0100, Marc Zyngier a écrit :
>>> Hi Alban,
>>>
>>> On 16/09/16 00:02, Alban Browaeys wrote:
>>>> 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?
>>
>> mine is:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts
>>
>> I got a report of this issue to another odroid :
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts
>>
>>
>>
>> they both get their settings from :
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi
>>
>> relevant in the chain are:
>> - combiner modified:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460
>
> How wonderful. This section is an utter pile of crap. Really.
> Having 0 as the trigger is illegal, and the valid values are fully
> documented in the GIC binding. No wonder things start breaking.


+CC Marek Szyprowski,

Yes, that is interesting pile. Lately we stomped into it as well... I
started fixing this today but didn't finish it yet.

> And that's from the following stuff:
>
>         &pinctrl_0 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x11400000 0x1000>;
>                 interrupts = <0 47 0>;
>         };
>
>         &pinctrl_1 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x11000000 0x1000>;
>                 interrupts = <0 46 0>;
>
>                 wakup_eint: wakeup-interrupt-controller {
>                         compatible = "samsung,exynos4210-wakeup-eint";
>                         interrupt-parent = <&gic>;
>                         interrupts = <0 32 0>;
>                 };
>         };
>
>         [...]
>
>         &pinctrl_3 {
>                 compatible = "samsung,exynos4x12-pinctrl";
>                 reg = <0x106E0000 0x1000>;
>                 interrupts = <0 72 0>;
>         };
>
> which perpetuates this fine tradition...
>
> At that stage, I'm not sure I should care. Does the workaround make your
> platform usable? The Samsung maintainers should really try and fix their
> DT, because it is a miracle this has made it that far.

Miracle or coincidence. :)

Thanks Geert and Alban for bringing this up, I'll add also yours reported-by.

Best regards,
Krzysztof

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

[Alban Browaeys]

Le vendredi 16 septembre 2016 à 13:55 +0100, Marc Zyngier a écrit :
> At that stage, I'm not sure I should care. Does the workaround make
> your
> platform usable?

However awkward this was all about logs and clearing worries. There was
no change in behavior  between the three cases : upstream, upstream
commit reverted and patched upstream.

Mind this hardware (the Odroid U2) has no hardware buttons. Only power
on plug.

Best regards,
Alban

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

[Alban Browaeys]

Le vendredi 16 septembre 2016 ? 13:55 +0100, Marc Zyngier a ?crit?:
> At that stage, I'm not sure I should care. Does the workaround make
> your
> platform usable?

However awkward this was all about logs and clearing worries. There was
no change in behavior ?between the three cases : upstream, upstream
commit reverted and patched upstream.

Mind this hardware (the Odroid U2) has no hardware buttons. Only power
on plug.

Best regards,
Alban

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

[Alban Browaeys]

Le vendredi 16 septembre 2016 à 13:55 +0100, Marc Zyngier a écrit :
> At that stage, I'm not sure I should care. Does the workaround make
> your
> platform usable?

However awkward this was all about logs and clearing worries. There was
no change in behavior  between the three cases : upstream, upstream
commit reverted and patched upstream.

Mind this hardware (the Odroid U2) has no hardware buttons. Only power
on plug.

Best regards,
Alban

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@arm.com> 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@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

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@arm.com> 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...