[PATCH 1/3] ARM: SAMSUNG: Add config option for number of additional GPIO pins.

[PATCH 1/3] ARM: SAMSUNG: Add config option for number of additional GPIO pins.

[Kukjin Kim]

This patch adds a configuration option for specifing the number
of additional GPIO pins to be used in addition to the GPIO pins
supported onchip.

Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/plat-samsung/Kconfig |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 900b463..faec4b8 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -66,6 +66,14 @@ config S3C_GPIO_PULL_UP
 	help
 	  Internal configuration to enable the correct GPIO pull helper
 
+config SAMSUNG_GPIO_EXTRA
+	int "Number of additional GPIO pins"
+	default 0
+	help
+	  Use additional GPIO space in addition to the GPIO's the SOC
+	  provides. This allows expanding the GPIO space for use with
+	  GPIO expanders.
+
 # device definitions to compile in
 
 config S3C_DEV_HSMMC
-- 
1.6.2.5

[PATCH 1/3] ARM: SAMSUNG: Add config option for number of additional GPIO pins.

[Mark Brown]

On Mon, Jan 11, 2010 at 06:44:06PM +0900, Kukjin Kim wrote:

> +config SAMSUNG_GPIO_EXTRA
> +	int "Number of additional GPIO pins"
> +	default 0
> +	help
> +	  Use additional GPIO space in addition to the GPIO's the SOC
> +	  provides. This allows expanding the GPIO space for use with
> +	  GPIO expanders.
> +

The way this has been handled for previous Samsung variants is to make
it entirely hidden from users and provide selectable values for defining
the number so you end up with something like:

	config SAMSUNG_GPIO_EXTRA
	int
	default 64 if SAMSUNG_GPIO_64
	default 32 if SAMSUNG_GPIO_32
	default 0

The board drivers then select one of the numeric options.  They should
know how many GPIOs they need, while end users rebuilding their kernels
don't need to know this since at least code if not hardware changes will
be required to change it usefully.

[PATCH 1/3] ARM: SAMSUNG: Add config option for number of additional GPIO pins.

[Ben Dooks]

On Mon, Jan 11, 2010 at 10:33:39AM +0000, Mark Brown wrote:
> On Mon, Jan 11, 2010 at 06:44:06PM +0900, Kukjin Kim wrote:
> 
> > +config SAMSUNG_GPIO_EXTRA
> > +	int "Number of additional GPIO pins"
> > +	default 0
> > +	help
> > +	  Use additional GPIO space in addition to the GPIO's the SOC
> > +	  provides. This allows expanding the GPIO space for use with
> > +	  GPIO expanders.
> > +
> 
> The way this has been handled for previous Samsung variants is to make
> it entirely hidden from users and provide selectable values for defining
> the number so you end up with something like:
> 
> 	config SAMSUNG_GPIO_EXTRA
> 	int
> 	default 64 if SAMSUNG_GPIO_64
> 	default 32 if SAMSUNG_GPIO_32
> 	default 0
> 
> The board drivers then select one of the numeric options.  They should
> know how many GPIOs they need, while end users rebuilding their kernels
> don't need to know this since at least code if not hardware changes will
> be required to change it usefully.

I've put a question out about trying to change Kconfig to allow
a configuration like this to be set to a minimum value from other
config variables.

Having this as a user settable field might be useful if plugging on
GPIO expanders at runtime.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[PATCH 1/3] ARM: SAMSUNG: Add config option for number of additional GPIO pins.

[Mark Brown]

On Mon, Jan 11, 2010 at 12:27:15PM +0000, Ben Dooks wrote:

> Having this as a user settable field might be useful if plugging on
> GPIO expanders at runtime.

If that gets to be anything other than an extremely niche use case the
GPIO allocation would need to have a dynamic limit anyway, though.

[PATCH 1/3] ARM: SAMSUNG: Add config option for number of additional GPIO pins.

[Ben Dooks]

On Mon, Jan 11, 2010 at 12:49:27PM +0000, Mark Brown wrote:
> On Mon, Jan 11, 2010 at 12:27:15PM +0000, Ben Dooks wrote:
> 
> > Having this as a user settable field might be useful if plugging on
> > GPIO expanders at runtime.
> 
> If that gets to be anything other than an extremely niche use case the
> GPIO allocation would need to have a dynamic limit anyway, though.

Yes, however a kconfig case where you can set the minimum value of an
integer would make the whole extra-gpio configuration much cleaner and
avoid having toadd further configuration symbols when someone wants an
extra 256 or whatever GPIOS.

Having a dynamic GPIO range would also be nice, someone should look at
adding it to the GPIO code.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[PATCH 1/3] ARM: SAMSUNG: Add config option for number of additional GPIO pins.

[Mark Brown]

On Tue, Jan 12, 2010 at 12:15:26AM +0000, Ben Dooks wrote:
> On Mon, Jan 11, 2010 at 12:49:27PM +0000, Mark Brown wrote:

> > If that gets to be anything other than an extremely niche use case the
> > GPIO allocation would need to have a dynamic limit anyway, though.

> Yes, however a kconfig case where you can set the minimum value of an
> integer would make the whole extra-gpio configuration much cleaner and
> avoid having toadd further configuration symbols when someone wants an
> extra 256 or whatever GPIOS.

Yes, it'd be very much nicer if Kconfig supported that sort of
dependency but without that the hack with selects does seem about as
good as it gets if you want to do it via Kconfig.  I rather suspect this
is why it's common to have an ifdef tree for these things rather than
trying to push them through Kconfig.

[PATCH 1/3] ARM: SAMSUNG: Add config option for number ofadditional GPIO pins.

[H Hartley Sweeten]

On Monday, January 11, 2010 5:15 PM, Ben Dooks wrote:
> On Mon, Jan 11, 2010 at 12:49:27PM +0000, Mark Brown wrote:
>> On Mon, Jan 11, 2010 at 12:27:15PM +0000, Ben Dooks wrote:
>> 
>>> Having this as a user settable field might be useful if plugging on
>>> GPIO expanders at runtime.
>> 
>> If that gets to be anything other than an extremely niche use case the
>> GPIO allocation would need to have a dynamic limit anyway, though.
>
> Yes, however a kconfig case where you can set the minimum value of an
> integer would make the whole extra-gpio configuration much cleaner and
> avoid having toadd further configuration symbols when someone wants an
> extra 256 or whatever GPIOS.
>
> Having a dynamic GPIO range would also be nice, someone should look at
> adding it to the GPIO code.

I would also like to see a dynamic GPIO range but it looks like it will
create a lot of overhead for all the gpiolib calls.

The only way I can see to make it dynamic is by putting gpio_desc (one
for each gpio) into a list and adding the gpio number to struct gpio_desc:

struct gpio_desc {
	struct gpio_chip	*chip;
+	unsigned		nr;
	unsigned long	flags;
...
- static struct gpio_desc gpio_desc[MAX_NR_GPIOS];
+ struct gpio_dev {
+	struct list_head	list;
+	struct gpio_desc	*gpio_desc;
+ };
+
+ static LIST_HEAD(gpio_devices);

Then changing the inline gpio_to_chip so that it works thru the list
in order to find the proper gpio_desc for the gpio in order to return
the chip.

-/* caller holds gpio_lock *OR* gpio is marked as requested */
-static inline struct gpio_chip *gpio_to_chip(unsigned gpio)
-{
-	return gpio_desc[gpio].chip;
-}
+/* is the locking needed for the list? */
+static struct gpio_chip *gpio_to_chip(unsigned gpio)
+{
+	struct gpio_chip *chip;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gpio_lock, flags);
+	list_for_each_entry(gpio_dev, &gpio_devices, list) {
+		if (gpio_dev->gpio_desc->nr == gpio) {
+			chip = gpio_dev->gpio_desc->chip;
+			goto found;
+		}
+	}
+	chip = NULL;
+found:
+	spin_unlock(&gpio_lock);
+	return chip;
+}

Plus, a similar function will be needed to get gpio_desc for
everything in gpiolib that currently uses gpio_desc[].  I'm sure
there are a lot of other issues but I'm just trying to work out
a way to make the GPIO range dynamic.

An alternative would be to just put each gpio chip into a list.
Each gpio chip would then contain another list of the individual
gpio's.

Any other ideas?

Regards,
Hartley