[reply behalf on Taipin]

Hi Baolin,

On Sat, 1 Aug 2020 at 07:59, Baolin Wang [off-list ref] wrote:

On Wed, Jul 29, 2020 at 5:35 PM Chunyan Zhang [off-list ref] wrote:

quoted

From: Taiping Lai <redacted>

The raw interrupt status of GPIO maybe set before the interrupt is enabled,
which would trigger the interrupt event once enabled it from user side.
This is the case for edge interrupts only. Adding a clear operation when
setting interrupt type can avoid that.

Fixes: 9a3821c2bb47 ("gpio: Add GPIO driver for Spreadtrum SC9860 platform")
Signed-off-by: Taiping Lai <redacted>
Signed-off-by: Chunyan Zhang <redacted>
---
 drivers/gpio/gpio-sprd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpio-sprd.c b/drivers/gpio/gpio-sprd.c
index d7314d39ab65..36ea8a3bd451 100644
--- a/drivers/gpio/gpio-sprd.c
+++ b/drivers/gpio/gpio-sprd.c

@@ -149,17 +149,20 @@ static int sprd_gpio_irq_set_type(struct irq_data *data,
                sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 0);
                sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 0);
                sprd_gpio_update(chip, offset, SPRD_GPIO_IEV, 1);
+               sprd_gpio_update(chip, offset, SPRD_GPIO_IC, 1);

I think you should move this abonormal interrupt clearing operation to
sprd_gpio_request(), when users request a GPIO.

We have a few considerations:
1) Like described in the commit message, the problem this patch solves
is for edge interrupt only; The interrupt requested by user is
IRQ_TYPE_LEVEL_HIGH as default, so clearing interrupt when request is
useless.
2) We can set interrupt type to edge when request, and following up
with clearing it, but the problem is still there once users set the
interrupt type to level trggier.
3) We can add a clear operation after each time of setting interrupt
enable bit, but it is redundant for level trigger interrupt.

Therefore, adding a clear operation when setting interrupt type seems
the best solutions which I can think out so far.

quoted

                irq_set_handler_locked(data, handle_edge_irq);
                break;
        case IRQ_TYPE_EDGE_FALLING:
                sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 0);
                sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 0);
                sprd_gpio_update(chip, offset, SPRD_GPIO_IEV, 0);
+               sprd_gpio_update(chip, offset, SPRD_GPIO_IC, 1);
                irq_set_handler_locked(data, handle_edge_irq);
                break;
        case IRQ_TYPE_EDGE_BOTH:
                sprd_gpio_update(chip, offset, SPRD_GPIO_IS, 0);
                sprd_gpio_update(chip, offset, SPRD_GPIO_IBE, 1);
+               sprd_gpio_update(chip, offset, SPRD_GPIO_IC, 1);
                irq_set_handler_locked(data, handle_edge_irq);
                break;
        case IRQ_TYPE_LEVEL_HIGH:
--
2.20.1

--
Baolin Wang