[PATCH 2/3] gpiolib: implement gpiochip_reserve - Kernel

This is a discussion on [PATCH 2/3] gpiolib: implement gpiochip_reserve - Kernel ; Function gpiochip_reserve() reserves range of gpios to use with platform code only, that is, this function used to mark specified range of gpios unavailable for the dynamic gpio base allocator. Signed-off-by: Anton Vorontsov --- drivers/gpio/gpiolib.c | 51 +++++++++++++++++++++++++++++++++++++++++-- include/asm-generic/gpio.h | ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: [PATCH 2/3] gpiolib: implement gpiochip_reserve

  1. [PATCH 2/3] gpiolib: implement gpiochip_reserve

    Function gpiochip_reserve() reserves range of gpios to use with
    platform code only, that is, this function used to mark specified
    range of gpios unavailable for the dynamic gpio base allocator.

    Signed-off-by: Anton Vorontsov
    ---
    drivers/gpio/gpiolib.c | 51 +++++++++++++++++++++++++++++++++++++++++--
    include/asm-generic/gpio.h | 1 +
    2 files changed, 49 insertions(+), 3 deletions(-)

    diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
    index 81d81c9..2149e0c 100644
    --- a/drivers/gpio/gpiolib.c
    +++ b/drivers/gpio/gpiolib.c
    @@ -43,6 +43,7 @@ struct gpio_desc {
    /* flag symbols are bit numbers */
    #define FLAG_REQUESTED 0
    #define FLAG_IS_OUT 1
    +#define FLAG_RESERVED 2

    #ifdef CONFIG_DEBUG_FS
    const char *label;
    @@ -80,6 +81,48 @@ static inline struct gpio_chip *gpio_to_chip(unsigned gpio)
    return gpio_desc[gpio].chip;
    }

    +/**
    + * gpiochip_reserve() - reserve range of gpios to use with platform code only
    + * @start: starting gpio number
    + * @ngpio: number of gpios to reserve
    + * Context: potentially before irqs or kmalloc will work
    + *
    + * Returns a negative errno if any gpio within the range already reserved
    + * or registered. Otherwise it returns zero as a success code.
    + * Use this function to mark specified range of gpios unavailable for the
    + * dynamic gpio base allocator.
    + */
    +int __must_check gpiochip_reserve(int start, int ngpio)
    +{
    + int ret = 0;
    + unsigned long flags;
    + int i;
    +
    + if (!gpio_is_valid(start) || !gpio_is_valid(start + ngpio))
    + return -EINVAL;
    +
    + spin_lock_irqsave(&gpio_lock, flags);
    +
    + for (i = start; i < start + ngpio; i++) {
    + struct gpio_desc *desc = &gpio_desc[i];
    +
    + if (desc->chip || test_bit(FLAG_RESERVED, &desc->flags)) {
    + ret = -EBUSY;
    + goto err;
    + }
    +
    + set_bit(FLAG_RESERVED, &desc->flags);
    + }
    +
    + pr_debug("%s: reserved gpios from %d to %d\n",
    + __func__, start, start + ngpio - 1);
    +err:
    + spin_unlock_irqrestore(&gpio_lock, flags);
    +
    + return ret;
    +}
    +EXPORT_SYMBOL_GPL(gpiochip_reserve);
    +
    static int gpiochip_find_base(int ngpio)
    {
    int i;
    @@ -87,9 +130,10 @@ static int gpiochip_find_base(int ngpio)
    int base = -ENOSPC;

    for (i = ARCH_NR_GPIOS - 1; i >= 0 ; i--) {
    - struct gpio_chip *chip = gpio_desc[i].chip;
    + struct gpio_desc *desc = &gpio_desc[i];
    + struct gpio_chip *chip = desc->chip;

    - if (!chip) {
    + if (!chip && !test_bit(FLAG_RESERVED, &desc->flags)) {
    spare++;
    if (spare == ngpio) {
    base = i;
    @@ -97,7 +141,8 @@ static int gpiochip_find_base(int ngpio)
    }
    } else {
    spare = 0;
    - i -= chip->ngpio - 1;
    + if (chip)
    + i -= chip->ngpio - 1;
    }
    }

    diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
    index 464c5b3..20f5d67 100644
    --- a/include/asm-generic/gpio.h
    +++ b/include/asm-generic/gpio.h
    @@ -74,6 +74,7 @@ struct gpio_chip {

    extern const char *gpiochip_is_requested(struct gpio_chip *chip,
    unsigned offset);
    +extern int __must_check gpiochip_reserve(int start, int ngpio);

    /* add/remove chips */
    extern int gpiochip_add(struct gpio_chip *chip);
    --
    1.5.2.2

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

  2. Re: [PATCH 2/3] gpiolib: implement gpiochip_reserve

    On Tuesday 11 March 2008, Anton Vorontsov wrote:
    > Function gpiochip_reserve() reserves range of gpios to use with
    > platform code only, that is, this function used to mark specified
    > range of gpios unavailable for the dynamic gpio base allocator.


    Since this is for platform code only, it doesn't need to stick
    around after initcalls have run. So please mark it as __init so
    that we minimize the footprint cost.

    - Dave

    >
    > Signed-off-by: Anton Vorontsov
    > ---
    > drivers/gpio/gpiolib.c | 51 +++++++++++++++++++++++++++++++++++++++++--
    > include/asm-generic/gpio.h | 1 +
    > 2 files changed, 49 insertions(+), 3 deletions(-)
    >
    > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
    > index 81d81c9..2149e0c 100644
    > --- a/drivers/gpio/gpiolib.c
    > +++ b/drivers/gpio/gpiolib.c
    > @@ -43,6 +43,7 @@ struct gpio_desc {
    > /* flag symbols are bit numbers */
    > #define FLAG_REQUESTED 0
    > #define FLAG_IS_OUT 1
    > +#define FLAG_RESERVED 2
    >
    > #ifdef CONFIG_DEBUG_FS
    > const char *label;
    > @@ -80,6 +81,48 @@ static inline struct gpio_chip *gpio_to_chip(unsigned gpio)
    > return gpio_desc[gpio].chip;
    > }
    >
    > +/**
    > + * gpiochip_reserve() - reserve range of gpios to use with platform code only
    > + * @start: starting gpio number
    > + * @ngpio: number of gpios to reserve
    > + * Context: potentially before irqs or kmalloc will work
    > + *
    > + * Returns a negative errno if any gpio within the range already reserved
    > + * or registered. Otherwise it returns zero as a success code.
    > + * Use this function to mark specified range of gpios unavailable for the
    > + * dynamic gpio base allocator.
    > + */
    > +int __must_check gpiochip_reserve(int start, int ngpio)
    > +{
    > + int ret = 0;
    > + unsigned long flags;
    > + int i;
    > +
    > + if (!gpio_is_valid(start) || !gpio_is_valid(start + ngpio))
    > + return -EINVAL;
    > +
    > + spin_lock_irqsave(&gpio_lock, flags);
    > +
    > + for (i = start; i < start + ngpio; i++) {
    > + struct gpio_desc *desc = &gpio_desc[i];
    > +
    > + if (desc->chip || test_bit(FLAG_RESERVED, &desc->flags)) {
    > + ret = -EBUSY;
    > + goto err;
    > + }
    > +
    > + set_bit(FLAG_RESERVED, &desc->flags);
    > + }
    > +
    > + pr_debug("%s: reserved gpios from %d to %d\n",
    > + __func__, start, start + ngpio - 1);
    > +err:
    > + spin_unlock_irqrestore(&gpio_lock, flags);
    > +
    > + return ret;
    > +}
    > +EXPORT_SYMBOL_GPL(gpiochip_reserve);
    > +
    > static int gpiochip_find_base(int ngpio)
    > {
    > int i;
    > @@ -87,9 +130,10 @@ static int gpiochip_find_base(int ngpio)
    > int base = -ENOSPC;
    >
    > for (i = ARCH_NR_GPIOS - 1; i >= 0 ; i--) {
    > - struct gpio_chip *chip = gpio_desc[i].chip;
    > + struct gpio_desc *desc = &gpio_desc[i];
    > + struct gpio_chip *chip = desc->chip;
    >
    > - if (!chip) {
    > + if (!chip && !test_bit(FLAG_RESERVED, &desc->flags)) {
    > spare++;
    > if (spare == ngpio) {
    > base = i;
    > @@ -97,7 +141,8 @@ static int gpiochip_find_base(int ngpio)
    > }
    > } else {
    > spare = 0;
    > - i -= chip->ngpio - 1;
    > + if (chip)
    > + i -= chip->ngpio - 1;


    Isn't this little optimization a bit error prone? It
    presumes the reserved bits and gpio chips are in fairly
    close synch. I'd just remove it ... it shouldn't be a
    common path.

    > }
    > }
    >
    > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
    > index 464c5b3..20f5d67 100644
    > --- a/include/asm-generic/gpio.h
    > +++ b/include/asm-generic/gpio.h
    > @@ -74,6 +74,7 @@ struct gpio_chip {
    >
    > extern const char *gpiochip_is_requested(struct gpio_chip *chip,
    > unsigned offset);
    > +extern int __must_check gpiochip_reserve(int start, int ngpio);
    >
    > /* add/remove chips */
    > extern int gpiochip_add(struct gpio_chip *chip);
    > --
    > 1.5.2.2
    >



    --
    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: [PATCH 2/3] gpiolib: implement gpiochip_reserve

    On Tue, Mar 11, 2008 at 01:11:37PM -0800, David Brownell wrote:
    > On Tuesday 11 March 2008, Anton Vorontsov wrote:
    > > Function gpiochip_reserve() reserves range of gpios to use with
    > > platform code only, that is, this function used to mark specified
    > > range of gpios unavailable for the dynamic gpio base allocator.

    >
    > Since this is for platform code only, it doesn't need to stick
    > around after initcalls have run. So please mark it as __init so
    > that we minimize the footprint cost.


    Good idea, thanks.

    [...]
    > > for (i = ARCH_NR_GPIOS - 1; i >= 0 ; i--) {
    > > - struct gpio_chip *chip = gpio_desc[i].chip;
    > > + struct gpio_desc *desc = &gpio_desc[i];
    > > + struct gpio_chip *chip = desc->chip;
    > >
    > > - if (!chip) {
    > > + if (!chip && !test_bit(FLAG_RESERVED, &desc->flags)) {
    > > spare++;
    > > if (spare == ngpio) {
    > > base = i;
    > > @@ -97,7 +141,8 @@ static int gpiochip_find_base(int ngpio)
    > > }
    > > } else {
    > > spare = 0;
    > > - i -= chip->ngpio - 1;
    > > + if (chip)
    > > + i -= chip->ngpio - 1;

    >
    > Isn't this little optimization a bit error prone?


    So far I don't see how exactly it could be error prone.

    > It
    > presumes the reserved bits and gpio chips are in fairly
    > close synch.


    Probably I don't understand the synch part here. The thing is that
    I'm replacing if (!chip) with if (!chip && !test_bit(FLAG_RESERVED,
    &desc->flags)), so now "else" code might be executed when gpio is
    either reserved or the range of gpios is already managed by the chip.
    In later case we can skip whole range, because we know its size.
    In case of reserved or spare gpio we can't do that, so we simply
    try the next one. Anything wrong with this scheme?..


    Below the patch with __init attribute added.

    - - - -
    From: Anton Vorontsov
    Subject: gpiolib: implement gpiochip_reserve

    Function gpiochip_reserve() reserves range of gpios to use with
    platform code only, that is, this function used to mark specified
    range of gpios unavailable for the dynamic gpio base allocator.

    Signed-off-by: Anton Vorontsov
    ---
    drivers/gpio/gpiolib.c | 51 +++++++++++++++++++++++++++++++++++++++++--
    include/asm-generic/gpio.h | 1 +
    2 files changed, 49 insertions(+), 3 deletions(-)

    diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
    index 81d81c9..372bcc5 100644
    --- a/drivers/gpio/gpiolib.c
    +++ b/drivers/gpio/gpiolib.c
    @@ -43,6 +43,7 @@ struct gpio_desc {
    /* flag symbols are bit numbers */
    #define FLAG_REQUESTED 0
    #define FLAG_IS_OUT 1
    +#define FLAG_RESERVED 2

    #ifdef CONFIG_DEBUG_FS
    const char *label;
    @@ -80,6 +81,48 @@ static inline struct gpio_chip *gpio_to_chip(unsigned gpio)
    return gpio_desc[gpio].chip;
    }

    +/**
    + * gpiochip_reserve() - reserve range of gpios to use with platform code only
    + * @start: starting gpio number
    + * @ngpio: number of gpios to reserve
    + * Context: potentially before irqs or kmalloc will work
    + *
    + * Returns a negative errno if any gpio within the range already reserved
    + * or registered. Otherwise it returns zero as a success code.
    + * Use this function to mark specified range of gpios unavailable for the
    + * dynamic gpio base allocator.
    + */
    +int __init __must_check gpiochip_reserve(int start, int ngpio)
    +{
    + int ret = 0;
    + unsigned long flags;
    + int i;
    +
    + if (!gpio_is_valid(start) || !gpio_is_valid(start + ngpio))
    + return -EINVAL;
    +
    + spin_lock_irqsave(&gpio_lock, flags);
    +
    + for (i = start; i < start + ngpio; i++) {
    + struct gpio_desc *desc = &gpio_desc[i];
    +
    + if (desc->chip || test_bit(FLAG_RESERVED, &desc->flags)) {
    + ret = -EBUSY;
    + goto err;
    + }
    +
    + set_bit(FLAG_RESERVED, &desc->flags);
    + }
    +
    + pr_debug("%s: reserved gpios from %d to %d\n",
    + __func__, start, start + ngpio - 1);
    +err:
    + spin_unlock_irqrestore(&gpio_lock, flags);
    +
    + return ret;
    +}
    +EXPORT_SYMBOL_GPL(gpiochip_reserve);
    +
    static int gpiochip_find_base(int ngpio)
    {
    int i;
    @@ -87,9 +130,10 @@ static int gpiochip_find_base(int ngpio)
    int base = -ENOSPC;

    for (i = ARCH_NR_GPIOS - 1; i >= 0 ; i--) {
    - struct gpio_chip *chip = gpio_desc[i].chip;
    + struct gpio_desc *desc = &gpio_desc[i];
    + struct gpio_chip *chip = desc->chip;

    - if (!chip) {
    + if (!chip && !test_bit(FLAG_RESERVED, &desc->flags)) {
    spare++;
    if (spare == ngpio) {
    base = i;
    @@ -97,7 +141,8 @@ static int gpiochip_find_base(int ngpio)
    }
    } else {
    spare = 0;
    - i -= chip->ngpio - 1;
    + if (chip)
    + i -= chip->ngpio - 1;
    }
    }

    diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
    index 464c5b3..ecf675a 100644
    --- a/include/asm-generic/gpio.h
    +++ b/include/asm-generic/gpio.h
    @@ -74,6 +74,7 @@ struct gpio_chip {

    extern const char *gpiochip_is_requested(struct gpio_chip *chip,
    unsigned offset);
    +extern int __init __must_check gpiochip_reserve(int start, int ngpio);

    /* add/remove chips */
    extern int gpiochip_add(struct gpio_chip *chip);
    --
    1.5.2.2

    --
    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: [PATCH 2/3] gpiolib: implement gpiochip_reserve

    On Wednesday 12 March 2008, Anton Vorontsov wrote:
    > > It
    > > presumes the reserved bits and gpio chips are in fairly
    > > close synch.

    >
    > Probably I don't understand the synch part here. The thing is that
    > I'm replacing if (!chip) with if (!chip && !test_bit(FLAG_RESERVED,
    > &desc->flags)), so now "else" code might be executed when gpio is
    > either reserved or the range of gpios is already managed by the chip.


    OK then.


    > Below the patch with __init attribute added.
    >
    > - - - -
    > From: Anton Vorontsov
    > Subject: gpiolib: implement gpiochip_reserve
    >
    > Function gpiochip_reserve() reserves range of gpios to use with
    > platform code only, that is, this function used to mark specified
    > range of gpios unavailable for the dynamic gpio base allocator.
    >
    > Signed-off-by: Anton Vorontsov


    Acked-by: David Brownell

    > ---
    > ...

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