* [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported @ 2013-03-07 16:33 Laxman Dewangan 2013-03-07 16:33 ` [PATCH V2 RESEND 1/2] regmap: irq: Add support for interrupt type Laxman Dewangan 2013-03-12 19:35 ` [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported Mark Brown 0 siblings, 2 replies; 5+ messages in thread From: Laxman Dewangan @ 2013-03-07 16:33 UTC (permalink / raw) To: broonie, gregkh; +Cc: linux-kernel, swarren, Laxman Dewangan Ignore the mask register write if mask_base is not provided by regmap irq client. Also assume that all interrupts are enabled in this case. This is useful when device does not have explicit interrupt mask register but control the interrupt enabling/disabling by other mechanism like irq type/irq edge registers. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- Changes from V1: - Rewrite description. - Assume interrupt enable when there is no mask register. - Resending in case of if it is missed. drivers/base/regmap/regmap-irq.c | 16 +++++++++++++++- 1 files changed, 15 insertions(+), 1 deletions(-) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index 8ca32bc..a8896ac 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -80,6 +80,9 @@ static void regmap_irq_sync_unlock(struct irq_data *data) * suppress pointless writes. */ for (i = 0; i < d->chip->num_regs; i++) { + if (!d->chip->mask_base) + goto skip_mask_reg_update; + reg = d->chip->mask_base + (i * map->reg_stride * d->irq_reg_stride); if (d->chip->mask_invert) @@ -92,6 +95,7 @@ static void regmap_irq_sync_unlock(struct irq_data *data) dev_err(d->map->dev, "Failed to sync masks in %x\n", reg); +skip_mask_reg_update: reg = d->chip->wake_base + (i * map->reg_stride * d->irq_reg_stride); if (d->wake_buf) { @@ -491,7 +495,17 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags, /* Mask all the interrupts by default */ for (i = 0; i < chip->num_regs; i++) { - d->mask_buf[i] = d->mask_buf_def[i]; + /** + * If no mask base register then assuming all interrupt are + * unmasked. + */ + if (!chip->mask_base) { + d->mask_buf[i] = ~d->mask_buf_def[i]; + continue; + } else { + d->mask_buf[i] = d->mask_buf_def[i]; + } + reg = chip->mask_base + (i * map->reg_stride * d->irq_reg_stride); if (chip->mask_invert) -- 1.7.1.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V2 RESEND 1/2] regmap: irq: Add support for interrupt type 2013-03-07 16:33 [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported Laxman Dewangan @ 2013-03-07 16:33 ` Laxman Dewangan 2013-03-12 19:55 ` Mark Brown 2013-03-12 19:35 ` [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported Mark Brown 1 sibling, 1 reply; 5+ messages in thread From: Laxman Dewangan @ 2013-03-07 16:33 UTC (permalink / raw) To: broonie, gregkh; +Cc: linux-kernel, swarren, Laxman Dewangan Add support of setting the irq_type of the interrupt which is registered through regmap irq framework. The client who register the interrupt can pass the supported irq type through irq chip data along with the type mask and type value of each type which is supported. Client also need to provide the type base register address and number of type register to write into the device. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- Changes from V1: - Make the irq type value in place of bit wise allocation as suggested in patch V1. - Also added the suboffset register for type so that type configuration can be acoomodated in multiple register. - Rewrite the description. - Resending in case of it it is missed. drivers/base/regmap/regmap-irq.c | 126 ++++++++++++++++++++++++++++++++++++++ include/linux/regmap.h | 40 ++++++++++++ 2 files changed, 166 insertions(+), 0 deletions(-) diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c index 4706c63..8ca32bc 100644 --- a/drivers/base/regmap/regmap-irq.c +++ b/drivers/base/regmap/regmap-irq.c @@ -39,8 +39,11 @@ struct regmap_irq_chip_data { unsigned int *mask_buf; unsigned int *mask_buf_def; unsigned int *wake_buf; + unsigned int *type_buf; + unsigned int *type_buf_def; unsigned int irq_reg_stride; + unsigned int type_reg_stride; }; static inline const @@ -107,6 +110,24 @@ static void regmap_irq_sync_unlock(struct irq_data *data) } } + for (i = 0; i < d->chip->num_type_reg; i++) { + int j; + for (j = 0; j < d->chip->num_type_sub_reg; ++j) { + int k = i * d->chip->num_type_sub_reg + j; + + if (!d->type_buf_def[k]) + continue; + + reg = d->chip->type_base + + (i * map->reg_stride * d->type_reg_stride) + j; + ret = regmap_update_bits(d->map, reg, + d->type_buf_def[k], d->type_buf[k]); + if (ret != 0) + dev_err(d->map->dev, + "Failed to sync type in %x\n", reg); + } + } + if (d->chip->runtime_pm) pm_runtime_put(map->dev); @@ -141,6 +162,56 @@ static void regmap_irq_disable(struct irq_data *data) d->mask_buf[irq_data->reg_offset / map->reg_stride] |= irq_data->mask; } +static int regmap_irq_set_type(struct irq_data *data, unsigned int type) +{ + struct regmap_irq_chip_data *d = irq_data_get_irq_chip_data(data); + struct regmap *map = d->map; + const struct regmap_irq *irq_data = irq_to_regmap_irq(d, data->hwirq); + int reg; + + if (!irq_data->type_supported_flags) + return 0; + + type &= irq_data->type_supported_flags; + reg = (irq_data->type_reg_offset / map->reg_stride) * + d->chip->num_type_sub_reg + + irq_data->type_reg_sub_offset; + d->type_buf[reg] &= ~irq_data->type_mask; + + switch (type) { + case IRQ_TYPE_EDGE_FALLING: + d->type_buf[reg] |= + irq_data->type_value[REGMAP_IRQ_TYPE_FALLING]; + break; + + case IRQ_TYPE_EDGE_RISING: + d->type_buf[reg] |= + irq_data->type_value[REGMAP_IRQ_TYPE_RISING]; + break; + + case IRQ_TYPE_EDGE_BOTH: + d->type_buf[reg] |= + irq_data->type_value[REGMAP_IRQ_TYPE_BOTH]; + break; + + case IRQ_TYPE_LEVEL_HIGH: + d->type_buf[reg] |= + irq_data->type_value[REGMAP_IRQ_TYPE_LEVEL_HIGH]; + break; + + case IRQ_TYPE_LEVEL_LOW: + d->type_buf[reg] |= + irq_data->type_value[REGMAP_IRQ_TYPE_LEVEL_LOW]; + break; + + default: + d->type_buf[reg] |= + irq_data->type_value[REGMAP_IRQ_TYPE_NONE]; + break; + } + return 0; +} + static int regmap_irq_set_wake(struct irq_data *data, unsigned int on) { struct regmap_irq_chip_data *d = irq_data_get_irq_chip_data(data); @@ -167,6 +238,7 @@ static const struct irq_chip regmap_irq_chip = { .irq_bus_sync_unlock = regmap_irq_sync_unlock, .irq_disable = regmap_irq_disable, .irq_enable = regmap_irq_enable, + .irq_set_type = regmap_irq_set_type, .irq_set_wake = regmap_irq_set_wake, }; @@ -375,6 +447,20 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags, goto err_alloc; } + if (chip->num_type_reg) { + d->type_buf_def = kzalloc(sizeof(unsigned int) * + chip->num_type_reg * + chip->num_type_sub_reg, GFP_KERNEL); + if (!d->type_buf_def) + goto err_alloc; + + d->type_buf = kzalloc(sizeof(unsigned int) * + chip->num_type_reg * chip->num_type_sub_reg, + GFP_KERNEL); + if (!d->type_buf) + goto err_alloc; + } + d->irq_chip = regmap_irq_chip; d->irq_chip.name = chip->name; d->irq = irq; @@ -387,6 +473,8 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags, else d->irq_reg_stride = 1; + d->type_reg_stride = chip->type_reg_stride ? : 1; + if (!map->use_single_rw && map->reg_stride == 1 && d->irq_reg_stride == 1) { d->status_reg_buf = kmalloc(map->format.val_bytes * @@ -442,6 +530,40 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags, } } + if (chip->num_type_reg) { + for (i = 0; i < chip->num_irqs; i++) { + int tv = chip->irqs[i].type_value[REGMAP_IRQ_TYPE_NONE]; + + reg = (chip->irqs[i].type_reg_offset / + map->reg_stride) * chip->num_type_sub_reg + + chip->irqs[i].type_reg_sub_offset; + d->type_buf_def[reg] |= chip->irqs[i].type_mask; + d->type_buf[reg] |= (tv & chip->irqs[i].type_mask); + } + + for (i = 0; i < chip->num_type_reg; ++i) { + int j; + for (j = 0; j < chip->num_type_sub_reg; ++j) { + int k = i * chip->num_type_sub_reg + j; + + if (!d->type_buf_def[k]) + continue; + + reg = chip->type_base + (i * map->reg_stride * + d->type_reg_stride) + j; + + ret = regmap_update_bits(map, reg, + d->type_buf_def[k], d->type_buf[k]); + if (ret != 0) { + dev_err(map->dev, + "Failed to set type in 0x%x: %x\n", + reg, ret); + goto err_alloc; + } + } + } + } + if (irq_base) d->domain = irq_domain_add_legacy(map->dev->of_node, chip->num_irqs, irq_base, 0, @@ -468,6 +590,8 @@ int regmap_add_irq_chip(struct regmap *map, int irq, int irq_flags, err_domain: /* Should really dispose of the domain but... */ err_alloc: + kfree(d->type_buf); + kfree(d->type_buf_def); kfree(d->wake_buf); kfree(d->mask_buf_def); kfree(d->mask_buf); @@ -491,6 +615,8 @@ void regmap_del_irq_chip(int irq, struct regmap_irq_chip_data *d) free_irq(irq, d); /* We should unmap the domain but... */ + kfree(d->type_buf); + kfree(d->type_buf_def); kfree(d->wake_buf); kfree(d->mask_buf_def); kfree(d->mask_buf); diff --git a/include/linux/regmap.h b/include/linux/regmap.h index e34f9f1..c07fcb8 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -376,14 +376,42 @@ bool regmap_reg_in_ranges(unsigned int reg, unsigned int nranges); /** + * The Regmap IRQ type Index + * REGMAP_IRQ_TYPE_NONE is used for setting inital value for clearing type. + */ +enum { + REGMAP_IRQ_TYPE_NONE, + REGMAP_IRQ_TYPE_RISING, + REGMAP_IRQ_TYPE_FALLING, + REGMAP_IRQ_TYPE_BOTH, + REGMAP_IRQ_TYPE_LEVEL_HIGH, + REGMAP_IRQ_TYPE_LEVEL_LOW, + + /* Last entry to get maximum index */ + REGMAP_IRQ_TYPE_NR, +}; + +/** * Description of an IRQ for the generic regmap irq_chip. * * @reg_offset: Offset of the status/mask register within the bank * @mask: Mask used to flag/control the register. + * @type_reg_offset: Offset register for the irq type setting. + * @type_reg_sub_offset: Suboffset for type register if type mask are + * accomodated in the multiple register. + * @type_supported_flags: Supported interrupt type as per interrupt.h. + * All supported flags are ORed. + * type_mask: Type mask for getting related register bits. + * @type_value: The type value in array form to set value for a given flag. */ struct regmap_irq { unsigned int reg_offset; unsigned int mask; + unsigned int type_reg_offset; + unsigned int type_reg_sub_offset; + unsigned int type_supported_flags; + unsigned int type_mask; + unsigned int type_value[REGMAP_IRQ_TYPE_NR]; }; /** @@ -397,6 +425,7 @@ struct regmap_irq { * @mask_base: Base mask register address. * @ack_base: Base ack address. If zero then the chip is clear on read. * @wake_base: Base address for wake enables. If zero unsupported. + * @type_base: Base address for irq type. If zero unsupported. * @irq_reg_stride: Stride to use for chips where registers are not contiguous. * @runtime_pm: Hold a runtime PM lock on the device when accessing it. * @@ -404,6 +433,12 @@ struct regmap_irq { * @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. + * @num_type_sub_reg: Number of sub register for type. It may be possible that + * type configuration register for given set on interrupt from one + * register are having multiple register for type configuration. + * @type_reg_stride: Stride to use for chips where type registers are not + * contiguous. */ struct regmap_irq_chip { const char *name; @@ -412,6 +447,7 @@ struct regmap_irq_chip { unsigned int mask_base; unsigned int ack_base; unsigned int wake_base; + unsigned int type_base; unsigned int irq_reg_stride; unsigned int mask_invert; unsigned int wake_invert; @@ -421,6 +457,10 @@ struct regmap_irq_chip { const struct regmap_irq *irqs; int num_irqs; + + int num_type_reg; + int num_type_sub_reg; + unsigned int type_reg_stride; }; struct regmap_irq_chip_data; -- 1.7.1.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2 RESEND 1/2] regmap: irq: Add support for interrupt type 2013-03-07 16:33 ` [PATCH V2 RESEND 1/2] regmap: irq: Add support for interrupt type Laxman Dewangan @ 2013-03-12 19:55 ` Mark Brown 2013-03-16 14:46 ` Laxman Dewangan 0 siblings, 1 reply; 5+ messages in thread From: Mark Brown @ 2013-03-12 19:55 UTC (permalink / raw) To: Laxman Dewangan; +Cc: gregkh, linux-kernel, swarren [-- Attachment #1: Type: text/plain, Size: 1205 bytes --] On Thu, Mar 07, 2013 at 10:03:17PM +0530, Laxman Dewangan wrote: > /** > + * The Regmap IRQ type Index > + * REGMAP_IRQ_TYPE_NONE is used for setting inital value for clearing type. > + */ > +enum { > + REGMAP_IRQ_TYPE_NONE, > + REGMAP_IRQ_TYPE_RISING, > + REGMAP_IRQ_TYPE_FALLING, > + REGMAP_IRQ_TYPE_BOTH, > + REGMAP_IRQ_TYPE_LEVEL_HIGH, > + REGMAP_IRQ_TYPE_LEVEL_LOW, > + > + /* Last entry to get maximum index */ > + REGMAP_IRQ_TYPE_NR, Why can't we just use the normal IRQ_TYPE macros? > + * @type_reg_sub_offset: Suboffset for type register if type mask are > + * accomodated in the multiple register. > + * @type_supported_flags: Supported interrupt type as per interrupt.h. > + * All supported flags are ORed. > + * type_mask: Type mask for getting related register bits. > + * @type_value: The type value in array form to set value for a given flag. This is making my head hurt, it's a bit confusing - I'm having to think too hard to understand what all these different fields do and how they interact. Especially type_reg_sub_offset which feels like it must be part of a wider feature to support a greater range of register layouts. Can you clarify a bit please? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2 RESEND 1/2] regmap: irq: Add support for interrupt type 2013-03-12 19:55 ` Mark Brown @ 2013-03-16 14:46 ` Laxman Dewangan 0 siblings, 0 replies; 5+ messages in thread From: Laxman Dewangan @ 2013-03-16 14:46 UTC (permalink / raw) To: Mark Brown Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, Stephen Warren On Wednesday 13 March 2013 01:25 AM, Mark Brown wrote: > * PGP Signed by an unknown key > > On Thu, Mar 07, 2013 at 10:03:17PM +0530, Laxman Dewangan wrote: > >> /** >> + * The Regmap IRQ type Index >> + * REGMAP_IRQ_TYPE_NONE is used for setting inital value for clearing type. >> + */ >> +enum { >> + REGMAP_IRQ_TYPE_NONE, >> + REGMAP_IRQ_TYPE_RISING, >> + REGMAP_IRQ_TYPE_FALLING, >> + REGMAP_IRQ_TYPE_BOTH, >> + REGMAP_IRQ_TYPE_LEVEL_HIGH, >> + REGMAP_IRQ_TYPE_LEVEL_LOW, >> + >> + /* Last entry to get maximum index */ >> + REGMAP_IRQ_TYPE_NR, > Why can't we just use the normal IRQ_TYPE macros? IRQ_TYPE is bit wise allocated and I wanted to have on serieal index to assign value on array indexed with type. > >> + * @type_reg_sub_offset: Suboffset for type register if type mask are >> + * accomodated in the multiple register. >> + * @type_supported_flags: Supported interrupt type as per interrupt.h. >> + * All supported flags are ORed. >> + * type_mask: Type mask for getting related register bits. >> + * @type_value: The type value in array form to set value for a given flag. > This is making my head hurt, it's a bit confusing - I'm having to think > too hard to understand what all these different fields do and how they > interact. Especially type_reg_sub_offset which feels like it must be > part of a wider feature to support a greater range of register layouts. > > Can you clarify a bit please? > Sorry for complicating the things. When I added this feature I was working on two PMICs, Max77660/MAX77663 and the Palmas. I also consider how it is possible to handle in the tps65910 but seems not possible. All this driver uses the regmap-irq. (MAX77660 is not in mainline and working on upstreaming this) On Max77660/Max77663, the interrupt support is having 2 level, one top level for main interrupt like GPIO, RTC interrupt and second level in the sub module like for RTC submodule: alram or timer in RTC interrupt mask/status, for GPIO submodule: gpio control and gpio interrupt status. GPIO sub module of MAX77660/MAX77663 is as follows: - Top level interrupt for gpio module. - GPIO module have GPIOx_CNTRL where bit6:5 shows the rising/falling edge interrupt and GPIO_INT_STATUS shows the interrupt status. GPIO0_CNTL(0x20): bit 6:5: rising/falling edge of gpio0 GPIO1_CNTL(0x21): bit 65:5 rising/falling edge of gpio1 :::::::::::::::::::::::::::: GPIO7_CNTRL(0x27): bit 65:5 rising/falling edge of gpio7 GPIO_INT_STATUS: bit 0 for gpio0, bit 1 for gpio1, bit 2 for gpio2 etc. There is no separate mask register here. For palmas: All interrupts are on same level, there is no two level of interrupts. There is 4 interrupt mask/status interrupt. INT4_MASK/STATUS: bit 0 for gpio0, bit 1 for gpio1, bit 2 for gpio2 etc. INT4_EDGE1_REG: bit 1:0 for gpio0 rising/falling, bit 3:2 for gpio1 rising falling... INT4_EDGE2_REG: bit 1:0 for gpio4 rising/falling, bit 3:2 for gpio5 rising falling... It has mask, interrupt status, edge1, edg2 as part of set of register. For other interrups also there is same sets but edge1 and edge2 are reserved. To support rising/falling through regmap, I consider above two type of register set. Here as we can have more than 2 register for edge configuration, I make register_sub_offset with type_register to address edge1 and edge2 register. Now, Palmas series have multiple PMICs and currently I am wokring on tps65913 and tps80036. It is easy to support interrupt through regmap-irq for tps65913 but not possible to tps80036. In tps65913, there is 4 int status/mask sets properly address spaced. INT1_STATUS 0x0, INT2_STATUS 0x5, INT3_STATUS 0xA, INT4_STATUS 0xF But tps80036 added few more interrupts and so INT5_STATUS which is not at 0x14 but it is at different 0x15 and INT6_STATUS 0x1A. So using regmap-irq for tps80036 is not possible. So given above limitation on palma, I think I should not complicate type support, only add register_type and drop the register_type_offset, typically to support the MAX77660. let me know your opinion. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported 2013-03-07 16:33 [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported Laxman Dewangan 2013-03-07 16:33 ` [PATCH V2 RESEND 1/2] regmap: irq: Add support for interrupt type Laxman Dewangan @ 2013-03-12 19:35 ` Mark Brown 1 sibling, 0 replies; 5+ messages in thread From: Mark Brown @ 2013-03-12 19:35 UTC (permalink / raw) To: Laxman Dewangan; +Cc: gregkh, linux-kernel, swarren [-- Attachment #1: Type: text/plain, Size: 572 bytes --] On Thu, Mar 07, 2013 at 10:03:16PM +0530, Laxman Dewangan wrote: > Ignore the mask register write if mask_base is not provided by > regmap irq client. Also assume that all interrupts are enabled > in this case. > This is useful when device does not have explicit interrupt mask > register but control the interrupt enabling/disabling by other > mechanism like irq type/irq edge registers. I would really expect that the operations that mask things would be failing if we can't perform them, things are going to get upset if they get interrupts they thought were masked. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-03-16 14:48 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-07 16:33 [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported Laxman Dewangan 2013-03-07 16:33 ` [PATCH V2 RESEND 1/2] regmap: irq: Add support for interrupt type Laxman Dewangan 2013-03-12 19:55 ` Mark Brown 2013-03-16 14:46 ` Laxman Dewangan 2013-03-12 19:35 ` [PATCH V2 RESEND 2/2] regmap: irq: do not write mask register if it is not supported Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox