From e3a2e87893125bcd99bd7e1ddf9bfc421e492572 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Thu, 23 Oct 2014 17:27:07 +0900 Subject: gpio: rename gpio_lock_as_irq to gpiochip_lock_as_irq This function actually operates on a gpio_chip, so its prefix should reflect that fact for consistency with other functions defined in gpio/driver.h. Signed-off-by: Alexandre Courbot Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index e8e98ca25ec7..50e18a4b3a9f 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -495,7 +495,7 @@ static int gpiochip_irq_reqres(struct irq_data *d) { struct gpio_chip *chip = irq_data_get_irq_chip_data(d); - if (gpio_lock_as_irq(chip, d->hwirq)) { + if (gpiochip_lock_as_irq(chip, d->hwirq)) { chip_err(chip, "unable to lock HW IRQ %lu for IRQ\n", d->hwirq); @@ -508,7 +508,7 @@ static void gpiochip_irq_relres(struct irq_data *d) { struct gpio_chip *chip = irq_data_get_irq_chip_data(d); - gpio_unlock_as_irq(chip, d->hwirq); + gpiochip_unlock_as_irq(chip, d->hwirq); } static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset) @@ -1332,14 +1332,14 @@ int gpiod_to_irq(const struct gpio_desc *desc) EXPORT_SYMBOL_GPL(gpiod_to_irq); /** - * gpio_lock_as_irq() - lock a GPIO to be used as IRQ + * gpiochip_lock_as_irq() - lock a GPIO to be used as IRQ * @chip: the chip the GPIO to lock belongs to * @offset: the offset of the GPIO to lock as IRQ * * This is used directly by GPIO drivers that want to lock down * a certain GPIO line to be used for IRQs. */ -int gpio_lock_as_irq(struct gpio_chip *chip, unsigned int offset) +int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset) { if (offset >= chip->ngpio) return -EINVAL; @@ -1354,24 +1354,24 @@ int gpio_lock_as_irq(struct gpio_chip *chip, unsigned int offset) set_bit(FLAG_USED_AS_IRQ, &chip->desc[offset].flags); return 0; } -EXPORT_SYMBOL_GPL(gpio_lock_as_irq); +EXPORT_SYMBOL_GPL(gpiochip_lock_as_irq); /** - * gpio_unlock_as_irq() - unlock a GPIO used as IRQ + * gpiochip_unlock_as_irq() - unlock a GPIO used as IRQ * @chip: the chip the GPIO to lock belongs to * @offset: the offset of the GPIO to lock as IRQ * * This is used directly by GPIO drivers that want to indicate * that a certain GPIO is no longer used exclusively for IRQ. */ -void gpio_unlock_as_irq(struct gpio_chip *chip, unsigned int offset) +void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset) { if (offset >= chip->ngpio) return; clear_bit(FLAG_USED_AS_IRQ, &chip->desc[offset].flags); } -EXPORT_SYMBOL_GPL(gpio_unlock_as_irq); +EXPORT_SYMBOL_GPL(gpiochip_unlock_as_irq); /** * gpiod_get_raw_value_cansleep() - return a gpio's raw value -- cgit v1.2.3 From 5f42424354f5b0ca5413b4fb8528d150692c85b7 Mon Sep 17 00:00:00 2001 From: Rojhalat Ibrahim Date: Tue, 4 Nov 2014 17:12:06 +0100 Subject: gpiolib: allow simultaneous setting of multiple GPIO outputs Introduce new functions gpiod_set_array & gpiod_set_raw_array to the consumer interface which allow setting multiple outputs with just one function call. Also add an optional set_multiple function to the driver interface. Without an implementation of that function in the chip driver outputs are set sequentially. Implementing the set_multiple function in a chip driver allows for: - Improved performance for certain use cases. The original motivation for this was the task of configuring an FPGA. In that specific case, where 9 GPIO lines have to be set many times, configuration time goes down from 48 s to 20 s when using the new function. - Simultaneous glitch-free setting of multiple pins on any kind of parallel bus attached to GPIOs provided they all reside on the same chip and bank. Limitations: Performance is only improved for normal high-low outputs. Open drain and open source outputs are always set separately from each other. Those kinds of outputs could probably be accelerated in a similar way if we could forgo the error checking when setting GPIO directions. Change log: v6: - rebase on current linux-gpio devel branch v5: - check can_sleep property per chip - remove superfluous checks - supplement documentation v4: - add gpiod_set_array function for setting logical values - change interface of the set_multiple driver function to use unsigned long as type for the bit fields - use generic bitops (which also use unsigned long for bit fields) - do not use ARCH_NR_GPIOS any more v3: - add documentation - change commit message v2: - use descriptor interface - allow arbitrary groups of GPIOs spanning multiple chips Signed-off-by: Rojhalat Ibrahim Reviewed-by: Alexandre Courbot Reviewed-by: Mark Brown Signed-off-by: Linus Walleij --- Documentation/gpio/consumer.txt | 27 +++++++ drivers/gpio/gpiolib.c | 168 ++++++++++++++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 38 +++++++++ include/linux/gpio/driver.h | 4 + 4 files changed, 237 insertions(+) (limited to 'drivers/gpio/gpiolib.c') diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt index 6ce544191ca6..c67f806401a5 100644 --- a/Documentation/gpio/consumer.txt +++ b/Documentation/gpio/consumer.txt @@ -199,6 +199,33 @@ The active-low state of a GPIO can also be queried using the following call: Note that these functions should only be used with great moderation ; a driver should not have to care about the physical line level. + +Set multiple GPIO outputs with a single function call +----------------------------------------------------- +The following functions set the output values of an array of GPIOs: + + void gpiod_set_array(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) + void gpiod_set_raw_array(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) + void gpiod_set_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) + void gpiod_set_raw_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) + +The array can be an arbitrary set of GPIOs. The functions will try to set +GPIOs belonging to the same bank or chip simultaneously if supported by the +corresponding chip driver. In that case a significantly improved performance +can be expected. If simultaneous setting is not possible the GPIOs will be set +sequentially. +Note that for optimal performance GPIOs belonging to the same chip should be +contiguous within the array of descriptors. + + GPIOs mapped to IRQs -------------------- GPIO lines can quite often be used as IRQs. You can get the IRQ number diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 50e18a4b3a9f..eb739a51e774 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1254,6 +1254,88 @@ static void _gpiod_set_raw_value(struct gpio_desc *desc, bool value) chip->set(chip, gpio_chip_hwgpio(desc), value); } +/* + * set multiple outputs on the same chip; + * use the chip's set_multiple function if available; + * otherwise set the outputs sequentially; + * @mask: bit mask array; one bit per output; BITS_PER_LONG bits per word + * defines which outputs are to be changed + * @bits: bit value array; one bit per output; BITS_PER_LONG bits per word + * defines the values the outputs specified by mask are to be set to + */ +static void gpio_chip_set_multiple(struct gpio_chip *chip, + unsigned long *mask, unsigned long *bits) +{ + if (chip->set_multiple) { + chip->set_multiple(chip, mask, bits); + } else { + int i; + for (i = 0; i < chip->ngpio; i++) { + if (mask[BIT_WORD(i)] == 0) { + /* no more set bits in this mask word; + * skip ahead to the next word */ + i = (BIT_WORD(i) + 1) * BITS_PER_LONG - 1; + continue; + } + /* set outputs if the corresponding mask bit is set */ + if (__test_and_clear_bit(i, mask)) { + chip->set(chip, i, test_bit(i, bits)); + } + } + } +} + +static void gpiod_set_array_priv(bool raw, bool can_sleep, + unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) +{ + int i = 0; + + while (i < array_size) { + struct gpio_chip *chip = desc_array[i]->chip; + unsigned long mask[BITS_TO_LONGS(chip->ngpio)]; + unsigned long bits[BITS_TO_LONGS(chip->ngpio)]; + int count = 0; + + if (!can_sleep) { + WARN_ON(chip->can_sleep); + } + memset(mask, 0, sizeof(mask)); + do { + struct gpio_desc *desc = desc_array[i]; + int hwgpio = gpio_chip_hwgpio(desc); + int value = value_array[i]; + + if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags)) + value = !value; + trace_gpio_value(desc_to_gpio(desc), 0, value); + /* + * collect all normal outputs belonging to the same chip + * open drain and open source outputs are set individually + */ + if (test_bit(FLAG_OPEN_DRAIN, &desc->flags)) { + _gpio_set_open_drain_value(desc,value); + } else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) { + _gpio_set_open_source_value(desc, value); + } else { + __set_bit(hwgpio, mask); + if (value) { + __set_bit(hwgpio, bits); + } else { + __clear_bit(hwgpio, bits); + } + count++; + } + i++; + } while ((i < array_size) && (desc_array[i]->chip == chip)); + /* push collected bits to outputs */ + if (count != 0) { + gpio_chip_set_multiple(chip, mask, bits); + } + } +} + /** * gpiod_set_raw_value() - assign a gpio's raw value * @desc: gpio whose value will be assigned @@ -1298,6 +1380,48 @@ void gpiod_set_value(struct gpio_desc *desc, int value) } EXPORT_SYMBOL_GPL(gpiod_set_value); +/** + * gpiod_set_raw_array() - assign values to an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be assigned + * @value_array: array of values to assign + * + * Set the raw values of the GPIOs, i.e. the values of the physical lines + * without regard for their ACTIVE_LOW status. + * + * This function should be called from contexts where we cannot sleep, and will + * complain if the GPIO chip functions potentially sleep. + */ +void gpiod_set_raw_array(unsigned int array_size, + struct gpio_desc **desc_array, int *value_array) +{ + if (!desc_array) + return; + gpiod_set_array_priv(true, false, array_size, desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_set_raw_array); + +/** + * gpiod_set_array() - assign values to an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be assigned + * @value_array: array of values to assign + * + * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status + * into account. + * + * This function should be called from contexts where we cannot sleep, and will + * complain if the GPIO chip functions potentially sleep. + */ +void gpiod_set_array(unsigned int array_size, + struct gpio_desc **desc_array, int *value_array) +{ + if (!desc_array) + return; + gpiod_set_array_priv(false, false, array_size, desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_set_array); + /** * gpiod_cansleep() - report whether gpio value access may sleep * @desc: gpio to check @@ -1457,6 +1581,50 @@ void gpiod_set_value_cansleep(struct gpio_desc *desc, int value) } EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep); +/** + * gpiod_set_raw_array_cansleep() - assign values to an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be assigned + * @value_array: array of values to assign + * + * Set the raw values of the GPIOs, i.e. the values of the physical lines + * without regard for their ACTIVE_LOW status. + * + * This function is to be called from contexts that can sleep. + */ +void gpiod_set_raw_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) +{ + might_sleep_if(extra_checks); + if (!desc_array) + return; + gpiod_set_array_priv(true, true, array_size, desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_set_raw_array_cansleep); + +/** + * gpiod_set_array_cansleep() - assign values to an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be assigned + * @value_array: array of values to assign + * + * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status + * into account. + * + * This function is to be called from contexts that can sleep. + */ +void gpiod_set_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) +{ + might_sleep_if(extra_checks); + if (!desc_array) + return; + gpiod_set_array_priv(false, true, array_size, desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_set_array_cansleep); + /** * gpiod_add_lookup_table() - register GPIO device consumers * @table: table of consumers to register diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index 12f146fa6604..83c0a61c605d 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -74,14 +74,24 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value); /* Value get/set from non-sleeping context */ int gpiod_get_value(const struct gpio_desc *desc); void gpiod_set_value(struct gpio_desc *desc, int value); +void gpiod_set_array(unsigned int array_size, + struct gpio_desc **desc_array, int *value_array); int gpiod_get_raw_value(const struct gpio_desc *desc); void gpiod_set_raw_value(struct gpio_desc *desc, int value); +void gpiod_set_raw_array(unsigned int array_size, + struct gpio_desc **desc_array, int *value_array); /* Value get/set from sleeping context */ int gpiod_get_value_cansleep(const struct gpio_desc *desc); void gpiod_set_value_cansleep(struct gpio_desc *desc, int value); +void gpiod_set_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array); int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc); void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value); +void gpiod_set_raw_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array); int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce); @@ -210,6 +220,13 @@ static inline void gpiod_set_value(struct gpio_desc *desc, int value) /* GPIO can never have been requested */ WARN_ON(1); } +static inline void gpiod_set_array(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); +} static inline int gpiod_get_raw_value(const struct gpio_desc *desc) { /* GPIO can never have been requested */ @@ -221,6 +238,13 @@ static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value) /* GPIO can never have been requested */ WARN_ON(1); } +static inline void gpiod_set_raw_array(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); +} static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc) { @@ -233,6 +257,13 @@ static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value) /* GPIO can never have been requested */ WARN_ON(1); } +static inline void gpiod_set_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); +} static inline int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc) { /* GPIO can never have been requested */ @@ -245,6 +276,13 @@ static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, /* GPIO can never have been requested */ WARN_ON(1); } +static inline void gpiod_set_raw_array_cansleep(unsigned int array_size, + struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); +} static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) { diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index ff200a75501e..c497c62889d1 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -32,6 +32,7 @@ struct seq_file; * @get: returns value for signal "offset"; for output signals this * returns either the value actually sensed, or zero * @set: assigns output value for signal "offset" + * @set_multiple: assigns output values for multiple signals defined by "mask" * @set_debounce: optional hook for setting debounce time for specified gpio in * interrupt triggered gpio chips * @to_irq: optional hook supporting non-static gpio_to_irq() mappings; @@ -89,6 +90,9 @@ struct gpio_chip { unsigned offset); void (*set)(struct gpio_chip *chip, unsigned offset, int value); + void (*set_multiple)(struct gpio_chip *chip, + unsigned long *mask, + unsigned long *bits); int (*set_debounce)(struct gpio_chip *chip, unsigned offset, unsigned debounce); -- cgit v1.2.3 From 86256d1fceff058d5afea6dfcc8a2eac18a71e95 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 17 Nov 2014 15:31:59 +0100 Subject: gpio: Check if base is positive before calling gpio_is_valid() It doesn't make much sense to make some (possible expensive) calls to gpio_is_valid() first, and to ignore the result if the base number is negative. Check for a positive base number first. Signed-off-by: Geert Uytterhoeven Reviewed-by: Alexandre Courbot Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index eb739a51e774..12d981a5be66 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -227,8 +227,8 @@ int gpiochip_add(struct gpio_chip *chip) unsigned id; int base = chip->base; - if ((!gpio_is_valid(base) || !gpio_is_valid(base + chip->ngpio - 1)) - && base >= 0) { + if (base >= 0 && + (!gpio_is_valid(base) || !gpio_is_valid(base + chip->ngpio - 1))) { status = -EINVAL; goto fail; } -- cgit v1.2.3 From 14e85c0e69d5c7fdbd963edbbec1dc5cdd385200 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Wed, 19 Nov 2014 16:51:27 +0900 Subject: 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 is 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. This patch uses kcalloc() in gpiochip_add(), which removes the ability to add a chip before kcalloc() can operate. I am not aware of such cases, but if someone bisects up to this patch then I will be proven wrong... Signed-off-by: Alexandre Courbot Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 12d981a5be66..5619922ebf44 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,22 @@ 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); + + WARN(1, "invalid GPIO %d\n", gpio); + return NULL; } EXPORT_SYMBOL_GPL(gpio_to_desc); @@ -91,7 +101,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); @@ -206,7 +216,7 @@ static int gpiochip_add_to_list(struct gpio_chip *chip) /** * gpiochip_add() - register a gpio_chip * @chip: the chip to register, with chip->base initialized - * Context: potentially before irqs or kmalloc will work + * Context: potentially before irqs will work * * Returns a negative errno if the chip can't be registered, such as * because the chip->base is invalid or already associated with a @@ -226,12 +236,11 @@ int gpiochip_add(struct gpio_chip *chip) int status = 0; unsigned id; int base = chip->base; + struct gpio_desc *descs; - if (base >= 0 && - (!gpio_is_valid(base) || !gpio_is_valid(base + chip->ngpio - 1))) { - status = -EINVAL; - goto fail; - } + descs = kcalloc(chip->ngpio, sizeof(descs[0]), GFP_KERNEL); + if (!descs) + return -ENOMEM; spin_lock_irqsave(&gpio_lock, flags); @@ -247,10 +256,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 +273,8 @@ int gpiochip_add(struct gpio_chip *chip) } } + chip->desc = descs; + spin_unlock_irqrestore(&gpio_lock, flags); #ifdef CONFIG_PINCTRL @@ -291,6 +300,9 @@ int gpiochip_add(struct gpio_chip *chip) unlock: spin_unlock_irqrestore(&gpio_lock, flags); fail: + kfree(descs); + chip->desc = NULL; + /* 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 +343,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->desc = NULL; } EXPORT_SYMBOL_GPL(gpiochip_remove); -- cgit v1.2.3 From 8e53b0f190af2954309bbad76a78177ead15d824 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Tue, 25 Nov 2014 17:16:31 +0900 Subject: gpio: remove const modifier from gpiod_get_direction() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although gpiod_get_direction() can be considered side-effect free for consumers, its internals involve setting or clearing bits in the affected GPIO descriptor, for which we need to force-cast the const descriptor variable to non-const. This could lead to incorrect behavior if the compiler decides to optimize here, so remove this const attribute. The intent is to make gpiod_get_direction() private anyway, so it does not really matter. Reported-by: Uwe Kleine-König Signed-off-by: Alexandre Courbot Acked-by: Uwe Kleine-König Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib-sysfs.c | 2 +- drivers/gpio/gpiolib.c | 8 +++----- include/linux/gpio/consumer.h | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c index 781fbed00fc3..2ac1800b58bb 100644 --- a/drivers/gpio/gpiolib-sysfs.c +++ b/drivers/gpio/gpiolib-sysfs.c @@ -41,7 +41,7 @@ static DEFINE_MUTEX(sysfs_lock); static ssize_t gpio_direction_show(struct device *dev, struct device_attribute *attr, char *buf) { - const struct gpio_desc *desc = dev_get_drvdata(dev); + struct gpio_desc *desc = dev_get_drvdata(dev); ssize_t status; mutex_lock(&sysfs_lock); diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 5619922ebf44..0b271ef87c09 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -148,7 +148,7 @@ static int gpiochip_find_base(int ngpio) * * This function may sleep if gpiod_cansleep() is true. */ -int gpiod_get_direction(const struct gpio_desc *desc) +int gpiod_get_direction(struct gpio_desc *desc) { struct gpio_chip *chip; unsigned offset; @@ -164,13 +164,11 @@ int gpiod_get_direction(const struct gpio_desc *desc) if (status > 0) { /* GPIOF_DIR_IN, or other positive */ status = 1; - /* FLAG_IS_OUT is just a cache of the result of get_direction(), - * so it does not affect constness per se */ - clear_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags); + clear_bit(FLAG_IS_OUT, &desc->flags); } if (status == 0) { /* GPIOF_DIR_OUT */ - set_bit(FLAG_IS_OUT, &((struct gpio_desc *)desc)->flags); + set_bit(FLAG_IS_OUT, &desc->flags); } return status; } diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index 83c0a61c605d..d54d158ca327 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -66,7 +66,7 @@ __devm_gpiod_get_index_optional(struct device *dev, const char *con_id, unsigned int index, enum gpiod_flags flags); void devm_gpiod_put(struct device *dev, struct gpio_desc *desc); -int gpiod_get_direction(const struct gpio_desc *desc); +int gpiod_get_direction(struct gpio_desc *desc); int gpiod_direction_input(struct gpio_desc *desc); int gpiod_direction_output(struct gpio_desc *desc, int value); int gpiod_direction_output_raw(struct gpio_desc *desc, int value); -- cgit v1.2.3 From 0e9a5edf5d01356fa4a561772a25d97b3fd13de6 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Tue, 2 Dec 2014 23:15:05 +0900 Subject: gpio: fix deferred probe detection for legacy API Commit 14e85c0e69d5 ("gpio: remove gpio_descs global array") changed gpio_to_desc()'s behavior to return NULL not only for GPIOs numbers not in the valid range, but also for all GPIOs whose controller has not been probed yet. Although this behavior is more correct (nothing hints that these GPIO numbers will be populated later), this affects gpio_request() and gpio_request_one() which call gpiod_request() with a NULL descriptor, causing it to return -EINVAL instead of the expected -EPROBE_DEFER for a non-probed GPIO. gpiod_request() is only called with a descriptor obtained from gpio_to_desc() from these two functions, so address the issue there. Other ways to obtain GPIOs rely on well-defined mappings and can thus return -EPROBE_DEFER only for relevant GPIOs, and are thus not affected by this issue. Reported-by: Geert Uytterhoeven Signed-off-by: Alexandre Courbot Tested-by: Geert Uytterhoeven Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib-legacy.c | 12 +++++++++++- drivers/gpio/gpiolib.c | 4 +++- 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib-legacy.c b/drivers/gpio/gpiolib-legacy.c index 078ae6c2df79..8b830996fe02 100644 --- a/drivers/gpio/gpiolib-legacy.c +++ b/drivers/gpio/gpiolib-legacy.c @@ -24,6 +24,10 @@ int gpio_request_one(unsigned gpio, unsigned long flags, const char *label) desc = gpio_to_desc(gpio); + /* Compatibility: assume unavailable "valid" GPIOs will appear later */ + if (!desc && gpio_is_valid(gpio)) + return -EPROBE_DEFER; + err = gpiod_request(desc, label); if (err) return err; @@ -62,7 +66,13 @@ EXPORT_SYMBOL_GPL(gpio_request_one); int gpio_request(unsigned gpio, const char *label) { - return gpiod_request(gpio_to_desc(gpio), label); + struct gpio_desc *desc = gpio_to_desc(gpio); + + /* Compatibility: assume unavailable "valid" GPIOs will appear later */ + if (!desc && gpio_is_valid(gpio)) + return -EPROBE_DEFER; + + return gpiod_request(desc, label); } EXPORT_SYMBOL_GPL(gpio_request); diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 0b271ef87c09..56b7c5de95a0 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -77,7 +77,9 @@ struct gpio_desc *gpio_to_desc(unsigned gpio) spin_unlock_irqrestore(&gpio_lock, flags); - WARN(1, "invalid GPIO %d\n", gpio); + if (!gpio_is_valid(gpio)) + WARN(1, "invalid GPIO %d\n", gpio); + return NULL; } EXPORT_SYMBOL_GPL(gpio_to_desc); -- cgit v1.2.3