On Tue, Feb 11, 2020 at 09:59:30AM +0100, Thomas Gleixner wrote: > Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes: > > On Tue, Feb 11, 2020 at 10:35:44AM +0300, Dan Carpenter wrote: > >> Smatch thinks that gpio_siox_irq_set_type() can be called from > >> probe_irq_on(). In that case the call to spin_unlock_irq() would > >> renable IRQs too early. > >> > >> Fixes: be8c8facc707 ("gpio: new driver to work with a 8x12 siox") > >> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > >> --- > >> drivers/gpio/gpio-siox.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpio/gpio-siox.c b/drivers/gpio/gpio-siox.c > >> index 311f66757b92..578b71760939 100644 > >> --- a/drivers/gpio/gpio-siox.c > >> +++ b/drivers/gpio/gpio-siox.c > >> @@ -133,10 +133,11 @@ static int gpio_siox_irq_set_type(struct irq_data *d, u32 type) > >> struct irq_chip *ic = irq_data_get_irq_chip(d); > >> struct gpio_siox_ddata *ddata = > >> container_of(ic, struct gpio_siox_ddata, ichip); > >> + unsigned long flags; > >> > >> - spin_lock_irq(&ddata->irqlock); > >> + spin_lock_irqsave(&ddata->irqlock, flags); > >> ddata->irq_type[d->hwirq] = type; > >> - spin_unlock_irq(&ddata->irqlock); > >> + spin_unlock_irqrestore(&ddata->irqlock, flags); > > > > "Normally" the .irq_set_type() callback is called from irq_set_irq_type. > > There desc->lock is taken with raw_spin_lock_irqsave (as part of > > irq_get_desc_buslock()), so I assume irqs are always disabled when the > > type changes? tglx? > > > > If so, the better fix would be to change to spin_lock/spin_unlock > > instead. Also given that a raw spinlock is taken, the irqlock here > > should be changed to a raw one, too? > > Indeed. Looking at the driver, all of the spin_lock_irq() usage in the > irqchip callbacks is broken. > > So this needs two changes: > > 1) Convert to raw spin lock, as this won't work on RT otherwise > > 2) s/lock_irq/lock/ for all irqchip callbacks. Are you sure about the calls in gpio_siox_get_data()? This is only called by siox_poll() which doesn't disable irqs. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |