The 10/13/2022 09:30, Michael Walle wrote: Hi Michael, > > We lose the interrupt here, as the HW will not generate another one > > but at later point we read again the line status. And if the line is > > active then we kick again the interrupt handler again. > > Ahh, thanks for explaining. That also explains the read below. > > Will you send a proper patch? No worries. Yes, I will do that. > > -michael > > > > > > > > > > + > > > > /* Enable the interrupt now */ > > > > gpiochip_enable_irq(chip, gpio); > > > > regmap_update_bits(info->map, REG(OCELOT_GPIO_INTR_ENA, info, gpio), > > > > bit, bit); > > > > > > > > /* > > > > - * In case the interrupt line is still active and the interrupt > > > > - * controller has not seen any changes in the interrupt line, then it > > > > - * means that there happen another interrupt while the line was > > > > active. > > > > + * In case the interrupt line is still active then it means that > > > > + * there happen another interrupt while the line was active. > > > > * So we missed that one, so we need to kick the interrupt again > > > > * handler. > > > > */ > > > > - if (active && !ack) { > > > > + regmap_read(info->map, REG(OCELOT_GPIO_IN, info, gpio), &val); > > > > + if ((!(val & bit) && trigger_level == IRQ_TYPE_LEVEL_LOW) || > > > > + (val & bit && trigger_level == IRQ_TYPE_LEVEL_HIGH)) > > > > + active = true; > > > > > > Why do you read the line state twice? What happens if the line state > > > changes right after you've read it? > > > > Here we need to read again the status because we might have clear the > > ack of interrupt. > > If the line becomes active right after this read, then the HW will > > generate another interrupt as the interrupt is enabled and ack is > > cleared. > > > > > > > > > + > > > > + if (active) { > > > > struct ocelot_irq_work *work; > > > > > > > > work = kmalloc(sizeof(*work), GFP_ATOMIC); > > > > > > So yes, maybe the trade-off that there will be two interrupts are > > > better than this additional patch. But it should be documented > > > somewhere, even if it's just a comment in this driver. > > > > > > -michael -- /Horatiu