| Message ID | 1464875584-31505-1-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 2, 2016 at 3:53 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > commit 93a6f9168f2f ("rtc: ab8500: remove misuse of IRQF_NO_SUSPEND flag") > introduced the dev_pm_set_wake_irq() call to register a wake > IRQ for the AB8500 driver. If this looks like a deja vu it is because I sent it before, then thought my suspend/resume bugs were due to something else, fixed the other things, and return to the conclusion that this patch is indeed needed too. Sorry if it's confusing, please apply this patch. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/06/16 14:53, Linus Walleij wrote: > commit 93a6f9168f2f ("rtc: ab8500: remove misuse of IRQF_NO_SUSPEND flag") > introduced the dev_pm_set_wake_irq() call to register a wake > IRQ for the AB8500 driver. > > However this causes a regression since device_init_wakeup() must be > called *after* dev_pm_set_wake_irq() not *before* it. Before this > patch we get an error message like this during system resume from > sleep: > I am unable to understand this. Because it's the exact same sequence in rtc-pl031.c which works fine for me on ARM64 Juno platform. So I am struggle to understand the linkage of the backtrace to this calling sequence as I can't see any dependency. Let me know if you have already figured out where exactly does it go wrong. Prior to commit 93a6f9168f2f, set_irq_wake was never tested and hence one possible reason I can think of is set_irq_wake_real may be failing on suspend but the error value is ignored. -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3 June 2016 at 11:04, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > On 02/06/16 14:53, Linus Walleij wrote: >> >> commit 93a6f9168f2f ("rtc: ab8500: remove misuse of IRQF_NO_SUSPEND flag") >> introduced the dev_pm_set_wake_irq() call to register a wake >> IRQ for the AB8500 driver. >> >> However this causes a regression since device_init_wakeup() must be >> called *after* dev_pm_set_wake_irq() not *before* it. Before this >> patch we get an error message like this during system resume from >> sleep: I think it's actually the opposite. device_init_wakeup() must be called *before* dev_pm_set_wake_irq(), as otherwise dev_pm_set_wake_irq() will fail. Indeed this probably "solves" the problem for you, although only by hiding it, as there is no error check of the return code from dev_pm_set_wake_irq(). >> > > I am unable to understand this. Because it's the exact same sequence in > rtc-pl031.c which works fine for me on ARM64 Juno platform. So I am > struggle to understand the linkage of the backtrace to this calling > sequence as I can't see any dependency. Let me know if you have already > figured out where exactly does it go wrong. > > Prior to commit 93a6f9168f2f, set_irq_wake was never tested and hence > one possible reason I can think of is set_irq_wake_real may be failing > on suspend but the error value is ignored. Yes, something else is wrong here! Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/rtc/rtc-ab8500.c b/drivers/rtc/rtc-ab8500.c index 24a0af650a1b..5e9cb4132eb1 100644 --- a/drivers/rtc/rtc-ab8500.c +++ b/drivers/rtc/rtc-ab8500.c @@ -482,8 +482,6 @@ static int ab8500_rtc_probe(struct platform_device *pdev) return -ENODEV; } - device_init_wakeup(&pdev->dev, true); - rtc = devm_rtc_device_register(&pdev->dev, "ab8500-rtc", (struct rtc_class_ops *)platid->driver_data, THIS_MODULE); @@ -500,6 +498,8 @@ static int ab8500_rtc_probe(struct platform_device *pdev) return err; dev_pm_set_wake_irq(&pdev->dev, irq); + device_init_wakeup(&pdev->dev, true); + platform_set_drvdata(pdev, rtc); err = ab8500_sysfs_rtc_register(&pdev->dev);
commit 93a6f9168f2f ("rtc: ab8500: remove misuse of IRQF_NO_SUSPEND flag") introduced the dev_pm_set_wake_irq() call to register a wake IRQ for the AB8500 driver. However this causes a regression since device_init_wakeup() must be called *after* dev_pm_set_wake_irq() not *before* it. Before this patch we get an error message like this during system resume from sleep: [ 217.585571] PM: noirq resume of devices complete after 0.762 msecs [ 217.585815] ------------[ cut here ]------------ [ 217.585845] WARNING: CPU: 0 PID: 200 at ../kernel/irq/manage.c:603 irq_set_irq_wake+0xc4/0xf8 [ 217.585845] Unbalanced IRQ 396 wake disable [ 217.585845] Modules linked in: [ 217.585876] CPU: 0 PID: 200 Comm: sh Tainted: G W 4.6.0-rc1-00010-gc6ade5a1da15-dirty #123 [ 217.585876] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support) [ 217.585906] [<c010e81c>] (unwind_backtrace) from [<c010b1b4>] (show_stack+0x10/0x14) [ 217.585937] [<c010b1b4>] (show_stack) from [<c034a150>] (dump_stack+0x8c/0xa0) [ 217.585937] [<c034a150>] (dump_stack) from [<c0121138>] (__warn+0xe8/0x100) [ 217.585937] [<c0121138>] (__warn) from [<c0121188>] (warn_slowpath_fmt+0x38/0x48) [ 217.585967] [<c0121188>] (warn_slowpath_fmt) from [<c0164f84>] (irq_set_irq_wake+0xc4/0xf8) [ 217.585967] [<c0164f84>] (irq_set_irq_wake) from [<c03c3648>] (device_wakeup_disarm_wake_irqs+0x30/0x48) [ 217.585998] [<c03c3648>] (device_wakeup_disarm_wake_irqs) from [<c03c152c>] (dpm_resume_noirq+0x208/0x21c) [ 217.585998] [<c03c152c>] (dpm_resume_noirq) from [<c0160248>] (suspend_devices_and_enter+0x1dc/0x444) [ 217.585998] [<c0160248>] (suspend_devices_and_enter) from [<c01606ac>] (pm_suspend+0x1fc/0x25c) [ 217.585998] [<c01606ac>] (pm_suspend) from [<c015f420>] (state_store+0x68/0xb8) [ 217.586029] [<c015f420>] (state_store) from [<c0266758>] (kernfs_fop_write+0xb8/0x1b4) [ 217.586029] [<c0266758>] (kernfs_fop_write) from [<c0204978>] (__vfs_write+0x1c/0xd8) [ 217.586059] [<c0204978>] (__vfs_write) from [<c02057fc>] (vfs_write+0x90/0x19c) [ 217.586059] [<c02057fc>] (vfs_write) from [<c02067a0>] (SyS_write+0x44/0x9c) [ 217.586059] [<c02067a0>] (SyS_write) from [<c0107740>] (ret_fast_syscall+0x0/0x3c) [ 217.586090] ---[ end trace 78c75240315212f4 ]--- This patch fixes it. Cc: stable@vger.kernel.org Fixes: 93a6f9168f2f ("rtc: ab8500: remove misuse of IRQF_NO_SUSPEND flag") Cc: Sudeep Holla <sudeep.holla@arm.com> Cc: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/rtc/rtc-ab8500.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.4.11 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html