[v2,1/5] ARC: SMP: Use "unsigned virq" in smp_ipi_irq_setup instead of "signed irq"
diff mbox

Message ID 1477313194-2310-1-git-send-email-yuriy.kolerov@synopsys.com
State New
Headers show

Commit Message

Yuriy Kolerov Oct. 24, 2016, 12:46 p.m. UTC
This function takes a cpu number and a virq number and registers an
appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
used in several places of ARC platform code - hwirq is passed instead
of virq. This patch is intended to clarify declaration of virq argument
to prevent passing of hwirq instead of virq in future.

Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
---
 arch/arc/include/asm/smp.h | 4 ++--
 arch/arc/kernel/smp.c      | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Vineet Gupta Oct. 24, 2016, 8:06 p.m. UTC | #1
Hi Yuriy,

On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> This function takes a cpu number and a virq number and registers an
> appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
> used in several places of ARC platform code - hwirq is passed instead
> of virq. This patch is intended to clarify declaration of virq argument
> to prevent passing of hwirq instead of virq in future.
> 
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>

So this series is missing the cover letter which makes it a bit hard to gauge the
end goal.
It seems patch [1-3]/5 are fixes which can be applied right away.
And [4-5]/5 help fix the irq affinity setting. What this doesn't say is which
platforms does it fix. And do we not need the hierarchical irq domains, specially
for the AXS platform which has 3 interrupt controllers stacked up.

> ---
>  arch/arc/include/asm/smp.h | 4 ++--
>  arch/arc/kernel/smp.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
> index 89fdd1b..3ebebbc 100644
> --- a/arch/arc/include/asm/smp.h
> +++ b/arch/arc/include/asm/smp.h
> @@ -37,9 +37,9 @@ extern const char *arc_platform_smp_cpuinfo(void);
>   * API expected BY platform smp code (FROM arch smp code)
>   *
>   * smp_ipi_irq_setup:
> - *	Takes @cpu and @irq to which the arch-common ISR is hooked up
> + *	Takes @cpu and @virq to which the arch-common ISR is hooked up
>   */
> -extern int smp_ipi_irq_setup(int cpu, int irq);
> +extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
>  
>  /*
>   * struct plat_smp_ops	- SMP callbacks provided by platform to ARC SMP
> diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> index f183cc6..ec4d956 100644
> --- a/arch/arc/kernel/smp.c
> +++ b/arch/arc/kernel/smp.c
> @@ -351,7 +351,7 @@ irqreturn_t do_IPI(int irq, void *dev_id)
>   */
>  static DEFINE_PER_CPU(int, ipi_dev);
>  
> -int smp_ipi_irq_setup(int cpu, int irq)
> +int smp_ipi_irq_setup(int cpu, unsigned int virq)
>  {
>  	int *dev = per_cpu_ptr(&ipi_dev, cpu);
>  
> @@ -359,12 +359,12 @@ int smp_ipi_irq_setup(int cpu, int irq)
>  	if (!cpu) {
>  		int rc;
>  
> -		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
> +		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
>  		if (rc)
> -			panic("Percpu IRQ request failed for %d\n", irq);
> +			panic("Percpu IRQ request failed for %d\n", virq);
>  	}
>  
> -	enable_percpu_irq(irq, 0);
> +	enable_percpu_irq(virq, 0);
>  
>  	return 0;
>  }
>
Vineet Gupta Oct. 25, 2016, 2:28 a.m. UTC | #2
On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> This function takes a cpu number and a virq number and registers an
> appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
> used in several places of ARC platform code - hwirq is passed instead
> of virq. This patch is intended to clarify declaration of virq argument
> to prevent passing of hwirq instead of virq in future.
> 
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> ---
>  arch/arc/include/asm/smp.h | 4 ++--
>  arch/arc/kernel/smp.c      | 8 ++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
> index 89fdd1b..3ebebbc 100644
> --- a/arch/arc/include/asm/smp.h
> +++ b/arch/arc/include/asm/smp.h
> @@ -37,9 +37,9 @@ extern const char *arc_platform_smp_cpuinfo(void);
>   * API expected BY platform smp code (FROM arch smp code)
>   *
>   * smp_ipi_irq_setup:
> - *	Takes @cpu and @irq to which the arch-common ISR is hooked up
> + *	Takes @cpu and @virq to which the arch-common ISR is hooked up

If you agree to my comment on 2/5, then yeah change here to say hwirq, but IMO
this can be squashed with 2/5.

>   */
> -extern int smp_ipi_irq_setup(int cpu, int irq);
> +extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
>  
>  /*
>   * struct plat_smp_ops	- SMP callbacks provided by platform to ARC SMP
> diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> index f183cc6..ec4d956 100644
> --- a/arch/arc/kernel/smp.c
> +++ b/arch/arc/kernel/smp.c
> @@ -351,7 +351,7 @@ irqreturn_t do_IPI(int irq, void *dev_id)
>   */
>  static DEFINE_PER_CPU(int, ipi_dev);
>  
> -int smp_ipi_irq_setup(int cpu, int irq)
> +int smp_ipi_irq_setup(int cpu, unsigned int virq)
>  {
>  	int *dev = per_cpu_ptr(&ipi_dev, cpu);
>  
> @@ -359,12 +359,12 @@ int smp_ipi_irq_setup(int cpu, int irq)
>  	if (!cpu) {
>  		int rc;
>  
> -		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
> +		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
>  		if (rc)
> -			panic("Percpu IRQ request failed for %d\n", irq);
> +			panic("Percpu IRQ request failed for %d\n", virq);
>  	}
>  
> -	enable_percpu_irq(irq, 0);
> +	enable_percpu_irq(virq, 0);
>  
>  	return 0;
>  }
>
Yuriy Kolerov Oct. 25, 2016, 5:26 p.m. UTC | #3
Hi Vineet,

