From 1066cfbdfa3f5c401870fad577fe63d1171a5bcd Mon Sep 17 00:00:00 2001 From: Guru Das Srinagesh Date: Wed, 10 Mar 2021 16:39:52 -0800 Subject: [PATCH 1/4] regmap-irq: Extend sub-irq to support non-fixed reg strides Qualcomm's MFD chips have a top level interrupt status register and sub-irqs (peripherals). When a bit in the main status register goes high, it means that the peripheral corresponding to that bit has an unserviced interrupt. If the bit is not set, this means that the corresponding peripheral does not. Commit a2d21848d9211d ("regmap: regmap-irq: Add main status register support") introduced the sub-irq logic that is currently applied only when reading status registers, but not for any other functions like acking or masking. Extend the use of sub-irq to all other functions, with two caveats regarding the specification of offsets: - Each member of the sub_reg_offsets array should be of length 1 - The specified offsets should be the unequal strides for each sub-irq device. In QCOM's case, all the *_base registers are to be configured to the base addresses of the first sub-irq group, with offsets of each subsequent group calculated as a difference from these addresses. Continuing from the example mentioned in the cover letter: /* * Address of MISC_INT_MASK = 0x1011 * Address of TEMP_ALARM_INT_MASK = 0x2011 * Address of GPIO01_INT_MASK = 0x3011 * * Calculate offsets as: * offset_0 = 0x1011 - 0x1011 = 0 (to access MISC's * registers) * offset_1 = 0x2011 - 0x1011 = 0x1000 * offset_2 = 0x3011 - 0x1011 = 0x2000 */ static unsigned int sub_unit0_offsets[] = {0}; static unsigned int sub_unit1_offsets[] = {0x1000}; static unsigned int sub_unit2_offsets[] = {0x2000}; static struct regmap_irq_sub_irq_map chip_sub_irq_offsets[] = { REGMAP_IRQ_MAIN_REG_OFFSET(sub_unit0_offsets), REGMAP_IRQ_MAIN_REG_OFFSET(sub_unit0_offsets), REGMAP_IRQ_MAIN_REG_OFFSET(sub_unit0_offsets), }; static struct regmap_irq_chip chip_irq_chip = { --------8<-------- .not_fixed_stride = true, .mask_base = MISC_INT_MASK, .type_base = MISC_INT_TYPE, .ack_base = MISC_INT_ACK, .sub_reg_offsets = chip_sub_irq_offsets, --------8<-------- }; Signed-off-by: Guru Das Srinagesh Link: https://lore.kernel.org/r/526562423eaa58b4075362083f561841f1d6956c.1615423027.git.gurus@codeaurora.org Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-irq.c | 81 +++++++++++++++++++++----------- include/linux/regmap.h | 7 +++ 2 files changed, 60 insertions(+), 28 deletions(-) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index 19db764ffa4a..e1d8fc9ef040 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -45,6 +45,27 @@ struct regmap_irq_chip_data { bool clear_status:1; }; +static int sub_irq_reg(struct regmap_irq_chip_data *data, + unsigned int base_reg, int i) +{ + const struct regmap_irq_chip *chip = data->chip; + struct regmap *map = data->map; + struct regmap_irq_sub_irq_map *subreg; + unsigned int offset; + int reg = 0; + + if (!chip->sub_reg_offsets || !chip->not_fixed_stride) { + /* Assume linear mapping */ + reg = base_reg + (i * map->reg_stride * data->irq_reg_stride); + } else { + subreg = &chip->sub_reg_offsets[i]; + offset = subreg->offset[0]; + reg = base_reg + offset; + } + + return reg; +} + static inline const struct regmap_irq *irq_to_regmap_irq(struct regmap_irq_chip_data *data, int irq) @@ -87,8 +108,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data) if (d->clear_status) { for (i = 0; i < d->chip->num_regs; i++) { - reg = d->chip->status_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->status_base, i); ret = regmap_read(map, reg, &val); if (ret) @@ -108,8 +128,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data) if (!d->chip->mask_base) continue; - reg = d->chip->mask_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->mask_base, i); if (d->chip->mask_invert) { ret = regmap_irq_update_bits(d, reg, d->mask_buf_def[i], ~d->mask_buf[i]); @@ -136,8 +155,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data) dev_err(d->map->dev, "Failed to sync masks in %x\n", reg); - reg = d->chip->wake_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->wake_base, i); if (d->wake_buf) { if (d->chip->wake_invert) ret = regmap_irq_update_bits(d, reg, @@ -161,8 +179,8 @@ static void regmap_irq_sync_unlock(struct irq_data *data) * it'll be ignored in irq handler, then may introduce irq storm */ if (d->mask_buf[i] && (d->chip->ack_base || d->chip->use_ack)) { - reg = d->chip->ack_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->ack_base, i); + /* some chips ack by write 0 */ if (d->chip->ack_invert) ret = regmap_write(map, reg, ~d->mask_buf[i]); @@ -187,8 +205,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data) for (i = 0; i < d->chip->num_type_reg; i++) { if (!d->type_buf_def[i]) continue; - reg = d->chip->type_base + - (i * map->reg_stride * d->type_reg_stride); + reg = sub_irq_reg(d, d->chip->type_base, i); if (d->chip->type_invert) ret = regmap_irq_update_bits(d, reg, d->type_buf_def[i], ~d->type_buf[i]); @@ -352,8 +369,15 @@ static inline int read_sub_irq_data(struct regmap_irq_chip_data *data, for (i = 0; i < subreg->num_regs; i++) { unsigned int offset = subreg->offset[i]; - ret = regmap_read(map, chip->status_base + offset, - &data->status_buf[offset]); + 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[offset]); + if (ret) break; } @@ -474,10 +498,9 @@ static irqreturn_t regmap_irq_thread(int irq, void *d) } else { for (i = 0; i < data->chip->num_regs; i++) { - ret = regmap_read(map, chip->status_base + - (i * map->reg_stride - * data->irq_reg_stride), - &data->status_buf[i]); + unsigned int reg = sub_irq_reg(data, + data->chip->status_base, i); + ret = regmap_read(map, reg, &data->status_buf[i]); if (ret != 0) { dev_err(map->dev, @@ -499,8 +522,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d) data->status_buf[i] &= ~data->mask_buf[i]; if (data->status_buf[i] && (chip->ack_base || chip->use_ack)) { - reg = chip->ack_base + - (i * map->reg_stride * data->irq_reg_stride); + reg = sub_irq_reg(data, data->chip->ack_base, i); + if (chip->ack_invert) ret = regmap_write(map, reg, ~data->status_buf[i]); @@ -605,6 +628,12 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, return -EINVAL; } + if (chip->not_fixed_stride) { + 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) { @@ -700,8 +729,8 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, if (!chip->mask_base) continue; - reg = chip->mask_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->mask_base, i); + if (chip->mask_invert) ret = regmap_irq_update_bits(d, reg, d->mask_buf[i], ~d->mask_buf[i]); @@ -725,8 +754,7 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, continue; /* Ack masked but set interrupts */ - reg = chip->status_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->status_base, i); ret = regmap_read(map, reg, &d->status_buf[i]); if (ret != 0) { dev_err(map->dev, "Failed to read IRQ status: %d\n", @@ -735,8 +763,7 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, } if (d->status_buf[i] && (chip->ack_base || chip->use_ack)) { - reg = chip->ack_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->ack_base, i); if (chip->ack_invert) ret = regmap_write(map, reg, ~(d->status_buf[i] & d->mask_buf[i])); @@ -765,8 +792,7 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, if (d->wake_buf) { for (i = 0; i < chip->num_regs; i++) { d->wake_buf[i] = d->mask_buf_def[i]; - reg = chip->wake_base + - (i * map->reg_stride * d->irq_reg_stride); + reg = sub_irq_reg(d, d->chip->wake_base, i); if (chip->wake_invert) ret = regmap_irq_update_bits(d, reg, @@ -786,8 +812,7 @@ 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 = chip->type_base + - (i * map->reg_stride * d->type_reg_stride); + reg = sub_irq_reg(d, d->chip->type_base, i); ret = regmap_read(map, reg, &d->type_buf_def[i]); diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 2cc4ecd36298..18910bd809f7 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -1378,6 +1378,9 @@ struct regmap_irq_sub_irq_map { * 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. * @@ -1404,6 +1407,9 @@ struct regmap_irq_sub_irq_map { * @clear_on_unmask: For chips with interrupts cleared on read: read the status * registers before unmasking interrupts to clear any bits * set when they were masked. + * @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. * @runtime_pm: Hold a runtime PM lock on the device when accessing it. * * @num_regs: Number of registers in each control bank. @@ -1450,6 +1456,7 @@ struct regmap_irq_chip { bool type_invert:1; bool type_in_mask:1; bool clear_on_unmask:1; + bool not_fixed_stride:1; int num_regs; From 4c5014456305482412b35a081ca0fb4fefd69764 Mon Sep 17 00:00:00 2001 From: Guru Das Srinagesh Date: Wed, 24 Mar 2021 12:28:53 -0700 Subject: [PATCH 2/4] regmap-irq: Introduce virtual regs to handle more config regs Add "virtual" registers support to handle any irq configuration registers in addition to the ones the framework currently supports (status, mask, unmask, wake, type and ack). These are non-standard registers that further configure irq type on some devices, so enable the framework to add a variable number of them. Signed-off-by: Guru Das Srinagesh Link: https://lore.kernel.org/r/a1787067004b0e11cb960319082764397469215a.1616613838.git.gurus@codeaurora.org Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-irq.c | 38 +++++++++++++++++++++++++++++++- include/linux/regmap.h | 5 +++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index e1d8fc9ef040..d1ade76a6c93 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -38,6 +38,7 @@ struct regmap_irq_chip_data { unsigned int *wake_buf; unsigned int *type_buf; unsigned int *type_buf_def; + unsigned int **virt_buf; unsigned int irq_reg_stride; unsigned int type_reg_stride; @@ -94,7 +95,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data) { struct regmap_irq_chip_data *d = irq_data_get_irq_chip_data(data); struct regmap *map = d->map; - int i, ret; + int i, j, ret; u32 reg; u32 unmask_offset; u32 val; @@ -218,6 +219,20 @@ 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 = sub_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); + } + } + } + if (d->chip->runtime_pm) pm_runtime_put(map->dev); @@ -691,6 +706,24 @@ 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(unsigned int), + GFP_KERNEL); + if (!d->virt_buf[i]) + goto err_alloc; + } + } + d->irq_chip = regmap_irq_chip; d->irq_chip.name = chip->name; d->irq = irq; @@ -863,6 +896,9 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, kfree(d->mask_buf); kfree(d->status_buf); kfree(d->status_reg_buf); + for (i = 0; i < chip->num_virt_regs; i++) + kfree(d->virt_buf[i]); + kfree(d->virt_buf); kfree(d); return ret; } diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 18910bd809f7..97ec73383e47 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -1393,6 +1393,7 @@ struct regmap_irq_sub_irq_map { * 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. + * @virt_reg_base: Base addresses for extra config regs. * @irq_reg_stride: Stride to use for chips where registers are not contiguous. * @init_ack_masked: Ack all masked interrupts once during initalization. * @mask_invert: Inverted mask register: cleared bits are masked out. @@ -1417,6 +1418,8 @@ struct regmap_irq_sub_irq_map { * assigned based on the index in the array of the interrupt. * @num_irqs: Number of descriptors. * @num_type_reg: Number of type registers. + * @num_virt_regs: Number of non-standard irq configuration registers. + * If zero unsupported. * @type_reg_stride: Stride to use for chips where type registers are not * contiguous. * @handle_pre_irq: Driver specific callback to handle interrupt from device @@ -1444,6 +1447,7 @@ struct regmap_irq_chip { unsigned int ack_base; unsigned int wake_base; unsigned int type_base; + unsigned int *virt_reg_base; unsigned int irq_reg_stride; bool mask_writeonly:1; bool init_ack_masked:1; @@ -1464,6 +1468,7 @@ struct regmap_irq_chip { int num_irqs; int num_type_reg; + int num_virt_regs; unsigned int type_reg_stride; int (*handle_pre_irq)(void *irq_drv_data); From 394409aafd017adfcffd075595cb01cc456a9327 Mon Sep 17 00:00:00 2001 From: Guru Das Srinagesh Date: Wed, 24 Mar 2021 12:28:54 -0700 Subject: [PATCH 3/4] regmap-irq: Add driver callback to configure virtual regs Enable drivers to configure and modify "virtual" registers, which are non-standard registers that further configure irq type on some devices. Since they are non-standard, enable drivers to configure them according to their particular idiosyncrasies by specifying an optional callback function while registering with the framework. Signed-off-by: Guru Das Srinagesh Link: https://lore.kernel.org/r/07e058cdec2297d15c95c825aa0263064d962d5a.1616613838.git.gurus@codeaurora.org Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-irq.c | 5 +++++ include/linux/regmap.h | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index d1ade76a6c93..e6343ccc6aa1 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -333,6 +333,11 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type) default: return -EINVAL; } + + if (d->chip->set_type_virt) + return d->chip->set_type_virt(d->virt_buf, type, data->hwirq, + reg); + return 0; } diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 97ec73383e47..f87a11a5cc4a 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -1426,6 +1426,8 @@ struct regmap_irq_sub_irq_map { * before regmap_irq_handler process the interrupts. * @handle_post_irq: Driver specific callback to handle interrupt from device * after handling the interrupts in regmap_irq_handler(). + * @set_type_virt: Driver specific callback to extend regmap_irq_set_type() + * and configure virt regs. * @irq_drv_data: Driver specific IRQ data which is passed as parameter when * driver specific pre/post interrupt handler is called. * @@ -1473,6 +1475,8 @@ struct regmap_irq_chip { int (*handle_pre_irq)(void *irq_drv_data); int (*handle_post_irq)(void *irq_drv_data); + int (*set_type_virt)(unsigned int **buf, unsigned int type, + unsigned long hwirq, int reg); void *irq_drv_data; }; From 14e13b1ce92ea278fc0d7bb95b340b46cff624ab Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 6 Apr 2021 17:40:02 +0100 Subject: [PATCH 4/4] regmap-irq: Fix dereference of a potentially null d->virt_buf The clean up of struct d can potentiallly index into a null array d->virt_buf causing errorenous pointer dereferencing issues on kfree calls. Fix this by adding a null check on d->virt_buf before attempting to traverse the array to kfree the objects. Addresses-Coverity: ("Dereference after null check") Fixes: 4c5014456305 ("regmap-irq: Introduce virtual regs to handle more config regs") Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20210406164002.430221-1-colin.king@canonical.com Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-irq.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index e6343ccc6aa1..760296a4b606 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -901,9 +901,11 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode, kfree(d->mask_buf); kfree(d->status_buf); kfree(d->status_reg_buf); - for (i = 0; i < chip->num_virt_regs; i++) - kfree(d->virt_buf[i]); - kfree(d->virt_buf); + if (d->virt_buf) { + for (i = 0; i < chip->num_virt_regs; i++) + kfree(d->virt_buf[i]); + kfree(d->virt_buf); + } kfree(d); return ret; }