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.
A patch to pass the device regmap and irq_drv_data as a parameters for
the struct regmap_irq_chip set_type_config() is included. This is needed
by idio_24_set_type_config() in order to update the type configuration
on the device as well as irq_drv_data for idio_24_handle_mask_sync().
A patch moving the struct gpio_regmap declaration to linux/gpio/regmap.h
is also included. This is needed by idio_24_reg_mask_xlate() in order to
determine the current offset's direction by using gpio->regmap in
regmap_read(). One point to consider is whether an alternative solution
is better of passing regmap in the reg_mask_xlate() parameter list; this
would avoid the need to include <linux/gpio/driver.h> in order to
resolve an incomplete type warning for struct gpio_chip due to the move.
William Breathitt Gray (3):
regmap: Pass regmap and irq_drv_data as parameters for
set_type_config()
gpio: gpio-regmap: Expose struct gpio_regmap in linux/gpio/regmap.h
gpio: pcie-idio-24: Migrate to the regmap API
drivers/base/regmap/regmap-irq.c | 13 +-
drivers/gpio/Kconfig | 3 +
drivers/gpio/gpio-pcie-idio-24.c | 697 ++++++++++++-------------------
drivers/gpio/gpio-regmap.c | 20 -
include/linux/gpio/regmap.h | 23 +-
include/linux/regmap.h | 12 +-
6 files changed, 303 insertions(+), 465 deletions(-)
base-commit: 4827aae061337251bb91801b316157a78b845ec7
--
2.39.2
Allow struct regmap_irq_chip set_type_config() callbacks to access the
device regmap and irq_drv_data by passing them as parameters.
Signed-off-by: William Breathitt Gray <[email protected]>
---
drivers/base/regmap/regmap-irq.c | 13 +++++++++----
include/linux/regmap.h | 12 ++++++++----
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index a8f185430a07..eac55a3af6d9 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -333,8 +333,9 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type)
}
if (d->chip->set_type_config) {
- ret = d->chip->set_type_config(d->config_buf, type,
- irq_data, reg);
+ ret = d->chip->set_type_config(map, d->config_buf, type,
+ irq_data, reg,
+ d->chip->irq_drv_data);
if (ret)
return ret;
}
@@ -650,18 +651,22 @@ EXPORT_SYMBOL_GPL(regmap_irq_get_irq_reg_linear);
/**
* regmap_irq_set_type_config_simple() - Simple IRQ type configuration callback.
+ * @map: The regmap for the device.
* @buf: Buffer containing configuration register values, this is a 2D array of
* `num_config_bases` rows, each of `num_config_regs` elements.
* @type: The requested IRQ type.
* @irq_data: The IRQ being configured.
* @idx: Index of the irq's config registers within each array `buf[i]`
+ * @irq_drv_data: Driver specific IRQ data
*
* This is a &struct regmap_irq_chip->set_type_config callback suitable for
* chips with one config register. Register values are updated according to
* the &struct regmap_irq_type data associated with an IRQ.
*/
-int regmap_irq_set_type_config_simple(unsigned int **buf, unsigned int type,
- const struct regmap_irq *irq_data, int idx)
+int regmap_irq_set_type_config_simple(struct regmap *map, unsigned int **buf,
+ unsigned int type,
+ const struct regmap_irq *irq_data,
+ int idx, void *irq_drv_data)
{
const struct regmap_irq_type *t = &irq_data->type;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index a3bc695bcca0..49073f5ae87a 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1648,8 +1648,10 @@ struct regmap_irq_chip {
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,
- const struct regmap_irq *irq_data, int idx);
+ int (*set_type_config)(struct regmap *map, unsigned int **buf,
+ unsigned int type,
+ const struct regmap_irq *irq_data, int idx,
+ void *irq_drv_data);
unsigned int (*get_irq_reg)(struct regmap_irq_chip_data *data,
unsigned int base, int index);
void *irq_drv_data;
@@ -1657,8 +1659,10 @@ struct regmap_irq_chip {
unsigned int regmap_irq_get_irq_reg_linear(struct regmap_irq_chip_data *data,
unsigned int base, int index);
-int regmap_irq_set_type_config_simple(unsigned int **buf, unsigned int type,
- const struct regmap_irq *irq_data, int idx);
+int regmap_irq_set_type_config_simple(struct regmap *map, unsigned int **buf,
+ unsigned int type,
+ const struct regmap_irq *irq_data,
+ int idx, void *irq_drv_data);
int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags,
int irq_base, const struct regmap_irq_chip *chip,
--
2.39.2
A struct gpio_regmap is passed as a parameter for reg_mask_xlate(), but
for callbacks to access its members the declaration must be exposed.
Move the struct gpio_regmap declaration from drivers/gpio/gpio-regmap.c
to include/linux/gpio/regmap.h so callbacks can properly interact with
struct gpio_regmap members.
Signed-off-by: William Breathitt Gray <[email protected]>
---
drivers/gpio/gpio-regmap.c | 20 --------------------
include/linux/gpio/regmap.h | 23 ++++++++++++++++++++++-
2 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index fca17d478984..ad34750779c7 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -11,26 +11,6 @@
#include <linux/module.h>
#include <linux/regmap.h>
-struct gpio_regmap {
- struct device *parent;
- struct regmap *regmap;
- struct gpio_chip gpio_chip;
-
- int reg_stride;
- int ngpio_per_reg;
- unsigned int reg_dat_base;
- unsigned int reg_set_base;
- unsigned int reg_clr_base;
- unsigned int reg_dir_in_base;
- unsigned int reg_dir_out_base;
-
- int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
- unsigned int offset, unsigned int *reg,
- unsigned int *mask);
-
- void *driver_data;
-};
-
static unsigned int gpio_regmap_addr(unsigned int addr)
{
if (addr == GPIO_REGMAP_ADDR_ZERO)
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index a9f7b7faf57b..1132c0f7e907 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -3,15 +3,36 @@
#ifndef _LINUX_GPIO_REGMAP_H
#define _LINUX_GPIO_REGMAP_H
+#include <linux/gpio/driver.h>
+
struct device;
struct fwnode_handle;
-struct gpio_regmap;
struct irq_domain;
struct regmap;
#define GPIO_REGMAP_ADDR_ZERO ((unsigned int)(-1))
#define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
+struct gpio_regmap {
+ struct device *parent;
+ struct regmap *regmap;
+ struct gpio_chip gpio_chip;
+
+ int reg_stride;
+ int ngpio_per_reg;
+ unsigned int reg_dat_base;
+ unsigned int reg_set_base;
+ unsigned int reg_clr_base;
+ unsigned int reg_dir_in_base;
+ unsigned int reg_dir_out_base;
+
+ int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
+ unsigned int offset, unsigned int *reg,
+ unsigned int *mask);
+
+ void *driver_data;
+};
+
/**
* struct gpio_regmap_config - Description of a generic regmap gpio_chip.
* @parent: The parent device
--
2.39.2
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 PCIe-IDIO-24 series of devices, the following BARs are
available:
BAR[0]: memory mapped PEX8311
BAR[1]: I/O mapped PEX8311
BAR[2]: I/O mapped card registers
There are 24 FET Output lines, 24 Isolated Input lines, and 8 TTL/CMOS
lines (which may be configured for either output or input). The GPIO
lines are exposed by the following card registers:
Base +0x0-0x2 (Read/Write): FET Outputs
Base +0xB (Read/Write): TTL/CMOS
Base +0x4-0x6 (Read): Isolated Inputs
Base +0x7 (Read): TTL/CMOS
In order for the device to support interrupts, the PLX PEX8311 internal
PCI wire interrupt and local interrupt input must first be enabled.
The following card registers for Change-Of-State may be used:
Base +0x8-0xA (Read): COS Status Inputs
Base +0x8-0xA (Write): COS Clear Inputs
Base +0xB (Read): COS Status TTL/CMOS
Base +0xB (Write): COS Clear TTL/CMOS
Base +0xE (Read/Write): COS Enable
The COS Enable register is used to enable/disable interrupts and
configure the interrupt levels; each bit maps to a group of eight inputs
as described below:
Bit 0: IRQ EN Rising Edge IN0-7
Bit 1: IRQ EN Rising Edge IN8-15
Bit 2: IRQ EN Rising Edge IN16-23
Bit 3: IRQ EN Rising Edge TTL0-7
Bit 4: IRQ EN Falling Edge IN0-7
Bit 5: IRQ EN Falling Edge IN8-15
Bit 6: IRQ EN Falling Edge IN16-23
Bit 7: IRQ EN Falling Edge TTL0-7
An interrupt is asserted when a change-of-state matching the interrupt
level configuration respective for a particular group of eight inputs
with enabled COS is detected.
The COS Status registers may be read to determine which inputs have
changed; if interrupts were enabled, an IRQ will be generated for the
set bits in these registers. Writing the value read from the COS Status
register back to the respective COS Clear register will clear just those
interrupts.
Cc: Arnaud de Turckheim <[email protected]>
Cc: John Hentges <[email protected]>
Cc: Jay Dolan <[email protected]>
Signed-off-by: William Breathitt Gray <[email protected]>
---
drivers/gpio/Kconfig | 3 +
drivers/gpio/gpio-pcie-idio-24.c | 697 ++++++++++++-------------------
2 files changed, 264 insertions(+), 436 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 406e8bda487f..06c7a96e6033 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1578,7 +1578,10 @@ config GPIO_PCI_IDIO_16
config GPIO_PCIE_IDIO_24
tristate "ACCES PCIe-IDIO-24 GPIO support"
+ select REGMAP_IRQ
+ select REGMAP_MMIO
select GPIOLIB_IRQCHIP
+ select GPIO_REGMAP
help
Enables GPIO support for the ACCES PCIe-IDIO-24 family (PCIe-IDIO-24,
PCIe-IDI-24, PCIe-IDO-24, PCIe-IDIO-12). An interrupt is generated
diff --git a/drivers/gpio/gpio-pcie-idio-24.c b/drivers/gpio/gpio-pcie-idio-24.c
index 8a9b98fa418f..b643ad3ab6f6 100644
--- a/drivers/gpio/gpio-pcie-idio-24.c
+++ b/drivers/gpio/gpio-pcie-idio-24.c
@@ -15,17 +15,15 @@
* This driver supports the following ACCES devices: PCIe-IDIO-24,
* PCIe-IDI-24, PCIe-IDO-24, and PCIe-IDIO-12.
*/
-#include <linux/bitmap.h>
-#include <linux/bitops.h>
+#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/gpio/regmap.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>
/*
@@ -68,416 +66,230 @@
#define PLX_PEX8311_PCI_LCS_INTCSR 0x68
#define INTCSR_INTERNAL_PCI_WIRE BIT(8)
#define INTCSR_LOCAL_INPUT BIT(11)
-
-/**
- * struct idio_24_gpio_reg - GPIO device registers structure
- * @out0_7: Read: FET Outputs 0-7
- * Write: FET Outputs 0-7
- * @out8_15: Read: FET Outputs 8-15
- * Write: FET Outputs 8-15
- * @out16_23: Read: FET Outputs 16-23
- * Write: FET Outputs 16-23
- * @ttl_out0_7: Read: TTL/CMOS Outputs 0-7
- * Write: TTL/CMOS Outputs 0-7
- * @in0_7: Read: Isolated Inputs 0-7
- * Write: Reserved
- * @in8_15: Read: Isolated Inputs 8-15
- * Write: Reserved
- * @in16_23: Read: Isolated Inputs 16-23
- * Write: Reserved
- * @ttl_in0_7: Read: TTL/CMOS Inputs 0-7
- * Write: Reserved
- * @cos0_7: Read: COS Status Inputs 0-7
- * Write: COS Clear Inputs 0-7
- * @cos8_15: Read: COS Status Inputs 8-15
- * Write: COS Clear Inputs 8-15
- * @cos16_23: Read: COS Status Inputs 16-23
- * Write: COS Clear Inputs 16-23
- * @cos_ttl0_7: Read: COS Status TTL/CMOS 0-7
- * Write: COS Clear TTL/CMOS 0-7
- * @ctl: Read: Control Register
- * Write: Control Register
- * @reserved: Read: Reserved
- * Write: Reserved
- * @cos_enable: Read: COS Enable
- * Write: COS Enable
- * @soft_reset: Read: IRQ Output Pin Status
- * Write: Software Board Reset
- */
-struct idio_24_gpio_reg {
- u8 out0_7;
- u8 out8_15;
- u8 out16_23;
- u8 ttl_out0_7;
- u8 in0_7;
- u8 in8_15;
- u8 in16_23;
- u8 ttl_in0_7;
- u8 cos0_7;
- u8 cos8_15;
- u8 cos16_23;
- u8 cos_ttl0_7;
- u8 ctl;
- u8 reserved;
- u8 cos_enable;
- u8 soft_reset;
+#define IDIO_24_ENABLE_IRQ (INTCSR_INTERNAL_PCI_WIRE | INTCSR_LOCAL_INPUT)
+
+#define IDIO_24_DAT_BASE 0x0
+#define IDIO_24_OUT_BASE IDIO_24_DAT_BASE
+#define IDIO_24_TTLCMOS_OUT_REG (IDIO_24_DAT_BASE + 0x3)
+#define IDIO_24_IN_BASE (IDIO_24_DAT_BASE + 0x4)
+#define IDIO_24_TTLCMOS_IN_REG (IDIO_24_DAT_BASE + 0x7)
+#define IDIO_24_STATUS_BASE (IDIO_24_DAT_BASE + 0x8)
+#define IDIO_24_ACK_BASE IDIO_24_STATUS_BASE
+#define IDIO_24_CONTROL_REG (IDIO_24_DAT_BASE + 0xC)
+#define IDIO_24_COS_ENABLE (IDIO_24_DAT_BASE + 0xE)
+#define IDIO_24_SOFT_RESET (IDIO_24_DAT_BASE + 0xF)
+
+#define CONTROL_REG_OUT_MODE BIT(1)
+
+#define COS_ENABLE_RISING BIT(1)
+#define COS_ENABLE_FALLING BIT(4)
+#define COS_ENABLE_BOTH (COS_ENABLE_RISING | COS_ENABLE_FALLING)
+
+static const struct regmap_range pex8311_intcsr_wr_ranges[] = {
+ regmap_reg_range(0x0, 0x0),
};
-
-/**
- * struct idio_24_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
- * @irq_mask: I/O bits affected by interrupts
- */
-struct idio_24_gpio {
- struct gpio_chip chip;
- raw_spinlock_t lock;
- __u8 __iomem *plx;
- struct idio_24_gpio_reg __iomem *reg;
- unsigned long irq_mask;
+static const struct regmap_range pex8311_intcsr_rd_ranges[] = {
+ regmap_reg_range(0x0, 0x0),
+};
+static const struct regmap_range pex8311_intcsr_volatile_ranges[] = {
+ regmap_reg_range(0x0, 0x0),
+};
+static const struct regmap_access_table pex8311_intcsr_wr_table = {
+ .yes_ranges = pex8311_intcsr_wr_ranges,
+ .n_yes_ranges = ARRAY_SIZE(pex8311_intcsr_wr_ranges),
+};
+static const struct regmap_access_table pex8311_intcsr_rd_table = {
+ .yes_ranges = pex8311_intcsr_rd_ranges,
+ .n_yes_ranges = ARRAY_SIZE(pex8311_intcsr_rd_ranges),
+};
+static const struct regmap_access_table pex8311_intcsr_volatile_table = {
+ .yes_ranges = pex8311_intcsr_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(pex8311_intcsr_volatile_ranges),
};
-static int idio_24_gpio_get_direction(struct gpio_chip *chip,
- unsigned int offset)
-{
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- const unsigned long out_mode_mask = BIT(1);
-
- /* FET Outputs */
- if (offset < 24)
- return GPIO_LINE_DIRECTION_OUT;
-
- /* Isolated Inputs */
- if (offset < 48)
- return GPIO_LINE_DIRECTION_IN;
-
- /* TTL/CMOS I/O */
- /* OUT MODE = 1 when TTL/CMOS Output Mode is set */
- if (ioread8(&idio24gpio->reg->ctl) & out_mode_mask)
- return GPIO_LINE_DIRECTION_OUT;
-
- return GPIO_LINE_DIRECTION_IN;
-}
-
-static int idio_24_gpio_direction_input(struct gpio_chip *chip,
- unsigned int offset)
-{
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- unsigned long flags;
- unsigned int ctl_state;
- const unsigned long out_mode_mask = BIT(1);
+static const struct regmap_config pex8311_intcsr_regmap_config = {
+ .name = "pex8311_intcsr",
+ .reg_bits = 32,
+ .reg_stride = 1,
+ .val_bits = 32,
+ .io_port = true,
+ .max_register = 0x0,
+ .wr_table = &pex8311_intcsr_wr_table,
+ .rd_table = &pex8311_intcsr_rd_table,
+ .volatile_table = &pex8311_intcsr_volatile_table,
+};
- /* TTL/CMOS I/O */
- if (offset > 47) {
- raw_spin_lock_irqsave(&idio24gpio->lock, flags);
+static const struct regmap_range idio_24_wr_ranges[] = {
+ regmap_reg_range(0x0, 0x3), regmap_reg_range(0x8, 0xC),
+ regmap_reg_range(0xE, 0xF),
+};
+static const struct regmap_range idio_24_rd_ranges[] = {
+ regmap_reg_range(0x0, 0xC), regmap_reg_range(0xE, 0xF),
+};
+static const struct regmap_range idio_24_volatile_ranges[] = {
+ regmap_reg_range(0x4, 0xB), regmap_reg_range(0xF, 0xF),
+};
+static const struct regmap_access_table idio_24_wr_table = {
+ .yes_ranges = idio_24_wr_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_24_wr_ranges),
+};
+static const struct regmap_access_table idio_24_rd_table = {
+ .yes_ranges = idio_24_rd_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_24_rd_ranges),
+};
+static const struct regmap_access_table idio_24_volatile_table = {
+ .yes_ranges = idio_24_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(idio_24_volatile_ranges),
+};
- /* Clear TTL/CMOS Output Mode */
- ctl_state = ioread8(&idio24gpio->reg->ctl) & ~out_mode_mask;
- iowrite8(ctl_state, &idio24gpio->reg->ctl);
+static const struct regmap_config idio_24_regmap_config = {
+ .reg_bits = 8,
+ .reg_stride = 1,
+ .val_bits = 8,
+ .io_port = true,
+ .max_register = 0xF,
+ .wr_table = &idio_24_wr_table,
+ .rd_table = &idio_24_rd_table,
+ .volatile_table = &idio_24_volatile_table,
+ .cache_type = REGCACHE_FLAT,
+};
- raw_spin_unlock_irqrestore(&idio24gpio->lock, flags);
+#define IDIO_24_NGPIO_PER_REG 8
+#define IDIO_24_REGMAP_IRQ(_id) \
+ [24 + _id] = { \
+ .reg_offset = (_id) / IDIO_24_NGPIO_PER_REG, \
+ .mask = BIT((_id) % IDIO_24_NGPIO_PER_REG), \
+ .type = { .types_supported = IRQ_TYPE_EDGE_BOTH }, \
}
+#define IDIO_24_IIN_IRQ(_id) IDIO_24_REGMAP_IRQ(_id)
+#define IDIO_24_TTL_IRQ(_id) IDIO_24_REGMAP_IRQ(24 + _id)
+
+static const struct regmap_irq idio_24_regmap_irqs[] = {
+ IDIO_24_IIN_IRQ(0), IDIO_24_IIN_IRQ(1), IDIO_24_IIN_IRQ(2), /* IIN 0-2 */
+ IDIO_24_IIN_IRQ(3), IDIO_24_IIN_IRQ(4), IDIO_24_IIN_IRQ(5), /* IIN 3-5 */
+ IDIO_24_IIN_IRQ(6), IDIO_24_IIN_IRQ(7), IDIO_24_IIN_IRQ(8), /* IIN 6-8 */
+ IDIO_24_IIN_IRQ(9), IDIO_24_IIN_IRQ(10), IDIO_24_IIN_IRQ(11), /* IIN 9-11 */
+ IDIO_24_IIN_IRQ(12), IDIO_24_IIN_IRQ(13), IDIO_24_IIN_IRQ(14), /* IIN 12-14 */
+ IDIO_24_IIN_IRQ(15), IDIO_24_IIN_IRQ(16), IDIO_24_IIN_IRQ(17), /* IIN 15-17 */
+ IDIO_24_IIN_IRQ(18), IDIO_24_IIN_IRQ(19), IDIO_24_IIN_IRQ(20), /* IIN 18-20 */
+ IDIO_24_IIN_IRQ(21), IDIO_24_IIN_IRQ(22), IDIO_24_IIN_IRQ(23), /* IIN 21-23 */
+ IDIO_24_TTL_IRQ(0), IDIO_24_TTL_IRQ(1), IDIO_24_TTL_IRQ(2), /* TTL 0-2 */
+ IDIO_24_TTL_IRQ(3), IDIO_24_TTL_IRQ(4), IDIO_24_TTL_IRQ(5), /* TTL 3-5 */
+ IDIO_24_TTL_IRQ(6), IDIO_24_TTL_IRQ(7), /* TTL 6-7 */
+};
- return 0;
-}
-
-static int idio_24_gpio_direction_output(struct gpio_chip *chip,
- unsigned int offset, int value)
+static int idio_24_set_type_config(struct regmap *const map,
+ unsigned int **const buf,
+ const unsigned int type,
+ const struct regmap_irq *const irq_data,
+ const int idx, void *const irq_drv_data)
{
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- unsigned long flags;
- unsigned int ctl_state;
- const unsigned long out_mode_mask = BIT(1);
-
- /* TTL/CMOS I/O */
- if (offset > 47) {
- raw_spin_lock_irqsave(&idio24gpio->lock, flags);
-
- /* Set TTL/CMOS Output Mode */
- ctl_state = ioread8(&idio24gpio->reg->ctl) | out_mode_mask;
- iowrite8(ctl_state, &idio24gpio->reg->ctl);
+ const unsigned int offset = irq_data->reg_offset;
+ const unsigned int rising = COS_ENABLE_RISING << offset;
+ const unsigned int falling = COS_ENABLE_FALLING << offset;
+ const unsigned int mask = COS_ENABLE_BOTH << offset;
+ unsigned int *const irq_type = irq_drv_data;
+ unsigned int new;
+ unsigned int cos_enable;
+ int err;
- raw_spin_unlock_irqrestore(&idio24gpio->lock, flags);
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ new = rising;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ new = falling;
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ new = mask;
+ break;
+ default:
+ return -EINVAL;
}
- chip->set(chip, offset, value);
- return 0;
-}
-
-static int idio_24_gpio_get(struct gpio_chip *chip, unsigned int offset)
-{
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- const unsigned long offset_mask = BIT(offset % 8);
- const unsigned long out_mode_mask = BIT(1);
-
- /* FET Outputs */
- if (offset < 8)
- return !!(ioread8(&idio24gpio->reg->out0_7) & offset_mask);
-
- if (offset < 16)
- return !!(ioread8(&idio24gpio->reg->out8_15) & offset_mask);
-
- if (offset < 24)
- return !!(ioread8(&idio24gpio->reg->out16_23) & offset_mask);
-
- /* Isolated Inputs */
- if (offset < 32)
- return !!(ioread8(&idio24gpio->reg->in0_7) & offset_mask);
-
- if (offset < 40)
- return !!(ioread8(&idio24gpio->reg->in8_15) & offset_mask);
-
- if (offset < 48)
- return !!(ioread8(&idio24gpio->reg->in16_23) & offset_mask);
+ /* replace old bitmap with new bitmap */
+ *irq_type = (*irq_type & ~mask) | (new & mask);
- /* TTL/CMOS Outputs */
- if (ioread8(&idio24gpio->reg->ctl) & out_mode_mask)
- return !!(ioread8(&idio24gpio->reg->ttl_out0_7) & offset_mask);
-
- /* TTL/CMOS Inputs */
- return !!(ioread8(&idio24gpio->reg->ttl_in0_7) & offset_mask);
-}
-
-static int idio_24_gpio_get_multiple(struct gpio_chip *chip,
- unsigned long *mask, unsigned long *bits)
-{
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- unsigned long offset;
- unsigned long gpio_mask;
- void __iomem *ports[] = {
- &idio24gpio->reg->out0_7, &idio24gpio->reg->out8_15,
- &idio24gpio->reg->out16_23, &idio24gpio->reg->in0_7,
- &idio24gpio->reg->in8_15, &idio24gpio->reg->in16_23,
- };
- size_t index;
- unsigned long port_state;
- const unsigned long out_mode_mask = BIT(1);
-
- /* clear bits array to a clean slate */
- bitmap_zero(bits, chip->ngpio);
-
- for_each_set_clump8(offset, gpio_mask, mask, ARRAY_SIZE(ports) * 8) {
- index = offset / 8;
-
- /* read bits from current gpio port (port 6 is TTL GPIO) */
- if (index < 6)
- port_state = ioread8(ports[index]);
- else if (ioread8(&idio24gpio->reg->ctl) & out_mode_mask)
- port_state = ioread8(&idio24gpio->reg->ttl_out0_7);
- else
- port_state = ioread8(&idio24gpio->reg->ttl_in0_7);
-
- port_state &= gpio_mask;
-
- bitmap_set_value8(bits, port_state, offset);
- }
+ err = regmap_read(map, IDIO_24_COS_ENABLE, &cos_enable);
+ if (err)
+ return err;
+ /* if COS is currently enabled */
+ if (cos_enable & mask)
+ return regmap_update_bits(map, IDIO_24_COS_ENABLE, mask,
+ *irq_type);
return 0;
}
-static void idio_24_gpio_set(struct gpio_chip *chip, unsigned int offset,
- int value)
+static int idio_24_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 idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- const unsigned long out_mode_mask = BIT(1);
- void __iomem *base;
- const unsigned int mask = BIT(offset % 8);
- unsigned long flags;
- unsigned int out_state;
-
- /* Isolated Inputs */
- if (offset > 23 && offset < 48)
- return;
-
- /* TTL/CMOS Inputs */
- if (offset > 47 && !(ioread8(&idio24gpio->reg->ctl) & out_mode_mask))
- return;
-
- /* TTL/CMOS Outputs */
- if (offset > 47)
- base = &idio24gpio->reg->ttl_out0_7;
- /* FET Outputs */
- else if (offset > 15)
- base = &idio24gpio->reg->out16_23;
- else if (offset > 7)
- base = &idio24gpio->reg->out8_15;
- else
- base = &idio24gpio->reg->out0_7;
-
- raw_spin_lock_irqsave(&idio24gpio->lock, flags);
-
- if (value)
- out_state = ioread8(base) | mask;
- else
- out_state = ioread8(base) & ~mask;
-
- iowrite8(out_state, base);
-
- raw_spin_unlock_irqrestore(&idio24gpio->lock, flags);
-}
+ const unsigned int irq_type_mask = COS_ENABLE_BOTH << index;
+ unsigned int *const irq_type = irq_drv_data;
-static void idio_24_gpio_set_multiple(struct gpio_chip *chip,
- unsigned long *mask, unsigned long *bits)
-{
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- unsigned long offset;
- unsigned long gpio_mask;
- void __iomem *ports[] = {
- &idio24gpio->reg->out0_7, &idio24gpio->reg->out8_15,
- &idio24gpio->reg->out16_23
- };
- size_t index;
- unsigned long bitmask;
- unsigned long flags;
- unsigned long out_state;
- const unsigned long out_mode_mask = BIT(1);
-
- for_each_set_clump8(offset, gpio_mask, mask, ARRAY_SIZE(ports) * 8) {
- index = offset / 8;
-
- bitmask = bitmap_get_value8(bits, offset) & gpio_mask;
-
- raw_spin_lock_irqsave(&idio24gpio->lock, flags);
-
- /* read bits from current gpio port (port 6 is TTL GPIO) */
- if (index < 6) {
- out_state = ioread8(ports[index]);
- } else if (ioread8(&idio24gpio->reg->ctl) & out_mode_mask) {
- out_state = ioread8(&idio24gpio->reg->ttl_out0_7);
- } else {
- /* skip TTL GPIO if set for input */
- raw_spin_unlock_irqrestore(&idio24gpio->lock, flags);
- continue;
- }
-
- /* set requested bit states */
- out_state &= ~gpio_mask;
- out_state |= bitmask;
+ /* if all are masked, disable interrupts */
+ if (mask_buf == mask_buf_def)
+ return regmap_update_bits(map, IDIO_24_COS_ENABLE,
+ irq_type_mask, ~irq_type_mask);
- /* write bits for current gpio port (port 6 is TTL GPIO) */
- if (index < 6)
- iowrite8(out_state, ports[index]);
- else
- iowrite8(out_state, &idio24gpio->reg->ttl_out0_7);
-
- raw_spin_unlock_irqrestore(&idio24gpio->lock, flags);
- }
+ return regmap_update_bits(map, IDIO_24_COS_ENABLE, irq_type_mask,
+ *irq_type);
}
-static void idio_24_irq_ack(struct irq_data *data)
+static int idio_24_reg_mask_xlate(struct gpio_regmap *const gpio,
+ const unsigned int base,
+ const unsigned int offset,
+ unsigned int *const reg,
+ unsigned int *const mask)
{
-}
-
-static void idio_24_irq_mask(struct irq_data *data)
-{
- struct gpio_chip *const chip = irq_data_get_irq_chip_data(data);
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- unsigned long flags;
- const unsigned long bit_offset = irqd_to_hwirq(data) - 24;
- unsigned char new_irq_mask;
- const unsigned long bank_offset = bit_offset / 8;
- unsigned char cos_enable_state;
-
- raw_spin_lock_irqsave(&idio24gpio->lock, flags);
-
- idio24gpio->irq_mask &= ~BIT(bit_offset);
- new_irq_mask = idio24gpio->irq_mask >> bank_offset * 8;
-
- if (!new_irq_mask) {
- cos_enable_state = ioread8(&idio24gpio->reg->cos_enable);
-
- /* Disable Rising Edge detection */
- cos_enable_state &= ~BIT(bank_offset);
- /* Disable Falling Edge detection */
- cos_enable_state &= ~BIT(bank_offset + 4);
-
- iowrite8(cos_enable_state, &idio24gpio->reg->cos_enable);
- }
-
- raw_spin_unlock_irqrestore(&idio24gpio->lock, flags);
-}
-
-static void idio_24_irq_unmask(struct irq_data *data)
-{
- struct gpio_chip *const chip = irq_data_get_irq_chip_data(data);
- struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip);
- unsigned long flags;
- unsigned char prev_irq_mask;
- const unsigned long bit_offset = irqd_to_hwirq(data) - 24;
- const unsigned long bank_offset = bit_offset / 8;
- unsigned char cos_enable_state;
-
- raw_spin_lock_irqsave(&idio24gpio->lock, flags);
+ const unsigned int out_stride = offset / IDIO_24_NGPIO_PER_REG;
+ const unsigned int in_stride = (offset - 24) / IDIO_24_NGPIO_PER_REG;
+ int err;
+ unsigned int ctrl_reg;
- prev_irq_mask = idio24gpio->irq_mask >> bank_offset * 8;
- idio24gpio->irq_mask |= BIT(bit_offset);
+ switch (base) {
+ case IDIO_24_DAT_BASE:
+ *mask = BIT(offset % IDIO_24_NGPIO_PER_REG);
- if (!prev_irq_mask) {
- cos_enable_state = ioread8(&idio24gpio->reg->cos_enable);
+ /* FET Outputs */
+ if (offset < 24) {
+ *reg = IDIO_24_OUT_BASE + out_stride;
+ return 0;
+ }
- /* Enable Rising Edge detection */
- cos_enable_state |= BIT(bank_offset);
- /* Enable Falling Edge detection */
- cos_enable_state |= BIT(bank_offset + 4);
+ /* Isolated Inputs */
+ if (offset < 48) {
+ *reg = IDIO_24_IN_BASE + in_stride;
+ return 0;
+ }
- iowrite8(cos_enable_state, &idio24gpio->reg->cos_enable);
- }
+ err = regmap_read(gpio->regmap, IDIO_24_CONTROL_REG, &ctrl_reg);
+ if (err)
+ return err;
- raw_spin_unlock_irqrestore(&idio24gpio->lock, flags);
-}
+ /* TTL/CMOS Outputs */
+ if (ctrl_reg & CONTROL_REG_OUT_MODE) {
+ *reg = IDIO_24_TTLCMOS_OUT_REG;
+ return 0;
+ }
-static int idio_24_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)
+ /* TTL/CMOS Inputs */
+ *reg = IDIO_24_TTLCMOS_IN_REG;
+ return 0;
+ case IDIO_24_CONTROL_REG:
+ /* We can only set direction for TTL/CMOS lines */
+ if (offset < 48)
+ return -EOPNOTSUPP;
+
+ *reg = IDIO_24_CONTROL_REG;
+ *mask = CONTROL_REG_OUT_MODE;
+ return 0;
+ default:
+ /* Should never reach this path */
return -EINVAL;
-
- return 0;
-}
-
-static struct irq_chip idio_24_irqchip = {
- .name = "pcie-idio-24",
- .irq_ack = idio_24_irq_ack,
- .irq_mask = idio_24_irq_mask,
- .irq_unmask = idio_24_irq_unmask,
- .irq_set_type = idio_24_irq_set_type
-};
-
-static irqreturn_t idio_24_irq_handler(int irq, void *dev_id)
-{
- struct idio_24_gpio *const idio24gpio = dev_id;
- unsigned long irq_status;
- struct gpio_chip *const chip = &idio24gpio->chip;
- unsigned long irq_mask;
- int gpio;
-
- raw_spin_lock(&idio24gpio->lock);
-
- /* Read Change-Of-State status */
- irq_status = ioread32(&idio24gpio->reg->cos0_7);
-
- raw_spin_unlock(&idio24gpio->lock);
-
- /* Make sure our device generated IRQ */
- if (!irq_status)
- return IRQ_NONE;
-
- /* Handle only unmasked IRQ */
- irq_mask = idio24gpio->irq_mask & irq_status;
-
- for_each_set_bit(gpio, &irq_mask, chip->ngpio - 24)
- generic_handle_domain_irq(chip->irq.domain, gpio + 24);
-
- raw_spin_lock(&idio24gpio->lock);
-
- /* Clear Change-Of-State status */
- iowrite32(irq_status, &idio24gpio->reg->cos0_7);
-
- raw_spin_unlock(&idio24gpio->lock);
-
- return IRQ_HANDLED;
+ }
}
#define IDIO_24_NGPIO 56
@@ -494,16 +306,18 @@ static const char *idio_24_names[IDIO_24_NGPIO] = {
static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct device *const dev = &pdev->dev;
- struct idio_24_gpio *idio24gpio;
int err;
const size_t pci_plx_bar_index = 1;
const size_t pci_bar_index = 2;
const char *const name = pci_name(pdev);
- struct gpio_irq_chip *girq;
-
- idio24gpio = devm_kzalloc(dev, sizeof(*idio24gpio), GFP_KERNEL);
- if (!idio24gpio)
- return -ENOMEM;
+ struct gpio_regmap_config gpio_config = {};
+ void __iomem *pex8311_intcsr;
+ void __iomem *idio_24_regs;
+ struct regmap *pex8311_intcsr_map;
+ struct regmap *idio_24_map;
+ struct regmap_irq_chip *chip;
+ unsigned int irq_type;
+ struct regmap_irq_chip_data *chip_data;
err = pcim_enable_device(pdev);
if (err) {
@@ -517,57 +331,68 @@ static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
return err;
}
- idio24gpio->plx = pcim_iomap_table(pdev)[pci_plx_bar_index];
- idio24gpio->reg = pcim_iomap_table(pdev)[pci_bar_index];
-
- idio24gpio->chip.label = name;
- idio24gpio->chip.parent = dev;
- idio24gpio->chip.owner = THIS_MODULE;
- idio24gpio->chip.base = -1;
- idio24gpio->chip.ngpio = IDIO_24_NGPIO;
- idio24gpio->chip.names = idio_24_names;
- idio24gpio->chip.get_direction = idio_24_gpio_get_direction;
- idio24gpio->chip.direction_input = idio_24_gpio_direction_input;
- idio24gpio->chip.direction_output = idio_24_gpio_direction_output;
- idio24gpio->chip.get = idio_24_gpio_get;
- idio24gpio->chip.get_multiple = idio_24_gpio_get_multiple;
- idio24gpio->chip.set = idio_24_gpio_set;
- idio24gpio->chip.set_multiple = idio_24_gpio_set_multiple;
-
- girq = &idio24gpio->chip.irq;
- girq->chip = &idio_24_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;
-
- raw_spin_lock_init(&idio24gpio->lock);
+ pex8311_intcsr = pcim_iomap_table(pdev)[pci_plx_bar_index] + PLX_PEX8311_PCI_LCS_INTCSR;
+ idio_24_regs = pcim_iomap_table(pdev)[pci_bar_index];
+
+ pex8311_intcsr_map = devm_regmap_init_mmio(dev, pex8311_intcsr,
+ &pex8311_intcsr_regmap_config);
+ if (IS_ERR(pex8311_intcsr_map))
+ return dev_err_probe(dev, PTR_ERR(pex8311_intcsr_map),
+ "Unable to initialize PEX8311 register map\n");
+ idio_24_map = devm_regmap_init_mmio(dev, idio_24_regs,
+ &idio_24_regmap_config);
+ if (IS_ERR(idio_24_map))
+ return dev_err_probe(dev, PTR_ERR(idio_24_map),
+ "Unable to initialize register map\n");
+
+ chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->irq_drv_data = devm_kzalloc(dev, sizeof(irq_type), GFP_KERNEL);
+ if (!irq_type)
+ return -ENOMEM;
+
+ chip->name = name;
+ chip->status_base = IDIO_24_STATUS_BASE;
+ chip->mask_base = IDIO_24_COS_ENABLE;
+ chip->ack_base = IDIO_24_ACK_BASE;
+ chip->num_regs = 4;
+ chip->irqs = idio_24_regmap_irqs;
+ chip->num_irqs = ARRAY_SIZE(idio_24_regmap_irqs);
+ chip->handle_mask_sync = idio_24_handle_mask_sync;
+ chip->set_type_config = idio_24_set_type_config;
/* Software board reset */
- iowrite8(0, &idio24gpio->reg->soft_reset);
+ err = regmap_write(idio_24_map, IDIO_24_SOFT_RESET, 0);
+ if (err)
+ return err;
/*
* enable PLX PEX8311 internal PCI wire interrupt and local interrupt
* input
*/
- iowrite8((INTCSR_INTERNAL_PCI_WIRE | INTCSR_LOCAL_INPUT) >> 8,
- idio24gpio->plx + PLX_PEX8311_PCI_LCS_INTCSR + 1);
-
- err = devm_gpiochip_add_data(dev, &idio24gpio->chip, idio24gpio);
- if (err) {
- dev_err(dev, "GPIO registering failed (%d)\n", err);
+ err = regmap_update_bits(pex8311_intcsr_map, 0x0, IDIO_24_ENABLE_IRQ,
+ IDIO_24_ENABLE_IRQ);
+ if (err)
return err;
- }
-
- err = devm_request_irq(dev, pdev->irq, idio_24_irq_handler, IRQF_SHARED,
- name, idio24gpio);
- if (err) {
- dev_err(dev, "IRQ handler registering failed (%d)\n", err);
- return err;
- }
- return 0;
+ err = devm_regmap_add_irq_chip(dev, idio_24_map, pdev->irq, 0, 0, chip,
+ &chip_data);
+ if (err)
+ return dev_err_probe(dev, err, "IRQ registration failed\n");
+
+ gpio_config.parent = dev;
+ gpio_config.regmap = idio_24_map;
+ gpio_config.ngpio = IDIO_24_NGPIO;
+ gpio_config.names = idio_24_names;
+ gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(IDIO_24_DAT_BASE);
+ gpio_config.reg_set_base = GPIO_REGMAP_ADDR(IDIO_24_DAT_BASE);
+ gpio_config.reg_dir_out_base = GPIO_REGMAP_ADDR(IDIO_24_CONTROL_REG);
+ gpio_config.ngpio_per_reg = IDIO_24_NGPIO_PER_REG;
+ gpio_config.irq_domain = regmap_irq_get_domain(chip_data);
+ gpio_config.reg_mask_xlate = idio_24_reg_mask_xlate;
+
+ return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
}
static const struct pci_device_id idio_24_pci_dev_id[] = {
--
2.39.2
On Mon, Feb 27, 2023 at 08:53:39PM -0500, William Breathitt Gray wrote:
> A patch to pass the device regmap and irq_drv_data as a parameters for
> the struct regmap_irq_chip set_type_config() is included. This is needed
> by idio_24_set_type_config() in order to update the type configuration
> on the device as well as irq_drv_data for idio_24_handle_mask_sync().
The values from the config buffer are supposed to be written out in
regmap_irq_sync_unlock() - why is something custom needed here?
On Tue, Feb 28, 2023 at 07:09:50PM +0000, Mark Brown wrote:
> On Mon, Feb 27, 2023 at 08:53:39PM -0500, William Breathitt Gray wrote:
>
> > A patch to pass the device regmap and irq_drv_data as a parameters for
> > the struct regmap_irq_chip set_type_config() is included. This is needed
> > by idio_24_set_type_config() in order to update the type configuration
> > on the device as well as irq_drv_data for idio_24_handle_mask_sync().
>
> The values from the config buffer are supposed to be written out in
> regmap_irq_sync_unlock() - why is something custom needed here?
The PCIe-IDIO-24 "COS Enable" serves a dual purpose of interrupt
enabling/disabling as well as configuring the interrupt types. Since
this register is used for masking, config buffer would clobber the
register if we use it in this particular case. Instead, we ignore the
config buffer and configure the type directly for the device (handling
the case where interrupts are masked and shouldn't be enabled).
William Breathitt Gray
On Mon, Feb 27, 2023 at 09:19:28PM -0500, William Breathitt Gray wrote:
> On Tue, Feb 28, 2023 at 07:09:50PM +0000, Mark Brown wrote:
> > The values from the config buffer are supposed to be written out in
> > regmap_irq_sync_unlock() - why is something custom needed here?
> The PCIe-IDIO-24 "COS Enable" serves a dual purpose of interrupt
> enabling/disabling as well as configuring the interrupt types. Since
> this register is used for masking, config buffer would clobber the
> register if we use it in this particular case. Instead, we ignore the
> config buffer and configure the type directly for the device (handling
> the case where interrupts are masked and shouldn't be enabled).
Could you be more concrete about what's going on here please? In what
way does this "COS Enable" serve these dual functions and why do they
clobber each other?
On Tue, Feb 28, 2023 at 07:28:45PM +0000, Mark Brown wrote:
> On Mon, Feb 27, 2023 at 09:19:28PM -0500, William Breathitt Gray wrote:
> > On Tue, Feb 28, 2023 at 07:09:50PM +0000, Mark Brown wrote:
>
> > > The values from the config buffer are supposed to be written out in
> > > regmap_irq_sync_unlock() - why is something custom needed here?
>
> > The PCIe-IDIO-24 "COS Enable" serves a dual purpose of interrupt
> > enabling/disabling as well as configuring the interrupt types. Since
> > this register is used for masking, config buffer would clobber the
> > register if we use it in this particular case. Instead, we ignore the
> > config buffer and configure the type directly for the device (handling
> > the case where interrupts are masked and shouldn't be enabled).
>
> Could you be more concrete about what's going on here please? In what
> way does this "COS Enable" serve these dual functions and why do they
> clobber each other?
An explanation of the device registers is provided in [PATCH 3/3];
here's the relevant portion:
The COS Enable register is used to enable/disable interrupts and
configure the interrupt levels; each bit maps to a group of eight inputs
as described below:
Bit 0: IRQ EN Rising Edge IN0-7
Bit 1: IRQ EN Rising Edge IN8-15
Bit 2: IRQ EN Rising Edge IN16-23
Bit 3: IRQ EN Rising Edge TTL0-7
Bit 4: IRQ EN Falling Edge IN0-7
Bit 5: IRQ EN Falling Edge IN8-15
Bit 6: IRQ EN Falling Edge IN16-23
Bit 7: IRQ EN Falling Edge TTL0-7
An interrupt is asserted when a change-of-state matching the interrupt
level configuration respective for a particular group of eight inputs
with enabled COS is detected.
So in order to mask lines, the respective bits need to be set to 0.
However, if we use the regmap-irq config buffer to set the type, this
mask will be cloberred and the disabled lines become enabled. To prevent
the clobber, we can save the type configuration to irq_drv_data for use
later in handle_mask_sync() and then update the type in this COS Enable
register only when the lines are unmasked.
William Breathitt Gray
Hi,
Am 2023-02-28 02:53, schrieb William Breathitt Gray:
> A struct gpio_regmap is passed as a parameter for reg_mask_xlate(), but
> for callbacks to access its members the declaration must be exposed.
That parameter is only an opaque one to call any gpio_regmap_*().
> Move the struct gpio_regmap declaration from drivers/gpio/gpio-regmap.c
> to include/linux/gpio/regmap.h so callbacks can properly interact with
> struct gpio_regmap members.
That struct should be kept private. It seems you only need the
regmap. Either introduce a gpio_regmap_get_regmap() or add the
regmap to a private struct and use gpio_regmap_get_drvdata().
-michael
On Tue, Feb 28, 2023 at 09:44:02PM +0100, Michael Walle wrote:
> Hi,
>
> Am 2023-02-28 02:53, schrieb William Breathitt Gray:
> > A struct gpio_regmap is passed as a parameter for reg_mask_xlate(), but
> > for callbacks to access its members the declaration must be exposed.
>
> That parameter is only an opaque one to call any gpio_regmap_*().
>
> > Move the struct gpio_regmap declaration from drivers/gpio/gpio-regmap.c
> > to include/linux/gpio/regmap.h so callbacks can properly interact with
> > struct gpio_regmap members.
>
> That struct should be kept private. It seems you only need the
> regmap. Either introduce a gpio_regmap_get_regmap() or add the
> regmap to a private struct and use gpio_regmap_get_drvdata().
>
> -michael
Ah, I'll drop this patch then and use gpio_regmap_get_drvdata() in v2
instead to get access to the regmap.
William Breathitt Gray
Hi William,
I love your patch! Perhaps something to improve:
[auto build test WARNING on 4827aae061337251bb91801b316157a78b845ec7]
url: https://github.com/intel-lab-lkp/linux/commits/William-Breathitt-Gray/regmap-Pass-regmap-and-irq_drv_data-as-parameters-for-set_type_config/20230301-030010
base: 4827aae061337251bb91801b316157a78b845ec7
patch link: https://lore.kernel.org/r/39f4c4b7083b2a50973e0ac5b4b1db5040fcda53.1677547393.git.william.gray%40linaro.org
patch subject: [PATCH 3/3] gpio: pcie-idio-24: Migrate to the regmap API
config: x86_64-randconfig-a001-20230227 (https://download.01.org/0day-ci/archive/20230301/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/6f59a8e427da386890ca2eaccab41064a250ebea
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review William-Breathitt-Gray/regmap-Pass-regmap-and-irq_drv_data-as-parameters-for-set_type_config/20230301-030010
git checkout 6f59a8e427da386890ca2eaccab41064a250ebea
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpio/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/gpio/gpio-pcie-idio-24.c:353:7: warning: variable 'irq_type' is uninitialized when used here [-Wuninitialized]
if (!irq_type)
^~~~~~~~
drivers/gpio/gpio-pcie-idio-24.c:319:23: note: initialize the variable 'irq_type' to silence this warning
unsigned int irq_type;
^
= 0
1 warning generated.
vim +/irq_type +353 drivers/gpio/gpio-pcie-idio-24.c
305
306 static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
307 {
308 struct device *const dev = &pdev->dev;
309 int err;
310 const size_t pci_plx_bar_index = 1;
311 const size_t pci_bar_index = 2;
312 const char *const name = pci_name(pdev);
313 struct gpio_regmap_config gpio_config = {};
314 void __iomem *pex8311_intcsr;
315 void __iomem *idio_24_regs;
316 struct regmap *pex8311_intcsr_map;
317 struct regmap *idio_24_map;
318 struct regmap_irq_chip *chip;
319 unsigned int irq_type;
320 struct regmap_irq_chip_data *chip_data;
321
322 err = pcim_enable_device(pdev);
323 if (err) {
324 dev_err(dev, "Failed to enable PCI device (%d)\n", err);
325 return err;
326 }
327
328 err = pcim_iomap_regions(pdev, BIT(pci_plx_bar_index) | BIT(pci_bar_index), name);
329 if (err) {
330 dev_err(dev, "Unable to map PCI I/O addresses (%d)\n", err);
331 return err;
332 }
333
334 pex8311_intcsr = pcim_iomap_table(pdev)[pci_plx_bar_index] + PLX_PEX8311_PCI_LCS_INTCSR;
335 idio_24_regs = pcim_iomap_table(pdev)[pci_bar_index];
336
337 pex8311_intcsr_map = devm_regmap_init_mmio(dev, pex8311_intcsr,
338 &pex8311_intcsr_regmap_config);
339 if (IS_ERR(pex8311_intcsr_map))
340 return dev_err_probe(dev, PTR_ERR(pex8311_intcsr_map),
341 "Unable to initialize PEX8311 register map\n");
342 idio_24_map = devm_regmap_init_mmio(dev, idio_24_regs,
343 &idio_24_regmap_config);
344 if (IS_ERR(idio_24_map))
345 return dev_err_probe(dev, PTR_ERR(idio_24_map),
346 "Unable to initialize register map\n");
347
348 chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
349 if (!chip)
350 return -ENOMEM;
351
352 chip->irq_drv_data = devm_kzalloc(dev, sizeof(irq_type), GFP_KERNEL);
> 353 if (!irq_type)
354 return -ENOMEM;
355
356 chip->name = name;
357 chip->status_base = IDIO_24_STATUS_BASE;
358 chip->mask_base = IDIO_24_COS_ENABLE;
359 chip->ack_base = IDIO_24_ACK_BASE;
360 chip->num_regs = 4;
361 chip->irqs = idio_24_regmap_irqs;
362 chip->num_irqs = ARRAY_SIZE(idio_24_regmap_irqs);
363 chip->handle_mask_sync = idio_24_handle_mask_sync;
364 chip->set_type_config = idio_24_set_type_config;
365
366 /* Software board reset */
367 err = regmap_write(idio_24_map, IDIO_24_SOFT_RESET, 0);
368 if (err)
369 return err;
370 /*
371 * enable PLX PEX8311 internal PCI wire interrupt and local interrupt
372 * input
373 */
374 err = regmap_update_bits(pex8311_intcsr_map, 0x0, IDIO_24_ENABLE_IRQ,
375 IDIO_24_ENABLE_IRQ);
376 if (err)
377 return err;
378
379 err = devm_regmap_add_irq_chip(dev, idio_24_map, pdev->irq, 0, 0, chip,
380 &chip_data);
381 if (err)
382 return dev_err_probe(dev, err, "IRQ registration failed\n");
383
384 gpio_config.parent = dev;
385 gpio_config.regmap = idio_24_map;
386 gpio_config.ngpio = IDIO_24_NGPIO;
387 gpio_config.names = idio_24_names;
388 gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(IDIO_24_DAT_BASE);
389 gpio_config.reg_set_base = GPIO_REGMAP_ADDR(IDIO_24_DAT_BASE);
390 gpio_config.reg_dir_out_base = GPIO_REGMAP_ADDR(IDIO_24_CONTROL_REG);
391 gpio_config.ngpio_per_reg = IDIO_24_NGPIO_PER_REG;
392 gpio_config.irq_domain = regmap_irq_get_domain(chip_data);
393 gpio_config.reg_mask_xlate = idio_24_reg_mask_xlate;
394
395 return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
396 }
397
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests