mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] pch_gpio: transform mutex into spinlock
@ 2011-06-21 14:17 Alexander Stein
  2011-06-27 22:47 ` Grant Likely
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Stein @ 2011-06-21 14:17 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-kernel, Tomoya MORINAGA, Alexander Stein

gpio_chip.can_sleep is 0, but gpio_set_value & friends lock a mutex. Thus
those functions are not callable from interrupt context.
Changing mutex into spinlock.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
I just noticed that the patch introduced at
http://marc.info/?l=linux-kernel&m=130863151431202&w=2
also added a spinlock. I think those two spinlock can be merged into one.

 drivers/gpio/pch_gpio.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/pch_gpio.c b/drivers/gpio/pch_gpio.c
index 36919e7..262c200 100644
--- a/drivers/gpio/pch_gpio.c
+++ b/drivers/gpio/pch_gpio.c
@@ -62,15 +62,16 @@ struct pch_gpio {
 	struct device *dev;
 	struct gpio_chip gpio;
 	struct pch_gpio_reg_data pch_gpio_reg;
-	struct mutex lock;
+	struct spinlock lock;
 };
 
 static void pch_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
 {
 	u32 reg_val;
+	unsigned long flags;
 	struct pch_gpio *chip =	container_of(gpio, struct pch_gpio, gpio);
 
-	mutex_lock(&chip->lock);
+	spin_lock_irqsave(&chip->lock, flags);
 	reg_val = ioread32(&chip->reg->po);
 	if (val)
 		reg_val |= (1 << nr);
@@ -78,7 +79,7 @@ static void pch_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
 		reg_val &= ~(1 << nr);
 
 	iowrite32(reg_val, &chip->reg->po);
-	mutex_unlock(&chip->lock);
+	spin_unlock_irqrestore(&chip->lock, flags);
 }
 
 static int pch_gpio_get(struct gpio_chip *gpio, unsigned nr)
@@ -94,8 +95,9 @@ static int pch_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 	struct pch_gpio *chip =	container_of(gpio, struct pch_gpio, gpio);
 	u32 pm;
 	u32 reg_val;
+	unsigned long flags;
 
-	mutex_lock(&chip->lock);
+	spin_lock_irqsave(&chip->lock, flags);
 	pm = ioread32(&chip->reg->pm) & PCH_GPIO_ALL_PINS;
 	pm |= (1 << nr);
 	iowrite32(pm, &chip->reg->pm);
@@ -107,7 +109,7 @@ static int pch_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
 		reg_val &= ~(1 << nr);
 	iowrite32(reg_val, &chip->reg->po);
 
-	mutex_unlock(&chip->lock);
+	spin_unlock_irqrestore(&chip->lock, flags);
 
 	return 0;
 }
@@ -116,12 +118,13 @@ static int pch_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
 {
 	struct pch_gpio *chip =	container_of(gpio, struct pch_gpio, gpio);
 	u32 pm;
+	unsigned long flags;
 
-	mutex_lock(&chip->lock);
+	spin_lock_irqsave(&chip->lock, flags);
 	pm = ioread32(&chip->reg->pm) & PCH_GPIO_ALL_PINS; /*bits 0-11*/
 	pm &= ~(1 << nr);
 	iowrite32(pm, &chip->reg->pm);
-	mutex_unlock(&chip->lock);
+	spin_unlock_irqrestore(&chip->lock, flags);
 
 	return 0;
 }
@@ -194,7 +197,7 @@ static int __devinit pch_gpio_probe(struct pci_dev *pdev,
 
 	chip->reg = chip->base;
 	pci_set_drvdata(pdev, chip);
-	mutex_init(&chip->lock);
+	spin_lock_init(&chip->lock);
 	pch_gpio_setup(chip);
 	ret = gpiochip_add(&chip->gpio);
 	if (ret) {
-- 
1.7.3.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] pch_gpio: transform mutex into spinlock
  2011-06-21 14:17 [PATCH] pch_gpio: transform mutex into spinlock Alexander Stein
@ 2011-06-27 22:47 ` Grant Likely
  2011-06-28  1:02   ` Tomoya MORINAGA
  0 siblings, 1 reply; 3+ messages in thread
From: Grant Likely @ 2011-06-27 22:47 UTC (permalink / raw)
  To: Alexander Stein; +Cc: linux-kernel, Tomoya MORINAGA

On Tue, Jun 21, 2011 at 04:17:27PM +0200, Alexander Stein wrote:
> gpio_chip.can_sleep is 0, but gpio_set_value & friends lock a mutex. Thus
> those functions are not callable from interrupt context.
> Changing mutex into spinlock.
> 
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>

Tomoya, what do you think of this patch?

> ---
> I just noticed that the patch introduced at
> http://marc.info/?l=linux-kernel&m=130863151431202&w=2
> also added a spinlock. I think those two spinlock can be merged into one.
> 
>  drivers/gpio/pch_gpio.c |   19 +++++++++++--------
>  1 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpio/pch_gpio.c b/drivers/gpio/pch_gpio.c
> index 36919e7..262c200 100644
> --- a/drivers/gpio/pch_gpio.c
> +++ b/drivers/gpio/pch_gpio.c
> @@ -62,15 +62,16 @@ struct pch_gpio {
>  	struct device *dev;
>  	struct gpio_chip gpio;
>  	struct pch_gpio_reg_data pch_gpio_reg;
> -	struct mutex lock;
> +	struct spinlock lock;
>  };
>  
>  static void pch_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
>  {
>  	u32 reg_val;
> +	unsigned long flags;
>  	struct pch_gpio *chip =	container_of(gpio, struct pch_gpio, gpio);
>  
> -	mutex_lock(&chip->lock);
> +	spin_lock_irqsave(&chip->lock, flags);
>  	reg_val = ioread32(&chip->reg->po);
>  	if (val)
>  		reg_val |= (1 << nr);
> @@ -78,7 +79,7 @@ static void pch_gpio_set(struct gpio_chip *gpio, unsigned nr, int val)
>  		reg_val &= ~(1 << nr);
>  
>  	iowrite32(reg_val, &chip->reg->po);
> -	mutex_unlock(&chip->lock);
> +	spin_unlock_irqrestore(&chip->lock, flags);
>  }
>  
>  static int pch_gpio_get(struct gpio_chip *gpio, unsigned nr)
> @@ -94,8 +95,9 @@ static int pch_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
>  	struct pch_gpio *chip =	container_of(gpio, struct pch_gpio, gpio);
>  	u32 pm;
>  	u32 reg_val;
> +	unsigned long flags;
>  
> -	mutex_lock(&chip->lock);
> +	spin_lock_irqsave(&chip->lock, flags);
>  	pm = ioread32(&chip->reg->pm) & PCH_GPIO_ALL_PINS;
>  	pm |= (1 << nr);
>  	iowrite32(pm, &chip->reg->pm);
> @@ -107,7 +109,7 @@ static int pch_gpio_direction_output(struct gpio_chip *gpio, unsigned nr,
>  		reg_val &= ~(1 << nr);
>  	iowrite32(reg_val, &chip->reg->po);
>  
> -	mutex_unlock(&chip->lock);
> +	spin_unlock_irqrestore(&chip->lock, flags);
>  
>  	return 0;
>  }
> @@ -116,12 +118,13 @@ static int pch_gpio_direction_input(struct gpio_chip *gpio, unsigned nr)
>  {
>  	struct pch_gpio *chip =	container_of(gpio, struct pch_gpio, gpio);
>  	u32 pm;
> +	unsigned long flags;
>  
> -	mutex_lock(&chip->lock);
> +	spin_lock_irqsave(&chip->lock, flags);
>  	pm = ioread32(&chip->reg->pm) & PCH_GPIO_ALL_PINS; /*bits 0-11*/
>  	pm &= ~(1 << nr);
>  	iowrite32(pm, &chip->reg->pm);
> -	mutex_unlock(&chip->lock);
> +	spin_unlock_irqrestore(&chip->lock, flags);
>  
>  	return 0;
>  }
> @@ -194,7 +197,7 @@ static int __devinit pch_gpio_probe(struct pci_dev *pdev,
>  
>  	chip->reg = chip->base;
>  	pci_set_drvdata(pdev, chip);
> -	mutex_init(&chip->lock);
> +	spin_lock_init(&chip->lock);
>  	pch_gpio_setup(chip);
>  	ret = gpiochip_add(&chip->gpio);
>  	if (ret) {
> -- 
> 1.7.3.4
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] pch_gpio: transform mutex into spinlock
  2011-06-27 22:47 ` Grant Likely
@ 2011-06-28  1:02   ` Tomoya MORINAGA
  0 siblings, 0 replies; 3+ messages in thread
From: Tomoya MORINAGA @ 2011-06-28  1:02 UTC (permalink / raw)
  To: Grant Likely; +Cc: Alexander Stein, linux-kernel, toshiharu-linux

(2011/06/28 7:47), Grant Likely wrote:
> On Tue, Jun 21, 2011 at 04:17:27PM +0200, Alexander Stein wrote:
>> gpio_chip.can_sleep is 0, but gpio_set_value&  friends lock a mutex. Thus
>> those functions are not callable from interrupt context.
>> Changing mutex into spinlock.
>>
>> Signed-off-by: Alexander Stein<alexander.stein@systec-electronic.com>
>
> Tomoya, what do you think of this patch?
>

Seeing some upstreamed GPIO driver, Alexander's comment looks right.

Best Regards,

tomoya
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-06-28  1:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-21 14:17 [PATCH] pch_gpio: transform mutex into spinlock Alexander Stein
2011-06-27 22:47 ` Grant Likely
2011-06-28  1:02   ` Tomoya MORINAGA

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome