| Message ID | 1a1752ce7455bac8f2505a9790da891a830699f5.1404665589.git.himangi774@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Please note that I submitted a patch on 02/07/14 to create this function which was acked by Linus Walleij on 05/07/14. Great minds think alike, and all that. However, I think that the version I submitted better replicates the original (non devm) functionality, see below. I didn't, however, add it to the documentation. +1 on that. On 06/07/14 18:47, Himangi Saraogi wrote: > This patch introduces the function devm_request_gpio_array that > allocates multiple GPIOs in a single call in a managed manner. The > function is also included in the documentation and a declaration is > added in include/linux/gpio.h. > > Signed-off-by: Himangi Saraogi <himangi774@gmail.com> > --- > Documentation/driver-model/devres.txt | 1 + > drivers/gpio/devres.c | 21 +++++++++++++++++++++ > include/linux/gpio.h | 2 ++ > 3 files changed, 24 insertions(+) > > diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt > index 9e2098e..756f6cf 100644 > --- a/Documentation/driver-model/devres.txt > +++ b/Documentation/driver-model/devres.txt > @@ -337,6 +337,7 @@ GPIO > devm_gpiod_put() > devm_gpio_request() > devm_gpio_request_one() > + devm_gpio_request_array() > devm_gpio_free() > > SND > diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c > index 65978cf..adae7fa 100644 > --- a/drivers/gpio/devres.c > +++ b/drivers/gpio/devres.c > @@ -229,6 +229,27 @@ int devm_gpio_request_one(struct device *dev, unsigned gpio, > EXPORT_SYMBOL(devm_gpio_request_one); > > /** > + * devm_gpio_request_array - request multiple GPIOs in a single call > + * @dev: device to request for > + * @array: array of the 'struct gpio' > + * @num: how many GPIOs in the array > + */ > +int devm_gpio_request_array(struct device *dev, const struct gpio *array, > + size_t num) > +{ > + int i, err; > + > + for (i = 0; i < num; i++, array++) { > + err = devm_gpio_request_one(dev, array->gpio, array->flags, > + array->label); > + if (err) > + return err; The failure path in the version I submitted frees up any already allocated gpios on the first failure. > + } > + return 0; > +} > +EXPORT_SYMBOL_GPL(devm_gpio_request_array); > + > +/** > * devm_gpio_free - free a GPIO > * @dev: device to free GPIO for > * @gpio: GPIO to free > diff --git a/include/linux/gpio.h b/include/linux/gpio.h > index 85aa5d0..c85f243 100644 > --- a/include/linux/gpio.h > +++ b/include/linux/gpio.h > @@ -84,6 +84,8 @@ struct device; > int devm_gpio_request(struct device *dev, unsigned gpio, const char *label); > int devm_gpio_request_one(struct device *dev, unsigned gpio, > unsigned long flags, const char *label); > +int devm_gpio_request_array(struct device *dev, const struct gpio *array, > + size_t num); > void devm_gpio_free(struct device *dev, unsigned int gpio); > > #else /* ! CONFIG_GPIOLIB */ >
On Wed, 9 Jul 2014, Rob Jones wrote: > Please note that I submitted a patch on 02/07/14 to create this > function which was acked by Linus Walleij on 05/07/14. > > Great minds think alike, and all that. > > However, I think that the version I submitted better replicates the > original (non devm) functionality, see below. > > I didn't, however, add it to the documentation. +1 on that. > > On 06/07/14 18:47, Himangi Saraogi wrote: > > This patch introduces the function devm_request_gpio_array that > > allocates multiple GPIOs in a single call in a managed manner. The > > function is also included in the documentation and a declaration is > > added in include/linux/gpio.h. > > > > Signed-off-by: Himangi Saraogi <himangi774@gmail.com> > > --- > > Documentation/driver-model/devres.txt | 1 + > > drivers/gpio/devres.c | 21 +++++++++++++++++++++ > > include/linux/gpio.h | 2 ++ > > 3 files changed, 24 insertions(+) > > > > diff --git a/Documentation/driver-model/devres.txt > > b/Documentation/driver-model/devres.txt > > index 9e2098e..756f6cf 100644 > > --- a/Documentation/driver-model/devres.txt > > +++ b/Documentation/driver-model/devres.txt > > @@ -337,6 +337,7 @@ GPIO > > devm_gpiod_put() > > devm_gpio_request() > > devm_gpio_request_one() > > + devm_gpio_request_array() > > devm_gpio_free() > > > > SND > > diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c > > index 65978cf..adae7fa 100644 > > --- a/drivers/gpio/devres.c > > +++ b/drivers/gpio/devres.c > > @@ -229,6 +229,27 @@ int devm_gpio_request_one(struct device *dev, unsigned > > gpio, > > EXPORT_SYMBOL(devm_gpio_request_one); > > > > /** > > + * devm_gpio_request_array - request multiple GPIOs in a single call > > + * @dev: device to request for > > + * @array: array of the 'struct gpio' > > + * @num: how many GPIOs in the array > > + */ > > +int devm_gpio_request_array(struct device *dev, const struct gpio *array, > > + size_t num) > > +{ > > + int i, err; > > + > > + for (i = 0; i < num; i++, array++) { > > + err = devm_gpio_request_one(dev, array->gpio, array->flags, > > + array->label); > > + if (err) > > + return err; > > The failure path in the version I submitted frees up any already > allocated gpios on the first failure. Himangi first proposed doing that, but I thought it was not really in the spirit of devm functions, which is to leave that implicit. No strong opinion on the matter, though. julia > > > + } > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(devm_gpio_request_array); > > + > > +/** > > * devm_gpio_free - free a GPIO > > * @dev: device to free GPIO for > > * @gpio: GPIO to free > > diff --git a/include/linux/gpio.h b/include/linux/gpio.h > > index 85aa5d0..c85f243 100644 > > --- a/include/linux/gpio.h > > +++ b/include/linux/gpio.h > > @@ -84,6 +84,8 @@ struct device; > > int devm_gpio_request(struct device *dev, unsigned gpio, const char > > *label); > > int devm_gpio_request_one(struct device *dev, unsigned gpio, > > unsigned long flags, const char *label); > > +int devm_gpio_request_array(struct device *dev, const struct gpio *array, > > + size_t num); > > void devm_gpio_free(struct device *dev, unsigned int gpio); > > > > #else /* ! CONFIG_GPIOLIB */ > > > > -- > Rob Jones > Codethink Ltd > mailto:rob.jones@codethink.co.uk > tel:+44 161 236 5575 >
On 09/07/14 12:52, Julia Lawall wrote: > > > On Wed, 9 Jul 2014, Rob Jones wrote: > >> Please note that I submitted a patch on 02/07/14 to create this >> function which was acked by Linus Walleij on 05/07/14. >> >> Great minds think alike, and all that. >> >> However, I think that the version I submitted better replicates the >> original (non devm) functionality, see below. <snip> >>> +int devm_gpio_request_array(struct device *dev, const struct gpio *array, >>> + size_t num) >>> +{ >>> + int i, err; >>> + >>> + for (i = 0; i < num; i++, array++) { >>> + err = devm_gpio_request_one(dev, array->gpio, array->flags, >>> + array->label); >>> + if (err) >>> + return err; >> >> The failure path in the version I submitted frees up any already >> allocated gpios on the first failure. > > Himangi first proposed doing that, but I thought it was not really in the > spirit of devm functions, which is to leave that implicit. No strong > opinion on the matter, though. > Interestingly, we came at it from the other direction: I originally didn't have the unwind in and our internal review guys suggested that it was in the original function so putting it in would: 1. make this a better analogue of the original 2. help avoid deadlocks 3. allow the driver to retry, perhaps requesting reduced functionality. > julia > <snip>
On Wed, Jul 9, 2014 at 1:18 PM, Rob Jones <rob.jones@codethink.co.uk> wrote: > Please note that I submitted a patch on 02/07/14 to create this > function which was acked by Linus Walleij on 05/07/14. > > Great minds think alike, and all that. My mind certainly isn't any great :-( My left hand seems to be unaware of what the right hand is doing. Did I ACK that for merging through some other tree then? And I guess if Himangi want to submit these ASoC patches, they will have to go through the same tree as those patches in that case? And would you consider implementing a gpiod version...? > However, I think that the version I submitted better replicates the > original (non devm) functionality, see below. > > I didn't, however, add it to the documentation. +1 on that. Can you include that oneliner in your patch then to reduce problems? Yours, Linus Walleij
On 10/07/14 10:21, Linus Walleij wrote: > On Wed, Jul 9, 2014 at 1:18 PM, Rob Jones <rob.jones@codethink.co.uk> wrote: > >> Please note that I submitted a patch on 02/07/14 to create this >> function which was acked by Linus Walleij on 05/07/14. >> >> Great minds think alike, and all that. > > My mind certainly isn't any great :-( > I doubt that :-) > My left hand seems to be unaware of what the right hand is doing. > > Did I ACK that for merging through some other tree then? > You did: https://lkml.org/lkml/2014/7/4/715 > And I guess if Himangi want to submit these ASoC patches, they > will have to go through the same tree as those patches in that case? > I don't know enough to comment. > And would you consider implementing a gpiod version...? > I will certainly consider it. I'm in the middle of something else right now but I should be available in a day or two and I'll have a look. >> However, I think that the version I submitted better replicates the >> original (non devm) functionality, see below. >> >> I didn't, however, add it to the documentation. +1 on that. > > Can you include that oneliner in your patch then to reduce > problems? > Certainly, with acknowledgement to Himangi Saraogi. Should I resubmit the patch series with amendment or should I put it in as a separate patch? Sorry for the noobie question but I *am* new to the kernel lists. > Yours, > Linus Walleij > >
Hello Rob, On Thu, Jul 10, 2014 at 1:01 PM, Rob Jones <rob.jones@codethink.co.uk> wrote: > > > On 10/07/14 10:21, Linus Walleij wrote: >> >> On Wed, Jul 9, 2014 at 1:18 PM, Rob Jones <rob.jones@codethink.co.uk> >> wrote: >> >>> Please note that I submitted a patch on 02/07/14 to create this >>> function which was acked by Linus Walleij on 05/07/14. >>> >>> Great minds think alike, and all that. >> >> >> My mind certainly isn't any great :-( >> > > I doubt that :-) > > >> My left hand seems to be unaware of what the right hand is doing. >> >> Did I ACK that for merging through some other tree then? >> > > You did: https://lkml.org/lkml/2014/7/4/715 > > >> And I guess if Himangi want to submit these ASoC patches, they >> will have to go through the same tree as those patches in that case? >> > > I don't know enough to comment. > > >> And would you consider implementing a gpiod version...? >> > > I will certainly consider it. I'm in the middle of something else right > now but I should be available in a day or two and I'll have a look. > > I'm working on a driver for a PMIC which uses a set of GPIO to select a particular register. Right now I'm getting each GPIO independently but Linus mentioned [0] the devm_gpio_request_array() patch you posted. Now, I'm not using the integer-based GPIO API but the new descriptor-based one so I was wondering if you are already implementing a devm_gpiod_get_array() or if you want me to implement it myself. Best regards, Javier [0]: https://lkml.org/lkml/2014/7/10/166
On 11/07/14 01:35, Javier Martinez Canillas wrote: > Hello Rob, > > On Thu, Jul 10, 2014 at 1:01 PM, Rob Jones <rob.jones@codethink.co.uk> wrote: <snip> >> >> I will certainly consider it. I'm in the middle of something else right >> now but I should be available in a day or two and I'll have a look. >> >> > > I'm working on a driver for a PMIC which uses a set of GPIO to select > a particular register. Right now I'm getting each GPIO independently > but Linus mentioned [0] the devm_gpio_request_array() patch you > posted. > > Now, I'm not using the integer-based GPIO API but the new > descriptor-based one so I was wondering if you are already > implementing a devm_gpiod_get_array() or if you want me to implement > it myself. > As I said, I'm deep in something else for the next day or two so if you need it urgently you should probably do it yourself but if you don't mind waiting until next week, I can have a go. Your call, let me know. > Best regards, > Javier > > [0]: https://lkml.org/lkml/2014/7/10/166 > -- > 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 >
Hello Rob, On Fri, Jul 11, 2014 at 12:03 PM, Rob Jones <rob.jones@codethink.co.uk> wrote: > > > On 11/07/14 01:35, Javier Martinez Canillas wrote: >> >> Hello Rob, >> >> On Thu, Jul 10, 2014 at 1:01 PM, Rob Jones <rob.jones@codethink.co.uk> >> wrote: > > > <snip> > > >>> >>> I will certainly consider it. I'm in the middle of something else right >>> now but I should be available in a day or two and I'll have a look. >>> >>> >> >> I'm working on a driver for a PMIC which uses a set of GPIO to select >> a particular register. Right now I'm getting each GPIO independently >> but Linus mentioned [0] the devm_gpio_request_array() patch you >> posted. >> >> Now, I'm not using the integer-based GPIO API but the new >> descriptor-based one so I was wondering if you are already >> implementing a devm_gpiod_get_array() or if you want me to implement >> it myself. >> > > As I said, I'm deep in something else for the next day or two so if you > need it urgently you should probably do it yourself but if you don't > mind waiting until next week, I can have a go. > > Your call, let me know. > I'm not in a hurry, just wanted to know if it was on your plate. I'll wait for your patch then, thanks! Best regards, Javier
I'm afraid too many other things have been landing on my plate and I don't think I will be able to get on this for the foreseeable future, sorry about this. On 11/07/14 11:07, Javier Martinez Canillas wrote: > Hello Rob, > > On Fri, Jul 11, 2014 at 12:03 PM, Rob Jones <rob.jones@codethink.co.uk> wrote: >> >> >> On 11/07/14 01:35, Javier Martinez Canillas wrote: >>> >>> Hello Rob, >>> >>> On Thu, Jul 10, 2014 at 1:01 PM, Rob Jones <rob.jones@codethink.co.uk> >>> wrote: >> >> >> <snip> >> >> >>>> >>>> I will certainly consider it. I'm in the middle of something else right >>>> now but I should be available in a day or two and I'll have a look. >>>> >>>> >>> >>> I'm working on a driver for a PMIC which uses a set of GPIO to select >>> a particular register. Right now I'm getting each GPIO independently >>> but Linus mentioned [0] the devm_gpio_request_array() patch you >>> posted. >>> >>> Now, I'm not using the integer-based GPIO API but the new >>> descriptor-based one so I was wondering if you are already >>> implementing a devm_gpiod_get_array() or if you want me to implement >>> it myself. >>> >> >> As I said, I'm deep in something else for the next day or two so if you >> need it urgently you should probably do it yourself but if you don't >> mind waiting until next week, I can have a go. >> >> Your call, let me know. >> > > I'm not in a hurry, just wanted to know if it was on your plate. I'll > wait for your patch then, thanks! > > Best regards, > Javier > >
diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt index 9e2098e..756f6cf 100644 --- a/Documentation/driver-model/devres.txt +++ b/Documentation/driver-model/devres.txt @@ -337,6 +337,7 @@ GPIO devm_gpiod_put() devm_gpio_request() devm_gpio_request_one() + devm_gpio_request_array() devm_gpio_free() SND diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c index 65978cf..adae7fa 100644 --- a/drivers/gpio/devres.c +++ b/drivers/gpio/devres.c @@ -229,6 +229,27 @@ int devm_gpio_request_one(struct device *dev, unsigned gpio, EXPORT_SYMBOL(devm_gpio_request_one); /** + * devm_gpio_request_array - request multiple GPIOs in a single call + * @dev: device to request for + * @array: array of the 'struct gpio' + * @num: how many GPIOs in the array + */ +int devm_gpio_request_array(struct device *dev, const struct gpio *array, + size_t num) +{ + int i, err; + + for (i = 0; i < num; i++, array++) { + err = devm_gpio_request_one(dev, array->gpio, array->flags, + array->label); + if (err) + return err; + } + return 0; +} +EXPORT_SYMBOL_GPL(devm_gpio_request_array); + +/** * devm_gpio_free - free a GPIO * @dev: device to free GPIO for * @gpio: GPIO to free diff --git a/include/linux/gpio.h b/include/linux/gpio.h index 85aa5d0..c85f243 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -84,6 +84,8 @@ struct device; int devm_gpio_request(struct device *dev, unsigned gpio, const char *label); int devm_gpio_request_one(struct device *dev, unsigned gpio, unsigned long flags, const char *label); +int devm_gpio_request_array(struct device *dev, const struct gpio *array, + size_t num); void devm_gpio_free(struct device *dev, unsigned int gpio); #else /* ! CONFIG_GPIOLIB */
This patch introduces the function devm_request_gpio_array that allocates multiple GPIOs in a single call in a managed manner. The function is also included in the documentation and a declaration is added in include/linux/gpio.h. Signed-off-by: Himangi Saraogi <himangi774@gmail.com> --- Documentation/driver-model/devres.txt | 1 + drivers/gpio/devres.c | 21 +++++++++++++++++++++ include/linux/gpio.h | 2 ++ 3 files changed, 24 insertions(+)