Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1251693 > unrolled thread

[PATCH 0/5] Fix regression introduced by set_irq_flags() removal

Started byThomas Petazzoni <thomas.petazzoni@free-electrons.com>
First post2015-10-20 15:30 +0200
Last post2015-10-20 16:10 +0200
Articles 11 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/5] Fix regression introduced by set_irq_flags() removal Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-10-20 15:30 +0200
    Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-10-20 16:10 +0200
      Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-20 16:20 +0200
        Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-10-20 16:30 +0200
        Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Gleixner <tglx@linutronix.de> - 2015-10-20 21:30 +0200
          Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Gleixner <tglx@linutronix.de> - 2015-10-22 10:10 +0200
      Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Gleixner <tglx@linutronix.de> - 2015-10-20 21:30 +0200
        [PATCH] irqchip: irq-armada-370-xp: fix regression by clearing IRQ_NOAUTOEN Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-10-21 15:50 +0200
          Re: [PATCH] irqchip: irq-armada-370-xp: fix regression by clearing  IRQ_NOAUTOEN Jason Cooper <jason@lakedaemon.net> - 2015-10-21 16:50 +0200
        Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-10-21 15:50 +0200
    Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal Jason Cooper <jason@lakedaemon.net> - 2015-10-20 16:10 +0200

#1251693 — [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

FromThomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date2015-10-20 15:30 +0200
Subject[PATCH 0/5] Fix regression introduced by set_irq_flags() removal
Message-ID<qlF9g-4YG-13@gated-at.bofh.it>
Thomas, Jason, Marc, Rob,

In commit d17cab4451df ("irqchip: Kill off set_irq_flags usage"), Rob
Herring modified the irqchip drivers to not use the ARM-specific
set_irq_flags() and instead rely on various functions provided by the
core irq subsystem.

While his commit was supposed to have no functional effect, it in fact
does have one effect: the IRQ_NOAUTOEN flag used to be *cleared* for
all interrupts and it is now *set* by default.

Thanks to this flag being *cleared* by default, the irq-armada-370-xp
was able to properly re-enable per-CPU interrupts at resume time. Now
that this flag is *set*, the irqd_irq_disabled() function no longer
indicates that such per-CPU interrupts are enabled (and in fact a
CPU-global flag to tell whether a per-CPU is enabled or not is
silly). Due to this, our local timer per-CPU interrupt is no longer
re-enabled at resume time on Armada XP, on the boot CPU, which causes
a hang at resume time.

This is a regression between 4.2 (where suspend/resume works fine) and
4.3-rc (where suspend/resume is broken). Reverting d17cab4451df1 on
top of 4.3-rc makes the problem go away (of course you also need to
revert eb811129ed9ea so that set_irq_flags is re-introduced).

The minimal fix would be to clear the IRQ_NOAUTOEN flag so that we get
back to the original situation. However, this does not really seem
like the right fix.

Instead, this patch series proposes to add an is_enabled_percpu_irq()
function to the core irq subsystem, which is then used by the
irq-armada-370-xp to find out if such or such per-CPU interrupt should
be re-enabled at resume time on the boot CPU.

The organization of the patch series is as follows:

 - PATCH 1 introduces the is_enabled_percpu_irq() function.

 - PATCH 2 does a minor refactoring of armada_xp_mpic_secondary_init()
   to prepare the following patch.

 - PATCH 3 changes the irq-armada-370-xp driver to use the
   is_enabled_percpu_irq() to re-enable the per-CPU interrupts on the
   boot CPU at resume time, and also modifies the secondary CPU
   notifier to re-enable per-CPU interrupts if needed.

 - PATCH 4 and 5 are further cleanups/improvements to the
   irq-armada-370-xp, which are not needed to fix the problem.

Since this is fixing a regression introduced between 4.2 and 4.3-rc,
it would be great if patches 1 to 3 could be merged in 4.3. The last
two patches are only cosmetic, so merging them for 4.4 is of course
the way to go.

Thanks,

Thomas

Thomas Petazzoni (5):
  kernel: irq: implement is_enabled_percpu_irq()
  irqchip: armada-370-xp: prepare additions to
    armada_xp_mpic_secondary_init()
  irqchip: armada-370-xp: re-enable per-CPU interrupts at resume time
  irqchip: armada-370-xp: re-order register definitions
  irqchip: armada-370-xp: document the overall driver logic

 drivers/irqchip/irq-armada-370-xp.c | 151 +++++++++++++++++++++++++++++++-----
 include/linux/interrupt.h           |   1 +
 kernel/irq/chip.c                   |   5 ++
 kernel/irq/internals.h              |   1 +
 kernel/irq/manage.c                 |  19 +++++
 5 files changed, 158 insertions(+), 19 deletions(-)

