Re: [PATCH 6/6] gpio: macsmc: Add IRQ support

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Sep 06, 2022 at 04:00:31PM +0900, Hector Martin wrote:
> On 02/09/2022 22.21, Linus Walleij wrote:
> >> +       switch (type & IRQ_TYPE_SENSE_MASK) {
> >> +       case IRQ_TYPE_LEVEL_HIGH:
> >> +               mode = IRQ_MODE_HIGH;
> >> +               break;
> >> +       case IRQ_TYPE_LEVEL_LOW:
> >> +               mode = IRQ_MODE_LOW;
> >> +               break;
> >> +       case IRQ_TYPE_EDGE_RISING:
> >> +               mode = IRQ_MODE_RISING;
> >> +               break;
> >> +       case IRQ_TYPE_EDGE_FALLING:
> >> +               mode = IRQ_MODE_FALLING;
> >> +               break;
> >> +       case IRQ_TYPE_EDGE_BOTH:
> >> +               mode = IRQ_MODE_BOTH;
> >> +               break;
> >> +       default:
> >> +               return -EINVAL;
> > 
> > I don't know how level IRQs would work on this essentially
> > message-passing process context interrupt. Maybe I am getting
> > it all wrong, but for level the line should be held low/high until
> > the IRQ is serviced, it would be possible to test if this actually
> > works by *not* servicing an IRQ and see if the SMC then sends
> > another message notifier for the same IRQ.
> > 
> > I strongly suspect that actually only edges are supported, but
> > there might be semantics I don't understand here.
> 
> IIRC that is exactly what happens - the SMC will re-fire the IRQ after
> the ACK if it is set to level mode and still at the active level.
> 
> I do remember testing all the modes carefully when implementing this to
> figure out what the precise semantics are, and I *think* I agonized over
> the flow handlers quite a bit and decided this way would work properly
> for all the modes, but it's been a while so I'd have to take a look
> again to convince myself again :)

Thanks for the clarification - I think it would be useful to put some
of that as comments before the CMD_IRQ_ACK write to head off any
questions about this in the future. Something like this maybe?

        /*
         * This is not an "ack" int he i8253 PIC sense - it is used for level
         * interrupts as well. The SMC will re-fire the interrupt event after
         * this ACK if the level interrupt is still active.
         */
        if (apple_smc_write_u32(smcgp->smc, key, CMD_IRQ_ACK | 1) < 0)
                dev_err(smcgp->dev, "GPIO IRQ ack failed for %p4ch\n", &key);

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!