Add to_irq fields to gpiolib (with sample implementation) - Kernel

This is a discussion on Add to_irq fields to gpiolib (with sample implementation) - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: Add to_irq fields to gpiolib (with sample implementation)

  1. Add to_irq fields to gpiolib (with sample implementation)

    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

    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 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

    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'


  2. Re: Add to_irq fields to gpiolib (with sample implementation)

    2008/7/18 Ben Dooks :
    > 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
    >
    > 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 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
    >
    > 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/mailma...nux-arm-kernel
    > FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php
    > Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
    >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: Add to_irq fields to gpiolib (with sample implementation)

    On Monday 21 July 2008, Ramax Lo wrote:
    > 2008/7/18 Ben Dooks :
    > > 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


    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 code may want to inline the I/O calls when
    > > * the GPIO is constant and refers to some always-present controller,
    > >
    > >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: Add to_irq fields to gpiolib (with sample implementation)

    On Mon, Jul 21, 2008 at 06:43:02PM +0800, Ramax Lo wrote:
    > 2008/7/18 Ben Dooks :
    > > 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
    > >
    > > 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 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
    > >
    > > 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/mailma...nux-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/mailma...nux-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.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: Add to_irq fields to gpiolib (with sample implementation)

    2008/7/22 Ben Dooks :
    > On Mon, Jul 21, 2008 at 06:43:02PM +0800, Ramax Lo wrote:
    >> 2008/7/18 Ben Dooks :
    >> > 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
    >> >
    >> > 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 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
    >> >
    >> > 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/mailma...nux-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/mailma...nux-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.
    >
    >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread