linux-kernel - [RFC] gpio: remove gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <1414825380-15322-1-git-send-email-acourbot@nvidia.com>
Date:	Sat,  1 Nov 2014 16:03:00 +0900
From:	Alexandre Courbot <acourbot@...dia.com>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
	Alexandre Courbot <acourbot@...dia.com>
Subject: [RFC] gpio: remove gpio_descs global array

Replace the ARCH_NR_GPIOS-sized static array of GPIO descriptors by
dynamically-allocated arrays for each GPIO chip.

This change makes gpio_to_desc() perform in O(n) (where n is the number
of GPIO chips registered) instead of O(1), however since n is rarely
bigger than 1 or 2 no noticeable performance issue should be expected.
Besides this provides more incentive for GPIO consumers to move to the
gpiod interface. One could use a O(log(n)) structure to link the GPIO
chips together, but considering the low limit of n the hypothetical
performance benefit is probably not worth the added complexity.

The issue with this patch is its use of kzalloc() in gpiochip_add(), a
function potentially called during early boot, before kzalloc() becomes
usable. Hence its [RFC] status until we can find a solution for this or
agree that this issue is actually never ran into (from which point can
one use kzalloc()?)

While writing this I noticed a couple of weird things done in
gpiochip_add()/gpiochip_remove() and I suspect other people will find
them as well, but for now I'd like to focus on the kzalloc() issue and
see if we can push this forward. :)

Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
---
 drivers/gpio/gpiolib.c | 48 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 50e18a4..013e01e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -47,8 +47,6 @@
  */
 DEFINE_SPINLOCK(gpio_lock);
 
-static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];
-
 #define GPIO_OFFSET_VALID(chip, offset) (offset >= 0 && offset < chip->ngpio)
 
 static DEFINE_MUTEX(gpio_lookup_lock);
@@ -65,10 +63,23 @@ static inline void desc_set_label(struct gpio_desc *d, const char *label)
  */
 struct gpio_desc *gpio_to_desc(unsigned gpio)
 {
-	if (WARN(!gpio_is_valid(gpio), "invalid GPIO %d\n", gpio))
-		return NULL;
-	else
-		return &gpio_desc[gpio];
+	struct gpio_chip *chip;
+	unsigned long flags;
+
+	spin_lock_irqsave(&gpio_lock, flags);
+
+	list_for_each_entry(chip, &gpio_chips, list) {
+		if (chip->base <= gpio && chip->base + chip->ngpio > gpio) {
+			spin_unlock_irqrestore(&gpio_lock, flags);
+			return &chip->desc[gpio - chip->base];
+		}
+	}
+
+	spin_unlock_irqrestore(&gpio_lock, flags);
+
+	pr_warn("invalid GPIO %d\n", gpio);
+	dump_stack();
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(gpio_to_desc);
 
@@ -91,7 +102,7 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip,
  */
 int desc_to_gpio(const struct gpio_desc *desc)
 {
-	return desc - &gpio_desc[0];
+	return desc->chip->base + (desc - &desc->chip->desc[0]);
 }
 EXPORT_SYMBOL_GPL(desc_to_gpio);
 
@@ -226,12 +237,13 @@ int gpiochip_add(struct gpio_chip *chip)
 	int		status = 0;
 	unsigned	id;
 	int		base = chip->base;
+	struct gpio_desc *descs;
 
-	if ((!gpio_is_valid(base) || !gpio_is_valid(base + chip->ngpio - 1))
-			&& base >= 0) {
-		status = -EINVAL;
-		goto fail;
-	}
+	/* TODO context is potentially before kmalloc will work - how can we */
+	/* workaround this? */
+	descs = kzalloc(sizeof(descs[0] * chip->ngpio, GFP_KERNEL);
+	if (!descs)
+		return -ENOMEM;
 
 	spin_lock_irqsave(&gpio_lock, flags);
 
@@ -247,10 +259,8 @@ int gpiochip_add(struct gpio_chip *chip)
 	status = gpiochip_add_to_list(chip);
 
 	if (status == 0) {
-		chip->desc = &gpio_desc[chip->base];
-
 		for (id = 0; id < chip->ngpio; id++) {
-			struct gpio_desc *desc = &chip->desc[id];
+			struct gpio_desc *desc = &descs[id];
 			desc->chip = chip;
 
 			/* REVISIT:  most hardware initializes GPIOs as
@@ -266,6 +276,8 @@ int gpiochip_add(struct gpio_chip *chip)
 		}
 	}
 
+	chip->desc = descs;
+
 	spin_unlock_irqrestore(&gpio_lock, flags);
 
 #ifdef CONFIG_PINCTRL
@@ -291,6 +303,9 @@ int gpiochip_add(struct gpio_chip *chip)
 unlock:
 	spin_unlock_irqrestore(&gpio_lock, flags);
 fail:
+	chip->desc = NULL;
+	kfree(descs);
+
 	/* failures here can mean systems won't boot... */
 	pr_err("%s: GPIOs %d..%d (%s) failed to register\n", __func__,
 		chip->base, chip->base + chip->ngpio - 1,
@@ -331,6 +346,9 @@ void gpiochip_remove(struct gpio_chip *chip)
 	list_del(&chip->list);
 	spin_unlock_irqrestore(&gpio_lock, flags);
 	gpiochip_unexport(chip);
+
+	kfree(chip->desc);
+	chip->descs = NULL;
 }
 EXPORT_SYMBOL_GPL(gpiochip_remove);
 
-- 
2.1.3

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