* Suspend broken on 3.3? @ 2012-03-22 11:09 Joe Woodward 2012-03-22 17:33 ` Kevin Hilman 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-22 11:09 UTC (permalink / raw) To: linux-omap Is system suspend broken on stock 3.3? I have a working stock 3.2 (with patches to fix runtime_pm for DSS2), and system suspend works just fine! This is running on a variety of GUMSTIX boards (both OMAP3530 and AM3703-based). I've just updated to stock 3.3 and suspend returns immediately when entered: # echo mem > /sys/power/state [ 72.391693] PM: Syncing filesystems ... done. [ 72.397003] Freezing user space processes ... (elapsed 0.02 seconds) done. [ 72.425201] Freezing remaining freezable tasks ... (elapsed 0.02 seconds) done. [ 72.456451] Suspending console(s) (use no_console_suspend to debug) [ 72.581695] PM: suspend of devices complete after 115.977 msecs [ 72.584594] PM: late suspend of devices complete after 2.868 msecs [ 72.612060] Successfully put all powerdomains to target state [ 72.613983] PM: early resume of devices complete after 1.770 msecs [ 72.941894] PM: resume of devices complete after 326.630 msecs [ 72.975677] Restarting tasks ... done. I've looked in the debugfs and see: # cat suspend_stats success: 1 fail: 0 failed_freeze: 0 failed_prepare: 0 failed_suspend: 0 failed_suspend_noirq: 0 failed_resume: 0 failed_resume_noirq: 0 failures: last_failed_dev: last_failed_errno: 0 0 last_failed_step: # cat wakeup_sources name active_count event_count hit_count active_since total_time max_time last_change gpio-keys 0 0 0 0 0 0 0 (null) 0 0 0 0 0 0 0 (null) 0 0 0 0 0 0 0 1-0048 0 0 0 0 0 0 0 omap_uart.3 0 0 0 0 0 0 0 omap_uart.2 0 0 0 0 0 0 0 So have no idea what is causing the wakeup. How should I go about debugging this further? Has anyone else tried suspend on 3.3? I've tried this both with my custom defconfig and omap2plus_defconfig. I do have some modifications to the GUMSTIX overo board file (minor changes like defining some extra buttons, and setting the correct LCD parameters), but I'm fairly certain these shouldn't be causing any problems... Cheers, Joe ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-22 11:09 Suspend broken on 3.3? Joe Woodward @ 2012-03-22 17:33 ` Kevin Hilman 2012-03-26 13:41 ` Joe Woodward 0 siblings, 1 reply; 40+ messages in thread From: Kevin Hilman @ 2012-03-22 17:33 UTC (permalink / raw) To: Joe Woodward; +Cc: linux-omap "Joe Woodward" <jw@terrafix.co.uk> writes: > Is system suspend broken on stock 3.3? I hope not. :) It *should* work, I'm using it regularily here, and "it works for me" (I'm sure that's just what you want to hear.) :) > I have a working stock 3.2 (with patches to fix runtime_pm for DSS2), and system suspend works just fine! > > This is running on a variety of GUMSTIX boards (both OMAP3530 and AM3703-based). I currently only have a 3530-based Gumstix Overo (although a AM3xxx-based one is on the way, thanks Gumstix!), but it's working fine for me on my Overo. Stock v3.3 won't boot on Overo because of the smsc911x regulator issues recently discusssed, so if you're using Overo, you also need the patch in Tony's fix-smsc911x-regulator branch. After that, suspend/resume is working fine for me using omap2plus_defconfig. I tried both with initramfs and with MMC rootfs. Can you try without your board file changes, using vanilla v3.3 + smsc911x fix above and using omap2plus_defconfig? Also, please share the kernel command-line you're using. Thanks, Kevin ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-22 17:33 ` Kevin Hilman @ 2012-03-26 13:41 ` Joe Woodward 2012-03-27 0:34 ` Kevin Hilman 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-26 13:41 UTC (permalink / raw) To: Kevin Hilman; +Cc: linux-omap -----Original Message----- From: Kevin Hilman <khilman@ti.com> To: "Joe Woodward" <jw@terrafix.co.uk> Cc: "linux-omap\@vger.kernel.org" <linux-omap@vger.kernel.org> Date: Thu, 22 Mar 2012 10:33:56 -0700 Subject: Re: Suspend broken on 3.3? > "Joe Woodward" <jw@terrafix.co.uk> writes: > > > Is system suspend broken on stock 3.3? > > I hope not. :) > > It *should* work, I'm using it regularily here, and "it works for me" > (I'm sure that's just what you want to hear.) :) > > > I have a working stock 3.2 (with patches to fix runtime_pm for DSS2), > and system suspend works just fine! > > > > This is running on a variety of GUMSTIX boards (both OMAP3530 and > AM3703-based). > > I currently only have a 3530-based Gumstix Overo (although a > AM3xxx-based one is on the way, thanks Gumstix!), but it's working fine > for me on my Overo. > > Stock v3.3 won't boot on Overo because of the smsc911x regulator issues > recently discusssed, so if you're using Overo, you also need the patch > in Tony's fix-smsc911x-regulator branch. > > After that, suspend/resume is working fine for me using > omap2plus_defconfig. I tried both with initramfs and with MMC rootfs. > > Can you try without your board file changes, using vanilla v3.3 + > smsc911x fix above and using omap2plus_defconfig? > > Also, please share the kernel command-line you're using. Right, I've stepped back a bit and dug out a GUSMTIX Palo43 carrier board on which to test the Overo OMAP3530 COM and I've found: - Running a stock 3.3 (with absolutely no changes) does indeed suspend correctly. - Running the 3.3 kernel with my (minor) board modifications (basically defining some buttons) suspends correctly as well. Then I went back to my original board and the 3.3 still wakes up from suspend immediately. So I had a think, and the only real differences between my board the the GUMSTIX Palo43 board is that I am using multiple UARTs. Up to this point I've only wanted to wake on the console (ttyO2), and not any other UARTs so I've stopped them waking with: echo disabled > /sys/devices/platform/omap/omap_uart.0/power/wakeup echo disabled > /sys/devices/platform/omap/omap_uart.1/power/wakeup I wanted to check that this still worked, so tried disabling wakeup on the console (ttyO2): echo disabled > /sys/devices/platform/omap/omap_uart.2/power/wakeup And if I do "echo mem > /sys/power/state" I was expecting to stay in suspend when I typed on my keyboard... However, the kernel still woke from suspend, which leads me to believe that the UART wakeup hasn't been disabled? Could you test if this is also the case your end? Cheers, Joe > Thanks, > > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-26 13:41 ` Joe Woodward @ 2012-03-27 0:34 ` Kevin Hilman 2012-03-27 13:53 ` Raja, Govindraj 2012-03-27 14:39 ` Joe Woodward 0 siblings, 2 replies; 40+ messages in thread From: Kevin Hilman @ 2012-03-27 0:34 UTC (permalink / raw) To: Joe Woodward; +Cc: linux-omap, govindraj.raja, Felipe Balbi +Govindraj, "Joe Woodward" <jw@terrafix.co.uk> writes: > Right, I've stepped back a bit and dug out a GUSMTIX Palo43 carrier > board on which to test the Overo OMAP3530 COM and I've found: > - Running a stock 3.3 (with absolutely no changes) does indeed suspend correctly. > - Running the 3.3 kernel with my (minor) board modifications > (basically defining some buttons) suspends correctly as well. > > Then I went back to my original board and the 3.3 still wakes up from > suspend immediately. So I had a think, and the only real differences > between my board the the GUMSTIX Palo43 board is that I am using > multiple UARTs. > > Up to this point I've only wanted to wake on the console (ttyO2), and > not any other UARTs so I've stopped them waking with: > echo disabled > /sys/devices/platform/omap/omap_uart.0/power/wakeup > echo disabled > /sys/devices/platform/omap/omap_uart.1/power/wakeup > > I wanted to check that this still worked, so tried disabling wakeup on > the console (ttyO2): > echo disabled > /sys/devices/platform/omap/omap_uart.2/power/wakeup > > And if I do "echo mem > /sys/power/state" I was expecting to stay in > suspend when I typed on my keyboard... However, the kernel still woke > from suspend, which leads me to believe that the UART wakeup hasn't > been disabled? Just to confirm: did the above work for you before v3.3? > Could you test if this is also the case your end? Yes, I get the same behavior, which is indeed broken. Govindraj, can you look into this? A quick glance suggests that disabling wakeups via the sysfs file is only disabling runtime PM, but not actually disabling wakups at the module-level or at the IO ring. Kevin ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-27 0:34 ` Kevin Hilman @ 2012-03-27 13:53 ` Raja, Govindraj 2012-03-27 21:37 ` Kevin Hilman 2012-03-27 14:39 ` Joe Woodward 1 sibling, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-03-27 13:53 UTC (permalink / raw) To: Kevin Hilman; +Cc: Joe Woodward, linux-omap, Felipe Balbi Hi Kevin, On Tue, Mar 27, 2012 at 6:04 AM, Kevin Hilman <khilman@ti.com> wrote: > +Govindraj, > > "Joe Woodward" <jw@terrafix.co.uk> writes: > >> Right, I've stepped back a bit and dug out a GUSMTIX Palo43 carrier >> board on which to test the Overo OMAP3530 COM and I've found: >> - Running a stock 3.3 (with absolutely no changes) does indeed suspend correctly. >> - Running the 3.3 kernel with my (minor) board modifications >> (basically defining some buttons) suspends correctly as well. >> >> Then I went back to my original board and the 3.3 still wakes up from >> suspend immediately. So I had a think, and the only real differences >> between my board the the GUMSTIX Palo43 board is that I am using >> multiple UARTs. >> >> Up to this point I've only wanted to wake on the console (ttyO2), and >> not any other UARTs so I've stopped them waking with: >> echo disabled > /sys/devices/platform/omap/omap_uart.0/power/wakeup >> echo disabled > /sys/devices/platform/omap/omap_uart.1/power/wakeup >> >> I wanted to check that this still worked, so tried disabling wakeup on >> the console (ttyO2): >> echo disabled > /sys/devices/platform/omap/omap_uart.2/power/wakeup >> >> And if I do "echo mem > /sys/power/state" I was expecting to stay in >> suspend when I typed on my keyboard... However, the kernel still woke >> from suspend, which leads me to believe that the UART wakeup hasn't >> been disabled? > > Just to confirm: did the above work for you before v3.3? > >> Could you test if this is also the case your end? > > Yes, I get the same behavior, which is indeed broken. > > Govindraj, can you look into this? > > A quick glance suggests that disabling wakeups via the sysfs file is > only disabling runtime PM, but not actually disabling wakups at the > module-level or at the IO ring. > I have started looking into this, disabling and enabling of wake-ups from .runtime_suspend needs some changes as in here[1] with that I see pad wakeup getting disabled and it doesn't wake up after enabling off mode and suspending. If clocks left enabled form uart driver during system wide suspend -> _od_suspend_noirq -> .runtime_suspend from uart driver (with [1]) -> omap_hwmod_disable_wakeup -> omap_device_idle But module level wakeup from sysc reg also needs to be disabled, with which I see some strange behavior, even though _disable_wakeup updates sysc reg it seem to wakeup, but removing SYSC_HAS_ENAWAKEUP flag from .sysc flags from hmwod data I see module level wakeup failing after disabling wakeup. Still checking on this. -- Thanks, Govindraj.R [1]: From 7f92f73006a82fdd7328fe7906fbf094b503cd13 Mon Sep 17 00:00:00 2001 From: "Govindraj.R" <govindraj.raja@ti.com> Date: Tue, 27 Mar 2012 18:55:00 +0530 Subject: [PATCH] OMAP2+: UART: Correct the wakeup enable mechanism The module level wakeups are enabled by default during bootup init from hmwod framework and pad wakeup will be disabled. Correct the condition check in uart runtime suspend path to enable/disable wakeups. Signed-off-by: Govindraj.R <govindraj.raja@ti.com> --- arch/arm/plat-omap/include/plat/omap-serial.h | 3 ++- drivers/tty/serial/omap-serial.c | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h index 9ff4444..386a25b 100644 --- a/arch/arm/plat-omap/include/plat/omap-serial.h +++ b/arch/arm/plat-omap/include/plat/omap-serial.h @@ -130,7 +130,8 @@ struct uart_omap_port { unsigned long port_activity; u32 context_loss_cnt; u32 errata; - u8 wakeups_enabled; + u8 pad_wakeups_enabled; + u8 module_wakeups_enabled; struct pm_qos_request pm_qos_request; u32 latency; diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 0121486..0a35b7e 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -1458,6 +1458,12 @@ static int serial_omap_probe(struct platform_device *pdev) up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE; } + /* Module level wakeup from sysc(BIT[2]) + * will be enabled during boot + * from hwmod framework. + */ + up->module_wakeups_enabled = true; + up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; up->calc_latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; pm_qos_add_request(&up->pm_qos_request, @@ -1586,14 +1592,16 @@ static int serial_omap_runtime_suspend(struct device *dev) up->context_loss_cnt = pdata->get_context_loss_count(dev); if (device_may_wakeup(dev)) { - if (!up->wakeups_enabled) { + if (!up->pad_wakeups_enabled || !up->module_wakeups_enabled) { pdata->enable_wakeup(up->pdev, true); - up->wakeups_enabled = true; + up->module_wakeups_enabled = true; + up->pad_wakeups_enabled = true; } } else { - if (up->wakeups_enabled) { + if (up->pad_wakeups_enabled || up->module_wakeups_enabled) { pdata->enable_wakeup(up->pdev, false); - up->wakeups_enabled = false; + up->module_wakeups_enabled = false; + up->pad_wakeups_enabled = false; } } -- 1.7.5.4 -- 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 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-27 13:53 ` Raja, Govindraj @ 2012-03-27 21:37 ` Kevin Hilman 2012-03-28 10:59 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Kevin Hilman @ 2012-03-27 21:37 UTC (permalink / raw) To: Raja, Govindraj; +Cc: Joe Woodward, linux-omap, Felipe Balbi "Raja, Govindraj" <govindraj.raja@ti.com> writes: > Hi Kevin, > > On Tue, Mar 27, 2012 at 6:04 AM, Kevin Hilman <khilman@ti.com> wrote: >> +Govindraj, >> >> "Joe Woodward" <jw@terrafix.co.uk> writes: >> >>> Right, I've stepped back a bit and dug out a GUSMTIX Palo43 carrier >>> board on which to test the Overo OMAP3530 COM and I've found: >>> - Running a stock 3.3 (with absolutely no changes) does indeed suspend correctly. >>> - Running the 3.3 kernel with my (minor) board modifications >>> (basically defining some buttons) suspends correctly as well. >>> >>> Then I went back to my original board and the 3.3 still wakes up from >>> suspend immediately. So I had a think, and the only real differences >>> between my board the the GUMSTIX Palo43 board is that I am using >>> multiple UARTs. >>> >>> Up to this point I've only wanted to wake on the console (ttyO2), and >>> not any other UARTs so I've stopped them waking with: >>> echo disabled > /sys/devices/platform/omap/omap_uart.0/power/wakeup >>> echo disabled > /sys/devices/platform/omap/omap_uart.1/power/wakeup >>> >>> I wanted to check that this still worked, so tried disabling wakeup on >>> the console (ttyO2): >>> echo disabled > /sys/devices/platform/omap/omap_uart.2/power/wakeup >>> >>> And if I do "echo mem > /sys/power/state" I was expecting to stay in >>> suspend when I typed on my keyboard... However, the kernel still woke >>> from suspend, which leads me to believe that the UART wakeup hasn't >>> been disabled? >> >> Just to confirm: did the above work for you before v3.3? >> >>> Could you test if this is also the case your end? >> >> Yes, I get the same behavior, which is indeed broken. >> >> Govindraj, can you look into this? >> >> A quick glance suggests that disabling wakeups via the sysfs file is >> only disabling runtime PM, but not actually disabling wakups at the >> module-level or at the IO ring. >> > > I have started looking into this, disabling and enabling of wake-ups > from .runtime_suspend needs some changes as in here[1] with that I see pad > wakeup getting disabled and it doesn't wake up after enabling off mode > and suspending. Thanks for looking into this. > If clocks left enabled form uart driver during system wide suspend > -> _od_suspend_noirq > -> .runtime_suspend from uart driver (with [1]) > -> omap_hwmod_disable_wakeup > -> omap_device_idle > > But module level wakeup from sysc reg also needs to be disabled, > with which I see some strange behavior, even though _disable_wakeup > updates sysc reg it seem to wakeup, but removing SYSC_HAS_ENAWAKEUP > flag from .sysc flags from hmwod data I see module level wakeup failing after > disabling wakeup. Still checking on this. > > -- > Thanks, > Govindraj.R > > [1]: > > From 7f92f73006a82fdd7328fe7906fbf094b503cd13 Mon Sep 17 00:00:00 2001 > From: "Govindraj.R" <govindraj.raja@ti.com> > Date: Tue, 27 Mar 2012 18:55:00 +0530 > Subject: [PATCH] OMAP2+: UART: Correct the wakeup enable mechanism > > The module level wakeups are enabled by default during bootup > init from hmwod framework and pad wakeup will be disabled. > > Correct the condition check in uart runtime suspend path to > enable/disable wakeups. > > Signed-off-by: Govindraj.R <govindraj.raja@ti.com> > --- > arch/arm/plat-omap/include/plat/omap-serial.h | 3 ++- > drivers/tty/serial/omap-serial.c | 16 ++++++++++++---- > 2 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h > b/arch/arm/plat-omap/include/plat/omap-serial.h > index 9ff4444..386a25b 100644 > --- a/arch/arm/plat-omap/include/plat/omap-serial.h > +++ b/arch/arm/plat-omap/include/plat/omap-serial.h > @@ -130,7 +130,8 @@ struct uart_omap_port { > unsigned long port_activity; > u32 context_loss_cnt; > u32 errata; > - u8 wakeups_enabled; > + u8 pad_wakeups_enabled; > + u8 module_wakeups_enabled; Why do you need 2 flags when they are always managed together. Kevin > struct pm_qos_request pm_qos_request; > u32 latency; > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > index 0121486..0a35b7e 100644 > --- a/drivers/tty/serial/omap-serial.c > +++ b/drivers/tty/serial/omap-serial.c > @@ -1458,6 +1458,12 @@ static int serial_omap_probe(struct > platform_device *pdev) > up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE; > } > > + /* Module level wakeup from sysc(BIT[2]) > + * will be enabled during boot > + * from hwmod framework. > + */ > + up->module_wakeups_enabled = true; > + > up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; > up->calc_latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; > pm_qos_add_request(&up->pm_qos_request, > @@ -1586,14 +1592,16 @@ static int serial_omap_runtime_suspend(struct > device *dev) > up->context_loss_cnt = pdata->get_context_loss_count(dev); > > if (device_may_wakeup(dev)) { > - if (!up->wakeups_enabled) { > + if (!up->pad_wakeups_enabled || !up->module_wakeups_enabled) { > pdata->enable_wakeup(up->pdev, true); > - up->wakeups_enabled = true; > + up->module_wakeups_enabled = true; > + up->pad_wakeups_enabled = true; > } > } else { > - if (up->wakeups_enabled) { > + if (up->pad_wakeups_enabled || up->module_wakeups_enabled) { > pdata->enable_wakeup(up->pdev, false); > - up->wakeups_enabled = false; > + up->module_wakeups_enabled = false; > + up->pad_wakeups_enabled = false; > } > } -- 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-27 21:37 ` Kevin Hilman @ 2012-03-28 10:59 ` Raja, Govindraj 2012-03-28 15:38 ` Joe Woodward 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-03-28 10:59 UTC (permalink / raw) To: Kevin Hilman; +Cc: Joe Woodward, linux-omap, Felipe Balbi On Wed, Mar 28, 2012 at 3:07 AM, Kevin Hilman <khilman@ti.com> wrote: > "Raja, Govindraj" <govindraj.raja@ti.com> writes: > >> Hi Kevin, >> >> On Tue, Mar 27, 2012 at 6:04 AM, Kevin Hilman <khilman@ti.com> wrote: >>> +Govindraj, >>> >>> "Joe Woodward" <jw@terrafix.co.uk> writes: >>> >>>> Right, I've stepped back a bit and dug out a GUSMTIX Palo43 carrier >>>> board on which to test the Overo OMAP3530 COM and I've found: >>>> - Running a stock 3.3 (with absolutely no changes) does indeed suspend correctly. >>>> - Running the 3.3 kernel with my (minor) board modifications >>>> (basically defining some buttons) suspends correctly as well. >>>> >>>> Then I went back to my original board and the 3.3 still wakes up from >>>> suspend immediately. So I had a think, and the only real differences >>>> between my board the the GUMSTIX Palo43 board is that I am using >>>> multiple UARTs. >>>> >>>> Up to this point I've only wanted to wake on the console (ttyO2), and >>>> not any other UARTs so I've stopped them waking with: >>>> echo disabled > /sys/devices/platform/omap/omap_uart.0/power/wakeup >>>> echo disabled > /sys/devices/platform/omap/omap_uart.1/power/wakeup >>>> >>>> I wanted to check that this still worked, so tried disabling wakeup on >>>> the console (ttyO2): >>>> echo disabled > /sys/devices/platform/omap/omap_uart.2/power/wakeup >>>> >>>> And if I do "echo mem > /sys/power/state" I was expecting to stay in >>>> suspend when I typed on my keyboard... However, the kernel still woke >>>> from suspend, which leads me to believe that the UART wakeup hasn't >>>> been disabled? >>> >>> Just to confirm: did the above work for you before v3.3? >>> >>>> Could you test if this is also the case your end? >>> >>> Yes, I get the same behavior, which is indeed broken. >>> >>> Govindraj, can you look into this? >>> >>> A quick glance suggests that disabling wakeups via the sysfs file is >>> only disabling runtime PM, but not actually disabling wakups at the >>> module-level or at the IO ring. >>> >> >> I have started looking into this, disabling and enabling of wake-ups >> from .runtime_suspend needs some changes as in here[1] with that I see pad >> wakeup getting disabled and it doesn't wake up after enabling off mode >> and suspending. > > Thanks for looking into this. > Looks like the module wakeup event handling left to default value during runtime clean up is causing the wakeup to happen Here is the patch[1] to fix the same tested on 3430SDP. -- Thanks, Govindraj.R [1]: From 5a5126750d527811547a45de9546c3e0f0fac77d Mon Sep 17 00:00:00 2001 From: "Govindraj.R" <govindraj.raja@ti.com> Date: Tue, 27 Mar 2012 18:55:00 +0530 Subject: [PATCH] OMAP2+: UART: Correct the module level wakeup enable/disable mechanism The commit (62f3ec5 ARM: OMAP2+: UART: Add wakeup mechanism for omap-uarts) removed module level wakeup enable/disable mechanism and retained only the pad wakeup handling. On 24xx/34xx/36xx Module level wakeup events are enabled/disabled using PM_WKEN1_CORE/PM_WKEN_PER regs. The module level wakeups are enabled by default from bootloader, however the wakeups can be enabled/disabled using sysfs entry echo disabled > /sys/devices/platform/omap/omap_uart.X/power/wakeup [X=0,1,2,3] Since module level wakeups were left enabled from bootup and when wakeups were disabled from sysfs uart module level wakeups were still happening as they were not getting disabled. Adding the support to handle module level wakeups for omap2/3 socs. Signed-off-by: Govindraj.R <govindraj.raja@ti.com> --- arch/arm/mach-omap2/serial.c | 95 +++++++++++++++++++++++++++++++++++++++++- 1 files changed, 93 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c index f590afc..92ff94c 100644 --- a/arch/arm/mach-omap2/serial.c +++ b/arch/arm/mach-omap2/serial.c @@ -56,6 +56,10 @@ struct omap_uart_state { int num; int can_sleep; + void __iomem *wk_st; + void __iomem *wk_en; + u32 wk_mask; + struct list_head node; struct omap_hwmod *oh; struct platform_device *pdev; @@ -82,17 +86,46 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = { }; #ifdef CONFIG_PM + +static void omap_uart_disable_module_wakeup(struct omap_uart_state *uart) +{ + /* Clear wake-enable bit */ + if (uart->wk_en && uart->wk_mask) { + u32 v = __raw_readl(uart->wk_en); + v &= ~uart->wk_mask; + __raw_writel(v, uart->wk_en); + } +} + +static void omap_uart_enable_module_wakeup(struct omap_uart_state *uart) +{ + /* Set wake-enable bit */ + if (uart->wk_en && uart->wk_mask) { + u32 v = __raw_readl(uart->wk_en); + v |= uart->wk_mask; + __raw_writel(v, uart->wk_en); + } +} + static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable) { struct omap_device *od = to_omap_device(pdev); + struct omap_uart_state *uart; if (!od) return; - if (enable) + list_for_each_entry(uart, &uart_list, node) + if (pdev->id == uart->num) + break; + + if (enable) { + omap_uart_enable_module_wakeup(uart); omap_hwmod_enable_wakeup(od->hwmods[0]); - else + } else { + omap_uart_disable_module_wakeup(uart); omap_hwmod_disable_wakeup(od->hwmods[0]); + } } /* @@ -114,7 +147,64 @@ static void omap_uart_set_smartidle(struct platform_device *pdev) omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_SMART); } +static void omap_uart_idle_init(struct omap_uart_state *uart) +{ + if (cpu_is_omap34xx() && !cpu_is_ti816x()) { + u32 mod = (uart->num == 2) ? OMAP3430_PER_MOD : CORE_MOD; + + uart->wk_en = OMAP34XX_PRM_REGADDR(mod, PM_WKEN1); + uart->wk_st = OMAP34XX_PRM_REGADDR(mod, PM_WKST1); + switch (uart->num) { + case 0: + uart->wk_mask = OMAP3430_ST_UART1_MASK; + break; + case 1: + uart->wk_mask = OMAP3430_ST_UART2_MASK; + break; + case 2: + uart->wk_mask = OMAP3430_ST_UART3_MASK; + break; + case 3: + uart->wk_mask = OMAP3630_ST_UART4_MASK; + break; + } + } else if (cpu_is_omap24xx()) { + + if (cpu_is_omap2430()) { + uart->wk_en = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKEN1); + uart->wk_st = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKST1); + } else if (cpu_is_omap2420()) { + uart->wk_en = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKEN1); + uart->wk_st = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKST1); + } + switch (uart->num) { + case 0: + uart->wk_mask = OMAP24XX_ST_UART1_MASK; + break; + case 1: + uart->wk_mask = OMAP24XX_ST_UART2_MASK; + break; + case 2: + uart->wk_mask = OMAP24XX_ST_UART3_MASK; + break; + } + } else { + uart->wk_en = 0; + uart->wk_st = 0; + uart->wk_mask = 0; + } + + /* Module level wakeups might be left enabled from + * bootloader disable it. Module level wakeup/pad_wakeup + * will be enabled if port is wakeup capaable after gating uart + * port clocks from omap-serial driver. + * Wakeup capability can be enabled or disbaled using sysfs entry. + */ + omap_uart_disable_module_wakeup(uart); +} + #else +static void omap_uart_idle_init(struct omap_uart_state *uart) {} static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable) {} static void omap_uart_set_noidle(struct platform_device *pdev) {} @@ -345,6 +435,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata, oh = uart->oh; name = DRIVER_NAME; + omap_uart_idle_init(uart); omap_up.dma_enabled = info->dma_enabled; omap_up.uartclk = OMAP24XX_BASE_BAUD * 16; omap_up.flags = UPF_BOOT_AUTOCONF; -- 1.7.5.4 -- 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 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-28 10:59 ` Raja, Govindraj @ 2012-03-28 15:38 ` Joe Woodward 2012-03-28 17:46 ` Kevin Hilman 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-28 15:38 UTC (permalink / raw) To: Raja, Govindraj, Kevin Hilman; +Cc: linux-omap, Felipe Balbi -----Original Message----- From: "Raja, Govindraj" <govindraj.raja@ti.com> To: Kevin Hilman <khilman@ti.com> Cc: Joe Woodward <jw@terrafix.co.uk>, "linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>, Felipe Balbi <balbi@ti.com> Date: Wed, 28 Mar 2012 16:29:53 +0530 Subject: Re: Suspend broken on 3.3? > On Wed, Mar 28, 2012 at 3:07 AM, Kevin Hilman <khilman@ti.com> wrote: > > "Raja, Govindraj" <govindraj.raja@ti.com> writes: > > > >> Hi Kevin, > >> > >> On Tue, Mar 27, 2012 at 6:04 AM, Kevin Hilman <khilman@ti.com> > wrote: > >>> +Govindraj, > >>> > >>> "Joe Woodward" <jw@terrafix.co.uk> writes: > >>> > >>>> Right, I've stepped back a bit and dug out a GUSMTIX Palo43 > carrier > >>>> board on which to test the Overo OMAP3530 COM and I've found: > >>>> - Running a stock 3.3 (with absolutely no changes) does indeed > suspend correctly. > >>>> - Running the 3.3 kernel with my (minor) board modifications > >>>> (basically defining some buttons) suspends correctly as well. > >>>> > >>>> Then I went back to my original board and the 3.3 still wakes up > from > >>>> suspend immediately. So I had a think, and the only real > differences > >>>> between my board the the GUMSTIX Palo43 board is that I am using > >>>> multiple UARTs. > >>>> > >>>> Up to this point I've only wanted to wake on the console (ttyO2), > and > >>>> not any other UARTs so I've stopped them waking with: > >>>> echo disabled > > /sys/devices/platform/omap/omap_uart.0/power/wakeup > >>>> echo disabled > > /sys/devices/platform/omap/omap_uart.1/power/wakeup > >>>> > >>>> I wanted to check that this still worked, so tried disabling > wakeup on > >>>> the console (ttyO2): > >>>> echo disabled > > /sys/devices/platform/omap/omap_uart.2/power/wakeup > >>>> > >>>> And if I do "echo mem > /sys/power/state" I was expecting to stay > in > >>>> suspend when I typed on my keyboard... However, the kernel still > woke > >>>> from suspend, which leads me to believe that the UART wakeup > hasn't > >>>> been disabled? > >>> > >>> Just to confirm: did the above work for you before v3.3? > >>> > >>>> Could you test if this is also the case your end? > >>> > >>> Yes, I get the same behavior, which is indeed broken. > >>> > >>> Govindraj, can you look into this? > >>> > >>> A quick glance suggests that disabling wakeups via the sysfs file > is > >>> only disabling runtime PM, but not actually disabling wakups at the > >>> module-level or at the IO ring. > >>> > >> > >> I have started looking into this, disabling and enabling of wake-ups > >> from .runtime_suspend needs some changes as in here[1] with that I > see pad > >> wakeup getting disabled and it doesn't wake up after enabling off > mode > >> and suspending. > > > > Thanks for looking into this. > > > > Looks like the module wakeup event handling left to default > value during runtime clean up is causing the wakeup to happen > > Here is the patch[1] to fix the same tested on 3430SDP. > > -- > Thanks, Thanks, This patch fixes the suspend problem for me, but there is another UART issue... Basically I've got a fairly high speed data source (in UART terms, >900KBaud) pumping data to the OMAP (such as GPS positions). I don't want this to wake me when suspended (which this patch fixes). However, it seems on 3.3 that I get a lot of corruption/lost characters, which I'm assuming is because the UART is implementing runtime-PM. So my next question is: How do I disable runtime-PM/force-always-on for a given UART? Can this be done via the sysfs? Or where are the runtime-PM constraints set for the UART? I'm guessing they'll work for 115200Baud, but my high speed UART fowls these? Cheers, Joe > Govindraj.R > > [1]: > > > From 5a5126750d527811547a45de9546c3e0f0fac77d Mon Sep 17 00:00:00 2001 > From: "Govindraj.R" <govindraj.raja@ti.com> > Date: Tue, 27 Mar 2012 18:55:00 +0530 > Subject: [PATCH] OMAP2+: UART: Correct the module level wakeup > enable/disable > mechanism > > The commit (62f3ec5 ARM: OMAP2+: UART: Add wakeup mechanism for > omap-uarts) > removed module level wakeup enable/disable mechanism and retained only > the pad wakeup handling. > > On 24xx/34xx/36xx Module level wakeup events are enabled/disabled using > PM_WKEN1_CORE/PM_WKEN_PER regs. The module level wakeups are enabled by > default from bootloader, however the wakeups can be enabled/disabled > using sysfs entry > echo disabled > /sys/devices/platform/omap/omap_uart.X/power/wakeup > [X=0,1,2,3] > > Since module level wakeups were left enabled from bootup and when > wakeups were disabled from sysfs uart module level wakeups were > still happening as they were not getting disabled. > > Adding the support to handle module level wakeups for omap2/3 socs. > > Signed-off-by: Govindraj.R <govindraj.raja@ti.com> > --- > arch/arm/mach-omap2/serial.c | 95 > +++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 93 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/serial.c > b/arch/arm/mach-omap2/serial.c > index f590afc..92ff94c 100644 > --- a/arch/arm/mach-omap2/serial.c > +++ b/arch/arm/mach-omap2/serial.c > @@ -56,6 +56,10 @@ struct omap_uart_state { > int num; > int can_sleep; > > + void __iomem *wk_st; > + void __iomem *wk_en; > + u32 wk_mask; > + > struct list_head node; > struct omap_hwmod *oh; > struct platform_device *pdev; > @@ -82,17 +86,46 @@ static struct omap_uart_port_info > omap_serial_default_info[] __initdata = { > }; > > #ifdef CONFIG_PM > + > +static void omap_uart_disable_module_wakeup(struct omap_uart_state > *uart) > +{ > + /* Clear wake-enable bit */ > + if (uart->wk_en && uart->wk_mask) { > + u32 v = __raw_readl(uart->wk_en); > + v &= ~uart->wk_mask; > + __raw_writel(v, uart->wk_en); > + } > +} > + > +static void omap_uart_enable_module_wakeup(struct omap_uart_state > *uart) > +{ > + /* Set wake-enable bit */ > + if (uart->wk_en && uart->wk_mask) { > + u32 v = __raw_readl(uart->wk_en); > + v |= uart->wk_mask; > + __raw_writel(v, uart->wk_en); > + } > +} > + > static void omap_uart_enable_wakeup(struct platform_device *pdev, bool > enable) > { > struct omap_device *od = to_omap_device(pdev); > + struct omap_uart_state *uart; > > if (!od) > return; > > - if (enable) > + list_for_each_entry(uart, &uart_list, node) > + if (pdev->id == uart->num) > + break; > + > + if (enable) { > + omap_uart_enable_module_wakeup(uart); > omap_hwmod_enable_wakeup(od->hwmods[0]); > - else > + } else { > + omap_uart_disable_module_wakeup(uart); > omap_hwmod_disable_wakeup(od->hwmods[0]); > + } > } > > /* > @@ -114,7 +147,64 @@ static void omap_uart_set_smartidle(struct > platform_device *pdev) > omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_SMART); > } > > +static void omap_uart_idle_init(struct omap_uart_state *uart) > +{ > + if (cpu_is_omap34xx() && !cpu_is_ti816x()) { > + u32 mod = (uart->num == 2) ? OMAP3430_PER_MOD : CORE_MOD; > + > + uart->wk_en = OMAP34XX_PRM_REGADDR(mod, PM_WKEN1); > + uart->wk_st = OMAP34XX_PRM_REGADDR(mod, PM_WKST1); > + switch (uart->num) { > + case 0: > + uart->wk_mask = OMAP3430_ST_UART1_MASK; > + break; > + case 1: > + uart->wk_mask = OMAP3430_ST_UART2_MASK; > + break; > + case 2: > + uart->wk_mask = OMAP3430_ST_UART3_MASK; > + break; > + case 3: > + uart->wk_mask = OMAP3630_ST_UART4_MASK; > + break; > + } > + } else if (cpu_is_omap24xx()) { > + > + if (cpu_is_omap2430()) { > + uart->wk_en = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKEN1); > + uart->wk_st = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKST1); > + } else if (cpu_is_omap2420()) { > + uart->wk_en = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKEN1); > + uart->wk_st = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKST1); > + } > + switch (uart->num) { > + case 0: > + uart->wk_mask = OMAP24XX_ST_UART1_MASK; > + break; > + case 1: > + uart->wk_mask = OMAP24XX_ST_UART2_MASK; > + break; > + case 2: > + uart->wk_mask = OMAP24XX_ST_UART3_MASK; > + break; > + } > + } else { > + uart->wk_en = 0; > + uart->wk_st = 0; > + uart->wk_mask = 0; > + } > + > + /* Module level wakeups might be left enabled from > + * bootloader disable it. Module level wakeup/pad_wakeup > + * will be enabled if port is wakeup capaable after gating uart > + * port clocks from omap-serial driver. > + * Wakeup capability can be enabled or disbaled using sysfs entry. > + */ > + omap_uart_disable_module_wakeup(uart); > +} > + > #else > +static void omap_uart_idle_init(struct omap_uart_state *uart) {} > static void omap_uart_enable_wakeup(struct platform_device *pdev, bool > enable) > {} > static void omap_uart_set_noidle(struct platform_device *pdev) {} > @@ -345,6 +435,7 @@ void __init omap_serial_init_port(struct > omap_board_data *bdata, > oh = uart->oh; > name = DRIVER_NAME; > > + omap_uart_idle_init(uart); > omap_up.dma_enabled = info->dma_enabled; > omap_up.uartclk = OMAP24XX_BASE_BAUD * 16; > omap_up.flags = UPF_BOOT_AUTOCONF; > -- > 1.7.5.4 > -- > 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 -- 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-28 15:38 ` Joe Woodward @ 2012-03-28 17:46 ` Kevin Hilman 2012-03-29 8:35 ` Joe Woodward 0 siblings, 1 reply; 40+ messages in thread From: Kevin Hilman @ 2012-03-28 17:46 UTC (permalink / raw) To: Joe Woodward Cc: Raja, Govindraj, linux-omap, Felipe Balbi, Paul Walmsley, neilb +Paul, NeilBrown who both have worked on/around recent UART breakage since v3.2 "Joe Woodward" <jw@terrafix.co.uk> writes: [...] > This patch fixes the suspend problem for me, but there is another UART issue... > > Basically I've got a fairly high speed data source (in UART terms, > >900KBaud) pumping data to the OMAP (such as GPS positions). > > I don't want this to wake me when suspended (which this patch fixes). > > However, it seems on 3.3 that I get a lot of corruption/lost > characters, which I'm assuming is because the UART is implementing > runtime-PM. > > So my next question is: How do I disable runtime-PM/force-always-on > for a given UART? Can this be done via the sysfs? Yes, but the boot-time default for this is that the UARTs have runtime PM disabled since the default autosuspend timeout is set to -1. You must be setting an autosuspend timeout >0 if you're seeing runtime PM kick in. That being said, even with an autosuspend timeout enabled, you can disable runtime PM by setting the /sys/devices/.../power/control knob to 'on' (instead of auto, which means it's controle by runtime PM): echo on > /sys/devices/platform/omap/omap_uart.2/power/control That will disable runtime PM and leave the UARTs always clocked. > Or where are the runtime-PM constraints set for the UART? Look for this in the driver: /* calculate wakeup latency constraint */ up->calc_latency = (USEC_PER_SEC * up->port.fifosize) / (baud / 8); > I'm guessing they'll work for 115200Baud, but my high speed UART fowls > these? The constraint calculations take into account baud rate, but are known to be somewhat broken currently. You might want to experiment with Paul's work on fixing up the QoS contstraint calculation[1] to see if it helps as well. That is available here Kevin [1] git://git.pwsan.com/linux-2.6 omap_serial_fix_pm_qos_formula_devel_3.4 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-28 17:46 ` Kevin Hilman @ 2012-03-29 8:35 ` Joe Woodward 2012-03-29 9:14 ` Shubhrajyoti Datta 2012-03-29 10:26 ` Paul Walmsley 0 siblings, 2 replies; 40+ messages in thread From: Joe Woodward @ 2012-03-29 8:35 UTC (permalink / raw) To: Kevin Hilman Cc: Raja, Govindraj, linux-omap, Felipe Balbi, Paul Walmsley, neilb -----Original Message----- From: Kevin Hilman <khilman@ti.com> To: "Joe Woodward" <jw@terrafix.co.uk> Cc: "Raja\, Govindraj" <govindraj.raja@ti.com>, linux-omap@vger.kernel.org, "Felipe Balbi" <balbi@ti.com>, Paul Walmsley <paul@pwsan.com>, neilb@suse.de Date: Wed, 28 Mar 2012 10:46:23 -0700 Subject: Re: Suspend broken on 3.3? > +Paul, NeilBrown who both have worked on/around recent UART breakage > since v3.2 > > "Joe Woodward" <jw@terrafix.co.uk> writes: > > [...] > > > This patch fixes the suspend problem for me, but there is another > UART issue... > > > > Basically I've got a fairly high speed data source (in UART terms, > > >900KBaud) pumping data to the OMAP (such as GPS positions). > > > > I don't want this to wake me when suspended (which this patch fixes). > > > > However, it seems on 3.3 that I get a lot of corruption/lost > > characters, which I'm assuming is because the UART is implementing > > runtime-PM. > > > > So my next question is: How do I disable runtime-PM/force-always-on > > for a given UART? Can this be done via the sysfs? > > Yes, but the boot-time default for this is that the UARTs have runtime > PM disabled since the default autosuspend timeout is set to -1. > > You must be setting an autosuspend timeout >0 if you're seeing runtime > PM kick in. > > That being said, even with an autosuspend timeout enabled, you can > disable runtime PM by setting the /sys/devices/.../power/control knob > to > 'on' (instead of auto, which means it's controle by runtime PM): > > echo on > /sys/devices/platform/omap/omap_uart.2/power/control > Right, First an apology... After checking /sys/devices/platform/omap/omap_uart.2/power/control I can see that runtime-PM is indeed disabled. After digging a bit further I found that the problem isn't lost characters or character corruption at all... The UART is actually at 430KBaud (not 900KBaud as I mentioned earlier). The data received is very bursty (i.e. sets of messages every second or so), containing a sync sequence to indicate a start of packet. The received bytes should be: 0x01, 0x52, 0x41 ....rest of packet. This works 100% of the time on 3.2, but on 3.3 I sometimes (but not always) get: 0x01, 0x00, 0x52, 0x41. i.e. there is a NULL/0x00 inserted after the first character. All this is tested using a very simple userspace application thats reads data from ttyO1. Any ideas? Should I kick open a new thread as it's not really to do with suspend anymore? Thanks, Joe > That will disable runtime PM and leave the UARTs always clocked. > > > Or where are the runtime-PM constraints set for the UART? > > Look for this in the driver: > > /* calculate wakeup latency constraint */ > up->calc_latency = (USEC_PER_SEC * up->port.fifosize) / (baud / 8); > > > I'm guessing they'll work for 115200Baud, but my high speed UART > fowls > > these? > > The constraint calculations take into account baud rate, but are known > to be somewhat broken currently. > > You might want to experiment with Paul's work on fixing up the QoS > contstraint calculation[1] to see if it helps as well. That is > available here > > Kevin > > [1] git://git.pwsan.com/linux-2.6 > omap_serial_fix_pm_qos_formula_devel_3.4 > -- > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-29 8:35 ` Joe Woodward @ 2012-03-29 9:14 ` Shubhrajyoti Datta 2012-03-29 9:46 ` Joe Woodward 2012-03-29 10:26 ` Paul Walmsley 1 sibling, 1 reply; 40+ messages in thread From: Shubhrajyoti Datta @ 2012-03-29 9:14 UTC (permalink / raw) To: Joe Woodward Cc: Kevin Hilman, Raja, Govindraj, linux-omap, Felipe Balbi, Paul Walmsley, neilb Hi Joe, > After digging a bit further I found that the problem isn't lost characters or character corruption at all... > > The UART is actually at 430KBaud (not 900KBaud as I mentioned earlier). How did you verify that register read? The data received is very bursty (i.e. sets of messages every second or so), containing a > sync sequence to indicate a start of packet. > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of packet. > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but not always) get: 0x01, 0x00, 0x52, 0x41. > > i.e. there is a NULL/0x00 inserted after the first character. > > All this is tested using a very simple userspace application thats reads data from ttyO1. > > Any ideas? Should I kick open a new thread as it's not really to do with suspend anymore? Is there any flow control you are using? > ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-29 9:14 ` Shubhrajyoti Datta @ 2012-03-29 9:46 ` Joe Woodward 0 siblings, 0 replies; 40+ messages in thread From: Joe Woodward @ 2012-03-29 9:46 UTC (permalink / raw) To: Shubhrajyoti Datta Cc: Kevin Hilman, Raja, Govindraj, linux-omap, Felipe Balbi, Paul Walmsley, neilb > Hi Joe, > > > After digging a bit further I found that the problem isn't lost > characters or character corruption at all... > > > > The UART is actually at 460KBaud (not 900KBaud as I mentioned > earlier). > How did you verify that register read? > I actually looked at the setting applied in my code (which I should have done earlier, oops :p). ... newtio.c_iflag = (IGNPAR); newtio.c_oflag = 0; newtio.c_cflag = B460800 | CS8 | CLOCAL | CREAD; newtio.c_lflag = 0; newtio.c_cc[VINTR] = 0; /* Ctrl-c */ newtio.c_cc[VQUIT] = 0; /* Ctrl-\ */ newtio.c_cc[VERASE] = 0; /* del */ newtio.c_cc[VKILL] = 0; /* @ */ newtio.c_cc[VEOF] = 0; /* Ctrl-d */ newtio.c_cc[VTIME] = 0; /* inter-character timer unused */ newtio.c_cc[VMIN] = 1; /* blocking read until 1 character arrives */ newtio.c_cc[VSWTC] = 0; /* '\0' */ newtio.c_cc[VSTART] = 0; /* Ctrl-q */ newtio.c_cc[VSTOP] = 0; /* Ctrl-s */ newtio.c_cc[VSUSP] = 0; /* Ctrl-z */ newtio.c_cc[VEOL] = 0; /* '\0' */ newtio.c_cc[VREPRINT] = 0; /* Ctrl-r */ newtio.c_cc[VDISCARD] = 0; /* Ctrl-u */ newtio.c_cc[VWERASE] = 0; /* Ctrl-w */ newtio.c_cc[VLNEXT] = 0; /* Ctrl-v */ newtio.c_cc[VEOL2] = 0; /* '\0' */ /* Clean the modem line and activate the settings for the port */ tcflush (handle->serialFD, TCIFLUSH); tcsetattr (handle->serialFD, TCSANOW, &newtio); ... I then loop read from a file descriptor to see the received bytes: serialFD = open ("/dev/ttyO0", O_RDWR | O_NOCTTY); ... while (1) { count = read (serialFD, buffer, MAXIMUM_LINE_LENGTH); ... debug here... ... } And it's by inspecting the bytes read that I noticed the inserted 0x00 on 3.3. > > The data received is very bursty (i.e. sets of messages every second > or so), containing a > > sync sequence to indicate a start of packet. > > > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of packet. > > > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but not > always) get: 0x01, 0x00, 0x52, 0x41. > > > > i.e. there is a NULL/0x00 inserted after the first character. > > > > All this is tested using a very simple userspace application thats > reads data from ttyO1. > > > > Any ideas? Should I kick open a new thread as it's not really to do > with suspend anymore? > Is there any flow control you are using? No flow control, but lack of flow control hasn't caused problems up to 3.2. Cheers, Joe > > > > -- > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-29 8:35 ` Joe Woodward 2012-03-29 9:14 ` Shubhrajyoti Datta @ 2012-03-29 10:26 ` Paul Walmsley 2012-03-29 11:27 ` Joe Woodward 1 sibling, 1 reply; 40+ messages in thread From: Paul Walmsley @ 2012-03-29 10:26 UTC (permalink / raw) To: Joe Woodward Cc: Kevin Hilman, Raja\, Govindraj, linux-omap, Felipe Balbi, neilb Hello Joe, thanks for reporting this. Some thoughts -- really just pure speculation -- but I hope some of it might be useful for you... On Thu, 29 Mar 2012, Joe Woodward wrote: > After digging a bit further I found that the problem isn't lost characters or character corruption at all... > > The UART is actually at 430KBaud (not 900KBaud as I mentioned earlier). 430Kbps? Could you confirm this? OMAP UARTs don't support that rate as far as I know - the closest is 460Kbps (OMAP34xx TRM vZR Table 17-1 "UART Mode Baud Rates, Divisor Values, and Error Rates). If one was desperate, it might be possible to get 430Kbps by tweaking other parts of the clock tree though... > The data received is very bursty (i.e. sets of messages every second or > so), containing a sync sequence to indicate a start of packet. > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of packet. > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but not always) get: 0x01, 0x00, 0x52, 0x41. > > i.e. there is a NULL/0x00 inserted after the first character. Is this on the serial console, or on a non-console serial port? Looking at drivers/tty/serial/omap-serial.c:receive_chars(), I wonder if the driver is somehow reading bytes from the RX FIFO when it's empty. It's unclear to me how this could happen. But you might want to try doing an LSR read right before entering the loop in drivers/tty/serial/omap-serial.c:receive_chars(). In stock v3.3, this would mean adding lsr = serial_in(up, UART_LSR); at line 190 of drivers/tty/serial/omap-serial.c. You might also try the DMA path as an experiment. This will totally wedge power management due to an insanely low timer expiration in that path, but at least might help narrow the problem down. To do so with a quick hack, you could set omap_serial_default_info.dma_enabled to true instead of false at arch/arm/mach-omap2/serial.c:76. And one other thing to try: does the behavior change if you set uart_debug to true at arch/arm/mach-omap2/serial.c:278 ? - Paul ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-29 10:26 ` Paul Walmsley @ 2012-03-29 11:27 ` Joe Woodward 2012-03-29 11:40 ` Joe Woodward 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-29 11:27 UTC (permalink / raw) To: Paul Walmsley Cc: Kevin Hilman, Raja\, Govindraj, linux-omap, Felipe Balbi, neilb > Hello Joe, > > thanks for reporting this. Some thoughts -- really just pure > speculation > -- but I hope some of it might be useful for you... > > On Thu, 29 Mar 2012, Joe Woodward wrote: > > > After digging a bit further I found that the problem isn't lost > characters or character corruption at all... > > > > The UART is actually at 430KBaud (not 900KBaud as I mentioned > earlier). > > 430Kbps? Could you confirm this? OMAP UARTs don't support that rate > as > far as I know - the closest is 460Kbps (OMAP34xx TRM vZR Table 17-1 > "UART > Mode Baud Rates, Divisor Values, and Error Rates). If one was > desperate, > it might be possible to get 430Kbps by tweaking other parts of the > clock > tree though... Sorry for the confusion... It's 460KBaud - the 430 was just a typo in my previous mail... > > > The data received is very bursty (i.e. sets of messages every second > or > > so), containing a sync sequence to indicate a start of packet. > > > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of packet. > > > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but not > always) get: 0x01, 0x00, 0x52, 0x41. > > > > i.e. there is a NULL/0x00 inserted after the first character. > > Is this on the serial console, or on a non-console serial port? > > Looking at drivers/tty/serial/omap-serial.c:receive_chars(), I wonder > if > the driver is somehow reading bytes from the RX FIFO when it's empty. > It's unclear to me how this could happen. But you might want to try > doing > an LSR read right before entering the loop in > drivers/tty/serial/omap-serial.c:receive_chars(). In stock v3.3, this > would mean adding > > lsr = serial_in(up, UART_LSR); > > at line 190 of drivers/tty/serial/omap-serial.c. > Adding this is made no obvious difference. > > You might also try the DMA path as an experiment. This will totally > wedge > power management due to an insanely low timer expiration in that path, > but > at least might help narrow the problem down. To do so with a quick > hack, > you could set omap_serial_default_info.dma_enabled to true instead of > false at arch/arm/mach-omap2/serial.c:76. > This did the trick (I've added it in addition to the LSR read above, i'll back out the LSR read and see if it still works). When DMA is enabled the behaviour (as seen from my application in userspace) is the same as on the stock 3.2 kernel (i.e. for me it works :) ). Cheers, Joe > > And one other thing to try: does the behavior change if you set > uart_debug > to true at arch/arm/mach-omap2/serial.c:278 ? > > > - Paul > -- > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-29 11:27 ` Joe Woodward @ 2012-03-29 11:40 ` Joe Woodward 2012-03-29 14:29 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-29 11:40 UTC (permalink / raw) To: Joe Woodward, Paul Walmsley Cc: Kevin Hilman, Raja\, Govindraj, linux-omap, Felipe Balbi, neilb -----Original Message----- From: "Joe Woodward" <jw@terrafix.co.uk> To: "Paul Walmsley" <paul@pwsan.com> Cc: "Kevin Hilman" <khilman@ti.com>, "Raja\\, Govindraj" <govindraj.raja@ti.com>, linux-omap@vger.kernel.org, "Felipe Balbi" <balbi@ti.com>, neilb@suse.de Date: Thu, 29 Mar 2012 12:27:55 +0100 Subject: Re: Suspend broken on 3.3? > > Hello Joe, > > > > thanks for reporting this. Some thoughts -- really just pure > > speculation > > -- but I hope some of it might be useful for you... > > > > On Thu, 29 Mar 2012, Joe Woodward wrote: > > > > > After digging a bit further I found that the problem isn't lost > > characters or character corruption at all... > > > > > > The UART is actually at 430KBaud (not 900KBaud as I mentioned > > earlier). > > > > 430Kbps? Could you confirm this? OMAP UARTs don't support that rate > > as > > far as I know - the closest is 460Kbps (OMAP34xx TRM vZR Table 17-1 > > "UART > > Mode Baud Rates, Divisor Values, and Error Rates). If one was > > desperate, > > it might be possible to get 430Kbps by tweaking other parts of the > > clock > > tree though... > > Sorry for the confusion... It's 460KBaud - the 430 was just a typo in > my previous mail... > > > > > > The data received is very bursty (i.e. sets of messages every > second > > or > > > so), containing a sync sequence to indicate a start of packet. > > > > > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of packet. > > > > > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but not > > always) get: 0x01, 0x00, 0x52, 0x41. > > > > > > i.e. there is a NULL/0x00 inserted after the first character. > > > > Is this on the serial console, or on a non-console serial port? > > > > Looking at drivers/tty/serial/omap-serial.c:receive_chars(), I wonder > > if > > the driver is somehow reading bytes from the RX FIFO when it's empty. > > > It's unclear to me how this could happen. But you might want to try > > doing > > an LSR read right before entering the loop in > > drivers/tty/serial/omap-serial.c:receive_chars(). In stock v3.3, > this > > would mean adding > > > > lsr = serial_in(up, UART_LSR); > > > > at line 190 of drivers/tty/serial/omap-serial.c. > > > > Adding this is made no obvious difference. > > > > > You might also try the DMA path as an experiment. This will totally > > wedge > > power management due to an insanely low timer expiration in that > path, > > but > > at least might help narrow the problem down. To do so with a quick > > hack, > > you could set omap_serial_default_info.dma_enabled to true instead of > > false at arch/arm/mach-omap2/serial.c:76. > > > > This did the trick (I've added it in addition to the LSR read above, > i'll back out the LSR read and see if it still works). > When DMA is enabled the behaviour (as seen from my application in > userspace) is the same as on the stock 3.2 kernel (i.e. for me it works > :) ). > I've just realised that if anyone has joined this thread late, then I'm running in a state with Govindraj's patch in a previous mail in this thread applied to serial.c to fix the suspend issues due to the UART wakeup's not being correctly changed from userspace via sysfs. It may actually by this patch that is causing the interrupt-enabled serial driver to have broken? The tty that is causing me a problem does have wake-from-suspend disabled for it from userspace... Cheers, Joe > Cheers, > Joe > > > > > And one other thing to try: does the behavior change if you set > > uart_debug > > to true at arch/arm/mach-omap2/serial.c:278 ? > > > > > > - Paul > > -- > > 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 > > > -- > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-29 11:40 ` Joe Woodward @ 2012-03-29 14:29 ` Raja, Govindraj 2012-03-30 7:53 ` Joe Woodward 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-03-29 14:29 UTC (permalink / raw) To: Joe Woodward; +Cc: Paul Walmsley, Kevin Hilman, linux-omap, Felipe Balbi, neilb On Thu, Mar 29, 2012 at 5:10 PM, Joe Woodward <jw@terrafix.co.uk> wrote: > > > -----Original Message----- > From: "Joe Woodward" <jw@terrafix.co.uk> > To: "Paul Walmsley" <paul@pwsan.com> > Cc: "Kevin Hilman" <khilman@ti.com>, "Raja\\, Govindraj" <govindraj.raja@ti.com>, linux-omap@vger.kernel.org, "Felipe Balbi" <balbi@ti.com>, neilb@suse.de > Date: Thu, 29 Mar 2012 12:27:55 +0100 > Subject: Re: Suspend broken on 3.3? > >> > Hello Joe, >> > >> > thanks for reporting this. Some thoughts -- really just pure >> > speculation >> > -- but I hope some of it might be useful for you... >> > >> > On Thu, 29 Mar 2012, Joe Woodward wrote: >> > >> > > After digging a bit further I found that the problem isn't lost >> > characters or character corruption at all... >> > > >> > > The UART is actually at 430KBaud (not 900KBaud as I mentioned >> > earlier). >> > >> > 430Kbps? Could you confirm this? OMAP UARTs don't support that rate >> > as >> > far as I know - the closest is 460Kbps (OMAP34xx TRM vZR Table 17-1 >> > "UART >> > Mode Baud Rates, Divisor Values, and Error Rates). If one was >> > desperate, >> > it might be possible to get 430Kbps by tweaking other parts of the >> > clock >> > tree though... >> >> Sorry for the confusion... It's 460KBaud - the 430 was just a typo in >> my previous mail... >> >> > >> > > The data received is very bursty (i.e. sets of messages every >> second >> > or >> > > so), containing a sync sequence to indicate a start of packet. >> > > >> > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of packet. >> > > >> > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but not >> > always) get: 0x01, 0x00, 0x52, 0x41. >> > > >> > > i.e. there is a NULL/0x00 inserted after the first character. >> > >> > Is this on the serial console, or on a non-console serial port? >> > >> > Looking at drivers/tty/serial/omap-serial.c:receive_chars(), I wonder >> > if >> > the driver is somehow reading bytes from the RX FIFO when it's empty. >> >> > It's unclear to me how this could happen. But you might want to try >> > doing >> > an LSR read right before entering the loop in >> > drivers/tty/serial/omap-serial.c:receive_chars(). In stock v3.3, >> this >> > would mean adding >> > >> > lsr = serial_in(up, UART_LSR); >> > >> > at line 190 of drivers/tty/serial/omap-serial.c. >> > >> >> Adding this is made no obvious difference. >> >> > >> > You might also try the DMA path as an experiment. This will totally >> > wedge >> > power management due to an insanely low timer expiration in that >> path, >> > but >> > at least might help narrow the problem down. To do so with a quick >> > hack, >> > you could set omap_serial_default_info.dma_enabled to true instead of >> > false at arch/arm/mach-omap2/serial.c:76. >> > >> >> This did the trick (I've added it in addition to the LSR read above, >> i'll back out the LSR read and see if it still works). >> When DMA is enabled the behaviour (as seen from my application in >> userspace) is the same as on the stock 3.2 kernel (i.e. for me it works >> :) ). >> > > I've just realised that if anyone has joined this thread late, then I'm running in a state with Govindraj's patch in a previous mail in this thread applied to serial.c to > fix the suspend issues due to the UART wakeup's not being correctly changed from userspace via sysfs. > > It may actually by this patch that is causing the interrupt-enabled serial driver to have broken? The tty that is causing me a problem does have wake-from-suspend > disabled for it from userspace... Is the patch shared earlier causing this issue of getting 0x00 from rx randomly ? -- Thanks, 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-29 14:29 ` Raja, Govindraj @ 2012-03-30 7:53 ` Joe Woodward 2012-03-30 8:46 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-30 7:53 UTC (permalink / raw) To: Raja, Govindraj Cc: Paul Walmsley, Kevin Hilman, linux-omap, Felipe Balbi, neilb -----Original Message----- From: "Raja, Govindraj" <govindraj.raja@ti.com> To: Joe Woodward <jw@terrafix.co.uk> Cc: Paul Walmsley <paul@pwsan.com>, Kevin Hilman <khilman@ti.com>, linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de Date: Thu, 29 Mar 2012 19:59:54 +0530 Subject: Re: Suspend broken on 3.3? > On Thu, Mar 29, 2012 at 5:10 PM, Joe Woodward <jw@terrafix.co.uk> > wrote: > > > > > > -----Original Message----- > > From: "Joe Woodward" <jw@terrafix.co.uk> > > To: "Paul Walmsley" <paul@pwsan.com> > > Cc: "Kevin Hilman" <khilman@ti.com>, "Raja\\, Govindraj" > <govindraj.raja@ti.com>, linux-omap@vger.kernel.org, "Felipe Balbi" > <balbi@ti.com>, neilb@suse.de > > Date: Thu, 29 Mar 2012 12:27:55 +0100 > > Subject: Re: Suspend broken on 3.3? > > > >> > Hello Joe, > >> > > >> > thanks for reporting this. Some thoughts -- really just pure > >> > speculation > >> > -- but I hope some of it might be useful for you... > >> > > >> > On Thu, 29 Mar 2012, Joe Woodward wrote: > >> > > >> > > After digging a bit further I found that the problem isn't lost > >> > characters or character corruption at all... > >> > > > >> > > The UART is actually at 430KBaud (not 900KBaud as I mentioned > >> > earlier). > >> > > >> > 430Kbps? Could you confirm this? OMAP UARTs don't support that > rate > >> > as > >> > far as I know - the closest is 460Kbps (OMAP34xx TRM vZR Table > 17-1 > >> > "UART > >> > Mode Baud Rates, Divisor Values, and Error Rates). If one was > >> > desperate, > >> > it might be possible to get 430Kbps by tweaking other parts of the > >> > clock > >> > tree though... > >> > >> Sorry for the confusion... It's 460KBaud - the 430 was just a typo > in > >> my previous mail... > >> > >> > > >> > > The data received is very bursty (i.e. sets of messages every > >> second > >> > or > >> > > so), containing a sync sequence to indicate a start of packet. > >> > > > >> > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of > packet. > >> > > > >> > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but > not > >> > always) get: 0x01, 0x00, 0x52, 0x41. > >> > > > >> > > i.e. there is a NULL/0x00 inserted after the first character. > >> > > >> > Is this on the serial console, or on a non-console serial port? > >> > > >> > Looking at drivers/tty/serial/omap-serial.c:receive_chars(), I > wonder > >> > if > >> > the driver is somehow reading bytes from the RX FIFO when it's > empty. > >> > >> > It's unclear to me how this could happen. But you might want to > try > >> > doing > >> > an LSR read right before entering the loop in > >> > drivers/tty/serial/omap-serial.c:receive_chars(). In stock v3.3, > >> this > >> > would mean adding > >> > > >> > lsr = serial_in(up, UART_LSR); > >> > > >> > at line 190 of drivers/tty/serial/omap-serial.c. > >> > > >> > >> Adding this is made no obvious difference. > >> > >> > > >> > You might also try the DMA path as an experiment. This will > totally > >> > wedge > >> > power management due to an insanely low timer expiration in that > >> path, > >> > but > >> > at least might help narrow the problem down. To do so with a > quick > >> > hack, > >> > you could set omap_serial_default_info.dma_enabled to true instead > of > >> > false at arch/arm/mach-omap2/serial.c:76. > >> > > >> > >> This did the trick (I've added it in addition to the LSR read above, > >> i'll back out the LSR read and see if it still works). > >> When DMA is enabled the behaviour (as seen from my application in > >> userspace) is the same as on the stock 3.2 kernel (i.e. for me it > works > >> :) ). > >> > > > > I've just realised that if anyone has joined this thread late, then > I'm running in a state with Govindraj's patch in a previous mail in > this thread applied to serial.c to > > fix the suspend issues due to the UART wakeup's not being correctly > changed from userspace via sysfs. > > > > It may actually by this patch that is causing the interrupt-enabled > serial driver to have broken? The tty that is causing me a problem does > have wake-from-suspend > > disabled for it from userspace... > > Is the patch shared earlier causing this issue of getting 0x00 from rx > randomly ? > In short, yes. I've gone back to a stock 3.3 kernel and the serial ports give the correct data, but suspend fails (due to not being able to disable wake-from-serial). I then applied your patch and could disable wakeup on the serial ports and suspend correctly, but the (non-console) serial port starts to misbehave. Forcing the driver to be DMA-enabled caused everything to work again. So something in the patch is causing the (default) interrupt-enabled serial driver to occassionally fail. Sorry for the goose chase yesterday. I didn't even think to check if the patch caused the issue as it seemed a bit unrelated. Cheers, Joe > -- > Thanks, > 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 -- 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-30 7:53 ` Joe Woodward @ 2012-03-30 8:46 ` Raja, Govindraj 2012-03-30 9:26 ` Joe Woodward 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-03-30 8:46 UTC (permalink / raw) To: Joe Woodward; +Cc: Paul Walmsley, Kevin Hilman, linux-omap, Felipe Balbi, neilb [-- Attachment #1: Type: text/plain, Size: 5498 bytes --] On Fri, Mar 30, 2012 at 1:23 PM, Joe Woodward <jw@terrafix.co.uk> wrote: > -----Original Message----- > From: "Raja, Govindraj" <govindraj.raja@ti.com> > To: Joe Woodward <jw@terrafix.co.uk> > Cc: Paul Walmsley <paul@pwsan.com>, Kevin Hilman <khilman@ti.com>, linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de > Date: Thu, 29 Mar 2012 19:59:54 +0530 > Subject: Re: Suspend broken on 3.3? > >> On Thu, Mar 29, 2012 at 5:10 PM, Joe Woodward <jw@terrafix.co.uk> >> wrote: >> > >> > >> > -----Original Message----- >> > From: "Joe Woodward" <jw@terrafix.co.uk> >> > To: "Paul Walmsley" <paul@pwsan.com> >> > Cc: "Kevin Hilman" <khilman@ti.com>, "Raja\\, Govindraj" >> <govindraj.raja@ti.com>, linux-omap@vger.kernel.org, "Felipe Balbi" >> <balbi@ti.com>, neilb@suse.de >> > Date: Thu, 29 Mar 2012 12:27:55 +0100 >> > Subject: Re: Suspend broken on 3.3? >> > >> >> > Hello Joe, >> >> > >> >> > thanks for reporting this. Some thoughts -- really just pure >> >> > speculation >> >> > -- but I hope some of it might be useful for you... >> >> > >> >> > On Thu, 29 Mar 2012, Joe Woodward wrote: >> >> > >> >> > > After digging a bit further I found that the problem isn't lost >> >> > characters or character corruption at all... >> >> > > >> >> > > The UART is actually at 430KBaud (not 900KBaud as I mentioned >> >> > earlier). >> >> > >> >> > 430Kbps? Could you confirm this? OMAP UARTs don't support that >> rate >> >> > as >> >> > far as I know - the closest is 460Kbps (OMAP34xx TRM vZR Table >> 17-1 >> >> > "UART >> >> > Mode Baud Rates, Divisor Values, and Error Rates). If one was >> >> > desperate, >> >> > it might be possible to get 430Kbps by tweaking other parts of the >> >> > clock >> >> > tree though... >> >> >> >> Sorry for the confusion... It's 460KBaud - the 430 was just a typo >> in >> >> my previous mail... >> >> >> >> > >> >> > > The data received is very bursty (i.e. sets of messages every >> >> second >> >> > or >> >> > > so), containing a sync sequence to indicate a start of packet. >> >> > > >> >> > > The received bytes should be: 0x01, 0x52, 0x41 ....rest of >> packet. >> >> > > >> >> > > This works 100% of the time on 3.2, but on 3.3 I sometimes (but >> not >> >> > always) get: 0x01, 0x00, 0x52, 0x41. >> >> > > >> >> > > i.e. there is a NULL/0x00 inserted after the first character. >> >> > >> >> > Is this on the serial console, or on a non-console serial port? >> >> > >> >> > Looking at drivers/tty/serial/omap-serial.c:receive_chars(), I >> wonder >> >> > if >> >> > the driver is somehow reading bytes from the RX FIFO when it's >> empty. >> >> >> >> > It's unclear to me how this could happen. But you might want to >> try >> >> > doing >> >> > an LSR read right before entering the loop in >> >> > drivers/tty/serial/omap-serial.c:receive_chars(). In stock v3.3, >> >> this >> >> > would mean adding >> >> > >> >> > lsr = serial_in(up, UART_LSR); >> >> > >> >> > at line 190 of drivers/tty/serial/omap-serial.c. >> >> > >> >> >> >> Adding this is made no obvious difference. >> >> >> >> > >> >> > You might also try the DMA path as an experiment. This will >> totally >> >> > wedge >> >> > power management due to an insanely low timer expiration in that >> >> path, >> >> > but >> >> > at least might help narrow the problem down. To do so with a >> quick >> >> > hack, >> >> > you could set omap_serial_default_info.dma_enabled to true instead >> of >> >> > false at arch/arm/mach-omap2/serial.c:76. >> >> > >> >> >> >> This did the trick (I've added it in addition to the LSR read above, >> >> i'll back out the LSR read and see if it still works). >> >> When DMA is enabled the behaviour (as seen from my application in >> >> userspace) is the same as on the stock 3.2 kernel (i.e. for me it >> works >> >> :) ). >> >> >> > >> > I've just realised that if anyone has joined this thread late, then >> I'm running in a state with Govindraj's patch in a previous mail in >> this thread applied to serial.c to >> > fix the suspend issues due to the UART wakeup's not being correctly >> changed from userspace via sysfs. >> > >> > It may actually by this patch that is causing the interrupt-enabled >> serial driver to have broken? The tty that is causing me a problem does >> have wake-from-suspend >> > disabled for it from userspace... >> >> Is the patch shared earlier causing this issue of getting 0x00 from rx >> randomly ? >> > > In short, yes. > > I've gone back to a stock 3.3 kernel and the serial ports give the correct data, but suspend fails (due to not being able to disable wake-from-serial). > > I then applied your patch and could disable wakeup on the serial ports and suspend correctly, but the (non-console) serial port starts to misbehave. > > Forcing the driver to be DMA-enabled caused everything to work again. So something in the patch is causing the (default) interrupt-enabled serial driver to > occassionally fail. > Disabling module level wakeup by default on bootup in previous shared patch in serial.c in omap_uart_idle_init => omap_uart_disable_module_wakeup might be causing this issue and probably this should be left enabled by default and be disabled only when requested from sysfs on suspend. Could you please try attached patch and let me know if this solves the rx issue as well, without using dma mode. -- Thanks, Govindraj.R [-- Attachment #2: 0001-OMAP2-UART-Correct-the-module-level-wakeup-enable-di.patch --] [-- Type: application/octet-stream, Size: 6654 bytes --] From 6a5143110405ce397b5941b756041ecc6e1f0561 Mon Sep 17 00:00:00 2001 From: "Govindraj.R" <govindraj.raja@ti.com> Date: Tue, 27 Mar 2012 18:55:00 +0530 Subject: [PATCH] OMAP2+: UART: Correct the module level wakeup enable/disable mechanism The commit (62f3ec5 ARM: OMAP2+: UART: Add wakeup mechanism for omap-uarts) removed module level wakeup enable/disable mechanism and retained only the pad wakeup handling. On 24xx/34xx/36xx Module level wakeup events are enabled/disabled using PM_WKEN1_CORE/PM_WKEN_PER regs. The module level wakeups are enabled by default from bootloader, however the wakeups can be enabled/disabled using sysfs entry echo disabled > /sys/devices/platform/omap/omap_uart.X/power/wakeup [X=0,1,2,3] Since module level wakeups were left enabled from bootup and when wakeups were disabled from sysfs uart module level wakeups were still happening as they were not getting disabled. Adding the support to handle module level wakeups for omap2/3 socs. Signed-off-by: Govindraj.R <govindraj.raja@ti.com> --- arch/arm/mach-omap2/serial.c | 88 ++++++++++++++++++++++++- arch/arm/plat-omap/include/plat/omap-serial.h | 3 +- drivers/tty/serial/omap-serial.c | 15 +++- 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c index 0cdd359..9312d6b 100644 --- a/arch/arm/mach-omap2/serial.c +++ b/arch/arm/mach-omap2/serial.c @@ -41,6 +41,7 @@ #include "prm-regbits-34xx.h" #include "control.h" #include "mux.h" +#include "iomap.h" /* * NOTE: By default the serial auto_suspend timeout is disabled as it causes @@ -55,6 +56,10 @@ struct omap_uart_state { int num; + void __iomem *wk_st; + void __iomem *wk_en; + u32 wk_mask; + struct list_head node; struct omap_hwmod *oh; }; @@ -80,17 +85,46 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = { }; #ifdef CONFIG_PM + +static void omap_uart_disable_module_wakeup(struct omap_uart_state *uart) +{ + /* Clear wake-enable bit */ + if (uart->wk_en && uart->wk_mask) { + u32 v = __raw_readl(uart->wk_en); + v &= ~uart->wk_mask; + __raw_writel(v, uart->wk_en); + } +} + +static void omap_uart_enable_module_wakeup(struct omap_uart_state *uart) +{ + /* Set wake-enable bit */ + if (uart->wk_en && uart->wk_mask) { + u32 v = __raw_readl(uart->wk_en); + v |= uart->wk_mask; + __raw_writel(v, uart->wk_en); + } +} + static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable) { struct omap_device *od = to_omap_device(pdev); + struct omap_uart_state *uart; if (!od) return; - if (enable) + list_for_each_entry(uart, &uart_list, node) + if (pdev->id == uart->num) + break; + + if (enable) { + omap_uart_enable_module_wakeup(uart); omap_hwmod_enable_wakeup(od->hwmods[0]); - else + } else { + omap_uart_disable_module_wakeup(uart); omap_hwmod_disable_wakeup(od->hwmods[0]); + } } /* @@ -112,7 +146,56 @@ static void omap_uart_set_smartidle(struct platform_device *pdev) omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_SMART); } +static void omap_uart_idle_init(struct omap_uart_state *uart) +{ + if (cpu_is_omap34xx() && !cpu_is_ti816x()) { + u32 mod = (uart->num == 2) ? OMAP3430_PER_MOD : CORE_MOD; + + uart->wk_en = OMAP34XX_PRM_REGADDR(mod, PM_WKEN1); + uart->wk_st = OMAP34XX_PRM_REGADDR(mod, PM_WKST1); + switch (uart->num) { + case 0: + uart->wk_mask = OMAP3430_ST_UART1_MASK; + break; + case 1: + uart->wk_mask = OMAP3430_ST_UART2_MASK; + break; + case 2: + uart->wk_mask = OMAP3430_ST_UART3_MASK; + break; + case 3: + uart->wk_mask = OMAP3630_ST_UART4_MASK; + break; + } + } else if (cpu_is_omap24xx()) { + + if (cpu_is_omap2430()) { + uart->wk_en = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKEN1); + uart->wk_st = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKST1); + } else if (cpu_is_omap2420()) { + uart->wk_en = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKEN1); + uart->wk_st = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKST1); + } + switch (uart->num) { + case 0: + uart->wk_mask = OMAP24XX_ST_UART1_MASK; + break; + case 1: + uart->wk_mask = OMAP24XX_ST_UART2_MASK; + break; + case 2: + uart->wk_mask = OMAP24XX_ST_UART3_MASK; + break; + } + } else { + uart->wk_en = 0; + uart->wk_st = 0; + uart->wk_mask = 0; + } +} + #else +static void omap_uart_idle_init(struct omap_uart_state *uart) {} static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable) {} static void omap_uart_set_noidle(struct platform_device *pdev) {} @@ -343,6 +426,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata, oh = uart->oh; name = DRIVER_NAME; + omap_uart_idle_init(uart); omap_up.dma_enabled = info->dma_enabled; omap_up.uartclk = OMAP24XX_BASE_BAUD * 16; omap_up.flags = UPF_BOOT_AUTOCONF; diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h index 9ff4444..386a25b 100644 --- a/arch/arm/plat-omap/include/plat/omap-serial.h +++ b/arch/arm/plat-omap/include/plat/omap-serial.h @@ -130,7 +130,8 @@ struct uart_omap_port { unsigned long port_activity; u32 context_loss_cnt; u32 errata; - u8 wakeups_enabled; + u8 pad_wakeups_enabled; + u8 module_wakeups_enabled; struct pm_qos_request pm_qos_request; u32 latency; diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 0121486..bb0a74d 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -1458,6 +1458,11 @@ static int serial_omap_probe(struct platform_device *pdev) up->uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE; } + /* Module level wakeup will be left enabled + * from bootloader. + */ + up->module_wakeups_enabled = true; + up->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; up->calc_latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; pm_qos_add_request(&up->pm_qos_request, @@ -1586,14 +1591,16 @@ static int serial_omap_runtime_suspend(struct device *dev) up->context_loss_cnt = pdata->get_context_loss_count(dev); if (device_may_wakeup(dev)) { - if (!up->wakeups_enabled) { + if (!up->pad_wakeups_enabled || !up->module_wakeups_enabled) { pdata->enable_wakeup(up->pdev, true); - up->wakeups_enabled = true; + up->module_wakeups_enabled = true; + up->pad_wakeups_enabled = true; } } else { - if (up->wakeups_enabled) { + if (up->pad_wakeups_enabled || up->module_wakeups_enabled) { pdata->enable_wakeup(up->pdev, false); - up->wakeups_enabled = false; + up->module_wakeups_enabled = false; + up->pad_wakeups_enabled = false; } } -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-30 8:46 ` Raja, Govindraj @ 2012-03-30 9:26 ` Joe Woodward 2012-03-30 10:15 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-30 9:26 UTC (permalink / raw) To: Raja, Govindraj Cc: Paul Walmsley, Kevin Hilman, linux-omap, Felipe Balbi, neilb ...[snip]... > > Could you please try attached patch and let me know if this solves the > rx issue as well, > without using dma mode. > Right, I think we've getting closer, but still not quite there... Firstly, the patch adds an include to "iomap.h" - but this doesn't exist in stock 3.3. Simply removing this include seemed to allow the compile to complete successfully. With this patch applied (and not in DMA mode) I now get the following: - If I leave wake-from-suspend enabled for the serial port then it works correctly (i.e. no lost/extra 0x00 characters). - If I disable wake-from-suspend for the serial port and go through a suspend cycle (i.e. suspend and then wake), then the serial port starts to misbehave as before. - If I then re-enable wake-from-suspend and go through a suspend cycle it starts to work correctly again. So the problem is still that if wake-from-suspend is disabled for a serial port, and a suspend cycle is performed then when woken the serial port will not function correctly if operating in interrupt-mode. Cheers, Joe > -- > Thanks, > Govindraj.R ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-30 9:26 ` Joe Woodward @ 2012-03-30 10:15 ` Raja, Govindraj 2012-03-30 11:04 ` Joe Woodward 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-03-30 10:15 UTC (permalink / raw) To: Joe Woodward; +Cc: Paul Walmsley, Kevin Hilman, linux-omap, Felipe Balbi, neilb [-- Attachment #1: Type: text/plain, Size: 1523 bytes --] On Fri, Mar 30, 2012 at 2:56 PM, Joe Woodward <jw@terrafix.co.uk> wrote: > ...[snip]... >> >> Could you please try attached patch and let me know if this solves the >> rx issue as well, >> without using dma mode. >> > > Right, > > I think we've getting closer, but still not quite there... > > Firstly, the patch adds an include to "iomap.h" - but this doesn't exist in stock 3.3. Simply removing this include seemed to allow the compile to complete > successfully. Sorry I forgot to specify this is needed for latest mainline. > > With this patch applied (and not in DMA mode) I now get the following: > - If I leave wake-from-suspend enabled for the serial port then it works correctly (i.e. no lost/extra 0x00 characters). > - If I disable wake-from-suspend for the serial port and go through a suspend cycle (i.e. suspend and then wake), then the serial port starts to misbehave as > before. > - If I then re-enable wake-from-suspend and go through a suspend cycle it starts to work correctly again. > > So the problem is still that if wake-from-suspend is disabled for a serial port, and a suspend cycle is performed then when woken the serial port will not function > correctly if operating in interrupt-mode. I tried it on my 3430sdp but strangely I don't see a 0x00 char read after disabling uart wakeups and waking up by keypad press. Probably as a quick try we can try doing uart_insert_char only if data was read from rx fifo (Attached patch) -- Thanks, Govindraj.R [-- Attachment #2: rx_fifo_check.patch --] [-- Type: application/octet-stream, Size: 971 bytes --] diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 05620fb..13a15c6 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -206,10 +206,14 @@ static inline void receive_chars(struct uart_omap_port *up, unsigned int flag, lsr = *status; unsigned char ch = 0; int max_count = 256; + u8 recd_ch = false; do { - if (likely(lsr & UART_LSR_DR)) + if (likely(lsr & UART_LSR_DR)) { ch = serial_in(up, UART_RX); + recd_ch = true; + } + flag = TTY_NORMAL; up->port.icount.rx++; @@ -258,7 +262,9 @@ static inline void receive_chars(struct uart_omap_port *up, if (uart_handle_sysrq_char(&up->port, ch)) goto ignore_char; - uart_insert_char(&up->port, lsr, UART_LSR_OE, ch, flag); + + if (recd_ch) + uart_insert_char(&up->port, lsr, UART_LSR_OE, ch, flag); ignore_char: lsr = serial_in(up, UART_LSR); } while ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (max_count-- > 0)); ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-30 10:15 ` Raja, Govindraj @ 2012-03-30 11:04 ` Joe Woodward 2012-03-30 12:24 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-30 11:04 UTC (permalink / raw) To: Raja, Govindraj Cc: Paul Walmsley, Kevin Hilman, linux-omap, Felipe Balbi, neilb -----Original Message----- From: "Raja, Govindraj" <govindraj.raja@ti.com> To: Joe Woodward <jw@terrafix.co.uk> Cc: Paul Walmsley <paul@pwsan.com>, Kevin Hilman <khilman@ti.com>, linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de Date: Fri, 30 Mar 2012 15:45:19 +0530 Subject: Re: Suspend broken on 3.3? > On Fri, Mar 30, 2012 at 2:56 PM, Joe Woodward <jw@terrafix.co.uk> > wrote: > > ...[snip]... > >> > >> Could you please try attached patch and let me know if this solves > the > >> rx issue as well, > >> without using dma mode. > >> > > > > Right, > > > > I think we've getting closer, but still not quite there... > > > > Firstly, the patch adds an include to "iomap.h" - but this doesn't > exist in stock 3.3. Simply removing this include seemed to allow the > compile to complete > > successfully. > > Sorry I forgot to specify this is needed for latest mainline. > > > > > With this patch applied (and not in DMA mode) I now get the > following: > > - If I leave wake-from-suspend enabled for the serial port then it > works correctly (i.e. no lost/extra 0x00 characters). > > - If I disable wake-from-suspend for the serial port and go through > a suspend cycle (i.e. suspend and then wake), then the serial port > starts to misbehave as > > before. > > - If I then re-enable wake-from-suspend and go through a suspend > cycle it starts to work correctly again. > > > > So the problem is still that if wake-from-suspend is disabled for a > serial port, and a suspend cycle is performed then when woken the > serial port will not function > > correctly if operating in interrupt-mode. > > I tried it on my 3430sdp but strangely I don't see a 0x00 char read > after disabling uart wakeups > and waking up by keypad press. > > Probably as a quick try we can try doing uart_insert_char only if data > was read from rx fifo > (Attached patch) > Sadly the patch makes no difference. I'm suprised you aren't seeing similar effects. I've done more testing, and a simplified test case is as follows: - take a stock 3.3 kernel and apply your patch to allow disabling of wake-from-suspend for the serial ports. - disable wake-from-suspend for the console tty: echo disable > /sys/devices/platform/omap/omap_uart.2/power/wakeup - perform a suspend (you'll need a button or something to wake you now that the console wont). echo mem > /sys/power/state At this point the console is noticable/painfully slow. No characters are lost, but it's obvious something isn't right! Cheers, Joe > -- > Thanks, > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-30 11:04 ` Joe Woodward @ 2012-03-30 12:24 ` Raja, Govindraj 2012-04-02 10:43 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-03-30 12:24 UTC (permalink / raw) To: Joe Woodward; +Cc: Paul Walmsley, Kevin Hilman, linux-omap, Felipe Balbi, neilb On Fri, Mar 30, 2012 at 4:34 PM, Joe Woodward <jw@terrafix.co.uk> wrote: > > > -----Original Message----- > From: "Raja, Govindraj" <govindraj.raja@ti.com> > To: Joe Woodward <jw@terrafix.co.uk> > Cc: Paul Walmsley <paul@pwsan.com>, Kevin Hilman <khilman@ti.com>, linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de > Date: Fri, 30 Mar 2012 15:45:19 +0530 > Subject: Re: Suspend broken on 3.3? > >> On Fri, Mar 30, 2012 at 2:56 PM, Joe Woodward <jw@terrafix.co.uk> >> wrote: >> > ...[snip]... >> >> >> >> Could you please try attached patch and let me know if this solves >> the >> >> rx issue as well, >> >> without using dma mode. >> >> >> > >> > Right, >> > >> > I think we've getting closer, but still not quite there... >> > >> > Firstly, the patch adds an include to "iomap.h" - but this doesn't >> exist in stock 3.3. Simply removing this include seemed to allow the >> compile to complete >> > successfully. >> >> Sorry I forgot to specify this is needed for latest mainline. >> >> > >> > With this patch applied (and not in DMA mode) I now get the >> following: >> > - If I leave wake-from-suspend enabled for the serial port then it >> works correctly (i.e. no lost/extra 0x00 characters). >> > - If I disable wake-from-suspend for the serial port and go through >> a suspend cycle (i.e. suspend and then wake), then the serial port >> starts to misbehave as >> > before. >> > - If I then re-enable wake-from-suspend and go through a suspend >> cycle it starts to work correctly again. >> > >> > So the problem is still that if wake-from-suspend is disabled for a >> serial port, and a suspend cycle is performed then when woken the >> serial port will not function >> > correctly if operating in interrupt-mode. >> >> I tried it on my 3430sdp but strangely I don't see a 0x00 char read >> after disabling uart wakeups >> and waking up by keypad press. >> >> Probably as a quick try we can try doing uart_insert_char only if data >> was read from rx fifo >> (Attached patch) >> > > Sadly the patch makes no difference. > > I'm suprised you aren't seeing similar effects. > > I've done more testing, and a simplified test case is as follows: > - take a stock 3.3 kernel and apply your patch to allow disabling of wake-from-suspend for the serial ports. > - disable wake-from-suspend for the console tty: > echo disable > /sys/devices/platform/omap/omap_uart.2/power/wakeup > - perform a suspend (you'll need a button or something to wake you now that the console wont). > echo mem > /sys/power/state > > At this point the console is noticable/painfully slow. No characters are lost, but it's obvious something isn't right! Yes I see this behavior with console now it becomes very slow after we disable module level wakeups. One difference to note is : in 3.2 => full system PM is prevented in idle path if uart is active i.e, MPU will not hit retention in 3.3 => MPU will hit retention independently of uart is active or not. I think the way to get MPU qos for uart port activity while in irq mode is to use PM_WKEN1 reg settings, meaning enabling module level wakeup event generation. Disabling it is causing this slow response. Checking if we can workaround this scenario. -- Thanks, 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-30 12:24 ` Raja, Govindraj @ 2012-04-02 10:43 ` Raja, Govindraj 2012-04-02 12:37 ` Joe Woodward 2012-04-04 14:56 ` Paul Walmsley 0 siblings, 2 replies; 40+ messages in thread From: Raja, Govindraj @ 2012-04-02 10:43 UTC (permalink / raw) To: Joe Woodward, Kevin Hilman; +Cc: Paul Walmsley, linux-omap, Felipe Balbi, neilb On Fri, Mar 30, 2012 at 5:54 PM, Raja, Govindraj <govindraj.raja@ti.com> wrote: > On Fri, Mar 30, 2012 at 4:34 PM, Joe Woodward <jw@terrafix.co.uk> wrote: >> >> >> -----Original Message----- >> From: "Raja, Govindraj" <govindraj.raja@ti.com> >> To: Joe Woodward <jw@terrafix.co.uk> >> Cc: Paul Walmsley <paul@pwsan.com>, Kevin Hilman <khilman@ti.com>, linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de >> Date: Fri, 30 Mar 2012 15:45:19 +0530 >> Subject: Re: Suspend broken on 3.3? >> >>> On Fri, Mar 30, 2012 at 2:56 PM, Joe Woodward <jw@terrafix.co.uk> >>> wrote: >>> > ...[snip]... >>> >> >>> >> Could you please try attached patch and let me know if this solves >>> the >>> >> rx issue as well, >>> >> without using dma mode. >>> >> >>> > >>> > Right, >>> > >>> > I think we've getting closer, but still not quite there... >>> > >>> > Firstly, the patch adds an include to "iomap.h" - but this doesn't >>> exist in stock 3.3. Simply removing this include seemed to allow the >>> compile to complete >>> > successfully. >>> >>> Sorry I forgot to specify this is needed for latest mainline. >>> >>> > >>> > With this patch applied (and not in DMA mode) I now get the >>> following: >>> > - If I leave wake-from-suspend enabled for the serial port then it >>> works correctly (i.e. no lost/extra 0x00 characters). >>> > - If I disable wake-from-suspend for the serial port and go through >>> a suspend cycle (i.e. suspend and then wake), then the serial port >>> starts to misbehave as >>> > before. >>> > - If I then re-enable wake-from-suspend and go through a suspend >>> cycle it starts to work correctly again. >>> > >>> > So the problem is still that if wake-from-suspend is disabled for a >>> serial port, and a suspend cycle is performed then when woken the >>> serial port will not function >>> > correctly if operating in interrupt-mode. >>> >>> I tried it on my 3430sdp but strangely I don't see a 0x00 char read >>> after disabling uart wakeups >>> and waking up by keypad press. >>> >>> Probably as a quick try we can try doing uart_insert_char only if data >>> was read from rx fifo >>> (Attached patch) >>> >> >> Sadly the patch makes no difference. >> >> I'm suprised you aren't seeing similar effects. >> >> I've done more testing, and a simplified test case is as follows: >> - take a stock 3.3 kernel and apply your patch to allow disabling of wake-from-suspend for the serial ports. >> - disable wake-from-suspend for the console tty: >> echo disable > /sys/devices/platform/omap/omap_uart.2/power/wakeup >> - perform a suspend (you'll need a button or something to wake you now that the console wont). >> echo mem > /sys/power/state >> >> At this point the console is noticable/painfully slow. No characters are lost, but it's obvious something isn't right! > > > Yes I see this behavior with console now it becomes very slow after we > disable module level wakeups. > > One difference to note is : > > in 3.2 => full system PM is prevented in idle path if uart is active > i.e, MPU will not hit retention > > in 3.3 => MPU will hit retention independently of uart is active or not. > I think the way to get MPU qos for uart port activity > while in irq mode is to use PM_WKEN1 > reg settings, meaning enabling module level wakeup event > generation. Disabling it is causing this > slow response. > > Checking if we can workaround this scenario. As of now what I can think of is to update qos reqest to prevent MPU from transitioning in case of port activity if wakeup capability for port is disabled. -- Thanks, 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-02 10:43 ` Raja, Govindraj @ 2012-04-02 12:37 ` Joe Woodward 2012-04-03 6:56 ` Govindraj 2012-04-04 14:56 ` Paul Walmsley 1 sibling, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-04-02 12:37 UTC (permalink / raw) To: Raja, Govindraj, Kevin Hilman Cc: Paul Walmsley, linux-omap, Felipe Balbi, neilb -----Original Message----- From: "Raja, Govindraj" <govindraj.raja@ti.com> To: Joe Woodward <jw@terrafix.co.uk>, Kevin Hilman <khilman@ti.com> Cc: Paul Walmsley <paul@pwsan.com>, linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de Date: Mon, 2 Apr 2012 16:13:13 +0530 Subject: Re: Suspend broken on 3.3? > On Fri, Mar 30, 2012 at 5:54 PM, Raja, Govindraj > <govindraj.raja@ti.com> wrote: > > On Fri, Mar 30, 2012 at 4:34 PM, Joe Woodward <jw@terrafix.co.uk> > wrote: > >> > >> > >> -----Original Message----- > >> From: "Raja, Govindraj" <govindraj.raja@ti.com> > >> To: Joe Woodward <jw@terrafix.co.uk> > >> Cc: Paul Walmsley <paul@pwsan.com>, Kevin Hilman <khilman@ti.com>, > linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de > >> Date: Fri, 30 Mar 2012 15:45:19 +0530 > >> Subject: Re: Suspend broken on 3.3? > >> > >>> On Fri, Mar 30, 2012 at 2:56 PM, Joe Woodward <jw@terrafix.co.uk> > >>> wrote: > >>> > ...[snip]... > >>> >> > >>> >> Could you please try attached patch and let me know if this > solves > >>> the > >>> >> rx issue as well, > >>> >> without using dma mode. > >>> >> > >>> > > >>> > Right, > >>> > > >>> > I think we've getting closer, but still not quite there... > >>> > > >>> > Firstly, the patch adds an include to "iomap.h" - but this > doesn't > >>> exist in stock 3.3. Simply removing this include seemed to allow > the > >>> compile to complete > >>> > successfully. > >>> > >>> Sorry I forgot to specify this is needed for latest mainline. > >>> > >>> > > >>> > With this patch applied (and not in DMA mode) I now get the > >>> following: > >>> > - If I leave wake-from-suspend enabled for the serial port then > it > >>> works correctly (i.e. no lost/extra 0x00 characters). > >>> > - If I disable wake-from-suspend for the serial port and go > through > >>> a suspend cycle (i.e. suspend and then wake), then the serial port > >>> starts to misbehave as > >>> > before. > >>> > - If I then re-enable wake-from-suspend and go through a > suspend > >>> cycle it starts to work correctly again. > >>> > > >>> > So the problem is still that if wake-from-suspend is disabled for > a > >>> serial port, and a suspend cycle is performed then when woken the > >>> serial port will not function > >>> > correctly if operating in interrupt-mode. > >>> > >>> I tried it on my 3430sdp but strangely I don't see a 0x00 char read > >>> after disabling uart wakeups > >>> and waking up by keypad press. > >>> > >>> Probably as a quick try we can try doing uart_insert_char only if > data > >>> was read from rx fifo > >>> (Attached patch) > >>> > >> > >> Sadly the patch makes no difference. > >> > >> I'm suprised you aren't seeing similar effects. > >> > >> I've done more testing, and a simplified test case is as follows: > >> - take a stock 3.3 kernel and apply your patch to allow disabling of > wake-from-suspend for the serial ports. > >> - disable wake-from-suspend for the console tty: > >> echo disable > /sys/devices/platform/omap/omap_uart.2/power/wakeup > >> - perform a suspend (you'll need a button or something to wake you > now that the console wont). > >> echo mem > /sys/power/state > >> > >> At this point the console is noticable/painfully slow. No characters > are lost, but it's obvious something isn't right! > > > > > > Yes I see this behavior with console now it becomes very slow after > we > > disable module level wakeups. > > > > One difference to note is : > > > > in 3.2 => full system PM is prevented in idle path if uart is active > > i.e, MPU will not hit retention > > > > in 3.3 => MPU will hit retention independently of uart is active or > not. > > I think the way to get MPU qos for uart port > activity > > while in irq mode is to use PM_WKEN1 > > reg settings, meaning enabling module level wakeup > event > > generation. Disabling it is causing this > > slow response. > > > > Checking if we can workaround this scenario. > > As of now what I can think of is to update qos reqest to prevent MPU > from transitioning in case of port activity if wakeup capability for > port > is disabled. > Does that get us back to the same behaviour as in 3.2? If thats the best we can do, then I guess that'll have to do. Will similar problems exist when in DMA-mode (as we I set the DMA flag it seemed to work ok)? It does seem a little strange from my naive point of view: surely what peripherals/pins are used to wake the device from suspend should not affect how the device chooses to enable/disable clocks/power-domains to save power when running? Cheers, Joe > -- > Thanks, > 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 -- 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-02 12:37 ` Joe Woodward @ 2012-04-03 6:56 ` Govindraj 0 siblings, 0 replies; 40+ messages in thread From: Govindraj @ 2012-04-03 6:56 UTC (permalink / raw) To: Joe Woodward Cc: Raja, Govindraj, Kevin Hilman, Paul Walmsley, linux-omap, Felipe Balbi, neilb On Mon, Apr 2, 2012 at 6:07 PM, Joe Woodward <jw@terrafix.co.uk> wrote: > > > -----Original Message----- > From: "Raja, Govindraj" <govindraj.raja@ti.com> > To: Joe Woodward <jw@terrafix.co.uk>, Kevin Hilman <khilman@ti.com> > Cc: Paul Walmsley <paul@pwsan.com>, linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de > Date: Mon, 2 Apr 2012 16:13:13 +0530 > Subject: Re: Suspend broken on 3.3? > >> On Fri, Mar 30, 2012 at 5:54 PM, Raja, Govindraj >> <govindraj.raja@ti.com> wrote: >> > On Fri, Mar 30, 2012 at 4:34 PM, Joe Woodward <jw@terrafix.co.uk> >> wrote: >> >> >> >> >> >> -----Original Message----- >> >> From: "Raja, Govindraj" <govindraj.raja@ti.com> >> >> To: Joe Woodward <jw@terrafix.co.uk> >> >> Cc: Paul Walmsley <paul@pwsan.com>, Kevin Hilman <khilman@ti.com>, >> linux-omap@vger.kernel.org, Felipe Balbi <balbi@ti.com>, neilb@suse.de >> >> Date: Fri, 30 Mar 2012 15:45:19 +0530 >> >> Subject: Re: Suspend broken on 3.3? >> >> >> >>> On Fri, Mar 30, 2012 at 2:56 PM, Joe Woodward <jw@terrafix.co.uk> >> >>> wrote: >> >>> > ...[snip]... >> >>> >> >> >>> >> Could you please try attached patch and let me know if this >> solves >> >>> the >> >>> >> rx issue as well, >> >>> >> without using dma mode. >> >>> >> >> >>> > >> >>> > Right, >> >>> > >> >>> > I think we've getting closer, but still not quite there... >> >>> > >> >>> > Firstly, the patch adds an include to "iomap.h" - but this >> doesn't >> >>> exist in stock 3.3. Simply removing this include seemed to allow >> the >> >>> compile to complete >> >>> > successfully. >> >>> >> >>> Sorry I forgot to specify this is needed for latest mainline. >> >>> >> >>> > >> >>> > With this patch applied (and not in DMA mode) I now get the >> >>> following: >> >>> > - If I leave wake-from-suspend enabled for the serial port then >> it >> >>> works correctly (i.e. no lost/extra 0x00 characters). >> >>> > - If I disable wake-from-suspend for the serial port and go >> through >> >>> a suspend cycle (i.e. suspend and then wake), then the serial port >> >>> starts to misbehave as >> >>> > before. >> >>> > - If I then re-enable wake-from-suspend and go through a >> suspend >> >>> cycle it starts to work correctly again. >> >>> > >> >>> > So the problem is still that if wake-from-suspend is disabled for >> a >> >>> serial port, and a suspend cycle is performed then when woken the >> >>> serial port will not function >> >>> > correctly if operating in interrupt-mode. >> >>> >> >>> I tried it on my 3430sdp but strangely I don't see a 0x00 char read >> >>> after disabling uart wakeups >> >>> and waking up by keypad press. >> >>> >> >>> Probably as a quick try we can try doing uart_insert_char only if >> data >> >>> was read from rx fifo >> >>> (Attached patch) >> >>> >> >> >> >> Sadly the patch makes no difference. >> >> >> >> I'm suprised you aren't seeing similar effects. >> >> >> >> I've done more testing, and a simplified test case is as follows: >> >> - take a stock 3.3 kernel and apply your patch to allow disabling of >> wake-from-suspend for the serial ports. >> >> - disable wake-from-suspend for the console tty: >> >> echo disable > /sys/devices/platform/omap/omap_uart.2/power/wakeup >> >> - perform a suspend (you'll need a button or something to wake you >> now that the console wont). >> >> echo mem > /sys/power/state >> >> >> >> At this point the console is noticable/painfully slow. No characters >> are lost, but it's obvious something isn't right! >> > >> > >> > Yes I see this behavior with console now it becomes very slow after >> we >> > disable module level wakeups. >> > >> > One difference to note is : >> > >> > in 3.2 => full system PM is prevented in idle path if uart is active >> > i.e, MPU will not hit retention >> > >> > in 3.3 => MPU will hit retention independently of uart is active or >> not. >> > I think the way to get MPU qos for uart port >> activity >> > while in irq mode is to use PM_WKEN1 >> > reg settings, meaning enabling module level wakeup >> event >> > generation. Disabling it is causing this >> > slow response. >> > >> > Checking if we can workaround this scenario. >> >> As of now what I can think of is to update qos reqest to prevent MPU >> from transitioning in case of port activity if wakeup capability for >> port >> is disabled. >> > > Does that get us back to the same behaviour as in 3.2? If thats the best we can do, then I guess that'll have to do. > yes, only if we are in non-dma mode with wakeup disabled. > Will similar problems exist when in DMA-mode (as we I set the DMA flag it seemed to work ok)? In dma mode we might not need MPU for uart fifo ops and while in irq mode we need MPU for fifo ops, while mpu is hitting low power states way to get mpu qos for uart ops is by generating module level wakeup events, if module level wakeups are disabled and if uart does relies on mpu for fifo ops we have to prevent MPU from hitting low power states. > > It does seem a little strange from my naive point of view: surely what peripherals/pins are used to wake the device from suspend should not affect how the device > chooses to enable/disable clocks/power-domains to save power when running? > -- 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-02 10:43 ` Raja, Govindraj 2012-04-02 12:37 ` Joe Woodward @ 2012-04-04 14:56 ` Paul Walmsley 2012-04-04 16:13 ` Raja, Govindraj 1 sibling, 1 reply; 40+ messages in thread From: Paul Walmsley @ 2012-04-04 14:56 UTC (permalink / raw) To: Raja, Govindraj Cc: Joe Woodward, Kevin Hilman, linux-omap, Felipe Balbi, neilb Hello Govindraj On Mon, 2 Apr 2012, Raja, Govindraj wrote: > As of now what I can think of is to update qos reqest to prevent MPU > from transitioning in case of port activity if wakeup capability for port > is disabled. Haven't been following this thread closely, but this doesn't seem right. Why would changing the UART driver's ability to wake from suspend via the sysfs file prevent the driver from using hardware wakeups to wake from dynamic idle? - Paul ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-04 14:56 ` Paul Walmsley @ 2012-04-04 16:13 ` Raja, Govindraj 2012-04-06 0:29 ` Paul Walmsley 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-04-04 16:13 UTC (permalink / raw) To: Paul Walmsley; +Cc: Joe Woodward, Kevin Hilman, linux-omap, Felipe Balbi, neilb On Wed, Apr 4, 2012 at 8:26 PM, Paul Walmsley <paul@pwsan.com> wrote: > Hello Govindraj > > On Mon, 2 Apr 2012, Raja, Govindraj wrote: > >> As of now what I can think of is to update qos reqest to prevent MPU >> from transitioning in case of port activity if wakeup capability for port >> is disabled. > > Haven't been following this thread closely, but this doesn't seem right. > Why would changing the UART driver's ability to wake from suspend via the > sysfs file prevent the driver from using hardware wakeups to wake from > dynamic idle? IIUC wakeup capability is needed in suspend path or in cpu idle path. once uart wakeup capability is disabled from sysfs the Module level wakeup from PM_WKEN1 reg on omap3 and pad wakeup from pin mux data given will be disabled.. So after we enter suspend and wakeup from suspend using keypad press, now through pm workqueue the MPU enters retention and uart module level wakeups disabled at this point console becomes slower to respond. Now enabling wakeups from sysfs enter suspend/resume to enable wakeups and once we come back from wakeup console response is better. So disabling uart module level wake up and allowing MPU to enter retention is making console slow. -- Thanks, Govindraj.R ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-04 16:13 ` Raja, Govindraj @ 2012-04-06 0:29 ` Paul Walmsley 2012-04-06 14:21 ` Kevin Hilman 0 siblings, 1 reply; 40+ messages in thread From: Paul Walmsley @ 2012-04-06 0:29 UTC (permalink / raw) To: Raja, Govindraj, Kevin Hilman Cc: Joe Woodward, linux-omap, Felipe Balbi, neilb On Wed, 4 Apr 2012, Raja, Govindraj wrote: > On Wed, Apr 4, 2012 at 8:26 PM, Paul Walmsley <paul@pwsan.com> wrote: > > On Mon, 2 Apr 2012, Raja, Govindraj wrote: > > > >> As of now what I can think of is to update qos reqest to prevent MPU > >> from transitioning in case of port activity if wakeup capability for port > >> is disabled. > > > > Haven't been following this thread closely, but this doesn't seem right. > > Why would changing the UART driver's ability to wake from suspend via the > > sysfs file prevent the driver from using hardware wakeups to wake from > > dynamic idle? > > > IIUC wakeup capability is needed in suspend path or in cpu idle path. > > once uart wakeup capability is disabled from sysfs the Module level > wakeup from PM_WKEN1 reg on omap3 and pad wakeup from pin mux data given > will be disabled.. As far as I know, that sysfs wakeup file is not meant to disable all module-level wakeup. Kevin can correct me if I'm wrong, but I think it's only supposed to control wakeup from suspend. So in my opinion, that sysfs file should not affect dynamic module-level, or I/O ring wakeup at all. If it is intended to affect dynamic idle wakeup, then we also will need code to prevent the UART from disabling its clocks when the sysfs wakeup file is set to disabled. > So after we enter suspend and wakeup from suspend using keypad press, > now through pm workqueue the MPU enters retention and uart module level > wakeups disabled at this point console becomes slower to respond. > > Now enabling wakeups from sysfs enter suspend/resume to enable wakeups > and once we come back from wakeup console response is better. > > So disabling uart module level wake up and allowing MPU to enter retention > is making console slow. - Paul ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-06 0:29 ` Paul Walmsley @ 2012-04-06 14:21 ` Kevin Hilman 2012-04-09 11:27 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Kevin Hilman @ 2012-04-06 14:21 UTC (permalink / raw) To: Paul Walmsley Cc: Raja, Govindraj, Joe Woodward, linux-omap, Felipe Balbi, neilb Paul Walmsley <paul@pwsan.com> writes: > On Wed, 4 Apr 2012, Raja, Govindraj wrote: > >> On Wed, Apr 4, 2012 at 8:26 PM, Paul Walmsley <paul@pwsan.com> wrote: >> > On Mon, 2 Apr 2012, Raja, Govindraj wrote: >> > >> >> As of now what I can think of is to update qos reqest to prevent MPU >> >> from transitioning in case of port activity if wakeup capability for port >> >> is disabled. >> > >> > Haven't been following this thread closely, but this doesn't seem right. >> > Why would changing the UART driver's ability to wake from suspend via the >> > sysfs file prevent the driver from using hardware wakeups to wake from >> > dynamic idle? >> >> >> IIUC wakeup capability is needed in suspend path or in cpu idle path. >> >> once uart wakeup capability is disabled from sysfs the Module level >> wakeup from PM_WKEN1 reg on omap3 and pad wakeup from pin mux data given >> will be disabled.. > > As far as I know, that sysfs wakeup file is not meant to disable > all module-level wakeup. Kevin can correct me if I'm wrong, but I think > it's only supposed to control wakeup from suspend. In theory, sysfs control is meant for static suspend. ISTR though that we were using it for idle as well so there weren't unncessary UART wakeups from idle on UART activity that was not serial console. Kevin ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-06 14:21 ` Kevin Hilman @ 2012-04-09 11:27 ` Raja, Govindraj 2012-04-09 14:27 ` Kevin Hilman 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-04-09 11:27 UTC (permalink / raw) To: Kevin Hilman, Paul Walmsley; +Cc: Joe Woodward, linux-omap, Felipe Balbi, neilb Hi Kevin / Paul, On Fri, Apr 6, 2012 at 7:51 PM, Kevin Hilman <khilman@ti.com> wrote: > Paul Walmsley <paul@pwsan.com> writes: > >> On Wed, 4 Apr 2012, Raja, Govindraj wrote: >> >>> On Wed, Apr 4, 2012 at 8:26 PM, Paul Walmsley <paul@pwsan.com> wrote: >>> > On Mon, 2 Apr 2012, Raja, Govindraj wrote: >>> > >>> >> As of now what I can think of is to update qos reqest to prevent MPU >>> >> from transitioning in case of port activity if wakeup capability for port >>> >> is disabled. >>> > >>> > Haven't been following this thread closely, but this doesn't seem right. >>> > Why would changing the UART driver's ability to wake from suspend via the >>> > sysfs file prevent the driver from using hardware wakeups to wake from >>> > dynamic idle? >>> >>> >>> IIUC wakeup capability is needed in suspend path or in cpu idle path. >>> >>> once uart wakeup capability is disabled from sysfs the Module level >>> wakeup from PM_WKEN1 reg on omap3 and pad wakeup from pin mux data given >>> will be disabled.. >> >> As far as I know, that sysfs wakeup file is not meant to disable >> all module-level wakeup. Kevin can correct me if I'm wrong, but I think >> it's only supposed to control wakeup from suspend. > > In theory, sysfs control is meant for static suspend. ISTR though that > we were using it for idle as well so there weren't unncessary UART > wakeups from idle on UART activity that was not serial console. So incase of uart wakeups are disabled and uart tx/rx is requested can we prevent MPU from low power state. -- Thanks, 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-09 11:27 ` Raja, Govindraj @ 2012-04-09 14:27 ` Kevin Hilman 2012-04-09 16:01 ` Paul Walmsley 0 siblings, 1 reply; 40+ messages in thread From: Kevin Hilman @ 2012-04-09 14:27 UTC (permalink / raw) To: Raja, Govindraj Cc: Paul Walmsley, Joe Woodward, linux-omap, Felipe Balbi, neilb "Raja, Govindraj" <govindraj.raja@ti.com> writes: [...] > So incase of uart wakeups are disabled and uart tx/rx is requested > can we prevent MPU from low power state. I think that would be a mistake. The main point of disabling UART wakeups is to save power by preventing unwanted wakups on UARTs that don't need/want them. If we then leave MPU enabled because UART wakeups are disabled, that would defeat the power-saving goals of disabling wakeups in the first place. Presumably, if a user disables UART wakeups, it means either 1) that UART is not used or 2) performance is not critical. IMO, we simply need to ensure that the defaults are correct. When UARTs are initialized/enabled wakeups should be enabled by default. The user can then override this if desired, and will obviously see a performance impact. But that's what happens with wakeups disabled. Kevin ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-09 14:27 ` Kevin Hilman @ 2012-04-09 16:01 ` Paul Walmsley 2012-04-09 17:10 ` Kevin Hilman 0 siblings, 1 reply; 40+ messages in thread From: Paul Walmsley @ 2012-04-09 16:01 UTC (permalink / raw) To: Kevin Hilman Cc: Raja\, Govindraj, Joe Woodward, linux-omap, Felipe Balbi, neilb, linux-serial cc linux-serial Hi On Mon, 9 Apr 2012, Kevin Hilman wrote: > Presumably, if a user disables UART wakeups, it means either 1) that > UART is not used #1 seems easy enough to handle without the use of power/wakeup. If there are no users of the TTY, then the driver can simply not configure hardware wakeups. > or 2) performance is not critical. Can you think of a use-case for #2? > IMO, we simply need to ensure that the defaults are correct. When UARTs > are initialized/enabled wakeups should be enabled by default. The user > can then override this if desired, and will obviously see a performance > impact. But that's what happens with wakeups disabled. I don't understand why a user would ever want to disable dynamic wakeups on an in-use serial port via the sysfs power/wakeup file. (Disabling wakeups from suspend is a different matter, of course.) The OMAP UART driver needs hardware wakeups to function for FIFO drain wakeups, etc. So to me it really doesn't make sense to disable those types of wakeup events, ever. But maybe you know of some use-case that I don't? If the user just wants a transmit-only serial port, they could use the termios CREAD flag as Neil mentioned a few months ago, and the driver could disable wakeups on incoming RXD (modulo any active flow control of course). - Paul ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-09 16:01 ` Paul Walmsley @ 2012-04-09 17:10 ` Kevin Hilman 2012-04-10 9:26 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Kevin Hilman @ 2012-04-09 17:10 UTC (permalink / raw) To: Paul Walmsley Cc: Raja, Govindraj, Joe Woodward, linux-omap, Felipe Balbi, neilb, linux-serial Paul Walmsley <paul@pwsan.com> writes: [...] > I don't understand why a user would ever want to disable dynamic wakeups > on an in-use serial port via the sysfs power/wakeup file. (Disabling > wakeups from suspend is a different matter, of course.) The OMAP UART > driver needs hardware wakeups to function for FIFO drain wakeups, etc. > So to me it really doesn't make sense to disable those types of wakeup > events, ever. But maybe you know of some use-case that I don't? No, I don't have a use-case in mind. The more I try to remember why we added support to use the sysfs wakeup attribute to manage idle, the more I think we can probably drop it now. IIRC, it was added because on most boards we used to blindly initialize all the UARTs, including default wakeup settings (we still do this on many boards.) However, now that we have a real PM-aware driver for OMAP UARTs, this should all be handled from the driver itself, so the sysfs wakeup attribute should go back to only managing wakeups from suspend and wakeups during idle should always be on for in-use UARTs. Kevin ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-09 17:10 ` Kevin Hilman @ 2012-04-10 9:26 ` Raja, Govindraj 2012-04-10 18:03 ` Kevin Hilman 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-04-10 9:26 UTC (permalink / raw) To: Kevin Hilman Cc: Paul Walmsley, Joe Woodward, linux-omap, Felipe Balbi, neilb, linux-serial Hi Kevin, On Mon, Apr 9, 2012 at 10:40 PM, Kevin Hilman <khilman@ti.com> wrote: > Paul Walmsley <paul@pwsan.com> writes: > > [...] > >> I don't understand why a user would ever want to disable dynamic wakeups >> on an in-use serial port via the sysfs power/wakeup file. (Disabling >> wakeups from suspend is a different matter, of course.) The OMAP UART >> driver needs hardware wakeups to function for FIFO drain wakeups, etc. >> So to me it really doesn't make sense to disable those types of wakeup >> events, ever. But maybe you know of some use-case that I don't? > > No, I don't have a use-case in mind. > > The more I try to remember why we added support to use the sysfs wakeup > attribute to manage idle, the more I think we can probably drop it now. > IIRC, it was added because on most boards we used to blindly initialize > all the UARTs, including default wakeup settings (we still do this on > many boards.) > > However, now that we have a real PM-aware driver for OMAP UARTs, this > should all be handled from the driver itself, so the sysfs wakeup > attribute should go back to only managing wakeups from suspend and > wakeups during idle should always be on for in-use UARTs. Just to summarize on how the behavior should be IIUC if user disables uart wakeup from sysfs and does system wide suspend it should _not_ wakeup from uart. And if the system is woken up from suspend due to keypad press and uart resumes we have keep module level wakeup enabled from here. We might need some minor changes in omap-serial to have this behavior which I plan to do once we are aligned on this. -- Thanks, 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-10 9:26 ` Raja, Govindraj @ 2012-04-10 18:03 ` Kevin Hilman 2012-04-11 13:13 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Kevin Hilman @ 2012-04-10 18:03 UTC (permalink / raw) To: Raja, Govindraj Cc: Paul Walmsley, Joe Woodward, linux-omap, Felipe Balbi, neilb, linux-serial "Raja, Govindraj" <govindraj.raja@ti.com> writes: > Hi Kevin, > > On Mon, Apr 9, 2012 at 10:40 PM, Kevin Hilman <khilman@ti.com> wrote: >> Paul Walmsley <paul@pwsan.com> writes: >> >> [...] >> >>> I don't understand why a user would ever want to disable dynamic wakeups >>> on an in-use serial port via the sysfs power/wakeup file. (Disabling >>> wakeups from suspend is a different matter, of course.) The OMAP UART >>> driver needs hardware wakeups to function for FIFO drain wakeups, etc. >>> So to me it really doesn't make sense to disable those types of wakeup >>> events, ever. But maybe you know of some use-case that I don't? >> >> No, I don't have a use-case in mind. >> >> The more I try to remember why we added support to use the sysfs wakeup >> attribute to manage idle, the more I think we can probably drop it now. >> IIRC, it was added because on most boards we used to blindly initialize >> all the UARTs, including default wakeup settings (we still do this on >> many boards.) >> >> However, now that we have a real PM-aware driver for OMAP UARTs, this >> should all be handled from the driver itself, so the sysfs wakeup >> attribute should go back to only managing wakeups from suspend and >> wakeups during idle should always be on for in-use UARTs. > > Just to summarize on how the behavior should be IIUC if user disables uart > wakeup from sysfs and does system wide suspend it should _not_ wakeup > from uart. Correct. > And if the system is woken up from suspend due to keypad press and > uart resumes we have keep module level wakeup enabled from here. Keypad press, or any other wakeup source, yes. Basically, UART wakeups (module and IO) should be enabled all the time, *except* when suspending and wakeups were disabled by sysfs control. > We might need some minor changes in omap-serial to have this behavior > which I plan to do once we are aligned on this. Sure, this changes previous behavior based on assumptions that are no longer true (namely, UARTs only disabled in idle path), so I would expect some changes. 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-10 18:03 ` Kevin Hilman @ 2012-04-11 13:13 ` Raja, Govindraj 2012-04-11 19:34 ` Paul Walmsley 0 siblings, 1 reply; 40+ messages in thread From: Raja, Govindraj @ 2012-04-11 13:13 UTC (permalink / raw) To: Kevin Hilman Cc: Paul Walmsley, Joe Woodward, linux-omap, Felipe Balbi, neilb, linux-serial On Tue, Apr 10, 2012 at 11:33 PM, Kevin Hilman <khilman@ti.com> wrote: > "Raja, Govindraj" <govindraj.raja@ti.com> writes: > >> Hi Kevin, >> >> On Mon, Apr 9, 2012 at 10:40 PM, Kevin Hilman <khilman@ti.com> wrote: >>> Paul Walmsley <paul@pwsan.com> writes: [...] >> >> Just to summarize on how the behavior should be IIUC if user disables uart >> wakeup from sysfs and does system wide suspend it should _not_ wakeup >> from uart. > > Correct. > >> And if the system is woken up from suspend due to keypad press and >> uart resumes we have keep module level wakeup enabled from here. > > Keypad press, or any other wakeup source, yes. > > Basically, UART wakeups (module and IO) should be enabled all the time, > *except* when suspending and wakeups were disabled by sysfs control. > Here is the patch [1] to do the same. Tested on beagle-XM with retention and off mode in suspend path and idle path by disabling/enabling the uart wakeups from sysfs for the console. -- Thanks, Govindraj.R [1]: >From 4e2502015e8b69d3a5047ae9f92820e4833e6d74 Mon Sep 17 00:00:00 2001 From: "Govindraj.R" <govindraj.raja@ti.com> Date: Tue, 27 Mar 2012 18:55:00 +0530 Subject: [PATCH] OMAP2+: UART: Correct the module level wakeup enable/disable mechanism The commit (62f3ec5 ARM: OMAP2+: UART: Add wakeup mechanism for omap-uarts) removed module level wakeup enable/disable mechanism and retained only the pad wakeup handling. On 24xx/34xx/36xx Module level wakeup events are enabled/disabled using PM_WKEN1_CORE/PM_WKEN_PER regs. The module level wakeups are enabled by default from bootloader, however the wakeups can be enabled/disabled using sysfs entry echo disabled > /sys/devices/platform/omap/omap_uart.X/power/wakeup [X=0,1,2,3] Since module level wakeups were left enabled from bootup and when wakeups were disabled from sysfs uart module level wakeups were still happening as they were not getting disabled. The wakeup can be left enabled by default and should be disabled only when disabled from sysfs and thus prevent system from uart wakeup in suspend path. However in idle path the wakeup can be enabled and thus uart can wakeup after gating of uart functional clocks. Thanks to Kevin Hilman <khilman@ti.com> for suggesting this. Discussion References: http://www.spinics.net/lists/linux-omap/msg67764.html http://www.spinics.net/lists/linux-omap/msg67838.html Signed-off-by: Govindraj.R <govindraj.raja@ti.com> --- arch/arm/mach-omap2/serial.c | 88 +++++++++++++++++++++++++++++++++++++- drivers/tty/serial/omap-serial.c | 30 +++++-------- 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c index 0cdd359..9312d6b 100644 --- a/arch/arm/mach-omap2/serial.c +++ b/arch/arm/mach-omap2/serial.c @@ -41,6 +41,7 @@ #include "prm-regbits-34xx.h" #include "control.h" #include "mux.h" +#include "iomap.h" /* * NOTE: By default the serial auto_suspend timeout is disabled as it causes @@ -55,6 +56,10 @@ struct omap_uart_state { int num; + void __iomem *wk_st; + void __iomem *wk_en; + u32 wk_mask; + struct list_head node; struct omap_hwmod *oh; }; @@ -80,17 +85,46 @@ static struct omap_uart_port_info omap_serial_default_info[] __initdata = { }; #ifdef CONFIG_PM + +static void omap_uart_disable_module_wakeup(struct omap_uart_state *uart) +{ + /* Clear wake-enable bit */ + if (uart->wk_en && uart->wk_mask) { + u32 v = __raw_readl(uart->wk_en); + v &= ~uart->wk_mask; + __raw_writel(v, uart->wk_en); + } +} + +static void omap_uart_enable_module_wakeup(struct omap_uart_state *uart) +{ + /* Set wake-enable bit */ + if (uart->wk_en && uart->wk_mask) { + u32 v = __raw_readl(uart->wk_en); + v |= uart->wk_mask; + __raw_writel(v, uart->wk_en); + } +} + static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable) { struct omap_device *od = to_omap_device(pdev); + struct omap_uart_state *uart; if (!od) return; - if (enable) + list_for_each_entry(uart, &uart_list, node) + if (pdev->id == uart->num) + break; + + if (enable) { + omap_uart_enable_module_wakeup(uart); omap_hwmod_enable_wakeup(od->hwmods[0]); - else + } else { + omap_uart_disable_module_wakeup(uart); omap_hwmod_disable_wakeup(od->hwmods[0]); + } } /* @@ -112,7 +146,56 @@ static void omap_uart_set_smartidle(struct platform_device *pdev) omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_SMART); } +static void omap_uart_idle_init(struct omap_uart_state *uart) +{ + if (cpu_is_omap34xx() && !cpu_is_ti816x()) { + u32 mod = (uart->num == 2) ? OMAP3430_PER_MOD : CORE_MOD; + + uart->wk_en = OMAP34XX_PRM_REGADDR(mod, PM_WKEN1); + uart->wk_st = OMAP34XX_PRM_REGADDR(mod, PM_WKST1); + switch (uart->num) { + case 0: + uart->wk_mask = OMAP3430_ST_UART1_MASK; + break; + case 1: + uart->wk_mask = OMAP3430_ST_UART2_MASK; + break; + case 2: + uart->wk_mask = OMAP3430_ST_UART3_MASK; + break; + case 3: + uart->wk_mask = OMAP3630_ST_UART4_MASK; + break; + } + } else if (cpu_is_omap24xx()) { + + if (cpu_is_omap2430()) { + uart->wk_en = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKEN1); + uart->wk_st = OMAP2430_PRM_REGADDR(CORE_MOD, PM_WKST1); + } else if (cpu_is_omap2420()) { + uart->wk_en = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKEN1); + uart->wk_st = OMAP2420_PRM_REGADDR(CORE_MOD, PM_WKST1); + } + switch (uart->num) { + case 0: + uart->wk_mask = OMAP24XX_ST_UART1_MASK; + break; + case 1: + uart->wk_mask = OMAP24XX_ST_UART2_MASK; + break; + case 2: + uart->wk_mask = OMAP24XX_ST_UART3_MASK; + break; + } + } else { + uart->wk_en = 0; + uart->wk_st = 0; + uart->wk_mask = 0; + } +} + #else +static void omap_uart_idle_init(struct omap_uart_state *uart) {} static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable) {} static void omap_uart_set_noidle(struct platform_device *pdev) {} @@ -343,6 +426,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata, oh = uart->oh; name = DRIVER_NAME; + omap_uart_idle_init(uart); omap_up.dma_enabled = info->dma_enabled; omap_up.uartclk = OMAP24XX_BASE_BAUD * 16; omap_up.flags = UPF_BOOT_AUTOCONF; diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 0121486..3dec1cf 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -930,13 +930,6 @@ serial_omap_pm(struct uart_port *port, unsigned int state, serial_out(up, UART_EFR, efr); serial_out(up, UART_LCR, 0); - if (!device_may_wakeup(&up->pdev->dev)) { - if (!state) - pm_runtime_forbid(&up->pdev->dev); - else - pm_runtime_allow(&up->pdev->dev); - } - pm_runtime_put(&up->pdev->dev); } @@ -1184,10 +1177,16 @@ static struct uart_driver serial_omap_reg = { static int serial_omap_suspend(struct device *dev) { struct uart_omap_port *up = dev_get_drvdata(dev); + struct omap_uart_port_info *pdata = dev->platform_data; if (up) { uart_suspend_port(&serial_omap_reg, &up->port); flush_work_sync(&up->qos_work); + + if (!device_may_wakeup(dev)) { + pdata->enable_wakeup(up->pdev, false); + up->wakeups_enabled = false; + } } return 0; @@ -1585,18 +1584,6 @@ static int serial_omap_runtime_suspend(struct device *dev) if (pdata->get_context_loss_count) up->context_loss_cnt = pdata->get_context_loss_count(dev); - if (device_may_wakeup(dev)) { - if (!up->wakeups_enabled) { - pdata->enable_wakeup(up->pdev, true); - up->wakeups_enabled = true; - } - } else { - if (up->wakeups_enabled) { - pdata->enable_wakeup(up->pdev, false); - up->wakeups_enabled = false; - } - } - /* Errata i291 */ if (up->use_dma && pdata->set_forceidle && (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE)) @@ -1621,6 +1608,11 @@ static int serial_omap_runtime_resume(struct device *dev) serial_omap_restore_context(up); } + if (!up->wakeups_enabled) { + pdata->enable_wakeup(up->pdev, true); + up->wakeups_enabled = true; + } + /* Errata i291 */ if (up->use_dma && pdata->set_noidle && (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE)) -- 1.7.9 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-11 13:13 ` Raja, Govindraj @ 2012-04-11 19:34 ` Paul Walmsley 2012-04-12 11:51 ` Raja, Govindraj 0 siblings, 1 reply; 40+ messages in thread From: Paul Walmsley @ 2012-04-11 19:34 UTC (permalink / raw) To: Raja, Govindraj Cc: Kevin Hilman, Joe Woodward, linux-omap, Felipe Balbi, neilb, linux-serial Hi a few brief comments based on a quick scan: On Wed, 11 Apr 2012, Raja, Govindraj wrote: > Here is the patch [1] to do the same. - I don't see where it affects I/O wakeups for the UART. If I/O wakeups are still set on the UART pads, won't that still wake the chip up from suspend, even if module-level wakeups are disabled? - The UART driver and integration code should not be reading from or writing to registers outside the UART IP block. PRM register reads and writes belong in the PRM code, which should then export a higher-level interface to the calling code. This is because, aside from making the code easier to read and debug, we're trying to move the PRM and CM code to drivers/. - The code to change the PM_WKEN* and test the PM_WKST* bits should probably be called from omap_hwmod_{enable,disable}_wakeup(), not the UART code directly. The UART code shouldn't need to care about the hardware details; it should just ask the integration layer to enable or disable module-level wakeups. As you work on these changes, please split them up into several different topic series - one for the PRM changes, one for hwmod code/data changes, and one for the UART driver/integration changes. Just note the dependencies in the series description E-mails. That way, we can avoid merge conflicts. - Paul ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-04-11 19:34 ` Paul Walmsley @ 2012-04-12 11:51 ` Raja, Govindraj 0 siblings, 0 replies; 40+ messages in thread From: Raja, Govindraj @ 2012-04-12 11:51 UTC (permalink / raw) To: Paul Walmsley Cc: Kevin Hilman, Joe Woodward, linux-omap, Felipe Balbi, neilb, linux-serial On Thu, Apr 12, 2012 at 1:04 AM, Paul Walmsley <paul@pwsan.com> wrote: > Hi > > a few brief comments based on a quick scan: > > On Wed, 11 Apr 2012, Raja, Govindraj wrote: > >> Here is the patch [1] to do the same. > > - I don't see where it affects I/O wakeups for the UART. If I/O wakeups > are still set on the UART pads, won't that still wake the chip up from > suspend, even if module-level wakeups are disabled? pdata->enable_wakeup => omap_uart_enable_wakeup was disabling both module level and pad wakeup. omap_uart_enable_wakeup => has enabling/disabling both module level and pad wakeup together. > > - The UART driver and integration code should not be reading from or > writing to registers outside the UART IP block. PRM register reads and > writes belong in the PRM code, which should then export a higher-level > interface to the calling code. This is because, aside from making the > code easier to read and debug, we're trying to move the PRM and CM code to > drivers/. okay. > > - The code to change the PM_WKEN* and test the PM_WKST* bits should > probably be called from omap_hwmod_{enable,disable}_wakeup(), not the UART > code directly. The UART code shouldn't need to care about the hardware > details; it should just ask the integration layer to enable or disable > module-level wakeups. To implement this I plan to extend the omap_hwmod_omap2_prcm structure like this: (unaligned tmp code snippet) diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h index 8070145..5c7711b 100644 --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h @@ -343,6 +343,8 @@ struct omap_hwmod_class_sysconfig { * @idlest_reg_id: IDLEST register ID (e.g., 3 for CM_IDLEST3) * @idlest_idle_bit: register bit shift for CM_IDLEST slave idle bit * @idlest_stdby_bit: register bit shift for CM_IDLEST master standby bit + * @module_wakeup_offs: PRCM register offset for PM_WKEN + * @module_wakeup_bit: regiter bit mask for PM_WKEN * * @prcm_reg_id and @module_bit are specific to the AUTOIDLE, WKST, * WKEN, GRPSEL registers. In an ideal world, no extra information @@ -357,6 +359,8 @@ struct omap_hwmod_omap2_prcm { u8 idlest_reg_id; u8 idlest_idle_bit; u8 idlest_stdby_bit; + s16 module_wakeup_offs; + u32 module_wakeup_bit; }; > > As you work on these changes, please split them up into several different > topic series - one for the PRM changes, one for hwmod code/data changes, > and one for the UART driver/integration changes. Just note the > dependencies in the series description E-mails. That way, we can avoid > merge conflicts. > Yes fine. Since most changes will be on /mach-omap2/omap_hwmod*.c Do you prefer the patches to be on any particular tree/branch where hwmod fixes are already queued. -- Thanks, 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 ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-27 0:34 ` Kevin Hilman 2012-03-27 13:53 ` Raja, Govindraj @ 2012-03-27 14:39 ` Joe Woodward 2012-03-27 21:28 ` Kevin Hilman 1 sibling, 1 reply; 40+ messages in thread From: Joe Woodward @ 2012-03-27 14:39 UTC (permalink / raw) To: Kevin Hilman; +Cc: linux-omap, govindraj.raja, Felipe Balbi ...snip... > Just to confirm: did the above work for you before v3.3? > I've checked and v3.2 works correctly: echo "enabled" > /sys/devices/platform/omap/omap_uart.2/power/wakeup => device wakes from console presses echo "disabled" > /sys/devices/platform/omap/omap_uart.2/power/wakeup => device does not wake from console presses Thanks for looking in to this! Cheers, Joe > > Could you test if this is also the case your end? > > Yes, I get the same behavior, which is indeed broken. > > Govindraj, can you look into this? > > A quick glance suggests that disabling wakeups via the sysfs file is > only disabling runtime PM, but not actually disabling wakups at the > module-level or at the IO ring. > > 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 ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: Suspend broken on 3.3? 2012-03-27 14:39 ` Joe Woodward @ 2012-03-27 21:28 ` Kevin Hilman 0 siblings, 0 replies; 40+ messages in thread From: Kevin Hilman @ 2012-03-27 21:28 UTC (permalink / raw) To: Joe Woodward; +Cc: linux-omap, govindraj.raja, Felipe Balbi "Joe Woodward" <jw@terrafix.co.uk> writes: > ...snip... > >> Just to confirm: did the above work for you before v3.3? >> > > I've checked and v3.2 works correctly: > > echo "enabled" > /sys/devices/platform/omap/omap_uart.2/power/wakeup => device wakes from console presses > echo "disabled" > /sys/devices/platform/omap/omap_uart.2/power/wakeup => device does not wake from console presses OK, that's what I suspected. Thanks for confirming. Kevin ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2012-04-12 11:51 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-03-22 11:09 Suspend broken on 3.3? Joe Woodward 2012-03-22 17:33 ` Kevin Hilman 2012-03-26 13:41 ` Joe Woodward 2012-03-27 0:34 ` Kevin Hilman 2012-03-27 13:53 ` Raja, Govindraj 2012-03-27 21:37 ` Kevin Hilman 2012-03-28 10:59 ` Raja, Govindraj 2012-03-28 15:38 ` Joe Woodward 2012-03-28 17:46 ` Kevin Hilman 2012-03-29 8:35 ` Joe Woodward 2012-03-29 9:14 ` Shubhrajyoti Datta 2012-03-29 9:46 ` Joe Woodward 2012-03-29 10:26 ` Paul Walmsley 2012-03-29 11:27 ` Joe Woodward 2012-03-29 11:40 ` Joe Woodward 2012-03-29 14:29 ` Raja, Govindraj 2012-03-30 7:53 ` Joe Woodward 2012-03-30 8:46 ` Raja, Govindraj 2012-03-30 9:26 ` Joe Woodward 2012-03-30 10:15 ` Raja, Govindraj 2012-03-30 11:04 ` Joe Woodward 2012-03-30 12:24 ` Raja, Govindraj 2012-04-02 10:43 ` Raja, Govindraj 2012-04-02 12:37 ` Joe Woodward 2012-04-03 6:56 ` Govindraj 2012-04-04 14:56 ` Paul Walmsley 2012-04-04 16:13 ` Raja, Govindraj 2012-04-06 0:29 ` Paul Walmsley 2012-04-06 14:21 ` Kevin Hilman 2012-04-09 11:27 ` Raja, Govindraj 2012-04-09 14:27 ` Kevin Hilman 2012-04-09 16:01 ` Paul Walmsley 2012-04-09 17:10 ` Kevin Hilman 2012-04-10 9:26 ` Raja, Govindraj 2012-04-10 18:03 ` Kevin Hilman 2012-04-11 13:13 ` Raja, Govindraj 2012-04-11 19:34 ` Paul Walmsley 2012-04-12 11:51 ` Raja, Govindraj 2012-03-27 14:39 ` Joe Woodward 2012-03-27 21:28 ` Kevin Hilman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.