Skip to contentSkip to conversation

GPIO PL061: Adding Clk framework support

PATCH Superseded Viresh KUMAR 4 participants 6 messages Jun 21, 2010 – Jun 22, 2010 GPIO

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Viresh KUMAR

author
#0 lore raw

PATCH v2 1/1 GPIO PL061: Adding Clk framework support

From: Viresh KUMAR <[email protected]>

GPIO Clk is never enabled on Platforms, like: SPEAr, which are based upon clock framework and use PL061 driver. This patch adds support for Clk enabling and disabling as and when gpios are requested and freed.

Modifications since V1:
- Removed usage_count, as already done by clk framework.
- Checking error case of clk_enable.

Signed-off-by Viresh Kumar <[email protected]>
---
 drivers/gpio/pl061.c |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)
drivers/gpio/pl061.c +34 -3
index 105701a..fd37349 100644
@@ -11,7 +11,10 @@
  *
  * Data sheet: ARM DDI 0190B, September 2000
  */
+
+#include <linux/clk.h>
 #include <linux/spinlock.h>
+#include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/list.h>

@@ -56,8 +59,26 @@ struct pl061_gpio {
 	void __iomem		*base;
 	unsigned		irq_base;
 	struct gpio_chip	gc;
+	struct clk		*clk;
 };
 
+static int pl061_request(struct gpio_chip *gc, unsigned offset)
+{
+	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
+
+	if (offset >= gc->ngpio)
+		return -EINVAL;
+
+	return clk_enable(chip->clk);
+}
+
+static void pl061_free(struct gpio_chip *gc, unsigned offset)
+{
+	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
+
+	clk_disable(chip->clk);
+}
+
 static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
 {
 	struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);

@@ -260,10 +281,18 @@ static int __init pl061_probe(struct amba_device *dev, struct amba_id *id)
 		goto release_region;
 	}
 
+	chip->clk = clk_get(&dev->dev, NULL);
+	if (IS_ERR(chip->clk)) {
+		ret = PTR_ERR(chip->clk);
+		goto iounmap;
+	}
+
 	spin_lock_init(&chip->lock);
 	spin_lock_init(&chip->irq_lock);
 	INIT_LIST_HEAD(&chip->list);
 
+	chip->gc.request = pl061_request;
+	chip->gc.free = pl061_free;
 	chip->gc.direction_input = pl061_direction_input;
 	chip->gc.direction_output = pl061_direction_output;
 	chip->gc.get = pl061_get_value;

@@ -279,7 +308,7 @@ static int __init pl061_probe(struct amba_device *dev, struct amba_id *id)
 
 	ret = gpiochip_add(&chip->gc);
 	if (ret)
-		goto iounmap;
+		goto free_clk;
 
 	/*
 	 * irq_chip support

@@ -292,7 +321,7 @@ static int __init pl061_probe(struct amba_device *dev, struct amba_id *id)
 	irq = dev->irq[0];
 	if (irq < 0) {
 		ret = -ENODEV;
-		goto iounmap;
+		goto free_clk;
 	}
 	set_irq_chained_handler(irq, pl061_irq_handler);
 	if (!test_and_set_bit(irq, init_irq)) { /* list initialized? */

@@ -300,7 +329,7 @@ static int __init pl061_probe(struct amba_device *dev, struct amba_id *id)
 		if (chip_list == NULL) {
 			clear_bit(irq, init_irq);
 			ret = -ENOMEM;
-			goto iounmap;
+			goto free_clk;
 		}
 		INIT_LIST_HEAD(chip_list);
 		set_irq_data(irq, chip_list);

@@ -323,6 +352,8 @@ static int __init pl061_probe(struct amba_device *dev, struct amba_id *id)
 
 	return 0;
 
+free_clk:
+	clk_put(chip->clk);
 iounmap:
 	iounmap(chip->base);
 release_region:
Signature · 1 line
1.6.0.2
#1 lore raw

PATCH v2 1/1 GPIO PL061: Adding Clk framework support

Quoted 3 lines · Viresh KUMAR

GPIO Clk is never enabled on Platforms, like: SPEAr, which are based upon clock framework and use PL061 driver. This patch adds support for Clk enabling and disabling as and when gpios are requested and freed.

[...]

drivers/gpio/pl061.c static int __init pl061_probe(struct amba_device *dev, struct amba_id *id) · line 288

Quoted 5 lines
drivers/gpio/pl061.c +5 -0
+	chip->clk = clk_get(&dev->dev, NULL);
+	if (IS_ERR(chip->clk)) {
+		ret = PTR_ERR(chip->clk);
+		goto iounmap;
+	}

