diff mbox series

[v3,1/4] gpio: idio-16: Migrate to the regmap API

Message ID dc230046590e27f7c349456a087bc7ddbe635fee.1679693714.git.william.gray@linaro.org
State Superseded
Headers show
Series Migrate IDIO-16 GPIO drivers to regmap API | expand

Commit Message

William Breathitt Gray March 24, 2023, 9:45 p.m. UTC
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 regmap API, the idio-16 library is reduced to simply a
devm_idio_16_regmap_register() function and a configuration structure
struct idio_16_regmap_config.

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

For IDIO-16 devices we have the following IRQ registers:

    Base Address +1 (Write): Clear Interrupt
    Base Address +2 (Read): Enable Interrupt
    Base Address +2 (Write): Disable Interrupt

An interrupt is asserted whenever a change-of-state is detected on any
of the inputs. Any write to 0x2 will disable interrupts, while any read
will enable interrupts. Interrupts are cleared by a write to 0x1.

For 104-IDIO-16 devices, there is no IRQ status register, so software
has to assume that if an interrupt is raised then it was for the
104-IDIO-16 device.

For PCI-IDIO-16 devices, there is an additional IRQ register:

    Base Address +6 (Read): Interrupt Status

Interrupt status can be read from 0x6 where bit 2 set indicates that an
IRQ has been generated.

Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
Changes in v3:
 - Refactor for map parameter removal from handle_mask_sync(); e.g.
   declare struct idio_16_data and utilize for irq_drv_data, inline
   data->map in idio_16_handle_mask_sync(), etc
 - Cleanups and line wrapping to 100 characters rather than 80
 - Change dev_err_probe() to dev_err(); devm_idio_16_regmap_register()
   is technically not a probe() callback
 - Drop superfluous comment line in struct idio_16_state

 drivers/gpio/Kconfig        |   3 +
 drivers/gpio/gpio-idio-16.c | 160 ++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpio-idio-16.h |  26 ++++++
 3 files changed, 189 insertions(+)

Comments

Andy Shevchenko March 27, 2023, 10:44 a.m. UTC | #1
On Fri, Mar 24, 2023 at 05:45:41PM -0400, 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 regmap API, the idio-16 library is reduced to simply a
> devm_idio_16_regmap_register() function and a configuration structure
> struct idio_16_regmap_config.
> 
> Legacy functions and code will be removed once all consumers have
> migrated to the new idio-16 library interface.
> 
> For IDIO-16 devices we have the following IRQ registers:
> 
>     Base Address +1 (Write): Clear Interrupt
>     Base Address +2 (Read): Enable Interrupt
>     Base Address +2 (Write): Disable Interrupt
> 
> An interrupt is asserted whenever a change-of-state is detected on any
> of the inputs. Any write to 0x2 will disable interrupts, while any read
> will enable interrupts. Interrupts are cleared by a write to 0x1.
> 
> For 104-IDIO-16 devices, there is no IRQ status register, so software
> has to assume that if an interrupt is raised then it was for the
> 104-IDIO-16 device.
> 
> For PCI-IDIO-16 devices, there is an additional IRQ register:
> 
>     Base Address +6 (Read): Interrupt Status
> 
> Interrupt status can be read from 0x6 where bit 2 set indicates that an
> IRQ has been generated.

...

> +static int idio_16_reg_mask_xlate(struct gpio_regmap *const gpio, const unsigned int base,
> +				  const unsigned int offset, unsigned int *const reg,
> +				  unsigned int *const mask)
> +{
> +	unsigned int stride;

> +	if (base != IDIO_16_DAT_BASE)
> +		/* Should never reach this path */
> +		return -EINVAL;

Then why do we have this test at all?

> +	/* Input lines start at GPIO 16 */
> +	if (offset < 16) {
> +		stride = offset / IDIO_16_NGPIO_PER_REG;
> +		*reg = IDIO_16_OUT_BASE + stride * IDIO_16_REG_STRIDE;
> +	} else {
> +		stride = (offset - 16) / IDIO_16_NGPIO_PER_REG;
> +		*reg = IDIO_16_IN_BASE + stride * IDIO_16_REG_STRIDE;
> +	}
> +
> +	*mask = BIT(offset % IDIO_16_NGPIO_PER_REG);
> +
> +	return 0;
> +}

