* [PATCH v1 1/6] regmap-irq: Fix typo in documentation for .get_irq_reg()
2023-05-09 11:00 [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Aidan MacDonald
@ 2023-05-09 11:00 ` Aidan MacDonald
2023-05-09 11:00 ` [PATCH v1 2/6] regmap-irq: Remove virtual registers Aidan MacDonald
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Aidan MacDonald @ 2023-05-09 11:00 UTC (permalink / raw)
To: broonie; +Cc: gregkh, rafael, linux-kernel
It refers to a non-existent "num_type_settings" value, which is
an old name I'd used during development of config registers and
later dropped because it wasn't very clear.
The correct bound for the range is num_config_regs, which can
be verified by checking the implementation.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
include/linux/regmap.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index c2b9cc5db824..45fdb6e93617 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1606,7 +1606,7 @@ struct regmap_irq_chip_data;
* addresses. The base register will be one of @status_base,
* @mask_base, etc., @main_status, or any of @config_base.
* The index will be in the range [0, num_main_regs[ for the
- * main status base, [0, num_type_settings[ for any config
+ * main status base, [0, num_config_regs[ for any config
* register base, and [0, num_regs[ for any other base.
* If unspecified then regmap_irq_get_irq_reg_linear() is used.
* @irq_drv_data: Driver specific IRQ data which is passed as parameter when
--
2.39.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v1 2/6] regmap-irq: Remove virtual registers
2023-05-09 11:00 [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Aidan MacDonald
2023-05-09 11:00 ` [PATCH v1 1/6] regmap-irq: Fix typo in documentation for .get_irq_reg() Aidan MacDonald
@ 2023-05-09 11:00 ` Aidan MacDonald
2023-05-09 11:00 ` [PATCH v1 3/6] regmap-irq: Remove type registers Aidan MacDonald
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Aidan MacDonald @ 2023-05-09 11:00 UTC (permalink / raw)
To: broonie; +Cc: gregkh, rafael, linux-kernel
No remaining users, and it's been replaced by config registers.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/base/regmap/regmap-irq.c | 48 --------------------------------
include/linux/regmap.h | 12 --------
2 files changed, 60 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index b99bb2369fff..615b1ced2a5e 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -41,7 +41,6 @@ struct regmap_irq_chip_data {
unsigned int *wake_buf;
unsigned int *type_buf;
unsigned int *type_buf_def;
- unsigned int **virt_buf;
unsigned int **config_buf;
unsigned int irq_reg_stride;
@@ -197,20 +196,6 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
}
}
- if (d->chip->num_virt_regs) {
- for (i = 0; i < d->chip->num_virt_regs; i++) {
- for (j = 0; j < d->chip->num_regs; j++) {
- reg = d->get_irq_reg(d, d->chip->virt_reg_base[i],
- j);
- ret = regmap_write(map, reg, d->virt_buf[i][j]);
- if (ret != 0)
- dev_err(d->map->dev,
- "Failed to write virt 0x%x: %d\n",
- reg, ret);
- }
- }
- }
-
for (i = 0; i < d->chip->num_config_bases; i++) {
for (j = 0; j < d->chip->num_config_regs; j++) {
reg = d->get_irq_reg(d, d->chip->config_base[i], j);
@@ -321,13 +306,6 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type)
return -EINVAL;
}
- if (d->chip->set_type_virt) {
- ret = d->chip->set_type_virt(d->virt_buf, type, data->hwirq,
- reg);
- if (ret)
- return ret;
- }
-
if (d->chip->set_type_config) {
ret = d->chip->set_type_config(d->config_buf, type, irq_data,
reg, d->chip->irq_drv_data);
@@ -759,9 +737,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
if (chip->num_type_reg)
dev_warn(map->dev, "type registers are deprecated; use config registers instead");
- if (chip->num_virt_regs || chip->virt_reg_base || chip->set_type_virt)
- dev_warn(map->dev, "virtual registers are deprecated; use config registers instead");
-
if (irq_base) {
irq_base = irq_alloc_descs(irq_base, 0, chip->num_irqs, 0);
if (irq_base < 0) {
@@ -825,24 +800,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
goto err_alloc;
}
- if (chip->num_virt_regs) {
- /*
- * Create virt_buf[chip->num_extra_config_regs][chip->num_regs]
- */
- d->virt_buf = kcalloc(chip->num_virt_regs, sizeof(*d->virt_buf),
- GFP_KERNEL);
- if (!d->virt_buf)
- goto err_alloc;
-
- for (i = 0; i < chip->num_virt_regs; i++) {
- d->virt_buf[i] = kcalloc(chip->num_regs,
- sizeof(**d->virt_buf),
- GFP_KERNEL);
- if (!d->virt_buf[i])
- goto err_alloc;
- }
- }
-
if (chip->num_config_bases && chip->num_config_regs) {
/*
* Create config_buf[num_config_bases][num_config_regs]
@@ -1064,11 +1021,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
kfree(d->mask_buf);
kfree(d->status_buf);
kfree(d->status_reg_buf);
- if (d->virt_buf) {
- for (i = 0; i < chip->num_virt_regs; i++)
- kfree(d->virt_buf[i]);
- kfree(d->virt_buf);
- }
if (d->config_buf) {
for (i = 0; i < chip->num_config_bases; i++)
kfree(d->config_buf[i]);
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 45fdb6e93617..ec3c241542c9 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1544,8 +1544,6 @@ struct regmap_irq_chip_data;
* @wake_base: Base address for wake enables. If zero unsupported.
* @type_base: Base address for irq type. If zero unsupported. Deprecated,
* use @config_base instead.
- * @virt_reg_base: Base addresses for extra config regs. Deprecated, use
- * @config_base instead.
* @config_base: Base address for IRQ type config regs. If null unsupported.
* @irq_reg_stride: Stride to use for chips where registers are not contiguous.
* @init_ack_masked: Ack all masked interrupts once during initalization.
@@ -1586,9 +1584,6 @@ struct regmap_irq_chip_data;
*
* @num_type_reg: Number of type registers. Deprecated, use config registers
* instead.
- * @num_virt_regs: Number of non-standard irq configuration registers.
- * If zero unsupported. Deprecated, use config registers
- * instead.
* @num_config_bases: Number of config base registers.
* @num_config_regs: Number of config registers for each config base register.
*
@@ -1598,9 +1593,6 @@ struct regmap_irq_chip_data;
* after handling the interrupts in regmap_irq_handler().
* @handle_mask_sync: Callback used to handle IRQ mask syncs. The index will be
* in the range [0, num_regs)
- * @set_type_virt: Driver specific callback to extend regmap_irq_set_type()
- * and configure virt regs. Deprecated, use @set_type_config
- * callback and config registers instead.
* @set_type_config: Callback used for configuring irq types.
* @get_irq_reg: Callback for mapping (base register, index) pairs to register
* addresses. The base register will be one of @status_base,
@@ -1630,7 +1622,6 @@ struct regmap_irq_chip {
unsigned int ack_base;
unsigned int wake_base;
unsigned int type_base;
- unsigned int *virt_reg_base;
const unsigned int *config_base;
unsigned int irq_reg_stride;
unsigned int init_ack_masked:1;
@@ -1652,7 +1643,6 @@ struct regmap_irq_chip {
int num_irqs;
int num_type_reg;
- int num_virt_regs;
int num_config_bases;
int num_config_regs;
@@ -1661,8 +1651,6 @@ struct regmap_irq_chip {
int (*handle_mask_sync)(struct regmap *map, int index,
unsigned int mask_buf_def,
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,
void *irq_drv_data);
--
2.39.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v1 3/6] regmap-irq: Remove type registers
2023-05-09 11:00 [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Aidan MacDonald
2023-05-09 11:00 ` [PATCH v1 1/6] regmap-irq: Fix typo in documentation for .get_irq_reg() Aidan MacDonald
2023-05-09 11:00 ` [PATCH v1 2/6] regmap-irq: Remove virtual registers Aidan MacDonald
@ 2023-05-09 11:00 ` Aidan MacDonald
2023-05-11 1:24 ` Mark Brown
2023-05-09 11:00 ` [PATCH v1 4/6] regmap-irq: Remove support for not_fixed_stride Aidan MacDonald
` (3 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Aidan MacDonald @ 2023-05-09 11:00 UTC (permalink / raw)
To: broonie; +Cc: gregkh, rafael, linux-kernel
No remaining users, these have been replaced by config registers.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/base/regmap/regmap-irq.c | 82 ++++----------------------------
include/linux/regmap.h | 7 ---
2 files changed, 8 insertions(+), 81 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 615b1ced2a5e..b27db5d0b8a7 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -182,20 +182,6 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
}
}
- /* Don't update the type bits if we're using mask bits for irq type. */
- if (!d->chip->type_in_mask) {
- for (i = 0; i < d->chip->num_type_reg; i++) {
- if (!d->type_buf_def[i])
- continue;
- reg = d->get_irq_reg(d, d->chip->type_base, i);
- ret = regmap_update_bits(d->map, reg,
- d->type_buf_def[i], d->type_buf[i]);
- if (ret != 0)
- dev_err(d->map->dev, "Failed to sync type in %x\n",
- reg);
- }
- }
-
for (i = 0; i < d->chip->num_config_bases; i++) {
for (j = 0; j < d->chip->num_config_regs; j++) {
reg = d->get_irq_reg(d, d->chip->config_base[i], j);
@@ -274,36 +260,11 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type)
reg = t->type_reg_offset / map->reg_stride;
- if (t->type_reg_mask)
- d->type_buf[reg] &= ~t->type_reg_mask;
- else
- d->type_buf[reg] &= ~(t->type_falling_val |
- t->type_rising_val |
- t->type_level_low_val |
- t->type_level_high_val);
- switch (type) {
- case IRQ_TYPE_EDGE_FALLING:
- d->type_buf[reg] |= t->type_falling_val;
- break;
-
- case IRQ_TYPE_EDGE_RISING:
- d->type_buf[reg] |= t->type_rising_val;
- break;
-
- case IRQ_TYPE_EDGE_BOTH:
- d->type_buf[reg] |= (t->type_falling_val |
- t->type_rising_val);
- break;
-
- case IRQ_TYPE_LEVEL_HIGH:
- d->type_buf[reg] |= t->type_level_high_val;
- break;
-
- case IRQ_TYPE_LEVEL_LOW:
- d->type_buf[reg] |= t->type_level_low_val;
- break;
- default:
- return -EINVAL;
+ if (d->chip->type_in_mask) {
+ ret = regmap_irq_set_type_config_simple(&d->type_buf, type,
+ irq_data, reg, d->chip->irq_drv_data);
+ if (ret)
+ return ret;
}
if (d->chip->set_type_config) {
@@ -708,8 +669,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
struct regmap_irq_chip_data *d;
int i;
int ret = -ENOMEM;
- int num_type_reg;
- int num_regs;
u32 reg;
if (chip->num_regs <= 0)
@@ -734,9 +693,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
return -EINVAL;
}
- if (chip->num_type_reg)
- dev_warn(map->dev, "type registers are deprecated; use config registers instead");
-
if (irq_base) {
irq_base = irq_alloc_descs(irq_base, 0, chip->num_irqs, 0);
if (irq_base < 0) {
@@ -781,21 +737,13 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
goto err_alloc;
}
- /*
- * Use num_config_regs if defined, otherwise fall back to num_type_reg
- * to maintain backward compatibility.
- */
- num_type_reg = chip->num_config_regs ? chip->num_config_regs
- : chip->num_type_reg;
- num_regs = chip->type_in_mask ? chip->num_regs : num_type_reg;
- if (num_regs) {
- d->type_buf_def = kcalloc(num_regs,
+ if (chip->type_in_mask) {
+ d->type_buf_def = kcalloc(chip->num_regs,
sizeof(*d->type_buf_def), GFP_KERNEL);
if (!d->type_buf_def)
goto err_alloc;
- d->type_buf = kcalloc(num_regs, sizeof(*d->type_buf),
- GFP_KERNEL);
+ d->type_buf = kcalloc(chip->num_regs, sizeof(*d->type_buf), GFP_KERNEL);
if (!d->type_buf)
goto err_alloc;
}
@@ -971,20 +919,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
}
}
- if (chip->num_type_reg && !chip->type_in_mask) {
- for (i = 0; i < chip->num_type_reg; ++i) {
- reg = d->get_irq_reg(d, d->chip->type_base, i);
-
- ret = regmap_read(map, reg, &d->type_buf_def[i]);
-
- if (ret) {
- dev_err(map->dev, "Failed to get type defaults at 0x%x: %d\n",
- reg, ret);
- goto err_alloc;
- }
- }
- }
-
if (irq_base)
d->domain = irq_domain_create_legacy(fwnode, chip->num_irqs,
irq_base, 0,
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index ec3c241542c9..af93b369a19f 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1542,8 +1542,6 @@ struct regmap_irq_chip_data;
* @ack_base: Base ack address. If zero then the chip is clear on read.
* Using zero value is possible with @use_ack bit.
* @wake_base: Base address for wake enables. If zero unsupported.
- * @type_base: Base address for irq type. If zero unsupported. Deprecated,
- * use @config_base instead.
* @config_base: Base address for IRQ type config regs. If null unsupported.
* @irq_reg_stride: Stride to use for chips where registers are not contiguous.
* @init_ack_masked: Ack all masked interrupts once during initalization.
@@ -1581,9 +1579,6 @@ struct regmap_irq_chip_data;
* @irqs: Descriptors for individual IRQs. Interrupt numbers are
* assigned based on the index in the array of the interrupt.
* @num_irqs: Number of descriptors.
- *
- * @num_type_reg: Number of type registers. Deprecated, use config registers
- * instead.
* @num_config_bases: Number of config base registers.
* @num_config_regs: Number of config registers for each config base register.
*
@@ -1621,7 +1616,6 @@ struct regmap_irq_chip {
unsigned int unmask_base;
unsigned int ack_base;
unsigned int wake_base;
- unsigned int type_base;
const unsigned int *config_base;
unsigned int irq_reg_stride;
unsigned int init_ack_masked:1;
@@ -1642,7 +1636,6 @@ struct regmap_irq_chip {
const struct regmap_irq *irqs;
int num_irqs;
- int num_type_reg;
int num_config_bases;
int num_config_regs;
--
2.39.2
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1 3/6] regmap-irq: Remove type registers
2023-05-09 11:00 ` [PATCH v1 3/6] regmap-irq: Remove type registers Aidan MacDonald
@ 2023-05-11 1:24 ` Mark Brown
2023-05-11 9:09 ` Aidan MacDonald
0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2023-05-11 1:24 UTC (permalink / raw)
To: Aidan MacDonald; +Cc: gregkh, rafael, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 200 bytes --]
On Tue, May 09, 2023 at 12:00:57PM +0100, Aidan MacDonald wrote:
> No remaining users, these have been replaced by config registers.
This doesn't apply against current code, please check and resend.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 3/6] regmap-irq: Remove type registers
2023-05-11 1:24 ` Mark Brown
@ 2023-05-11 9:09 ` Aidan MacDonald
0 siblings, 0 replies; 10+ messages in thread
From: Aidan MacDonald @ 2023-05-11 9:09 UTC (permalink / raw)
To: Mark Brown; +Cc: gregkh, rafael, linux-kernel
Mark Brown <broonie@kernel.org> writes:
> On Tue, May 09, 2023 at 12:00:57PM +0100, Aidan MacDonald wrote:
>> No remaining users, these have been replaced by config registers.
>
> This doesn't apply against current code, please check and resend.
Apologies, I'll rebase on top of your regmap tree.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 4/6] regmap-irq: Remove support for not_fixed_stride
2023-05-09 11:00 [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Aidan MacDonald
` (2 preceding siblings ...)
2023-05-09 11:00 ` [PATCH v1 3/6] regmap-irq: Remove type registers Aidan MacDonald
@ 2023-05-09 11:00 ` Aidan MacDonald
2023-05-09 11:00 ` [PATCH v1 5/6] regmap-irq: Minor adjustments to .handle_mask_sync() Aidan MacDonald
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Aidan MacDonald @ 2023-05-09 11:00 UTC (permalink / raw)
To: broonie; +Cc: gregkh, rafael, linux-kernel
No remaining users, use a custom .get_irq_reg() callback instead.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/base/regmap/regmap-irq.c | 43 +++-----------------------------
include/linux/regmap.h | 9 -------
2 files changed, 3 insertions(+), 49 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index b27db5d0b8a7..41568927aa4c 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -329,15 +329,8 @@ static inline int read_sub_irq_data(struct regmap_irq_chip_data *data,
unsigned int offset = subreg->offset[i];
unsigned int index = offset / map->reg_stride;
- if (chip->not_fixed_stride)
- ret = regmap_read(map,
- chip->status_base + offset,
- &data->status_buf[b]);
- else
- ret = regmap_read(map,
- chip->status_base + offset,
- &data->status_buf[index]);
-
+ ret = regmap_read(map, chip->status_base + offset,
+ &data->status_buf[index]);
if (ret)
break;
}
@@ -392,17 +385,7 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
* sake of simplicity. and add bulk reads only if needed
*/
for (i = 0; i < chip->num_main_regs; i++) {
- /*
- * For not_fixed_stride, don't use ->get_irq_reg().
- * It would produce an incorrect result.
- */
- if (data->chip->not_fixed_stride)
- reg = chip->main_status +
- i * map->reg_stride * data->irq_reg_stride;
- else
- reg = data->get_irq_reg(data,
- chip->main_status, i);
-
+ reg = data->get_irq_reg(data, chip->main_status, i);
ret = regmap_read(map, reg, &data->main_status_buf[i]);
if (ret) {
dev_err(map->dev,
@@ -568,20 +551,8 @@ static const struct irq_domain_ops regmap_domain_ops = {
unsigned int regmap_irq_get_irq_reg_linear(struct regmap_irq_chip_data *data,
unsigned int base, int index)
{
- const struct regmap_irq_chip *chip = data->chip;
struct regmap *map = data->map;
- /*
- * FIXME: This is for backward compatibility and should be removed
- * when not_fixed_stride is dropped (it's only used by qcom-pm8008).
- */
- if (chip->not_fixed_stride && chip->sub_reg_offsets) {
- struct regmap_irq_sub_irq_map *subreg;
-
- subreg = &chip->sub_reg_offsets[0];
- return base + subreg->offset[0];
- }
-
return base + index * map->reg_stride * data->irq_reg_stride;
}
EXPORT_SYMBOL_GPL(regmap_irq_get_irq_reg_linear);
@@ -685,14 +656,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
return -EINVAL;
}
- if (chip->not_fixed_stride) {
- dev_warn(map->dev, "not_fixed_stride is deprecated; use ->get_irq_reg() instead");
-
- for (i = 0; i < chip->num_regs; i++)
- if (chip->sub_reg_offsets[i].num_regs != 1)
- return -EINVAL;
- }
-
if (irq_base) {
irq_base = irq_alloc_descs(irq_base, 0, chip->num_irqs, 0);
if (irq_base < 0) {
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index af93b369a19f..cd98df7abb6f 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1528,9 +1528,6 @@ struct regmap_irq_chip_data;
* status_base. Should contain num_regs arrays.
* Can be provided for chips with more complex mapping than
* 1.st bit to 1.st sub-reg, 2.nd bit to 2.nd sub-reg, ...
- * When used with not_fixed_stride, each one-element array
- * member contains offset calculated as address from each
- * peripheral to first peripheral.
* @num_main_regs: Number of 'main status' irq registers for chips which have
* main_status set.
*
@@ -1567,11 +1564,6 @@ struct regmap_irq_chip_data;
* registers before unmasking interrupts to clear any bits
* set when they were masked.
* @runtime_pm: Hold a runtime PM lock on the device when accessing it.
- * @not_fixed_stride: Used when chip peripherals are not laid out with fixed
- * stride. Must be used with sub_reg_offsets containing the
- * offsets to each peripheral. Deprecated; the same thing
- * can be accomplished with a @get_irq_reg callback, without
- * the need for a @sub_reg_offsets table.
* @no_status: No status register: all interrupts assumed generated by device.
*
* @num_regs: Number of registers in each control bank.
@@ -1628,7 +1620,6 @@ struct regmap_irq_chip {
unsigned int type_in_mask:1;
unsigned int clear_on_unmask:1;
unsigned int runtime_pm:1;
- unsigned int not_fixed_stride:1;
unsigned int no_status:1;
int num_regs;
--
2.39.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v1 5/6] regmap-irq: Minor adjustments to .handle_mask_sync()
2023-05-09 11:00 [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Aidan MacDonald
` (3 preceding siblings ...)
2023-05-09 11:00 ` [PATCH v1 4/6] regmap-irq: Remove support for not_fixed_stride Aidan MacDonald
@ 2023-05-09 11:00 ` Aidan MacDonald
2023-05-09 11:01 ` [PATCH v1 6/6] regmap-irq: Drop backward compatibility for inverted mask/unmask Aidan MacDonald
2023-05-11 5:43 ` [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Mark Brown
6 siblings, 0 replies; 10+ messages in thread
From: Aidan MacDonald @ 2023-05-09 11:00 UTC (permalink / raw)
To: broonie; +Cc: gregkh, rafael, linux-kernel
If a .handle_mask_sync() callback is provided it supersedes all
inbuilt handling of mask registers, and judging by the commit
69af4bcaa08d ("regmap-irq: Add handle_mask_sync() callback") it
is intended to completely replace all default IRQ masking logic.
The implementation has two minor inconsistencies, which can be
fixed without breaking compatibility:
(1) mask_base must be set to enable .handle_mask_sync(), even
though mask_base is otherwise unused. This is easily fixed
because mask_base is already optional.
(2) Unmask registers aren't accounted for -- they are part of
the default IRQ masking logic and are just a bit-inverted
version of mask registers. It would be a bad idea to allow
them to be used at the same time as .handle_mask_sync(),
as the result would be confusing and unmaintainable, so
make sure this can't happen.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/base/regmap/regmap-irq.c | 68 ++++++++++++++++----------------
1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 41568927aa4c..898d1c6c5b75 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -113,24 +113,22 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
* suppress pointless writes.
*/
for (i = 0; i < d->chip->num_regs; i++) {
- if (d->mask_base) {
- if (d->chip->handle_mask_sync)
- d->chip->handle_mask_sync(d->map, i,
- d->mask_buf_def[i],
- d->mask_buf[i],
- d->chip->irq_drv_data);
- else {
- reg = d->get_irq_reg(d, d->mask_base, i);
- ret = regmap_update_bits(d->map, reg,
- d->mask_buf_def[i],
- d->mask_buf[i]);
- if (ret)
- dev_err(d->map->dev, "Failed to sync masks in %x\n",
- reg);
- }
+ if (d->chip->handle_mask_sync)
+ d->chip->handle_mask_sync(d->map, i,
+ d->mask_buf_def[i],
+ d->mask_buf[i],
+ d->chip->irq_drv_data);
+
+ if (d->mask_base && !d->chip->handle_mask_sync) {
+ reg = d->get_irq_reg(d, d->mask_base, i);
+ ret = regmap_update_bits(d->map, reg,
+ d->mask_buf_def[i],
+ d->mask_buf[i]);
+ if (ret)
+ dev_err(d->map->dev, "Failed to sync masks in %x\n", reg);
}
- if (d->unmask_base) {
+ if (d->unmask_base && !d->chip->handle_mask_sync) {
reg = d->get_irq_reg(d, d->unmask_base, i);
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i], ~d->mask_buf[i]);
@@ -786,28 +784,28 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
for (i = 0; i < chip->num_regs; i++) {
d->mask_buf[i] = d->mask_buf_def[i];
- if (d->mask_base) {
- if (chip->handle_mask_sync) {
- ret = chip->handle_mask_sync(d->map, i,
- d->mask_buf_def[i],
- d->mask_buf[i],
- chip->irq_drv_data);
- if (ret)
- goto err_alloc;
- } else {
- reg = d->get_irq_reg(d, d->mask_base, i);
- ret = regmap_update_bits(d->map, reg,
- d->mask_buf_def[i],
- d->mask_buf[i]);
- if (ret) {
- dev_err(map->dev, "Failed to set masks in 0x%x: %d\n",
- reg, ret);
- goto err_alloc;
- }
+ if (chip->handle_mask_sync) {
+ ret = chip->handle_mask_sync(d->map, i,
+ d->mask_buf_def[i],
+ d->mask_buf[i],
+ chip->irq_drv_data);
+ if (ret)
+ goto err_alloc;
+ }
+
+ if (d->mask_base && !chip->handle_mask_sync) {
+ reg = d->get_irq_reg(d, d->mask_base, i);
+ ret = regmap_update_bits(d->map, reg,
+ d->mask_buf_def[i],
+ d->mask_buf[i]);
+ if (ret) {
+ dev_err(map->dev, "Failed to set masks in 0x%x: %d\n",
+ reg, ret);
+ goto err_alloc;
}
}
- if (d->unmask_base) {
+ if (d->unmask_base && !chip->handle_mask_sync) {
reg = d->get_irq_reg(d, d->unmask_base, i);
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i], ~d->mask_buf[i]);
--
2.39.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v1 6/6] regmap-irq: Drop backward compatibility for inverted mask/unmask
2023-05-09 11:00 [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Aidan MacDonald
` (4 preceding siblings ...)
2023-05-09 11:00 ` [PATCH v1 5/6] regmap-irq: Minor adjustments to .handle_mask_sync() Aidan MacDonald
@ 2023-05-09 11:01 ` Aidan MacDonald
2023-05-11 5:43 ` [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Mark Brown
6 siblings, 0 replies; 10+ messages in thread
From: Aidan MacDonald @ 2023-05-09 11:01 UTC (permalink / raw)
To: broonie; +Cc: gregkh, rafael, linux-kernel
All users must now specify .mask_unmask_non_inverted = true to
ensure they are using the expected semantics: 1s disable IRQs
in the mask registers, and enable IRQs in the unmask registers.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
drivers/base/regmap/regmap-irq.c | 44 ++++++++------------------------
1 file changed, 11 insertions(+), 33 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 898d1c6c5b75..87dfcec9da5a 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -30,9 +30,6 @@ struct regmap_irq_chip_data {
int irq;
int wake_count;
- unsigned int mask_base;
- unsigned int unmask_base;
-
void *status_reg_buf;
unsigned int *main_status_buf;
unsigned int *status_buf;
@@ -119,8 +116,8 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
d->mask_buf[i],
d->chip->irq_drv_data);
- if (d->mask_base && !d->chip->handle_mask_sync) {
- reg = d->get_irq_reg(d, d->mask_base, i);
+ if (d->chip->mask_base && !d->chip->handle_mask_sync) {
+ reg = d->get_irq_reg(d, d->chip->mask_base, i);
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i],
d->mask_buf[i]);
@@ -128,8 +125,8 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
dev_err(d->map->dev, "Failed to sync masks in %x\n", reg);
}
- if (d->unmask_base && !d->chip->handle_mask_sync) {
- reg = d->get_irq_reg(d, d->unmask_base, i);
+ if (d->chip->unmask_base && !d->chip->handle_mask_sync) {
+ reg = d->get_irq_reg(d, d->chip->unmask_base, i);
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i], ~d->mask_buf[i]);
if (ret)
@@ -646,6 +643,9 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
if (chip->clear_on_unmask && (chip->ack_base || chip->use_ack))
return -EINVAL;
+ if (chip->mask_base && chip->unmask_base && !chip->mask_unmask_non_inverted)
+ return -EINVAL;
+
for (i = 0; i < chip->num_irqs; i++) {
if (chip->irqs[i].reg_offset % map->reg_stride)
return -EINVAL;
@@ -734,28 +734,6 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
d->chip = chip;
d->irq_base = irq_base;
- if (chip->mask_base && chip->unmask_base &&
- !chip->mask_unmask_non_inverted) {
- /*
- * Chips that specify both mask_base and unmask_base used to
- * get inverted mask behavior by default, with no way to ask
- * for the normal, non-inverted behavior. This "inverted by
- * default" behavior is deprecated, but we have to support it
- * until existing drivers have been fixed.
- *
- * Existing drivers should be updated by swapping mask_base
- * and unmask_base and setting mask_unmask_non_inverted=true.
- * New drivers should always set the flag.
- */
- dev_warn(map->dev, "mask_base and unmask_base are inverted, please fix it");
-
- d->mask_base = chip->unmask_base;
- d->unmask_base = chip->mask_base;
- } else {
- d->mask_base = chip->mask_base;
- d->unmask_base = chip->unmask_base;
- }
-
if (chip->irq_reg_stride)
d->irq_reg_stride = chip->irq_reg_stride;
else
@@ -793,8 +771,8 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
goto err_alloc;
}
- if (d->mask_base && !chip->handle_mask_sync) {
- reg = d->get_irq_reg(d, d->mask_base, i);
+ if (chip->mask_base && !chip->handle_mask_sync) {
+ reg = d->get_irq_reg(d, chip->mask_base, i);
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i],
d->mask_buf[i]);
@@ -805,8 +783,8 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
}
}
- if (d->unmask_base && !chip->handle_mask_sync) {
- reg = d->get_irq_reg(d, d->unmask_base, i);
+ if (chip->unmask_base && !chip->handle_mask_sync) {
+ reg = d->get_irq_reg(d, chip->unmask_base, i);
ret = regmap_update_bits(d->map, reg,
d->mask_buf_def[i], ~d->mask_buf[i]);
if (ret) {
--
2.39.2
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality
2023-05-09 11:00 [PATCH v1 0/6] regmap-irq: Cleanups and remove unused functionality Aidan MacDonald
` (5 preceding siblings ...)
2023-05-09 11:01 ` [PATCH v1 6/6] regmap-irq: Drop backward compatibility for inverted mask/unmask Aidan MacDonald
@ 2023-05-11 5:43 ` Mark Brown
6 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2023-05-11 5:43 UTC (permalink / raw)
To: Aidan MacDonald; +Cc: gregkh, rafael, linux-kernel
On Tue, 09 May 2023 12:00:54 +0100, Aidan MacDonald wrote:
> This is a straightforward patch series, mostly just removing a bunch
> of old features that were only used by a handful of drivers.
>
> - 1/6 is a documentation fix
> - 2/6, 3/6, and 4/6 remove unused, deprecated functionality
> - 5/6 makes the behavior of .handle_mask_sync() a bit more consistent
> w.r.t. mask and unmask registers, to aid maintainability.
> - 6/6 removes now-unused "inverted mask/unmask" compatibility code.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
Thanks!
[1/6] regmap-irq: Fix typo in documentation for .get_irq_reg()
commit: 212bc1ce618dd7c734920a68988fe4b473f09f40
[2/6] regmap-irq: Remove virtual registers
commit: f33a751d5a7fe03b11d95e6a33e1fdd3b4f8ec18
[3/6] regmap-irq: Remove type registers
(no commit info)
[4/6] regmap-irq: Remove support for not_fixed_stride
(no commit info)
[5/6] regmap-irq: Minor adjustments to .handle_mask_sync()
(no commit info)
[6/6] regmap-irq: Drop backward compatibility for inverted mask/unmask
(no commit info)
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 10+ messages in thread