From: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
To: afzal mohammed <afzal.mohd.ma@gmail.com>
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 17:03:07 +0100 [thread overview]
Message-ID: <20200311160307.GA15464@alpha.franken.de> (raw)
In-Reply-To: <20200311131210.GA5115@afzalpc>
On Wed, Mar 11, 2020 at 06:42:10PM +0530, afzal mohammed wrote:
> 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 request_irq() is not a 1:1 replacement for our current setup_irq().
Or put it the another way our setup_irq() might be buggy, when used for
shared interrupts.
> 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.
I'm not sure, I like the adding of string pointers as dev_id arguments
in your patch. How can we make sure they are unique enough for the use
case ? I guess using handler as dev_id does a better job here.
And before doing that, lets clean up some of the IRQF_SHARED usage first.
All sni IRQF_SHARED can go away, the interrupt lines are exclusive there.
loongson2ef/lemote-2f/irq.c: looks like the only user of
LOONGSON_NORTH_BRIDGE_IRQ, so IRQF_SHARED could go as well.
Could someone confirm that ?
All other need to stay, IMHO.
And v4 is already in mips-next, so I need an incremental patch please.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
_______________________________________________
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 16:03 UTC|newest]
Thread overview: 28+ 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 20:31 ` Thomas Bogendoerfer
2020-03-05 11:57 ` [PATCH v4] " afzal mohammed
2020-03-06 12:47 ` Thomas Bogendoerfer
2020-03-11 5:31 ` Nathan Chancellor
2020-03-11 7:56 ` afzal mohammed
2020-03-11 9:03 ` afzal mohammed
2020-03-11 10:42 ` Thomas Bogendoerfer
2020-03-11 13:12 ` afzal mohammed
2020-03-11 16:03 ` Thomas Bogendoerfer [this message]
2020-03-11 16:32 ` 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 17:19 ` Guenter Roeck
2020-03-15 7:11 ` Nathan Chancellor
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-11 15:27 ` [PATCH] MIPS: pass non-NULL dev_id on shared request_irq() 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-14 1:07 ` afzal mohammed
2020-03-14 5:21 ` maobibo
2020-03-14 6:49 ` afzal mohammed
2020-03-14 10:28 ` Guenter Roeck
2020-03-14 11:42 ` afzal mohammed
2020-03-05 12:29 ` [PATCH v3] " afzal mohammed
2020-03-05 12:42 ` afzal mohammed
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=20200311160307.GA15464@alpha.franken.de \
--to=tsbogend@alpha.franken.de \
--cc=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 \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).