[PATCH] gpio: siox: potentially enabling IRQs too early

[PATCH] gpio: siox: potentially enabling IRQs too early

[Dan Carpenter]

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@oracle.com>
---
 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);
 
 	return 0;
 }
-- 
2.11.0

Re: [PATCH] gpio: siox: potentially enabling IRQs too early

[Uwe Kleine-König]

[Dropped Gavin Schenk from recipients as he left Eckelmann. Added tglx.]

Hello,

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@oracle.com>
> ---
>  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?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH] gpio: siox: potentially enabling IRQs too early

[Thomas Gleixner]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> 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@oracle.com>
>> ---
>>  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.

Thanks,

        tglx

Re: [PATCH] gpio: siox: potentially enabling IRQs too early

[Uwe Kleine-König]

On Tue, Feb 11, 2020 at 09:59:30AM +0100, Thomas Gleixner wrote:
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> 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@oracle.com>
> >> ---
> >>  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/ |

Re: [PATCH] gpio: siox: potentially enabling IRQs too early

[Thomas Gleixner]

Uwe,

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.

Thanks,

        tglx




  

[PATCH] gpio: siox: use raw spinlock for irq related locking

[Uwe Kleine-König]

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 --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;
-- 
2.24.0

Re: [PATCH] gpio: siox: use raw spinlock for irq related locking

[Thorsten Scherer]

Hello,

AFAICT this is all good.

Unfortunately i don't have any idea on how to test out the difference
this patch makes on a real SIOX.

Any hints? Is it necessary at all?

Thank you.

Kind regards
Thorsten

Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>

On Tue, Feb 11, 2020 at 02:51:21PM +0100, Uwe Kleine-König wrote:
> 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>
> ---
> > 
> > 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 --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;
> -- 
> 2.24.0
> 

--
Thorsten Scherer | Eckelmann AG | www.eckelmann.de |

Re: [PATCH] gpio: siox: use raw spinlock for irq related locking

[Uwe Kleine-König]

Hello Thorsten,

On Fri, Feb 14, 2020 at 12:02:38PM +0100, Thorsten Scherer wrote:
> AFAICT this is all good.
> 
> Unfortunately i don't have any idea on how to test out the difference
> this patch makes on a real SIOX.

When I started looking into the problem I expected that the lock
debugging would catch these problems. But either I did something wrong
or there is no mechanism that catches

 - a spinlock is taken when there is already a raw spinlock taken
 - spin_lock_irq is used with irqs already off

And I wonder if there are reasons I don't see that make these two tests
a bad idea.

> Any hints? Is it necessary at all?

Apart from "normal" testing that SIOX still works I have no good
suggestion. Having said that I would be surprised if my patch breaked
something. (But it wouldn't be the first time such a surprise happens
:-)

Best regards
Uwe
-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH] gpio: siox: use raw spinlock for irq related locking

[Linus Walleij]

On Tue, Feb 11, 2020 at 2:59 PM Uwe Kleine-König <uwe@kleine-koenig.org> wrote:

> 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>

Patch applied. Is this a regression so I should put it in fixes?
I put it for v5.7 for now but I can easily change that.

Yours,
Linus Walleij

Re: [PATCH] gpio: siox: use raw spinlock for irq related locking

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 959 bytes --]

On 2/21/20 2:52 PM, Linus Walleij wrote:
> On Tue, Feb 11, 2020 at 2:59 PM Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> 
>> 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>
> 
> Patch applied. Is this a regression so I should put it in fixes?
> I put it for v5.7 for now but I can easily change that.

I don't care much. AFAIK siox is only used at Eckelmann and we will have
to deploy the fix there anyhow. Conceptually the conversion to raw is
only relevant for RT (please correct me if I'm wrong), but too early
enablement of irqs probably can yield bad races.

Best regards
Uwe


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]