RE: [PATCH] net: txgbe: fix GPIO interrupt blocking

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Wed, Feb 21, 2024 10:36 PM, Andrew Lunn wrote:
> On Tue, Feb 20, 2024 at 05:25:26PM +0800, Jiawen Wu wrote:
> > On Mon, Feb 19, 2024 12:45 AM, Andrew Lunn wrote:
> > > On Sun, Feb 18, 2024 at 05:04:52PM +0800, Jiawen Wu wrote:
> > > > On Tue, Feb 6, 2024 11:29 PM, Andrew Lunn wrote:
> > > > > On Tue, Feb 06, 2024 at 03:08:24PM +0800, Jiawen Wu wrote:
> > > > > > GPIO interrupt is generated before MAC IRQ is enabled, it causes
> > > > > > subsequent GPIO interrupts that can no longer be reported if it is
> > > > > > not cleared in time. So clear GPIO interrupt status at the right
> > > > > > time.
> > > > >
> > > > > This does not sound correct. Since this is an interrupt controller, it
> > > > > is a level interrupt. If its not cleared, as soon as the parent
> > > > > interrupt is re-enabled, is should cause another interrupt at the
> > > > > parent level. Servicing that interrupt, should case a descent to the
> > > > > child, which will service the interrupt, and atomically clear the
> > > > > interrupt status.
> > > > >
> > > > > Is something wrong here, like you are using edge interrupts, not
> > > > > level?
> > > >
> > > > Yes, it is edge interrupt.
> > >
> > > So fix this first, use level interrupts.
> >
> > I have a question here.
> >
> > I've been setting the interrupt type in chip->irq_set_type. The 'type' is
> > passed as IRQ_TYPE_EDGE_BOTH. Then I config GPIO registers based on
> > this type, and use edge interrupts. Who decides this type? Can I change
> > it at will?
> 
> There are a few different mechanism. In DT you can specify it as part
> of the phandle reference. You can also pass flags to
> 
> request_irq(unsigned int irq, irq_handler_t handler, unsigned long flags,
> 	    const char *name, void *dev)
> 
> #define IRQF_TRIGGER_NONE	0x00000000
> #define IRQF_TRIGGER_RISING	0x00000001
> #define IRQF_TRIGGER_FALLING	0x00000002
> #define IRQF_TRIGGER_HIGH	0x00000004
> #define IRQF_TRIGGER_LOW	0x00000008

There are flags passed in sfp.c:

err = devm_request_threaded_irq(sfp->dev, sfp->gpio_irq[i],
				NULL, sfp_irq,
				IRQF_ONESHOT |
				IRQF_TRIGGER_RISING |
				IRQF_TRIGGER_FALLING,
				sfp_irq_name, sfp);

So specific GPIO IRQs are request as edge interrupts. It looks like I can't
change it.