2023-03-31 21:08:11

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v4 0/4] Migrate IDIO-16 GPIO drivers to regmap API

Changes in v4:
- Remove superfluous base check in idio_16_reg_mask_xlate()
- Format idio_16_names[] to 8 GPIO per line
- Utilize dev_err_probe() in idio_16_regmap_register()

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-idio-16 and pci-idio-16 modules depend
on the IDIO-16 library and are thus updated accordingly.

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.

This patchset depends on the "Drop map from handle_mask_sync()
parameters" patchset [0].

[0] https://lore.kernel.org/all/[email protected]/

William Breathitt Gray (4):
gpio: idio-16: Migrate to the regmap API
gpio: 104-idio-16: Migrate to the regmap API
gpio: pci-idio-16: Migrate to the regmap API
gpio: idio-16: Remove unused legacy interface

drivers/gpio/Kconfig | 7 +-
drivers/gpio/gpio-104-idio-16.c | 286 +++++++------------------------
drivers/gpio/gpio-idio-16.c | 254 ++++++++++++++-------------
drivers/gpio/gpio-idio-16.h | 79 +++------
drivers/gpio/gpio-pci-idio-16.c | 294 +++++++-------------------------
5 files changed, 289 insertions(+), 631 deletions(-)


base-commit: 4ea0c97776bf8c63805eb0f8182d9c20072219d9
prerequisite-patch-id: cd19046150b7cff1be4ac7152198777aa960a3df
prerequisite-patch-id: bd3e3830d9ce4f3876a77483364d7190b7fdffa7
--
2.39.2


2023-03-31 21:08:25

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v4 3/4] gpio: pci-idio-16: Migrate to the regmap API

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. Migrate the pci-idio-16 module to the new
idio-16 library interface leveraging the gpio-regmap API.

Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: William Breathitt Gray <[email protected]>
---
Changes in v4: none

drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-pci-idio-16.c | 294 +++++++-------------------------
2 files changed, 62 insertions(+), 234 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9f7ec4f3fdbf..e19b2612e67c 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1580,7 +1580,7 @@ config GPIO_PCH

config GPIO_PCI_IDIO_16
tristate "ACCES PCI-IDIO-16 GPIO support"
- select GPIOLIB_IRQCHIP
+ select REGMAP_MMIO
select GPIO_IDIO_16
help
Enables GPIO support for the ACCES PCI-IDIO-16. An interrupt is
diff --git a/drivers/gpio/gpio-pci-idio-16.c b/drivers/gpio/gpio-pci-idio-16.c
index 6726c32e31e6..5da67e0c83ff 100644
--- a/drivers/gpio/gpio-pci-idio-16.c
+++ b/drivers/gpio/gpio-pci-idio-16.c
@@ -5,214 +5,75 @@
*/
#include <linux/bits.h>
#include <linux/device.h>
-#include <linux/errno.h>
-#include <linux/gpio/driver.h>
-#include <linux/interrupt.h>
-#include <linux/irqdesc.h>
+#include <linux/err.h>
+#include <linux/irq.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
-#include <linux/spinlock.h>
+#include <linux/regmap.h>
#include <linux/types.h>

#include "gpio-idio-16.h"

