Re: [patch 2.6.25-rc5 1/2] gpiolib: dynamic gpio number allocation

Previous thread: [PATCH 1/12] ali14xx: add "vlb_clock=" parameter by Bartlomiej Zolnierkiewicz on Thursday, March 13, 2008 - 3:43 pm. (14 messages)

Next thread: [patch 2.6.25-rc5 2/2] gpiochip_reserve() by David Brownell on Thursday, March 13, 2008 - 3:52 pm. (3 messages)
From: David Brownell
Date: Thursday, March 13, 2008 - 3:49 pm

From: Anton Vorontsov <avorontsov@ru.mvista.com>

If gpio_chip->base is negative during registration, gpiolib performs
dynamic base allocation. This is useful for devices that aren't always
present, such as GPIOs on hotplugged devices rather than mainboards.
(This behavior was previously specified but not implemented.)

To avoid using any numbers that may have been explicitly assigned but
not yet registered, this dynamic allocation assigns GPIO numbers from
the biggest number on down, instead of from the smallest on up.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
 drivers/gpio/gpiolib.c |   52 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

--- g26.orig/drivers/gpio/gpiolib.c	2008-03-13 14:41:52.000000000 -0700
+++ g26/drivers/gpio/gpiolib.c	2008-03-13 14:51:50.000000000 -0700
@@ -80,6 +80,33 @@ static inline struct gpio_chip *gpio_to_
 	return gpio_desc[gpio].chip;
 }
 
+/* dynamic allocation of GPIOs, e.g. on a hotplugged device */
+static int gpiochip_find_base(int ngpio)
+{
+	int i;
+	int spare = 0;
+	int base = -ENOSPC;
+
+	for (i = ARCH_NR_GPIOS - 1; i >= 0 ; i--) {
+		struct gpio_chip *chip = gpio_desc[i].chip;
+
+		if (!chip) {
+			spare++;
+			if (spare == ngpio) {
+				base = i;
+				break;
+			}
+		} else {
+			spare = 0;
+			i -= chip->ngpio - 1;
+		}
+	}
+
+	if (gpio_is_valid(base))
+		pr_debug("%s: found new base at %d\n", __func__, base);
+	return base;
+}
+
 /**
  * gpiochip_add() - register a gpio_chip
  * @chip: the chip to register, with chip->base initialized
@@ -88,38 +115,49 @@ static inline struct gpio_chip *gpio_to_
  * Returns a negative errno if the chip can't be registered, such as
  * because the chip->base is invalid or already associated with a
  * different chip.  Otherwise it returns zero as a success code.
+ *
+ * If chip->base is negative, this requests dynamic assignment of
+ * a ...
From: Andrew Morton
Date: Thursday, March 13, 2008 - 4:00 pm

On Thu, 13 Mar 2008 14:49:53 -0800

hm.  I suppose that if someone want a huge number of GPIOs then we can
convert this to a bitmap or an IDR tree easily enough.

Shouldn't ARCH_NR_GPIOS be CONFIG_NR_GPIOS?


--

From: David Brownell
Date: Thursday, March 13, 2008 - 4:18 pm

Actually, I tried IDRs for a while and they broke platforms
which needed to initialize and use GPIOs early: before kmalloc

No more than NR_IRQS is settable via Kconfig.  And for
rather similar reasons.  :)

- Dave
--

From: Andrew Morton
Date: Thursday, March 13, 2008 - 4:28 pm

On Thu, 13 Mar 2008 15:18:58 -0800


What are those reasons?
--

From: Anton Vorontsov
Date: Thursday, March 13, 2008 - 4:54 pm

Heh.. FWIW, I didn't notice any slowness of that linear search.

I didn't bother to try anything more complicated than linear search
for mere 256 GPIOs. I doubt that IDRs will pour out into any
measurable win even for something like 1024 GPIOs.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
--

From: David Brownell
Date: Thursday, March 13, 2008 - 5:53 pm

The slowness of IDRs was needing to use them for the
routine lookups ... versus the current array index,
which costs a fraction of an instruction cycle and
doesn't need separate locks.

Or were you implying they should be used for something
other than mapping GPIO numbers to controllers/state?

- Dave
--

From: Andrew Morton
Date: Thursday, March 13, 2008 - 7:17 pm

For dynamic allocation.  There should be no need for lookups outside
register/unregister.


Where did the CONFIG_NR_GPIOS discussion disappear to?
--

From: David Brownell
Date: Thursday, March 13, 2008 - 10:52 pm

So -- a secondary data structure used only for allocation?
With the primary one as it is now? 

If allocation were a hotspot something like that might be
worth considering ... though such duplication is usually
 
http://marc.info/?l=linux-kernel&m=120545979527097&w=2

I could maybe see a CONFIG_NR_EXTRA_GPIOS.
--

From: David Brownell
Date: Thursday, March 13, 2008 - 6:54 pm

Keeping a lid on the amount of space wasted by unused
table entries is one factor; it's an implementation
tradeoff.  In normal usage the number of IRQs (or GPIOs)
is defined by the board (or system) design, and there's
no real point to allowing more.

That said, NR_IRQS is kind of inflexible.  It's not easy
to provide board-specific overrides for cases like having
a few FPGAs or other external IRQ (or GPIO!) controllers
which chain IRQs and plug into those tables...

Both could use a way to extend a platform-defined minimum
to support a configurable number of external controllers.
Lacking that, both have somewhat ad-hoc solutions to make
sure board variants can be set up with the same kernel.
It basically boils down to making sure there are some extra
entries at end-of-table, and policies to allocate them.

- Dave


--

Previous thread: [PATCH 1/12] ali14xx: add "vlb_clock=" parameter by Bartlomiej Zolnierkiewicz on Thursday, March 13, 2008 - 3:43 pm. (14 messages)

Next thread: [patch 2.6.25-rc5 2/2] gpiochip_reserve() by David Brownell on Thursday, March 13, 2008 - 3:52 pm. (3 messages)