From: Bjorn Helgaas <helgaas@kernel.org>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Heiner Kallweit <hkallweit1@gmail.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
linux-kernel@vger.kernel.org,
MyungJoo Ham <myungjoo.ham@samsung.com>,
Chanwoo Choi <cw00.choi@samsung.com>,
Sebastian Reichel <sre@kernel.org>, Milo Kim <milo.kim@ti.com>,
Krzysztof Kozlowski <krzk@kernel.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH] PCI: let pci_request_irq properly deal with threaded interrupts
Date: Tue, 31 Jul 2018 06:13:17 -0500 [thread overview]
Message-ID: <20180731111317.GJ45322@bhelgaas-glaptop.roam.corp.google.com> (raw)
In-Reply-To: <20180731080111.GG17271@n2100.armlinux.org.uk>
On Tue, Jul 31, 2018 at 09:01:12AM +0100, Russell King - ARM Linux wrote:
> On Tue, Jul 31, 2018 at 12:50:21AM +0200, Thomas Gleixner wrote:
> > On Mon, 30 Jul 2018, Bjorn Helgaas wrote:
> >
> > > [+cc maintainers of possibly erroneous callers of request_threaded_irq()]
> > >
> > > On Mon, Jul 30, 2018 at 04:30:28PM -0500, Bjorn Helgaas wrote:
> > > > [+cc Thomas, Christoph, LKML]
> > > >
> > > > On Mon, Jul 30, 2018 at 12:03:42AM +0200, Heiner Kallweit wrote:
> > > > > If we have a threaded interrupt with the handler being NULL, then
> > > > > request_threaded_irq() -> __setup_irq() will complain and bail out
> > > > > if the IRQF_ONESHOT flag isn't set. Therefore check for the handler
> > > > > being NULL and set IRQF_ONESHOT in this case.
> > > > >
> > > > > This change is needed to migrate the mei_me driver to
> > > > > pci_alloc_irq_vectors() and pci_request_irq().
> > > > >
> > > > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> > > >
> > > > I'd like an ack from Thomas because this requirement about IRQF_ONESHOT
> > > > usage isn't mentioned in the request_threaded_irq() function doc or
> > > > Documentation/
> > >
> > > Possibly these other request_threaded_irq() callers are similarly
> > > broken? I can't tell for sure about tda998x_create(), but all the
> > > others certainly call request_threaded_irq() with "handler == NULL"
> > > and irqflags that do not contain IRQF_ONESHOT:
> > >
> > > tda998x_create()
> > > request_threaded_irq(client->irq, NULL, ..., irqd_get_trigger_type(), ...)
> > > (I can't tell what irqd_get_trigger_type() does)
> >
> > It reads the trigger type back from the irq chip (level/edge/polarity) but
> > does not return with the ONESHOT bit set.
>
> What tree are you (Bjorn) using there? Looking in my git history,
> request_threaded_irq() was added in January 2014, as:
>
> + irqf_trigger =
> + irqd_get_trigger_type(irq_get_irq_data(client->irq));
> + ret = request_threaded_irq(client->irq, NULL,
> + tda998x_irq_thread,
> + irqf_trigger | IRQF_ONESHOT,
> + "tda998x", priv);
>
> and updated more recently for the CEC support in ae81553c30ef
> ("drm/i2c: tda998x: allow interrupt to be shared") to be:
>
> - irqf_trigger =
> + irq_flags =
> irqd_get_trigger_type(irq_get_irq_data(client->irq));
> + irq_flags |= IRQF_SHARED | IRQF_ONESHOT;
> ret = request_threaded_irq(client->irq, NULL,
> - tda998x_irq_thread,
> - irqf_trigger | IRQF_ONESHOT,
> + tda998x_irq_thread, irq_flags,
> "tda998x", priv);
>
> In all cases, IRQF_ONESHOT has been set for this threaded interrupt.
>
> I've also checked drm-next, and it doesn't show anything like what
> Bjorn quoted.
>
> Confused. Bjorn, can you double check that what you think are
> erroneous request_threaded_irq() calls are actually as per what
> you quoted please?
I was looking at v4.18-rc1, but in the case of tda998x_create(), I
overlooked this line:
irq_flags |= IRQF_SHARED | IRQF_ONESHOT;
Sorry!
next prev parent reply other threads:[~2018-07-31 11:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <d004975f-03a8-59f5-56ef-f4e1f8e12dfb@gmail.com>
2018-07-30 21:30 ` [PATCH] PCI: let pci_request_irq properly deal with threaded interrupts Bjorn Helgaas
2018-07-30 21:50 ` Heiner Kallweit
2018-07-30 21:54 ` Andy Shevchenko
2018-07-30 22:36 ` Thomas Gleixner
2018-07-31 6:15 ` Marc Zyngier
2018-08-01 19:51 ` Heiner Kallweit
2018-08-03 14:09 ` Thomas Gleixner
2018-08-03 19:25 ` Heiner Kallweit
2018-08-03 19:40 ` Thomas Gleixner
2018-08-03 20:55 ` Heiner Kallweit
2018-07-30 22:42 ` Bjorn Helgaas
2018-07-30 22:50 ` Thomas Gleixner
2018-07-31 7:29 ` Lee Jones
2018-07-31 8:01 ` Russell King - ARM Linux
2018-07-31 11:13 ` Bjorn Helgaas [this message]
2018-07-31 8:08 ` Krzysztof Kozlowski
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=20180731111317.GJ45322@bhelgaas-glaptop.roam.corp.google.com \
--to=helgaas@kernel.org \
--cc=b.zolnierkie@samsung.com \
--cc=bhelgaas@google.com \
--cc=cw00.choi@samsung.com \
--cc=hch@lst.de \
--cc=hkallweit1@gmail.com \
--cc=krzk@kernel.org \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=milo.kim@ti.com \
--cc=myungjoo.ham@samsung.com \
--cc=sre@kernel.org \
--cc=tglx@linutronix.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: 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).