[RFC PATCH 0/5] gpio: add PMIO support to gpio-mmio

[RFC PATCH 0/5] gpio: add PMIO support to gpio-mmio

[Jose Javier Rodriguez Barbarin]

This series is an RFC for adding port-mapped I/O (PMIO) support to
gpio-mmio.

The initial motivation was to add PMIO support to gpio-menz127, as we
plan to support this across MCB client drivers. Since gpio-menz127
currently relies on the gpio_generic_chip API, adding PMIO support only
in that driver would require a significant refactoring, including
separate callbacks for memory-mapped and port-mapped accesses.

While looking into this, I noticed a TODO item added by Linus Walleij
about extending gpio-mmio to support port-mapped devices. Based on that,
this series explores adding PMIO support to gpio-mmio instead of
handling it in individual drivers.

The main goal of this approach is to preserve compatibility with
existing MMIO drivers using gpio_generic_chip while extending the API to
also describe PMIO-backed registers. To achieve that, the series extends
struct gpio_generic_chip_config with dedicated fields for port-mapped
registers.

To handle the different register address types used by MMIO
(void __iomem *) and PMIO (unsigned long), this series introduces a
small wrapper structure that can represent both. The read_reg() and
write_reg() callbacks are then updated to operate on this common
representation.

This series has been tested with gpio-menz127, and the driver worked
correctly with both MMIO and PMIO devices.

This is being sent as RFC because I would like feedback on the overall
approach before proceeding further.

In particular, feedback would be appreciated on:
- whether extending gpio_generic_chip_config is the right direction;
- whether introducing a common MMIO/PMIO register descriptor is
  acceptable;
- whether PMIO support should instead be implemented differently in
  gpio-mmio.

Jose Javier Rodriguez Barbarin (5):
  gpio: generic: add a generic register wrapper for MMIO and PMIO
  gpio: generic: extend gpio_generic_chip_config with PMIO register
    fields
  gpio: generic: add io_port to struct gpio_generic_chip
  gpio: mmio: convert accessors to generic register descriptors
  gpio: mmio: add port-mapped read/write callbacks

 drivers/gpio/gpio-mmio.c     | 277 +++++++++++++++++++++++++----------
 include/linux/gpio/generic.h |  49 +++++--
 2 files changed, 243 insertions(+), 83 deletions(-)

-- 
2.53.0

[RFC PATCH 1/5] gpio: generic: add a generic register wrapper for MMIO and PMIO

[Jose Javier Rodriguez Barbarin]

For adding support to port-mapped devices within gpio_generic_chip, a new
data type is required for passing to read_reg() and write_reg()
callbacks as I/O port-mapped and memory-mapped registers use different
data types.

Introduce a new struct gpio_chip_reg to encapsulate mmio addresses and
I/O port numbers.

Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
---
 include/linux/gpio/generic.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/gpio/generic.h b/include/linux/gpio/generic.h
index ff566dc9c3cb..87c624070901 100644
--- a/include/linux/gpio/generic.h
+++ b/include/linux/gpio/generic.h
@@ -18,6 +18,7 @@ struct device;
 #define GPIO_GENERIC_NO_SET_ON_INPUT		BIT(6)
 #define GPIO_GENERIC_PINCTRL_BACKEND		BIT(7) /* Call pinctrl direction setters */
 #define GPIO_GENERIC_NO_INPUT			BIT(8) /* only output */
+#define GPIO_GENERIC_PORT_MAPPED		BIT(9) /* port-mapped */
 
 /**
  * struct gpio_generic_chip_config - Generic GPIO chip configuration data
@@ -56,6 +57,19 @@ struct gpio_generic_chip_config {
 	unsigned long flags;
 };
 
+/**
+ * struct gpio_chip_reg - Generic GPIO chip register descriptor for MMIO or port-mapped I/O
+ * @mmio: MMIO register address.
+ * @port: I/O Port register address.
+ *
+ * Describes a GPIO chip register located either in MMIO space or in
+ * port-mapped I/O space.
+ */
+struct gpio_chip_reg {
+	void __iomem *mmio;
+	unsigned long port;
+};
+
 /**
  * struct gpio_generic_chip - Generic GPIO chip implementation.
  * @gc: The underlying struct gpio_chip object, implementing low-level GPIO
-- 
2.53.0

[RFC PATCH 2/5] gpio: generic: extend gpio_generic_chip_config with PMIO register fields

[Jose Javier Rodriguez Barbarin]

Instead of replacing the register type with gpio_chip_reg, it is better
to extend the structure by adding dedicated fields for I/O port
registers.

This ensures compatibility with memory-mapped devices.

Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
---
 include/linux/gpio/generic.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/gpio/generic.h b/include/linux/gpio/generic.h
index 87c624070901..9a71a8fbe56e 100644
--- a/include/linux/gpio/generic.h
+++ b/include/linux/gpio/generic.h
@@ -43,6 +43,11 @@ struct device;
  *         assumed that setting a line to 1 in this register will turn that
  *         line into an input line. Conversely, setting the line to 0 will
  *         turn that line into an output.
+ * @dat_port: Port-mapped counterpart of @dat.
+ * @set_port: Port-mapped counterpart of @set.
+ * @clr_port: Port-mapped counterpart of @clr.
+ * @dirout_port: Port-mapped counterpart of @dirout.
+ * @dirin_port: Port-mapped counterpart of @dirin.
  * @flags: Different flags that will affect the behaviour of the device, such
  *         as endianness etc.
  */
