Re: [PATCH V1 07/15] spmi: pmic-arb: clear the latched status of the interrupt

[kgunda]

On 2017-06-01 03:33, Stephen Boyd wrote:
> On 05/30, Kiran Gunda wrote:
> > From: Abhijeet Dharmapurikar <adharmap@xxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxx>
> > Signed-off-by: Kiran Gunda <kgunda@xxxxxxxxxxxxxx>
>
> Reviewed-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
Thanks for reviewing and acking this.