* Add to_irq fields to gpiolib (with sample implementation)
@ 2008-07-18 13:04 Ben Dooks
2008-07-21 10:43 ` Ramax Lo
0 siblings, 1 reply; 5+ messages in thread
From: Ben Dooks @ 2008-07-18 13:04 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel; +Cc: David Brownell
[-- Attachment #1: Type: text/plain, Size: 4648 bytes --]
The two patches form a pair of patches to show
that we should consider adding an to_irq field
to the gpio_chip structure and gpiolib support.
The reason is that if we add support for devices
registering gpio to also register interrputs, then
a single arch-dependant interrupt mapping is not
going to be sufficient.
Note, this set does not remove any clashing
definitions that may have of gpio_to_irq.
---
GPIO: Add generic gpio_to_irq call.
Add gpio_to_irq() implementation allowing the
gpio_chip registration to also specify an function
to map GPIO offsets into IRQs.
Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Index: linux-2.6.26-quilt3/drivers/gpio/gpiolib.c
===================================================================
--- linux-2.6.26-quilt3.orig/drivers/gpio/gpiolib.c 2008-07-18 00:40:52.000000000 +0100
+++ linux-2.6.26-quilt3/drivers/gpio/gpiolib.c 2008-07-18 00:52:07.000000000 +0100
@@ -339,6 +339,36 @@ const char *gpiochip_is_requested(struct
}
EXPORT_SYMBOL_GPL(gpiochip_is_requested);
+int gpio_to_irq(unsigned gpio)
+{
+ struct gpio_chip *chip;
+ struct gpio_desc *desc = &gpio_desc[gpio];
+ unsigned long flags;
+ int status = -EINVAL;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ if (!gpio_is_valid(gpio))
+ goto fail;
+
+ chip = desc->chip;
+ if (!chip || !chip->to_irq)
+ goto fail;
+
+ gpio -= chip->base;
+ if (gpio >= chip->ngpio)
+ goto fail;
+
+ status = chip->to_irq(chip, gpio);
+
+ fail:
+ spin_unlock_irqrestore(&gpio_lock, flags);
+ if (status)
+ pr_debug("%s: gpio-%d status %d\n",
+ __func__, gpio, status);
+ return status;
+}
+EXPORT_SYMBOL_GPL(gpio_to_irq);
/* Drivers MUST set GPIO direction before making get/set calls. In
* some cases this is done in early boot, before IRQs are enabled.
Index: linux-2.6.26-quilt3/include/asm-generic/gpio.h
===================================================================
--- linux-2.6.26-quilt3.orig/include/asm-generic/gpio.h 2008-07-18 00:40:52.000000000 +0100
+++ linux-2.6.26-quilt3/include/asm-generic/gpio.h 2008-07-18 00:46:32.000000000 +0100
@@ -40,6 +40,7 @@ struct module;
* @dbg_show: optional routine to show contents in debugfs; default code
* will be used when this is omitted, but custom code can show extra
* state (such as pullup/pulldown configuration).
+ * @to_irq: convert gpio offset to IRQ number.
* @base: identifies the first GPIO number handled by this chip; or, if
* negative during registration, requests dynamic ID allocation.
* @ngpio: the number of GPIOs handled by this controller; the last GPIO
@@ -71,6 +72,9 @@ struct gpio_chip {
unsigned offset, int value);
void (*dbg_show)(struct seq_file *s,
struct gpio_chip *chip);
+ int (*to_irq)(struct gpio_chip *chip,
+ unsigned offset);
+
int base;
u16 ngpio;
unsigned can_sleep:1;
@@ -97,6 +101,7 @@ extern int gpio_direction_output(unsigne
extern int gpio_get_value_cansleep(unsigned gpio);
extern void gpio_set_value_cansleep(unsigned gpio, int value);
+extern int gpio_to_irq(unsigned gpio);
/* A platform's <asm/gpio.h> code may want to inline the I/O calls when
* the GPIO is constant and refers to some always-present controller,
S3C24XX: Add gpio_to_irq implementation
Add the necessary to_irq fields for the S3C24XX
GPIO banks that support IRQs.
Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Index: linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c
===================================================================
--- linux-2.6.26-quilt3.orig/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:35:15.000000000 +0100
+++ linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:37:39.000000000 +0100
@@ -150,6 +150,19 @@ static int s3c24xx_gpiolib_banka_output(
return 0;
}
+static int s3c24xx_gpiolib_bankf_toirq(struct gpio_chip *chip, unsigned offset)
+{
+ if (offset < 4)
+ return IRQ_EINT0 + offset;
+
+ return IRQ_EINT4 + (offset - 4);
+}
+
+static int s3c24xx_gpiolib_bankg_toirq(struct gpio_chip *chip, unsigned offset)
+{
+ return IRQ_EINT8 + offset;
+}
+
struct s3c24xx_gpio_chip gpios[] = {
[0] = {
@@ -228,6 +241,7 @@ struct s3c24xx_gpio_chip gpios[] = {
.direction_output = s3c24xx_gpiolib_output,
.set = s3c24xx_gpiolib_set,
.get = s3c24xx_gpiolib_get,
+ .to_irq = s3c24xx_gpiolib_bankf_toirq,
},
},
[6] = {
@@ -241,6 +255,7 @@ struct s3c24xx_gpio_chip gpios[] = {
.direction_output = s3c24xx_gpiolib_output,
.set = s3c24xx_gpiolib_set,
.get = s3c24xx_gpiolib_get,
+ .to_irq = s3c24xx_gpiolib_bankg_toirq,
},
},
};
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
[-- Attachment #2: simtec-gpiolib-add-irqmappings.patch --]
[-- Type: text/x-diff, Size: 2779 bytes --]
GPIO: Add generic gpio_to_irq call.
Add gpio_to_irq() implementation allowing the
gpio_chip registration to also specify an function
to map GPIO offsets into IRQs.
Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Index: linux-2.6.26-quilt3/drivers/gpio/gpiolib.c
===================================================================
--- linux-2.6.26-quilt3.orig/drivers/gpio/gpiolib.c 2008-07-18 00:40:52.000000000 +0100
+++ linux-2.6.26-quilt3/drivers/gpio/gpiolib.c 2008-07-18 00:52:07.000000000 +0100
@@ -339,6 +339,36 @@ const char *gpiochip_is_requested(struct
}
EXPORT_SYMBOL_GPL(gpiochip_is_requested);
+int gpio_to_irq(unsigned gpio)
+{
+ struct gpio_chip *chip;
+ struct gpio_desc *desc = &gpio_desc[gpio];
+ unsigned long flags;
+ int status = -EINVAL;
+
+ spin_lock_irqsave(&gpio_lock, flags);
+
+ if (!gpio_is_valid(gpio))
+ goto fail;
+
+ chip = desc->chip;
+ if (!chip || !chip->to_irq)
+ goto fail;
+
+ gpio -= chip->base;
+ if (gpio >= chip->ngpio)
+ goto fail;
+
+ status = chip->to_irq(chip, gpio);
+
+ fail:
+ spin_unlock_irqrestore(&gpio_lock, flags);
+ if (status)
+ pr_debug("%s: gpio-%d status %d\n",
+ __func__, gpio, status);
+ return status;
+}
+EXPORT_SYMBOL_GPL(gpio_to_irq);
/* Drivers MUST set GPIO direction before making get/set calls. In
* some cases this is done in early boot, before IRQs are enabled.
Index: linux-2.6.26-quilt3/include/asm-generic/gpio.h
===================================================================
--- linux-2.6.26-quilt3.orig/include/asm-generic/gpio.h 2008-07-18 00:40:52.000000000 +0100
+++ linux-2.6.26-quilt3/include/asm-generic/gpio.h 2008-07-18 00:46:32.000000000 +0100
@@ -40,6 +40,7 @@ struct module;
* @dbg_show: optional routine to show contents in debugfs; default code
* will be used when this is omitted, but custom code can show extra
* state (such as pullup/pulldown configuration).
+ * @to_irq: convert gpio offset to IRQ number.
* @base: identifies the first GPIO number handled by this chip; or, if
* negative during registration, requests dynamic ID allocation.
* @ngpio: the number of GPIOs handled by this controller; the last GPIO
@@ -71,6 +72,9 @@ struct gpio_chip {
unsigned offset, int value);
void (*dbg_show)(struct seq_file *s,
struct gpio_chip *chip);
+ int (*to_irq)(struct gpio_chip *chip,
+ unsigned offset);
+
int base;
u16 ngpio;
unsigned can_sleep:1;
@@ -97,6 +101,7 @@ extern int gpio_direction_output(unsigne
extern int gpio_get_value_cansleep(unsigned gpio);
extern void gpio_set_value_cansleep(unsigned gpio, int value);
+extern int gpio_to_irq(unsigned gpio);
/* A platform's <asm/gpio.h> code may want to inline the I/O calls when
* the GPIO is constant and refers to some always-present controller,
[-- Attachment #3: simtec-s3c24xx-gpiolib-toirq.patch --]
[-- Type: text/x-diff, Size: 1377 bytes --]
S3C24XX: Add gpio_to_irq implementation
Add the necessary to_irq fields for the S3C24XX
GPIO banks that support IRQs.
Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Index: linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c
===================================================================
--- linux-2.6.26-quilt3.orig/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:35:15.000000000 +0100
+++ linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:37:39.000000000 +0100
@@ -150,6 +150,19 @@ static int s3c24xx_gpiolib_banka_output(
return 0;
}
+static int s3c24xx_gpiolib_bankf_toirq(struct gpio_chip *chip, unsigned offset)
+{
+ if (offset < 4)
+ return IRQ_EINT0 + offset;
+
+ return IRQ_EINT4 + (offset - 4);
+}
+
+static int s3c24xx_gpiolib_bankg_toirq(struct gpio_chip *chip, unsigned offset)
+{
+ return IRQ_EINT8 + offset;
+}
+
struct s3c24xx_gpio_chip gpios[] = {
[0] = {
@@ -228,6 +241,7 @@ struct s3c24xx_gpio_chip gpios[] = {
.direction_output = s3c24xx_gpiolib_output,
.set = s3c24xx_gpiolib_set,
.get = s3c24xx_gpiolib_get,
+ .to_irq = s3c24xx_gpiolib_bankf_toirq,
},
},
[6] = {
@@ -241,6 +255,7 @@ struct s3c24xx_gpio_chip gpios[] = {
.direction_output = s3c24xx_gpiolib_output,
.set = s3c24xx_gpiolib_set,
.get = s3c24xx_gpiolib_get,
+ .to_irq = s3c24xx_gpiolib_bankg_toirq,
},
},
};
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Add to_irq fields to gpiolib (with sample implementation)
2008-07-18 13:04 Add to_irq fields to gpiolib (with sample implementation) Ben Dooks
@ 2008-07-21 10:43 ` Ramax Lo
2008-07-22 11:03 ` David Brownell
2008-07-22 14:15 ` Ben Dooks
0 siblings, 2 replies; 5+ messages in thread
From: Ramax Lo @ 2008-07-21 10:43 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-arm-kernel, linux-kernel
2008/7/18 Ben Dooks <ben-linux@fluff.org>:
> The two patches form a pair of patches to show
> that we should consider adding an to_irq field
> to the gpio_chip structure and gpiolib support.
>
Indeed, it's necessary to add a new field to provide
a general interface.
> The reason is that if we add support for devices
> registering gpio to also register interrputs, then
> a single arch-dependant interrupt mapping is not
> going to be sufficient.
>
> Note, this set does not remove any clashing
> definitions that may have of gpio_to_irq.
>
> ---
> GPIO: Add generic gpio_to_irq call.
>
> Add gpio_to_irq() implementation allowing the
> gpio_chip registration to also specify an function
> to map GPIO offsets into IRQs.
>
> Signed-off-by: Ben Dooks <ben-linux@fluff.org>
>
> Index: linux-2.6.26-quilt3/drivers/gpio/gpiolib.c
> ===================================================================
> --- linux-2.6.26-quilt3.orig/drivers/gpio/gpiolib.c 2008-07-18 00:40:52.000000000 +0100
> +++ linux-2.6.26-quilt3/drivers/gpio/gpiolib.c 2008-07-18 00:52:07.000000000 +0100
> @@ -339,6 +339,36 @@ const char *gpiochip_is_requested(struct
> }
> EXPORT_SYMBOL_GPL(gpiochip_is_requested);
>
> +int gpio_to_irq(unsigned gpio)
> +{
> + struct gpio_chip *chip;
> + struct gpio_desc *desc = &gpio_desc[gpio];
> + unsigned long flags;
> + int status = -EINVAL;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> +
> + if (!gpio_is_valid(gpio))
> + goto fail;
> +
> + chip = desc->chip;
> + if (!chip || !chip->to_irq)
> + goto fail;
> +
> + gpio -= chip->base;
> + if (gpio >= chip->ngpio)
> + goto fail;
> +
> + status = chip->to_irq(chip, gpio);
> +
> + fail:
> + spin_unlock_irqrestore(&gpio_lock, flags);
> + if (status)
> + pr_debug("%s: gpio-%d status %d\n",
> + __func__, gpio, status);
> + return status;
> +}
> +EXPORT_SYMBOL_GPL(gpio_to_irq);
>
Is it possible to define it as __gpio_to_irq(), and let people
define their macro or inline function, like the case of
__gpio_get_value(), to maintain compatibility?
> /* Drivers MUST set GPIO direction before making get/set calls. In
> * some cases this is done in early boot, before IRQs are enabled.
> Index: linux-2.6.26-quilt3/include/asm-generic/gpio.h
> ===================================================================
> --- linux-2.6.26-quilt3.orig/include/asm-generic/gpio.h 2008-07-18 00:40:52.000000000 +0100
> +++ linux-2.6.26-quilt3/include/asm-generic/gpio.h 2008-07-18 00:46:32.000000000 +0100
> @@ -40,6 +40,7 @@ struct module;
> * @dbg_show: optional routine to show contents in debugfs; default code
> * will be used when this is omitted, but custom code can show extra
> * state (such as pullup/pulldown configuration).
> + * @to_irq: convert gpio offset to IRQ number.
> * @base: identifies the first GPIO number handled by this chip; or, if
> * negative during registration, requests dynamic ID allocation.
> * @ngpio: the number of GPIOs handled by this controller; the last GPIO
> @@ -71,6 +72,9 @@ struct gpio_chip {
> unsigned offset, int value);
> void (*dbg_show)(struct seq_file *s,
> struct gpio_chip *chip);
> + int (*to_irq)(struct gpio_chip *chip,
> + unsigned offset);
> +
> int base;
> u16 ngpio;
> unsigned can_sleep:1;
> @@ -97,6 +101,7 @@ extern int gpio_direction_output(unsigne
> extern int gpio_get_value_cansleep(unsigned gpio);
> extern void gpio_set_value_cansleep(unsigned gpio, int value);
>
> +extern int gpio_to_irq(unsigned gpio);
>
> /* A platform's <asm/gpio.h> code may want to inline the I/O calls when
> * the GPIO is constant and refers to some always-present controller,
>
>
> S3C24XX: Add gpio_to_irq implementation
>
> Add the necessary to_irq fields for the S3C24XX
> GPIO banks that support IRQs.
>
> Signed-off-by: Ben Dooks <ben-linux@fluff.org>
>
> Index: linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c
> ===================================================================
> --- linux-2.6.26-quilt3.orig/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:35:15.000000000 +0100
> +++ linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:37:39.000000000 +0100
> @@ -150,6 +150,19 @@ static int s3c24xx_gpiolib_banka_output(
> return 0;
> }
>
> +static int s3c24xx_gpiolib_bankf_toirq(struct gpio_chip *chip, unsigned offset)
> +{
> + if (offset < 4)
> + return IRQ_EINT0 + offset;
> +
> + return IRQ_EINT4 + (offset - 4);
> +}
> +
> +static int s3c24xx_gpiolib_bankg_toirq(struct gpio_chip *chip, unsigned offset)
> +{
> + return IRQ_EINT8 + offset;
> +}
> +
>
> struct s3c24xx_gpio_chip gpios[] = {
> [0] = {
> @@ -228,6 +241,7 @@ struct s3c24xx_gpio_chip gpios[] = {
> .direction_output = s3c24xx_gpiolib_output,
> .set = s3c24xx_gpiolib_set,
> .get = s3c24xx_gpiolib_get,
> + .to_irq = s3c24xx_gpiolib_bankf_toirq,
> },
> },
> [6] = {
> @@ -241,6 +255,7 @@ struct s3c24xx_gpio_chip gpios[] = {
> .direction_output = s3c24xx_gpiolib_output,
> .set = s3c24xx_gpiolib_set,
> .get = s3c24xx_gpiolib_get,
> + .to_irq = s3c24xx_gpiolib_bankg_toirq,
> },
> },
> };
>
>
> --
> Ben (ben@fluff.org, http://www.fluff.org/)
>
> 'a smiley only costs 4 bytes'
>
> -------------------------------------------------------------------
> List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php
> Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Add to_irq fields to gpiolib (with sample implementation)
2008-07-21 10:43 ` Ramax Lo
@ 2008-07-22 11:03 ` David Brownell
2008-07-22 14:15 ` Ben Dooks
1 sibling, 0 replies; 5+ messages in thread
From: David Brownell @ 2008-07-22 11:03 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-arm-kernel, Ramax Lo, linux-kernel
On Monday 21 July 2008, Ramax Lo wrote:
> 2008/7/18 Ben Dooks <ben-linux@fluff.org>:
> > The two patches form a pair of patches to show
> > that we should consider adding an to_irq field
> > to the gpio_chip structure and gpiolib support.
> >
>
> Indeed, it's necessary to add a new field to provide
> a general interface.
>
> > The reason is that if we add support for devices
> > registering gpio to also register interrputs, then
> > a single arch-dependant interrupt mapping is not
> > going to be sufficient.
Fair enough, I guess ... although this does change
the cost of that mapping, and using this code also
presumes cooperation from that arch code. (It must
at least avoid pre-allocating every IRQ number so
that new chips -- MFD or otherwise -- can't add more.)
What about irq_to_gpio() calls though?
> > Note, this set does not remove any clashing
> > definitions that may have of gpio_to_irq.
And it shouldn't even define that call. It should
define an __gpio_to_irq() call so that arch code
can switch over incrementally (where it wants to).
> > ---
> > GPIO: Add generic gpio_to_irq call.
> >
> > Add gpio_to_irq() implementation allowing the
> > gpio_chip registration to also specify an function
> > to map GPIO offsets into IRQs.
> >
> > Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Diffstats please ... and relevant update to Documentation/gpio.txt,
minimally the stuff saying gpio_to_irq() costs on the order of an
addition or subtraction.
Right now drivers/input/keyboard/gpio_keys.c would be most
affected by increasing those costs; most other callers are
during setup code. It might need updates.
> >
> > Index: linux-2.6.26-quilt3/drivers/gpio/gpiolib.c
> > ===================================================================
> > --- linux-2.6.26-quilt3.orig/drivers/gpio/gpiolib.c 2008-07-18 00:40:52.000000000 +0100
> > +++ linux-2.6.26-quilt3/drivers/gpio/gpiolib.c 2008-07-18 00:52:07.000000000 +0100
> > @@ -339,6 +339,36 @@ const char *gpiochip_is_requested(struct
> > }
> > EXPORT_SYMBOL_GPL(gpiochip_is_requested);
> >
> > +int gpio_to_irq(unsigned gpio)
> > +{
> > + struct gpio_chip *chip;
> > + struct gpio_desc *desc = &gpio_desc[gpio];
> > + unsigned long flags;
> > + int status = -EINVAL;
> > +
> > + spin_lock_irqsave(&gpio_lock, flags);
> > +
> > + if (!gpio_is_valid(gpio))
> > + goto fail;
Notice that since it's defined to be an error to use this call
on anything that's not had gpio_direction_input() called, and
thus anything that's not been requested... you could avoid grabbing
that spinlock, testing whether the GPIO is valid, and whether
the gpio_chip is null.
> > +
> > + chip = desc->chip;
> > + if (!chip || !chip->to_irq)
> > + goto fail;
> > +
> > + gpio -= chip->base;
> > + if (gpio >= chip->ngpio)
> > + goto fail;
> > +
> > + status = chip->to_irq(chip, gpio);
> > +
> > + fail:
> > + spin_unlock_irqrestore(&gpio_lock, flags);
> > + if (status)
> > + pr_debug("%s: gpio-%d status %d\n",
> > + __func__, gpio, status);
> > + return status;
> > +}
> > +EXPORT_SYMBOL_GPL(gpio_to_irq);
> >
>
> Is it possible to define it as __gpio_to_irq(), and let people
> define their macro or inline function, like the case of
> __gpio_get_value(), to maintain compatibility?
Yes, and IMO that should be done. Along with kerneldoc
for this new __gpio_to_irq() call.
> > --- linux-2.6.26-quilt3.orig/include/asm-generic/gpio.h 2008-07-18 00:40:52.000000000 +0100
> > +++ linux-2.6.26-quilt3/include/asm-generic/gpio.h 2008-07-18 00:46:32.000000000 +0100
> > @@ -40,6 +40,7 @@ struct module;
> > * @dbg_show: optional routine to show contents in debugfs; default code
> > * will be used when this is omitted, but custom code can show extra
> > * state (such as pullup/pulldown configuration).
> > + * @to_irq: convert gpio offset to IRQ number.
> > * @base: identifies the first GPIO number handled by this chip; or, if
> > * negative during registration, requests dynamic ID allocation.
> > * @ngpio: the number of GPIOs handled by this controller; the last GPIO
> > @@ -71,6 +72,9 @@ struct gpio_chip {
> > unsigned offset, int value);
> > void (*dbg_show)(struct seq_file *s,
> > struct gpio_chip *chip);
> > + int (*to_irq)(struct gpio_chip *chip,
> > + unsigned offset);
> > +
> > int base;
> > u16 ngpio;
> > unsigned can_sleep:1;
> > @@ -97,6 +101,7 @@ extern int gpio_direction_output(unsigne
> > extern int gpio_get_value_cansleep(unsigned gpio);
> > extern void gpio_set_value_cansleep(unsigned gpio, int value);
> >
> > +extern int gpio_to_irq(unsigned gpio);
> >
> > /* A platform's <asm/gpio.h> code may want to inline the I/O calls when
> > * the GPIO is constant and refers to some always-present controller,
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Add to_irq fields to gpiolib (with sample implementation)
2008-07-21 10:43 ` Ramax Lo
2008-07-22 11:03 ` David Brownell
@ 2008-07-22 14:15 ` Ben Dooks
2008-07-23 3:47 ` Ramax Lo
1 sibling, 1 reply; 5+ messages in thread
From: Ben Dooks @ 2008-07-22 14:15 UTC (permalink / raw)
To: Ramax Lo; +Cc: Ben Dooks, linux-arm-kernel, linux-kernel
On Mon, Jul 21, 2008 at 06:43:02PM +0800, Ramax Lo wrote:
> 2008/7/18 Ben Dooks <ben-linux@fluff.org>:
> > The two patches form a pair of patches to show
> > that we should consider adding an to_irq field
> > to the gpio_chip structure and gpiolib support.
> >
>
> Indeed, it's necessary to add a new field to provide
> a general interface.
>
> > The reason is that if we add support for devices
> > registering gpio to also register interrputs, then
> > a single arch-dependant interrupt mapping is not
> > going to be sufficient.
> >
> > Note, this set does not remove any clashing
> > definitions that may have of gpio_to_irq.
> >
> > ---
> > GPIO: Add generic gpio_to_irq call.
> >
> > Add gpio_to_irq() implementation allowing the
> > gpio_chip registration to also specify an function
> > to map GPIO offsets into IRQs.
> >
> > Signed-off-by: Ben Dooks <ben-linux@fluff.org>
> >
> > Index: linux-2.6.26-quilt3/drivers/gpio/gpiolib.c
> > ===================================================================
> > --- linux-2.6.26-quilt3.orig/drivers/gpio/gpiolib.c 2008-07-18 00:40:52.000000000 +0100
> > +++ linux-2.6.26-quilt3/drivers/gpio/gpiolib.c 2008-07-18 00:52:07.000000000 +0100
> > @@ -339,6 +339,36 @@ const char *gpiochip_is_requested(struct
> > }
> > EXPORT_SYMBOL_GPL(gpiochip_is_requested);
> >
> > +int gpio_to_irq(unsigned gpio)
> > +{
> > + struct gpio_chip *chip;
> > + struct gpio_desc *desc = &gpio_desc[gpio];
> > + unsigned long flags;
> > + int status = -EINVAL;
> > +
> > + spin_lock_irqsave(&gpio_lock, flags);
> > +
> > + if (!gpio_is_valid(gpio))
> > + goto fail;
> > +
> > + chip = desc->chip;
> > + if (!chip || !chip->to_irq)
> > + goto fail;
> > +
> > + gpio -= chip->base;
> > + if (gpio >= chip->ngpio)
> > + goto fail;
> > +
> > + status = chip->to_irq(chip, gpio);
> > +
> > + fail:
> > + spin_unlock_irqrestore(&gpio_lock, flags);
> > + if (status)
> > + pr_debug("%s: gpio-%d status %d\n",
> > + __func__, gpio, status);
> > + return status;
> > +}
> > +EXPORT_SYMBOL_GPL(gpio_to_irq);
> >
>
> Is it possible to define it as __gpio_to_irq(), and let people
> define their macro or inline function, like the case of
> __gpio_get_value(), to maintain compatibility?
Ok, should I resubmit with this changed?
> > /* Drivers MUST set GPIO direction before making get/set calls. In
> > * some cases this is done in early boot, before IRQs are enabled.
> > Index: linux-2.6.26-quilt3/include/asm-generic/gpio.h
> > ===================================================================
> > --- linux-2.6.26-quilt3.orig/include/asm-generic/gpio.h 2008-07-18 00:40:52.000000000 +0100
> > +++ linux-2.6.26-quilt3/include/asm-generic/gpio.h 2008-07-18 00:46:32.000000000 +0100
> > @@ -40,6 +40,7 @@ struct module;
> > * @dbg_show: optional routine to show contents in debugfs; default code
> > * will be used when this is omitted, but custom code can show extra
> > * state (such as pullup/pulldown configuration).
> > + * @to_irq: convert gpio offset to IRQ number.
> > * @base: identifies the first GPIO number handled by this chip; or, if
> > * negative during registration, requests dynamic ID allocation.
> > * @ngpio: the number of GPIOs handled by this controller; the last GPIO
> > @@ -71,6 +72,9 @@ struct gpio_chip {
> > unsigned offset, int value);
> > void (*dbg_show)(struct seq_file *s,
> > struct gpio_chip *chip);
> > + int (*to_irq)(struct gpio_chip *chip,
> > + unsigned offset);
> > +
> > int base;
> > u16 ngpio;
> > unsigned can_sleep:1;
> > @@ -97,6 +101,7 @@ extern int gpio_direction_output(unsigne
> > extern int gpio_get_value_cansleep(unsigned gpio);
> > extern void gpio_set_value_cansleep(unsigned gpio, int value);
> >
> > +extern int gpio_to_irq(unsigned gpio);
> >
> > /* A platform's <asm/gpio.h> code may want to inline the I/O calls when
> > * the GPIO is constant and refers to some always-present controller,
> >
> >
> > S3C24XX: Add gpio_to_irq implementation
> >
> > Add the necessary to_irq fields for the S3C24XX
> > GPIO banks that support IRQs.
> >
> > Signed-off-by: Ben Dooks <ben-linux@fluff.org>
> >
> > Index: linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c
> > ===================================================================
> > --- linux-2.6.26-quilt3.orig/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:35:15.000000000 +0100
> > +++ linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:37:39.000000000 +0100
> > @@ -150,6 +150,19 @@ static int s3c24xx_gpiolib_banka_output(
> > return 0;
> > }
> >
> > +static int s3c24xx_gpiolib_bankf_toirq(struct gpio_chip *chip, unsigned offset)
> > +{
> > + if (offset < 4)
> > + return IRQ_EINT0 + offset;
> > +
> > + return IRQ_EINT4 + (offset - 4);
> > +}
> > +
> > +static int s3c24xx_gpiolib_bankg_toirq(struct gpio_chip *chip, unsigned offset)
> > +{
> > + return IRQ_EINT8 + offset;
> > +}
> > +
> >
> > struct s3c24xx_gpio_chip gpios[] = {
> > [0] = {
> > @@ -228,6 +241,7 @@ struct s3c24xx_gpio_chip gpios[] = {
> > .direction_output = s3c24xx_gpiolib_output,
> > .set = s3c24xx_gpiolib_set,
> > .get = s3c24xx_gpiolib_get,
> > + .to_irq = s3c24xx_gpiolib_bankf_toirq,
> > },
> > },
> > [6] = {
> > @@ -241,6 +255,7 @@ struct s3c24xx_gpio_chip gpios[] = {
> > .direction_output = s3c24xx_gpiolib_output,
> > .set = s3c24xx_gpiolib_set,
> > .get = s3c24xx_gpiolib_get,
> > + .to_irq = s3c24xx_gpiolib_bankg_toirq,
> > },
> > },
> > };
> >
> >
> > --
> > Ben (ben@fluff.org, http://www.fluff.org/)
> >
> > 'a smiley only costs 4 bytes'
> >
> > -------------------------------------------------------------------
> > List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> > FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php
> > Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
> >
>
> -------------------------------------------------------------------
> List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php
> Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
--
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Add to_irq fields to gpiolib (with sample implementation)
2008-07-22 14:15 ` Ben Dooks
@ 2008-07-23 3:47 ` Ramax Lo
0 siblings, 0 replies; 5+ messages in thread
From: Ramax Lo @ 2008-07-23 3:47 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-arm-kernel, linux-kernel
2008/7/22 Ben Dooks <ben-linux@fluff.org>:
> On Mon, Jul 21, 2008 at 06:43:02PM +0800, Ramax Lo wrote:
>> 2008/7/18 Ben Dooks <ben-linux@fluff.org>:
>> > The two patches form a pair of patches to show
>> > that we should consider adding an to_irq field
>> > to the gpio_chip structure and gpiolib support.
>> >
>>
>> Indeed, it's necessary to add a new field to provide
>> a general interface.
>>
>> > The reason is that if we add support for devices
>> > registering gpio to also register interrputs, then
>> > a single arch-dependant interrupt mapping is not
>> > going to be sufficient.
>> >
>> > Note, this set does not remove any clashing
>> > definitions that may have of gpio_to_irq.
>> >
>> > ---
>> > GPIO: Add generic gpio_to_irq call.
>> >
>> > Add gpio_to_irq() implementation allowing the
>> > gpio_chip registration to also specify an function
>> > to map GPIO offsets into IRQs.
>> >
>> > Signed-off-by: Ben Dooks <ben-linux@fluff.org>
>> >
>> > Index: linux-2.6.26-quilt3/drivers/gpio/gpiolib.c
>> > ===================================================================
>> > --- linux-2.6.26-quilt3.orig/drivers/gpio/gpiolib.c 2008-07-18 00:40:52.000000000 +0100
>> > +++ linux-2.6.26-quilt3/drivers/gpio/gpiolib.c 2008-07-18 00:52:07.000000000 +0100
>> > @@ -339,6 +339,36 @@ const char *gpiochip_is_requested(struct
>> > }
>> > EXPORT_SYMBOL_GPL(gpiochip_is_requested);
>> >
>> > +int gpio_to_irq(unsigned gpio)
>> > +{
>> > + struct gpio_chip *chip;
>> > + struct gpio_desc *desc = &gpio_desc[gpio];
>> > + unsigned long flags;
>> > + int status = -EINVAL;
>> > +
>> > + spin_lock_irqsave(&gpio_lock, flags);
>> > +
>> > + if (!gpio_is_valid(gpio))
>> > + goto fail;
>> > +
>> > + chip = desc->chip;
>> > + if (!chip || !chip->to_irq)
>> > + goto fail;
>> > +
>> > + gpio -= chip->base;
>> > + if (gpio >= chip->ngpio)
>> > + goto fail;
>> > +
>> > + status = chip->to_irq(chip, gpio);
>> > +
>> > + fail:
>> > + spin_unlock_irqrestore(&gpio_lock, flags);
>> > + if (status)
>> > + pr_debug("%s: gpio-%d status %d\n",
>> > + __func__, gpio, status);
>> > + return status;
>> > +}
>> > +EXPORT_SYMBOL_GPL(gpio_to_irq);
>> >
>>
>> Is it possible to define it as __gpio_to_irq(), and let people
>> define their macro or inline function, like the case of
>> __gpio_get_value(), to maintain compatibility?
>
> Ok, should I resubmit with this changed?
>
Yes, thanks.
>> > /* Drivers MUST set GPIO direction before making get/set calls. In
>> > * some cases this is done in early boot, before IRQs are enabled.
>> > Index: linux-2.6.26-quilt3/include/asm-generic/gpio.h
>> > ===================================================================
>> > --- linux-2.6.26-quilt3.orig/include/asm-generic/gpio.h 2008-07-18 00:40:52.000000000 +0100
>> > +++ linux-2.6.26-quilt3/include/asm-generic/gpio.h 2008-07-18 00:46:32.000000000 +0100
>> > @@ -40,6 +40,7 @@ struct module;
>> > * @dbg_show: optional routine to show contents in debugfs; default code
>> > * will be used when this is omitted, but custom code can show extra
>> > * state (such as pullup/pulldown configuration).
>> > + * @to_irq: convert gpio offset to IRQ number.
>> > * @base: identifies the first GPIO number handled by this chip; or, if
>> > * negative during registration, requests dynamic ID allocation.
>> > * @ngpio: the number of GPIOs handled by this controller; the last GPIO
>> > @@ -71,6 +72,9 @@ struct gpio_chip {
>> > unsigned offset, int value);
>> > void (*dbg_show)(struct seq_file *s,
>> > struct gpio_chip *chip);
>> > + int (*to_irq)(struct gpio_chip *chip,
>> > + unsigned offset);
>> > +
>> > int base;
>> > u16 ngpio;
>> > unsigned can_sleep:1;
>> > @@ -97,6 +101,7 @@ extern int gpio_direction_output(unsigne
>> > extern int gpio_get_value_cansleep(unsigned gpio);
>> > extern void gpio_set_value_cansleep(unsigned gpio, int value);
>> >
>> > +extern int gpio_to_irq(unsigned gpio);
>> >
>> > /* A platform's <asm/gpio.h> code may want to inline the I/O calls when
>> > * the GPIO is constant and refers to some always-present controller,
>> >
>> >
>> > S3C24XX: Add gpio_to_irq implementation
>> >
>> > Add the necessary to_irq fields for the S3C24XX
>> > GPIO banks that support IRQs.
>> >
>> > Signed-off-by: Ben Dooks <ben-linux@fluff.org>
>> >
>> > Index: linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c
>> > ===================================================================
>> > --- linux-2.6.26-quilt3.orig/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:35:15.000000000 +0100
>> > +++ linux-2.6.26-quilt3/arch/arm/plat-s3c24xx/gpiolib.c 2008-07-18 12:37:39.000000000 +0100
>> > @@ -150,6 +150,19 @@ static int s3c24xx_gpiolib_banka_output(
>> > return 0;
>> > }
>> >
>> > +static int s3c24xx_gpiolib_bankf_toirq(struct gpio_chip *chip, unsigned offset)
>> > +{
>> > + if (offset < 4)
>> > + return IRQ_EINT0 + offset;
>> > +
>> > + return IRQ_EINT4 + (offset - 4);
>> > +}
>> > +
>> > +static int s3c24xx_gpiolib_bankg_toirq(struct gpio_chip *chip, unsigned offset)
>> > +{
>> > + return IRQ_EINT8 + offset;
>> > +}
>> > +
>> >
>> > struct s3c24xx_gpio_chip gpios[] = {
>> > [0] = {
>> > @@ -228,6 +241,7 @@ struct s3c24xx_gpio_chip gpios[] = {
>> > .direction_output = s3c24xx_gpiolib_output,
>> > .set = s3c24xx_gpiolib_set,
>> > .get = s3c24xx_gpiolib_get,
>> > + .to_irq = s3c24xx_gpiolib_bankf_toirq,
>> > },
>> > },
>> > [6] = {
>> > @@ -241,6 +255,7 @@ struct s3c24xx_gpio_chip gpios[] = {
>> > .direction_output = s3c24xx_gpiolib_output,
>> > .set = s3c24xx_gpiolib_set,
>> > .get = s3c24xx_gpiolib_get,
>> > + .to_irq = s3c24xx_gpiolib_bankg_toirq,
>> > },
>> > },
>> > };
>> >
>> >
>> > --
>> > Ben (ben@fluff.org, http://www.fluff.org/)
>> >
>> > 'a smiley only costs 4 bytes'
>> >
>> > -------------------------------------------------------------------
>> > List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
>> > FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php
>> > Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
>> >
>>
>> -------------------------------------------------------------------
>> List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
>> FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php
>> Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
>
> --
> --
> Ben
>
> Q: What's a light-year?
> A: One-third less calories than a regular year.
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-07-23 3:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-18 13:04 Add to_irq fields to gpiolib (with sample implementation) Ben Dooks
2008-07-21 10:43 ` Ramax Lo
2008-07-22 11:03 ` David Brownell
2008-07-22 14:15 ` Ben Dooks
2008-07-23 3:47 ` Ramax Lo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox