diff mbox series

[01/10] regmap-irq: Add get_irq_reg to support unusual register layouts

Message ID 20220603135714.12007-2-aidanmacdonald.0x0@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for AXP192 PMIC | expand

Commit Message

Aidan MacDonald June 3, 2022, 1:57 p.m. UTC
Add a new callback, get_irq_reg, for regmap IRQ chips, to support devices
with unusual register layouts. This is required in the rare cases where
the offset of an IRQ register is not constant with respect to the base
register. This is probably best illustrated with an example:

            mask    status
    IRQ0    0x40    0x44
    IRQ1    0x41    0x45
    IRQ2    0x42    0x46
    IRQ3    0x43    0x47
    IRQ4    0x4a    0x4d

If we set mask_base = 0x40 and status_base = 0x44, the offsets of each
register relative to the base are:

            mask    status
    IRQ0    0       0
    IRQ1    1       1
    IRQ2    2       2
    IRQ3    3       3
    IRQ4    10      9

The existing mapping mechanisms can't include IRQ4 in the same irqchip
as IRQ0-3 because the offset of IRQ4's register depends on which type
of register we're asking for, ie. which base register is used.

The get_irq_reg callback allows drivers to specify an arbitrary mapping
of (base register, register index) pairs to register addresses, instead
of the default linear mapping "base_register + register_index". This
allows unusual layouts, like the one above, to be handled using a single
regmap IRQ chip.

The drawback is that when get_irq_reg is used, it's impossible to use
bulk reads for status registers even if some of them are contiguous,
because the mapping is opaque to regmap-irq. This should be acceptable
for the case of a few infrequently-polled status registers.

Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
 drivers/base/regmap/regmap-irq.c | 19 +++++++++----------
 include/linux/regmap.h           |  5 +++++
 2 files changed, 14 insertions(+), 10 deletions(-)

Comments

Guru Das Srinagesh June 6, 2022, 5:43 p.m. UTC | #1
On Fri, Jun 03, 2022 at 02:57:05PM +0100, Aidan MacDonald wrote:
> Add a new callback, get_irq_reg, for regmap IRQ chips, to support devices
> with unusual register layouts. This is required in the rare cases where
> the offset of an IRQ register is not constant with respect to the base
> register. This is probably best illustrated with an example:
> 
>             mask    status
>     IRQ0    0x40    0x44
>     IRQ1    0x41    0x45
>     IRQ2    0x42    0x46
>     IRQ3    0x43    0x47
>     IRQ4    0x4a    0x4d
> 
> If we set mask_base = 0x40 and status_base = 0x44, the offsets of each
> register relative to the base are:
> 
>             mask    status
>     IRQ0    0       0
>     IRQ1    1       1
>     IRQ2    2       2
>     IRQ3    3       3
>     IRQ4    10      9
> 
> The existing mapping mechanisms can't include IRQ4 in the same irqchip
> as IRQ0-3 because the offset of IRQ4's register depends on which type
> of register we're asking for, ie. which base register is used.
> 
> The get_irq_reg callback allows drivers to specify an arbitrary mapping
> of (base register, register index) pairs to register addresses, instead
> of the default linear mapping "base_register + register_index". This
> allows unusual layouts, like the one above, to be handled using a single
> regmap IRQ chip.
> 
> The drawback is that when get_irq_reg is used, it's impossible to use
> bulk reads for status registers even if some of them are contiguous,
> because the mapping is opaque to regmap-irq. This should be acceptable
> for the case of a few infrequently-polled status registers.

This patch does two things:

1. Add a new callback `get_irq_reg`
2. Replace unmask_offset calculation with call to sub_irq_reg()

Could you please split the patch into two to better reflect this?

Thank you.

Guru Das.
Aidan MacDonald June 7, 2022, 10:46 a.m. UTC | #2
Guru Das Srinagesh <quic_gurus@quicinc.com> writes:

