Unbalanced IRQ wake disable during resume from static suspend

Unbalanced IRQ wake disable during resume from static suspend

[Paul Walmsley]

Hi,

On current linux-omap master (commit 
7fd1cffc56ca9944fec583eb2ecda5ce88b36bff "Linux-omap rebuilt: Updated to 
-rc4"), when resuming from static suspend, several "Unbalanced IRQ 
wake disable" warnings are generated, one for each of the serial ports.
This is on Beagle 35xx rev C2.

Is anyone else seeing this?


- Paul

root@beagleboard:~# echo 1 > /debug/pm_debug/sleep_while_idle
root@beagleboard:~# echo 1 > /debug/pm_debug/enable_off_mode
root@beagleboard:~# echo 5 > /sys/devices/platform/omap/omap-hsuart.0/sleep_timeout
root@beagleboard:~# echo 5 > /sys/devices/platform/omap/omap-hsuart.1/sleep_timeout
root@beagleboard:~# echo 5 > /sys/devices/platform/omap/omap-hsuart.2/sleep_timeout
root@beagleboard:~# echo enabled > /sys/devices/platform/omap/omap-hsuart.0/tty/ttyO0/power/wakeup
root@beagleboard:~# echo enabled > /sys/devices/platform/omap/omap-hsuart.1/tty/ttyO1/power/wakeup
root@beagleboard:~# echo enabled > /sys/devices/platform/omap/omap-hsuart.2/tty/ttyO2/power/wakeup
root@beagleboard:~# 
root@beagleboard:~# 
root@beagleboard:~# 
root@beagleboard:~# echo mem > /sys/power/state 
[   29.023498] PM: Syncing filesystems ... done.
[   29.548889] PM: Preparing system for mem sleep
[   29.564788] Freezing user space processes ... (elapsed 0.01 seconds) done.
[   29.589416] Freezing remaining freezable tasks ... (elapsed 0.02 seconds) done.
[   29.620666] PM: Entering mem sleep
[   29.626098] Suspending console(s) (use no_console_suspend to debug)
[   29.752685] PM: suspend of devices complete after 115.386 msecs
[   29.756103] PM: late suspend of devices complete after 3.387 msecs
[   29.756195] Disabling non-boot CPUs ...
[   29.756896] Powerdomain (mpu_pwrdm) didn't enter target state 0
[   29.756927] Powerdomain (neon_pwrdm) didn't enter target state 0
[   29.756958] Powerdomain (core_pwrdm) didn't enter target state 0
[   29.756958] Powerdomain (per_pwrdm) didn't enter target state 0
[   29.756988] Could not enter target state in pm_suspend
[   29.759094] PM: early resume of devices complete after 1.922 msecs
[   29.759887] ------------[ cut here ]------------
[   29.759948] WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
[   29.759948] Unbalanced IRQ 72 wake disable
[   29.759979] Modules linked in:
[   29.760009] [<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>] (warn_slowpath_common+0x4c/0x64)
[   29.760070] [<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c
)
[   29.760101] [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>] (set_irq_wake+0x80/0xe4)
[   29.760131] [<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>] (uart_resume_port+0x84/0x248)
[   29.760192] [<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>] (serial_omap_resume+0x20/0x2c)
[   29.760223] [<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>] (platform_pm_resume+0x48/0x54)
[   29.760253] [<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>] (pm_op+0x6c/0xac)
[   29.760284] [<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>] (device_resume+0x58/0x10c)
[   29.760314] [<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>] (dpm_resume_end+0xf4/0x360)
[   29.760375] [<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>] (suspend_devices_and_enter+0x1ac/
0x200)
[   29.760406] [<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>] (enter_state+0xe0/0x1
38)
[   29.760437] [<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>] (state_store+0x90/0xb8)
[   29.760498] [<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>] (kobj_attr_store+0x18/0x1c)
[   29.760528] [<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>] (sysfs_write_file+0x10c/0x144)
[   29.760589] [<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>] (vfs_write+0xac/0x134)
[   29.760620] [<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>] (sys_write+0x3c/0x68)
[   29.760650] [<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>] (ret_fast_syscall+0x0/0x3c)
[   29.760681] ---[ end trace 19fe50b7b47ba94f ]---
[   29.760742] ------------[ cut here ]------------
[   29.760772] WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
[   29.760803] Unbalanced IRQ 73 wake disable
[   29.760803] Modules linked in:
[   29.760833] [<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>] (warn_slowpath_common+0x4c/0x64)
[   29.760864] [<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c
)
[   29.760894] [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>] (set_irq_wake+0x80/0xe4)
[   29.760955] [<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>] (uart_resume_port+0x84/0x248)
[   29.760986] [<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>] (serial_omap_resume+0x20/0x2c)
[   29.761016] [<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>] (platform_pm_resume+0x48/0x54)
[   29.761047] [<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>] (pm_op+0x6c/0xac)
[   29.761077] [<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>] (device_resume+0x58/0x10c)
[   29.761108] [<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>] (dpm_resume_end+0xf4/0x360)
[   29.761138] [<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>] (suspend_devices_and_enter+0x1ac/
0x200)
[   29.761199] [<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>] (enter_state+0xe0/0x1
38)
[   29.761230] [<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>] (state_store+0x90/0xb8)
[   29.761260] [<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>] (kobj_attr_store+0x18/0x1c)
[   29.761291] [<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>] (sysfs_write_file+0x10c/0x144)
[   29.761352] [<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>] (vfs_write+0xac/0x134)
[   29.761383] [<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>] (sys_write+0x3c/0x68)
[   29.761413] [<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>] (ret_fast_syscall+0x0/0x3c)
[   29.761444] ---[ end trace 19fe50b7b47ba950 ]---
[   29.761474] ------------[ cut here ]------------
[   29.761505] WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
[   29.761535] Unbalanced IRQ 74 wake disable
[   29.761535] Modules linked in:
[   29.761566] [<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>] (warn_slowpath_common+0x4c/0x64)
[   29.761596] [<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c
)
[   29.761657] [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>] (set_irq_wake+0x80/0xe4)
[   29.761688] [<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>] (uart_resume_port+0x84/0x248)
[   29.761718] [<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>] (serial_omap_resume+0x20/0x2c)
[   29.761749] [<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>] (platform_pm_resume+0x48/0x54)
[   29.761779] [<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>] (pm_op+0x6c/0xac)
[   29.761810] [<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>] (device_resume+0x58/0x10c)
[   29.761840] [<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>] (dpm_resume_end+0xf4/0x360)
[   29.761871] [<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>] (suspend_devices_and_enter+0x1ac/
0x200)
[   29.761932] [<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>] (enter_state+0xe0/0x1
38)
[   29.761962] [<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>] (state_store+0x90/0xb8)
[   29.761993] [<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>] (kobj_attr_store+0x18/0x1c)
[   29.762023] [<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>] (sysfs_write_file+0x10c/0x144)
[   29.762054] [<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>] (vfs_write+0xac/0x134)
[   29.762115] [<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>] (sys_write+0x3c/0x68)
[   29.762145] [<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>] (ret_fast_syscall+0x0/0x3c)
[   29.762145] ---[ end trace 19fe50b7b47ba951 ]---
[   30.040374] PM: resume of devices complete after 280.883 msecs
[   30.044677] omap_device: i2c_omap.1: new worst case deactivate latency 0: 183105
[   30.663726] PM: Finishing wakeup.
[   30.667327] Restarting tasks ... done.
root@beagleboard:~#

Re: Unbalanced IRQ wake disable during resume from static suspend

[Paul Walmsley]

On Thu, 2 Dec 2010, Paul Walmsley wrote:

> On current linux-omap master (commit 
> 7fd1cffc56ca9944fec583eb2ecda5ce88b36bff "Linux-omap rebuilt: Updated to 
> -rc4"), when resuming from static suspend, several "Unbalanced IRQ 
> wake disable" warnings are generated, one for each of the serial ports.
> This is on Beagle 35xx rev C2.

Just to follow up on this, these messages are not logged when 
/sys/devices/platform/omap/omap-hsuart.*/tty/ttyO*/power/wakeup are set to 
'disabled'.

Also this is with omap2plus_defconfig.


- Paul

RE: Unbalanced IRQ wake disable during resume from static suspend

[Santosh Shilimkar]

Paul,
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Paul Walmsley
> Sent: Thursday, December 02, 2010 1:44 PM
> To: linux-omap@vger.kernel.org; Govindraj
> Cc: khilman@deeprootsystems.com
> Subject: Re: Unbalanced IRQ wake disable during resume from static
suspend
>
> On Thu, 2 Dec 2010, Paul Walmsley wrote:
>
> > On current linux-omap master (commit
> > 7fd1cffc56ca9944fec583eb2ecda5ce88b36bff "Linux-omap rebuilt: Updated
to
> > -rc4"), when resuming from static suspend, several "Unbalanced IRQ
> > wake disable" warnings are generated, one for each of the serial
ports.
> > This is on Beagle 35xx rev C2.
>
> Just to follow up on this, these messages are not logged when
> /sys/devices/platform/omap/omap-hsuart.*/tty/ttyO*/power/wakeup are set
to
> 'disabled'.
>
> Also this is with omap2plus_defconfig.
>
Just a wild guess here but is this because the 'set_wake' is
not setup and then fw might be returning some error whenever
driver invoke this API as part of enable_irq_wake() callback

If that being the case, below patch might might help. Can
somebody try this out ?

Regards,
Santosh

omap: irq: Dummy handler for enable_irq_wake

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/irq.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 32eeabe..6a964d3 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -143,11 +143,21 @@ static void omap_mask_ack_irq(unsigned int irq)
 	omap_ack_irq(irq);
 }

+#ifdef CONFIG_PM
+static int omap_set_wake(unsigned int irq, unsigned int on)
+{
+	return 0;
+}
+#else
+#define omap_set_wake NULL
+#endif
+
 static struct irq_chip omap_irq_chip = {
 	.name	= "INTC",
 	.ack	= omap_mask_ack_irq,
 	.mask	= omap_mask_irq,
 	.unmask	= omap_unmask_irq,
+	.set_wake = omap_set_wake,
 };

 static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)
-- 
1.6.0.4

Re: Unbalanced IRQ wake disable during resume from static suspend

[Govindraj]

On Thu, Dec 2, 2010 at 2:38 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> Paul,
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Paul Walmsley
>> Sent: Thursday, December 02, 2010 1:44 PM
>> To: linux-omap@vger.kernel.org; Govindraj
>> Cc: khilman@deeprootsystems.com
>> Subject: Re: Unbalanced IRQ wake disable during resume from static
> suspend
>>
>> On Thu, 2 Dec 2010, Paul Walmsley wrote:
>>
>> > On current linux-omap master (commit
>> > 7fd1cffc56ca9944fec583eb2ecda5ce88b36bff "Linux-omap rebuilt: Updated
> to
>> > -rc4"), when resuming from static suspend, several "Unbalanced IRQ
>> > wake disable" warnings are generated, one for each of the serial
> ports.
>> > This is on Beagle 35xx rev C2.
>>
>> Just to follow up on this, these messages are not logged when
>> /sys/devices/platform/omap/omap-hsuart.*/tty/ttyO*/power/wakeup are set
> to
>> 'disabled'.
>>
>> Also this is with omap2plus_defconfig.
>>
> Just a wild guess here but is this because the 'set_wake' is
> not setup and then fw might be returning some error whenever
> driver invoke this API as part of enable_irq_wake() callback
>
> If that being the case, below patch might might help. Can
> somebody try this out ?
>

Tested on 3630SDP.
The below patch helps to avoid the warnings.

--
Regards
Govindraj.R


> Regards,
> Santosh
>
> omap: irq: Dummy handler for enable_irq_wake
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/mach-omap2/irq.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 32eeabe..6a964d3 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -143,11 +143,21 @@ static void omap_mask_ack_irq(unsigned int irq)
>        omap_ack_irq(irq);
>  }
>
> +#ifdef CONFIG_PM
> +static int omap_set_wake(unsigned int irq, unsigned int on)
> +{
> +       return 0;
> +}
> +#else
> +#define omap_set_wake NULL
> +#endif
> +
>  static struct irq_chip omap_irq_chip = {
>        .name   = "INTC",
>        .ack    = omap_mask_ack_irq,
>        .mask   = omap_mask_irq,
>        .unmask = omap_unmask_irq,
> +       .set_wake = omap_set_wake,
>  };
>
>  static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)
> --
> 1.6.0.4
>



-- 
---
Regards,
Govindraj.R
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Unbalanced IRQ wake disable during resume from static suspend

[Santosh Shilimkar]

[-- Attachment #1: Type: text/plain, Size: 5045 bytes --]

Thanks Govind for testing it.

> -----Original Message-----
> From: Govindraj [mailto:govindraj.ti@gmail.com]
> Sent: Thursday, December 02, 2010 4:05 PM
> To: Santosh Shilimkar
> Cc: Paul Walmsley; linux-omap@vger.kernel.org;
khilman@deeprootsystems.com
> Subject: Re: Unbalanced IRQ wake disable during resume from static
suspend
>
> On Thu, Dec 2, 2010 at 2:38 PM, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
> > Paul,
> >> -----Original Message-----
> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> owner@vger.kernel.org] On Behalf Of Paul Walmsley
> >> Sent: Thursday, December 02, 2010 1:44 PM
> >> To: linux-omap@vger.kernel.org; Govindraj
> >> Cc: khilman@deeprootsystems.com
> >> Subject: Re: Unbalanced IRQ wake disable during resume from static
> > suspend
> >>
> >> On Thu, 2 Dec 2010, Paul Walmsley wrote:
> >>
> >> > On current linux-omap master (commit
> >> > 7fd1cffc56ca9944fec583eb2ecda5ce88b36bff "Linux-omap rebuilt:
Updated
> > to
> >> > -rc4"), when resuming from static suspend, several "Unbalanced IRQ
> >> > wake disable" warnings are generated, one for each of the serial
> > ports.
> >> > This is on Beagle 35xx rev C2.
> >>
> >> Just to follow up on this, these messages are not logged when
> >> /sys/devices/platform/omap/omap-hsuart.*/tty/ttyO*/power/wakeup are
set
> > to
> >> 'disabled'.
> >>
> >> Also this is with omap2plus_defconfig.
> >>
> > Just a wild guess here but is this because the 'set_wake' is
> > not setup and then fw might be returning some error whenever
> > driver invoke this API as part of enable_irq_wake() callback
> >
> > If that being the case, below patch might might help. Can
> > somebody try this out ?
> >
>
> Tested on 3630SDP.
> The below patch helps to avoid the warnings.
>

Here is the refreshed version with change log. Also
attached the patch in case google mails raps it up.

>From 0170159ae34957efb839d9143123f7ced795f62e Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Thu, 2 Dec 2010 14:36:17 +0530
Subject: [PATCH] omap: irq: Dummy handler for enable_irq_wake

With PM enabled, enable_irq_wake() has a callback for
interrupt controllers. But since omap INTC handler did not had this
call back associated, it was always returning -ENXIO, which was
breaking the existing drivers. This patch adds dummy handlers in
in order to avoid breaking of existing drivers.

Without this patch you get below warnings with wakeup enabled on devices

------------[ cut here ]------------
WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
Unbalanced IRQ 72 wake disable
Modules linked in:
[<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>]
(warn_slowpath_common+0x4c/0x64)
[<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>]
(warn_slowpath_fmt+0x2c/0x3c

[<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>]
(set_irq_wake+0x80/0xe4)
[<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>]
(uart_resume_port+0x84/0x248)
[<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>]
(serial_omap_resume+0x20/0x2c)
[<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>]
(platform_pm_resume+0x48/0x54)
[<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>]
(pm_op+0x6c/0xac)
[<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>]
(device_resume+0x58/0x10c)
[<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>]
(dpm_resume_end+0xf4/0x360)
[<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>]
(suspend_devices_and_enter+0x1ac/
0x200)
[<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>]
(enter_state+0xe0/0x1
38)
[<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>]
(state_store+0x90/0xb8)
[<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>]
(kobj_attr_store+0x18/0x1c)
[<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>]
(sysfs_write_file+0x10c/0x144)
[<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>]
(vfs_write+0xac/0x134)
[<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>]
(sys_write+0x3c/0x68)
[<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>]
(ret_fast_syscall+0x0/0x3c)
---[ end trace 19fe50b7b47ba94f ]---

Reported-by: Paul Walmsley <paul@pwsan.com>
Tested-by: Govindraj.R <govindraj.raja@ti.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/irq.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 32eeabe..6a964d3 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -143,11 +143,21 @@ static void omap_mask_ack_irq(unsigned int irq)
 	omap_ack_irq(irq);
 }

+#ifdef CONFIG_PM
+static int omap_set_wake(unsigned int irq, unsigned int on)
+{
+	return 0;
+}
+#else
+#define omap_set_wake NULL
+#endif
+
 static struct irq_chip omap_irq_chip = {
 	.name	= "INTC",
 	.ack	= omap_mask_ack_irq,
 	.mask	= omap_mask_irq,
 	.unmask	= omap_unmask_irq,
+	.set_wake = omap_set_wake,
 };

 static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)
