Re: [PATCH 2/4] irqchip/armada-370-xp: Exit ipi_resume() early if IPI is not used

["Marek Behún" <kabel@kernel.org>]

On Wed, 19 Jun 2024 19:11:44 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> On Wed, Jun 19, 2024 at 04:11:32PM +0200, Marek Behún wrote:
> > From: Pali Rohár <pali@kernel.org>
> > 
> > Exit ipi_resume() early if IPI is not used.
> > 
> > IPI is used only on systems where the mpic controller does not have a
> > parent GIC IRQ (e.g. on Armada XP).
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > [ changed commit message and moved the code change into ipi_resume() ]
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> >  drivers/irqchip/irq-armada-370-xp.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
> > index f488c35d9130..65f21624263e 100644
> > --- a/drivers/irqchip/irq-armada-370-xp.c
> > +++ b/drivers/irqchip/irq-armada-370-xp.c
> > @@ -29,6 +29,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/syscore_ops.h>
> >  #include <linux/msi.h>
> > +#include <linux/types.h>
> >  #include <asm/mach/arch.h>
> >  #include <asm/exception.h>
> >  #include <asm/smp_plat.h>
> > @@ -156,6 +157,12 @@ static DEFINE_MUTEX(msi_used_lock);
> >  static phys_addr_t msi_doorbell_addr;
> >  #endif
> >  
> > +static inline bool is_ipi_available(void)
> > +{
> > +	/* IPI is used only if we do not have parent irq */
> > +	return parent_irq <= 0;
> > +}
> > +
> >  static inline bool is_percpu_irq(irq_hw_number_t irq)
> >  {
> >  	if (irq <= ARMADA_370_XP_MAX_PER_CPU_IRQS)
> > @@ -429,6 +436,9 @@ static void ipi_resume(void)
> >  {
> >  	int i;
> >  
> > +	if (!is_ipi_available())
> > +		return;  
> 
> But of a nitpick, but it seems odd calling ipi_resume() if it does not
> exist. I would prefer
> 
>        
> 	if (is_ipi_available())
> 		ipi_resume();
> 
> At the two places ipi_resume() is called.

That is how it was in the original patch, just hardcoded (there
was no is_ipi_available() call, instead it was

  /* comment */
  if (parent_irq <= 0)
    ...

I guess I was trying to be smart in deduplicating code, and I got too
far the other way.

I will fix this in v2.

Marek