| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87d2s0zi7v.fsf@linaro.org>
Date: Wed, 05 Jun 2013 08:32:20 -0700
From: Kevin Hilman <khilman@...aro.org>
To: Grygorii Strashko <grygorii.strashko@...com>
Cc: <rtc-linux@...glegroups.com>,
Andrew Morton <akpm@...ux-foundation.org>,
<linux-omap@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linaro-kernel@...ts.linaro.org>,
Alessandro Zummo <a.zummo@...ertech.it>,
Tony Lindgren <tony@...mide.com>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rtc: rtc-twl: ensure IRQ is wakeup enabled
Grygorii Strashko <grygorii.strashko@...com> writes:
> On 06/04/2013 08:46 PM, Kevin Hilman wrote:
>> Grygorii Strashko <grygorii.strashko@...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@...ertech.it>
>>>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>>>> Cc: Tony Lindgren <tony@...mide.com>
>>>> Signed-off-by: Kevin Hilman <khilman@...aro.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
You're right for the parent TWL IRQ, but the child interrupts (e.g. RTC)
would still abort suspend if wakeup enabled.
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists