On Thu, Jan 21, 2021 at 8:09 PM mtk15103 [off-list ref] wrote:

On Thu, 2021-01-21 at 16:55 +0800, Nicolas Boichat wrote:

quoted

On Thu, Jan 21, 2021 at 3:52 PM Hailong Fan [off-list ref] wrote:

quoted

When flipping the polarity will be generated interrupt under certain
circumstances, but GPIO external signal has not changed.
Then, mask the interrupt before polarity setting, and clear the
unexpected interrupt after trigger type setting completed.

Signed-off-by: Hailong Fan <redacted>
---
Resend since some server reject.
---
 drivers/pinctrl/mediatek/mtk-eint.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mediatek/mtk-eint.c b/drivers/pinctrl/mediatek/mtk-eint.c
index 22736f60c16c..3acda6bb401e 100644
--- a/drivers/pinctrl/mediatek/mtk-eint.c
+++ b/drivers/pinctrl/mediatek/mtk-eint.c

@@ -157,6 +157,7 @@ static void mtk_eint_ack(struct irq_data *d)
 static int mtk_eint_set_type(struct irq_data *d, unsigned int type)
 {
        struct mtk_eint *eint = irq_data_get_irq_chip_data(d);
+       unsigned int unmask;

bool?

Yes,thanks.

quoted

quoted

        u32 mask = BIT(d->hwirq & 0x1f);
        void __iomem *reg;

@@ -173,6 +174,13 @@ static int mtk_eint_set_type(struct irq_data *d, unsigned int type)
        else
                eint->dual_edge[d->hwirq] = 0;

+       if (!mtk_eint_get_mask(eint, d->hwirq)) {
+               mtk_eint_mask(d);
+               unmask = 1;
+       } else {
+               unmask = 0;
+       }
+
        if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) {
                reg = mtk_eint_get_offset(eint, d->hwirq, eint->regs->pol_clr);
                writel(mask, reg);

@@ -189,8 +197,9 @@ static int mtk_eint_set_type(struct irq_data *d, unsigned int type)
                writel(mask, reg);
        }

-       if (eint->dual_edge[d->hwirq])
-               mtk_eint_flip_edge(eint, d->hwirq);

Why are you dropping this? Aren't we at risk to miss the first edge
after mtk_eint_set_type is called?

mtk_eint_unmask() will do it.
If unmask != 1, user need to call mtk_eint_unmask() to enable the
interrupt before use it, thanks.

Makes sense, I just have one more worry:
https://elixir.bootlin.com/linux/latest/source/drivers/pinctrl/mediatek/mtk-eint.c#L122

mtk_eint_unmask unmasks the interrupt _before_ the edge is flipped,
could this cause a spurious interrupt? On any unmask operation -- not
just on mtk_eint_set_type (so this is technically another problem,
that should be fixed as a separate patch)

quoted

quoted

+       mtk_eint_ack(d);
+       if (unmask == 1)

Just `if (unmask)`

Yes,thanks.

quoted

quoted

+               mtk_eint_unmask(d);

        return 0;
 }
--
2.18.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel