diff mbox

[OpenWrt-Devel,v2,2/3] mcs814x: fix interrupts

Message ID 1437994095-16492-3-git-send-email-guenther.kelleter@devolo.de
State Superseded
Headers show

Commit Message

Günther Kelleter July 27, 2015, 10:48 a.m. UTC
create explicit 1:1 mapping before mcs814x_alloc_gc/irq_setup_generic_chip
marks all interrupts used and prevents mapping by dts init.
IRQ 0 is the timer interrupt and is not illegal!

Was broken since kernel 3.14.

Signed-off-by: Günther Kelleter <guenther.kelleter@devolo.de>
---
 target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c |  6 +++++-
 target/linux/mcs814x/patches-3.18/015-timer-irq.patch       | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)
 create mode 100644 target/linux/mcs814x/patches-3.18/015-timer-irq.patch

Comments

Florian Fainelli July 27, 2015, 5:50 p.m. UTC | #1
On Jul 27, 2015 3:54 AM, "Günther Kelleter" <guenther.kelleter@devolo.de>
wrote:
>
> create explicit 1:1 mapping before mcs814x_alloc_gc/irq_setup_generic_chip
> marks all interrupts used and prevents mapping by dts init.
> IRQ 0 is the timer interrupt and is not illegal!

Is the second hunk of the patch still necessary then? Some other people
seem to have run into similar problems on Dove, you might to look at what
they did to avoid having to remove the check against virq 0.

>
> Was broken since kernel 3.14.
>
> Signed-off-by: Günther Kelleter <guenther.kelleter@devolo.de>
> ---
>  target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c |  6 +++++-
>  target/linux/mcs814x/patches-3.18/015-timer-irq.patch       | 11
+++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
>  create mode 100644 target/linux/mcs814x/patches-3.18/015-timer-irq.patch
>
> diff --git a/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c
b/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c
> index f84c412..fd4345f 100644
> --- a/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c
> +++ b/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c
> @@ -71,6 +71,7 @@ static const struct of_device_id mcs814x_intc_ids[] = {
>  void __init mcs814x_of_irq_init(void)
>  {
>         struct device_node *np;
> +       struct irq_domain *domain;
>
>         np = of_find_matching_node(NULL, mcs814x_intc_ids);
>         if (!np)
> @@ -80,7 +81,10 @@ void __init mcs814x_of_irq_init(void)
>         if (!mcs814x_intc_base)
>                 panic("unable to map intc cpu registers\n");
>
> -       irq_domain_add_simple(np, 32, 0, &irq_generic_chip_ops, NULL);
> +       domain = irq_domain_add_simple(np, 32, 0, &irq_domain_simple_ops,
NULL);
> +       if (!domain)
> +               panic("unable to add irq domain\n");
> +       irq_create_strict_mappings(domain, 0, 0, 32);
>
>         of_node_put(np);
>
> diff --git a/target/linux/mcs814x/patches-3.18/015-timer-irq.patch
b/target/linux/mcs814x/patches-3.18/015-timer-irq.patch
> new file mode 100644
> index 0000000..9bbb094
> --- /dev/null
> +++ b/target/linux/mcs814x/patches-3.18/015-timer-irq.patch
> @@ -0,0 +1,11 @@
> +--- a/kernel/irq/irqdesc.c
> ++++ b/kernel/irq/irqdesc.c
> +@@ -381,7 +381,7 @@ int __handle_domain_irq(struct irq_domai
> +        * Some hardware gives randomly wrong interrupts.  Rather
> +        * than crashing, do something sensible.
> +        */
> +-      if (unlikely(!irq || irq >= nr_irqs)) {
> ++      if (unlikely(irq >= nr_irqs)) {
> +               ack_bad_irq(irq);
> +               ret = -EINVAL;
> +       } else {
> --
> 2.4.6.89.g851dcf4
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Günther Kelleter July 28, 2015, 7:53 a.m. UTC | #2
Hi

I must admit that I couldn’t find a better solution. I took me a whole day to find this way to fix it, because the documentation of all this generic interrupt stuff is just bad and incomplete.
The problem here was that irq_setup_generic_chip finally calls irq_mark_irq for all 32 interrupts and then a mapping isn’t possible anymore by the dts init code, since all interrupts are marked as already allocated. The .map method of irq_generic_chip_ops is also not working due to many missing initialisations of the generic chip structure.

Timer seems to be initialized before all this generic interrupt code initializations and is using irq desc 0 always.

If someone who looks through this generic interrupts has a clean fix, please tell me.

I’m looking into the Dove interrupt code though.

Günther

From: f.fainelli@gmail.com [mailto:f.fainelli@gmail.com] On Behalf Of Florian Fainelli

Sent: Monday, July 27, 2015 7:51 PM
To: Guenther Kelleter
Cc: OpenWrt Development List
Subject: Re: [OpenWrt-Devel] [PATCH v2 2/3] mcs814x: fix interrupts


On Jul 27, 2015 3:54 AM, "Günther Kelleter" <guenther.kelleter@devolo.de<mailto:guenther.kelleter@devolo.de>> wrote:
>

> create explicit 1:1 mapping before mcs814x_alloc_gc/irq_setup_generic_chip

> marks all interrupts used and prevents mapping by dts init.

> IRQ 0 is the timer interrupt and is not illegal!


Is the second hunk of the patch still necessary then? Some other people seem to have run into similar problems on Dove, you might to look at what they did to avoid having to remove the check against virq 0.

>

> Was broken since kernel 3.14.

>

> Signed-off-by: Günther Kelleter <guenther.kelleter@devolo.de<mailto:guenther.kelleter@devolo.de>>

> ---

>  target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c |  6 +++++-

>  target/linux/mcs814x/patches-3.18/015-timer-irq.patch       | 11 +++++++++++

>  2 files changed, 16 insertions(+), 1 deletion(-)

>  create mode 100644 target/linux/mcs814x/patches-3.18/015-timer-irq.patch

>

> diff --git a/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c b/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c

> index f84c412..fd4345f 100644

> --- a/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c

> +++ b/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c

> @@ -71,6 +71,7 @@ static const struct of_device_id mcs814x_intc_ids[] = {

>  void __init mcs814x_of_irq_init(void)

>  {

>         struct device_node *np;

> +       struct irq_domain *domain;

>

>         np = of_find_matching_node(NULL, mcs814x_intc_ids);

>         if (!np)

> @@ -80,7 +81,10 @@ void __init mcs814x_of_irq_init(void)

>         if (!mcs814x_intc_base)

>                 panic("unable to map intc cpu registers\n");

>

> -       irq_domain_add_simple(np, 32, 0, &irq_generic_chip_ops, NULL);

> +       domain = irq_domain_add_simple(np, 32, 0, &irq_domain_simple_ops, NULL);

> +       if (!domain)

> +               panic("unable to add irq domain\n");

> +       irq_create_strict_mappings(domain, 0, 0, 32);

>

>         of_node_put(np);

>

> diff --git a/target/linux/mcs814x/patches-3.18/015-timer-irq.patch b/target/linux/mcs814x/patches-3.18/015-timer-irq.patch

> new file mode 100644

> index 0000000..9bbb094

> --- /dev/null

> +++ b/target/linux/mcs814x/patches-3.18/015-timer-irq.patch

> @@ -0,0 +1,11 @@

> +--- a/kernel/irq/irqdesc.c

> ++++ b/kernel/irq/irqdesc.c

> +@@ -381,7 +381,7 @@ int __handle_domain_irq(struct irq_domai

> +        * Some hardware gives randomly wrong interrupts.  Rather

> +        * than crashing, do something sensible.

> +        */

> +-      if (unlikely(!irq || irq >= nr_irqs)) {

> ++      if (unlikely(irq >= nr_irqs)) {

> +               ack_bad_irq(irq);

> +               ret = -EINVAL;

> +       } else {

> --

> 2.4.6.89.g851dcf4

> _______________________________________________

> openwrt-devel mailing list

> openwrt-devel@lists.openwrt.org<mailto:openwrt-devel@lists.openwrt.org>

> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Günther Kelleter July 28, 2015, 1:02 p.m. UTC | #3
mach-dove/irq.c doesn’t use generic irq chip API. This doesn’t help me any further.


From: f.fainelli@gmail.com [mailto:f.fainelli@gmail.com] On Behalf Of Florian Fainelli

Sent: Monday, July 27, 2015 7:51 PM
To: Guenther Kelleter
Cc: OpenWrt Development List
Subject: Re: [OpenWrt-Devel] [PATCH v2 2/3] mcs814x: fix interrupts


On Jul 27, 2015 3:54 AM, "Günther Kelleter" <guenther.kelleter@devolo.de<mailto:guenther.kelleter@devolo.de>> wrote:
>

> create explicit 1:1 mapping before mcs814x_alloc_gc/irq_setup_generic_chip

> marks all interrupts used and prevents mapping by dts init.

> IRQ 0 is the timer interrupt and is not illegal!


Is the second hunk of the patch still necessary then? Some other people seem to have run into similar problems on Dove, you might to look at what they did to avoid having to remove the check against virq 0.

>
diff mbox

Patch

diff --git a/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c b/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c
index f84c412..fd4345f 100644
--- a/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c
+++ b/target/linux/mcs814x/files-3.18/arch/arm/mach-mcs814x/irq.c
@@ -71,6 +71,7 @@  static const struct of_device_id mcs814x_intc_ids[] = {
 void __init mcs814x_of_irq_init(void)
 {
 	struct device_node *np;
+	struct irq_domain *domain;
 
 	np = of_find_matching_node(NULL, mcs814x_intc_ids);
 	if (!np)
@@ -80,7 +81,10 @@  void __init mcs814x_of_irq_init(void)
 	if (!mcs814x_intc_base)
 		panic("unable to map intc cpu registers\n");
 
-	irq_domain_add_simple(np, 32, 0, &irq_generic_chip_ops, NULL);
+	domain = irq_domain_add_simple(np, 32, 0, &irq_domain_simple_ops, NULL);
+	if (!domain)
+		panic("unable to add irq domain\n");
+	irq_create_strict_mappings(domain, 0, 0, 32);
 
 	of_node_put(np);
 
diff --git a/target/linux/mcs814x/patches-3.18/015-timer-irq.patch b/target/linux/mcs814x/patches-3.18/015-timer-irq.patch
new file mode 100644
index 0000000..9bbb094
--- /dev/null
+++ b/target/linux/mcs814x/patches-3.18/015-timer-irq.patch
@@ -0,0 +1,11 @@ 
+--- a/kernel/irq/irqdesc.c
++++ b/kernel/irq/irqdesc.c
+@@ -381,7 +381,7 @@ int __handle_domain_irq(struct irq_domai
+ 	 * Some hardware gives randomly wrong interrupts.  Rather
+ 	 * than crashing, do something sensible.
+ 	 */
+-	if (unlikely(!irq || irq >= nr_irqs)) {
++	if (unlikely(irq >= nr_irqs)) {
+ 		ack_bad_irq(irq);
+ 		ret = -EINVAL;
+ 	} else {