[RFC PATCH] arm/imx/gpio: add spinlock protection

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Baruch Siach]

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

Cc: Juergen Beisert <j.beisert@pengutronix.de>
Cc: Daniel Mack <daniel@caiaq.de>
Reported-by: rpkamiak at rockwellcollins.com
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 arch/arm/plat-mxc/gpio.c              |   28 +++++++++++++++++++++++++---
 arch/arm/plat-mxc/include/mach/gpio.h |    3 +++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
index 71437c6..a8a33cd 100644
--- a/arch/arm/plat-mxc/gpio.c
+++ b/arch/arm/plat-mxc/gpio.c
@@ -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 );
 
diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
index 894d2f8..a37724a 100644
--- a/arch/arm/plat-mxc/include/mach/gpio.h
+++ b/arch/arm/plat-mxc/include/mach/gpio.h
@@ -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);
-- 
1.7.1

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Uwe Kleine-König]

Hello,

[I added David to Cc and so didn't strip the patch.]

On Sun, Jul 04, 2010 at 10:15:13AM +0300, Baruch Siach wrote:
> The GPIO and IRQ/GPIO registers need protection from concurrent access for
> operations that are not atomic.
> 
> Cc: Juergen Beisert <j.beisert@pengutronix.de>
> Cc: Daniel Mack <daniel@caiaq.de>
> Reported-by: rpkamiak at rockwellcollins.com
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
I think this is OK, I just wonder if it's worth to introduce two
spinlocks.

Best regards
Uwe

> ---
>  arch/arm/plat-mxc/gpio.c              |   28 +++++++++++++++++++++++++---
>  arch/arm/plat-mxc/include/mach/gpio.h |    3 +++
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
> index 71437c6..a8a33cd 100644
> --- a/arch/arm/plat-mxc/gpio.c
> +++ b/arch/arm/plat-mxc/gpio.c
> @@ -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 );
>  
> diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
> index 894d2f8..a37724a 100644
> --- a/arch/arm/plat-mxc/include/mach/gpio.h
> +++ b/arch/arm/plat-mxc/include/mach/gpio.h
> @@ -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);
> -- 
> 1.7.1

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

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Sascha Hauer]

On Sun, Jul 04, 2010 at 10:15:13AM +0300, Baruch Siach wrote:
> 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.

Sascha

> 
> Cc: Juergen Beisert <j.beisert@pengutronix.de>
> Cc: Daniel Mack <daniel@caiaq.de>
> Reported-by: rpkamiak at rockwellcollins.com
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  arch/arm/plat-mxc/gpio.c              |   28 +++++++++++++++++++++++++---
>  arch/arm/plat-mxc/include/mach/gpio.h |    3 +++
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
> index 71437c6..a8a33cd 100644
> --- a/arch/arm/plat-mxc/gpio.c
> +++ b/arch/arm/plat-mxc/gpio.c
> @@ -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 );
>  
> diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
> index 894d2f8..a37724a 100644
> --- a/arch/arm/plat-mxc/include/mach/gpio.h
> +++ b/arch/arm/plat-mxc/include/mach/gpio.h
> @@ -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);
> -- 
> 1.7.1
> 
> 

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

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Baruch Siach]

Hi Sascha,

On Mon, Jul 05, 2010 at 09:52:18AM +0200, Sascha Hauer wrote:
> On Sun, Jul 04, 2010 at 10:15:13AM +0300, Baruch Siach wrote:
> > 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.

What about the .set_type method?

Adding David Brownell to CC.

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

baruch