-/**
- * struct idio_16_gpio - GPIO device private data structure
- * @chip: instance of the gpio_chip
- * @lock: synchronization lock to prevent I/O race conditions
- * @reg: I/O address offset for the GPIO device registers
- * @state: ACCES IDIO-16 device state
- * @irq_mask: I/O bits affected by interrupts
- */
-struct idio_16_gpio {
- struct gpio_chip chip;
- raw_spinlock_t lock;
- struct idio_16 __iomem *reg;
- struct idio_16_state state;
- unsigned long irq_mask;
+static const struct regmap_range idio_16_wr_ranges[] = {
+ regmap_reg_range(0x0, 0x2), regmap_reg_range(0x3, 0x4),
};
-
-static int idio_16_gpio_get_direction(struct gpio_chip *chip,
- unsigned int offset)
-{
- if (idio_16_get_direction(offset))
- return GPIO_LINE_DIRECTION_IN;
-
- return GPIO_LINE_DIRECTION_OUT;
-}
-
-static int idio_16_gpio_direction_input(struct gpio_chip *chip,
- unsigned int offset)
-{
- return 0;
-}
-
-static int idio_16_gpio_direction_output(struct gpio_chip *chip,
- unsigned int offset, int value)
-{
- chip->set(chip, offset, value);
- return 0;
-}
-
-static int idio_16_gpio_get(struct gpio_chip *chip, unsigned int offset)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- return idio_16_get(idio16gpio->reg, &idio16gpio->state, offset);
-}
-
-static int idio_16_gpio_get_multiple(struct gpio_chip *chip,
- unsigned long *mask, unsigned long *bits)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- idio_16_get_multiple(idio16gpio->reg, &idio16gpio->state, mask, bits);
- return 0;
-}
-
-static void idio_16_gpio_set(struct gpio_chip *chip, unsigned int offset,
- int value)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- idio_16_set(idio16gpio->reg, &idio16gpio->state, offset, value);
-}
-
-static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
- unsigned long *mask, unsigned long *bits)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- idio_16_set_multiple(idio16gpio->reg, &idio16gpio->state, mask, bits);
-}
-
-static void idio_16_irq_ack(struct irq_data *data)
-{
-}
-
-static void idio_16_irq_mask(struct irq_data *data)
-{
- struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
- const unsigned long mask = BIT(irqd_to_hwirq(data));
- unsigned long flags;
-
- idio16gpio->irq_mask &= ~mask;
-
- if (!idio16gpio->irq_mask) {
- raw_spin_lock_irqsave(&idio16gpio->lock, flags);
-
- iowrite8(0, &idio16gpio->reg->irq_ctl);
-
- raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
- }
-
- gpiochip_disable_irq(chip, irqd_to_hwirq(data));
-}
-
-static void idio_16_irq_unmask(struct irq_data *data)
-{
- struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
- const unsigned long mask = BIT(irqd_to_hwirq(data));
- const unsigned long prev_irq_mask = idio16gpio->irq_mask;
- unsigned long flags;
-
- gpiochip_enable_irq(chip, irqd_to_hwirq(data));
-
- idio16gpio->irq_mask |= mask;
-
- if (!prev_irq_mask) {
- raw_spin_lock_irqsave(&idio16gpio->lock, flags);
-
- ioread8(&idio16gpio->reg->irq_ctl);
-
- raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
- }
-}
-
-static int idio_16_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 idio_16_irqchip = {
- .name = "pci-idio-16",
- .irq_ack = idio_16_irq_ack,
- .irq_mask = idio_16_irq_mask,
- .irq_unmask = idio_16_irq_unmask,
- .irq_set_type = idio_16_irq_set_type,
- .flags = IRQCHIP_IMMUTABLE,
- GPIOCHIP_IRQ_RESOURCE_HELPERS,
+static const struct regmap_range idio_16_rd_ranges[] = {
+ regmap_reg_range(0x1, 0x2), regmap_reg_range(0x5, 0x6),
};
-
-static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
-{
- struct idio_16_gpio *const idio16gpio = dev_id;
- unsigned int irq_status;
- struct gpio_chip *const chip = &idio16gpio->chip;
- int gpio;
-
- raw_spin_lock(&idio16gpio->lock);
-
- irq_status = ioread8(&idio16gpio->reg->irq_status);
-
- raw_spin_unlock(&idio16gpio->lock);
-
- /* Make sure our device generated IRQ */
- if (!(irq_status & 0x3) || !(irq_status & 0x4))
- return IRQ_NONE;
-
- for_each_set_bit(gpio, &idio16gpio->irq_mask, chip->ngpio)
- generic_handle_domain_irq(chip->irq.domain, gpio);
-
- raw_spin_lock(&idio16gpio->lock);
-
- /* Clear interrupt */
- iowrite8(0, &idio16gpio->reg->in0_7);
-
- raw_spin_unlock(&idio16gpio->lock);
-
- return IRQ_HANDLED;
-}
-
-#define IDIO_16_NGPIO 32
-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"
+static const struct regmap_range idio_16_precious_ranges[] = {
+ regmap_reg_range(0x2, 0x2),
+};
+static const struct regmap_access_table idio_16_wr_table = {
+ .yes_ranges = idio_16_wr_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_16_wr_ranges),
+};
+static const struct regmap_access_table idio_16_rd_table = {
+ .yes_ranges = idio_16_rd_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_16_rd_ranges),
+};
+static const struct regmap_access_table idio_16_precious_table = {
+ .yes_ranges = idio_16_precious_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_16_precious_ranges),
+};
+static const struct regmap_config idio_16_regmap_config = {
+ .reg_bits = 8,
+ .reg_stride = 1,
+ .val_bits = 8,
+ .io_port = true,
+ .max_register = 0x6,
+ .wr_table = &idio_16_wr_table,
+ .rd_table = &idio_16_rd_table,
+ .volatile_table = &idio_16_rd_table,
+ .precious_table = &idio_16_precious_table,
+ .cache_type = REGCACHE_FLAT,
};

