From: Mark Brown <[email protected]>
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 <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
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
On Tue, Jul 01, 2014 at 12:55:26PM +0100, Mark Brown wrote:
> From: Mark Brown <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Mark Brown <[email protected]>
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.
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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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!