* [PATCH] rtc: ab8500: move device_init_wakeup() call
@ 2016-04-11 7:54 Linus Walleij
2016-04-11 8:56 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2016-04-11 7:54 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni
Cc: rtc-linux, Linus Walleij, stable, Sudeep Holla, Ulf Hansson
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(-)
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);
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: ab8500: move device_init_wakeup() call
2016-04-11 7:54 Linus Walleij
@ 2016-04-11 8:56 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2016-04-11 8:56 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni
Cc: rtc-linux@googlegroups.com, Linus Walleij, stable, Sudeep Holla,
Ulf Hansson
On Mon, Apr 11, 2016 at 9:54 AM, 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.
>
> 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:
Bah it seems this is not the real problem :(
We end up with unbalanced wake disable for some totally
different reason, maybe in the core.
Drop this patch for now.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] rtc: ab8500: move device_init_wakeup() call
@ 2016-06-02 13:53 Linus Walleij
2016-06-02 13:56 ` Linus Walleij
2016-06-03 9:04 ` Sudeep Holla
0 siblings, 2 replies; 6+ messages in thread
From: Linus Walleij @ 2016-06-02 13:53 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni
Cc: rtc-linux, Linus Walleij, stable, Sudeep Holla, Ulf Hansson
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(-)
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);
--
2.4.11
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: ab8500: move device_init_wakeup() call
2016-06-02 13:53 [PATCH] rtc: ab8500: move device_init_wakeup() call Linus Walleij
@ 2016-06-02 13:56 ` Linus Walleij
2016-06-03 9:04 ` Sudeep Holla
1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2016-06-02 13:56 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni
Cc: rtc-linux@googlegroups.com, Linus Walleij, stable, Sudeep Holla,
Ulf Hansson
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: ab8500: move device_init_wakeup() call
2016-06-02 13:53 [PATCH] rtc: ab8500: move device_init_wakeup() call Linus Walleij
2016-06-02 13:56 ` Linus Walleij
@ 2016-06-03 9:04 ` Sudeep Holla
2016-06-03 9:40 ` Ulf Hansson
1 sibling, 1 reply; 6+ messages in thread
From: Sudeep Holla @ 2016-06-03 9:04 UTC (permalink / raw)
To: Linus Walleij
Cc: Alessandro Zummo, Alexandre Belloni, Sudeep Holla, rtc-linux,
stable, Ulf Hansson
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: ab8500: move device_init_wakeup() call
2016-06-03 9:04 ` Sudeep Holla
@ 2016-06-03 9:40 ` Ulf Hansson
0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2016-06-03 9:40 UTC (permalink / raw)
To: Sudeep Holla, Linus Walleij
Cc: Alessandro Zummo, Alexandre Belloni, rtc-linux, # 4.0+
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-03 9:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-02 13:53 [PATCH] rtc: ab8500: move device_init_wakeup() call Linus Walleij
2016-06-02 13:56 ` Linus Walleij
2016-06-03 9:04 ` Sudeep Holla
2016-06-03 9:40 ` Ulf Hansson
-- strict thread matches above, loose matches on Subject: below --
2016-04-11 7:54 Linus Walleij
2016-04-11 8:56 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox