2019-03-15 18:52:12

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH] rtc: cros-ec: Fail suspend/resume if wake IRQ can't be configured

If we encounter a failure during suspend where this RTC was programmed
to wakeup the system from suspend, but that wakeup couldn't be
configured because the system didn't support wakeup interrupts, we'll
run into the following warning:

Unbalanced IRQ 166 wake disable
WARNING: CPU: 7 PID: 3071 at kernel/irq/manage.c:669 irq_set_irq_wake+0x108/0x278

This happens because the suspend process isn't aborted when the RTC
fails to configure the wakeup IRQ. Instead, we continue suspending the
system and then another suspend callback fails the suspend process and
"unwinds" the previously suspended drivers by calling their resume
callbacks. When we get back to resuming this RTC driver, we'll call
disable_irq_wake() on an IRQ that hasn't been configured for wake.

Let's just fail suspend/resume here if we can't configure the system to
wake and the user has chosen to wakeup with this device. This fixes this
warning and makes the code more robust in case there are systems out
there that can't wakeup from suspend on this line but the user has
chosen to do so.

Cc: Enric Balletbo i Serra <[email protected]>
Cc: Evan Green <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/rtc/rtc-cros-ec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
index e5444296075e..4d6bf9304ceb 100644
--- a/drivers/rtc/rtc-cros-ec.c
+++ b/drivers/rtc/rtc-cros-ec.c
@@ -298,7 +298,7 @@ static int cros_ec_rtc_suspend(struct device *dev)
struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);

if (device_may_wakeup(dev))
- enable_irq_wake(cros_ec_rtc->cros_ec->irq);
+ return enable_irq_wake(cros_ec_rtc->cros_ec->irq);

return 0;
}
@@ -309,7 +309,7 @@ static int cros_ec_rtc_resume(struct device *dev)
struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);

if (device_may_wakeup(dev))
- disable_irq_wake(cros_ec_rtc->cros_ec->irq);
+ return disable_irq_wake(cros_ec_rtc->cros_ec->irq);

return 0;
}
--
Sent by a computer through tubes



2019-03-15 21:19:54

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH] rtc: cros-ec: Fail suspend/resume if wake IRQ can't be configured

Hi Stephen,

On Fri, Mar 15, 2019 at 11:51:12AM -0700, Stephen Boyd wrote:
> If we encounter a failure during suspend where this RTC was programmed
> to wakeup the system from suspend, but that wakeup couldn't be
> configured because the system didn't support wakeup interrupts, we'll
> run into the following warning:
>
> Unbalanced IRQ 166 wake disable
> WARNING: CPU: 7 PID: 3071 at kernel/irq/manage.c:669 irq_set_irq_wake+0x108/0x278
>
> This happens because the suspend process isn't aborted when the RTC
> fails to configure the wakeup IRQ. Instead, we continue suspending the
> system and then another suspend callback fails the suspend process and
> "unwinds" the previously suspended drivers by calling their resume
> callbacks. When we get back to resuming this RTC driver, we'll call
> disable_irq_wake() on an IRQ that hasn't been configured for wake.
>
> Let's just fail suspend/resume here if we can't configure the system to
> wake and the user has chosen to wakeup with this device. This fixes this
> warning and makes the code more robust in case there are systems out
> there that can't wakeup from suspend on this line but the user has
> chosen to do so.
>
> Cc: Enric Balletbo i Serra <[email protected]>
> Cc: Evan Green <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>

Acked-By: Benson Leung <[email protected]>


--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (1.60 kB)
signature.asc (849.00 B)
Download all attachments

2019-03-20 10:39:16

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH] rtc: cros-ec: Fail suspend/resume if wake IRQ can't be configured

On 15/03/2019 11:51:12-0700, Stephen Boyd wrote:
> If we encounter a failure during suspend where this RTC was programmed
> to wakeup the system from suspend, but that wakeup couldn't be
> configured because the system didn't support wakeup interrupts, we'll
> run into the following warning:
>
> Unbalanced IRQ 166 wake disable
> WARNING: CPU: 7 PID: 3071 at kernel/irq/manage.c:669 irq_set_irq_wake+0x108/0x278
>
> This happens because the suspend process isn't aborted when the RTC
> fails to configure the wakeup IRQ. Instead, we continue suspending the
> system and then another suspend callback fails the suspend process and
> "unwinds" the previously suspended drivers by calling their resume
> callbacks. When we get back to resuming this RTC driver, we'll call
> disable_irq_wake() on an IRQ that hasn't been configured for wake.
>
> Let's just fail suspend/resume here if we can't configure the system to
> wake and the user has chosen to wakeup with this device. This fixes this
> warning and makes the code more robust in case there are systems out
> there that can't wakeup from suspend on this line but the user has
> chosen to do so.
>
> Cc: Enric Balletbo i Serra <[email protected]>
> Cc: Evan Green <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/rtc/rtc-cros-ec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Applied, thanks.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com