Message-ID: <20170531220324.GH20170@codeaurora.org>
Date:   Wed, 31 May 2017 15:03:24 -0700
From:   Stephen Boyd <sboyd@...eaurora.org>
To:     Kiran Gunda <kgunda@...eaurora.org>
Cc:     Abhijeet Dharmapurikar <adharmap@...eaurora.org>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Subbaraman Narayanamurthy <subbaram@...eaurora.org>,
        David Collins <collinsd@...eaurora.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        adharmap@...cinc.com, aghayal@....qualcomm.com
Subject: Re: [PATCH V1 07/15] spmi: pmic-arb: clear the latched status of the
 interrupt

On 05/30, Kiran Gunda wrote:
> From: Abhijeet Dharmapurikar <adharmap@...eaurora.org>
> 
> PMIC interrupts each have an internal latched status bit which is
> not visible from any register.  This status bit is set as soon as
> the conditions specified in the interrupt type and polarity
> registers are met even if the interrupt is not enabled.  When it
> is set, nothing else changes within the PMIC and no interrupt
> notification packets are sent.  If the internal latched status
> bit is set when an interrupt is enabled, then the value is
> immediately propagated into the interrupt latched status register
> and an interrupt notification packet is sent out from the PMIC
> over SPMI.
> 
> This PMIC hardware behavior can lead to a situation where the
> handler for a level triggered interrupt is called immediately
> after enable_irq() is called even though the interrupt physically
> triggered while it was disabled within the genirq framework.
> This situation takes place if the the interrupt fires twice after

Double 'the'

> calling disable_irq().  The first time it fires, the level flow
> handler will mask and disregard it.  Unfortunately, the second
> time it fires, the internal latched status bit is set within the
> PMIC and no further notification is received.

because the interrupt has been disabled.

> When enable_irq()
> is called later, the interrupt is unmasked (enabled in the PMIC)
> which results in the PMIC immediately sending an interrupt
> notification packet out over SPMI.  This breaks the semantics
> of level triggered interrupts within the genirq framework since
> they should be completely ignored while disabled.

Ok. I wonder why the hardware latches interrupts at all.

> 
> The PMIC internal latched status behavior also affects how
> interrupts are treated during suspend.  While entering suspend,
> all interrupts not specified as wakeup mode are masked.  Upon
> resume, these interrupts are unmasked.  Thus if any of the
> non-wakeup PMIC interrupts fired while the system was suspended,
> then the PMIC will send interrupt notification packets out via
> SPMI as soon as they are unmasked during resume.  This behavior
> violates genirq semantics as well since non-wakeup interrupts
> should be completely ignored during suspend.
> 
> Modify the qpnpint_irq_unmask() function so that the interrupt
> latched status clear register is written immediately before the
> interrupt enable register.  This clears the internal latched
> status bit of the interrupt so that it cannot trigger spuriously
> immediately upon being enabled.
> 
> Also, while resuming an irq, an unmask could be called even if it
> was not previously masked.  So, before writing these registers,
> check if the interrupt is already enabled within the PMIC. If it
> is, then no further register writes are required.  This
> condition check ensures that a valid latched status register bit
> is not cleared until it is properly handled.
> 
> Signed-off-by: Abhijeet Dharmapurikar <adharmap@...eaurora.org>
> Signed-off-by: Kiran Gunda <kgunda@...eaurora.org>

Reviewed-by: Stephen Boyd <sboyd@...eaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Powered by blists - more mailing lists