[PATCH] GPIO: Add gpio_lookup

[PATCH] GPIO: Add gpio_lookup

[Jonathan Corbet]

GPIOs have names associated with them, but drivers cannot use those names
and must thus rely on hardwired GPIO numbers.  That, in turn, makes dynamic
assignment hard to use.  This patch adds a little function gpio_lookup()
which returns the GPIO number associated with a name.

This function will be used by the viafb camera driver.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 Documentation/gpio.txt     |    8 ++++++++
 drivers/gpio/gpiolib.c     |   25 +++++++++++++++++++++++++
 include/asm-generic/gpio.h |    1 +
 include/linux/gpio.h       |    5 +++++
 4 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
index fa4dc07..b3b3dd5 100644
--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -253,6 +253,14 @@ pin setup (e.g. controlling which pin the GPIO uses, pullup/pulldown).
 Also note that it's your responsibility to have stopped using a GPIO
 before you free it.
 
+GPIO users can look up GPIO numbers using the names which were provided by
+the GPIO driver, using:
+
+	int gpio_lookup(const char *name);
+
+The return value will be the associated GPIO number, or -EINVAL if no GPIO
+with the given name is found.
+
 
 GPIOs mapped to IRQs
 --------------------
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 662ed92..b95c9e3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1102,6 +1102,31 @@ void gpio_free(unsigned gpio)
 }
 EXPORT_SYMBOL_GPL(gpio_free);
 
+/**
+ * gpio_lookup - Look up a GPIO by name
+ * @name: the name of the desired GPIO
+ *
+ * Returns the GPIO number associated with name, or -EINVAL if
+ * the GPIO is not found.
+ */
+int gpio_lookup(const char *name)
+{
+	int i;
+
+	for (i = 0; i < ARCH_NR_GPIOS; i++) {
+		struct gpio_chip *chip = gpio_desc[i].chip;
+
+		if (chip == NULL || chip->names == NULL)
+			continue;
+		if (!strcmp (name, chip->names[i - chip->base])) {
+			printk(KERN_NOTICE "grbn found %d\n", i);
+			return i;
+		}
+	}
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(gpio_lookup);
+
 
 /**
  * gpiochip_is_requested - return string iff signal was requested
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 66d6106..667b86a 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -116,6 +116,7 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
  */
 extern int gpio_request(unsigned gpio, const char *label);
 extern void gpio_free(unsigned gpio);
+extern int gpio_lookup(const char *name);
 
 extern int gpio_direction_input(unsigned gpio);
 extern int gpio_direction_output(unsigned gpio, int value);
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 059bd18..2c3f179 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -41,6 +41,11 @@ static inline void gpio_free(unsigned gpio)
 	WARN_ON(1);
 }
 
+static inline int gpio_lookup(const char *name)
+{
+	return -ENOSYS;
+}
+
 static inline int gpio_direction_input(unsigned gpio)
 {
 	return -ENOSYS;
-- 
1.6.2.5

Re: [PATCH v2] GPIO: Add gpio_lookup

[Jonathan Corbet]

Functionally the same, but will hopefully head off a visit from the
checkpatch police.  I could *swear* I saved that buffer before
committing...  sorry for the noise.

jon

---
GPIO: add gpio_lookup()

GPIOs have names associated with them, but drivers cannot use those names
and must thus rely on hardwired GPIO numbers.  That, in turn, makes dynamic
assignment hard to use.  This patch adds a little function gpio_lookup()
which returns the GPIO number associated with a name.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 Documentation/gpio.txt     |    8 ++++++++
 drivers/gpio/gpiolib.c     |   25 +++++++++++++++++++++++++
 include/asm-generic/gpio.h |    1 +
 include/linux/gpio.h       |    5 +++++
 4 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
index fa4dc07..b3b3dd5 100644
--- a/Documentation/gpio.txt
+++ b/Documentation/gpio.txt
@@ -253,6 +253,14 @@ pin setup (e.g. controlling which pin the GPIO uses, pullup/pulldown).
 Also note that it's your responsibility to have stopped using a GPIO
 before you free it.
 
+GPIO users can look up GPIO numbers using the names which were provided by
+the GPIO driver, using:
+
+	int gpio_lookup(const char *name);
+
+The return value will be the associated GPIO number, or -EINVAL if no GPIO
+with the given name is found.
+
 
 GPIOs mapped to IRQs
 --------------------
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 662ed92..99d796c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1102,6 +1102,31 @@ void gpio_free(unsigned gpio)
 }
 EXPORT_SYMBOL_GPL(gpio_free);
 