> > Cc: Juergen Beisert <j.beisert@pengutronix.de>
> > Cc: Daniel Mack <daniel@caiaq.de>
> > Reported-by: rpkamiak at rockwellcollins.com
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> >  arch/arm/plat-mxc/gpio.c              |   28 +++++++++++++++++++++++++---
> >  arch/arm/plat-mxc/include/mach/gpio.h |    3 +++
> >  2 files changed, 28 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
> > index 71437c6..a8a33cd 100644
> > --- a/arch/arm/plat-mxc/gpio.c
> > +++ b/arch/arm/plat-mxc/gpio.c
> > @@ -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 );
> >  
> > diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
> > index 894d2f8..a37724a 100644
> > --- a/arch/arm/plat-mxc/include/mach/gpio.h
> > +++ b/arch/arm/plat-mxc/include/mach/gpio.h
> > @@ -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);
> > -- 
> > 1.7.1
> > 
> > 
> 
> -- 
> 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 |

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Sascha Hauer]

On Tue, Jul 06, 2010 at 08:00:34AM +0300, Baruch Siach wrote:
> Hi Sascha,
> 
> On Mon, Jul 05, 2010 at 09:52:18AM +0200, Sascha Hauer wrote:
> > On Sun, Jul 04, 2010 at 10:15:13AM +0300, Baruch Siach wrote:
> > > 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.
> 
> What about the .set_type method?

Is only called with interrupts disabled.

> 
> Adding David Brownell to CC.
> 
> > 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.

Sascha

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

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Baruch Siach]

Hi Sascha,

On Tue, Jul 06, 2010 at 09:17:02AM +0200, Sascha Hauer wrote:
> On Tue, Jul 06, 2010 at 08:00:34AM +0300, Baruch Siach wrote:
> > On Mon, Jul 05, 2010 at 09:52:18AM +0200, Sascha Hauer wrote:
> > > On Sun, Jul 04, 2010 at 10:15:13AM +0300, Baruch Siach wrote:
> > > > 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.
> > 
> > What about the .set_type method?
> 
> Is only called with interrupts disabled.

OK.

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

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.

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

True.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Sascha Hauer]

On Tue, Jul 06, 2010 at 10:40:43AM +0300, Baruch Siach wrote:
> Hi Sascha,
> 
> On Tue, Jul 06, 2010 at 09:17:02AM +0200, Sascha Hauer wrote:
> > On Tue, Jul 06, 2010 at 08:00:34AM +0300, Baruch Siach wrote:
> > > On Mon, Jul 05, 2010 at 09:52:18AM +0200, Sascha Hauer wrote:
> > > > On Sun, Jul 04, 2010 at 10:15:13AM +0300, Baruch Siach wrote:
> > > > > 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.
> > > 
> > > What about the .set_type method?
> > 
> > Is only called with interrupts disabled.
> 
> OK.
> 
> > > > 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.
> 
> 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.

> 
> The __gpio_set_value function seems not acquire any lock before calling 
> chip->set.
> 
> > 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.

Sascha


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

[RFC PATCH] arm/imx/gpio: add spinlock protection

[Baruch Siach]

On Tue, Jul 06, 2010 at 12:07:48PM +0200, Sascha Hauer wrote:
> On Tue, Jul 06, 2010 at 10:40:43AM +0300, Baruch Siach wrote:
> > On Tue, Jul 06, 2010 at 09:17:02AM +0200, Sascha Hauer wrote:
> > > On Tue, Jul 06, 2010 at 08:00:34AM +0300, Baruch Siach wrote:
> > > > On Mon, Jul 05, 2010 at 09:52:18AM +0200, Sascha Hauer wrote:
> > > > > On Sun, Jul 04, 2010 at 10:15:13AM +0300, Baruch Siach wrote:
> > > > > > 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.
> > > 
> > > 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.
> 
> > 
> > The __gpio_set_value function seems not acquire any lock before calling 
> > chip->set.
> > 
> > > 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.

Agreed. I'll post an updated patch shortly.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[PATCH v2] arm/imx/gpio: add spinlock protection

[Baruch Siach]

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

Cc: stable at kernel.org
Cc: Juergen Beisert <j.beisert@pengutronix.de>
Cc: Daniel Mack <daniel@caiaq.de>
Reported-by: rpkamiak at rockwellcollins.com
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
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(-)

diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
index 71437c6..9ebbd31 100644
--- a/arch/arm/plat-mxc/gpio.c
+++ b/arch/arm/plat-mxc/gpio.c
@@ -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 );
 
diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
index 894d2f8..6bd932c 100644
--- a/arch/arm/plat-mxc/include/mach/gpio.h
+++ b/arch/arm/plat-mxc/include/mach/gpio.h
@@ -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);
-- 
1.7.1

[PATCH v2] arm/imx/gpio: add spinlock protection

[Baruch Siach]

Hi Sascha,

On Tue, Jul 06, 2010 at 02:03:22PM +0300, Baruch Siach wrote:
> The GPIO registers need protection from concurrent access for operations that
> are not atomic.

Ping?
This looks like an -rc and -stable issue to me.

> Cc: stable at kernel.org
> Cc: Juergen Beisert <j.beisert@pengutronix.de>
> Cc: Daniel Mack <daniel@caiaq.de>
> Reported-by: rpkamiak at rockwellcollins.com
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 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(-)
> 
> diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
> index 71437c6..9ebbd31 100644
> --- a/arch/arm/plat-mxc/gpio.c
> +++ b/arch/arm/plat-mxc/gpio.c
> @@ -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 );
>  
> diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
> index 894d2f8..6bd932c 100644
> --- a/arch/arm/plat-mxc/include/mach/gpio.h
> +++ b/arch/arm/plat-mxc/include/mach/gpio.h
> @@ -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);
> -- 
> 1.7.1
> 

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[PATCH v2] arm/imx/gpio: add spinlock protection

[Uwe Kleine-König]

Hey Sascha,

On Tue, Jul 06, 2010 at 02:03:22PM +0300, Baruch Siach wrote:
> The GPIO registers need protection from concurrent access for operations that
> are not atomic.
> 
> Cc: stable at kernel.org
> Cc: Juergen Beisert <j.beisert@pengutronix.de>
> Cc: Daniel Mack <daniel@caiaq.de>
> Reported-by: rpkamiak at rockwellcollins.com
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> 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(-)
> 
> diff --git a/arch/arm/plat-mxc/gpio.c b/arch/arm/plat-mxc/gpio.c
> index 71437c6..9ebbd31 100644
> --- a/arch/arm/plat-mxc/gpio.c
> +++ b/arch/arm/plat-mxc/gpio.c
> @@ -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 );
>  
> diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
> index 894d2f8..6bd932c 100644
> --- a/arch/arm/plat-mxc/include/mach/gpio.h
> +++ b/arch/arm/plat-mxc/include/mach/gpio.h
> @@ -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);

can you please fold this into Baruch's patch:

diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
index 661fbc6..af33b74 100644
--- a/arch/arm/plat-mxc/include/mach/gpio.h
+++ b/arch/arm/plat-mxc/include/mach/gpio.h
@@ -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>
 
This fixes a build failure with mx27_defconfig.

Best regards
Uwe

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

[PATCH] ARM: imx: Fix build failure when including <mach/gpio.h> without <linux/spinlock.h>

[Uwe Kleine-König]

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 <u.kleine-koenig@pengutronix.de>
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(-)

diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
index 661fbc6..af33b74 100644
--- a/arch/arm/plat-mxc/include/mach/gpio.h
+++ b/arch/arm/plat-mxc/include/mach/gpio.h
@@ -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>
 
-- 
1.7.1

[PATCH] ARM: imx: Fix build failure when including <mach/gpio.h> without <linux/spinlock.h>

[Baruch Siach]

Hi Uwe,

On Mon, Aug 02, 2010 at 09:29:43AM +0200, Uwe Kleine-K?nig wrote:
> 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 <u.kleine-koenig@pengutronix.de>

Acked-by: Baruch Siach <baruch@tkos.co.il>

Thanks,
baruch

> 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(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/gpio.h b/arch/arm/plat-mxc/include/mach/gpio.h
> index 661fbc6..af33b74 100644
> --- a/arch/arm/plat-mxc/include/mach/gpio.h
> +++ b/arch/arm/plat-mxc/include/mach/gpio.h
> @@ -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>

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -