mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding
@ 2024-08-09 14:23 Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 01/11] gpio: make gpio.h header self-contained Ahmad Fatoum
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:23 UTC (permalink / raw)
  To: barebox

So far, GPIO bias configuration was done exclusively by pinctrl drivers.
All barebox pinctrl drivers work by consuming a device tree node with
a binding that differs from driver to driver and then applying the
configuration described within.

Neither GPIO or pinctrl node have any insight on what in particular is
being configured.

This is problematic when wanting to support following device tree
binding, which is so far being ignored:

  gpios = <&gpioe 7 (GPIO_ACTIVE_LOW | GPIO_PULL_DOWN)>;

This series enables support for this binding for gpio-keys on STM32
platforms. More support will follow in future when providers are
extended to support the gpio_ops::set_config operation and consumers are
switched to use the GPIO descriptor API.

Ahmad Fatoum (11):
  gpio: make gpio.h header self-contained
  gpiolib: permit GPIO drivers to implement struct gpio_ops::set_config
  pinctrl: stm32: implement generic struct gpio_ops::set_config
  gpiolib: turn request/active_low into bit flags
  gpiolib: don't use GPIO number API in of_hog_gpio
  gpiolib: store all OF flags into GPIO descriptor
  gpiolib: add support for OF GPIO configuration binding
  gpiolib: use dev_gpiod_get_index device node argument throughout
  gpiolib: export function to get struct gpio_desc from index
  input: gpio_keys: switch to GPIO descriptor API
  input: gpio-keys: request with label in DT if available

 drivers/gpio/Kconfig            |  14 ++
 drivers/gpio/gpiolib.c          | 254 +++++++++++++++++++++++++-------
 drivers/input/Kconfig           |   2 +-
 drivers/input/gpio_keys.c       |  69 +++++----
 drivers/of/Kconfig              |  15 ++
 drivers/pinctrl/Kconfig         |   1 +
 drivers/pinctrl/pinctrl-stm32.c |  41 ++++++
 include/gpio.h                  |   4 +
 include/linux/gpio/consumer.h   |  19 +++
 include/of_gpio.h               |   6 +
 10 files changed, 337 insertions(+), 88 deletions(-)

-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/11] gpio: make gpio.h header self-contained
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
@ 2024-08-09 14:23 ` Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 02/11] gpiolib: permit GPIO drivers to implement struct gpio_ops::set_config Ahmad Fatoum
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:23 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The header uses WARN_ON and struct device, but defines neither.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/gpio.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/gpio.h b/include/gpio.h
index a56aba050a03..cc56c91c3ee2 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -7,6 +7,7 @@
 #include <linux/list.h>
 #include <linux/iopoll.h>
 #include <linux/bitops.h>
+#include <linux/bug.h>
 
 #ifdef CONFIG_GENERIC_GPIO
 void gpio_set_value(unsigned gpio, int value);
@@ -31,6 +32,8 @@ static inline int gpio_direction_input(unsigned gpio)
 }
 #endif
 
+struct device;
+
 #ifdef CONFIG_GPIOLIB
 void gpio_set_active(unsigned gpio, bool state);
 int gpio_is_active(unsigned gpio);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 02/11] gpiolib: permit GPIO drivers to implement struct gpio_ops::set_config
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 01/11] gpio: make gpio.h header self-contained Ahmad Fatoum
@ 2024-08-09 14:23 ` Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 03/11] pinctrl: stm32: implement generic " Ahmad Fatoum
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:23 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

So far, GPIO bias configuration was done exclusively by pinctrl drivers.
All barebox pinctrl drivers work by consuming a device tree node with
a binding that differs from driver to driver and then applying the
configuration described within.

Neither GPIO or pinctrl node have any insight on what in particular is
being configured.

This is problematic when wanting to support following device tree
binding, which is so far being ignored:

  gpios = <&gpioe 7 (GPIO_ACTIVE_LOW | GPIO_PULL_DOWN)>;

In preparation for enabling support, let's give GPIO drivers a way to
set configuration. This will be implemented by the STM32 pinctrl/gpio
driver in a follow-up commit.

The new functionality will be selected by a new OF_GPIO_PINCONF option
that is going to be introduced later.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/Kconfig          | 14 ++++++++++++++
 drivers/gpio/gpiolib.c        | 28 ++++++++++++++++++++++++++++
 include/gpio.h                |  1 +
 include/linux/gpio/consumer.h |  9 +++++++++
 4 files changed, 52 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d9bb68e06b25..76b11d93aa64 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -10,6 +10,20 @@ menu "GPIO"
 config GPIO_GENERIC
 	bool
 
+config HAVE_GPIO_PINCONF
+	bool
+	help
+	  This is selected by drivers who implement struct gpio_ops::set_config
+
+config GPIO_PINCONF
+	bool "support for configuring GPIO lines directly" if COMPILE_TEST
+	help
+	  This option enables compilation and use of the optional
+	  struct gpio_ops::set_config callback. Drivers that export
+	  it allow configuring bias and other configuration directly
+	  on the GPIO line without having to interact directly with
+	  the pinctrl subsystem.
+
 config GPIO_DIGIC
 	bool "GPIO support for Canon DIGIC"
 	depends on ARCH_DIGIC || COMPILE_TEST
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f34dce0e98c6..e8d788d4d791 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -238,6 +238,34 @@ void gpiod_put_array(struct gpio_descs *descs)
 }
 EXPORT_SYMBOL_GPL(gpiod_put_array);
 
