lkml.org 
[lkml]   [2014]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [patch 09/26] arm: mmp: Remove pointless fiddling with irq internals
From
On Thu, Feb 27, 2014 at 9:37 AM, Chao Xie <xiechao.mail@gmail.com> wrote:
> On Mon, Feb 24, 2014 at 7:31 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Mon, 24 Feb 2014, Haojian Zhuang wrote:
>>
>>> On Mon, Feb 24, 2014 at 2:07 PM, Chao Xie <xiechao.mail@gmail.com> wrote:
>>> > On Mon, Feb 24, 2014 at 7:17 AM, Uwe Kleine-König
>>> > <u.kleine-koenig@pengutronix.de> wrote:
>>> >> Hi Thomas,
>>> >>
>>> >> On Sun, Feb 23, 2014 at 09:40:13PM -0000, Thomas Gleixner wrote:
>>> >>> The pm-mmp2 and pm-pxa910 power management related irq_set_wake
>>> >>> callbacks fiddle pointlessly with the irq actions for no reason except
>>> >>> for lack of understanding how the wakeup mechanism works.
>>> >>>
>>> >>> On supsend the core disables all interrupts lazily, i.e. it does not
>>> >>> mask them at the irq controller level. So any interrupt which is
>>> >>> firing during supsend will mark the corresponding interrupt line as
>>> >> s/supsend/suspend/ twice
>>> >>> pending. Just before the core powers down it checks whether there are
>>> >>> interrupts pending from interrupt lines which are marked as wakeup
>>> >>> sources and if so it aborts the resume and resends the interrupts.
>>> >> It's the suspend that is aborted, not the resume.
>>> >>
>>> >> Other than that your change looks fine.
>>> >>
>>> > For pxa910 and MMP2, wake up source only wake up the AP subsystem.
>>> > The AP subsystem includes the APMU(AP Power Mangament Unit) and cores.
>>> > Now the core is still powered down. APMU will check the interrupt
>>> > lines, and find
>>> > that there are interrupt pending, it will power on the cores.
>>> > So if the irq is disabled, even wake up source can wake up AP subsystem, but the
>>> > core is still powered down. It will not be powered up by APMU.
>>> >
>>>
>>> Yes, suspend/resume can't work if the above code is removed.
>>>
>>> Interrupt source (logic AND with interrupt mask register) is connected
>>> to MPMU as
>>> wakeup source. If the interrupt is disabled, there's no wakeup source event.
>>>
>>> And APMU is waken up by MPMU.
>>>
>>> So please don't remove the above code. We must keep these interrupt lines active
>>> to wake up the whole system.
>>
>> They are kept active at the interrupt controller level. You just
>> refuse to understand how the internals of the interrupt subsystem
>> work.
>>
> If no irq_disable callback is hooked, when do irq_disable, it will not
> actually disable
> the interrupt, it will depend on next time when the irq happens, the
> handler will first mask
> the interrupt as this interrupt never happens.
> So after system suspended, the interrupt happens, but the device
> driver will not recieve this interrupt
> because it is masked.
> It results in that the device driver miss a important interrupt which
> related to something need to be
> handled. If user application for example android has power managment
> daemon. It will find that nothing
> to handle, it will make the system enter suspend again.
>
Let me list the logic to make it easier to understand.

suspend_enter()
--> dpm_suspend_end()
--> dpm_suspend_noirq()
--> suspend_device_irqs()
--> __disable_irq()
--> set IRQS_SUSPENDED && call
irq_disable() if necessary
--> syscore_suspend()
--> check_wakeup_irqs()
If there's no pending irq in suspend process &&
IRQS_SUSPENDED is set,
then mask the irq.

Yes, we didn't implement disable_irq(). But we must implement mask_irq().

So system suspends. Then system will never be waken up by this irq any
more since
it's masked.


>> And even if you would need this flag, then fiddling with the irq desc
>> internals is a big NONO. There is a proper way to hand that in.
>>
>
> So can you suggest the proper way to handle it? Thanks.
>
>> Thanks,
>>
>> tglx
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2014-02-27 04:01    [W:0.071 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site