-- 
1.6.0.4

[-- Attachment #2: 0001-omap-irq-Dummy-handler-for-enable_irq_wake.patch --]
[-- Type: application/octet-stream, Size: 3172 bytes --]

From 0170159ae34957efb839d9143123f7ced795f62e Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Thu, 2 Dec 2010 14:36:17 +0530
Subject: [PATCH] omap: irq: Dummy handler for enable_irq_wake

With PM enabled, enable_irq_wake() has a callback for
interrupt controllers. But since omap INTC handler did not had this
call back associated, it was always returning -ENXIO, which was
breaking the existing drivers. This patch adds dummy handlers in
in order to avoid breaking of existing drivers.

Without this patch you get below warnings with wakeup enabled on devices

------------[ cut here ]------------
WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
Unbalanced IRQ 72 wake disable
Modules linked in:
[<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>] (warn_slowpath_common+0x4c/0x64)
[<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c

[<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>] (set_irq_wake+0x80/0xe4)
[<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>] (uart_resume_port+0x84/0x248)
[<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>] (serial_omap_resume+0x20/0x2c)
[<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>] (platform_pm_resume+0x48/0x54)
[<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>] (pm_op+0x6c/0xac)
[<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>] (device_resume+0x58/0x10c)
[<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>] (dpm_resume_end+0xf4/0x360)
[<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>] (suspend_devices_and_enter+0x1ac/
0x200)
[<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>] (enter_state+0xe0/0x1
38)
[<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>] (state_store+0x90/0xb8)
[<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>] (kobj_attr_store+0x18/0x1c)
[<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>] (sysfs_write_file+0x10c/0x144)
[<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>] (vfs_write+0xac/0x134)
[<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>] (sys_write+0x3c/0x68)
[<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>] (ret_fast_syscall+0x0/0x3c)
---[ end trace 19fe50b7b47ba94f ]---

Reported-by: Paul Walmsley <paul@pwsan.com>
Tested-by: Govindraj.R <govindraj.raja@ti.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/irq.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 32eeabe..6a964d3 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -143,11 +143,21 @@ static void omap_mask_ack_irq(unsigned int irq)
 	omap_ack_irq(irq);
 }
 
+#ifdef CONFIG_PM
+static int omap_set_wake(unsigned int irq, unsigned int on)
+{
+	return 0;
+}
+#else
+#define omap_set_wake NULL
+#endif
+
 static struct irq_chip omap_irq_chip = {
 	.name	= "INTC",
 	.ack	= omap_mask_ack_irq,
 	.mask	= omap_mask_irq,
 	.unmask	= omap_unmask_irq,
+	.set_wake = omap_set_wake,
 };
 
 static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)