+static int gpio_do_set_config(struct gpio_chip *gc, unsigned int offset,
+			      unsigned long config)
+{
+	if (!IS_ENABLED(CONFIG_GPIO_PINCONF))
+		return -ENOSYS;
+	if (!gc->ops->set_config)
+		return -ENOTSUPP;
+
+	return gc->ops->set_config(gc, offset, config);
+}
+
+/**
+ * gpiod_set_config - sets @config for a GPIO
+ * @desc: descriptor of the GPIO for which to set the configuration
+ * @config: Same packed config format as generic pinconf
+ *
+ * Returns:
+ * 0 on success, %-ENOTSUPP if the controller doesn't support setting the
+ * configuration.
+ */
+int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
+{
+	VALIDATE_DESC(desc);
+
+	return gpio_do_set_config(desc->chip, gpiodesc_chip_offset(desc), config);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_config);
+
 /**
  * gpiod_set_raw_value() - assign a gpio's raw value
  * @desc: gpio whose value will be assigned
diff --git a/include/gpio.h b/include/gpio.h
index cc56c91c3ee2..beda10efde08 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -195,6 +195,7 @@ struct gpio_ops {
 	int (*get_direction)(struct gpio_chip *chip, unsigned offset);
 	int (*get)(struct gpio_chip *chip, unsigned offset);
 	void (*set)(struct gpio_chip *chip, unsigned offset, int value);
+	int (*set_config)(struct gpio_chip *chip, unsigned offset, unsigned long config);
 
 #if defined(CONFIG_OF_GPIO)
 	/*
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index e04f516b3155..64366a6f3302 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -69,6 +69,8 @@ static inline struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 
 #ifdef CONFIG_GPIOLIB
 
+int gpiod_set_config(struct gpio_desc *desc, unsigned long config);
+
 int gpiod_direction_input(struct gpio_desc *desc);
 
 int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
@@ -97,6 +99,13 @@ int gpiod_set_array_value(unsigned int array_size,
 
 #else
 
+static inline int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return 0;
+}
+
 static inline int gpiod_direction_input(struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 03/11] pinctrl: stm32: implement generic struct gpio_ops::set_config
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 01/11] gpio: make gpio.h header self-contained Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 02/11] gpiolib: permit GPIO drivers to implement struct gpio_ops::set_config Ahmad Fatoum
@ 2024-08-09 14:23 ` Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 04/11] gpiolib: turn request/active_low into bit flags Ahmad Fatoum
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:23 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

To enable use with the newly added gpiod_set_config API,
implement the operation for the STM32.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pinctrl/Kconfig         |  1 +
 drivers/pinctrl/pinctrl-stm32.c | 41 +++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 2ff99a39c877..ee15acdcdb35 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -103,6 +103,7 @@ config PINCTRL_VF610
 config PINCTRL_STM32
 	bool "STM32 pinctrl support" if  COMPILE_TEST
 	default y if ARCH_STM32
+	select HAVE_GPIO_PINCONF
 	help
 	  Pinmux and GPIO controller found on STM32 family
 endif
diff --git a/drivers/pinctrl/pinctrl-stm32.c b/drivers/pinctrl/pinctrl-stm32.c
index 4a4b03ac0ea7..e2fd12117095 100644
--- a/drivers/pinctrl/pinctrl-stm32.c
+++ b/drivers/pinctrl/pinctrl-stm32.c
@@ -15,6 +15,7 @@
 #include <hwspinlock.h>
 #include <malloc.h>
 #include <linux/clk.h>
+#include <linux/pinctrl/pinconf-generic.h>
 #include <soc/stm32/gpio.h>
 
 #define STM32_PIN_NO(x) ((x) << 8)
@@ -258,12 +259,52 @@ static int stm32_gpio_direction_output(struct gpio_chip *chip,
 	return 0;
 }
 
+static int stm32_gpio_set_config(struct gpio_chip *chip,
+				 unsigned int gpio, unsigned long config)
+{
+	struct stm32_gpio_bank *bank = to_stm32_gpio_bank(chip);
+	enum pin_config_param param;
+	u32 arg;
+
+	param = pinconf_to_config_param(config);
+	arg = pinconf_to_config_argument(config);
+
+	switch (param) {
+	case PIN_CONFIG_DRIVE_PUSH_PULL:
+		__stm32_pmx_set_output_type(bank->base, gpio, STM32_PIN_OUT_PUSHPULL);
+		break;
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		__stm32_pmx_set_output_type(bank->base, gpio, STM32_PIN_OUT_OPENDRAIN);
+		break;
+	case PIN_CONFIG_SLEW_RATE:
+		__stm32_pmx_set_speed(bank->base, gpio, arg);
+		break;
+	case PIN_CONFIG_BIAS_DISABLE:
+		__stm32_pmx_set_bias(bank->base, gpio, STM32_PIN_NO_BIAS);
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		__stm32_pmx_set_bias(bank->base, gpio, STM32_PIN_PULL_UP);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		__stm32_pmx_set_bias(bank->base, gpio, STM32_PIN_PULL_DOWN);
+		break;
+	case PIN_CONFIG_OUTPUT:
+		__stm32_pmx_gpio_output(bank->base, gpio, arg);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
 static struct gpio_ops stm32_gpio_ops = {
 	.direction_input = stm32_gpio_direction_input,
 	.direction_output = stm32_gpio_direction_output,
 	.get_direction = stm32_gpio_get_direction,
 	.get = stm32_gpio_get,
 	.set = stm32_gpio_set,
+	.set_config = IS_ENABLED(CONFIG_GPIO_PINCONF) ? stm32_gpio_set_config : NULL,
 };
 
 static int stm32_gpiochip_add(struct stm32_gpio_bank *bank,
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 04/11] gpiolib: turn request/active_low into bit flags
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2024-08-09 14:23 ` [PATCH 03/11] pinctrl: stm32: implement generic " Ahmad Fatoum
@ 2024-08-09 14:23 ` Ahmad Fatoum
  2024-08-09 14:23 ` [PATCH 05/11] gpiolib: don't use GPIO number API in of_hog_gpio Ahmad Fatoum
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:23 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

We will be adding mroe flags for pull and single-ended pin
configuration, so prepare for that by turning the individual struct
members of bool types into a flag field.

Linux defines its own flags here (e.g. FLAG_PULL_UP) and translates
to/from them, but for barebox, let's just use the values described in
enum of_gpio_flags, even for non-OF platforms.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 41 ++++++++++++++++++++++++++++-------------
 include/of_gpio.h      |  1 +
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8d788d4d791..70c78a9ad4af 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -16,10 +16,9 @@ static LIST_HEAD(chip_list);
 
 struct gpio_desc {
 	struct gpio_chip *chip;
-	bool requested;
-	bool active_low;
 	char *label;
 	const char *name;
+	u32 flags; /* OR-d enum of_gpio_flags */
 };
 
 /*
@@ -62,9 +61,14 @@ static int gpio_desc_alloc(void)
 }
 pure_initcall(gpio_desc_alloc);
 
+static inline bool gpiod_is_requested(struct gpio_desc *desc)
+{
+	return desc->flags & OF_GPIO_REQUESTED;
+}
+
 static int gpio_ensure_requested(struct gpio_desc *desc, int gpio)
 {
-	if (desc->requested)
+	if (gpiod_is_requested(desc))
 		return 0;
 
 	return gpio_request(gpio, "gpio");
@@ -86,20 +90,28 @@ static unsigned gpiodesc_chip_offset(const struct gpio_desc *desc)
 	return (desc - gpio_desc) - desc->chip->base + desc->chip->gpio_offset;
 }
 
+static int gpiod_is_active_low(const struct gpio_desc *desc)
+{
+	return desc->flags & OF_GPIO_ACTIVE_LOW;
+}
+
 static int gpio_adjust_value(const struct gpio_desc *desc,
 			     int value)
 {
 	if (value < 0)
 		return value;
 
-	return !!value ^ desc->active_low;
+	if (gpiod_is_active_low(desc))
+		value = !value;
+
+	return (bool)value;
 }
 
 static int gpiodesc_request(struct gpio_desc *desc, const char *label)
 {
 	int ret;
 
-	if (desc->requested) {
+	if (gpiod_is_requested(desc)) {
 		ret = -EBUSY;
 		goto done;
 	}
@@ -113,8 +125,7 @@ static int gpiodesc_request(struct gpio_desc *desc, const char *label)
 			goto done;
 	}
 
-	desc->requested = true;
-	desc->active_low = false;
+	desc->flags = OF_GPIO_REQUESTED;
 	desc->label = xstrdup(label);
 
 done:
@@ -132,7 +143,7 @@ int gpio_find_by_label(const char *label)
 	for (i = 0; i < ARCH_NR_GPIOS; i++) {
 		struct gpio_desc *info = &gpio_desc[i];
 
-		if (!info->requested || !info->chip || !info->label)
+		if (!gpiod_is_requested(info) || !info->chip || !info->label)
 			continue;
 
 		if (!strcmp(info->label, label))
@@ -189,14 +200,13 @@ bool gpio_slice_acquired(unsigned gpio)
 
 static void gpiodesc_free(struct gpio_desc *desc)
 {
-	if (!desc->requested)
+	if (!gpiod_is_requested(desc))
 		return;
 
 	if (desc->chip->ops->free)
 		desc->chip->ops->free(desc->chip, gpiodesc_chip_offset(desc));
 
-	desc->requested = false;
-	desc->active_low = false;
+	desc->flags = 0;
 	free(desc->label);
 	desc->label = NULL;
 }
@@ -508,7 +518,8 @@ static int gpiodesc_request_one(struct gpio_desc *desc, unsigned long flags,
 	if (err)
 		return err;
 
-	desc->active_low = active_low;
+	if (active_low)
+		desc->flags |= OF_GPIO_ACTIVE_LOW;
 
 	if (dir_in)
 		err = gpiod_direction_input(desc);
@@ -1148,6 +1159,7 @@ static int do_gpiolib(int argc, char *argv[])
 
 	for (i = 0; i < ARCH_NR_GPIOS; i++) {
 		struct gpio_desc *desc = &gpio_desc[i];
+		bool requested, active_low;
 		int val = -1, dir = -1;
 		int idx;
 
@@ -1173,10 +1185,13 @@ static int do_gpiolib(int argc, char *argv[])
 		if (desc->chip->ops->get)
 			val = desc->chip->ops->get(desc->chip, idx);
 
+		requested  = gpiod_is_requested(desc);
+		active_low = gpiod_is_active_low(desc);
+
 		printf("  GPIO %4d: %-3s %-3s %-9s %-20s %-20s\n", chip ? idx : i,
 			(dir < 0) ? "unk" : ((dir == GPIOF_DIR_IN) ? "in" : "out"),
 			(val < 0) ? "unk" : ((val == 0) ? "lo" : "hi"),
-		        desc->requested ? (desc->active_low ? "active low" : "true") : "false",
+		        requested ? (active_low ? "active low" : "true") : "false",
 			desc->name ? desc->name : "",
 			desc->label ? desc->label : "");
 	}
diff --git a/include/of_gpio.h b/include/of_gpio.h
index 794a9926cdbb..a7a3493473c8 100644
--- a/include/of_gpio.h
+++ b/include/of_gpio.h
@@ -17,6 +17,7 @@
  */
 enum of_gpio_flags {
 	OF_GPIO_ACTIVE_LOW = 0x1,
+	OF_GPIO_REQUESTED = 0x80000000, /* internal use */
 };
 
 #ifdef CONFIG_OF_GPIO
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 05/11] gpiolib: don't use GPIO number API in of_hog_gpio
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2024-08-09 14:23 ` [PATCH 04/11] gpiolib: turn request/active_low into bit flags Ahmad Fatoum
@ 2024-08-09 14:23 ` Ahmad Fatoum
  2024-08-09 14:24 ` [PATCH 06/11] gpiolib: store all OF flags into GPIO descriptor Ahmad Fatoum
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:23 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The GPIO number API is legacy that's meant to be a layer on top
of GPIO descriptors, which should be used directly for new code.

of_hog_gpio which is called after registering adding GPIO chips also
uses the number API, which is quite unnecessary: gpio_get_num iterates
over all GPIO chips to find out the number only to convert it to a
descriptor inside of gpio_request_one.

Instead of that, let's use gpiochip_get_desc, which just does some
arithmetic to get the descriptor as we know the gpiochip must be there.

This prepares us for propagating OF flags in a later commit.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 70c78a9ad4af..0c6c0fcc1aeb 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -90,6 +90,11 @@ static unsigned gpiodesc_chip_offset(const struct gpio_desc *desc)
 	return (desc - gpio_desc) - desc->chip->base + desc->chip->gpio_offset;
 }
 
+static __maybe_unused struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, int gpio)
+{
+	return gpio_desc + chip->base + gpio - chip->gpio_offset;
+}
+
 static int gpiod_is_active_low(const struct gpio_desc *desc)
 {
 	return desc->flags & OF_GPIO_ACTIVE_LOW;
@@ -643,6 +648,7 @@ static int gpiochip_find_base(int ngpio)
 	return base;
 }
 
+
 #ifdef CONFIG_OF_GPIO
 
 static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
@@ -651,7 +657,7 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 	struct device_node *chip_np = chip->dev->of_node;
 	unsigned long flags = 0;
 	u32 gpio_cells, gpio_num, gpio_flags;
-	int ret, gpio;
+	int ret;
 	const char *name = NULL;
 
 	ret = of_property_read_u32(chip_np, "#gpio-cells", &gpio_cells);
@@ -678,16 +684,6 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 	if (gpio_flags & OF_GPIO_ACTIVE_LOW)
 		flags |= GPIOF_ACTIVE_LOW;
 
-	gpio = gpio_get_num(chip->dev, gpio_num);
-	if (gpio == -EPROBE_DEFER)
-		return gpio;
-
-	if (gpio < 0) {
-		dev_err(chip->dev, "unable to get gpio %u\n", gpio_num);
-		return gpio;
-	}
-
-
 	/*
 	 * Note that, in order to be compatible with Linux, the code
 	 * below interprets 'output-high' as to mean 'output-active'.
@@ -714,7 +710,8 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 	if (ret < 0)
 		name = np->name;
 
-	return gpio_request_one(gpio, flags, name);
+	return gpiodesc_request_one(gpiochip_get_desc(chip, gpio_num),
+				    flags, name);
 }
 
 static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 06/11] gpiolib: store all OF flags into GPIO descriptor
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2024-08-09 14:23 ` [PATCH 05/11] gpiolib: don't use GPIO number API in of_hog_gpio Ahmad Fatoum
@ 2024-08-09 14:24 ` Ahmad Fatoum
  2024-08-09 14:24 ` [PATCH 07/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The last cell in GPIO OF description usually only holds either
GPIO_ACTIVE_HIGH or GPIO_ACTIVE_LOW, but some device trees also contain
extra flags like GPIO_PULL_UP or GPIO_OPEN_DRAIN.

To prepare acting on these flags, store all of them into the GPIO descriptor.

Like Linux, we have two kinds of flags, lookup flags from DT and request
flags from the drivers. To avoid confusion, we give gpiodesc_request_one
two parameters for each and have that function worry about combining
them.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 45 ++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0c6c0fcc1aeb..539fd3c7c25f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -504,8 +504,20 @@ int gpio_direction_input(unsigned gpio)
 }
 EXPORT_SYMBOL(gpio_direction_input);
 
