* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-01 11:55 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-01 11:55 UTC (permalink / raw) To: Liam Girdwood, Linus Walleij, Russell King Cc: linux-arm-kernel, linux-kernel, linaro-kernel, Alexandre Courbot, Mark Brown From: Mark Brown <broonie@linaro.org> Even though we mostly use GPIO descriptors internally we still use gpio_request_one() to request so we need to pair that with gpio_free() to release the GPIO. Reported-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Mark Brown <broonie@linaro.org> --- drivers/regulator/core.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index c563d93125cd..80381409f856 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1710,7 +1710,12 @@ static void regulator_ena_gpio_free(struct regulator_dev *rdev) if (pin->gpiod == rdev->ena_pin->gpiod) { if (pin->request_count <= 1) { pin->request_count = 0; - gpiod_put(pin->gpiod); + /* + * Since we requested with gpio_request_one() + * we still need to free with gpio_free() + * for now. + */ + gpio_free(desc_to_gpio(pin->gpiod)); list_del(&pin->list); kfree(pin); } else { -- 2.0.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-01 11:55 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-01 11:55 UTC (permalink / raw) To: linux-arm-kernel From: Mark Brown <broonie@linaro.org> Even though we mostly use GPIO descriptors internally we still use gpio_request_one() to request so we need to pair that with gpio_free() to release the GPIO. Reported-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Mark Brown <broonie@linaro.org> --- drivers/regulator/core.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index c563d93125cd..80381409f856 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1710,7 +1710,12 @@ static void regulator_ena_gpio_free(struct regulator_dev *rdev) if (pin->gpiod == rdev->ena_pin->gpiod) { if (pin->request_count <= 1) { pin->request_count = 0; - gpiod_put(pin->gpiod); + /* + * Since we requested with gpio_request_one() + * we still need to free with gpio_free() + * for now. + */ + gpio_free(desc_to_gpio(pin->gpiod)); list_del(&pin->list); kfree(pin); } else { -- 2.0.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] regulator: core: Still free GPIOs using gpio_free() 2014-07-01 11:55 ` Mark Brown @ 2014-07-01 12:06 ` Russell King - ARM Linux -1 siblings, 0 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2014-07-01 12:06 UTC (permalink / raw) To: Mark Brown Cc: Liam Girdwood, Linus Walleij, linux-arm-kernel, linux-kernel, linaro-kernel, Alexandre Courbot, Mark Brown On Tue, Jul 01, 2014 at 12:55:26PM +0100, Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > Even though we mostly use GPIO descriptors internally we still use > gpio_request_one() to request so we need to pair that with gpio_free() to > release the GPIO. > > Reported-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Mark Brown <broonie@linaro.org> This doesn't explain the reason... I see the current GPIO code doing this: void gpio_free(unsigned gpio) { gpiod_free(gpio_to_desc(gpio)); } EXPORT_SYMBOL_GPL(gpio_free); -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-01 12:06 ` Russell King - ARM Linux 0 siblings, 0 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2014-07-01 12:06 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 01, 2014 at 12:55:26PM +0100, Mark Brown wrote: > From: Mark Brown <broonie@linaro.org> > > Even though we mostly use GPIO descriptors internally we still use > gpio_request_one() to request so we need to pair that with gpio_free() to > release the GPIO. > > Reported-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Mark Brown <broonie@linaro.org> This doesn't explain the reason... I see the current GPIO code doing this: void gpio_free(unsigned gpio) { gpiod_free(gpio_to_desc(gpio)); } EXPORT_SYMBOL_GPL(gpio_free); -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] regulator: core: Still free GPIOs using gpio_free() 2014-07-01 12:06 ` Russell King - ARM Linux @ 2014-07-01 12:11 ` Mark Brown -1 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-01 12:11 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Liam Girdwood, Linus Walleij, linux-arm-kernel, linux-kernel, linaro-kernel, Alexandre Courbot [-- Attachment #1: Type: text/plain, Size: 619 bytes --] On Tue, Jul 01, 2014 at 01:06:22PM +0100, Russell King - ARM Linux wrote: > On Tue, Jul 01, 2014 at 12:55:26PM +0100, Mark Brown wrote: > > Even though we mostly use GPIO descriptors internally we still use > > gpio_request_one() to request so we need to pair that with gpio_free() to > > release the GPIO. > This doesn't explain the reason... I see the current GPIO code doing this: > void gpio_free(unsigned gpio) > { > gpiod_free(gpio_to_desc(gpio)); > } > EXPORT_SYMBOL_GPL(gpio_free); Huh, good point (though it still seems a bit neater to match up the request and release functions I guess). Linus? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-01 12:11 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-01 12:11 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 01, 2014 at 01:06:22PM +0100, Russell King - ARM Linux wrote: > On Tue, Jul 01, 2014 at 12:55:26PM +0100, Mark Brown wrote: > > Even though we mostly use GPIO descriptors internally we still use > > gpio_request_one() to request so we need to pair that with gpio_free() to > > release the GPIO. > This doesn't explain the reason... I see the current GPIO code doing this: > void gpio_free(unsigned gpio) > { > gpiod_free(gpio_to_desc(gpio)); > } > EXPORT_SYMBOL_GPL(gpio_free); Huh, good point (though it still seems a bit neater to match up the request and release functions I guess). Linus? -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140701/833c328d/attachment.sig> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] regulator: core: Still free GPIOs using gpio_free() 2014-07-01 12:11 ` Mark Brown @ 2014-07-01 12:24 ` Alexandre Courbot -1 siblings, 0 replies; 14+ messages in thread From: Alexandre Courbot @ 2014-07-01 12:24 UTC (permalink / raw) To: Mark Brown Cc: Russell King - ARM Linux, Liam Girdwood, Linus Walleij, linux-arm-kernel, Linux Kernel Mailing List, linaro-kernel On Tue, Jul 1, 2014 at 9:11 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jul 01, 2014 at 01:06:22PM +0100, Russell King - ARM Linux wrote: >> On Tue, Jul 01, 2014 at 12:55:26PM +0100, Mark Brown wrote: > >> > Even though we mostly use GPIO descriptors internally we still use >> > gpio_request_one() to request so we need to pair that with gpio_free() to >> > release the GPIO. > >> This doesn't explain the reason... I see the current GPIO code doing this: > >> void gpio_free(unsigned gpio) >> { >> gpiod_free(gpio_to_desc(gpio)); >> } >> EXPORT_SYMBOL_GPL(gpio_free); > > Huh, good point (though it still seems a bit neater to match up the > request and release functions I guess). Linus? Yeah it's a nice habit to free the GPIOs using the same interface as the one used to obtain them. Although if we are to allow the regulator framework to request GPIO through both the integer and descriptor interfaces, we certainly don't want to track that information forever. On top of that we want to limit the use of gpio_to_desc() and desc_to_gpio() in drivers as much as possible, so I'd say I prefer to see gpiod_free() used no matter where the GPIO came from. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-01 12:24 ` Alexandre Courbot 0 siblings, 0 replies; 14+ messages in thread From: Alexandre Courbot @ 2014-07-01 12:24 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 1, 2014 at 9:11 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jul 01, 2014 at 01:06:22PM +0100, Russell King - ARM Linux wrote: >> On Tue, Jul 01, 2014 at 12:55:26PM +0100, Mark Brown wrote: > >> > Even though we mostly use GPIO descriptors internally we still use >> > gpio_request_one() to request so we need to pair that with gpio_free() to >> > release the GPIO. > >> This doesn't explain the reason... I see the current GPIO code doing this: > >> void gpio_free(unsigned gpio) >> { >> gpiod_free(gpio_to_desc(gpio)); >> } >> EXPORT_SYMBOL_GPL(gpio_free); > > Huh, good point (though it still seems a bit neater to match up the > request and release functions I guess). Linus? Yeah it's a nice habit to free the GPIOs using the same interface as the one used to obtain them. Although if we are to allow the regulator framework to request GPIO through both the integer and descriptor interfaces, we certainly don't want to track that information forever. On top of that we want to limit the use of gpio_to_desc() and desc_to_gpio() in drivers as much as possible, so I'd say I prefer to see gpiod_free() used no matter where the GPIO came from. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] regulator: core: Still free GPIOs using gpio_free() 2014-07-01 12:24 ` Alexandre Courbot @ 2014-07-01 12:27 ` Mark Brown -1 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-01 12:27 UTC (permalink / raw) To: Alexandre Courbot Cc: Russell King - ARM Linux, Liam Girdwood, Linus Walleij, linux-arm-kernel, Linux Kernel Mailing List, linaro-kernel [-- Attachment #1: Type: text/plain, Size: 880 bytes --] On Tue, Jul 01, 2014 at 09:24:22PM +0900, Alexandre Courbot wrote: > On Tue, Jul 1, 2014 at 9:11 PM, Mark Brown <broonie@kernel.org> wrote: > > Huh, good point (though it still seems a bit neater to match up the > > request and release functions I guess). Linus? > Yeah it's a nice habit to free the GPIOs using the same interface as > the one used to obtain them. Although if we are to allow the regulator > framework to request GPIO through both the integer and descriptor > interfaces, we certainly don't want to track that information forever. > On top of that we want to limit the use of gpio_to_desc() and > desc_to_gpio() in drivers as much as possible, so I'd say I prefer to > see gpiod_free() used no matter where the GPIO came from. Right, but Linus said we didn't need this conversion to descriptors so I'm a bit confused as to what the issue he was raising was. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-01 12:27 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-01 12:27 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 01, 2014 at 09:24:22PM +0900, Alexandre Courbot wrote: > On Tue, Jul 1, 2014 at 9:11 PM, Mark Brown <broonie@kernel.org> wrote: > > Huh, good point (though it still seems a bit neater to match up the > > request and release functions I guess). Linus? > Yeah it's a nice habit to free the GPIOs using the same interface as > the one used to obtain them. Although if we are to allow the regulator > framework to request GPIO through both the integer and descriptor > interfaces, we certainly don't want to track that information forever. > On top of that we want to limit the use of gpio_to_desc() and > desc_to_gpio() in drivers as much as possible, so I'd say I prefer to > see gpiod_free() used no matter where the GPIO came from. Right, but Linus said we didn't need this conversion to descriptors so I'm a bit confused as to what the issue he was raising was. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140701/cf003a82/attachment.sig> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] regulator: core: Still free GPIOs using gpio_free() 2014-07-01 12:27 ` Mark Brown @ 2014-07-08 8:33 ` Linus Walleij -1 siblings, 0 replies; 14+ messages in thread From: Linus Walleij @ 2014-07-08 8:33 UTC (permalink / raw) To: Mark Brown Cc: Alexandre Courbot, Russell King - ARM Linux, Liam Girdwood, linux-arm-kernel, Linux Kernel Mailing List, linaro-kernel On Tue, Jul 1, 2014 at 2:27 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jul 01, 2014 at 09:24:22PM +0900, Alexandre Courbot wrote: >> On Tue, Jul 1, 2014 at 9:11 PM, Mark Brown <broonie@kernel.org> wrote: > >> > Huh, good point (though it still seems a bit neater to match up the >> > request and release functions I guess). Linus? > >> Yeah it's a nice habit to free the GPIOs using the same interface as >> the one used to obtain them. Although if we are to allow the regulator >> framework to request GPIO through both the integer and descriptor >> interfaces, we certainly don't want to track that information forever. > >> On top of that we want to limit the use of gpio_to_desc() and >> desc_to_gpio() in drivers as much as possible, so I'd say I prefer to >> see gpiod_free() used no matter where the GPIO came from. > > Right, but Linus said we didn't need this conversion to descriptors so > I'm a bit confused as to what the issue he was raising was. Now I'm probably confusing myself... All drivers should as far as possible use descriptors internally, doing things like gpio_to_desc() only for backward-compatibility where necessary, with the intent of eventually ridding the integer interfaces entirely. It may be an eternal project, but hey, we are atleast two of us pursuing it. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-08 8:33 ` Linus Walleij 0 siblings, 0 replies; 14+ messages in thread From: Linus Walleij @ 2014-07-08 8:33 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 1, 2014 at 2:27 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Jul 01, 2014 at 09:24:22PM +0900, Alexandre Courbot wrote: >> On Tue, Jul 1, 2014 at 9:11 PM, Mark Brown <broonie@kernel.org> wrote: > >> > Huh, good point (though it still seems a bit neater to match up the >> > request and release functions I guess). Linus? > >> Yeah it's a nice habit to free the GPIOs using the same interface as >> the one used to obtain them. Although if we are to allow the regulator >> framework to request GPIO through both the integer and descriptor >> interfaces, we certainly don't want to track that information forever. > >> On top of that we want to limit the use of gpio_to_desc() and >> desc_to_gpio() in drivers as much as possible, so I'd say I prefer to >> see gpiod_free() used no matter where the GPIO came from. > > Right, but Linus said we didn't need this conversion to descriptors so > I'm a bit confused as to what the issue he was raising was. Now I'm probably confusing myself... All drivers should as far as possible use descriptors internally, doing things like gpio_to_desc() only for backward-compatibility where necessary, with the intent of eventually ridding the integer interfaces entirely. It may be an eternal project, but hey, we are atleast two of us pursuing it. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] regulator: core: Still free GPIOs using gpio_free() 2014-07-08 8:33 ` Linus Walleij @ 2014-07-08 11:53 ` Mark Brown -1 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-08 11:53 UTC (permalink / raw) To: Linus Walleij Cc: Alexandre Courbot, Russell King - ARM Linux, Liam Girdwood, linux-arm-kernel, Linux Kernel Mailing List, linaro-kernel [-- Attachment #1: Type: text/plain, Size: 724 bytes --] On Tue, Jul 08, 2014 at 10:33:23AM +0200, Linus Walleij wrote: > On Tue, Jul 1, 2014 at 2:27 PM, Mark Brown <broonie@kernel.org> wrote: > > Right, but Linus said we didn't need this conversion to descriptors so > > I'm a bit confused as to what the issue he was raising was. > Now I'm probably confusing myself... > All drivers should as far as possible use descriptors internally, doing > things like gpio_to_desc() only for backward-compatibility where > necessary, with the intent of eventually ridding the integer interfaces > entirely. > It may be an eternal project, but hey, we are atleast two of us > pursuing it. OK, so in that case Russell's code should be fine then and we don't need to do anything - great! [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] regulator: core: Still free GPIOs using gpio_free() @ 2014-07-08 11:53 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2014-07-08 11:53 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 08, 2014 at 10:33:23AM +0200, Linus Walleij wrote: > On Tue, Jul 1, 2014 at 2:27 PM, Mark Brown <broonie@kernel.org> wrote: > > Right, but Linus said we didn't need this conversion to descriptors so > > I'm a bit confused as to what the issue he was raising was. > Now I'm probably confusing myself... > All drivers should as far as possible use descriptors internally, doing > things like gpio_to_desc() only for backward-compatibility where > necessary, with the intent of eventually ridding the integer interfaces > entirely. > It may be an eternal project, but hey, we are atleast two of us > pursuing it. OK, so in that case Russell's code should be fine then and we don't need to do anything - great! -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140708/7ce51eba/attachment.sig> ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-07-08 12:34 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-01 11:55 [PATCH] regulator: core: Still free GPIOs using gpio_free() Mark Brown 2014-07-01 11:55 ` Mark Brown 2014-07-01 12:06 ` Russell King - ARM Linux 2014-07-01 12:06 ` Russell King - ARM Linux 2014-07-01 12:11 ` Mark Brown 2014-07-01 12:11 ` Mark Brown 2014-07-01 12:24 ` Alexandre Courbot 2014-07-01 12:24 ` Alexandre Courbot 2014-07-01 12:27 ` Mark Brown 2014-07-01 12:27 ` Mark Brown 2014-07-08 8:33 ` Linus Walleij 2014-07-08 8:33 ` Linus Walleij 2014-07-08 11:53 ` Mark Brown 2014-07-08 11:53 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.