+/**
+ * gpio_lookup - Look up a GPIO by name
+ * @name: the name of the desired GPIO
+ *
+ * Returns the GPIO number associated with name, or -EINVAL if
+ * the GPIO is not found.
+ */
+int gpio_lookup(const char *name)
+{
+	int i;
+
+	for (i = 0; i < ARCH_NR_GPIOS; i++) {
+		struct gpio_chip *chip = gpio_desc[i].chip;
+
+		if (chip == NULL || chip->names == NULL)
+			continue;
+		if (!strcmp(name, chip->names[i - chip->base])) {
+			printk(KERN_NOTICE "grbn found %d\n", i);
+			return i;
+		}
+	}
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(gpio_lookup);
+
 
 /**
  * gpiochip_is_requested - return string iff signal was requested
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 66d6106..667b86a 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -116,6 +116,7 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
  */
 extern int gpio_request(unsigned gpio, const char *label);
 extern void gpio_free(unsigned gpio);
+extern int gpio_lookup(const char *name);
 
 extern int gpio_direction_input(unsigned gpio);
 extern int gpio_direction_output(unsigned gpio, int value);
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 059bd18..2c3f179 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -41,6 +41,11 @@ static inline void gpio_free(unsigned gpio)
 	WARN_ON(1);
 }
 
