[PATCH v3 0/9] Migrate i8255 GPIO drivers to regmap API

[PATCH v3 0/9] Migrate i8255 GPIO drivers to regmap API

[William Breathitt Gray]

Changes in v3:
 - Changed handle_mask_sync description to show range using a more
   typical mathematical notation ('[' changed to ')')
 - Split addition of new i8255 library regmap interface and removal of
   old i8255 library interface to separate patches
 - Split migration of 104-dio-48e, 104-idi-48, and gpio-mm modules to
   regmap-irq, gpio-regmap, and new i8255 library interface to separate
   patches
 - Simplified logic in dio48e_handle_mask_sync(); the previous version
   separated the conditional evaluations from the if statements which
   made it more complicated than it needed to be
 - Removed initial interrupt clearing in dio48e_probe(); superfluous
   because clear_on_unmask is set to true so the interrupts will be
   cleared anyway later.
 - Replace IDI48_IRQ_REG with IDI48_IRQ_STATUS and IDI48_IRQ_ENABLE to
   make the intent of this register clear

The regmap API supports IO port accessors so we can take advantage of
regmap abstractions rather than handling access to the device registers
directly in the driver. The 104-dio-48e and gpio-mm modules depend on
the i8255 library and are thus updated accordingly.

The 104-IDI-48 is hardwired as an input-only device. Because the i8255
control registers are not exposed on the 104-IDI-48, the i8255 library
doesn't provide much benefit here. In this case it's simpler to utilize
the gpio_regmap API directly, so this patch series does such and removes
the i8255 library dependency from the 104-idi-48 module.

The first patch in this series adjusts gpio_regmap to always set
gpio_chip get_direction. This patch is larger independent of the rest of
the patches in the series and can be picked up separately if desired.
I'm including it here because it allows the input-only 104-idi-48 driver
to continue reporting offset directions after migrating to gpio_regmap.

A handle_mask_sync() callback is introduced for the regmap_irq API. This
is used by the 104-dio-48e driver to handle its enable/disable interrupt
functionality. The 104-DIO-48E has a single register to handle
enabling/disabling interrupts: a write of any value to this register
enables interrupts, while any read disables interrupts.

By leveraging the gpio_regmap API, the i8255 library is reduced to
simply a devm_i8255_regmap_register() function, a configuration
structure struct i8255_regmap_config, and a helper macro
i8255_volatile_regmap_range() provided to simplify volatile PPI register
hinting for the regmap.

The regmap_irq API is leveraged by the 104-idi-48 and 104-dio-48e
modules to support their IRQ functionality. Do their respective regmap
configurations need use_hwlock set to true in this case, or is adequate
locking already handled by the regmap_irq API?

William Breathitt Gray (9):
  gpio: regmap: Always set gpio_chip get_direction
  regmap-irq: Add handle_mask_sync() callback
  gpio: 104-dio-48e: Migrate to the regmap-irq API
  gpio: 104-idi-48: Migrate to the regmap-irq API
  gpio: 104-idi-48: Migrate to gpio-regmap API
  gpio: i8255: Migrate to gpio-regmap API
  gpio: 104-dio-48e: Migrate to regmap API
  gpio: gpio-mm: Migrate to regmap API
  gpio: i8255: Remove unused legacy interface

 drivers/base/regmap/regmap-irq.c |  44 +++-
 drivers/gpio/Kconfig             |   5 +-
 drivers/gpio/gpio-104-dio-48e.c  | 395 ++++++++++---------------------
 drivers/gpio/gpio-104-idi-48.c   | 325 ++++++++-----------------
 drivers/gpio/gpio-gpio-mm.c      | 153 +++---------
 drivers/gpio/gpio-i8255.c        | 320 +++++++------------------
 drivers/gpio/gpio-i8255.h        |  54 ++---
 drivers/gpio/gpio-regmap.c       |   7 +-
 include/linux/regmap.h           |   5 +
 9 files changed, 412 insertions(+), 896 deletions(-)


base-commit: 3687a82b1db1c827cc4b367e3efde3235f68d9f6
-- 
2.38.1

[PATCH v3 1/9] gpio: regmap: Always set gpio_chip get_direction

[William Breathitt Gray]