...

> +static const char *idio_16_names[IDIO_16_NGPIO] = {
> +	"OUT0", "OUT1", "OUT2", "OUT3", "OUT4", "OUT5", "OUT6", "OUT7", "OUT8", "OUT9", "OUT10",
> +	"OUT11", "OUT12", "OUT13", "OUT14", "OUT15", "IIN0", "IIN1", "IIN2", "IIN3", "IIN4", "IIN5",
> +	"IIN6", "IIN7", "IIN8", "IIN9", "IIN10", "IIN11", "IIN12", "IIN13", "IIN14", "IIN15",

I believe this would look much better if formatted on the 8 basis per line.

> +};

...

> +int devm_idio_16_regmap_register(struct device *const dev,
> +				 const struct idio_16_regmap_config *const config)
> +{
> +	struct gpio_regmap_config gpio_config = {};
> +	int err;
> +	struct idio_16_data *data;
> +	struct regmap_irq_chip *chip;
> +	struct regmap_irq_chip_data *chip_data;
> +
> +	if (!config->parent)
> +		return -EINVAL;
> +
> +	if (!config->map)
> +		return -EINVAL;
> +
> +	if (!config->regmap_irqs)
> +		return -EINVAL;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +	data->map = config->map;
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	chip->name = dev_name(dev);
> +	chip->status_base = IDIO_16_INTERRUPT_STATUS;
> +	chip->mask_base = IDIO_16_ENABLE_IRQ;
> +	chip->ack_base = IDIO_16_CLEAR_INTERRUPT;
> +	chip->no_status = config->no_status;
> +	chip->num_regs = 1;
> +	chip->irqs = config->regmap_irqs;
> +	chip->num_irqs = config->num_regmap_irqs;
> +	chip->handle_mask_sync = idio_16_handle_mask_sync;
> +	chip->irq_drv_data = data;
> +
> +	/* Disable IRQ to prevent spurious interrupts before we're ready */
> +	err = regmap_write(data->map, IDIO_16_DISABLE_IRQ, 0x00);
> +	if (err)
> +		return err;
> +
> +	err = devm_regmap_add_irq_chip(dev, data->map, config->irq, 0, 0, chip, &chip_data);
> +	if (err) {

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

		return dev_err_probe(...);

devm_*() can't be called otherwise. If it's not the case the caller abuses
devm_*() to begin with. So, it's the _part_ of the ->probe() sequence.

> +	}
> +
> +	if (config->filters) {
> +		/* Deactivate input filters */
> +		err = regmap_write(data->map, IDIO_16_DEACTIVATE_INPUT_FILTERS, 0x00);
> +		if (err)
> +			return err;
> +	}
> +
> +	gpio_config.parent = config->parent;
> +	gpio_config.regmap = data->map;
> +	gpio_config.ngpio = IDIO_16_NGPIO;
> +	gpio_config.names = idio_16_names;
> +	gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(IDIO_16_DAT_BASE);
> +	gpio_config.reg_set_base = GPIO_REGMAP_ADDR(IDIO_16_DAT_BASE);
> +	gpio_config.ngpio_per_reg = IDIO_16_NGPIO_PER_REG;
> +	gpio_config.reg_stride = IDIO_16_REG_STRIDE;
> +	gpio_config.irq_domain = regmap_irq_get_domain(chip_data);
> +	gpio_config.reg_mask_xlate = idio_16_reg_mask_xlate;
> +
> +	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
> +}
diff mbox series

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 68f58b0ba79f..415b86cfd1a9 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -111,6 +111,9 @@  config GPIO_MAX730X
 
 config GPIO_IDIO_16
 	tristate
+	select REGMAP_IRQ
+	select GPIOLIB_IRQCHIP
+	select GPIO_REGMAP
 	help
 	  Enables support for the idio-16 library functions. The idio-16 library
 	  provides functions to facilitate communication with devices within the
diff --git a/drivers/gpio/gpio-idio-16.c b/drivers/gpio/gpio-idio-16.c
index 13315242d220..e8507f16b1c2 100644
--- a/drivers/gpio/gpio-idio-16.c
+++ b/drivers/gpio/gpio-idio-16.c
@@ -4,9 +4,13 @@ 
  * 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>
 
