Re: gpio interrupts

[Jared McNeill <jmcneill%invisible.ca@localhost>]

On Thu, 7 Apr 2016, Manuel Bouyer wrote:

> On Thu, Apr 07, 2016 at 05:34:00PM -0300, Jared McNeill wrote:
> > Hi Manuel --
> >
> > A few comments:
> >
> > > The attached patch adds two functions to gpio.c:
> > > gpio_pin_wait() is called by a thread that wants to wait on an interrupt.
> > > A thread can wait only on a single pin.
> >
> > Any particular reason you went with this approach instead of say, something
> > that follows the typical foo_intr_establish pattern? I think "register a
> > callback" would be a much more typical use-case for a device driver here.
>
> In a SMP world you want to do less in interrupt handlers and more in
> kernel threads, basically. Also, in this kind of setup, the
> device raising an interrupt is most likely behind a bus that itself needs
> interrupts to work (i2c, spi, uart, ...). So the action triggered by
> the interrupt can't be part of the interrupt handler, it has to execute
> in a thread.
> Also, if we ever makes this usable up to userland, condvar will be needed.

Maybe it makes sense to have both a low level "gpio_intr_establish", and a 
higher level function that builds on top of that for convenience of 
drivers that prefer a synchronous API.

Thinking of fdtbus gpiokeys driver, where its job is to receive the 
notification and dispatch that event to sysmon. Sysmon already has a 
thread, and having only a synchronous-only API means the gpiokeys driver 
needs to manage yet another one.

> I could add a callback scheme if there is a need for that, but this
> raises a question: how to handle IPL properly ?
> AFAIK the only MI interface is splfoo()/splx(), splraise/spllower are
> not mandatory.

Good question. I ran into the same issue with the PCIe driver for tegra 
(sys/arch/arm/nvidia/tegra_pcie.c) and didn't come up with a good 
solution there either.

Cheers,
Jared