@@ -54,6 +59,11 @@ struct gpio_generic_chip_config {
 	void __iomem *clr;
 	void __iomem *dirout;
 	void __iomem *dirin;
+	unsigned long dat_port;
+	unsigned long set_port;
+	unsigned long clr_port;
+	unsigned long dirout_port;
+	unsigned long dirin_port;
 	unsigned long flags;
 };
 
-- 
2.53.0

[RFC PATCH 3/5] gpio: generic: add io_port to struct gpio_generic_chip

[Jose Javier Rodriguez Barbarin]

Add an io_port field to struct gpio_generic_chip. Initialize it
depending on whether the GPIO_GENERIC_PORT_MAPPED flag is set.

Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
---
 drivers/gpio/gpio-mmio.c     | 1 +
 include/linux/gpio/generic.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index edbcaad57d00..37e1ed6569e6 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -645,6 +645,7 @@ int gpio_generic_chip_init(struct gpio_generic_chip *chip,
 	gc->base = -1;
 	gc->request = gpio_mmio_request;
 	chip->be_bits = !!(flags & GPIO_GENERIC_BIG_ENDIAN);
+	chip->io_port = !!(flags & GPIO_GENERIC_PORT_MAPPED);
 
 	ret = gpiochip_get_ngpios(gc, dev);
 	if (ret)
diff --git a/include/linux/gpio/generic.h b/include/linux/gpio/generic.h
index 9a71a8fbe56e..eec63f8fe144 100644
--- a/include/linux/gpio/generic.h
+++ b/include/linux/gpio/generic.h
@@ -111,6 +111,7 @@ struct gpio_generic_chip {
 	unsigned long (*read_reg)(void __iomem *reg);
 	void (*write_reg)(void __iomem *reg, unsigned long data);
 	bool be_bits;
+	bool io_port;
 	void __iomem *reg_dat;
 	void __iomem *reg_set;
 	void __iomem *reg_clr;
-- 
2.53.0

[RFC PATCH 4/5] gpio: mmio: convert accessors to generic register descriptors

[Jose Javier Rodriguez Barbarin]

Convert the gpio-mmio accessors to use struct gpio_chip_reg instead of
the previous MMIO-only register type.

This allows the same accessors to operate on both MMIO and PMIO
registers and aligns gpio-mmio with the updated gpio_generic_chip API.

Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
---
 drivers/gpio/gpio-mmio.c     | 174 ++++++++++++++++++++---------------
 include/linux/gpio/generic.h |  24 +++--
 2 files changed, 117 insertions(+), 81 deletions(-)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 37e1ed6569e6..247ce5b76441 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -62,66 +62,83 @@ o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
 
 #include "gpiolib.h"
 
-static void gpio_mmio_write8(void __iomem *reg, unsigned long data)
+static void gpio_mmio_write8(struct gpio_chip_reg *reg, unsigned long data)
 {
-	writeb(data, reg);
+	writeb(data, reg->mmio);
 }
 
-static unsigned long gpio_mmio_read8(void __iomem *reg)
+static unsigned long gpio_mmio_read8(struct gpio_chip_reg *reg)
 {
-	return readb(reg);
+	return readb(reg->mmio);
 }
 
-static void gpio_mmio_write16(void __iomem *reg, unsigned long data)
+static void gpio_mmio_write16(struct gpio_chip_reg *reg, unsigned long data)
 {
-	writew(data, reg);
+	writew(data, reg->mmio);
 }
 
-static unsigned long gpio_mmio_read16(void __iomem *reg)
+static unsigned long gpio_mmio_read16(struct gpio_chip_reg *reg)
 {
-	return readw(reg);
+	return readw(reg->mmio);
 }
 
-static void gpio_mmio_write32(void __iomem *reg, unsigned long data)
+static void gpio_mmio_write32(struct gpio_chip_reg *reg, unsigned long data)
 {
-	writel(data, reg);
+	writel(data, reg->mmio);
 }
 
-static unsigned long gpio_mmio_read32(void __iomem *reg)
+static unsigned long gpio_mmio_read32(struct gpio_chip_reg *reg)
 {
-	return readl(reg);
+	return readl(reg->mmio);
 }
 
 #if BITS_PER_LONG >= 64
-static void gpio_mmio_write64(void __iomem *reg, unsigned long data)
+static void gpio_mmio_write64(struct gpio_chip_reg *reg, unsigned long data)
 {
-	writeq(data, reg);
+	writeq(data, reg->mmio);
 }
 
-static unsigned long gpio_mmio_read64(void __iomem *reg)
+static unsigned long gpio_mmio_read64(struct gpio_chip_reg *reg)
 {
-	return readq(reg);
+	return readq(reg->mmio);
 }
 #endif /* BITS_PER_LONG >= 64 */
 
-static void gpio_mmio_write16be(void __iomem *reg, unsigned long data)
+static void gpio_mmio_write16be(struct gpio_chip_reg *reg, unsigned long data)
 {
-	iowrite16be(data, reg);
+	iowrite16be(data, reg->mmio);
 }
 
-static unsigned long gpio_mmio_read16be(void __iomem *reg)
+static unsigned long gpio_mmio_read16be(struct gpio_chip_reg *reg)
 {
-	return ioread16be(reg);
+	return ioread16be(reg->mmio);
 }
 
-static void gpio_mmio_write32be(void __iomem *reg, unsigned long data)
+static void gpio_mmio_write32be(struct gpio_chip_reg *reg, unsigned long data)
 {
-	iowrite32be(data, reg);
+	iowrite32be(data, reg->mmio);
 }
 
-static unsigned long gpio_mmio_read32be(void __iomem *reg)
+static unsigned long gpio_mmio_read32be(struct gpio_chip_reg *reg)
 {
-	return ioread32be(reg);
+	return ioread32be(reg->mmio);
+}
+
+static inline void gpio_chip_reg_init(struct gpio_chip_reg *reg, bool io_port,
+				 void __iomem *addr, unsigned long port)
+{
+	reg->mmio = NULL;
+	reg->port = 0;
+
+	if (io_port)
+		reg->port = port;
+	else
+		reg->mmio = addr;
+}
+
+static inline bool gpio_chip_reg_is_set(struct gpio_chip_reg *reg)
+{
+	return reg->mmio != NULL || reg->port != 0;
 }
 
 static unsigned long gpio_mmio_line2mask(struct gpio_chip *gc, unsigned int line)
@@ -140,9 +157,9 @@ static int gpio_mmio_get_set(struct gpio_chip *gc, unsigned int gpio)
 	bool dir = !!(chip->sdir & pinmask);
 
 	if (dir)
-		return !!(chip->read_reg(chip->reg_set) & pinmask);
+		return !!(chip->read_reg(&chip->reg_set) & pinmask);
 
-	return !!(chip->read_reg(chip->reg_dat) & pinmask);
+	return !!(chip->read_reg(&chip->reg_dat) & pinmask);
 }
 
 /*
@@ -162,9 +179,9 @@ static int gpio_mmio_get_set_multiple(struct gpio_chip *gc, unsigned long *mask,
 	get_mask = *mask & ~chip->sdir;
 
 	if (set_mask)
-		*bits |= chip->read_reg(chip->reg_set) & set_mask;
+		*bits |= chip->read_reg(&chip->reg_set) & set_mask;
 	if (get_mask)
-		*bits |= chip->read_reg(chip->reg_dat) & get_mask;
+		*bits |= chip->read_reg(&chip->reg_dat) & get_mask;
 
 	return 0;
 }
@@ -173,7 +190,7 @@ static int gpio_mmio_get(struct gpio_chip *gc, unsigned int gpio)
 {
 	struct gpio_generic_chip *chip = to_gpio_generic_chip(gc);
 
-	return !!(chip->read_reg(chip->reg_dat) & gpio_mmio_line2mask(gc, gpio));
+	return !!(chip->read_reg(&chip->reg_dat) & gpio_mmio_line2mask(gc, gpio));
 }
 
 /*
@@ -186,7 +203,7 @@ static int gpio_mmio_get_multiple(struct gpio_chip *gc, unsigned long *mask,
 
 	/* Make sure we first clear any bits that are zero when we read the register */
 	*bits &= ~*mask;
-	*bits |= chip->read_reg(chip->reg_dat) & *mask;
+	*bits |= chip->read_reg(&chip->reg_dat) & *mask;
 	return 0;
 }
 
