arm/imx/gpio: add spinlock protection

The GPIO and IRQ/GPIO registers need protection from concurrent access for operations that are not atomic.

Cc Juergen Beisert <[email protected]>

Cc Daniel Mack <[email protected]>

Reported-by rpkamiak at rockwellcollins.com

Signed-off-by Baruch Siach <[email protected]>

---
 arch/arm/plat-mxc/gpio.c              |   28 +++++++++++++++++++++++++---
 arch/arm/plat-mxc/include/mach/gpio.h |    3 +++
 2 files changed, 28 insertions(+), 3 deletions(-)

arch/arm/plat-mxc/gpio.c +25 -3

index 71437c6..a8a33cd 100644
@@ -56,10 +56,13 @@ static void _set_gpio_irqenable(struct mxc_gpio_port *port, u32 index,
 				int enable)
 {
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->irq_lock, flags);
 	l = __raw_readl(port->base + GPIO_IMR);
 	l = (l & (~(1 << index))) | (!!enable << index);
 	__raw_writel(l, port->base + GPIO_IMR);
+	spin_unlock_irqrestore(&port->irq_lock, flags);
 }
 
 static void gpio_ack_irq(u32 irq)

@@ -87,8 +90,11 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 	u32 gpio = irq_to_gpio(irq);
 	struct mxc_gpio_port *port = &mxc_gpio_ports[gpio / 32];
 	u32 bit, val;
-	int edge;
+	int edge, rc = 0;
 	void __iomem *reg = port->base;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->irq_lock, flags);
 
 	port->both_edges &= ~(1 << (gpio & 31));
 	switch (type) {

@@ -116,7 +122,8 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 		edge = GPIO_INT_HIGH_LEV;
 		break;
 	default:
-		return -EINVAL;
+		rc = -EINVAL;
+		goto out;
 	}
 
 	reg += GPIO_ICR1 + ((gpio & 0x10) >> 2); /* lower or upper register */

@@ -125,9 +132,12 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 	__raw_writel(val | (edge << (bit << 1)), reg);
 	_clear_gpio_irqstatus(port, gpio & 0x1f);
 
-	return 0;
+out:
+	spin_unlock_irqrestore(&port->irq_lock, flags);
+	return rc;
 }
 
+/* caller must hold port->irq_lock */
 static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
 {
 	void __iomem *reg = port->base;

@@ -157,12 +167,15 @@ static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
 static void mxc_gpio_irq_handler(struct mxc_gpio_port *port, u32 irq_stat)
 {
 	u32 gpio_irq_no_base = port->virtual_irq_start;
+	unsigned long flags;
 
 	while (irq_stat != 0) {
 		int irqoffset = fls(irq_stat) - 1;
 
+		spin_lock_irqsave(&port->irq_lock, flags);
 		if (port->both_edges & (1 << irqoffset))
 			mxc_flip_edge(port, irqoffset);
+		spin_unlock_irqrestore(&port->irq_lock, flags);
 
 		generic_handle_irq(gpio_irq_no_base + irqoffset);
 

@@ -214,13 +227,16 @@ static void _set_gpio_direction(struct gpio_chip *chip, unsigned offset,
 	struct mxc_gpio_port *port =
 		container_of(chip, struct mxc_gpio_port, chip);
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = __raw_readl(port->base + GPIO_GDIR);
 	if (dir)
 		l |= 1 << offset;
 	else
 		l &= ~(1 << offset);
 	__raw_writel(l, port->base + GPIO_GDIR);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -229,9 +245,12 @@ static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 		container_of(chip, struct mxc_gpio_port, chip);
 	void __iomem *reg = port->base + GPIO_DR;
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = (__raw_readl(reg) & (~(1 << offset))) | (value << offset);
 	__raw_writel(l, reg);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static int mxc_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -285,6 +304,9 @@ int __init mxc_gpio_init(struct mxc_gpio_port *port, int cnt)
 		port[i].chip.base = i * 32;
 		port[i].chip.ngpio = 32;
 
+		spin_lock_init(&port[i].lock);
+		spin_lock_init(&port[i].irq_lock);
+
 		/* its a serious configuration bug when it fails */
 		BUG_ON( gpiochip_add(&port[i].chip) < 0 );
 

arch/arm/plat-mxc/include/mach/gpio.h +3 -0

index 894d2f8..a37724a 100644
@@ -36,6 +36,9 @@ struct mxc_gpio_port {
 	int virtual_irq_start;
 	struct gpio_chip chip;
 	u32 both_edges;
+
+	spinlock_t	lock;		/* GPIO registers */
+	spinlock_t	irq_lock;	/* IRQ registers */
 };
 
 int mxc_gpio_init(struct mxc_gpio_port*, int);

Signature · 1 line

1.7.1

The GPIO and IRQ/GPIO registers need protection from concurrent access for operations that are not atomic.

Cc Juergen Beisert <[email protected]>

Cc Daniel Mack <[email protected]>

Reported-by rpkamiak at rockwellcollins.com

Signed-off-by Baruch Siach <[email protected]>

---
 arch/arm/plat-mxc/gpio.c              |   28 +++++++++++++++++++++++++---
 arch/arm/plat-mxc/include/mach/gpio.h |    3 +++
 2 files changed, 28 insertions(+), 3 deletions(-)

arch/arm/plat-mxc/gpio.c +25 -3

index 71437c6..a8a33cd 100644
@@ -56,10 +56,13 @@ static void _set_gpio_irqenable(struct mxc_gpio_port *port, u32 index,
 				int enable)
 {
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->irq_lock, flags);
 	l = __raw_readl(port->base + GPIO_IMR);
 	l = (l & (~(1 << index))) | (!!enable << index);
 	__raw_writel(l, port->base + GPIO_IMR);
+	spin_unlock_irqrestore(&port->irq_lock, flags);
 }
 
 static void gpio_ack_irq(u32 irq)

@@ -87,8 +90,11 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 	u32 gpio = irq_to_gpio(irq);
 	struct mxc_gpio_port *port = &mxc_gpio_ports[gpio / 32];
 	u32 bit, val;
-	int edge;
+	int edge, rc = 0;
 	void __iomem *reg = port->base;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->irq_lock, flags);
 
 	port->both_edges &= ~(1 << (gpio & 31));
 	switch (type) {

@@ -116,7 +122,8 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 		edge = GPIO_INT_HIGH_LEV;
 		break;
 	default:
-		return -EINVAL;
+		rc = -EINVAL;
+		goto out;
 	}
 
 	reg += GPIO_ICR1 + ((gpio & 0x10) >> 2); /* lower or upper register */

@@ -125,9 +132,12 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 	__raw_writel(val | (edge << (bit << 1)), reg);
 	_clear_gpio_irqstatus(port, gpio & 0x1f);
 
-	return 0;
+out:
+	spin_unlock_irqrestore(&port->irq_lock, flags);
+	return rc;
 }
 
+/* caller must hold port->irq_lock */
 static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
 {
 	void __iomem *reg = port->base;

@@ -157,12 +167,15 @@ static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
 static void mxc_gpio_irq_handler(struct mxc_gpio_port *port, u32 irq_stat)
 {
 	u32 gpio_irq_no_base = port->virtual_irq_start;
+	unsigned long flags;
 
 	while (irq_stat != 0) {
 		int irqoffset = fls(irq_stat) - 1;
 
+		spin_lock_irqsave(&port->irq_lock, flags);
 		if (port->both_edges & (1 << irqoffset))
 			mxc_flip_edge(port, irqoffset);
+		spin_unlock_irqrestore(&port->irq_lock, flags);
 
 		generic_handle_irq(gpio_irq_no_base + irqoffset);
 

@@ -214,13 +227,16 @@ static void _set_gpio_direction(struct gpio_chip *chip, unsigned offset,
 	struct mxc_gpio_port *port =
 		container_of(chip, struct mxc_gpio_port, chip);
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = __raw_readl(port->base + GPIO_GDIR);
 	if (dir)
 		l |= 1 << offset;
 	else
 		l &= ~(1 << offset);
 	__raw_writel(l, port->base + GPIO_GDIR);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -229,9 +245,12 @@ static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 		container_of(chip, struct mxc_gpio_port, chip);
 	void __iomem *reg = port->base + GPIO_DR;
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = (__raw_readl(reg) & (~(1 << offset))) | (value << offset);
 	__raw_writel(l, reg);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static int mxc_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -285,6 +304,9 @@ int __init mxc_gpio_init(struct mxc_gpio_port *port, int cnt)
 		port[i].chip.base = i * 32;
 		port[i].chip.ngpio = 32;
 
+		spin_lock_init(&port[i].lock);
+		spin_lock_init(&port[i].irq_lock);
+
 		/* its a serious configuration bug when it fails */
 		BUG_ON( gpiochip_add(&port[i].chip) < 0 );
 

arch/arm/plat-mxc/include/mach/gpio.h +3 -0

index 894d2f8..a37724a 100644
@@ -36,6 +36,9 @@ struct mxc_gpio_port {
 	int virtual_irq_start;
 	struct gpio_chip chip;
 	u32 both_edges;
+
+	spinlock_t	lock;		/* GPIO registers */
+	spinlock_t	irq_lock;	/* IRQ registers */
 };
 
 int mxc_gpio_init(struct mxc_gpio_port*, int);

Signature · 1 line

1.7.1

Quoted 2 lines · Baruch Siach

The GPIO and IRQ/GPIO registers need protection from concurrent access for operations that are not atomic.

I don't think we need locking here. mxc_gpio_irq_handler is called with desc->lock held (from the parent interrupt, not the chained interrupts). Other functions like enable_irq/disable_irq which result in mask/unmask operations run with interrupts disabled.

Apart from this other architectures do not use locking here aswell.

Quoted 138 lines

Cc Juergen Beisert <[email protected]>

Cc Daniel Mack <[email protected]>

Reported-by rpkamiak at rockwellcollins.com

Signed-off-by Baruch Siach <[email protected]>

---
 arch/arm/plat-mxc/gpio.c              |   28 +++++++++++++++++++++++++---
 arch/arm/plat-mxc/include/mach/gpio.h |    3 +++
 2 files changed, 28 insertions(+), 3 deletions(-)

arch/arm/plat-mxc/gpio.c +25 -3

index 71437c6..a8a33cd 100644
@@ -56,10 +56,13 @@ static void _set_gpio_irqenable(struct mxc_gpio_port *port, u32 index,
 				int enable)
 {
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->irq_lock, flags);
 	l = __raw_readl(port->base + GPIO_IMR);
 	l = (l & (~(1 << index))) | (!!enable << index);
 	__raw_writel(l, port->base + GPIO_IMR);
+	spin_unlock_irqrestore(&port->irq_lock, flags);
 }
 
 static void gpio_ack_irq(u32 irq)

@@ -87,8 +90,11 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 	u32 gpio = irq_to_gpio(irq);
 	struct mxc_gpio_port *port = &mxc_gpio_ports[gpio / 32];
 	u32 bit, val;
-	int edge;
+	int edge, rc = 0;
 	void __iomem *reg = port->base;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->irq_lock, flags);
 
 	port->both_edges &= ~(1 << (gpio & 31));
 	switch (type) {

@@ -116,7 +122,8 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 		edge = GPIO_INT_HIGH_LEV;
 		break;
 	default:
-		return -EINVAL;
+		rc = -EINVAL;
+		goto out;
 	}
 
 	reg += GPIO_ICR1 + ((gpio & 0x10) >> 2); /* lower or upper register */

@@ -125,9 +132,12 @@ static int gpio_set_irq_type(u32 irq, u32 type)
 	__raw_writel(val | (edge << (bit << 1)), reg);
 	_clear_gpio_irqstatus(port, gpio & 0x1f);
 
-	return 0;
+out:
+	spin_unlock_irqrestore(&port->irq_lock, flags);
+	return rc;
 }
 
+/* caller must hold port->irq_lock */
 static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
 {
 	void __iomem *reg = port->base;

@@ -157,12 +167,15 @@ static void mxc_flip_edge(struct mxc_gpio_port *port, u32 gpio)
 static void mxc_gpio_irq_handler(struct mxc_gpio_port *port, u32 irq_stat)
 {
 	u32 gpio_irq_no_base = port->virtual_irq_start;
+	unsigned long flags;
 
 	while (irq_stat != 0) {
 		int irqoffset = fls(irq_stat) - 1;
 
+		spin_lock_irqsave(&port->irq_lock, flags);
 		if (port->both_edges & (1 << irqoffset))
 			mxc_flip_edge(port, irqoffset);
+		spin_unlock_irqrestore(&port->irq_lock, flags);
 
 		generic_handle_irq(gpio_irq_no_base + irqoffset);
 

@@ -214,13 +227,16 @@ static void _set_gpio_direction(struct gpio_chip *chip, unsigned offset,
 	struct mxc_gpio_port *port =
 		container_of(chip, struct mxc_gpio_port, chip);
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = __raw_readl(port->base + GPIO_GDIR);
 	if (dir)
 		l |= 1 << offset;
 	else
 		l &= ~(1 << offset);
 	__raw_writel(l, port->base + GPIO_GDIR);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -229,9 +245,12 @@ static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 		container_of(chip, struct mxc_gpio_port, chip);
 	void __iomem *reg = port->base + GPIO_DR;
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = (__raw_readl(reg) & (~(1 << offset))) | (value << offset);
 	__raw_writel(l, reg);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static int mxc_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -285,6 +304,9 @@ int __init mxc_gpio_init(struct mxc_gpio_port *port, int cnt)
 		port[i].chip.base = i * 32;
 		port[i].chip.ngpio = 32;
 
+		spin_lock_init(&port[i].lock);
+		spin_lock_init(&port[i].irq_lock);
+
 		/* its a serious configuration bug when it fails */
 		BUG_ON( gpiochip_add(&port[i].chip) < 0 );
 

arch/arm/plat-mxc/include/mach/gpio.h +3 -0

index 894d2f8..a37724a 100644
@@ -36,6 +36,9 @@ struct mxc_gpio_port {
 	int virtual_irq_start;
 	struct gpio_chip chip;
 	u32 both_edges;
+
+	spinlock_t	lock;		/* GPIO registers */
+	spinlock_t	irq_lock;	/* IRQ registers */
 };
 
 int mxc_gpio_init(struct mxc_gpio_port*, int);

Signature · 1 line

1.7.1

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Hi Sascha,

Quoted 4 lines · Sascha Hauer

Quoted 2 lines · Baruch Siach

The GPIO and IRQ/GPIO registers need protection from concurrent access for operations that are not atomic.

Quoted 4 lines

I don't think we need locking here. mxc_gpio_irq_handler is called with desc->lock held (from the parent interrupt, not the chained interrupts). Other functions like enable_irq/disable_irq which result in mask/unmask operations run with interrupts disabled.

What about the .set_type method?

Adding David Brownell to CC.

Quoted 1 line

Apart from this other architectures do not use locking here aswell.

The Nomadic gpio driver does use a spinlock for mask/unmask operations.

What about the _set_gpio_direction, and mxc_gpio_set? These functions may be called from a process context (e.g., via sysfs). A context switch between __raw_readl and __raw_writel will cause corruption.

Quoted 5 lines · Baruch Siach

Quoted 4 lines · Sascha Hauer

Quoted 2 lines · Baruch Siach

The GPIO and IRQ/GPIO registers need protection from concurrent access for operations that are not atomic.

Quoted 6 lines

Quoted 4 lines

I don't think we need locking here. mxc_gpio_irq_handler is called with desc->lock held (from the parent interrupt, not the chained interrupts). Other functions like enable_irq/disable_irq which result in mask/unmask operations run with interrupts disabled.

What about the .set_type method?

Is only called with interrupts disabled.

Quoted 7 lines

Quoted 1 line

Apart from this other architectures do not use locking here aswell.

The Nomadic gpio driver does use a spinlock for mask/unmask operations.

What about the _set_gpio_direction, and mxc_gpio_set? These functions may be called from a process context (e.g., via sysfs). A context switch between __raw_readl and __raw_writel will cause corruption.

The gpio_chip functions are protected by a single spinlock in gpiolib.

The gpio related registers and the irq related regsiters are totally orthogonal, so we need no locking between these registers.

Hi Sascha,

Quoted 6 lines · Sascha Hauer

Quoted 5 lines · Baruch Siach

Quoted 4 lines · Sascha Hauer

Quoted 2 lines · Baruch Siach

The GPIO and IRQ/GPIO registers need protection from concurrent access for operations that are not atomic.

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

I don't think we need locking here. mxc_gpio_irq_handler is called with desc->lock held (from the parent interrupt, not the chained interrupts). Other functions like enable_irq/disable_irq which result in mask/unmask operations run with interrupts disabled.

What about the .set_type method?

Is only called with interrupts disabled.

OK.

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

Apart from this other architectures do not use locking here aswell.

The Nomadic gpio driver does use a spinlock for mask/unmask operations.

What about the _set_gpio_direction, and mxc_gpio_set? These functions may be called from a process context (e.g., via sysfs). A context switch between __raw_readl and __raw_writel will cause corruption.

Quoted 2 lines

The gpio_chip functions are protected by a single spinlock in gpiolib.

gpio_direction_input uses the gpio_lock for its own internal sanity check, and releases it before calling chip->direction_input. The same goes for gpio_direction_output.

The __gpio_set_value function seems not acquire any lock before calling chip->set.

Quoted 2 lines

The gpio related registers and the irq related regsiters are totally orthogonal, so we need no locking between these registers.

True.

Quoted 14 lines · Baruch Siach

Quoted 13 lines · Sascha Hauer

Quoted 11 lines · Baruch Siach

Quoted 4 lines · Sascha Hauer

Quoted 2 lines · Baruch Siach

The GPIO and IRQ/GPIO registers need protection from concurrent access for operations that are not atomic.

Apart from this other architectures do not use locking here aswell.

The Nomadic gpio driver does use a spinlock for mask/unmask operations.

What about the _set_gpio_direction, and mxc_gpio_set? These functions may be called from a process context (e.g., via sysfs). A context switch between __raw_readl and __raw_writel will cause corruption.

Quoted 6 lines

Quoted 2 lines

The gpio_chip functions are protected by a single spinlock in gpiolib.

gpio_direction_input uses the gpio_lock for its own internal sanity check, and releases it before calling chip->direction_input. The same goes for gpio_direction_output.

Ok, true.

Quoted 4 lines

The __gpio_set_value function seems not acquire any lock before calling chip->set.

Quoted 4 lines

Quoted 2 lines

The gpio related registers and the irq related regsiters are totally orthogonal, so we need no locking between these registers.

True.

This means we need locking for the gpio functions but not for the irq functions.

The GPIO registers need protection from concurrent access for operations that are not atomic.

Cc stable at kernel.org

Cc Juergen Beisert <[email protected]>

Cc Daniel Mack <[email protected]>

Reported-by rpkamiak at rockwellcollins.com

Signed-off-by Baruch Siach <[email protected]>

---
Changes v1 -> v2

Remove redundant protection of GPIO/IRQ registers

 arch/arm/plat-mxc/gpio.c              |    8 ++++++++
 arch/arm/plat-mxc/include/mach/gpio.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

arch/arm/plat-mxc/gpio.c +8 -0

index 71437c6..9ebbd31 100644
@@ -214,13 +214,16 @@ static void _set_gpio_direction(struct gpio_chip *chip, unsigned offset,
 	struct mxc_gpio_port *port =
 		container_of(chip, struct mxc_gpio_port, chip);
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = __raw_readl(port->base + GPIO_GDIR);
 	if (dir)
 		l |= 1 << offset;
 	else
 		l &= ~(1 << offset);
 	__raw_writel(l, port->base + GPIO_GDIR);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -229,9 +232,12 @@ static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 		container_of(chip, struct mxc_gpio_port, chip);
 	void __iomem *reg = port->base + GPIO_DR;
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = (__raw_readl(reg) & (~(1 << offset))) | (value << offset);
 	__raw_writel(l, reg);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static int mxc_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -285,6 +291,8 @@ int __init mxc_gpio_init(struct mxc_gpio_port *port, int cnt)
 		port[i].chip.base = i * 32;
 		port[i].chip.ngpio = 32;
 
+		spin_lock_init(&port[i].lock);
+
 		/* its a serious configuration bug when it fails */
 		BUG_ON( gpiochip_add(&port[i].chip) < 0 );
 

arch/arm/plat-mxc/include/mach/gpio.h +1 -0

index 894d2f8..6bd932c 100644
@@ -36,6 +36,7 @@ struct mxc_gpio_port {
 	int virtual_irq_start;
 	struct gpio_chip chip;
 	u32 both_edges;
+	spinlock_t lock;
 };
 
 int mxc_gpio_init(struct mxc_gpio_port*, int);

Signature · 1 line

1.7.1

The GPIO registers need protection from concurrent access for operations that are not atomic.

Cc stable at kernel.org

Cc Juergen Beisert <[email protected]>

Cc Daniel Mack <[email protected]>

Reported-by rpkamiak at rockwellcollins.com

Signed-off-by Baruch Siach <[email protected]>

---
Changes v1 -> v2

Remove redundant protection of GPIO/IRQ registers

 arch/arm/plat-mxc/gpio.c              |    8 ++++++++
 arch/arm/plat-mxc/include/mach/gpio.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

arch/arm/plat-mxc/gpio.c +8 -0

index 71437c6..9ebbd31 100644
@@ -214,13 +214,16 @@ static void _set_gpio_direction(struct gpio_chip *chip, unsigned offset,
 	struct mxc_gpio_port *port =
 		container_of(chip, struct mxc_gpio_port, chip);
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = __raw_readl(port->base + GPIO_GDIR);
 	if (dir)
 		l |= 1 << offset;
 	else
 		l &= ~(1 << offset);
 	__raw_writel(l, port->base + GPIO_GDIR);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

@@ -229,9 +232,12 @@ static void mxc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 		container_of(chip, struct mxc_gpio_port, chip);
 	void __iomem *reg = port->base + GPIO_DR;
 	u32 l;
+	unsigned long flags;
 
+	spin_lock_irqsave(&port->lock, flags);
 	l = (__raw_readl(reg) & (~(1 << offset))) | (value << offset);
 	__raw_writel(l, reg);
+	spin_unlock_irqrestore(&port->lock, flags);
 }
 
 static int mxc_gpio_get(struct gpio_chip *chip, unsigned offset)

@@ -285,6 +291,8 @@ int __init mxc_gpio_init(struct mxc_gpio_port *port, int cnt)
 		port[i].chip.base = i * 32;
 		port[i].chip.ngpio = 32;
 
+		spin_lock_init(&port[i].lock);
+
 		/* its a serious configuration bug when it fails */
 		BUG_ON( gpiochip_add(&port[i].chip) < 0 );
 

arch/arm/plat-mxc/include/mach/gpio.h +0 -0

index 894d2f8..6bd932c 100644

This is a follow up to

14cb0de (arm/imx/gpio: add spinlock protection)

and fixes the following build failure:

CC arch/arm/mach-imx/pcm970-baseboard.o
In file included from arch/arm/include/asm/gpio.h:6,
from include/linux/gpio.h:8,
from arch/arm/mach-imx/pcm970-baseboard.c:20:
arch/arm/plat-mxc/include/mach/gpio.h:40: error: expected specifier-qualifier-list before 'spinlock_t'

Signed-off-by Uwe Kleine-K?nig <[email protected]>

Cc stable at kernel.org

---
Kernel-Version: v2.6.35-rc1-133-g14cb0de

Hello,

as 14cb0de has a Cc: for stable, I added it here, too.

Thanks
Uwe

 arch/arm/plat-mxc/include/mach/gpio.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/plat-mxc/include/mach/gpio.h +1 -0

index 661fbc6..af33b74 100644
@@ -19,6 +19,7 @@
 #ifndef __ASM_ARCH_MXC_GPIO_H__
 #define __ASM_ARCH_MXC_GPIO_H__
 
+#include <linux/spinlock.h>
 #include <mach/hardware.h>
 #include <asm-generic/gpio.h>