mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 0/9] gpio: add proper gpiod API
@ 2023-06-22  7:23 Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 1/9] driver: include dev_print and family from <driver.h> Ahmad Fatoum
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox

The gpiod_ (GPIO descriptor) API used with Linux differs from barebox'
normal GPIO API:

 - gpiod handles are opaque pointers and not an integer, which users
   have an expectation of stability for

 - gpiod API uses logic levels by default with separate raw API for
   physical level instead of physical level by default and separate
   API taking active level into account.

The barebox gpiod_ API mimics the latter point, but still uses integers
requiring ugly and arguably error prone conversions when porting kernel
code.

This series fixes that by adding proper struct gpio_desc API like in
Linux and then builds upon that to port the kernel gpio-mux driver.

v1 -> v2:
  - drop unrelated help command patch
  - drop gpio-mux patch for separate sending out
  - rebase onto next
  - fix and add stubs for !CONFIG_GPIOLIB
  - rename internal gpioinfo_ functions to gpiodesc_

v1 was here:
https://lore.barebox.org/barebox/20230621092700.GE18491@pengutronix.de/T/#t

Ahmad Fatoum (9):
  driver: include dev_print and family from <driver.h>
  include: linux/printk: define new dev_errp_probe
  gpio: have gpiod_ functions return and accept pointers
  gpio: gpiolib: rename struct gpio_info to gpio_desc
  gpiolib: export proper gpio descriptor API
  bitmap: implement bitmap_{to,from}_arr{32,64}
  gpiolib: factor out finding gpio property
  gpiolib: add support for requesting and setting gpiod arrays
  gpiolib: rename gpioinfo_ to gpiodesc_

 drivers/gpio/gpio-pca953x.c              |   9 +-
 drivers/gpio/gpiolib.c                   | 528 +++++++++++++++++------
 drivers/mci/mci_spi.c                    |  13 +-
 drivers/mtd/nand/atmel/nand-controller.c |  40 +-
 drivers/mtd/nand/nand_base.c             |   6 +-
 drivers/net/designware_eqos.c            |  26 +-
 drivers/net/ksz8873.c                    |  13 +-
 drivers/net/ksz9477.c                    |  13 +-
 drivers/net/realtek-dsa/realtek-mdio.c   |  10 +-
 drivers/net/realtek-dsa/realtek-smi.c    |  18 +-
 drivers/net/realtek-dsa/realtek.h        |   6 +-
 drivers/net/sja1105.c                    |  25 +-
 drivers/nvmem/starfive-otp.c             |  12 +-
 drivers/pci/pcie-dw-rockchip.c           |  14 +-
 drivers/power/reset/gpio-poweroff.c      |  14 +-
 drivers/power/reset/gpio-restart.c       |  23 +-
 drivers/regulator/fixed.c                |  27 +-
 drivers/sound/gpio-beeper.c              |  14 +-
 drivers/usb/misc/onboard_usb_hub.c       |  11 +-
 drivers/video/mipi_dbi.c                 |   8 +-
 drivers/video/panel-ilitek-ili9341.c     |  17 +-
 drivers/video/panel-mipi-dbi.c           |  17 +-
 drivers/watchdog/gpio_wdt.c              |  22 +-
 include/driver.h                         |   1 +
 include/gpiod.h                          |  79 +---
 include/linux/bitmap.h                   | 101 +++++
 include/linux/gpio/consumer.h            | 224 ++++++++++
 include/linux/mtd/rawnand.h              |   4 +-
 include/linux/printk.h                   |   3 +
 include/video/mipi_dbi.h                 |   7 +-
 lib/bitmap.c                             | 103 +++++
 31 files changed, 1019 insertions(+), 389 deletions(-)
 create mode 100644 include/linux/gpio/consumer.h

-- 
2.39.2




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

* [PATCH v2 1/9] driver: include dev_print and family from <driver.h>
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 2/9] include: linux/printk: define new dev_errp_probe Ahmad Fatoum
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

Linux drivers get dev_info and friends transitively when including
<linux/device.h>, so have barebox follow suit.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/driver.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/driver.h b/include/driver.h
index d012cd006dcb..2651cddecc21 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -9,6 +9,7 @@
 #include <linux/list.h>
 #include <linux/ioport.h>
 #include <linux/uuid.h>
+#include <linux/printk.h>
 #include <of.h>
 #include <filetype.h>
 
-- 
2.39.2




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

* [PATCH v2 2/9] include: linux/printk: define new dev_errp_probe
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 1/9] driver: include dev_print and family from <driver.h> Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 3/9] gpio: have gpiod_ functions return and accept pointers Ahmad Fatoum
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

We have a number of APIs like regulator, clocks or reset, which return
an error pointer. To format that pointer for dev_err_probe, we need to
pass it through PTR_ERR(). Let's make such code more concise by defining
dev_errp_probe, which calls PTR_ERR() on its second argument.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/printk.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 057b355aa129..978e853bab48 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -3,6 +3,7 @@
 #define __LINUX_PRINTK_H
 
 #include <linux/list.h>
+#include <linux/err.h>
 #include <printk.h>
 #include <stdarg.h>
 
@@ -89,6 +90,8 @@ static inline int dev_err_probe(struct device *dev, int err, const char *fmt,
 }
 #endif
 
+#define dev_errp_probe(dev, errptr, args...) dev_err_probe((dev), PTR_ERR(errptr), args)
+
 #define __pr_printk(level, format, args...) \
 	({	\
 		(level) <= LOGLEVEL ? pr_print((level), (format), ##args) : 0; \
-- 
2.39.2




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

* [PATCH v2 3/9] gpio: have gpiod_ functions return and accept pointers
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 1/9] driver: include dev_print and family from <driver.h> Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 2/9] include: linux/printk: define new dev_errp_probe Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 4/9] gpio: gpiolib: rename struct gpio_info to gpio_desc Ahmad Fatoum
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

The gpiod_ (GPIO descriptor) API used with Linux differs from barebox'
normal GPIO API:

 - gpiod handles are opaque pointers and not an integer, which users
   have an expectation of stability for

 - gpiod API uses logic levels by default with separate raw API for
   physical level instead of physical level by default and separate
   API taking active level into account.

The barebox gpiod_ API mimics the latter point, but still uses integers
requiring ugly and arguably error prone conversions when porting kernel
code. Let's improve upon that by just encoding the integer into a
pointer variable for API compatibility.

Later commits will switch barebox GPIO support to use actual GPIO
descriptors without consulting the numeric indices, but we do this
temporary switch here, so we can split up provider and consumer changes.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpio-pca953x.c              |   9 +-
 drivers/gpio/gpiolib.c                   |  21 ++--
 drivers/mci/mci_spi.c                    |  13 +-
 drivers/mtd/nand/atmel/nand-controller.c |  40 +++---
 drivers/mtd/nand/nand_base.c             |   6 +-
 drivers/net/designware_eqos.c            |  26 ++--
 drivers/net/ksz8873.c                    |  13 +-
 drivers/net/ksz9477.c                    |  13 +-
 drivers/net/realtek-dsa/realtek-mdio.c   |  10 +-
 drivers/net/realtek-dsa/realtek-smi.c    |  18 +--
 drivers/net/realtek-dsa/realtek.h        |   6 +-
 drivers/net/sja1105.c                    |  25 ++--
 drivers/nvmem/starfive-otp.c             |  12 +-
 drivers/pci/pcie-dw-rockchip.c           |  14 +--
 drivers/power/reset/gpio-poweroff.c      |  14 +--
 drivers/power/reset/gpio-restart.c       |  23 ++--
 drivers/regulator/fixed.c                |  27 ++---
 drivers/sound/gpio-beeper.c              |  14 +--
 drivers/usb/misc/onboard_usb_hub.c       |  11 +-
 drivers/video/mipi_dbi.c                 |   8 +-
 drivers/video/panel-ilitek-ili9341.c     |  17 +--
 drivers/video/panel-mipi-dbi.c           |  17 +--
 drivers/watchdog/gpio_wdt.c              |  22 ++--
 include/gpiod.h                          |  79 +-----------
 include/linux/gpio/consumer.h            | 148 +++++++++++++++++++++++
 include/linux/mtd/rawnand.h              |   4 +-
 include/video/mipi_dbi.h                 |   7 +-
 27 files changed, 345 insertions(+), 272 deletions(-)
 create mode 100644 include/linux/gpio/consumer.h

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 2736efce95ad..cee60f7a65fc 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -12,7 +12,7 @@
 #include <common.h>
 #include <malloc.h>
 #include <driver.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <regulator.h>
 #include <xfuncs.h>
 #include <errno.h>
@@ -419,7 +419,8 @@ static int pca953x_probe(struct device *dev)
 	struct pca953x_platform_data *pdata;
 	struct pca953x_chip *chip;
 	struct regulator *reg;
-	int reset_gpio, ret;
+	struct gpio_desc *reset_gpio;
+	int ret;
 	u32 invert = 0;
 
 	chip = xzalloc(sizeof(struct pca953x_chip));
@@ -442,8 +443,8 @@ static int pca953x_probe(struct device *dev)
 
 	chip->client = client;
 
-	reset_gpio = gpiod_get(dev, "reset", GPIOD_OUT_LOW);
-	if (!gpio_is_valid(reset_gpio) && reset_gpio != -ENOENT)
+	reset_gpio = gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(reset_gpio))
 		dev_warn(dev, "Failed to get 'reset' GPIO (ignored)\n");
 
 	reg = regulator_get(dev, "vcc");
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 368fea919835..ee1bc5f6156f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -7,7 +7,7 @@
 #include <complete.h>
 #include <gpio.h>
 #include <of_gpio.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <errno.h>
 #include <malloc.h>
 
@@ -670,19 +670,20 @@ static const char *gpio_suffixes[] = {
 };
 
 /* Linux compatibility helper: Get a GPIO descriptor from device tree */
-int dev_gpiod_get_index(struct device *dev,
+struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 			struct device_node *np,
 			const char *_con_id, int index,
 			enum gpiod_flags flags,
 			const char *label)
 {
+	struct gpio_desc *desc = NULL;
 	enum of_gpio_flags of_flags;
 	char *buf = NULL, *con_id;
 	int gpio;
 	int ret, i;
 
 	if (!np)
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);
 
 	for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
 		if (_con_id)
@@ -691,17 +692,19 @@ int dev_gpiod_get_index(struct device *dev,
 			con_id = basprintf("%s", gpio_suffixes[i]);
 
 		if (!con_id)
-			return -ENOMEM;
+			return ERR_PTR(-ENOMEM);
 
 		gpio = of_get_named_gpio_flags(np, con_id, index, &of_flags);
 		free(con_id);
 
-		if (gpio_is_valid(gpio))
+		if (gpio_is_valid(gpio)) {
+			desc = __tmp_gpio_to_desc(gpio);
 			break;
+		}
 	}
 
-	if (!gpio_is_valid(gpio))
-		return gpio < 0 ? gpio : -EINVAL;
+	if (!desc)
+		return ERR_PTR(gpio < 0 ? gpio : -EINVAL);
 
 	if (of_flags & OF_GPIO_ACTIVE_LOW)
 		flags |= GPIOF_ACTIVE_LOW;
@@ -715,10 +718,10 @@ int dev_gpiod_get_index(struct device *dev,
 			label = dev_name(dev);
 	}
 
-	ret = gpio_request_one(gpio, flags, label);
+	ret = gpio_request_one(__tmp_desc_to_gpio(desc), flags, label);
 	free(buf);
 
-	return ret ?: gpio;
+	return ret ? ERR_PTR(ret): desc;
 }
 #endif
 
diff --git a/drivers/mci/mci_spi.c b/drivers/mci/mci_spi.c
index 593ec5c47c14..ad743d19d9f8 100644
--- a/drivers/mci/mci_spi.c
+++ b/drivers/mci/mci_spi.c
@@ -19,7 +19,7 @@
 #include <crc.h>
 #include <crc7.h>
 #include <of.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 
 #define to_spi_host(mci) container_of(mci, struct mmc_spi_host, mci)
 #define spi_setup(spi) spi->master->setup(spi)
@@ -49,7 +49,7 @@ struct mmc_spi_host {
 	struct mci_host	mci;
 	struct spi_device	*spi;
 	struct device	*dev;
-	int detect_pin;
+	struct gpio_desc *detect_pin;
 
 	/* for bulk data transfers */
 	struct spi_transfer	t_tx;
@@ -360,10 +360,10 @@ static int spi_mci_card_present(struct mci_host *mci)
 	int			ret;
 
 	/* No gpio, assume card is present */
-	if (!gpio_is_valid(host->detect_pin))
+	if (IS_ERR_OR_NULL(host->detect_pin))
 		return 1;
 
-	ret = gpio_get_value(host->detect_pin);
+	ret = gpiod_get_value(host->detect_pin);
 
 	return ret == 0 ? 1 : 0;
 }
@@ -434,11 +434,12 @@ static int spi_mci_probe(struct device *dev)
 
 	host->mci.voltages = MMC_VDD_32_33 | MMC_VDD_33_34;
 	host->mci.host_caps = MMC_CAP_SPI;
-	host->detect_pin = -EINVAL;
 
 	if (np) {
 		host->mci.devname = xstrdup(of_alias_get(np));
-		host->detect_pin = gpiod_get(dev, NULL, GPIOD_IN);
+		host->detect_pin = gpiod_get_optional(dev, NULL, GPIOD_IN);
+		if (IS_ERR(host->detect_pin))
+			dev_warn(dev, "Failed to get 'reset' GPIO (ignored)\n");
 	}
 
 	mci_register(&host->mci);
diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c
index 6cd770074131..cb74b365c8e4 100644
--- a/drivers/mtd/nand/atmel/nand-controller.c
+++ b/drivers/mtd/nand/atmel/nand-controller.c
@@ -47,7 +47,7 @@
 
 #include <linux/clk.h>
 #include <linux/genalloc.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <mfd/syscon.h>
 #include <linux/mfd/syscon/atmel-matrix.h>
 #include <linux/mfd/syscon/atmel-smc.h>
@@ -130,7 +130,7 @@ enum atmel_nand_rb_type {
 struct atmel_nand_rb {
 	enum atmel_nand_rb_type type;
 	union {
-		int gpio;
+		struct gpio_desc *gpio;
 		int id;
 	};
 };
@@ -138,7 +138,7 @@ struct atmel_nand_rb {
 struct atmel_nand_cs {
 	int id;
 	struct atmel_nand_rb rb;
-	int csgpio;
+	struct gpio_desc *csgpio;
 	struct {
 		void __iomem *virt;
 	} io;
@@ -152,7 +152,7 @@ struct atmel_nand {
 	struct nand_chip base;
 	struct atmel_nand_cs *activecs;
 	struct atmel_pmecc_user *pmecc;
-	int cdgpio;
+	struct gpio_desc *cdgpio;
 	int numcs;
 	struct atmel_nand_cs cs[];
 };
@@ -1437,7 +1437,7 @@ static struct atmel_nand *atmel_nand_create(struct atmel_nand_controller *nc,
 					    int reg_cells)
 {
 	struct atmel_nand *nand;
-	int gpio;
+	struct gpio_desc *gpio;
 	int numcs, ret, i;
 
 	numcs = of_property_count_elems_of_size(np, "reg",
@@ -1451,16 +1451,17 @@ static struct atmel_nand *atmel_nand_create(struct atmel_nand_controller *nc,
 	if (!nand)
 		return ERR_PTR(-ENOMEM);
 
-	nand->cdgpio = -ENOENT;
 	nand->numcs = numcs;
 
 	gpio = dev_gpiod_get(nc->dev, np, "det", GPIOD_IN, "nand-det");
-	if (gpio < 0 && gpio != -ENOENT) {
-		ret = dev_err_probe(nc->dev, gpio, "Failed to get detect gpio\n");
-		return ERR_PTR(ret);
+	if (IS_ERR(gpio) && PTR_ERR(gpio) != -ENOENT) {
+		dev_err(nc->dev,
+			"Failed to get detect gpio (err = %ld)\n",
+			PTR_ERR(gpio));
+		return ERR_CAST(gpio);
 	}
 
-	if (gpio < 0)
+	if (!IS_ERR(gpio))
 		nand->cdgpio = gpio;
 
 	for (i = 0; i < numcs; i++) {
@@ -1483,7 +1484,6 @@ static struct atmel_nand *atmel_nand_create(struct atmel_nand_controller *nc,
 		}
 
 		nand->cs[i].id = val;
-		nand->cs[i].csgpio = -ENOENT;
 
 		nand->cs[i].io.virt = IOMEM(res.start);
 		ret = dev_request_resource(nc->dev, &res);
@@ -1498,24 +1498,24 @@ static struct atmel_nand *atmel_nand_create(struct atmel_nand_controller *nc,
 			nand->cs[i].rb.id = val;
 		} else {
 			gpio = dev_gpiod_get_index(nc->dev, np, "rb", i, GPIOD_IN, "nand-rb");
-			if (gpio < 0 && gpio != -ENOENT) {
-				ret = dev_err_probe(nc->dev, gpio, "Failed to get R/B gpio\n");
-				return ERR_PTR(ret);
+			if (IS_ERR(gpio) && PTR_ERR(gpio) != -ENOENT) {
+				dev_errp_probe(nc->dev, gpio, "Failed to get detect gpio\n");
+				return ERR_CAST(gpio);
 			}
 
-			if (gpio < 0) {
+			if (!IS_ERR(gpio)) {
 				nand->cs[i].rb.type = ATMEL_NAND_GPIO_RB;
 				nand->cs[i].rb.gpio = gpio;
 			}
 		}
 
 		gpio = dev_gpiod_get_index(nc->dev, np, "cs", i, GPIOD_OUT_HIGH, "nand-cs");
-		if (gpio < 0 && gpio != -ENOENT) {
-			ret = dev_err_probe(nc->dev, gpio, "Failed to get CS gpio\n");
-			return ERR_PTR(ret);
+		if (IS_ERR(gpio) && PTR_ERR(gpio) != -ENOENT) {
+			dev_errp_probe(nc->dev, gpio, "Failed to get CS gpio\n");
+			return ERR_CAST(gpio);
 		}
 
-		if (gpio < 0)
+		if (!IS_ERR(gpio))
 			nand->cs[i].csgpio = gpio;
 	}
 
@@ -1533,7 +1533,7 @@ atmel_nand_controller_add_nand(struct atmel_nand_controller *nc,
 	int ret;
 
 	/* No card inserted, skip this NAND. */
-	if (gpio_is_valid(nand->cdgpio) && gpiod_get_value(nand->cdgpio)) {
+	if (nand->cdgpio && gpiod_get_value(nand->cdgpio)) {
 		dev_info(nc->dev, "No SmartMedia card inserted.\n");
 		return 0;
 	}
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 55218b120210..2599e8c8c2c7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -36,7 +36,7 @@
 #include <asm/byteorder.h>
 #include <io.h>
 #include <malloc.h>
-#include <gpio.h>
+#include <linux/gpio/consumer.h>
 #include <module.h>
 #include <of_mtd.h>
 
@@ -810,10 +810,10 @@ EXPORT_SYMBOL_GPL(nand_soft_waitrdy);
  *
  * Return 0 if the R/B pin indicates chip is ready, a negative error otherwise.
  */
-int nand_gpio_waitrdy(struct nand_chip *chip, int gpio,
+int nand_gpio_waitrdy(struct nand_chip *chip, struct gpio_desc *gpio,
 		      unsigned long timeout_ms)
 {
-	return gpio_poll_timeout_us(gpio, true, timeout_ms * USEC_PER_MSEC);
+	return gpiod_poll_timeout_us(gpio, true, timeout_ms * USEC_PER_MSEC);
 };
 EXPORT_SYMBOL_GPL(nand_gpio_waitrdy);
 
diff --git a/drivers/net/designware_eqos.c b/drivers/net/designware_eqos.c
index 3fa5bf58c2cc..5e5c9ebe6875 100644
--- a/drivers/net/designware_eqos.c
+++ b/drivers/net/designware_eqos.c
@@ -9,7 +9,7 @@
 #include <common.h>
 #include <init.h>
 #include <gpio.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <dma.h>
 #include <net.h>
 #include <of_net.h>
@@ -200,21 +200,21 @@ struct eqos_desc {
 
 static int eqos_phy_reset(struct device *dev, struct eqos *eqos)
 {
-	int phy_reset;
+	struct gpio_desc *phy_reset;
 	u32 delays[3] = { 0, 0, 0 };
 
-	phy_reset = gpiod_get(dev, "snps,reset", GPIOF_OUT_INIT_ACTIVE);
+	phy_reset = gpiod_get_optional(dev, "snps,reset", GPIOF_OUT_INIT_ACTIVE);
+	if (IS_ERR(phy_reset)) {
+		dev_warn(dev, "Failed to get 'snps,reset' GPIO (ignored)\n");
+	} else if (phy_reset) {
+		of_property_read_u32_array(dev->of_node,
+					   "snps,reset-delays-us",
+					   delays, ARRAY_SIZE(delays));
 
-	if (!gpio_is_valid(phy_reset))
-		return 0;
-
-	of_property_read_u32_array(dev->of_node,
-				   "snps,reset-delays-us",
-				   delays, ARRAY_SIZE(delays));
-
-	udelay(delays[1]);
-	gpio_set_active(phy_reset, false);
-	udelay(delays[2]);
+		udelay(delays[1]);
+		gpiod_set_value(phy_reset, false);
+		udelay(delays[2]);
+	}
 
 	return 0;
 }
diff --git a/drivers/net/ksz8873.c b/drivers/net/ksz8873.c
index 35ef0d2da2fb..7e98ec492a71 100644
--- a/drivers/net/ksz8873.c
+++ b/drivers/net/ksz8873.c
@@ -3,7 +3,7 @@
 #include <common.h>
 #include <complete.h>
 #include <dsa.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <linux/mii.h>
 #include <linux/mdio.h>
 #include <net.h>
@@ -369,7 +369,8 @@ static int ksz8873_probe_mdio(struct phy_device *mdiodev)
 	const struct ksz8873_dcfg *dcfg;
 	struct ksz8873_switch *priv;
 	struct dsa_switch *ds;
-	int ret, gpio;
+	struct gpio_desc *gpio;
+	int ret;
 	u8 id0, id1;
 
 	priv = xzalloc(sizeof(*priv));
@@ -389,10 +390,12 @@ static int ksz8873_probe_mdio(struct phy_device *mdiodev)
 		return dev_err_probe(dev, PTR_ERR(priv->regmap),
 				     "Failed to initialize regmap.\n");
 
-	gpio = gpiod_get(dev, "reset", GPIOF_OUT_INIT_ACTIVE);
-	if (gpio_is_valid(gpio)) {
+	gpio = gpiod_get_optional(dev, "reset", GPIOF_OUT_INIT_ACTIVE);
+	if (IS_ERR(gpio)) {
+		dev_warn(dev, "Failed to get 'reset' GPIO (ignored)\n");
+	} else if (gpio) {
 		mdelay(1);
-		gpio_set_active(gpio, false);
+		gpiod_set_value(gpio, false);
 	}
 
 	ret = ksz_read8(priv, KSZ8873_CHIP_ID0, &id0);
diff --git a/drivers/net/ksz9477.c b/drivers/net/ksz9477.c
index 4b876055f6e8..61142aa97b1a 100644
--- a/drivers/net/ksz9477.c
+++ b/drivers/net/ksz9477.c
@@ -3,7 +3,7 @@
 #include <common.h>
 #include <complete.h>
 #include <dsa.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <net.h>
 #include <platform_data/ksz9477_reg.h>
 #include <spi/spi.h>
@@ -410,7 +410,8 @@ static int microchip_switch_probe(struct device *dev)
 {
 	struct device *hw_dev;
 	struct ksz_switch *priv;
-	int ret = 0, gpio;
+	struct gpio_desc *gpio;
+	int ret = 0;
 	struct dsa_switch *ds;
 
 	priv = xzalloc(sizeof(*priv));
@@ -432,10 +433,12 @@ static int microchip_switch_probe(struct device *dev)
 	if (ret)
 		return ret;
 
-	gpio = gpiod_get(dev, "reset", GPIOF_OUT_INIT_ACTIVE);
-	if (gpio_is_valid(gpio)) {
+	gpio = gpiod_get_optional(dev, "reset", GPIOF_OUT_INIT_ACTIVE);
+	if (IS_ERR(gpio)) {
+		dev_warn(dev, "Failed to get 'reset' GPIO (ignored)\n");
+	} else if (gpio) {
 		mdelay(1);
-		gpio_set_active(gpio, false);
+		gpiod_set_value(gpio, false);
 	}
 
 	ksz_reset_switch(dev->priv);
diff --git a/drivers/net/realtek-dsa/realtek-mdio.c b/drivers/net/realtek-dsa/realtek-mdio.c
index a869308035c4..6776609b0480 100644
--- a/drivers/net/realtek-dsa/realtek-mdio.c
+++ b/drivers/net/realtek-dsa/realtek-mdio.c
@@ -22,7 +22,7 @@
 #include <of_device.h>
 #include <regmap.h>
 #include <clock.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <linux/printk.h>
 #include <linux/mdio.h>
 
@@ -151,11 +151,11 @@ static int realtek_mdio_probe(struct phy_device *mdiodev)
 	/* TODO: if power is software controlled, set up any regulators here */
 	priv->leds_disabled = of_property_read_bool(np, "realtek,disable-leds");
 
-	priv->reset = gpiod_get(dev, "reset", GPIOD_OUT_LOW);
-	if (priv->reset < 0 && priv->reset != -ENOENT)
-		return dev_err_probe(dev, priv->reset, "failed to get RESET GPIO\n");
+	priv->reset = gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->reset))
+		return dev_errp_probe(dev, priv->reset, "failed to get RESET GPIO\n");
 
-	if (gpio_is_valid(priv->reset)) {
+	if (priv->reset) {
 		gpiod_set_value(priv->reset, 1);
 		dev_dbg(dev, "asserted RESET\n");
 		mdelay(REALTEK_HW_STOP_DELAY);
diff --git a/drivers/net/realtek-dsa/realtek-smi.c b/drivers/net/realtek-dsa/realtek-smi.c
index 35b3d30a060e..8e6f0e2ad94f 100644
--- a/drivers/net/realtek-dsa/realtek-smi.c
+++ b/drivers/net/realtek-dsa/realtek-smi.c
@@ -33,7 +33,7 @@
 #include <linux/mdio.h>
 #include <linux/printk.h>
 #include <clock.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <driver.h>
 #include <regmap.h>
 #include <linux/bitops.h>
@@ -413,11 +413,11 @@ static int realtek_smi_probe(struct device *dev)
 
 	/* TODO: if power is software controlled, set up any regulators here */
 
-	priv->reset = gpiod_get(dev, "reset", GPIOD_OUT_LOW);
-	if (priv->reset < 0 && priv->reset != -ENOENT)
-		return dev_err_probe(dev, priv->reset, "failed to get RESET GPIO\n");
+	priv->reset = gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->reset))
+		return dev_errp_probe(dev, priv->reset, "failed to get RESET GPIO\n");
 
-	if (gpio_is_valid(priv->reset)) {
+	if (priv->reset) {
 		gpiod_set_value(priv->reset, 1);
 		dev_dbg(dev, "asserted RESET\n");
 		mdelay(REALTEK_HW_STOP_DELAY);
@@ -428,12 +428,12 @@ static int realtek_smi_probe(struct device *dev)
 
 	/* Fetch MDIO pins */
 	priv->mdc = gpiod_get(dev, "mdc", GPIOD_OUT_LOW);
-	if (!gpio_is_valid(priv->mdc))
-		return dev_err_probe(dev, priv->mdc, "failed to get MDC GPIO\n");
+	if (IS_ERR(priv->mdc))
+		return dev_errp_probe(dev, priv->mdc, "failed to get MDC GPIO\n");
 
 	priv->mdio = gpiod_get(dev, "mdio", GPIOD_OUT_LOW);
-	if (!gpio_is_valid(priv->mdio))
-		return dev_err_probe(dev, priv->mdio, "failed to get MDIO GPIO\n");
+	if (IS_ERR(priv->mdio))
+		return dev_errp_probe(dev, priv->mdio, "failed to get MDIO GPIO\n");
 
 	priv->leds_disabled = of_property_read_bool(np, "realtek,disable-leds");
 
diff --git a/drivers/net/realtek-dsa/realtek.h b/drivers/net/realtek-dsa/realtek.h
index 46a2282f6e6d..ac84b18cdd14 100644
--- a/drivers/net/realtek-dsa/realtek.h
+++ b/drivers/net/realtek-dsa/realtek.h
@@ -21,9 +21,9 @@ struct realtek_ops;
 
 struct realtek_priv {
 	struct device		*dev;
-	int			reset;
-	int			mdc;
-	int			mdio;
+	struct gpio_desc	*reset;
+	struct gpio_desc	*mdc;
+	struct gpio_desc	*mdio;
 	union {
 		struct regmap		*map;
 		struct regmap		*map_nolock;
diff --git a/drivers/net/sja1105.c b/drivers/net/sja1105.c
index ef874ed4829d..d88a5e2fcf24 100644
--- a/drivers/net/sja1105.c
+++ b/drivers/net/sja1105.c
@@ -10,7 +10,7 @@
 
 #include <common.h>
 #include <dsa.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <linux/bitrev.h>
 #include <linux/if_vlan.h>
 #include <net.h>
@@ -2883,18 +2883,19 @@ static int sja1105_check_device_id(struct sja1105_private *priv)
 static int sja1105_hw_reset(struct device *dev, unsigned int pulse_len,
 			    unsigned int startup_delay)
 {
-	int gpio;
+	struct gpio_desc *gpio;
 
-	gpio = gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
-	if (gpio < 0)
-		return 0;
-
-	gpiod_set_value(gpio, 1);
-	/* Wait for minimum reset pulse length */
-	mdelay(pulse_len);
-	gpiod_set_value(gpio, 0);
-	/* Wait until chip is ready after reset */
-	mdelay(startup_delay);
+	gpio = gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio)) {
+		dev_warn(dev, "Failed to get 'reset' GPIO (ignored)\n");
+	} else if (gpio) {
+		gpiod_set_value(gpio, 1);
+		/* Wait for minimum reset pulse length */
+		mdelay(pulse_len);
+		gpiod_set_value(gpio, 0);
+		/* Wait until chip is ready after reset */
+		mdelay(startup_delay);
+	}
 
 	return 0;
 }
diff --git a/drivers/nvmem/starfive-otp.c b/drivers/nvmem/starfive-otp.c
index d672b93eddf4..91069dc78ebf 100644
--- a/drivers/nvmem/starfive-otp.c
+++ b/drivers/nvmem/starfive-otp.c
@@ -8,7 +8,7 @@
 #include <malloc.h>
 #include <xfuncs.h>
 #include <errno.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <init.h>
 #include <net.h>
 #include <io.h>
@@ -87,7 +87,7 @@
  */
 
 struct starfive_otp {
-	int power_gpio;
+	struct gpio_desc *power_gpio;
 	struct starfive_otp_regs __iomem *regs;
 };
 
@@ -112,7 +112,7 @@ static int starfive_otp_read(void *ctx, unsigned offset, unsigned *val)
 {
 	struct starfive_otp *priv = ctx;
 
-	gpio_set_active(priv->power_gpio, true);
+	gpiod_set_value(priv->power_gpio, true);
 	mdelay(10);
 
 	//otp set to read mode
@@ -122,7 +122,7 @@ static int starfive_otp_read(void *ctx, unsigned offset, unsigned *val)
 	/* read all requested fuses */
 	*val = readl(&priv->regs->mem[offset / 4]);
 
-	gpio_set_active(priv->power_gpio, false);
+	gpiod_set_value(priv->power_gpio, false);
 	mdelay(5);
 
 	return 0;
@@ -178,8 +178,8 @@ static int starfive_otp_probe(struct device *dev)
 
 	priv->regs = IOMEM(iores->start);
 	priv->power_gpio = gpiod_get(dev, "power", GPIOD_OUT_LOW);
-	if (priv->power_gpio < 0)
-		return priv->power_gpio;
+	if (IS_ERR(priv->power_gpio))
+		return PTR_ERR(priv->power_gpio);
 
 	map = regmap_init(dev, &starfive_otp_regmap_bus, priv, &config);
 	if (IS_ERR(map))
diff --git a/drivers/pci/pcie-dw-rockchip.c b/drivers/pci/pcie-dw-rockchip.c
index def32c988e28..cc771e2cae20 100644
--- a/drivers/pci/pcie-dw-rockchip.c
+++ b/drivers/pci/pcie-dw-rockchip.c
@@ -22,7 +22,7 @@
 #include <linux/kernel.h>
 #include <of_address.h>
 #include <of_pci.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <linux/pci.h>
 #include <linux/phy/phy.h>
 #include <linux/reset.h>
@@ -61,7 +61,7 @@ struct rockchip_pcie {
 	struct clk_bulk_data		*clks;
 	unsigned int			clk_cnt;
 	struct reset_control		*rst;
-	int				rst_gpio;
+	struct gpio_desc		*rst_gpio;
 	struct regulator                *vpcie3v3;
 	struct irq_domain		*irq_domain;
 };
@@ -106,7 +106,7 @@ static int rockchip_pcie_start_link(struct dw_pcie *pci)
 	struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
 
 	/* Reset device */
-	gpio_set_value(rockchip->rst_gpio, 0);
+	gpiod_set_value(rockchip->rst_gpio, 0);
 
 	rockchip_pcie_enable_ltssm(rockchip);
 
@@ -120,7 +120,7 @@ static int rockchip_pcie_start_link(struct dw_pcie *pci)
 	 * 100us as we don't know how long should the device need to reset.
 	 */
 	mdelay(100);
-	gpio_set_value(rockchip->rst_gpio, 1);
+	gpiod_set_value(rockchip->rst_gpio, 1);
 
 	return 0;
 }
@@ -177,9 +177,9 @@ static int rockchip_pcie_resource_get(struct device *dev,
 		return PTR_ERR(r);
 	rockchip->pci.dbi_base = IOMEM(r->start);
 
-	rockchip->rst_gpio = gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
-	if (rockchip->rst_gpio < 0 && rockchip->rst_gpio != -ENOENT)
-		return rockchip->rst_gpio;
+	rockchip->rst_gpio = gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(rockchip->rst_gpio))
+		return PTR_ERR(rockchip->rst_gpio);
 
 	rockchip->rst = reset_control_array_get(dev);
 	if (IS_ERR(rockchip->rst))
diff --git a/drivers/power/reset/gpio-poweroff.c b/drivers/power/reset/gpio-poweroff.c
index 48a479814e6b..cafa1387ce0b 100644
--- a/drivers/power/reset/gpio-poweroff.c
+++ b/drivers/power/reset/gpio-poweroff.c
@@ -10,14 +10,14 @@
 #include <common.h>
 #include <driver.h>
 #include <poweroff.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 
 #define DEFAULT_TIMEOUT_MS 3000
 /*
  * Hold configuration here, cannot be more than one instance of the driver
  * since pm_power_off itself is global.
  */
-static int reset_gpio;
+static struct gpio_desc *reset_gpio;
 static u32 timeout = DEFAULT_TIMEOUT_MS;
 static u32 active_delay = 100;
 static u32 inactive_delay = 100;
@@ -25,15 +25,15 @@ static u32 inactive_delay = 100;
 static void gpio_poweroff_do_poweroff(struct poweroff_handler *handler)
 {
 	/* drive it active, also inactive->active edge */
-	gpio_direction_active(reset_gpio, true);
+	gpiod_direction_output(reset_gpio, true);
 	mdelay(active_delay);
 
 	/* drive inactive, also active->inactive edge */
-	gpio_set_active(reset_gpio, false);
+	gpiod_set_value(reset_gpio, false);
 	mdelay(inactive_delay);
 
 	/* drive it active, also inactive->active edge */
-	gpio_set_active(reset_gpio, true);
+	gpiod_set_value(reset_gpio, true);
 
 	/* give it some time */
 	mdelay(timeout);
@@ -65,8 +65,8 @@ static int gpio_poweroff_probe(struct device *dev)
 	of_property_read_u32(np, "timeout-ms", &timeout);
 
 	reset_gpio = gpiod_get(dev, NULL, flags);
-	if (reset_gpio < 0)
-		return reset_gpio;
+	if (IS_ERR(reset_gpio))
+		return PTR_ERR(reset_gpio);
 
 	handler.name = "gpio-poweroff";
 	handler.poweroff = gpio_poweroff_do_poweroff;
diff --git a/drivers/power/reset/gpio-restart.c b/drivers/power/reset/gpio-restart.c
index 946acd4164c4..ed2748f91092 100644
--- a/drivers/power/reset/gpio-restart.c
+++ b/drivers/power/reset/gpio-restart.c
@@ -9,10 +9,10 @@
 #include <common.h>
 #include <driver.h>
 #include <restart.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 
 struct gpio_restart {
-	int reset_gpio;
+	struct gpio_desc *reset_gpio;
 	struct restart_handler restart_handler;
 	u32 active_delay_ms;
 	u32 inactive_delay_ms;
@@ -25,15 +25,15 @@ static void __noreturn gpio_restart_handle(struct restart_handler *this)
 		container_of(this, struct gpio_restart, restart_handler);
 
 	/* drive it active, also inactive->active edge */
-	gpio_direction_active(gpio_restart->reset_gpio, true);
+	gpiod_direction_output(gpio_restart->reset_gpio, true);
 	mdelay(gpio_restart->active_delay_ms);
 
 	/* drive inactive, also active->inactive edge */
-	gpio_set_active(gpio_restart->reset_gpio, false);
+	gpiod_direction_output(gpio_restart->reset_gpio, false);
 	mdelay(gpio_restart->inactive_delay_ms);
 
 	/* drive it active, also inactive->active edge */
-	gpio_set_active(gpio_restart->reset_gpio, true);
+	gpiod_direction_output(gpio_restart->reset_gpio, true);
 
 	/* give it some time */
 	mdelay(gpio_restart->wait_delay_ms);
@@ -47,6 +47,7 @@ static int gpio_restart_probe(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	struct gpio_restart *gpio_restart;
+	struct gpio_desc *gpiod;
 	bool open_source = false;
 	u32 property;
 	int ret;
@@ -55,15 +56,11 @@ static int gpio_restart_probe(struct device *dev)
 
 	open_source = of_property_read_bool(np, "open-source");
 
-	gpio_restart->reset_gpio = gpiod_get(dev, NULL,
-			open_source ? GPIOD_IN : GPIOD_OUT_LOW);
-	ret = gpio_restart->reset_gpio;
-	if (ret < 0) {
-		if (ret != -EPROBE_DEFER)
-			dev_err(dev, "Could not get reset GPIO\n");
-		return ret;
-	}
+	gpiod = gpiod_get(dev, NULL, open_source ? GPIOD_IN : GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod))
+		return dev_errp_probe(dev, gpiod, "Could not get reset GPIO\n");
 
+	gpio_restart->reset_gpio = gpiod;
 	gpio_restart->restart_handler.restart = gpio_restart_handle;
 	gpio_restart->restart_handler.name = "gpio-restart";
 	gpio_restart->restart_handler.priority = 129;
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 10428cac7ad3..0edb5ceb104b 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -9,12 +9,10 @@
 #include <init.h>
 #include <regulator.h>
 #include <of.h>
-#include <of_gpio.h>
-#include <gpio.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 
 struct regulator_fixed {
-	int gpio;
+	struct gpio_desc *gpio;
 	struct regulator_dev rdev;
 	struct regulator_desc rdesc;
 };
@@ -23,20 +21,14 @@ static int regulator_fixed_enable(struct regulator_dev *rdev)
 {
 	struct regulator_fixed *fix = container_of(rdev, struct regulator_fixed, rdev);
 
-	if (!gpio_is_valid(fix->gpio))
-		return 0;
-
-	return gpio_direction_active(fix->gpio, true);
+	return gpiod_direction_output(fix->gpio, true);
 }
 
 static int regulator_fixed_disable(struct regulator_dev *rdev)
 {
 	struct regulator_fixed *fix = container_of(rdev, struct regulator_fixed, rdev);
 
-	if (!gpio_is_valid(fix->gpio))
-		return 0;
-
-	return gpio_direction_active(fix->gpio, false);
+	return gpiod_direction_output(fix->gpio, false);
 }
 
 const static struct regulator_ops fixed_ops = {
@@ -55,14 +47,11 @@ static int regulator_fixed_probe(struct device *dev)
 		return -EINVAL;
 
 	fix = xzalloc(sizeof(*fix));
-	fix->gpio = -EINVAL;
 
-	if (of_get_property(np, "gpio", NULL)) {
-		fix->gpio = gpiod_get(dev, NULL, GPIOD_ASIS);
-		if (fix->gpio < 0) {
-			ret = fix->gpio;
-			goto err;
-		}
+	fix->gpio = gpiod_get_optional(dev, NULL, GPIOD_ASIS);
+	if (IS_ERR(fix->gpio)) {
+		ret = PTR_ERR(fix->gpio);
+		goto err;
 	}
 
 	fix->rdesc.ops = &fixed_ops;
diff --git a/drivers/sound/gpio-beeper.c b/drivers/sound/gpio-beeper.c
index b809e999bf8b..8fc3ce941016 100644
--- a/drivers/sound/gpio-beeper.c
+++ b/drivers/sound/gpio-beeper.c
@@ -7,10 +7,10 @@
 #include <regulator.h>
 #include <sound.h>
 #include <of.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 
 struct gpio_beeper {
-	int gpio;
+	struct gpio_desc *gpio;
 	struct sound_card card;
 };
 
@@ -18,7 +18,7 @@ static int gpio_beeper_beep(struct sound_card *card, unsigned freq, unsigned dur
 {
 	struct gpio_beeper *beeper = container_of(card, struct gpio_beeper, card);
 
-	gpio_set_active(beeper->gpio, freq);
+	gpiod_set_value(beeper->gpio, freq);
 	return 0;
 }
 
@@ -27,13 +27,11 @@ static int gpio_beeper_probe(struct device *dev)
 	struct device_node *np = dev->of_node;
 	struct gpio_beeper *beeper;
 	struct sound_card *card;
-	int gpio;
+	struct gpio_desc *gpio;
 
 	gpio = gpiod_get(dev, NULL, GPIOD_OUT_LOW);
-	if (gpio < 0) {
-		dev_err(dev, "failed to request gpio: %pe\n", ERR_PTR(gpio));
-		return gpio;
-	}
+	if (IS_ERR(gpio))
+		return dev_errp_probe(dev, gpio, "failed to request gpio\n");
 
 	beeper = xzalloc(sizeof(*beeper));
 	beeper->gpio = gpio;
diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
index b5c68098fdcf..6339a3e4ec18 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -6,7 +6,7 @@
  */
 
 #include <driver.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <init.h>
 #include <of.h>
 #include <linux/printk.h>
@@ -31,7 +31,7 @@ struct onboard_hub {
 	struct regulator *vdd;
 	struct device *dev;
 	const struct onboard_hub_pdata *pdata;
-	int reset_gpio;
+	struct gpio_desc *reset_gpio;
 };
 
 static int onboard_hub_power_on(struct onboard_hub *hub)
@@ -65,9 +65,10 @@ static int onboard_hub_probe(struct device *dev)
 	if (IS_ERR(hub->vdd))
 		return PTR_ERR(hub->vdd);
 
-	hub->reset_gpio = gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
-	if (hub->reset_gpio < 0 && hub->reset_gpio != -ENOENT)
-		return dev_err_probe(dev, hub->reset_gpio, "failed to get reset GPIO\n");
+	hub->reset_gpio = gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(hub->reset_gpio))
+		return dev_errp_probe(dev, hub->reset_gpio,
+				     "failed to get reset GPIO\n");
 
 	hub->dev = dev;
 
diff --git a/drivers/video/mipi_dbi.c b/drivers/video/mipi_dbi.c
index a2333150b9d8..2f8d6ecc72cf 100644
--- a/drivers/video/mipi_dbi.c
+++ b/drivers/video/mipi_dbi.c
@@ -11,7 +11,7 @@
 #include <dma.h>
 #include <linux/kernel.h>
 #include <linux/sizes.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <regulator.h>
 #include <spi/spi.h>
 #include <video/backlight.h>
@@ -421,7 +421,7 @@ int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev, struct fb_ops *ops,
  */
 void mipi_dbi_hw_reset(struct mipi_dbi *dbi)
 {
-	if (!gpio_is_valid(dbi->reset))
+	if (!dbi->reset)
 		return;
 
 	gpiod_set_value(dbi->reset, 0);
@@ -671,14 +671,14 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *dbi, u8 *cmd,
  * Zero on success, negative error code on failure.
  */
 int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *dbi,
-		      int dc)
+		      struct gpio_desc *dc)
 {
 	struct device *dev = &spi->dev;
 
 	dbi->spi = spi;
 	dbi->read_commands = mipi_dbi_dcs_read_commands;
 
-	if (!gpio_is_valid(dc)) {
+	if (!dc) {
 		dev_dbg(dev, "MIPI DBI Type-C 1 unsupported\n");
 		return -ENOSYS;
 	}
diff --git a/drivers/video/panel-ilitek-ili9341.c b/drivers/video/panel-ilitek-ili9341.c
index 3c2ca5b681f2..4d03a8513e4b 100644
--- a/drivers/video/panel-ilitek-ili9341.c
+++ b/drivers/video/panel-ilitek-ili9341.c
@@ -15,7 +15,7 @@
 
 #include <common.h>
 #include <linux/bitops.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <of.h>
 #include <regulator.h>
 #include <spi/spi.h>
@@ -162,8 +162,8 @@ struct ili9341 {
 	struct device *dev;
 	struct vpl vpl;
 	const struct ili9341_config *conf;
-	int reset_gpio;
-	int dc_gpio;
+	struct gpio_desc *reset_gpio;
+	struct gpio_desc *dc_gpio;
 	struct mipi_dbi *dbi;
 	u32 max_spi_speed;
 	struct regulator_bulk_data supplies[3];
@@ -458,7 +458,8 @@ static int ili9341_ioctl(struct vpl *vpl, unsigned int port,
 	}
 }
 
-static int ili9341_dpi_probe(struct spi_device *spi, int dc, int reset)
+static int ili9341_dpi_probe(struct spi_device *spi,
+			     struct gpio_desc *dc, struct gpio_desc *reset)
 {
 	struct device *dev = &spi->dev;
 	struct ili9341 *ili;
@@ -508,14 +509,14 @@ static int ili9341_dpi_probe(struct spi_device *spi, int dc, int reset)
 static int ili9341_probe(struct device *dev)
 {
 	struct spi_device *spi = to_spi_device(dev);
-	int dc, reset;
+	struct gpio_desc *dc, *reset;
 
-	reset = gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
-	if (!gpio_is_valid(reset) && reset != -ENOENT)
+	reset = gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset))
 		dev_err(dev, "Failed to get gpio 'reset'\n");
 
 	dc = gpiod_get(dev, "dc", GPIOD_OUT_LOW);
-	if (!gpio_is_valid(dc) && dc != -ENOENT)
+	if (IS_ERR(dc))
 		dev_err(dev, "Failed to get gpio 'dc'\n");
 
 	return ili9341_dpi_probe(spi, dc, reset);
diff --git a/drivers/video/panel-mipi-dbi.c b/drivers/video/panel-mipi-dbi.c
index 7fada69d6f26..fecb232796cc 100644
--- a/drivers/video/panel-mipi-dbi.c
+++ b/drivers/video/panel-mipi-dbi.c
@@ -9,7 +9,7 @@
 #include <common.h>
 #include <fb.h>
 #include <firmware.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 #include <linux/printk.h>
 #include <of.h>
 #include <regulator.h>
@@ -263,7 +263,7 @@ static int panel_mipi_dbi_spi_probe(struct device *dev)
 	struct spi_device *spi = to_spi_device(dev);
 	struct mipi_dbi *dbi;
 	struct fb_info *info;
-	int dc;
+	struct gpio_desc *dc;
 	int ret;
 
 	dbidev = kzalloc(sizeof(*dbidev), GFP_KERNEL);
@@ -290,13 +290,14 @@ static int panel_mipi_dbi_spi_probe(struct device *dev)
 
 	dbidev->backlight_node = of_parse_phandle(dev->of_node, "backlight", 0);
 
-	dbi->reset = gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
-	if (dbi->reset < 0 && dbi->reset != -ENOENT)
-		return dev_err_probe(dev, dbi->reset, "Failed to get GPIO 'reset'\n");
+	dbi->reset = gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(dbi->reset))
+		return dev_errp_probe(dev, dbi->reset,
+				     "Failed to get GPIO 'reset'\n");
 
-	dc = gpiod_get(dev, "dc", GPIOD_OUT_LOW);
-	if (dc < 0 && dc != -ENOENT)
-		return dev_err_probe(dev, dc, "Failed to get GPIO 'dc'\n");
+	dc = gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+	if (IS_ERR(dc))
+		return dev_errp_probe(dev, dc, "Failed to get GPIO 'dc'\n");
 
 	ret = mipi_dbi_spi_init(spi, dbi, dc);
 	if (ret)
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
index 2dbfbe4dd967..5fa98f87c6b0 100644
--- a/drivers/watchdog/gpio_wdt.c
+++ b/drivers/watchdog/gpio_wdt.c
@@ -9,7 +9,7 @@
 #include <driver.h>
 #include <watchdog.h>
 #include <superio.h>
-#include <gpiod.h>
+#include <linux/gpio/consumer.h>
 
 enum {
 	HW_ALGO_TOGGLE,
@@ -17,7 +17,7 @@ enum {
 };
 
 struct gpio_wdt_priv {
-	int		gpio;
+	struct gpio_desc *gpiod;
 	bool		state;
 	bool		started;
 	unsigned int	hw_algo;
@@ -32,11 +32,11 @@ static inline struct gpio_wdt_priv *to_gpio_wdt_priv(struct watchdog *wdd)
 static void gpio_wdt_disable(struct gpio_wdt_priv *priv)
 {
 	/* Eternal ping */
-	gpio_set_active(priv->gpio, 1);
+	gpiod_set_value(priv->gpiod, 1);
 
 	/* Put GPIO back to tristate */
 	if (priv->hw_algo == HW_ALGO_TOGGLE)
-		gpio_direction_input(priv->gpio);
+		gpiod_direction_input(priv->gpiod);
 
 	priv->started = false;
 }
@@ -47,13 +47,13 @@ static void gpio_wdt_ping(struct gpio_wdt_priv *priv)
 	case HW_ALGO_TOGGLE:
 		/* Toggle output pin */
 		priv->state = !priv->state;
-		gpio_set_active(priv->gpio, priv->state);
+		gpiod_set_value(priv->gpiod, priv->state);
 		break;
 	case HW_ALGO_LEVEL:
 		/* Pulse */
-		gpio_set_active(priv->gpio, true);
+		gpiod_set_value(priv->gpiod, true);
 		udelay(1);
-		gpio_set_active(priv->gpio, false);
+		gpiod_set_value(priv->gpiod, false);
 		break;
 	}
 }
@@ -61,7 +61,7 @@ static void gpio_wdt_ping(struct gpio_wdt_priv *priv)
 static void gpio_wdt_start(struct gpio_wdt_priv *priv)
 {
 	priv->state = false;
-	gpio_direction_active(priv->gpio, priv->state);
+	gpiod_direction_output(priv->gpiod, priv->state);
 	priv->started = true;
 }
 
@@ -113,9 +113,9 @@ static int gpio_wdt_probe(struct device *dev)
 		return -EINVAL;
 	}
 
-	priv->gpio = gpiod_get(dev, NULL, gflags);
-	if (priv->gpio < 0)
-		return priv->gpio;
+	priv->gpiod = gpiod_get(dev, NULL, gflags);
+	if (IS_ERR(priv->gpiod))
+		return PTR_ERR(priv->gpiod);
 
 	priv->wdd.hwdev		= dev;
 	priv->wdd.timeout_max	= hw_margin / 1000;
diff --git a/include/gpiod.h b/include/gpiod.h
index df02b86e1ee8..23b88c903193 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -2,83 +2,6 @@
 #ifndef __GPIOD_H_
 #define __GPIOD_H_
 
-#include <gpio.h>
-#include <of_gpio.h>
-#include <driver.h>
-
-/**
- * Optional flags that can be passed to one of gpiod_* to configure direction
- * and output value. These values cannot be OR'd.
- */
-enum gpiod_flags {
-	GPIOD_ASIS	= 0,
-	GPIOD_IN	= GPIOF_IN,
-	/*
-	 * To change this later to a different logic level (i.e. taking
-	 * active low into account), use gpiod_set_value()
-	 */
-	GPIOD_OUT_LOW	= GPIOF_OUT_INIT_INACTIVE,
-	GPIOD_OUT_HIGH	= GPIOF_OUT_INIT_ACTIVE,
-};
-
-#ifdef CONFIG_OFDEVICE
-
-/* returned gpio descriptor can be passed to any normal gpio_* function */
-int dev_gpiod_get_index(struct device *dev,
-			struct device_node *np,
-			const char *_con_id, int index,
-			enum gpiod_flags flags,
-			const char *label);
-
-#else
-static inline int dev_gpiod_get_index(struct device *dev,
-		struct device_node *np,
-		const char *_con_id, int index,
-		enum gpiod_flags flags,
-		const char *label)
-{
-	return -ENODEV;
-}
-#endif
-
-static inline int dev_gpiod_get(struct device *dev,
-				struct device_node *np,
-				const char *con_id,
-				enum gpiod_flags flags,
-				const char *label)
-{
-	return dev_gpiod_get_index(dev, np, con_id, 0, flags, label);
-}
-
-static inline int gpiod_get(struct device *dev,
-			    const char *_con_id,
-			    enum gpiod_flags flags)
-{
-	return dev_gpiod_get(dev, dev->of_node, _con_id, flags, NULL);
-}
-
-static inline void gpiod_direction_input(int gpio)
-{
-	gpio_direction_input(gpio);
-}
-
-static inline void gpiod_direction_output(int gpio, bool value)
-{
-	if (gpio != -ENOENT)
-		gpio_direction_active(gpio, value);
-}
-
-static inline void gpiod_set_value(int gpio, bool value)
-{
-	gpiod_direction_output(gpio, value);
-}
-
-static inline int gpiod_get_value(int gpio)
-{
-	if (gpio < 0)
-		return gpio;
-
-	return gpio_is_active(gpio);
-}
+#error "API changed from numbers to descriptors. #include <linux/gpiod/consumer.h>"
 
 #endif
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
new file mode 100644
index 000000000000..577b3955848e
--- /dev/null
+++ b/include/linux/gpio/consumer.h
@@ -0,0 +1,148 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_GPIO_CONSUMER_H
+#define __LINUX_GPIO_CONSUMER_H
+
+#include <gpio.h>
+#include <of_gpio.h>
+#include <driver.h>
+#include <linux/err.h>
+#include <errno.h>
+#include <linux/iopoll.h>
+
+/**
+ * Optional flags that can be passed to one of gpiod_* to configure direction
+ * and output value. These values cannot be OR'd.
+ */
+enum gpiod_flags {
+	GPIOD_ASIS	= 0,
+	GPIOD_IN	= GPIOF_IN,
+	/*
+	 * To change this later to a different logic level (i.e. taking
+	 * active low into account), use gpiod_set_value()
+	 */
+	GPIOD_OUT_LOW	= GPIOF_OUT_INIT_INACTIVE,
+	GPIOD_OUT_HIGH	= GPIOF_OUT_INIT_ACTIVE,
+};
+
+#define GPIO_DESC_OK		BIT(BITS_PER_LONG - 1)
+
+#define gpiod_not_found(desc)	(IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
+
+struct gpio_desc;
+
+static inline int __tmp_desc_to_gpio(struct gpio_desc *desc)
+{
+	if (!desc)
+		return -ENOENT;
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	return ((ulong)desc & ~GPIO_DESC_OK);
+}
+
+static inline struct gpio_desc *__tmp_gpio_to_desc(int gpio)
+{
+	if (gpio == -ENOENT)
+		return NULL;
+	if (gpio < 0)
+		return ERR_PTR(gpio);
+
+	return (struct gpio_desc *)(gpio | GPIO_DESC_OK);
+}
+
+#ifdef CONFIG_OFDEVICE
+
+/* returned gpio descriptor can be passed to any normal gpio_* function */
+struct gpio_desc *dev_gpiod_get_index(struct device *dev,
+			struct device_node *np,
+			const char *_con_id, int index,
+			enum gpiod_flags flags,
+			const char *label);
+
+#else
+static inline struct gpio_desc *dev_gpiod_get_index(struct device *dev,
+		struct device_node *np,
+		const char *_con_id, int index,
+		enum gpiod_flags flags,
+		const char *label)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
+static inline struct gpio_desc *dev_gpiod_get(struct device *dev,
+				struct device_node *np,
+				const char *con_id,
+				enum gpiod_flags flags,
+				const char *label)
+{
+	return dev_gpiod_get_index(dev, np, con_id, 0, flags, label);
+}
+
+static inline struct gpio_desc *gpiod_get(struct device *dev,
+			    const char *_con_id,
+			    enum gpiod_flags flags)
+{
+	return dev_gpiod_get(dev, dev->of_node, _con_id, flags, NULL);
+}
+
+static inline struct gpio_desc *__must_check
+gpiod_get_optional(struct device *dev, const char *con_id,
+		   enum gpiod_flags flags)
+{
+	struct gpio_desc *desc;
+
+	desc = gpiod_get(dev, con_id, flags);
+	if (gpiod_not_found(desc))
+		return NULL;
+
+	return desc;
+}
+
+static inline int gpiod_direction_input(struct gpio_desc *gpiod)
+{
+	if (gpiod_not_found(gpiod))
+		return 0;
+
+	return gpio_direction_input(__tmp_desc_to_gpio(gpiod));
+}
+
+static inline int gpiod_direction_output(struct gpio_desc *gpiod, bool value)
+{
+	if (gpiod_not_found(gpiod))
+		return 0;
+
+	return gpio_direction_active(__tmp_desc_to_gpio(gpiod), value);
+}
+
+static inline int gpiod_set_value(struct gpio_desc *gpiod, bool value)
+{
+	return gpiod_direction_output(gpiod, value);
+}
+
+static inline int gpiod_get_value(struct gpio_desc *gpiod)
+{
+	if (gpiod_not_found(gpiod))
+		return 0;
+	if (IS_ERR(gpiod))
+		return PTR_ERR(gpiod);
+
+	return gpio_is_active(__tmp_desc_to_gpio(gpiod));
+}
+
+/**
+ * gpiod_poll_timeout_us - poll till gpio descriptor reaches requested active state
+ * @gpiod: gpio descriptor to poll
+ * @active: wait till gpio is active if true, wait till it's inactive if false
+ * @timeout_us: timeout in microseconds
+ *
+ * during the wait barebox pollers are called, if any.
+ */
+#define gpiod_poll_timeout_us(gpiod, active, timeout_us)		\
+	({								\
+		int __state;						\
+		readx_poll_timeout(gpiod_get_value, gpiod, __state,	\
+				   __state == (active), timeout_us);	\
+	})
+
+#endif
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index ce8944c481df..54a788cc18eb 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1436,12 +1436,14 @@ void nand_wait_ready(struct nand_chip *chip);
  */
 void nand_cleanup(struct nand_chip *chip);
 
+struct gpio_desc;
+
 /*
  * External helper for controller drivers that have to implement the WAITRDY
  * instruction and have no physical pin to check it.
  */
 int nand_soft_waitrdy(struct nand_chip *chip, unsigned long timeout_ms);
-int nand_gpio_waitrdy(struct nand_chip *chip, int gpio,
+int nand_gpio_waitrdy(struct nand_chip *chip, struct gpio_desc *gpio,
 		      unsigned long timeout_ms);
 
 /* Select/deselect a NAND target. */
diff --git a/include/video/mipi_dbi.h b/include/video/mipi_dbi.h
index a15264c83392..c1c2a620edc2 100644
--- a/include/video/mipi_dbi.h
+++ b/include/video/mipi_dbi.h
@@ -15,6 +15,7 @@
 
 struct regulator;
 struct fb_videomode;
+struct gpio_desc;
 
 /**
  * struct mipi_dbi - MIPI DBI interface
@@ -39,7 +40,7 @@ struct mipi_dbi {
 	/**
 	 * @reset: Optional reset gpio
 	 */
-	int reset;
+	struct gpio_desc *reset;
 
 	/* Type C specific */
 
@@ -51,7 +52,7 @@ struct mipi_dbi {
 	/**
 	 * @dc: Optional D/C gpio.
 	 */
-	int dc;
+	struct gpio_desc *dc;
 
 	struct list_head list;
 };
@@ -122,7 +123,7 @@ static inline const char *mipi_dbi_name(struct mipi_dbi *dbi)
 }
 
 int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *dbi,
-		      int dc);
+		      struct gpio_desc *dc);
 int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev,
 		      struct fb_ops *ops, struct fb_videomode *mode);
 void mipi_dbi_fb_damage(struct fb_info *info, const struct fb_rect *rect);
-- 
2.39.2




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

* [PATCH v2 4/9] gpio: gpiolib: rename struct gpio_info to gpio_desc
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2023-06-22  7:23 ` [PATCH v2 3/9] gpio: have gpiod_ functions return and accept pointers Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 5/9] gpiolib: export proper gpio descriptor API Ahmad Fatoum
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

We already have GPIO descriptors in use internally, but return an
index into the array of all available GPIOs to the outside world.
In preparation for returning a pointer, let's rename the gpio_info
struct into gpio_desc and the gi variables into desc.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 206 ++++++++++++++++++++---------------------
 1 file changed, 103 insertions(+), 103 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index ee1bc5f6156f..07ca2d31a633 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -13,7 +13,7 @@
 
 static LIST_HEAD(chip_list);
 
-struct gpio_info {
+struct gpio_desc {
 	struct gpio_chip *chip;
 	bool requested;
 	bool active_low;
@@ -21,25 +21,25 @@ struct gpio_info {
 	const char *name;
 };
 
-static struct gpio_info *gpio_desc;
+static struct gpio_desc *gpio_desc;
 
 static int gpio_desc_alloc(void)
 {
-	gpio_desc = xzalloc(sizeof(struct gpio_info) * ARCH_NR_GPIOS);
+	gpio_desc = xzalloc(sizeof(struct gpio_desc) * ARCH_NR_GPIOS);
 
 	return 0;
 }
 pure_initcall(gpio_desc_alloc);
 
-static int gpio_ensure_requested(struct gpio_info *gi, int gpio)
+static int gpio_ensure_requested(struct gpio_desc *desc, int gpio)
 {
-	if (gi->requested)
+	if (desc->requested)
 		return 0;
 
 	return gpio_request(gpio, "gpio");
 }
 
-static struct gpio_info *gpio_to_desc(unsigned gpio)
+static struct gpio_desc *gpio_to_desc(unsigned gpio)
 {
 	if (gpio_is_valid(gpio))
 		if (gpio_desc[gpio].chip)
@@ -50,46 +50,46 @@ static struct gpio_info *gpio_to_desc(unsigned gpio)
 	return NULL;
 }
 
-static unsigned gpioinfo_chip_offset(struct gpio_info *gi)
+static unsigned gpioinfo_chip_offset(struct gpio_desc *desc)
 {
-	return (gi - gpio_desc) - gi->chip->base;
+	return (desc - gpio_desc) - desc->chip->base;
 }
 
-static int gpio_adjust_value(struct gpio_info *gi,
+static int gpio_adjust_value(struct gpio_desc *desc,
 			     int value)
 {
 	if (value < 0)
 		return value;
 
-	return !!value ^ gi->active_low;
+	return !!value ^ desc->active_low;
 }
 
-static int gpioinfo_request(struct gpio_info *gi, const char *label)
+static int gpioinfo_request(struct gpio_desc *desc, const char *label)
 {
 	int ret;
 
-	if (gi->requested) {
+	if (desc->requested) {
 		ret = -EBUSY;
 		goto done;
 	}
 
 	ret = 0;
 
-	if (gi->chip->ops->request) {
-		ret = gi->chip->ops->request(gi->chip,
-					     gpioinfo_chip_offset(gi));
+	if (desc->chip->ops->request) {
+		ret = desc->chip->ops->request(desc->chip,
+					     gpioinfo_chip_offset(desc));
 		if (ret)
 			goto done;
 	}
 
-	gi->requested = true;
-	gi->active_low = false;
-	gi->label = xstrdup(label);
+	desc->requested = true;
+	desc->active_low = false;
+	desc->label = xstrdup(label);
 
 done:
 	if (ret)
 		pr_err("_gpio_request: gpio-%td (%s) status %d\n",
-		       gi - gpio_desc, label ? : "?", ret);
+		       desc - gpio_desc, label ? : "?", ret);
 
 	return ret;
 }
@@ -99,7 +99,7 @@ int gpio_find_by_label(const char *label)
 	int i;
 
 	for (i = 0; i < ARCH_NR_GPIOS; i++) {
-		struct gpio_info *info = &gpio_desc[i];
+		struct gpio_desc *info = &gpio_desc[i];
 
 		if (!info->requested || !info->chip || !info->label)
 			continue;
@@ -116,7 +116,7 @@ int gpio_find_by_name(const char *name)
 	int i;
 
 	for (i = 0; i < ARCH_NR_GPIOS; i++) {
-		struct gpio_info *info = &gpio_desc[i];
+		struct gpio_desc *info = &gpio_desc[i];
 
 		if (!info->chip || !info->name)
 			continue;
@@ -130,174 +130,174 @@ int gpio_find_by_name(const char *name)
 
 int gpio_request(unsigned gpio, const char *label)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	if (!gi) {
+	if (!desc) {
 		pr_err("_gpio_request: gpio-%d (%s) status %d\n",
 			 gpio, label ? : "?", -ENODEV);
 		return -ENODEV;
 	}
 
-	return gpioinfo_request(gi, label);
+	return gpioinfo_request(desc, label);
 }
 
-static void gpioinfo_free(struct gpio_info *gi)
+static void gpioinfo_free(struct gpio_desc *desc)
 {
-	if (!gi->requested)
+	if (!desc->requested)
 		return;
 
-	if (gi->chip->ops->free)
-		gi->chip->ops->free(gi->chip, gpioinfo_chip_offset(gi));
+	if (desc->chip->ops->free)
+		desc->chip->ops->free(desc->chip, gpioinfo_chip_offset(desc));
 
-	gi->requested = false;
-	gi->active_low = false;
-	free(gi->label);
-	gi->label = NULL;
+	desc->requested = false;
+	desc->active_low = false;
+	free(desc->label);
+	desc->label = NULL;
 }
 
 void gpio_free(unsigned gpio)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	if (!gi)
+	if (!desc)
 		return;
 
-	gpioinfo_free(gi);
+	gpioinfo_free(desc);
 }
 
-static void gpioinfo_set_value(struct gpio_info *gi, int value)
+static void gpioinfo_set_value(struct gpio_desc *desc, int value)
 {
-	if (gi->chip->ops->set)
-		gi->chip->ops->set(gi->chip, gpioinfo_chip_offset(gi), value);
+	if (desc->chip->ops->set)
+		desc->chip->ops->set(desc->chip, gpioinfo_chip_offset(desc), value);
 }
 
 void gpio_set_value(unsigned gpio, int value)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	if (!gi)
+	if (!desc)
 		return;
 
-	if (gpio_ensure_requested(gi, gpio))
+	if (gpio_ensure_requested(desc, gpio))
 		return;
 
-	gpioinfo_set_value(gi, value);
+	gpioinfo_set_value(desc, value);
 }
 EXPORT_SYMBOL(gpio_set_value);
 
 void gpio_set_active(unsigned gpio, bool value)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	if (!gi)
+	if (!desc)
 		return;
 
-	gpio_set_value(gpio, gpio_adjust_value(gi, value));
+	gpio_set_value(gpio, gpio_adjust_value(desc, value));
 }
 EXPORT_SYMBOL(gpio_set_active);
 
-static int gpioinfo_get_value(struct gpio_info *gi)
+static int gpioinfo_get_value(struct gpio_desc *desc)
 {
-	if (!gi->chip->ops->get)
+	if (!desc->chip->ops->get)
 		return -ENOSYS;
 
-	return gi->chip->ops->get(gi->chip, gpioinfo_chip_offset(gi));
+	return desc->chip->ops->get(desc->chip, gpioinfo_chip_offset(desc));
 }
 
 int gpio_get_value(unsigned gpio)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 	int ret;
 
-	if (!gi)
+	if (!desc)
 		return -ENODEV;
 
-	ret = gpio_ensure_requested(gi, gpio);
+	ret = gpio_ensure_requested(desc, gpio);
 	if (ret)
 		return ret;
 
-	return gpioinfo_get_value(gi);
+	return gpioinfo_get_value(desc);
 }
 EXPORT_SYMBOL(gpio_get_value);
 
 int gpio_is_active(unsigned gpio)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	if (!gi)
+	if (!desc)
 		return -ENODEV;
 
-	return gpio_adjust_value(gi, gpio_get_value(gpio));
+	return gpio_adjust_value(desc, gpio_get_value(gpio));
 }
 EXPORT_SYMBOL(gpio_is_active);
 
-static int gpioinfo_direction_output(struct gpio_info *gi, int value)
+static int gpioinfo_direction_output(struct gpio_desc *desc, int value)
 {
-	if (!gi->chip->ops->direction_output)
+	if (!desc->chip->ops->direction_output)
 		return -ENOSYS;
 
-	return gi->chip->ops->direction_output(gi->chip,
-					       gpioinfo_chip_offset(gi), value);
+	return desc->chip->ops->direction_output(desc->chip,
+					       gpioinfo_chip_offset(desc), value);
 }
 
 int gpio_direction_output(unsigned gpio, int value)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 	int ret;
 
-	if (!gi)
+	if (!desc)
 		return -ENODEV;
 
-	ret = gpio_ensure_requested(gi, gpio);
+	ret = gpio_ensure_requested(desc, gpio);
 	if (ret)
 		return ret;
 
-	return gpioinfo_direction_output(gi, value);
+	return gpioinfo_direction_output(desc, value);
 }
 EXPORT_SYMBOL(gpio_direction_output);
 
-static int gpioinfo_direction_active(struct gpio_info *gi, bool value)
+static int gpioinfo_direction_active(struct gpio_desc *desc, bool value)
 {
-	return gpioinfo_direction_output(gi, gpio_adjust_value(gi, value));
+	return gpioinfo_direction_output(desc, gpio_adjust_value(desc, value));
 }
 
 int gpio_direction_active(unsigned gpio, bool value)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	if (!gi)
+	if (!desc)
 		return -ENODEV;
 
-	return gpioinfo_direction_active(gi, value);
+	return gpioinfo_direction_active(desc, value);
 }
 EXPORT_SYMBOL(gpio_direction_active);
 
-static int gpioinfo_direction_input(struct gpio_info *gi)
+static int gpioinfo_direction_input(struct gpio_desc *desc)
 {
-	if (!gi->chip->ops->direction_input)
+	if (!desc->chip->ops->direction_input)
 		return -ENOSYS;
 
-	return gi->chip->ops->direction_input(gi->chip,
-					      gpioinfo_chip_offset(gi));
+	return desc->chip->ops->direction_input(desc->chip,
+					      gpioinfo_chip_offset(desc));
 }
 
 int gpio_direction_input(unsigned gpio)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 	int ret;
 
-	if (!gi)
+	if (!desc)
 		return -ENODEV;
 
-	ret = gpio_ensure_requested(gi, gpio);
+	ret = gpio_ensure_requested(desc, gpio);
 	if (ret)
 		return ret;
 
-	return gpioinfo_direction_input(gi);
+	return gpioinfo_direction_input(desc);
 }
 EXPORT_SYMBOL(gpio_direction_input);
 
-static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
+static int gpioinfo_request_one(struct gpio_desc *desc, unsigned long flags,
 				const char *label)
 {
 	int err;
@@ -312,21 +312,21 @@ static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
 	const bool init_active = (flags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
 	const bool init_high   = (flags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
 
-	err = gpioinfo_request(gi, label);
+	err = gpioinfo_request(desc, label);
 	if (err)
 		return err;
 
-	gi->active_low = active_low;
+	desc->active_low = active_low;
 
 	if (dir_in)
-		err = gpioinfo_direction_input(gi);
+		err = gpioinfo_direction_input(desc);
 	else if (logical)
-		err = gpioinfo_direction_active(gi, init_active);
+		err = gpioinfo_direction_active(desc, init_active);
 	else
-		err = gpioinfo_direction_output(gi, init_high);
+		err = gpioinfo_direction_output(desc, init_high);
 
 	if (err)
-		gpioinfo_free(gi);
+		gpioinfo_free(desc);
 
 	return err;
 }
@@ -339,12 +339,12 @@ static int gpioinfo_request_one(struct gpio_info *gi, unsigned long flags,
  */
 int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	if (!gi)
+	if (!desc)
 		return -ENODEV;
 
-	return gpioinfo_request_one(gi, flags, label);
+	return gpioinfo_request_one(desc, flags, label);
 }
 EXPORT_SYMBOL_GPL(gpio_request_one);
 
@@ -782,9 +782,9 @@ int gpio_get_num(struct device *dev, int gpio)
 
 struct gpio_chip *gpio_get_chip(int gpio)
 {
-	struct gpio_info *gi = gpio_to_desc(gpio);
+	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	return gi ? gi->chip : NULL;
+	return desc ? desc->chip : NULL;
 }
 
 #ifdef CONFIG_CMD_GPIO
@@ -809,38 +809,38 @@ static int do_gpiolib(int argc, char *argv[])
 	}
 
 	for (i = 0; i < ARCH_NR_GPIOS; i++) {
-		struct gpio_info *gi = &gpio_desc[i];
+		struct gpio_desc *desc = &gpio_desc[i];
 		int val = -1, dir = -1;
 		int idx;
 
-		if (!gi->chip)
+		if (!desc->chip)
 			continue;
 
-		if (chip && chip != gi->chip)
+		if (chip && chip != desc->chip)
 			continue;
 
 		/* print chip information and header on first gpio */
-		if (gi->chip->base == i) {
+		if (desc->chip->base == i) {
 			printf("\nGPIOs %u-%u, chip %s:\n",
-				gi->chip->base,
-				gi->chip->base + gi->chip->ngpio - 1,
-				dev_name(gi->chip->dev));
+				desc->chip->base,
+				desc->chip->base + desc->chip->ngpio - 1,
+				dev_name(desc->chip->dev));
 			printf("             %-3s %-3s %-9s %-20s %-20s\n", "dir", "val", "requested", "name", "label");
 		}
 
-		idx = i - gi->chip->base;
+		idx = i - desc->chip->base;
 
-		if (gi->chip->ops->get_direction)
-			dir = gi->chip->ops->get_direction(gi->chip, idx);
-		if (gi->chip->ops->get)
-			val = gi->chip->ops->get(gi->chip, idx);
+		if (desc->chip->ops->get_direction)
+			dir = desc->chip->ops->get_direction(desc->chip, idx);
+		if (desc->chip->ops->get)
+			val = desc->chip->ops->get(desc->chip, idx);
 
 		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"),
-		        gi->requested ? (gi->active_low ? "active low" : "true") : "false",
-			gi->name ? gi->name : "",
-			gi->label ? gi->label : "");
+		        desc->requested ? (desc->active_low ? "active low" : "true") : "false",
+			desc->name ? desc->name : "",
+			desc->label ? desc->label : "");
 	}
 
 	return 0;
-- 
2.39.2




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

* [PATCH v2 5/9] gpiolib: export proper gpio descriptor API
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2023-06-22  7:23 ` [PATCH v2 4/9] gpio: gpiolib: rename struct gpio_info to gpio_desc Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 6/9] bitmap: implement bitmap_{to,from}_arr{32,64} Ahmad Fatoum
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

Our current gpiod API doesn't return actual struct gpio_desc pointers
that can be dereferenced by the GPIO core, like in Linux. We actually
have all the infrastructure in place to do that, but we just aren't
using this yet. Rename the relevant gpioinfo_ functions to gpiod_,
document them, export them and have them observe the same semantics as
their Linux equivalents.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c        | 173 ++++++++++++++++++++++++++++++----
 include/linux/gpio/consumer.h | 130 ++++++++++++++-----------
 2 files changed, 227 insertions(+), 76 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 07ca2d31a633..11d2506deca4 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -21,6 +21,36 @@ struct gpio_desc {
 	const char *name;
 };
 
+/*
+ * This descriptor validation needs to be inserted verbatim into each
+ * function taking a descriptor, so we need to use a preprocessor
+ * macro to avoid endless duplication. If the desc is NULL it is an
+ * optional GPIO and calls should just bail out.
+ */
+static int validate_desc(const struct gpio_desc *desc, const char *func)
+{
+	if (!desc)
+		return 0;
+	if (IS_ERR(desc)) {
+		pr_warn("%s: invalid GPIO (errorpointer)\n", func);
+		return PTR_ERR(desc);
+	}
+
+	return 1;
+}
+
+#define VALIDATE_DESC(desc) do { \
+	int __valid = validate_desc(desc, __func__); \
+	if (__valid <= 0) \
+		return __valid; \
+	} while (0)
+
+#define VALIDATE_DESC_VOID(desc) do { \
+	int __valid = validate_desc(desc, __func__); \
+	if (__valid <= 0) \
+		return; \
+	} while (0)
+
 static struct gpio_desc *gpio_desc;
 
 static int gpio_desc_alloc(void)
@@ -50,12 +80,12 @@ static struct gpio_desc *gpio_to_desc(unsigned gpio)
 	return NULL;
 }
 
-static unsigned gpioinfo_chip_offset(struct gpio_desc *desc)
+static unsigned gpioinfo_chip_offset(const struct gpio_desc *desc)
 {
 	return (desc - gpio_desc) - desc->chip->base;
 }
 
-static int gpio_adjust_value(struct gpio_desc *desc,
+static int gpio_adjust_value(const struct gpio_desc *desc,
 			     int value)
 {
 	if (value < 0)
@@ -159,17 +189,40 @@ void gpio_free(unsigned gpio)
 {
 	struct gpio_desc *desc = gpio_to_desc(gpio);
 
+	gpioinfo_free(desc);
+}
+
+/**
+ * gpiod_put - dispose of a GPIO descriptor
+ * @desc:	GPIO descriptor to dispose of
+ *
+ * No descriptor can be used after gpiod_put() has been called on it.
+ */
+void gpiod_put(struct gpio_desc *desc)
+{
 	if (!desc)
 		return;
 
 	gpioinfo_free(desc);
 }
+EXPORT_SYMBOL(gpiod_put);
 
-static void gpioinfo_set_value(struct gpio_desc *desc, int value)
+/**
+ * gpiod_set_raw_value() - assign a gpio's raw value
+ * @desc: gpio whose value will be assigned
+ * @value: value to assign
+ *
+ * Set the raw value of the GPIO, i.e. the value of its physical line without
+ * regard for its ACTIVE_LOW status.
+ */
+void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 {
+	VALIDATE_DESC_VOID(desc);
+
 	if (desc->chip->ops->set)
 		desc->chip->ops->set(desc->chip, gpioinfo_chip_offset(desc), value);
 }
+EXPORT_SYMBOL(gpiod_set_raw_value);
 
 void gpio_set_value(unsigned gpio, int value)
 {
@@ -181,10 +234,25 @@ void gpio_set_value(unsigned gpio, int value)
 	if (gpio_ensure_requested(desc, gpio))
 		return;
 
-	gpioinfo_set_value(desc, value);
+	gpiod_set_raw_value(desc, value);
 }
 EXPORT_SYMBOL(gpio_set_value);
 
+/**
+ * gpiod_set_value() - assign a gpio's value
+ * @desc: gpio whose value will be assigned
+ * @value: value to assign
+ *
+ * Set the logical value of the GPIO, i.e. taking its ACTIVE_LOW,
+ * OPEN_DRAIN and OPEN_SOURCE flags into account.
+ */
+void gpiod_set_value(struct gpio_desc *desc, int value)
+{
+	VALIDATE_DESC_VOID(desc);
+	gpiod_set_raw_value(desc, gpio_adjust_value(desc, value));
+}
+EXPORT_SYMBOL_GPL(gpiod_set_value);
+
 void gpio_set_active(unsigned gpio, bool value)
 {
 	struct gpio_desc *desc = gpio_to_desc(gpio);
@@ -192,17 +260,27 @@ void gpio_set_active(unsigned gpio, bool value)
 	if (!desc)
 		return;
 
-	gpio_set_value(gpio, gpio_adjust_value(desc, value));
+	gpiod_set_value(desc, value);
 }
 EXPORT_SYMBOL(gpio_set_active);
 
-static int gpioinfo_get_value(struct gpio_desc *desc)
+/**
+ * gpiod_get_raw_value() - return a gpio's raw value
+ * @desc: gpio whose value will be returned
+ *
+ * Return the GPIO's raw value, i.e. the value of the physical line disregarding
+ * its ACTIVE_LOW status, or negative errno on failure.
+ */
+int gpiod_get_raw_value(const struct gpio_desc *desc)
 {
+	VALIDATE_DESC(desc);
+
 	if (!desc->chip->ops->get)
 		return -ENOSYS;
 
 	return desc->chip->ops->get(desc->chip, gpioinfo_chip_offset(desc));
 }
+EXPORT_SYMBOL_GPL(gpiod_get_raw_value);
 
 int gpio_get_value(unsigned gpio)
 {
@@ -216,10 +294,25 @@ int gpio_get_value(unsigned gpio)
 	if (ret)
 		return ret;
 
-	return gpioinfo_get_value(desc);
+	return gpiod_get_raw_value(desc);
 }
 EXPORT_SYMBOL(gpio_get_value);
 
+/**
+ * gpiod_get_value() - return a gpio's value
+ * @desc: gpio whose value will be returned
+ *
+ * Return the GPIO's logical value, i.e. taking the ACTIVE_LOW status into
+ * account, or negative errno on failure.
+ */
+int gpiod_get_value(const struct gpio_desc *desc)
+{
+	VALIDATE_DESC(desc);
+
+	return gpio_adjust_value(desc, gpiod_get_raw_value(desc));
+}
+EXPORT_SYMBOL_GPL(gpiod_get_value);
+
 int gpio_is_active(unsigned gpio)
 {
 	struct gpio_desc *desc = gpio_to_desc(gpio);
@@ -227,18 +320,32 @@ int gpio_is_active(unsigned gpio)
 	if (!desc)
 		return -ENODEV;
 
-	return gpio_adjust_value(desc, gpio_get_value(gpio));
+	return gpiod_get_value(desc);
 }
 EXPORT_SYMBOL(gpio_is_active);
 
-static int gpioinfo_direction_output(struct gpio_desc *desc, int value)
+/**
+ * gpiod_direction_output_raw - set the GPIO direction to output
+ * @desc:	GPIO to set to output
+ * @value:	initial output value of the GPIO
+ *
+ * Set the direction of the passed GPIO to output, such as gpiod_set_value() can
+ * be called safely on it. The initial value of the output must be specified
+ * as raw value on the physical line without regard for the ACTIVE_LOW status.
+ *
+ * Return 0 in case of success, else an error code.
+ */
+int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
 {
+	VALIDATE_DESC(desc);
+
 	if (!desc->chip->ops->direction_output)
 		return -ENOSYS;
 
 	return desc->chip->ops->direction_output(desc->chip,
 					       gpioinfo_chip_offset(desc), value);
 }
+EXPORT_SYMBOL(gpiod_direction_output_raw);
 
 int gpio_direction_output(unsigned gpio, int value)
 {
@@ -252,13 +359,27 @@ int gpio_direction_output(unsigned gpio, int value)
 	if (ret)
 		return ret;
 
-	return gpioinfo_direction_output(desc, value);
+	return gpiod_direction_output_raw(desc, value);
 }
 EXPORT_SYMBOL(gpio_direction_output);
 
-static int gpioinfo_direction_active(struct gpio_desc *desc, bool value)
+/**
+ * gpiod_direction_output - set the GPIO direction to output
+ * @desc:	GPIO to set to output
+ * @value:	initial output value of the GPIO
+ *
+ * Set the direction of the passed GPIO to output, such as gpiod_set_value() can
+ * be called safely on it. The initial value of the output must be specified
+ * as the logical value of the GPIO, i.e. taking its ACTIVE_LOW status into
+ * account.
+ *
+ * Return 0 in case of success, else an error code.
+ */
+int gpiod_direction_output(struct gpio_desc *desc, int value)
 {
-	return gpioinfo_direction_output(desc, gpio_adjust_value(desc, value));
+	VALIDATE_DESC(desc);
+
+	return gpiod_direction_output_raw(desc, gpio_adjust_value(desc, value));
 }
 
 int gpio_direction_active(unsigned gpio, bool value)
@@ -268,18 +389,30 @@ int gpio_direction_active(unsigned gpio, bool value)
 	if (!desc)
 		return -ENODEV;
 
-	return gpioinfo_direction_active(desc, value);
+	return gpiod_direction_output(desc, value);
 }
 EXPORT_SYMBOL(gpio_direction_active);
 
-static int gpioinfo_direction_input(struct gpio_desc *desc)
+/**
+ * gpiod_direction_input - set the GPIO direction to input
+ * @desc:	GPIO to set to input
+ *
+ * Set the direction of the passed GPIO to input, such as gpiod_get_value() can
+ * be called safely on it.
+ *
+ * Return 0 in case of success, else an error code.
+ */
+int gpiod_direction_input(struct gpio_desc *desc)
 {
+	VALIDATE_DESC(desc);
+
 	if (!desc->chip->ops->direction_input)
 		return -ENOSYS;
 
 	return desc->chip->ops->direction_input(desc->chip,
 					      gpioinfo_chip_offset(desc));
 }
+EXPORT_SYMBOL(gpiod_direction_input);
 
 int gpio_direction_input(unsigned gpio)
 {
@@ -293,7 +426,7 @@ int gpio_direction_input(unsigned gpio)
 	if (ret)
 		return ret;
 
-	return gpioinfo_direction_input(desc);
+	return gpiod_direction_input(desc);
 }
 EXPORT_SYMBOL(gpio_direction_input);
 
@@ -319,11 +452,11 @@ static int gpioinfo_request_one(struct gpio_desc *desc, unsigned long flags,
 	desc->active_low = active_low;
 
 	if (dir_in)
-		err = gpioinfo_direction_input(desc);
+		err = gpiod_direction_input(desc);
 	else if (logical)
-		err = gpioinfo_direction_active(desc, init_active);
+		err = gpiod_direction_output(desc, init_active);
 	else
-		err = gpioinfo_direction_output(desc, init_high);
+		err = gpiod_direction_output_raw(desc, init_high);
 
 	if (err)
 		gpioinfo_free(desc);
@@ -698,7 +831,7 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 		free(con_id);
 
 		if (gpio_is_valid(gpio)) {
-			desc = __tmp_gpio_to_desc(gpio);
+			desc = gpio_to_desc(gpio);
 			break;
 		}
 	}
@@ -718,7 +851,7 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 			label = dev_name(dev);
 	}
 
-	ret = gpio_request_one(__tmp_desc_to_gpio(desc), flags, label);
+	ret = gpioinfo_request_one(desc, flags, label);
 	free(buf);
 
 	return ret ? ERR_PTR(ret): desc;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 577b3955848e..2547e4ba7be3 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -5,8 +5,7 @@
 #include <gpio.h>
 #include <of_gpio.h>
 #include <driver.h>
-#include <linux/err.h>
-#include <errno.h>
+#include <linux/bug.h>
 #include <linux/iopoll.h>
 
 /**
@@ -24,33 +23,11 @@ enum gpiod_flags {
 	GPIOD_OUT_HIGH	= GPIOF_OUT_INIT_ACTIVE,
 };
 
-#define GPIO_DESC_OK		BIT(BITS_PER_LONG - 1)
-
 #define gpiod_not_found(desc)	(IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
 
 struct gpio_desc;
 
-static inline int __tmp_desc_to_gpio(struct gpio_desc *desc)
-{
-	if (!desc)
-		return -ENOENT;
-	if (IS_ERR(desc))
-		return PTR_ERR(desc);
-
-	return ((ulong)desc & ~GPIO_DESC_OK);
-}
-
-static inline struct gpio_desc *__tmp_gpio_to_desc(int gpio)
-{
-	if (gpio == -ENOENT)
-		return NULL;
-	if (gpio < 0)
-		return ERR_PTR(gpio);
-
-	return (struct gpio_desc *)(gpio | GPIO_DESC_OK);
-}
-
-#ifdef CONFIG_OFDEVICE
+#if defined(CONFIG_OFDEVICE) && defined(CONFIG_GPIOLIB)
 
 /* returned gpio descriptor can be passed to any normal gpio_* function */
 struct gpio_desc *dev_gpiod_get_index(struct device *dev,
@@ -70,6 +47,78 @@ static inline struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 }
 #endif
 
+#ifdef CONFIG_GPIOLIB
+
+int gpiod_direction_input(struct gpio_desc *desc);
+
+int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
+int gpiod_direction_output(struct gpio_desc *desc, int value);
+
+void gpiod_set_raw_value(struct gpio_desc *desc, int value);
+void gpiod_set_value(struct gpio_desc *desc, int value);
+
+int gpiod_get_raw_value(const struct gpio_desc *desc);
+int gpiod_get_value(const struct gpio_desc *desc);
+
+void gpiod_put(struct gpio_desc *desc);
+
+#else
+
+static inline int gpiod_direction_input(struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return 0;
+}
+
+static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return 0;
+}
+
+static inline int gpiod_direction_output(struct gpio_desc *desc, int value)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return 0;
+}
+
+static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+}
+
+static inline void gpiod_set_value(struct gpio_desc *desc, int value)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+}
+
+static inline int gpiod_get_raw_value(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return 0;
+}
+
+static inline int gpiod_get_value(const struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return 0;
+}
+
+static inline void gpiod_put(struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+}
+
+#endif
+
 static inline struct gpio_desc *dev_gpiod_get(struct device *dev,
 				struct device_node *np,
 				const char *con_id,
@@ -99,37 +148,6 @@ gpiod_get_optional(struct device *dev, const char *con_id,
 	return desc;
 }
 
-static inline int gpiod_direction_input(struct gpio_desc *gpiod)
-{
-	if (gpiod_not_found(gpiod))
-		return 0;
-
-	return gpio_direction_input(__tmp_desc_to_gpio(gpiod));
-}
-
-static inline int gpiod_direction_output(struct gpio_desc *gpiod, bool value)
-{
-	if (gpiod_not_found(gpiod))
-		return 0;
-
-	return gpio_direction_active(__tmp_desc_to_gpio(gpiod), value);
-}
-
-static inline int gpiod_set_value(struct gpio_desc *gpiod, bool value)
-{
-	return gpiod_direction_output(gpiod, value);
-}
-
-static inline int gpiod_get_value(struct gpio_desc *gpiod)
-{
-	if (gpiod_not_found(gpiod))
-		return 0;
-	if (IS_ERR(gpiod))
-		return PTR_ERR(gpiod);
-
-	return gpio_is_active(__tmp_desc_to_gpio(gpiod));
-}
-
 /**
  * gpiod_poll_timeout_us - poll till gpio descriptor reaches requested active state
  * @gpiod: gpio descriptor to poll
-- 
2.39.2




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

* [PATCH v2 6/9] bitmap: implement bitmap_{to,from}_arr{32,64}
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2023-06-22  7:23 ` [PATCH v2 5/9] gpiolib: export proper gpio descriptor API Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 7/9] gpiolib: factor out finding gpio property Ahmad Fatoum
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

Converting from a bitmap to a u32 or u64 array of bits and back can
involve endianness conversions depending on platform. Import the Linux
functions that take care of this.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/bitmap.h | 101 ++++++++++++++++++++++++++++++++++++++++
 lib/bitmap.c           | 103 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 204 insertions(+)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 7d06fac68d08..9ec1ee2d1422 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -56,6 +56,10 @@
  * bitmap_find_free_region(bitmap, bits, order)	Find and allocate bit region
  * bitmap_release_region(bitmap, pos, order)	Free specified bit region
  * bitmap_allocate_region(bitmap, pos, order)	Allocate specified bit region
+ * bitmap_from_arr32(dst, buf, nbits)           Copy nbits from u32[] buf to dst
+ * bitmap_from_arr64(dst, buf, nbits)           Copy nbits from u64[] buf to dst
+ * bitmap_to_arr32(buf, src, nbits)             Copy nbits from buf to u32[] dst
+ * bitmap_to_arr64(buf, src, nbits)             Copy nbits from buf to u64[] dst
  */
 
 /*
@@ -178,6 +182,103 @@ static inline void bitmap_copy(unsigned long *dst, const unsigned long *src,
 	}
 }
 
+/*
+ * Copy bitmap and clear tail bits in last word.
+ */
+static inline void bitmap_copy_clear_tail(unsigned long *dst,
+		const unsigned long *src, unsigned int nbits)
+{
+	bitmap_copy(dst, src, nbits);
+	if (nbits % BITS_PER_LONG)
+		dst[nbits / BITS_PER_LONG] &= BITMAP_LAST_WORD_MASK(nbits);
+}
+
+/*
+ * On 32-bit systems bitmaps are represented as u32 arrays internally. On LE64
+ * machines the order of hi and lo parts of numbers match the bitmap structure.
+ * In both cases conversion is not needed when copying data from/to arrays of
+ * u32. But in LE64 case, typecast in bitmap_copy_clear_tail() may lead
+ * to out-of-bound access. To avoid that, both LE and BE variants of 64-bit
+ * architectures are not using bitmap_copy_clear_tail().
+ */
+#if BITS_PER_LONG == 64
+void bitmap_from_arr32(unsigned long *bitmap, const u32 *buf,
+							unsigned int nbits);
+void bitmap_to_arr32(u32 *buf, const unsigned long *bitmap,
+							unsigned int nbits);
+#else
+#define bitmap_from_arr32(bitmap, buf, nbits)			\
+	bitmap_copy_clear_tail((unsigned long *) (bitmap),	\
+			(const unsigned long *) (buf), (nbits))
+#define bitmap_to_arr32(buf, bitmap, nbits)			\
+	bitmap_copy_clear_tail((unsigned long *) (buf),		\
+			(const unsigned long *) (bitmap), (nbits))
+#endif
+
+/*
+ * On 64-bit systems bitmaps are represented as u64 arrays internally. On LE32
+ * machines the order of hi and lo parts of numbers match the bitmap structure.
+ * In both cases conversion is not needed when copying data from/to arrays of
+ * u64.
+ */
+#if (BITS_PER_LONG == 32) && defined(__BIG_ENDIAN)
+void bitmap_from_arr64(unsigned long *bitmap, const u64 *buf, unsigned int nbits);
+void bitmap_to_arr64(u64 *buf, const unsigned long *bitmap, unsigned int nbits);
+#else
+#define bitmap_from_arr64(bitmap, buf, nbits)			\
+	bitmap_copy_clear_tail((unsigned long *)(bitmap), (const unsigned long *)(buf), (nbits))
+#define bitmap_to_arr64(buf, bitmap, nbits)			\
+	bitmap_copy_clear_tail((unsigned long *)(buf), (const unsigned long *)(bitmap), (nbits))
+#endif
+
+/**
+ * BITMAP_FROM_U64() - Represent u64 value in the format suitable for bitmap.
+ * @n: u64 value
+ *
+ * Linux bitmaps are internally arrays of unsigned longs, i.e. 32-bit
+ * integers in 32-bit environment, and 64-bit integers in 64-bit one.
+ *
+ * There are four combinations of endianness and length of the word in linux
+ * ABIs: LE64, BE64, LE32 and BE32.
+ *
+ * On 64-bit kernels 64-bit LE and BE numbers are naturally ordered in
+ * bitmaps and therefore don't require any special handling.
+ *
+ * On 32-bit kernels 32-bit LE ABI orders lo word of 64-bit number in memory
+ * prior to hi, and 32-bit BE orders hi word prior to lo. The bitmap on the
+ * other hand is represented as an array of 32-bit words and the position of
+ * bit N may therefore be calculated as: word #(N/32) and bit #(N%32) in that
+ * word.  For example, bit #42 is located at 10th position of 2nd word.
+ * It matches 32-bit LE ABI, and we can simply let the compiler store 64-bit
+ * values in memory as it usually does. But for BE we need to swap hi and lo
+ * words manually.
+ *
+ * With all that, the macro BITMAP_FROM_U64() does explicit reordering of hi and
+ * lo parts of u64.  For LE32 it does nothing, and for BE environment it swaps
+ * hi and lo words, as is expected by bitmap.
+ */
+#if BITS_PER_LONG == 64
+#define BITMAP_FROM_U64(n) (n)
+#else
+#define BITMAP_FROM_U64(n) ((unsigned long) ((u64)(n) & ULONG_MAX)), \
+				((unsigned long) ((u64)(n) >> 32))
+#endif
+
+/**
+ * bitmap_from_u64 - Check and swap words within u64.
+ *  @mask: source bitmap
+ *  @dst:  destination bitmap
+ *
+ * In 32-bit Big Endian kernel, when using ``(u32 *)(&val)[*]``
+ * to read u64 mask, we will get the wrong word.
+ * That is ``(u32 *)(&val)[0]`` gets the upper 32 bits,
+ * but we expect the lower 32-bits of u64.
+ */
+static inline void bitmap_from_u64(unsigned long *dst, u64 mask)
+{
+	bitmap_from_arr64(dst, &mask, 64);
+}
+
 static inline int bitmap_and(unsigned long *dst, const unsigned long *src1,
 			const unsigned long *src2, int nbits)
 {
diff --git a/lib/bitmap.c b/lib/bitmap.c
index dfc0f06b13c4..7614c4d21337 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -847,3 +847,106 @@ unsigned long *bitmap_xzalloc(unsigned int nbits)
 {
 	return xzalloc(BITS_TO_LONGS(nbits) * sizeof(unsigned long));
 }
+
+#if BITS_PER_LONG == 64
+/**
+ * bitmap_from_arr32 - copy the contents of u32 array of bits to bitmap
+ *	@bitmap: array of unsigned longs, the destination bitmap
+ *	@buf: array of u32 (in host byte order), the source bitmap
+ *	@nbits: number of bits in @bitmap
+ */
+void bitmap_from_arr32(unsigned long *bitmap, const u32 *buf, unsigned int nbits)
+{
+	unsigned int i, halfwords;
+
+	halfwords = DIV_ROUND_UP(nbits, 32);
+	for (i = 0; i < halfwords; i++) {
+		bitmap[i/2] = (unsigned long) buf[i];
+		if (++i < halfwords)
+			bitmap[i/2] |= ((unsigned long) buf[i]) << 32;
+	}
+
+	/* Clear tail bits in last word beyond nbits. */
+	if (nbits % BITS_PER_LONG)
+		bitmap[(halfwords - 1) / 2] &= BITMAP_LAST_WORD_MASK(nbits);
+}
+EXPORT_SYMBOL(bitmap_from_arr32);
+
+/**
+ * bitmap_to_arr32 - copy the contents of bitmap to a u32 array of bits
+ *	@buf: array of u32 (in host byte order), the dest bitmap
+ *	@bitmap: array of unsigned longs, the source bitmap
+ *	@nbits: number of bits in @bitmap
+ */
+void bitmap_to_arr32(u32 *buf, const unsigned long *bitmap, unsigned int nbits)
+{
+	unsigned int i, halfwords;
+
+	halfwords = DIV_ROUND_UP(nbits, 32);
+	for (i = 0; i < halfwords; i++) {
+		buf[i] = (u32) (bitmap[i/2] & UINT_MAX);
+		if (++i < halfwords)
+			buf[i] = (u32) (bitmap[i/2] >> 32);
+	}
+
+	/* Clear tail bits in last element of array beyond nbits. */
+	if (nbits % BITS_PER_LONG)
+		buf[halfwords - 1] &= (u32) (UINT_MAX >> ((-nbits) & 31));
+}
+EXPORT_SYMBOL(bitmap_to_arr32);
+#endif
+
+#if (BITS_PER_LONG == 32) && defined(__BIG_ENDIAN)
+/**
+ * bitmap_from_arr64 - copy the contents of u64 array of bits to bitmap
+ *	@bitmap: array of unsigned longs, the destination bitmap
+ *	@buf: array of u64 (in host byte order), the source bitmap
+ *	@nbits: number of bits in @bitmap
+ */
+void bitmap_from_arr64(unsigned long *bitmap, const u64 *buf, unsigned int nbits)
+{
+	int n;
+
+	for (n = nbits; n > 0; n -= 64) {
+		u64 val = *buf++;
+
+		*bitmap++ = val;
+		if (n > 32)
+			*bitmap++ = val >> 32;
+	}
+
+	/*
+	 * Clear tail bits in the last word beyond nbits.
+	 *
+	 * Negative index is OK because here we point to the word next
+	 * to the last word of the bitmap, except for nbits == 0, which
+	 * is tested implicitly.
+	 */
+	if (nbits % BITS_PER_LONG)
+		bitmap[-1] &= BITMAP_LAST_WORD_MASK(nbits);
+}
+EXPORT_SYMBOL(bitmap_from_arr64);
+
+/**
+ * bitmap_to_arr64 - copy the contents of bitmap to a u64 array of bits
+ *	@buf: array of u64 (in host byte order), the dest bitmap
+ *	@bitmap: array of unsigned longs, the source bitmap
+ *	@nbits: number of bits in @bitmap
+ */
+void bitmap_to_arr64(u64 *buf, const unsigned long *bitmap, unsigned int nbits)
+{
+	const unsigned long *end = bitmap + BITS_TO_LONGS(nbits);
+
+	while (bitmap < end) {
+		*buf = *bitmap++;
+		if (bitmap < end)
+			*buf |= (u64)(*bitmap++) << 32;
+		buf++;
+	}
+
+	/* Clear tail bits in the last element of array beyond nbits. */
+	if (nbits % 64)
+		buf[-1] &= GENMASK_ULL((nbits - 1) % 64, 0);
+}
+EXPORT_SYMBOL(bitmap_to_arr64);
+#endif
-- 
2.39.2




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

* [PATCH v2 7/9] gpiolib: factor out finding gpio property
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2023-06-22  7:23 ` [PATCH v2 6/9] bitmap: implement bitmap_{to,from}_arr{32,64} Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 8/9] gpiolib: add support for requesting and setting gpiod arrays Ahmad Fatoum
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

The GPIO property may end with -gpios or -gpio or if there's only a
single GPIO, may have no dash at all. We will add more functions that
need to find the gpio property, so let's factor this out into
of_find_gpio_property.

This introduces functional change: When a foo-gpios property is found,
but points at an invalid GPIO, that failure will be propagated instead
of trying foo-gpio. This is deemed acceptable as this sounds like the
saner choice anyway.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 61 +++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 11d2506deca4..f34337f6446f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -802,21 +802,12 @@ static const char *gpio_suffixes[] = {
 	"gpio",
 };
 
-/* Linux compatibility helper: Get a GPIO descriptor from device tree */
-struct gpio_desc *dev_gpiod_get_index(struct device *dev,
-			struct device_node *np,
-			const char *_con_id, int index,
-			enum gpiod_flags flags,
-			const char *label)
+static struct property *of_find_gpio_property(struct device_node *np,
+					      const char *_con_id)
 {
-	struct gpio_desc *desc = NULL;
-	enum of_gpio_flags of_flags;
-	char *buf = NULL, *con_id;
-	int gpio;
-	int ret, i;
-
-	if (!np)
-		return ERR_PTR(-ENODEV);
+	struct property *pp = NULL;
+	char *con_id;
+	int i;
 
 	for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
 		if (_con_id)
@@ -827,26 +818,52 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 		if (!con_id)
 			return ERR_PTR(-ENOMEM);
 
-		gpio = of_get_named_gpio_flags(np, con_id, index, &of_flags);
+		pp = of_find_property(np, con_id, NULL);
 		free(con_id);
 
-		if (gpio_is_valid(gpio)) {
-			desc = gpio_to_desc(gpio);
-			break;
-		}
+		if (pp)
+			return pp;
 	}
 
-	if (!desc)
+	return NULL;
+}
+
+/* Linux compatibility helper: Get a GPIO descriptor from device tree */
+struct gpio_desc *dev_gpiod_get_index(struct device *dev,
+			struct device_node *np,
+			const char *con_id, int index,
+			enum gpiod_flags flags,
+			const char *label)
+{
+	struct gpio_desc *desc = NULL;
+	enum of_gpio_flags of_flags;
+	struct property *pp;
+	char *buf = NULL;
+	int gpio;
+	int ret;
+
+	if (!np)
+		return ERR_PTR(-ENODEV);
+
+	pp = of_find_gpio_property(np, con_id);
+	if (!pp)
+		return ERR_PTR(-ENOENT);
+
+	gpio = of_get_named_gpio_flags(dev->device_node, pp->name,
+				       index, &of_flags);
+	if (!gpio_is_valid(gpio))
 		return ERR_PTR(gpio < 0 ? gpio : -EINVAL);
 
+	desc = gpio_to_desc(gpio);
+
 	if (of_flags & OF_GPIO_ACTIVE_LOW)
 		flags |= GPIOF_ACTIVE_LOW;
 
 	buf = NULL;
 
 	if (!label) {
-		if (_con_id)
-			label = buf = basprintf("%s-%s", dev_name(dev), _con_id);
+		if (con_id)
+			label = buf = basprintf("%s-%s", dev_name(dev), con_id);
 		else
 			label = dev_name(dev);
 	}
-- 
2.39.2




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

* [PATCH v2 8/9] gpiolib: add support for requesting and setting gpiod arrays
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2023-06-22  7:23 ` [PATCH v2 7/9] gpiolib: factor out finding gpio property Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-22  7:23 ` [PATCH v2 9/9] gpiolib: rename gpioinfo_ to gpiodesc_ Ahmad Fatoum
  2023-06-23  9:34 ` [PATCH v2 0/9] gpio: add proper gpiod API Sascha Hauer
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

The added API is aligned with the API in Linux, but has the caveat that
barebox has no set_multiple callbacks implemented in the GPIO drivers,
so switching may result in a bad intermediate state. We add a note about
that and implement the API in the easiest possible way.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c        | 123 ++++++++++++++++++++++++++++++++++
 include/linux/gpio/consumer.h |  58 ++++++++++++++++
 2 files changed, 181 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f34337f6446f..2808ac3612fe 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/overflow.h>
 #include <errno.h>
 #include <malloc.h>
 
@@ -207,6 +208,21 @@ void gpiod_put(struct gpio_desc *desc)
 }
 EXPORT_SYMBOL(gpiod_put);
 
+/**
+ * gpiod_put_array - dispose of multiple GPIO descriptors
+ * @descs:	struct gpio_descs containing an array of descriptors
+ */
+void gpiod_put_array(struct gpio_descs *descs)
+{
+	unsigned int i;
+
+	for (i = 0; i < descs->ndescs; i++)
+		gpiod_put(descs->desc[i]);
+
+	kfree(descs);
+}
+EXPORT_SYMBOL_GPL(gpiod_put_array);
+
 /**
  * gpiod_set_raw_value() - assign a gpio's raw value
  * @desc: gpio whose value will be assigned
@@ -873,8 +889,115 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 
 	return ret ? ERR_PTR(ret): desc;
 }
+
+/**
+ * gpiod_count - return the number of GPIOs associated with a device / function
+ *		or -ENOENT if no GPIO has been assigned to the requested function
+ * @dev:	GPIO consumer, can be NULL for system-global GPIOs
+ * @_con_id:	function within the GPIO consumer
+ */
+int gpiod_count(struct device *dev, const char *con_id)
+{
+	struct device_node *np = dev_of_node(dev);
+	struct property *pp;
+
+	if (!np)
+		return -ENODEV;
+
+	pp = of_find_gpio_property(np, con_id);
+	if (!pp)
+		return -ENOENT;
+
+	return of_gpio_named_count(np, pp->name);
+}
+EXPORT_SYMBOL_GPL(gpiod_count);
+
+/**
+ * gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function
+ * @dev:	GPIO consumer, can be NULL for system-global GPIOs
+ * @con_id:	function within the GPIO consumer
+ * @flags:	optional GPIO initialization flags
+ *
+ * This function acquires all the GPIOs defined under a given function.
+ *
+ * Return a struct gpio_descs containing an array of descriptors, -ENOENT if
+ * no GPIO has been assigned to the requested function, or another IS_ERR()
+ * code if an error occurred while trying to acquire the GPIOs.
+ */
+struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
+						const char *con_id,
+						enum gpiod_flags flags)
+{
+	struct gpio_desc *desc;
+	struct gpio_descs *descs;
+	int count;
+
+	count = gpiod_count(dev, con_id);
+	if (count < 0)
+		return ERR_PTR(count);
+
+	descs = kzalloc(struct_size(descs, desc, count), GFP_KERNEL);
+	if (!descs)
+		return ERR_PTR(-ENOMEM);
+
+	for (descs->ndescs = 0; descs->ndescs < count; descs->ndescs++) {
+		desc = dev_gpiod_get_index(dev, dev_of_node(dev), con_id,
+					   descs->ndescs, flags, NULL);
+		if (IS_ERR(desc)) {
+			gpiod_put_array(descs);
+			return ERR_CAST(desc);
+		}
+
+		descs->desc[descs->ndescs] = desc;
+	}
+
+	return descs;
+}
+EXPORT_SYMBOL_GPL(gpiod_get_array);
+
 #endif
 
+static int gpiod_set_array_value_complex(bool raw,
+					 unsigned int array_size,
+					 struct gpio_desc **desc_array,
+					 struct gpio_array *array_info,
+					 unsigned long *value_bitmap)
+{
+	int i;
+
+	BUG_ON(array_info != NULL);
+
+	for (i = 0; i < array_size; i++)
+		gpiod_set_value(desc_array[i], test_bit(i, value_bitmap));
+
+	return 0;
+}
+
+/**
+ * gpiod_set_array_value() - assign values to an array of GPIOs
+ * @array_size: number of elements in the descriptor array / value bitmap
+ * @desc_array: array of GPIO descriptors whose values will be assigned
+ * @array_info: information on applicability of fast bitmap processing path
+ * @value_bitmap: bitmap of values to assign
+ *
+ * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
+ * into account. NOTE: This function has no special handling for GPIOs
+ * in the same bank that could've been set atomically: GPIO sequencing
+ * is not guaranteed to always remain in the same order.
+ */
+int gpiod_set_array_value(unsigned int array_size,
+			  struct gpio_desc **desc_array,
+			  struct gpio_array *array_info,
+			  unsigned long *value_bitmap)
+{
+	if (!desc_array)
+		return -EINVAL;
+	return gpiod_set_array_value_complex(false, array_size,
+					     desc_array, array_info,
+					     value_bitmap);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_array_value);
+
 int gpiochip_add(struct gpio_chip *chip)
 {
 	int i;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 2547e4ba7be3..531ed1472546 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -26,6 +26,24 @@ enum gpiod_flags {
 #define gpiod_not_found(desc)	(IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
 
 struct gpio_desc;
+struct gpio_array;
+
+/**
+ * struct gpio_descs - Struct containing an array of descriptors that can be
+ *                     obtained using gpiod_get_array()
+ *
+ * @info:	Pointer to the opaque gpio_array structure
+ * @ndescs:	Number of held descriptors
+ * @desc:	Array of pointers to GPIO descriptors
+ */
+struct gpio_descs {
+	unsigned int ndescs;
+	/* info is used for fastpath, which we don't have in barebox.
+	 * We define the member anyway, as not to change API
+	 */
+	struct gpio_array *info;
+	DECLARE_FLEX_ARRAY(struct gpio_desc *, desc);
+};
 
 #if defined(CONFIG_OFDEVICE) && defined(CONFIG_GPIOLIB)
 
@@ -62,6 +80,19 @@ int gpiod_get_value(const struct gpio_desc *desc);
 
 void gpiod_put(struct gpio_desc *desc);
 
+int gpiod_count(struct device *dev, const char *con_id);
+
+struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
+						const char *con_id,
+						enum gpiod_flags flags);
+
+void gpiod_put_array(struct gpio_descs *descs);
+
+int gpiod_set_array_value(unsigned int array_size,
+			  struct gpio_desc **desc_array,
+			  struct gpio_array *array_info,
+			  unsigned long *value_bitmap);
+
 #else
 
 static inline int gpiod_direction_input(struct gpio_desc *desc)
@@ -117,6 +148,33 @@ static inline void gpiod_put(struct gpio_desc *desc)
 	WARN_ON(desc);
 }
 
+static inline int gpiod_count(struct device *dev, const char *con_id)
+{
+	return 0;
+}
+
+static inline struct gpio_descs *__must_check
+gpiod_get_array(struct device *dev, const char *con_id, enum gpiod_flags flags)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline void gpiod_put_array(struct gpio_descs *descs)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(descs);
+}
+
+static inline int gpiod_set_array_value(unsigned int array_size,
+			  struct gpio_desc **desc_array,
+			  struct gpio_array *array_info,
+			  unsigned long *value_bitmap)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc_array);
+	return 0;
+}
+
 #endif
 
 static inline struct gpio_desc *dev_gpiod_get(struct device *dev,
-- 
2.39.2




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

* [PATCH v2 9/9] gpiolib: rename gpioinfo_ to gpiodesc_
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2023-06-22  7:23 ` [PATCH v2 8/9] gpiolib: add support for requesting and setting gpiod arrays Ahmad Fatoum
@ 2023-06-22  7:23 ` Ahmad Fatoum
  2023-06-23  9:34 ` [PATCH v2 0/9] gpio: add proper gpiod API Sascha Hauer
  9 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2023-06-22  7:23 UTC (permalink / raw)
  To: barebox; +Cc: Marco Felsch, Ahmad Fatoum

Before renaming struct gpio_info to struct gpio_desc, the name made
sense. With the rename, it might confuse readers, so rename it to
gpiodesc_ instead. The naming scheme is thus: gpiod_ for public API
and gpiodesc_ for private API.

Suggested-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/gpio/gpiolib.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 2808ac3612fe..bcfdd5a0dd30 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -81,7 +81,7 @@ static struct gpio_desc *gpio_to_desc(unsigned gpio)
 	return NULL;
 }
 
-static unsigned gpioinfo_chip_offset(const struct gpio_desc *desc)
+static unsigned gpiodesc_chip_offset(const struct gpio_desc *desc)
 {
 	return (desc - gpio_desc) - desc->chip->base;
 }
@@ -95,7 +95,7 @@ static int gpio_adjust_value(const struct gpio_desc *desc,
 	return !!value ^ desc->active_low;
 }
 
-static int gpioinfo_request(struct gpio_desc *desc, const char *label)
+static int gpiodesc_request(struct gpio_desc *desc, const char *label)
 {
 	int ret;
 
@@ -108,7 +108,7 @@ static int gpioinfo_request(struct gpio_desc *desc, const char *label)
 
 	if (desc->chip->ops->request) {
 		ret = desc->chip->ops->request(desc->chip,
-					     gpioinfo_chip_offset(desc));
+					     gpiodesc_chip_offset(desc));
 		if (ret)
 			goto done;
 	}
@@ -169,16 +169,16 @@ int gpio_request(unsigned gpio, const char *label)
 		return -ENODEV;
 	}
 
-	return gpioinfo_request(desc, label);
+	return gpiodesc_request(desc, label);
 }
 
-static void gpioinfo_free(struct gpio_desc *desc)
+static void gpiodesc_free(struct gpio_desc *desc)
 {
 	if (!desc->requested)
 		return;
 
 	if (desc->chip->ops->free)
-		desc->chip->ops->free(desc->chip, gpioinfo_chip_offset(desc));
+		desc->chip->ops->free(desc->chip, gpiodesc_chip_offset(desc));
 
 	desc->requested = false;
 	desc->active_low = false;
@@ -190,7 +190,7 @@ void gpio_free(unsigned gpio)
 {
 	struct gpio_desc *desc = gpio_to_desc(gpio);
 
-	gpioinfo_free(desc);
+	gpiodesc_free(desc);
 }
 
 /**
@@ -204,7 +204,7 @@ void gpiod_put(struct gpio_desc *desc)
 	if (!desc)
 		return;
 
-	gpioinfo_free(desc);
+	gpiodesc_free(desc);
 }
 EXPORT_SYMBOL(gpiod_put);
 
@@ -236,7 +236,7 @@ void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 	VALIDATE_DESC_VOID(desc);
 
 	if (desc->chip->ops->set)
-		desc->chip->ops->set(desc->chip, gpioinfo_chip_offset(desc), value);
+		desc->chip->ops->set(desc->chip, gpiodesc_chip_offset(desc), value);
 }
 EXPORT_SYMBOL(gpiod_set_raw_value);
 
@@ -294,7 +294,7 @@ int gpiod_get_raw_value(const struct gpio_desc *desc)
 	if (!desc->chip->ops->get)
 		return -ENOSYS;
 
-	return desc->chip->ops->get(desc->chip, gpioinfo_chip_offset(desc));
+	return desc->chip->ops->get(desc->chip, gpiodesc_chip_offset(desc));
 }
 EXPORT_SYMBOL_GPL(gpiod_get_raw_value);
 
@@ -359,7 +359,7 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
 		return -ENOSYS;
 
 	return desc->chip->ops->direction_output(desc->chip,
-					       gpioinfo_chip_offset(desc), value);
+					       gpiodesc_chip_offset(desc), value);
 }
 EXPORT_SYMBOL(gpiod_direction_output_raw);
 
@@ -426,7 +426,7 @@ int gpiod_direction_input(struct gpio_desc *desc)
 		return -ENOSYS;
 
 	return desc->chip->ops->direction_input(desc->chip,
-					      gpioinfo_chip_offset(desc));
+					      gpiodesc_chip_offset(desc));
 }
 EXPORT_SYMBOL(gpiod_direction_input);
 
@@ -446,7 +446,7 @@ int gpio_direction_input(unsigned gpio)
 }
 EXPORT_SYMBOL(gpio_direction_input);
 
-static int gpioinfo_request_one(struct gpio_desc *desc, unsigned long flags,
+static int gpiodesc_request_one(struct gpio_desc *desc, unsigned long flags,
 				const char *label)
 {
 	int err;
@@ -461,7 +461,7 @@ static int gpioinfo_request_one(struct gpio_desc *desc, unsigned long flags,
 	const bool init_active = (flags & GPIOF_INIT_ACTIVE) == GPIOF_INIT_ACTIVE;
 	const bool init_high   = (flags & GPIOF_INIT_HIGH) == GPIOF_INIT_HIGH;
 
-	err = gpioinfo_request(desc, label);
+	err = gpiodesc_request(desc, label);
 	if (err)
 		return err;
 
@@ -475,7 +475,7 @@ static int gpioinfo_request_one(struct gpio_desc *desc, unsigned long flags,
 		err = gpiod_direction_output_raw(desc, init_high);
 
 	if (err)
-		gpioinfo_free(desc);
+		gpiodesc_free(desc);
 
 	return err;
 }
@@ -493,7 +493,7 @@ int gpio_request_one(unsigned gpio, unsigned long flags, const char *label)
 	if (!desc)
 		return -ENODEV;
 
-	return gpioinfo_request_one(desc, flags, label);
+	return gpiodesc_request_one(desc, flags, label);
 }
 EXPORT_SYMBOL_GPL(gpio_request_one);
 
@@ -884,7 +884,7 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 			label = dev_name(dev);
 	}
 
-	ret = gpioinfo_request_one(desc, flags, label);
+	ret = gpiodesc_request_one(desc, flags, label);
 	free(buf);
 
 	return ret ? ERR_PTR(ret): desc;
-- 
2.39.2




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

* Re: [PATCH v2 0/9] gpio: add proper gpiod API
  2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2023-06-22  7:23 ` [PATCH v2 9/9] gpiolib: rename gpioinfo_ to gpiodesc_ Ahmad Fatoum
@ 2023-06-23  9:34 ` Sascha Hauer
  9 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2023-06-23  9:34 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Thu, Jun 22, 2023 at 09:23:20AM +0200, Ahmad Fatoum wrote:
> The gpiod_ (GPIO descriptor) API used with Linux differs from barebox'
> normal GPIO API:
> 
>  - gpiod handles are opaque pointers and not an integer, which users
>    have an expectation of stability for
> 
>  - gpiod API uses logic levels by default with separate raw API for
>    physical level instead of physical level by default and separate
>    API taking active level into account.
> 
> The barebox gpiod_ API mimics the latter point, but still uses integers
> requiring ugly and arguably error prone conversions when porting kernel
> code.
> 
> This series fixes that by adding proper struct gpio_desc API like in
> Linux and then builds upon that to port the kernel gpio-mux driver.

Applied, thanks

Sascha

> 
> v1 -> v2:
>   - drop unrelated help command patch
>   - drop gpio-mux patch for separate sending out
>   - rebase onto next
>   - fix and add stubs for !CONFIG_GPIOLIB
>   - rename internal gpioinfo_ functions to gpiodesc_
> 
> v1 was here:
> https://lore.barebox.org/barebox/20230621092700.GE18491@pengutronix.de/T/#t
> 
> Ahmad Fatoum (9):
>   driver: include dev_print and family from <driver.h>
>   include: linux/printk: define new dev_errp_probe
>   gpio: have gpiod_ functions return and accept pointers
>   gpio: gpiolib: rename struct gpio_info to gpio_desc
>   gpiolib: export proper gpio descriptor API
>   bitmap: implement bitmap_{to,from}_arr{32,64}
>   gpiolib: factor out finding gpio property
>   gpiolib: add support for requesting and setting gpiod arrays
>   gpiolib: rename gpioinfo_ to gpiodesc_
> 
>  drivers/gpio/gpio-pca953x.c              |   9 +-
>  drivers/gpio/gpiolib.c                   | 528 +++++++++++++++++------
>  drivers/mci/mci_spi.c                    |  13 +-
>  drivers/mtd/nand/atmel/nand-controller.c |  40 +-
>  drivers/mtd/nand/nand_base.c             |   6 +-
>  drivers/net/designware_eqos.c            |  26 +-
>  drivers/net/ksz8873.c                    |  13 +-
>  drivers/net/ksz9477.c                    |  13 +-
>  drivers/net/realtek-dsa/realtek-mdio.c   |  10 +-
>  drivers/net/realtek-dsa/realtek-smi.c    |  18 +-
>  drivers/net/realtek-dsa/realtek.h        |   6 +-
>  drivers/net/sja1105.c                    |  25 +-
>  drivers/nvmem/starfive-otp.c             |  12 +-
>  drivers/pci/pcie-dw-rockchip.c           |  14 +-
>  drivers/power/reset/gpio-poweroff.c      |  14 +-
>  drivers/power/reset/gpio-restart.c       |  23 +-
>  drivers/regulator/fixed.c                |  27 +-
>  drivers/sound/gpio-beeper.c              |  14 +-
>  drivers/usb/misc/onboard_usb_hub.c       |  11 +-
>  drivers/video/mipi_dbi.c                 |   8 +-
>  drivers/video/panel-ilitek-ili9341.c     |  17 +-
>  drivers/video/panel-mipi-dbi.c           |  17 +-
>  drivers/watchdog/gpio_wdt.c              |  22 +-
>  include/driver.h                         |   1 +
>  include/gpiod.h                          |  79 +---
>  include/linux/bitmap.h                   | 101 +++++
>  include/linux/gpio/consumer.h            | 224 ++++++++++
>  include/linux/mtd/rawnand.h              |   4 +-
>  include/linux/printk.h                   |   3 +
>  include/video/mipi_dbi.h                 |   7 +-
>  lib/bitmap.c                             | 103 +++++
>  31 files changed, 1019 insertions(+), 389 deletions(-)
>  create mode 100644 include/linux/gpio/consumer.h
> 
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

end of thread, other threads:[~2023-06-23  9:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-22  7:23 [PATCH v2 0/9] gpio: add proper gpiod API Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 1/9] driver: include dev_print and family from <driver.h> Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 2/9] include: linux/printk: define new dev_errp_probe Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 3/9] gpio: have gpiod_ functions return and accept pointers Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 4/9] gpio: gpiolib: rename struct gpio_info to gpio_desc Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 5/9] gpiolib: export proper gpio descriptor API Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 6/9] bitmap: implement bitmap_{to,from}_arr{32,64} Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 7/9] gpiolib: factor out finding gpio property Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 8/9] gpiolib: add support for requesting and setting gpiod arrays Ahmad Fatoum
2023-06-22  7:23 ` [PATCH v2 9/9] gpiolib: rename gpioinfo_ to gpiodesc_ Ahmad Fatoum
2023-06-23  9:34 ` [PATCH v2 0/9] gpio: add proper gpiod API Sascha Hauer

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