-static int idio_16_irq_init_hw(struct gpio_chip *gc)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(gc);
-
- /* Disable IRQ by default and clear any pending interrupt */
- iowrite8(0, &idio16gpio->reg->irq_ctl);
- iowrite8(0, &idio16gpio->reg->in0_7);
+/* Only input lines (GPIO 16-31) support interrupts */
+#define IDIO_16_REGMAP_IRQ(_id) \
+ [16 + _id] = { \
+ .mask = BIT(2), \
+ .type = { .types_supported = IRQ_TYPE_EDGE_BOTH }, \
+ }

- return 0;
-}
+static const struct regmap_irq idio_16_regmap_irqs[] = {
+ IDIO_16_REGMAP_IRQ(0), IDIO_16_REGMAP_IRQ(1), IDIO_16_REGMAP_IRQ(2), /* 0-2 */
+ IDIO_16_REGMAP_IRQ(3), IDIO_16_REGMAP_IRQ(4), IDIO_16_REGMAP_IRQ(5), /* 3-5 */
+ IDIO_16_REGMAP_IRQ(6), IDIO_16_REGMAP_IRQ(7), IDIO_16_REGMAP_IRQ(8), /* 6-8 */
+ IDIO_16_REGMAP_IRQ(9), IDIO_16_REGMAP_IRQ(10), IDIO_16_REGMAP_IRQ(11), /* 9-11 */
+ IDIO_16_REGMAP_IRQ(12), IDIO_16_REGMAP_IRQ(13), IDIO_16_REGMAP_IRQ(14), /* 12-14 */
+ IDIO_16_REGMAP_IRQ(15), /* 15 */
+};

static int idio_16_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct device *const dev = &pdev->dev;
- struct idio_16_gpio *idio16gpio;
int err;
const size_t pci_bar_index = 2;
const char *const name = pci_name(pdev);
- struct gpio_irq_chip *girq;
-
- idio16gpio = devm_kzalloc(dev, sizeof(*idio16gpio), GFP_KERNEL);
- if (!idio16gpio)
- return -ENOMEM;
+ struct idio_16_regmap_config config = {};
+ void __iomem *regs;
+ struct regmap *map;

err = pcim_enable_device(pdev);
if (err) {
@@ -226,53 +87,20 @@ static int idio_16_probe(struct pci_dev *pdev, const struct pci_device_id *id)
return err;
}

- idio16gpio->reg = pcim_iomap_table(pdev)[pci_bar_index];
+ regs = pcim_iomap_table(pdev)[pci_bar_index];

- /* Deactivate input filters */
- iowrite8(0, &idio16gpio->reg->filter_ctl);
+ map = devm_regmap_init_mmio(dev, regs, &idio_16_regmap_config);
+ if (IS_ERR(map))
+ return dev_err_probe(dev, PTR_ERR(map), "Unable to initialize register map\n");

- idio16gpio->chip.label = name;
- idio16gpio->chip.parent = dev;
- idio16gpio->chip.owner = THIS_MODULE;
- idio16gpio->chip.base = -1;
- idio16gpio->chip.ngpio = IDIO_16_NGPIO;
- idio16gpio->chip.names = idio_16_names;
- idio16gpio->chip.get_direction = idio_16_gpio_get_direction;
- idio16gpio->chip.direction_input = idio_16_gpio_direction_input;
- idio16gpio->chip.direction_output = idio_16_gpio_direction_output;
- idio16gpio->chip.get = idio_16_gpio_get;
- idio16gpio->chip.get_multiple = idio_16_gpio_get_multiple;
- idio16gpio->chip.set = idio_16_gpio_set;
- idio16gpio->chip.set_multiple = idio_16_gpio_set_multiple;
-
- idio_16_state_init(&idio16gpio->state);
-
- girq = &idio16gpio->chip.irq;
- gpio_irq_chip_set_chip(girq, &idio_16_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 = idio_16_irq_init_hw;
-
- raw_spin_lock_init(&idio16gpio->lock);
-
- err = devm_gpiochip_add_data(dev, &idio16gpio->chip, idio16gpio);
- if (err) {
- dev_err(dev, "GPIO registering failed (%d)\n", err);
- return err;
- }
-
- err = devm_request_irq(dev, pdev->irq, idio_16_irq_handler, IRQF_SHARED,
- name, idio16gpio);
- if (err) {
- dev_err(dev, "IRQ handler registering failed (%d)\n", err);
- return err;
- }
+ config.parent = dev;
+ config.map = map;
+ config.regmap_irqs = idio_16_regmap_irqs;
+ config.num_regmap_irqs = ARRAY_SIZE(idio_16_regmap_irqs);
+ config.irq = pdev->irq;
+ config.filters = true;

- return 0;
+ return devm_idio_16_regmap_register(dev, &config);
}