-- 
2.6.2

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

[toc] | [next] | [standalone]


#1251720 — Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

FromThomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date2015-10-20 16:10 +0200
SubjectRe: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal
Message-ID<qlFLX-5WV-3@gated-at.bofh.it>
In reply to#1251693
Jason,

On Tue, 20 Oct 2015 14:04:27 +0000, Jason Cooper wrote:

> Whole series,
> 
> Reviewed by: Jason Cooper <jason@lakedaemon.net>

Thanks.

> Thomas (tglx), if you're happy with the core changes, let me know and
> I'll cue the series up.
> 
> We know it's a bit late in the -rc cycle, but the alternative (reverting
> Rob's patch) would likely be more destabilizing at this point.

As discussed on IRC, another simpler (code line wise) solution is to
simply clear the IRQ_NOAUTOEN flag in the irq-armada-370-xp, which
brings us back to what set_irq_flags() was doing, without actually
reverting Rob's patch.

However, relying on IRQ_NOAUTOEN being cleared doesn't seem like the
right long term solution, which is why I implemented what I believe is
a (hopefully) better long term solution.

But if you/tglx want something simple/stupid for 4.3 (one-line patch)
and then the real stuff for 4.4, I'd be happy to provide a series
implementing this idea.

Let me know what you and tglx think.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251737

FromRussell King - ARM Linux <linux@arm.linux.org.uk>
Date2015-10-20 16:20 +0200
Message-ID<qlFVF-69D-35@gated-at.bofh.it>
In reply to#1251720
On Tue, Oct 20, 2015 at 04:08:28PM +0200, Thomas Petazzoni wrote:
> As discussed on IRC, another simpler (code line wise) solution is to
> simply clear the IRQ_NOAUTOEN flag in the irq-armada-370-xp, which
> brings us back to what set_irq_flags() was doing, without actually
> reverting Rob's patch.
> 
> However, relying on IRQ_NOAUTOEN being cleared doesn't seem like the
> right long term solution, which is why I implemented what I believe is
> a (hopefully) better long term solution.

However, this is rather worrying.  NOAUTOEN is supposed to avoid enabling
the interrupt when the interrupt is claimed.

If, as a result of Rob's patch, we now have a load of IRQs which are
marked with NOAUTOEN which weren't, that's quite a large regression -
possibly one which hasn't been properly found (not everyone tests -rc
kernels) and we may be better to revert Rob's patch to avoid lots of
breakge being reported when 4.3 is released.

I think Rob's patches need another review in light of this, to determine
how much breakage there is here, and a decision how to proceed made on
that basis.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251748 — Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

FromThomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date2015-10-20 16:30 +0200
SubjectRe: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal
Message-ID<qlG5k-6l5-25@gated-at.bofh.it>
In reply to#1251737
Russell,

On Tue, 20 Oct 2015 15:17:36 +0100, Russell King - ARM Linux wrote:

> However, this is rather worrying.  NOAUTOEN is supposed to avoid enabling
> the interrupt when the interrupt is claimed.
> 
> If, as a result of Rob's patch, we now have a load of IRQs which are
> marked with NOAUTOEN which weren't, that's quite a large regression -
> possibly one which hasn't been properly found (not everyone tests -rc
> kernels) and we may be better to revert Rob's patch to avoid lots of
> breakge being reported when 4.3 is released.

I believe the problem is only for per-CPU interrupts. We have
IRQ_NOAUTOEN set for per-CPU interrupts because:

