Re: [Qualcomm PM8921 MFD v2 2/6] mfd: pm8xxx: Add irq support

[Abhijeet Dharmapurikar]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/