@@ -209,7 +226,7 @@ static int gpio_mmio_get_multiple_be(struct gpio_chip *gc, unsigned long *mask,
 		readmask |= gpio_mmio_line2mask(gc, bit);
 
 	/* Read the register */
-	val = chip->read_reg(chip->reg_dat) & readmask;
+	val = chip->read_reg(&chip->reg_dat) & readmask;
 
 	/*
 	 * Mirror the result into the "bits" result, this will give line 0
@@ -238,7 +255,7 @@ static int gpio_mmio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	else
 		chip->sdata &= ~mask;
 
-	chip->write_reg(chip->reg_dat, chip->sdata);
+	chip->write_reg(&chip->reg_dat, chip->sdata);
 
 	return 0;
 }
@@ -250,9 +267,9 @@ static int gpio_mmio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
 	unsigned long mask = gpio_mmio_line2mask(gc, gpio);
 
 	if (val)
-		chip->write_reg(chip->reg_set, mask);
+		chip->write_reg(&chip->reg_set, mask);
 	else
-		chip->write_reg(chip->reg_clr, mask);
+		chip->write_reg(&chip->reg_clr, mask);
 
 	return 0;
 }
@@ -269,7 +286,7 @@ static int gpio_mmio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	else
 		chip->sdata &= ~mask;
 
-	chip->write_reg(chip->reg_set, chip->sdata);
+	chip->write_reg(&chip->reg_set, chip->sdata);
 
 	return 0;
 }
@@ -297,7 +314,7 @@ static void gpio_mmio_multiple_get_masks(struct gpio_chip *gc,
 static void gpio_mmio_set_multiple_single_reg(struct gpio_chip *gc,
 					      unsigned long *mask,
 					      unsigned long *bits,
-					      void __iomem *reg)
+					      struct gpio_chip_reg *reg)
 {
 	struct gpio_generic_chip *chip = to_gpio_generic_chip(gc);
 	unsigned long set_mask, clear_mask;
@@ -317,7 +334,7 @@ static int gpio_mmio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
 {
 	struct gpio_generic_chip *chip = to_gpio_generic_chip(gc);
 
-	gpio_mmio_set_multiple_single_reg(gc, mask, bits, chip->reg_dat);
+	gpio_mmio_set_multiple_single_reg(gc, mask, bits, &chip->reg_dat);
 
 	return 0;
 }
@@ -327,7 +344,7 @@ static int gpio_mmio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
 {
 	struct gpio_generic_chip *chip = to_gpio_generic_chip(gc);
 
-	gpio_mmio_set_multiple_single_reg(gc, mask, bits, chip->reg_set);
+	gpio_mmio_set_multiple_single_reg(gc, mask, bits, &chip->reg_set);
 
 	return 0;
 }
@@ -342,9 +359,9 @@ static int gpio_mmio_set_multiple_with_clear(struct gpio_chip *gc,
 	gpio_mmio_multiple_get_masks(gc, mask, bits, &set_mask, &clear_mask);
 
 	if (set_mask)
-		chip->write_reg(chip->reg_set, set_mask);
+		chip->write_reg(&chip->reg_set, set_mask);
 	if (clear_mask)
-		chip->write_reg(chip->reg_clr, clear_mask);
+		chip->write_reg(&chip->reg_clr, clear_mask);
 
 	return 0;
 }
@@ -394,10 +411,10 @@ static int gpio_mmio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 	scoped_guard(raw_spinlock_irqsave, &chip->lock) {
 		chip->sdir &= ~gpio_mmio_line2mask(gc, gpio);
 
-		if (chip->reg_dir_in)
-			chip->write_reg(chip->reg_dir_in, ~chip->sdir);
-		if (chip->reg_dir_out)
-			chip->write_reg(chip->reg_dir_out, chip->sdir);
+		if (gpio_chip_reg_is_set(&chip->reg_dir_in))
+			chip->write_reg(&chip->reg_dir_in, ~chip->sdir);
+		if (gpio_chip_reg_is_set(&chip->reg_dir_out))
+			chip->write_reg(&chip->reg_dir_out, chip->sdir);
 	}
 
 	return gpio_mmio_dir_return(gc, gpio, false);
@@ -414,14 +431,14 @@ static int gpio_mmio_get_dir(struct gpio_chip *gc, unsigned int gpio)
 		return GPIO_LINE_DIRECTION_IN;
 	}
 
-	if (chip->reg_dir_out) {
-		if (chip->read_reg(chip->reg_dir_out) & gpio_mmio_line2mask(gc, gpio))
+	if (gpio_chip_reg_is_set(&chip->reg_dir_out)) {
+		if (chip->read_reg(&chip->reg_dir_out) & gpio_mmio_line2mask(gc, gpio))
 			return GPIO_LINE_DIRECTION_OUT;
 		return GPIO_LINE_DIRECTION_IN;
 	}
 
-	if (chip->reg_dir_in)
-		if (!(chip->read_reg(chip->reg_dir_in) & gpio_mmio_line2mask(gc, gpio)))
+	if (gpio_chip_reg_is_set(&chip->reg_dir_in))
+		if (!(chip->read_reg(&chip->reg_dir_in) & gpio_mmio_line2mask(gc, gpio)))
 			return GPIO_LINE_DIRECTION_OUT;
 
 	return GPIO_LINE_DIRECTION_IN;
@@ -435,10 +452,10 @@ static void gpio_mmio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 
 	chip->sdir |= gpio_mmio_line2mask(gc, gpio);
 
-	if (chip->reg_dir_in)
-		chip->write_reg(chip->reg_dir_in, ~chip->sdir);
-	if (chip->reg_dir_out)
-		chip->write_reg(chip->reg_dir_out, chip->sdir);
+	if (gpio_chip_reg_is_set(&chip->reg_dir_in))
+		chip->write_reg(&chip->reg_dir_in, ~chip->sdir);
+	if (gpio_chip_reg_is_set(&chip->reg_dir_out))
+		chip->write_reg(&chip->reg_dir_out, chip->sdir);
 }
 
 static int gpio_mmio_dir_out_dir_first(struct gpio_chip *gc, unsigned int gpio,
@@ -504,6 +521,20 @@ static int gpio_mmio_setup_accessors(struct device *dev,
 	return 0;
 }
 
+/*
+ * Initialize registers based on whether the config is io_port or not.
+ */
+static void gpio_mmio_setup_regs(struct gpio_generic_chip *chip,
+			      const struct gpio_generic_chip_config *cfg)
+{
+	gpio_chip_reg_init(&chip->reg_dat, chip->io_port, cfg->dat, cfg->dat_port);
+	gpio_chip_reg_init(&chip->reg_dat, chip->io_port, cfg->dat, cfg->dat_port);
+	gpio_chip_reg_init(&chip->reg_set, chip->io_port, cfg->set, cfg->set_port);
+	gpio_chip_reg_init(&chip->reg_clr, chip->io_port, cfg->clr, cfg->clr_port);
+	gpio_chip_reg_init(&chip->reg_dir_in, chip->io_port, cfg->dirin, cfg->dirin_port);
+	gpio_chip_reg_init(&chip->reg_dir_out, chip->io_port, cfg->dirout, cfg->dirout_port);
+}
+
 /*
  * Create the device and allocate the resources.  For setting GPIO's there are
  * three supported configurations:
@@ -531,17 +562,15 @@ static int gpio_mmio_setup_io(struct gpio_generic_chip *chip,
 {
 	struct gpio_chip *gc = &chip->gc;
 
-	chip->reg_dat = cfg->dat;
-	if (!chip->reg_dat)
+	if (!gpio_chip_reg_is_set(&chip->reg_dat))
 		return -EINVAL;
 
-	if (cfg->set && cfg->clr) {
-		chip->reg_set = cfg->set;
-		chip->reg_clr = cfg->clr;
+	if (gpio_chip_reg_is_set(&chip->reg_set) &&
+	    gpio_chip_reg_is_set(&chip->reg_clr)) {
 		gc->set = gpio_mmio_set_with_clear;
 		gc->set_multiple = gpio_mmio_set_multiple_with_clear;
-	} else if (cfg->set && !cfg->clr) {
-		chip->reg_set = cfg->set;
+	} else if (gpio_chip_reg_is_set(&chip->reg_set) &&
+		   !gpio_chip_reg_is_set(&chip->reg_clr)) {
 		gc->set = gpio_mmio_set_set;
 		gc->set_multiple = gpio_mmio_set_multiple_set;
 	} else if (cfg->flags & GPIO_GENERIC_NO_OUTPUT) {
@@ -579,10 +608,8 @@ static int gpio_mmio_setup_direction(struct gpio_generic_chip *chip,
 				     const struct gpio_generic_chip_config *cfg)
 {
 	struct gpio_chip *gc = &chip->gc;
-
-	if (cfg->dirout || cfg->dirin) {
-		chip->reg_dir_out = cfg->dirout;
-		chip->reg_dir_in = cfg->dirin;
+	if (gpio_chip_reg_is_set(&chip->reg_dir_out) ||
+	gpio_chip_reg_is_set(&chip->reg_dir_in)) {
 		if (cfg->flags & GPIO_GENERIC_NO_SET_ON_INPUT)
 			gc->direction_output = gpio_mmio_dir_out_dir_first;
 		else
@@ -651,6 +678,8 @@ int gpio_generic_chip_init(struct gpio_generic_chip *chip,
 	if (ret)
 		gc->ngpio = chip->bits;
 
+	gpio_mmio_setup_regs(chip, cfg);
+
 	ret = gpio_mmio_setup_io(chip, cfg);
 	if (ret)
 		return ret;
@@ -670,10 +699,10 @@ int gpio_generic_chip_init(struct gpio_generic_chip *chip,
 		gc->free = gpiochip_generic_free;
 	}
 
-	chip->sdata = chip->read_reg(chip->reg_dat);
+	chip->sdata = chip->read_reg(&chip->reg_dat);
 	if (gc->set == gpio_mmio_set_set &&
 			!(flags & GPIO_GENERIC_UNREADABLE_REG_SET))
-		chip->sdata = chip->read_reg(chip->reg_set);
+		chip->sdata = chip->read_reg(&chip->reg_set);
 
 	if (flags & GPIO_GENERIC_UNREADABLE_REG_DIR)
 		chip->dir_unreadable = true;
@@ -681,20 +710,21 @@ int gpio_generic_chip_init(struct gpio_generic_chip *chip,
 	/*
 	 * Inspect hardware to find initial direction setting.
 	 */
-	if ((chip->reg_dir_out || chip->reg_dir_in) &&
+	if ((gpio_chip_reg_is_set(&chip->reg_dir_out) || gpio_chip_reg_is_set(&chip->reg_dir_in)) &&
 	    !(flags & GPIO_GENERIC_UNREADABLE_REG_DIR)) {
-		if (chip->reg_dir_out)
-			chip->sdir = chip->read_reg(chip->reg_dir_out);
-		else if (chip->reg_dir_in)
-			chip->sdir = ~chip->read_reg(chip->reg_dir_in);
+		if (gpio_chip_reg_is_set(&chip->reg_dir_out))
+			chip->sdir = chip->read_reg(&chip->reg_dir_out);
+		else if (gpio_chip_reg_is_set(&chip->reg_dir_in))
+			chip->sdir = ~chip->read_reg(&chip->reg_dir_in);
 		/*
 		 * If we have two direction registers, synchronise
 		 * input setting to output setting, the library
 		 * can not handle a line being input and output at
 		 * the same time.
 		 */
-		if (chip->reg_dir_out && chip->reg_dir_in)
-			chip->write_reg(chip->reg_dir_in, ~chip->sdir);
+		if (gpio_chip_reg_is_set(&chip->reg_dir_out) &&
+		    gpio_chip_reg_is_set(&chip->reg_dir_in))
+			chip->write_reg(&chip->reg_dir_in, ~chip->sdir);
 	}
 
 	return ret;
diff --git a/include/linux/gpio/generic.h b/include/linux/gpio/generic.h
index eec63f8fe144..d0de8f77d0bc 100644
--- a/include/linux/gpio/generic.h
+++ b/include/linux/gpio/generic.h
@@ -108,15 +108,15 @@ struct gpio_chip_reg {
  */
 struct gpio_generic_chip {
 	struct gpio_chip gc;
-	unsigned long (*read_reg)(void __iomem *reg);
-	void (*write_reg)(void __iomem *reg, unsigned long data);
+	unsigned long (*read_reg)(struct gpio_chip_reg *reg);
+	void (*write_reg)(struct gpio_chip_reg *reg, unsigned long data);
 	bool be_bits;
 	bool io_port;
-	void __iomem *reg_dat;
-	void __iomem *reg_set;
-	void __iomem *reg_clr;
-	void __iomem *reg_dir_out;
-	void __iomem *reg_dir_in;
+	struct gpio_chip_reg reg_dat;
+	struct gpio_chip_reg reg_set;
+	struct gpio_chip_reg reg_clr;
+	struct gpio_chip_reg reg_dir_out;
+	struct gpio_chip_reg reg_dir_in;
 	bool dir_unreadable;
 	bool pinctrl;
 	int bits;
@@ -168,10 +168,13 @@ gpio_generic_chip_set(struct gpio_generic_chip *chip, unsigned int offset,
 static inline unsigned long
 gpio_generic_read_reg(struct gpio_generic_chip *chip, void __iomem *reg)
 {
+	struct gpio_chip_reg rg;
+
 	if (WARN_ON(!chip->read_reg))
 		return 0;
 
-	return chip->read_reg(reg);
+	rg.mmio = reg;
+	return chip->read_reg(&rg);
 }
 
 /**
@@ -183,10 +186,13 @@ gpio_generic_read_reg(struct gpio_generic_chip *chip, void __iomem *reg)
 static inline void gpio_generic_write_reg(struct gpio_generic_chip *chip,
 					  void __iomem *reg, unsigned long val)
 {
+	struct gpio_chip_reg rg;
+
 	if (WARN_ON(!chip->write_reg))
 		return;
 
-	chip->write_reg(reg, val);
+	rg.mmio = reg;
+	chip->write_reg(&rg, val);
 }
 
 #define gpio_generic_chip_lock(gen_gc) \
-- 
2.53.0

[RFC PATCH 5/5] gpio: mmio: add port-mapped read/write callbacks

[Jose Javier Rodriguez Barbarin]

Implement PMIO read and write callbacks for 8-, 16- and 32-bit
register accesses using the corresponding port I/O helpers.

Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
---
 drivers/gpio/gpio-mmio.c | 102 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 247ce5b76441..9b20283632a9 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -124,6 +124,60 @@ static unsigned long gpio_mmio_read32be(struct gpio_chip_reg *reg)
 	return ioread32be(reg->mmio);
 }
 
+#ifdef CONFIG_HAS_IOPORT
+
+static void gpio_port_write8(struct gpio_chip_reg *reg, unsigned long data)
+{
+	outb(data, reg->port);
+}
+
+static unsigned long gpio_port_read8(struct gpio_chip_reg *reg)
+{
+	return inb(reg->port);
+}
+
+static void gpio_port_write16(struct gpio_chip_reg *reg, unsigned long data)
+{
+	outw(data, reg->port);
+}
+
+static unsigned long gpio_port_read16(struct gpio_chip_reg *reg)
+{
+	return inw(reg->port);
+}
+
+static void gpio_port_write32(struct gpio_chip_reg *reg, unsigned long data)
+{
+	outl(data, reg->port);
+}
+
+static unsigned long gpio_port_read32(struct gpio_chip_reg *reg)
+{
+	return inl(reg->port);
+}
+
+static void gpio_port_write16be(struct gpio_chip_reg *reg, unsigned long data)
+{
+	outw(swab16(data), reg->port);
+}
+
+static unsigned long gpio_port_read16be(struct gpio_chip_reg *reg)
+{
+	return swab16(inw(reg->port));
+}
+
+static void gpio_port_write32be(struct gpio_chip_reg *reg, unsigned long data)
+{
+	outl(swab32(data), reg->port);
+}
+
+static unsigned long gpio_port_read32be(struct gpio_chip_reg *reg)
+{
+	return swab32(inl(reg->port));
+}
+
+#endif /* CONFIG_HAS_IOPORT */
+
 static inline void gpio_chip_reg_init(struct gpio_chip_reg *reg, bool io_port,
 				 void __iomem *addr, unsigned long port)
 {
@@ -474,6 +528,46 @@ static int gpio_mmio_dir_out_val_first(struct gpio_chip *gc, unsigned int gpio,
 	return gpio_mmio_dir_return(gc, gpio, true);
 }
 
+static int gpio_port_setup_accessors(struct device *dev,
+				     struct gpio_generic_chip *chip,
+				     bool byte_be)
+{
+#ifdef CONFIG_HAS_IOPORT
+	switch (chip->bits) {
+	case 8:
+		chip->read_reg	= gpio_port_read8;
+		chip->write_reg	= gpio_port_write8;
+		break;
+	case 16:
+		if (byte_be) {
+			chip->read_reg	= gpio_port_read16be;
+			chip->write_reg	= gpio_port_write16be;
+		} else {
+			chip->read_reg	= gpio_port_read16;
+			chip->write_reg	= gpio_port_write16;
+		}
+		break;
+	case 32:
+		if (byte_be) {
+			chip->read_reg	= gpio_port_read32be;
+			chip->write_reg	= gpio_port_write32be;
+		} else {
+			chip->read_reg	= gpio_port_read32;
+			chip->write_reg	= gpio_port_write32;
+		}
+		break;
+	default:
+		dev_err(dev, "unsupported data width %u bits\n", chip->bits);
+		return -EINVAL;
+	}
+
+	return 0;
+#else
+	dev_err(dev, "not supported because of missing I/O resource\n");
+	return -ENXIO;
+#endif /* CONFIG_HAS_IOPORT */
+}
+
 static int gpio_mmio_setup_accessors(struct device *dev,
 				     struct gpio_generic_chip *chip,
 				     bool byte_be)
@@ -684,8 +778,12 @@ int gpio_generic_chip_init(struct gpio_generic_chip *chip,
 	if (ret)
 		return ret;
 
-	ret = gpio_mmio_setup_accessors(dev, chip,
-				    flags & GPIO_GENERIC_BIG_ENDIAN_BYTE_ORDER);
+	if (chip->io_port)
+		ret = gpio_port_setup_accessors(dev, chip,
+					flags & GPIO_GENERIC_BIG_ENDIAN_BYTE_ORDER);
+	else
+		ret = gpio_mmio_setup_accessors(dev, chip,
+					flags & GPIO_GENERIC_BIG_ENDIAN_BYTE_ORDER);
 	if (ret)
 		return ret;
 
-- 
2.53.0

Re: [RFC PATCH 1/5] gpio: generic: add a generic register wrapper for MMIO and PMIO

[Linus Walleij]

Hi Jose,

thanks for your patch!

On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:

> For adding support to port-mapped devices within gpio_generic_chip, a new
> data type is required for passing to read_reg() and write_reg()
> callbacks as I/O port-mapped and memory-mapped registers use different
> data types.
>
> Introduce a new struct gpio_chip_reg to encapsulate mmio addresses and
> I/O port numbers.
>
> Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>

Overall this is just fine, but I have one idea:

> +struct gpio_chip_reg {
> +       void __iomem *mmio;
> +       unsigned long port;
> +};

Since you're always using one or the other, can we use a union?

union gpio_chip_reg {
    void __iomem *mmio;
    unsigned long port;
};

This way it will use the same amount of memory no matter which version is
used.

OK it's a micro-optimization but also elegant because it reflects the fact that
we just use one or the other.

Yours,
Linus Walleij

Re: [RFC PATCH 2/5] gpio: generic: extend gpio_generic_chip_config with PMIO register fields

[Linus Walleij]

Hi Jose,

thanks for your patch!

On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:

> Instead of replacing the register type with gpio_chip_reg, it is better
> to extend the structure by adding dedicated fields for I/O port
> registers.
>
> This ensures compatibility with memory-mapped devices.
>
> Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
(...)
> @@ -54,6 +59,11 @@ struct gpio_generic_chip_config {
>         void __iomem *clr;
>         void __iomem *dirout;
>         void __iomem *dirin;
> +       unsigned long dat_port;
> +       unsigned long set_port;
> +       unsigned long clr_port;
> +       unsigned long dirout_port;
> +       unsigned long dirin_port;

Isn't it easier to just create a new config type?

struct gpio_generic_port_chip_config { ... }?

It's only three more entries apart from this that will be duplicate.

This is only used during config anyway so it should be fine I think.

Yours,
Linus Walleij

Re: [RFC PATCH 3/5] gpio: generic: add io_port to struct gpio_generic_chip

[Linus Walleij]

On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:

> Add an io_port field to struct gpio_generic_chip. Initialize it
> depending on whether the GPIO_GENERIC_PORT_MAPPED flag is set.
>
> Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>

This looks reasonable!

Yours,
Linus Walleij

Re: [RFC PATCH 4/5] gpio: mmio: convert accessors to generic register descriptors

[Linus Walleij]

Hi Jose,

thanks for your patch!

On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:

> Convert the gpio-mmio accessors to use struct gpio_chip_reg instead of
> the previous MMIO-only register type.
>
> This allows the same accessors to operate on both MMIO and PMIO
> registers and aligns gpio-mmio with the updated gpio_generic_chip API.
>
> Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>

This should work the same fine with a union as described per
patch 1, and this is where that will start saving a bunch of memory.

Possibly squash patch 1 into this patch? The new struct/union
isn't used until here.

Yours,
Linus Walleij

Re: [RFC PATCH 5/5] gpio: mmio: add port-mapped read/write callbacks

[Linus Walleij]

Hi Jose,

thanks for your patch!

On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:

> Implement PMIO read and write callbacks for 8-, 16- and 32-bit
> register accesses using the corresponding port I/O helpers.
>
> Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>

Overall this looks fine to me, but with the change that it is moved
over to using the union.

Yours,
Linus Walleij

Re: [RFC PATCH 0/5] gpio: add PMIO support to gpio-mmio

[Linus Walleij]

Hi Jose,

thanks for your proposal!!

I'm very happy to see some traction on this. I add WBG to CC because he
wrote so many port-mapped drivers that I think he'll be thrilled to make
use of this as well.

On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:

> This series is an RFC for adding port-mapped I/O (PMIO) support to
> gpio-mmio.
(...)
> In particular, feedback would be appreciated on:
> - whether extending gpio_generic_chip_config is the right direction;

Pointed out on the patch that you can just create a
gpio_generic_port_chip_config
or something like that, it's only used at config time (usually locally
in probe()) resulting in a transient stack allocation anyway.

Also that makes it easier to see what's going on.

> - whether introducing a common MMIO/PMIO register descriptor is
>   acceptable;
> - whether PMIO support should instead be implemented differently in
>   gpio-mmio.

The main feedback I have is to use a union between port and
MMIO address instead of a struct with both.

It makes it clear that we only ever use one of them and saves
some memory, especially since we use several instances of
it per generic chip later in the code.

Yours,
Linus Walleij

Re: [RFC PATCH 3/5] gpio: generic: add io_port to struct gpio_generic_chip

[Linus Walleij]

On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
<dev-josejavier.rodriguez@duagon.com> wrote:

> @@ -645,6 +645,7 @@ int gpio_generic_chip_init(struct gpio_generic_chip *chip,
>         gc->base = -1;
>         gc->request = gpio_mmio_request;
>         chip->be_bits = !!(flags & GPIO_GENERIC_BIG_ENDIAN);
> +       chip->io_port = !!(flags & GPIO_GENERIC_PORT_MAPPED);

Actually, if you follow my idead to use a special config for port-based
chips, struct gpio_generic_port_chip_config, that means that you will
need to refactor-split this:

int gpio_generic_chip_init(struct gpio_generic_chip *chip,
                           const struct gpio_generic_chip_config *cfg)

Something like this:

static int gpio_generic_chip_init_common(struct gpio_generic_chip *chip,
    unsigned long flags, struct device *dev)

int gpio_generic_chip_init(struct gpio_generic_chip *chip,
                           const struct gpio_generic_chip_config *cfg)

int gpio_generic_port_chip_init(struct gpio_generic_chip *chip,
                           const struct gpio_generic_port_chip_config *cfg)

(Those prototypes half-guessed by looking at the code...)

So that each variant calls the common code for the generic part and
the MMIO-only respective port-only portions are kept inside
the specialized init functions, with gpio_generic_port_chip_init()
ifdef:ed out for !PORT_IO.

In that case, chip->io_port can just uncoditionally be assigned
true from gpio_generic_port_chip_init() and you don't even need
the special flag for port mapped IO.

Yours,
Linus Walleij

Re: [RFC PATCH 4/5] gpio: mmio: convert accessors to generic register descriptors

[Jose Javier Rodriguez Barbarin]

On Thu, Apr 09, 2026 at 10:07:24AM +0200, Linus Walleij wrote:
> Hi Jose,
> 
> thanks for your patch!
> 
Hi Linus,

Thanks for your answer

> On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
> <dev-josejavier.rodriguez@duagon.com> wrote:
> 
> > Convert the gpio-mmio accessors to use struct gpio_chip_reg instead of
> > the previous MMIO-only register type.
> >
> > This allows the same accessors to operate on both MMIO and PMIO
> > registers and aligns gpio-mmio with the updated gpio_generic_chip API.
> >
> > Signed-off-by: Jose Javier Rodriguez Barbarin <dev-josejavier.rodriguez@duagon.com>
> 
> This should work the same fine with a union as described per
> patch 1, and this is where that will start saving a bunch of memory.
> 
> Possibly squash patch 1 into this patch? The new struct/union
> isn't used until here.

Yes, I can squash both patches. I sent them separated to make a quick
introduction of the new structure in first patch but as you pointed
out, it is better to squash them.

> 
> Yours,
> Linus Walleij

Regards,

Javier R.

Re: [RFC PATCH 0/5] gpio: add PMIO support to gpio-mmio

[Jose Javier Rodriguez Barbarin]

On Thu, Apr 09, 2026 at 10:14:17AM +0200, Linus Walleij wrote:
> Hi Jose,
> 
> thanks for your proposal!!

You are welcome :)

> 
> I'm very happy to see some traction on this. I add WBG to CC because he
> wrote so many port-mapped drivers that I think he'll be thrilled to make
> use of this as well.

I was a bit worried about if my approach was correct or not, so I'm really
glad to hear such positive feedback.

> 
> On Tue, Apr 7, 2026 at 8:49 PM Jose Javier Rodriguez Barbarin
> <dev-josejavier.rodriguez@duagon.com> wrote:
> 
> > This series is an RFC for adding port-mapped I/O (PMIO) support to
> > gpio-mmio.
> (...)
> > In particular, feedback would be appreciated on:
> > - whether extending gpio_generic_chip_config is the right direction;
> 
> Pointed out on the patch that you can just create a
> gpio_generic_port_chip_config
> or something like that, it's only used at config time (usually locally
> in probe()) resulting in a transient stack allocation anyway.
> 
> Also that makes it easier to see what's going on.

Seems interesting.

As you pointed out, creating the new gpio_generic_port_chip_config would
need more refactoring to adapt the new structure. I'm OK with that so I'm
working on it. I will include those changes on v2.

> 
> > - whether introducing a common MMIO/PMIO register descriptor is
> >   acceptable;
> > - whether PMIO support should instead be implemented differently in
> >   gpio-mmio.
> 
> The main feedback I have is to use a union between port and
> MMIO address instead of a struct with both.
> 
> It makes it clear that we only ever use one of them and saves
> some memory, especially since we use several instances of
> it per generic chip later in the code.
> 

Annotated. I will include the union on v2.

> Yours,
> Linus Walleij

Thanks for your review and for your comments/suggestions. I really
appreciate that.

Regards,

Javier R.