Re: [PATCH v2] gpio: tqmx86: Add GPIO from for this IO controller

[Julia Cartwright <julia@xxxxxx>]

On Mon, Jan 14, 2019 at 02:39:52PM +0100, Andrew Lunn wrote:
> On Mon, Jan 14, 2019 at 11:09:07AM +0100, Linus Walleij wrote:
> > Hi Andrew,
> >
> > (Cc Julia C for the RT spinlock question.)
> >
[..]
> > > +struct tqmx86_gpio_data {
> > > +       struct gpio_chip        chip;
> > > +       struct irq_chip         irq_chip;
> > > +       void __iomem            *io_base;
> > > +       int                     irq;
> > > +       spinlock_t              spinlock;
> > 
> > I am not an expert in RT but I think this needs to be a
> > raw_spinlock_t (and use raw accessors) to work with realtime.
> > 
> > But maybe that just apply to chained IRQ handlers?
> 
> Not my area of expertise also.
> 
> I just read Documentation/driver-api/gpio/driver.rst.
>
> That suggests i should use raw spinlocks.

Because many of the struct irq_chip operations are invoked under struct
irq_desc::lock, which is raw, there will be issues on RT unless you also
make use of raw_spinlock_t here.

Converting to using raw_spinlock_t here doesn't look like it's going to
be a problem, seeing as this driver already does very little under the
lock.

> This could be used as a fast chained IRQ, in that it is hanging off
> the main x86 interrupt controller, and accessing the GPIO interrupt
> registers are also fast, non-blocking operations.

The implementation, as it stands, will also fall over w/ forced
irqthreading.

In particular, the tqmx86_gpio_irq_handler will be force threaded, and
executed with interrupts enabled, which will be problematic when
dispatching to downstream handlers..

This should work fine if you use the chained irq approach, even in the
force irqthreading case.

   Julia