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:3.184 / U:0.220 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site