-static int gpiodesc_request_one(struct gpio_desc *desc, unsigned long flags,
-				const char *label)
+/**
+ * gpiodesc_request_one - request GPIO descriptor
+ * @desc: GPIO descriptor
+ * @lflags: OF lookup flags for this GPIO or 0 if default
+ * such as GPIOF_ACTIVE_LOW
+ * @dflags: descriptor request flags (GPIOF_*) for this GPIO or 0 if default
+ *
+ * Function is used to request a GPIO descriptor and configure it for use.
+ *
+ * Returns:
+ * 0 on success and a negative error code otherwise.
+ */
+static int gpiodesc_request_one(struct gpio_desc *desc, unsigned long lflags,
+				unsigned long dflags, const char *label)
 {
 	int err;
 
@@ -513,16 +525,17 @@ static int gpiodesc_request_one(struct gpio_desc *desc, unsigned long flags,
 	 * Not all of the flags below are mulit-bit, but, for the sake
 	 * of consistency, the code is written as if all of them were.
 	 */
-	const bool active_low  = (flags & GPIOF_ACTIVE_LOW) == GPIOF_ACTIVE_LOW;
-	const bool dir_in      = (flags & GPIOF_DIR_IN) == GPIOF_DIR_IN;
-	const bool logical     = (flags & GPIOF_LOGICAL) == GPIOF_LOGICAL;
-	const bool init_active = (flags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
-	const bool init_high   = (flags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
+	const bool active_low  = (dflags & GPIOF_ACTIVE_LOW) == GPIOF_ACTIVE_LOW;
+	const bool dir_in      = (dflags & GPIOF_DIR_IN) == GPIOF_DIR_IN;
+	const bool logical     = (dflags & GPIOF_LOGICAL) == GPIOF_LOGICAL;
+	const bool init_active = (dflags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
+	const bool init_high   = (dflags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
 
 	err = gpiodesc_request(desc, label);
 	if (err)
 		return err;
 
+	desc->flags |= lflags;
 	if (active_low)
 		desc->flags |= OF_GPIO_ACTIVE_LOW;
 
@@ -552,7 +565,7 @@ int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 	if (!desc)
 		return -ENODEV;
 
-	return gpiodesc_request_one(desc, flags, label);
+	return gpiodesc_request_one(desc, 0, flags, label);
 }
 EXPORT_SYMBOL_GPL(gpio_request_one);
 
@@ -656,7 +669,7 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 {
 	struct device_node *chip_np = chip->dev->of_node;
 	unsigned long flags = 0;
-	u32 gpio_cells, gpio_num, gpio_flags;
+	u32 gpio_cells, gpio_num, of_flags;
 	int ret;
 	const char *name = NULL;
 
@@ -677,13 +690,10 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 		return ret;
 
 	ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells + 1,
-					 &gpio_flags);
+					 &of_flags);
 	if (ret)
 		return ret;
 
-	if (gpio_flags & OF_GPIO_ACTIVE_LOW)
-		flags |= GPIOF_ACTIVE_LOW;
-
 	/*
 	 * Note that, in order to be compatible with Linux, the code
 	 * below interprets 'output-high' as to mean 'output-active'.
@@ -711,7 +721,7 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 		name = np->name;
 
 	return gpiodesc_request_one(gpiochip_get_desc(chip, gpio_num),
-				    flags, name);
+				    of_flags, flags, name);
 }
 
 static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
@@ -901,7 +911,7 @@ static struct property *of_find_gpio_property(struct device_node *np,
 struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 			struct device_node *np,
 			const char *con_id, int index,
-			enum gpiod_flags flags,
+			enum gpiod_flags dflags,
 			const char *label)
 {
 	struct gpio_desc *desc = NULL;
@@ -925,9 +935,6 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 
 	desc = gpio_to_desc(gpio);
 
-	if (of_flags & OF_GPIO_ACTIVE_LOW)
-		flags |= GPIOF_ACTIVE_LOW;
-
 	buf = NULL;
 
 	if (!label) {
@@ -937,7 +944,7 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 			label = dev_name(dev);
 	}
 
-	ret = gpiodesc_request_one(desc, flags, label);
+	ret = gpiodesc_request_one(desc, of_flags, dflags, label);
 	free(buf);
 
 	return ret ? ERR_PTR(ret): desc;
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 07/11] gpiolib: add support for OF GPIO configuration binding
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2024-08-09 14:24 ` [PATCH 06/11] gpiolib: store all OF flags into GPIO descriptor Ahmad Fatoum
@ 2024-08-09 14:24 ` Ahmad Fatoum
  2024-08-09 14:24 ` [PATCH 08/11] gpiolib: use dev_gpiod_get_index device node argument throughout Ahmad Fatoum
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Previous commits laid the groundwork:

  - Flags in the DT are saved into GPIO descriptors
  - GPIO drivers can implement a set_config operation

Let's wire them together, so we call the set_config operation when
requesting a GPIO as necessary.

For this to be usable:

  - The GPIO consumer must request the GPIO with gpiod_get
  - The GPIO provider must implement set_config

This is not yet the case for the overwhelming majority of barebox GPIO
consumers and providers, so we allow disabling this functionality via a
Kconfig option.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 94 +++++++++++++++++++++++++++++++++++++++++-
 drivers/of/Kconfig     | 15 +++++++
 include/of_gpio.h      |  5 +++
 3 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 539fd3c7c25f..4a1792a8df1f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -8,6 +8,7 @@
 #include <gpio.h>
 #include <of_gpio.h>
 #include <linux/gpio/consumer.h>
+#include <linux/pinctrl/pinconf-generic.h>
 #include <linux/overflow.h>
 #include <errno.h>
 #include <malloc.h>
@@ -437,6 +438,61 @@ int gpio_direction_output(unsigned gpio, int value)
 }
 EXPORT_SYMBOL(gpio_direction_output);
 
+static int gpio_set_config_with_argument(struct gpio_desc *desc,
+					 enum pin_config_param mode,
+					 u32 argument)
+{
+	unsigned long config;
+
+	config = pinconf_to_config_packed(mode, argument);
+	return gpio_do_set_config(desc->chip, gpiodesc_chip_offset(desc), config);
+}
+
+static int gpio_set_config_with_argument_optional(struct gpio_desc *desc,
+						  enum pin_config_param mode,
+						  u32 argument)
+{
+	int ret;
+
+	ret = gpio_set_config_with_argument(desc, mode, argument);
+	if (ret != -ENOTSUPP)
+		return ret;
+	return 0;
+}
+
+static int gpio_set_config(struct gpio_desc *desc, enum pin_config_param mode)
+{
+	return gpio_set_config_with_argument(desc, mode, 0);
+}
+
+static int gpio_set_bias(struct gpio_desc *desc)
+{
+	enum pin_config_param bias;
+	unsigned int arg;
+
+	if (desc->flags & OF_GPIO_PULL_DISABLE)
+		bias = PIN_CONFIG_BIAS_DISABLE;
+	else if (desc->flags & OF_GPIO_PULL_UP)
+		bias = PIN_CONFIG_BIAS_PULL_UP;
+	else if (desc->flags & OF_GPIO_PULL_DOWN)
+		bias = PIN_CONFIG_BIAS_PULL_DOWN;
+	else
+		return 0;
+
+	switch (bias) {
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+	case PIN_CONFIG_BIAS_PULL_UP:
+		arg = 1;
+		break;
+
+	default:
+		arg = 0;
+		break;
+	}
+
+	return gpio_set_config_with_argument_optional(desc, bias, arg);
+}
+
 /**
  * gpiod_direction_output - set the GPIO direction to output
  * @desc:	GPIO to set to output
@@ -451,8 +507,36 @@ EXPORT_SYMBOL(gpio_direction_output);
  */
 int gpiod_direction_output(struct gpio_desc *desc, int value)
 {
+	int ret;
+
 	VALIDATE_DESC(desc);
 
+	if (IS_ENABLED(CONFIG_GPIO_PINCONF)) {
+		if (desc->flags & (OF_GPIO_OPEN_DRAIN | OF_GPIO_SINGLE_ENDED)) {
+			/* First see if we can enable open drain in hardware */
+			ret = gpio_set_config(desc, PIN_CONFIG_DRIVE_OPEN_DRAIN);
+			if (!ret)
+				goto set_output_value;
+			/* Emulate open drain by not actively driving the line high */
+			if (value)
+				return gpiod_direction_input(desc);
+		} else if (desc->flags & OF_GPIO_SINGLE_ENDED) {
+			ret = gpio_set_config(desc, PIN_CONFIG_DRIVE_OPEN_SOURCE);
+			if (!ret)
+				goto set_output_value;
+			/* Emulate open source by not actively driving the line low */
+			if (!value)
+				return gpiod_direction_input(desc);
+		} else {
+			gpio_set_config(desc, PIN_CONFIG_DRIVE_PUSH_PULL);
+		}
+
+set_output_value:
+		ret = gpio_set_bias(desc);
+		if (ret)
+			return ret;
+	}
+
 	return gpiod_direction_output_raw(desc, gpio_adjust_value(desc, value));
 }
 
@@ -478,13 +562,19 @@ EXPORT_SYMBOL(gpio_direction_active);
  */
 int gpiod_direction_input(struct gpio_desc *desc)
 {
+	int ret;
+
 	VALIDATE_DESC(desc);
 
 	if (!desc->chip->ops->direction_input)
 		return -ENOSYS;
 
-	return desc->chip->ops->direction_input(desc->chip,
-					      gpiodesc_chip_offset(desc));
+	ret = desc->chip->ops->direction_input(desc->chip,
+					       gpiodesc_chip_offset(desc));
+	if (ret == 0 && IS_ENABLED(CONFIG_GPIO_PINCONF))
+		ret = gpio_set_bias(desc);
+
+	return ret;
 }
 EXPORT_SYMBOL(gpiod_direction_input);
 
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 2791100a2d9c..6c9aedf355b4 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -40,6 +40,21 @@ config OF_GPIO
 	depends on OFDEVICE
 	def_bool y
 
+config OF_GPIO_PINCONF
+	depends on OF_GPIO && HAVE_GPIO_PINCONF
+	select GPIO_PINCONF
+	bool "Enable support for extra GPIO pin configuration binding"
+	default y
+	help
+	  In addition to the normal pinctrl-names/pinctrl-X binding, there's
+	  also a binding to add flags like GPIO_OPEN_DRAIN or GPIO_PULL_UP
+	  OR-ed into the cell of the gpios property used for
+	  GPIO_ACTIVE_HIGH/LOW. This latter binding is optional and many
+	  drivers don't support it.
+
+	  If unsure and not size conscious, say y here to enable the
+	  extra binding.
+
 config OF_PCI
 	bool
 	depends on PCI
diff --git a/include/of_gpio.h b/include/of_gpio.h
index a7a3493473c8..4ab5de6ed580 100644
--- a/include/of_gpio.h
+++ b/include/of_gpio.h
@@ -17,6 +17,11 @@
  */
 enum of_gpio_flags {
 	OF_GPIO_ACTIVE_LOW = 0x1,
+	OF_GPIO_SINGLE_ENDED = 0x2,
+	OF_GPIO_OPEN_DRAIN = 0x4,
+	OF_GPIO_PULL_UP = 0x10,
+	OF_GPIO_PULL_DOWN = 0x20,
+	OF_GPIO_PULL_DISABLE = 0x40,
 	OF_GPIO_REQUESTED = 0x80000000, /* internal use */
 };
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 08/11] gpiolib: use dev_gpiod_get_index device node argument throughout
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2024-08-09 14:24 ` [PATCH 07/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
@ 2024-08-09 14:24 ` Ahmad Fatoum
  2024-08-09 14:24 ` [PATCH 09/11] gpiolib: export function to get struct gpio_desc from index Ahmad Fatoum
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

dev_gpiod_get_index takes both a consumer device and a consumer device
tree node. The consumer device tree node should be used for all lookups,
but it was used only for one of two lookups so far.

This would break using the function on subnodes of a device, so remedy
that.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4a1792a8df1f..5d5458374869 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1018,8 +1018,7 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 	if (!pp)
 		return ERR_PTR(-ENOENT);
 
-	gpio = of_get_named_gpio_flags(dev->device_node, pp->name,
-				       index, &of_flags);
+	gpio = of_get_named_gpio_flags(np, pp->name, index, &of_flags);
 	if (!gpio_is_valid(gpio))
 		return ERR_PTR(gpio < 0 ? gpio : -EINVAL);
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 09/11] gpiolib: export function to get struct gpio_desc from index
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2024-08-09 14:24 ` [PATCH 08/11] gpiolib: use dev_gpiod_get_index device node argument throughout Ahmad Fatoum
@ 2024-08-09 14:24 ` Ahmad Fatoum
  2024-08-09 14:24 ` [PATCH 10/11] input: gpio_keys: switch to GPIO descriptor API Ahmad Fatoum
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

For use in legacy non-DT board code, let's introduce a function that
translates an absolute GPIO number into the corresponding GPIO descriptor.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c        | 26 ++++++++++++++++++++------
 include/linux/gpio/consumer.h | 10 ++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5d5458374869..146eaf9af138 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -642,6 +642,24 @@ static int gpiodesc_request_one(struct gpio_desc *desc, unsigned long lflags,
 	return err;
 }
 
+struct gpio_desc *gpiod_request_one(unsigned gpio,
+				    unsigned long flags, const char *label)
+{
+	struct gpio_desc *desc = gpio_to_desc(gpio);
+	int ret;
+
+	if (!desc)
+		return ERR_PTR(-ENODEV);
+
+	ret = gpiodesc_request_one(desc, 0, flags, label);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return desc;
+}
+EXPORT_SYMBOL_GPL(gpio_request_one);
+
+
 /**
  * gpio_request_one - request a single GPIO with initial configuration
  * @gpio:	the GPIO number
@@ -650,15 +668,11 @@ static int gpiodesc_request_one(struct gpio_desc *desc, unsigned long lflags,
  */
 int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 {
-	struct gpio_desc *desc = gpio_to_desc(gpio);
-
-	if (!desc)
-		return -ENODEV;
-
-	return gpiodesc_request_one(desc, 0, flags, label);
+	return PTR_ERR_OR_ZERO(gpiod_request_one(gpio, flags, label));
 }
 EXPORT_SYMBOL_GPL(gpio_request_one);
 
+
 /**
  * gpio_request_array - request multiple GPIOs in a single call
  * @array:	array of the 'struct gpio'
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 64366a6f3302..f0d5bf7b255b 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -97,6 +97,9 @@ int gpiod_set_array_value(unsigned int array_size,
 			  struct gpio_array *array_info,
 			  unsigned long *value_bitmap);
 
+struct gpio_desc *gpiod_request_one(unsigned gpio,
+				    unsigned long flags, const char *label);
+
 #else
 
 static inline int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
@@ -186,6 +189,13 @@ static inline int gpiod_set_array_value(unsigned int array_size,
 	return 0;
 }
 
+static inline
+struct gpio_desc *gpiod_request_one(unsigned gpio,
+				    unsigned long flags, const char *label)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 #endif
 
 static inline struct gpio_desc *dev_gpiod_get(struct device *dev,
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 10/11] input: gpio_keys: switch to GPIO descriptor API
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2024-08-09 14:24 ` [PATCH 09/11] gpiolib: export function to get struct gpio_desc from index Ahmad Fatoum
@ 2024-08-09 14:24 ` Ahmad Fatoum
  2024-08-09 14:24 ` [PATCH 11/11] input: gpio-keys: request with label in DT if available Ahmad Fatoum
  2024-08-14 11:00 ` [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Sascha Hauer
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:24 UTC (permalink / raw)
  To: barebox; +Cc: Chris Fiege, Ahmad Fatoum

Pull-ups that are only configured in the OF GPIO description like:

  gpios = <&gpioe 7 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;

are currently ignored. This is because we open coded determination of
the GPIO flags and discarded all of them, except for active-low, thereby
giving gpiolib no chance to apply other flags.

Fix this by using dev_gpiod_get instead to parse the GPIO description
and request it.

This lets us support the aforementioned binding and handles active low
configuration transparently.

Reported-by: Chris Fiege <c.fiege@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/input/Kconfig     |  2 +-
 drivers/input/gpio_keys.c | 66 ++++++++++++++++++---------------------
 2 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index 01e0b722c20f..fc4198b556f1 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -25,7 +25,7 @@ config INPUT_EVBUG
 
 config KEYBOARD_GPIO
 	bool "GPIO Buttons"
-	depends on GENERIC_GPIO
+	depends on GPIOLIB
 	select POLLER
 	select INPUT
 	help
diff --git a/drivers/input/gpio_keys.c b/drivers/input/gpio_keys.c
index c897acf3bd02..3701817788ca 100644
--- a/drivers/input/gpio_keys.c
+++ b/drivers/input/gpio_keys.c
@@ -9,16 +9,13 @@
 #include <clock.h>
 #include <gpio_keys.h>
 #include <poller.h>
-#include <gpio.h>
-#include <of_gpio.h>
+#include <linux/gpio/consumer.h>
 #include <input/input.h>
 
 struct gpio_key {
+	struct gpio_desc *gpio;
 	int code;
 
-	int gpio;
-	int active_low;
-
 	int previous_state;
 
 	int debounce_interval;
@@ -37,31 +34,29 @@ static void gpio_key_poller(void *data)
 {
 	struct gpio_keys *gk = data;
 	struct gpio_key *gb;
-	int i, val;
+	int i, pressed;
 
 	for (i = 0; i < gk->nbuttons; i++) {
 		gb = &gk->buttons[i];
 
-		if (gpio_slice_acquired(gb->gpio))
+		if (gpiod_slice_acquired(gb->gpio))
 			goto out;
 	}
 
 	for (i = 0; i < gk->nbuttons; i++) {
 
 		gb = &gk->buttons[i];
-		val = gpio_get_value(gb->gpio);
+		pressed = gpiod_get_value(gb->gpio);
 
 		if (!is_timeout(gb->debounce_start, gb->debounce_interval * MSECOND))
 			continue;
 
-		if (val != gb->previous_state) {
-			int pressed = val != gb->active_low;
-
+		if (pressed != gb->previous_state) {
 			gb->debounce_start = get_time_ns();
 			input_report_key_event(&gk->input, gb->code, pressed);
-			dev_dbg(gk->input.parent, "%s gpio(%d) as %d\n",
-				pressed ? "pressed" : "released", gb->gpio, gb->code);
-			gb->previous_state = val;
+			dev_dbg(gk->input.parent, "%s gpio #%d as %d\n",
+				pressed ? "pressed" : "released", i, gb->code);
+			gb->previous_state = pressed;
 		}
 	}
 out:
@@ -85,9 +80,20 @@ static int gpio_keys_probe_pdata(struct gpio_keys *gk, struct device *dev)
 	gk->nbuttons = pdata->nbuttons;
 
 	for (i = 0; i < gk->nbuttons; i++) {
-		gk->buttons[i].gpio = pdata->buttons[i].gpio;
+		struct gpio_desc *gpio;
+		ulong flags = GPIOF_DIR_IN;
+
+		if (pdata->buttons[i].active_low)
+			flags |= GPIOF_ACTIVE_LOW;
+
+		gpio = gpiod_request_one(pdata->buttons[i].gpio, flags, "gpio_keys");
+		if (IS_ERR(gpio)) {
+			pr_err("gpio_keys: gpio #%d can not be requested\n", i);
+			return PTR_ERR(gpio);
+		}
+
+		gk->buttons[i].gpio = gpio;
 		gk->buttons[i].code = pdata->buttons[i].code;
-		gk->buttons[i].active_low = pdata->buttons[i].active_low;
 		gk->buttons[i].debounce_interval = 20;
 	}
 
@@ -106,15 +112,16 @@ static int gpio_keys_probe_dt(struct gpio_keys *gk, struct device *dev)
 	gk->buttons = xzalloc(gk->nbuttons * sizeof(*gk->buttons));
 
 	for_each_child_of_node(np, npkey) {
-		enum of_gpio_flags gpioflags;
+		struct gpio_desc *gpio;
 		uint32_t keycode;
 
-		gk->buttons[i].gpio = of_get_named_gpio_flags(npkey, "gpios", 0, &gpioflags);
-		if (gk->buttons[i].gpio < 0)
-			return gk->buttons[i].gpio;
+		gpio = dev_gpiod_get(dev, npkey, "gpios", GPIOD_IN, NULL);
+		if (IS_ERR(gpio)) {
+			pr_err("gpio_keys: gpio #%d can not be requested\n", i);
+			return PTR_ERR(gpio);
+		}
 
-		if (gpioflags & OF_GPIO_ACTIVE_LOW)
-			gk->buttons[i].active_low = 1;
+		gk->buttons[i].gpio = gpio;
 
 		ret = of_property_read_u32(npkey, "linux,code", &keycode);
 		if (ret)
@@ -135,12 +142,12 @@ static int gpio_keys_probe_dt(struct gpio_keys *gk, struct device *dev)
 
 static int __init gpio_keys_probe(struct device *dev)
 {
-	int ret, i, gpio;
+	int ret;
 	struct gpio_keys *gk;
 
 	gk = xzalloc(sizeof(*gk));
 
-	if (dev->of_node)
+	if (dev_of_node(dev))
 		ret = gpio_keys_probe_dt(gk, dev);
 	else
 		ret = gpio_keys_probe_pdata(gk, dev);
@@ -148,17 +155,6 @@ static int __init gpio_keys_probe(struct device *dev)
 	if (ret)
 		return ret;
 
-	for (i = 0; i < gk->nbuttons; i++) {
-		gpio = gk->buttons[i].gpio;
-		ret = gpio_request(gpio, "gpio_keys");
-		if (ret) {
-			pr_err("gpio_keys: (%d) can not be requested\n", gpio);
-			return ret;
-		}
-		gpio_direction_input(gpio);
-		gk->buttons[i].previous_state = gk->buttons[i].active_low;
-	}
-
 	gk->input.parent = dev;
 	ret = input_device_register(&gk->input);
 	if (ret)
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 11/11] input: gpio-keys: request with label in DT if available
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (9 preceding siblings ...)
  2024-08-09 14:24 ` [PATCH 10/11] input: gpio_keys: switch to GPIO descriptor API Ahmad Fatoum
@ 2024-08-09 14:24 ` Ahmad Fatoum
  2024-08-14 11:00 ` [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Sascha Hauer
  11 siblings, 0 replies; 13+ messages in thread
From: Ahmad Fatoum @ 2024-08-09 14:24 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

gpio-keys may have optional label properties, which we so far ignored.
Let's use them instead of generating a label from the device name
should they be available.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/input/gpio_keys.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/gpio_keys.c b/drivers/input/gpio_keys.c
index 3701817788ca..b52738f5ccb2 100644
--- a/drivers/input/gpio_keys.c
+++ b/drivers/input/gpio_keys.c
@@ -112,10 +112,13 @@ static int gpio_keys_probe_dt(struct gpio_keys *gk, struct device *dev)
 	gk->buttons = xzalloc(gk->nbuttons * sizeof(*gk->buttons));
 
 	for_each_child_of_node(np, npkey) {
+		const char *label = NULL;
 		struct gpio_desc *gpio;
 		uint32_t keycode;
 
-		gpio = dev_gpiod_get(dev, npkey, "gpios", GPIOD_IN, NULL);
+		of_property_read_string(npkey, "label", &label);
+
+		gpio = dev_gpiod_get(dev, npkey, NULL, GPIOD_IN, label);
 		if (IS_ERR(gpio)) {
 			pr_err("gpio_keys: gpio #%d can not be requested\n", i);
 			return PTR_ERR(gpio);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding
  2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
                   ` (10 preceding siblings ...)
  2024-08-09 14:24 ` [PATCH 11/11] input: gpio-keys: request with label in DT if available Ahmad Fatoum
@ 2024-08-14 11:00 ` Sascha Hauer
  11 siblings, 0 replies; 13+ messages in thread
From: Sascha Hauer @ 2024-08-14 11:00 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Fri, 09 Aug 2024 16:23:54 +0200, Ahmad Fatoum wrote:
> So far, GPIO bias configuration was done exclusively by pinctrl drivers.
> All barebox pinctrl drivers work by consuming a device tree node with
> a binding that differs from driver to driver and then applying the
> configuration described within.
> 
> Neither GPIO or pinctrl node have any insight on what in particular is
> being configured.
> 
> [...]

Applied, thanks!

[01/11] gpio: make gpio.h header self-contained
        https://git.pengutronix.de/cgit/barebox/commit/?id=14b85136923b (link may not be stable)
[02/11] gpiolib: permit GPIO drivers to implement struct gpio_ops::set_config
        https://git.pengutronix.de/cgit/barebox/commit/?id=588ae0dda1a2 (link may not be stable)
[03/11] pinctrl: stm32: implement generic struct gpio_ops::set_config
        https://git.pengutronix.de/cgit/barebox/commit/?id=043337f00f61 (link may not be stable)
[04/11] gpiolib: turn request/active_low into bit flags
        https://git.pengutronix.de/cgit/barebox/commit/?id=3c702bf6699d (link may not be stable)
[05/11] gpiolib: don't use GPIO number API in of_hog_gpio
        https://git.pengutronix.de/cgit/barebox/commit/?id=76f484d35d47 (link may not be stable)
[06/11] gpiolib: store all OF flags into GPIO descriptor
        https://git.pengutronix.de/cgit/barebox/commit/?id=e89e662f4082 (link may not be stable)
[07/11] gpiolib: add support for OF GPIO configuration binding
        https://git.pengutronix.de/cgit/barebox/commit/?id=269dad50c259 (link may not be stable)
[08/11] gpiolib: use dev_gpiod_get_index device node argument throughout
        https://git.pengutronix.de/cgit/barebox/commit/?id=f2c8dfbabd6b (link may not be stable)
[09/11] gpiolib: export function to get struct gpio_desc from index
        https://git.pengutronix.de/cgit/barebox/commit/?id=996f924de8dd (link may not be stable)
[10/11] input: gpio_keys: switch to GPIO descriptor API
        https://git.pengutronix.de/cgit/barebox/commit/?id=2fe7e9ecc63b (link may not be stable)
[11/11] input: gpio-keys: request with label in DT if available
        https://git.pengutronix.de/cgit/barebox/commit/?id=5e94d0bc7430 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-08-14 11:01 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-09 14:23 [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
2024-08-09 14:23 ` [PATCH 01/11] gpio: make gpio.h header self-contained Ahmad Fatoum
2024-08-09 14:23 ` [PATCH 02/11] gpiolib: permit GPIO drivers to implement struct gpio_ops::set_config Ahmad Fatoum
2024-08-09 14:23 ` [PATCH 03/11] pinctrl: stm32: implement generic " Ahmad Fatoum
2024-08-09 14:23 ` [PATCH 04/11] gpiolib: turn request/active_low into bit flags Ahmad Fatoum
2024-08-09 14:23 ` [PATCH 05/11] gpiolib: don't use GPIO number API in of_hog_gpio Ahmad Fatoum
2024-08-09 14:24 ` [PATCH 06/11] gpiolib: store all OF flags into GPIO descriptor Ahmad Fatoum
2024-08-09 14:24 ` [PATCH 07/11] gpiolib: add support for OF GPIO configuration binding Ahmad Fatoum
2024-08-09 14:24 ` [PATCH 08/11] gpiolib: use dev_gpiod_get_index device node argument throughout Ahmad Fatoum
2024-08-09 14:24 ` [PATCH 09/11] gpiolib: export function to get struct gpio_desc from index Ahmad Fatoum
2024-08-09 14:24 ` [PATCH 10/11] input: gpio_keys: switch to GPIO descriptor API Ahmad Fatoum
2024-08-09 14:24 ` [PATCH 11/11] input: gpio-keys: request with label in DT if available Ahmad Fatoum
2024-08-14 11:00 ` [PATCH 00/11] gpiolib: add support for OF GPIO configuration binding Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox