| Message ID | 1359822572-26009-7-git-send-email-acourbot@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Feb 2, 2013 at 5:29 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: > Re-implement gpiochip_find_base using the list of chips instead of the > global gpio_desc[] array. This makes it both simpler and more efficient, > and is needed to remove the global descriptors array. > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> So Haojian just submitted a different patch to the same piece of code staring to search for GPIO number in ascending order instead, but I NACKed it. This looks like it is preserving this userspace-sensitive semantic so that dynamically added chips will still get the same assigned numbers. But I want some guarantees, so state clearly in the commit that any dynamically assigned chip will get the same base address after this change as it got before it, and please take this opportunity to add a comment stating that this search method is vital for userspace ABIs, and must be preserved. Yours, Linus Walleij
On 02/06/2013 02:21 AM, Linus Walleij wrote: > This looks like it is preserving this userspace-sensitive semantic > so that dynamically added chips will still get the same assigned > numbers. It does (it should, at least), the assigned ranges should be strictly identical to the previous version. While testing I also made sure all chips had the same GPIO range. > But I want some guarantees, so state clearly in the commit > that any dynamically assigned chip will get the same base > address after this change as it got before it, and please take > this opportunity to add a comment stating that this search > method is vital for userspace ABIs, and must be preserved. Done. I will take the blame if something goes wrong. :) Thanks, Alex.
On Wed, 6 Feb 2013 13:48:19 +0900, Alex Courbot <acourbot@nvidia.com> wrote: > On 02/06/2013 02:21 AM, Linus Walleij wrote: > > This looks like it is preserving this userspace-sensitive semantic > > so that dynamically added chips will still get the same assigned > > numbers. > > It does (it should, at least), the assigned ranges should be strictly > identical to the previous version. While testing I also made sure all > chips had the same GPIO range. > > > But I want some guarantees, so state clearly in the commit > > that any dynamically assigned chip will get the same base > > address after this change as it got before it, and please take > > this opportunity to add a comment stating that this search > > method is vital for userspace ABIs, and must be preserved. > > Done. I will take the blame if something goes wrong. :) Applied, thanks g.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index e473ded..af4c350 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -126,30 +126,25 @@ struct gpio_chip *gpio_to_chip(unsigned gpio) /* dynamic allocation of GPIOs, e.g. on a hotplugged device */ static int gpiochip_find_base(int ngpio) { - int i; - int spare = 0; - int base = -ENOSPC; - - for (i = ARCH_NR_GPIOS - 1; i >= 0 ; i--) { - struct gpio_desc *desc = &gpio_desc[i]; - struct gpio_chip *chip = desc->chip; - - if (!chip) { - spare++; - if (spare == ngpio) { - base = i; - break; - } - } else { - spare = 0; - if (chip) - i -= chip->ngpio - 1; - } + struct gpio_chip *chip; + int base = ARCH_NR_GPIOS - ngpio; + + list_for_each_entry_reverse(chip, &gpio_chips, list) { + /* found a free space? */ + if (chip->base + chip->ngpio <= base) + break; + else + /* nope, check the space right before the chip */ + base = chip->base - ngpio; } - if (gpio_is_valid(base)) + if (gpio_is_valid(base)) { pr_debug("%s: found new base at %d\n", __func__, base); - return base; + return base; + } else { + pr_err("%s: cannot find free range\n", __func__); + return -ENOSPC; + } } /* caller ensures gpio is valid and requested, chip->get_direction may sleep */
Re-implement gpiochip_find_base using the list of chips instead of the global gpio_desc[] array. This makes it both simpler and more efficient, and is needed to remove the global descriptors array. Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> --- drivers/gpio/gpiolib.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-)