On Thursday, May 11, 2023 8:32 PM, Andrew Lunn wrote:
> > +static int txgbe_gpio_get(struct gpio_chip *chip, unsigned int
> > +offset) {
> > + struct wx *wx = gpiochip_get_data(chip);
> > + struct txgbe *txgbe = wx->priv;
> > + int val;
> > +
> > + val = rd32m(wx, WX_GPIO_EXT, BIT(offset));
> > +
> > + txgbe->gpio_orig &= ~BIT(offset);
> > + txgbe->gpio_orig |= val;
> > +
> > + return !!(val & BIT(offset));
> > +}
>
> > +static void txgbe_irq_handler(struct irq_desc *desc) {
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > + struct wx *wx = irq_desc_get_handler_data(desc);
> > + struct txgbe *txgbe = wx->priv;
> > + irq_hw_number_t hwirq;
> > + unsigned long gpioirq;
> > + struct gpio_chip *gc;
> > + u32 gpio;
> > +
> > + chained_irq_enter(chip, desc);
> > +
> > + gpioirq = rd32(wx, WX_GPIO_INTSTATUS);
> > +
> > + /* workaround for hysteretic gpio interrupts */
> > + gpio = rd32(wx, WX_GPIO_EXT);
> > + if (!gpioirq)
> > + gpioirq = txgbe->gpio_orig ^ gpio;
>
> Please could you expand on the comment. Are you saying that
> WX_GPIO_INTSTATUS sometimes does not contain the GPIO which caused the
> interrupt? If so, you then compare the last gpio_get with the current value and
> assume that is what caused the interrupt?
Yes. Sometime there is a lag in WX_GPIO_INTSTATUS. When the GPIO interrupt
cause, the GPIO state has been back to its previous state. So I added this
workaround to save some...but only if there are other interrupts at the same
time, i.e. txgbe_irq_handler() called.
But I will remove it in the next version, because I find a more accurate solution.
>
> > +
> > + gc = txgbe->gpio;
> > + for_each_set_bit(hwirq, &gpioirq, gc->ngpio)
> > + generic_handle_domain_irq(gc->irq.domain, hwirq);
> > +
> > + chained_irq_exit(chip, desc);
> > +
> > + /* unmask interrupt */
> > + if (netif_running(wx->netdev))
> > + wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
>
> Is that a hardware requirement, that interrupts only work when the interface is
> running? Interrupts are not normally conditional like this, at least when the SoC
> provides the GPIO controller.
Should we handle the interrupts when interface is not running?
