[PATCH v2] gpio: make of_get_named_gpiod_flags() private

[PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Alexandre Courbot]

of_get_named_gpiod_flags() is visible and directly usable by GPIO
consumers, but it really should not as the gpiod interface relies
on the simpler gpiod_get() to provide properly-configured GPIOs.

of_get_named_gpiod_flags() is just used internally by gpiolib to
implement gpiod_get(), and by the old of_get_named_gpio_flags()
function, therefore it makes sense to make it gpiolib-private.

As a side-effect, the unused (and unneeded) of_get_gpiod_flags()
inline function is also removed, and of_get_named_gpio_flags() is moved
from a static inline function to a regular one in gpiolib-of.c

This results in all references to gpiod_* functions in of_gpio.h being
gone, which is the way it should be since this file is part of the old
integer GPIO interface.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes since v1:
- Fixed compilation error when CONFIG_OF_GPIO is not defined
- Fixed warning due to of_gpio_flags enum not being declared
  in private gpiolib.h header

 drivers/gpio/gpiolib-of.c | 14 ++++++++++++++
 drivers/gpio/gpiolib.h    |  5 +++++
 include/linux/of_gpio.h   | 35 +++--------------------------------
 3 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index db98d3a..af7e25c 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -96,6 +96,20 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 }
 EXPORT_SYMBOL(of_get_named_gpiod_flags);
 
+int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
+			    int index, enum of_gpio_flags *flags)
+{
+	struct gpio_desc *desc;
+
+	desc = of_get_named_gpiod_flags(np, list_name, index, flags);
+
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+	else
+		return desc_to_gpio(desc);
+}
+EXPORT_SYMBOL(of_get_named_gpio_flags);
+
 /**
  * of_gpio_simple_xlate - translate gpio_spec to the GPIO number and flags
  * @gc:		pointer to the gpio_chip structure
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index cf09294..1a4103d 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -15,6 +15,8 @@
 #include <linux/err.h>
 #include <linux/device.h>
 
+enum of_gpio_flags;
+
 /**
  * struct acpi_gpio_info - ACPI GPIO specific information
  * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
@@ -46,4 +48,7 @@ acpi_get_gpiod_by_index(struct device *dev, int index,
 int gpiochip_request_own_desc(struct gpio_desc *desc, const char *label);
 void gpiochip_free_own_desc(struct gpio_desc *desc);
 
+struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
+		   const char *list_name, int index, enum of_gpio_flags *flags);
+
 #endif /* GPIOLIB_H */
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index f14123a..38fc050 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -19,7 +19,6 @@
 #include <linux/errno.h>
 #include <linux/gpio.h>
 #include <linux/of.h>
-#include <linux/gpio/consumer.h>
 
 struct device_node;
 
@@ -48,7 +47,7 @@ static inline struct of_mm_gpio_chip *to_of_mm_gpio_chip(struct gpio_chip *gc)
 	return container_of(gc, struct of_mm_gpio_chip, gc);
 }
 
-extern struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
+extern int of_get_named_gpio_flags(struct device_node *np,
 		const char *list_name, int index, enum of_gpio_flags *flags);
 
 extern int of_mm_gpiochip_add(struct device_node *np,
@@ -63,10 +62,10 @@ extern int of_gpio_simple_xlate(struct gpio_chip *gc,
 #else /* CONFIG_OF_GPIO */
 
 /* Drivers may not strictly depend on the GPIO support, so let them link. */
-static inline struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
+static inline int of_get_named_gpio_flags(struct device_node *np,
 		const char *list_name, int index, enum of_gpio_flags *flags)
 {
-	return ERR_PTR(-ENOSYS);
+	return -ENOSYS;
 }
 
 static inline int of_gpio_simple_xlate(struct gpio_chip *gc,
@@ -81,18 +80,6 @@ static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
 
 #endif /* CONFIG_OF_GPIO */
 
-static inline int of_get_named_gpio_flags(struct device_node *np,
-		const char *list_name, int index, enum of_gpio_flags *flags)
-{
-	struct gpio_desc *desc;
-	desc = of_get_named_gpiod_flags(np, list_name, index, flags);
-
-	if (IS_ERR(desc))
-		return PTR_ERR(desc);
-	else
-		return desc_to_gpio(desc);
-}
-
 /**
  * of_gpio_named_count() - Count GPIOs for a device
  * @np:		device node to count GPIOs for
@@ -129,22 +116,6 @@ static inline int of_gpio_count(struct device_node *np)
 	return of_gpio_named_count(np, "gpios");
 }
 
-/**
- * of_get_gpiod_flags() - Get a GPIO descriptor and flags to use with GPIO API
- * @np:		device node to get GPIO from
- * @index:	index of the GPIO
- * @flags:	a flags pointer to fill in
- *
- * Returns GPIO descriptor to use with Linux generic GPIO API, or a errno
- * value on the error condition. If @flags is not NULL the function also fills
- * in flags for the GPIO.
- */
-static inline struct gpio_desc *of_get_gpiod_flags(struct device_node *np,
-					int index, enum of_gpio_flags *flags)
-{
-	return of_get_named_gpiod_flags(np, "gpios", index, flags);
-}
-
 static inline int of_get_gpio_flags(struct device_node *np, int index,
 		      enum of_gpio_flags *flags)
 {
-- 
1.9.2

Re: [PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

On Sat, May 17, 2014 at 02:54:50PM +0900, Alexandre Courbot wrote:
> of_get_named_gpiod_flags() is visible and directly usable by GPIO
> consumers, but it really should not as the gpiod interface relies
> on the simpler gpiod_get() to provide properly-configured GPIOs.
> 
> of_get_named_gpiod_flags() is just used internally by gpiolib to
> implement gpiod_get(), and by the old of_get_named_gpio_flags()
> function, therefore it makes sense to make it gpiolib-private.
> 
> As a side-effect, the unused (and unneeded) of_get_gpiod_flags()
> inline function is also removed, and of_get_named_gpio_flags() is moved
> from a static inline function to a regular one in gpiolib-of.c
> 
> This results in all references to gpiod_* functions in of_gpio.h being
> gone, which is the way it should be since this file is part of the old
> integer GPIO interface.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> Changes since v1:
> - Fixed compilation error when CONFIG_OF_GPIO is not defined
> - Fixed warning due to of_gpio_flags enum not being declared
>   in private gpiolib.h header
> 
>  drivers/gpio/gpiolib-of.c | 14 ++++++++++++++
>  drivers/gpio/gpiolib.h    |  5 +++++
>  include/linux/of_gpio.h   | 35 +++--------------------------------
>  3 files changed, 22 insertions(+), 32 deletions(-)

There are cases where GPIOs need to be obtained from device tree nodes
without a corresponding struct device. Do you have any ideas on how to
support such use-cases?

There's currently no API to do that except of_get_named_gpio_flags().
But that doesn't handled flags automatically in a way that gpiod_get()
does.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Alexandre Courbot]

On Mon, May 19, 2014 at 10:28 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Sat, May 17, 2014 at 02:54:50PM +0900, Alexandre Courbot wrote:
>> of_get_named_gpiod_flags() is visible and directly usable by GPIO
>> consumers, but it really should not as the gpiod interface relies
>> on the simpler gpiod_get() to provide properly-configured GPIOs.
>>
>> of_get_named_gpiod_flags() is just used internally by gpiolib to
>> implement gpiod_get(), and by the old of_get_named_gpio_flags()
>> function, therefore it makes sense to make it gpiolib-private.
>>
>> As a side-effect, the unused (and unneeded) of_get_gpiod_flags()
>> inline function is also removed, and of_get_named_gpio_flags() is moved
>> from a static inline function to a regular one in gpiolib-of.c
>>
>> This results in all references to gpiod_* functions in of_gpio.h being
>> gone, which is the way it should be since this file is part of the old
>> integer GPIO interface.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> Changes since v1:
>> - Fixed compilation error when CONFIG_OF_GPIO is not defined
>> - Fixed warning due to of_gpio_flags enum not being declared
>>   in private gpiolib.h header
>>
>>  drivers/gpio/gpiolib-of.c | 14 ++++++++++++++
>>  drivers/gpio/gpiolib.h    |  5 +++++
>>  include/linux/of_gpio.h   | 35 +++--------------------------------
>>  3 files changed, 22 insertions(+), 32 deletions(-)
>
> There are cases where GPIOs need to be obtained from device tree nodes
> without a corresponding struct device. Do you have any ideas on how to
> support such use-cases?
>
> There's currently no API to do that except of_get_named_gpio_flags().
> But that doesn't handled flags automatically in a way that gpiod_get()
> does.

Yeah, there is probably some legacy stuff that will need a few more
functions to be handled using gpiod.

As much as possible I'd like to avoid provider-specific functions, but
in the worst case we can imagine making another function available for
that:

struct gpio_desc *of_gpiod_get_index(struct device_node *np, const
char *con_id, unsigned int idx);

that will handle flags properly and everything. Would that do the trick?

Re: [PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2682 bytes --]

On Tue, May 20, 2014 at 12:07:40AM +0900, Alexandre Courbot wrote:
> On Mon, May 19, 2014 at 10:28 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Sat, May 17, 2014 at 02:54:50PM +0900, Alexandre Courbot wrote:
> >> of_get_named_gpiod_flags() is visible and directly usable by GPIO
> >> consumers, but it really should not as the gpiod interface relies
> >> on the simpler gpiod_get() to provide properly-configured GPIOs.
> >>
> >> of_get_named_gpiod_flags() is just used internally by gpiolib to
> >> implement gpiod_get(), and by the old of_get_named_gpio_flags()
> >> function, therefore it makes sense to make it gpiolib-private.
> >>
> >> As a side-effect, the unused (and unneeded) of_get_gpiod_flags()
> >> inline function is also removed, and of_get_named_gpio_flags() is moved
> >> from a static inline function to a regular one in gpiolib-of.c
> >>
> >> This results in all references to gpiod_* functions in of_gpio.h being
> >> gone, which is the way it should be since this file is part of the old
> >> integer GPIO interface.
> >>
> >> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >> ---
> >> Changes since v1:
> >> - Fixed compilation error when CONFIG_OF_GPIO is not defined
> >> - Fixed warning due to of_gpio_flags enum not being declared
> >>   in private gpiolib.h header
> >>
> >>  drivers/gpio/gpiolib-of.c | 14 ++++++++++++++
> >>  drivers/gpio/gpiolib.h    |  5 +++++
> >>  include/linux/of_gpio.h   | 35 +++--------------------------------
> >>  3 files changed, 22 insertions(+), 32 deletions(-)
> >
> > There are cases where GPIOs need to be obtained from device tree nodes
> > without a corresponding struct device. Do you have any ideas on how to
> > support such use-cases?
> >
> > There's currently no API to do that except of_get_named_gpio_flags().
> > But that doesn't handled flags automatically in a way that gpiod_get()
> > does.
> 
> Yeah, there is probably some legacy stuff that will need a few more
> functions to be handled using gpiod.
> 
> As much as possible I'd like to avoid provider-specific functions, but
> in the worst case we can imagine making another function available for
> that:
> 
> struct gpio_desc *of_gpiod_get_index(struct device_node *np, const
> char *con_id, unsigned int idx);
> 
> that will handle flags properly and everything. Would that do the trick?

Yes, I think it would. Perhaps with a of_gpiod_get() wrapper that sets
idx = 0. I'm aware of patches in the works that will require this, but
until they're posted I don't think we'll need this. We can revisit at
that time. Just wanted to mention this as a heads-up.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Linus Walleij]

On Sat, May 17, 2014 at 7:54 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:

> of_get_named_gpiod_flags() is visible and directly usable by GPIO
> consumers, but it really should not as the gpiod interface relies
> on the simpler gpiod_get() to provide properly-configured GPIOs.
>
> of_get_named_gpiod_flags() is just used internally by gpiolib to
> implement gpiod_get(), and by the old of_get_named_gpio_flags()
> function, therefore it makes sense to make it gpiolib-private.
>
> As a side-effect, the unused (and unneeded) of_get_gpiod_flags()
> inline function is also removed, and of_get_named_gpio_flags() is moved
> from a static inline function to a regular one in gpiolib-of.c
>
> This results in all references to gpiod_* functions in of_gpio.h being
> gone, which is the way it should be since this file is part of the old
> integer GPIO interface.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> Changes since v1:
> - Fixed compilation error when CONFIG_OF_GPIO is not defined
> - Fixed warning due to of_gpio_flags enum not being declared
>   in private gpiolib.h header

Applied this v2 version and pushed for testing, let's hope it flies
properly this time :-D

Yours,
Linus Walleij

Re: [PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Alexandre Courbot]

On Wed, May 21, 2014 at 6:16 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, May 17, 2014 at 7:54 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
>
>> of_get_named_gpiod_flags() is visible and directly usable by GPIO
>> consumers, but it really should not as the gpiod interface relies
>> on the simpler gpiod_get() to provide properly-configured GPIOs.
>>
>> of_get_named_gpiod_flags() is just used internally by gpiolib to
>> implement gpiod_get(), and by the old of_get_named_gpio_flags()
>> function, therefore it makes sense to make it gpiolib-private.
>>
>> As a side-effect, the unused (and unneeded) of_get_gpiod_flags()
>> inline function is also removed, and of_get_named_gpio_flags() is moved
>> from a static inline function to a regular one in gpiolib-of.c
>>
>> This results in all references to gpiod_* functions in of_gpio.h being
>> gone, which is the way it should be since this file is part of the old
>> integer GPIO interface.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> Changes since v1:
>> - Fixed compilation error when CONFIG_OF_GPIO is not defined
>> - Fixed warning due to of_gpio_flags enum not being declared
>>   in private gpiolib.h header
>
> Applied this v2 version and pushed for testing, let's hope it flies
> properly this time :-D

Looks like the world did not break this time. However I noticed that
the commit in your tree (and -next) included the changes since v1 in
its log. Is it intended?

Re: [PATCH v2] gpio: make of_get_named_gpiod_flags() private

[Linus Walleij]

On Fri, May 23, 2014 at 5:34 AM, Alexandre Courbot <gnurou@gmail.com> wrote:

> Looks like the world did not break this time. However I noticed that
> the commit in your tree (and -next) included the changes since v1 in
> its log. Is it intended?

Yeah I copy that into the changelog sometimes. If I need that info
it's easier to have it in the commit message than looking up the mail.
Grant had this preference too.

Yours,
Linus Walleij