[PATCH 2/5] gpio: 104-idi-48: Implement and utilize register structures

From: William Breathitt Gray
Date: Mon Jun 06 2022 - 10:36:58 EST


Reduce magic numbers and improve code readability by implementing and
utilizing named register data structures.

Signed-off-by: William Breathitt Gray <william.gray@xxxxxxxxxx>
---
drivers/gpio/gpio-104-idi-48.c | 128 ++++++++++++++++-----------------
1 file changed, 63 insertions(+), 65 deletions(-)

diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c
index 9521ece3ebef..1f8c556d9013 100644
--- a/drivers/gpio/gpio-104-idi-48.c
+++ b/drivers/gpio/gpio-104-idi-48.c
@@ -20,6 +20,7 @@
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/spinlock.h>
+#include <linux/types.h>

#define IDI_48_EXTENT 8
#define MAX_NUM_IDI_48 max_num_isa_dev(IDI_48_EXTENT)
@@ -33,13 +34,28 @@ static unsigned int irq[MAX_NUM_IDI_48];
module_param_hw_array(irq, uint, irq, NULL, 0);
MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers");

+/**
+ * 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
* @lock: synchronization lock to prevent I/O race conditions
* @ack_lock: synchronization lock to prevent IRQ handler race conditions
* @irq_mask: input bits affected by interrupts
- * @base: base port address of the GPIO device
+ * @reg: I/O address offset for the device registers
* @cos_enb: Change-Of-State IRQ enable boundaries mask
*/
struct idi_48_gpio {
@@ -47,7 +63,7 @@ struct idi_48_gpio {
raw_spinlock_t lock;
spinlock_t ack_lock;
unsigned char irq_mask[6];
- void __iomem *base;
+ struct idi_48_reg __iomem *reg;
unsigned char cos_enb;
};

@@ -64,39 +80,38 @@ static int idi_48_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
static int idi_48_gpio_get(struct gpio_chip *chip, unsigned offset)
{
struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
- unsigned i;
- static const unsigned int register_offset[6] = { 0, 1, 2, 4, 5, 6 };
- void __iomem *port_addr;
- unsigned mask;
-
- for (i = 0; i < 48; i += 8)
- if (offset < i + 8) {
- port_addr = idi48gpio->base + register_offset[i / 8];
- mask = BIT(offset - i);
-
- return !!(ioread8(port_addr) & mask);
- }
-
- /* The following line should never execute since offset < 48 */
- return 0;
+ struct idi_48_reg __iomem *const reg = idi48gpio->reg;
+ const unsigned long boundary = offset / 8;
+ const unsigned long group = boundary / 3;
+ u8 __iomem *const port_addr = group ? reg->port1 : reg->port0;
+ const unsigned long in_port = boundary - (group * 3);
+ const unsigned long mask = BIT(offset % 8);
+
+ return !!(ioread8(port_addr + in_port) & mask);
}

static int idi_48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask,
unsigned long *bits)
{
struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip);
+ struct idi_48_reg __iomem *const reg = idi48gpio->reg;
unsigned long offset;
unsigned long gpio_mask;
- static const size_t ports[] = { 0, 1, 2, 4, 5, 6 };
- void __iomem *port_addr;
+ unsigned long boundary;
+ unsigned long group;
+ unsigned long in_port;
+ u8 __iomem *port_addr;
unsigned long port_state;

/* clear bits array to a clean slate */
bitmap_zero(bits, chip->ngpio);

- for_each_set_clump8(offset, gpio_mask, mask, ARRAY_SIZE(ports) * 8) {
- port_addr = idi48gpio->base + ports[offset / 8];
- port_state = ioread8(port_addr) & gpio_mask;
+ for_each_set_clump8(offset, gpio_mask, mask, chip->ngpio) {
+ boundary = offset / 8;
+ group = boundary / 3;
+ port_addr = group ? reg->port1 : reg->port0;
+ in_port = boundary - (group * 3);
+ port_state = ioread8(port_addr + in_port) & gpio_mask;

bitmap_set_value8(bits, port_state, offset);
}
@@ -113,30 +128,21 @@ 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 offset = irqd_to_hwirq(data);
- unsigned i;
- unsigned mask;
- unsigned boundary;
+ const unsigned long boundary = offset / 8;
+ const unsigned long mask = BIT(offset % 8);
unsigned long flags;

- for (i = 0; i < 48; i += 8)
- if (offset < i + 8) {
- mask = BIT(offset - i);
- boundary = i / 8;
-
- idi48gpio->irq_mask[boundary] &= ~mask;
-
- if (!idi48gpio->irq_mask[boundary]) {
- idi48gpio->cos_enb &= ~BIT(boundary);
-
- raw_spin_lock_irqsave(&idi48gpio->lock, flags);
+ idi48gpio->irq_mask[boundary] &= ~mask;

- iowrite8(idi48gpio->cos_enb, idi48gpio->base + 7);
+ /* Exit early if there are still input lines with IRQ unmasked */
+ if (idi48gpio->irq_mask[boundary])
+ return;

- raw_spin_unlock_irqrestore(&idi48gpio->lock, flags);
- }
+ idi48gpio->cos_enb &= ~BIT(boundary);

- return;
- }
+ raw_spin_lock_irqsave(&idi48gpio->lock, flags);
+ iowrite8(idi48gpio->cos_enb, &idi48gpio->reg->irq);
+ raw_spin_unlock_irqrestore(&idi48gpio->lock, flags);
}

static void idi_48_irq_unmask(struct irq_data *data)
@@ -144,32 +150,24 @@ 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 offset = irqd_to_hwirq(data);
- unsigned i;
- unsigned mask;
- unsigned boundary;
+ const unsigned long boundary = offset / 8;
+ const unsigned long mask = BIT(offset % 8);
unsigned prev_irq_mask;
unsigned long flags;

- for (i = 0; i < 48; i += 8)
- if (offset < i + 8) {
- mask = BIT(offset - i);
- boundary = i / 8;
- prev_irq_mask = idi48gpio->irq_mask[boundary];
-
- idi48gpio->irq_mask[boundary] |= mask;
+ prev_irq_mask = idi48gpio->irq_mask[boundary];

- if (!prev_irq_mask) {
- idi48gpio->cos_enb |= BIT(boundary);
+ idi48gpio->irq_mask[boundary] |= mask;

- raw_spin_lock_irqsave(&idi48gpio->lock, flags);
+ /* Exit early if IRQ was already unmasked for this boundary */
+ if (prev_irq_mask)
+ return;

- iowrite8(idi48gpio->cos_enb, idi48gpio->base + 7);
+ idi48gpio->cos_enb |= BIT(boundary);

- raw_spin_unlock_irqrestore(&idi48gpio->lock, flags);
- }
-
- return;
- }
+ raw_spin_lock_irqsave(&idi48gpio->lock, flags);
+ iowrite8(idi48gpio->cos_enb, &idi48gpio->reg->irq);
+ raw_spin_unlock_irqrestore(&idi48gpio->lock, flags);
}

static int idi_48_irq_set_type(struct irq_data *data, unsigned flow_type)
@@ -204,7 +202,7 @@ static irqreturn_t idi_48_irq_handler(int irq, void *dev_id)

raw_spin_lock(&idi48gpio->lock);

- cos_status = ioread8(idi48gpio->base + 7);
+ cos_status = ioread8(&idi48gpio->reg->irq);

raw_spin_unlock(&idi48gpio->lock);

@@ -250,8 +248,8 @@ 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->base + 7);
- ioread8(idi48gpio->base + 7);
+ iowrite8(0, &idi48gpio->reg->irq);
+ ioread8(&idi48gpio->reg->irq);

return 0;
}
@@ -273,8 +271,8 @@ static int idi_48_probe(struct device *dev, unsigned int id)
return -EBUSY;
}

- idi48gpio->base = devm_ioport_map(dev, base[id], IDI_48_EXTENT);
- if (!idi48gpio->base)
+ idi48gpio->reg = devm_ioport_map(dev, base[id], IDI_48_EXTENT);
+ if (!idi48gpio->reg)
return -ENOMEM;

idi48gpio->chip.label = name;
--
2.36.1