[PATCH] irqchip: armada: Fix chained per-cpu interrupts

[Maxime Ripard]

Hi,

On Thu, Feb 26, 2015 at 11:26:28AM +0100, Gregory CLEMENT wrote:
> Hi Maxime,
> 
> On 26/02/2015 10:55, Maxime Ripard wrote:
> > On the Cortex-A9-based Armada SoCs, the MPIC is not the primary interrupt
> > controller. Yet, it still has to handle some per-cpu interrupt.
> > 
> > To do so, it is chained with the GIC using a per-cpu interrupt. However, the
> > current code only call irq_set_chained_handler, which is called and enable that
> > interrupt only on the boot CPU, which means that the parent per-CPU interrupt
> > is never unmasked on the secondary CPUs, preventing the per-CPU interrupt to
> > actually work as expected.
> > 
> > This was not seen until now since the only MPIC PPI users were the Marvell
> > timers that were not working, but not used either since the system use the ARM
> > TWD by default, and the ethernet controllers, that are faking there interrupts
> > as SPI, and don't really expect to have interrupts on the secondary cores
> > anyway.
> > 
> > Add a CPU notifier that will enable the PPI on the secondary cores when they
> > are brought up.
> > 
> > Cc: <stable at kernel.org> # 3.15+
> > Signed-off-by: Maxime Ripard <maxime.ripard at free-electrons.com>
> > ---
> >  drivers/irqchip/irq-armada-370-xp.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
> > index 463c235acbdc..137ee37a33ed 100644
> > --- a/drivers/irqchip/irq-armada-370-xp.c
> > +++ b/drivers/irqchip/irq-armada-370-xp.c
> > @@ -69,6 +69,7 @@ static void __iomem *per_cpu_int_base;
> >  static void __iomem *main_int_base;
> >  static struct irq_domain *armada_370_xp_mpic_domain;
> >  static u32 doorbell_mask_reg;
> > +static int parent_irq;
> >  #ifdef CONFIG_PCI_MSI
> >  static struct irq_domain *armada_370_xp_msi_domain;
> >  static DECLARE_BITMAP(msi_used, PCI_MSI_DOORBELL_NR);
> > @@ -356,6 +357,7 @@ static int armada_xp_mpic_secondary_init(struct notifier_block *nfb,
> >  {
> >  	if (action == CPU_STARTING || action == CPU_STARTING_FROZEN)
> >  		armada_xp_mpic_smp_cpu_init();
> > +
> >  	return NOTIFY_OK;
> >  }
> >  
> > @@ -364,6 +366,20 @@ static struct notifier_block armada_370_xp_mpic_cpu_notifier = {
> >  	.priority = 100,
> >  };
> >  
> The following function is called as soon as the MPIC is used as a secondary
> interrupt controller. So it will be the case for the Armada 375 and Armada 39x too. It
> also seems to not be related to be used in an SoC or an other, so I think that the
> function name is misleading. What about just using mpic_secondary_init and
> mpic_cpu_notifier ?
> 
> I know we prefixed the mpic function with armada_370_xp or armada_xp, but looking
> back, it was a mistake.

I don't know, that code needs to be run only in the cases where the
MPIC is a secondary interrupt controller, which rules out the armada
370/XP.

I was trying to make such a distinction, but indeed the wording is
quite poor.

Do you have some suggestions?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/4bbcca38/attachment.sig>