| Message ID | cover.1669100542.git.william.gray@linaro.org |
|---|---|
| Headers | show |
| Series | Migrate i8255 GPIO drivers to regmap API | expand |
On Wed, Nov 23, 2022 at 07:42:24PM +0200, Andy Shevchenko wrote: > On Tue, Nov 22, 2022 at 02:11:03AM -0500, William Breathitt Gray wrote: > > +/** > > + * struct i8255_regmap_config - Configuration for the register map of an i8255 > > + * @parent: parent device > > + * @map: regmap for the i8255 > > + * @num_ppi: number of i8255 Programmable Peripheral Interface > > + * @names: (optional) array of names for gpios > > + * @domain: (optional) IRQ domain if the controller is interrupt-capable > > > + * Note: The regmap is expected to have cache enabled and i8255 control > > + * registers not marked as volatile. > > Have you considered to catch wrong configurations by BUILD_BUG_ON() / > static_assert() / another means of validation? Ideally, I'd like to check for these configurations, but struct regmap is an opaque type (the definition is in drivers/base/regmap/internal.h). Do you know if there is some way to query a struct regmap for whether a particular register has cache enabled or is marked as volatile? William Breathitt Gray
On Tue, Nov 22, 2022 at 02:11:00AM -0500, William Breathitt Gray wrote: > The regmap API supports IO port accessors so we can take advantage of > regmap abstractions rather than handling access to the device registers > directly in the driver. > > For the 104-dio-48e we have the following IRQ registers (0xB and 0xF): > > Base Address +B (Write): Enable Interrupt > Base Address +B (Read): Disable Interrupt > Base Address +F (Read/Write): Clear Interrupt > > Any write to 0xB will enable interrupts, while any read will disable > interrupts. Interrupts are cleared by a read or any write to 0xF. > There's no IRQ status register, so software has to assume that if an > interrupt is raised then it was for the 104-DIO-48E device. ... > +/* only bit 3 on each respective Port C supports interrupts */ > +#define DIO48E_REGMAP_IRQ(_ppi) \ > + [19 + (_ppi) * 24] = { \ > + .mask = BIT(_ppi), \ > + .type = { .types_supported = IRQ_TYPE_EDGE_RISING, }, \ When {} on a single line, the trailing comma is not needed. .type = { .types_supported = IRQ_TYPE_EDGE_RISING }, \ would work as well. A nit: I would put \ on the same column by using TABs before each of them. > } ... > + /* if all previously masked, enable interrupts when unmasking */ > + if (prev_mask == all_masked) { > + err = regmap_write(map, DIO48E_ENABLE_INTERRUPT, 0x00); > + if (err) > + return err; > + /* if all are currently masked, disable interrupts */ > + } else if (mask_buf == all_masked) { > + err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val); > + if (err) > + return err; > + } Haven't looked at the rest of the series, but if there is nothing with this code piece, the above can be optimized to if (prev_mask == all_masked) return regmap_write(map, DIO48E_ENABLE_INTERRUPT, 0x00); if (mask_buf == all_masked) return regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val); ... > + /* Initialize device interrupt state */ > + err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val); > + if (err) > + return err; Use ->init_hw() callback for this. ... > + err = devm_regmap_add_irq_chip(dev, map, irq[id], 0, 0, chip, > + &chip_data); I would leave this on one line. It's only 82. > + if (err) { > + dev_err(dev, "IRQ registration failed (%d)\n", err); > + return err; > + }
On Tue, Nov 22, 2022 at 02:11:02AM -0500, William Breathitt Gray wrote: > The regmap API supports IO port accessors so we can take advantage of > regmap abstractions rather than handling access to the device registers > directly in the driver. Despite the underlying interface being based on > i8255, it is simpler to use the gpio-regmap API directly because the > 104-IDI-48 device features only input signals. Therefore, the dependence > on the i8255 GPIO library is removed in this patch. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> (One nit-pick below) > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: William Breathitt Gray <william.gray@linaro.org> > --- > drivers/gpio/Kconfig | 2 +- > drivers/gpio/gpio-104-idi-48.c | 97 +++++++--------------------------- > 2 files changed, 21 insertions(+), 78 deletions(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 6892979e511a..dd34039fc31b 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -873,7 +873,7 @@ config GPIO_104_IDI_48 > select ISA_BUS_API > select REGMAP_IRQ > select GPIOLIB_IRQCHIP > - select GPIO_I8255 > + select GPIO_REGMAP > help > Enables GPIO support for the ACCES 104-IDI-48 family (104-IDI-48A, > 104-IDI-48AC, 104-IDI-48B, 104-IDI-48BC). The base port addresses for > diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c > index f77c05571062..2584f411ae67 100644 > --- a/drivers/gpio/gpio-104-idi-48.c > +++ b/drivers/gpio/gpio-104-idi-48.c > @@ -9,7 +9,7 @@ > #include <linux/bits.h> > #include <linux/device.h> > #include <linux/err.h> > -#include <linux/gpio/driver.h> > +#include <linux/gpio/regmap.h> > #include <linux/interrupt.h> > #include <linux/ioport.h> > #include <linux/irq.h> > @@ -20,10 +20,6 @@ > #include <linux/regmap.h> > #include <linux/types.h> > > -#include "gpio-i8255.h" > - > -MODULE_IMPORT_NS(I8255); > - > #define IDI_48_EXTENT 8 > #define MAX_NUM_IDI_48 max_num_isa_dev(IDI_48_EXTENT) > > @@ -40,56 +36,17 @@ MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers"); > #define IDI48_IRQ_STATUS 0x7 > #define IDI48_IRQ_ENABLE IDI48_IRQ_STATUS > > -/** > - * struct idi_48_reg - device register structure > - * @port0: Port 0 Inputs > - * @unused: Unused > - * @port1: Port 1 Inputs > - * @irq: Read: IRQ Status Register/IRQ Clear > - * Write: IRQ Enable/Disable > - */ > -struct idi_48_reg { > - u8 port0[3]; > - u8 unused; > - u8 port1[3]; > - u8 irq; > -}; > - > -/** > - * struct idi_48_gpio - GPIO device private data structure > - * @chip: instance of the gpio_chip > - * @reg: I/O address offset for the device registers > - */ > -struct idi_48_gpio { > - struct gpio_chip chip; > - struct idi_48_reg __iomem *reg; > -}; > - > -static int idi_48_gpio_get_direction(struct gpio_chip *chip, unsigned int offset) > -{ > - return GPIO_LINE_DIRECTION_IN; > -} > - > -static int idi_48_gpio_direction_input(struct gpio_chip *chip, unsigned int offset) > -{ > - return 0; > -} > - > -static int idi_48_gpio_get(struct gpio_chip *chip, unsigned int offset) > -{ > - struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip); > - void __iomem *const ppi = idi48gpio->reg; > - > - return i8255_get(ppi, offset); > -} > - > -static int idi_48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask, > - unsigned long *bits) > +static int idi_48_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base, > + unsigned int offset, unsigned int *reg, > + unsigned int *mask) > { > - struct idi_48_gpio *const idi48gpio = gpiochip_get_data(chip); > - void __iomem *const ppi = idi48gpio->reg; > + const unsigned int line = offset % 8; > + const unsigned int stride = offset / 8; > + const unsigned int port = (stride / 3) * 4; > + const unsigned int port_stride = stride % 3; > > - i8255_get_multiple(ppi, mask, bits, chip->ngpio); > + *reg = base + port + port_stride; > + *mask = BIT(line); > > return 0; > } > @@ -166,18 +123,14 @@ static const char *idi48_names[IDI48_NGPIO] = { > > static int idi_48_probe(struct device *dev, unsigned int id) > { > - struct idi_48_gpio *idi48gpio; > const char *const name = dev_name(dev); > + struct gpio_regmap_config config = {0}; {} will work in the same way. > void __iomem *regs; > struct regmap *map; > struct regmap_irq_chip *chip; > struct regmap_irq_chip_data *chip_data; > int err; > > - idi48gpio = devm_kzalloc(dev, sizeof(*idi48gpio), GFP_KERNEL); > - if (!idi48gpio) > - return -ENOMEM; > - > if (!devm_request_region(dev, base[id], IDI_48_EXTENT, name)) { > dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", > base[id], base[id] + IDI_48_EXTENT); > @@ -187,7 +140,6 @@ static int idi_48_probe(struct device *dev, unsigned int id) > regs = devm_ioport_map(dev, base[id], IDI_48_EXTENT); > if (!regs) > return -ENOMEM; > - idi48gpio->reg = regs; > > map = devm_regmap_init_mmio(dev, regs, &idi48_regmap_config); > if (IS_ERR(map)) > @@ -212,25 +164,16 @@ static int idi_48_probe(struct device *dev, unsigned int id) > return err; > } > > - idi48gpio->chip.label = name; > - idi48gpio->chip.parent = dev; > - idi48gpio->chip.owner = THIS_MODULE; > - idi48gpio->chip.base = -1; > - idi48gpio->chip.ngpio = IDI48_NGPIO; > - idi48gpio->chip.names = idi48_names; > - idi48gpio->chip.get_direction = idi_48_gpio_get_direction; > - idi48gpio->chip.direction_input = idi_48_gpio_direction_input; > - idi48gpio->chip.get = idi_48_gpio_get; > - idi48gpio->chip.get_multiple = idi_48_gpio_get_multiple; > - > - err = devm_gpiochip_add_data(dev, &idi48gpio->chip, idi48gpio); > - if (err) { > - dev_err(dev, "GPIO registering failed (%d)\n", err); > - return err; > - } > + config.parent = dev; > + config.regmap = map; > + config.ngpio = IDI48_NGPIO; > + config.names = idi48_names; > + config.reg_dat_base = GPIO_REGMAP_ADDR(0x0); > + config.ngpio_per_reg = 8; > + config.reg_mask_xlate = idi_48_reg_mask_xlate; > + config.irq_domain = regmap_irq_get_domain(chip_data); > > - return gpiochip_irqchip_add_domain(&idi48gpio->chip, > - regmap_irq_get_domain(chip_data)); > + return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &config)); > } > > static struct isa_driver idi_48_driver = { > -- > 2.38.1 >
On Tue, Nov 22, 2022 at 02:11:03AM -0500, William Breathitt Gray wrote: > The regmap API supports IO port accessors so we can take advantage of > regmap abstractions rather than handling access to the device registers > directly in the driver. > > By leveraging the gpio-regmap API, the i8255 library is reduced to > simply a devm_i8255_regmap_register() function, a configuration > structure struct i8255_regmap_config, and a helper macro > i8255_volatile_regmap_range() provided to simplify volatile PPI register > hinting for the regmap. > > Legacy functions and code will be removed once all consumers have > migrated to the new i8255 library interface. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> (See one remark below and once comment to address) > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: William Breathitt Gray <william.gray@linaro.org> > --- > drivers/gpio/Kconfig | 1 + > drivers/gpio/gpio-i8255.c | 119 ++++++++++++++++++++++++++++++++++---- > drivers/gpio/gpio-i8255.h | 27 +++++++++ > 3 files changed, 135 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index dd34039fc31b..88dfdc62992f 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -831,6 +831,7 @@ menu "Port-mapped I/O GPIO drivers" > > config GPIO_I8255 > tristate > + select GPIO_REGMAP > help > Enables support for the i8255 interface library functions. The i8255 > interface library provides functions to facilitate communication with > diff --git a/drivers/gpio/gpio-i8255.c b/drivers/gpio/gpio-i8255.c > index 9b97db418df1..9ecb2e9b97f9 100644 > --- a/drivers/gpio/gpio-i8255.c > +++ b/drivers/gpio/gpio-i8255.c > @@ -4,23 +4,31 @@ > * Copyright (C) 2022 William Breathitt Gray > */ > #include <linux/bitmap.h> > +#include <linux/device.h> > #include <linux/err.h> > #include <linux/export.h> > +#include <linux/gpio/regmap.h> > #include <linux/io.h> > #include <linux/module.h> > +#include <linux/regmap.h> > #include <linux/spinlock.h> > #include <linux/types.h> > > #include "gpio-i8255.h" > > +#define I8255_NGPIO 24 > +#define I8255_NGPIO_PER_REG 8 > #define I8255_CONTROL_PORTC_LOWER_DIRECTION BIT(0) > #define I8255_CONTROL_PORTB_DIRECTION BIT(1) > #define I8255_CONTROL_PORTC_UPPER_DIRECTION BIT(3) > #define I8255_CONTROL_PORTA_DIRECTION BIT(4) > #define I8255_CONTROL_MODE_SET BIT(7) > -#define I8255_PORTA 0 > -#define I8255_PORTB 1 > -#define I8255_PORTC 2 > +#define I8255_PORTA 0x0 > +#define I8255_PORTB 0x1 > +#define I8255_PORTC 0x2 > +#define I8255_CONTROL 0x3 > +#define I8255_REG_DAT_BASE I8255_PORTA > +#define I8255_REG_DIR_IN_BASE I8255_CONTROL > > static int i8255_get_port(struct i8255 __iomem *const ppi, > const unsigned long io_port, const unsigned long mask) > @@ -31,20 +39,19 @@ static int i8255_get_port(struct i8255 __iomem *const ppi, > return ioread8(&ppi[bank].port[ppi_port]) & mask; > } > > -static u8 i8255_direction_mask(const unsigned long offset) > +static int i8255_direction_mask(const unsigned int offset) > { > - const unsigned long port_offset = offset % 8; > - const unsigned long io_port = offset / 8; > - const unsigned long ppi_port = io_port % 3; > + const unsigned int stride = offset / I8255_NGPIO_PER_REG; > + const unsigned int line = offset % I8255_NGPIO_PER_REG; > > - switch (ppi_port) { > + switch (stride) { > case I8255_PORTA: > return I8255_CONTROL_PORTA_DIRECTION; > case I8255_PORTB: > return I8255_CONTROL_PORTB_DIRECTION; > case I8255_PORTC: > /* Port C can be configured by nibble */ > - if (port_offset >= 4) > + if (line >= 4) > return I8255_CONTROL_PORTC_UPPER_DIRECTION; > return I8255_CONTROL_PORTC_LOWER_DIRECTION; > default: > @@ -53,6 +60,49 @@ static u8 i8255_direction_mask(const unsigned long offset) > } > } > > +static int i8255_ppi_init(struct regmap *const map, const unsigned int base) > +{ > + int err; > + > + /* Configure all ports to MODE 0 output mode */ > + err = regmap_write(map, base + I8255_CONTROL, I8255_CONTROL_MODE_SET); > + if (err) > + return err; > + > + /* Initialize all GPIO to output 0 */ > + err = regmap_write(map, base + I8255_PORTA, 0x00); > + if (err) > + return err; > + err = regmap_write(map, base + I8255_PORTB, 0x00); > + if (err) > + return err; > + return regmap_write(map, base + I8255_PORTC, 0x00); > +} > + > +static int i8255_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base, > + unsigned int offset, unsigned int *reg, > + unsigned int *mask) > +{ > + const unsigned int ppi = offset / I8255_NGPIO; > + const unsigned int ppi_offset = offset % I8255_NGPIO; > + const unsigned int stride = ppi_offset / I8255_NGPIO_PER_REG; > + const unsigned int line = ppi_offset % I8255_NGPIO_PER_REG; > + > + switch (base) { > + case I8255_REG_DAT_BASE: > + *reg = base + stride + ppi * 4; > + *mask = BIT(line); > + return 0; > + case I8255_REG_DIR_IN_BASE: > + *reg = base + ppi * 4; > + *mask = i8255_direction_mask(ppi_offset); > + return 0; > + default: > + /* Should never reach this path */ > + return -EINVAL; > + } > +} > + > static void i8255_set_port(struct i8255 __iomem *const ppi, > struct i8255_state *const state, > const unsigned long io_port, > @@ -93,7 +143,7 @@ void i8255_direction_input(struct i8255 __iomem *const ppi, > spin_lock_irqsave(&state[bank].lock, flags); > > state[bank].control_state |= I8255_CONTROL_MODE_SET; > - state[bank].control_state |= i8255_direction_mask(offset); > + state[bank].control_state |= i8255_direction_mask(offset % 24); > > iowrite8(state[bank].control_state, &ppi[bank].control); > > @@ -125,7 +175,7 @@ void i8255_direction_output(struct i8255 __iomem *const ppi, > spin_lock_irqsave(&state[bank].lock, flags); > > state[bank].control_state |= I8255_CONTROL_MODE_SET; > - state[bank].control_state &= ~i8255_direction_mask(offset); > + state[bank].control_state &= ~i8255_direction_mask(offset % 24); > > iowrite8(state[bank].control_state, &ppi[bank].control); > > @@ -165,7 +215,7 @@ int i8255_get_direction(const struct i8255_state *const state, > const unsigned long io_port = offset / 8; > const unsigned long bank = io_port / 3; > > - return !!(state[bank].control_state & i8255_direction_mask(offset)); > + return !!(state[bank].control_state & i8255_direction_mask(offset % 24)); > } > EXPORT_SYMBOL_NS_GPL(i8255_get_direction, I8255); > > @@ -282,6 +332,51 @@ void i8255_state_init(struct i8255_state *const state, > } > EXPORT_SYMBOL_NS_GPL(i8255_state_init, I8255); > > +/** > + * devm_i8255_regmap_register - Register an i8255 GPIO controller > + * @dev: device that is registering this i8255 GPIO device > + * @config: configuration for i8255_regmap_config > + * > + * Registers an Intel 8255 Programmable Peripheral Interface GPIO controller. > + * Returns 0 on success and negative error number on failure. > + */ > +int devm_i8255_regmap_register(struct device *const dev, > + const struct i8255_regmap_config *const config) > +{ > + struct gpio_regmap_config gpio_config = {0}; > + unsigned long i; > + int err; > + > + if (!config->parent) > + return -EINVAL; > + > + if (!config->map) > + return -EINVAL; > + > + if (!config->num_ppi) > + return -EINVAL; > + > + for (i = 0; i < config->num_ppi; i++) { > + err = i8255_ppi_init(config->map, i * 4); > + if (err) > + return err; > + } > + > + gpio_config.parent = config->parent; > + gpio_config.regmap = config->map; > + gpio_config.ngpio = I8255_NGPIO * config->num_ppi; > + gpio_config.names = config->names; > + gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE); > + gpio_config.reg_set_base = GPIO_REGMAP_ADDR(I8255_REG_DAT_BASE); > + gpio_config.reg_dir_in_base = GPIO_REGMAP_ADDR(I8255_REG_DIR_IN_BASE); > + gpio_config.ngpio_per_reg = I8255_NGPIO_PER_REG; > + gpio_config.irq_domain = config->domain; > + gpio_config.reg_mask_xlate = i8255_reg_mask_xlate; > + > + return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config)); > +} > +EXPORT_SYMBOL_NS_GPL(devm_i8255_regmap_register, I8255); > + > MODULE_AUTHOR("William Breathitt Gray"); > MODULE_DESCRIPTION("Intel 8255 Programmable Peripheral Interface"); > MODULE_LICENSE("GPL"); > diff --git a/drivers/gpio/gpio-i8255.h b/drivers/gpio/gpio-i8255.h > index d9084aae9446..6ec987835c14 100644 > --- a/drivers/gpio/gpio-i8255.h > +++ b/drivers/gpio/gpio-i8255.h > @@ -3,6 +3,9 @@ > #ifndef _I8255_H_ > #define _I8255_H_ > +#include <linux/device.h> > +#include <linux/irqdomain.h> > +#include <linux/regmap.h> As far as I can see you have no users for these headers. You may inform compiler by providing forward declarations. Like struct device; after the inclusion block. > #include <linux/spinlock.h> > #include <linux/types.h> > > @@ -26,6 +29,30 @@ struct i8255_state { > u8 control_state; > }; > > +#define i8255_volatile_regmap_range(_base) regmap_reg_range(_base, _base + 0x2) > + > +/** > + * struct i8255_regmap_config - Configuration for the register map of an i8255 > + * @parent: parent device > + * @map: regmap for the i8255 > + * @num_ppi: number of i8255 Programmable Peripheral Interface > + * @names: (optional) array of names for gpios > + * @domain: (optional) IRQ domain if the controller is interrupt-capable > + * Note: The regmap is expected to have cache enabled and i8255 control > + * registers not marked as volatile. Have you considered to catch wrong configurations by BUILD_BUG_ON() / static_assert() / another means of validation? > + */ > +struct i8255_regmap_config { > + struct device *parent; > + struct regmap *map; > + int num_ppi; > + const char *const *names; > + struct irq_domain *domain; > +}; > + > +int devm_i8255_regmap_register(struct device *dev, > + const struct i8255_regmap_config *config); > + > void i8255_direction_input(struct i8255 __iomem *ppi, struct i8255_state *state, > unsigned long offset); > void i8255_direction_output(struct i8255 __iomem *ppi, > -- > 2.38.1 >
On Sun, Nov 27, 2022 at 07:31:48PM +0100, Michael Walle wrote: > Am 2022-11-22 11:29, schrieb William Breathitt Gray: > > On Wed, Nov 23, 2022 at 05:01:53PM +0200, Andy Shevchenko wrote: > > > On Tue, Nov 22, 2022 at 02:11:00AM -0500, William Breathitt Gray > > > wrote: > > > > + /* Initialize device interrupt state */ > > > > + err = regmap_read(map, DIO48E_DISABLE_INTERRUPT, &val); > > > > + if (err) > > > > + return err; > > > > > > Use ->init_hw() callback for this. > > > > In a subsequent patch 7/9 we remove direct gpio_chip registration in > > favor of the i8255 library registration via gpio_regmap. It doesn't look > > like gpio_regmap_register() sets the init_hw() callback. > > > > Michael, do you see any issues if I introduce init_hw() to > > gpio_regmap_config? Or do you think this IRQ initialization belongs > > somewhere else? > > Something like the following? > gpiochip->init_hw = config.irq_init_hw; > > gpiochip doesn't seem to be the correct place, gpiochip_add_irqchip() > is a noop for gpio-regmap, right? So using gpiochip_irqchip_init_hw() > seems wrong. > > Maybe make gpio-regmap call it on its own? But really we just connect > the regmap-irq to the gpiochip irqdomain. What is the purpose of the > .init_hw callback? I've looked at other drivers which use regmap-irq > and they all seem to just initialize the hardware in their _probe(). The purpose of that callback is to initialize IRQ part of the GPIO hardware in the appropriate point of time. Of course there are drivers that are using it and it's not in their ->probe():s, however you can tell that in the same flow, because it's called synchronously.
Am 2022-11-28 10:41, schrieb Andy Shevchenko: > Of course there are drivers that are using it and it's not in > their ->probe():s I was speaking of gpio drivers which use the regmap-irq stuff. I couldn't find any which are using {devm_,}regmap_add_irq_chip*() and gpiochip.init_hw(). -michael
On Mon, Nov 28, 2022 at 10:56:06AM +0100, Michael Walle wrote: > Am 2022-11-28 10:41, schrieb Andy Shevchenko: > > Of course there are drivers that are using it and it's not in > > their ->probe():s > > I was speaking of gpio drivers which use the regmap-irq stuff. I > couldn't find any which are using {devm_,}regmap_add_irq_chip*() > and gpiochip.init_hw(). Ah, that's true.
Changes in v3: - Changed handle_mask_sync description to show range using a more typical mathematical notation ('[' changed to ')') - Split addition of new i8255 library regmap interface and removal of old i8255 library interface to separate patches - Split migration of 104-dio-48e, 104-idi-48, and gpio-mm modules to regmap-irq, gpio-regmap, and new i8255 library interface to separate patches - Simplified logic in dio48e_handle_mask_sync(); the previous version separated the conditional evaluations from the if statements which made it more complicated than it needed to be - Removed initial interrupt clearing in dio48e_probe(); superfluous because clear_on_unmask is set to true so the interrupts will be cleared anyway later. - Replace IDI48_IRQ_REG with IDI48_IRQ_STATUS and IDI48_IRQ_ENABLE to make the intent of this register clear The regmap API supports IO port accessors so we can take advantage of regmap abstractions rather than handling access to the device registers directly in the driver. The 104-dio-48e and gpio-mm modules depend on the i8255 library and are thus updated accordingly. The 104-IDI-48 is hardwired as an input-only device. Because the i8255 control registers are not exposed on the 104-IDI-48, the i8255 library doesn't provide much benefit here. In this case it's simpler to utilize the gpio_regmap API directly, so this patch series does such and removes the i8255 library dependency from the 104-idi-48 module. The first patch in this series adjusts gpio_regmap to always set gpio_chip get_direction. This patch is larger independent of the rest of the patches in the series and can be picked up separately if desired. I'm including it here because it allows the input-only 104-idi-48 driver to continue reporting offset directions after migrating to gpio_regmap. A handle_mask_sync() callback is introduced for the regmap_irq API. This is used by the 104-dio-48e driver to handle its enable/disable interrupt functionality. The 104-DIO-48E has a single register to handle enabling/disabling interrupts: a write of any value to this register enables interrupts, while any read disables interrupts. By leveraging the gpio_regmap API, the i8255 library is reduced to simply a devm_i8255_regmap_register() function, a configuration structure struct i8255_regmap_config, and a helper macro i8255_volatile_regmap_range() provided to simplify volatile PPI register hinting for the regmap. The regmap_irq API is leveraged by the 104-idi-48 and 104-dio-48e modules to support their IRQ functionality. Do their respective regmap configurations need use_hwlock set to true in this case, or is adequate locking already handled by the regmap_irq API? William Breathitt Gray (9): gpio: regmap: Always set gpio_chip get_direction regmap-irq: Add handle_mask_sync() callback gpio: 104-dio-48e: Migrate to the regmap-irq API gpio: 104-idi-48: Migrate to the regmap-irq API gpio: 104-idi-48: Migrate to gpio-regmap API gpio: i8255: Migrate to gpio-regmap API gpio: 104-dio-48e: Migrate to regmap API gpio: gpio-mm: Migrate to regmap API gpio: i8255: Remove unused legacy interface drivers/base/regmap/regmap-irq.c | 44 +++- drivers/gpio/Kconfig | 5 +- drivers/gpio/gpio-104-dio-48e.c | 395 ++++++++++--------------------- drivers/gpio/gpio-104-idi-48.c | 325 ++++++++----------------- drivers/gpio/gpio-gpio-mm.c | 153 +++--------- drivers/gpio/gpio-i8255.c | 320 +++++++------------------ drivers/gpio/gpio-i8255.h | 54 ++--- drivers/gpio/gpio-regmap.c | 7 +- include/linux/regmap.h | 5 + 9 files changed, 412 insertions(+), 896 deletions(-) base-commit: 3687a82b1db1c827cc4b367e3efde3235f68d9f6