static const struct pci_device_id idio_16_pci_dev_id[] = {
--
2.39.2

2023-03-31 21:08:30

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v4 2/4] gpio: 104-idio-16: Migrate to the regmap API

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. Migrate the 104-idio-16 module to the new
idio-16 library interface leveraging the gpio-regmap API.

Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: William Breathitt Gray <[email protected]>
---
Changes in v4: none

drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-104-idio-16.c | 286 +++++++-------------------------
2 files changed, 64 insertions(+), 224 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 415b86cfd1a9..9f7ec4f3fdbf 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -860,7 +860,7 @@ config GPIO_104_IDIO_16
tristate "ACCES 104-IDIO-16 GPIO support"
depends on PC104
select ISA_BUS_API
- select GPIOLIB_IRQCHIP
+ select REGMAP_MMIO
select GPIO_IDIO_16
help
Enables GPIO support for the ACCES 104-IDIO-16 family (104-IDIO-16,
diff --git a/drivers/gpio/gpio-104-idio-16.c b/drivers/gpio/gpio-104-idio-16.c
index 098fbefdbe22..21035dadee46 100644
--- a/drivers/gpio/gpio-104-idio-16.c
+++ b/drivers/gpio/gpio-104-idio-16.c
@@ -6,19 +6,16 @@
* This driver supports the following ACCES devices: 104-IDIO-16,
* 104-IDIO-16E, 104-IDO-16, 104-IDIO-8, 104-IDIO-8E, and 104-IDO-8.
*/
-#include <linux/bitmap.h>
+#include <linux/bits.h>
#include <linux/device.h>
-#include <linux/errno.h>
-#include <linux/gpio/driver.h>
-#include <linux/io.h>
+#include <linux/err.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-idio-16.h"
@@ -36,187 +33,62 @@ static unsigned int num_irq;
module_param_hw_array(irq, uint, irq, &num_irq, 0);
MODULE_PARM_DESC(irq, "ACCES 104-IDIO-16 interrupt line numbers");

-/**
- * struct idio_16_gpio - GPIO device private data structure
- * @chip: instance of the gpio_chip
- * @lock: synchronization lock to prevent I/O race conditions
- * @irq_mask: I/O bits affected by interrupts
- * @reg: I/O address offset for the device registers
- * @state: ACCES IDIO-16 device state
- */
-struct idio_16_gpio {
- struct gpio_chip chip;
- raw_spinlock_t lock;
- unsigned long irq_mask;
- struct idio_16 __iomem *reg;
- struct idio_16_state state;
+static const struct regmap_range idio_16_wr_ranges[] = {
+ regmap_reg_range(0x0, 0x2), regmap_reg_range(0x4, 0x4),
};
-
-static int idio_16_gpio_get_direction(struct gpio_chip *chip,
- unsigned int offset)
-{
- if (idio_16_get_direction(offset))
- return GPIO_LINE_DIRECTION_IN;
-
- return GPIO_LINE_DIRECTION_OUT;
-}
-
-static int idio_16_gpio_direction_input(struct gpio_chip *chip,
- unsigned int offset)
-{
- return 0;
-}
-
-static int idio_16_gpio_direction_output(struct gpio_chip *chip,
- unsigned int offset, int value)
-{
- chip->set(chip, offset, value);
- return 0;
-}
-
-static int idio_16_gpio_get(struct gpio_chip *chip, unsigned int offset)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- return idio_16_get(idio16gpio->reg, &idio16gpio->state, offset);
-}
-
-static int idio_16_gpio_get_multiple(struct gpio_chip *chip,
- unsigned long *mask, unsigned long *bits)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- idio_16_get_multiple(idio16gpio->reg, &idio16gpio->state, mask, bits);
-
- return 0;
-}
-
-static void idio_16_gpio_set(struct gpio_chip *chip, unsigned int offset,
- int value)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- idio_16_set(idio16gpio->reg, &idio16gpio->state, offset, value);
-}
-
-static void idio_16_gpio_set_multiple(struct gpio_chip *chip,
- unsigned long *mask, unsigned long *bits)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
-
- idio_16_set_multiple(idio16gpio->reg, &idio16gpio->state, mask, bits);
-}
-
-static void idio_16_irq_ack(struct irq_data *data)
-{
-}
-
-static void idio_16_irq_mask(struct irq_data *data)
-{
- struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
- const unsigned long offset = irqd_to_hwirq(data);
- unsigned long flags;
-
- idio16gpio->irq_mask &= ~BIT(offset);
- gpiochip_disable_irq(chip, offset);
-
- if (!idio16gpio->irq_mask) {
- raw_spin_lock_irqsave(&idio16gpio->lock, flags);
-
- iowrite8(0, &idio16gpio->reg->irq_ctl);
-
- raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
- }
-}
-
-static void idio_16_irq_unmask(struct irq_data *data)
-{
- struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip);
- const unsigned long offset = irqd_to_hwirq(data);
- const unsigned long prev_irq_mask = idio16gpio->irq_mask;
- unsigned long flags;
-
- gpiochip_enable_irq(chip, offset);
- idio16gpio->irq_mask |= BIT(offset);
-
- if (!prev_irq_mask) {
- raw_spin_lock_irqsave(&idio16gpio->lock, flags);
-
- ioread8(&idio16gpio->reg->irq_ctl);
-
- raw_spin_unlock_irqrestore(&idio16gpio->lock, flags);
- }
-}
-
-static int idio_16_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 idio_16_irqchip = {
- .name = "104-idio-16",
- .irq_ack = idio_16_irq_ack,
- .irq_mask = idio_16_irq_mask,
- .irq_unmask = idio_16_irq_unmask,
- .irq_set_type = idio_16_irq_set_type,
- .flags = IRQCHIP_IMMUTABLE,
- GPIOCHIP_IRQ_RESOURCE_HELPERS,
+static const struct regmap_range idio_16_rd_ranges[] = {
+ regmap_reg_range(0x1, 0x2), regmap_reg_range(0x5, 0x5),
};
-
-static irqreturn_t idio_16_irq_handler(int irq, void *dev_id)
-{
- struct idio_16_gpio *const idio16gpio = dev_id;
- struct gpio_chip *const chip = &idio16gpio->chip;
- int gpio;
-
- for_each_set_bit(gpio, &idio16gpio->irq_mask, chip->ngpio)
- generic_handle_domain_irq(chip->irq.domain, gpio);
-
- raw_spin_lock(&idio16gpio->lock);
-
- iowrite8(0, &idio16gpio->reg->in0_7);
-
- raw_spin_unlock(&idio16gpio->lock);
-
- return IRQ_HANDLED;
-}
-
-#define IDIO_16_NGPIO 32
-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"
+static const struct regmap_range idio_16_precious_ranges[] = {
+ regmap_reg_range(0x2, 0x2),
+};
+static const struct regmap_access_table idio_16_wr_table = {
+ .yes_ranges = idio_16_wr_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_16_wr_ranges),
+};
+static const struct regmap_access_table idio_16_rd_table = {
+ .yes_ranges = idio_16_rd_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_16_rd_ranges),
+};
+static const struct regmap_access_table idio_16_precious_table = {
+ .yes_ranges = idio_16_precious_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_16_precious_ranges),
+};
+static const struct regmap_config idio_16_regmap_config = {
+ .reg_bits = 8,
+ .reg_stride = 1,
+ .val_bits = 8,
+ .io_port = true,
+ .max_register = 0x5,
+ .wr_table = &idio_16_wr_table,
+ .rd_table = &idio_16_rd_table,
+ .volatile_table = &idio_16_rd_table,
+ .precious_table = &idio_16_precious_table,
+ .cache_type = REGCACHE_FLAT,
};

-static int idio_16_irq_init_hw(struct gpio_chip *gc)
-{
- struct idio_16_gpio *const idio16gpio = gpiochip_get_data(gc);
-
- /* Disable IRQ by default */
- iowrite8(0, &idio16gpio->reg->irq_ctl);
- iowrite8(0, &idio16gpio->reg->in0_7);
+/* Only input lines (GPIO 16-31) support interrupts */
+#define IDIO_16_REGMAP_IRQ(_id) \
+ [16 + _id] = { \
+ .mask = BIT(_id), \
+ .type = { .types_supported = IRQ_TYPE_EDGE_BOTH }, \
+ }

- return 0;
-}
+static const struct regmap_irq idio_16_regmap_irqs[] = {
+ IDIO_16_REGMAP_IRQ(0), IDIO_16_REGMAP_IRQ(1), IDIO_16_REGMAP_IRQ(2), /* 0-2 */
+ IDIO_16_REGMAP_IRQ(3), IDIO_16_REGMAP_IRQ(4), IDIO_16_REGMAP_IRQ(5), /* 3-5 */
+ IDIO_16_REGMAP_IRQ(6), IDIO_16_REGMAP_IRQ(7), IDIO_16_REGMAP_IRQ(8), /* 6-8 */
+ IDIO_16_REGMAP_IRQ(9), IDIO_16_REGMAP_IRQ(10), IDIO_16_REGMAP_IRQ(11), /* 9-11 */
+ IDIO_16_REGMAP_IRQ(12), IDIO_16_REGMAP_IRQ(13), IDIO_16_REGMAP_IRQ(14), /* 12-14 */
+ IDIO_16_REGMAP_IRQ(15), /* 15 */
+};