@@ -14,6 +18,162 @@ 
 
 #define DEFAULT_SYMBOL_NAMESPACE GPIO_IDIO_16
 
+#define IDIO_16_DAT_BASE 0x0
+#define IDIO_16_OUT_BASE IDIO_16_DAT_BASE
+#define IDIO_16_IN_BASE (IDIO_16_DAT_BASE + 1)
+#define IDIO_16_CLEAR_INTERRUPT 0x1
+#define IDIO_16_ENABLE_IRQ 0x2
+#define IDIO_16_DEACTIVATE_INPUT_FILTERS 0x3
+#define IDIO_16_DISABLE_IRQ IDIO_16_ENABLE_IRQ
+#define IDIO_16_INTERRUPT_STATUS 0x6
+
+#define IDIO_16_NGPIO 32
+#define IDIO_16_NGPIO_PER_REG 8
+#define IDIO_16_REG_STRIDE 4
+
+struct idio_16_data {
+	struct regmap *map;
+	unsigned int irq_mask;
+};
+
+static int idio_16_handle_mask_sync(const int index, const unsigned int mask_buf_def,
+				    const unsigned int mask_buf, void *const irq_drv_data)
+{
+	struct idio_16_data *const data = irq_drv_data;
+	const unsigned int prev_mask = data->irq_mask;
+	int err;
+	unsigned int val;
+
+	/* 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 */
+	data->irq_mask = mask_buf;
+
+	/* if all previously masked, enable interrupts when unmasking */
+	if (prev_mask == mask_buf_def) {
+		err = regmap_write(data->map, IDIO_16_CLEAR_INTERRUPT, 0x00);
+		if (err)
+			return err;
+		return regmap_read(data->map, IDIO_16_ENABLE_IRQ, &val);
+	}
+
+	/* if all are currently masked, disable interrupts */
+	if (mask_buf == mask_buf_def)
+		return regmap_write(data->map, IDIO_16_DISABLE_IRQ, 0x00);
+
+	return 0;
+}
+
+static int idio_16_reg_mask_xlate(struct gpio_regmap *const gpio, const unsigned int base,
+				  const unsigned int offset, unsigned int *const reg,
+				  unsigned int *const mask)
+{
+	unsigned int stride;
+
+	if (base != IDIO_16_DAT_BASE)
+		/* Should never reach this path */
+		return -EINVAL;
+
+	/* Input lines start at GPIO 16 */
+	if (offset < 16) {
+		stride = offset / IDIO_16_NGPIO_PER_REG;
+		*reg = IDIO_16_OUT_BASE + stride * IDIO_16_REG_STRIDE;
+	} else {
+		stride = (offset - 16) / IDIO_16_NGPIO_PER_REG;
+		*reg = IDIO_16_IN_BASE + stride * IDIO_16_REG_STRIDE;
+	}
+
+	*mask = BIT(offset % IDIO_16_NGPIO_PER_REG);
+
+	return 0;
+}
+
+static const char *idio_16_names[IDIO_16_NGPIO] = {
+	"OUT0", "OUT1", "OUT2", "OUT3", "OUT4", "OUT5", "OUT6", "OUT7", "OUT8", "OUT9", "OUT10",
+	"OUT11", "OUT12", "OUT13", "OUT14", "OUT15", "IIN0", "IIN1", "IIN2", "IIN3", "IIN4", "IIN5",
+	"IIN6", "IIN7", "IIN8", "IIN9", "IIN10", "IIN11", "IIN12", "IIN13", "IIN14", "IIN15",
+};
+
+/**
+ * devm_idio_16_regmap_register - Register an IDIO-16 GPIO device
+ * @dev:	device that is registering this IDIO-16 GPIO device
+ * @config:	configuration for idio_16_regmap_config
+ *
+ * Registers an IDIO-16 GPIO device. Returns 0 on success and negative error number on failure.
+ */
+int devm_idio_16_regmap_register(struct device *const dev,
+				 const struct idio_16_regmap_config *const config)
+{
+	struct gpio_regmap_config gpio_config = {};
+	int err;
+	struct idio_16_data *data;
+	struct regmap_irq_chip *chip;
+	struct regmap_irq_chip_data *chip_data;
+
+	if (!config->parent)
+		return -EINVAL;
+
+	if (!config->map)
+		return -EINVAL;
+
+	if (!config->regmap_irqs)
+		return -EINVAL;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	data->map = config->map;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->name = dev_name(dev);
+	chip->status_base = IDIO_16_INTERRUPT_STATUS;
+	chip->mask_base = IDIO_16_ENABLE_IRQ;
+	chip->ack_base = IDIO_16_CLEAR_INTERRUPT;
+	chip->no_status = config->no_status;
+	chip->num_regs = 1;
+	chip->irqs = config->regmap_irqs;
+	chip->num_irqs = config->num_regmap_irqs;
+	chip->handle_mask_sync = idio_16_handle_mask_sync;
+	chip->irq_drv_data = data;
+
+	/* Disable IRQ to prevent spurious interrupts before we're ready */
+	err = regmap_write(data->map, IDIO_16_DISABLE_IRQ, 0x00);
+	if (err)
+		return err;
+
+	err = devm_regmap_add_irq_chip(dev, data->map, config->irq, 0, 0, chip, &chip_data);
+	if (err) {
+		dev_err(dev, "IRQ registration failed (%d)\n", err);
+		return err;
+	}
+
+	if (config->filters) {
+		/* Deactivate input filters */
+		err = regmap_write(data->map, IDIO_16_DEACTIVATE_INPUT_FILTERS, 0x00);
+		if (err)
+			return err;
+	}
+
+	gpio_config.parent = config->parent;
+	gpio_config.regmap = data->map;
+	gpio_config.ngpio = IDIO_16_NGPIO;
+	gpio_config.names = idio_16_names;
+	gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(IDIO_16_DAT_BASE);
+	gpio_config.reg_set_base = GPIO_REGMAP_ADDR(IDIO_16_DAT_BASE);
+	gpio_config.ngpio_per_reg = IDIO_16_NGPIO_PER_REG;
+	gpio_config.reg_stride = IDIO_16_REG_STRIDE;
+	gpio_config.irq_domain = regmap_irq_get_domain(chip_data);
+	gpio_config.reg_mask_xlate = idio_16_reg_mask_xlate;
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
+}
+EXPORT_SYMBOL_GPL(devm_idio_16_regmap_register);
+
 /**
  * idio_16_get - get signal value at signal offset
  * @reg:	ACCES IDIO-16 device registers
diff --git a/drivers/gpio/gpio-idio-16.h b/drivers/gpio/gpio-idio-16.h
index 928f8251a2bd..255bd8504ed7 100644
--- a/drivers/gpio/gpio-idio-16.h
+++ b/drivers/gpio/gpio-idio-16.h
@@ -6,6 +6,30 @@ 
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
+struct device;
+struct regmap;
+struct regmap_irq;
+
+/**
+ * struct idio_16_regmap_config - Configuration for the IDIO-16 register map
+ * @parent:		parent device
+ * @map:		regmap for the IDIO-16 device
+ * @regmap_irqs:	descriptors for individual IRQs
+ * @num_regmap_irqs:	number of IRQ descriptors
+ * @irq:		IRQ number for the IDIO-16 device
+ * @no_status:		device has no status register
+ * @filters:		device has input filters
+ */
+struct idio_16_regmap_config {
+	struct device *parent;
+	struct regmap *map;
+	const struct regmap_irq *regmap_irqs;
+	int num_regmap_irqs;
+	unsigned int irq;
+	bool no_status;
+	bool filters;
+};
+
 /**
  * struct idio_16 - IDIO-16 registers structure
  * @out0_7:	Read: FET Drive Outputs 0-7
@@ -68,4 +92,6 @@  void idio_16_set_multiple(struct idio_16 __iomem *reg,
 			  const unsigned long *mask, const unsigned long *bits);
 void idio_16_state_init(struct idio_16_state *state);
 
+int devm_idio_16_regmap_register(struct device *dev, const struct idio_16_regmap_config *config);
+
 #endif /* _IDIO_16_H_ */