diff mbox series

gpio: siox: use raw spinlock for irq related locking

Message ID 20200211135121.15752-1-uwe@kleine-koenig.org
State New
Headers show
Series gpio: siox: use raw spinlock for irq related locking | expand

Commit Message

Uwe Kleine-König Feb. 11, 2020, 1:51 p.m. UTC
All the irq related callbacks are called with the (raw) spinlock
desc->lock being held. So the lock here must be raw as well. Also irqs
were already disabled by the caller for the irq chip callbacks, so the
non-irq variants of spin_lock must be used there.

Fixes: be8c8facc707 ("gpio: new driver to work with a 8x12 siox")
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Hello,

On Tue, Feb 11, 2020 at 02:18:23PM +0100, Thomas Gleixner wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> > On Tue, Feb 11, 2020 at 09:59:30AM +0100, Thomas Gleixner wrote:
> >> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
> >> 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.
> 
> I explicitely said: "for all irqchip callbacks".
> 
> gpio_siox_get_data() is not a irq chip callback, right? So obviously it
> has to stay there.

Ah, I read "irqchip callbacks" as "spinlock calls". Thanks to restate
this for me.

Thanks
Uwe

 drivers/gpio/gpio-siox.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-siox.c b/drivers/gpio/gpio-siox.c
index 311f66757b92..26e1fe092304 100644
--- a/drivers/gpio/gpio-siox.c
+++ b/drivers/gpio/gpio-siox.c
@@ -15,7 +15,7 @@  struct gpio_siox_ddata {
 	u8 setdata[1];
 	u8 getdata[3];
 
-	spinlock_t irqlock;
+	raw_spinlock_t irqlock;
 	u32 irq_enable;
 	u32 irq_status;
 	u32 irq_type[20];
@@ -44,7 +44,7 @@  static int gpio_siox_get_data(struct siox_device *sdevice, const u8 buf[])
 
 	mutex_lock(&ddata->lock);
 
-	spin_lock_irq(&ddata->irqlock);
+	raw_spin_lock_irq(&ddata->irqlock);
 
 	for (offset = 0; offset < 12; ++offset) {
 		unsigned int bitpos = 11 - offset;
@@ -66,7 +66,7 @@  static int gpio_siox_get_data(struct siox_device *sdevice, const u8 buf[])
 
 	trigger = ddata->irq_status & ddata->irq_enable;
 
-	spin_unlock_irq(&ddata->irqlock);
+	raw_spin_unlock_irq(&ddata->irqlock);
 
 	ddata->getdata[0] = buf[0];
 	ddata->getdata[1] = buf[1];
@@ -84,9 +84,9 @@  static int gpio_siox_get_data(struct siox_device *sdevice, const u8 buf[])
 			 * handler of the irq chip. But it doesn't, so we have
 			 * to clean the irq_status here.
 			 */
-			spin_lock_irq(&ddata->irqlock);
+			raw_spin_lock_irq(&ddata->irqlock);
 			ddata->irq_status &= ~(1 << offset);
-			spin_unlock_irq(&ddata->irqlock);
+			raw_spin_unlock_irq(&ddata->irqlock);
 
 			handle_nested_irq(irq);
 		}
@@ -101,9 +101,9 @@  static void gpio_siox_irq_ack(struct irq_data *d)
 	struct gpio_siox_ddata *ddata =
 		container_of(ic, struct gpio_siox_ddata, ichip);
 
-	spin_lock_irq(&ddata->irqlock);
+	raw_spin_lock(&ddata->irqlock);
 	ddata->irq_status &= ~(1 << d->hwirq);
-	spin_unlock_irq(&ddata->irqlock);
+	raw_spin_unlock(&ddata->irqlock);
 }
 
 static void gpio_siox_irq_mask(struct irq_data *d)
@@ -112,9 +112,9 @@  static void gpio_siox_irq_mask(struct irq_data *d)
 	struct gpio_siox_ddata *ddata =
 		container_of(ic, struct gpio_siox_ddata, ichip);
 
-	spin_lock_irq(&ddata->irqlock);
+	raw_spin_lock(&ddata->irqlock);
 	ddata->irq_enable &= ~(1 << d->hwirq);
-	spin_unlock_irq(&ddata->irqlock);
+	raw_spin_unlock(&ddata->irqlock);
 }
 
 static void gpio_siox_irq_unmask(struct irq_data *d)
@@ -123,9 +123,9 @@  static void gpio_siox_irq_unmask(struct irq_data *d)
 	struct gpio_siox_ddata *ddata =
 		container_of(ic, struct gpio_siox_ddata, ichip);
 
-	spin_lock_irq(&ddata->irqlock);
+	raw_spin_lock(&ddata->irqlock);
 	ddata->irq_enable |= 1 << d->hwirq;
-	spin_unlock_irq(&ddata->irqlock);
+	raw_spin_unlock(&ddata->irqlock);
 }
 
 static int gpio_siox_irq_set_type(struct irq_data *d, u32 type)
@@ -134,9 +134,9 @@  static int gpio_siox_irq_set_type(struct irq_data *d, u32 type)
 	struct gpio_siox_ddata *ddata =
 		container_of(ic, struct gpio_siox_ddata, ichip);
 
-	spin_lock_irq(&ddata->irqlock);
+	raw_spin_lock(&ddata->irqlock);
 	ddata->irq_type[d->hwirq] = type;
-	spin_unlock_irq(&ddata->irqlock);
+	raw_spin_unlock(&ddata->irqlock);
 
 	return 0;
 }
@@ -222,7 +222,7 @@  static int gpio_siox_probe(struct siox_device *sdevice)
 	dev_set_drvdata(dev, ddata);
 
 	mutex_init(&ddata->lock);
-	spin_lock_init(&ddata->irqlock);
+	raw_spin_lock_init(&ddata->irqlock);
 
 	ddata->gchip.base = -1;
 	ddata->gchip.can_sleep = 1;