gpiolib-legacy: Request gpio without touching direction

Message ID 20180524130117.2io3vjzj6x7nncfd@lnxeinarv.se.axis.com
State New
Headers show
Series
  • gpiolib-legacy: Request gpio without touching direction
Related show

Commit Message

Einar Vading May 24, 2018, 1:01 p.m.
Adds function to request a gpio without touching the direction of the
gpio upon export.

This is useful if some user space application know what state the gpio
should be in and it is important that this state is kept during reboot.

Signed-off-by: Einar Vading <einarv@axis.com>
---
 drivers/gpio/gpiolib-legacy.c | 50 +++++++++++++++++++++++++----------
 include/asm-generic/gpio.h    |  2 ++
 include/linux/gpio.h          |  6 +++++
 3 files changed, 44 insertions(+), 14 deletions(-)

Comments

Linus Walleij May 25, 2018, 11:43 a.m. | #1
On Thu, May 24, 2018 at 3:01 PM, Einar Vading <einar.vading@axis.com> wrote:

Hi Einar,

thanks for your patch!

Unfortunately it can not be applied.

> Adds function to request a gpio without touching the direction of the
> gpio upon export.
>
> This is useful if some user space application know what state the gpio
> should be in and it is important that this state is kept during reboot.
>
> Signed-off-by: Einar Vading <einarv@axis.com>

The sysfs ABI is deprecated since two years:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/Documentation/ABI/obsolete/sysfs-gpio?id=fe95046e960b4b76e73dc1486955d93f47276134

This means we do not extend or add features to the sysfs ABI.

The new chardev ABI should be used for all new features to
userspace. Here are the example tools for GPIO descriptor
access:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/gpio

What is even more featured for commandline and apps is
libgpiod:
https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/

If you want to add a new flag for not touching the direction
of the GPIO when requesting, by patching
include/uapi/linux/gpio.h then I'm all game for that!

We currently have these:
/* Linerequest flags */
#define GPIOHANDLE_REQUEST_INPUT        (1UL << 0)
#define GPIOHANDLE_REQUEST_OUTPUT       (1UL << 1)
#define GPIOHANDLE_REQUEST_ACTIVE_LOW   (1UL << 2)
#define GPIOHANDLE_REQUEST_OPEN_DRAIN   (1UL << 3)
#define GPIOHANDLE_REQUEST_OPEN_SOURCE  (1UL << 4)

We could definately add an "ASIS" flag if you can find a way to
implement it nicely!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Einar Vading May 25, 2018, 1:02 p.m. | #2
On Fri, May 25, 2018 at 01:43:59PM +0200, Linus Walleij wrote:
> On Thu, May 24, 2018 at 3:01 PM, Einar Vading <einar.vading@axis.com> wrote:
> 
> Hi Einar,

Hi,

> 
> thanks for your patch!

thanks for taking the time to look at it.

> 
> Unfortunately it can not be applied.
> 
> > Adds function to request a gpio without touching the direction of the
> > gpio upon export.
> >
> > This is useful if some user space application know what state the gpio
> > should be in and it is important that this state is kept during reboot.
> >
> > Signed-off-by: Einar Vading <einarv@axis.com>
> 
> The sysfs ABI is deprecated since two years:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/Documentation/ABI/obsolete/sysfs-gpio?id=fe95046e960b4b76e73dc1486955d93f47276134
> 
> This means we do not extend or add features to the sysfs ABI.o

Ok, well it should be relatively easy to maintain this patch out of tree then =)

> 
> The new chardev ABI should be used for all new features to
> userspace. Here are the example tools for GPIO descriptor
> access:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/gpio
> 
> What is even more featured for commandline and apps is
> libgpiod:
> https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/

I know, and I'm dying to get to port our stuff over to use it. But time,
releases and business are in my way ATM.

> 
> If you want to add a new flag for not touching the direction
> of the GPIO when requesting, by patching
> include/uapi/linux/gpio.h then I'm all game for that!
> 
> We currently have these:
> /* Linerequest flags */
> #define GPIOHANDLE_REQUEST_INPUT        (1UL << 0)
> #define GPIOHANDLE_REQUEST_OUTPUT       (1UL << 1)
> #define GPIOHANDLE_REQUEST_ACTIVE_LOW   (1UL << 2)
> #define GPIOHANDLE_REQUEST_OPEN_DRAIN   (1UL << 3)
> #define GPIOHANDLE_REQUEST_OPEN_SOURCE  (1UL << 4)
> 
> We could definately add an "ASIS" flag if you can find a way to
> implement it nicely!

