On Thu, Feb 27, 2014 at 9:37 AM, Chao Xie <xiechao.m...@gmail.com> wrote: > On Mon, Feb 24, 2014 at 7:31 PM, Thomas Gleixner <t...@linutronix.de> wrote: >> On Mon, 24 Feb 2014, Haojian Zhuang wrote: >> >>> On Mon, Feb 24, 2014 at 2:07 PM, Chao Xie <xiechao.m...@gmail.com> wrote: >>> > On Mon, Feb 24, 2014 at 7:17 AM, Uwe Kleine-König >>> > <u.kleine-koe...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/