+static inline int gpio_lookup(const char *name)
+{
+	return -ENOSYS;
+}
+
 static inline int gpio_direction_input(unsigned gpio)
 {
 	return -ENOSYS;
-- 
1.6.2.5

Re: [PATCH v2] GPIO: Add gpio_lookup

[Ben Nizette]

On Sat, 2009-10-10 at 13:53 -0600, Jonathan Corbet wrote:
> GPIO: add gpio_lookup()
> 
> GPIOs have names associated with them, but drivers cannot use those names
> and must thus rely on hardwired GPIO numbers.  That, in turn, makes dynamic
> assignment hard to use.  This patch adds a little function gpio_lookup()
> which returns the GPIO number associated with a name.
> 

OK for something supposedly basic, all this gpio stuff is getting messy.
The gpio framework was written to use gpio numbers as they were
recognised as being common to all possible implementations of the
framework.  Now gpiolib, one implementation (ok I think currently the
only implementation) of that framework, has the ability to have names
associated with the gpios it handles so userspace can get a better idea
of which gpio are which.  This is kind of OK (see below) because gpiolib
and gpio-sysfs are inextricably linked.  With the addition of this
patch, drivers are going to start not depending on the gpio framework
but on an implementation of that framework - gpiolib.  Or at least
require that all future implementations track this dual namespace.

I didn't notice Daniel's original patch fly by otherwise I would have
said something at the time, but storing the names at the chip level
seems an odd idea.  Given that at that stage they were only for sysfs
id, why not just give them as an argument to gpio_export()?

Back to this patch though, the gpio names have to come from the platform
code via some platform_data for the gpio chip [1], the driver then looks
up that pre-defined name to find the gpio number.  Why not just pass the
gpio number straight to the end device driver through platform_data and
bypass the middle-man?

At a higher level, there have been a number of pushes recently; for one
reason or another people really really seem to want the dual number/name
namespace inside the kernel.  With this patch we're at the stage where
this is true and gpiolib has functionality outside the gpio framework
concepts.  In order to keep drivers properly cross-platform we have some
options:  We can keep adding concepts and required functionality to the
gpio framework interface, recognise that some drivers won't be quite as
cross-platform as they could/should be or simply note that it's now been
2.5 years since the gpio framework and gpiolib were merged and it's
still the only implementation; maybe it's time to ditch the distinction
and simply make gpiolib /the/ way you wire up gpios.

David's call really, thoughts?

	--Ben.

[1] which is ugly for at least avr32, the only platform I know
intimately, but not my current point

Re: [PATCH v2] GPIO: Add gpio_lookup

[Jonathan Corbet]

On Mon, 12 Oct 2009 17:11:44 +1100
Ben Nizette <bn@niasdigital.com> wrote:

> Back to this patch though, the gpio names have to come from the platform
> code via some platform_data for the gpio chip [1], the driver then looks
> up that pre-defined name to find the gpio number.  Why not just pass the
> gpio number straight to the end device driver through platform_data and
> bypass the middle-man?

That's true in a situation where you've one One Platform to Bind Them
All, yes.  But if you have (say) GPIOs provided by a PCI-connected
peripheral which are needed in (say) a camera driver, there is no one
platform which can manage all those GPIO numbers.  In particular, I've
done a driver for viafb-based GPIOs; these devices can show up in any
of a number of x86-based systems.  I honestly don't know why it would
make sense to try to hardware numbers to these GPIOs when using names
and dynamic numbering is so much more flexible - and we are already
tracking the names.

Perhaps it would make sense to implement a proper namespace layer for
GPIOs rather than continuing to grow something which evidently sneaked
in through the back door.  Until that's done, though, I think this
patch is useful.  But it it's really unwanted I'll find some other way.

Thanks,

jon

Re: [PATCH v2] GPIO: Add gpio_lookup

[Ben Nizette]

On Mon, 2009-10-12 at 09:23 -0600, Jonathan Corbet wrote:
> On Mon, 12 Oct 2009 17:11:44 +1100
> Ben Nizette <bn@niasdigital.com> wrote:
> 
> > Back to this patch though, the gpio names have to come from the platform
> > code via some platform_data for the gpio chip [1], the driver then looks
> > up that pre-defined name to find the gpio number.  Why not just pass the
> > gpio number straight to the end device driver through platform_data and
> > bypass the middle-man?
> 
> That's true in a situation where you've one One Platform to Bind Them
> All, yes.  But if you have (say) GPIOs provided by a PCI-connected
> peripheral which are needed in (say) a camera driver, there is no one
> platform which can manage all those GPIO numbers.  In particular, I've
> done a driver for viafb-based GPIOs; these devices can show up in any
> of a number of x86-based systems.  I honestly don't know why it would
> make sense to try to hardware numbers to these GPIOs when using names
> and dynamic numbering is so much more flexible - and we are already
> tracking the names.
> 
> Perhaps it would make sense to implement a proper namespace layer for
> GPIOs rather than continuing to grow something which evidently sneaked
> in through the back door.  Until that's done, though, I think this
> patch is useful.  But it it's really unwanted I'll find some other way.
> 

Fair enough; I hadn't seen gpio controllers on such buses before but
that makes sense.  If the naming information isn't coming from platform
code but from some driver itself this seems fairly sane.

If gpios are going to be named, and it seems they are, then personally
I'd like to see the names field of struct gpio_chip go and a proper
namespace layer inserted.  Something which allows gpio_name() and
gpio_lookup() and decentralises that responsibility would be nice (the
chip could call gpio_name itself if it has reason).  That code could be
common to all gpio framework implementations.

But a) I don't have the hours to do this myself and b) it's really
David's call, I'm just an interested bystander :-)

	--Ben

Re: [PATCH v2] GPIO: Add gpio_lookup

[David Brownell]

On Tuesday 13 October 2009, Ben Nizette wrote:
> If gpios are going to be named, and it seems they are,

I'm not so sure of that.  Keep in mind that "name" implies, to me,
focussing on name-centric operation, with lookup and browsing.
Neither of those mechanisms is fundamental to what GPIOs solve.

So far I've not seen any use case that can't be addressed within
the current framework, which keys only on numeric IDs.  If we *did*
see such cases appear in real-world scenarios, we could explore
what that means.


> then personally 
> I'd like to see the names field of struct gpio_chip go and a proper
> namespace layer inserted.  Something which allows gpio_name() and
> gpio_lookup() and decentralises that responsibility would be nice (the
> chip could call gpio_name itself if it has reason).  That code could be
> common to all gpio framework implementations.

But why would we need such lookups in the first place?

In-kernel, they are not needed, for either static or dynamic ID
assignment schemes.  Just register the numbers so they're available
as needed.

For userspace, I think gpio_export_link() is the best way to
go ... it scopes the names sanely, so there's no requirement
for driver-provided labels to be globally unique.

- Dave

Re: [PATCH v2] GPIO: Add gpio_lookup

[David Brownell]

