[PATCH] regulator: core: Still free GPIOs using gpio_free()

[PATCH] regulator: core: Still free GPIOs using gpio_free()

[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

[PATCH] regulator: core: Still free GPIOs using gpio_free()

[Russell King - ARM Linux]

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.

[PATCH] regulator: core: Still free GPIOs using gpio_free()

[Mark Brown]

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>

[PATCH] regulator: core: Still free GPIOs using gpio_free()

[Alexandre Courbot]

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.

[PATCH] regulator: core: Still free GPIOs using gpio_free()

[Mark Brown]

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>

[PATCH] regulator: core: Still free GPIOs using gpio_free()

[Linus Walleij]

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

[PATCH] regulator: core: Still free GPIOs using gpio_free()

[Mark Brown]

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>