Re: [PATCH v2 1/5] irqchip: add dumb demultiplexer implementation

From: Thomas Gleixner
Date: Thu Jan 15 2015 - 04:12:38 EST


On Wed, 14 Jan 2015, Rob Herring wrote:
> On Wed, Jan 14, 2015 at 4:36 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> > 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.
>
> This change will break on old kernels with a new dtb. Would you be
> happy if a BIOS update required a new kernel? Fixing this for any
> platform requires a dtb update which may not be possible on some
> platforms. I don't have a problem with this breakage for 1 platform
> and the at91 guys may not care, but we'd ultimately be changing how
> all shared irqs are specified for all DT. Maybe we decide that this is
> how we want to describe things, but that needs much wider discussion
> and agreement.

We do not change shared interrupts in any way. We provide an
alternative mechanism for braindead hardware. And if the at91 folks
are fine with the DT change, then it's their decision. Nothing forces
this on everyone.

> > 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.
>
> What if during suspend you move all actions w/o IRQF_NO_SUSPEND to a
> suspended action list? This would leave only the actions with
> IRQF_NO_SUSPEND set in the active action list. The cost would be a
> pointer in irq_desc and moving the actions during suspend and resume.

That's exactly what we want NOT. Because it prevents us to do proper
sanity checks for IRQF_NO_SUSPEND. We've been there and I rejected it
for that very reason.

> There are probably ways to do this demux irqchip without a DT change.

What's the problem with a DT change for a single platform, if the
maintainers are willing to take it and deal with the fallout?

And in case of AT91 the new kernel will simply work with the old DT
and just emit the same warning vs. the IRQF_NO_SUSPEND mismatch. Older
kernels won't work with a new DT, but that's about it.

Aside of that, I'm quite amused about your DT update worries. DTs
break with every kernel version on very popular platforms in very
weird and subtle ways.

Thanks,

tglx

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/