[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4D7A80DD.7000909@codeaurora.org>
Date: Fri, 11 Mar 2011 12:06:53 -0800
From: Abhijeet Dharmapurikar <adharmap@...eaurora.org>
To: Thomas Gleixner <tglx@...utronix.de>
CC: davidb@...eaurora.org, "David S. Miller" <davem@...emloft.net>,
Andrew Morton <akpm@...ux-foundation.org>,
Bryan Huntsman <bryanh@...eaurora.org>,
Daniel Walker <dwalker@...o99.com>,
David Collins <collinsd@...eaurora.org>,
Grant Likely <grant.likely@...retlab.ca>,
Greg Kroah-Hartman <gregkh@...e.de>,
Joe Perches <joe@...ches.com>,
Russell King <linux@....linux.org.uk>,
Samuel Ortiz <sameo@...ux.intel.com>,
Stepan Moskovchenko <stepanm@...eaurora.org>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
Linus Walleij <linus.walleij@...rricsson.com>,
linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [Qualcomm PM8921 MFD v2 2/6] mfd: pm8xxx: Add irq support
Thomas Gleixner wrote:
> On Fri, 11 Mar 2011, Abhijeet Dharmapurikar wrote:
>>>> Yes however while updating the code I noticed that I would need to keep
>>>> account of all the interrupts enabled and all the interrupts marked
>>>> wakeup.
>>>> This aids in switching to the wakeup set in the suspend callback and the
>>>> enabled set in the resume callback. I will update the resume callback to
>>>> enable the interrupts in irqs_allowed(the local state storage) in the next
>>>> patch (my current patch does not do that).
>>>>
>>>> IOW I need to keep the local state storage.
>>> Wrong. The interrupts are disabled and reenabled by the core code and
>>> not by some extra suspend/resume callbacks in your driver. The core
>>> checks those marked as IRQ_WAKE, the wake callback to the irq chip is
>>> only there if you need to set up some hardware register in order to
>>> make the wake functionality work. So again, you don't need local state
>>> as the core tracks the state for you.
>> Help me understand this, the core code calls disable on all the interrupts
>> while going to suspend. Notice that I have no disable callback, which means
>> those interrupts remain unmasked.
>>
>> The genirq code does not mask the interrupt while going to suspend, it only
>> calls disable(), which I understand should not mask the interrupt for
>> check_wakeup_irqs() to work.
>>
>> If I don't mask that accelerometer interrupts in the interrupt controller's
>> suspend() the phone will wakeup every time the user moves around, draining the
>> battery unnecessarily.
>
> That's why we mark the interrupts which can wake up from suspend with
> set_wake() so you can configure your hardware accordingly. That's how
> all other stuff works, at least how it's supposed to work.
>
> If there is no way to tell the interrupt controller which interrupts
> are wakeup sources and which are not, then working around it with
> local state and private suspend/resume functions is the WRONG answer.
>
> Simply because this kind of misdesigned hardware will creep up over
> and over and we want to handle these cases in the core. Even for a
> sinlge instance like yours solving it in the core is the right thing
> to do, because it's a ~3 lines patch to the core code to get this
> done.
~3 lines patch to the code sounds promising. Please tell me how?
--
--
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm
Innovation Center, Inc. is a member of the Code Aurora Forum.
--
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