Discussion:
[PATCH v2 0/6] Add support for BCM7271 style interrupt controller
Doug Berger
2017-07-19 19:07:28 UTC
Permalink
This patch set extends the functionality of the irq-brcmstb-l2 interrupt
controller driver to cover a hardware variant first introduced in the
BCM7271 SoC. The main difference between this variant and the block
found in earlier brcmstb SoCs is that this variant only supports level
sensitive interrupts and therefore does not latch the interrupt state
based on edges. Since there is no longer a need to ack interrupts with
a register write to clear the latch the register map has been changed.

Therefore the change to add support for the new hardware block is to
abstract the register accesses to accommodate different maps and to
identify the block with a new device-tree compatible string.

I also took the opportunity to make some small efficiency enhancements
to the driver. One of these was to make use of the slightly more
efficient irq_mask_ack method. However, I discovered that the defined
irq_gc_mask_disable_reg_and_ack() generic irq function was insufficient
for my needs. The first three commits of this set are intended to be a
correction of the existing generic irq implementation to provide a
function that can be used by interrupt controller drivers for their
irq_mask_ack method when disable/enable registers are used for masking
and interrupts are acknowledged by setting a bit in an ack register.

I believe these first three commits should be added to the irq/core
repository and possibly stable branches. The remaining commits should be
added to the irqchip/core repository but I have included the complete
set here for improved context since the irqchip patches are dependent on
the irq patches. This entire set is therefore based on the irq/core
master branch. Please let me know if you would like a different
packaging.

If the changes to genirq are not acceptable I can implement the
irq_mask_ask method locally in the irq-brcmstb-l2 driver and submit
that on its own.

Changes in v2:

- removed unused permutations of irq_mask_ack methods
- added Reviewed-by and Acked-by responses from first submission

Doug Berger (5):
genirq: generic chip: add irq_gc_mask_disable_and_ack_set()
genirq: generic chip: remove irq_gc_mask_disable_reg_and_ack()
irqchip: brcmstb-l2: Remove some processing from the handler
irqchip: brcmstb-l2: Abstract register accesses
irqchip: brcmstb-l2: Add support for the BCM7271 L2 controller

Florian Fainelli (1):
irqchip/tango: Use irq_gc_mask_disable_and_ack_set

.../bindings/interrupt-controller/brcm,l2-intc.txt | 3 +-
drivers/irqchip/irq-brcmstb-l2.c | 145 ++++++++++++++-------
drivers/irqchip/irq-tango.c | 2 +-
include/linux/irq.h | 2 +-
kernel/irq/generic-chip.c | 15 ++-
5 files changed, 114 insertions(+), 53 deletions(-)
--
2.13.0
Doug Berger
2017-07-19 19:07:29 UTC
Permalink
The irq_gc_mask_disable_reg_and_ack() function name implies that it
provides the combined functions of irq_gc_mask_disable_reg() and
irq_gc_ack(). However, the implementation does not actually do
that since it writes the mask instead of the disable register. It
also does not maintain the mask cache which makes it inappropriate
to use with other masking functions.

In addition, commit 659fb32d1b67 ("genirq: replace irq_gc_ack() with
{set,clr}_bit variants (fwd)") effectively renamed irq_gc_ack() to
irq_gc_ack_set_bit() so this function probably should have also been
renamed at that time.

The generic chip code currently provides three functions for use
with the irq_mask member of the irq_chip structure and two functions
for use with the irq_ack member of the irq_chip structure. These
functions could be combined into six functions for use with the
irq_mask_ack member of the irq_chip structure. However, since only
one of the combinations is currently used, only the function
irq_gc_mask_disable_and_ack_set() is added by this commit.

The '_reg' and '_bit' portions of the base function name were left
out of the new combined function name in an attempt to keep the
function name length manageable with the 80 character source code
line length while still allowing the distinct aspects of each
combination to be captured by the name.

If other combinations are desired in the future please add them to
the irq generic chip library at that time.

Signed-off-by: Doug Berger <***@gmail.com>
---
include/linux/irq.h | 1 +
kernel/irq/generic-chip.c | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 00db35b61e9e..5b27f65c47d0 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1003,6 +1003,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d);
void irq_gc_ack_set_bit(struct irq_data *d);
void irq_gc_ack_clr_bit(struct irq_data *d);
void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
+void irq_gc_mask_disable_and_ack_set(struct irq_data *d);
void irq_gc_eoi(struct irq_data *d);
int irq_gc_set_wake(struct irq_data *d, unsigned int on);

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index f7086b78ad6e..7f61b6e9f5ca 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -151,6 +151,31 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
}

/**
+ * irq_gc_mask_disable_and_ack_set - Mask and ack pending interrupt
+ * @d: irq_data
+ *
+ * This generic implementation of the irq_mask_ack method is for chips
+ * with separate enable/disable registers instead of a single mask
+ * register and where a pending interrupt is acknowledged by setting a
+ * bit.
+ *
+ * Note: This is the only permutation currently used. Similar generic
+ * functions should be added here if other permutations are required.
+ */
+void irq_gc_mask_disable_and_ack_set(struct irq_data *d)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ u32 mask = d->mask;
+
+ irq_gc_lock(gc);
+ irq_reg_writel(gc, mask, ct->regs.disable);
+ *ct->mask_cache &= ~mask;
+ irq_reg_writel(gc, mask, ct->regs.ack);
+ irq_gc_unlock(gc);
+}
+
+/**
* irq_gc_eoi - EOI interrupt
* @d: irq_data
*/
--
2.13.0
Doug Berger
2017-07-19 19:07:30 UTC
Permalink
From: Florian Fainelli <***@gmail.com>

The only usage of the irq_gc_mask_disable_reg_and_ack() function
is by the Tango irqchip driver. This usage is replaced by the
irq_gc_mask_disable_and_ack_set() function since it provides the
intended functionality.

Fixes: 4bba66899ac6 ("irqchip/tango: Add support for Sigma Designs SMP86xx/SMP87xx interrupt controller")
Signed-off-by: Florian Fainelli <***@gmail.com>
Acked-by: Mans Rullgard <***@mansr.com>
Signed-off-by: Doug Berger <***@gmail.com>
---
drivers/irqchip/irq-tango.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-tango.c b/drivers/irqchip/irq-tango.c
index bdbb5c0ff7fe..0c085303a583 100644
--- a/drivers/irqchip/irq-tango.c
+++ b/drivers/irqchip/irq-tango.c
@@ -141,7 +141,7 @@ static void __init tangox_irq_init_chip(struct irq_chip_generic *gc,
for (i = 0; i < 2; i++) {
ct[i].chip.irq_ack = irq_gc_ack_set_bit;
ct[i].chip.irq_mask = irq_gc_mask_disable_reg;
- ct[i].chip.irq_mask_ack = irq_gc_mask_disable_reg_and_ack;
+ ct[i].chip.irq_mask_ack = irq_gc_mask_disable_and_ack_set;
ct[i].chip.irq_unmask = irq_gc_unmask_enable_reg;
ct[i].chip.irq_set_type = tangox_irq_set_type;
ct[i].chip.name = gc->domain->name;
--
2.13.0
Marc Gonzalez
2017-07-24 16:40:31 UTC
Permalink
[ Trimming CC list ]
Post by Doug Berger
The only usage of the irq_gc_mask_disable_reg_and_ack() function
is by the Tango irqchip driver. This usage is replaced by the
irq_gc_mask_disable_and_ack_set() function since it provides the
intended functionality.
Fixes: 4bba66899ac6 ("irqchip/tango: Add support for Sigma Designs SMP86xx/SMP87xx interrupt controller")
---
drivers/irqchip/irq-tango.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-tango.c b/drivers/irqchip/irq-tango.c
index bdbb5c0ff7fe..0c085303a583 100644
--- a/drivers/irqchip/irq-tango.c
+++ b/drivers/irqchip/irq-tango.c
@@ -141,7 +141,7 @@ static void __init tangox_irq_init_chip(struct irq_chip_generic *gc,
for (i = 0; i < 2; i++) {
ct[i].chip.irq_ack = irq_gc_ack_set_bit;
ct[i].chip.irq_mask = irq_gc_mask_disable_reg;
- ct[i].chip.irq_mask_ack = irq_gc_mask_disable_reg_and_ack;
+ ct[i].chip.irq_mask_ack = irq_gc_mask_disable_and_ack_set;
ct[i].chip.irq_unmask = irq_gc_unmask_enable_reg;
ct[i].chip.irq_set_type = tangox_irq_set_type;
ct[i].chip.name = gc->domain->name;
(I had a look only at patches 1 and 2)

irq_gc_mask_disable_reg() =
irq_reg_writel(gc, mask, ct->regs.disable);
*ct->mask_cache &= ~mask;

irq_gc_ack_set_bit() =
irq_reg_writel(gc, mask, ct->regs.ack);


irq_gc_mask_disable_reg_and_ack() =
irq_reg_writel(gc, mask, ct->regs.mask); // regs.mask not defined for tango driver
irq_reg_writel(gc, mask, ct->regs.ack);

It will try to write at offset 0, which is a read-only register
so no disaster, but interrupt is not disabled. I wonder why
everything doesn't blow up...

irq_gc_mask_disable_and_ack_set() =
irq_reg_writel(gc, mask, ct->regs.disable);
*ct->mask_cache &= ~mask;
irq_reg_writel(gc, mask, ct->regs.ack);


Your change makes sense.

But note that mask_ack_irq() is defined as:

static inline void mask_ack_irq(struct irq_desc *desc)
{
if (desc->irq_data.chip->irq_mask_ack) {
desc->irq_data.chip->irq_mask_ack(&desc->irq_data);
irq_state_set_masked(desc);
} else {
mask_irq(desc);
if (desc->irq_data.chip->irq_ack)
desc->irq_data.chip->irq_ack(&desc->irq_data);
}
}

So IIUC, if we don't define a irq_mask_ack() callback,
it will just call irq_mask() and irq_ack() which will
do the right thing.

So an alternative is simply to delete the assignment
of ct[i].chip.irq_mask_ack. What do you think?

diff --git a/drivers/irqchip/irq-tango.c b/drivers/irqchip/irq-tango.c
index bdbb5c0ff7fe..825085cdab99 100644
--- a/drivers/irqchip/irq-tango.c
+++ b/drivers/irqchip/irq-tango.c
@@ -141,7 +141,6 @@ static void __init tangox_irq_init_chip(struct irq_chip_generic *gc,
for (i = 0; i < 2; i++) {
ct[i].chip.irq_ack = irq_gc_ack_set_bit;
ct[i].chip.irq_mask = irq_gc_mask_disable_reg;
- ct[i].chip.irq_mask_ack = irq_gc_mask_disable_reg_and_ack;
ct[i].chip.irq_unmask = irq_gc_unmask_enable_reg;
ct[i].chip.irq_set_type = tangox_irq_set_type;
ct[i].chip.name = gc->domain->name;


One thing I'm not sure about is why we need to ack for level
interrupts... We should be able to get away with defining ack()
only for edge interrupts... I'm confused. If I try to do that,
I get:

[ 1.430547] irq 20: nobody cared (try booting with the "irqpoll" option)
[ 1.437283] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0-rc1 #148
[ 1.443754] Hardware name: Sigma Tango DT
[ 1.447812] [<c010e890>] (unwind_backtrace) from [<c010b01c>] (show_stack+0x10/0x14)
[ 1.455598] [<c010b01c>] (show_stack) from [<c04a3fc4>] (dump_stack+0x84/0x98)
[ 1.462864] [<c04a3fc4>] (dump_stack) from [<c0159d80>] (__report_bad_irq+0x28/0xcc)
[ 1.470650] [<c0159d80>] (__report_bad_irq) from [<c015a188>] (note_interrupt+0x28c/0x2dc)
[ 1.478959] [<c015a188>] (note_interrupt) from [<c0157688>] (handle_irq_event_percpu+0x4c/0x58)
[ 1.487702] [<c0157688>] (handle_irq_event_percpu) from [<c01576cc>] (handle_irq_event+0x38/0x5c)
[ 1.496621] [<c01576cc>] (handle_irq_event) from [<c015aa84>] (handle_level_irq+0xa8/0x124)
[ 1.505017] [<c015aa84>] (handle_level_irq) from [<c01568bc>] (generic_handle_irq+0x24/0x34)
[ 1.513501] [<c01568bc>] (generic_handle_irq) from [<c02f9dc0>] (tangox_dispatch_irqs+0x4c/0x58)
[ 1.522333] [<c02f9dc0>] (tangox_dispatch_irqs) from [<c02f9e18>] (tangox_irq_handler+0x4c/0xb0)
[ 1.531163] [<c02f9e18>] (tangox_irq_handler) from [<c01568bc>] (generic_handle_irq+0x24/0x34)
[ 1.539819] [<c01568bc>] (generic_handle_irq) from [<c0156df4>] (__handle_domain_irq+0x5c/0xb4)
[ 1.548562] [<c0156df4>] (__handle_domain_irq) from [<c0101450>] (gic_handle_irq+0x48/0x8c)
[ 1.556956] [<c0101450>] (gic_handle_irq) from [<c010bb4c>] (__irq_svc+0x6c/0xa8)


Regards.
Doug Berger
2017-07-24 17:54:33 UTC
Permalink
Post by Marc Gonzalez
[ Trimming CC list ]
Post by Doug Berger
The only usage of the irq_gc_mask_disable_reg_and_ack() function
is by the Tango irqchip driver. This usage is replaced by the
irq_gc_mask_disable_and_ack_set() function since it provides the
intended functionality.
Fixes: 4bba66899ac6 ("irqchip/tango: Add support for Sigma Designs SMP86xx/SMP87xx interrupt controller")
---
drivers/irqchip/irq-tango.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-tango.c b/drivers/irqchip/irq-tango.c
index bdbb5c0ff7fe..0c085303a583 100644
--- a/drivers/irqchip/irq-tango.c
+++ b/drivers/irqchip/irq-tango.c
@@ -141,7 +141,7 @@ static void __init tangox_irq_init_chip(struct irq_chip_generic *gc,
for (i = 0; i < 2; i++) {
ct[i].chip.irq_ack = irq_gc_ack_set_bit;
ct[i].chip.irq_mask = irq_gc_mask_disable_reg;
- ct[i].chip.irq_mask_ack = irq_gc_mask_disable_reg_and_ack;
+ ct[i].chip.irq_mask_ack = irq_gc_mask_disable_and_ack_set;
ct[i].chip.irq_unmask = irq_gc_unmask_enable_reg;
ct[i].chip.irq_set_type = tangox_irq_set_type;
ct[i].chip.name = gc->domain->name;
(I had a look only at patches 1 and 2)
irq_gc_mask_disable_reg() =
irq_reg_writel(gc, mask, ct->regs.disable);
*ct->mask_cache &= ~mask;
irq_gc_ack_set_bit() =
irq_reg_writel(gc, mask, ct->regs.ack);
irq_gc_mask_disable_reg_and_ack() =
irq_reg_writel(gc, mask, ct->regs.mask); // regs.mask not defined for tango driver
irq_reg_writel(gc, mask, ct->regs.ack);
It will try to write at offset 0, which is a read-only register
so no disaster, but interrupt is not disabled. I wonder why
everything doesn't blow up...
irq_gc_mask_disable_and_ack_set() =
irq_reg_writel(gc, mask, ct->regs.disable);
*ct->mask_cache &= ~mask;
irq_reg_writel(gc, mask, ct->regs.ack);
Your change makes sense.
static inline void mask_ack_irq(struct irq_desc *desc)
{
if (desc->irq_data.chip->irq_mask_ack) {
desc->irq_data.chip->irq_mask_ack(&desc->irq_data);
irq_state_set_masked(desc);
} else {
mask_irq(desc);
if (desc->irq_data.chip->irq_ack)
desc->irq_data.chip->irq_ack(&desc->irq_data);
}
}
So IIUC, if we don't define a irq_mask_ack() callback,
it will just call irq_mask() and irq_ack() which will
do the right thing.
So an alternative is simply to delete the assignment
of ct[i].chip.irq_mask_ack. What do you think?
Yes, you understand correctly. The irq_mask_ack method is entirely
optional and I assume that is why this issue went undetected for so
long; however, it is slightly more efficient to combine the functions
(even if the ack is unnecessary) which is why I chose to do so for my
changes to the irqchip-brcmstb-l2 driver where I first discovered this
issue. How much value the improved efficiency has is certainly
debatable, but interrupt handling is one area where people might care
about such a small difference. As the irqchip-tango driver maintainer
you are welcome to decide whether or not the irq_mask_ack method makes
sense to you.

I wanted to use the method for the irqchip-brcmstb-l2 driver which is
based on the irq generic chip implementation. I discovered that the
irq_gc_mask_disable_reg_and_ack() function was not the function I needed
despite the name and that it appeared not to be the function the
irqchip-tango driver needed as well. My desire here was to correct that
error and provide a standard set of generic implementations of
irq_mask_ack so that other drivers could avoid a similar mistake in an
attempt to provide a service to the community. That was pared down at
Thomas Gleixner's request to correcting just the one implementation that
I wanted to use.
Post by Marc Gonzalez
diff --git a/drivers/irqchip/irq-tango.c b/drivers/irqchip/irq-tango.c
index bdbb5c0ff7fe..825085cdab99 100644
--- a/drivers/irqchip/irq-tango.c
+++ b/drivers/irqchip/irq-tango.c
@@ -141,7 +141,6 @@ static void __init tangox_irq_init_chip(struct irq_chip_generic *gc,
for (i = 0; i < 2; i++) {
ct[i].chip.irq_ack = irq_gc_ack_set_bit;
ct[i].chip.irq_mask = irq_gc_mask_disable_reg;
- ct[i].chip.irq_mask_ack = irq_gc_mask_disable_reg_and_ack;
ct[i].chip.irq_unmask = irq_gc_unmask_enable_reg;
ct[i].chip.irq_set_type = tangox_irq_set_type;
ct[i].chip.name = gc->domain->name;
One thing I'm not sure about is why we need to ack for level
interrupts... We should be able to get away with defining ack()
only for edge interrupts... I'm confused. If I try to do that,
[ 1.430547] irq 20: nobody cared (try booting with the "irqpoll" option)
[ 1.437283] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.13.0-rc1 #148
[ 1.443754] Hardware name: Sigma Tango DT
[ 1.447812] [<c010e890>] (unwind_backtrace) from [<c010b01c>] (show_stack+0x10/0x14)
[ 1.455598] [<c010b01c>] (show_stack) from [<c04a3fc4>] (dump_stack+0x84/0x98)
[ 1.462864] [<c04a3fc4>] (dump_stack) from [<c0159d80>] (__report_bad_irq+0x28/0xcc)
[ 1.470650] [<c0159d80>] (__report_bad_irq) from [<c015a188>] (note_interrupt+0x28c/0x2dc)
[ 1.478959] [<c015a188>] (note_interrupt) from [<c0157688>] (handle_irq_event_percpu+0x4c/0x58)
[ 1.487702] [<c0157688>] (handle_irq_event_percpu) from [<c01576cc>] (handle_irq_event+0x38/0x5c)
[ 1.496621] [<c01576cc>] (handle_irq_event) from [<c015aa84>] (handle_level_irq+0xa8/0x124)
[ 1.505017] [<c015aa84>] (handle_level_irq) from [<c01568bc>] (generic_handle_irq+0x24/0x34)
[ 1.513501] [<c01568bc>] (generic_handle_irq) from [<c02f9dc0>] (tangox_dispatch_irqs+0x4c/0x58)
[ 1.522333] [<c02f9dc0>] (tangox_dispatch_irqs) from [<c02f9e18>] (tangox_irq_handler+0x4c/0xb0)
[ 1.531163] [<c02f9e18>] (tangox_irq_handler) from [<c01568bc>] (generic_handle_irq+0x24/0x34)
[ 1.539819] [<c01568bc>] (generic_handle_irq) from [<c0156df4>] (__handle_domain_irq+0x5c/0xb4)
[ 1.548562] [<c0156df4>] (__handle_domain_irq) from [<c0101450>] (gic_handle_irq+0x48/0x8c)
[ 1.556956] [<c0101450>] (gic_handle_irq) from [<c010bb4c>] (__irq_svc+0x6c/0xa8)
Whether an interrupt requires acknowledgement is a function of the
design of the interrupt controller hardware. As you say, in principle
it should not be necessary to acknowledge a level sensitive interrupt.
In fact, that is the fundamental difference between the new hardware
implementation of the brcmstb-l2 interrupt controller in the BCM7271
device and previous versions (i.e. it only supports level interrupts and
therefore does not require acknowledgement). However, it is not unusual
for the interrupt controller designers to latch the state of triggered
interrupts especially on controllers that can be configured for
different types of interrupt triggers (e.g. rising edge, falling edge,
active low, ...) and so it becomes necessary to acknowledge level
interrupts on such controllers to clear that latched state.

I don't know the requirements of the tango interrupt controller hardware.
Post by Marc Gonzalez
Regards.
Thanks,
Doug
Marc Gonzalez
2017-07-25 13:08:49 UTC
Permalink
irq_gc_mask_disable_reg_and_ack() is not equivalent to
irq_gc_mask_disable_reg() and irq_gc_ack_set_bit().

Leave the irq_mask_ack callback undefined, and let the irqchip
framework use irq_mask and irq_ack instead.

Reported-by: Doug Berger <***@gmail.com>
Fixes: 4bba66899ac6 ("irqchip/tango: Add support for Sigma Designs SMP86xx/SMP87xx interrupt controller")
Signed-off-by: Marc Gonzalez <***@sigmadesigns.com>
Cc: ***@vger.kernel.org
---
As discussed previously, it is acceptable for tango to rely
on mask_ack_irq() doing the right thing(TM).
---
drivers/irqchip/irq-tango.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-tango.c b/drivers/irqchip/irq-tango.c
index bdbb5c0ff7fe..825085cdab99 100644
--- a/drivers/irqchip/irq-tango.c
+++ b/drivers/irqchip/irq-tango.c
@@ -141,7 +141,6 @@ static void __init tangox_irq_init_chip(struct irq_chip_generic *gc,
for (i = 0; i < 2; i++) {
ct[i].chip.irq_ack = irq_gc_ack_set_bit;
ct[i].chip.irq_mask = irq_gc_mask_disable_reg;
- ct[i].chip.irq_mask_ack = irq_gc_mask_disable_reg_and_ack;
ct[i].chip.irq_unmask = irq_gc_unmask_enable_reg;
ct[i].chip.irq_set_type = tangox_irq_set_type;
ct[i].chip.name = gc->domain->name;
--
2.11.0
Måns Rullgård
2017-07-25 13:16:36 UTC
Permalink
Post by Marc Gonzalez
irq_gc_mask_disable_reg_and_ack() is not equivalent to
irq_gc_mask_disable_reg() and irq_gc_ack_set_bit().
Leave the irq_mask_ack callback undefined, and let the irqchip
framework use irq_mask and irq_ack instead.
Fixes: 4bba66899ac6 ("irqchip/tango: Add support for Sigma Designs SMP86xx/SMP87xx interrupt controller")
---
As discussed previously, it is acceptable for tango to rely
on mask_ack_irq() doing the right thing(TM).
---
drivers/irqchip/irq-tango.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/irqchip/irq-tango.c b/drivers/irqchip/irq-tango.c
index bdbb5c0ff7fe..825085cdab99 100644
--- a/drivers/irqchip/irq-tango.c
+++ b/drivers/irqchip/irq-tango.c
@@ -141,7 +141,6 @@ static void __init tangox_irq_init_chip(struct irq_chip_generic *gc,
for (i = 0; i < 2; i++) {
ct[i].chip.irq_ack = irq_gc_ack_set_bit;
ct[i].chip.irq_mask = irq_gc_mask_disable_reg;
- ct[i].chip.irq_mask_ack = irq_gc_mask_disable_reg_and_ack;
ct[i].chip.irq_unmask = irq_gc_unmask_enable_reg;
ct[i].chip.irq_set_type = tangox_irq_set_type;
ct[i].chip.name = gc->domain->name;
--
What happened to the patch adding the proper combined function?
--
Måns Rullgård
Marc Gonzalez
2017-07-25 13:26:40 UTC
Permalink
Post by Måns Rullgård
What happened to the patch adding the proper combined function?
It appears you're not CCed on v2.

https://patchwork.kernel.org/patch/9859799/
Post by Måns Rullgård
Yes, you understand correctly. The irq_mask_ack method is entirely
optional and I assume that is why this issue went undetected for so
long; however, it is slightly more efficient to combine the functions
(even if the ack is unnecessary) which is why I chose to do so for my
changes to the irqchip-brcmstb-l2 driver where I first discovered this
issue. How much value the improved efficiency has is certainly
debatable, but interrupt handling is one area where people might care
about such a small difference. As the irqchip-tango driver maintainer
you are welcome to decide whether or not the irq_mask_ack method makes
sense to you.
My preference goes to leaving the irq_mask_ack callback undefined,
and let the irqchip framework use irq_mask and irq_ack instead.

Regards.
Måns Rullgård
2017-07-25 13:29:20 UTC
Permalink
Post by Marc Gonzalez
Post by Måns Rullgård
What happened to the patch adding the proper combined function?
It appears you're not CCed on v2.
https://patchwork.kernel.org/patch/9859799/
Post by Måns Rullgård
Yes, you understand correctly. The irq_mask_ack method is entirely
optional and I assume that is why this issue went undetected for so
long; however, it is slightly more efficient to combine the functions
(even if the ack is unnecessary) which is why I chose to do so for my
changes to the irqchip-brcmstb-l2 driver where I first discovered this
issue. How much value the improved efficiency has is certainly
debatable, but interrupt handling is one area where people might care
about such a small difference. As the irqchip-tango driver maintainer
you are welcome to decide whether or not the irq_mask_ack method makes
sense to you.
My preference goes to leaving the irq_mask_ack callback undefined,
and let the irqchip framework use irq_mask and irq_ack instead.
Why would you prefer the less efficient way?
--
Måns Rullgård
Marc Gonzalez
2017-07-25 14:15:02 UTC
Permalink
Post by Marc Gonzalez
irq_gc_mask_disable_reg_and_ack() is not equivalent to
irq_gc_mask_disable_reg() and irq_gc_ack_set_bit().
Leave the irq_mask_ack callback undefined, and let the irqchip
framework use irq_mask and irq_ack instead.
Fixes: 4bba66899ac6 ("irqchip/tango: Add support for Sigma Designs SMP86xx/SMP87xx interrupt controller")
FWIW, the lockup reported in the thread below disappears
once the patch is applied.

https://lkml.org/lkml/2016/10/21/709

Regards.

Doug Berger
2017-07-19 19:07:32 UTC
Permalink
Saving the generic chip pointer in the brcmstb_l2_intc_data prevents
the need to call irq_get_domain_generic_chip(). Also don't need to
save parent_irq and base there since local variables in the
brcmstb_l2_intc_of_init() function are just as good.

The handle_edge_irq flow or chained_irq_enter takes care of the
acknowledgement of the interrupt so it is redundant to clear it in
brcmstb_l2_intc_irq_handle().

irq_linear_revmap() is a fast path equivalent of irq_find_mapping()
that is appropriate to use for domain controllers of this type.

Defining irq_mask_ack is slightly more efficient than just
implementing irq_mask and irq_ack seperately.

Reviewed-by: Florian Fainelli <***@gmail.com>
Signed-off-by: Doug Berger <***@gmail.com>
---
drivers/irqchip/irq-brcmstb-l2.c | 46 +++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/irqchip/irq-brcmstb-l2.c b/drivers/irqchip/irq-brcmstb-l2.c
index bddf169c4b37..977ae55d47d4 100644
--- a/drivers/irqchip/irq-brcmstb-l2.c
+++ b/drivers/irqchip/irq-brcmstb-l2.c
@@ -1,7 +1,7 @@
/*
* Generic Broadcom Set Top Box Level 2 Interrupt controller driver
*
- * Copyright (C) 2014 Broadcom Corporation
+ * Copyright (C) 2014-2017 Broadcom
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -41,9 +41,8 @@

/* L2 intc private data structure */
struct brcmstb_l2_intc_data {
- int parent_irq;
- void __iomem *base;
struct irq_domain *domain;
+ struct irq_chip_generic *gc;
bool can_wake;
u32 saved_mask; /* for suspend/resume */
};
@@ -51,15 +50,14 @@ struct brcmstb_l2_intc_data {
static void brcmstb_l2_intc_irq_handle(struct irq_desc *desc)
{
struct brcmstb_l2_intc_data *b = irq_desc_get_handler_data(desc);
- struct irq_chip_generic *gc = irq_get_domain_generic_chip(b->domain, 0);
struct irq_chip *chip = irq_desc_get_chip(desc);
unsigned int irq;
u32 status;

chained_irq_enter(chip, desc);

- status = irq_reg_readl(gc, CPU_STATUS) &
- ~(irq_reg_readl(gc, CPU_MASK_STATUS));
+ status = irq_reg_readl(b->gc, CPU_STATUS) &
+ ~(irq_reg_readl(b->gc, CPU_MASK_STATUS));

if (status == 0) {
raw_spin_lock(&desc->lock);
@@ -70,10 +68,8 @@ static void brcmstb_l2_intc_irq_handle(struct irq_desc *desc)

do {
irq = ffs(status) - 1;
- /* ack at our level */
- irq_reg_writel(gc, 1 << irq, CPU_CLEAR);
status &= ~(1 << irq);
- generic_handle_irq(irq_find_mapping(b->domain, irq));
+ generic_handle_irq(irq_linear_revmap(b->domain, irq));
} while (status);
out:
chained_irq_exit(chip, desc);
@@ -116,32 +112,33 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
{
unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
struct brcmstb_l2_intc_data *data;
- struct irq_chip_generic *gc;
struct irq_chip_type *ct;
int ret;
unsigned int flags;
+ int parent_irq;
+ void __iomem *base;

data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;

- data->base = of_iomap(np, 0);
- if (!data->base) {
+ base = of_iomap(np, 0);
+ if (!base) {
pr_err("failed to remap intc L2 registers\n");
ret = -ENOMEM;
goto out_free;
}

/* Disable all interrupts by default */
- writel(0xffffffff, data->base + CPU_MASK_SET);
+ writel(0xffffffff, base + CPU_MASK_SET);

/* Wakeup interrupts may be retained from S5 (cold boot) */
data->can_wake = of_property_read_bool(np, "brcm,irq-can-wake");
if (!data->can_wake)
- writel(0xffffffff, data->base + CPU_CLEAR);
+ writel(0xffffffff, base + CPU_CLEAR);

- data->parent_irq = irq_of_parse_and_map(np, 0);
- if (!data->parent_irq) {
+ parent_irq = irq_of_parse_and_map(np, 0);
+ if (!parent_irq) {
pr_err("failed to find parent interrupt\n");
ret = -EINVAL;
goto out_unmap;
@@ -170,18 +167,19 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
}

/* Set the IRQ chaining logic */
- irq_set_chained_handler_and_data(data->parent_irq,
+ irq_set_chained_handler_and_data(parent_irq,
brcmstb_l2_intc_irq_handle, data);

- gc = irq_get_domain_generic_chip(data->domain, 0);
- gc->reg_base = data->base;
- gc->private = data;
- ct = gc->chip_types;
+ data->gc = irq_get_domain_generic_chip(data->domain, 0);
+ data->gc->reg_base = base;
+ data->gc->private = data;
+ ct = data->gc->chip_types;

ct->chip.irq_ack = irq_gc_ack_set_bit;
ct->regs.ack = CPU_CLEAR;

ct->chip.irq_mask = irq_gc_mask_disable_reg;
+ ct->chip.irq_mask_ack = irq_gc_mask_disable_and_ack_set;
ct->regs.disable = CPU_MASK_SET;

ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
@@ -194,19 +192,19 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
/* This IRQ chip can wake the system, set all child interrupts
* in wake_enabled mask
*/
- gc->wake_enabled = 0xffffffff;
+ data->gc->wake_enabled = 0xffffffff;
ct->chip.irq_set_wake = irq_gc_set_wake;
}

pr_info("registered L2 intc (mem: 0x%p, parent irq: %d)\n",
- data->base, data->parent_irq);
+ base, parent_irq);

return 0;

out_free_domain:
irq_domain_remove(data->domain);
out_unmap:
- iounmap(data->base);
+ iounmap(base);
out_free:
kfree(data);
return ret;
--
2.13.0
Doug Berger
2017-07-19 19:07:31 UTC
Permalink
Any usage of the irq_gc_mask_disable_reg_and_ack() function has
been replaced with the desired functionality.

The incorrect and ambiguously named function is removed here to
prevent accidental misuse.

Signed-off-by: Doug Berger <***@gmail.com>
---
include/linux/irq.h | 1 -
kernel/irq/generic-chip.c | 16 ----------------
2 files changed, 17 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 5b27f65c47d0..c73e2eca4abd 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1002,7 +1002,6 @@ void irq_gc_mask_clr_bit(struct irq_data *d);
void irq_gc_unmask_enable_reg(struct irq_data *d);
void irq_gc_ack_set_bit(struct irq_data *d);
void irq_gc_ack_clr_bit(struct irq_data *d);
-void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
void irq_gc_mask_disable_and_ack_set(struct irq_data *d);
void irq_gc_eoi(struct irq_data *d);
int irq_gc_set_wake(struct irq_data *d, unsigned int on);
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 7f61b6e9f5ca..49ea3347719e 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -135,22 +135,6 @@ void irq_gc_ack_clr_bit(struct irq_data *d)
}

/**
- * irq_gc_mask_disable_reg_and_ack - Mask and ack pending interrupt
- * @d: irq_data
- */
-void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
-{
- struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = d->mask;
-
- irq_gc_lock(gc);
- irq_reg_writel(gc, mask, ct->regs.mask);
- irq_reg_writel(gc, mask, ct->regs.ack);
- irq_gc_unlock(gc);
-}
-
-/**
* irq_gc_mask_disable_and_ack_set - Mask and ack pending interrupt
* @d: irq_data
*
--
2.13.0
Doug Berger
2017-07-19 19:07:34 UTC
Permalink
Add the initialization of the generic irq chip for the BCM7271 L2
interrupt controller. This controller only supports level
interrupts and uses the "brcm,bcm7271-l2-intc" compatibility
string.

Reviewed-by: Florian Fainelli <***@gmail.com>
Acked-by: Rob Herring <***@kernel.org>
Signed-off-by: Doug Berger <***@gmail.com>
---
.../bindings/interrupt-controller/brcm,l2-intc.txt | 3 +-
drivers/irqchip/irq-brcmstb-l2.c | 86 ++++++++++++++++------
2 files changed, 66 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt
index 448273a30a11..36df06c5c567 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,l2-intc.txt
@@ -2,7 +2,8 @@ Broadcom Generic Level 2 Interrupt Controller

Required properties:

-- compatible: should be "brcm,l2-intc"
+- compatible: should be "brcm,l2-intc" for latched interrupt controllers
+ should be "brcm,bcm7271-l2-intc" for level interrupt controllers
- reg: specifies the base physical address and size of the registers
- interrupt-controller: identifies the node as an interrupt controller
- #interrupt-cells: specifies the number of cells needed to encode an
diff --git a/drivers/irqchip/irq-brcmstb-l2.c b/drivers/irqchip/irq-brcmstb-l2.c
index ce3850530e2b..f77e6c9530dc 100644
--- a/drivers/irqchip/irq-brcmstb-l2.c
+++ b/drivers/irqchip/irq-brcmstb-l2.c
@@ -31,13 +31,34 @@
#include <linux/irqchip.h>
#include <linux/irqchip/chained_irq.h>

-/* Register offsets in the L2 interrupt controller */
-#define CPU_STATUS 0x00
-#define CPU_SET 0x04
-#define CPU_CLEAR 0x08
-#define CPU_MASK_STATUS 0x0c
-#define CPU_MASK_SET 0x10
-#define CPU_MASK_CLEAR 0x14
+struct brcmstb_intc_init_params {
+ irq_flow_handler_t handler;
+ int cpu_status;
+ int cpu_clear;
+ int cpu_mask_status;
+ int cpu_mask_set;
+ int cpu_mask_clear;
+};
+
+/* Register offsets in the L2 latched interrupt controller */
+static const struct brcmstb_intc_init_params l2_edge_intc_init = {
+ .handler = handle_edge_irq,
+ .cpu_status = 0x00,
+ .cpu_clear = 0x08,
+ .cpu_mask_status = 0x0c,
+ .cpu_mask_set = 0x10,
+ .cpu_mask_clear = 0x14
+};
+
+/* Register offsets in the L2 level interrupt controller */
+static const struct brcmstb_intc_init_params l2_lvl_intc_init = {
+ .handler = handle_level_irq,
+ .cpu_status = 0x00,
+ .cpu_clear = -1, /* Register not present */
+ .cpu_mask_status = 0x04,
+ .cpu_mask_set = 0x08,
+ .cpu_mask_clear = 0x0C
+};

/* L2 intc private data structure */
struct brcmstb_l2_intc_data {
@@ -102,7 +123,7 @@ static void brcmstb_l2_intc_resume(struct irq_data *d)
struct brcmstb_l2_intc_data *b = gc->private;

irq_gc_lock(gc);
- if (ct->chip.irq_ack != irq_gc_noop) {
+ if (ct->chip.irq_ack) {
/* Clear unmasked non-wakeup interrupts */
irq_reg_writel(gc, ~b->saved_mask & ~gc->wake_active,
ct->regs.ack);
@@ -115,7 +136,9 @@ static void brcmstb_l2_intc_resume(struct irq_data *d)
}

static int __init brcmstb_l2_intc_of_init(struct device_node *np,
- struct device_node *parent)
+ struct device_node *parent,
+ const struct brcmstb_intc_init_params
+ *init_params)
{
unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
struct brcmstb_l2_intc_data *data;
@@ -137,12 +160,12 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
}

/* Disable all interrupts by default */
- writel(0xffffffff, base + CPU_MASK_SET);
+ writel(0xffffffff, base + init_params->cpu_mask_set);

/* Wakeup interrupts may be retained from S5 (cold boot) */
data->can_wake = of_property_read_bool(np, "brcm,irq-can-wake");
- if (!data->can_wake)
- writel(0xffffffff, base + CPU_CLEAR);
+ if (!data->can_wake && (init_params->cpu_clear >= 0))
+ writel(0xffffffff, base + init_params->cpu_clear);

parent_irq = irq_of_parse_and_map(np, 0);
if (!parent_irq) {
@@ -167,7 +190,7 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,

/* Allocate a single Generic IRQ chip for this node */
ret = irq_alloc_domain_generic_chips(data->domain, 32, 1,
- np->full_name, handle_edge_irq, clr, 0, flags);
+ np->full_name, init_params->handler, clr, 0, flags);
if (ret) {
pr_err("failed to allocate generic irq chip\n");
goto out_free_domain;
@@ -180,21 +203,26 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
data->gc = irq_get_domain_generic_chip(data->domain, 0);
data->gc->reg_base = base;
data->gc->private = data;
- data->status_offset = CPU_STATUS;
- data->mask_offset = CPU_MASK_STATUS;
+ data->status_offset = init_params->cpu_status;
+ data->mask_offset = init_params->cpu_mask_status;

ct = data->gc->chip_types;

- ct->chip.irq_ack = irq_gc_ack_set_bit;
- ct->regs.ack = CPU_CLEAR;
+ if (init_params->cpu_clear >= 0) {
+ ct->regs.ack = init_params->cpu_clear;
+ ct->chip.irq_ack = irq_gc_ack_set_bit;
+ ct->chip.irq_mask_ack = irq_gc_mask_disable_and_ack_set;
+ } else {
+ /* No Ack - but still slightly more efficient to define this */
+ ct->chip.irq_mask_ack = irq_gc_mask_disable_reg;
+ }

ct->chip.irq_mask = irq_gc_mask_disable_reg;
- ct->chip.irq_mask_ack = irq_gc_mask_disable_and_ack_set;
- ct->regs.disable = CPU_MASK_SET;
- ct->regs.mask = CPU_MASK_STATUS;
+ ct->regs.disable = init_params->cpu_mask_set;
+ ct->regs.mask = init_params->cpu_mask_status;

ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
- ct->regs.enable = CPU_MASK_CLEAR;
+ ct->regs.enable = init_params->cpu_mask_clear;

ct->chip.irq_suspend = brcmstb_l2_intc_suspend;
ct->chip.irq_resume = brcmstb_l2_intc_resume;
@@ -220,4 +248,18 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
kfree(data);
return ret;
}
-IRQCHIP_DECLARE(brcmstb_l2_intc, "brcm,l2-intc", brcmstb_l2_intc_of_init);
+
+int __init brcmstb_l2_edge_intc_of_init(struct device_node *np,
+ struct device_node *parent)
+{
+ return brcmstb_l2_intc_of_init(np, parent, &l2_edge_intc_init);
+}
+IRQCHIP_DECLARE(brcmstb_l2_intc, "brcm,l2-intc", brcmstb_l2_edge_intc_of_init);
+
+int __init brcmstb_l2_lvl_intc_of_init(struct device_node *np,
+ struct device_node *parent)
+{
+ return brcmstb_l2_intc_of_init(np, parent, &l2_lvl_intc_init);
+}
+IRQCHIP_DECLARE(bcm7271_l2_intc, "brcm,bcm7271-l2-intc",
+ brcmstb_l2_lvl_intc_of_init);
--
2.13.0
Doug Berger
2017-07-19 19:07:33 UTC
Permalink
Added register block offsets to the brcmstb_l2_intc_data structure
for the status and mask registers to support reading the active
interupts in an abstracted way. It seems like an irq_chip method
should have been provided for this, but it's not there yet.

Abstracted the implementation of the handler, suspend, and resume
functions to not use any hard coded register offsets.

Reviewed-by: Florian Fainelli <***@gmail.com>
Signed-off-by: Doug Berger <***@gmail.com>
---
drivers/irqchip/irq-brcmstb-l2.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-brcmstb-l2.c b/drivers/irqchip/irq-brcmstb-l2.c
index 977ae55d47d4..ce3850530e2b 100644
--- a/drivers/irqchip/irq-brcmstb-l2.c
+++ b/drivers/irqchip/irq-brcmstb-l2.c
@@ -43,6 +43,8 @@
struct brcmstb_l2_intc_data {
struct irq_domain *domain;
struct irq_chip_generic *gc;
+ int status_offset;
+ int mask_offset;
bool can_wake;
u32 saved_mask; /* for suspend/resume */
};
@@ -56,8 +58,8 @@ static void brcmstb_l2_intc_irq_handle(struct irq_desc *desc)

chained_irq_enter(chip, desc);

- status = irq_reg_readl(b->gc, CPU_STATUS) &
- ~(irq_reg_readl(b->gc, CPU_MASK_STATUS));
+ status = irq_reg_readl(b->gc, b->status_offset) &
+ ~(irq_reg_readl(b->gc, b->mask_offset));

if (status == 0) {
raw_spin_lock(&desc->lock);
@@ -78,16 +80,17 @@ static void brcmstb_l2_intc_irq_handle(struct irq_desc *desc)
static void brcmstb_l2_intc_suspend(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
struct brcmstb_l2_intc_data *b = gc->private;

irq_gc_lock(gc);
/* Save the current mask */
- b->saved_mask = irq_reg_readl(gc, CPU_MASK_STATUS);
+ b->saved_mask = irq_reg_readl(gc, ct->regs.mask);

if (b->can_wake) {
/* Program the wakeup mask */
- irq_reg_writel(gc, ~gc->wake_active, CPU_MASK_SET);
- irq_reg_writel(gc, gc->wake_active, CPU_MASK_CLEAR);
+ irq_reg_writel(gc, ~gc->wake_active, ct->regs.disable);
+ irq_reg_writel(gc, gc->wake_active, ct->regs.enable);
}
irq_gc_unlock(gc);
}
@@ -95,15 +98,19 @@ static void brcmstb_l2_intc_suspend(struct irq_data *d)
static void brcmstb_l2_intc_resume(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
struct brcmstb_l2_intc_data *b = gc->private;

irq_gc_lock(gc);
- /* Clear unmasked non-wakeup interrupts */
- irq_reg_writel(gc, ~b->saved_mask & ~gc->wake_active, CPU_CLEAR);
+ if (ct->chip.irq_ack != irq_gc_noop) {
+ /* Clear unmasked non-wakeup interrupts */
+ irq_reg_writel(gc, ~b->saved_mask & ~gc->wake_active,
+ ct->regs.ack);
+ }

/* Restore the saved mask */
- irq_reg_writel(gc, b->saved_mask, CPU_MASK_SET);
- irq_reg_writel(gc, ~b->saved_mask, CPU_MASK_CLEAR);
+ irq_reg_writel(gc, b->saved_mask, ct->regs.disable);
+ irq_reg_writel(gc, ~b->saved_mask, ct->regs.enable);
irq_gc_unlock(gc);
}

@@ -173,6 +180,9 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
data->gc = irq_get_domain_generic_chip(data->domain, 0);
data->gc->reg_base = base;
data->gc->private = data;
+ data->status_offset = CPU_STATUS;
+ data->mask_offset = CPU_MASK_STATUS;
+
ct = data->gc->chip_types;

ct->chip.irq_ack = irq_gc_ack_set_bit;
@@ -181,6 +191,7 @@ static int __init brcmstb_l2_intc_of_init(struct device_node *np,
ct->chip.irq_mask = irq_gc_mask_disable_reg;
ct->chip.irq_mask_ack = irq_gc_mask_disable_and_ack_set;
ct->regs.disable = CPU_MASK_SET;
+ ct->regs.mask = CPU_MASK_STATUS;

ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
ct->regs.enable = CPU_MASK_CLEAR;
--
2.13.0
Loading...