If you only have reg_dat_base set, then it is input-only; if you only
have reg_set_base set, then it is output-only. Thus, we can always set
gpio_chip get_direction to gpio_regmap_get_direction and return
GPIO_LINE_DIRECTION_IN/GPIO_LINE_DIRECTION_OUT given the respective
register base addresses configuration.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Michael Walle <michael@walle.cc>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/gpio-regmap.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 6383136cbe59..f907c9c19fce 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -111,6 +111,11 @@ static int gpio_regmap_get_direction(struct gpio_chip *chip,
 	unsigned int base, val, reg, mask;
 	int invert, ret;
 
+	if (gpio->reg_dat_base && !gpio->reg_set_base)
+		return GPIO_LINE_DIRECTION_IN;
+	if (gpio->reg_set_base && !gpio->reg_dat_base)
+		return GPIO_LINE_DIRECTION_OUT;
+
 	if (gpio->reg_dir_out_base) {
 		base = gpio_regmap_addr(gpio->reg_dir_out_base);
 		invert = 0;
@@ -265,8 +270,8 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 	else if (gpio->reg_set_base)
 		chip->set = gpio_regmap_set;
 
+	chip->get_direction = gpio_regmap_get_direction;
 	if (gpio->reg_dir_in_base || gpio->reg_dir_out_base) {
-		chip->get_direction = gpio_regmap_get_direction;
 		chip->direction_input = gpio_regmap_direction_input;
 		chip->direction_output = gpio_regmap_direction_output;
 	}
-- 
2.38.1

[PATCH v3 2/9] regmap-irq: Add handle_mask_sync() callback

[William Breathitt Gray]

Provide a public callback handle_mask_sync() that drivers can use when
they have more complex IRQ masking logic. The default implementation is
regmap_irq_handle_mask_sync(), used if the chip doesn't provide its own
callback.

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/base/regmap/regmap-irq.c | 44 ++++++++++++++++++++++----------
 include/linux/regmap.h           |  5 ++++
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 4ef9488d05cd..968681fa8d09 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -115,12 +115,20 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 	 */
 	for (i = 0; i < d->chip->num_regs; i++) {
 		if (d->mask_base) {
-			reg = d->get_irq_reg(d, d->mask_base, i);
-			ret = regmap_update_bits(d->map, reg,
-					d->mask_buf_def[i], d->mask_buf[i]);
-			if (ret)
-				dev_err(d->map->dev, "Failed to sync masks in %x\n",
-					reg);
+			if (d->chip->handle_mask_sync)
+				d->chip->handle_mask_sync(d->map, i,
+							  d->mask_buf_def[i],
+							  d->mask_buf[i],
+							  d->chip->irq_drv_data);
+			else {
+				reg = d->get_irq_reg(d, d->mask_base, i);
+				ret = regmap_update_bits(d->map, reg,
+						d->mask_buf_def[i],
+						d->mask_buf[i]);
+				if (ret)
+					dev_err(d->map->dev, "Failed to sync masks in %x\n",
+						reg);
+			}
 		}
 
 		if (d->unmask_base) {
@@ -917,13 +925,23 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 		d->mask_buf[i] = d->mask_buf_def[i];
 
 		if (d->mask_base) {
-			reg = d->get_irq_reg(d, d->mask_base, i);
-			ret = regmap_update_bits(d->map, reg,
-					d->mask_buf_def[i], d->mask_buf[i]);
-			if (ret) {
-				dev_err(map->dev, "Failed to set masks in 0x%x: %d\n",
-					reg, ret);
-				goto err_alloc;
+			if (chip->handle_mask_sync) {
+				ret = chip->handle_mask_sync(d->map, i,
+							     d->mask_buf_def[i],
+							     d->mask_buf[i],
+							     chip->irq_drv_data);
+				if (ret)
+					goto err_alloc;
+			} else {
+				reg = d->get_irq_reg(d, d->mask_base, i);
+				ret = regmap_update_bits(d->map, reg,
+						d->mask_buf_def[i],
+						d->mask_buf[i]);
+				if (ret) {
+					dev_err(map->dev, "Failed to set masks in 0x%x: %d\n",
+						reg, ret);
+					goto err_alloc;
+				}
 			}
 		}
 
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index ca3434dca3a0..cc07645501af 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1542,6 +1542,8 @@ struct regmap_irq_chip_data;
  *		     before regmap_irq_handler process the interrupts.
  * @handle_post_irq: Driver specific callback to handle interrupt from device
  *		     after handling the interrupts in regmap_irq_handler().
+ * @handle_mask_sync: Callback used to handle IRQ mask syncs. The index will be
+ *		      in the range [0, num_regs)
  * @set_type_virt:   Driver specific callback to extend regmap_irq_set_type()
  *		     and configure virt regs. Deprecated, use @set_type_config
  *		     callback and config registers instead.
@@ -1603,6 +1605,9 @@ struct regmap_irq_chip {
 
 	int (*handle_pre_irq)(void *irq_drv_data);
 	int (*handle_post_irq)(void *irq_drv_data);
+	int (*handle_mask_sync)(struct regmap *map, int index,
+				unsigned int mask_buf_def,
+				unsigned int mask_buf, void *irq_drv_data);
 	int (*set_type_virt)(unsigned int **buf, unsigned int type,
 			     unsigned long hwirq, int reg);
 	int (*set_type_config)(unsigned int **buf, unsigned int type,
-- 
2.38.1

[PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[William Breathitt Gray]

The regmap API supports IO port accessors so we can take advantage of
regmap abstractions rather than handling access to the device registers
directly in the driver.

For the 104-dio-48e we have the following IRQ registers (0xB and 0xF):

    Base Address +B (Write): Enable Interrupt
    Base Address +B (Read): Disable Interrupt
    Base Address +F (Read/Write): Clear Interrupt

Any write to 0xB will enable interrupts, while any read will disable
interrupts. Interrupts are cleared by a read or any write to 0xF.
There's no IRQ status register, so software has to assume that if an
interrupt is raised then it was for the 104-DIO-48E device.

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/Kconfig            |   1 +
 drivers/gpio/gpio-104-dio-48e.c | 274 ++++++++++++++++----------------
 2 files changed, 135 insertions(+), 140 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ec7cfd4f52b1..b62bef4e563d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -845,6 +845,7 @@ config GPIO_104_DIO_48E
 	tristate "ACCES 104-DIO-48E GPIO support"
 	depends on PC104
 	select ISA_BUS_API
+	select REGMAP_IRQ
 	select GPIOLIB_IRQCHIP
 	select GPIO_I8255
 	help
diff --git a/drivers/gpio/gpio-104-dio-48e.c b/drivers/gpio/gpio-104-dio-48e.c
index 7b8829c8e423..fcee3dc81902 100644
--- a/drivers/gpio/gpio-104-dio-48e.c
+++ b/drivers/gpio/gpio-104-dio-48e.c
@@ -8,17 +8,15 @@
  */
 #include <linux/bits.h>
 #include <linux/device.h>
-#include <linux/errno.h>
+#include <linux/err.h>
 #include <linux/gpio/driver.h>
-#include <linux/io.h>
 #include <linux/ioport.h>
-#include <linux/interrupt.h>
-#include <linux/irqdesc.h>
+#include <linux/irq.h>
 #include <linux/isa.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
-#include <linux/spinlock.h>
+#include <linux/regmap.h>
 #include <linux/types.h>
 
 #include "gpio-i8255.h"
@@ -38,46 +36,30 @@ static unsigned int num_irq;
 module_param_hw_array(irq, uint, irq, &num_irq, 0);
 MODULE_PARM_DESC(irq, "ACCES 104-DIO-48E interrupt line numbers");
 
+#define DIO48E_ENABLE_INTERRUPT 0xB
+#define DIO48E_DISABLE_INTERRUPT DIO48E_ENABLE_INTERRUPT
+#define DIO48E_CLEAR_INTERRUPT 0xF
+
 #define DIO48E_NUM_PPI 2
 
 /**
  * struct dio48e_reg - device register structure
  * @ppi:		Programmable Peripheral Interface groups
- * @enable_buffer:	Enable/Disable Buffer groups
- * @unused1:		Unused
- * @enable_interrupt:	Write: Enable Interrupt
- *			Read: Disable Interrupt
- * @unused2:		Unused
- * @enable_counter:	Write: Enable Counter/Timer Addressing
- *			Read: Disable Counter/Timer Addressing
- * @unused3:		Unused
- * @clear_interrupt:	Clear Interrupt
  */
 struct dio48e_reg {
 	struct i8255 ppi[DIO48E_NUM_PPI];
-	u8 enable_buffer[DIO48E_NUM_PPI];
-	u8 unused1;
-	u8 enable_interrupt;
-	u8 unused2;
-	u8 enable_counter;
-	u8 unused3;
-	u8 clear_interrupt;
 };
 
 /**
  * struct dio48e_gpio - GPIO device private data structure
  * @chip:		instance of the gpio_chip
  * @ppi_state:		PPI device states
- * @lock:		synchronization lock to prevent I/O race conditions
  * @reg:		I/O address offset for the device registers
- * @irq_mask:		I/O bits affected by interrupts
  */
 struct dio48e_gpio {
 	struct gpio_chip chip;
 	struct i8255_state ppi_state[DIO48E_NUM_PPI];
-	raw_spinlock_t lock;
 	struct dio48e_reg __iomem *reg;
-	unsigned char irq_mask;
 };
 
 static int dio48e_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
@@ -144,106 +126,95 @@ static void dio48e_gpio_set_multiple(struct gpio_chip *chip,
 			   bits, chip->ngpio);
 }
 
-static void dio48e_irq_ack(struct irq_data *data)
-{
-}
-
-static void dio48e_irq_mask(struct irq_data *data)
-{
-	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-	const unsigned long offset = irqd_to_hwirq(data);
-	unsigned long flags;
-
-	/* only bit 3 on each respective Port C supports interrupts */
-	if (offset != 19 && offset != 43)
-		return;
-
-	raw_spin_lock_irqsave(&dio48egpio->lock, flags);
-
-	if (offset == 19)
-		dio48egpio->irq_mask &= ~BIT(0);
-	else
-		dio48egpio->irq_mask &= ~BIT(1);
-	gpiochip_disable_irq(chip, offset);
-
-	if (!dio48egpio->irq_mask)
-		/* disable interrupts */
-		ioread8(&dio48egpio->reg->enable_interrupt);
-
-	raw_spin_unlock_irqrestore(&dio48egpio->lock, flags);
-}
-
-static void dio48e_irq_unmask(struct irq_data *data)
-{
-	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-	const unsigned long offset = irqd_to_hwirq(data);
-	unsigned long flags;
-
-	/* only bit 3 on each respective Port C supports interrupts */
-	if (offset != 19 && offset != 43)
-		return;
-
-	raw_spin_lock_irqsave(&dio48egpio->lock, flags);
+static const struct regmap_range dio48e_wr_ranges[] = {
+	regmap_reg_range(0x0, 0x9), regmap_reg_range(0xB, 0xB),
+	regmap_reg_range(0xD, 0xD), regmap_reg_range(0xF, 0xF),
+};
+static const struct regmap_range dio48e_rd_ranges[] = {
+	regmap_reg_range(0x0, 0x2), regmap_reg_range(0x4, 0x6),
+	regmap_reg_range(0xB, 0xB), regmap_reg_range(0xD, 0xD),
+	regmap_reg_range(0xF, 0xF),
+};
+static const struct regmap_range dio48e_volatile_ranges[] = {
+	i8255_volatile_regmap_range(0x0), i8255_volatile_regmap_range(0x4),
+	regmap_reg_range(0xB, 0xB), regmap_reg_range(0xD, 0xD),
+	regmap_reg_range(0xF, 0xF),
+};
+static const struct regmap_range dio48e_precious_ranges[] = {
+	regmap_reg_range(0xB, 0xB), regmap_reg_range(0xD, 0xD),
+	regmap_reg_range(0xF, 0xF),
+};
+static const struct regmap_access_table dio48e_wr_table = {
+	.yes_ranges = dio48e_wr_ranges,
+	.n_yes_ranges = ARRAY_SIZE(dio48e_wr_ranges),
+};
+static const struct regmap_access_table dio48e_rd_table = {
+	.yes_ranges = dio48e_rd_ranges,
+	.n_yes_ranges = ARRAY_SIZE(dio48e_rd_ranges),
+};
+static const struct regmap_access_table dio48e_volatile_table = {
+	.yes_ranges = dio48e_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(dio48e_volatile_ranges),
+};
+static const struct regmap_access_table dio48e_precious_table = {
+	.yes_ranges = dio48e_precious_ranges,
+	.n_yes_ranges = ARRAY_SIZE(dio48e_precious_ranges),
+};
+static const struct regmap_config dio48e_regmap_config = {
+	.reg_bits = 8,
+	.reg_stride = 1,
+	.val_bits = 8,
+	.io_port = true,
+	.max_register = 0xF,
+	.wr_table = &dio48e_wr_table,
+	.rd_table = &dio48e_rd_table,
+	.volatile_table = &dio48e_volatile_table,
+	.precious_table = &dio48e_precious_table,
+	.cache_type = REGCACHE_FLAT,
+};
 
-	if (!dio48egpio->irq_mask) {
-		/* enable interrupts */
-		iowrite8(0x00, &dio48egpio->reg->clear_interrupt);
-		iowrite8(0x00, &dio48egpio->reg->enable_interrupt);
+/* only bit 3 on each respective Port C supports interrupts */
+#define DIO48E_REGMAP_IRQ(_ppi) \
+	[19 + (_ppi) * 24] = { \
+		.mask = BIT(_ppi), \
+		.type = { .types_supported = IRQ_TYPE_EDGE_RISING, }, \
 	}
 
-	gpiochip_enable_irq(chip, offset);
-	if (offset == 19)
-		dio48egpio->irq_mask |= BIT(0);
-	else
-		dio48egpio->irq_mask |= BIT(1);
-
-	raw_spin_unlock_irqrestore(&dio48egpio->lock, flags);
-}
-
-static int dio48e_irq_set_type(struct irq_data *data, unsigned int flow_type)
-{
-	const unsigned long offset = irqd_to_hwirq(data);
-
-	/* only bit 3 on each respective Port C supports interrupts */
-	if (offset != 19 && offset != 43)
-		return -EINVAL;
-
-	if (flow_type != IRQ_TYPE_NONE && flow_type != IRQ_TYPE_EDGE_RISING)
-		return -EINVAL;
-
-	return 0;
-}
-
-static const struct irq_chip dio48e_irqchip = {
-	.name = "104-dio-48e",
-	.irq_ack = dio48e_irq_ack,
-	.irq_mask = dio48e_irq_mask,
-	.irq_unmask = dio48e_irq_unmask,
-	.irq_set_type = dio48e_irq_set_type,
-	.flags = IRQCHIP_IMMUTABLE,
-	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+static const struct regmap_irq dio48e_regmap_irqs[] = {
+	DIO48E_REGMAP_IRQ(0), DIO48E_REGMAP_IRQ(1),
 };
 
-static irqreturn_t dio48e_irq_handler(int irq, void *dev_id)
+static int dio48e_handle_mask_sync(struct regmap *const map, const int index,
+				   const unsigned int mask_buf_def,
+				   const unsigned int mask_buf,
+				   void *const irq_drv_data)
 {
-	struct dio48e_gpio *const dio48egpio = dev_id;
-	struct gpio_chip *const chip = &dio48egpio->chip;
-	const unsigned long irq_mask = dio48egpio->irq_mask;
-	unsigned long gpio;
-
-	for_each_set_bit(gpio, &irq_mask, 2)
-		generic_handle_domain_irq(chip->irq.domain,
-					  19 + gpio*24);
-
-	raw_spin_lock(&dio48egpio->lock);
-
-	iowrite8(0x00, &dio48egpio->reg->clear_interrupt);
+	unsigned int *const irq_mask = irq_drv_data;
+	const unsigned int prev_mask = *irq_mask;
+	const unsigned int all_masked = 0x3;
+	unsigned int val;
+	int err;
 
-	raw_spin_unlock(&dio48egpio->lock);
+	/* exit early if no change since the previous mask */
+	if (mask_buf == prev_mask)
+		return 0;
+
+	/* remember the current mask for the next mask sync */
+	*irq_mask = mask_buf;
+
+	/* if all previously masked, enable interrupts when unmasking */
+	if (prev_mask == all_masked) {
+		err = regmap_write(map, DIO48E_ENABLE_INTERRUPT, 0x00);
+		if (err)
+			return err;
+	/* if all are currently masked, disable interrupts */
+	} else if (mask_buf == all_masked) {
+		err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
+		if (err)
+			return err;
+	}
 
-	return IRQ_HANDLED;
+	return 0;
 }
 
 #define DIO48E_NGPIO 48
@@ -295,8 +266,13 @@ static int dio48e_probe(struct device *dev, unsigned int id)
 {
 	struct dio48e_gpio *dio48egpio;
 	const char *const name = dev_name(dev);
-	struct gpio_irq_chip *girq;
+	void __iomem *regs;
+	struct regmap *map;
+	unsigned int val;
 	int err;
+	struct regmap_irq_chip *chip;
+	unsigned int irq_mask;
+	struct regmap_irq_chip_data *chip_data;
 
 	dio48egpio = devm_kzalloc(dev, sizeof(*dio48egpio), GFP_KERNEL);
 	if (!dio48egpio)
@@ -308,10 +284,46 @@ static int dio48e_probe(struct device *dev, unsigned int id)
 		return -EBUSY;
 	}
 
-	dio48egpio->reg = devm_ioport_map(dev, base[id], DIO48E_EXTENT);
-	if (!dio48egpio->reg)
+	regs = devm_ioport_map(dev, base[id], DIO48E_EXTENT);
+	if (!regs)
+		return -ENOMEM;
+	dio48egpio->reg = regs;
+
+	map = devm_regmap_init_mmio(dev, regs, &dio48e_regmap_config);
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
 		return -ENOMEM;
 
+	chip->irq_drv_data = devm_kzalloc(dev, sizeof(irq_mask), GFP_KERNEL);
+	if (!chip->irq_drv_data)
+		return -ENOMEM;
+
+	chip->name = name;
+	/* No IRQ status register so use CLEAR_INTERRUPT register instead */
+	chip->status_base = DIO48E_CLEAR_INTERRUPT;
+	chip->mask_base = DIO48E_ENABLE_INTERRUPT;
+	chip->clear_on_unmask = true;
+	chip->status_invert = true;
+	chip->num_regs = 1;
+	chip->irqs = dio48e_regmap_irqs;
+	chip->num_irqs = ARRAY_SIZE(dio48e_regmap_irqs);
+	chip->handle_mask_sync = dio48e_handle_mask_sync;
+
+	/* Initialize device interrupt state */
+	err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
+	if (err)
+		return err;
+
+	err = devm_regmap_add_irq_chip(dev, map, irq[id], 0, 0, chip,
+				       &chip_data);
+	if (err) {
+		dev_err(dev, "IRQ registration failed (%d)\n", err);
+		return err;
+	}
+
 	dio48egpio->chip.label = name;
 	dio48egpio->chip.parent = dev;
 	dio48egpio->chip.owner = THIS_MODULE;
@@ -326,18 +338,6 @@ static int dio48e_probe(struct device *dev, unsigned int id)
 	dio48egpio->chip.set = dio48e_gpio_set;
 	dio48egpio->chip.set_multiple = dio48e_gpio_set_multiple;
 
-	girq = &dio48egpio->chip.irq;
-	gpio_irq_chip_set_chip(girq, &dio48e_irqchip);
-	/* This will let us handle the parent IRQ in the driver */
-	girq->parent_handler = NULL;
-	girq->num_parents = 0;
-	girq->parents = NULL;
-	girq->default_type = IRQ_TYPE_NONE;
-	girq->handler = handle_edge_irq;
-	girq->init_hw = dio48e_irq_init_hw;
-
-	raw_spin_lock_init(&dio48egpio->lock);
-
 	i8255_state_init(dio48egpio->ppi_state, DIO48E_NUM_PPI);
 	dio48e_init_ppi(dio48egpio->reg->ppi, dio48egpio->ppi_state);
 
@@ -347,14 +347,8 @@ static int dio48e_probe(struct device *dev, unsigned int id)
 		return err;
 	}
 
-	err = devm_request_irq(dev, irq[id], dio48e_irq_handler, 0, name,
-		dio48egpio);
-	if (err) {
-		dev_err(dev, "IRQ handler registering failed (%d)\n", err);
-		return err;
-	}
-
-	return 0;
+	return gpiochip_irqchip_add_domain(&dio48egpio->chip,
+					   regmap_irq_get_domain(chip_data));
 }
 
 static struct isa_driver dio48e_driver = {
-- 
2.38.1

[PATCH v3 4/9] gpio: 104-idi-48: Migrate to the regmap-irq API

[William Breathitt Gray]

The regmap API supports IO port accessors so we can take advantage of
regmap abstractions rather than handling access to the device registers
directly in the driver.

For the 104-idi-48, we get an IRQ register with some status information
and basic masking, but it's broken down by banks rather than individual
GPIO. There are six banks (8 GPIO lines each) that correspond to the
lower six bits of the IRQ register (bits 0-5):

    Base Address + 7 (Read): IRQ Status Register/IRQ Clear
        Bit 0-5: Respective Bank IRQ Statuses
        Bit 6: IRQ Status (Active Low)
        Bit 7: IRQ Enable Status
    Base Address + 7 (Write): IRQ Enable/Disable
        Bit 0-5: Respective Bank IRQ Enable/Disable

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/Kconfig           |   1 +
 drivers/gpio/gpio-104-idi-48.c | 250 ++++++++++++---------------------
 2 files changed, 93 insertions(+), 158 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b62bef4e563d..6892979e511a 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -871,6 +871,7 @@ config GPIO_104_IDI_48
 	tristate "ACCES 104-IDI-48 GPIO support"
 	depends on PC104
 	select ISA_BUS_API
+	select REGMAP_IRQ
 	select GPIOLIB_IRQCHIP
 	select GPIO_I8255
 	help
diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c
index c5e231fde1af..f77c05571062 100644
--- a/drivers/gpio/gpio-104-idi-48.c
+++ b/drivers/gpio/gpio-104-idi-48.c
@@ -8,17 +8,16 @@
  */
 #include <linux/bits.h>
 #include <linux/device.h>
-#include <linux/errno.h>
+#include <linux/err.h>
 #include <linux/gpio/driver.h>
-#include <linux/io.h>
-#include <linux/ioport.h>
 #include <linux/interrupt.h>
-#include <linux/irqdesc.h>
+#include <linux/ioport.h>
+#include <linux/irq.h>
 #include <linux/isa.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
-#include <linux/spinlock.h>
+#include <linux/regmap.h>
 #include <linux/types.h>
 
 #include "gpio-i8255.h"
@@ -38,6 +37,9 @@ static unsigned int num_irq;
 module_param_hw_array(irq, uint, irq, &num_irq, 0);
 MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers");
 
+#define IDI48_IRQ_STATUS 0x7
+#define IDI48_IRQ_ENABLE IDI48_IRQ_STATUS
+
 /**
  * struct idi_48_reg - device register structure
  * @port0:	Port 0 Inputs
@@ -56,17 +58,11 @@ struct idi_48_reg {
 /**
  * struct idi_48_gpio - GPIO device private data structure
  * @chip:	instance of the gpio_chip
- * @lock:	synchronization lock to prevent I/O race conditions
- * @irq_mask:	input bits affected by interrupts
  * @reg:	I/O address offset for the device registers
- * @cos_enb:	Change-Of-State IRQ enable boundaries mask
  */
 struct idi_48_gpio {
 	struct gpio_chip chip;
-	spinlock_t lock;
-	unsigned char irq_mask[6];
 	struct idi_48_reg __iomem *reg;
-	unsigned char cos_enb;
 };
 
 static int idi_48_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
@@ -98,125 +94,65 @@ static int idi_48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
 	return 0;
 }
 
-static void idi_48_irq_ack(struct irq_data *data)
-{
-}
-
-static void idi_48_irq_mask(struct irq_data *data)
-{
-	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
-	struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
-	const unsigned int offset = irqd_to_hwirq(data);
-	const unsigned long boundary = offset / 8;
-	const unsigned long mask = BIT(offset % 8);
-	unsigned long flags;
-
-	spin_lock_irqsave(&idi48gpio->lock, flags);
-
-	idi48gpio->irq_mask[boundary] &= ~mask;
-	gpiochip_disable_irq(chip, offset);
-
-	/* Exit early if there are still input lines with IRQ unmasked */
-	if (idi48gpio->irq_mask[boundary])
-		goto exit;
-
-	idi48gpio->cos_enb &= ~BIT(boundary);
-
-	iowrite8(idi48gpio->cos_enb, &idi48gpio->reg->irq);
-
-exit:
-	spin_unlock_irqrestore(&idi48gpio->lock, flags);
-}
-
-static void idi_48_irq_unmask(struct irq_data *data)
-{
-	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
-	struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
-	const unsigned int offset = irqd_to_hwirq(data);
-	const unsigned long boundary = offset / 8;
-	const unsigned long mask = BIT(offset % 8);
-	unsigned int prev_irq_mask;
-	unsigned long flags;
-
-	spin_lock_irqsave(&idi48gpio->lock, flags);
-
-	prev_irq_mask = idi48gpio->irq_mask[boundary];
-
-	gpiochip_enable_irq(chip, offset);
-	idi48gpio->irq_mask[boundary] |= mask;
-
-	/* Exit early if IRQ was already unmasked for this boundary */
-	if (prev_irq_mask)
-		goto exit;
-
-	idi48gpio->cos_enb |= BIT(boundary);
-
-	iowrite8(idi48gpio->cos_enb, &idi48gpio->reg->irq);
-
-exit:
-	spin_unlock_irqrestore(&idi48gpio->lock, flags);
-}
-
-static int idi_48_irq_set_type(struct irq_data *data, unsigned int flow_type)
-{
-	/* The only valid irq types are none and both-edges */
-	if (flow_type != IRQ_TYPE_NONE &&
-		(flow_type & IRQ_TYPE_EDGE_BOTH) != IRQ_TYPE_EDGE_BOTH)
-		return -EINVAL;
-
-	return 0;
-}
-
-static const struct irq_chip idi_48_irqchip = {
-	.name = "104-idi-48",
-	.irq_ack = idi_48_irq_ack,
-	.irq_mask = idi_48_irq_mask,
-	.irq_unmask = idi_48_irq_unmask,
-	.irq_set_type = idi_48_irq_set_type,
-	.flags = IRQCHIP_IMMUTABLE,
-	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+static const struct regmap_range idi_48_wr_ranges[] = {
+	regmap_reg_range(0x0, 0x6),
+};
+static const struct regmap_range idi_48_rd_ranges[] = {
+	regmap_reg_range(0x0, 0x2), regmap_reg_range(0x4, 0x7),
+};
+static const struct regmap_range idi_48_precious_ranges[] = {
+	regmap_reg_range(0x7, 0x7),
+};
+static const struct regmap_access_table idi_48_wr_table = {
+	.no_ranges = idi_48_wr_ranges,
+	.n_no_ranges = ARRAY_SIZE(idi_48_wr_ranges),
+};
+static const struct regmap_access_table idi_48_rd_table = {
+	.yes_ranges = idi_48_rd_ranges,
+	.n_yes_ranges = ARRAY_SIZE(idi_48_rd_ranges),
+};
+static const struct regmap_access_table idi_48_precious_table = {
+	.yes_ranges = idi_48_precious_ranges,
+	.n_yes_ranges = ARRAY_SIZE(idi_48_precious_ranges),
+};
+static const struct regmap_config idi48_regmap_config = {
+	.reg_bits = 8,
+	.reg_stride = 1,
+	.val_bits = 8,
+	.io_port = true,
+	.max_register = 0x6,
+	.wr_table = &idi_48_wr_table,
+	.rd_table = &idi_48_rd_table,
+	.precious_table = &idi_48_precious_table,
 };
 
-static irqreturn_t idi_48_irq_handler(int irq, void *dev_id)
-{
-	struct idi_48_gpio *const idi48gpio = dev_id;
-	unsigned long cos_status;
-	unsigned long boundary;
-	unsigned long irq_mask;
-	unsigned long bit_num;
-	unsigned long gpio;
-	struct gpio_chip *const chip = &idi48gpio->chip;
-
-	spin_lock(&idi48gpio->lock);
-
-	cos_status = ioread8(&idi48gpio->reg->irq);
-
-	/* IRQ Status (bit 6) is active low (0 = IRQ generated by device) */
-	if (cos_status & BIT(6)) {
-		spin_unlock(&idi48gpio->lock);
-		return IRQ_NONE;
-	}
-
-	/* Bit 0-5 indicate which Change-Of-State boundary triggered the IRQ */
-	cos_status &= 0x3F;
-
-	for_each_set_bit(boundary, &cos_status, 6) {
-		irq_mask = idi48gpio->irq_mask[boundary];
-
-		for_each_set_bit(bit_num, &irq_mask, 8) {
-			gpio = bit_num + boundary * 8;
+#define IDI48_NGPIO 48
 
-			generic_handle_domain_irq(chip->irq.domain,
-						  gpio);
-		}
+#define IDI48_REGMAP_IRQ(_id) \
+	[_id] = { \
+		.mask = BIT((_id) / 8), \
+		.type = { .types_supported = IRQ_TYPE_EDGE_BOTH, }, \
 	}
 
-	spin_unlock(&idi48gpio->lock);
-
-	return IRQ_HANDLED;
-}
+static const struct regmap_irq idi48_regmap_irqs[IDI48_NGPIO] = {
+	IDI48_REGMAP_IRQ(0), IDI48_REGMAP_IRQ(1), IDI48_REGMAP_IRQ(2),
+	IDI48_REGMAP_IRQ(3), IDI48_REGMAP_IRQ(4), IDI48_REGMAP_IRQ(5),
+	IDI48_REGMAP_IRQ(6), IDI48_REGMAP_IRQ(7), IDI48_REGMAP_IRQ(8),
+	IDI48_REGMAP_IRQ(9), IDI48_REGMAP_IRQ(10), IDI48_REGMAP_IRQ(11),
+	IDI48_REGMAP_IRQ(12), IDI48_REGMAP_IRQ(13), IDI48_REGMAP_IRQ(14),
+	IDI48_REGMAP_IRQ(15), IDI48_REGMAP_IRQ(16), IDI48_REGMAP_IRQ(17),
+	IDI48_REGMAP_IRQ(18), IDI48_REGMAP_IRQ(19), IDI48_REGMAP_IRQ(20),
+	IDI48_REGMAP_IRQ(21), IDI48_REGMAP_IRQ(22), IDI48_REGMAP_IRQ(23),
+	IDI48_REGMAP_IRQ(24), IDI48_REGMAP_IRQ(25), IDI48_REGMAP_IRQ(26),
+	IDI48_REGMAP_IRQ(27), IDI48_REGMAP_IRQ(28), IDI48_REGMAP_IRQ(29),
+	IDI48_REGMAP_IRQ(30), IDI48_REGMAP_IRQ(31), IDI48_REGMAP_IRQ(32),
+	IDI48_REGMAP_IRQ(33), IDI48_REGMAP_IRQ(34), IDI48_REGMAP_IRQ(35),
+	IDI48_REGMAP_IRQ(36), IDI48_REGMAP_IRQ(37), IDI48_REGMAP_IRQ(38),
+	IDI48_REGMAP_IRQ(39), IDI48_REGMAP_IRQ(40), IDI48_REGMAP_IRQ(41),
+	IDI48_REGMAP_IRQ(42), IDI48_REGMAP_IRQ(43), IDI48_REGMAP_IRQ(44),
+	IDI48_REGMAP_IRQ(45), IDI48_REGMAP_IRQ(46), IDI48_REGMAP_IRQ(47),
+};
 
-#define IDI48_NGPIO 48
 static const char *idi48_names[IDI48_NGPIO] = {
 	"Bit 0 A", "Bit 1 A", "Bit 2 A", "Bit 3 A", "Bit 4 A", "Bit 5 A",
 	"Bit 6 A", "Bit 7 A", "Bit 8 A", "Bit 9 A", "Bit 10 A", "Bit 11 A",
@@ -228,22 +164,14 @@ static const char *idi48_names[IDI48_NGPIO] = {
 	"Bit 18 B", "Bit 19 B", "Bit 20 B", "Bit 21 B", "Bit 22 B", "Bit 23 B"
 };
 
-static int idi_48_irq_init_hw(struct gpio_chip *gc)
-{
-	struct idi_48_gpio *const idi48gpio = gpiochip_get_data(gc);
-
-	/* Disable IRQ by default */
-	iowrite8(0, &idi48gpio->reg->irq);
-	ioread8(&idi48gpio->reg->irq);
-
-	return 0;
-}
-
 static int idi_48_probe(struct device *dev, unsigned int id)
 {
 	struct idi_48_gpio *idi48gpio;
 	const char *const name = dev_name(dev);
-	struct gpio_irq_chip *girq;
+	void __iomem *regs;
+	struct regmap *map;
+	struct regmap_irq_chip *chip;
+	struct regmap_irq_chip_data *chip_data;
 	int err;
 
 	idi48gpio = devm_kzalloc(dev, sizeof(*idi48gpio), GFP_KERNEL);
@@ -256,10 +184,34 @@ static int idi_48_probe(struct device *dev, unsigned int id)
 		return -EBUSY;
 	}
 
-	idi48gpio->reg = devm_ioport_map(dev, base[id], IDI_48_EXTENT);
-	if (!idi48gpio->reg)
+	regs = devm_ioport_map(dev, base[id], IDI_48_EXTENT);
+	if (!regs)
+		return -ENOMEM;
+	idi48gpio->reg = regs;
+
+	map = devm_regmap_init_mmio(dev, regs, &idi48_regmap_config);
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
 		return -ENOMEM;
 
+	chip->name = name;
+	chip->status_base = IDI48_IRQ_STATUS;
+	chip->unmask_base = IDI48_IRQ_ENABLE;
+	chip->clear_on_unmask = true;
+	chip->num_regs = 1;
+	chip->irqs = idi48_regmap_irqs;
+	chip->num_irqs = ARRAY_SIZE(idi48_regmap_irqs);
+
+	err = devm_regmap_add_irq_chip(dev, map, irq[id], IRQF_SHARED, 0, chip,
+				       &chip_data);
+	if (err) {
+		dev_err(dev, "IRQ registration failed (%d)\n", err);
+		return err;
+	}
+
 	idi48gpio->chip.label = name;
 	idi48gpio->chip.parent = dev;
 	idi48gpio->chip.owner = THIS_MODULE;
@@ -271,32 +223,14 @@ static int idi_48_probe(struct device *dev, unsigned int id)
 	idi48gpio->chip.get = idi_48_gpio_get;
 	idi48gpio->chip.get_multiple = idi_48_gpio_get_multiple;
 
-	girq = &idi48gpio->chip.irq;
-	gpio_irq_chip_set_chip(girq, &idi_48_irqchip);
-	/* This will let us handle the parent IRQ in the driver */
-	girq->parent_handler = NULL;
-	girq->num_parents = 0;
-	girq->parents = NULL;
-	girq->default_type = IRQ_TYPE_NONE;
-	girq->handler = handle_edge_irq;
-	girq->init_hw = idi_48_irq_init_hw;
-
-	spin_lock_init(&idi48gpio->lock);
-
 	err = devm_gpiochip_add_data(dev, &idi48gpio->chip, idi48gpio);
 	if (err) {
 		dev_err(dev, "GPIO registering failed (%d)\n", err);
 		return err;
 	}
 
-	err = devm_request_irq(dev, irq[id], idi_48_irq_handler, IRQF_SHARED,
-		name, idi48gpio);
-	if (err) {
-		dev_err(dev, "IRQ handler registering failed (%d)\n", err);
-		return err;
-	}
-
-	return 0;
+	return gpiochip_irqchip_add_domain(&idi48gpio->chip,
+					   regmap_irq_get_domain(chip_data));
 }
 
 static struct isa_driver idi_48_driver = {
-- 
2.38.1

[PATCH v3 5/9] gpio: 104-idi-48: Migrate to gpio-regmap API

[William Breathitt Gray]

The regmap API supports IO port accessors so we can take advantage of
regmap abstractions rather than handling access to the device registers
directly in the driver. Despite the underlying interface being based on
i8255, it is simpler to use the gpio-regmap API directly because the
104-IDI-48 device features only input signals. Therefore, the dependence
on the i8255 GPIO library is removed in this patch.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/Kconfig           |  2 +-
 drivers/gpio/gpio-104-idi-48.c | 97 +++++++---------------------------
 2 files changed, 21 insertions(+), 78 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 6892979e511a..dd34039fc31b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -873,7 +873,7 @@ config GPIO_104_IDI_48
 	select ISA_BUS_API
 	select REGMAP_IRQ
 	select GPIOLIB_IRQCHIP
-	select GPIO_I8255
+	select GPIO_REGMAP
 	help
 	  Enables GPIO support for the ACCES 104-IDI-48 family (104-IDI-48A,
 	  104-IDI-48AC, 104-IDI-48B, 104-IDI-48BC). The base port addresses for
diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c
index f77c05571062..2584f411ae67 100644
--- a/drivers/gpio/gpio-104-idi-48.c
+++ b/drivers/gpio/gpio-104-idi-48.c
@@ -9,7 +9,7 @@
 #include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/err.h>
-#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
 #include <linux/interrupt.h>
 #include <linux/ioport.h>
 #include <linux/irq.h>
@@ -20,10 +20,6 @@
 #include <linux/regmap.h>
 #include <linux/types.h>
 
-#include "gpio-i8255.h"
-
-MODULE_IMPORT_NS(I8255);
-
 #define IDI_48_EXTENT 8
 #define MAX_NUM_IDI_48 max_num_isa_dev(IDI_48_EXTENT)
 
@@ -40,56 +36,17 @@ MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers");
 #define IDI48_IRQ_STATUS 0x7
 #define IDI48_IRQ_ENABLE IDI48_IRQ_STATUS
 
-/**
- * struct idi_48_reg - device register structure
- * @port0:	Port 0 Inputs
- * @unused:	Unused
- * @port1:	Port 1 Inputs
- * @irq:	Read: IRQ Status Register/IRQ Clear
- *		Write: IRQ Enable/Disable
- */
-struct idi_48_reg {
-	u8 port0[3];
-	u8 unused;
-	u8 port1[3];
-	u8 irq;
-};
-
-/**
- * struct idi_48_gpio - GPIO device private data structure
- * @chip:	instance of the gpio_chip
- * @reg:	I/O address offset for the device registers
- */
-struct idi_48_gpio {
-	struct gpio_chip chip;
-	struct idi_48_reg __iomem *reg;
-};
-
-static int idi_48_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
-{
-	return GPIO_LINE_DIRECTION_IN;
-}
-
-static int idi_48_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
-{
-	return 0;
-}
-
-static int idi_48_gpio_get(struct gpio_chip *chip, unsigned int offset)
-{
-	struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
-	void __iomem *const ppi = idi48gpio->reg;
-
-	return i8255_get(ppi, offset);
-}
-
-static int idi_48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
-	unsigned long *bits)
+static int idi_48_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
+				 unsigned int offset, unsigned int *reg,
+				 unsigned int *mask)
 {
-	struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
-	void __iomem *const ppi = idi48gpio->reg;
+	const unsigned int line = offset % 8;
+	const unsigned int stride = offset / 8;
+	const unsigned int port = (stride / 3) * 4;
+	const unsigned int port_stride = stride % 3;
 
-	i8255_get_multiple(ppi, mask, bits, chip->ngpio);
+	*reg = base + port + port_stride;
+	*mask = BIT(line);
 
 	return 0;
 }
@@ -166,18 +123,14 @@ static const char *idi48_names[IDI48_NGPIO] = {
 
 static int idi_48_probe(struct device *dev, unsigned int id)
 {
-	struct idi_48_gpio *idi48gpio;
 	const char *const name = dev_name(dev);
+	struct gpio_regmap_config config = {0};
 	void __iomem *regs;
 	struct regmap *map;
 	struct regmap_irq_chip *chip;
 	struct regmap_irq_chip_data *chip_data;
 	int err;
 
-	idi48gpio = devm_kzalloc(dev, sizeof(*idi48gpio), GFP_KERNEL);
-	if (!idi48gpio)
-		return -ENOMEM;
-
 	if (!devm_request_region(dev, base[id], IDI_48_EXTENT, name)) {
 		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
 			base[id], base[id] + IDI_48_EXTENT);
@@ -187,7 +140,6 @@ static int idi_48_probe(struct device *dev, unsigned int id)
 	regs = devm_ioport_map(dev, base[id], IDI_48_EXTENT);
 	if (!regs)
 		return -ENOMEM;
-	idi48gpio->reg = regs;
 
 	map = devm_regmap_init_mmio(dev, regs, &idi48_regmap_config);
 	if (IS_ERR(map))
@@ -212,25 +164,16 @@ static int idi_48_probe(struct device *dev, unsigned int id)
 		return err;
 	}
 
-	idi48gpio->chip.label = name;
-	idi48gpio->chip.parent = dev;
-	idi48gpio->chip.owner = THIS_MODULE;
-	idi48gpio->chip.base = -1;
-	idi48gpio->chip.ngpio = IDI48_NGPIO;
-	idi48gpio->chip.names = idi48_names;
-	idi48gpio->chip.get_direction = idi_48_gpio_get_direction;
-	idi48gpio->chip.direction_input = idi_48_gpio_direction_input;
-	idi48gpio->chip.get = idi_48_gpio_get;
-	idi48gpio->chip.get_multiple = idi_48_gpio_get_multiple;
-
-	err = devm_gpiochip_add_data(dev, &idi48gpio->chip, idi48gpio);
-	if (err) {
-		dev_err(dev, "GPIO registering failed (%d)\n", err);
-		return err;
-	}
+	config.parent = dev;
+	config.regmap = map;
+	config.ngpio = IDI48_NGPIO;
+	config.names = idi48_names;
+	config.reg_dat_base = GPIO_REGMAP_ADDR(0x0);
+	config.ngpio_per_reg = 8;
+	config.reg_mask_xlate = idi_48_reg_mask_xlate;
+	config.irq_domain = regmap_irq_get_domain(chip_data);
 
-	return gpiochip_irqchip_add_domain(&idi48gpio->chip,
-					   regmap_irq_get_domain(chip_data));
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &config));
 }
 
 static struct isa_driver idi_48_driver = {
-- 
2.38.1

[PATCH v3 6/9] gpio: i8255: Migrate to gpio-regmap API

[William Breathitt Gray]

The regmap API supports IO port accessors so we can take advantage of
regmap abstractions rather than handling access to the device registers
directly in the driver.

By leveraging the gpio-regmap API, the i8255 library is reduced to
simply a devm_i8255_regmap_register() function, a configuration
structure struct i8255_regmap_config, and a helper macro
i8255_volatile_regmap_range() provided to simplify volatile PPI register
hinting for the regmap.

Legacy functions and code will be removed once all consumers have
migrated to the new i8255 library interface.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/Kconfig      |   1 +
 drivers/gpio/gpio-i8255.c | 119 ++++++++++++++++++++++++++++++++++----
 drivers/gpio/gpio-i8255.h |  27 +++++++++
 3 files changed, 135 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index dd34039fc31b..88dfdc62992f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -831,6 +831,7 @@ menu "Port-mapped I/O GPIO drivers"
 
 config GPIO_I8255
 	tristate
+	select GPIO_REGMAP
 	help
 	  Enables support for the i8255 interface library functions. The i8255
 	  interface library provides functions to facilitate communication with
diff --git a/drivers/gpio/gpio-i8255.c b/drivers/gpio/gpio-i8255.c
index 9b97db418df1..9ecb2e9b97f9 100644
--- a/drivers/gpio/gpio-i8255.c
+++ b/drivers/gpio/gpio-i8255.c
@@ -4,23 +4,31 @@
  * Copyright (C) 2022 William Breathitt Gray
  */
 #include <linux/bitmap.h>
+#include <linux/device.h>
 #include <linux/err.h>
 #include <linux/export.h>
+#include <linux/gpio/regmap.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
 #include "gpio-i8255.h"
 
+#define I8255_NGPIO 24
+#define I8255_NGPIO_PER_REG 8
 #define I8255_CONTROL_PORTC_LOWER_DIRECTION BIT(0)
 #define I8255_CONTROL_PORTB_DIRECTION BIT(1)
 #define I8255_CONTROL_PORTC_UPPER_DIRECTION BIT(3)
 #define I8255_CONTROL_PORTA_DIRECTION BIT(4)
 #define I8255_CONTROL_MODE_SET BIT(7)
-#define I8255_PORTA 0
-#define I8255_PORTB 1
-#define I8255_PORTC 2
+#define I8255_PORTA 0x0
+#define I8255_PORTB 0x1
+#define I8255_PORTC 0x2
+#define I8255_CONTROL 0x3
+#define I8255_REG_DAT_BASE I8255_PORTA
+#define I8255_REG_DIR_IN_BASE I8255_CONTROL
 
 static int i8255_get_port(struct i8255 __iomem *const ppi,
 			  const unsigned long io_port, const unsigned long mask)
@@ -31,20 +39,19 @@ static int i8255_get_port(struct i8255 __iomem *const ppi,
 	return ioread8(&ppi[bank].port[ppi_port]) & mask;
 }
 
-static u8 i8255_direction_mask(const unsigned long offset)
+static int i8255_direction_mask(const unsigned int offset)
 {
-	const unsigned long port_offset = offset % 8;
-	const unsigned long io_port = offset / 8;
-	const unsigned long ppi_port = io_port % 3;
+	const unsigned int stride = offset / I8255_NGPIO_PER_REG;
+	const unsigned int line = offset % I8255_NGPIO_PER_REG;
 
-	switch (ppi_port) {
+	switch (stride) {
 	case I8255_PORTA:
 		return I8255_CONTROL_PORTA_DIRECTION;
 	case I8255_PORTB:
 		return I8255_CONTROL_PORTB_DIRECTION;
 	case I8255_PORTC:
 		/* Port C can be configured by nibble */
-		if (port_offset >= 4)
+		if (line >= 4)
 			return I8255_CONTROL_PORTC_UPPER_DIRECTION;
 		return I8255_CONTROL_PORTC_LOWER_DIRECTION;
 	default:
@@ -53,6 +60,49 @@ static u8 i8255_direction_mask(const unsigned long offset)
 	}
 }
 
+static int i8255_ppi_init(struct regmap *const map, const unsigned int base)
+{
+	int err;
+
+	/* Configure all ports to MODE 0 output mode */
+	err = regmap_write(map, base + I8255_CONTROL, I8255_CONTROL_MODE_SET);
+	if (err)
+		return err;
+
+	/* Initialize all GPIO to output 0 */
+	err = regmap_write(map, base + I8255_PORTA, 0x00);
+	if (err)
+		return err;
+	err = regmap_write(map, base + I8255_PORTB, 0x00);
+	if (err)
+		return err;
+	return regmap_write(map, base + I8255_PORTC, 0x00);
+}
+
+static int i8255_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
+				unsigned int offset, unsigned int *reg,
+				unsigned int *mask)
+{
+	const unsigned int ppi = offset / I8255_NGPIO;
+	const unsigned int ppi_offset = offset % I8255_NGPIO;
+	const unsigned int stride = ppi_offset / I8255_NGPIO_PER_REG;
+	const unsigned int line = ppi_offset % I8255_NGPIO_PER_REG;
+
+	switch (base) {
+	case I8255_REG_DAT_BASE:
+		*reg = base + stride + ppi * 4;
+		*mask = BIT(line);
+		return 0;
+	case I8255_REG_DIR_IN_BASE:
+		*reg = base + ppi * 4;
+		*mask = i8255_direction_mask(ppi_offset);
+		return 0;
+	default:
+		/* Should never reach this path */
+		return -EINVAL;
+	}
+}
+
 static void i8255_set_port(struct i8255 __iomem *const ppi,
 			   struct i8255_state *const state,
 			   const unsigned long io_port,
@@ -93,7 +143,7 @@ void i8255_direction_input(struct i8255 __iomem *const ppi,
 	spin_lock_irqsave(&state[bank].lock, flags);
 
 	state[bank].control_state |= I8255_CONTROL_MODE_SET;
-	state[bank].control_state |= i8255_direction_mask(offset);
+	state[bank].control_state |= i8255_direction_mask(offset % 24);
 
 	iowrite8(state[bank].control_state, &ppi[bank].control);
 
@@ -125,7 +175,7 @@ void i8255_direction_output(struct i8255 __iomem *const ppi,
 	spin_lock_irqsave(&state[bank].lock, flags);
 
 	state[bank].control_state |= I8255_CONTROL_MODE_SET;
-	state[bank].control_state &= ~i8255_direction_mask(offset);
+	state[bank].control_state &= ~i8255_direction_mask(offset % 24);
 
 	iowrite8(state[bank].control_state, &ppi[bank].control);
 
@@ -165,7 +215,7 @@ int i8255_get_direction(const struct i8255_state *const state,
 	const unsigned long io_port = offset / 8;
 	const unsigned long bank = io_port / 3;
 
-	return !!(state[bank].control_state & i8255_direction_mask(offset));
+	return !!(state[bank].control_state & i8255_direction_mask(offset % 24));
 }
 EXPORT_SYMBOL_NS_GPL(i8255_get_direction, I8255);
 
@@ -282,6 +332,51 @@ void i8255_state_init(struct i8255_state *const state,
 }
 EXPORT_SYMBOL_NS_GPL(i8255_state_init, I8255);
 
+/**
+ * devm_i8255_regmap_register - Register an i8255 GPIO controller
+ * @dev:	device that is registering this i8255 GPIO device
+ * @config:	configuration for i8255_regmap_config
+ *
+ * Registers an Intel 8255 Programmable Peripheral Interface GPIO controller.
+ * Returns 0 on success and negative error number on failure.
+ */
+int devm_i8255_regmap_register(struct device *const dev,
+			       const struct i8255_regmap_config *const config)
+{
+	struct gpio_regmap_config gpio_config = {0};
+	unsigned long i;
+	int err;
+
+	if (!config->parent)
+		return -EINVAL;
+
+	if (!config->map)
+		return -EINVAL;
+
+	if (!config->num_ppi)
+		return -EINVAL;
+
+	for (i = 0; i < config->num_ppi; i++) {
+		err = i8255_ppi_init(config->map, i * 4);
+		if (err)
+			return err;
+	}
+
+	gpio_config.parent = config->parent;
+	gpio_config.regmap = config->map;
+	gpio_config.ngpio = I8255_NGPIO * config->num_ppi;
+	gpio_config.names = config->names;
+	gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE);
+	gpio_config.reg_set_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE);
+	gpio_config.reg_dir_in_base = GPIO_REGMAP_ADDR(I8255_REG_DIR_IN_BASE);
+	gpio_config.ngpio_per_reg = I8255_NGPIO_PER_REG;
+	gpio_config.irq_domain = config->domain;
+	gpio_config.reg_mask_xlate = i8255_reg_mask_xlate;
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
+}
+EXPORT_SYMBOL_NS_GPL(devm_i8255_regmap_register, I8255);
+
 MODULE_AUTHOR("William Breathitt Gray");
 MODULE_DESCRIPTION("Intel 8255 Programmable Peripheral Interface");
 MODULE_LICENSE("GPL");
diff --git a/drivers/gpio/gpio-i8255.h b/drivers/gpio/gpio-i8255.h
index d9084aae9446..6ec987835c14 100644
--- a/drivers/gpio/gpio-i8255.h
+++ b/drivers/gpio/gpio-i8255.h
@@ -3,6 +3,9 @@
 #ifndef _I8255_H_
 #define _I8255_H_
 
+#include <linux/device.h>
+#include <linux/irqdomain.h>
+#include <linux/regmap.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
@@ -26,6 +29,30 @@ struct i8255_state {
 	u8 control_state;
 };
 
+#define i8255_volatile_regmap_range(_base) regmap_reg_range(_base, _base + 0x2)
+
+/**
+ * struct i8255_regmap_config - Configuration for the register map of an i8255
+ * @parent:	parent device
+ * @map:	regmap for the i8255
+ * @num_ppi:	number of i8255 Programmable Peripheral Interface
+ * @names:	(optional) array of names for gpios
+ * @domain:	(optional) IRQ domain if the controller is interrupt-capable
+ *
+ * Note: The regmap is expected to have cache enabled and i8255 control
+ * registers not marked as volatile.
+ */
+struct i8255_regmap_config {
+	struct device *parent;
+	struct regmap *map;
+	int num_ppi;
+	const char *const *names;
+	struct irq_domain *domain;
+};
+
+int devm_i8255_regmap_register(struct device *dev,
+			       const struct i8255_regmap_config *config);
+
 void i8255_direction_input(struct i8255 __iomem *ppi, struct i8255_state *state,
 			   unsigned long offset);
 void i8255_direction_output(struct i8255 __iomem *ppi,
-- 
2.38.1

[PATCH v3 7/9] gpio: 104-dio-48e: Migrate to regmap API

[William Breathitt Gray]

The regmap API supports IO port accessors so we can take advantage of
regmap abstractions rather than handling access to the device registers
directly in the driver. The 104-dio-48e module is migrated to the new
i8255 library interface leveraging the gpio-regmap API.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/gpio-104-dio-48e.c | 147 ++------------------------------
 1 file changed, 7 insertions(+), 140 deletions(-)

diff --git a/drivers/gpio/gpio-104-dio-48e.c b/drivers/gpio/gpio-104-dio-48e.c
index fcee3dc81902..64f4044150b7 100644
--- a/drivers/gpio/gpio-104-dio-48e.c
+++ b/drivers/gpio/gpio-104-dio-48e.c
@@ -9,7 +9,6 @@
 #include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/err.h>
-#include <linux/gpio/driver.h>
 #include <linux/ioport.h>
 #include <linux/irq.h>
 #include <linux/isa.h>
@@ -42,90 +41,6 @@ MODULE_PARM_DESC(irq, "ACCES 104-DIO-48E interrupt line numbers");
 
 #define DIO48E_NUM_PPI 2
 
-/**
- * struct dio48e_reg - device register structure
- * @ppi:		Programmable Peripheral Interface groups
- */
-struct dio48e_reg {
-	struct i8255 ppi[DIO48E_NUM_PPI];
-};
-
-/**
- * struct dio48e_gpio - GPIO device private data structure
- * @chip:		instance of the gpio_chip
- * @ppi_state:		PPI device states
- * @reg:		I/O address offset for the device registers
- */
-struct dio48e_gpio {
-	struct gpio_chip chip;
-	struct i8255_state ppi_state[DIO48E_NUM_PPI];
-	struct dio48e_reg __iomem *reg;
-};
-
-static int dio48e_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-
-	if (i8255_get_direction(dio48egpio->ppi_state, offset))
-		return GPIO_LINE_DIRECTION_IN;
-
-	return GPIO_LINE_DIRECTION_OUT;
-}
-
-static int dio48e_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-
-	i8255_direction_input(dio48egpio->reg->ppi, dio48egpio->ppi_state,
-			      offset);
-
-	return 0;
-}
-
-static int dio48e_gpio_direction_output(struct gpio_chip *chip, unsigned int offset,
-					int value)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-
-	i8255_direction_output(dio48egpio->reg->ppi, dio48egpio->ppi_state,
-			       offset, value);
-
-	return 0;
-}
-
-static int dio48e_gpio_get(struct gpio_chip *chip, unsigned int offset)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-
-	return i8255_get(dio48egpio->reg->ppi, offset);
-}
-
-static int dio48e_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
-	unsigned long *bits)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-
-	i8255_get_multiple(dio48egpio->reg->ppi, mask, bits, chip->ngpio);
-
-	return 0;
-}
-
-static void dio48e_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-
-	i8255_set(dio48egpio->reg->ppi, dio48egpio->ppi_state, offset, value);
-}
-
-static void dio48e_gpio_set_multiple(struct gpio_chip *chip,
-	unsigned long *mask, unsigned long *bits)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
-
-	i8255_set_multiple(dio48egpio->reg->ppi, dio48egpio->ppi_state, mask,
-			   bits, chip->ngpio);
-}
-
 static const struct regmap_range dio48e_wr_ranges[] = {
 	regmap_reg_range(0x0, 0x9), regmap_reg_range(0xB, 0xB),
 	regmap_reg_range(0xD, 0xD), regmap_reg_range(0xF, 0xF),
@@ -237,35 +152,10 @@ static const char *dio48e_names[DIO48E_NGPIO] = {
 	"PPI Group 1 Port C 5", "PPI Group 1 Port C 6", "PPI Group 1 Port C 7"
 };
 
-static int dio48e_irq_init_hw(struct gpio_chip *gc)
-{
-	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(gc);
-
-	/* Disable IRQ by default */
-	ioread8(&dio48egpio->reg->enable_interrupt);
-
-	return 0;
-}
-
-static void dio48e_init_ppi(struct i8255 __iomem *const ppi,
-			    struct i8255_state *const ppi_state)
-{
-	const unsigned long ngpio = 24;
-	const unsigned long mask = GENMASK(ngpio - 1, 0);
-	const unsigned long bits = 0;
-	unsigned long i;
-
-	/* Initialize all GPIO to output 0 */
-	for (i = 0; i < DIO48E_NUM_PPI; i++) {
-		i8255_mode0_output(&ppi[i]);
-		i8255_set_multiple(&ppi[i], &ppi_state[i], &mask, &bits, ngpio);
-	}
-}
-
 static int dio48e_probe(struct device *dev, unsigned int id)
 {
-	struct dio48e_gpio *dio48egpio;
 	const char *const name = dev_name(dev);
+	struct i8255_regmap_config config = {0};
 	void __iomem *regs;
 	struct regmap *map;
 	unsigned int val;
@@ -274,10 +164,6 @@ static int dio48e_probe(struct device *dev, unsigned int id)
 	unsigned int irq_mask;
 	struct regmap_irq_chip_data *chip_data;
 
-	dio48egpio = devm_kzalloc(dev, sizeof(*dio48egpio), GFP_KERNEL);
-	if (!dio48egpio)
-		return -ENOMEM;
-
 	if (!devm_request_region(dev, base[id], DIO48E_EXTENT, name)) {
 		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
 			base[id], base[id] + DIO48E_EXTENT);
@@ -287,7 +173,6 @@ static int dio48e_probe(struct device *dev, unsigned int id)
 	regs = devm_ioport_map(dev, base[id], DIO48E_EXTENT);
 	if (!regs)
 		return -ENOMEM;
-	dio48egpio->reg = regs;
 
 	map = devm_regmap_init_mmio(dev, regs, &dio48e_regmap_config);
 	if (IS_ERR(map))
@@ -324,31 +209,13 @@ static int dio48e_probe(struct device *dev, unsigned int id)
 		return err;
 	}
 
-	dio48egpio->chip.label = name;
-	dio48egpio->chip.parent = dev;
-	dio48egpio->chip.owner = THIS_MODULE;
-	dio48egpio->chip.base = -1;
-	dio48egpio->chip.ngpio = DIO48E_NGPIO;
-	dio48egpio->chip.names = dio48e_names;
-	dio48egpio->chip.get_direction = dio48e_gpio_get_direction;
-	dio48egpio->chip.direction_input = dio48e_gpio_direction_input;
-	dio48egpio->chip.direction_output = dio48e_gpio_direction_output;
-	dio48egpio->chip.get = dio48e_gpio_get;
-	dio48egpio->chip.get_multiple = dio48e_gpio_get_multiple;
-	dio48egpio->chip.set = dio48e_gpio_set;
-	dio48egpio->chip.set_multiple = dio48e_gpio_set_multiple;
-
-	i8255_state_init(dio48egpio->ppi_state, DIO48E_NUM_PPI);
-	dio48e_init_ppi(dio48egpio->reg->ppi, dio48egpio->ppi_state);
-
-	err = devm_gpiochip_add_data(dev, &dio48egpio->chip, dio48egpio);
-	if (err) {
-		dev_err(dev, "GPIO registering failed (%d)\n", err);
-		return err;
-	}
+	config.parent = dev;
+	config.map = map;
+	config.num_ppi = DIO48E_NUM_PPI;
+	config.names = dio48e_names;
+	config.domain = regmap_irq_get_domain(chip_data);
 
-	return gpiochip_irqchip_add_domain(&dio48egpio->chip,
-					   regmap_irq_get_domain(chip_data));
+	return devm_i8255_regmap_register(dev, &config);
 }
 
 static struct isa_driver dio48e_driver = {
-- 
2.38.1

[PATCH v3 8/9] gpio: gpio-mm: Migrate to regmap API

[William Breathitt Gray]

The regmap API supports IO port accessors so we can take advantage of
regmap abstractions rather than handling access to the device registers
directly in the driver. The gpio-mm module is migrated to the new i8255
library interface leveraging the gpio-regmap API.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/gpio-gpio-mm.c | 153 +++++++-----------------------------
 1 file changed, 29 insertions(+), 124 deletions(-)

diff --git a/drivers/gpio/gpio-gpio-mm.c b/drivers/gpio/gpio-gpio-mm.c
index 2689671b6b01..ba8847485660 100644
--- a/drivers/gpio/gpio-gpio-mm.c
+++ b/drivers/gpio/gpio-gpio-mm.c
@@ -8,13 +8,13 @@
  */
 #include <linux/device.h>
 #include <linux/errno.h>
-#include <linux/gpio/driver.h>
-#include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/isa.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
 
 #include "gpio-i8255.h"
 
@@ -30,83 +30,22 @@ MODULE_PARM_DESC(base, "Diamond Systems GPIO-MM base addresses");
 
 #define GPIOMM_NUM_PPI 2
 
-/**
- * struct gpiomm_gpio - GPIO device private data structure
- * @chip:		instance of the gpio_chip
- * @ppi_state:		Programmable Peripheral Interface group states
- * @ppi:		Programmable Peripheral Interface groups
- */
-struct gpiomm_gpio {
-	struct gpio_chip chip;
-	struct i8255_state ppi_state[GPIOMM_NUM_PPI];
-	struct i8255 __iomem *ppi;
+static const struct regmap_range gpiomm_volatile_ranges[] = {
+	i8255_volatile_regmap_range(0x0), i8255_volatile_regmap_range(0x4),
+};
+static const struct regmap_access_table gpiomm_volatile_table = {
+	.yes_ranges = gpiomm_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(gpiomm_volatile_ranges),
+};
+static const struct regmap_config gpiomm_regmap_config = {
+	.reg_bits = 8,
+	.reg_stride = 1,
+	.val_bits = 8,
+	.io_port = true,
+	.max_register = 0x7,
+	.volatile_table = &gpiomm_volatile_table,
+	.cache_type = REGCACHE_FLAT,
 };
-
-static int gpiomm_gpio_get_direction(struct gpio_chip *chip,
-	unsigned int offset)
-{
-	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
-
-	if (i8255_get_direction(gpiommgpio->ppi_state, offset))
-		return GPIO_LINE_DIRECTION_IN;
-
-	return GPIO_LINE_DIRECTION_OUT;
-}
-
-static int gpiomm_gpio_direction_input(struct gpio_chip *chip,
-	unsigned int offset)
-{
-	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
-
-	i8255_direction_input(gpiommgpio->ppi, gpiommgpio->ppi_state, offset);
-
-	return 0;
-}
-
-static int gpiomm_gpio_direction_output(struct gpio_chip *chip,
-	unsigned int offset, int value)
-{
-	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
-
-	i8255_direction_output(gpiommgpio->ppi, gpiommgpio->ppi_state, offset,
-			       value);
-
-	return 0;
-}
-
-static int gpiomm_gpio_get(struct gpio_chip *chip, unsigned int offset)
-{
-	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
-
-	return i8255_get(gpiommgpio->ppi, offset);
-}
-
-static int gpiomm_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
-	unsigned long *bits)
-{
-	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
-
-	i8255_get_multiple(gpiommgpio->ppi, mask, bits, chip->ngpio);
-
-	return 0;
-}
-
-static void gpiomm_gpio_set(struct gpio_chip *chip, unsigned int offset,
-	int value)
-{
-	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
-
-	i8255_set(gpiommgpio->ppi, gpiommgpio->ppi_state, offset, value);
-}
-
-static void gpiomm_gpio_set_multiple(struct gpio_chip *chip,
-	unsigned long *mask, unsigned long *bits)
-{
-	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
-
-	i8255_set_multiple(gpiommgpio->ppi, gpiommgpio->ppi_state, mask, bits,
-			   chip->ngpio);
-}
 
 #define GPIOMM_NGPIO 48
 static const char *gpiomm_names[GPIOMM_NGPIO] = {
@@ -120,30 +59,11 @@ static const char *gpiomm_names[GPIOMM_NGPIO] = {
 	"Port 2C2", "Port 2C3", "Port 2C4", "Port 2C5", "Port 2C6", "Port 2C7",
 };
 
-static void gpiomm_init_dio(struct i8255 __iomem *const ppi,
-			    struct i8255_state *const ppi_state)
-{
-	const unsigned long ngpio = 24;
-	const unsigned long mask = GENMASK(ngpio - 1, 0);
-	const unsigned long bits = 0;
-	unsigned long i;
-
-	/* Initialize all GPIO to output 0 */
-	for (i = 0; i < GPIOMM_NUM_PPI; i++) {
-		i8255_mode0_output(&ppi[i]);
-		i8255_set_multiple(&ppi[i], &ppi_state[i], &mask, &bits, ngpio);
-	}
-}
-
 static int gpiomm_probe(struct device *dev, unsigned int id)
 {
-	struct gpiomm_gpio *gpiommgpio;
 	const char *const name = dev_name(dev);
-	int err;
-
-	gpiommgpio = devm_kzalloc(dev, sizeof(*gpiommgpio), GFP_KERNEL);
-	if (!gpiommgpio)
-		return -ENOMEM;
+	struct i8255_regmap_config config = {0};
+	void __iomem *regs;
 
 	if (!devm_request_region(dev, base[id], GPIOMM_EXTENT, name)) {
 		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
@@ -151,34 +71,19 @@ static int gpiomm_probe(struct device *dev, unsigned int id)
 		return -EBUSY;
 	}
 
-	gpiommgpio->ppi = devm_ioport_map(dev, base[id], GPIOMM_EXTENT);
-	if (!gpiommgpio->ppi)
+	regs = devm_ioport_map(dev, base[id], GPIOMM_EXTENT);
+	if (!regs)
 		return -ENOMEM;
 
-	gpiommgpio->chip.label = name;
-	gpiommgpio->chip.parent = dev;
-	gpiommgpio->chip.owner = THIS_MODULE;
-	gpiommgpio->chip.base = -1;
-	gpiommgpio->chip.ngpio = GPIOMM_NGPIO;
-	gpiommgpio->chip.names = gpiomm_names;
-	gpiommgpio->chip.get_direction = gpiomm_gpio_get_direction;
-	gpiommgpio->chip.direction_input = gpiomm_gpio_direction_input;
-	gpiommgpio->chip.direction_output = gpiomm_gpio_direction_output;
-	gpiommgpio->chip.get = gpiomm_gpio_get;
-	gpiommgpio->chip.get_multiple = gpiomm_gpio_get_multiple;
-	gpiommgpio->chip.set = gpiomm_gpio_set;
-	gpiommgpio->chip.set_multiple = gpiomm_gpio_set_multiple;
-
-	i8255_state_init(gpiommgpio->ppi_state, GPIOMM_NUM_PPI);
-	gpiomm_init_dio(gpiommgpio->ppi, gpiommgpio->ppi_state);
-
-	err = devm_gpiochip_add_data(dev, &gpiommgpio->chip, gpiommgpio);
-	if (err) {
-		dev_err(dev, "GPIO registering failed (%d)\n", err);
-		return err;
-	}
+	config.map = devm_regmap_init_mmio(dev, regs, &gpiomm_regmap_config);
+	if (IS_ERR(config.map))
+		return PTR_ERR(config.map);
+
+	config.parent = dev;
+	config.num_ppi = GPIOMM_NUM_PPI;
+	config.names = gpiomm_names;
 
-	return 0;
+	return devm_i8255_regmap_register(dev, &config);
 }
 
 static struct isa_driver gpiomm_driver = {
-- 
2.38.1

[PATCH v3 9/9] gpio: i8255: Remove unused legacy interface

[William Breathitt Gray]

All i8255 library consumers have migrated to the new interface
leveraging the gpio-regmap API. Legacy interface functions and code are
removed as no longer needed.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/gpio/gpio-i8255.c | 243 +-------------------------------------
 drivers/gpio/gpio-i8255.h |  39 ------
 2 files changed, 1 insertion(+), 281 deletions(-)

diff --git a/drivers/gpio/gpio-i8255.c b/drivers/gpio/gpio-i8255.c
index 9ecb2e9b97f9..64ab80fc4a1e 100644
--- a/drivers/gpio/gpio-i8255.c
+++ b/drivers/gpio/gpio-i8255.c
@@ -3,16 +3,13 @@
  * Intel 8255 Programmable Peripheral Interface
  * Copyright (C) 2022 William Breathitt Gray
  */
-#include <linux/bitmap.h>
+#include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/export.h>
 #include <linux/gpio/regmap.h>
-#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
 
 #include "gpio-i8255.h"
 
@@ -30,15 +27,6 @@
 #define I8255_REG_DAT_BASE I8255_PORTA
 #define I8255_REG_DIR_IN_BASE I8255_CONTROL
 
-static int i8255_get_port(struct i8255 __iomem *const ppi,
-			  const unsigned long io_port, const unsigned long mask)
-{
-	const unsigned long bank = io_port / 3;
-	const unsigned long ppi_port = io_port % 3;
-
-	return ioread8(&ppi[bank].port[ppi_port]) & mask;
-}
-
 static int i8255_direction_mask(const unsigned int offset)
 {
 	const unsigned int stride = offset / I8255_NGPIO_PER_REG;
@@ -103,235 +91,6 @@ static int i8255_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
 	}
 }
 
-static void i8255_set_port(struct i8255 __iomem *const ppi,
-			   struct i8255_state *const state,
-			   const unsigned long io_port,
-			   const unsigned long mask, const unsigned long bits)
-{
-	const unsigned long bank = io_port / 3;
-	const unsigned long ppi_port = io_port % 3;
-	unsigned long flags;
-	unsigned long out_state;
-
-	spin_lock_irqsave(&state[bank].lock, flags);
-
-	out_state = ioread8(&ppi[bank].port[ppi_port]);
-	out_state = (out_state & ~mask) | (bits & mask);
-	iowrite8(out_state, &ppi[bank].port[ppi_port]);
-
-	spin_unlock_irqrestore(&state[bank].lock, flags);
-}
-
-/**
- * i8255_direction_input - configure signal offset as input
- * @ppi:	Intel 8255 Programmable Peripheral Interface banks
- * @state:	devices states of the respective PPI banks
- * @offset:	signal offset to configure as input
- *
- * Configures a signal @offset as input for the respective Intel 8255
- * Programmable Peripheral Interface (@ppi) banks. The @state control_state
- * values are updated to reflect the new configuration.
- */
-void i8255_direction_input(struct i8255 __iomem *const ppi,
-			   struct i8255_state *const state,
-			   const unsigned long offset)
-{
-	const unsigned long io_port = offset / 8;
-	const unsigned long bank = io_port / 3;
-	unsigned long flags;
-
-	spin_lock_irqsave(&state[bank].lock, flags);
-
-	state[bank].control_state |= I8255_CONTROL_MODE_SET;
-	state[bank].control_state |= i8255_direction_mask(offset % 24);
-
-	iowrite8(state[bank].control_state, &ppi[bank].control);
-
-	spin_unlock_irqrestore(&state[bank].lock, flags);
-}
-EXPORT_SYMBOL_NS_GPL(i8255_direction_input, I8255);
-
-/**
- * i8255_direction_output - configure signal offset as output
- * @ppi:	Intel 8255 Programmable Peripheral Interface banks
- * @state:	devices states of the respective PPI banks
- * @offset:	signal offset to configure as output
- * @value:	signal value to output
- *
- * Configures a signal @offset as output for the respective Intel 8255
- * Programmable Peripheral Interface (@ppi) banks and sets the respective signal
- * output to the desired @value. The @state control_state values are updated to
- * reflect the new configuration.
- */
-void i8255_direction_output(struct i8255 __iomem *const ppi,
-			    struct i8255_state *const state,
-			    const unsigned long offset,
-			    const unsigned long value)
-{
-	const unsigned long io_port = offset / 8;
-	const unsigned long bank = io_port / 3;
-	unsigned long flags;
-
-	spin_lock_irqsave(&state[bank].lock, flags);
-
-	state[bank].control_state |= I8255_CONTROL_MODE_SET;
-	state[bank].control_state &= ~i8255_direction_mask(offset % 24);
-
-	iowrite8(state[bank].control_state, &ppi[bank].control);
-
-	spin_unlock_irqrestore(&state[bank].lock, flags);
-
-	i8255_set(ppi, state, offset, value);
-}
-EXPORT_SYMBOL_NS_GPL(i8255_direction_output, I8255);
-
-/**
- * i8255_get - get signal value at signal offset
- * @ppi:	Intel 8255 Programmable Peripheral Interface banks
- * @offset:	offset of signal to get
- *
- * Returns the signal value (0=low, 1=high) for the signal at @offset for the
- * respective Intel 8255 Programmable Peripheral Interface (@ppi) banks.
- */
-int i8255_get(struct i8255 __iomem *const ppi, const unsigned long offset)
-{
-	const unsigned long io_port = offset / 8;
-	const unsigned long offset_mask = BIT(offset % 8);
-
-	return !!i8255_get_port(ppi, io_port, offset_mask);
-}
-EXPORT_SYMBOL_NS_GPL(i8255_get, I8255);
-
-/**
- * i8255_get_direction - get the I/O direction for a signal offset
- * @state:	devices states of the respective PPI banks
- * @offset:	offset of signal to get direction
- *
- * Returns the signal direction (0=output, 1=input) for the signal at @offset.
- */
-int i8255_get_direction(const struct i8255_state *const state,
-			const unsigned long offset)
-{
-	const unsigned long io_port = offset / 8;
-	const unsigned long bank = io_port / 3;
-
-	return !!(state[bank].control_state & i8255_direction_mask(offset % 24));
-}
-EXPORT_SYMBOL_NS_GPL(i8255_get_direction, I8255);
-
-/**
- * i8255_get_multiple - get multiple signal values at multiple signal offsets
- * @ppi:	Intel 8255 Programmable Peripheral Interface banks
- * @mask:	mask of signals to get
- * @bits:	bitmap to store signal values
- * @ngpio:	number of GPIO signals of the respective PPI banks
- *
- * Stores in @bits the values (0=low, 1=high) for the signals defined by @mask
- * for the respective Intel 8255 Programmable Peripheral Interface (@ppi) banks.
- */
-void i8255_get_multiple(struct i8255 __iomem *const ppi,
-			const unsigned long *const mask,
-			unsigned long *const bits, const unsigned long ngpio)
-{
-	unsigned long offset;
-	unsigned long port_mask;
-	unsigned long io_port;
-	unsigned long port_state;
-
-	bitmap_zero(bits, ngpio);
-
-	for_each_set_clump8(offset, port_mask, mask, ngpio) {
-		io_port = offset / 8;
-		port_state = i8255_get_port(ppi, io_port, port_mask);
-
-		bitmap_set_value8(bits, port_state, offset);
-	}
-}
-EXPORT_SYMBOL_NS_GPL(i8255_get_multiple, I8255);
-
-/**
- * i8255_mode0_output - configure all PPI ports to MODE 0 output mode
- * @ppi:	Intel 8255 Programmable Peripheral Interface bank
- *
- * Configures all Intel 8255 Programmable Peripheral Interface (@ppi) ports to
- * MODE 0 (Basic Input/Output) output mode.
- */
-void i8255_mode0_output(struct i8255 __iomem *const ppi)
-{
-	iowrite8(I8255_CONTROL_MODE_SET, &ppi->control);
-}
-EXPORT_SYMBOL_NS_GPL(i8255_mode0_output, I8255);
-
-/**
- * i8255_set - set signal value at signal offset
- * @ppi:	Intel 8255 Programmable Peripheral Interface banks
- * @state:	devices states of the respective PPI banks
- * @offset:	offset of signal to set
- * @value:	value of signal to set
- *
- * Assigns output @value for the signal at @offset for the respective Intel 8255
- * Programmable Peripheral Interface (@ppi) banks.
- */
-void i8255_set(struct i8255 __iomem *const ppi, struct i8255_state *const state,
-	       const unsigned long offset, const unsigned long value)
-{
-	const unsigned long io_port = offset / 8;
-	const unsigned long port_offset = offset % 8;
-	const unsigned long mask = BIT(port_offset);
-	const unsigned long bits = value << port_offset;
-
-	i8255_set_port(ppi, state, io_port, mask, bits);
-}
-EXPORT_SYMBOL_NS_GPL(i8255_set, I8255);
-
-/**
- * i8255_set_multiple - set signal values at multiple signal offsets
- * @ppi:	Intel 8255 Programmable Peripheral Interface banks
- * @state:	devices states of the respective PPI banks
- * @mask:	mask of signals to set
- * @bits:	bitmap of signal output values
- * @ngpio:	number of GPIO signals of the respective PPI banks
- *
- * Assigns output values defined by @bits for the signals defined by @mask for
- * the respective Intel 8255 Programmable Peripheral Interface (@ppi) banks.
- */
-void i8255_set_multiple(struct i8255 __iomem *const ppi,
-			struct i8255_state *const state,
-			const unsigned long *const mask,
-			const unsigned long *const bits,
-			const unsigned long ngpio)
-{
-	unsigned long offset;
-	unsigned long port_mask;
-	unsigned long io_port;
-	unsigned long value;
-
-	for_each_set_clump8(offset, port_mask, mask, ngpio) {
-		io_port = offset / 8;
-		value = bitmap_get_value8(bits, offset);
-		i8255_set_port(ppi, state, io_port, port_mask, value);
-	}
-}
-EXPORT_SYMBOL_NS_GPL(i8255_set_multiple, I8255);
-
-/**
- * i8255_state_init - initialize i8255_state structure
- * @state:	devices states of the respective PPI banks
- * @nbanks:	number of Intel 8255 Programmable Peripheral Interface banks
- *
- * Initializes the @state of each Intel 8255 Programmable Peripheral Interface
- * bank for use in i8255 library functions.
- */
-void i8255_state_init(struct i8255_state *const state,
-		      const unsigned long nbanks)
-{
-	unsigned long bank;
-
-	for (bank = 0; bank < nbanks; bank++)
-		spin_lock_init(&state[bank].lock);
-}
-EXPORT_SYMBOL_NS_GPL(i8255_state_init, I8255);
-
 /**
  * devm_i8255_regmap_register - Register an i8255 GPIO controller
  * @dev:	device that is registering this i8255 GPIO device
diff --git a/drivers/gpio/gpio-i8255.h b/drivers/gpio/gpio-i8255.h
index 6ec987835c14..bc3023745e7b 100644
--- a/drivers/gpio/gpio-i8255.h
+++ b/drivers/gpio/gpio-i8255.h
@@ -6,28 +6,6 @@
 #include <linux/device.h>
 #include <linux/irqdomain.h>
 #include <linux/regmap.h>
-#include <linux/spinlock.h>
-#include <linux/types.h>
-
-/**
- * struct i8255 - Intel 8255 register structure
- * @port:	Port A, B, and C
- * @control:	Control register
- */
-struct i8255 {
-	u8 port[3];
-	u8 control;
-};
-
-/**
- * struct i8255_state - Intel 8255 state structure
- * @lock:		synchronization lock for accessing device state
- * @control_state:	Control register state
- */
-struct i8255_state {
-	spinlock_t lock;
-	u8 control_state;
-};
 
 #define i8255_volatile_regmap_range(_base) regmap_reg_range(_base, _base + 0x2)
 
@@ -53,21 +31,4 @@ struct i8255_regmap_config {
 int devm_i8255_regmap_register(struct device *dev,
 			       const struct i8255_regmap_config *config);
 
-void i8255_direction_input(struct i8255 __iomem *ppi, struct i8255_state *state,
-			   unsigned long offset);
-void i8255_direction_output(struct i8255 __iomem *ppi,
-			    struct i8255_state *state, unsigned long offset,
-			    unsigned long value);
-int i8255_get(struct i8255 __iomem *ppi, unsigned long offset);
-int i8255_get_direction(const struct i8255_state *state, unsigned long offset);
-void i8255_get_multiple(struct i8255 __iomem *ppi, const unsigned long *mask,
-			unsigned long *bits, unsigned long ngpio);
-void i8255_mode0_output(struct i8255 __iomem *const ppi);
-void i8255_set(struct i8255 __iomem *ppi, struct i8255_state *state,
-	       unsigned long offset, unsigned long value);
-void i8255_set_multiple(struct i8255 __iomem *ppi, struct i8255_state *state,
-			const unsigned long *mask, const unsigned long *bits,
-			unsigned long ngpio);
-void i8255_state_init(struct i8255_state *const state, unsigned long nbanks);
-
 #endif /* _I8255_H_ */
-- 
2.38.1

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

On Wed, Nov 23, 2022 at 05:01:53PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 22, 2022 at 02:11:00AM -0500, William Breathitt Gray wrote:
> > +	/* Initialize device interrupt state */
> > +	err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
> > +	if (err)
> > +		return err;
> 
> Use ->init_hw() callback for this.

In a subsequent patch 7/9 we remove direct gpio_chip registration in
favor of the i8255 library registration via gpio_regmap. It doesn't look
like gpio_regmap_register() sets the init_hw() callback.

Michael, do you see any issues if I introduce init_hw() to
gpio_regmap_config? Or do you think this IRQ initialization belongs
somewhere else?

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 6/9] gpio: i8255: Migrate to gpio-regmap API

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On Wed, Nov 23, 2022 at 07:42:24PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 22, 2022 at 02:11:03AM -0500, William Breathitt Gray wrote:
> > +/**
> > + * struct i8255_regmap_config - Configuration for the register map of an i8255
> > + * @parent:	parent device
> > + * @map:	regmap for the i8255
> > + * @num_ppi:	number of i8255 Programmable Peripheral Interface
> > + * @names:	(optional) array of names for gpios
> > + * @domain:	(optional) IRQ domain if the controller is interrupt-capable
> 
> > + * Note: The regmap is expected to have cache enabled and i8255 control
> > + * registers not marked as volatile.
> 
> Have you considered to catch wrong configurations by BUILD_BUG_ON() /
> static_assert() / another means of validation?

Ideally, I'd like to check for these configurations, but struct regmap
is an opaque type (the definition is in drivers/base/regmap/internal.h).
Do you know if there is some way to query a struct regmap for whether a
particular register has cache enabled or is marked as volatile?

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Andy Shevchenko]

On Tue, Nov 22, 2022 at 02:11:00AM -0500, William Breathitt Gray wrote:
> The regmap API supports IO port accessors so we can take advantage of
> regmap abstractions rather than handling access to the device registers
> directly in the driver.
> 
> For the 104-dio-48e we have the following IRQ registers (0xB and 0xF):
> 
>     Base Address +B (Write): Enable Interrupt
>     Base Address +B (Read): Disable Interrupt
>     Base Address +F (Read/Write): Clear Interrupt
> 
> Any write to 0xB will enable interrupts, while any read will disable
> interrupts. Interrupts are cleared by a read or any write to 0xF.
> There's no IRQ status register, so software has to assume that if an
> interrupt is raised then it was for the 104-DIO-48E device.

...

> +/* only bit 3 on each respective Port C supports interrupts */
> +#define DIO48E_REGMAP_IRQ(_ppi) \
> +	[19 + (_ppi) * 24] = { \
> +		.mask = BIT(_ppi), \
> +		.type = { .types_supported = IRQ_TYPE_EDGE_RISING, }, \

When {} on a single line, the trailing comma is not needed.

		.type = { .types_supported = IRQ_TYPE_EDGE_RISING }, \

would work as well.

A nit: I would put \ on the same column by using TABs before each of them.

>  	}

...

> +	/* if all previously masked, enable interrupts when unmasking */
> +	if (prev_mask == all_masked) {
> +		err = regmap_write(map, DIO48E_ENABLE_INTERRUPT, 0x00);
> +		if (err)
> +			return err;
> +	/* if all are currently masked, disable interrupts */
> +	} else if (mask_buf == all_masked) {
> +		err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
> +		if (err)
> +			return err;
> +	}

Haven't looked at the rest of the series, but if there is nothing with this
code piece, the above can be optimized to

	if (prev_mask == all_masked)
		return regmap_write(map, DIO48E_ENABLE_INTERRUPT, 0x00);

	if (mask_buf == all_masked)
		return regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);

...

> +	/* Initialize device interrupt state */
> +	err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
> +	if (err)
> +		return err;

Use ->init_hw() callback for this.

...

> +	err = devm_regmap_add_irq_chip(dev, map, irq[id], 0, 0, chip,
> +				       &chip_data);

I would leave this on one line. It's only 82.

> +	if (err) {
> +		dev_err(dev, "IRQ registration failed (%d)\n", err);
> +		return err;
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/9] gpio: 104-idi-48: Migrate to the regmap-irq API

[Andy Shevchenko]

On Tue, Nov 22, 2022 at 02:11:01AM -0500, William Breathitt Gray wrote:
> The regmap API supports IO port accessors so we can take advantage of
> regmap abstractions rather than handling access to the device registers
> directly in the driver.
> 
> For the 104-idi-48, we get an IRQ register with some status information
> and basic masking, but it's broken down by banks rather than individual
> GPIO. There are six banks (8 GPIO lines each) that correspond to the
> lower six bits of the IRQ register (bits 0-5):
> 
>     Base Address + 7 (Read): IRQ Status Register/IRQ Clear
>         Bit 0-5: Respective Bank IRQ Statuses
>         Bit 6: IRQ Status (Active Low)
>         Bit 7: IRQ Enable Status
>     Base Address + 7 (Write): IRQ Enable/Disable
>         Bit 0-5: Respective Bank IRQ Enable/Disable

> Cc: Mark Brown <broonie@kernel.org>

Hint: you may use --cc parameter to `git send-email ...` to Cc additional people.

...

> +#define IDI48_REGMAP_IRQ(_id) \
> +	[_id] = { \
> +		.mask = BIT((_id) / 8), \
> +		.type = { .types_supported = IRQ_TYPE_EDGE_BOTH, }, \
>  	}

Same comments as per previous patch.

...

> +static const struct regmap_irq idi48_regmap_irqs[IDI48_NGPIO] = {
> +	IDI48_REGMAP_IRQ(0), IDI48_REGMAP_IRQ(1), IDI48_REGMAP_IRQ(2),
> +	IDI48_REGMAP_IRQ(3), IDI48_REGMAP_IRQ(4), IDI48_REGMAP_IRQ(5),
> +	IDI48_REGMAP_IRQ(6), IDI48_REGMAP_IRQ(7), IDI48_REGMAP_IRQ(8),
> +	IDI48_REGMAP_IRQ(9), IDI48_REGMAP_IRQ(10), IDI48_REGMAP_IRQ(11),
> +	IDI48_REGMAP_IRQ(12), IDI48_REGMAP_IRQ(13), IDI48_REGMAP_IRQ(14),
> +	IDI48_REGMAP_IRQ(15), IDI48_REGMAP_IRQ(16), IDI48_REGMAP_IRQ(17),
> +	IDI48_REGMAP_IRQ(18), IDI48_REGMAP_IRQ(19), IDI48_REGMAP_IRQ(20),
> +	IDI48_REGMAP_IRQ(21), IDI48_REGMAP_IRQ(22), IDI48_REGMAP_IRQ(23),
> +	IDI48_REGMAP_IRQ(24), IDI48_REGMAP_IRQ(25), IDI48_REGMAP_IRQ(26),
> +	IDI48_REGMAP_IRQ(27), IDI48_REGMAP_IRQ(28), IDI48_REGMAP_IRQ(29),
> +	IDI48_REGMAP_IRQ(30), IDI48_REGMAP_IRQ(31), IDI48_REGMAP_IRQ(32),
> +	IDI48_REGMAP_IRQ(33), IDI48_REGMAP_IRQ(34), IDI48_REGMAP_IRQ(35),
> +	IDI48_REGMAP_IRQ(36), IDI48_REGMAP_IRQ(37), IDI48_REGMAP_IRQ(38),
> +	IDI48_REGMAP_IRQ(39), IDI48_REGMAP_IRQ(40), IDI48_REGMAP_IRQ(41),
> +	IDI48_REGMAP_IRQ(42), IDI48_REGMAP_IRQ(43), IDI48_REGMAP_IRQ(44),
> +	IDI48_REGMAP_IRQ(45), IDI48_REGMAP_IRQ(46), IDI48_REGMAP_IRQ(47),

Perhaps here would be nice to have it like

	IDI48_REGMAP_IRQ(45), IDI48_REGMAP_IRQ(46), IDI48_REGMAP_IRQ(47), /* 45-47 */

?

> +};

...

> +	err = devm_regmap_add_irq_chip(dev, map, irq[id], IRQF_SHARED, 0, chip,
> +				       &chip_data);
> +	if (err) {

> +		dev_err(dev, "IRQ registration failed (%d)\n", err);
> +		return err;

I'm wondering if you plan to switch to dev_err_probe() at some point.

> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 5/9] gpio: 104-idi-48: Migrate to gpio-regmap API

[Andy Shevchenko]

On Tue, Nov 22, 2022 at 02:11:02AM -0500, William Breathitt Gray wrote:
> The regmap API supports IO port accessors so we can take advantage of
> regmap abstractions rather than handling access to the device registers
> directly in the driver. Despite the underlying interface being based on
> i8255, it is simpler to use the gpio-regmap API directly because the
> 104-IDI-48 device features only input signals. Therefore, the dependence
> on the i8255 GPIO library is removed in this patch.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(One nit-pick below)

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
> ---
>  drivers/gpio/Kconfig           |  2 +-
>  drivers/gpio/gpio-104-idi-48.c | 97 +++++++---------------------------
>  2 files changed, 21 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 6892979e511a..dd34039fc31b 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -873,7 +873,7 @@ config GPIO_104_IDI_48
>  	select ISA_BUS_API
>  	select REGMAP_IRQ
>  	select GPIOLIB_IRQCHIP
> -	select GPIO_I8255
> +	select GPIO_REGMAP
>  	help
>  	  Enables GPIO support for the ACCES 104-IDI-48 family (104-IDI-48A,
>  	  104-IDI-48AC, 104-IDI-48B, 104-IDI-48BC). The base port addresses for
> diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c
> index f77c05571062..2584f411ae67 100644
> --- a/drivers/gpio/gpio-104-idi-48.c
> +++ b/drivers/gpio/gpio-104-idi-48.c
> @@ -9,7 +9,7 @@
>  #include <linux/bits.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> -#include <linux/gpio/driver.h>
> +#include <linux/gpio/regmap.h>
>  #include <linux/interrupt.h>
>  #include <linux/ioport.h>
>  #include <linux/irq.h>
> @@ -20,10 +20,6 @@
>  #include <linux/regmap.h>
>  #include <linux/types.h>
>  
> -#include "gpio-i8255.h"
> -
> -MODULE_IMPORT_NS(I8255);
> -
>  #define IDI_48_EXTENT 8
>  #define MAX_NUM_IDI_48 max_num_isa_dev(IDI_48_EXTENT)
>  
> @@ -40,56 +36,17 @@ MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers");
>  #define IDI48_IRQ_STATUS 0x7
>  #define IDI48_IRQ_ENABLE IDI48_IRQ_STATUS
>  
> -/**
> - * struct idi_48_reg - device register structure
> - * @port0:	Port 0 Inputs
> - * @unused:	Unused
> - * @port1:	Port 1 Inputs
> - * @irq:	Read: IRQ Status Register/IRQ Clear
> - *		Write: IRQ Enable/Disable
> - */
> -struct idi_48_reg {
> -	u8 port0[3];
> -	u8 unused;
> -	u8 port1[3];
> -	u8 irq;
> -};
> -
> -/**
> - * struct idi_48_gpio - GPIO device private data structure
> - * @chip:	instance of the gpio_chip
> - * @reg:	I/O address offset for the device registers
> - */
> -struct idi_48_gpio {
> -	struct gpio_chip chip;
> -	struct idi_48_reg __iomem *reg;
> -};
> -
> -static int idi_48_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> -{
> -	return GPIO_LINE_DIRECTION_IN;
> -}
> -
> -static int idi_48_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
> -{
> -	return 0;
> -}
> -
> -static int idi_48_gpio_get(struct gpio_chip *chip, unsigned int offset)
> -{
> -	struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
> -	void __iomem *const ppi = idi48gpio->reg;
> -
> -	return i8255_get(ppi, offset);
> -}
> -
> -static int idi_48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
> -	unsigned long *bits)
> +static int idi_48_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
> +				 unsigned int offset, unsigned int *reg,
> +				 unsigned int *mask)
>  {
> -	struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
> -	void __iomem *const ppi = idi48gpio->reg;
> +	const unsigned int line = offset % 8;
> +	const unsigned int stride = offset / 8;
> +	const unsigned int port = (stride / 3) * 4;
> +	const unsigned int port_stride = stride % 3;
>  
> -	i8255_get_multiple(ppi, mask, bits, chip->ngpio);
> +	*reg = base + port + port_stride;
> +	*mask = BIT(line);
>  
>  	return 0;
>  }
> @@ -166,18 +123,14 @@ static const char *idi48_names[IDI48_NGPIO] = {
>  
>  static int idi_48_probe(struct device *dev, unsigned int id)
>  {
> -	struct idi_48_gpio *idi48gpio;
>  	const char *const name = dev_name(dev);
> +	struct gpio_regmap_config config = {0};

{} will work in the same way.

>  	void __iomem *regs;
>  	struct regmap *map;
>  	struct regmap_irq_chip *chip;
>  	struct regmap_irq_chip_data *chip_data;
>  	int err;
>  
> -	idi48gpio = devm_kzalloc(dev, sizeof(*idi48gpio), GFP_KERNEL);
> -	if (!idi48gpio)
> -		return -ENOMEM;
> -
>  	if (!devm_request_region(dev, base[id], IDI_48_EXTENT, name)) {
>  		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
>  			base[id], base[id] + IDI_48_EXTENT);
> @@ -187,7 +140,6 @@ static int idi_48_probe(struct device *dev, unsigned int id)
>  	regs = devm_ioport_map(dev, base[id], IDI_48_EXTENT);
>  	if (!regs)
>  		return -ENOMEM;
> -	idi48gpio->reg = regs;
>  
>  	map = devm_regmap_init_mmio(dev, regs, &idi48_regmap_config);
>  	if (IS_ERR(map))
> @@ -212,25 +164,16 @@ static int idi_48_probe(struct device *dev, unsigned int id)
>  		return err;
>  	}
>  
> -	idi48gpio->chip.label = name;
> -	idi48gpio->chip.parent = dev;
> -	idi48gpio->chip.owner = THIS_MODULE;
> -	idi48gpio->chip.base = -1;
> -	idi48gpio->chip.ngpio = IDI48_NGPIO;
> -	idi48gpio->chip.names = idi48_names;
> -	idi48gpio->chip.get_direction = idi_48_gpio_get_direction;
> -	idi48gpio->chip.direction_input = idi_48_gpio_direction_input;
> -	idi48gpio->chip.get = idi_48_gpio_get;
> -	idi48gpio->chip.get_multiple = idi_48_gpio_get_multiple;
> -
> -	err = devm_gpiochip_add_data(dev, &idi48gpio->chip, idi48gpio);
> -	if (err) {
> -		dev_err(dev, "GPIO registering failed (%d)\n", err);
> -		return err;
> -	}
> +	config.parent = dev;
> +	config.regmap = map;
> +	config.ngpio = IDI48_NGPIO;
> +	config.names = idi48_names;
> +	config.reg_dat_base = GPIO_REGMAP_ADDR(0x0);
> +	config.ngpio_per_reg = 8;
> +	config.reg_mask_xlate = idi_48_reg_mask_xlate;
> +	config.irq_domain = regmap_irq_get_domain(chip_data);
>  
> -	return gpiochip_irqchip_add_domain(&idi48gpio->chip,
> -					   regmap_irq_get_domain(chip_data));
> +	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &config));
>  }
>  
>  static struct isa_driver idi_48_driver = {
> -- 
> 2.38.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 9/9] gpio: i8255: Remove unused legacy interface

[Andy Shevchenko]

On Tue, Nov 22, 2022 at 02:11:06AM -0500, William Breathitt Gray wrote:
> All i8255 library consumers have migrated to the new interface
> leveraging the gpio-regmap API. Legacy interface functions and code are
> removed as no longer needed.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
> ---
>  drivers/gpio/gpio-i8255.c | 243 +-------------------------------------
>  drivers/gpio/gpio-i8255.h |  39 ------
>  2 files changed, 1 insertion(+), 281 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-i8255.c b/drivers/gpio/gpio-i8255.c
> index 9ecb2e9b97f9..64ab80fc4a1e 100644
> --- a/drivers/gpio/gpio-i8255.c
> +++ b/drivers/gpio/gpio-i8255.c
> @@ -3,16 +3,13 @@
>   * Intel 8255 Programmable Peripheral Interface
>   * Copyright (C) 2022 William Breathitt Gray
>   */
> -#include <linux/bitmap.h>
> +#include <linux/bits.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
>  #include <linux/gpio/regmap.h>
> -#include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
>  
>  #include "gpio-i8255.h"
>  
> @@ -30,15 +27,6 @@
>  #define I8255_REG_DAT_BASE I8255_PORTA
>  #define I8255_REG_DIR_IN_BASE I8255_CONTROL
>  
> -static int i8255_get_port(struct i8255 __iomem *const ppi,
> -			  const unsigned long io_port, const unsigned long mask)
> -{
> -	const unsigned long bank = io_port / 3;
> -	const unsigned long ppi_port = io_port % 3;
> -
> -	return ioread8(&ppi[bank].port[ppi_port]) & mask;
> -}
> -
>  static int i8255_direction_mask(const unsigned int offset)
>  {
>  	const unsigned int stride = offset / I8255_NGPIO_PER_REG;
> @@ -103,235 +91,6 @@ static int i8255_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
>  	}
>  }
>  
> -static void i8255_set_port(struct i8255 __iomem *const ppi,
> -			   struct i8255_state *const state,
> -			   const unsigned long io_port,
> -			   const unsigned long mask, const unsigned long bits)
> -{
> -	const unsigned long bank = io_port / 3;
> -	const unsigned long ppi_port = io_port % 3;
> -	unsigned long flags;
> -	unsigned long out_state;
> -
> -	spin_lock_irqsave(&state[bank].lock, flags);
> -
> -	out_state = ioread8(&ppi[bank].port[ppi_port]);
> -	out_state = (out_state & ~mask) | (bits & mask);
> -	iowrite8(out_state, &ppi[bank].port[ppi_port]);
> -
> -	spin_unlock_irqrestore(&state[bank].lock, flags);
> -}
> -
> -/**
> - * i8255_direction_input - configure signal offset as input
> - * @ppi:	Intel 8255 Programmable Peripheral Interface banks
> - * @state:	devices states of the respective PPI banks
> - * @offset:	signal offset to configure as input
> - *
> - * Configures a signal @offset as input for the respective Intel 8255
> - * Programmable Peripheral Interface (@ppi) banks. The @state control_state
> - * values are updated to reflect the new configuration.
> - */
> -void i8255_direction_input(struct i8255 __iomem *const ppi,
> -			   struct i8255_state *const state,
> -			   const unsigned long offset)
> -{
> -	const unsigned long io_port = offset / 8;
> -	const unsigned long bank = io_port / 3;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&state[bank].lock, flags);
> -
> -	state[bank].control_state |= I8255_CONTROL_MODE_SET;
> -	state[bank].control_state |= i8255_direction_mask(offset % 24);
> -
> -	iowrite8(state[bank].control_state, &ppi[bank].control);
> -
> -	spin_unlock_irqrestore(&state[bank].lock, flags);
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_direction_input, I8255);
> -
> -/**
> - * i8255_direction_output - configure signal offset as output
> - * @ppi:	Intel 8255 Programmable Peripheral Interface banks
> - * @state:	devices states of the respective PPI banks
> - * @offset:	signal offset to configure as output
> - * @value:	signal value to output
> - *
> - * Configures a signal @offset as output for the respective Intel 8255
> - * Programmable Peripheral Interface (@ppi) banks and sets the respective signal
> - * output to the desired @value. The @state control_state values are updated to
> - * reflect the new configuration.
> - */
> -void i8255_direction_output(struct i8255 __iomem *const ppi,
> -			    struct i8255_state *const state,
> -			    const unsigned long offset,
> -			    const unsigned long value)
> -{
> -	const unsigned long io_port = offset / 8;
> -	const unsigned long bank = io_port / 3;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&state[bank].lock, flags);
> -
> -	state[bank].control_state |= I8255_CONTROL_MODE_SET;
> -	state[bank].control_state &= ~i8255_direction_mask(offset % 24);
> -
> -	iowrite8(state[bank].control_state, &ppi[bank].control);
> -
> -	spin_unlock_irqrestore(&state[bank].lock, flags);
> -
> -	i8255_set(ppi, state, offset, value);
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_direction_output, I8255);
> -
> -/**
> - * i8255_get - get signal value at signal offset
> - * @ppi:	Intel 8255 Programmable Peripheral Interface banks
> - * @offset:	offset of signal to get
> - *
> - * Returns the signal value (0=low, 1=high) for the signal at @offset for the
> - * respective Intel 8255 Programmable Peripheral Interface (@ppi) banks.
> - */
> -int i8255_get(struct i8255 __iomem *const ppi, const unsigned long offset)
> -{
> -	const unsigned long io_port = offset / 8;
> -	const unsigned long offset_mask = BIT(offset % 8);
> -
> -	return !!i8255_get_port(ppi, io_port, offset_mask);
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_get, I8255);
> -
> -/**
> - * i8255_get_direction - get the I/O direction for a signal offset
> - * @state:	devices states of the respective PPI banks
> - * @offset:	offset of signal to get direction
> - *
> - * Returns the signal direction (0=output, 1=input) for the signal at @offset.
> - */
> -int i8255_get_direction(const struct i8255_state *const state,
> -			const unsigned long offset)
> -{
> -	const unsigned long io_port = offset / 8;
> -	const unsigned long bank = io_port / 3;
> -
> -	return !!(state[bank].control_state & i8255_direction_mask(offset % 24));
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_get_direction, I8255);
> -
> -/**
> - * i8255_get_multiple - get multiple signal values at multiple signal offsets
> - * @ppi:	Intel 8255 Programmable Peripheral Interface banks
> - * @mask:	mask of signals to get
> - * @bits:	bitmap to store signal values
> - * @ngpio:	number of GPIO signals of the respective PPI banks
> - *
> - * Stores in @bits the values (0=low, 1=high) for the signals defined by @mask
> - * for the respective Intel 8255 Programmable Peripheral Interface (@ppi) banks.
> - */
> -void i8255_get_multiple(struct i8255 __iomem *const ppi,
> -			const unsigned long *const mask,
> -			unsigned long *const bits, const unsigned long ngpio)
> -{
> -	unsigned long offset;
> -	unsigned long port_mask;
> -	unsigned long io_port;
> -	unsigned long port_state;
> -
> -	bitmap_zero(bits, ngpio);
> -
> -	for_each_set_clump8(offset, port_mask, mask, ngpio) {
> -		io_port = offset / 8;
> -		port_state = i8255_get_port(ppi, io_port, port_mask);
> -
> -		bitmap_set_value8(bits, port_state, offset);
> -	}
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_get_multiple, I8255);
> -
> -/**
> - * i8255_mode0_output - configure all PPI ports to MODE 0 output mode
> - * @ppi:	Intel 8255 Programmable Peripheral Interface bank
> - *
> - * Configures all Intel 8255 Programmable Peripheral Interface (@ppi) ports to
> - * MODE 0 (Basic Input/Output) output mode.
> - */
> -void i8255_mode0_output(struct i8255 __iomem *const ppi)
> -{
> -	iowrite8(I8255_CONTROL_MODE_SET, &ppi->control);
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_mode0_output, I8255);
> -
> -/**
> - * i8255_set - set signal value at signal offset
> - * @ppi:	Intel 8255 Programmable Peripheral Interface banks
> - * @state:	devices states of the respective PPI banks
> - * @offset:	offset of signal to set
> - * @value:	value of signal to set
> - *
> - * Assigns output @value for the signal at @offset for the respective Intel 8255
> - * Programmable Peripheral Interface (@ppi) banks.
> - */
> -void i8255_set(struct i8255 __iomem *const ppi, struct i8255_state *const state,
> -	       const unsigned long offset, const unsigned long value)
> -{
> -	const unsigned long io_port = offset / 8;
> -	const unsigned long port_offset = offset % 8;
> -	const unsigned long mask = BIT(port_offset);
> -	const unsigned long bits = value << port_offset;
> -
> -	i8255_set_port(ppi, state, io_port, mask, bits);
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_set, I8255);
> -
> -/**
> - * i8255_set_multiple - set signal values at multiple signal offsets
> - * @ppi:	Intel 8255 Programmable Peripheral Interface banks
> - * @state:	devices states of the respective PPI banks
> - * @mask:	mask of signals to set
> - * @bits:	bitmap of signal output values
> - * @ngpio:	number of GPIO signals of the respective PPI banks
> - *
> - * Assigns output values defined by @bits for the signals defined by @mask for
> - * the respective Intel 8255 Programmable Peripheral Interface (@ppi) banks.
> - */
> -void i8255_set_multiple(struct i8255 __iomem *const ppi,
> -			struct i8255_state *const state,
> -			const unsigned long *const mask,
> -			const unsigned long *const bits,
> -			const unsigned long ngpio)
> -{
> -	unsigned long offset;
> -	unsigned long port_mask;
> -	unsigned long io_port;
> -	unsigned long value;
> -
> -	for_each_set_clump8(offset, port_mask, mask, ngpio) {
> -		io_port = offset / 8;
> -		value = bitmap_get_value8(bits, offset);
> -		i8255_set_port(ppi, state, io_port, port_mask, value);
> -	}
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_set_multiple, I8255);
> -
> -/**
> - * i8255_state_init - initialize i8255_state structure
> - * @state:	devices states of the respective PPI banks
> - * @nbanks:	number of Intel 8255 Programmable Peripheral Interface banks
> - *
> - * Initializes the @state of each Intel 8255 Programmable Peripheral Interface
> - * bank for use in i8255 library functions.
> - */
> -void i8255_state_init(struct i8255_state *const state,
> -		      const unsigned long nbanks)
> -{
> -	unsigned long bank;
> -
> -	for (bank = 0; bank < nbanks; bank++)
> -		spin_lock_init(&state[bank].lock);
> -}
> -EXPORT_SYMBOL_NS_GPL(i8255_state_init, I8255);
> -
>  /**
>   * devm_i8255_regmap_register - Register an i8255 GPIO controller
>   * @dev:	device that is registering this i8255 GPIO device
> diff --git a/drivers/gpio/gpio-i8255.h b/drivers/gpio/gpio-i8255.h
> index 6ec987835c14..bc3023745e7b 100644
> --- a/drivers/gpio/gpio-i8255.h
> +++ b/drivers/gpio/gpio-i8255.h
> @@ -6,28 +6,6 @@
>  #include <linux/device.h>
>  #include <linux/irqdomain.h>
>  #include <linux/regmap.h>
> -#include <linux/spinlock.h>
> -#include <linux/types.h>
> -
> -/**
> - * struct i8255 - Intel 8255 register structure
> - * @port:	Port A, B, and C
> - * @control:	Control register
> - */
> -struct i8255 {
> -	u8 port[3];
> -	u8 control;
> -};
> -
> -/**
> - * struct i8255_state - Intel 8255 state structure
> - * @lock:		synchronization lock for accessing device state
> - * @control_state:	Control register state
> - */
> -struct i8255_state {
> -	spinlock_t lock;
> -	u8 control_state;
> -};
>  
>  #define i8255_volatile_regmap_range(_base) regmap_reg_range(_base, _base + 0x2)
>  
> @@ -53,21 +31,4 @@ struct i8255_regmap_config {
>  int devm_i8255_regmap_register(struct device *dev,
>  			       const struct i8255_regmap_config *config);
>  
> -void i8255_direction_input(struct i8255 __iomem *ppi, struct i8255_state *state,
> -			   unsigned long offset);
> -void i8255_direction_output(struct i8255 __iomem *ppi,
> -			    struct i8255_state *state, unsigned long offset,
> -			    unsigned long value);
> -int i8255_get(struct i8255 __iomem *ppi, unsigned long offset);
> -int i8255_get_direction(const struct i8255_state *state, unsigned long offset);
> -void i8255_get_multiple(struct i8255 __iomem *ppi, const unsigned long *mask,
> -			unsigned long *bits, unsigned long ngpio);
> -void i8255_mode0_output(struct i8255 __iomem *const ppi);
> -void i8255_set(struct i8255 __iomem *ppi, struct i8255_state *state,
> -	       unsigned long offset, unsigned long value);
> -void i8255_set_multiple(struct i8255 __iomem *ppi, struct i8255_state *state,
> -			const unsigned long *mask, const unsigned long *bits,
> -			unsigned long ngpio);
> -void i8255_state_init(struct i8255_state *const state, unsigned long nbanks);
> -
>  #endif /* _I8255_H_ */
> -- 
> 2.38.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/9] gpio: i8255: Migrate to gpio-regmap API

[Andy Shevchenko]

On Tue, Nov 22, 2022 at 02:11:03AM -0500, William Breathitt Gray wrote:
> The regmap API supports IO port accessors so we can take advantage of
> regmap abstractions rather than handling access to the device registers
> directly in the driver.
> 
> By leveraging the gpio-regmap API, the i8255 library is reduced to
> simply a devm_i8255_regmap_register() function, a configuration
> structure struct i8255_regmap_config, and a helper macro
> i8255_volatile_regmap_range() provided to simplify volatile PPI register
> hinting for the regmap.
> 
> Legacy functions and code will be removed once all consumers have
> migrated to the new i8255 library interface.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(See one remark below and once comment to address)

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
> ---
>  drivers/gpio/Kconfig      |   1 +
>  drivers/gpio/gpio-i8255.c | 119 ++++++++++++++++++++++++++++++++++----
>  drivers/gpio/gpio-i8255.h |  27 +++++++++
>  3 files changed, 135 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index dd34039fc31b..88dfdc62992f 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -831,6 +831,7 @@ menu "Port-mapped I/O GPIO drivers"
>  
>  config GPIO_I8255
>  	tristate
> +	select GPIO_REGMAP
>  	help
>  	  Enables support for the i8255 interface library functions. The i8255
>  	  interface library provides functions to facilitate communication with
> diff --git a/drivers/gpio/gpio-i8255.c b/drivers/gpio/gpio-i8255.c
> index 9b97db418df1..9ecb2e9b97f9 100644
> --- a/drivers/gpio/gpio-i8255.c
> +++ b/drivers/gpio/gpio-i8255.c
> @@ -4,23 +4,31 @@
>   * Copyright (C) 2022 William Breathitt Gray
>   */
>  #include <linux/bitmap.h>
> +#include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
> +#include <linux/gpio/regmap.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/regmap.h>
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>  
>  #include "gpio-i8255.h"
>  
> +#define I8255_NGPIO 24
> +#define I8255_NGPIO_PER_REG 8
>  #define I8255_CONTROL_PORTC_LOWER_DIRECTION BIT(0)
>  #define I8255_CONTROL_PORTB_DIRECTION BIT(1)
>  #define I8255_CONTROL_PORTC_UPPER_DIRECTION BIT(3)
>  #define I8255_CONTROL_PORTA_DIRECTION BIT(4)
>  #define I8255_CONTROL_MODE_SET BIT(7)
> -#define I8255_PORTA 0
> -#define I8255_PORTB 1
> -#define I8255_PORTC 2
> +#define I8255_PORTA 0x0
> +#define I8255_PORTB 0x1
> +#define I8255_PORTC 0x2
> +#define I8255_CONTROL 0x3
> +#define I8255_REG_DAT_BASE I8255_PORTA
> +#define I8255_REG_DIR_IN_BASE I8255_CONTROL
>  
>  static int i8255_get_port(struct i8255 __iomem *const ppi,
>  			  const unsigned long io_port, const unsigned long mask)
> @@ -31,20 +39,19 @@ static int i8255_get_port(struct i8255 __iomem *const ppi,
>  	return ioread8(&ppi[bank].port[ppi_port]) & mask;
>  }
>  
> -static u8 i8255_direction_mask(const unsigned long offset)
> +static int i8255_direction_mask(const unsigned int offset)
>  {
> -	const unsigned long port_offset = offset % 8;
> -	const unsigned long io_port = offset / 8;
> -	const unsigned long ppi_port = io_port % 3;
> +	const unsigned int stride = offset / I8255_NGPIO_PER_REG;
> +	const unsigned int line = offset % I8255_NGPIO_PER_REG;
>  
> -	switch (ppi_port) {
> +	switch (stride) {
>  	case I8255_PORTA:
>  		return I8255_CONTROL_PORTA_DIRECTION;
>  	case I8255_PORTB:
>  		return I8255_CONTROL_PORTB_DIRECTION;
>  	case I8255_PORTC:
>  		/* Port C can be configured by nibble */
> -		if (port_offset >= 4)
> +		if (line >= 4)
>  			return I8255_CONTROL_PORTC_UPPER_DIRECTION;
>  		return I8255_CONTROL_PORTC_LOWER_DIRECTION;
>  	default:
> @@ -53,6 +60,49 @@ static u8 i8255_direction_mask(const unsigned long offset)
>  	}
>  }
>  
> +static int i8255_ppi_init(struct regmap *const map, const unsigned int base)
> +{
> +	int err;
> +
> +	/* Configure all ports to MODE 0 output mode */
> +	err = regmap_write(map, base + I8255_CONTROL, I8255_CONTROL_MODE_SET);
> +	if (err)
> +		return err;
> +
> +	/* Initialize all GPIO to output 0 */
> +	err = regmap_write(map, base + I8255_PORTA, 0x00);
> +	if (err)
> +		return err;
> +	err = regmap_write(map, base + I8255_PORTB, 0x00);
> +	if (err)
> +		return err;
> +	return regmap_write(map, base + I8255_PORTC, 0x00);
> +}
> +
> +static int i8255_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
> +				unsigned int offset, unsigned int *reg,
> +				unsigned int *mask)
> +{
> +	const unsigned int ppi = offset / I8255_NGPIO;
> +	const unsigned int ppi_offset = offset % I8255_NGPIO;
> +	const unsigned int stride = ppi_offset / I8255_NGPIO_PER_REG;
> +	const unsigned int line = ppi_offset % I8255_NGPIO_PER_REG;
> +
> +	switch (base) {
> +	case I8255_REG_DAT_BASE:
> +		*reg = base + stride + ppi * 4;
> +		*mask = BIT(line);
> +		return 0;
> +	case I8255_REG_DIR_IN_BASE:
> +		*reg = base + ppi * 4;
> +		*mask = i8255_direction_mask(ppi_offset);
> +		return 0;
> +	default:
> +		/* Should never reach this path */
> +		return -EINVAL;
> +	}
> +}
> +
>  static void i8255_set_port(struct i8255 __iomem *const ppi,
>  			   struct i8255_state *const state,
>  			   const unsigned long io_port,
> @@ -93,7 +143,7 @@ void i8255_direction_input(struct i8255 __iomem *const ppi,
>  	spin_lock_irqsave(&state[bank].lock, flags);
>  
>  	state[bank].control_state |= I8255_CONTROL_MODE_SET;
> -	state[bank].control_state |= i8255_direction_mask(offset);
> +	state[bank].control_state |= i8255_direction_mask(offset % 24);
>  
>  	iowrite8(state[bank].control_state, &ppi[bank].control);
>  
> @@ -125,7 +175,7 @@ void i8255_direction_output(struct i8255 __iomem *const ppi,
>  	spin_lock_irqsave(&state[bank].lock, flags);
>  
>  	state[bank].control_state |= I8255_CONTROL_MODE_SET;
> -	state[bank].control_state &= ~i8255_direction_mask(offset);
> +	state[bank].control_state &= ~i8255_direction_mask(offset % 24);
>  
>  	iowrite8(state[bank].control_state, &ppi[bank].control);
>  
> @@ -165,7 +215,7 @@ int i8255_get_direction(const struct i8255_state *const state,
>  	const unsigned long io_port = offset / 8;
>  	const unsigned long bank = io_port / 3;
>  
> -	return !!(state[bank].control_state & i8255_direction_mask(offset));
> +	return !!(state[bank].control_state & i8255_direction_mask(offset % 24));
>  }
>  EXPORT_SYMBOL_NS_GPL(i8255_get_direction, I8255);
>  
> @@ -282,6 +332,51 @@ void i8255_state_init(struct i8255_state *const state,
>  }
>  EXPORT_SYMBOL_NS_GPL(i8255_state_init, I8255);
>  
> +/**
> + * devm_i8255_regmap_register - Register an i8255 GPIO controller
> + * @dev:	device that is registering this i8255 GPIO device
> + * @config:	configuration for i8255_regmap_config
> + *
> + * Registers an Intel 8255 Programmable Peripheral Interface GPIO controller.
> + * Returns 0 on success and negative error number on failure.
> + */
> +int devm_i8255_regmap_register(struct device *const dev,
> +			       const struct i8255_regmap_config *const config)
> +{
> +	struct gpio_regmap_config gpio_config = {0};
> +	unsigned long i;
> +	int err;
> +
> +	if (!config->parent)
> +		return -EINVAL;
> +
> +	if (!config->map)
> +		return -EINVAL;
> +
> +	if (!config->num_ppi)
> +		return -EINVAL;
> +
> +	for (i = 0; i < config->num_ppi; i++) {
> +		err = i8255_ppi_init(config->map, i * 4);
> +		if (err)
> +			return err;
> +	}
> +
> +	gpio_config.parent = config->parent;
> +	gpio_config.regmap = config->map;
> +	gpio_config.ngpio = I8255_NGPIO * config->num_ppi;
> +	gpio_config.names = config->names;
> +	gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE);
> +	gpio_config.reg_set_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE);
> +	gpio_config.reg_dir_in_base = GPIO_REGMAP_ADDR(I8255_REG_DIR_IN_BASE);
> +	gpio_config.ngpio_per_reg = I8255_NGPIO_PER_REG;
> +	gpio_config.irq_domain = config->domain;
> +	gpio_config.reg_mask_xlate = i8255_reg_mask_xlate;
> +
> +	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_i8255_regmap_register, I8255);
> +
>  MODULE_AUTHOR("William Breathitt Gray");
>  MODULE_DESCRIPTION("Intel 8255 Programmable Peripheral Interface");
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/gpio/gpio-i8255.h b/drivers/gpio/gpio-i8255.h
> index d9084aae9446..6ec987835c14 100644
> --- a/drivers/gpio/gpio-i8255.h
> +++ b/drivers/gpio/gpio-i8255.h
> @@ -3,6 +3,9 @@
>  #ifndef _I8255_H_
>  #define _I8255_H_

> +#include <linux/device.h>
> +#include <linux/irqdomain.h>
> +#include <linux/regmap.h>

As far as I can see you have no users for these headers. You may inform
compiler by providing forward declarations. Like

	struct device;


after the inclusion block.

>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>  
> @@ -26,6 +29,30 @@ struct i8255_state {
>  	u8 control_state;
>  };
>  
> +#define i8255_volatile_regmap_range(_base) regmap_reg_range(_base, _base + 0x2)
> +
> +/**
> + * struct i8255_regmap_config - Configuration for the register map of an i8255
> + * @parent:	parent device
> + * @map:	regmap for the i8255
> + * @num_ppi:	number of i8255 Programmable Peripheral Interface
> + * @names:	(optional) array of names for gpios
> + * @domain:	(optional) IRQ domain if the controller is interrupt-capable

> + * Note: The regmap is expected to have cache enabled and i8255 control
> + * registers not marked as volatile.

Have you considered to catch wrong configurations by BUILD_BUG_ON() /
static_assert() / another means of validation?

> + */
> +struct i8255_regmap_config {
> +	struct device *parent;
> +	struct regmap *map;
> +	int num_ppi;
> +	const char *const *names;
> +	struct irq_domain *domain;
> +};
> +
> +int devm_i8255_regmap_register(struct device *dev,
> +			       const struct i8255_regmap_config *config);
> +
>  void i8255_direction_input(struct i8255 __iomem *ppi, struct i8255_state *state,
>  			   unsigned long offset);
>  void i8255_direction_output(struct i8255 __iomem *ppi,
> -- 
> 2.38.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 7/9] gpio: 104-dio-48e: Migrate to regmap API

[Andy Shevchenko]

On Tue, Nov 22, 2022 at 02:11:04AM -0500, William Breathitt Gray wrote:
> The regmap API supports IO port accessors so we can take advantage of
> regmap abstractions rather than handling access to the device registers
> directly in the driver. The 104-dio-48e module is migrated to the new
> i8255 library interface leveraging the gpio-regmap API.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(see also below)

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
> ---
>  drivers/gpio/gpio-104-dio-48e.c | 147 ++------------------------------
>  1 file changed, 7 insertions(+), 140 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-104-dio-48e.c b/drivers/gpio/gpio-104-dio-48e.c
> index fcee3dc81902..64f4044150b7 100644
> --- a/drivers/gpio/gpio-104-dio-48e.c
> +++ b/drivers/gpio/gpio-104-dio-48e.c
> @@ -9,7 +9,6 @@
>  #include <linux/bits.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> -#include <linux/gpio/driver.h>
>  #include <linux/ioport.h>
>  #include <linux/irq.h>
>  #include <linux/isa.h>
> @@ -42,90 +41,6 @@ MODULE_PARM_DESC(irq, "ACCES 104-DIO-48E interrupt line numbers");
>  
>  #define DIO48E_NUM_PPI 2
>  
> -/**
> - * struct dio48e_reg - device register structure
> - * @ppi:		Programmable Peripheral Interface groups
> - */
> -struct dio48e_reg {
> -	struct i8255 ppi[DIO48E_NUM_PPI];
> -};
> -
> -/**
> - * struct dio48e_gpio - GPIO device private data structure
> - * @chip:		instance of the gpio_chip
> - * @ppi_state:		PPI device states
> - * @reg:		I/O address offset for the device registers
> - */
> -struct dio48e_gpio {
> -	struct gpio_chip chip;
> -	struct i8255_state ppi_state[DIO48E_NUM_PPI];
> -	struct dio48e_reg __iomem *reg;
> -};
> -
> -static int dio48e_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
> -
> -	if (i8255_get_direction(dio48egpio->ppi_state, offset))
> -		return GPIO_LINE_DIRECTION_IN;
> -
> -	return GPIO_LINE_DIRECTION_OUT;
> -}
> -
> -static int dio48e_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
> -
> -	i8255_direction_input(dio48egpio->reg->ppi, dio48egpio->ppi_state,
> -			      offset);
> -
> -	return 0;
> -}
> -
> -static int dio48e_gpio_direction_output(struct gpio_chip *chip, unsigned int offset,
> -					int value)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
> -
> -	i8255_direction_output(dio48egpio->reg->ppi, dio48egpio->ppi_state,
> -			       offset, value);
> -
> -	return 0;
> -}
> -
> -static int dio48e_gpio_get(struct gpio_chip *chip, unsigned int offset)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
> -
> -	return i8255_get(dio48egpio->reg->ppi, offset);
> -}
> -
> -static int dio48e_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
> -	unsigned long *bits)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
> -
> -	i8255_get_multiple(dio48egpio->reg->ppi, mask, bits, chip->ngpio);
> -
> -	return 0;
> -}
> -
> -static void dio48e_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
> -
> -	i8255_set(dio48egpio->reg->ppi, dio48egpio->ppi_state, offset, value);
> -}
> -
> -static void dio48e_gpio_set_multiple(struct gpio_chip *chip,
> -	unsigned long *mask, unsigned long *bits)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip);
> -
> -	i8255_set_multiple(dio48egpio->reg->ppi, dio48egpio->ppi_state, mask,
> -			   bits, chip->ngpio);
> -}
> -
>  static const struct regmap_range dio48e_wr_ranges[] = {
>  	regmap_reg_range(0x0, 0x9), regmap_reg_range(0xB, 0xB),
>  	regmap_reg_range(0xD, 0xD), regmap_reg_range(0xF, 0xF),
> @@ -237,35 +152,10 @@ static const char *dio48e_names[DIO48E_NGPIO] = {
>  	"PPI Group 1 Port C 5", "PPI Group 1 Port C 6", "PPI Group 1 Port C 7"
>  };
>  
> -static int dio48e_irq_init_hw(struct gpio_chip *gc)
> -{
> -	struct dio48e_gpio *const dio48egpio = gpiochip_get_data(gc);
> -
> -	/* Disable IRQ by default */
> -	ioread8(&dio48egpio->reg->enable_interrupt);
> -
> -	return 0;
> -}
> -
> -static void dio48e_init_ppi(struct i8255 __iomem *const ppi,
> -			    struct i8255_state *const ppi_state)
> -{
> -	const unsigned long ngpio = 24;
> -	const unsigned long mask = GENMASK(ngpio - 1, 0);
> -	const unsigned long bits = 0;
> -	unsigned long i;
> -
> -	/* Initialize all GPIO to output 0 */
> -	for (i = 0; i < DIO48E_NUM_PPI; i++) {
> -		i8255_mode0_output(&ppi[i]);
> -		i8255_set_multiple(&ppi[i], &ppi_state[i], &mask, &bits, ngpio);
> -	}
> -}
> -
>  static int dio48e_probe(struct device *dev, unsigned int id)
>  {
> -	struct dio48e_gpio *dio48egpio;
>  	const char *const name = dev_name(dev);

> +	struct i8255_regmap_config config = {0};

{} will work okay.

>  	void __iomem *regs;
>  	struct regmap *map;
>  	unsigned int val;
> @@ -274,10 +164,6 @@ static int dio48e_probe(struct device *dev, unsigned int id)
>  	unsigned int irq_mask;
>  	struct regmap_irq_chip_data *chip_data;
>  
> -	dio48egpio = devm_kzalloc(dev, sizeof(*dio48egpio), GFP_KERNEL);
> -	if (!dio48egpio)
> -		return -ENOMEM;
> -
>  	if (!devm_request_region(dev, base[id], DIO48E_EXTENT, name)) {
>  		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
>  			base[id], base[id] + DIO48E_EXTENT);
> @@ -287,7 +173,6 @@ static int dio48e_probe(struct device *dev, unsigned int id)
>  	regs = devm_ioport_map(dev, base[id], DIO48E_EXTENT);
>  	if (!regs)
>  		return -ENOMEM;
> -	dio48egpio->reg = regs;
>  
>  	map = devm_regmap_init_mmio(dev, regs, &dio48e_regmap_config);
>  	if (IS_ERR(map))
> @@ -324,31 +209,13 @@ static int dio48e_probe(struct device *dev, unsigned int id)
>  		return err;
>  	}
>  
> -	dio48egpio->chip.label = name;
> -	dio48egpio->chip.parent = dev;
> -	dio48egpio->chip.owner = THIS_MODULE;
> -	dio48egpio->chip.base = -1;
> -	dio48egpio->chip.ngpio = DIO48E_NGPIO;
> -	dio48egpio->chip.names = dio48e_names;
> -	dio48egpio->chip.get_direction = dio48e_gpio_get_direction;
> -	dio48egpio->chip.direction_input = dio48e_gpio_direction_input;
> -	dio48egpio->chip.direction_output = dio48e_gpio_direction_output;
> -	dio48egpio->chip.get = dio48e_gpio_get;
> -	dio48egpio->chip.get_multiple = dio48e_gpio_get_multiple;
> -	dio48egpio->chip.set = dio48e_gpio_set;
> -	dio48egpio->chip.set_multiple = dio48e_gpio_set_multiple;
> -
> -	i8255_state_init(dio48egpio->ppi_state, DIO48E_NUM_PPI);
> -	dio48e_init_ppi(dio48egpio->reg->ppi, dio48egpio->ppi_state);
> -
> -	err = devm_gpiochip_add_data(dev, &dio48egpio->chip, dio48egpio);
> -	if (err) {
> -		dev_err(dev, "GPIO registering failed (%d)\n", err);
> -		return err;
> -	}
> +	config.parent = dev;
> +	config.map = map;
> +	config.num_ppi = DIO48E_NUM_PPI;
> +	config.names = dio48e_names;
> +	config.domain = regmap_irq_get_domain(chip_data);
>  
> -	return gpiochip_irqchip_add_domain(&dio48egpio->chip,
> -					   regmap_irq_get_domain(chip_data));
> +	return devm_i8255_regmap_register(dev, &config);
>  }
>  
>  static struct isa_driver dio48e_driver = {
> -- 
> 2.38.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 8/9] gpio: gpio-mm: Migrate to regmap API

[Andy Shevchenko]

On Tue, Nov 22, 2022 at 02:11:05AM -0500, William Breathitt Gray wrote:
> The regmap API supports IO port accessors so we can take advantage of
> regmap abstractions rather than handling access to the device registers
> directly in the driver. The gpio-mm module is migrated to the new i8255
> library interface leveraging the gpio-regmap API.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

(see also below)

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
> ---
>  drivers/gpio/gpio-gpio-mm.c | 153 +++++++-----------------------------
>  1 file changed, 29 insertions(+), 124 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-gpio-mm.c b/drivers/gpio/gpio-gpio-mm.c
> index 2689671b6b01..ba8847485660 100644
> --- a/drivers/gpio/gpio-gpio-mm.c
> +++ b/drivers/gpio/gpio-gpio-mm.c
> @@ -8,13 +8,13 @@
>   */
>  #include <linux/device.h>
>  #include <linux/errno.h>
> -#include <linux/gpio/driver.h>
> -#include <linux/io.h>
>  #include <linux/ioport.h>
>  #include <linux/isa.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
>  
>  #include "gpio-i8255.h"
>  
> @@ -30,83 +30,22 @@ MODULE_PARM_DESC(base, "Diamond Systems GPIO-MM base addresses");
>  
>  #define GPIOMM_NUM_PPI 2
>  
> -/**
> - * struct gpiomm_gpio - GPIO device private data structure
> - * @chip:		instance of the gpio_chip
> - * @ppi_state:		Programmable Peripheral Interface group states
> - * @ppi:		Programmable Peripheral Interface groups
> - */
> -struct gpiomm_gpio {
> -	struct gpio_chip chip;
> -	struct i8255_state ppi_state[GPIOMM_NUM_PPI];
> -	struct i8255 __iomem *ppi;
> +static const struct regmap_range gpiomm_volatile_ranges[] = {
> +	i8255_volatile_regmap_range(0x0), i8255_volatile_regmap_range(0x4),
> +};
> +static const struct regmap_access_table gpiomm_volatile_table = {
> +	.yes_ranges = gpiomm_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(gpiomm_volatile_ranges),
> +};
> +static const struct regmap_config gpiomm_regmap_config = {
> +	.reg_bits = 8,
> +	.reg_stride = 1,
> +	.val_bits = 8,
> +	.io_port = true,
> +	.max_register = 0x7,
> +	.volatile_table = &gpiomm_volatile_table,
> +	.cache_type = REGCACHE_FLAT,
>  };
> -
> -static int gpiomm_gpio_get_direction(struct gpio_chip *chip,
> -	unsigned int offset)
> -{
> -	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
> -
> -	if (i8255_get_direction(gpiommgpio->ppi_state, offset))
> -		return GPIO_LINE_DIRECTION_IN;
> -
> -	return GPIO_LINE_DIRECTION_OUT;
> -}
> -
> -static int gpiomm_gpio_direction_input(struct gpio_chip *chip,
> -	unsigned int offset)
> -{
> -	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
> -
> -	i8255_direction_input(gpiommgpio->ppi, gpiommgpio->ppi_state, offset);
> -
> -	return 0;
> -}
> -
> -static int gpiomm_gpio_direction_output(struct gpio_chip *chip,
> -	unsigned int offset, int value)
> -{
> -	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
> -
> -	i8255_direction_output(gpiommgpio->ppi, gpiommgpio->ppi_state, offset,
> -			       value);
> -
> -	return 0;
> -}
> -
> -static int gpiomm_gpio_get(struct gpio_chip *chip, unsigned int offset)
> -{
> -	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
> -
> -	return i8255_get(gpiommgpio->ppi, offset);
> -}
> -
> -static int gpiomm_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
> -	unsigned long *bits)
> -{
> -	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
> -
> -	i8255_get_multiple(gpiommgpio->ppi, mask, bits, chip->ngpio);
> -
> -	return 0;
> -}
> -
> -static void gpiomm_gpio_set(struct gpio_chip *chip, unsigned int offset,
> -	int value)
> -{
> -	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
> -
> -	i8255_set(gpiommgpio->ppi, gpiommgpio->ppi_state, offset, value);
> -}
> -
> -static void gpiomm_gpio_set_multiple(struct gpio_chip *chip,
> -	unsigned long *mask, unsigned long *bits)
> -{
> -	struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip);
> -
> -	i8255_set_multiple(gpiommgpio->ppi, gpiommgpio->ppi_state, mask, bits,
> -			   chip->ngpio);
> -}
>  
>  #define GPIOMM_NGPIO 48
>  static const char *gpiomm_names[GPIOMM_NGPIO] = {
> @@ -120,30 +59,11 @@ static const char *gpiomm_names[GPIOMM_NGPIO] = {
>  	"Port 2C2", "Port 2C3", "Port 2C4", "Port 2C5", "Port 2C6", "Port 2C7",
>  };
>  
> -static void gpiomm_init_dio(struct i8255 __iomem *const ppi,
> -			    struct i8255_state *const ppi_state)
> -{
> -	const unsigned long ngpio = 24;
> -	const unsigned long mask = GENMASK(ngpio - 1, 0);
> -	const unsigned long bits = 0;
> -	unsigned long i;
> -
> -	/* Initialize all GPIO to output 0 */
> -	for (i = 0; i < GPIOMM_NUM_PPI; i++) {
> -		i8255_mode0_output(&ppi[i]);
> -		i8255_set_multiple(&ppi[i], &ppi_state[i], &mask, &bits, ngpio);
> -	}
> -}
> -
>  static int gpiomm_probe(struct device *dev, unsigned int id)
>  {
> -	struct gpiomm_gpio *gpiommgpio;
>  	const char *const name = dev_name(dev);
> -	int err;
> -
> -	gpiommgpio = devm_kzalloc(dev, sizeof(*gpiommgpio), GFP_KERNEL);
> -	if (!gpiommgpio)
> -		return -ENOMEM;

> +	struct i8255_regmap_config config = {0};

{} will be okay as well.

> +	void __iomem *regs;
>  
>  	if (!devm_request_region(dev, base[id], GPIOMM_EXTENT, name)) {
>  		dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
> @@ -151,34 +71,19 @@ static int gpiomm_probe(struct device *dev, unsigned int id)
>  		return -EBUSY;
>  	}
>  
> -	gpiommgpio->ppi = devm_ioport_map(dev, base[id], GPIOMM_EXTENT);
> -	if (!gpiommgpio->ppi)
> +	regs = devm_ioport_map(dev, base[id], GPIOMM_EXTENT);
> +	if (!regs)
>  		return -ENOMEM;
>  
> -	gpiommgpio->chip.label = name;
> -	gpiommgpio->chip.parent = dev;
> -	gpiommgpio->chip.owner = THIS_MODULE;
> -	gpiommgpio->chip.base = -1;
> -	gpiommgpio->chip.ngpio = GPIOMM_NGPIO;
> -	gpiommgpio->chip.names = gpiomm_names;
> -	gpiommgpio->chip.get_direction = gpiomm_gpio_get_direction;
> -	gpiommgpio->chip.direction_input = gpiomm_gpio_direction_input;
> -	gpiommgpio->chip.direction_output = gpiomm_gpio_direction_output;
> -	gpiommgpio->chip.get = gpiomm_gpio_get;
> -	gpiommgpio->chip.get_multiple = gpiomm_gpio_get_multiple;
> -	gpiommgpio->chip.set = gpiomm_gpio_set;
> -	gpiommgpio->chip.set_multiple = gpiomm_gpio_set_multiple;
> -
> -	i8255_state_init(gpiommgpio->ppi_state, GPIOMM_NUM_PPI);
> -	gpiomm_init_dio(gpiommgpio->ppi, gpiommgpio->ppi_state);
> -
> -	err = devm_gpiochip_add_data(dev, &gpiommgpio->chip, gpiommgpio);
> -	if (err) {
> -		dev_err(dev, "GPIO registering failed (%d)\n", err);
> -		return err;
> -	}
> +	config.map = devm_regmap_init_mmio(dev, regs, &gpiomm_regmap_config);
> +	if (IS_ERR(config.map))
> +		return PTR_ERR(config.map);
> +
> +	config.parent = dev;
> +	config.num_ppi = GPIOMM_NUM_PPI;
> +	config.names = gpiomm_names;
>  
> -	return 0;
> +	return devm_i8255_regmap_register(dev, &config);
>  }
>  
>  static struct isa_driver gpiomm_driver = {
> -- 
> 2.38.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Michael Walle]

Hi,

[sorry this mail was just delivered now, although it seems
to be sent last Tuesday.]

Am 2022-11-22 11:29, schrieb William Breathitt Gray:
> On Wed, Nov 23, 2022 at 05:01:53PM +0200, Andy Shevchenko wrote:
>> On Tue, Nov 22, 2022 at 02:11:00AM -0500, William Breathitt Gray 
>> wrote:
>> > +	/* Initialize device interrupt state */
>> > +	err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
>> > +	if (err)
>> > +		return err;
>> 
>> Use ->init_hw() callback for this.
> 
> In a subsequent patch 7/9 we remove direct gpio_chip registration in
> favor of the i8255 library registration via gpio_regmap. It doesn't 
> look
> like gpio_regmap_register() sets the init_hw() callback.
> 
> Michael, do you see any issues if I introduce init_hw() to
> gpio_regmap_config? Or do you think this IRQ initialization belongs
> somewhere else?

Something like the following?
   gpiochip->init_hw = config.irq_init_hw;

gpiochip doesn't seem to be the correct place, gpiochip_add_irqchip()
is a noop for gpio-regmap, right? So using gpiochip_irqchip_init_hw()
seems wrong.

Maybe make gpio-regmap call it on its own? But really we just connect
the regmap-irq to the gpiochip irqdomain. What is the purpose of the
.init_hw callback? I've looked at other drivers which use regmap-irq
and they all seem to just initialize the hardware in their _probe().

-michael

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]

On Sun, Nov 27, 2022 at 07:31:48PM +0100, Michael Walle wrote:
> Hi,
> 
> [sorry this mail was just delivered now, although it seems
> to be sent last Tuesday.]

Actually I just sent it today, but you're right that it seems to have a
date of last Tuesday, so I must have configured something wrong on my
end before I sent it (sorry).

> 
> Am 2022-11-22 11:29, schrieb William Breathitt Gray:
> > On Wed, Nov 23, 2022 at 05:01:53PM +0200, Andy Shevchenko wrote:
> > > On Tue, Nov 22, 2022 at 02:11:00AM -0500, William Breathitt Gray
> > > wrote:
> > > > +	/* Initialize device interrupt state */
> > > > +	err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
> > > > +	if (err)
> > > > +		return err;
> > > 
> > > Use ->init_hw() callback for this.
> > 
> > In a subsequent patch 7/9 we remove direct gpio_chip registration in
> > favor of the i8255 library registration via gpio_regmap. It doesn't look
> > like gpio_regmap_register() sets the init_hw() callback.
> > 
> > Michael, do you see any issues if I introduce init_hw() to
> > gpio_regmap_config? Or do you think this IRQ initialization belongs
> > somewhere else?
> 
> Something like the following?
>   gpiochip->init_hw = config.irq_init_hw;
> 
> gpiochip doesn't seem to be the correct place, gpiochip_add_irqchip()
> is a noop for gpio-regmap, right? So using gpiochip_irqchip_init_hw()
> seems wrong.
> 
> Maybe make gpio-regmap call it on its own? But really we just connect
> the regmap-irq to the gpiochip irqdomain.