Yes, I agree with you and I will squash these patches.

> -----Original Message-----
> From: Vineet Gupta [mailto:vgupta@synopsys.com]
> Sent: Tuesday, October 25, 2016 5:29 AM
> To: Yuriy Kolerov <yuriy.kolerov@synopsys.com>; linux-snps-
> arc@lists.infradead.org
> Cc: Alexey.Brodkin@synopsys.com; marc.zyngier@arm.com;
> tglx@linutronix.de; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 1/5] ARC: SMP: Use "unsigned virq" in
> smp_ipi_irq_setup instead of "signed irq"
> 
> On 10/24/2016 05:46 AM, Yuriy Kolerov wrote:
> > This function takes a cpu number and a virq number and registers an
> > appropriate handler per cpu. However smp_ipi_irq_setup is incorrectly
> > used in several places of ARC platform code - hwirq is passed instead
> > of virq. This patch is intended to clarify declaration of virq
> > argument to prevent passing of hwirq instead of virq in future.
> >
> > Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> > ---
> >  arch/arc/include/asm/smp.h | 4 ++--
> >  arch/arc/kernel/smp.c      | 8 ++++----
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
> > index 89fdd1b..3ebebbc 100644
> > --- a/arch/arc/include/asm/smp.h
> > +++ b/arch/arc/include/asm/smp.h
> > @@ -37,9 +37,9 @@ extern const char *arc_platform_smp_cpuinfo(void);
> >   * API expected BY platform smp code (FROM arch smp code)
> >   *
> >   * smp_ipi_irq_setup:
> > - *	Takes @cpu and @irq to which the arch-common ISR is hooked up
> > + *	Takes @cpu and @virq to which the arch-common ISR is hooked up
> 
> If you agree to my comment on 2/5, then yeah change here to say hwirq, but
> IMO this can be squashed with 2/5.
> 
> >   */
> > -extern int smp_ipi_irq_setup(int cpu, int irq);
> > +extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
> >
> >  /*
> >   * struct plat_smp_ops	- SMP callbacks provided by platform to ARC
> SMP
> > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c index
> > f183cc6..ec4d956 100644
> > --- a/arch/arc/kernel/smp.c
> > +++ b/arch/arc/kernel/smp.c
> > @@ -351,7 +351,7 @@ irqreturn_t do_IPI(int irq, void *dev_id)
> >   */
> >  static DEFINE_PER_CPU(int, ipi_dev);
> >
> > -int smp_ipi_irq_setup(int cpu, int irq)
> > +int smp_ipi_irq_setup(int cpu, unsigned int virq)
> >  {
> >  	int *dev = per_cpu_ptr(&ipi_dev, cpu);
> >
> > @@ -359,12 +359,12 @@ int smp_ipi_irq_setup(int cpu, int irq)
> >  	if (!cpu) {
> >  		int rc;
> >
> > -		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
> > +		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
> >  		if (rc)
> > -			panic("Percpu IRQ request failed for %d\n", irq);
> > +			panic("Percpu IRQ request failed for %d\n", virq);
> >  	}
> >
> > -	enable_percpu_irq(irq, 0);
> > +	enable_percpu_irq(virq, 0);
> >
> >  	return 0;
> >  }
> >

Patch
diff mbox

diff --git a/arch/arc/include/asm/smp.h b/arch/arc/include/asm/smp.h
index 89fdd1b..3ebebbc 100644
--- a/arch/arc/include/asm/smp.h
+++ b/arch/arc/include/asm/smp.h
@@ -37,9 +37,9 @@  extern const char *arc_platform_smp_cpuinfo(void);
  * API expected BY platform smp code (FROM arch smp code)
  *
  * smp_ipi_irq_setup:
- *	Takes @cpu and @irq to which the arch-common ISR is hooked up
+ *	Takes @cpu and @virq to which the arch-common ISR is hooked up
  */
-extern int smp_ipi_irq_setup(int cpu, int irq);
+extern int smp_ipi_irq_setup(int cpu, unsigned int virq);
 
 /*
  * struct plat_smp_ops	- SMP callbacks provided by platform to ARC SMP
diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
index f183cc6..ec4d956 100644
--- a/arch/arc/kernel/smp.c
+++ b/arch/arc/kernel/smp.c
@@ -351,7 +351,7 @@  irqreturn_t do_IPI(int irq, void *dev_id)
  */
 static DEFINE_PER_CPU(int, ipi_dev);
 
-int smp_ipi_irq_setup(int cpu, int irq)
+int smp_ipi_irq_setup(int cpu, unsigned int virq)
 {
 	int *dev = per_cpu_ptr(&ipi_dev, cpu);
 
@@ -359,12 +359,12 @@  int smp_ipi_irq_setup(int cpu, int irq)
 	if (!cpu) {
 		int rc;
 
-		rc = request_percpu_irq(irq, do_IPI, "IPI Interrupt", dev);
+		rc = request_percpu_irq(virq, do_IPI, "IPI Interrupt", dev);
 		if (rc)
-			panic("Percpu IRQ request failed for %d\n", irq);
+			panic("Percpu IRQ request failed for %d\n", virq);
 	}
 
-	enable_percpu_irq(irq, 0);
+	enable_percpu_irq(virq, 0);
 
 	return 0;
 }