static inline void irq_set_percpu_devid_flags(unsigned int irq)
{
        irq_set_status_flags(irq,
                             IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
                             IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
}

Calling set_irq_flags() used to have the effect of *clearing* the
IRQ_NOAUTOEN flag:

-void set_irq_flags(unsigned int irq, unsigned int iflags)
-{
-       unsigned long clr = 0, set = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
-
-       if (irq >= nr_irqs) {
-               pr_err("Trying to set irq flags for IRQ%d\n", irq);
-               return;
-       }
-
-       if (iflags & IRQF_VALID)
-               clr |= IRQ_NOREQUEST;
-       if (iflags & IRQF_PROBE)
-               clr |= IRQ_NOPROBE;
-       if (!(iflags & IRQF_NOAUTOEN))
-               clr |= IRQ_NOAUTOEN;
-       /* Order is clear bits in "clr" then set bits in "set" */
-       irq_modify_status(irq, clr, set & ~clr);
-}

I.e, unless you were passing IRQF_NOAUTOEN in your set_irq_flags()
invocation, set_irq_flags() was automatically clearing the IRQ_NOAUTOEN
bit.

But this is really only a per-CPU interrupt problem, which probably
limits the potential regressions caused by Rob's change.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251983 — Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

FromThomas Gleixner <tglx@linutronix.de>
Date2015-10-20 21:30 +0200
SubjectRe: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal
Message-ID<qlKLD-4Jw-3@gated-at.bofh.it>
In reply to#1251737
On Tue, 20 Oct 2015, Russell King - ARM Linux wrote:

> On Tue, Oct 20, 2015 at 04:08:28PM +0200, Thomas Petazzoni wrote:
> > As discussed on IRC, another simpler (code line wise) solution is to
> > simply clear the IRQ_NOAUTOEN flag in the irq-armada-370-xp, which
> > brings us back to what set_irq_flags() was doing, without actually
> > reverting Rob's patch.
> > 
> > However, relying on IRQ_NOAUTOEN being cleared doesn't seem like the
> > right long term solution, which is why I implemented what I believe is
> > a (hopefully) better long term solution.
> 
> However, this is rather worrying.  NOAUTOEN is supposed to avoid enabling
> the interrupt when the interrupt is claimed.
> 
> If, as a result of Rob's patch, we now have a load of IRQs which are
> marked with NOAUTOEN which weren't, that's quite a large regression -
> possibly one which hasn't been properly found (not everyone tests -rc
> kernels) and we may be better to revert Rob's patch to avoid lots of
> breakge being reported when 4.3 is released.
> 
> I think Rob's patches need another review in light of this, to determine
> how much breakage there is here, and a decision how to proceed made on
> that basis.

I'll go over them tomorrow again and decide then what to do.

Thanks,

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

[toc] | [prev] | [next] | [standalone]


#1253569 — Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

FromThomas Gleixner <tglx@linutronix.de>
Date2015-10-22 10:10 +0200
SubjectRe: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal
Message-ID<qmj6G-4MD-23@gated-at.bofh.it>
In reply to#1251983
On Tue, 20 Oct 2015, Thomas Gleixner wrote:
> On Tue, 20 Oct 2015, Russell King - ARM Linux wrote:
> > I think Rob's patches need another review in light of this, to determine
> > how much breakage there is here, and a decision how to proceed made on
> > that basis.
> 
> I'll go over them tomorrow again and decide then what to do.

I went over them once more and the only affected wreck I found is the
armada one.

Thanks,

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

[toc] | [prev] | [next] | [standalone]


#1251981 — Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

FromThomas Gleixner <tglx@linutronix.de>
Date2015-10-20 21:30 +0200
SubjectRe: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal
Message-ID<qlKLD-4Jw-1@gated-at.bofh.it>
In reply to#1251720
On Tue, 20 Oct 2015, Thomas Petazzoni wrote:
> As discussed on IRC, another simpler (code line wise) solution is to
> simply clear the IRQ_NOAUTOEN flag in the irq-armada-370-xp, which
> brings us back to what set_irq_flags() was doing, without actually
> reverting Rob's patch.

I prefer that for 4.3.
 
> However, relying on IRQ_NOAUTOEN being cleared doesn't seem like the
> right long term solution, which is why I implemented what I believe is
> a (hopefully) better long term solution.

Agreed.

I'll go over the proposed solution tomorrow afternoon (I'm
traveling/conferencing...)

Thanks,

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

[toc] | [prev] | [next] | [standalone]


#1252835 — [PATCH] irqchip: irq-armada-370-xp: fix regression by clearing IRQ_NOAUTOEN

FromThomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date2015-10-21 15:50 +0200
Subject[PATCH] irqchip: irq-armada-370-xp: fix regression by clearing IRQ_NOAUTOEN
Message-ID<qm1Wa-4CG-9@gated-at.bofh.it>
In reply to#1251981
Commit d17cab4451df1 ("irqchip: Kill off set_irq_flags usage") changed
the code of armada_370_xp_mpic_irq_map() from using set_irq_flags() to
irq_set_probe().

While the commit log seems to imply that there are no functional
changes, there are indeed functional changes introduced by this
commit: the IRQ_NOAUTOEN flag is no longer cleared. This functional
change causes a regression on Armada XP, which no longer works
properly after suspend/resume because per-CPU interrupts remain
disabled.