I think you're right, it feels strange to handle IRQ initialization via
the GPIO framework. Maybe somewhere in regmap_irq might be more
appropriate?

> What is the purpose of the
> .init_hw callback? I've looked at other drivers which use regmap-irq
> and they all seem to just initialize the hardware in their _probe().
> 
> -michael

I'm not opposed to initializing the hardware in _probe(), although I can
see merit in pushing that operation instead closer to the framework
where the initialization is actually relevant.

Andy, maybe you can shed some light about .init_hw; I think you
introduced it to gpiolib in commit 9411e3aaa6342.

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Andy Shevchenko]

On Sun, Nov 27, 2022 at 07:31:48PM +0100, Michael Walle wrote:
> Am 2022-11-22 11:29, schrieb William Breathitt Gray:
> > On Wed, Nov 23, 2022 at 05:01:53PM +0200, Andy Shevchenko wrote:
> > > On Tue, Nov 22, 2022 at 02:11:00AM -0500, William Breathitt Gray
> > > wrote:
> > > > +	/* Initialize device interrupt state */
> > > > +	err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val);
> > > > +	if (err)
> > > > +		return err;
> > > 
> > > Use ->init_hw() callback for this.
> > 
> > In a subsequent patch 7/9 we remove direct gpio_chip registration in
> > favor of the i8255 library registration via gpio_regmap. It doesn't look
> > like gpio_regmap_register() sets the init_hw() callback.
> > 
> > Michael, do you see any issues if I introduce init_hw() to
> > gpio_regmap_config? Or do you think this IRQ initialization belongs
> > somewhere else?
> 
> Something like the following?
>   gpiochip->init_hw = config.irq_init_hw;
> 
> gpiochip doesn't seem to be the correct place, gpiochip_add_irqchip()
> is a noop for gpio-regmap, right? So using gpiochip_irqchip_init_hw()
> seems wrong.
> 
> Maybe make gpio-regmap call it on its own? But really we just connect
> the regmap-irq to the gpiochip irqdomain. What is the purpose of the
> .init_hw callback? I've looked at other drivers which use regmap-irq
> and they all seem to just initialize the hardware in their _probe().

