mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support
@ 2023-06-14 13:54 Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 01/10] driver: include dev_print and family from <driver.h> Ahmad Fatoum
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 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.

Ahmad Fatoum (10):
  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}
  commands: help: ignore options after first regular argument
  gpiolib: factor out finding gpio property
  gpiolib: add support for requesting and setting gpiod arrays
  drivers: port Linux mux framework and gpio-mux driver

 commands/help.c                          |  19 +-
 drivers/Kconfig                          |   1 +
 drivers/Makefile                         |   1 +
 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             |   4 +-
 drivers/mux/Kconfig                      |  29 ++
 drivers/mux/Makefile                     |  10 +
 drivers/mux/core.c                       | 472 ++++++++++++++++++++
 drivers/mux/gpio.c                       | 103 +++++
 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            | 137 ++++++
 include/linux/mtd/rawnand.h              |   4 +-
 include/linux/printk.h                   |   3 +
 include/video/mipi_dbi.h                 |   7 +-
 lib/bitmap.c                             | 103 +++++
 38 files changed, 1559 insertions(+), 395 deletions(-)
 create mode 100644 drivers/mux/Kconfig
 create mode 100644 drivers/mux/Makefile
 create mode 100644 drivers/mux/core.c
 create mode 100644 drivers/mux/gpio.c
 create mode 100644 include/linux/gpio/consumer.h

-- 
2.39.2




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

* [PATCH 01/10] driver: include dev_print and family from <driver.h>
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 02/10] include: linux/printk: define new dev_errp_probe Ahmad Fatoum
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

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

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 613a0e4cfad2..9b6f01ae41a3 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -10,6 +10,7 @@
 #include <linux/ioport.h>
 #include <linux/uuid.h>
 #include <linux/bitops.h>
+#include <linux/printk.h>
 #include <of.h>
 #include <filetype.h>
 
-- 
2.39.2




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

* [PATCH 02/10] include: linux/printk: define new dev_errp_probe
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 01/10] driver: include dev_print and family from <driver.h> Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 03/10] gpio: have gpiod_ functions return and accept pointers Ahmad Fatoum
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: 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.

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] 18+ messages in thread

* [PATCH 03/10] gpio: have gpiod_ functions return and accept pointers
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 01/10] driver: include dev_print and family from <driver.h> Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 02/10] include: linux/printk: define new dev_errp_probe Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 04/10] gpio: gpiolib: rename struct gpio_info to gpio_desc Ahmad Fatoum
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: 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.

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             |   4 +-
 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, 344 insertions(+), 271 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 f353ce50196c..0bceffbc0d6d 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>
 
@@ -662,19 +662,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)
@@ -683,17 +684,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;
@@ -707,10 +710,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..53e58a67155f 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -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] 18+ messages in thread

* [PATCH 04/10] gpio: gpiolib: rename struct gpio_info to gpio_desc
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 03/10] gpio: have gpiod_ functions return and accept pointers Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 05/10] gpiolib: export proper gpio descriptor API Ahmad Fatoum
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: 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.

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 0bceffbc0d6d..402d408be071 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);
 
@@ -774,9 +774,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
@@ -801,38 +801,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] 18+ messages in thread

* [PATCH 05/10] gpiolib: export proper gpio descriptor API
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 04/10] gpio: gpiolib: rename struct gpio_info to gpio_desc Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-20  5:20   ` Marco Felsch
  2023-06-14 13:54 ` [PATCH 06/10] bitmap: implement bitmap_{to,from}_arr{32,64} Ahmad Fatoum
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: 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.

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

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 402d408be071..25d91d250dc8 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);
@@ -690,7 +823,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;
 		}
 	}
@@ -710,7 +843,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..c89b0c48ee2b 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -5,8 +5,6 @@
 #include <gpio.h>
 #include <of_gpio.h>
 #include <driver.h>
-#include <linux/err.h>
-#include <errno.h>
 #include <linux/iopoll.h>
 
 /**
@@ -24,32 +22,10 @@ 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
 
 /* returned gpio descriptor can be passed to any normal gpio_* function */
@@ -60,7 +36,7 @@ struct gpio_desc *dev_gpiod_get_index(struct device *dev,
 			const char *label);
 
 #else
-static inline struct gpio_desc *dev_gpiod_get_index(struct device *dev,
+static inline gpio_desc *dev_gpiod_get_index(struct device *dev,
 		struct device_node *np,
 		const char *_con_id, int index,
 		enum gpiod_flags flags,
@@ -79,6 +55,8 @@ static inline struct gpio_desc *dev_gpiod_get(struct device *dev,
 	return dev_gpiod_get_index(dev, np, con_id, 0, flags, label);
 }
 
+void gpiod_put(struct gpio_desc *desc);
+
 static inline struct gpio_desc *gpiod_get(struct device *dev,
 			    const char *_con_id,
 			    enum gpiod_flags flags)
@@ -99,36 +77,16 @@ 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;
+int gpiod_direction_input(struct gpio_desc *gi);
 
-	return gpio_direction_input(__tmp_desc_to_gpio(gpiod));
-}
+int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
+int gpiod_direction_output(struct gpio_desc *desc, int value);
 
-static inline int gpiod_direction_output(struct gpio_desc *gpiod, bool value)
-{
-	if (gpiod_not_found(gpiod))
-		return 0;
+void gpiod_set_raw_value(struct gpio_desc *desc, int value);
+void gpiod_set_value(struct gpio_desc *desc, int value);
 
-	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));
-}
+int gpiod_get_raw_value(const struct gpio_desc *desc);
+int gpiod_get_value(const struct gpio_desc *desc);
 
 /**
  * gpiod_poll_timeout_us - poll till gpio descriptor reaches requested active state
-- 
2.39.2




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

* [PATCH 06/10] bitmap: implement bitmap_{to,from}_arr{32,64}
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 05/10] gpiolib: export proper gpio descriptor API Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 07/10] commands: help: ignore options after first regular argument Ahmad Fatoum
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: 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.

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] 18+ messages in thread

* [PATCH 07/10] commands: help: ignore options after first regular argument
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 06/10] bitmap: implement bitmap_{to,from}_arr{32,64} Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 08/10] gpiolib: factor out finding gpio property Ahmad Fatoum
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

If some-command -n foo bar -v fails, it would be nice to be able to
just stick help in front of the command to get help text. This doesn't
work currently, because getopt called for help will complain about not
knowing some-command's options. Fix this by only parsing help options up
to the first non-option argument (i.e. one that doesn't start with `-').

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/help.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/commands/help.c b/commands/help.c
index ba8542b90f01..87c1368d746d 100644
--- a/commands/help.c
+++ b/commands/help.c
@@ -3,7 +3,6 @@
 
 #include <common.h>
 #include <command.h>
-#include <getopt.h>
 #include <complete.h>
 
 
@@ -72,10 +71,16 @@ static void list_commands(int verbose)
 static int do_help(int argc, char *argv[])
 {
 	struct command *cmdtp;
-	int opt, verbose = 0, all = 0;
+	int verbose = 0, all = 0;
+	int argi;
 
-	while ((opt = getopt(argc, argv, "va")) > 0) {
-		switch (opt) {
+	/* We can't use getopt() here because we want to stop at the first
+	 * non-option to support, so we can just prefix help in front
+	 * of a command with options.
+	 */
+	argi = 1;
+	while (argi < argc && *argv[argi] == '-') {
+		switch (argv[argi++][1]) {
 		case 'v':
 			verbose = 1;
 			break;
@@ -93,20 +98,20 @@ static int do_help(int argc, char *argv[])
 		return 0;
 	}
 
-	if (optind == argc) {	/* show list of commands */
+	if (argi == argc) {	/* show list of commands */
 		list_commands(verbose);
 		return 0;
 	}
 
 
 	/* command help (long version) */
-	if ((cmdtp = find_cmd(argv[optind])) != NULL) {
+	if ((cmdtp = find_cmd(argv[argi])) != NULL) {
 		barebox_cmd_usage(cmdtp);
 		return 0;
 	} else {
 		printf ("Unknown command '%s' - try 'help'"
 			" without arguments for list of all"
-			" known commands\n\n", argv[optind]
+			" known commands\n\n", argv[argi]
 				);
 		return 1;
 	}
-- 
2.39.2




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

* [PATCH 08/10] gpiolib: factor out finding gpio property
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 07/10] commands: help: ignore options after first regular argument Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 09/10] gpiolib: add support for requesting and setting gpiod arrays Ahmad Fatoum
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: 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.

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 25d91d250dc8..3e57cf6239c3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -794,21 +794,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)
@@ -819,26 +810,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] 18+ messages in thread

* [PATCH 09/10] gpiolib: add support for requesting and setting gpiod arrays
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 08/10] gpiolib: factor out finding gpio property Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 13:54 ` [PATCH 10/10] drivers: port Linux mux framework and gpio-mux driver Ahmad Fatoum
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: 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.

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

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3e57cf6239c3..40010262bd81 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
@@ -865,8 +881,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 c89b0c48ee2b..6ad3b265c835 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -25,6 +25,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);
+};
 
 #ifdef CONFIG_OFDEVICE
 
@@ -103,4 +121,17 @@ int gpiod_get_value(const struct gpio_desc *desc);
 				   __state == (active), timeout_us);	\
 	})
 
+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);
+
 #endif
-- 
2.39.2




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

* [PATCH 10/10] drivers: port Linux mux framework and gpio-mux driver
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 09/10] gpiolib: add support for requesting and setting gpiod arrays Ahmad Fatoum
@ 2023-06-14 13:54 ` Ahmad Fatoum
  2023-06-14 15:21   ` Ahmad Fatoum
  2023-06-20  5:51 ` [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Marco Felsch
  2023-06-21  9:27 ` Sascha Hauer
  11 siblings, 1 reply; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 13:54 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The driver builds a single multiplexer controller using a number
of gpio pins. For N pins, there will be 2^N possible multiplexer
states.

The GPIO pins can be connected (by the hardware) to several
multiplexers. Consumer drivers in turn can reference the mux in
the device tree to control it. User interaction over the shell
is possible via the .state parameter that holds the currently
active state (or -1 if mux was not yet used). An additional .idle
parameter informs the user whether the mux has been claimed by a
consumer.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/Kconfig      |   1 +
 drivers/Makefile     |   1 +
 drivers/mux/Kconfig  |  29 +++
 drivers/mux/Makefile |  10 +
 drivers/mux/core.c   | 472 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mux/gpio.c   | 103 ++++++++++
 6 files changed, 616 insertions(+)
 create mode 100644 drivers/mux/Kconfig
 create mode 100644 drivers/mux/Makefile
 create mode 100644 drivers/mux/core.c
 create mode 100644 drivers/mux/gpio.c

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 2636eed1a3b5..aa12597df637 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -19,6 +19,7 @@ source "drivers/clk/Kconfig"
 source "drivers/clocksource/Kconfig"
 source "drivers/mfd/Kconfig"
 source "drivers/misc/Kconfig"
+source "drivers/mux/Kconfig"
 source "drivers/led/Kconfig"
 source "drivers/eeprom/Kconfig"
 source "drivers/input/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index fa899a2ab8c3..61c4c9c94020 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -20,6 +20,7 @@ obj-y	+= eeprom/
 obj-$(CONFIG_PWM) += pwm/
 obj-y	+= input/
 obj-y	+= misc/
+obj-$(CONFIG_MULTIPLEXER)	+= mux/
 obj-$(CONFIG_NVMEM) += nvmem/
 obj-y	+= dma/
 obj-y  += watchdog/
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
new file mode 100644
index 000000000000..c68f1d4130ee
--- /dev/null
+++ b/drivers/mux/Kconfig
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Multiplexer devices
+#
+
+config MULTIPLEXER
+	tristate
+	prompt "Multiplexer driver support" if COMPILE_TEST
+
+menu "Multiplexer drivers"
+	depends on MULTIPLEXER
+
+config MUX_GPIO
+	tristate "GPIO-controlled Multiplexer"
+	depends on GPIOLIB || COMPILE_TEST
+	help
+	  GPIO-controlled Multiplexer controller.
+
+	  The driver builds a single multiplexer controller using a number
+	  of gpio pins. For N pins, there will be 2^N possible multiplexer
+	  states. The GPIO pins can be connected (by the hardware) to several
+	  multiplexers.
+
+	  The barebox driver doesn't implement Linux' fastpath, which enables
+	  atomically switching GPIOs in the same bank where possible.
+	  This means that board code authors need to ensure that intermediate
+	  states when switching some of the GPIOs don't break anything.
+
+endmenu
diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
new file mode 100644
index 000000000000..22d285ed9d4b
--- /dev/null
+++ b/drivers/mux/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for multiplexer devices.
+#
+
+mux-core-objs			:= core.o
+mux-gpio-objs			:= gpio.o
+
+obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
+obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
new file mode 100644
index 000000000000..6d9fae423319
--- /dev/null
+++ b/drivers/mux/core.c
@@ -0,0 +1,472 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Multiplexer subsystem
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ */
+
+#define pr_fmt(fmt) "mux-core: " fmt
+
+#include <linux/ktime.h>
+#include <driver.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <init.h>
+#include <module.h>
+#include <linux/mux/consumer.h>
+#include <linux/mux/driver.h>
+#include <of.h>
+#include <stdio.h>
+#include <malloc.h>
+
+/*
+ * The idle-as-is "state" is not an actual state that may be selected, it
+ * only implies that the state should not be changed. So, use that state
+ * as indication that the cached state of the multiplexer is unknown.
+ */
+#define MUX_CACHE_UNKNOWN MUX_IDLE_AS_IS
+
+/**
+ * struct mux_state -	Represents a mux controller state specific to a given
+ *			consumer.
+ * @mux:		Pointer to a mux controller.
+ * @state:		State of the mux to be selected.
+ *
+ * This structure is specific to the consumer that acquires it and has
+ * information specific to that consumer.
+ */
+struct mux_state {
+	struct mux_control *mux;
+	unsigned int state;
+};
+
+static LIST_HEAD(mux_chips);
+
+static int mux_register_dev(struct mux_chip *mux_chip, const char *name, int id)
+{
+	mux_chip->dev.id = id;
+	dev_set_name(&mux_chip->dev, name);
+
+	return register_device(&mux_chip->dev);
+}
+
+/**
+ * mux_chip_alloc() - Allocate a mux-chip.
+ * @dev: The parent device implementing the mux interface.
+ * @controllers: The number of mux controllers to allocate for this chip.
+ * @sizeof_priv: Size of extra memory area for private use by the caller.
+ *
+ * After allocating the mux-chip with the desired number of mux controllers
+ * but before registering the chip, the mux driver is required to configure
+ * the number of valid mux states in the mux_chip->mux[N].states members and
+ * the desired idle state in the returned mux_chip->mux[N].idle_state members.
+ * The default idle state is MUX_IDLE_AS_IS. The mux driver also needs to
+ * provide a pointer to the operations struct in the mux_chip->ops member
+ * before registering the mux-chip with mux_chip_register.
+ *
+ * Return: A pointer to the new mux-chip, or an ERR_PTR with a negative errno.
+ */
+struct mux_chip *mux_chip_alloc(struct device *dev,
+				unsigned int controllers, size_t sizeof_priv)
+{
+	struct mux_chip *mux_chip;
+	int i;
+
+	if (WARN_ON(!dev || !controllers))
+		return ERR_PTR(-EINVAL);
+
+	mux_chip = kzalloc(sizeof(*mux_chip) +
+			   controllers * sizeof(*mux_chip->mux) +
+			   sizeof_priv, GFP_KERNEL);
+	if (!mux_chip)
+		return ERR_PTR(-ENOMEM);
+
+
+	mux_chip->mux = (struct mux_control *)(mux_chip + 1);
+	mux_chip->dev.parent = dev;
+	mux_chip->dev.of_node = dev->of_node;
+	mux_chip->dev.priv = mux_chip;
+
+	mux_chip->controllers = controllers;
+	for (i = 0; i < controllers; ++i) {
+		struct mux_control *mux = &mux_chip->mux[i];
+
+		mux->chip = mux_chip;
+		mux->cached_state = MUX_CACHE_UNKNOWN;
+		mux->idle_state = MUX_IDLE_AS_IS;
+		mux->idle = true;
+		mux->last_change = ktime_get();
+		mux->dev.id = i;
+		mux->dev.parent = &mux_chip->dev;
+	}
+
+	return mux_chip;
+}
+EXPORT_SYMBOL_GPL(mux_chip_alloc);
+
+static int mux_control_set(struct mux_control *mux, int state)
+{
+	int ret = mux->chip->ops->set(mux, state);
+
+	mux->cached_state = ret < 0 ? MUX_CACHE_UNKNOWN : state;
+	if (ret >= 0)
+		mux->last_change = ktime_get();
+
+	return ret;
+}
+
+static int mux_control_set_param(struct param_d *param, void *priv)
+{
+	struct mux_control *mux = priv;
+	int state = mux->cached_state;
+
+	if (state < 0 || state >= mux->states)
+		return -EINVAL;
+
+	return mux_control_set(mux, state);
+}
+
+/**
+ * mux_chip_register() - Register a mux-chip, thus readying the controllers
+ *			 for use.
+ * @mux_chip: The mux-chip to register.
+ *
+ * Return: Zero on success or a negative errno on error.
+ */
+int mux_chip_register(struct mux_chip *mux_chip)
+{
+	int i;
+	int ret;
+	const char *alias;
+	struct device_node *np = mux_chip->dev.of_node;
+
+	for (i = 0; i < mux_chip->controllers; ++i) {
+		struct mux_control *mux = &mux_chip->mux[i];
+
+		if (mux->idle_state == mux->cached_state)
+			continue;
+
+		ret = mux_control_set(mux, mux->idle_state);
+		if (ret < 0) {
+			dev_err(&mux_chip->dev, "unable to set idle state\n");
+			return ret;
+		}
+	}
+
+	alias = np ? of_alias_get(np) : NULL;
+	if (alias)
+		ret = mux_register_dev(mux_chip, alias, DEVICE_ID_SINGLE);
+	if (!alias || ret)
+		ret = mux_register_dev(mux_chip, "muxchip", DEVICE_ID_DYNAMIC);
+
+	if (ret < 0) {
+		dev_err(&mux_chip->dev,
+			"register_device failed in %s: %d\n", __func__, ret);
+		return ret;
+	}
+
+	if (IS_ENABLED(CONFIG_PARAMETER)) {
+		struct device *dev = NULL;
+
+		if (mux_chip->controllers == 1)
+			dev = &mux_chip->dev;
+
+		for (i = 0; i < mux_chip->controllers; ++i) {
+			struct mux_control *mux = &mux_chip->mux[i];
+			struct device *dev;
+
+			if (mux_chip->controllers == 1) {
+				dev = &mux_chip->dev;
+			} else  {
+				dev = &mux->dev;
+				dev_set_name(dev, "muxchip%uc", mux_chip->dev.id);
+				if (register_device(dev))
+					continue;
+			}
+
+			dev_add_param_bool_ro(dev, "idle", &mux->idle);
+			dev_add_param_int(dev, "state", mux_control_set_param,
+					  NULL, &mux->cached_state, "%d", mux);
+		}
+	}
+
+	list_add(&mux_chip->list, &mux_chips);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mux_chip_register);
+
+/**
+ * mux_control_states() - Query the number of multiplexer states.
+ * @mux: The mux-control to query.
+ *
+ * Return: The number of multiplexer states.
+ */
+unsigned int mux_control_states(struct mux_control *mux)
+{
+	return mux->states;
+}
+EXPORT_SYMBOL_GPL(mux_control_states);
+
+/*
+ * The mux->idle must be clear when calling this function.
+ */
+static int __mux_control_select(struct mux_control *mux, int state)
+{
+	int ret;
+
+	if (WARN_ON(state < 0 || state >= mux->states))
+		return -EINVAL;
+
+	if (mux->cached_state == state)
+		return 0;
+
+	ret = mux_control_set(mux, state);
+	if (ret >= 0)
+		return 0;
+
+	/* The mux update failed, try to revert if appropriate... */
+	if (mux->idle_state != MUX_IDLE_AS_IS)
+		mux_control_set(mux, mux->idle_state);
+
+	return ret;
+}
+
+static void mux_control_delay(struct mux_control *mux, unsigned int delay_us)
+{
+	ktime_t delayend;
+	s64 remaining;
+
+	if (!delay_us)
+		return;
+
+	delayend = ktime_add_us(mux->last_change, delay_us);
+	remaining = ktime_us_delta(delayend, ktime_get());
+	if (remaining > 0)
+		udelay(remaining);
+}
+
+/**
+ * mux_control_try_select_delay() - Try to select the given multiplexer state.
+ * @mux: The mux-control to request a change of state from.
+ * @state: The new requested state.
+ * @delay_us: The time to delay (in microseconds) if the mux state is changed.
+ *
+ * On successfully selecting the mux-control state, it will be locked until
+ * mux_control_deselect() is called.
+ *
+ * Therefore, make sure to call mux_control_deselect() when the operation is
+ * complete and the mux-control is free for others to use, but do not call
+ * mux_control_deselect() if mux_control_try_select() fails.
+ *
+ * Return: 0 when the mux-control state has the requested state or a negative
+ * errno on error. Specifically -EBUSY if the mux-control is contended.
+ */
+int mux_control_try_select_delay(struct mux_control *mux, unsigned int state,
+				 unsigned int delay_us)
+{
+	int ret;
+
+	if (!mux->idle)
+		return -EBUSY;
+	mux->idle = false;
+
+	ret = __mux_control_select(mux, state);
+	if (ret >= 0)
+		mux_control_delay(mux, delay_us);
+
+	if (ret < 0)
+		mux->idle = true;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_try_select_delay);
+
+/**
+ * mux_state_try_select_delay() - Try to select the given multiplexer state.
+ * @mstate: The mux-state to select.
+ * @delay_us: The time to delay (in microseconds) if the mux state is changed.
+ *
+ * On successfully selecting the mux-state, its mux-control will be locked
+ * until mux_state_deselect() is called.
+ *
+ * Therefore, make sure to call mux_state_deselect() when the operation is
+ * complete and the mux-control is free for others to use, but do not call
+ * mux_state_deselect() if mux_state_try_select() fails.
+ *
+ * Return: 0 when the mux-state has been selected or a negative errno on
+ * error. Specifically -EBUSY if the mux-control is contended.
+ */
+int mux_state_try_select_delay(struct mux_state *mstate, unsigned int delay_us)
+{
+	return mux_control_try_select_delay(mstate->mux, mstate->state, delay_us);
+}
+EXPORT_SYMBOL_GPL(mux_state_try_select_delay);
+
+/**
+ * mux_control_deselect() - Deselect the previously selected multiplexer state.
+ * @mux: The mux-control to deselect.
+ *
+ * It is required that a single call is made to mux_control_deselect() for
+ * each and every successful call made to either of mux_control_select() or
+ * mux_control_try_select().
+ *
+ * Return: 0 on success and a negative errno on error. An error can only
+ * occur if the mux has an idle state. Note that even if an error occurs, the
+ * mux-control is unlocked and is thus free for the next access.
+ */
+int mux_control_deselect(struct mux_control *mux)
+{
+	int ret = 0;
+
+	if (mux->idle_state != MUX_IDLE_AS_IS &&
+	    mux->idle_state != mux->cached_state)
+		ret = mux_control_set(mux, mux->idle_state);
+
+	mux->idle = true;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_deselect);
+
+/**
+ * mux_state_deselect() - Deselect the previously selected multiplexer state.
+ * @mstate: The mux-state to deselect.
+ *
+ * It is required that a single call is made to mux_state_deselect() for
+ * each and every successful call made to either of mux_state_select() or
+ * mux_state_try_select().
+ *
+ * Return: 0 on success and a negative errno on error. An error can only
+ * occur if the mux has an idle state. Note that even if an error occurs, the
+ * mux-control is unlocked and is thus free for the next access.
+ */
+int mux_state_deselect(struct mux_state *mstate)
+{
+	return mux_control_deselect(mstate->mux);
+}
+EXPORT_SYMBOL_GPL(mux_state_deselect);
+
+/* Note this function returns a reference to the mux_chip dev. */
+static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
+{
+	struct mux_chip *chip;
+
+	list_for_each_entry(chip, &mux_chips, list) {
+		if (chip->dev.device_node == np)
+			return chip;
+	}
+
+	return NULL;
+}
+
+/*
+ * mux_get() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ * @state: Pointer to where the requested state is returned, or NULL when
+ *         the required multiplexer states are handled by other means.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+static struct mux_control *mux_get(struct device *dev, const char *mux_name,
+				   unsigned int *state)
+{
+	struct device_node *np = dev->of_node;
+	struct of_phandle_args args;
+	struct mux_chip *mux_chip;
+	unsigned int controller;
+	int index = 0;
+	int ret;
+
+	if (mux_name) {
+		if (state)
+			index = of_property_match_string(np, "mux-state-names",
+							 mux_name);
+		else
+			index = of_property_match_string(np, "mux-control-names",
+							 mux_name);
+		if (index < 0) {
+			dev_err(dev, "mux controller '%s' not found\n",
+				mux_name);
+			return ERR_PTR(index);
+		}
+	}
+
+	if (state)
+		ret = of_parse_phandle_with_args(np,
+						 "mux-states", "#mux-state-cells",
+						 index, &args);
+	else
+		ret = of_parse_phandle_with_args(np,
+						 "mux-controls", "#mux-control-cells",
+						 index, &args);
+	if (ret) {
+		dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
+			np, state ? "state" : "control", mux_name ?: "", index);
+		return ERR_PTR(ret);
+	}
+
+	mux_chip = of_find_mux_chip_by_node(args.np);
+	of_node_put(args.np);
+	if (!mux_chip)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	controller = 0;
+	if (state) {
+		if (args.args_count > 2 || args.args_count == 0 ||
+		    (args.args_count < 2 && mux_chip->controllers > 1)) {
+			dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
+				np, args.np);
+			put_device(&mux_chip->dev);
+			return ERR_PTR(-EINVAL);
+		}
+
+		if (args.args_count == 2) {
+			controller = args.args[0];
+			*state = args.args[1];
+		} else {
+			*state = args.args[0];
+		}
+
+	} else {
+		if (args.args_count > 1 ||
+		    (!args.args_count && mux_chip->controllers > 1)) {
+			dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
+				np, args.np);
+			put_device(&mux_chip->dev);
+			return ERR_PTR(-EINVAL);
+		}
+
+		if (args.args_count)
+			controller = args.args[0];
+	}
+
+	if (controller >= mux_chip->controllers) {
+		dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
+			np, controller, args.np);
+		put_device(&mux_chip->dev);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return &mux_chip->mux[controller];
+}
+
+/**
+ * mux_control_get() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+{
+	return mux_get(dev, mux_name, NULL);
+}
+EXPORT_SYMBOL_GPL(mux_control_get);
+
+MODULE_DESCRIPTION("Multiplexer subsystem");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
new file mode 100644
index 000000000000..10d68b63cf57
--- /dev/null
+++ b/drivers/mux/gpio.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPIO-controlled multiplexer driver
+ *
+ * Copyright (C) 2017 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ */
+
+#include <linux/bitmap.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mod_devicetable.h>
+#include <module.h>
+#include <linux/mux/driver.h>
+#include <driver.h>
+#include <linux/property.h>
+
+struct mux_gpio {
+	struct gpio_descs *gpios;
+};
+
+static int mux_gpio_set(struct mux_control *mux, int state)
+{
+	struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
+	DECLARE_BITMAP(values, BITS_PER_TYPE(state));
+	u32 value = state;
+
+	bitmap_from_arr32(values, &value, BITS_PER_TYPE(value));
+
+	gpiod_set_array_value(mux_gpio->gpios->ndescs,
+			      mux_gpio->gpios->desc,
+			      mux_gpio->gpios->info, values);
+
+	return 0;
+}
+
+static const struct mux_control_ops mux_gpio_ops = {
+	.set = mux_gpio_set,
+};
+
+static const struct of_device_id mux_gpio_dt_ids[] = {
+	{ .compatible = "gpio-mux", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
+
+static int mux_gpio_probe(struct device *dev)
+{
+	struct mux_chip *mux_chip;
+	struct mux_gpio *mux_gpio;
+	int pins;
+	s32 idle_state;
+	int ret;
+
+	pins = gpiod_count(dev, "mux");
+	if (pins < 0)
+		return pins;
+
+	mux_chip = mux_chip_alloc(dev, 1, sizeof(*mux_gpio));
+	if (IS_ERR(mux_chip))
+		return PTR_ERR(mux_chip);
+
+	mux_gpio = mux_chip_priv(mux_chip);
+	mux_chip->ops = &mux_gpio_ops;
+
+	mux_gpio->gpios = gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
+	if (IS_ERR(mux_gpio->gpios))
+		return dev_err_probe(dev, PTR_ERR(mux_gpio->gpios),
+				     "failed to get gpios\n");
+	WARN_ON(pins != mux_gpio->gpios->ndescs);
+	mux_chip->mux->states = BIT(pins);
+
+	ret = device_property_read_u32(dev, "idle-state", (u32 *)&idle_state);
+	if (ret >= 0 && idle_state != MUX_IDLE_AS_IS) {
+		if (idle_state < 0 || idle_state >= mux_chip->mux->states) {
+			dev_err(dev, "invalid idle-state %u\n", idle_state);
+			return -EINVAL;
+		}
+
+		mux_chip->mux->idle_state = idle_state;
+	}
+
+	ret = mux_chip_register(mux_chip);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev, "%u-way mux-controller registered\n",
+		 mux_chip->mux->states);
+
+	return 0;
+}
+
+static struct driver mux_gpio_driver = {
+	.name = "gpio-mux",
+	.of_match_table	= mux_gpio_dt_ids,
+	.probe = mux_gpio_probe,
+};
+coredevice_platform_driver(mux_gpio_driver);
+
+MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.39.2




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

* Re: [PATCH 10/10] drivers: port Linux mux framework and gpio-mux driver
  2023-06-14 13:54 ` [PATCH 10/10] drivers: port Linux mux framework and gpio-mux driver Ahmad Fatoum
@ 2023-06-14 15:21   ` Ahmad Fatoum
  0 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 15:21 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Hi,

On 14.06.23 15:54, Ahmad Fatoum wrote:
> The driver builds a single multiplexer controller using a number
> of gpio pins. For N pins, there will be 2^N possible multiplexer
> states.
> 
> The GPIO pins can be connected (by the hardware) to several
> multiplexers. Consumer drivers in turn can reference the mux in
> the device tree to control it. User interaction over the shell
> is possible via the .state parameter that holds the currently
> active state (or -1 if mux was not yet used). An additional .idle
> parameter informs the user whether the mux has been claimed by a
> consumer.

I missed to include headers in this one patch. I'll wait for feedback
and either resend the series or just resend this patch separately
with the missing headers added if the rest is applied.

> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/Kconfig      |   1 +
>  drivers/Makefile     |   1 +
>  drivers/mux/Kconfig  |  29 +++
>  drivers/mux/Makefile |  10 +
>  drivers/mux/core.c   | 472 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/mux/gpio.c   | 103 ++++++++++
>  6 files changed, 616 insertions(+)
>  create mode 100644 drivers/mux/Kconfig
>  create mode 100644 drivers/mux/Makefile
>  create mode 100644 drivers/mux/core.c
>  create mode 100644 drivers/mux/gpio.c
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 2636eed1a3b5..aa12597df637 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -19,6 +19,7 @@ source "drivers/clk/Kconfig"
>  source "drivers/clocksource/Kconfig"
>  source "drivers/mfd/Kconfig"
>  source "drivers/misc/Kconfig"
> +source "drivers/mux/Kconfig"
>  source "drivers/led/Kconfig"
>  source "drivers/eeprom/Kconfig"
>  source "drivers/input/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index fa899a2ab8c3..61c4c9c94020 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -20,6 +20,7 @@ obj-y	+= eeprom/
>  obj-$(CONFIG_PWM) += pwm/
>  obj-y	+= input/
>  obj-y	+= misc/
> +obj-$(CONFIG_MULTIPLEXER)	+= mux/
>  obj-$(CONFIG_NVMEM) += nvmem/
>  obj-y	+= dma/
>  obj-y  += watchdog/
> diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig
> new file mode 100644
> index 000000000000..c68f1d4130ee
> --- /dev/null
> +++ b/drivers/mux/Kconfig
> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Multiplexer devices
> +#
> +
> +config MULTIPLEXER
> +	tristate
> +	prompt "Multiplexer driver support" if COMPILE_TEST
> +
> +menu "Multiplexer drivers"
> +	depends on MULTIPLEXER
> +
> +config MUX_GPIO
> +	tristate "GPIO-controlled Multiplexer"
> +	depends on GPIOLIB || COMPILE_TEST
> +	help
> +	  GPIO-controlled Multiplexer controller.
> +
> +	  The driver builds a single multiplexer controller using a number
> +	  of gpio pins. For N pins, there will be 2^N possible multiplexer
> +	  states. The GPIO pins can be connected (by the hardware) to several
> +	  multiplexers.
> +
> +	  The barebox driver doesn't implement Linux' fastpath, which enables
> +	  atomically switching GPIOs in the same bank where possible.
> +	  This means that board code authors need to ensure that intermediate
> +	  states when switching some of the GPIOs don't break anything.
> +
> +endmenu
> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
> new file mode 100644
> index 000000000000..22d285ed9d4b
> --- /dev/null
> +++ b/drivers/mux/Makefile
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for multiplexer devices.
> +#
> +
> +mux-core-objs			:= core.o
> +mux-gpio-objs			:= gpio.o
> +
> +obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
> +obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> new file mode 100644
> index 000000000000..6d9fae423319
> --- /dev/null
> +++ b/drivers/mux/core.c
> @@ -0,0 +1,472 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Multiplexer subsystem
> + *
> + * Copyright (C) 2017 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + */
> +
> +#define pr_fmt(fmt) "mux-core: " fmt
> +
> +#include <linux/ktime.h>
> +#include <driver.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <init.h>
> +#include <module.h>
> +#include <linux/mux/consumer.h>
> +#include <linux/mux/driver.h>
> +#include <of.h>
> +#include <stdio.h>
> +#include <malloc.h>
> +
> +/*
> + * The idle-as-is "state" is not an actual state that may be selected, it
> + * only implies that the state should not be changed. So, use that state
> + * as indication that the cached state of the multiplexer is unknown.
> + */
> +#define MUX_CACHE_UNKNOWN MUX_IDLE_AS_IS
> +
> +/**
> + * struct mux_state -	Represents a mux controller state specific to a given
> + *			consumer.
> + * @mux:		Pointer to a mux controller.
> + * @state:		State of the mux to be selected.
> + *
> + * This structure is specific to the consumer that acquires it and has
> + * information specific to that consumer.
> + */
> +struct mux_state {
> +	struct mux_control *mux;
> +	unsigned int state;
> +};
> +
> +static LIST_HEAD(mux_chips);
> +
> +static int mux_register_dev(struct mux_chip *mux_chip, const char *name, int id)
> +{
> +	mux_chip->dev.id = id;
> +	dev_set_name(&mux_chip->dev, name);
> +
> +	return register_device(&mux_chip->dev);
> +}
> +
> +/**
> + * mux_chip_alloc() - Allocate a mux-chip.
> + * @dev: The parent device implementing the mux interface.
> + * @controllers: The number of mux controllers to allocate for this chip.
> + * @sizeof_priv: Size of extra memory area for private use by the caller.
> + *
> + * After allocating the mux-chip with the desired number of mux controllers
> + * but before registering the chip, the mux driver is required to configure
> + * the number of valid mux states in the mux_chip->mux[N].states members and
> + * the desired idle state in the returned mux_chip->mux[N].idle_state members.
> + * The default idle state is MUX_IDLE_AS_IS. The mux driver also needs to
> + * provide a pointer to the operations struct in the mux_chip->ops member
> + * before registering the mux-chip with mux_chip_register.
> + *
> + * Return: A pointer to the new mux-chip, or an ERR_PTR with a negative errno.
> + */
> +struct mux_chip *mux_chip_alloc(struct device *dev,
> +				unsigned int controllers, size_t sizeof_priv)
> +{
> +	struct mux_chip *mux_chip;
> +	int i;
> +
> +	if (WARN_ON(!dev || !controllers))
> +		return ERR_PTR(-EINVAL);
> +
> +	mux_chip = kzalloc(sizeof(*mux_chip) +
> +			   controllers * sizeof(*mux_chip->mux) +
> +			   sizeof_priv, GFP_KERNEL);
> +	if (!mux_chip)
> +		return ERR_PTR(-ENOMEM);
> +
> +
> +	mux_chip->mux = (struct mux_control *)(mux_chip + 1);
> +	mux_chip->dev.parent = dev;
> +	mux_chip->dev.of_node = dev->of_node;
> +	mux_chip->dev.priv = mux_chip;
> +
> +	mux_chip->controllers = controllers;
> +	for (i = 0; i < controllers; ++i) {
> +		struct mux_control *mux = &mux_chip->mux[i];
> +
> +		mux->chip = mux_chip;
> +		mux->cached_state = MUX_CACHE_UNKNOWN;
> +		mux->idle_state = MUX_IDLE_AS_IS;
> +		mux->idle = true;
> +		mux->last_change = ktime_get();
> +		mux->dev.id = i;
> +		mux->dev.parent = &mux_chip->dev;
> +	}
> +
> +	return mux_chip;
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_alloc);
> +
> +static int mux_control_set(struct mux_control *mux, int state)
> +{
> +	int ret = mux->chip->ops->set(mux, state);
> +
> +	mux->cached_state = ret < 0 ? MUX_CACHE_UNKNOWN : state;
> +	if (ret >= 0)
> +		mux->last_change = ktime_get();
> +
> +	return ret;
> +}
> +
> +static int mux_control_set_param(struct param_d *param, void *priv)
> +{
> +	struct mux_control *mux = priv;
> +	int state = mux->cached_state;
> +
> +	if (state < 0 || state >= mux->states)
> +		return -EINVAL;
> +
> +	return mux_control_set(mux, state);
> +}
> +
> +/**
> + * mux_chip_register() - Register a mux-chip, thus readying the controllers
> + *			 for use.
> + * @mux_chip: The mux-chip to register.
> + *
> + * Return: Zero on success or a negative errno on error.
> + */
> +int mux_chip_register(struct mux_chip *mux_chip)
> +{
> +	int i;
> +	int ret;
> +	const char *alias;
> +	struct device_node *np = mux_chip->dev.of_node;
> +
> +	for (i = 0; i < mux_chip->controllers; ++i) {
> +		struct mux_control *mux = &mux_chip->mux[i];
> +
> +		if (mux->idle_state == mux->cached_state)
> +			continue;
> +
> +		ret = mux_control_set(mux, mux->idle_state);
> +		if (ret < 0) {
> +			dev_err(&mux_chip->dev, "unable to set idle state\n");
> +			return ret;
> +		}
> +	}
> +
> +	alias = np ? of_alias_get(np) : NULL;
> +	if (alias)
> +		ret = mux_register_dev(mux_chip, alias, DEVICE_ID_SINGLE);
> +	if (!alias || ret)
> +		ret = mux_register_dev(mux_chip, "muxchip", DEVICE_ID_DYNAMIC);
> +
> +	if (ret < 0) {
> +		dev_err(&mux_chip->dev,
> +			"register_device failed in %s: %d\n", __func__, ret);
> +		return ret;
> +	}
> +
> +	if (IS_ENABLED(CONFIG_PARAMETER)) {
> +		struct device *dev = NULL;
> +
> +		if (mux_chip->controllers == 1)
> +			dev = &mux_chip->dev;
> +
> +		for (i = 0; i < mux_chip->controllers; ++i) {
> +			struct mux_control *mux = &mux_chip->mux[i];
> +			struct device *dev;
> +
> +			if (mux_chip->controllers == 1) {
> +				dev = &mux_chip->dev;
> +			} else  {
> +				dev = &mux->dev;
> +				dev_set_name(dev, "muxchip%uc", mux_chip->dev.id);
> +				if (register_device(dev))
> +					continue;
> +			}
> +
> +			dev_add_param_bool_ro(dev, "idle", &mux->idle);
> +			dev_add_param_int(dev, "state", mux_control_set_param,
> +					  NULL, &mux->cached_state, "%d", mux);
> +		}
> +	}
> +
> +	list_add(&mux_chip->list, &mux_chips);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_register);
> +
> +/**
> + * mux_control_states() - Query the number of multiplexer states.
> + * @mux: The mux-control to query.
> + *
> + * Return: The number of multiplexer states.
> + */
> +unsigned int mux_control_states(struct mux_control *mux)
> +{
> +	return mux->states;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_states);
> +
> +/*
> + * The mux->idle must be clear when calling this function.
> + */
> +static int __mux_control_select(struct mux_control *mux, int state)
> +{
> +	int ret;
> +
> +	if (WARN_ON(state < 0 || state >= mux->states))
> +		return -EINVAL;
> +
> +	if (mux->cached_state == state)
> +		return 0;
> +
> +	ret = mux_control_set(mux, state);
> +	if (ret >= 0)
> +		return 0;
> +
> +	/* The mux update failed, try to revert if appropriate... */
> +	if (mux->idle_state != MUX_IDLE_AS_IS)
> +		mux_control_set(mux, mux->idle_state);
> +
> +	return ret;
> +}
> +
> +static void mux_control_delay(struct mux_control *mux, unsigned int delay_us)
> +{
> +	ktime_t delayend;
> +	s64 remaining;
> +
> +	if (!delay_us)
> +		return;
> +
> +	delayend = ktime_add_us(mux->last_change, delay_us);
> +	remaining = ktime_us_delta(delayend, ktime_get());
> +	if (remaining > 0)
> +		udelay(remaining);
> +}
> +
> +/**
> + * mux_control_try_select_delay() - Try to select the given multiplexer state.
> + * @mux: The mux-control to request a change of state from.
> + * @state: The new requested state.
> + * @delay_us: The time to delay (in microseconds) if the mux state is changed.
> + *
> + * On successfully selecting the mux-control state, it will be locked until
> + * mux_control_deselect() is called.
> + *
> + * Therefore, make sure to call mux_control_deselect() when the operation is
> + * complete and the mux-control is free for others to use, but do not call
> + * mux_control_deselect() if mux_control_try_select() fails.
> + *
> + * Return: 0 when the mux-control state has the requested state or a negative
> + * errno on error. Specifically -EBUSY if the mux-control is contended.
> + */
> +int mux_control_try_select_delay(struct mux_control *mux, unsigned int state,
> +				 unsigned int delay_us)
> +{
> +	int ret;
> +
> +	if (!mux->idle)
> +		return -EBUSY;
> +	mux->idle = false;
> +
> +	ret = __mux_control_select(mux, state);
> +	if (ret >= 0)
> +		mux_control_delay(mux, delay_us);
> +
> +	if (ret < 0)
> +		mux->idle = true;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_try_select_delay);
> +
> +/**
> + * mux_state_try_select_delay() - Try to select the given multiplexer state.
> + * @mstate: The mux-state to select.
> + * @delay_us: The time to delay (in microseconds) if the mux state is changed.
> + *
> + * On successfully selecting the mux-state, its mux-control will be locked
> + * until mux_state_deselect() is called.
> + *
> + * Therefore, make sure to call mux_state_deselect() when the operation is
> + * complete and the mux-control is free for others to use, but do not call
> + * mux_state_deselect() if mux_state_try_select() fails.
> + *
> + * Return: 0 when the mux-state has been selected or a negative errno on
> + * error. Specifically -EBUSY if the mux-control is contended.
> + */
> +int mux_state_try_select_delay(struct mux_state *mstate, unsigned int delay_us)
> +{
> +	return mux_control_try_select_delay(mstate->mux, mstate->state, delay_us);
> +}
> +EXPORT_SYMBOL_GPL(mux_state_try_select_delay);
> +
> +/**
> + * mux_control_deselect() - Deselect the previously selected multiplexer state.
> + * @mux: The mux-control to deselect.
> + *
> + * It is required that a single call is made to mux_control_deselect() for
> + * each and every successful call made to either of mux_control_select() or
> + * mux_control_try_select().
> + *
> + * Return: 0 on success and a negative errno on error. An error can only
> + * occur if the mux has an idle state. Note that even if an error occurs, the
> + * mux-control is unlocked and is thus free for the next access.
> + */
> +int mux_control_deselect(struct mux_control *mux)
> +{
> +	int ret = 0;
> +
> +	if (mux->idle_state != MUX_IDLE_AS_IS &&
> +	    mux->idle_state != mux->cached_state)
> +		ret = mux_control_set(mux, mux->idle_state);
> +
> +	mux->idle = true;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_deselect);
> +
> +/**
> + * mux_state_deselect() - Deselect the previously selected multiplexer state.
> + * @mstate: The mux-state to deselect.
> + *
> + * It is required that a single call is made to mux_state_deselect() for
> + * each and every successful call made to either of mux_state_select() or
> + * mux_state_try_select().
> + *
> + * Return: 0 on success and a negative errno on error. An error can only
> + * occur if the mux has an idle state. Note that even if an error occurs, the
> + * mux-control is unlocked and is thus free for the next access.
> + */
> +int mux_state_deselect(struct mux_state *mstate)
> +{
> +	return mux_control_deselect(mstate->mux);
> +}
> +EXPORT_SYMBOL_GPL(mux_state_deselect);
> +
> +/* Note this function returns a reference to the mux_chip dev. */
> +static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> +{
> +	struct mux_chip *chip;
> +
> +	list_for_each_entry(chip, &mux_chips, list) {
> +		if (chip->dev.device_node == np)
> +			return chip;
> +	}
> +
> +	return NULL;
> +}
> +
> +/*
> + * mux_get() - Get the mux-control for a device.
> + * @dev: The device that needs a mux-control.
> + * @mux_name: The name identifying the mux-control.
> + * @state: Pointer to where the requested state is returned, or NULL when
> + *         the required multiplexer states are handled by other means.
> + *
> + * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
> + */
> +static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> +				   unsigned int *state)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct of_phandle_args args;
> +	struct mux_chip *mux_chip;
> +	unsigned int controller;
> +	int index = 0;
> +	int ret;
> +
> +	if (mux_name) {
> +		if (state)
> +			index = of_property_match_string(np, "mux-state-names",
> +							 mux_name);
> +		else
> +			index = of_property_match_string(np, "mux-control-names",
> +							 mux_name);
> +		if (index < 0) {
> +			dev_err(dev, "mux controller '%s' not found\n",
> +				mux_name);
> +			return ERR_PTR(index);
> +		}
> +	}
> +
> +	if (state)
> +		ret = of_parse_phandle_with_args(np,
> +						 "mux-states", "#mux-state-cells",
> +						 index, &args);
> +	else
> +		ret = of_parse_phandle_with_args(np,
> +						 "mux-controls", "#mux-control-cells",
> +						 index, &args);
> +	if (ret) {
> +		dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> +			np, state ? "state" : "control", mux_name ?: "", index);
> +		return ERR_PTR(ret);
> +	}
> +
> +	mux_chip = of_find_mux_chip_by_node(args.np);
> +	of_node_put(args.np);
> +	if (!mux_chip)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	controller = 0;
> +	if (state) {
> +		if (args.args_count > 2 || args.args_count == 0 ||
> +		    (args.args_count < 2 && mux_chip->controllers > 1)) {
> +			dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
> +				np, args.np);
> +			put_device(&mux_chip->dev);
> +			return ERR_PTR(-EINVAL);
> +		}
> +
> +		if (args.args_count == 2) {
> +			controller = args.args[0];
> +			*state = args.args[1];
> +		} else {
> +			*state = args.args[0];
> +		}
> +
> +	} else {
> +		if (args.args_count > 1 ||
> +		    (!args.args_count && mux_chip->controllers > 1)) {
> +			dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
> +				np, args.np);
> +			put_device(&mux_chip->dev);
> +			return ERR_PTR(-EINVAL);
> +		}
> +
> +		if (args.args_count)
> +			controller = args.args[0];
> +	}
> +
> +	if (controller >= mux_chip->controllers) {
> +		dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
> +			np, controller, args.np);
> +		put_device(&mux_chip->dev);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return &mux_chip->mux[controller];
> +}
> +
> +/**
> + * mux_control_get() - Get the mux-control for a device.
> + * @dev: The device that needs a mux-control.
> + * @mux_name: The name identifying the mux-control.
> + *
> + * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
> + */
> +struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> +{
> +	return mux_get(dev, mux_name, NULL);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_get);
> +
> +MODULE_DESCRIPTION("Multiplexer subsystem");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
> new file mode 100644
> index 000000000000..10d68b63cf57
> --- /dev/null
> +++ b/drivers/mux/gpio.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * GPIO-controlled multiplexer driver
> + *
> + * Copyright (C) 2017 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + */
> +
> +#include <linux/bitmap.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/mod_devicetable.h>
> +#include <module.h>
> +#include <linux/mux/driver.h>
> +#include <driver.h>
> +#include <linux/property.h>
> +
> +struct mux_gpio {
> +	struct gpio_descs *gpios;
> +};
> +
> +static int mux_gpio_set(struct mux_control *mux, int state)
> +{
> +	struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
> +	DECLARE_BITMAP(values, BITS_PER_TYPE(state));
> +	u32 value = state;
> +
> +	bitmap_from_arr32(values, &value, BITS_PER_TYPE(value));
> +
> +	gpiod_set_array_value(mux_gpio->gpios->ndescs,
> +			      mux_gpio->gpios->desc,
> +			      mux_gpio->gpios->info, values);
> +
> +	return 0;
> +}
> +
> +static const struct mux_control_ops mux_gpio_ops = {
> +	.set = mux_gpio_set,
> +};
> +
> +static const struct of_device_id mux_gpio_dt_ids[] = {
> +	{ .compatible = "gpio-mux", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
> +
> +static int mux_gpio_probe(struct device *dev)
> +{
> +	struct mux_chip *mux_chip;
> +	struct mux_gpio *mux_gpio;
> +	int pins;
> +	s32 idle_state;
> +	int ret;
> +
> +	pins = gpiod_count(dev, "mux");
> +	if (pins < 0)
> +		return pins;
> +
> +	mux_chip = mux_chip_alloc(dev, 1, sizeof(*mux_gpio));
> +	if (IS_ERR(mux_chip))
> +		return PTR_ERR(mux_chip);
> +
> +	mux_gpio = mux_chip_priv(mux_chip);
> +	mux_chip->ops = &mux_gpio_ops;
> +
> +	mux_gpio->gpios = gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
> +	if (IS_ERR(mux_gpio->gpios))
> +		return dev_err_probe(dev, PTR_ERR(mux_gpio->gpios),
> +				     "failed to get gpios\n");
> +	WARN_ON(pins != mux_gpio->gpios->ndescs);
> +	mux_chip->mux->states = BIT(pins);
> +
> +	ret = device_property_read_u32(dev, "idle-state", (u32 *)&idle_state);
> +	if (ret >= 0 && idle_state != MUX_IDLE_AS_IS) {
> +		if (idle_state < 0 || idle_state >= mux_chip->mux->states) {
> +			dev_err(dev, "invalid idle-state %u\n", idle_state);
> +			return -EINVAL;
> +		}
> +
> +		mux_chip->mux->idle_state = idle_state;
> +	}
> +
> +	ret = mux_chip_register(mux_chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	dev_info(dev, "%u-way mux-controller registered\n",
> +		 mux_chip->mux->states);
> +
> +	return 0;
> +}
> +
> +static struct driver mux_gpio_driver = {
> +	.name = "gpio-mux",
> +	.of_match_table	= mux_gpio_dt_ids,
> +	.probe = mux_gpio_probe,
> +};
> +coredevice_platform_driver(mux_gpio_driver);
> +
> +MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
> +MODULE_LICENSE("GPL v2");

-- 
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] 18+ messages in thread

* Re: [PATCH 05/10] gpiolib: export proper gpio descriptor API
  2023-06-14 13:54 ` [PATCH 05/10] gpiolib: export proper gpio descriptor API Ahmad Fatoum
@ 2023-06-20  5:20   ` Marco Felsch
  2023-06-20  5:55     ` Ahmad Fatoum
  0 siblings, 1 reply; 18+ messages in thread
From: Marco Felsch @ 2023-06-20  5:20 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Hi Ahmad,

thanks a lot for this patchset, please see my comments below.

On 23-06-14, Ahmad Fatoum wrote:
> 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.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/gpio/gpiolib.c        | 173 ++++++++++++++++++++++++++++++----
>  include/linux/gpio/consumer.h |  62 ++----------
>  2 files changed, 163 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 402d408be071..25d91d250dc8 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)