On Monday 12 October 2009, Jonathan Corbet wrote:
> On Mon, 12 Oct 2009 17:11:44 +1100
> Ben Nizette <bn@niasdigital.com> wrote:
> 
> > Back to this patch though, the gpio names have to come from the platform
> > code via some platform_data for the gpio chip [1], the driver then looks
> > up that pre-defined name to find the gpio number.  Why not just pass the
> > gpio number straight to the end device driver through platform_data and
> > bypass the middle-man?
> 
> That's true in a situation where you've one One Platform to Bind Them
> All, yes.

What do you mean by "platform" though?  Clearly not what I mean!
You make it sound like an evil place, Barad-dûr hosting an assult
on the Forces Of Light!  :)

If you mean to imply that only statically assigned GPIO numbers
can work, I'll disagree.


> But if you have (say) GPIOs provided by a PCI-connected 
> peripheral which are needed in (say) a camera driver, there is no one
> platform which can manage all those GPIO numbers.

The basic trick is to have gpio chip drivers call back with enough
information to start hooking up the GPIO signals to consumers.  The
OS is only responsible for remembering things ... like how to connect
a phone number or IP address to the right destination.

See for example the pcf875x driver.  When it registers, it reports via
callback that the stage is setup() for performance.  Later it can report
that it's time to teardown() that part of the show.

Whatever sets up that PCI board has to know basically what's there, and
when it sets up the gpio chips it can provide the callbacks needed to
hook up e.g. "base + 19" as the "camera active" LED.  No lookup() needed.

Notice also how doing it that way provides clean ways to sequence the
startup and teardown of complex hardware stacks.  (Too many drivers tend
to ignore those issues, IMO, and for complex hardware modules that can
cause much trouble.)


> In particular, I've 
> done a driver for viafb-based GPIOs; these devices can show up in any
> of a number of x86-based systems.  I honestly don't know why it would
> make sense to try to hardware numbers to these GPIOs when using names
> and dynamic numbering is so much more flexible - and we are already
> tracking the names.

There are video drivers that solve this without needing name-to-ID
style lookups.  I recall discussing them well over a year ago... the
numbering is dynamic, yes.


> Perhaps it would make sense to implement a proper namespace layer for
> GPIOs

Today, that namespace is numeric.  And as far as I know, it's
sufficient ... although it's not intended for browsing, any more
than, say, phone numbers or IP address numbers.


> rather than continuing to grow something which evidently sneaked 
> in through the back door.  Until that's done, though, I think this
> patch is useful.  But it it's really unwanted I'll find some other way.

All *names* for GPIOs snuck through the back door ... except for
the labels in /sys/kernel/debug/gpio, which are diagnostic-only
and not unique.  And the gpio_export_link(), with sysfs names that
are unique within a clearly-defined driver context.

- Dave

Re: [PATCH v2] GPIO: Add gpio_lookup

[David Brownell]

On Sunday 11 October 2009, Ben Nizette wrote:
> OK for something supposedly basic, all this gpio stuff is getting messy.