Due to how the hardware registers work, the irq-armada-370-xp cannot
simply save/restore a bunch of registers at suspend/resume to make
sure that the interrupts remain in the same state after
resuming. Therefore, it relies on the kernel to say whether the
interrupt is disabled or not, using the irqd_irq_disabled()
function. This was all working fine while the IRQ_NOAUTOEN flag was
cleared.

With the change introduced by Rob Herring in d17cab4451df1, the
IRQ_NOAUTOEN flag is now set for all interrupts. irqd_irq_disabled()
returns false for per-CPU interrupts, and therefore our per-CPU
interrupts are no longer re-enabled after resume.

This commit works around this problem by clearing again the
IRQ_NOAUTOEN flags, so that we are back to the situation we had before
commit d17cab4451df1. This work around is proposed as a minimal fix
for the problem, while a better long-term solution is being worked on.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Here is the one-line minimal change that tglx said would be more
acceptable to have in 4.3.
---
 drivers/irqchip/irq-armada-370-xp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 655cb96..389318a 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -317,6 +317,7 @@ static int armada_370_xp_mpic_irq_map(struct irq_domain *h,
 					handle_level_irq);
 	}
 	irq_set_probe(virq);
+	irq_clear_status_flags(virq, IRQ_NOAUTOEN);
 
 	return 0;
 }
-- 
2.6.2

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

[toc] | [prev] | [next] | [standalone]


#1252890 — Re: [PATCH] irqchip: irq-armada-370-xp: fix regression by clearing IRQ_NOAUTOEN

FromJason Cooper <jason@lakedaemon.net>
Date2015-10-21 16:50 +0200
SubjectRe: [PATCH] irqchip: irq-armada-370-xp: fix regression by clearing IRQ_NOAUTOEN
Message-ID<qm2Se-606-33@gated-at.bofh.it>
In reply to#1252835
Hey Thomas-i, Thomases, :)

On Wed, Oct 21, 2015 at 03:48:15PM +0200, Thomas Petazzoni wrote:
> Commit d17cab4451df1 ("irqchip: Kill off set_irq_flags usage") changed
> the code of armada_370_xp_mpic_irq_map() from using set_irq_flags() to
> irq_set_probe().
> 
> While the commit log seems to imply that there are no functional
> changes, there are indeed functional changes introduced by this
> commit: the IRQ_NOAUTOEN flag is no longer cleared. This functional
> change causes a regression on Armada XP, which no longer works
> properly after suspend/resume because per-CPU interrupts remain
> disabled.
> 
> Due to how the hardware registers work, the irq-armada-370-xp cannot
> simply save/restore a bunch of registers at suspend/resume to make
> sure that the interrupts remain in the same state after
> resuming. Therefore, it relies on the kernel to say whether the
> interrupt is disabled or not, using the irqd_irq_disabled()
> function. This was all working fine while the IRQ_NOAUTOEN flag was
> cleared.
> 
> With the change introduced by Rob Herring in d17cab4451df1, the
> IRQ_NOAUTOEN flag is now set for all interrupts. irqd_irq_disabled()
> returns false for per-CPU interrupts, and therefore our per-CPU
> interrupts are no longer re-enabled after resume.
> 
> This commit works around this problem by clearing again the
> IRQ_NOAUTOEN flags, so that we are back to the situation we had before
> commit d17cab4451df1. This work around is proposed as a minimal fix
> for the problem, while a better long-term solution is being worked on.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> Here is the one-line minimal change that tglx said would be more
> acceptable to have in 4.3.
> ---
>  drivers/irqchip/irq-armada-370-xp.c | 1 +
>  1 file changed, 1 insertion(+)

Applied to irqchip/urgent with minor tweaking of the subject line.  I
trust at this point it's ok to pull in -rc2.

thx,

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

[toc] | [prev] | [next] | [standalone]


#1252837 — Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

FromThomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date2015-10-21 15:50 +0200
SubjectRe: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal
Message-ID<qm1Wb-4CG-25@gated-at.bofh.it>
In reply to#1251981
Thomas,

On Tue, 20 Oct 2015 21:23:29 +0200 (CEST), Thomas Gleixner wrote:
> On Tue, 20 Oct 2015, Thomas Petazzoni wrote:
> > As discussed on IRC, another simpler (code line wise) solution is to
> > simply clear the IRQ_NOAUTOEN flag in the irq-armada-370-xp, which
> > brings us back to what set_irq_flags() was doing, without actually
> > reverting Rob's patch.
> 
> I prefer that for 4.3.

Done, I've proposed a patch as a reply to this e-mail. It's a one-liner.

