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

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