Sigh.  :(

One can only _expect_ that as infrastructure gets more widely
used, it (unfortunately) starts expanding.  And one _hopes_ the
growth does not turn into bloat.  This is a constant battle with
all tools and infrastructure.  


> The gpio framework was written to use gpio numbers as they were
> recognised as being common to all possible implementations of the
> framework.  

I'm not sure I'd put it that way.  Most SoC-integrated GPIOs
had vendor docs using names like "GPIO-n"; a few combined them
with bank IDs, like "Port C-19".  In all cases, the IDs could
be mapped to numbers ... ('C' - 'A') * 32 + 19, etc.  And mostly
they *needed* to have such a mapping, since GPIOs are often the
main flavor of external IRQ signal, and IRQs are numbered.

Embedded boards also use pcf8574 I2C GPIO expanders and such,
which can be modeled as just fancier banks where IO the calls
require sleeping access.  Enter gpiolib to pull such devices
into the API.

Plus of course, numbers are easily used as a global ID space,
on any platform.  Trivially, if one had a per-GPIO structure
(a thing to avoid, in general, for a single hardware bit!!),
the pointer to that structure is a number.  QED.


Originally, the only name-ish thing was labels for diagnostics,
to help associate for example GPIO-78 with nCS2 or SDA in the
/sys/kernel/debug/gpio listing or in diagnostics.

> I didn't notice Daniel's original patch fly by otherwise I would have
> said something at the time, but storing the names at the chip level
> seems an odd idea.  Given that at that stage they were only for sysfs
> id, why not just give them as an argument to gpio_export()?

Yeah, I wasn't wholly keen on that either.  Notice that we now
have gpio_export_link()... and that the names given in gpio bank
registration can conflict with the *existing* ones registered by
the gpio_request() primitives.

There's also some role conflict here ... the entity registering
the GPIO bank may not always know the purpose to which the GPIO
will be put, while the entity calling gpio_request() *does*.  And
what good is a name which presents the wrong purpose?

Plus, GPIOs may need to be registered very early -- preceding
core_initcall() in some cases -- which can be a long time before
the board-specific labels are available, e.g. in arch_initcall().

But the biggest issue is name uniqueness ...

- Dave

Re: [PATCH v2] GPIO: Add gpio_lookup

[David Brownell]

On Saturday 10 October 2009, Jonathan Corbet wrote:
> GPIO: add gpio_lookup()
> 
> GPIOs have names associated with them, but drivers cannot use those names
> and must thus rely on hardwired GPIO numbers.  That, in turn, makes dynamic
> assignment hard to use.  This patch adds a little function gpio_lookup()
> which returns the GPIO number associated with a name.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>

Not real keen on this; see separate emails, and below.


> ---
>  Documentation/gpio.txt     |    8 ++++++++
>  drivers/gpio/gpiolib.c     |   25 +++++++++++++++++++++++++
>  include/asm-generic/gpio.h |    1 +
>  include/linux/gpio.h       |    5 +++++
>  4 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/gpio.txt b/Documentation/gpio.txt
> index fa4dc07..b3b3dd5 100644
> --- a/Documentation/gpio.txt
> +++ b/Documentation/gpio.txt
> @@ -253,6 +253,14 @@ pin setup (e.g. controlling which pin the GPIO uses, pullup/pulldown).
>  Also note that it's your responsibility to have stopped using a GPIO
>  before you free it.
>  
> +GPIO users can look up GPIO numbers using the names which were provided by
> +the GPIO driver, using:
> +
> +	int gpio_lookup(const char *name);

This is missing a handle for the GPIO driver.

So for example if two video cards register a "backlight" GPIO,
nothing prevents this from returning the wrong one ... :(


> +
> +The return value will be the associated GPIO number, or -EINVAL if no GPIO
> +with the given name is found.

Easier all around to not presume lookup() functionality for GPIOs.


> +
>  
>  GPIOs mapped to IRQs
>  --------------------
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 662ed92..99d796c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1102,6 +1102,31 @@ void gpio_free(unsigned gpio)
>  }
>  EXPORT_SYMBOL_GPL(gpio_free);
>  
> +/**
> + * gpio_lookup - Look up a GPIO by name
> + * @name: the name of the desired GPIO
> + *
> + * Returns the GPIO number associated with name, or -EINVAL if
> + * the GPIO is not found.
> + */
> +int gpio_lookup(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARCH_NR_GPIOS; i++) {
> +		struct gpio_chip *chip = gpio_desc[i].chip;
> +
> +		if (chip == NULL || chip->names == NULL)
> +			continue;
> +		if (!strcmp(name, chip->names[i - chip->base])) {

But *any* chip can register such a name; nothing establishes or verifies
uniquness in any scope larger than that single chip...


> +			printk(KERN_NOTICE "grbn found %d\n", i);

We know the grbn should not have escaped.  But we don't
exactly know what it is!  Who is he/she?  And what is their
involvement with GPIOs?  :)


> +			return i;
> +		}
> +	}
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(gpio_lookup);
> +
>  
>  /**
>   * gpiochip_is_requested - return string iff signal was requested
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 66d6106..667b86a 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -116,6 +116,7 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
>   */
>  extern int gpio_request(unsigned gpio, const char *label);
>  extern void gpio_free(unsigned gpio);
> +extern int gpio_lookup(const char *name);
>  
>  extern int gpio_direction_input(unsigned gpio);
>  extern int gpio_direction_output(unsigned gpio, int value);
> diff --git a/include/linux/gpio.h b/include/linux/gpio.h
> index 059bd18..2c3f179 100644
> --- a/include/linux/gpio.h
> +++ b/include/linux/gpio.h
> @@ -41,6 +41,11 @@ static inline void gpio_free(unsigned gpio)
>  	WARN_ON(1);
>  }
>  
> +static inline int gpio_lookup(const char *name)
> +{
> +	return -ENOSYS;
> +}
> +
>  static inline int gpio_direction_input(unsigned gpio)
>  {
>  	return -ENOSYS;
> -- 
> 1.6.2.5
> 
> 

Re: [PATCH v2] GPIO: Add gpio_lookup

[Jonathan Corbet]

On Tue, 13 Oct 2009 14:10:40 -0700
David Brownell <david-b@pacbell.net> wrote:

> Not real keen on this; see separate emails, and below.

OK, so the story I'm getting is that each driver needs to set up its
own mechanism for obtaining GPIO numbers - it needs to create its own
lookup mechanism.  That's fine, I can do that; I just thought it made
sense to make use of the information that's already there.

Andrew, you might as well drop the patch.

(I'm less worried about the uniqueness side, BTW; it just means drivers
need to create meaningful names for their GPIOs.)

Thanks,

jon

Re: [PATCH v2] GPIO: Add gpio_lookup

[David Brownell]

On Tuesday 13 October 2009, Jonathan Corbet wrote:
> On Tue, 13 Oct 2009 14:10:40 -0700
> David Brownell <david-b@pacbell.net> wrote:
> 
> > Not real keen on this; see separate emails, and below.
> 
> OK, so the story I'm getting is that each driver needs to set up its
> own mechanism for obtaining GPIO numbers - it needs to create its own
> lookup mechanism.

Each driver needs its own *configuration* scheme.  Yes;
that's a very standard requirement.  ;)


> That's fine, I can do that; I just thought it made 
> sense to make use of the information that's already there.

It's not necessarily "there" though; or safe to use in this way,
should it happen to be present.


> Andrew, you might as well drop the patch.
> 
> (I'm less worried about the uniqueness side, BTW; it just means drivers
> need to create meaningful names for their GPIOs.)

I don't see how they *can* though ... unless they're dynamically
allocated using a scheme like "combine <this device's sysfs name>
with <token>".  Consider two PCI cards with two different GPIOs
for their "camera_active" LED... "camera_active" is meaningful,
but unusable because it's not unique.


Quick rule of thumb:  in absolutely *ANYTHING* to do with resource
lookup, see how the names/IDs are scoped.  That's the first place
problems show up.  If the scopes are not clearly defined, or there
is nothing to ensure uniqueness within that scope ... trouble.

- Dave


> 
> Thanks,
> 
> jon
> 
> 

Re: [PATCH v2] GPIO: Add gpio_lookup

[Mark Brown]

On Tue, Oct 13, 2009 at 03:53:58PM -0700, David Brownell wrote:
> On Tuesday 13 October 2009, Jonathan Corbet wrote:

> > (I'm less worried about the uniqueness side, BTW; it just means drivers
> > need to create meaningful names for their GPIOs.)

> I don't see how they *can* though ... unless they're dynamically
> allocated using a scheme like "combine <this device's sysfs name>
> with <token>".  Consider two PCI cards with two different GPIOs
> for their "camera_active" LED... "camera_active" is meaningful,
> but unusable because it's not unique.

I guess the clock API style dev/dev_name based deduping would do the job
here, though that's a separate question from the general usefulness of
this sort of lookup framework.

Re: [PATCH] GPIO: Add gpio_lookup

[Andrew Morton]

On Sat, 10 Oct 2009 13:48:14 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> +int gpio_lookup(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARCH_NR_GPIOS; i++) {
> +		struct gpio_chip *chip = gpio_desc[i].chip;
> +
> +		if (chip == NULL || chip->names == NULL)
> +			continue;
> +		if (!strcmp (name, chip->names[i - chip->base])) {
> +			printk(KERN_NOTICE "grbn found %d\n", i);
> +			return i;
> +		}
> +	}
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(gpio_lookup);

I'll assume the printk wasn't supposed to be there.

Re: [PATCH] GPIO: Add gpio_lookup

[Jonathan Corbet]

On Tue, 13 Oct 2009 11:05:07 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> I'll assume the printk wasn't supposed to be there.

Really?  I shouldn't fill everybody's logs with my silly debugging
output?

I blame the Death Flu that my kids brought home.  Sorry.

jon