| Message ID | 46dea76d0adfe25f30b564d9dc5b3f2b4de099c9.1503319573.git.lukas@wunner.de |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 21, 2017 at 3:12 PM, Lukas Wunner <lukas@wunner.de> wrote: > SPI-attached GPIO controllers typically read out all inputs in one go. > If callers desire the values of multipe inputs, ideally a single readout > should take place to return the desired values. However the current > driver API only offers a ->get callback but no ->get_multiple (unlike > ->set_multiple, which is present). Thus, to read multiple inputs, a > full readout needs to be performed for every single value (barring > driver-internal caching), which is inefficient. > > In fact, the lack of a ->get_multiple callback has been bemoaned > repeatedly by the gpio subsystem maintainer: > http://www.spinics.net/lists/linux-gpio/msg10571.html > http://www.spinics.net/lists/devicetree/msg121734.html > > Introduce the missing callback. Add corresponding consumer functions > such as gpiod_get_array_value(). Amend linehandle_ioctl() to take > advantage of the newly added infrastructure. > > Signed-off-by: Lukas Wunner <lukas@wunner.de> Lean, mean, and clean and introducing the interface that I've been missing for some time. Good work! Let's hope we can merge this early for v4.15 once v4.14-rc1 is out and pile some drivers supporting it on top of it too. I am only worried on how to get __assign_bit() into place in time, or whether I should carry that patch in the GPIO tree simply? We *could* add a local version of it and then switch it over with a follow-up patch... Could you please patch Documentation/gpio/consumer.txt to mention the new functions as well? Maybe we should have a section in Documentation/gpio/driver.txt, that doc is a bit incomplete right now. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 23, 2017 at 09:38:21AM +0200, Linus Walleij wrote: > On Mon, Aug 21, 2017 at 3:12 PM, Lukas Wunner <lukas@wunner.de> wrote: > > SPI-attached GPIO controllers typically read out all inputs in one go. > > If callers desire the values of multipe inputs, ideally a single readout > > should take place to return the desired values. However the current > > driver API only offers a ->get callback but no ->get_multiple (unlike > > ->set_multiple, which is present). Thus, to read multiple inputs, a > > full readout needs to be performed for every single value (barring > > driver-internal caching), which is inefficient. > > > > In fact, the lack of a ->get_multiple callback has been bemoaned > > repeatedly by the gpio subsystem maintainer: > > http://www.spinics.net/lists/linux-gpio/msg10571.html > > http://www.spinics.net/lists/devicetree/msg121734.html > > > > Introduce the missing callback. Add corresponding consumer functions > > such as gpiod_get_array_value(). Amend linehandle_ioctl() to take > > advantage of the newly added infrastructure. > > Could you please patch > Documentation/gpio/consumer.txt > to mention the new functions as well? Missed that, will fix in v2. There is an oddity in that document in that it claims gpiod_get_value() / gpiod_set_value() do not return errors. The kerneldoc in gpiolib.c and driver.h says otherwise, it allows the get functions to return a negative errno. Now what? Here are the two occurrences of the false claim in consumer.txt: It should be checked, since the get/set calls don't return errors and since misconfiguration is possible. The get/set calls do not return errors because "invalid GPIO" should have been reported earlier from gpiod_direction_*(). At least with SPI-attached GPIO controllers, the transmission is never guaranteed to succeed, so errors can occur both for input and output GPIOs. The MAX3191x is input-only and does pass SPI errors back to the caller. Output drivers such as gpio-74x164.c silently ignore SPI errors, which is arguably a problem. > Maybe we should have a section in Documentation/gpio/driver.txt, > that doc is a bit incomplete right now. First time I've seen that file. :-) I used the kerneldoc as guidance when writing the gpio-max3191x.c driver and found it sufficient, so I'm not sure how driver.txt could be expanded? Thanks! Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Aug 27, 2017 at 7:34 PM, Lukas Wunner <lukas@wunner.de> wrote: > On Wed, Aug 23, 2017 at 09:38:21AM +0200, Linus Walleij wrote: >> On Mon, Aug 21, 2017 at 3:12 PM, Lukas Wunner <lukas@wunner.de> wrote: >> > SPI-attached GPIO controllers typically read out all inputs in one go. >> > If callers desire the values of multipe inputs, ideally a single readout >> > should take place to return the desired values. However the current >> > driver API only offers a ->get callback but no ->get_multiple (unlike >> > ->set_multiple, which is present). Thus, to read multiple inputs, a >> > full readout needs to be performed for every single value (barring >> > driver-internal caching), which is inefficient. >> > >> > In fact, the lack of a ->get_multiple callback has been bemoaned >> > repeatedly by the gpio subsystem maintainer: >> > http://www.spinics.net/lists/linux-gpio/msg10571.html >> > http://www.spinics.net/lists/devicetree/msg121734.html >> > >> > Introduce the missing callback. Add corresponding consumer functions >> > such as gpiod_get_array_value(). Amend linehandle_ioctl() to take >> > advantage of the newly added infrastructure. >> >> Could you please patch >> Documentation/gpio/consumer.txt >> to mention the new functions as well? > > Missed that, will fix in v2. There is an oddity in that document > in that it claims gpiod_get_value() / gpiod_set_value() do not return > errors. The kerneldoc in gpiolib.c and driver.h says otherwise, > it allows the get functions to return a negative errno. Now what? Probably errors in the documentation. Feel free to correct it. > At least with SPI-attached GPIO controllers, the transmission is never > guaranteed to succeed, so errors can occur both for input and output > GPIOs. The MAX3191x is input-only and does pass SPI errors back to > the caller. Output drivers such as gpio-74x164.c silently ignore > SPI errors, which is arguably a problem. This has historical reasons. >> Maybe we should have a section in Documentation/gpio/driver.txt, >> that doc is a bit incomplete right now. > > First time I've seen that file. :-) I used the kerneldoc as guidance > when writing the gpio-max3191x.c driver and found it sufficient, so I'm > not sure how driver.txt could be expanded? Don't worry about it if you feel it's superfluous. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 31, 2017 at 03:48:15PM +0200, Linus Walleij wrote: > On Sun, Aug 27, 2017 at 7:34 PM, Lukas Wunner <lukas@wunner.de> wrote: > > At least with SPI-attached GPIO controllers, the transmission is never > > guaranteed to succeed, so errors can occur both for input and output > > GPIOs. The MAX3191x is input-only and does pass SPI errors back to > > the caller. Output drivers such as gpio-74x164.c silently ignore > > SPI errors, which is arguably a problem. > > This has historical reasons. Would you generally be open for allowing errors to be returned from the set functions as well? The Revolution Pi uses SPI-attached digital outputs and knowing if access to them fails would be useful. For consumers, switching the return value from void to int shouldn't cause any fallout. They just don't check the return value initially. However the ->set and ->set_multiple callbacks in all the GPIO drivers would need to be changed, that's where the real work is. Thanks, Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 31, 2017 at 5:46 PM, Lukas Wunner <lukas@wunner.de> wrote: > On Thu, Aug 31, 2017 at 03:48:15PM +0200, Linus Walleij wrote: >> On Sun, Aug 27, 2017 at 7:34 PM, Lukas Wunner <lukas@wunner.de> wrote: >> > At least with SPI-attached GPIO controllers, the transmission is never >> > guaranteed to succeed, so errors can occur both for input and output >> > GPIOs. The MAX3191x is input-only and does pass SPI errors back to >> > the caller. Output drivers such as gpio-74x164.c silently ignore >> > SPI errors, which is arguably a problem. >> >> This has historical reasons. > > Would you generally be open for allowing errors to be returned from > the set functions as well? Yes if we can make a series fixing the fallout. > The Revolution Pi uses SPI-attached > digital outputs and knowing if access to them fails would be useful. True. > For consumers, switching the return value from void to int shouldn't > cause any fallout. They just don't check the return value initially. Indeed. > However the ->set and ->set_multiple callbacks in all the GPIO drivers > would need to be changed, that's where the real work is. Yeah :/ But there are not so many of them, luckily. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, one question popped up regarding this patch that I'd like to clarify before reposting a revised series: On Mon, Aug 21, 2017 at 03:12:00PM +0200, Lukas Wunner wrote: > SPI-attached GPIO controllers typically read out all inputs in one go. > If callers desire the values of multipe inputs, ideally a single readout > should take place to return the desired values. However the current > driver API only offers a ->get callback but no ->get_multiple (unlike > ->set_multiple, which is present). Thus, to read multiple inputs, a > full readout needs to be performed for every single value (barring > driver-internal caching), which is inefficient. [...] > Introduce the missing callback. Add corresponding consumer functions > such as gpiod_get_array_value(). Amend linehandle_ioctl() to take > advantage of the newly added infrastructure. [...] > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -365,28 +365,28 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd, > struct linehandle_state *lh = filep->private_data; > void __user *ip = (void __user *)arg; > struct gpiohandle_data ghd; > + int vals[GPIOHANDLES_MAX]; > int i; > > if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) { > - int val; > + /* TODO: check if descriptors are really input */ > + int ret = gpiod_get_array_value_complex(false, > + true, > + lh->numdescs, > + (const struct gpio_desc **)lh->descs, > + vals); I've designed the function signature of gpiod_get_array_value_complex() gpiod_get_raw_array_value() gpiod_get_array_value() gpiod_get_raw_array_value_cansleep() gpiod_get_array_value_cansleep() such that a "const struct gpio_desc **" argument is passed in (the point being the const). This was done to enforce const-correctness and for consistency with the family of functions to read a single GPIO, such as gpiod_get_value() which take a "const struct gpio_desc *". However after actually using the newly introduced functions in a driver, I've discovered that the const keyword is mostly just an annoyance in this case: When acquiring GPIOs with gpiod_get_array(), the resulting struct gpio_desc ** is not const. Thus, a cast is necessary whenever feeding the result of gpiod_get_array() to gpiod_get_array_value(), which may well be the most common use case. (It's probably less common that folks construct the array by hand.) A cast is also already necessary for the invocation of gpiod_get_array_value_complex() quoted above. So I'm wondering if the const keyword does more harm than good in this case. Let me know what you think. Thanks, Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 4, 2017 at 10:32 PM, Lukas Wunner <lukas@wunner.de> wrote: > So I'm wondering if the const keyword does more harm than good in this case. > Let me know what you think. I've had similar experiences. I'm not very sensitive about it, do whatever makes most sense to you. What is important is that we get get_mutiple() in place. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Oct 07, 2017 at 01:23:17PM +0200, Linus Walleij wrote: > On Wed, Oct 4, 2017 at 10:32 PM, Lukas Wunner <lukas@wunner.de> wrote: > > So I'm wondering if the const keyword does more harm than good in this case. > > Let me know what you think. > > I've had similar experiences. > > I'm not very sensitive about it, do whatever makes most sense to you. > > What is important is that we get get_mutiple() in place. Yes. My apologies for the delay. I've dropped the const qualifier, the rationale for the compiler error it triggers is given in: http://c-faq.com/ansi/constmismatch.html Briefly, it is legal to pass a pointer to non-const if the function signature specifies const, but not for a pointer-to-pointer, i.e. a second level of indirection. The error is supposed to "help" the developer keep the const promises but I don't find it helpful at all. It necessitates a cast unless a const struct gpio_desc ** is passed in. When would that be the case in reality? Only if the array is statically declared, but usually the gpio_desc structs are obtained at runtime, so the the cast would likely *always* be needed. Thanks, Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 18bba1748a35..ea02916df0ad 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -365,28 +365,28 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd, struct linehandle_state *lh = filep->private_data; void __user *ip = (void __user *)arg; struct gpiohandle_data ghd; + int vals[GPIOHANDLES_MAX]; int i; if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) { - int val; + /* TODO: check if descriptors are really input */ + int ret = gpiod_get_array_value_complex(false, + true, + lh->numdescs, + (const struct gpio_desc **)lh->descs, + vals); + if (ret) + return ret; memset(&ghd, 0, sizeof(ghd)); - - /* TODO: check if descriptors are really input */ - for (i = 0; i < lh->numdescs; i++) { - val = gpiod_get_value_cansleep(lh->descs[i]); - if (val < 0) - return val; - ghd.values[i] = val; - } + for (i = 0; i < lh->numdescs; i++) + ghd.values[i] = vals[i]; if (copy_to_user(ip, &ghd, sizeof(ghd))) return -EFAULT; return 0; } else if (cmd == GPIOHANDLE_SET_LINE_VALUES_IOCTL) { - int vals[GPIOHANDLES_MAX]; - /* TODO: check if descriptors are really output */ if (copy_from_user(&ghd, ip, sizeof(ghd))) return -EFAULT; @@ -2473,6 +2473,71 @@ static int _gpiod_get_raw_value(const struct gpio_desc *desc) return value; } +static int gpio_chip_get_multiple(struct gpio_chip *chip, + unsigned long *mask, unsigned long *bits) +{ + if (chip->get_multiple) { + return chip->get_multiple(chip, mask, bits); + } else if (chip->get) { + int i, value; + + for_each_set_bit(i, mask, chip->ngpio) { + value = chip->get(chip, i); + if (value < 0) + return value; + __assign_bit(value, i, bits); + } + return 0; + } + return -EIO; +} + +int gpiod_get_array_value_complex(bool raw, bool can_sleep, + unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + int i = 0; + + while (i < array_size) { + struct gpio_chip *chip = desc_array[i]->gdev->chip; + unsigned long mask[BITS_TO_LONGS(chip->ngpio)]; + unsigned long bits[BITS_TO_LONGS(chip->ngpio)]; + int first, j, ret; + + if (!can_sleep) + WARN_ON(chip->can_sleep); + + /* collect all inputs belonging to the same chip */ + first = i; + memset(mask, 0, sizeof(mask)); + do { + const struct gpio_desc *desc = desc_array[i]; + int hwgpio = gpio_chip_hwgpio(desc); + + __set_bit(hwgpio, mask); + i++; + } while ((i < array_size) && + (desc_array[i]->gdev->chip == chip)); + + ret = gpio_chip_get_multiple(chip, mask, bits); + if (ret) + return ret; + + for (j = first; j < i; j++) { + const struct gpio_desc *desc = desc_array[j]; + int hwgpio = gpio_chip_hwgpio(desc); + int value = test_bit(hwgpio, bits); + + if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags)) + value = !value; + value_array[j] = value; + trace_gpio_value(desc_to_gpio(desc), 1, value); + } + } + return 0; +} + /** * gpiod_get_raw_value() - return a gpio's raw value * @desc: gpio whose value will be returned @@ -2521,6 +2586,53 @@ int gpiod_get_value(const struct gpio_desc *desc) } EXPORT_SYMBOL_GPL(gpiod_get_value); +/** + * gpiod_get_raw_array_value() - read raw values from an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be read + * @value_array: array to store the read values + * + * Read the raw values of the GPIOs, i.e. the values of the physical lines + * without regard for their ACTIVE_LOW status. Return 0 in case of success, + * else an error code. + * + * This function should be called from contexts where we cannot sleep, + * and it will complain if the GPIO chip functions potentially sleep. + */ +int gpiod_get_raw_array_value(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + if (!desc_array) + return -EINVAL; + return gpiod_get_array_value_complex(true, false, array_size, + desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value); + +/** + * gpiod_get_array_value() - read values from an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be read + * @value_array: array to store the read values + * + * Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status + * into account. Return 0 in case of success, else an error code. + * + * This function should be called from contexts where we cannot sleep, + * and it will complain if the GPIO chip functions potentially sleep. + */ +int gpiod_get_array_value(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + if (!desc_array) + return -EINVAL; + return gpiod_get_array_value_complex(false, false, array_size, + desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_get_array_value); + /* * _gpio_set_open_drain_value() - Set the open drain gpio's value. * @desc: gpio descriptor whose state need to be set. @@ -2950,6 +3062,53 @@ int gpiod_get_value_cansleep(const struct gpio_desc *desc) EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep); /** + * gpiod_get_raw_array_value_cansleep() - read raw values from an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be read + * @value_array: array to store the read values + * + * Read the raw values of the GPIOs, i.e. the values of the physical lines + * without regard for their ACTIVE_LOW status. Return 0 in case of success, + * else an error code. + * + * This function is to be called from contexts that can sleep. + */ +int gpiod_get_raw_array_value_cansleep(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + might_sleep_if(extra_checks); + if (!desc_array) + return -EINVAL; + return gpiod_get_array_value_complex(true, true, array_size, + desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep); + +/** + * gpiod_get_array_value_cansleep() - read values from an array of GPIOs + * @array_size: number of elements in the descriptor / value arrays + * @desc_array: array of GPIO descriptors whose values will be read + * @value_array: array to store the read values + * + * Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status + * into account. Return 0 in case of success, else an error code. + * + * This function is to be called from contexts that can sleep. + */ +int gpiod_get_array_value_cansleep(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + might_sleep_if(extra_checks); + if (!desc_array) + return -EINVAL; + return gpiod_get_array_value_complex(false, true, array_size, + desc_array, value_array); +} +EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep); + +/** * gpiod_set_raw_value_cansleep() - assign a gpio's raw value * @desc: gpio whose value will be assigned * @value: value to assign diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index d003ccb12781..fe75ed71c90f 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -180,6 +180,10 @@ static inline bool acpi_can_fallback_to_crs(struct acpi_device *adev, #endif struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum); +int gpiod_get_array_value_complex(bool raw, bool can_sleep, + unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array); void gpiod_set_array_value_complex(bool raw, bool can_sleep, unsigned int array_size, struct gpio_desc **desc_array, diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index 8f702fcbe485..0dfbf5a9349b 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -99,10 +99,16 @@ 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); +int gpiod_get_array_value(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array); void gpiod_set_value(struct gpio_desc *desc, int value); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, int *value_array); int gpiod_get_raw_value(const struct gpio_desc *desc); +int gpiod_get_raw_array_value(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array); void gpiod_set_raw_value(struct gpio_desc *desc, int value); void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, @@ -110,11 +116,17 @@ void gpiod_set_raw_array_value(unsigned int array_size, /* Value get/set from sleeping context */ int gpiod_get_value_cansleep(const struct gpio_desc *desc); +int gpiod_get_array_value_cansleep(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array); void gpiod_set_value_cansleep(struct gpio_desc *desc, int value); void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, int *value_array); int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc); +int gpiod_get_raw_array_value_cansleep(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array); void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value); void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, @@ -305,6 +317,14 @@ static inline int gpiod_get_value(const struct gpio_desc *desc) WARN_ON(1); return 0; } +static inline int gpiod_get_array_value(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); + return 0; +} static inline void gpiod_set_value(struct gpio_desc *desc, int value) { /* GPIO can never have been requested */ @@ -323,6 +343,14 @@ static inline int gpiod_get_raw_value(const struct gpio_desc *desc) WARN_ON(1); return 0; } +static inline int gpiod_get_raw_array_value(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); + return 0; +} static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value) { /* GPIO can never have been requested */ @@ -342,6 +370,14 @@ static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc) WARN_ON(1); return 0; } +static inline int gpiod_get_array_value_cansleep(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); + return 0; +} static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value) { /* GPIO can never have been requested */ @@ -360,6 +396,14 @@ static inline int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc) WARN_ON(1); return 0; } +static inline int gpiod_get_raw_array_value_cansleep(unsigned int array_size, + const struct gpio_desc **desc_array, + int *value_array) +{ + /* GPIO can never have been requested */ + WARN_ON(1); + return 0; +} static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value) { diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index c97f8325e8bf..f793530a8a1a 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -35,6 +35,8 @@ struct module; * @direction_input: configures signal "offset" as input, or returns error * @direction_output: configures signal "offset" as output, or returns error * @get: returns value for signal "offset", 0=low, 1=high, or negative error + * @get_multiple: reads values for multiple signals defined by "mask" and + * stores them in "bits", returns 0 on success or negative error * @set: assigns output value for signal "offset" * @set_multiple: assigns output values for multiple signals defined by "mask" * @set_config: optional hook for all kinds of settings. Uses the same @@ -126,6 +128,9 @@ struct gpio_chip { unsigned offset, int value); int (*get)(struct gpio_chip *chip, unsigned offset); + int (*get_multiple)(struct gpio_chip *chip, + unsigned long *mask, + unsigned long *bits); void (*set)(struct gpio_chip *chip, unsigned offset, int value); void (*set_multiple)(struct gpio_chip *chip,
SPI-attached GPIO controllers typically read out all inputs in one go. If callers desire the values of multipe inputs, ideally a single readout should take place to return the desired values. However the current driver API only offers a ->get callback but no ->get_multiple (unlike ->set_multiple, which is present). Thus, to read multiple inputs, a full readout needs to be performed for every single value (barring driver-internal caching), which is inefficient. In fact, the lack of a ->get_multiple callback has been bemoaned repeatedly by the gpio subsystem maintainer: http://www.spinics.net/lists/linux-gpio/msg10571.html http://www.spinics.net/lists/devicetree/msg121734.html Introduce the missing callback. Add corresponding consumer functions such as gpiod_get_array_value(). Amend linehandle_ioctl() to take advantage of the newly added infrastructure. Signed-off-by: Lukas Wunner <lukas@wunner.de> --- drivers/gpio/gpiolib.c | 181 +++++++++++++++++++++++++++++++++++++++--- drivers/gpio/gpiolib.h | 4 + include/linux/gpio/consumer.h | 44 ++++++++++ include/linux/gpio/driver.h | 5 ++ 4 files changed, 223 insertions(+), 11 deletions(-)