From mboxrd@z Thu Jan 1 00:00:00 1970 From: kgunda@codeaurora.org Subject: Re: [PATCH V1 07/15] spmi: pmic-arb: clear the latched status of the interrupt Date: Tue, 06 Jun 2017 16:25:30 +0530 Message-ID: <7b31b7adfab62557a5b9d210f3e27014@codeaurora.org> References: <1496147943-25822-1-git-send-email-kgunda@codeaurora.org> <1496147943-25822-8-git-send-email-kgunda@codeaurora.org> <20170531220324.GH20170@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170531220324.GH20170@codeaurora.org> Sender: linux-kernel-owner@vger.kernel.org To: Stephen Boyd Cc: Abhijeet Dharmapurikar , Christophe JAILLET , Subbaraman Narayanamurthy , David Collins , linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, adharmap@quicinc.com, aghayal@qti.qualcomm.com, linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org On 2017-06-01 03:33, Stephen Boyd wrote: > On 05/30, Kiran Gunda wrote: >> From: Abhijeet Dharmapurikar >> >> 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 >> Signed-off-by: Kiran Gunda > > Reviewed-by: Stephen Boyd Thanks for reviewing and acking this.