public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: tglx@linutronix.de (Thomas Gleixner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/5] irqchip: add dumb demultiplexer implementation
Date: Wed, 14 Jan 2015 11:36:09 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.11.1501141110190.5526@nanos> (raw)
In-Reply-To: <CAL_JsqJ5DSo6jmAuHPDdV6pMca8BVbg5V-GZ5=2qu96_QYXMtQ@mail.gmail.com>

On Tue, 13 Jan 2015, Rob Herring wrote:
> On Tue, Jan 13, 2015 at 12:46 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Some interrupt controllers are multiplexing several peripheral IRQs on
> > a single interrupt line.
> > While this is not a problem for most IRQs (as long as all peripherals
> > request the interrupt with IRQF_SHARED flag set), multiplexing timers and
> > other type of peripherals will generate a WARNING (mixing IRQF_NO_SUSPEND
> > and !IRQF_NO_SUSPEND is prohibited).
> >
> > Create a dumb irq demultiplexer which simply forwards interrupts to all
> > peripherals (exactly what's happening with IRQ_SHARED) but keep a unique
> > irq number for each peripheral, thus preventing the IRQF_NO_SUSPEND
> > and !IRQF_NO_SUSPEND mix on a given interrupt.
> 
> This really seems like a work-around for how IRQF_SHARED works. It

It's a workaround for a short coming of IRQF_SHARED.

IRQF_SHARED has a massive short coming versus suspend and wakeup
interrupts. If one of the demultiplexed interrupts is a valid wakeup
source then we have no sane way to express this with IRQF_SHARED
simply because the drivers need to be aware whether they run on stupid
or well designed hardware.

> seems like what is really desired is just per handler disabling. It is

So you want a magic API like disable/enable_irq_action()?

Certainly not.

You'd open just another can of worms which will bring us abuse and
hard to debug problems because driver writers think it's a good idea
to use it for random purposes.

Aside of that it would add another conditional into the interrupt
delivery hotpath which is not desired either.

> fragile in that devices can deadlock the system if the drivers don't
> disable the interrupt source before calling disable_irq. But unlike

Any misdesigned driver can do that for you.

> IRQF_SHARED, there is nothing explicit in the driver indicating it is
> designed to work properly with a shared interrupt line.

IRQF_SHARED is a pretty bad indicator. Look at all the drivers which
slap this flag onto request_irq() and have no single line of code
which makes sure that the driver would ever work on a shared line.

If it's just for annotational purposes, we can add a new IRQF flag,
which is required to request such a interrupt line.

> I see no reason to accept this into DT either. We already can support
> shared lines and modeling an OR gate as an interrupt controller is
> pointless.

It's absolutely not pointless.

All attempts to work around that have resulted in horrible bandaids so
far. That's why I guided Boris to implement this dummy demultiplexing
mechanism. It solves the problem at hand nicely without adding nasty
hackarounds into the suspend/resume code and inflicting platform
knowledge on multi-platform device drivers.

If you have a proper solution for the problem at hand which

   - avoids the demux dummy

   - works straight forward with suspend/resume/wakeup

   - does not add horrible new APIs

   - does not add conditionals to the interrupt hotpath

   - does not inflict platform knowledge about interrupt chip details
     on drivers

then I'm happy to take it.

But as long as you can't come up with anything sane, the demux dummy
is the best solution for this problem we've seen so far.

Thanks,

	tglx

  parent reply	other threads:[~2015-01-14 10:36 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-13 18:46 [PATCH v2 0/5] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
2015-01-13 18:46 ` [PATCH v2 1/5] irqchip: add dumb demultiplexer implementation Boris Brezillon
2015-01-13 21:00   ` Thomas Gleixner
2015-01-14  8:31     ` Boris Brezillon
2015-01-14  3:26   ` Rob Herring
2015-01-14  8:22     ` Boris Brezillon
2015-01-14 10:36     ` Thomas Gleixner [this message]
2015-01-14 22:24       ` Rob Herring
2015-01-14 22:55         ` Boris Brezillon
2015-01-15  9:44           ` Nicolas Ferre
2015-01-15  9:11         ` Thomas Gleixner
2015-01-15  9:26           ` Nicolas Ferre
2015-01-15 15:40           ` Rob Herring
2015-01-20 13:08             ` Thomas Gleixner
2015-01-14 13:36   ` Nicolas Ferre
2015-01-14 14:03     ` Boris Brezillon
2015-01-14 14:43       ` Nicolas Ferre
2015-01-13 18:46 ` [PATCH v2 2/5] irqchip: Add DT binding doc for dumb demuxer chips Boris Brezillon
2015-01-13 19:00   ` Jason Cooper
2015-01-13 20:52     ` Boris Brezillon
2015-01-14 18:56       ` Jason Cooper
2015-01-14 19:08         ` Boris Brezillon
2015-01-14 19:33           ` Jason Cooper
2015-01-14 13:42   ` Nicolas Ferre
2015-01-13 18:46 ` [PATCH v2 3/5] ARM: at91/dt: select DUMB_IRQ_DEMUX for all at91 SoCs Boris Brezillon
2015-01-14 13:45   ` Nicolas Ferre
2015-01-13 18:46 ` [PATCH v2 4/5] ARM: at91/dt: add AIC irq1 muxed peripheral id definitions Boris Brezillon
2015-01-14 13:21   ` Nicolas Ferre
2015-01-14 13:34     ` Boris Brezillon
2015-01-14 13:40       ` Nicolas Ferre
2015-01-13 18:46 ` [PATCH v2 5/5] ARM: at91/dt: define a dumb irq demultiplexer chip connected on irq1 Boris Brezillon
2015-01-14 13:48   ` Nicolas Ferre

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=alpine.DEB.2.11.1501141110190.5526@nanos \
    --to=tglx@linutronix.de \
    --cc=linux-arm-kernel@lists.infradead.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