Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 6/6] Input: snvs_pwrkey - only IRQ_HANDLED for our own events | From | André Draszik <> | Date | Wed, 26 Feb 2020 17:23:21 +0000 |
| |
On Wed, 2020-02-26 at 01:15 +0000, Robin Gong wrote: > On 2020/02/26 André Draszik <git@andred.net> wrote: > > The snvs_pwrkey shares the SNVS LPSR status register with the snvs_rtc. > > > > This driver here should only return IRQ_HANDLED if the status register > > indicates that the event we're handling in the irq handler was genuinely > > intended for this driver. Otheriwse the interrupt subsystem will assume the > > interrupt was handled successfully even though it wasn't at all. > > > > Signed-off-by: André Draszik <git@andred.net> > > Cc: "Horia Geantă" <horia.geanta@nxp.com> > > Cc: Aymen Sghaier <aymen.sghaier@nxp.com> > > Cc: Herbert Xu <herbert@gondor.apana.org.au> > > Cc: "David S. Miller" <davem@davemloft.net> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Shawn Guo <shawnguo@kernel.org> > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > > Cc: Fabio Estevam <festevam@gmail.com> > > Cc: NXP Linux Team <linux-imx@nxp.com> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Cc: Anson Huang <Anson.Huang@nxp.com> > > Cc: Robin Gong <yibin.gong@nxp.com> > > Cc: linux-crypto@vger.kernel.org > > Cc: devicetree@vger.kernel.org > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-input@vger.kernel.org > > > > --- > > v2: > > * no changes > > --- > > drivers/input/keyboard/snvs_pwrkey.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/input/keyboard/snvs_pwrkey.c > > b/drivers/input/keyboard/snvs_pwrkey.c > > index 382d2ae82c9b..980867886b34 100644 > > --- a/drivers/input/keyboard/snvs_pwrkey.c > > +++ b/drivers/input/keyboard/snvs_pwrkey.c > > @@ -82,7 +82,9 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void > > *dev_id) > > clk_enable(pdata->clk); > > > > regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status); > > - if (lp_status & SNVS_LPSR_SPO) { > > + lp_status &= SNVS_LPSR_SPO; > > + > > + if (lp_status) { > > if (pdata->minor_rev == 0) { > > /* > > * The first generation i.MX[6|7] SoCs only send an @@ -98,14 > > +100,14 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void > > *dev_id) > > mod_timer(&pdata->check_timer, > > jiffies + msecs_to_jiffies(DEBOUNCE_TIME)); > > } > > - } > > > > - /* clear SPO status */ > > - regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO); > > + /* clear SPO status */ > > + regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO); > But irq storm will come in once there is other interrupt triggered as unexpected, > although I never met it before. Could we drop this patch now? Others are ok for me.
I don't have strong feelings about this patch, but this bit merely changes behaviour to clear SP0 if SP0 was in fact != 0 in the first place, whereas before SP0 was always cleared, even if it was == 0 anyway. Seems more logical in my eyes.
> Reviewed-by: Robin Gong <yibin.gong@nxp> > > + } > > > > clk_disable(pdata->clk); > > > > - return IRQ_HANDLED; > > + return lp_status ? IRQ_HANDLED : IRQ_NONE;
If you're talking about this part, the rtc-snvs driver does the same in its interrupt handler. In other words, this driver here could prevent the rtc-snvs driver from seeing its events.
Cheers, Andre'
> > } > > > > static void imx_snvs_pwrkey_act(void *pdata) > > -- > > 2.23.0.rc1
| |