On Wed, 15 Jul 2020 at 14:06, Jeremy Kerr <jk@xxxxxxxxxxxxxxxxxxxx> wrote: > > Currently, the IRQ setup for the SGPIO driver enables all interrupts for > dual-edge trigger mode. Since the default handler is handle_bad_irq, any > state change on input GPIOs will trigger bad IRQ warnings. > > This change applies sensible (disabled) IRQ defaults. > > Signed-off-by: Jeremy Kerr <jk@xxxxxxxxxxxxxxxxxxxx> > --- > drivers/gpio/gpio-aspeed-sgpio.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-aspeed-sgpio.c b/drivers/gpio/gpio-aspeed-sgpio.c > index 927d46f159b8..23a3a40901d6 100644 > --- a/drivers/gpio/gpio-aspeed-sgpio.c > +++ b/drivers/gpio/gpio-aspeed-sgpio.c I've re-ordered the lines in the diff to make it easier to review: > @@ -451,9 +451,7 @@ static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio, > /* trigger type is edge */ > iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type1)); > /* dual edge trigger mode. */ > - iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_type2)); > + iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type2)); You're changing the trigger mode from dual edge to single edge. > - /* enable irq */ > - iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_enable)); And also removing the enabling of IRQs. This part makes sense, as it's what the commit message says. If you think a sensible default should be single edge (and I would agree with that change), perhaps update the comment to say "set single edge trigger mode" and mention it in your commit message. Cheers, Joel
