Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1452653 > unrolled thread
| Started by | John Stultz <john.stultz@linaro.org> |
|---|---|
| First post | 2016-07-30 06:40 +0200 |
| Last post | 2016-08-11 14:10 +0200 |
| Articles | 18 on this page of 38 — 6 participants |
Back to article view | Back to linux.kernel
[Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons John Stultz <john.stultz@linaro.org> - 2016-07-30 06:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-07-30 07:00 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-07-30 13:20 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Thomas Gleixner <tglx@linutronix.de> - 2016-07-30 10:20 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons John Stultz <john.stultz@linaro.org> - 2016-08-05 20:20 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-08 11:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-09 00:00 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-08 23:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-01 12:30 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons John Stultz <john.stultz@linaro.org> - 2016-08-06 23:50 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-08 11:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons John Stultz <john.stultz@linaro.org> - 2016-08-09 06:30 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-09 15:30 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-09 17:10 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-10 01:10 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-10 20:10 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-10 20:50 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-10 21:50 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-10 23:00 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-11 00:20 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-10 21:10 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-10 21:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-11 00:20 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-10 22:00 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-08 23:50 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-11 10:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-11 11:50 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-11 14:00 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-11 14:00 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-11 14:50 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Jon Hunter <jonathanh@nvidia.com> - 2016-08-11 15:30 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-11 15:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons John Stultz <john.stultz@linaro.org> - 2016-08-11 17:40 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-11 18:00 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-11 23:10 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-11 23:30 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Marc Zyngier <marc.zyngier@arm.com> - 2016-08-12 12:30 +0200
Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons Linus Walleij <linus.walleij@linaro.org> - 2016-08-11 14:10 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2016-08-10 21:10 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4H33-I0-13@gated-at.bofh.it> |
| In reply to | #1459168 |
On Wed, Aug 10, 2016 at 11:41 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > Is this platform related to the Dragonboard 410C? I've got one from > Sudeep, and it seems to work fine (though I've spotted a couple of > gotchas in the DT). Nopes this is the ARMv7 APQ8060, the original (first!) dragonboard. https://dflund.se/~triad/krad/dragonboard/ (You know me, I always use the odd hardware nobody else use...) > Do you see this symptom on all chained interrupts? Or just this > particular one? This appears on all IRQs on the PM (MFD) ASIC. Yours, Linus Walleij
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-10 21:40 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4Hw7-YV-39@gated-at.bofh.it> |
| In reply to | #1459582 |
On 10/08/16 14:50, Linus Walleij wrote: > On Wed, Aug 10, 2016 at 11:41 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > >> Is this platform related to the Dragonboard 410C? I've got one from >> Sudeep, and it seems to work fine (though I've spotted a couple of >> gotchas in the DT). > > Nopes this is the ARMv7 APQ8060, the original (first!) dragonboard. > https://dflund.se/~triad/krad/dragonboard/ > > (You know me, I always use the odd hardware nobody else use...) Guess what, I just found this exact sucker in the pile of "junk we won't ever use because it can't run mainline". I even booted one of your test images on it. Do you have a tree I can clone directly, with all the ugly patches applied? > >> Do you see this symptom on all chained interrupts? Or just this >> particular one? > > This appears on all IRQs on the PM (MFD) ASIC. These interrupts? 197: 1 0 pm8xxx 50 Edge pmic8xxx_pwrkey_release 198: 1 0 pm8xxx 51 Edge pmic8xxx_pwrkey_press 199: 8 0 pm8xxx 74 Edge pmic-keypad 200: 0 0 pm8xxx 75 Edge pmic-keypad-stuck 201: 0 0 pm8xxx 39 Edge pm8xxx_rtc_alarm How is the interrupt topology built? pm8085 -> tlmm -> GIC? Thanks, M. -- Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2016-08-11 00:20 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4K0W-2GI-23@gated-at.bofh.it> |
| In reply to | #1459692 |
On Wed, Aug 10, 2016 at 5:17 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > Guess what, I just found this exact sucker in the pile of > "junk we won't ever use because it can't run mainline". > I even booted one of your test images on it. > > Do you have a tree I can clone directly, with all the ugly patches > applied? Yeah: https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-integrator.git/log/?h=apq8060-dragonboard-ethernet The apq8060-dragonboard branch is clean (just necessary boot fixes, the flag fixes and the revert), gives the right IRQs: https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-integrator.git/log/?h=apq8060-dragonboard The webpage gives the details of how I build, my Makefile and initramfs.cpio etc if you want: https://dflund.se/~triad/krad/dragonboard/ >>> Do you see this symptom on all chained interrupts? Or just this >>> particular one? >> >> This appears on all IRQs on the PM (MFD) ASIC. > > These interrupts? > > 197: 1 0 pm8xxx 50 Edge pmic8xxx_pwrkey_release > 198: 1 0 pm8xxx 51 Edge pmic8xxx_pwrkey_press > 199: 8 0 pm8xxx 74 Edge pmic-keypad > 200: 0 0 pm8xxx 75 Edge pmic-keypad-stuck > 201: 0 0 pm8xxx 39 Edge pm8xxx_rtc_alarm Yes. Doesn' appear anymore in v4.8-rc1 > How is the interrupt topology built? pm8085 -> tlmm -> GIC? Yes AFAICT. pm8058 -> tlmm hwirq 88 -> GIC hwirq 16 Yours, Linus Walleij
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-10 22:00 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4G6Z-8uN-23@gated-at.bofh.it> |
| In reply to | #1459168 |
Hi Linus, On 10/08/16 00:03, Linus Walleij wrote: > On Tue, Aug 9, 2016 at 3:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote: > >> If that works, then does the following also work (without the above) ... >> >> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c >> index b4c1bc7c9ca2..e111b72e3162 100644 >> --- a/kernel/irq/chip.c >> +++ b/kernel/irq/chip.c >> @@ -824,6 +824,7 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle, >> irq_settings_set_norequest(desc); >> irq_settings_set_nothread(desc); >> desc->action = &chained_action; >> + __irq_set_trigger(desc, irqd_get_trigger_type(&desc->irq_data)); >> irq_startup(desc, true); >> } >> } >> >> It looks like there is a path for parent interrupts where the type >> is not getting set. If the above works then we can discuss with Thomas >> and Marc on the correct fix. > > I tried this on my problematic platform and then this happens: > > preparing networking... > [ 2.628246] ------------[ cut here ]------------ > [ 2.628303] WARNING: CPU: 0 PID: 92 at ../kernel/irq/chip.c:26 > bad_chained_irq+0x44/0x4c > [ 2.631939] Chained irq 109 should not call an action > [ 2.640008] Modules linked in: > [ 2.647909] CPU: 0 PID: 92 Comm: ip Not tainted > 4.8.0-rc1-00011-ga21e27b4cb66 #194 > [ 2.647996] Hardware name: Generic DT based system > [ 2.655486] [<c030f8c8>] (unwind_backtrace) from [<c030c714>] > (show_stack+0x10/0x14) > [ 2.660254] [<c030c714>] (show_stack) from [<c05df420>] > (dump_stack+0x78/0x8c) > [ 2.668147] [<c05df420>] (dump_stack) from [<c031cef4>] (__warn+0xec/0x104) > [ 2.675173] [<c031cef4>] (__warn) from [<c031cf44>] > (warn_slowpath_fmt+0x38/0x48) > [ 2.682033] [<c031cf44>] (warn_slowpath_fmt) from [<c0369160>] > (bad_chained_irq+0x44/0x4c) > [ 2.689687] [<c0369160>] (bad_chained_irq) from [<c0365e28>] > (__handle_irq_event_percpu+0x5c/0x290) > [ 2.697836] [<c0365e28>] (__handle_irq_event_percpu) from > [<c0366078>] (handle_irq_event_percpu+0x1c/0x58) > [ 2.706778] [<c0366078>] (handle_irq_event_percpu) from > [<c03660ec>] (handle_irq_event+0x38/0x5c) > [ 2.716498] [<c03660ec>] (handle_irq_event) from [<c03693f0>] > (handle_level_irq+0xc4/0x150) > [ 2.725438] [<c03693f0>] (handle_level_irq) from [<c036542c>] > (generic_handle_irq+0x24/0x34) > [ 2.733602] [<c036542c>] (generic_handle_irq) from [<c06127d4>] > (msm_gpio_irq_handler+0xc8/0x150) > [ 2.742280] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>] > (generic_handle_irq+0x24/0x34) > [ 2.751048] [<c036542c>] (generic_handle_irq) from [<c0365720>] > (__handle_domain_irq+0x7c/0xec) > [ 2.759901] [<c0365720>] (__handle_domain_irq) from [<c0301464>] > (gic_handle_irq+0x48/0x8c) > [ 2.768323] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>] > (__irq_svc+0x6c/0xa8) > [ 2.776644] Exception stack(0xdeca1d48 to 0xdeca1d90) > [ 2.784284] 1d40: deca1dc0 00000000 00000000 > deca0018 ffff8bd6 deca1dc0 > [ 2.789324] 1d60: 00000000 c0378ad0 60070013 00000000 00000000 > 001f3df8 c108fa04 deca1d98 > [ 2.797481] 1d80: c08b77fc c0377930 60070013 ffffffff > [ 2.805651] [<c08b8e4c>] (__irq_svc) from [<c0377930>] > (init_timer_key+0x28/0x104) > [ 2.810679] [<c0377930>] (init_timer_key) from [<c08b77fc>] > (schedule_timeout+0x48/0x410) > [ 2.818145] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>] > (msleep+0x2c/0x38) > [ 2.826399] [<c0378ad0>] (msleep) from [<c06c2980>] > (smsc911x_open+0x268/0x50c) > [ 2.833859] [<c06c2980>] (smsc911x_open) from [<c07c1994>] > (__dev_open+0xa8/0x10c) > [ 2.840887] [<c07c1994>] (__dev_open) from [<c07c1c1c>] > (__dev_change_flags+0x94/0x144) > [ 2.848525] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>] > (dev_change_flags+0x18/0x48) > [ 2.856428] [<c07c1ce4>] (dev_change_flags) from [<c0823048>] > (devinet_ioctl+0x6b0/0x768) > [ 2.865120] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>] > (sock_ioctl+0x1f4/0x2c8) > [ 2.873186] [<c07a4ac4>] (sock_ioctl) from [<c0432678>] > (do_vfs_ioctl+0x9c/0x910) > [ 2.880645] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>] > (SyS_ioctl+0x34/0x5c) > [ 2.888290] [<c0432f20>] (SyS_ioctl) from [<c0308480>] > (ret_fast_syscall+0x0/0x3c) > [ 2.895395] ---[ end trace a53e1e63b7bdfc4a ]--- > [ 2.903917] random: fast init done > [ 3.036378] random: crng init done > [ 3.883906] irq 109: nobody cared (try booting with the "irqpoll" option) > [ 3.883940] CPU: 0 PID: 92 Comm: ip Tainted: G W > 4.8.0-rc1-00011-ga21e27b4cb66 #194 > [ 3.889673] Hardware name: Generic DT based system > [ 3.898538] [<c030f8c8>] (unwind_backtrace) from [<c030c714>] > (show_stack+0x10/0x14) > [ 3.903137] [<c030c714>] (show_stack) from [<c05df420>] > (dump_stack+0x78/0x8c) > [ 3.911034] [<c05df420>] (dump_stack) from [<c0368804>] > (__report_bad_irq+0x28/0xcc) > [ 3.918065] [<c0368804>] (__report_bad_irq) from [<c0368c18>] > (note_interrupt+0x298/0x2e8) > [ 3.925971] [<c0368c18>] (note_interrupt) from [<c03660a8>] > (handle_irq_event_percpu+0x4c/0x58) > [ 3.934040] [<c03660a8>] (handle_irq_event_percpu) from > [<c03660ec>] (handle_irq_event+0x38/0x5c) > [ 3.942634] [<c03660ec>] (handle_irq_event) from [<c03693f0>] > (handle_level_irq+0xc4/0x150) > [ 3.951660] [<c03693f0>] (handle_level_irq) from [<c036542c>] > (generic_handle_irq+0x24/0x34) > [ 3.959821] [<c036542c>] (generic_handle_irq) from [<c06127d4>] > (msm_gpio_irq_handler+0xc8/0x150) > [ 3.968502] [<c06127d4>] (msm_gpio_irq_handler) from [<c036542c>] > (generic_handle_irq+0x24/0x34) > [ 3.977269] [<c036542c>] (generic_handle_irq) from [<c0365720>] > (__handle_domain_irq+0x7c/0xec) > [ 3.986122] [<c0365720>] (__handle_domain_irq) from [<c0301464>] > (gic_handle_irq+0x48/0x8c) > [ 3.994541] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>] > (__irq_svc+0x6c/0xa8) > [ 4.002865] Exception stack(0xdeca1c60 to 0xdeca1ca8) > [ 4.010507] 1c60: 00000000 c0abce68 c109f9c0 00000000 c109f9c0 > 00000000 deca0000 00000000 > [ 4.015546] 1c80: 00000282 deca1d48 c0210800 001f3df8 e080400c > deca1cb0 c0322330 c0322340 > [ 4.023701] 1ca0: 20070113 ffffffff > [ 4.031868] [<c08b8e4c>] (__irq_svc) from [<c0322340>] > (__do_softirq+0x9c/0x388) > [ 4.035166] [<c0322340>] (__do_softirq) from [<c03228f0>] > (irq_exit+0xc0/0xfc) > [ 4.042805] [<c03228f0>] (irq_exit) from [<c0365724>] > (__handle_domain_irq+0x80/0xec) > [ 4.049835] [<c0365724>] (__handle_domain_irq) from [<c0301464>] > (gic_handle_irq+0x48/0x8c) > [ 4.057735] [<c0301464>] (gic_handle_irq) from [<c08b8e4c>] > (__irq_svc+0x6c/0xa8) > [ 4.065887] Exception stack(0xdeca1d48 to 0xdeca1d90) > [ 4.073528] 1d40: deca1dc0 00000000 00000000 > deca0018 ffff8bd6 deca1dc0 > [ 4.078566] 1d60: 00000000 c0378ad0 60070013 00000000 00000000 > 001f3df8 c108fa04 deca1d98 > [ 4.086723] 1d80: c08b77fc c0377930 60070013 ffffffff > [ 4.094889] [<c08b8e4c>] (__irq_svc) from [<c0377930>] > (init_timer_key+0x28/0x104) > [ 4.099921] [<c0377930>] (init_timer_key) from [<c08b77fc>] > (schedule_timeout+0x48/0x410) > [ 4.107389] [<c08b77fc>] (schedule_timeout) from [<c0378ad0>] > (msleep+0x2c/0x38) > [ 4.115640] [<c0378ad0>] (msleep) from [<c06c2980>] > (smsc911x_open+0x268/0x50c) > [ 4.123100] [<c06c2980>] (smsc911x_open) from [<c07c1994>] > (__dev_open+0xa8/0x10c) > [ 4.130128] [<c07c1994>] (__dev_open) from [<c07c1c1c>] > (__dev_change_flags+0x94/0x144) > [ 4.137769] [<c07c1c1c>] (__dev_change_flags) from [<c07c1ce4>] > (dev_change_flags+0x18/0x48) > [ 4.145670] [<c07c1ce4>] (dev_change_flags) from [<c0823048>] > (devinet_ioctl+0x6b0/0x768) > [ 4.154357] [<c0823048>] (devinet_ioctl) from [<c07a4ac4>] > (sock_ioctl+0x1f4/0x2c8) > [ 4.162425] [<c07a4ac4>] (sock_ioctl) from [<c0432678>] > (do_vfs_ioctl+0x9c/0x910) > [ 4.169887] [<c0432678>] (do_vfs_ioctl) from [<c0432f20>] > (SyS_ioctl+0x34/0x5c) > [ 4.177529] [<c0432f20>] (SyS_ioctl) from [<c0308480>] > (ret_fast_syscall+0x0/0x3c) > [ 4.184635] handlers: > [ 4.192273] [<c036911c>] bad_chained_irq > [ 4.198255] Disabling IRQ #109 > (...) > [ 34.170316] smsc911x 1b800000.ethernet-ebi2 eth0: ISR failed > signaling test (IRQ 208) Is this platform related to the Dragonboard 410C? I've got one from Sudeep, and it seems to work fine (though I've spotted a couple of gotchas in the DT). Do you see this symptom on all chained interrupts? Or just this particular one? Thanks, M. -- Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2016-08-08 23:50 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s40AN-6RI-11@gated-at.bofh.it> |
| In reply to | #1457379 |
On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
> irq_fwspec *fwspec)
> * it now and return the interrupt number.
> */
> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> - irq_set_irq_type(virq, type);
> + irq_data = irq_get_irq_data(virq);
> + if (!irq_data)
> + return 0;
> +
> + irqd_set_trigger_type(irq_data, type);
> return virq;
> }
>
> If I revert just that, it works again.
This makes my platform work too.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-11 10:40 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4TGW-1oz-19@gated-at.bofh.it> |
| In reply to | #1458268 |
On 08/08/16 22:48, Linus Walleij wrote:
> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>
>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>> irq_fwspec *fwspec)
>> * it now and return the interrupt number.
>> */
>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>> - irq_set_irq_type(virq, type);
>> + irq_data = irq_get_irq_data(virq);
>> + if (!irq_data)
>> + return 0;
>> +
>> + irqd_set_trigger_type(irq_data, type);
>> return virq;
>> }
>>
>> If I revert just that, it works again.
>
> This makes my platform work too.
> Tested-by: Linus Walleij <linus.walleij@linaro.org>
Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
hunk doesn't fix it for me. I'm confused...
The interesting part is this:
109: 100000 0 msmgpio 88 Level (null)
which shows that the cascade interrupt has been disabled after 100000
unhandled interrupts. Somehow, this screams "misconfiguration"...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Jon Hunter <jonathanh@nvidia.com> |
|---|---|
| Date | 2016-08-11 11:50 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4UMG-24i-21@gated-at.bofh.it> |
| In reply to | #1460259 |
On 11/08/16 09:37, Marc Zyngier wrote:
> On 08/08/16 22:48, Linus Walleij wrote:
>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>
>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>> irq_fwspec *fwspec)
>>> * it now and return the interrupt number.
>>> */
>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>> - irq_set_irq_type(virq, type);
>>> + irq_data = irq_get_irq_data(virq);
>>> + if (!irq_data)
>>> + return 0;
>>> +
>>> + irqd_set_trigger_type(irq_data, type);
>>> return virq;
>>> }
>>>
>>> If I revert just that, it works again.
>>
>> This makes my platform work too.
>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>
> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> hunk doesn't fix it for me. I'm confused...
>
> The interesting part is this:
> 109: 100000 0 msmgpio 88 Level (null)
88 is the pm8058 parent interrupt and so I am surprised you would even
see this in /proc/interrupts as it should be a chained interrupt, right?
Are you seeing this with all the ethernet updates for the APQ8060 in
Linus' branch? I am curious what you see with stock v4.8-rc1 and if
interrupts work ok with the change I had proposed. Hard to tell if there
is more than one issue here.
Cheers
Jon
--
nvpublic
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-11 14:00 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4WOt-3kP-11@gated-at.bofh.it> |
| In reply to | #1460342 |
On 11/08/16 12:45, Marc Zyngier wrote:
> On 11/08/16 10:47, Jon Hunter wrote:
>>
>> On 11/08/16 09:37, Marc Zyngier wrote:
>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>
>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>> irq_fwspec *fwspec)
>>>>> * it now and return the interrupt number.
>>>>> */
>>>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>> - irq_set_irq_type(virq, type);
>>>>> + irq_data = irq_get_irq_data(virq);
>>>>> + if (!irq_data)
>>>>> + return 0;
>>>>> +
>>>>> + irqd_set_trigger_type(irq_data, type);
>>>>> return virq;
>>>>> }
>>>>>
>>>>> If I revert just that, it works again.
>>>>
>>>> This makes my platform work too.
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>> hunk doesn't fix it for me. I'm confused...
>>>
>>> The interesting part is this:
>>> 109: 100000 0 msmgpio 88 Level (null)
>>
>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>> see this in /proc/interrupts as it should be a chained interrupt, right?
>
> That's because it repeatedly fires without a proper handler, and only
> appears then.
>
>> Are you seeing this with all the ethernet updates for the APQ8060 in
>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>> interrupts work ok with the change I had proposed. Hard to tell if there
>> is more than one issue here.
>
> (mostly) stock v4.8-rc1 exhibits the same issue, and your fix doesn't
> help this particular issue. Reverting your fix *and* applying the above
> revert makes it work again. Which is just papering over the issue, as it
> only does something when the interrupt is seen for a second time.
Also: this behaviour only affects the second cascade (from the pm8058 to
the tlmm). I can perfectly configure the first one (from the tlmm to the
GIC) with your fix.
M.
--
Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-11 14:00 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4WOt-3kP-13@gated-at.bofh.it> |
| In reply to | #1460342 |
On 11/08/16 10:47, Jon Hunter wrote:
>
> On 11/08/16 09:37, Marc Zyngier wrote:
>> On 08/08/16 22:48, Linus Walleij wrote:
>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>
>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>> irq_fwspec *fwspec)
>>>> * it now and return the interrupt number.
>>>> */
>>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>> - irq_set_irq_type(virq, type);
>>>> + irq_data = irq_get_irq_data(virq);
>>>> + if (!irq_data)
>>>> + return 0;
>>>> +
>>>> + irqd_set_trigger_type(irq_data, type);
>>>> return virq;
>>>> }
>>>>
>>>> If I revert just that, it works again.
>>>
>>> This makes my platform work too.
>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>> hunk doesn't fix it for me. I'm confused...
>>
>> The interesting part is this:
>> 109: 100000 0 msmgpio 88 Level (null)
>
> 88 is the pm8058 parent interrupt and so I am surprised you would even
> see this in /proc/interrupts as it should be a chained interrupt, right?
That's because it repeatedly fires without a proper handler, and only
appears then.
> Are you seeing this with all the ethernet updates for the APQ8060 in
> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> interrupts work ok with the change I had proposed. Hard to tell if there
> is more than one issue here.
(mostly) stock v4.8-rc1 exhibits the same issue, and your fix doesn't
help this particular issue. Reverting your fix *and* applying the above
revert makes it work again. Which is just papering over the issue, as it
only does something when the interrupt is seen for a second time.
I must be missing something obvious...
M.
--
Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-11 14:50 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4XAS-3T5-7@gated-at.bofh.it> |
| In reply to | #1460342 |
On 11/08/16 10:47, Jon Hunter wrote:
>
> On 11/08/16 09:37, Marc Zyngier wrote:
>> On 08/08/16 22:48, Linus Walleij wrote:
>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>
>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>> irq_fwspec *fwspec)
>>>> * it now and return the interrupt number.
>>>> */
>>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>> - irq_set_irq_type(virq, type);
>>>> + irq_data = irq_get_irq_data(virq);
>>>> + if (!irq_data)
>>>> + return 0;
>>>> +
>>>> + irqd_set_trigger_type(irq_data, type);
>>>> return virq;
>>>> }
>>>>
>>>> If I revert just that, it works again.
>>>
>>> This makes my platform work too.
>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>> hunk doesn't fix it for me. I'm confused...
>>
>> The interesting part is this:
>> 109: 100000 0 msmgpio 88 Level (null)
>
> 88 is the pm8058 parent interrupt and so I am surprised you would even
> see this in /proc/interrupts as it should be a chained interrupt, right?
>
> Are you seeing this with all the ethernet updates for the APQ8060 in
> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> interrupts work ok with the change I had proposed. Hard to tell if there
> is more than one issue here.
Nailed the sucker:
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b4c1bc7..9d7284a 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
desc->name = name;
if (handle != handle_bad_irq && is_chained) {
+ int ret;
+
+ ret = __irq_set_trigger(desc,
+ irqd_get_trigger_type(&desc->irq_data));
+ WARN_ON(ret);
+ /*
+ * This is beyond ugly: .set_type may have overridden
+ * the flow, not not knowing that we're dealing with a
+ * chained handler. Reset it here because we know
+ * better.
+ */
+ desc->handle_irq = handle;
irq_settings_set_noprobe(desc);
irq_settings_set_norequest(desc);
irq_settings_set_nothread(desc);
Linus, Jon: Can you please confirm this fixes your respective issues?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Jon Hunter <jonathanh@nvidia.com> |
|---|---|
| Date | 2016-08-11 15:30 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4Ydz-4ms-1@gated-at.bofh.it> |
| In reply to | #1460478 |
On 11/08/16 13:46, Marc Zyngier wrote:
> On 11/08/16 10:47, Jon Hunter wrote:
>>
>> On 11/08/16 09:37, Marc Zyngier wrote:
>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>
>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>> irq_fwspec *fwspec)
>>>>> * it now and return the interrupt number.
>>>>> */
>>>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>> - irq_set_irq_type(virq, type);
>>>>> + irq_data = irq_get_irq_data(virq);
>>>>> + if (!irq_data)
>>>>> + return 0;
>>>>> +
>>>>> + irqd_set_trigger_type(irq_data, type);
>>>>> return virq;
>>>>> }
>>>>>
>>>>> If I revert just that, it works again.
>>>>
>>>> This makes my platform work too.
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>> hunk doesn't fix it for me. I'm confused...
>>>
>>> The interesting part is this:
>>> 109: 100000 0 msmgpio 88 Level (null)
>>
>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>> see this in /proc/interrupts as it should be a chained interrupt, right?
>>
>> Are you seeing this with all the ethernet updates for the APQ8060 in
>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>> interrupts work ok with the change I had proposed. Hard to tell if there
>> is more than one issue here.
>
> Nailed the sucker:
Great!
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
> desc->name = name;
>
> if (handle != handle_bad_irq && is_chained) {
> + int ret;
> +
> + ret = __irq_set_trigger(desc,
> + irqd_get_trigger_type(&desc->irq_data));
> + WARN_ON(ret);
You could wrap the entire call in the WARN_ON(). I was not sure if there
was a better way to handle that.
> + /*
> + * This is beyond ugly: .set_type may have overridden
> + * the flow, not not knowing that we're dealing with a
> + * chained handler. Reset it here because we know
> + * better.
> + */
> + desc->handle_irq = handle;
Yes I see the call to irq_set_handler in the pinctrl-msm.c set_type.
Good catch!
Apart from the above ...
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-11 15:40 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4Ynf-4pW-7@gated-at.bofh.it> |
| In reply to | #1460513 |
On 11/08/16 14:29, Jon Hunter wrote:
>
> On 11/08/16 13:46, Marc Zyngier wrote:
>> On 11/08/16 10:47, Jon Hunter wrote:
>>>
>>> On 11/08/16 09:37, Marc Zyngier wrote:
>>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>>
>>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>>> irq_fwspec *fwspec)
>>>>>> * it now and return the interrupt number.
>>>>>> */
>>>>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>>> - irq_set_irq_type(virq, type);
>>>>>> + irq_data = irq_get_irq_data(virq);
>>>>>> + if (!irq_data)
>>>>>> + return 0;
>>>>>> +
>>>>>> + irqd_set_trigger_type(irq_data, type);
>>>>>> return virq;
>>>>>> }
>>>>>>
>>>>>> If I revert just that, it works again.
>>>>>
>>>>> This makes my platform work too.
>>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>>
>>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>>> hunk doesn't fix it for me. I'm confused...
>>>>
>>>> The interesting part is this:
>>>> 109: 100000 0 msmgpio 88 Level (null)
>>>
>>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>>> see this in /proc/interrupts as it should be a chained interrupt, right?
>>>
>>> Are you seeing this with all the ethernet updates for the APQ8060 in
>>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>>> interrupts work ok with the change I had proposed. Hard to tell if there
>>> is more than one issue here.
>>
>> Nailed the sucker:
>
> Great!
>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index b4c1bc7..9d7284a 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>> desc->name = name;
>>
>> if (handle != handle_bad_irq && is_chained) {
>> + int ret;
>> +
>> + ret = __irq_set_trigger(desc,
>> + irqd_get_trigger_type(&desc->irq_data));
>> + WARN_ON(ret);
>
> You could wrap the entire call in the WARN_ON(). I was not sure if there
> was a better way to handle that.
Actually, I've decided to drop it. We already have a message in
__irq_set_trigger(), and if we really want to scream, that's the one we
should consider upgrading to a WARN_ON().
>
>> + /*
>> + * This is beyond ugly: .set_type may have overridden
>> + * the flow, not not knowing that we're dealing with a
>> + * chained handler. Reset it here because we know
>> + * better.
>> + */
>> + desc->handle_irq = handle;
>
> Yes I see the call to irq_set_handler in the pinctrl-msm.c set_type.
> Good catch!
I can't believe it took me that much time to realize that. Guess I need
an extended weekend! ;-)
>
> Apart from the above ...
>
> Acked-by: Jon Hunter <jonathanh@nvidia.com>
Thanks a lot Jon,
M.
--
Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | John Stultz <john.stultz@linaro.org> |
|---|---|
| Date | 2016-08-11 17:40 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s50fo-5Dk-11@gated-at.bofh.it> |
| In reply to | #1460478 |
On Thu, Aug 11, 2016 at 5:46 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 11/08/16 10:47, Jon Hunter wrote:
>>
>> On 11/08/16 09:37, Marc Zyngier wrote:
>>> On 08/08/16 22:48, Linus Walleij wrote:
>>>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>>
>>>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>>>> irq_fwspec *fwspec)
>>>>> * it now and return the interrupt number.
>>>>> */
>>>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>>>> - irq_set_irq_type(virq, type);
>>>>> + irq_data = irq_get_irq_data(virq);
>>>>> + if (!irq_data)
>>>>> + return 0;
>>>>> +
>>>>> + irqd_set_trigger_type(irq_data, type);
>>>>> return virq;
>>>>> }
>>>>>
>>>>> If I revert just that, it works again.
>>>>
>>>> This makes my platform work too.
>>>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
>>> hunk doesn't fix it for me. I'm confused...
>>>
>>> The interesting part is this:
>>> 109: 100000 0 msmgpio 88 Level (null)
>>
>> 88 is the pm8058 parent interrupt and so I am surprised you would even
>> see this in /proc/interrupts as it should be a chained interrupt, right?
>>
>> Are you seeing this with all the ethernet updates for the APQ8060 in
>> Linus' branch? I am curious what you see with stock v4.8-rc1 and if
>> interrupts work ok with the change I had proposed. Hard to tell if there
>> is more than one issue here.
>
> Nailed the sucker:
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
> desc->name = name;
>
> if (handle != handle_bad_irq && is_chained) {
> + int ret;
> +
> + ret = __irq_set_trigger(desc,
> + irqd_get_trigger_type(&desc->irq_data));
> + WARN_ON(ret);
> + /*
> + * This is beyond ugly: .set_type may have overridden
> + * the flow, not not knowing that we're dealing with a
> + * chained handler. Reset it here because we know
> + * better.
> + */
> + desc->handle_irq = handle;
> irq_settings_set_noprobe(desc);
> irq_settings_set_norequest(desc);
> irq_settings_set_nothread(desc);
>
> Linus, Jon: Can you please confirm this fixes your respective issues?
Yep. That works for me!
Tested-by: John Stultz <john.stultz@linaro.org>
Thanks so much for hunting this down!
-john
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-11 18:00 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s50yK-5KE-29@gated-at.bofh.it> |
| In reply to | #1460616 |
On 11/08/16 16:32, John Stultz wrote:
> On Thu, Aug 11, 2016 at 5:46 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
>> Nailed the sucker:
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index b4c1bc7..9d7284a 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
>> desc->name = name;
>>
>> if (handle != handle_bad_irq && is_chained) {
>> + int ret;
>> +
>> + ret = __irq_set_trigger(desc,
>> + irqd_get_trigger_type(&desc->irq_data));
>> + WARN_ON(ret);
>> + /*
>> + * This is beyond ugly: .set_type may have overridden
>> + * the flow, not not knowing that we're dealing with a
>> + * chained handler. Reset it here because we know
>> + * better.
>> + */
>> + desc->handle_irq = handle;
>> irq_settings_set_noprobe(desc);
>> irq_settings_set_norequest(desc);
>> irq_settings_set_nothread(desc);
>>
>> Linus, Jon: Can you please confirm this fixes your respective issues?
>
> Yep. That works for me!
>
> Tested-by: John Stultz <john.stultz@linaro.org>
>
> Thanks so much for hunting this down!
Thanks for the report and the testing! I'll post the final patch
shortly. tglx being away for a couple of weeks, it may take some time
before this hits mainline though.
M.
--
Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2016-08-11 23:10 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s55oJ-JH-1@gated-at.bofh.it> |
| In reply to | #1460478 |
On Thu, Aug 11, 2016 at 2:46 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Nailed the sucker:
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
> desc->name = name;
>
> if (handle != handle_bad_irq && is_chained) {
> + int ret;
> +
> + ret = __irq_set_trigger(desc,
> + irqd_get_trigger_type(&desc->irq_data));
> + WARN_ON(ret);
> + /*
> + * This is beyond ugly: .set_type may have overridden
> + * the flow, not not knowing that we're dealing with a
> + * chained handler. Reset it here because we know
> + * better.
> + */
> + desc->handle_irq = handle;
> irq_settings_set_noprobe(desc);
> irq_settings_set_norequest(desc);
> irq_settings_set_nothread(desc);
>
> Linus, Jon: Can you please confirm this fixes your respective issues?
Yes, bullseye.
Tested-by: Linus Walleij <linus.walleij@linaro.org>
Thanks for your efforts!
Yours,
Linus Walleij
[toc] | [prev] | [next] | [standalone]
| From | Bjorn Andersson <bjorn.andersson@linaro.org> |
|---|---|
| Date | 2016-08-11 23:30 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s55I6-S7-3@gated-at.bofh.it> |
| In reply to | #1460478 |
On Thu 11 Aug 05:46 PDT 2016, Marc Zyngier wrote:
> On 11/08/16 10:47, Jon Hunter wrote:
> >
> > On 11/08/16 09:37, Marc Zyngier wrote:
> >> On 08/08/16 22:48, Linus Walleij wrote:
> >>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
> >>>
> >>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
> >>>> irq_fwspec *fwspec)
> >>>> * it now and return the interrupt number.
> >>>> */
> >>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> >>>> - irq_set_irq_type(virq, type);
> >>>> + irq_data = irq_get_irq_data(virq);
> >>>> + if (!irq_data)
> >>>> + return 0;
> >>>> +
> >>>> + irqd_set_trigger_type(irq_data, type);
> >>>> return virq;
> >>>> }
> >>>>
> >>>> If I revert just that, it works again.
> >>>
> >>> This makes my platform work too.
> >>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> >>
> >> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> >> hunk doesn't fix it for me. I'm confused...
> >>
> >> The interesting part is this:
> >> 109: 100000 0 msmgpio 88 Level (null)
> >
> > 88 is the pm8058 parent interrupt and so I am surprised you would even
> > see this in /proc/interrupts as it should be a chained interrupt, right?
> >
> > Are you seeing this with all the ethernet updates for the APQ8060 in
> > Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> > interrupts work ok with the change I had proposed. Hard to tell if there
> > is more than one issue here.
>
> Nailed the sucker:
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b4c1bc7..9d7284a 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
> desc->name = name;
>
> if (handle != handle_bad_irq && is_chained) {
> + int ret;
> +
> + ret = __irq_set_trigger(desc,
> + irqd_get_trigger_type(&desc->irq_data));
> + WARN_ON(ret);
> + /*
> + * This is beyond ugly: .set_type may have overridden
> + * the flow, not not knowing that we're dealing with a
> + * chained handler. Reset it here because we know
> + * better.
> + */
Thanks for this Marc!
But it makes me (author of pinctrl-msm) wonder, am I supposed to not
implement .set_type like this for handling the transition between edge
and level handlers?
Regards,
Bjorn
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-12 12:30 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s5hSV-p0-3@gated-at.bofh.it> |
| In reply to | #1460806 |
On Thu, 11 Aug 2016 14:23:53 -0700
Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
Hi Bjorn,
> On Thu 11 Aug 05:46 PDT 2016, Marc Zyngier wrote:
>
> > On 11/08/16 10:47, Jon Hunter wrote:
> > >
> > > On 11/08/16 09:37, Marc Zyngier wrote:
> > >> On 08/08/16 22:48, Linus Walleij wrote:
> > >>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
> > >>>
> > >>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
> > >>>> irq_fwspec *fwspec)
> > >>>> * it now and return the interrupt number.
> > >>>> */
> > >>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> > >>>> - irq_set_irq_type(virq, type);
> > >>>> + irq_data = irq_get_irq_data(virq);
> > >>>> + if (!irq_data)
> > >>>> + return 0;
> > >>>> +
> > >>>> + irqd_set_trigger_type(irq_data, type);
> > >>>> return virq;
> > >>>> }
> > >>>>
> > >>>> If I revert just that, it works again.
> > >>>
> > >>> This makes my platform work too.
> > >>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
> > >>
> > >> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> > >> hunk doesn't fix it for me. I'm confused...
> > >>
> > >> The interesting part is this:
> > >> 109: 100000 0 msmgpio 88 Level (null)
> > >
> > > 88 is the pm8058 parent interrupt and so I am surprised you would even
> > > see this in /proc/interrupts as it should be a chained interrupt, right?
> > >
> > > Are you seeing this with all the ethernet updates for the APQ8060 in
> > > Linus' branch? I am curious what you see with stock v4.8-rc1 and if
> > > interrupts work ok with the change I had proposed. Hard to tell if there
> > > is more than one issue here.
> >
> > Nailed the sucker:
> >
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index b4c1bc7..9d7284a 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -820,6 +820,18 @@ __irq_do_set_handler(struct irq_desc *desc, irq_flow_handler_t handle,
> > desc->name = name;
> >
> > if (handle != handle_bad_irq && is_chained) {
> > + int ret;
> > +
> > + ret = __irq_set_trigger(desc,
> > + irqd_get_trigger_type(&desc->irq_data));
> > + WARN_ON(ret);
> > + /*
> > + * This is beyond ugly: .set_type may have overridden
> > + * the flow, not not knowing that we're dealing with a
> > + * chained handler. Reset it here because we know
> > + * better.
> > + */
>
> Thanks for this Marc!
>
> But it makes me (author of pinctrl-msm) wonder, am I supposed to not
> implement .set_type like this for handling the transition between edge
> and level handlers?
You definitely need to implement .set_type and set the flow handler
the way you do it, and there is hardly anything an irqchip driver can do
to detect that case.
The main issue is that as far as the core code is concerned, the
chained handler is just another flow handler. It just happen to be
provided by another irqchip.
We used to call .set_type early, and a driver like yours would set the
flow corresponding to the trigger of the interrupt. Later, the
secondary irqchip would then call irq_set_chained_handler_and_data(),
which would override the flow with the custom one. Now, we call it much
later, after the custom flow handler has been assigned. Kaboom, we
end-up calling the chained_action handler through one of the normal
flows, instead of going into our special flow.
We *could* make irq_set_handler_locked() check for this condition
though (testing for the chained_action pointer), probably at the cost of
un-inlining it.
I'll try to put something together next week, and see what sticks.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
[toc] | [prev] | [next] | [standalone]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2016-08-11 14:10 +0200 |
| Subject | Re: [Regression] "irqdomain: Don't set type when mapping an IRQ" breaks nexus7 gpio buttons |
| Message-ID | <s4WY9-3Dt-5@gated-at.bofh.it> |
| In reply to | #1460259 |
On Thu, Aug 11, 2016 at 10:37 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 08/08/16 22:48, Linus Walleij wrote:
>> On Sat, Aug 6, 2016 at 1:45 AM, John Stultz <john.stultz@linaro.org> wrote:
>>
>>> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct
>>> irq_fwspec *fwspec)
>>> * it now and return the interrupt number.
>>> */
>>> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
>>> - irq_set_irq_type(virq, type);
>>> + irq_data = irq_get_irq_data(virq);
>>> + if (!irq_data)
>>> + return 0;
>>> +
>>> + irqd_set_trigger_type(irq_data, type);
>>> return virq;
>>> }
>>>
>>> If I revert just that, it works again.
>>
>> This makes my platform work too.
>> Tested-by: Linus Walleij <linus.walleij@linaro.org>
>
> Hmmm. I'm now booting your kernel on the APQ8060, and reverting this
> hunk doesn't fix it for me. I'm confused...
Not quite following ... If you build the branch apq8060-dragonboard
which has these patches:
ARM: dts: MSM8660 remove flags from SPMI/MPP IRQs
Revert "irqdomain: Don't set type when mapping an IRQ
iio: pressure: bmp280: fix runtime suspend/resume crash
DO NOT MERGE: ARM: qcom: uglyfix
(I'm sorry about the two patches in the bottom needed to boot
this platform...)
You should be seeing e.g. the power button IRQs appear if you press
the "power" button close to the DC connector.
If you remove the revert, the interrupts are gone. For me, atleaset,
no reaction to the power button.
The issue is the same with any PMIC IRQ I try to use: sensors,
ethernet, SD card slot ... the SD card slot can also be tested right
off just like the power button. Just insert/eject an SD card in the
primary card slot.
Yours,
Linus Walleij
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web