Migrate IDIO-16 GPIO drivers to regmap API
Subsystems (4)
v3 cover Migrate IDIO-16 GPIO drivers to regmap API
Changes in v3:
- Refactor for map parameter removal from handle_mask_sync(); e.g. declare struct idio_16_data and utilize for irq_drv_data, inline data->map in idio_16_handle_mask_sync(), etc
- Cleanups and line wrapping to 100 characters rather than 80
- Change dev_err_probe() to dev_err(); devm_idio_16_regmap_register() is technically not a probe() callback
- Remove volatile regmap configuration structures; the existing read regmap table can be used to configure the volatile regmap table
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 | 257 ++++++++++++++++------------ drivers/gpio/gpio-idio-16.h | 79 +++------ drivers/gpio/gpio-pci-idio-16.c | 294 +++++++------------------------- 5 files changed, 293 insertions(+), 630 deletions(-)
base-commit: 2093bcd872321a5301470978231b23cc121e3476
prerequisite-patch-id: cd19046150b7cff1be4ac7152198777aa960a3df
prerequisite-patch-id: bd3e3830d9ce4f3876a77483364d7190b7fdffa7
Signature · 1 line
2.39.2
PATCH v3 1/4 gpio: 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.
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.
Legacy functions and code will be removed once all consumers have migrated to the new idio-16 library interface.
For IDIO-16 devices we have the following IRQ registers:
Base Address +1 (Write): Clear Interrupt
Base Address +2 (Read): Enable Interrupt
Base Address +2 (Write): Disable Interrupt
An interrupt is asserted whenever a change-of-state is detected on any of the inputs. Any write to 0x2 will disable interrupts, while any read will enable interrupts. Interrupts are cleared by a write to 0x1.
For 104-IDIO-16 devices, there is no IRQ status register, so software has to assume that if an interrupt is raised then it was for the 104-IDIO-16 device.
For PCI-IDIO-16 devices, there is an additional IRQ register:
Base Address +6 (Read): Interrupt Status
Interrupt status can be read from 0x6 where bit 2 set indicates that an IRQ has been generated.
---
Changes in v3:
- Refactor for map parameter removal from handle_mask_sync(); e.g. declare struct idio_16_data and utilize for irq_drv_data, inline data->map in idio_16_handle_mask_sync(), etc
- Cleanups and line wrapping to 100 characters rather than 80
- Change dev_err_probe() to dev_err(); devm_idio_16_regmap_register() is technically not a probe() callback
- Drop superfluous comment line in struct idio_16_state
drivers/gpio/Kconfig | 3 + drivers/gpio/gpio-idio-16.c | 160 ++++++++++++++++++++++++++++++++++++ drivers/gpio/gpio-idio-16.h | 26 ++++++ 3 files changed, 189 insertions(+)
drivers/gpio/Kconfig +3 -0
@@ -111,6 +111,9 @@ config GPIO_MAX730X config GPIO_IDIO_16 tristate + select REGMAP_IRQ + select GPIOLIB_IRQCHIP + select GPIO_REGMAP help Enables support for the idio-16 library functions. The idio-16 library provides functions to facilitate communication with devices within the
drivers/gpio/gpio-idio-16.c +160 -0
175 lines not on this page · raw message
drivers/gpio/gpio-idio-16.h +26 -0
@@ -6,6 +6,30 @@#include <linux/spinlock.h> #include <linux/types.h> +struct device ; +struct regmap ; +struct regmap_irq ; + +/** +* struct idio_16_regmap_config - Configuration for the IDIO-16 register map +* @parent: parent device +* @map: regmap for the IDIO-16 device +* @regmap_irqs: descriptors for individual IRQs +* @num_regmap_irqs: number of IRQ descriptors +* @irq: IRQ number for the IDIO-16 device +* @no_status: device has no status register +* @filters: device has input filters +*/ +struct idio_16_regmap_config { +struct device *parent; +struct regmap *map; +const struct regmap_irq *regmap_irqs; +int num_regmap_irqs; +unsigned int irq; +bool no_status; +bool filters; +}; +/** * struct idio_16 - IDIO-16 registers structure * @out0_7: Read: FET Drive Outputs 0-7
@@ -68,4 +92,6 @@ void idio_16_set_multiple(struct idio_16 __iomem *reg, 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_ */
Signature · 1 line
2.39.2
Re: [PATCH v3 1/4] gpio: idio-16: Migrate to the regmap API
Quoted 31 lines · William Breathitt Gray
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 regmap API, the idio-16 library is reduced to simply a
devm_idio_16_regmap_register()function and a configuration structure structidio_16_regmap_config.Legacy functions and code will be removed once all consumers have migrated to the new idio-16 library interface.
For IDIO-16 devices we have the following IRQ registers:
Base Address +1 (Write): Clear Interrupt
Base Address +2 (Read): Enable Interrupt
Base Address +2 (Write): Disable InterruptAn interrupt is asserted whenever a change-of-state is detected on any of the inputs. Any write to
0x2will disable interrupts, while any read will enable interrupts. Interrupts are cleared by a write to0x1.For 104-IDIO-16 devices, there is no IRQ status register, so software has to assume that if an interrupt is raised then it was for the 104-IDIO-16 device.
For PCI-IDIO-16 devices, there is an additional IRQ register:
Base Address +6 (Read): Interrupt Status
Interrupt status can be read from
0x6where bit 2 set indicates that an IRQ has been generated.
...
drivers/gpio/gpio-idio-16.c line 77
Quoted 9 lines
drivers/gpio/gpio-idio-16.c +8
-0+static int idio_16_reg_mask_xlate (struct gpio_regmap *const gpio,const unsigned int base, +const unsigned int offset,unsigned int *const reg, +unsigned int *const mask) +{ +unsigned int stride; +if (base !=IDIO_16_DAT_BASE ) +/* Should never reach this path */ +return -EINVAL ;
Then why do we have this test at all?
Quoted 13 lines
drivers/gpio/gpio-idio-16.c +13
-0+/* Input lines start at GPIO 16 */ +if (offset <16 ) { + stride = offset /IDIO_16_NGPIO_PER_REG ; + *reg =IDIO_16_OUT_BASE + stride *IDIO_16_REG_STRIDE ; + }else { + stride = (offset -16 ) /IDIO_16_NGPIO_PER_REG ; + *reg =IDIO_16_IN_BASE + stride *IDIO_16_REG_STRIDE ; + } + + *mask =BIT (offset %IDIO_16_NGPIO_PER_REG ); + +return 0 ; +}
...
drivers/gpio/gpio-idio-16.c line 96
Quoted 4 lines
drivers/gpio/gpio-idio-16.c +4
-0+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" ,
I believe this would look much better if formatted on the 8 basis per line.
Quoted 1 line
+};
...
Quoted 38 lines
drivers/gpio/gpio-idio-16.c +38
-0+int devm_idio_16_regmap_register (struct device *const dev, +const struct idio_16_regmap_config *const config) +{ +struct gpio_regmap_config gpio_config = {}; +int err; +struct idio_16_data *data; +struct regmap_irq_chip *chip; +struct regmap_irq_chip_data *chip_data; + +if (!config->parent) +return -EINVAL ; + +if (!config->map) +return -EINVAL ; + +if (!config->regmap_irqs) +return -EINVAL ; + + data =devm_kzalloc (dev,sizeof (*data),GFP_KERNEL ); +if (!data) +return -ENOMEM ; + data->map = config->map; + + chip =devm_kzalloc (dev,sizeof (*chip),GFP_KERNEL ); +if (!chip) +return -ENOMEM ; + + chip->name =dev_name (dev); + chip->status_base =IDIO_16_INTERRUPT_STATUS ; + chip->mask_base =IDIO_16_ENABLE_IRQ ; + chip->ack_base =IDIO_16_CLEAR_INTERRUPT ; + chip->no_status = config->no_status; + chip->num_regs =1 ; + chip->irqs = config->regmap_irqs; + chip->num_irqs = config->num_regmap_irqs; + chip->handle_mask_sync = idio_16_handle_mask_sync; + chip->irq_drv_data = data; +
drivers/gpio/gpio-idio-16.c line 152
Quoted 10 lines
drivers/gpio/gpio-idio-16.c +9
-0+/* Disable IRQ to prevent spurious interrupts before we're ready */ + err =regmap_write (data->map,IDIO_16_DISABLE_IRQ ,0x00 ); +if (err) +return err; + + err =devm_regmap_add_irq_chip (dev, data->map, config->irq,0 ,0 , chip, &chip_data); +if (err) { +dev_err (dev,"IRQ registration failed (%d)\n" , err); +return err;
return dev_err_probe(...);
devm_*() can't be called otherwise. If it's not the case the caller abuses devm_*() to begin with. So, it's the _part_ of the ->probe() sequence.
Quoted 22 lines
drivers/gpio/gpio-idio-16.c +22
-0+ } + +if (config->filters) { +/* Deactivate input filters */ + err =regmap_write (data->map,IDIO_16_DEACTIVATE_INPUT_FILTERS ,0x00 ); +if (err) +return err; + } + + gpio_config.parent = config->parent; + gpio_config.regmap = data->map; + gpio_config.ngpio =IDIO_16_NGPIO ; + gpio_config.names = idio_16_names; + gpio_config.reg_dat_base =GPIO_REGMAP_ADDR (IDIO_16_DAT_BASE ); + gpio_config.reg_set_base =GPIO_REGMAP_ADDR (IDIO_16_DAT_BASE ); + gpio_config.ngpio_per_reg =IDIO_16_NGPIO_PER_REG ; + gpio_config.reg_stride =IDIO_16_REG_STRIDE ; + gpio_config.irq_domain =regmap_irq_get_domain (chip_data); + gpio_config.reg_mask_xlate = idio_16_reg_mask_xlate; + +return PTR_ERR_OR_ZERO (devm_gpio_regmap_register (dev, &gpio_config)); +}
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v3 2/4 gpio: 104-idio-16: Migrate to the regmap API
In reply to William Breathitt GrayThe 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.
---
Changes in v3:
- Remove volatile regmap configuration structures; the existing read regmap table can be used to configure the volatile regmap table
- Line wrapping to 100 characters rather than 80
drivers/gpio/Kconfig | 2 +- drivers/gpio/gpio-104-idio-16.c | 286 +++++++------------------------- 2 files changed, 64 insertions(+), 224 deletions(-)
drivers/gpio/Kconfig +1 -1
@@ -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,
drivers/gpio/gpio-104-idio-16.c +63 -223
323 lines not on this page · raw message
Signature · 1 line
2.39.2
PATCH v3 3/4 gpio: pci-idio-16: Migrate to the regmap API
In reply to William Breathitt GrayThe 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.
---
Changes in v3:
- Remove volatile regmap configuration structures; the existing read regmap table can be used to configure the volatile regmap table
- Line wrapping to 100 characters rather than 80
drivers/gpio/Kconfig | 2 +- drivers/gpio/gpio-pci-idio-16.c | 294 +++++++------------------------- 2 files changed, 62 insertions(+), 234 deletions(-)
drivers/gpio/Kconfig +1 -1
@@ -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
drivers/gpio/gpio-pci-idio-16.c +61 -233
328 lines not on this page · raw message
Signature · 1 line
2.39.2
PATCH v3 4/4 gpio: idio-16: Remove unused legacy interface
In reply to William Breathitt GrayAll 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.
---
Changes in v3: none
drivers/gpio/gpio-idio-16.c | 131 +----------------------------------- drivers/gpio/gpio-idio-16.h | 65 ------------------ 2 files changed, 1 insertion(+), 195 deletions(-)
drivers/gpio/gpio-idio-16.c +1 -130
@@ -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"
@@ -174,133 +172,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 (®->in0_7) & (mask >>IDIO_16_NOUT )); - -if (offset <32 ) -return !!(ioread8 (®->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 (®->in0_7),16 ); -if (*mask &GENMASK (31 ,24 )) -bitmap_set_value8 (bits,ioread8 (®->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 ), ®->out0_7); -else -iowrite8 (bitmap_get_value8 (state->out_state,8 ), ®->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 ), ®->out0_7); -if (*mask &GENMASK (15 ,8 )) -iowrite8 (bitmap_get_value8 (state->out_state,8 ), ®->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" );
drivers/gpio/gpio-idio-16.h +0 -65
77 lines not on this page · raw message
Signature · 1 line
2.39.2