lkml.org 
[lkml]   [2013]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] rtc: rtc-twl: ensure IRQ is wakeup enabled
On 06/04/2013 08:46 PM, Kevin Hilman wrote:
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>
>> Hi Kevin,
>>
>> On 06/01/2013 01:37 AM, Kevin Hilman wrote:
>>> Currently, the RTC IRQ is never wakeup-enabled so is not capable of
>>> bringing the system out of suspend.
>>>
>>> On OMAP platforms, we have gotten by without this because the TWL RTC
>>> is on an I2C-connected chip which is capable of waking up the OMAP via
>>> the IO ring when the OMAP is in low-power states.
>>>
>>> However, if the OMAP suspends without hitting the low-power states
>>> (and the IO ring is not enabled), RTC wakeups will not work because
>>> the IRQ is not wakeup enabled.
>> As I understand, IRQ wake up capabilities are set/clear simultaneously with
>> IRQ unmasking/masking on OMAP4+ in omap-wakeupgen.c.
>> So, it should work without this patch on OMAP4+.
> It might work on OMAP4 for wakeup from suspend, but without properly
> declaring the IRQ as a wakeup source, it will not abort suspend if the
> RTC fires during the suspend process. To abort suspend, the IRQ must be
> declared as a wakeup IRQ.
>
>> But if TWL is used on non OMAP4+ platform then it is needed. (OMAP3:
>> I haven't found the place where IRQ wakeup capabilities are
>> configured, would be appreciate if you can point me on)
> IRQ wakeup is a genirq feature that trickles into the irq_chip (in OMAP3
> case, it's the twl4030 irq_chip.)
>
> On OMAP3, as mentioned in the changelog, RTC wake has been working fine
> without this because we default to CORE retention, so wakeup happens via
> the IO ring. However, if you prevent retention during suspend, then
> this IRQ will not wake the system.
>
> Kevin
>
>>> To fix, ensure the RTC IRQ is wakeup enabled whenever the RTC alarm is
>>> set.
>>>
>>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>>> ---
>>> drivers/rtc/rtc-twl.c | 16 ++++++++++++++--
>>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
>>> index 8751a52..bbda0fd 100644
>>> --- a/drivers/rtc/rtc-twl.c
>>> +++ b/drivers/rtc/rtc-twl.c
>>> @@ -213,12 +213,24 @@ static int mask_rtc_irq_bit(unsigned char bit)
>>> static int twl_rtc_alarm_irq_enable(struct device *dev, unsigned
>>> enabled)
>>> {
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + int irq = platform_get_irq(pdev, 0);
>>> + static bool twl_rtc_wake_enabled;
>>> int ret;
>>> - if (enabled)
>>> + if (enabled) {
>>> ret = set_rtc_irq_bit(BIT_RTC_INTERRUPTS_REG_IT_ALARM_M);
>>> - else
>>> + if (device_can_wakeup(dev) && !twl_rtc_wake_enabled) {
>>> + enable_irq_wake(irq);
>>> + twl_rtc_wake_enabled = true;
>>> + }
>>> + } else {
>>> ret = mask_rtc_irq_bit(BIT_RTC_INTERRUPTS_REG_IT_ALARM_M);
>>> + if (twl_rtc_wake_enabled) {
>>> + disable_irq_wake(irq);
>>> + twl_rtc_wake_enabled = false;
>>> + }
>>> + }
>>> return ret;
>>> }
>> twl-rtc has suspend/resume callbacks implemented, so I think it's the
>> better place
>> for this code and twl_rtc_wake_enabled can be dropped.
> In theory, that might be the better place (and that's where I put these
> at first), but unfortunately, it doesn't work that way because the
> twl6030-irq core enables/diables the parent IRQ wake feature using PM
> notifiers (which was done to avoid potential lock recursion[1].)
>
> During suspend, the notifier runs at suspend_prepare() time, which is
> well before the driver's ->suspend() method is called. The result is
> that the parents IRQ wakeup capabilies are never set.
Sorry, forget about this patch - have no questions for this patch anymore.

Thanks.

Just FYI. It seems, The suspend will never be aborted on OMAP4 by SYSN_IRQ
because of these two patches:
782baa2 mfd: Disable twl6030 IRQ during suspend
9c6079a genirq: Do not consider disabled wakeup irqs

-grygorii
>
> Kevin
>
>
> [1]
> commit ab2b9260df67e29d5bd69d989f2f84f8c2ed4238
> Author: Todd Poynor <toddpoynor@google.com>
> Date: Tue Oct 4 11:52:29 2011 +0200
>
> mfd: Fix twl6030 lockdep recursion warning on setting wake IRQs
>
> LOCKDEP explicitly sets all irq_desc locks as a single lock-class,
> causing "possible recursive locking detected" when the TWL RTC
> driver calls through enable_irq_wake to twl6030_irq_set_wake,
> which recursively calls irq_set_irq_wake. Although the
> irq_desc and lock are different, LOCKDEP treats these as
> equivalent, presumably due to problems that can be incurred
> when locking more than one irq_desc, so best to avoid this.
>
> Suspend/resume actions implemented as PM notifiers to avoid
> touch the TWL core for this.
>
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>



\
 
 \ /
  Last update: 2013-06-05 17:21    [W:0.081 / U:0.748 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site