Sounds good, I'll look into it when I get that far.

> 
> Yours,
> Linus Walleij

// Einar
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/gpio/gpiolib-legacy.c b/drivers/gpio/gpiolib-legacy.c
index 8b830996fe..521c8099bf 100644
--- a/drivers/gpio/gpiolib-legacy.c
+++ b/drivers/gpio/gpiolib-legacy.c
@@ -11,13 +11,8 @@  void gpio_free(unsigned gpio)
 }
 EXPORT_SYMBOL_GPL(gpio_free);
 
-/**
- * gpio_request_one - request a single GPIO with initial configuration
- * @gpio:	the GPIO number
- * @flags:	GPIO configuration as specified by GPIOF_*
- * @label:	a literal description string of this GPIO
- */
-int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
+static int _gpio_request_one(unsigned gpio, unsigned long flags,
+			     const char *label, bool set_dir)
 {
 	struct gpio_desc *desc;
 	int err;
@@ -41,14 +36,16 @@  int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 	if (flags & GPIOF_ACTIVE_LOW)
 		set_bit(FLAG_ACTIVE_LOW, &desc->flags);
 
-	if (flags & GPIOF_DIR_IN)
-		err = gpiod_direction_input(desc);
-	else
-		err = gpiod_direction_output_raw(desc,
-				(flags & GPIOF_INIT_HIGH) ? 1 : 0);
+	if (set_dir) {
+		if (flags & GPIOF_DIR_IN)
+			err = gpiod_direction_input(desc);
+		else
+			err = gpiod_direction_output_raw(desc,
+					(flags & GPIOF_INIT_HIGH) ? 1 : 0);
 
-	if (err)
-		goto free_gpio;
+		if (err)
+			goto free_gpio;
+	}
 
 	if (flags & GPIOF_EXPORT) {
 		err = gpiod_export(desc, flags & GPIOF_EXPORT_CHANGEABLE);
@@ -62,8 +59,33 @@  int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 	gpiod_free(desc);
 	return err;
 }
+
+/**
+ * gpio_request_one - request a single GPIO with initial configuration
+ * @gpio:	the GPIO number
+ * @flags:	GPIO configuration as specified by GPIOF_*
+ * @label:	a literal description string of this GPIO
+ */
+int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
+{
+	return _gpio_request_one(gpio, flags, label, true);
+}
 EXPORT_SYMBOL_GPL(gpio_request_one);
 
+/**
+ * gpio_request_one_keepdir - request a single GPIO with initial configuration
+ *			      but without setting direction on export.
+ * @gpio:	the GPIO number
+ * @flags:	GPIO configuration as specified by GPIOF_*
+ * @label:	a literal description string of this GPIO
+ */
+int gpio_request_one_keepdir(unsigned gpio, unsigned long flags,
+			     const char *label)
+{
+	return _gpio_request_one(gpio, flags, label, false);
+}
+EXPORT_SYMBOL_GPL(gpio_request_one_keepdir);
+
 int gpio_request(unsigned gpio, const char *label)
 {
 	struct gpio_desc *desc = gpio_to_desc(gpio);
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 8b74cff7c0..9e7d64d04e 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -115,6 +115,8 @@  static inline int __gpio_to_irq(unsigned gpio)
 }
 
 extern int gpio_request_one(unsigned gpio, unsigned long flags, const char *label);
+extern int gpio_request_one_keepdir(unsigned gpio, unsigned long flags,
+		const char *label);
 extern int gpio_request_array(const struct gpio *array, size_t num);
 extern void gpio_free_array(const struct gpio *array, size_t num);
 
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 8ef7fc0ce0..adca31cd88 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -113,6 +113,12 @@  static inline int gpio_request_one(unsigned gpio,
 	return -ENOSYS;
 }
 
+static inline int gpio_request_one_keepdir(unsigned gpio,
+		unsigned long flags, const char *label)
+{
+	return -ENOSYS;
+}
+
 static inline int gpio_request_array(const struct gpio *array, size_t num)
 {
 	return -ENOSYS;