The purpose of that callback is to initialize IRQ part of the GPIO hardware
in the appropriate point of time. Of course there are drivers that are using
it and it's not in their ->probe():s, however you can tell that in the same
flow, because it's called synchronously.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Andy Shevchenko]

On Sun, Nov 27, 2022 at 05:00:40PM -0500, William Breathitt Gray wrote:
> On Sun, Nov 27, 2022 at 07:31:48PM +0100, Michael Walle wrote:
> > Am 2022-11-22 11:29, schrieb William Breathitt Gray:

...

> > gpiochip doesn't seem to be the correct place, gpiochip_add_irqchip()
> > is a noop for gpio-regmap, right? So using gpiochip_irqchip_init_hw()
> > seems wrong.
> > 
> > Maybe make gpio-regmap call it on its own? But really we just connect
> > the regmap-irq to the gpiochip irqdomain.
> 
> I think you're right, it feels strange to handle IRQ initialization via
> the GPIO framework. Maybe somewhere in regmap_irq might be more
> appropriate?

The problem that that callback solves is possible interrupt storm, spurious
interrupts, and Use Before Initialized.

If you can guarantee that in your case it never happens, add a comment
and go on.

(It might be useful to tweak code a bit and try CONFIG_DEBUG_SHIRQ=y)

> > What is the purpose of the
> > .init_hw callback? I've looked at other drivers which use regmap-irq
> > and they all seem to just initialize the hardware in their _probe().
> > 
> > -michael
> 
> I'm not opposed to initializing the hardware in _probe(), although I can
> see merit in pushing that operation instead closer to the framework
> where the initialization is actually relevant.
> 
> Andy, maybe you can shed some light about .init_hw; I think you
> introduced it to gpiolib in commit 9411e3aaa6342.

It seems that commit message doesn't fully  explain the situation behind
that change. But it was observed in real life, see above.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Michael Walle]

Am 2022-11-28 10:41, schrieb Andy Shevchenko:
> Of course there are drivers that are using it and it's not in
> their ->probe():s

I was speaking of gpio drivers which use the regmap-irq stuff. I
couldn't find any which are using {devm_,}regmap_add_irq_chip*()
and gpiochip.init_hw().

-michael

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Andy Shevchenko]

On Mon, Nov 28, 2022 at 11:51:10AM +0200, Andy Shevchenko wrote:
> On Sun, Nov 27, 2022 at 05:00:40PM -0500, William Breathitt Gray wrote:
> > On Sun, Nov 27, 2022 at 07:31:48PM +0100, Michael Walle wrote:
> > > Am 2022-11-22 11:29, schrieb William Breathitt Gray:

...

> > > gpiochip doesn't seem to be the correct place, gpiochip_add_irqchip()
> > > is a noop for gpio-regmap, right? So using gpiochip_irqchip_init_hw()
> > > seems wrong.
> > > 
> > > Maybe make gpio-regmap call it on its own? But really we just connect
> > > the regmap-irq to the gpiochip irqdomain.
> > 
> > I think you're right, it feels strange to handle IRQ initialization via
> > the GPIO framework. Maybe somewhere in regmap_irq might be more
> > appropriate?
> 
> The problem that that callback solves is possible interrupt storm, spurious
> interrupts, and Use Before Initialized.
> 
> If you can guarantee that in your case it never happens, add a comment
> and go on.
> 
> (It might be useful to tweak code a bit and try CONFIG_DEBUG_SHIRQ=y)
> 
> > > What is the purpose of the
> > > .init_hw callback? I've looked at other drivers which use regmap-irq
> > > and they all seem to just initialize the hardware in their _probe().
> > > 
> > > -michael
> > 
> > I'm not opposed to initializing the hardware in _probe(), although I can
> > see merit in pushing that operation instead closer to the framework
> > where the initialization is actually relevant.
> > 
> > Andy, maybe you can shed some light about .init_hw; I think you
> > introduced it to gpiolib in commit 9411e3aaa6342.
> 
> It seems that commit message doesn't fully  explain the situation behind
> that change. But it was observed in real life, see above.

FWIW, real life example:
e986f0e602f1 ("pinctrl: intel: fix unexpected interrupt")

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Andy Shevchenko]

On Mon, Nov 28, 2022 at 10:56:06AM +0100, Michael Walle wrote:
> Am 2022-11-28 10:41, schrieb Andy Shevchenko:
> > Of course there are drivers that are using it and it's not in
> > their ->probe():s
> 
> I was speaking of gpio drivers which use the regmap-irq stuff. I
> couldn't find any which are using {devm_,}regmap_add_irq_chip*()
> and gpiochip.init_hw().

Ah, that's true.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/9] gpio: 104-dio-48e: Migrate to the regmap-irq API

[Andy Shevchenko]

On Mon, Nov 28, 2022 at 11:56:15AM +0200, Andy Shevchenko wrote:
> On Mon, Nov 28, 2022 at 11:51:10AM +0200, Andy Shevchenko wrote:
> > On Sun, Nov 27, 2022 at 05:00:40PM -0500, William Breathitt Gray wrote:
> > > On Sun, Nov 27, 2022 at 07:31:48PM +0100, Michael Walle wrote:
> > > > Am 2022-11-22 11:29, schrieb William Breathitt Gray:

...

> > > > gpiochip doesn't seem to be the correct place, gpiochip_add_irqchip()
> > > > is a noop for gpio-regmap, right? So using gpiochip_irqchip_init_hw()
> > > > seems wrong.
> > > > 
> > > > Maybe make gpio-regmap call it on its own? But really we just connect
> > > > the regmap-irq to the gpiochip irqdomain.
> > > 
> > > I think you're right, it feels strange to handle IRQ initialization via
> > > the GPIO framework. Maybe somewhere in regmap_irq might be more
> > > appropriate?
> > 
> > The problem that that callback solves is possible interrupt storm, spurious
> > interrupts, and Use Before Initialized.
> > 
> > If you can guarantee that in your case it never happens, add a comment
> > and go on.
> > 
> > (It might be useful to tweak code a bit and try CONFIG_DEBUG_SHIRQ=y)
> > 
> > > > What is the purpose of the
> > > > .init_hw callback? I've looked at other drivers which use regmap-irq
> > > > and they all seem to just initialize the hardware in their _probe().
> > > > 
> > > > -michael
> > > 
> > > I'm not opposed to initializing the hardware in _probe(), although I can
> > > see merit in pushing that operation instead closer to the framework
> > > where the initialization is actually relevant.
> > > 
> > > Andy, maybe you can shed some light about .init_hw; I think you
> > > introduced it to gpiolib in commit 9411e3aaa6342.
> > 
> > It seems that commit message doesn't fully  explain the situation behind
> > that change. But it was observed in real life, see above.
> 
> FWIW, real life example:
> e986f0e602f1 ("pinctrl: intel: fix unexpected interrupt")

And another one (seems found from SW perspective):
a33912061607 ("gpio: lynxpoint: Move hardware initialization to callback")

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v3 0/9] Migrate i8255 GPIO drivers to regmap API

[Mark Brown]

On Tue, 22 Nov 2022 02:10:57 -0500, William Breathitt Gray wrote:
> Changes in v3:
>  - Changed handle_mask_sync description to show range using a more
>    typical mathematical notation ('[' changed to ')')
>  - Split addition of new i8255 library regmap interface and removal of
>    old i8255 library interface to separate patches
>  - Split migration of 104-dio-48e, 104-idi-48, and gpio-mm modules to
>    regmap-irq, gpio-regmap, and new i8255 library interface to separate
>    patches
>  - Simplified logic in dio48e_handle_mask_sync(); the previous version
>    separated the conditional evaluations from the if statements which
>    made it more complicated than it needed to be
>  - Removed initial interrupt clearing in dio48e_probe(); superfluous
>    because clear_on_unmask is set to true so the interrupts will be
>    cleared anyway later.
>  - Replace IDI48_IRQ_REG with IDI48_IRQ_STATUS and IDI48_IRQ_ENABLE to
>    make the intent of this register clear
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[2/9] regmap-irq: Add handle_mask_sync() callback
      commit: 69af4bcaa08d06fd4d788a7f7193fb3c40ac6aba

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark