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 {
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);
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?
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.
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.
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
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!
linaro-kernel@lists.linaro.org
-
Alexandre Courbot -
Linus Walleij -
Mark Brown -
Russell King - ARM Linux