| Message ID | 3961090.tifPlxnXvR@pcimr |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jan 10, 2015 at 12:19 AM, Rojhalat Ibrahim <imr@rtschenk.de> wrote: > Introduce new functions for conveniently obtaining and disposing of an entire > array of GPIOs with one function call. Excellent, thanks for following up with this! :) > > Suggested-by: Alexandre Courbot <acourbot@nvidia.com> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de> > --- > Documentation/gpio/consumer.txt | 28 ++++++++++++- > drivers/gpio/gpiolib.c | 84 ++++++++++++++++++++++++++++++++++++++++ > include/linux/gpio/consumer.h | 42 ++++++++++++++++++++ > 3 files changed, 153 insertions(+), 1 deletion(-) > > diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt > index c67f806..d02f341 100644 > --- a/Documentation/gpio/consumer.txt > +++ b/Documentation/gpio/consumer.txt > @@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and > gpiod_get_index_optional() functions can be used. These functions return NULL > instead of -ENOENT if no GPIO has been assigned to the requested function: > > - > struct gpio_desc *gpiod_get_optional(struct device *dev, > const char *con_id, > enum gpiod_flags flags) > @@ -68,6 +67,29 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: > unsigned int index, > enum gpiod_flags flags) > > +For a function using multiple GPIOs all of those can be obtained with one call: > + > + int gpiod_get_array(struct device *dev, > + const char *con_id, > + struct gpio_desc **desc_array, > + unsigned int count, > + enum gpiod_flags flags) > + > +In this case an array for storing the GPIO descriptors and the number of GPIOs > +are required as additional arguments. This function either returns the > +requested number of GPIOs or an error code. It will not return a positive > +number if some but not all of the requested GPIOs could be obtained. I wonder if the following interface would not be more convenient, at least for the most common cases: struct gpiod_descs *gpiod_get_array(struct device *dev, const char *con_id, enum gpiod_flags flags); Which would *allocate* the array of descriptors to the right number of GPIOs and return them all. The current way of doing requires the user to 1) figure out how many GPIOs there are under (dev, con_id), 2) allocate a the GPIO array and 3) call this function. The array must also be explicitly freed after calling gpiod_put_array(). struct gpiod_descs would be something like this: struct gpiod_descs { unsigned count; struct gpiod_desc*[]; } It uses a zero-length array to store the actual descriptors and carries the count so the user does not have to remember it (it will always be needed anyway, and that way we don't have to pass it explicitly to gpiod_put_array()). The nice thing with this interface is that it works identically to gpiod_get(), excepted it can handle 1 GPIO or more. This would be handy in order to implement sysfs v2 on top of it (see https://lkml.org/lkml/2015/1/3/50). We could even have a macro to iterate over the GPIOs: #define for_each_gpio(gpiod_descs, gpiod_desc) ... If you think your current interface is necessary because users need to work with already-allocated arrays of descriptors, could you then rename it under a more "private" name (maybe gpiod_fill_array/gpiod_free_array?) and build the gpiod_get_array/gpiod_put_array I suggest on top of it? Also for the sake of completeness, may I suggest adding the following function to the gpiod interface: int gpiod_count(struct device *dev, const char *con_id); Which returns the number of GPIOs that are declared for (dev, con_id)? Right now gpiod_get_array() can only be used properly under the device tree (which provides of_gpio_named_count()), but we also need to enable it for ACPI and platform data. Having this function will allow users of gpiod_get_index() to know the boundaries they have to work with, no matter how the GPIO mappings are provided. It will also be needed if you implement the version of gpiod_get_array() I suggest. In your patch 2/2, you will then also want to replace the call to of_gpio_count() in mdio_mux_gpio_probe() by a call to gpiod_count(). > +The following function behaves differently: > + > + int gpiod_get_array_optional(struct device *dev, > + const char *con_id, > + struct gpio_desc **desc_array, > + unsigned int could, > + enum gpiod_flags flags) > + > +This one returns the number of GPIOs actually available which can be smaller > +than the requested number or even 0. There is no user for this function yet but I agree it is good to have. > + > Device-managed variants of these functions are also defined: Do you plan to also add devm variants of the new get_array() function(s)? > > struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id, > @@ -91,6 +113,10 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: > > void gpiod_put(struct gpio_desc *desc) > > +For an array of GPIOs this function can be used: > + > + void gpiod_put_array(struct gpio_desc **desc_array, unsigned int count) > + > It is strictly forbidden to use a descriptor after calling this function. The This paragraph will probably require a s/this/these to be gramatically correct. > device-managed variant is, unsurprisingly: > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 56b7c5d..d45fa9c 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1925,6 +1925,76 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, > EXPORT_SYMBOL_GPL(__gpiod_get_index_optional); > > /** > + * gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function > + * @dev: GPIO consumer, can be NULL for system-global GPIOs > + * @con_id: function within the GPIO consumer > + * @desc_array: descriptor array for the acquired GPIOs > + * @count: number of GPIOs to obtain in the consumer > + * @flags: optional GPIO initialization flags > + * > + * This function obtains the specified number of GPIOs starting with index 0 > + * for functions that define several GPIOs. > + * > + * Return the actual number of acquired GPIOs, -ENOENT if the actual number of > + * GPIOs assigned to the requested function is smaller than @count, > + * or another IS_ERR() code if an error occurred while trying to acquire the > + * GPIOs. > + */ > +int __must_check __gpiod_get_array(struct device *dev, const char *con_id, > + struct gpio_desc **desc_array, > + unsigned int count, > + enum gpiod_flags flags) > +{ > + int r; > + > + r = gpiod_get_array_optional(dev, con_id, desc_array, count, flags); > + if ((r >= 0) && (r != count)) { > + gpiod_put_array(desc_array, r); > + return -ENOENT; > + } > + return r; > +} > +EXPORT_SYMBOL_GPL(__gpiod_get_array); > + > +/** > + * gpiod_get_array_optional - obtain multiple GPIOs from a multi-index GPIO > + * function > + * @dev: GPIO consumer, can be NULL for system-global GPIOs > + * @con_id: function within the GPIO consumer > + * @desc_array: descriptor array for the acquired GPIOs > + * @count: number of GPIOs to obtain in the consumer > + * @flags: optional GPIO initialization flags > + * > + * This is equivalent to gpiod_get_array(), except that when the actual number > + * of GPIOs assigned to the requested function is smaller than @count it will > + * not return an error but the number of GPIOs actually available. > + */ > +int __must_check __gpiod_get_array_optional(struct device *dev, > + const char *con_id, > + struct gpio_desc **desc_array, > + unsigned int count, > + enum gpiod_flags flags) > +{ > + struct gpio_desc *desc; > + unsigned int n; > + > + for (n = 0; n < count; ) { > + desc = gpiod_get_index(dev, con_id, n, flags); > + if (IS_ERR(desc)) { > + if (PTR_ERR(desc) != -ENOENT) { > + gpiod_put_array(desc_array, n); > + return PTR_ERR(desc); > + } > + break; > + } > + desc_array[n] = desc; > + n++; > + } > + return n; > +} > +EXPORT_SYMBOL_GPL(__gpiod_get_array_optional); > + > +/** > * gpiod_put - dispose of a GPIO descriptor > * @desc: GPIO descriptor to dispose of > * > @@ -1936,6 +2006,20 @@ void gpiod_put(struct gpio_desc *desc) > } > EXPORT_SYMBOL_GPL(gpiod_put); > > +/** > + * gpiod_put_array - dispose of multiple GPIO descriptors > + * @desc_array: GPIO descriptor array > + * @count: number of GPIOs in the array > + */ > +void gpiod_put_array(struct gpio_desc **desc_array, unsigned int count) > +{ > + unsigned int n; > + > + for (n = 0; n < count; n++) > + gpiod_put(desc_array[n]); > +} > +EXPORT_SYMBOL_GPL(gpiod_put_array); The implementation looks good to me. I'd just like you to consider the alternative interface I propose, as I suspect it might make the life of users easier. -- 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 Wednesday 14 January 2015 14:11:16 Alexandre Courbot wrote: > On Sat, Jan 10, 2015 at 12:19 AM, Rojhalat Ibrahim <imr@rtschenk.de> wrote: > > Introduce new functions for conveniently obtaining and disposing of an entire > > array of GPIOs with one function call. > > Excellent, thanks for following up with this! :) > > > > > Suggested-by: Alexandre Courbot <acourbot@nvidia.com> > > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de> > > --- > > Documentation/gpio/consumer.txt | 28 ++++++++++++- > > drivers/gpio/gpiolib.c | 84 ++++++++++++++++++++++++++++++++++++++++ > > include/linux/gpio/consumer.h | 42 ++++++++++++++++++++ > > 3 files changed, 153 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt > > index c67f806..d02f341 100644 > > --- a/Documentation/gpio/consumer.txt > > +++ b/Documentation/gpio/consumer.txt > > @@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and > > gpiod_get_index_optional() functions can be used. These functions return NULL > > instead of -ENOENT if no GPIO has been assigned to the requested function: > > > > - > > struct gpio_desc *gpiod_get_optional(struct device *dev, > > const char *con_id, > > enum gpiod_flags flags) > > @@ -68,6 +67,29 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: > > unsigned int index, > > enum gpiod_flags flags) > > > > +For a function using multiple GPIOs all of those can be obtained with one call: > > + > > + int gpiod_get_array(struct device *dev, > > + const char *con_id, > > + struct gpio_desc **desc_array, > > + unsigned int count, > > + enum gpiod_flags flags) > > + > > +In this case an array for storing the GPIO descriptors and the number of GPIOs > > +are required as additional arguments. This function either returns the > > +requested number of GPIOs or an error code. It will not return a positive > > +number if some but not all of the requested GPIOs could be obtained. > > I wonder if the following interface would not be more convenient, at > least for the most common cases: > > struct gpiod_descs *gpiod_get_array(struct device *dev, const char > *con_id, enum gpiod_flags flags); > > Which would *allocate* the array of descriptors to the right number of > GPIOs and return them all. The current way of doing requires the user > to 1) figure out how many GPIOs there are under (dev, con_id), 2) > allocate a the GPIO array and 3) call this function. The array must > also be explicitly freed after calling gpiod_put_array(). > > struct gpiod_descs would be something like this: > > struct gpiod_descs { > unsigned count; > struct gpiod_desc*[]; > } > > It uses a zero-length array to store the actual descriptors and > carries the count so the user does not have to remember it (it will > always be needed anyway, and that way we don't have to pass it > explicitly to gpiod_put_array()). > > The nice thing with this interface is that it works identically to > gpiod_get(), excepted it can handle 1 GPIO or more. This would be > handy in order to implement sysfs v2 on top of it (see > https://lkml.org/lkml/2015/1/3/50). > > We could even have a macro to iterate over the GPIOs: > > #define for_each_gpio(gpiod_descs, gpiod_desc) ... > I see your point. It certainly makes sense. The gpiod_get_array function would have to not only allocate the array of descriptors but also the struct containing the pointer to the array. Alternatively gpiod_get_array could return a struct gpiod_descs instead of a pointer to such a struct: struct gpiod_descs gpiod_get_array(struct device *dev, const char *con_id, enum gpiod_flags flags); Errors could be returned either as a negative count or in an additional struct member. Would that also be an acceptable interface? Or should a gpiod_get_ function always return a pointer? > If you think your current interface is necessary because users need to > work with already-allocated arrays of descriptors, could you then > rename it under a more "private" name (maybe > gpiod_fill_array/gpiod_free_array?) and build the > gpiod_get_array/gpiod_put_array I suggest on top of it? > I think that's unnecessary for now. > Also for the sake of completeness, may I suggest adding the following > function to the gpiod interface: > > int gpiod_count(struct device *dev, const char *con_id); > > Which returns the number of GPIOs that are declared for (dev, con_id)? > Right now gpiod_get_array() can only be used properly under the device > tree (which provides of_gpio_named_count()), but we also need to > enable it for ACPI and platform data. > > Having this function will allow users of gpiod_get_index() to know the > boundaries they have to work with, no matter how the GPIO mappings are > provided. It will also be needed if you implement the version of > gpiod_get_array() I suggest. > > In your patch 2/2, you will then also want to replace the call to > of_gpio_count() in mdio_mux_gpio_probe() by a call to gpiod_count(). > I'll look into that. > > +The following function behaves differently: > > + > > + int gpiod_get_array_optional(struct device *dev, > > + const char *con_id, > > + struct gpio_desc **desc_array, > > + unsigned int could, > > + enum gpiod_flags flags) > > + > > +This one returns the number of GPIOs actually available which can be smaller > > +than the requested number or even 0. > > There is no user for this function yet but I agree it is good to have. > So for the interface you propose that would mean: gpiod_get_array returns with exactly the number of GPIO descriptors as gpiod_count returns or an error code whereas gpiod_get_array_optional can also return with fewer or no descriptors at all. Right? Or is one of the two unnecessary? > > + > > Device-managed variants of these functions are also defined: > > Do you plan to also add devm variants of the new get_array() function(s)? > I probably should. Need to look into it. > > > > struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id, > > @@ -91,6 +113,10 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: > > > > void gpiod_put(struct gpio_desc *desc) > > > > +For an array of GPIOs this function can be used: > > + > > + void gpiod_put_array(struct gpio_desc **desc_array, unsigned int count) > > + > > It is strictly forbidden to use a descriptor after calling this function. The > > This paragraph will probably require a s/this/these to be gramatically correct. > Right. > > device-managed variant is, unsurprisingly: > > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > > index 56b7c5d..d45fa9c 100644 > > --- a/drivers/gpio/gpiolib.c > > +++ b/drivers/gpio/gpiolib.c > > @@ -1925,6 +1925,76 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, > > EXPORT_SYMBOL_GPL(__gpiod_get_index_optional); > > > > /** > > + * gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function > > + * @dev: GPIO consumer, can be NULL for system-global GPIOs > > + * @con_id: function within the GPIO consumer > > + * @desc_array: descriptor array for the acquired GPIOs > > + * @count: number of GPIOs to obtain in the consumer > > + * @flags: optional GPIO initialization flags > > + * > > + * This function obtains the specified number of GPIOs starting with index 0 > > + * for functions that define several GPIOs. > > + * > > + * Return the actual number of acquired GPIOs, -ENOENT if the actual number of > > + * GPIOs assigned to the requested function is smaller than @count, > > + * or another IS_ERR() code if an error occurred while trying to acquire the > > + * GPIOs. > > + */ > > +int __must_check __gpiod_get_array(struct device *dev, const char *con_id, > > + struct gpio_desc **desc_array, > > + unsigned int count, > > + enum gpiod_flags flags) > > +{ > > + int r; > > + > > + r = gpiod_get_array_optional(dev, con_id, desc_array, count, flags); > > + if ((r >= 0) && (r != count)) { > > + gpiod_put_array(desc_array, r); > > + return -ENOENT; > > + } > > + return r; > > +} > > +EXPORT_SYMBOL_GPL(__gpiod_get_array); > > + > > +/** > > + * gpiod_get_array_optional - obtain multiple GPIOs from a multi-index GPIO > > + * function > > + * @dev: GPIO consumer, can be NULL for system-global GPIOs > > + * @con_id: function within the GPIO consumer > > + * @desc_array: descriptor array for the acquired GPIOs > > + * @count: number of GPIOs to obtain in the consumer > > + * @flags: optional GPIO initialization flags > > + * > > + * This is equivalent to gpiod_get_array(), except that when the actual number > > + * of GPIOs assigned to the requested function is smaller than @count it will > > + * not return an error but the number of GPIOs actually available. > > + */ > > +int __must_check __gpiod_get_array_optional(struct device *dev, > > + const char *con_id, > > + struct gpio_desc **desc_array, > > + unsigned int count, > > + enum gpiod_flags flags) > > +{ > > + struct gpio_desc *desc; > > + unsigned int n; > > + > > + for (n = 0; n < count; ) { > > + desc = gpiod_get_index(dev, con_id, n, flags); > > + if (IS_ERR(desc)) { > > + if (PTR_ERR(desc) != -ENOENT) { > > + gpiod_put_array(desc_array, n); > > + return PTR_ERR(desc); > > + } > > + break; > > + } > > + desc_array[n] = desc; > > + n++; > > + } > > + return n; > > +} > > +EXPORT_SYMBOL_GPL(__gpiod_get_array_optional); > > + > > +/** > > * gpiod_put - dispose of a GPIO descriptor > > * @desc: GPIO descriptor to dispose of > > * > > @@ -1936,6 +2006,20 @@ void gpiod_put(struct gpio_desc *desc) > > } > > EXPORT_SYMBOL_GPL(gpiod_put); > > > > +/** > > + * gpiod_put_array - dispose of multiple GPIO descriptors > > + * @desc_array: GPIO descriptor array > > + * @count: number of GPIOs in the array > > + */ > > +void gpiod_put_array(struct gpio_desc **desc_array, unsigned int count) > > +{ > > + unsigned int n; > > + > > + for (n = 0; n < count; n++) > > + gpiod_put(desc_array[n]); > > +} > > +EXPORT_SYMBOL_GPL(gpiod_put_array); > > The implementation looks good to me. I'd just like you to consider the > alternative interface I propose, as I suspect it might make the life > of users easier. > Thanks for reviewing this. -- 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, Jan 14, 2015 at 9:55 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote: > On Wednesday 14 January 2015 14:11:16 Alexandre Courbot wrote: >> On Sat, Jan 10, 2015 at 12:19 AM, Rojhalat Ibrahim <imr@rtschenk.de> wrote: >> > Introduce new functions for conveniently obtaining and disposing of an entire >> > array of GPIOs with one function call. >> >> Excellent, thanks for following up with this! :) >> >> > >> > Suggested-by: Alexandre Courbot <acourbot@nvidia.com> >> > Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de> >> > --- >> > Documentation/gpio/consumer.txt | 28 ++++++++++++- >> > drivers/gpio/gpiolib.c | 84 ++++++++++++++++++++++++++++++++++++++++ >> > include/linux/gpio/consumer.h | 42 ++++++++++++++++++++ >> > 3 files changed, 153 insertions(+), 1 deletion(-) >> > >> > diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt >> > index c67f806..d02f341 100644 >> > --- a/Documentation/gpio/consumer.txt >> > +++ b/Documentation/gpio/consumer.txt >> > @@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and >> > gpiod_get_index_optional() functions can be used. These functions return NULL >> > instead of -ENOENT if no GPIO has been assigned to the requested function: >> > >> > - >> > struct gpio_desc *gpiod_get_optional(struct device *dev, >> > const char *con_id, >> > enum gpiod_flags flags) >> > @@ -68,6 +67,29 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: >> > unsigned int index, >> > enum gpiod_flags flags) >> > >> > +For a function using multiple GPIOs all of those can be obtained with one call: >> > + >> > + int gpiod_get_array(struct device *dev, >> > + const char *con_id, >> > + struct gpio_desc **desc_array, >> > + unsigned int count, >> > + enum gpiod_flags flags) >> > + >> > +In this case an array for storing the GPIO descriptors and the number of GPIOs >> > +are required as additional arguments. This function either returns the >> > +requested number of GPIOs or an error code. It will not return a positive >> > +number if some but not all of the requested GPIOs could be obtained. >> >> I wonder if the following interface would not be more convenient, at >> least for the most common cases: >> >> struct gpiod_descs *gpiod_get_array(struct device *dev, const char >> *con_id, enum gpiod_flags flags); >> >> Which would *allocate* the array of descriptors to the right number of >> GPIOs and return them all. The current way of doing requires the user >> to 1) figure out how many GPIOs there are under (dev, con_id), 2) >> allocate a the GPIO array and 3) call this function. The array must >> also be explicitly freed after calling gpiod_put_array(). >> >> struct gpiod_descs would be something like this: >> >> struct gpiod_descs { >> unsigned count; >> struct gpiod_desc*[]; >> } >> >> It uses a zero-length array to store the actual descriptors and >> carries the count so the user does not have to remember it (it will >> always be needed anyway, and that way we don't have to pass it >> explicitly to gpiod_put_array()). >> >> The nice thing with this interface is that it works identically to >> gpiod_get(), excepted it can handle 1 GPIO or more. This would be >> handy in order to implement sysfs v2 on top of it (see >> https://lkml.org/lkml/2015/1/3/50). >> >> We could even have a macro to iterate over the GPIOs: >> >> #define for_each_gpio(gpiod_descs, gpiod_desc) ... >> > > I see your point. It certainly makes sense. > > The gpiod_get_array function would have to not only allocate the array of > descriptors but also the struct containing the pointer to the array. > Alternatively gpiod_get_array could return a struct gpiod_descs instead of > a pointer to such a struct: > > struct gpiod_descs gpiod_get_array(struct device *dev, > const char *con_id, > enum gpiod_flags flags); > > Errors could be returned either as a negative count or in an additional > struct member. > > Would that also be an acceptable interface? Or should a gpiod_get_ function > always return a pointer? If you use the following struct, you should be able to overcome that double-allocation issue: struct gpiod_descs { unsigned count; struct gpiod_desc*[] desc; }; The last member is a flexible array member. See https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html if you are not familiar with them. With such a declaration, you can do something like this: struct gpiod_descs *descs; int count = 10; descs = kzalloc(sizeof(*descs) + sizeof(descs->desc[0]) * count); descs->count = count; for (i = 0; i < count; i++) descs->desc[i] = ...; That way you can get away with a single allocation for the whole structure and return a pointer. > >> If you think your current interface is necessary because users need to >> work with already-allocated arrays of descriptors, could you then >> rename it under a more "private" name (maybe >> gpiod_fill_array/gpiod_free_array?) and build the >> gpiod_get_array/gpiod_put_array I suggest on top of it? >> > > I think that's unnecessary for now. > >> Also for the sake of completeness, may I suggest adding the following >> function to the gpiod interface: >> >> int gpiod_count(struct device *dev, const char *con_id); >> >> Which returns the number of GPIOs that are declared for (dev, con_id)? >> Right now gpiod_get_array() can only be used properly under the device >> tree (which provides of_gpio_named_count()), but we also need to >> enable it for ACPI and platform data. >> >> Having this function will allow users of gpiod_get_index() to know the >> boundaries they have to work with, no matter how the GPIO mappings are >> provided. It will also be needed if you implement the version of >> gpiod_get_array() I suggest. >> >> In your patch 2/2, you will then also want to replace the call to >> of_gpio_count() in mdio_mux_gpio_probe() by a call to gpiod_count(). >> > > I'll look into that. > >> > +The following function behaves differently: >> > + >> > + int gpiod_get_array_optional(struct device *dev, >> > + const char *con_id, >> > + struct gpio_desc **desc_array, >> > + unsigned int could, >> > + enum gpiod_flags flags) >> > + >> > +This one returns the number of GPIOs actually available which can be smaller >> > +than the requested number or even 0. >> >> There is no user for this function yet but I agree it is good to have. >> > > So for the interface you propose that would mean: > > gpiod_get_array returns with exactly the number of GPIO descriptors as > gpiod_count returns or an error code whereas gpiod_get_array_optional can also > return with fewer or no descriptors at all. > > Right? Or is one of the two unnecessary? Mmm if you switch to the interface I proposed, then I believe that gpiod_get_array_optional() becomes less useful since gpiod_get_array() will always return the "right" number of GPIOs. However it might still be good to have it for the same purpose as gpiod_get_optional() exists: to make it easier to handle the case where not having a mapping for the requested GPIO is not an error. So gpiod_get_array_optional() would just call gpiod_get_array(), and simply return NULL instead of -ENOENT if no mapping exist. Cheers, Alex. -- 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/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt index c67f806..d02f341 100644 --- a/Documentation/gpio/consumer.txt +++ b/Documentation/gpio/consumer.txt @@ -58,7 +58,6 @@ pattern where a GPIO is optional, the gpiod_get_optional() and gpiod_get_index_optional() functions can be used. These functions return NULL instead of -ENOENT if no GPIO has been assigned to the requested function: - struct gpio_desc *gpiod_get_optional(struct device *dev, const char *con_id, enum gpiod_flags flags) @@ -68,6 +67,29 @@ instead of -ENOENT if no GPIO has been assigned to the requested function: unsigned int index, enum gpiod_flags flags) +For a function using multiple GPIOs all of those can be obtained with one call: + + int gpiod_get_array(struct device *dev, + const char *con_id, + struct gpio_desc **desc_array, + unsigned int count, + enum gpiod_flags flags) + +In this case an array for storing the GPIO descriptors and the number of GPIOs +are required as additional arguments. This function either returns the +requested number of GPIOs or an error code. It will not return a positive +number if some but not all of the requested GPIOs could be obtained. +The following function behaves differently: + + int gpiod_get_array_optional(struct device *dev, + const char *con_id, + struct gpio_desc **desc_array, + unsigned int could, + enum gpiod_flags flags) + +This one returns the number of GPIOs actually available which can be smaller +than the requested number or even 0. + Device-managed variants of these functions are also defined: struct gpio_desc *devm_gpiod_get(struct device *dev, const char *con_id, @@ -91,6 +113,10 @@ A GPIO descriptor can be disposed of using the gpiod_put() function: void gpiod_put(struct gpio_desc *desc) +For an array of GPIOs this function can be used: + + void gpiod_put_array(struct gpio_desc **desc_array, unsigned int count) + It is strictly forbidden to use a descriptor after calling this function. The device-managed variant is, unsurprisingly: diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 56b7c5d..d45fa9c 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1925,6 +1925,76 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, EXPORT_SYMBOL_GPL(__gpiod_get_index_optional); /** + * gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function + * @dev: GPIO consumer, can be NULL for system-global GPIOs + * @con_id: function within the GPIO consumer + * @desc_array: descriptor array for the acquired GPIOs + * @count: number of GPIOs to obtain in the consumer + * @flags: optional GPIO initialization flags + * + * This function obtains the specified number of GPIOs starting with index 0 + * for functions that define several GPIOs. + * + * Return the actual number of acquired GPIOs, -ENOENT if the actual number of + * GPIOs assigned to the requested function is smaller than @count, + * or another IS_ERR() code if an error occurred while trying to acquire the + * GPIOs. + */ +int __must_check __gpiod_get_array(struct device *dev, const char *con_id, + struct gpio_desc **desc_array, + unsigned int count, + enum gpiod_flags flags) +{ + int r; + + r = gpiod_get_array_optional(dev, con_id, desc_array, count, flags); + if ((r >= 0) && (r != count)) { + gpiod_put_array(desc_array, r); + return -ENOENT; + } + return r; +} +EXPORT_SYMBOL_GPL(__gpiod_get_array); + +/** + * gpiod_get_array_optional - obtain multiple GPIOs from a multi-index GPIO + * function + * @dev: GPIO consumer, can be NULL for system-global GPIOs + * @con_id: function within the GPIO consumer + * @desc_array: descriptor array for the acquired GPIOs + * @count: number of GPIOs to obtain in the consumer + * @flags: optional GPIO initialization flags + * + * This is equivalent to gpiod_get_array(), except that when the actual number + * of GPIOs assigned to the requested function is smaller than @count it will + * not return an error but the number of GPIOs actually available. + */ +int __must_check __gpiod_get_array_optional(struct device *dev, + const char *con_id, + struct gpio_desc **desc_array, + unsigned int count, + enum gpiod_flags flags) +{ + struct gpio_desc *desc; + unsigned int n; + + for (n = 0; n < count; ) { + desc = gpiod_get_index(dev, con_id, n, flags); + if (IS_ERR(desc)) { + if (PTR_ERR(desc) != -ENOENT) { + gpiod_put_array(desc_array, n); + return PTR_ERR(desc); + } + break; + } + desc_array[n] = desc; + n++; + } + return n; +} +EXPORT_SYMBOL_GPL(__gpiod_get_array_optional); + +/** * gpiod_put - dispose of a GPIO descriptor * @desc: GPIO descriptor to dispose of * @@ -1936,6 +2006,20 @@ void gpiod_put(struct gpio_desc *desc) } EXPORT_SYMBOL_GPL(gpiod_put); +/** + * gpiod_put_array - dispose of multiple GPIO descriptors + * @desc_array: GPIO descriptor array + * @count: number of GPIOs in the array + */ +void gpiod_put_array(struct gpio_desc **desc_array, unsigned int count) +{ + unsigned int n; + + for (n = 0; n < count; n++) + gpiod_put(desc_array[n]); +} +EXPORT_SYMBOL_GPL(gpiod_put_array); + #ifdef CONFIG_DEBUG_FS static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip) diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index d54d158..f225d25 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -49,7 +49,17 @@ struct gpio_desc *__must_check __gpiod_get_index_optional(struct device *dev, const char *con_id, unsigned int index, enum gpiod_flags flags); +int __must_check __gpiod_get_array(struct device *dev, const char *con_id, + struct gpio_desc **desc_array, + unsigned int count, + enum gpiod_flags flags); +int __must_check __gpiod_get_array_optional(struct device *dev, + const char *con_id, + struct gpio_desc **desc_array, + unsigned int count, + enum gpiod_flags flags); void gpiod_put(struct gpio_desc *desc); +void gpiod_put_array(struct gpio_desc **desc_array, unsigned int count); struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id, @@ -135,6 +145,22 @@ __gpiod_get_index_optional(struct device *dev, const char *con_id, return ERR_PTR(-ENOSYS); } +static inline int __must_check +__gpiod_get_array(struct device *dev, const char *con_id, + struct gpio_desc **desc_array, unsigned int count, + enum gpiod_flags flags) +{ + return ERR_PTR(-ENOSYS); +} + +static inline int __must_check +__gpiod_get_array_optional(struct device *dev, const char *con_id, + struct gpio_desc **desc_array, unsigned int count, + enum gpiod_flags flags) +{ + return ERR_PTR(-ENOSYS); +} + static inline void gpiod_put(struct gpio_desc *desc) { might_sleep(); @@ -143,6 +169,15 @@ static inline void gpiod_put(struct gpio_desc *desc) WARN_ON(1); } +static inline void gpiod_put_array(struct gpio_desc **desc_array, + unsigned int count) +{ + might_sleep(); + + /* GPIO can never have been requested */ + WARN_ON(1); +} + static inline struct gpio_desc *__must_check __devm_gpiod_get(struct device *dev, const char *con_id, @@ -344,6 +379,13 @@ static inline int desc_to_gpio(const struct gpio_desc *desc) __gpiod_get_index_optional(dev, con_id, index, flags) #define gpiod_get_index_optional(varargs...) \ __gpiod_get_index_optional(varargs, 0) +#define __gpiod_get_array(dev, con_id, desc_array, count, flags, ...) \ + __gpiod_get_array(dev, con_id, desc_array, count, flags) +#define gpiod_get_array(varargs...) __gpiod_get_array(varargs, 0) +#define __gpiod_get_array_optional(dev, con_id, desc_array, count, flags, ...) \ + __gpiod_get_array_optional(dev, con_id, desc_array, count, flags) +#define gpiod_get_array_optional(varargs...) \ + __gpiod_get_array_optional(varargs, 0) #define __devm_gpiod_get(dev, con_id, flags, ...) \ __devm_gpiod_get(dev, con_id, flags) #define devm_gpiod_get(varargs...) __devm_gpiod_get(varargs, 0)
Introduce new functions for conveniently obtaining and disposing of an entire array of GPIOs with one function call. Suggested-by: Alexandre Courbot <acourbot@nvidia.com> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de> --- Documentation/gpio/consumer.txt | 28 ++++++++++++- drivers/gpio/gpiolib.c | 84 ++++++++++++++++++++++++++++++++++++++++ include/linux/gpio/consumer.h | 42 ++++++++++++++++++++ 3 files changed, 153 insertions(+), 1 deletion(-) -- 2.0.5 -- 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