* [PATCH] irqchip: bcm7120-l2: Fix interrupt status for multiple parent IRQs
@ 2015-07-23 22:52 Florian Fainelli
2015-07-27 6:15 ` [tip:irq/core] irqchip/bcm7120-l2: " tip-bot for Florian Fainelli
2015-07-28 0:51 ` [PATCH] irqchip: bcm7120-l2: " Gregory Fong
0 siblings, 2 replies; 3+ messages in thread
From: Florian Fainelli @ 2015-07-23 22:52 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mips, cernekee, jason, tglx, bcm-kernel-feedback-list,
gregory.0xf0, computersforpeace, Florian Fainelli
Our irq-bcm7120-l2 interrupt controller driver utilizes the same handler
function for the different parent interrupts it services: UPG_MAIN, UPG_BSC for
instance.
The problem is that function reads the IRQSTAT register which can combine
interrupt causes for different parent interrupts, such that we can end-up in
the following situation:
- CPU takes an interrupt
- bcm7120_l2_intc_irq_handle() reads IRQSTAT
- generic_handle_irq() is invoked
- there are still pending interrupts flagged in IRQSTAT from a different parent
- handle_bad_irq() is invoked for these since they come from a different irq_desc/irq
In order to fix this, make sure that we always mask IRQSTAT with the
appropriate bits that correspond go the parent interrupt source this is coming
from. To simplify things, associate an unique structure per parent interrupt
handler to avoid multiplying the number of lookups.
Fixes: a5042de2688d ("irqchip: bcm7120-l2: Add Broadcom BCM7120-style Level 2 interrupt controller")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/irqchip/irq-bcm7120-l2.c | 51 ++++++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 12 deletions(-)
diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index 3ba5cc780fcb..8302d45d13ac 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -47,14 +47,20 @@ struct bcm7120_l2_intc_data {
struct irq_domain *domain;
bool can_wake;
u32 irq_fwd_mask[MAX_WORDS];
- u32 irq_map_mask[MAX_WORDS];
+ struct bcm7120_l1_intc_data *l1_data;
int num_parent_irqs;
const __be32 *map_mask_prop;
};
+struct bcm7120_l1_intc_data {
+ struct bcm7120_l2_intc_data *b;
+ u32 irq_map_mask[MAX_WORDS];
+};
+
static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
{
- struct bcm7120_l2_intc_data *b = irq_desc_get_handler_data(desc);
+ struct bcm7120_l1_intc_data *data = irq_desc_get_handler_data(desc);
+ struct bcm7120_l2_intc_data *b = data->b;
struct irq_chip *chip = irq_desc_get_chip(desc);
unsigned int idx;
@@ -69,7 +75,8 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
irq_gc_lock(gc);
pending = irq_reg_readl(gc, b->stat_offset[idx]) &
- gc->mask_cache;
+ gc->mask_cache &
+ data->irq_map_mask[idx];
irq_gc_unlock(gc);
for_each_set_bit(hwirq, &pending, IRQS_PER_WORD) {
@@ -107,8 +114,9 @@ static void bcm7120_l2_intc_resume(struct irq_data *d)
static int bcm7120_l2_intc_init_one(struct device_node *dn,
struct bcm7120_l2_intc_data *data,
- int irq)
+ int irq, u32 *valid_mask)
{
+ struct bcm7120_l1_intc_data *l1_data = &data->l1_data[irq];
int parent_irq;
unsigned int idx;
@@ -120,18 +128,27 @@ static int bcm7120_l2_intc_init_one(struct device_node *dn,
/* For multiple parent IRQs with multiple words, this looks like:
* <irq0_w0 irq0_w1 irq1_w0 irq1_w1 ...>
+ *
+ * We need to associate a given parent interrupt with its corresponding
+ * map_mask in order to mask the status register with it because we
+ * have the same handler being called for multiple parent interrupts.
+ *
+ * This is typically something needed on BCM7xxx (STB chips).
*/
for (idx = 0; idx < data->n_words; idx++) {
if (data->map_mask_prop) {
- data->irq_map_mask[idx] |=
+ l1_data->irq_map_mask[idx] |=
be32_to_cpup(data->map_mask_prop +
irq * data->n_words + idx);
} else {
- data->irq_map_mask[idx] = 0xffffffff;
+ l1_data->irq_map_mask[idx] = 0xffffffff;
}
+ valid_mask[idx] |= l1_data->irq_map_mask[idx];
}
- irq_set_handler_data(parent_irq, data);
+ l1_data->b = data;
+
+ irq_set_handler_data(parent_irq, l1_data);
irq_set_chained_handler(parent_irq, bcm7120_l2_intc_irq_handle);
return 0;
@@ -214,6 +231,7 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
struct irq_chip_type *ct;
int ret = 0;
unsigned int idx, irq, flags;
+ u32 valid_mask[MAX_WORDS] = { };
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
@@ -226,9 +244,16 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
goto out_unmap;
}
+ data->l1_data = kcalloc(data->num_parent_irqs, sizeof(*data->l1_data),
+ GFP_KERNEL);
+ if (!data->l1_data) {
+ ret = -ENOMEM;
+ goto out_free_l1_data;
+ }
+
ret = iomap_regs_fn(dn, data);
if (ret < 0)
- goto out_unmap;
+ goto out_free_l1_data;
for (idx = 0; idx < data->n_words; idx++) {
__raw_writel(data->irq_fwd_mask[idx],
@@ -237,16 +262,16 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
}
for (irq = 0; irq < data->num_parent_irqs; irq++) {
- ret = bcm7120_l2_intc_init_one(dn, data, irq);
+ ret = bcm7120_l2_intc_init_one(dn, data, irq, valid_mask);
if (ret)
- goto out_unmap;
+ goto out_free_l1_data;
}
data->domain = irq_domain_add_linear(dn, IRQS_PER_WORD * data->n_words,
&irq_generic_chip_ops, NULL);
if (!data->domain) {
ret = -ENOMEM;
- goto out_unmap;
+ goto out_free_l1_data;
}
/* MIPS chips strapped for BE will automagically configure the
@@ -270,7 +295,7 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
irq = idx * IRQS_PER_WORD;
gc = irq_get_domain_generic_chip(data->domain, irq);
- gc->unused = 0xffffffff & ~data->irq_map_mask[idx];
+ gc->unused = 0xffffffff & ~valid_mask[idx];
gc->private = data;
ct = gc->chip_types;
@@ -300,6 +325,8 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
out_free_domain:
irq_domain_remove(data->domain);
+out_free_l1_data:
+ kfree(data->l1_data);
out_unmap:
for (idx = 0; idx < MAX_MAPPINGS; idx++) {
if (data->map_base[idx])
--
2.1.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [tip:irq/core] irqchip/bcm7120-l2: Fix interrupt status for multiple parent IRQs
2015-07-23 22:52 [PATCH] irqchip: bcm7120-l2: Fix interrupt status for multiple parent IRQs Florian Fainelli
@ 2015-07-27 6:15 ` tip-bot for Florian Fainelli
2015-07-28 0:51 ` [PATCH] irqchip: bcm7120-l2: " Gregory Fong
1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Florian Fainelli @ 2015-07-27 6:15 UTC (permalink / raw)
To: linux-tip-commits; +Cc: mingo, linux-kernel, tglx, f.fainelli, hpa
Commit-ID: 0aef3997e12a10d4dfb6e01133e2fe478b9aa5eb
Gitweb: http://git.kernel.org/tip/0aef3997e12a10d4dfb6e01133e2fe478b9aa5eb
Author: Florian Fainelli <f.fainelli@gmail.com>
AuthorDate: Thu, 23 Jul 2015 15:52:21 -0700
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 27 Jul 2015 08:09:38 +0200
irqchip/bcm7120-l2: Fix interrupt status for multiple parent IRQs
Our irq-bcm7120-l2 interrupt controller driver utilizes the same handler
function for the different parent interrupts it services: UPG_MAIN, UPG_BSC for
instance.
The problem is that function reads the IRQSTAT register which can combine
interrupt causes for different parent interrupts, such that we can end-up in
the following situation:
- CPU takes an interrupt
- bcm7120_l2_intc_irq_handle() reads IRQSTAT
- generic_handle_irq() is invoked
- there are still pending interrupts flagged in IRQSTAT from a different parent
- handle_bad_irq() is invoked for these since they come from a different irq_desc/irq
In order to fix this, make sure that we always mask IRQSTAT with the
appropriate bits that correspond go the parent interrupt source this is coming
from. To simplify things, associate an unique structure per parent interrupt
handler to avoid multiplying the number of lookups.
Fixes: a5042de2688d ("irqchip: bcm7120-l2: Add Broadcom BCM7120-style Level 2 interrupt controller")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Cc: linux-mips@linux-mips.org
Cc: cernekee@gmail.com
Cc: jason@lakedaemon.net
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: gregory.0xf0@gmail.com
Cc: computersforpeace@gmail.com
Link: http://lkml.kernel.org/r/1437691941-3100-1-git-send-email-f.fainelli@gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
drivers/irqchip/irq-bcm7120-l2.c | 52 ++++++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
index c885a5c..d3f9769 100644
--- a/drivers/irqchip/irq-bcm7120-l2.c
+++ b/drivers/irqchip/irq-bcm7120-l2.c
@@ -37,6 +37,11 @@
#define MAX_MAPPINGS (MAX_WORDS * 2)
#define IRQS_PER_WORD 32
+struct bcm7120_l1_intc_data {
+ struct bcm7120_l2_intc_data *b;
+ u32 irq_map_mask[MAX_WORDS];
+};
+
struct bcm7120_l2_intc_data {
unsigned int n_words;
void __iomem *map_base[MAX_MAPPINGS];
@@ -46,14 +51,15 @@ struct bcm7120_l2_intc_data {
struct irq_domain *domain;
bool can_wake;
u32 irq_fwd_mask[MAX_WORDS];
- u32 irq_map_mask[MAX_WORDS];
+ struct bcm7120_l1_intc_data *l1_data;
int num_parent_irqs;
const __be32 *map_mask_prop;
};
static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
{
- struct bcm7120_l2_intc_data *b = irq_desc_get_handler_data(desc);
+ struct bcm7120_l1_intc_data *data = irq_desc_get_handler_data(desc);
+ struct bcm7120_l2_intc_data *b = data->b;
struct irq_chip *chip = irq_desc_get_chip(desc);
unsigned int idx;
@@ -68,7 +74,8 @@ static void bcm7120_l2_intc_irq_handle(unsigned int irq, struct irq_desc *desc)
irq_gc_lock(gc);
pending = irq_reg_readl(gc, b->stat_offset[idx]) &
- gc->mask_cache;
+ gc->mask_cache &
+ data->irq_map_mask[idx];
irq_gc_unlock(gc);
for_each_set_bit(hwirq, &pending, IRQS_PER_WORD) {
@@ -104,8 +111,9 @@ static void bcm7120_l2_intc_resume(struct irq_chip_generic *gc)
static int bcm7120_l2_intc_init_one(struct device_node *dn,
struct bcm7120_l2_intc_data *data,
- int irq)
+ int irq, u32 *valid_mask)
{
+ struct bcm7120_l1_intc_data *l1_data = &data->l1_data[irq];
int parent_irq;
unsigned int idx;
@@ -117,20 +125,28 @@ static int bcm7120_l2_intc_init_one(struct device_node *dn,
/* For multiple parent IRQs with multiple words, this looks like:
* <irq0_w0 irq0_w1 irq1_w0 irq1_w1 ...>
+ *
+ * We need to associate a given parent interrupt with its corresponding
+ * map_mask in order to mask the status register with it because we
+ * have the same handler being called for multiple parent interrupts.
+ *
+ * This is typically something needed on BCM7xxx (STB chips).
*/
for (idx = 0; idx < data->n_words; idx++) {
if (data->map_mask_prop) {
- data->irq_map_mask[idx] |=
+ l1_data->irq_map_mask[idx] |=
be32_to_cpup(data->map_mask_prop +
irq * data->n_words + idx);
} else {
- data->irq_map_mask[idx] = 0xffffffff;
+ l1_data->irq_map_mask[idx] = 0xffffffff;
}
+ valid_mask[idx] |= l1_data->irq_map_mask[idx];
}
- irq_set_chained_handler_and_data(parent_irq,
- bcm7120_l2_intc_irq_handle, data);
+ l1_data->b = data;
+ irq_set_chained_handler_and_data(parent_irq,
+ bcm7120_l2_intc_irq_handle, l1_data);
return 0;
}
@@ -211,6 +227,7 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
struct irq_chip_type *ct;
int ret = 0;
unsigned int idx, irq, flags;
+ u32 valid_mask[MAX_WORDS] = { };
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
@@ -223,9 +240,16 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
goto out_unmap;
}
+ data->l1_data = kcalloc(data->num_parent_irqs, sizeof(*data->l1_data),
+ GFP_KERNEL);
+ if (!data->l1_data) {
+ ret = -ENOMEM;
+ goto out_free_l1_data;
+ }
+
ret = iomap_regs_fn(dn, data);
if (ret < 0)
- goto out_unmap;
+ goto out_free_l1_data;
for (idx = 0; idx < data->n_words; idx++) {
__raw_writel(data->irq_fwd_mask[idx],
@@ -234,16 +258,16 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
}
for (irq = 0; irq < data->num_parent_irqs; irq++) {
- ret = bcm7120_l2_intc_init_one(dn, data, irq);
+ ret = bcm7120_l2_intc_init_one(dn, data, irq, valid_mask);
if (ret)
- goto out_unmap;
+ goto out_free_l1_data;
}
data->domain = irq_domain_add_linear(dn, IRQS_PER_WORD * data->n_words,
&irq_generic_chip_ops, NULL);
if (!data->domain) {
ret = -ENOMEM;
- goto out_unmap;
+ goto out_free_l1_data;
}
/* MIPS chips strapped for BE will automagically configure the
@@ -267,7 +291,7 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
irq = idx * IRQS_PER_WORD;
gc = irq_get_domain_generic_chip(data->domain, irq);
- gc->unused = 0xffffffff & ~data->irq_map_mask[idx];
+ gc->unused = 0xffffffff & ~valid_mask[idx];
gc->private = data;
ct = gc->chip_types;
@@ -304,6 +328,8 @@ int __init bcm7120_l2_intc_probe(struct device_node *dn,
out_free_domain:
irq_domain_remove(data->domain);
+out_free_l1_data:
+ kfree(data->l1_data);
out_unmap:
for (idx = 0; idx < MAX_MAPPINGS; idx++) {
if (data->map_base[idx])
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] irqchip: bcm7120-l2: Fix interrupt status for multiple parent IRQs
2015-07-23 22:52 [PATCH] irqchip: bcm7120-l2: Fix interrupt status for multiple parent IRQs Florian Fainelli
2015-07-27 6:15 ` [tip:irq/core] irqchip/bcm7120-l2: " tip-bot for Florian Fainelli
@ 2015-07-28 0:51 ` Gregory Fong
1 sibling, 0 replies; 3+ messages in thread
From: Gregory Fong @ 2015-07-28 0:51 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, linux-mips, Kevin Cernekee, jason, tglx,
bcm-kernel-feedback-list, Brian Norris
On Thu, Jul 23, 2015 at 3:52 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Our irq-bcm7120-l2 interrupt controller driver utilizes the same handler
> function for the different parent interrupts it services: UPG_MAIN, UPG_BSC for
> instance.
>
> The problem is that function reads the IRQSTAT register which can combine
> interrupt causes for different parent interrupts, such that we can end-up in
> the following situation:
>
> - CPU takes an interrupt
> - bcm7120_l2_intc_irq_handle() reads IRQSTAT
> - generic_handle_irq() is invoked
> - there are still pending interrupts flagged in IRQSTAT from a different parent
> - handle_bad_irq() is invoked for these since they come from a different irq_desc/irq
>
> In order to fix this, make sure that we always mask IRQSTAT with the
> appropriate bits that correspond go the parent interrupt source this is coming
> from. To simplify things, associate an unique structure per parent interrupt
> handler to avoid multiplying the number of lookups.
>
> Fixes: a5042de2688d ("irqchip: bcm7120-l2: Add Broadcom BCM7120-style Level 2 interrupt controller")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Gregory Fong <gregory.0xf0@gmail.com>
> ---
> drivers/irqchip/irq-bcm7120-l2.c | 51 ++++++++++++++++++++++++++++++----------
> 1 file changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/irqchip/irq-bcm7120-l2.c b/drivers/irqchip/irq-bcm7120-l2.c
> index 3ba5cc780fcb..8302d45d13ac 100644
> --- a/drivers/irqchip/irq-bcm7120-l2.c
> +++ b/drivers/irqchip/irq-bcm7120-l2.c
> @@ -47,14 +47,20 @@ struct bcm7120_l2_intc_data {
> struct irq_domain *domain;
> bool can_wake;
> u32 irq_fwd_mask[MAX_WORDS];
> - u32 irq_map_mask[MAX_WORDS];
> + struct bcm7120_l1_intc_data *l1_data;
> int num_parent_irqs;
> const __be32 *map_mask_prop;
> };
>
> +struct bcm7120_l1_intc_data {
> + struct bcm7120_l2_intc_data *b;
> + u32 irq_map_mask[MAX_WORDS];
> +};
I'm not sure the name bcm7120_l1_intc_data is a good name for this,
but I can't think of a better one, and it really doesn't matter that
much.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-07-28 0:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-23 22:52 [PATCH] irqchip: bcm7120-l2: Fix interrupt status for multiple parent IRQs Florian Fainelli
2015-07-27 6:15 ` [tip:irq/core] irqchip/bcm7120-l2: " tip-bot for Florian Fainelli
2015-07-28 0:51 ` [PATCH] irqchip: bcm7120-l2: " Gregory Fong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).