Re: [PATCHv6] OMAP3: Serial: Improved sleep logic

[Kevin Hilman <khilman@deeprootsystems.com>]

<Tero.Kristo@nokia.com> writes:

>  
>
>>-----Original Message-----
>>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>>Sent: 10 March, 2010 20:21
>>To: Kristo Tero (Nokia-D/Tampere)
>>Cc: linux-omap@vger.kernel.org
>>Subject: Re: [PATCHv6] OMAP3: Serial: Improved sleep logic
>>
>>Kevin Hilman <khilman@deeprootsystems.com> writes:
>>
>>> Tero Kristo <tero.kristo@nokia.com> writes:
>>>
>>>> From: Tero Kristo <tero.kristo@nokia.com>
>>>>
>>>> This patch contains following improvements:
>>>> - Only RX interrupt will now kick the sleep prevent timer
>>>> - TX fifo status is checked before disabling clocks, this 
>>will prevent
>>>>   on-going transmission to be cut
>>>> - Smartidle is now enabled/disabled only while switching 
>>clocks, as having
>>>>   smartidle enabled while RX/TX prevents any wakeups from 
>>being received
>>>>   from UART module
>>>> - Added workqueue for wakeup checks, as jiffy timer access 
>>within the
>>>>   idle loop results into skewed timers as jiffy timers are stopped
>>>> - Added garbage_timer for ignoring the first character 
>>received during
>>>>   the first tick after clock enable, this prevents garbage 
>>characters to be
>>>>   received in low sleep states
>>>> - omap_uart_enable_irqs() changed to use enable_irq / 
>>disable_irq instead
>>>>   of request / free. Using request/free changes the 
>>behavior after first
>>>>   suspend due to reversed interrupt handler ordering
>>>>
>>>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>>>
>>> Thanks Tero.  This version looks good.
>>>
>>> Adding to pm-fixes queue for 2.6.34-rcX after minor change below...
>>>
>>
>>There's still something slightly strange going on here...
>>
>>I noticed via powertop that the garbage timer is now one of the top
>>reasons for wakeup in an idle system.  Seems like the garbage timer
>>is firing when it shouldn't be, and triggering unnecessary wakeups
>>
>>Based on powertop stats, the garbage timer is firing about 3x more
>>often than the GPtimer that should be waking the system.
>
> You get one timer expire each wakeup cycle, but the system actually fires its own timer for each UART, thus you get 3x. It is possible to optimize the timer a bit by only firing it if we have a wakeup from UART, but this probably causes occasional garbage to the console, if a wakeup from some other source than UART and UART RX happen at the same time. Might be the lesser of two evils though.
>
> I can experiment with this change a bit and see how it behaves.
>

OK, the 3x makes sense, but the garbage timer should never be the
cause of a wakeup.

Maybe you also need to be sure that the garbage timer is disabled
before clocks are disabled so it doesn't fire and cause a wakeup.

Kevin