Why do we stick with gpioinfo_* here? IMHO we could use
gpiodesc_chip_offset() since the gpioinfo is gone.

>  {
>  	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);

Same for gpioinfo_free() which could become gpiodesc_free().

> +}
> +
> +/**
> + * 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);

This function should be deleted since we don't support the global 'int'
gpio anymore.

> -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);

gpio_direction_input should be deleted as well?

Regards,
  Marco



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

* Re: [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (9 preceding siblings ...)
  2023-06-14 13:54 ` [PATCH 10/10] drivers: port Linux mux framework and gpio-mux driver Ahmad Fatoum
@ 2023-06-20  5:51 ` Marco Felsch
  2023-06-21  9:27 ` Sascha Hauer
  11 siblings, 0 replies; 18+ messages in thread
From: Marco Felsch @ 2023-06-20  5:51 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

Hi Ahmad,

On 23-06-14, 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.

thanks a lot for improving the code to be more aligned with Linux :)

Patches 1-9 lgtm, apart the minor comments in patch-5. So feel free to
add my:

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> 
> Ahmad Fatoum (10):
>   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}
>   commands: help: ignore options after first regular argument
>   gpiolib: factor out finding gpio property
>   gpiolib: add support for requesting and setting gpiod arrays
>   drivers: port Linux mux framework and gpio-mux driver
> 
>  commands/help.c                          |  19 +-
>  drivers/Kconfig                          |   1 +
>  drivers/Makefile                         |   1 +
>  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             |   4 +-
>  drivers/mux/Kconfig                      |  29 ++
>  drivers/mux/Makefile                     |  10 +
>  drivers/mux/core.c                       | 472 ++++++++++++++++++++
>  drivers/mux/gpio.c                       | 103 +++++
>  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            | 137 ++++++
>  include/linux/mtd/rawnand.h              |   4 +-
>  include/linux/printk.h                   |   3 +
>  include/video/mipi_dbi.h                 |   7 +-
>  lib/bitmap.c                             | 103 +++++
>  38 files changed, 1559 insertions(+), 395 deletions(-)
>  create mode 100644 drivers/mux/Kconfig
>  create mode 100644 drivers/mux/Makefile
>  create mode 100644 drivers/mux/core.c
>  create mode 100644 drivers/mux/gpio.c
>  create mode 100644 include/linux/gpio/consumer.h
> 
> -- 
> 2.39.2
> 
> 
> 



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

* Re: [PATCH 05/10] gpiolib: export proper gpio descriptor API
  2023-06-20  5:20   ` Marco Felsch
@ 2023-06-20  5:55     ` Ahmad Fatoum
  2023-06-20  6:13       ` Marco Felsch
  0 siblings, 1 reply; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-20  5:55 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

Hello Marco,

On 20.06.23 07:20, Marco Felsch wrote:
>> -static unsigned gpioinfo_chip_offset(struct gpio_desc *desc)
>> +static unsigned gpioinfo_chip_offset(const struct gpio_desc *desc)
> 
> Why do we stick with gpioinfo_* here? IMHO we could use
> gpiodesc_chip_offset() since the gpioinfo is gone.

gpioinfo_ is internal. gpiod_ is external. I thought about renaming
gpioinfo_ to gpiodesc_, but found it confusing, because they are
too similar in name.

>>  {
>>  	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);
> 
> Same for gpioinfo_free() which could become gpiodesc_free().

See above.

>>  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);
> 
> This function should be deleted since we don't support the global 'int'
> gpio anymore.

We do support it. Lots of code across the tree doesn't use gpiod_ API.

>>  EXPORT_SYMBOL(gpio_direction_input);
> 
> gpio_direction_input should be deleted as well?

Once we have switches all over to use gpiod_ API, we could drop it, yes.
Patches to migrate existing users are certainly welcome ;)

Cheers,
Ahmad

-- 
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] 18+ messages in thread

* Re: [PATCH 05/10] gpiolib: export proper gpio descriptor API
  2023-06-20  5:55     ` Ahmad Fatoum
@ 2023-06-20  6:13       ` Marco Felsch
  2023-06-20  6:18         ` Ahmad Fatoum
  0 siblings, 1 reply; 18+ messages in thread
From: Marco Felsch @ 2023-06-20  6:13 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On 23-06-20, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 20.06.23 07:20, Marco Felsch wrote:
> >> -static unsigned gpioinfo_chip_offset(struct gpio_desc *desc)
> >> +static unsigned gpioinfo_chip_offset(const struct gpio_desc *desc)
> > 
> > Why do we stick with gpioinfo_* here? IMHO we could use
> > gpiodesc_chip_offset() since the gpioinfo is gone.
> 
> gpioinfo_ is internal. gpiod_ is external. I thought about renaming
> gpioinfo_ to gpiodesc_, but found it confusing, because they are
> too similar in name.

The gpioinfo_ is even more confusing (at least to me) since the struct
the name is based on is renamed once this series gets merged.

> >>  {
> >>  	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);
> > 
> > Same for gpioinfo_free() which could become gpiodesc_free().
> 
> See above.
> 
> >>  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);
> > 
> > This function should be deleted since we don't support the global 'int'
> > gpio anymore.
> 
> We do support it. Lots of code across the tree doesn't use gpiod_ API.

Argh.. you're right. I thought, that your adaption patches do cover all
users. Sorry my bad.

> >>  EXPORT_SYMBOL(gpio_direction_input);
> > 
> > gpio_direction_input should be deleted as well?
> 
> Once we have switches all over to use gpiod_ API, we could drop it, yes.
> Patches to migrate existing users are certainly welcome ;)

Hm.. we should deprecate this function to make it clear and to encourage
board maintainers to switch ;)

> 
> Cheers,
> Ahmad
> 
> -- 
> 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] 18+ messages in thread

* Re: [PATCH 05/10] gpiolib: export proper gpio descriptor API
  2023-06-20  6:13       ` Marco Felsch
@ 2023-06-20  6:18         ` Ahmad Fatoum
  0 siblings, 0 replies; 18+ messages in thread
From: Ahmad Fatoum @ 2023-06-20  6:18 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

Hello Marco,

On 20.06.23 08:13, Marco Felsch wrote:
> On 23-06-20, Ahmad Fatoum wrote:
>> Hello Marco,
>>
>> On 20.06.23 07:20, Marco Felsch wrote:
>>>> -static unsigned gpioinfo_chip_offset(struct gpio_desc *desc)
>>>> +static unsigned gpioinfo_chip_offset(const struct gpio_desc *desc)
>>>
>>> Why do we stick with gpioinfo_* here? IMHO we could use
>>> gpiodesc_chip_offset() since the gpioinfo is gone.
>>
>> gpioinfo_ is internal. gpiod_ is external. I thought about renaming
>> gpioinfo_ to gpiodesc_, but found it confusing, because they are
>> too similar in name.
> 
> The gpioinfo_ is even more confusing (at least to me) since the struct
> the name is based on is renamed once this series gets merged.

I can send a follow up patch to rename the functions (or a v2
if there's other feedback).

>>> This function should be deleted since we don't support the global 'int'
>>> gpio anymore.
>>
>> We do support it. Lots of code across the tree doesn't use gpiod_ API.
> 
> Argh.. you're right. I thought, that your adaption patches do cover all
> users. Sorry my bad.
> 
>>>>  EXPORT_SYMBOL(gpio_direction_input);
>>>
>>> gpio_direction_input should be deleted as well?
>>
>> Once we have switches all over to use gpiod_ API, we could drop it, yes.
>> Patches to migrate existing users are certainly welcome ;)
> 
> Hm.. we should deprecate this function to make it clear and to encourage
> board maintainers to switch ;)

We can add deprecation warnings, but not before all in-tree users are
switched.

Cheers,
Ahmad



> 
>>
>> Cheers,
>> Ahmad
>>
>> -- 
>> 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 |
>>
>>
> 

-- 
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] 18+ messages in thread

* Re: [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support
  2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
                   ` (10 preceding siblings ...)
  2023-06-20  5:51 ` [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Marco Felsch
@ 2023-06-21  9:27 ` Sascha Hauer
  11 siblings, 0 replies; 18+ messages in thread
From: Sascha Hauer @ 2023-06-21  9:27 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Wed, Jun 14, 2023 at 03:54:42PM +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.
> 
> Ahmad Fatoum (10):
>   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}
>   commands: help: ignore options after first regular argument
>   gpiolib: factor out finding gpio property
>   gpiolib: add support for requesting and setting gpiod arrays
>   drivers: port Linux mux framework and gpio-mux driver

Applied 1-6 and 8-9, thanks

Sascha

-- 
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] 18+ messages in thread

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

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-14 13:54 [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 01/10] driver: include dev_print and family from <driver.h> Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 02/10] include: linux/printk: define new dev_errp_probe Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 03/10] gpio: have gpiod_ functions return and accept pointers Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 04/10] gpio: gpiolib: rename struct gpio_info to gpio_desc Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 05/10] gpiolib: export proper gpio descriptor API Ahmad Fatoum
2023-06-20  5:20   ` Marco Felsch
2023-06-20  5:55     ` Ahmad Fatoum
2023-06-20  6:13       ` Marco Felsch
2023-06-20  6:18         ` Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 06/10] bitmap: implement bitmap_{to,from}_arr{32,64} Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 07/10] commands: help: ignore options after first regular argument Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 08/10] gpiolib: factor out finding gpio property Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 09/10] gpiolib: add support for requesting and setting gpiod arrays Ahmad Fatoum
2023-06-14 13:54 ` [PATCH 10/10] drivers: port Linux mux framework and gpio-mux driver Ahmad Fatoum
2023-06-14 15:21   ` Ahmad Fatoum
2023-06-20  5:51 ` [PATCH 00/10] gpio: add proper gpiod API and gpio-mux support Marco Felsch
2023-06-21  9:27 ` Sascha Hauer

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