Add level active IRQ support to regmap-irq irqchip. Change breaks
existing regmap-irq type setting. Convert the existing drivers which
use regmap-irq with trigger type setting (gpio-max77620) to work
with this new approach. So we do not magically support level-active
IRQs on gpio-max77620 - but add support to the regmap-irq for chips
which support them =)
We do not support distinguishing situation where HW supports rising
and falling edge detection but not both. Separating this would require
inventing yet another flags for IRQ types.
Signed-off-by: Matti Vaittinen <[email protected]>
---
I did both the regmap-irq and max77620 changes in same commit because
I'd rather not cause spot where max77620 breaks. Besides the changes in
max77620 driver are trivial. Please let me know if this is not Ok.
Reason why I submit this patch now - even though my driver which would
use level active type setting with regmap-irq is not yet ready for
being submited - is that I'd like to minimize amount of existing drivers
we need to patch. And if we add level active irq support like this then
we must patch all existing drivers using type setting with regmap-irq.
So doing this now when only max77620 uses type setting may be easier
than postponing this to the future.
And finally - I don't have max77620 so I have only done _wery_ limited
testing. So I would really appreciate if someone had time to review this
thoroughly - and even happier if someone had possibility to try this out
with the max77620.
drivers/base/regmap/regmap-irq.c | 19 +++++++++++++++++--
drivers/gpio/gpio-max77620.c | 8 ++++++++
include/linux/regmap.h | 6 ++++++
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 429ca8ed7e51..5e7e10e4c1dc 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -214,11 +214,17 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type)
const struct regmap_irq *irq_data = irq_to_regmap_irq(d, data->hwirq);
int reg = irq_data->type_reg_offset / map->reg_stride;
- if (!(irq_data->type_rising_mask | irq_data->type_falling_mask))
+ if ((irq_data->types_supported & type) != type)
+ return -ENOTSUPP;
+
+ if (!(irq_data->type_rising_mask | irq_data->type_falling_mask |
+ irq_data->type_level_high_mask | irq_data->type_level_low_mask))
return 0;
d->type_buf[reg] &= ~(irq_data->type_falling_mask |
- irq_data->type_rising_mask);
+ irq_data->type_rising_mask |
+ irq_data->type_level_low_mask |
+ irq_data->type_level_high_mask);
switch (type) {
case IRQ_TYPE_EDGE_FALLING:
d->type_buf[reg] |= irq_data->type_falling_mask;
@@ -233,6 +239,13 @@ static int regmap_irq_set_type(struct irq_data *data, unsigned int type)
irq_data->type_rising_mask);
break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ d->type_buf[reg] |= irq_data->type_level_high_mask;
+ break;
+
+ case IRQ_TYPE_LEVEL_LOW:
+ d->type_buf[reg] |= irq_data->type_level_low_mask;
+ break;
default:
return -EINVAL;
}
@@ -602,6 +615,8 @@ 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++) {
+ if (!chip->irqs[i].types_supported)
+ continue;
reg = chip->irqs[i].type_reg_offset / map->reg_stride;
d->type_buf_def[reg] |= chip->irqs[i].type_rising_mask |
chip->irqs[i].type_falling_mask;
diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c
index 538bce4b5b42..e3b761b526c3 100644
--- a/drivers/gpio/gpio-max77620.c
+++ b/drivers/gpio/gpio-max77620.c
@@ -30,6 +30,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 0,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
[1] = {
.mask = MAX77620_IRQ_LVL2_GPIO_EDGE1,
@@ -37,6 +38,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 1,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
[2] = {
.mask = MAX77620_IRQ_LVL2_GPIO_EDGE2,
@@ -44,6 +46,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 2,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
[3] = {
.mask = MAX77620_IRQ_LVL2_GPIO_EDGE3,
@@ -51,6 +54,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 3,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
[4] = {
.mask = MAX77620_IRQ_LVL2_GPIO_EDGE4,
@@ -58,6 +62,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 4,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
[5] = {
.mask = MAX77620_IRQ_LVL2_GPIO_EDGE5,
@@ -65,6 +70,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 5,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
[6] = {
.mask = MAX77620_IRQ_LVL2_GPIO_EDGE6,
@@ -72,6 +78,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 6,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
[7] = {
.mask = MAX77620_IRQ_LVL2_GPIO_EDGE7,
@@ -79,6 +86,7 @@ static const struct regmap_irq max77620_gpio_irqs[] = {
.type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
.reg_offset = 0,
.type_reg_offset = 7,
+ .types_supported = IRQ_TYPE_EDGE_BOTH;
},
};
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index a367d59c301d..91c431ad98c3 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1098,6 +1098,9 @@ int regmap_fields_update_bits_base(struct regmap_field *field, unsigned int id,
* @type_reg_offset: Offset register for the irq type setting.
* @type_rising_mask: Mask bit to configure RISING type irq.
* @type_falling_mask: Mask bit to configure FALLING type irq.
+ * @type_level_low_mask: Mask bit to configure LEVEL_LOW type irq.
+ * @type_level_high_mask: Mask bit to configure LEVEL_HIGH type irq.
+ * @types_supported: logical OR of IRQ_TYPE_* flags indicating supported types.
*/
struct regmap_irq {
unsigned int reg_offset;
@@ -1105,6 +1108,9 @@ struct regmap_irq {
unsigned int type_reg_offset;
unsigned int type_rising_mask;
unsigned int type_falling_mask;
+ unsigned int type_level_low_mask;
+ unsigned int type_level_high_mask;
+ unsigned int types_supported;
};
#define REGMAP_IRQ_REG(_irq, _off, _mask) \
--
2.14.3
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
Hi Matti,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.20-rc6 next-20181207]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Matti-Vaittinen/regmap-regmap-irq-gpio-max77620-add-level-irq-support/20181210-212928
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-x011-201849 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> drivers/gpio/gpio-max77620.c:33:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
drivers/gpio/gpio-max77620.c:41:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
drivers/gpio/gpio-max77620.c:49:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
drivers/gpio/gpio-max77620.c:57:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
drivers/gpio/gpio-max77620.c:65:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
drivers/gpio/gpio-max77620.c:73:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
drivers/gpio/gpio-max77620.c:81:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
drivers/gpio/gpio-max77620.c:89:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
^
vim +33 drivers/gpio/gpio-max77620.c
25
26 static const struct regmap_irq max77620_gpio_irqs[] = {
27 [0] = {
28 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE0,
29 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
30 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
31 .reg_offset = 0,
32 .type_reg_offset = 0,
> 33 .types_supported = IRQ_TYPE_EDGE_BOTH;
34 },
35 [1] = {
36 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE1,
37 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
38 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
39 .reg_offset = 0,
40 .type_reg_offset = 1,
41 .types_supported = IRQ_TYPE_EDGE_BOTH;
42 },
43 [2] = {
44 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE2,
45 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
46 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
47 .reg_offset = 0,
48 .type_reg_offset = 2,
49 .types_supported = IRQ_TYPE_EDGE_BOTH;
50 },
51 [3] = {
52 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE3,
53 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
54 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
55 .reg_offset = 0,
56 .type_reg_offset = 3,
57 .types_supported = IRQ_TYPE_EDGE_BOTH;
58 },
59 [4] = {
60 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE4,
61 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
62 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
63 .reg_offset = 0,
64 .type_reg_offset = 4,
65 .types_supported = IRQ_TYPE_EDGE_BOTH;
66 },
67 [5] = {
68 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE5,
69 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
70 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
71 .reg_offset = 0,
72 .type_reg_offset = 5,
73 .types_supported = IRQ_TYPE_EDGE_BOTH;
74 },
75 [6] = {
76 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE6,
77 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
78 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
79 .reg_offset = 0,
80 .type_reg_offset = 6,
81 .types_supported = IRQ_TYPE_EDGE_BOTH;
82 },
83 [7] = {
84 .mask = MAX77620_IRQ_LVL2_GPIO_EDGE7,
85 .type_rising_mask = MAX77620_CNFG_GPIO_INT_RISING,
86 .type_falling_mask = MAX77620_CNFG_GPIO_INT_FALLING,
87 .reg_offset = 0,
88 .type_reg_offset = 7,
89 .types_supported = IRQ_TYPE_EDGE_BOTH;
90 },
91 };
92
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 12/10/2018 10:14 AM, Matti Vaittinen wrote:
> Add level active IRQ support to regmap-irq irqchip. Change breaks
> existing regmap-irq type setting. Convert the existing drivers which
> use regmap-irq with trigger type setting (gpio-max77620) to work
> with this new approach. So we do not magically support level-active
> IRQs on gpio-max77620 - but add support to the regmap-irq for chips
> which support them =)
>
> We do not support distinguishing situation where HW supports rising
> and falling edge detection but not both. Separating this would require
> inventing yet another flags for IRQ types.
>
> Signed-off-by: Matti Vaittinen <[email protected]>
> ---
> I did both the regmap-irq and max77620 changes in same commit because
> I'd rather not cause spot where max77620 breaks. Besides the changes in
> max77620 driver are trivial. Please let me know if this is not Ok.
>
> Reason why I submit this patch now - even though my driver which would
> use level active type setting with regmap-irq is not yet ready for
> being submited - is that I'd like to minimize amount of existing drivers
> we need to patch. And if we add level active irq support like this then
> we must patch all existing drivers using type setting with regmap-irq.
> So doing this now when only max77620 uses type setting may be easier
> than postponing this to the future.
>
> And finally - I don't have max77620 so I have only done _wery_ limited
> testing. So I would really appreciate if someone had time to review this
> thoroughly - and even happier if someone had possibility to try this out
> with the max77620.
>
[snip]
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index a367d59c301d..91c431ad98c3 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -1098,6 +1098,9 @@ int regmap_fields_update_bits_base(struct regmap_field *field, unsigned int id,
> * @type_reg_offset: Offset register for the irq type setting.
> * @type_rising_mask: Mask bit to configure RISING type irq.
> * @type_falling_mask: Mask bit to configure FALLING type irq.
> + * @type_level_low_mask: Mask bit to configure LEVEL_LOW type irq.
> + * @type_level_high_mask: Mask bit to configure LEVEL_HIGH type irq.
> + * @types_supported: logical OR of IRQ_TYPE_* flags indicating supported types.
While the existing interface is awful, you don't make it better.
.types_supported value is always derived from non-zero .type_*_mask values, so
it is simply not needed, as well as the whole change to gpio-max77620.c driver.
> */
> struct regmap_irq {
> unsigned int reg_offset;
> @@ -1105,6 +1108,9 @@ struct regmap_irq {
> unsigned int type_reg_offset;
> unsigned int type_rising_mask;
> unsigned int type_falling_mask;
> + unsigned int type_level_low_mask;
> + unsigned int type_level_high_mask;
> + unsigned int types_supported;
> };
>
> #define REGMAP_IRQ_REG(_irq, _off, _mask) \
>
--
Best wishes,
Vladimir
________________________________________
From: kbuild test robot [[email protected]]
Sent: Monday, December 10, 2018 4:19 PM
To: Vaittinen, Matti
Cc: [email protected]; Vaittinen, Matti; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Haikola, Heikki; Mutanen, Mikko
Subject: Re: [PATCH] regmap: regmap-irq/gpio-max77620: add level-irq support
> Hi Matti,
>
>Thank you for the patch! Yet something to improve:
>> drivers/gpio/gpio-max77620.c:33:40: error: expected '}' before ';' token
.types_supported = IRQ_TYPE_EDGE_BOTH;
Sorry folks, something that should not have happened... Please ignore this patch - I'll send new one where I also do ensure the max77620 gets compile tested - tomorrow.
Br.
Matti Vaittinen
Hello Vladimir,
Thanks for the review.
On Mon, Dec 10, 2018 at 05:16:28PM +0200, Vladimir Zapolskiy wrote:
> On 12/10/2018 10:14 AM, Matti Vaittinen wrote:
> > Add level active IRQ support to regmap-irq irqchip. Change breaks
> > existing regmap-irq type setting. Convert the existing drivers which
> > use regmap-irq with trigger type setting (gpio-max77620) to work
> > with this new approach. So we do not magically support level-active
> > IRQs on gpio-max77620 - but add support to the regmap-irq for chips
> > which support them =)
> >
> > We do not support distinguishing situation where HW supports rising
> > and falling edge detection but not both. Separating this would require
> > inventing yet another flags for IRQ types.
> >
> > Signed-off-by: Matti Vaittinen <[email protected]>
> > ---
> > I did both the regmap-irq and max77620 changes in same commit because
> > I'd rather not cause spot where max77620 breaks. Besides the changes in
> > max77620 driver are trivial. Please let me know if this is not Ok.
> >
> > Reason why I submit this patch now - even though my driver which would
> > use level active type setting with regmap-irq is not yet ready for
> > being submited - is that I'd like to minimize amount of existing drivers
> > we need to patch. And if we add level active irq support like this then
> > we must patch all existing drivers using type setting with regmap-irq.
> > So doing this now when only max77620 uses type setting may be easier
> > than postponing this to the future.
> >
> > And finally - I don't have max77620 so I have only done _wery_ limited
> > testing. So I would really appreciate if someone had time to review this
> > thoroughly - and even happier if someone had possibility to try this out
> > with the max77620.
> >
>
> [snip]
>
> > diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> > index a367d59c301d..91c431ad98c3 100644
> > --- a/include/linux/regmap.h
> > +++ b/include/linux/regmap.h
> > @@ -1098,6 +1098,9 @@ int regmap_fields_update_bits_base(struct regmap_field *field, unsigned int id,
> > * @type_reg_offset: Offset register for the irq type setting.
> > * @type_rising_mask: Mask bit to configure RISING type irq.
> > * @type_falling_mask: Mask bit to configure FALLING type irq.
> > + * @type_level_low_mask: Mask bit to configure LEVEL_LOW type irq.
> > + * @type_level_high_mask: Mask bit to configure LEVEL_HIGH type irq.
> > + * @types_supported: logical OR of IRQ_TYPE_* flags indicating supported types.
>
> While the existing interface is awful, you don't make it better.
>
> .types_supported value is always derived from non-zero .type_*_mask values, so
> it is simply not needed, as well as the whole change to gpio-max77620.c driver.
Right. I didn't consider the "type_inverted" flag in the struct
regmap_irq_chip. I thought that "mask" is actually a register value -
which could be zero for some HWs. Thanks for pointing that out. It will
really make "types_supported" useless.
So please just drop this patch for now. There seems to be no need to
touch the existing regmap-irq users after all so I can submit this patch
together with the driver which needs to support the level active IRQs.
>
> > */
> > struct regmap_irq {
> > unsigned int reg_offset;
> > @@ -1105,6 +1108,9 @@ struct regmap_irq {
> > unsigned int type_reg_offset;
> > unsigned int type_rising_mask;
> > unsigned int type_falling_mask;
> > + unsigned int type_level_low_mask;
> > + unsigned int type_level_high_mask;
> > + unsigned int types_supported;
> > };
> >
> > #define REGMAP_IRQ_REG(_irq, _off, _mask) \
> >
>
> --
> Best wishes,
> Vladimir
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
Hello Again All,
Sorry for multiple posts but I had second look at this and...
On Tue, Dec 11, 2018 at 08:38:25AM +0200, Matti Vaittinen wrote:
> Hello Vladimir,
>
> Thanks for the review.
>
> On Mon, Dec 10, 2018 at 05:16:28PM +0200, Vladimir Zapolskiy wrote:
> > On 12/10/2018 10:14 AM, Matti Vaittinen wrote:
> > > Add level active IRQ support to regmap-irq irqchip. Change breaks
> > > existing regmap-irq type setting. Convert the existing drivers which
> > > use regmap-irq with trigger type setting (gpio-max77620) to work
> > > with this new approach. So we do not magically support level-active
> > > IRQs on gpio-max77620 - but add support to the regmap-irq for chips
> > > which support them =)
> > >
> > > We do not support distinguishing situation where HW supports rising
> > > and falling edge detection but not both. Separating this would require
> > > inventing yet another flags for IRQ types.
> > >
> > > Signed-off-by: Matti Vaittinen <[email protected]>
> > > ---
> > > I did both the regmap-irq and max77620 changes in same commit because
> > > I'd rather not cause spot where max77620 breaks. Besides the changes in
> > > max77620 driver are trivial. Please let me know if this is not Ok.
> > >
> > > Reason why I submit this patch now - even though my driver which would
> > > use level active type setting with regmap-irq is not yet ready for
> > > being submited - is that I'd like to minimize amount of existing drivers
> > > we need to patch. And if we add level active irq support like this then
> > > we must patch all existing drivers using type setting with regmap-irq.
> > > So doing this now when only max77620 uses type setting may be easier
> > > than postponing this to the future.
> > >
> > > And finally - I don't have max77620 so I have only done _wery_ limited
> > > testing. So I would really appreciate if someone had time to review this
> > > thoroughly - and even happier if someone had possibility to try this out
> > > with the max77620.
> > >
> >
> > [snip]
> >
> > > diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> > > index a367d59c301d..91c431ad98c3 100644
> > > --- a/include/linux/regmap.h
> > > +++ b/include/linux/regmap.h
> > > @@ -1098,6 +1098,9 @@ int regmap_fields_update_bits_base(struct regmap_field *field, unsigned int id,
> > > * @type_reg_offset: Offset register for the irq type setting.
> > > * @type_rising_mask: Mask bit to configure RISING type irq.
> > > * @type_falling_mask: Mask bit to configure FALLING type irq.
> > > + * @type_level_low_mask: Mask bit to configure LEVEL_LOW type irq.
> > > + * @type_level_high_mask: Mask bit to configure LEVEL_HIGH type irq.
> > > + * @types_supported: logical OR of IRQ_TYPE_* flags indicating supported types.
> >
> > While the existing interface is awful, you don't make it better.
> >
> > .types_supported value is always derived from non-zero .type_*_mask values, so
> > it is simply not needed, as well as the whole change to gpio-max77620.c driver.
>
> Right. I didn't consider the "type_inverted" flag in the struct
> regmap_irq_chip. I thought that "mask" is actually a register value -
> which could be zero for some HWs. Thanks for pointing that out. It will
> really make "types_supported" useless.
After second check - type_inverted won't help if we have case where
both the 'all bits set' and 'all bits zeroed' are valid type specifiers.
We *could* have HW register specified as:
IRQ Trigger control reg:
bits [7:2] reserved - don't touch, undocumented magic debug stuff
bits [1:0] IRQ trigger type:
00 => Rising Edge
01 => Falling Edge
10 => Level Low
11 => Level High.
For such setup we would have:
type_rising_mask = 0x0,
type_falling_mask = 0x1,
type_level_low_mask = 0x2,
type_level_high_mask = 0x3,
- and I see no way of tellng the type_rising_mask is valid.
I admit this is actually not pretty. We don't really give *mask* here
but we give the actual configuration value - and actually, for example
transitioning from falling to low would make HW to wrongly go to
type_level_high - if the values here were really regarded as *masks*.
But when we look at the implementation, we are treating these masks as
actual values - so code does the correct thing and zeroes the whole
'type area':
d->type_buf[reg] &= ~(irq_data->type_falling_mask |
irq_data->type_rising_mask |
irq_data->type_level_low_mask |
irq_data->type_level_high_mask);
before applying the desired mask (value):
switch (type) {
case IRQ_TYPE_EDGE_FALLING:
d->type_buf[reg] |= irq_data->type_falling_mask;
break;
...
So I would still go by adding the types_supported field to advertice
also those types which are set using value '0'. I would additionally
consider renaming the type_*_mask to type_*_val. What's your take on
this?
>
> So please just drop this patch for now. There seems to be no need to
> touch the existing regmap-irq users after all so I can submit this patch
> together with the driver which needs to support the level active IRQs.
>
So, it seems we still need to patch the gpio-max77620.c, right? So I
guess I could try submitting the next version prior the rest of the
driver.
> >
> > > */
> > > struct regmap_irq {
> > > unsigned int reg_offset;
> > > @@ -1105,6 +1108,9 @@ struct regmap_irq {
> > > unsigned int type_reg_offset;
> > > unsigned int type_rising_mask;
> > > unsigned int type_falling_mask;
> > > + unsigned int type_level_low_mask;
> > > + unsigned int type_level_high_mask;
> > > + unsigned int types_supported;
> > > };
> > >
> > > #define REGMAP_IRQ_REG(_irq, _off, _mask) \
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~