* [PATCH] gpio: Add helpers to determin direction of gpiods
@ 2017-12-10 0:23 Linus Walleij
2017-12-12 22:42 ` Wolfram Sang
2017-12-13 10:15 ` Geert Uytterhoeven
0 siblings, 2 replies; 5+ messages in thread
From: Linus Walleij @ 2017-12-10 0:23 UTC (permalink / raw)
To: linux-gpio; +Cc: Linus Walleij, Wolfram Sang
The gpiod_get_direction() returns 1 for input and 0 for output
but it's pretty hard to remember which one is which and generally
unintuitive and messy to provide #defines so let's simply add
two static inlines to do the job.
Cc: Wolfram Sang <wsa@the-dreams.de>
Suggested-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Documentation/gpio/consumer.txt | 14 +++++++++++---
include/linux/gpio/consumer.h | 25 +++++++++++++++++++++++++
2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt
index 63e1bd1d88e3..c4b8e3d8c29f 100644
--- a/Documentation/gpio/consumer.txt
+++ b/Documentation/gpio/consumer.txt
@@ -162,8 +162,8 @@ The device-managed variants are, unsurprisingly:
Using GPIOs
===========
-Setting Direction
------------------
+Setting and Getting Direction
+-----------------------------
The first thing a driver must do with a GPIO is setting its direction. If no
direction-setting flags have been given to gpiod_get*(), this is done by
invoking one of the gpiod_direction_*() functions:
@@ -184,7 +184,15 @@ A driver can also query the current direction of a GPIO:
int gpiod_get_direction(const struct gpio_desc *desc)
-This function will return either GPIOF_DIR_IN or GPIOF_DIR_OUT.
+This function will return either 1 (input) or 0 (output). There are also
+these convenience helpers:
+
+ bool gpiod_is_input(const struct gpio_desc *desc)
+ bool gpiod_is_output(const struct gpio_desc *desc)
+
+These will not provide the same level of error fallbacks: if they fail to
+obtain the direction, they will print an error and report as input since this
+is usually safest.
Be aware that there is no default direction for GPIOs. Therefore, **using a GPIO
without setting its direction first is illegal and will result in undefined
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 7447d85dbe2f..4e5ee3ec1913 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -487,6 +487,31 @@ struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev,
#endif /* CONFIG_GPIOLIB */
+/*
+ * Helpers that quickly tell whether a line is input or output.
+ */
+static inline bool gpiod_is_input(struct gpio_desc *desc)
+{
+ int ret = gpiod_get_direction(desc);
+ if (ret < 0) {
+ pr_err("GPIO: failed to get direction\n");
+ /* It is usually safest to assume we are input */
+ return true;
+ }
+ return !!ret;
+}
+
+static inline bool gpiod_is_output(struct gpio_desc *desc)
+{
+ int ret = gpiod_get_direction(desc);
+ if (ret < 0) {
+ pr_err("GPIO: failed to get direction\n");
+ /* It is usually safest to assume we are input */
+ return false;
+ }
+ return !ret;
+}
+
static inline
struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
const char *con_id,
--
2.14.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: Add helpers to determin direction of gpiods
2017-12-10 0:23 [PATCH] gpio: Add helpers to determin direction of gpiods Linus Walleij
@ 2017-12-12 22:42 ` Wolfram Sang
2017-12-13 14:54 ` Linus Walleij
2017-12-13 10:15 ` Geert Uytterhoeven
1 sibling, 1 reply; 5+ messages in thread
From: Wolfram Sang @ 2017-12-12 22:42 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
Hey Linus,
thanks for doing this.
On Sun, Dec 10, 2017 at 01:23:40AM +0100, Linus Walleij wrote:
> The gpiod_get_direction() returns 1 for input and 0 for output
> but it's pretty hard to remember which one is which and generally
> unintuitive and messy to provide #defines so let's simply add
> two static inlines to do the job.
>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Suggested-by: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Documentation/gpio/consumer.txt | 14 +++++++++++---
> include/linux/gpio/consumer.h | 25 +++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 3 deletions(-)
You need to fix the kerneldoc for 'gpiod_get_direction", too. It still
mentions GPIOF_DIR_*.
> +static inline bool gpiod_is_input(struct gpio_desc *desc)
> +{
> + int ret = gpiod_get_direction(desc);
> + if (ret < 0) {
> + pr_err("GPIO: failed to get direction\n");
Is that really helpful for the user if we don't say which GPIO failed?
> + /* It is usually safest to assume we are input */
> + return true;
> + }
> + return !!ret;
> +}
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: Add helpers to determin direction of gpiods
2017-12-10 0:23 [PATCH] gpio: Add helpers to determin direction of gpiods Linus Walleij
2017-12-12 22:42 ` Wolfram Sang
@ 2017-12-13 10:15 ` Geert Uytterhoeven
1 sibling, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2017-12-13 10:15 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, Wolfram Sang
Hi Linus,
On Sun, Dec 10, 2017 at 1:23 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> The gpiod_get_direction() returns 1 for input and 0 for output
> but it's pretty hard to remember which one is which and generally
> unintuitive and messy to provide #defines so let's simply add
> two static inlines to do the job.
>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Suggested-by: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Documentation/gpio/consumer.txt | 14 +++++++++++---
> include/linux/gpio/consumer.h | 25 +++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt
> index 63e1bd1d88e3..c4b8e3d8c29f 100644
> --- a/Documentation/gpio/consumer.txt
> +++ b/Documentation/gpio/consumer.txt
> @@ -184,7 +184,15 @@ A driver can also query the current direction of a GPIO:
>
> int gpiod_get_direction(const struct gpio_desc *desc)
>
> -This function will return either GPIOF_DIR_IN or GPIOF_DIR_OUT.
> +This function will return either 1 (input) or 0 (output). There are also
> +these convenience helpers:
> +
> + bool gpiod_is_input(const struct gpio_desc *desc)
> + bool gpiod_is_output(const struct gpio_desc *desc)
> +
> +These will not provide the same level of error fallbacks: if they fail to
> +obtain the direction, they will print an error and report as input since this
> +is usually safest.
Leaving a pin configured as input is indeed the safest, from a hardware
point of view.
But I don't know if reporting to software that it is input is safe ;-)
Can this actually fail?
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -487,6 +487,31 @@ struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev,
>
> #endif /* CONFIG_GPIOLIB */
>
> +/*
> + * Helpers that quickly tell whether a line is input or output.
> + */
> +static inline bool gpiod_is_input(struct gpio_desc *desc)
> +{
> + int ret = gpiod_get_direction(desc);
> + if (ret < 0) {
> + pr_err("GPIO: failed to get direction\n");
Do we really want a copy of this string in every module using this function?
If you want to keep the error message, the function should be moved to
the GPIO code, I think.
> + /* It is usually safest to assume we are input */
> + return true;
> + }
> + return !!ret;
> +}
> +
> +static inline bool gpiod_is_output(struct gpio_desc *desc)
> +{
> + int ret = gpiod_get_direction(desc);
> + if (ret < 0) {
> + pr_err("GPIO: failed to get direction\n");
Likewise.
> + /* It is usually safest to assume we are input */
> + return false;
> + }
> + return !ret;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: Add helpers to determin direction of gpiods
2017-12-12 22:42 ` Wolfram Sang
@ 2017-12-13 14:54 ` Linus Walleij
2017-12-13 15:03 ` Wolfram Sang
0 siblings, 1 reply; 5+ messages in thread
From: Linus Walleij @ 2017-12-13 14:54 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-gpio, Linux-Renesas
On Tue, Dec 12, 2017 at 11:42 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> Documentation/gpio/consumer.txt | 14 +++++++++++---
>
> You need to fix the kerneldoc for 'gpiod_get_direction", too. It still
> mentions GPIOF_DIR_*.
More places? I can't find them!
I thought the patch removed that here:
-This function will return either GPIOF_DIR_IN or GPIOF_DIR_OUT.
+This function will return either 1 (input) or 0 (output). There are also
+these convenience helpers:
I can only find it in Documentation/gpio/gpio-legacy.txt which is
fine.
>> +static inline bool gpiod_is_input(struct gpio_desc *desc)
>> +{
>> + int ret = gpiod_get_direction(desc);
>> + if (ret < 0) {
>> + pr_err("GPIO: failed to get direction\n");
>
> Is that really helpful for the user if we don't say which GPIO failed?
I would have to add yet more APIs to do that.
Hm.
Together with Geert's comments I start to lean towards actually
just creating two new #defines in <linux/consumer.h>.
It has the upside that we can also check the return value
for errors.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: Add helpers to determin direction of gpiods
2017-12-13 14:54 ` Linus Walleij
@ 2017-12-13 15:03 ` Wolfram Sang
0 siblings, 0 replies; 5+ messages in thread
From: Wolfram Sang @ 2017-12-13 15:03 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-gpio, Linux-Renesas
[-- Attachment #1: Type: text/plain, Size: 505 bytes --]
> > You need to fix the kerneldoc for 'gpiod_get_direction", too. It still
> > mentions GPIOF_DIR_*.
>
> More places? I can't find them!
drivers/gpio/gpiolib.c
Line 198 in both 4.15-rc3 and linux-next.
Or as I said, the kerneldoc of gpiod_get_direction ;)
> Together with Geert's comments I start to lean towards actually
> just creating two new #defines in <linux/consumer.h>.
>
> It has the upside that we can also check the return value
> for errors.
Sounds good, in deed.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-13 15:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-10 0:23 [PATCH] gpio: Add helpers to determin direction of gpiods Linus Walleij
2017-12-12 22:42 ` Wolfram Sang
2017-12-13 14:54 ` Linus Walleij
2017-12-13 15:03 ` Wolfram Sang
2017-12-13 10:15 ` Geert Uytterhoeven
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.