Skip to contentSkip to conversation

GPIO: mvebu-gpio: Don't initialize the mask_cache

PATCH New Andrew Lunn 5 participants 6 messages Oct 27, 2012 – Oct 28, 2012 GPIO

lore
Share & tools
lore

Andrew Lunn

author
#0 lore raw

PATCH 1/1 GPIO: mvebu-gpio: Don't initialize the mask_cache

Due to the SMP nature of some of the chips, which have per CPU registers, the driver does not use the generic irq_gc_mask_set_bit() & irq_gc_mask_clr_bit() functions, which only support a single register. The driver has its own implementation of these functions, which can pick the correct register depending on the CPU being used. The functions do however use the gc->mask_cache value.

The call to irq_setup_generic_chip() was passing IRQ_GC_INIT_MASK_CACHE, which caused the gc->mask_cache to be initialized to the contents of some random register. This resulted in unexpected interrupts been delivered from random GPIO lines.

Signed-off-by Andrew Lunn <[email protected]>
---
 drivers/gpio/gpio-mvebu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpio-mvebu.c +1 -1
index eb42ab1..cf7afb9 100644
@@ -646,7 +646,7 @@ static int __devinit mvebu_gpio_probe(struct platform_device *pdev)
 	ct->handler = handle_edge_irq;
 	ct->chip.name = mvchip->chip.label;
 
-	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), IRQ_GC_INIT_MASK_CACHE,
+	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0,
 			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
 
 	/* Setup irq domain on top of the generic chip. */
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/1 GPIO: mvebu-gpio: Don't initialize the mask_cache

Quoted 7 lines · Andrew Lunn

Due to the SMP nature of some of the chips, which have per CPU registers, the driver does not use the generic irq_gc_mask_set_bit() & irq_gc_mask_clr_bit() functions, which only support a single register. The driver has its own implementation of these functions, which can pick the correct register depending on the CPU being used. The functions do however use the gc->mask_cache value.

Quoted 6 lines

The call to irq_setup_generic_chip() was passing IRQ_GC_INIT_MASK_CACHE, which caused the gc->mask_cache to be initialized to the contents of some random register. This resulted in unexpected interrupts been delivered from random GPIO lines.

Signed-off-by Andrew Lunn <[email protected]>

Thanks, patch applied to fixes. Unless Thomas screams...

Yours,
Linus Walleij

#2 lore raw

PATCH 1/1 GPIO: mvebu-gpio: Don't initialize the mask_cache

Quoted 9 lines · Linus Walleij
Quoted 7 lines · Andrew Lunn

Due to the SMP nature of some of the chips, which have per CPU registers, the driver does not use the generic irq_gc_mask_set_bit() & irq_gc_mask_clr_bit() functions, which only support a single register. The driver has its own implementation of these functions, which can pick the correct register depending on the CPU being used. The functions do however use the gc->mask_cache value.

Quoted 8 lines
Quoted 6 lines

The call to irq_setup_generic_chip() was passing IRQ_GC_INIT_MASK_CACHE, which caused the gc->mask_cache to be initialized to the contents of some random register. This resulted in unexpected interrupts been delivered from random GPIO lines.

Signed-off-by Andrew Lunn <[email protected]>

Thanks, patch applied to fixes. Unless Thomas screams...

Acked-by-the-not-screaming: Thomas Petazzoni <[email protected]>

Sounds good? :-)

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
author
#3 lore raw

PATCH 1/1 GPIO: mvebu-gpio: Don't initialize the mask_cache

Quoted 16 lines · Thomas Petazzoni
Quoted 14 lines · Linus Walleij
Quoted 12 lines · Andrew Lunn

Due to the SMP nature of some of the chips, which have per CPU registers, the driver does not use the generic irq_gc_mask_set_bit() & irq_gc_mask_clr_bit() functions, which only support a single register. The driver has its own implementation of these functions, which can pick the correct register depending on the CPU being used. The functions do however use the gc->mask_cache value.

The call to irq_setup_generic_chip() was passing IRQ_GC_INIT_MASK_CACHE, which caused the gc->mask_cache to be initialized to the contents of some random register. This resulted in unexpected interrupts been delivered from random GPIO lines.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Andrew Lunn <[email protected]>

Thanks, patch applied to fixes. Unless Thomas screams...

Acked-by-the-not-screaming: Thomas Petazzoni <[email protected]>

Sounds good? :-)

https://lwn.net/Articles/503829

Its at least new...

Andrew

#4 lore raw

PATCH 1/1 GPIO: mvebu-gpio: Don't initialize the mask_cache

In reply to Andrew Lunn
Quoted 7 lines · Andrew Lunn

Due to the SMP nature of some of the chips, which have per CPU registers, the driver does not use the generic irq_gc_mask_set_bit() & irq_gc_mask_clr_bit() functions, which only support a single register. The driver has its own implementation of these functions, which can pick the correct register depending on the CPU being used. The functions do however use the gc->mask_cache value.

Quoted 6 lines

The call to irq_setup_generic_chip() was passing IRQ_GC_INIT_MASK_CACHE, which caused the gc->mask_cache to be initialized to the contents of some random register. This resulted in unexpected interrupts been delivered from random GPIO lines.

Signed-off-by Andrew Lunn <[email protected]>
Tested-by Jamie Lentin <[email protected]>

This is what was causing my NAS to lock up when I turned the LEDs off. Thanks! The only one not working now is the power LED, patch coming soon to fix that.

Quoted 18 lines

---
drivers/gpio/gpio-mvebu.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpio-mvebu.c +1 -1
index eb42ab1..cf7afb9 100644
@@ -646,7 +646,7 @@ static int __devinit mvebu_gpio_probe(struct platform_device *pdev)
 	ct->handler = handle_edge_irq;
 	ct->chip.name = mvchip->chip.label;
 
-	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), IRQ_GC_INIT_MASK_CACHE,
+	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0,
 			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
 
 	/* Setup irq domain on top of the generic chip. */
Signature · 1 line
Jamie Lentin
#5 lore raw

PATCH 1/1 GPIO: mvebu-gpio: Don't initialize the mask_cache

In reply to Andrew Lunn

Am Samstag 27 Oktober 2012, 15:28:58 schrieb Andrew Lunn:

Quoted 22 lines

Due to the SMP nature of some of the chips, which have per CPU registers, the driver does not use the generic irq_gc_mask_set_bit() & irq_gc_mask_clr_bit() functions, which only support a single register. The driver has its own implementation of these functions, which can pick the correct register depending on the CPU being used. The functions do however use the gc->mask_cache value.

The call to irq_setup_generic_chip() was passing IRQ_GC_INIT_MASK_CACHE, which caused the gc->mask_cache to be initialized to the contents of some random register. This resulted in unexpected interrupts been delivered from random GPIO lines.

Signed-off-by Andrew Lunn <[email protected]>
---
 drivers/gpio/gpio-mvebu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpio-mvebu.c +0 -0
index eb42ab1..cf7afb9 100644

drivers/gpio/gpio-mvebu.c static int __devinit mvebu_gpio_probe(struct · line 652

Quoted 8 lines
drivers/gpio/gpio-mvebu.c +1 -1
@@ -646,7 +646,7 @@ static int __devinit mvebu_gpio_probe(struct
 platform_device *pdev) ct->handler = handle_edge_irq;
 	ct->chip.name = mvchip->chip.label;
 
-	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), IRQ_GC_INIT_MASK_CACHE,
+	irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0,
 			       IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
 
 	/* Setup irq domain on top of the generic chip. */
Tested-by Michael Walle <[email protected]>

Fixes my lock ups when booting, too.

Signature · 1 line
Michael