> On Fri, Jun 03, 2022 at 02:57:05PM +0100, Aidan MacDonald wrote:
>> Add a new callback, get_irq_reg, for regmap IRQ chips, to support devices
>> with unusual register layouts. This is required in the rare cases where
>> the offset of an IRQ register is not constant with respect to the base
>> register. This is probably best illustrated with an example:
>> 
>>             mask    status
>>     IRQ0    0x40    0x44
>>     IRQ1    0x41    0x45
>>     IRQ2    0x42    0x46
>>     IRQ3    0x43    0x47
>>     IRQ4    0x4a    0x4d
>> 
>> If we set mask_base = 0x40 and status_base = 0x44, the offsets of each
>> register relative to the base are:
>> 
>>             mask    status
>>     IRQ0    0       0
>>     IRQ1    1       1
>>     IRQ2    2       2
>>     IRQ3    3       3
>>     IRQ4    10      9
>> 
>> The existing mapping mechanisms can't include IRQ4 in the same irqchip
>> as IRQ0-3 because the offset of IRQ4's register depends on which type
>> of register we're asking for, ie. which base register is used.
>> 
>> The get_irq_reg callback allows drivers to specify an arbitrary mapping
>> of (base register, register index) pairs to register addresses, instead
>> of the default linear mapping "base_register + register_index". This
>> allows unusual layouts, like the one above, to be handled using a single
>> regmap IRQ chip.
>> 
>> The drawback is that when get_irq_reg is used, it's impossible to use
>> bulk reads for status registers even if some of them are contiguous,
>> because the mapping is opaque to regmap-irq. This should be acceptable
>> for the case of a few infrequently-polled status registers.
>
> This patch does two things:
>
> 1. Add a new callback `get_irq_reg`
> 2. Replace unmask_offset calculation with call to sub_irq_reg()
>
> Could you please split the patch into two to better reflect this?
>
> Thank you.
>
> Guru Das.

No problem, I'll do that in my v2.

Regards,
Aidan
diff mbox series

Patch

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 400c7412a7dc..e50437b18284 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -55,7 +55,9 @@  static int sub_irq_reg(struct regmap_irq_chip_data *data,
 	unsigned int offset;
 	int reg = 0;
 
-	if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
+	if (chip->get_irq_reg) {
+		reg = chip->get_irq_reg(base_reg, i);
+	} else if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
 		/* Assume linear mapping */
 		reg = base_reg + (i * map->reg_stride * data->irq_reg_stride);
 	} else {
@@ -97,7 +99,6 @@  static void regmap_irq_sync_unlock(struct irq_data *data)
 	struct regmap *map = d->map;
 	int i, j, ret;
 	u32 reg;
-	u32 unmask_offset;
 	u32 val;
 
 	if (d->chip->runtime_pm) {
@@ -141,11 +142,11 @@  static void regmap_irq_sync_unlock(struct irq_data *data)
 				dev_err(d->map->dev,
 					"Failed to sync unmasks in %x\n",
 					reg);
-			unmask_offset = d->chip->unmask_base -
-							d->chip->mask_base;
+
 			/* clear mask with unmask_base register */
+			reg = sub_irq_reg(d, d->chip->unmask_base, i);
 			ret = regmap_irq_update_bits(d,
-					reg + unmask_offset,
+					reg,
 					d->mask_buf_def[i],
 					d->mask_buf[i]);
 		} else {
@@ -480,7 +481,7 @@  static irqreturn_t regmap_irq_thread(int irq, void *d)
 
 		}
 	} else if (!map->use_single_read && map->reg_stride == 1 &&
-		   data->irq_reg_stride == 1) {
+		   data->irq_reg_stride == 1 && !chip->get_irq_reg) {
 
 		u8 *buf8 = data->status_reg_buf;
 		u16 *buf16 = data->status_reg_buf;
@@ -632,7 +633,6 @@  int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 	int ret = -ENOMEM;
 	int num_type_reg;
 	u32 reg;
-	u32 unmask_offset;
 
 	if (chip->num_regs <= 0)
 		return -EINVAL;
@@ -773,10 +773,9 @@  int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 			ret = regmap_irq_update_bits(d, reg,
 					 d->mask_buf[i], ~d->mask_buf[i]);
 		else if (d->chip->unmask_base) {
-			unmask_offset = d->chip->unmask_base -
-					d->chip->mask_base;
+			reg = sub_irq_reg(d, d->chip->unmask_base, i);
 			ret = regmap_irq_update_bits(d,
-					reg + unmask_offset,
+					reg,
 					d->mask_buf[i],
 					d->mask_buf[i]);
 		} else
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 8952fa3d0d59..4828021ab9e8 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1495,6 +1495,10 @@  struct regmap_irq_sub_irq_map {
  *		     after handling the interrupts in regmap_irq_handler().
  * @set_type_virt:   Driver specific callback to extend regmap_irq_set_type()
  *		     and configure virt regs.
+ * @get_irq_reg:     Callback to map a register index in range [0, num_regs[
+ *		     to a register, relative to a specific base register. This
+ *		     is mainly useful for devices where the register offsets
+ *		     change depending on the base register.
  * @irq_drv_data:    Driver specific IRQ data which is passed as parameter when
  *		     driver specific pre/post interrupt handler is called.
  *
@@ -1545,6 +1549,7 @@  struct regmap_irq_chip {
 	int (*handle_post_irq)(void *irq_drv_data);
 	int (*set_type_virt)(unsigned int **buf, unsigned int type,
 			     unsigned long hwirq, int reg);
+	int (*get_irq_reg)(unsigned int base_reg, int i);
 	void *irq_drv_data;
 };