-- 
1.6.0.4

Re: Unbalanced IRQ wake disable during resume from static suspend

[Kevin Hilman]

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

[...]

> Here is the refreshed version with change log. Also
> attached the patch in case google mails raps it up.
>
> From 0170159ae34957efb839d9143123f7ced795f62e Mon Sep 17 00:00:00 2001
> From: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Date: Thu, 2 Dec 2010 14:36:17 +0530
> Subject: [PATCH] omap: irq: Dummy handler for enable_irq_wake
>
> With PM enabled, enable_irq_wake() has a callback for
> interrupt controllers. But since omap INTC handler did not had this
> call back associated, it was always returning -ENXIO, which was
> breaking the existing drivers. This patch adds dummy handlers in
> in order to avoid breaking of existing drivers.

Can you describe a little more exacly how drivers were being broken
(e.g. describe why this caused unbalanced IRQs, etc.)

Other than that this looks like the right fix.

I guess this hasn't been seen before since we haven't tested the sysfs
wakeup interface for the omap-serial driver.  For on-chip OMAP UARTs,
using the sysfs interface isn't needed as the serial core is already
doing device_init_wakeup(dev, true);

Kevin

> Without this patch you get below warnings with wakeup enabled on devices
>
> ------------[ cut here ]------------
> WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
> Unbalanced IRQ 72 wake disable
> Modules linked in:
> [<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>]
> (warn_slowpath_common+0x4c/0x64)
> [<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>]
> (warn_slowpath_fmt+0x2c/0x3c
>
> [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>]
> (set_irq_wake+0x80/0xe4)
> [<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>]
> (uart_resume_port+0x84/0x248)
> [<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>]
> (serial_omap_resume+0x20/0x2c)
> [<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>]
> (platform_pm_resume+0x48/0x54)
> [<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>]
> (pm_op+0x6c/0xac)
> [<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>]
> (device_resume+0x58/0x10c)
> [<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>]
> (dpm_resume_end+0xf4/0x360)
> [<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>]
> (suspend_devices_and_enter+0x1ac/
> 0x200)
> [<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>]
> (enter_state+0xe0/0x1
> 38)
> [<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>]
> (state_store+0x90/0xb8)
> [<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>]
> (kobj_attr_store+0x18/0x1c)
> [<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>]
> (sysfs_write_file+0x10c/0x144)
> [<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>]
> (vfs_write+0xac/0x134)
> [<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>]
> (sys_write+0x3c/0x68)
> [<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>]
> (ret_fast_syscall+0x0/0x3c)
> ---[ end trace 19fe50b7b47ba94f ]---
>
> Reported-by: Paul Walmsley <paul@pwsan.com>
> Tested-by: Govindraj.R <govindraj.raja@ti.com>
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/mach-omap2/irq.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 32eeabe..6a964d3 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -143,11 +143,21 @@ static void omap_mask_ack_irq(unsigned int irq)
>  	omap_ack_irq(irq);
>  }
>
> +#ifdef CONFIG_PM
> +static int omap_set_wake(unsigned int irq, unsigned int on)
> +{
> +	return 0;
> +}
> +#else
> +#define omap_set_wake NULL
> +#endif
> +
>  static struct irq_chip omap_irq_chip = {
>  	.name	= "INTC",
>  	.ack	= omap_mask_ack_irq,
>  	.mask	= omap_mask_irq,
>  	.unmask	= omap_unmask_irq,
> +	.set_wake = omap_set_wake,
>  };
>
>  static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)

RE: Unbalanced IRQ wake disable during resume from static suspend

[Santosh Shilimkar]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Thursday, December 02, 2010 7:46 PM
> To: Santosh Shilimkar
> Cc: Govindraj; Paul Walmsley; linux-omap@vger.kernel.org
> Subject: Re: Unbalanced IRQ wake disable during resume from static
suspend
>
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
> [...]
>
> > Here is the refreshed version with change log. Also
> > attached the patch in case google mails raps it up.
> >
> > From 0170159ae34957efb839d9143123f7ced795f62e Mon Sep 17 00:00:00 2001
> > From: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Date: Thu, 2 Dec 2010 14:36:17 +0530
> > Subject: [PATCH] omap: irq: Dummy handler for enable_irq_wake
> >
> > With PM enabled, enable_irq_wake() has a callback for
> > interrupt controllers. But since omap INTC handler did not had this
> > call back associated, it was always returning -ENXIO, which was
> > breaking the existing drivers. This patch adds dummy handlers in
> > in order to avoid breaking of existing drivers.
>
> Can you describe a little more exacly how drivers were being broken
> (e.g. describe why this caused unbalanced IRQs, etc.)
>
Whenever " set_irq_wake_real" fails, the desc->wake_depth =0
And that generates a WARN_ON

