linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Nicolas Ferre <nicolas.ferre@atmel.com>,
	Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>,
	Alexandre Belloni <alexandre.belloni@free-electrons.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <Pawel.Moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"lee.jones@linaro.org" <lee.jones@linaro.org>
Subject: Re: [PATCH v4 3/5] irqchip: Add DT binding doc for the virtual irq demuxer chip
Date: Fri, 20 Feb 2015 10:31:44 +0000	[thread overview]
Message-ID: <20150220103144.GI13659@leverpostej> (raw)
In-Reply-To: <5225828.OI8hbGqrC4@vostro.rjw.lan>

[...]

> > > IRQF_NO_SUSPEND and wakeup fundamentally don't match due to the way
> > > wakeup is implemented in the IRQ core now.
> > > 
> > > Unless drivers with IRQF_NO_SUSPEND do the wakeup behind the core's back
> > > which is just disgusting and should never happen.
> > 
> > I completely agree that using IRQF_NO_SUSPEND in that manner is a
> > disgusting abuse. It seems like some drivers are abusing it for wakeup,
> > and those need to be corrected.
> > 
> > If those are corrected, the same issue with mismatch will happen with
> > those wakeup interrupts, and we need to fix that somehow given people
> > seem to already be relying on being able to share a line with a wakeup
> > device.
> 
> The way we handle wakeup interrupts in the core prevents that, because
> wakeup is handled at the IRQ level rather than at a handler (irqaction)
> level.  Interrupt handlers from irqactions are not run for wakeup
> interrupts at all after suspend_device_irqs(), so if you have anyone
> sharing an IRQ with a wakeup source, their interrupt handler won't be
> run anyway then.

I had missed that fact; thank you for correcting me!

> > I propose we add a new IRQF_BIKESHED, meaning that this interrupt line
> > may be shared with an IRQF_NO_SUSPEND or wakeup interrupt handler.
> 
> As I said, sharing an IRQ with a wakeup source is OK (worst case you'll
> cause spurious wakeup interrupts to occur if your device is not suspended
> properly).  The problem is *entirely* about IRQF_NO_SUSPEND.

I see that now.

> So I claim that the only things having legitimate reasons to ever use
> IRQF_NO_SUSPEND are (a) timers and (b) IPIs.  There really are no other
> cases to worry about in my view, but I may be overlooking something.

Assuming you also include any chained irqchip handlers necessary for
those timers or IPIs, that makes sense to me.

> Anyway, I wouldn't like to add flags allowing driver writers to do fishy
> things in a hush-hush manner.  The warning trigger is there for a good
> reason and if we allow it to be avoided, that has to be for a good reason
> too.  In other words, it shouldn't be too easy to do that.

Sure.

Given all of the above I'll go back to the IRQF_SHARED_TIMER_OK approach
you proposed, along with documentation updates and comments at usage
sites to make it clear when it is valid to use.

Thank you for bearing with me so far.

Mark.

  reply	other threads:[~2015-02-20 10:32 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-29 10:33 [PATCH v4 0/5] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
2015-01-29 10:33 ` [PATCH v4 1/5] genirq: Authorize chained handlers to remain disabled when initialized Boris Brezillon
2015-01-29 10:33 ` [PATCH v4 2/5] irqchip: add virtual demultiplexer implementation Boris Brezillon
2015-02-10 15:00   ` Peter Zijlstra
2015-02-10 15:20     ` Boris Brezillon
2015-02-10 15:43     ` [PATCH] genirq: fix virtual irq demuxer related comments Boris Brezillon
2015-02-10 16:14       ` Peter Zijlstra
2015-02-20 16:12         ` Mark Rutland
2015-02-20 16:17           ` Peter Zijlstra
2015-02-10 15:48   ` [PATCH v4 2/5] irqchip: add virtual demultiplexer implementation Mark Rutland
2015-01-29 10:33 ` [PATCH v4 3/5] irqchip: Add DT binding doc for the virtual irq demuxer chip Boris Brezillon
2015-02-10 15:36   ` Mark Rutland
2015-02-10 15:52     ` Boris Brezillon
2015-02-10 16:06       ` Boris Brezillon
2015-02-10 16:16       ` Mark Rutland
2015-02-10 16:20         ` Boris Brezillon
2015-02-10 20:48       ` Mark Rutland
2015-02-11  8:53         ` Boris Brezillon
2015-02-11 11:11           ` Mark Rutland
2015-02-11 12:24             ` Boris Brezillon
2015-02-11 12:36               ` Mark Rutland
2015-02-11 13:38                 ` Alexandre Belloni
2015-02-11 13:48                   ` Mark Rutland
2015-02-11 14:55               ` Rafael J. Wysocki
2015-02-11 14:43                 ` Mark Rutland
2015-02-11 15:17                   ` Rafael J. Wysocki
2015-02-11 15:03                     ` Boris Brezillon
2015-02-11 15:39                       ` Rafael J. Wysocki
2015-02-11 15:23                         ` Mark Rutland
2015-02-11 15:12                     ` Mark Rutland
2015-02-11 15:51                       ` Rafael J. Wysocki
2015-02-11 15:57                         ` Mark Rutland
2015-02-11 16:15                           ` Boris Brezillon
2015-02-11 16:32                             ` Mark Rutland
2015-02-11 16:38                               ` Boris Brezillon
2015-02-11 17:17                                 ` Mark Rutland
2015-02-20 14:22                                 ` Mark Rutland
2015-02-20 14:53                                   ` Boris Brezillon
2015-02-20 15:16                                     ` Mark Rutland
2015-02-23 17:00                                       ` Boris Brezillon
2015-02-23 18:14                                         ` Mark Rutland
2015-02-23 20:16                                           ` Boris Brezillon
2015-02-11 16:42                             ` Rafael J. Wysocki
2015-02-11 16:28                               ` Boris Brezillon
2015-02-11 17:13                               ` Mark Rutland
2015-02-11 17:29                                 ` Boris Brezillon
2015-02-12 10:52                                   ` Mark Rutland
2015-02-12 11:09                                     ` Boris Brezillon
2015-02-12 11:23                                       ` Mark Rutland
2015-02-16  9:49                                 ` Peter Zijlstra
2015-02-16  9:28                         ` Peter Zijlstra
2015-02-16 12:23                           ` Mark Rutland
2015-02-19  1:16                             ` Rafael J. Wysocki
2015-02-19 11:23                               ` Mark Rutland
2015-02-19 22:35                                 ` Rafael J. Wysocki
2015-02-20 10:31                                   ` Mark Rutland [this message]
2015-02-24  1:02                                     ` Rafael J. Wysocki
2015-02-24  8:42                                       ` Boris Brezillon
2015-02-11 14:45                 ` Boris Brezillon
2015-02-11 14:39             ` Rafael J. Wysocki
2015-02-11  9:11         ` Peter Zijlstra
2015-02-11 11:15           ` Mark Rutland
2015-02-11 14:31             ` Rafael J. Wysocki
2015-02-11 14:14               ` Mark Rutland
2015-02-11 15:07                 ` Rafael J. Wysocki
2015-02-11 15:03                   ` Mark Rutland
2015-02-11 14:34         ` Rafael J. Wysocki
2015-01-29 10:33 ` [PATCH v4 4/5] ARM: at91/dt: select VIRT_IRQ_DEMUX for all at91 SoCs Boris Brezillon
2015-01-29 10:33 ` [PATCH v4 5/5] ARM: at91/dt: define a virtual irq demultiplexer chip connected on irq1 Boris Brezillon
2015-02-09 15:47 ` [PATCH v4 0/5] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon

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=20150220103144.GI13659@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=Pawel.Moll@arm.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jason@lakedaemon.net \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolas.ferre@atmel.com \
    --cc=peterz@infradead.org \
    --cc=plagnioj@jcrosoft.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@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).