From: afzal mohammed <afzal.mohd.ma@gmail.com> To: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Nathan Chancellor <natechancellor@gmail.com>, linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Ralf Baechle <ralf@linux-mips.org>, Paul Burton <paulburton@kernel.org>, Florian Fainelli <f.fainelli@gmail.com>, bcm-kernel-feedback-list@broadcom.com, "Maciej W. Rozycki" <macro@linux-mips.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Keguang Zhang <keguang.zhang@gmail.com>, Huacai Chen <chenhc@lemote.com>, John Crispin <john@phrozen.org> Subject: Re: [PATCH v4] MIPS: Replace setup_irq() by request_irq() Date: Wed, 11 Mar 2020 18:42:10 +0530 [thread overview] Message-ID: <20200311131210.GA5115@afzalpc> (raw) In-Reply-To: <20200311104217.GA10615@alpha.franken.de> Hi Thomas, On Wed, Mar 11, 2020 at 11:42:17AM +0100, Thomas Bogendoerfer wrote: > On Wed, Mar 11, 2020 at 02:33:08PM +0530, afzal mohammed wrote: > > diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c > > int r4k_clockevent_init(void) > > { > > - unsigned long flags = IRQF_PERCPU | IRQF_TIMER | IRQF_SHARED; > > + unsigned long flags = IRQF_PERCPU | IRQF_TIMER; > I don't see why this should help. In my tree only sgi-ip30 removes > IRQF_SHARED from flags, but then it uses setup_percpu_irq(). > What do I miss ? You did not miss anything. Though it works, i took a wrong route following the tags & arrived at that solution in a hurry. (struct irqaction used in sgi-ip30 was used here earlier w/ setup_irq). The problem is sanity checks in request_irq() [ rather in request_thread_iq() ] if (((irqflags & IRQF_SHARED) && !dev_id) || (!(irqflags & IRQF_SHARED) && (irqflags & IRQF_COND_SUSPEND)) || ((irqflags & IRQF_NO_SUSPEND) && (irqflags & IRQF_COND_SUSPEND))) return -EINVAL; If IRQF_SHARED is passed, it exepcts a non-NULL dev_id, here it is NULL, setup_irq() doesn't have any check like that. So i think proper solution is to add a non NULL dev_id, as removing IRQF_SHARED might affect some platforms that might be using that interrupt line shared. Patch with non-NULL dev_id below, it works w/ Nathan's test case. Would you prefer an incremental patch or a fixed up v5 patch ? i will test build the other cases as well & sent the patch later today, though it is trivial, just being paranoid, since bitten by build error earlier. Also there was no meaningful pointer to pass as dev_id except in one case, so i have used name itself. Regards afzal --->8--- diff --git a/arch/mips/dec/setup.c b/arch/mips/dec/setup.c index c8bbac0c5051..cbbb27fbab19 100644 --- a/arch/mips/dec/setup.c +++ b/arch/mips/dec/setup.c @@ -758,7 +758,7 @@ void __init arch_init_irq(void) /* Register the bus error interrupt. */ if (dec_interrupt[DEC_IRQ_BUS] >= 0 && busirq_handler) { if (request_irq(dec_interrupt[DEC_IRQ_BUS], busirq_handler, - busirq_flags, "bus error", NULL)) + busirq_flags, "bus error", "bus error")) pr_err("Failed to register bus error interrupt\n"); } /* Register the HALT interrupt. */ diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c index 64e917dfe6b2..4b19810c67d4 100644 --- a/arch/mips/kernel/cevt-r4k.c +++ b/arch/mips/kernel/cevt-r4k.c @@ -292,7 +292,7 @@ int r4k_clockevent_init(void) cp0_timer_irq_installed = 1; - if (request_irq(irq, c0_compare_interrupt, flags, "timer", NULL)) + if (request_irq(irq, c0_compare_interrupt, flags, "timer", cd)) pr_err("Failed to request irq %d (timer)\n", irq); return 0; diff --git a/arch/mips/loongson2ef/lemote-2f/irq.c b/arch/mips/loongson2ef/lemote-2f/irq.c index 34e15e8b7a8f..1c99e83cabae 100644 --- a/arch/mips/loongson2ef/lemote-2f/irq.c +++ b/arch/mips/loongson2ef/lemote-2f/irq.c @@ -109,7 +109,7 @@ void __init mach_init_irq(void) /* setup north bridge irq (bonito) */ if (request_irq(LOONGSON_NORTH_BRIDGE_IRQ, ip6_action, - IRQF_SHARED | IRQF_NO_THREAD, "cascade", NULL)) + IRQF_SHARED | IRQF_NO_THREAD, "cascade", "cascade")) pr_err("Failed to register north bridge cascade interrupt\n"); /* setup source bridge irq (i8259) */ if (request_irq(LOONGSON_SOUTH_BRIDGE_IRQ, no_action, diff --git a/arch/mips/pmcs-msp71xx/msp_time.c b/arch/mips/pmcs-msp71xx/msp_time.c index baf0da8b4c98..0601a3f7f8f6 100644 --- a/arch/mips/pmcs-msp71xx/msp_time.c +++ b/arch/mips/pmcs-msp71xx/msp_time.c @@ -81,7 +81,7 @@ unsigned int get_c0_compare_int(void) /* MIPS_MT modes may want timer for second VPE */ if ((get_current_vpe()) && !tim_installed) { if (request_irq(MSP_INT_VPE1_TIMER, c0_compare_interrupt, flags, - "timer", NULL)) + "timer", "timer")) pr_err("Failed to register timer interrupt\n"); tim_installed++; } diff --git a/arch/mips/sni/a20r.c b/arch/mips/sni/a20r.c index ba966d62fb4b..1bfea4aae124 100644 --- a/arch/mips/sni/a20r.c +++ b/arch/mips/sni/a20r.c @@ -223,7 +223,7 @@ void __init sni_a20r_irq_init(void) sni_hwint = a20r_hwint; change_c0_status(ST0_IM, IE_IRQ0); if (request_irq(SNI_A20R_IRQ_BASE + 3, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); } diff --git a/arch/mips/sni/pcit.c b/arch/mips/sni/pcit.c index 4a850ab03398..15080155cc9a 100644 --- a/arch/mips/sni/pcit.c +++ b/arch/mips/sni/pcit.c @@ -245,7 +245,7 @@ void __init sni_pcit_irq_init(void) sni_hwint = sni_pcit_hwint; change_c0_status(ST0_IM, IE_IRQ1); if (request_irq(SNI_PCIT_INT_START + 6, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); } @@ -260,7 +260,7 @@ void __init sni_pcit_cplus_irq_init(void) sni_hwint = sni_pcit_hwint_cplus; change_c0_status(ST0_IM, IE_IRQ0); if (request_irq(MIPS_CPU_IRQ_BASE + 3, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); } diff --git a/arch/mips/sni/rm200.c b/arch/mips/sni/rm200.c index ba1f2fc6a43e..2b4a6448b428 100644 --- a/arch/mips/sni/rm200.c +++ b/arch/mips/sni/rm200.c @@ -473,10 +473,10 @@ void __init sni_rm200_irq_init(void) sni_hwint = sni_rm200_hwint; change_c0_status(ST0_IM, IE_IRQ0); if (request_irq(SNI_RM200_INT_START + 0, sni_rm200_i8259A_irq_handler, - IRQF_SHARED, "onboard ISA", NULL)) + IRQF_SHARED, "onboard ISA", "onboard ISA")) pr_err("Failed to register onboard ISA interrupt\n"); if (request_irq(SNI_RM200_INT_START + 1, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); }
WARNING: multiple messages have this Message-ID (diff)
From: afzal mohammed <afzal.mohd.ma@gmail.com> To: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Florian Fainelli <f.fainelli@gmail.com>, Paul Burton <paulburton@kernel.org>, linux-mips@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>, linux-kernel@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com, Jiaxun Yang <jiaxun.yang@flygoat.com>, "Maciej W. Rozycki" <macro@linux-mips.org>, John Crispin <john@phrozen.org>, Huacai Chen <chenhc@lemote.com>, Nathan Chancellor <natechancellor@gmail.com>, Keguang Zhang <keguang.zhang@gmail.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4] MIPS: Replace setup_irq() by request_irq() Date: Wed, 11 Mar 2020 18:42:10 +0530 [thread overview] Message-ID: <20200311131210.GA5115@afzalpc> (raw) In-Reply-To: <20200311104217.GA10615@alpha.franken.de> Hi Thomas, On Wed, Mar 11, 2020 at 11:42:17AM +0100, Thomas Bogendoerfer wrote: > On Wed, Mar 11, 2020 at 02:33:08PM +0530, afzal mohammed wrote: > > diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c > > int r4k_clockevent_init(void) > > { > > - unsigned long flags = IRQF_PERCPU | IRQF_TIMER | IRQF_SHARED; > > + unsigned long flags = IRQF_PERCPU | IRQF_TIMER; > I don't see why this should help. In my tree only sgi-ip30 removes > IRQF_SHARED from flags, but then it uses setup_percpu_irq(). > What do I miss ? You did not miss anything. Though it works, i took a wrong route following the tags & arrived at that solution in a hurry. (struct irqaction used in sgi-ip30 was used here earlier w/ setup_irq). The problem is sanity checks in request_irq() [ rather in request_thread_iq() ] if (((irqflags & IRQF_SHARED) && !dev_id) || (!(irqflags & IRQF_SHARED) && (irqflags & IRQF_COND_SUSPEND)) || ((irqflags & IRQF_NO_SUSPEND) && (irqflags & IRQF_COND_SUSPEND))) return -EINVAL; If IRQF_SHARED is passed, it exepcts a non-NULL dev_id, here it is NULL, setup_irq() doesn't have any check like that. So i think proper solution is to add a non NULL dev_id, as removing IRQF_SHARED might affect some platforms that might be using that interrupt line shared. Patch with non-NULL dev_id below, it works w/ Nathan's test case. Would you prefer an incremental patch or a fixed up v5 patch ? i will test build the other cases as well & sent the patch later today, though it is trivial, just being paranoid, since bitten by build error earlier. Also there was no meaningful pointer to pass as dev_id except in one case, so i have used name itself. Regards afzal --->8--- diff --git a/arch/mips/dec/setup.c b/arch/mips/dec/setup.c index c8bbac0c5051..cbbb27fbab19 100644 --- a/arch/mips/dec/setup.c +++ b/arch/mips/dec/setup.c @@ -758,7 +758,7 @@ void __init arch_init_irq(void) /* Register the bus error interrupt. */ if (dec_interrupt[DEC_IRQ_BUS] >= 0 && busirq_handler) { if (request_irq(dec_interrupt[DEC_IRQ_BUS], busirq_handler, - busirq_flags, "bus error", NULL)) + busirq_flags, "bus error", "bus error")) pr_err("Failed to register bus error interrupt\n"); } /* Register the HALT interrupt. */ diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c index 64e917dfe6b2..4b19810c67d4 100644 --- a/arch/mips/kernel/cevt-r4k.c +++ b/arch/mips/kernel/cevt-r4k.c @@ -292,7 +292,7 @@ int r4k_clockevent_init(void) cp0_timer_irq_installed = 1; - if (request_irq(irq, c0_compare_interrupt, flags, "timer", NULL)) + if (request_irq(irq, c0_compare_interrupt, flags, "timer", cd)) pr_err("Failed to request irq %d (timer)\n", irq); return 0; diff --git a/arch/mips/loongson2ef/lemote-2f/irq.c b/arch/mips/loongson2ef/lemote-2f/irq.c index 34e15e8b7a8f..1c99e83cabae 100644 --- a/arch/mips/loongson2ef/lemote-2f/irq.c +++ b/arch/mips/loongson2ef/lemote-2f/irq.c @@ -109,7 +109,7 @@ void __init mach_init_irq(void) /* setup north bridge irq (bonito) */ if (request_irq(LOONGSON_NORTH_BRIDGE_IRQ, ip6_action, - IRQF_SHARED | IRQF_NO_THREAD, "cascade", NULL)) + IRQF_SHARED | IRQF_NO_THREAD, "cascade", "cascade")) pr_err("Failed to register north bridge cascade interrupt\n"); /* setup source bridge irq (i8259) */ if (request_irq(LOONGSON_SOUTH_BRIDGE_IRQ, no_action, diff --git a/arch/mips/pmcs-msp71xx/msp_time.c b/arch/mips/pmcs-msp71xx/msp_time.c index baf0da8b4c98..0601a3f7f8f6 100644 --- a/arch/mips/pmcs-msp71xx/msp_time.c +++ b/arch/mips/pmcs-msp71xx/msp_time.c @@ -81,7 +81,7 @@ unsigned int get_c0_compare_int(void) /* MIPS_MT modes may want timer for second VPE */ if ((get_current_vpe()) && !tim_installed) { if (request_irq(MSP_INT_VPE1_TIMER, c0_compare_interrupt, flags, - "timer", NULL)) + "timer", "timer")) pr_err("Failed to register timer interrupt\n"); tim_installed++; } diff --git a/arch/mips/sni/a20r.c b/arch/mips/sni/a20r.c index ba966d62fb4b..1bfea4aae124 100644 --- a/arch/mips/sni/a20r.c +++ b/arch/mips/sni/a20r.c @@ -223,7 +223,7 @@ void __init sni_a20r_irq_init(void) sni_hwint = a20r_hwint; change_c0_status(ST0_IM, IE_IRQ0); if (request_irq(SNI_A20R_IRQ_BASE + 3, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); } diff --git a/arch/mips/sni/pcit.c b/arch/mips/sni/pcit.c index 4a850ab03398..15080155cc9a 100644 --- a/arch/mips/sni/pcit.c +++ b/arch/mips/sni/pcit.c @@ -245,7 +245,7 @@ void __init sni_pcit_irq_init(void) sni_hwint = sni_pcit_hwint; change_c0_status(ST0_IM, IE_IRQ1); if (request_irq(SNI_PCIT_INT_START + 6, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); } @@ -260,7 +260,7 @@ void __init sni_pcit_cplus_irq_init(void) sni_hwint = sni_pcit_hwint_cplus; change_c0_status(ST0_IM, IE_IRQ0); if (request_irq(MIPS_CPU_IRQ_BASE + 3, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); } diff --git a/arch/mips/sni/rm200.c b/arch/mips/sni/rm200.c index ba1f2fc6a43e..2b4a6448b428 100644 --- a/arch/mips/sni/rm200.c +++ b/arch/mips/sni/rm200.c @@ -473,10 +473,10 @@ void __init sni_rm200_irq_init(void) sni_hwint = sni_rm200_hwint; change_c0_status(ST0_IM, IE_IRQ0); if (request_irq(SNI_RM200_INT_START + 0, sni_rm200_i8259A_irq_handler, - IRQF_SHARED, "onboard ISA", NULL)) + IRQF_SHARED, "onboard ISA", "onboard ISA")) pr_err("Failed to register onboard ISA interrupt\n"); if (request_irq(SNI_RM200_INT_START + 1, sni_isa_irq_handler, - IRQF_SHARED, "ISA", NULL)) + IRQF_SHARED, "ISA", "ISA")) pr_err("Failed to register ISA interrupt\n"); } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-03-11 13:12 UTC|newest] Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-04 0:55 [PATCH v3] MIPS: Replace setup_irq() by request_irq() afzal mohammed 2020-03-04 0:55 ` afzal mohammed 2020-03-04 20:31 ` Thomas Bogendoerfer 2020-03-04 20:31 ` Thomas Bogendoerfer 2020-03-05 11:57 ` [PATCH v4] " afzal mohammed 2020-03-05 11:57 ` afzal mohammed 2020-03-06 12:47 ` Thomas Bogendoerfer 2020-03-06 12:47 ` Thomas Bogendoerfer 2020-03-11 5:31 ` Nathan Chancellor 2020-03-11 5:31 ` Nathan Chancellor 2020-03-11 7:56 ` afzal mohammed 2020-03-11 7:56 ` afzal mohammed 2020-03-11 9:03 ` afzal mohammed 2020-03-11 9:03 ` afzal mohammed 2020-03-11 10:42 ` Thomas Bogendoerfer 2020-03-11 10:42 ` Thomas Bogendoerfer 2020-03-11 13:12 ` afzal mohammed [this message] 2020-03-11 13:12 ` afzal mohammed 2020-03-11 16:03 ` Thomas Bogendoerfer 2020-03-11 16:03 ` Thomas Bogendoerfer 2020-03-11 16:32 ` afzal mohammed 2020-03-11 16:32 ` afzal mohammed 2020-03-13 12:11 ` afzal mohammed 2020-03-13 12:11 ` afzal mohammed 2020-03-14 8:13 ` [PATCH v2] MIPS: pass non-NULL dev_id on shared request_irq() afzal mohammed 2020-03-14 8:13 ` afzal mohammed 2020-03-14 17:19 ` Guenter Roeck 2020-03-14 17:19 ` Guenter Roeck 2020-03-15 7:11 ` Nathan Chancellor 2020-03-15 7:11 ` Nathan Chancellor 2020-03-16 15:32 ` Thomas Bogendoerfer 2020-03-16 15:32 ` Thomas Bogendoerfer 2020-03-14 6:55 ` [PATCH v4] MIPS: Replace setup_irq() by request_irq() afzal mohammed 2020-03-14 6:55 ` afzal mohammed 2020-03-11 15:27 ` [PATCH] MIPS: pass non-NULL dev_id on shared request_irq() afzal mohammed 2020-03-11 15:27 ` afzal mohammed 2020-03-11 16:06 ` afzal mohammed 2020-03-11 16:06 ` afzal mohammed 2020-03-13 16:47 ` [PATCH v4] MIPS: Replace setup_irq() by request_irq() Guenter Roeck 2020-03-13 16:47 ` Guenter Roeck 2020-03-14 1:07 ` afzal mohammed 2020-03-14 1:07 ` afzal mohammed 2020-03-14 5:21 ` maobibo 2020-03-14 6:49 ` afzal mohammed 2020-03-14 6:49 ` afzal mohammed 2020-03-14 10:28 ` Guenter Roeck 2020-03-14 10:28 ` Guenter Roeck 2020-03-14 11:42 ` afzal mohammed 2020-03-14 11:42 ` afzal mohammed 2020-03-05 12:29 ` [PATCH v3] " afzal mohammed 2020-03-05 12:29 ` afzal mohammed 2020-03-05 12:42 ` afzal mohammed 2020-03-05 12:42 ` afzal mohammed 2020-03-04 20:38 ` kbuild test robot 2020-03-04 20:38 ` kbuild test robot 2020-03-04 20:38 ` kbuild test robot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200311131210.GA5115@afzalpc \ --to=afzal.mohd.ma@gmail.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=chenhc@lemote.com \ --cc=f.fainelli@gmail.com \ --cc=jiaxun.yang@flygoat.com \ --cc=john@phrozen.org \ --cc=keguang.zhang@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mips@vger.kernel.org \ --cc=macro@linux-mips.org \ --cc=natechancellor@gmail.com \ --cc=paulburton@kernel.org \ --cc=ralf@linux-mips.org \ --cc=tsbogend@alpha.franken.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.