if (desc->wake_depth == 0) {
			WARN(1, "Unbalanced IRQ %d wake disable\n", irq);


> Other than that this looks like the right fix.
>
> I guess this hasn't been seen before since we haven't tested the sysfs
> wakeup interface for the omap-serial driver.  For on-chip OMAP UARTs,
> using the sysfs interface isn't needed as the serial core is already
> doing device_init_wakeup(dev, true);
>
> Kevin
>
> > Without this patch you get below warnings with wakeup enabled on
devices
> >
> > ------------[ cut here ]------------
> > WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
> > Unbalanced IRQ 72 wake disable
> > Modules linked in:
> > [<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>]
> > (warn_slowpath_common+0x4c/0x64)
> > [<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>]
> > (warn_slowpath_fmt+0x2c/0x3c
> >
> > [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>]
> > (set_irq_wake+0x80/0xe4)
> > [<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>]
> > (uart_resume_port+0x84/0x248)
> > [<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>]
> > (serial_omap_resume+0x20/0x2c)
> > [<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>]
> > (platform_pm_resume+0x48/0x54)
> > [<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>]
> > (pm_op+0x6c/0xac)
> > [<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>]
> > (device_resume+0x58/0x10c)
> > [<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>]
> > (dpm_resume_end+0xf4/0x360)
> > [<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>]
> > (suspend_devices_and_enter+0x1ac/
> > 0x200)
> > [<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>]
> > (enter_state+0xe0/0x1
> > 38)
> > [<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>]
> > (state_store+0x90/0xb8)
> > [<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>]
> > (kobj_attr_store+0x18/0x1c)
> > [<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>]
> > (sysfs_write_file+0x10c/0x144)
> > [<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>]
> > (vfs_write+0xac/0x134)
> > [<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>]
> > (sys_write+0x3c/0x68)
> > [<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>]
> > (ret_fast_syscall+0x0/0x3c)
> > ---[ end trace 19fe50b7b47ba94f ]---
> >
> > Reported-by: Paul Walmsley <paul@pwsan.com>
> > Tested-by: Govindraj.R <govindraj.raja@ti.com>
> >
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > ---
> >  arch/arm/mach-omap2/irq.c |   10 ++++++++++
> >  1 files changed, 10 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> > index 32eeabe..6a964d3 100644
> > --- a/arch/arm/mach-omap2/irq.c
> > +++ b/arch/arm/mach-omap2/irq.c
> > @@ -143,11 +143,21 @@ static void omap_mask_ack_irq(unsigned int irq)
> >  	omap_ack_irq(irq);
> >  }
> >
> > +#ifdef CONFIG_PM
> > +static int omap_set_wake(unsigned int irq, unsigned int on)
> > +{
> > +	return 0;
> > +}
> > +#else
> > +#define omap_set_wake NULL
> > +#endif
> > +
> >  static struct irq_chip omap_irq_chip = {
> >  	.name	= "INTC",
> >  	.ack	= omap_mask_ack_irq,
> >  	.mask	= omap_mask_irq,
> >  	.unmask	= omap_unmask_irq,
> > +	.set_wake = omap_set_wake,
> >  };
> >
> >  static void __init omap_irq_bank_init_one(struct omap_irq_bank *bank)

Re: Unbalanced IRQ wake disable during resume from static suspend

[Paul Walmsley]

Hi Kevin

On Thu, 2 Dec 2010, Kevin Hilman wrote:

> I guess this hasn't been seen before since we haven't tested the sysfs
> wakeup interface for the omap-serial driver.  For on-chip OMAP UARTs,
> using the sysfs interface isn't needed as the serial core is already
> doing device_init_wakeup(dev, true);

Is this the code you're referring to, in serial_core.c?

	tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
	if (likely(!IS_ERR(tty_dev))) {
		device_init_wakeup(tty_dev, 1);
		device_set_wakeup_enable(tty_dev, 0);
	} else

I may be misreading it, but it appears that the code leaves wakeups 
disabled for the serial port, by default.

As an aside, this code is somewhat perplexing: it doesn't seem accurate to 
assume that every serial device really is capable of waking up the system.


- Paul

RE: Unbalanced IRQ wake disable during resume from static suspend

[Paul Walmsley]

Hello Santosh

On Thu, 2 Dec 2010, Santosh Shilimkar wrote:

> Just a wild guess here but is this because the 'set_wake' is
> not setup and then fw might be returning some error whenever
> driver invoke this API as part of enable_irq_wake() callback
> 
> If that being the case, below patch might might help. Can
> somebody try this out ?

This patch might remove the warnings, but I doubt that it solves the root 
cause.  In any case, it doesn't seem correct to unconditionally return 0 
(success) from an omap_irq_wake() function, given that the OMAP INTC has 
no functionality in this regard.

The real problem appears to be in drivers/serial/serial_core.c.  
uart_suspend_port() doesn't check the return value of enable_irq_wake().  
Seems to me that it needs to save that return value somewhere and not 
bother calling disable_irq_wake() in uart_resume_port() if 
enable_irq_wake() returned an error.

That's the patch that I'd suggest that you guys put together and send to 
the Linux serial people.


- Paul

RE: Unbalanced IRQ wake disable during resume from static suspend

[Santosh Shilimkar]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Friday, December 03, 2010 3:53 PM
> To: Santosh Shilimkar
> Cc: linux-omap@vger.kernel.org; Govindraj; khilman@deeprootsystems.com
> Subject: RE: Unbalanced IRQ wake disable during resume from static
suspend
>
> Hello Santosh
>
> On Thu, 2 Dec 2010, Santosh Shilimkar wrote:
>
> > Just a wild guess here but is this because the 'set_wake' is
> > not setup and then fw might be returning some error whenever
> > driver invoke this API as part of enable_irq_wake() callback
> >
> > If that being the case, below patch might might help. Can
> > somebody try this out ?
>
> This patch might remove the warnings, but I doubt that it solves the
root
> cause.  In any case, it doesn't seem correct to unconditionally return 0
> (success) from an omap_irq_wake() function, given that the OMAP INTC has
> no functionality in this regard.
>
> The real problem appears to be in drivers/serial/serial_core.c.
> uart_suspend_port() doesn't check the return value of enable_irq_wake().
> Seems to me that it needs to save that return value somewhere and not
> bother calling disable_irq_wake() in uart_resume_port() if
> enable_irq_wake() returned an error.
>
> That's the patch that I'd suggest that you guys put together and send to
> the Linux serial people.
>
You are right Paul. This will actually fix the broken driver rather than
masking it. Will spin a patch for the same

Re: Unbalanced IRQ wake disable during resume from static suspend

[Govindraj]

On Fri, Dec 3, 2010 at 4:38 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
>> -----Original Message-----
>> From: Paul Walmsley [mailto:paul@pwsan.com]
>> Sent: Friday, December 03, 2010 3:53 PM
>> To: Santosh Shilimkar
>> Cc: linux-omap@vger.kernel.org; Govindraj; khilman@deeprootsystems.com
>> Subject: RE: Unbalanced IRQ wake disable during resume from static
> suspend
>>
>> Hello Santosh
>>
>> On Thu, 2 Dec 2010, Santosh Shilimkar wrote:
>>
>> > Just a wild guess here but is this because the 'set_wake' is
>> > not setup and then fw might be returning some error whenever
>> > driver invoke this API as part of enable_irq_wake() callback
>> >
>> > If that being the case, below patch might might help. Can
>> > somebody try this out ?
>>
>> This patch might remove the warnings, but I doubt that it solves the
> root
>> cause.  In any case, it doesn't seem correct to unconditionally return 0
>> (success) from an omap_irq_wake() function, given that the OMAP INTC has
>> no functionality in this regard.
>>
>> The real problem appears to be in drivers/serial/serial_core.c.
>> uart_suspend_port() doesn't check the return value of enable_irq_wake().
>> Seems to me that it needs to save that return value somewhere and not
>> bother calling disable_irq_wake() in uart_resume_port() if
>> enable_irq_wake() returned an error.
>>
>> That's the patch that I'd suggest that you guys put together and send to
>> the Linux serial people.
>>
> You are right Paul. This will actually fix the broken driver rather than
> masking it. Will spin a patch for the same
>

Hi Santosh/Paul,

How about this patch?
[Tested on 3630SDP

---
Regards,
Govindraj.R


From cb4e79a645b530f83f55d801ab054cc438ada0dd Mon Sep 17 00:00:00 2001
From: Govindraj.R <govindraj.raja@ti.com>
Date: Fri, 3 Dec 2010 16:42:14 +0530
Subject: [PATCH] Serial: Unbalanced IRQ wake disable during resume
from static suspend

Check for return status for enable_irq_wake if irq_wake
interface is not available then during resume unconditional
disabling of irq_wake can throw below warning.

------------[ cut here ]------------
WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
Unbalanced IRQ 72 wake disable
Modules linked in:
[<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>]
(warn_slowpath_common+0x4c/0x64)
[<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>]
(warn_slowpath_fmt+0x2c/0x3c
[<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>]
(set_irq_wake+0x80/0xe4)
[<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>]
(uart_resume_port+0x84/0x248)
[<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>]
(serial_omap_resume+0x20/0x2c)
[<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>]
(platform_pm_resume+0x48/0x54)
[<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>]
(pm_op+0x6c/0xac)
[<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>]
(device_resume+0x58/0x10c)
[<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>]
(dpm_resume_end+0xf4/0x360)
[<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>]
(suspend_devices_and_enter+0x1ac/0x200)
[<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>]
(enter_state+0xe0/0x138)
[<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>]
(state_store+0x90/0xb8)
[<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>]
(kobj_attr_store+0x18/0x1c)
[<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>]
(sysfs_write_file+0x10c/0x144)
[<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>]
(vfs_write+0xac/0x134)
[<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>]
(sys_write+0x3c/0x68)
[<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>]
(ret_fast_syscall+0x0/0x3c)
---[ end trace 19fe50b7b47ba94f ]---

Thus add a flag to check the return status for irq_wake
based on flag disable the irq_wake

Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 drivers/serial/serial_core.c |    6 ++++--
 include/linux/serial_core.h  |    1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index cd85112..0466815 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1990,7 +1990,8 @@ int uart_suspend_port(struct uart_driver *drv,
struct uart_port *uport)

 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (device_may_wakeup(tty_dev)) {
-		enable_irq_wake(uport->irq);
+		if (!enable_irq_wake(uport->irq))
+			uport->irq_wake = 1;
 		put_device(tty_dev);
 		mutex_unlock(&port->mutex);
 		return 0;
@@ -2056,7 +2057,8 @@ int uart_resume_port(struct uart_driver *drv,
struct uart_port *uport)

 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (!uport->suspended && device_may_wakeup(tty_dev)) {
-		disable_irq_wake(uport->irq);
+		if (uport->irq_wake)
+			disable_irq_wake(uport->irq);
 		mutex_unlock(&port->mutex);
 		return 0;
 	}
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 295e898..88e73c3 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -359,6 +359,7 @@ struct uart_port {
 	struct device		*dev;			/* parent device */
 	unsigned char		hub6;			/* this should be in the 8250 driver */
 	unsigned char		suspended;
+	unsigned char		irq_wake;		/* Irq_wakeup Available */
 	unsigned char		unused[2];
 	void			*private_data;		/* generic platform data pointer */
 };
-- 
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Unbalanced IRQ wake disable during resume from static suspend

[Santosh Shilimkar]

> -----Original Message-----
> From: Govindraj [mailto:govindraj.ti@gmail.com]
> Sent: Friday, December 03, 2010 4:51 PM
> To: Santosh Shilimkar
> Cc: Paul Walmsley; linux-omap@vger.kernel.org;
khilman@deeprootsystems.com
> Subject: Re: Unbalanced IRQ wake disable during resume from static
suspend
>
> On Fri, Dec 3, 2010 at 4:38 PM, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
> >> -----Original Message-----
> >> From: Paul Walmsley [mailto:paul@pwsan.com]
> >> Sent: Friday, December 03, 2010 3:53 PM
> >> To: Santosh Shilimkar
> >> Cc: linux-omap@vger.kernel.org; Govindraj;
khilman@deeprootsystems.com
> >> Subject: RE: Unbalanced IRQ wake disable during resume from static
> > suspend
> >>
> >> Hello Santosh
> >>
> >> On Thu, 2 Dec 2010, Santosh Shilimkar wrote:
> >>
> >> > Just a wild guess here but is this because the 'set_wake' is
> >> > not setup and then fw might be returning some error whenever
> >> > driver invoke this API as part of enable_irq_wake() callback
> >> >
> >> > If that being the case, below patch might might help. Can
> >> > somebody try this out ?
> >>
> >> This patch might remove the warnings, but I doubt that it solves the
> > root
> >> cause.  In any case, it doesn't seem correct to unconditionally
return
> 0
> >> (success) from an omap_irq_wake() function, given that the OMAP INTC
> has
> >> no functionality in this regard.
> >>
> >> The real problem appears to be in drivers/serial/serial_core.c.
> >> uart_suspend_port() doesn't check the return value of
enable_irq_wake().
> >> Seems to me that it needs to save that return value somewhere and not
> >> bother calling disable_irq_wake() in uart_resume_port() if
> >> enable_irq_wake() returned an error.
> >>
> >> That's the patch that I'd suggest that you guys put together and send
> to
> >> the Linux serial people.
> >>
> > You are right Paul. This will actually fix the broken driver rather
than
> > masking it. Will spin a patch for the same
> >
>
> Hi Santosh/Paul,
>
> How about this patch?
Looks inline with what Paul suggested.

> [Tested on 3630SDP
>
> ---
> Regards,
> Govindraj.R
>
>
> From cb4e79a645b530f83f55d801ab054cc438ada0dd Mon Sep 17 00:00:00 2001
> From: Govindraj.R <govindraj.raja@ti.com>
> Date: Fri, 3 Dec 2010 16:42:14 +0530
> Subject: [PATCH] Serial: Unbalanced IRQ wake disable during resume
> from static suspend
>
> Check for return status for enable_irq_wake if irq_wake
> interface is not available then during resume unconditional
> disabling of irq_wake can throw below warning.
>
> ------------[ cut here ]------------
> WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
> Unbalanced IRQ 72 wake disable
> Modules linked in:
> [<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>]
> (warn_slowpath_common+0x4c/0x64)
> [<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>]
> (warn_slowpath_fmt+0x2c/0x3c
> [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>]
> (set_irq_wake+0x80/0xe4)
> [<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>]
> (uart_resume_port+0x84/0x248)
> [<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>]
> (serial_omap_resume+0x20/0x2c)
> [<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>]
> (platform_pm_resume+0x48/0x54)
> [<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>]
> (pm_op+0x6c/0xac)
> [<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>]
> (device_resume+0x58/0x10c)
> [<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>]
> (dpm_resume_end+0xf4/0x360)
> [<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>]
> (suspend_devices_and_enter+0x1ac/0x200)
> [<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>]
> (enter_state+0xe0/0x138)
> [<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>]
> (state_store+0x90/0xb8)
> [<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>]
> (kobj_attr_store+0x18/0x1c)
> [<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>]
> (sysfs_write_file+0x10c/0x144)
> [<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>]
> (vfs_write+0xac/0x134)
> [<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>]
> (sys_write+0x3c/0x68)
> [<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>]
> (ret_fast_syscall+0x0/0x3c)
> ---[ end trace 19fe50b7b47ba94f ]---
>
> Thus add a flag to check the return status for irq_wake
> based on flag disable the irq_wake
>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
>  drivers/serial/serial_core.c |    6 ++++--
>  include/linux/serial_core.h  |    1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> index cd85112..0466815 100644
> --- a/drivers/serial/serial_core.c
> +++ b/drivers/serial/serial_core.c
> @@ -1990,7 +1990,8 @@ int uart_suspend_port(struct uart_driver *drv,
> struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match,
serial_match_port);
>  	if (device_may_wakeup(tty_dev)) {
> -		enable_irq_wake(uport->irq);
> +		if (!enable_irq_wake(uport->irq))
> +			uport->irq_wake = 1;
>  		put_device(tty_dev);
>  		mutex_unlock(&port->mutex);
>  		return 0;
> @@ -2056,7 +2057,8 @@ int uart_resume_port(struct uart_driver *drv,
> struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match,
serial_match_port);
>  	if (!uport->suspended && device_may_wakeup(tty_dev)) {
> -		disable_irq_wake(uport->irq);
> +		if (uport->irq_wake)
> +			disable_irq_wake(uport->irq);
>  		mutex_unlock(&port->mutex);
>  		return 0;
>  	}
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 295e898..88e73c3 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -359,6 +359,7 @@ struct uart_port {
>  	struct device		*dev;			/* parent device
*/
>  	unsigned char		hub6;			/* this should be
in the
> 8250 driver */
>  	unsigned char		suspended;
> +	unsigned char		irq_wake;		/* Irq_wakeup
Available */
>  	unsigned char		unused[2];
>  	void			*private_data;		/* generic
platform data
> pointer */
>  };
> --
> 1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Unbalanced IRQ wake disable during resume from static suspend

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hi Kevin
>
> On Thu, 2 Dec 2010, Kevin Hilman wrote:
>
>> I guess this hasn't been seen before since we haven't tested the sysfs
>> wakeup interface for the omap-serial driver.  For on-chip OMAP UARTs,
>> using the sysfs interface isn't needed as the serial core is already
>> doing device_init_wakeup(dev, true);
>
> Is this the code you're referring to, in serial_core.c?
>
> 	tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
> 	if (likely(!IS_ERR(tty_dev))) {
> 		device_init_wakeup(tty_dev, 1);
> 		device_set_wakeup_enable(tty_dev, 0);
> 	} else
>
> I may be misreading it, but it appears that the code leaves wakeups 
> disabled for the serial port, by default.

No, I was referring to the code in mach-omap2/serial.c, in omap_serial_init_port():

	if ((cpu_is_omap34xx() && uart->padconf) ||
	    (uart->wk_en && uart->wk_mask)) {
		device_init_wakeup(&od->pdev.dev, true);
		DEV_CREATE_FILE(&od->pdev.dev, &dev_attr_sleep_timeout);
	}

Kevin

> As an aside, this code is somewhat perplexing: it doesn't seem accurate to 
> assume that every serial device really is capable of waking up the system.

Re: Unbalanced IRQ wake disable during resume from static suspend

[Kevin Hilman]

Govindraj <govindraj.ti@gmail.com> writes:

> On Fri, Dec 3, 2010 at 4:38 PM, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
>>> -----Original Message-----
>>> From: Paul Walmsley [mailto:paul@pwsan.com]
>>> Sent: Friday, December 03, 2010 3:53 PM
>>> To: Santosh Shilimkar
>>> Cc: linux-omap@vger.kernel.org; Govindraj; khilman@deeprootsystems.com
>>> Subject: RE: Unbalanced IRQ wake disable during resume from static
>> suspend
>>>
>>> Hello Santosh
>>>
>>> On Thu, 2 Dec 2010, Santosh Shilimkar wrote:
>>>
>>> > Just a wild guess here but is this because the 'set_wake' is
>>> > not setup and then fw might be returning some error whenever
>>> > driver invoke this API as part of enable_irq_wake() callback
>>> >
>>> > If that being the case, below patch might might help. Can
>>> > somebody try this out ?
>>>
>>> This patch might remove the warnings, but I doubt that it solves the
>> root
>>> cause.  In any case, it doesn't seem correct to unconditionally return 0
>>> (success) from an omap_irq_wake() function, given that the OMAP INTC has
>>> no functionality in this regard.
>>>
>>> The real problem appears to be in drivers/serial/serial_core.c.
>>> uart_suspend_port() doesn't check the return value of enable_irq_wake().
>>> Seems to me that it needs to save that return value somewhere and not
>>> bother calling disable_irq_wake() in uart_resume_port() if
>>> enable_irq_wake() returned an error.
>>>
>>> That's the patch that I'd suggest that you guys put together and send to
>>> the Linux serial people.
>>>
>> You are right Paul. This will actually fix the broken driver rather than
>> masking it. Will spin a patch for the same
>>
>
> Hi Santosh/Paul,
>
> How about this patch?
> [Tested on 3630SDP

also tested on 36xx/zoom3 and 34xx/n900

>
> From cb4e79a645b530f83f55d801ab054cc438ada0dd Mon Sep 17 00:00:00 2001
> From: Govindraj.R <govindraj.raja@ti.com>
> Date: Fri, 3 Dec 2010 16:42:14 +0530
> Subject: [PATCH] Serial: Unbalanced IRQ wake disable during resume
> from static suspend
>
> Check for return status for enable_irq_wake if irq_wake
> interface is not available then during resume unconditional
> disabling of irq_wake can throw below warning.

This doesn't read well.  How about:

Subject: serial: omap-serial: avoid unbalanced IRQ disable

Changelog:

To avoid unbalanced IRQ wake disable, ensure that wakeups are disabled
only when wakeups have been successfully enabled. 

With this, you don't need to include the backtrace as the unbalanced IRQ
disable is understood from the subject & changelog.


> ------------[ cut here ]------------
> WARNING: at kernel/irq/manage.c:382 set_irq_wake+0x80/0xe4()
> Unbalanced IRQ 72 wake disable
> Modules linked in:
> [<c0062a28>] (unwind_backtrace+0x0/0xec) from [<c0092260>]
> (warn_slowpath_common+0x4c/0x64)
> [<c0092260>] (warn_slowpath_common+0x4c/0x64) from [<c00922f8>]
> (warn_slowpath_fmt+0x2c/0x3c
> [<c00922f8>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00d3238>]
> (set_irq_wake+0x80/0xe4)
> [<c00d3238>] (set_irq_wake+0x80/0xe4) from [<c029dd60>]
> (uart_resume_port+0x84/0x248)
> [<c029dd60>] (uart_resume_port+0x84/0x248) from [<c02a2338>]
> (serial_omap_resume+0x20/0x2c)
> [<c02a2338>] (serial_omap_resume+0x20/0x2c) from [<c02a92d4>]
> (platform_pm_resume+0x48/0x54)
> [<c02a92d4>] (platform_pm_resume+0x48/0x54) from [<c02abd1c>]
> (pm_op+0x6c/0xac)
> [<c02abd1c>] (pm_op+0x6c/0xac) from [<c02ac0fc>]
> (device_resume+0x58/0x10c)
> [<c02ac0fc>] (device_resume+0x58/0x10c) from [<c02ac2ec>]
> (dpm_resume_end+0xf4/0x360)
> [<c02ac2ec>] (dpm_resume_end+0xf4/0x360) from [<c00cf58c>]
> (suspend_devices_and_enter+0x1ac/0x200)
> [<c00cf58c>] (suspend_devices_and_enter+0x1ac/0x200) from [<c00cf6c0>]
> (enter_state+0xe0/0x138)
> [<c00cf6c0>] (enter_state+0xe0/0x138) from [<c00ced18>]
> (state_store+0x90/0xb8)
> [<c00ced18>] (state_store+0x90/0xb8) from [<c0243b98>]
> (kobj_attr_store+0x18/0x1c)
> [<c0243b98>] (kobj_attr_store+0x18/0x1c) from [<c0176128>]
> (sysfs_write_file+0x10c/0x144)
> [<c0176128>] (sysfs_write_file+0x10c/0x144) from [<c0125528>]
> (vfs_write+0xac/0x134)
> [<c0125528>] (vfs_write+0xac/0x134) from [<c012565c>]
> (sys_write+0x3c/0x68)
> [<c012565c>] (sys_write+0x3c/0x68) from [<c005bb00>]
> (ret_fast_syscall+0x0/0x3c)
> ---[ end trace 19fe50b7b47ba94f ]---
>
> Thus add a flag to check the return status for irq_wake
> based on flag disable the irq_wake
>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
>  drivers/serial/serial_core.c |    6 ++++--
>  include/linux/serial_core.h  |    1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> index cd85112..0466815 100644
> --- a/drivers/serial/serial_core.c
> +++ b/drivers/serial/serial_core.c
> @@ -1990,7 +1990,8 @@ int uart_suspend_port(struct uart_driver *drv,
> struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
>  	if (device_may_wakeup(tty_dev)) {
> -		enable_irq_wake(uport->irq);
> +		if (!enable_irq_wake(uport->irq))
> +			uport->irq_wake = 1;
>  		put_device(tty_dev);
>  		mutex_unlock(&port->mutex);
>  		return 0;
> @@ -2056,7 +2057,8 @@ int uart_resume_port(struct uart_driver *drv,
> struct uart_port *uport)
>
>  	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
>  	if (!uport->suspended && device_may_wakeup(tty_dev)) {
> -		disable_irq_wake(uport->irq);
> +		if (uport->irq_wake)
> +			disable_irq_wake(uport->irq);

to be thorough, you should probably set the flag to zero here too

>  		mutex_unlock(&port->mutex);
>  		return 0;
>  	}
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 295e898..88e73c3 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -359,6 +359,7 @@ struct uart_port {
>  	struct device		*dev;			/* parent device */
>  	unsigned char		hub6;			/* this should be in the 8250 driver */
>  	unsigned char		suspended;
> +	unsigned char		irq_wake;		/* Irq_wakeup Available */

comment not necessary

>  	unsigned char		unused[2];
>  	void			*private_data;		/* generic platform data pointer */
>  };

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html