Have you verified that all platforms using this driver already have
clocks with the appropriate names? Otherwise this patch will break
those platforms.

Rabin

#2 lore raw

PATCH v2 1/1 GPIO PL061: Adding Clk framework support

2010/6/21 Rabin Vincent <[email protected]>:

Quoted 11 lines · Viresh KUMAR
Quoted 3 lines

GPIO Clk is never enabled on Platforms, like: SPEAr, which are based upon clock framework and use PL061 driver. This patch adds support for Clk enabling and disabling as and when gpios are requested and freed.

[...]

Quoted 5 lines

+ ? ? chip->clk = clk_get(&dev->dev, NULL);
+ ? ? if (IS_ERR(chip->clk)) {
+ ? ? ? ? ? ? ret = PTR_ERR(chip->clk);
+ ? ? ? ? ? ? goto iounmap;
+ ? ? }

Quoted 3 lines

Have you verified that all platforms using this driver already have clocks with the appropriate names? ? Otherwise this patch will break those platforms.

The ARM Versatiles and RealViews are always clocked I think, so clock support should be optional.

When I added a clock lookup to the PL08x block recently I used a construct like this for an optional clock:

chip->clk = clk_get(&dev->dev, NULL);

if (IS_ERR(chip->clk)) {
       ret = PTR_ERR(chip->clk);
       if (ret == -ENOENT)
          /* No block clock in this platform */
          chip->clk = NULL;
       else
          goto iounmap;
}

This will work whenever clkdevice is used, since that returns -ENOENT for nonexisting entries.

Then:

if (chip->clk)
clk_enable(chip->clk);

etc

Yours,
Linus Walleij

author
#3 lore raw

PATCH v2 1/1 GPIO PL061: Adding Clk framework support

Quoted 28 lines · Linus Walleij

2010/6/21 Rabin Vincent <[email protected]>:

Quoted 10 lines · Viresh KUMAR

drivers/gpio/pl061.c static int __init pl061_probe(struct amba_device *dev, struct amba_id *id) · line 288

Quoted 5 lines
drivers/gpio/pl061.c +5 -0
+     chip->clk = clk_get(&dev->dev, NULL);
+     if (IS_ERR(chip->clk)) {
+             ret = PTR_ERR(chip->clk);
+             goto iounmap;
+     }

Have you verified that all platforms using this driver already have
clocks with the appropriate names? Otherwise this patch will break
those platforms.

The ARM Versatiles and RealViews are always clocked I think, so clock support should be optional.

When I added a clock lookup to the PL08x block recently I used a construct like this for an optional clock:

chip->clk = clk_get(&dev->dev, NULL);

if (IS_ERR(chip->clk)) {
       ret = PTR_ERR(chip->clk);
       if (ret == -ENOENT)
          /* No block clock in this platform */
          chip->clk = NULL;
       else
          goto iounmap;
}
Quoted 8 lines

This will work whenever clkdevice is used, since that returns -ENOENT for nonexisting entries.

Then:

if (chip->clk)
clk_enable(chip->clk);

Will modify it, as suggested.

thanks
viresh.

#4 lore raw

PATCH v2 1/1 GPIO PL061: Adding Clk framework support

In reply to Viresh KUMAR

Hi Viresh,

Quoted 11 lines · Viresh KUMAR

From: Viresh KUMAR <[email protected]>

GPIO Clk is never enabled on Platforms, like: SPEAr, which are based upon clock framework and use PL061 driver. This patch adds support for Clk enabling and disabling as and when gpios are requested and freed.

Modifications since V1:
- Removed usage_count, as already done by clk framework.
- Checking error case of clk_enable.

Signed-off-by Viresh Kumar <[email protected]>
Acked-by Baruch Siach <[email protected]>
Signature · 3 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
author
#5 lore raw

PATCH v2 1/1 GPIO PL061: Adding Clk framework support

Quoted 9 lines · Baruch Siach

Hi Viresh,

Quoted 6 lines · Viresh KUMAR

From: Viresh KUMAR <[email protected]>

GPIO Clk is never enabled on Platforms, like: SPEAr, which are based upon clock framework and use PL061 driver. This patch adds support for Clk enabling and disabling as and when gpios are requested and freed.

Quoted 8 lines
Quoted 5 lines

Modifications since V1:
- Removed usage_count, as already done by clk framework.
- Checking error case of clk_enable.

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

Baruch,

Sorry, but you need to review another version.

viresh.