> > However, relying on IRQ_NOAUTOEN being cleared doesn't seem like the
> > right long term solution, which is why I implemented what I believe is
> > a (hopefully) better long term solution.
> 
> Agreed.
> 
> I'll go over the proposed solution tomorrow afternoon (I'm
> traveling/conferencing...)

Sure, no problem.

I also noticed that MSI interrupts are not properly re-enabled after
resume, but it's not a regression from 4.2, as I can also reproduce the
issue on 4.2, so I'll handle that separately.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1251722

FromJason Cooper <jason@lakedaemon.net>
Date2015-10-20 16:10 +0200
Message-ID<qlFLX-5WV-5@gated-at.bofh.it>
In reply to#1251693
Thomas (tglx also),

On Tue, Oct 20, 2015 at 03:23:50PM +0200, Thomas Petazzoni wrote:
> Thomas, Jason, Marc, Rob,
> 
> In commit d17cab4451df ("irqchip: Kill off set_irq_flags usage"), Rob
> Herring modified the irqchip drivers to not use the ARM-specific
> set_irq_flags() and instead rely on various functions provided by the
> core irq subsystem.
> 
> While his commit was supposed to have no functional effect, it in fact
> does have one effect: the IRQ_NOAUTOEN flag used to be *cleared* for
> all interrupts and it is now *set* by default.
> 
> Thanks to this flag being *cleared* by default, the irq-armada-370-xp
> was able to properly re-enable per-CPU interrupts at resume time. Now
> that this flag is *set*, the irqd_irq_disabled() function no longer
> indicates that such per-CPU interrupts are enabled (and in fact a
> CPU-global flag to tell whether a per-CPU is enabled or not is
> silly). Due to this, our local timer per-CPU interrupt is no longer
> re-enabled at resume time on Armada XP, on the boot CPU, which causes
> a hang at resume time.
> 
> This is a regression between 4.2 (where suspend/resume works fine) and
> 4.3-rc (where suspend/resume is broken). Reverting d17cab4451df1 on
> top of 4.3-rc makes the problem go away (of course you also need to
> revert eb811129ed9ea so that set_irq_flags is re-introduced).
> 
> The minimal fix would be to clear the IRQ_NOAUTOEN flag so that we get
> back to the original situation. However, this does not really seem
> like the right fix.
> 
> Instead, this patch series proposes to add an is_enabled_percpu_irq()
> function to the core irq subsystem, which is then used by the
> irq-armada-370-xp to find out if such or such per-CPU interrupt should
> be re-enabled at resume time on the boot CPU.
> 
> The organization of the patch series is as follows:
> 
>  - PATCH 1 introduces the is_enabled_percpu_irq() function.
> 
>  - PATCH 2 does a minor refactoring of armada_xp_mpic_secondary_init()
>    to prepare the following patch.
> 
>  - PATCH 3 changes the irq-armada-370-xp driver to use the
>    is_enabled_percpu_irq() to re-enable the per-CPU interrupts on the
>    boot CPU at resume time, and also modifies the secondary CPU
>    notifier to re-enable per-CPU interrupts if needed.
> 
>  - PATCH 4 and 5 are further cleanups/improvements to the
>    irq-armada-370-xp, which are not needed to fix the problem.
> 
> Since this is fixing a regression introduced between 4.2 and 4.3-rc,
> it would be great if patches 1 to 3 could be merged in 4.3. The last
> two patches are only cosmetic, so merging them for 4.4 is of course
> the way to go.
> 
> Thanks,
> 
> Thomas
> 
> Thomas Petazzoni (5):
>   kernel: irq: implement is_enabled_percpu_irq()
>   irqchip: armada-370-xp: prepare additions to
>     armada_xp_mpic_secondary_init()
>   irqchip: armada-370-xp: re-enable per-CPU interrupts at resume time
>   irqchip: armada-370-xp: re-order register definitions
>   irqchip: armada-370-xp: document the overall driver logic
> 
>  drivers/irqchip/irq-armada-370-xp.c | 151 +++++++++++++++++++++++++++++++-----
>  include/linux/interrupt.h           |   1 +
>  kernel/irq/chip.c                   |   5 ++
>  kernel/irq/internals.h              |   1 +
>  kernel/irq/manage.c                 |  19 +++++
>  5 files changed, 158 insertions(+), 19 deletions(-)

Whole series,

Reviewed by: Jason Cooper <jason@lakedaemon.net>


Thomas (tglx), if you're happy with the core changes, let me know and
I'll cue the series up.

We know it's a bit late in the -rc cycle, but the alternative (reverting
Rob's patch) would likely be more destabilizing at this point.

thx,

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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web