Re: [patch 2.6.27-rc7] gpiolib: request/free hooks

Previous thread: [PATCH 0/39] Ocfs2 updates for 2.6.28 by Mark Fasheh on Wednesday, September 24, 2008 - 3:00 pm. (62 messages)

Next thread: [patch 00/04] RFC: Staging tree (drivers/staging) by Greg KH on Wednesday, September 24, 2008 - 4:00 pm. (36 messages)
From: David Brownell
Date: Wednesday, September 24, 2008 - 3:08 pm

From: David Brownell <dbrownell@users.sourceforge.net>

Add a new internal mechanism to gpiolib to support low power
operations by letting gpio_chip instances see when their GPIOs
are in use.  When no GPIOs are active, chips may be able to 
enter lower powered runtime states by disabling clocks and/or
power domains.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 Documentation/gpio.txt     |    4 ++++
 drivers/gpio/gpiolib.c     |   42 ++++++++++++++++++++++++++++++------------
 include/asm-generic/gpio.h |    9 +++++++++
 3 files changed, 43 insertions(+), 12 deletions(-)

--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -240,6 +240,10 @@ signal, or (b) something wrongly believe
 needed to manage a signal that's in active use.  That is, requesting a
 GPIO can serve as a kind of lock.
 
+Some platforms may also use knowledge about what GPIOs are active for
+power management, such as by powering down unused chip sectors and, more
+easily, gating off unused clocks.
+
 These two calls are optional because not not all current Linux platforms
 offer such functionality in their GPIO support; a valid implementation
 could return success for all gpio_request() calls.  Unlike the other calls,
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -69,14 +69,18 @@ static inline void desc_set_label(struct
  * those calls have no teeth) we can't avoid autorequesting.  This nag
  * message should motivate switching to explicit requests...
  */
-static void gpio_ensure_requested(struct gpio_desc *desc)
+static void gpio_ensure_requested(struct gpio_desc *desc, unsigned offset)
 {
 	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0) {
-		pr_warning("GPIO-%d autorequested\n", (int)(desc - gpio_desc));
+		struct gpio_chip *chip = desc->chip;
+		int gpio = chip->base + offset;
+
+		pr_warning("GPIO-%d autorequested\n", gpio);
 		desc_set_label(desc, "[auto]");
-		if (!try_module_get(desc->chip->owner))
-			pr_err("GPIO-%d: module can't ...
From: Magnus Damm
Date: Saturday, September 27, 2008 - 8:00 am

Hi David,


Looking good. I'm currently hacking on some pinmuxed gpio code for
SuperH, and I'd like to use these request/free callbacks to select
proper pinmux state.


The code above doesn't catch double gpio_request() user calls
properly. Or rather, the user will receive an error but the
chip->request() callback may get called twice.

What about modifying the gpiolib code to handle that? I think that
sounds like a better idea than cover ing that case in the chip code...

Thanks!

/ magnus
--

From: David Brownell
Date: Saturday, September 27, 2008 - 11:29 am

I'm not so keen on that particular overloading, but I can
understand why it might be wanted on systems which have a
happy one-to-one mapping between GPIOs and pins.

If you do that, be ready to provide a pinmux-only interface

Yeah.  Better to test-and-set the flag and then request, and backout
if it fails, than the other way.  Just who wrote that crap in the
first place?  Sigh.  (Notice it's done that way already in the code
path handling implicit requesting ... )

I'll send an updated patch along soonish.

- Dave
--

From: Magnus Damm
Date: Sunday, September 28, 2008 - 8:48 pm

In the SuperH case GPIO pin direction selection is done in the same
way as selecting pin function. So in and out directions can be seen as
two pin functions. And since we want to support GPIO pin direction we
may as well support setting pin functions as well.



Thank you!

/ magnus
--

From: David Brownell
Date: Monday, September 29, 2008 - 10:21 am

Just don't expect the GPIO framework to address the problem of
how to configure one of those pins for a non-GPIO function.

Such pinmux problems deserve different programming interfaces.


... after I come up with a happy fix for the locking goofs;
those new methods may not be called under spinlock protection.

- Dave
--

From: David Brownell
Date: Sunday, October 5, 2008 - 9:57 pm

> > > I'll send an updated patch along soonish.

This updated version seems to behave for me; against RC8.
It resolves the locking issues I noticed, as well as the
record-keeping one you commented on.  Comments?

- Dave

==================
From: David Brownell <dbrownell@users.sourceforge.net>

Add a new internal mechanism to gpiolib to support low power
operations by letting gpio_chip instances see when their GPIOs
are in use.  When no GPIOs are active, chips may be able to 
enter lower powered runtime states by disabling clocks and/or
power domains.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 Documentation/gpio.txt     |    4 +
 drivers/gpio/gpiolib.c     |   91 ++++++++++++++++++++++++++++++++++++-------
 include/asm-generic/gpio.h |    9 ++++
 3 files changed, 91 insertions(+), 13 deletions(-)

--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -240,6 +240,10 @@ signal, or (b) something wrongly believe
 needed to manage a signal that's in active use.  That is, requesting a
 GPIO can serve as a kind of lock.
 
+Some platforms may also use knowledge about what GPIOs are active for
+power management, such as by powering down unused chip sectors and, more
+easily, gating off unused clocks.
+
 These two calls are optional because not not all current Linux platforms
 offer such functionality in their GPIO support; a valid implementation
 could return success for all gpio_request() calls.  Unlike the other calls,
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -67,17 +67,28 @@ static inline void desc_set_label(struct
  * when setting direction, and otherwise illegal.  Until board setup code
  * and drivers use explicit requests everywhere (which won't happen when
  * those calls have no teeth) we can't avoid autorequesting.  This nag
- * message should motivate switching to explicit requests...
+ * message should motivate switching to explicit requests... so should
+ * the weaker cleanup after faults, compared to ...
Previous thread: [PATCH 0/39] Ocfs2 updates for 2.6.28 by Mark Fasheh on Wednesday, September 24, 2008 - 3:00 pm. (62 messages)

Next thread: [patch 00/04] RFC: Staging tree (drivers/staging) by Greg KH on Wednesday, September 24, 2008 - 4:00 pm. (36 messages)