static int idio_16_probe(struct device *dev, unsigned int id)
{
- struct idio_16_gpio *idio16gpio;
const char *const name = dev_name(dev);
- struct gpio_irq_chip *girq;
- int err;
-
- idio16gpio = devm_kzalloc(dev, sizeof(*idio16gpio), GFP_KERNEL);
- if (!idio16gpio)
- return -ENOMEM;
+ struct idio_16_regmap_config config = {};
+ void __iomem *regs;
+ struct regmap *map;

if (!devm_request_region(dev, base[id], IDIO_16_EXTENT, name)) {
dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
@@ -224,54 +96,22 @@ static int idio_16_probe(struct device *dev, unsigned int id)
return -EBUSY;
}

- idio16gpio->reg = devm_ioport_map(dev, base[id], IDIO_16_EXTENT);
- if (!idio16gpio->reg)
+ regs = devm_ioport_map(dev, base[id], IDIO_16_EXTENT);
+ if (!regs)
return -ENOMEM;

- idio16gpio->chip.label = name;
- idio16gpio->chip.parent = dev;
- idio16gpio->chip.owner = THIS_MODULE;
- idio16gpio->chip.base = -1;
- idio16gpio->chip.ngpio = IDIO_16_NGPIO;
- idio16gpio->chip.names = idio_16_names;
- idio16gpio->chip.get_direction = idio_16_gpio_get_direction;
- idio16gpio->chip.direction_input = idio_16_gpio_direction_input;
- idio16gpio->chip.direction_output = idio_16_gpio_direction_output;
- idio16gpio->chip.get = idio_16_gpio_get;
- idio16gpio->chip.get_multiple = idio_16_gpio_get_multiple;
- idio16gpio->chip.set = idio_16_gpio_set;
- idio16gpio->chip.set_multiple = idio_16_gpio_set_multiple;
-
- idio_16_state_init(&idio16gpio->state);
- /* FET off states are represented by bit values of "1" */
- bitmap_fill(idio16gpio->state.out_state, IDIO_16_NOUT);
-
- girq = &idio16gpio->chip.irq;
- gpio_irq_chip_set_chip(girq, &idio_16_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 = idio_16_irq_init_hw;
+ map = devm_regmap_init_mmio(dev, regs, &idio_16_regmap_config);
+ if (IS_ERR(map))
+ return dev_err_probe(dev, PTR_ERR(map), "Unable to initialize register map\n");

- raw_spin_lock_init(&idio16gpio->lock);
-
- err = devm_gpiochip_add_data(dev, &idio16gpio->chip, idio16gpio);
- if (err) {
- dev_err(dev, "GPIO registering failed (%d)\n", err);
- return err;
- }
-
- err = devm_request_irq(dev, irq[id], idio_16_irq_handler, 0, name,
- idio16gpio);
- if (err) {
- dev_err(dev, "IRQ handler registering failed (%d)\n", err);
- return err;
- }
+ config.parent = dev;
+ config.map = map;
+ config.regmap_irqs = idio_16_regmap_irqs;
+ config.num_regmap_irqs = ARRAY_SIZE(idio_16_regmap_irqs);
+ config.irq = irq[id];
+ config.no_status = true;

- return 0;
+ return devm_idio_16_regmap_register(dev, &config);
}

static struct isa_driver idio_16_driver = {
--
2.39.2

2023-03-31 21:08:47

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v4 4/4] gpio: idio-16: Remove unused legacy interface

All idio-16 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 <[email protected]>
Signed-off-by: William Breathitt Gray <[email protected]>
---
Changes in v4: none

drivers/gpio/gpio-idio-16.c | 131 +-----------------------------------
drivers/gpio/gpio-idio-16.h | 65 ------------------
2 files changed, 1 insertion(+), 195 deletions(-)

diff --git a/drivers/gpio/gpio-idio-16.c b/drivers/gpio/gpio-idio-16.c
index f9349e8d7fdc..53b1eb876a12 100644
--- a/drivers/gpio/gpio-idio-16.c
+++ b/drivers/gpio/gpio-idio-16.c
@@ -3,15 +3,13 @@
* GPIO library for the ACCES IDIO-16 family
* 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-idio-16.h"
@@ -169,133 +167,6 @@ int devm_idio_16_regmap_register(struct device *const dev,
}
EXPORT_SYMBOL_GPL(devm_idio_16_regmap_register);

-/**
- * idio_16_get - get signal value at signal offset
- * @reg: ACCES IDIO-16 device registers
- * @state: ACCES IDIO-16 device state
- * @offset: offset of signal to get
- *
- * Returns the signal value (0=low, 1=high) for the signal at @offset.
- */
-int idio_16_get(struct idio_16 __iomem *const reg,
- struct idio_16_state *const state, const unsigned long offset)
-{
- const unsigned long mask = BIT(offset);
-
- if (offset < IDIO_16_NOUT)
- return test_bit(offset, state->out_state);
-
- if (offset < 24)
- return !!(ioread8(&reg->in0_7) & (mask >> IDIO_16_NOUT));
-
- if (offset < 32)
- return !!(ioread8(&reg->in8_15) & (mask >> 24));
-
- return -EINVAL;
-}
-EXPORT_SYMBOL_GPL(idio_16_get);
-
-/**
- * idio_16_get_multiple - get multiple signal values at multiple signal offsets
- * @reg: ACCES IDIO-16 device registers
- * @state: ACCES IDIO-16 device state
- * @mask: mask of signals to get
- * @bits: bitmap to store signal values
- *
- * Stores in @bits the values (0=low, 1=high) for the signals defined by @mask.
- */
-void idio_16_get_multiple(struct idio_16 __iomem *const reg,
- struct idio_16_state *const state,
- const unsigned long *const mask,
- unsigned long *const bits)
-{
- unsigned long flags;
- const unsigned long out_mask = GENMASK(IDIO_16_NOUT - 1, 0);
-
- spin_lock_irqsave(&state->lock, flags);
-
- bitmap_replace(bits, bits, state->out_state, &out_mask, IDIO_16_NOUT);
- if (*mask & GENMASK(23, 16))
- bitmap_set_value8(bits, ioread8(&reg->in0_7), 16);
- if (*mask & GENMASK(31, 24))
- bitmap_set_value8(bits, ioread8(&reg->in8_15), 24);
-
- spin_unlock_irqrestore(&state->lock, flags);
-}
-EXPORT_SYMBOL_GPL(idio_16_get_multiple);
-
-/**
- * idio_16_set - set signal value at signal offset
- * @reg: ACCES IDIO-16 device registers
- * @state: ACCES IDIO-16 device state
- * @offset: offset of signal to set
- * @value: value of signal to set
- *
- * Assigns output @value for the signal at @offset.
- */
-void idio_16_set(struct idio_16 __iomem *const reg,
- struct idio_16_state *const state, const unsigned long offset,
- const unsigned long value)
-{
- unsigned long flags;
-
- if (offset >= IDIO_16_NOUT)
- return;
-
- spin_lock_irqsave(&state->lock, flags);
-
- __assign_bit(offset, state->out_state, value);
- if (offset < 8)
- iowrite8(bitmap_get_value8(state->out_state, 0), &reg->out0_7);
- else
- iowrite8(bitmap_get_value8(state->out_state, 8), &reg->out8_15);
-
- spin_unlock_irqrestore(&state->lock, flags);
-}
-EXPORT_SYMBOL_GPL(idio_16_set);
-
-/**
- * idio_16_set_multiple - set signal values at multiple signal offsets
- * @reg: ACCES IDIO-16 device registers
- * @state: ACCES IDIO-16 device state
- * @mask: mask of signals to set
- * @bits: bitmap of signal output values
- *
- * Assigns output values defined by @bits for the signals defined by @mask.
- */
-void idio_16_set_multiple(struct idio_16 __iomem *const reg,
- struct idio_16_state *const state,
- const unsigned long *const mask,
- const unsigned long *const bits)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&state->lock, flags);
-
- bitmap_replace(state->out_state, state->out_state, bits, mask,
- IDIO_16_NOUT);
- if (*mask & GENMASK(7, 0))
- iowrite8(bitmap_get_value8(state->out_state, 0), &reg->out0_7);
- if (*mask & GENMASK(15, 8))
- iowrite8(bitmap_get_value8(state->out_state, 8), &reg->out8_15);
-
- spin_unlock_irqrestore(&state->lock, flags);
-}
-EXPORT_SYMBOL_GPL(idio_16_set_multiple);
-
-/**
- * idio_16_state_init - initialize idio_16_state structure
- * @state: ACCES IDIO-16 device state
- *
- * Initializes the ACCES IDIO-16 device @state for use in idio-16 library
- * functions.
- */
-void idio_16_state_init(struct idio_16_state *const state)
-{
- spin_lock_init(&state->lock);
-}
-EXPORT_SYMBOL_GPL(idio_16_state_init);
-
MODULE_AUTHOR("William Breathitt Gray");
MODULE_DESCRIPTION("ACCES IDIO-16 GPIO Library");
MODULE_LICENSE("GPL");
diff --git a/drivers/gpio/gpio-idio-16.h b/drivers/gpio/gpio-idio-16.h
index 255bd8504ed7..93b08ad73065 100644
--- a/drivers/gpio/gpio-idio-16.h
+++ b/drivers/gpio/gpio-idio-16.h
@@ -3,9 +3,6 @@
#ifndef _IDIO_16_H_
#define _IDIO_16_H_

-#include <linux/spinlock.h>
-#include <linux/types.h>
-
struct device;
struct regmap;
struct regmap_irq;
@@ -30,68 +27,6 @@ struct idio_16_regmap_config {
bool filters;
};

-/**
- * struct idio_16 - IDIO-16 registers structure
- * @out0_7: Read: FET Drive Outputs 0-7
- * Write: FET Drive Outputs 0-7
- * @in0_7: Read: Isolated Inputs 0-7
- * Write: Clear Interrupt
- * @irq_ctl: Read: Enable IRQ
- * Write: Disable IRQ
- * @filter_ctl: Read: Activate Input Filters 0-15
- * Write: Deactivate Input Filters 0-15
- * @out8_15: Read: FET Drive Outputs 8-15
- * Write: FET Drive Outputs 8-15
- * @in8_15: Read: Isolated Inputs 8-15
- * Write: Unused
- * @irq_status: Read: Interrupt status
- * Write: Unused
- */
-struct idio_16 {
- u8 out0_7;
- u8 in0_7;
- u8 irq_ctl;
- u8 filter_ctl;
- u8 out8_15;
- u8 in8_15;
- u8 irq_status;
-};
-
-#define IDIO_16_NOUT 16
-
-/**
- * struct idio_16_state - IDIO-16 state structure
- * @lock: synchronization lock for accessing device state
- * @out_state: output signals state
- */
-struct idio_16_state {
- spinlock_t lock;
- DECLARE_BITMAP(out_state, IDIO_16_NOUT);
-};
-
-/**
- * idio_16_get_direction - get the I/O direction for a signal offset
- * @offset: offset of signal to get direction
- *
- * Returns the signal direction (0=output, 1=input) for the signal at @offset.
- */
-static inline int idio_16_get_direction(const unsigned long offset)
-{
- return (offset >= IDIO_16_NOUT) ? 1 : 0;
-}
-
-int idio_16_get(struct idio_16 __iomem *reg, struct idio_16_state *state,
- unsigned long offset);
-void idio_16_get_multiple(struct idio_16 __iomem *reg,
- struct idio_16_state *state,
- const unsigned long *mask, unsigned long *bits);
-void idio_16_set(struct idio_16 __iomem *reg, struct idio_16_state *state,
- unsigned long offset, unsigned long value);
-void idio_16_set_multiple(struct idio_16 __iomem *reg,
- struct idio_16_state *state,
- 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_ */
--
2.39.2

2023-04-03 18:25:29

by William Breathitt Gray

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] Migrate IDIO-16 GPIO drivers to regmap API

On Fri, Mar 31, 2023 at 05:05:22PM -0400, William Breathitt Gray wrote:
> Changes in v4:
> - Remove superfluous base check in idio_16_reg_mask_xlate()
> - Format idio_16_names[] to 8 GPIO per line
> - Utilize dev_err_probe() in idio_16_regmap_register()
>
> 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-idio-16 and pci-idio-16 modules depend
> on the IDIO-16 library and are thus updated accordingly.
>
> 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.
>
> This patchset depends on the "Drop map from handle_mask_sync()
> parameters" patchset [0].
>
> [0] https://lore.kernel.org/all/[email protected]/
>
> William Breathitt Gray (4):
> gpio: idio-16: Migrate to the regmap API
> gpio: 104-idio-16: Migrate to the regmap API
> gpio: pci-idio-16: Migrate to the regmap API
> gpio: idio-16: Remove unused legacy interface
>
> drivers/gpio/Kconfig | 7 +-
> drivers/gpio/gpio-104-idio-16.c | 286 +++++++------------------------
> drivers/gpio/gpio-idio-16.c | 254 ++++++++++++++-------------
> drivers/gpio/gpio-idio-16.h | 79 +++------
> drivers/gpio/gpio-pci-idio-16.c | 294 +++++++-------------------------
> 5 files changed, 289 insertions(+), 631 deletions(-)
>
>
> base-commit: 4ea0c97776bf8c63805eb0f8182d9c20072219d9
> prerequisite-patch-id: cd19046150b7cff1be4ac7152198777aa960a3df
> prerequisite-patch-id: bd3e3830d9ce4f3876a77483364d7190b7fdffa7
> --
> 2.39.2

The 104-idio-16 and pci-idio-16 drivers here likely need
use_raw_spinlock enabled for their respective regmap_config structures
to prevent deadlocks as described in the i8255 fixes patchset [0]. I'll
submit a v5 of this patchset to fix that.

[0] https://lore.kernel.org/all/[email protected]/


Attachments:
(No filename) (2.02 kB)
signature.asc (235.00 B)
Download all attachments