public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map
@ 2015-07-31  8:44 Jon Hunter
  2015-07-31  8:44 ` [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance Jon Hunter
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jon Hunter @ 2015-07-31  8:44 UTC (permalink / raw)
  To: Marc Zyngier, Russell King, Nicolas Pitre, Thomas Gleixner,
	Jason Cooper
  Cc: linux-kernel, linux-arm-kernel, Jon Hunter

The gic_init_bases() function initialises an array that stores the mapping
between the GIC and CPUs. This array is a global array that is
unconditionally initialised on every call to gic_init_bases(). Although,
it is not common for there to be more than one GIC instance, there are
some devices that do support nested GIC controllers and gic_init_bases()
can be called more than once.

A 2nd call to gic_init_bases() will clear the previous CPU mapping and
will only setup the mapping again for the CPU calling gic_init_bases().
Fix this by only allowing the CPU map to be configured for the primary GIC.

For secondary GICs the CPU map is not relevant because these GICs do not
directly route the interrupts to the main CPU(s) but to other GICs or
devices.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
V2 changes:
- Rebased on v4.2-rc4

 drivers/irqchip/irq-gic.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4dd88264dff5..dc5090543eca 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -401,19 +401,26 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	int i;
 
 	/*
-	 * Get what the GIC says our CPU mask is.
+	 * Setting up the CPU map is only relevant for the primary GIC
+	 * because any nested/secondary GICs do not directly interface
+	 * with the CPU(s).
 	 */
-	BUG_ON(cpu >= NR_GIC_CPU_IF);
-	cpu_mask = gic_get_cpumask(gic);
-	gic_cpu_map[cpu] = cpu_mask;
+	if (gic == &gic_data[0]) {
+		/*
+		 * Get what the GIC says our CPU mask is.
+		 */
+		BUG_ON(cpu >= NR_GIC_CPU_IF);
+		cpu_mask = gic_get_cpumask(gic);
+		gic_cpu_map[cpu] = cpu_mask;
 
-	/*
-	 * Clear our mask from the other map entries in case they're
-	 * still undefined.
-	 */
-	for (i = 0; i < NR_GIC_CPU_IF; i++)
-		if (i != cpu)
-			gic_cpu_map[i] &= ~cpu_mask;
+		/*
+		 * Clear our mask from the other map entries in case they're
+		 * still undefined.
+		 */
+		for (i = 0; i < NR_GIC_CPU_IF; i++)
+			if (i != cpu)
+				gic_cpu_map[i] &= ~cpu_mask;
+	}
 
 	gic_cpu_config(dist_base, NULL);
 
@@ -930,13 +937,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 	}
 
 	/*
-	 * Initialize the CPU interface map to all CPUs.
-	 * It will be refined as each CPU probes its ID.
-	 */
-	for (i = 0; i < NR_GIC_CPU_IF; i++)
-		gic_cpu_map[i] = 0xff;
-
-	/*
 	 * Find out how many interrupts are supported.
 	 * The GIC only supports up to 1020 interrupt sources.
 	 */
@@ -981,6 +981,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 		return;
 
 	if (gic_nr == 0) {
+		/*
+		 * Initialize the CPU interface map to all CPUs.
+		 * It will be refined as each CPU probes its ID.
+		 * This is only necessary for the primary GIC.
+		 */
+		for (i = 0; i < NR_GIC_CPU_IF; i++)
+			gic_cpu_map[i] = 0xff;
 #ifdef CONFIG_SMP
 		set_smp_cross_call(gic_raise_softirq);
 		register_cpu_notifier(&gic_cpu_notifier);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance
  2015-07-31  8:44 [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map Jon Hunter
@ 2015-07-31  8:44 ` Jon Hunter
  2015-08-03 10:02   ` Marc Zyngier
  2015-08-04 12:18   ` [tip:irq/core] " tip-bot for Jon Hunter
  2015-08-03  9:58 ` [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map Marc Zyngier
  2015-08-04 12:18 ` [tip:irq/core] " tip-bot for Jon Hunter
  2 siblings, 2 replies; 6+ messages in thread
From: Jon Hunter @ 2015-07-31  8:44 UTC (permalink / raw)
  To: Marc Zyngier, Russell King, Nicolas Pitre, Thomas Gleixner,
	Jason Cooper
  Cc: linux-kernel, linux-arm-kernel, Jon Hunter

Commit 3228950621d9 ("irqchip: gic: Preserve gic V2 bypass bits in cpu
ctrl register") added a new function, gic_cpu_if_up(), to program the
GIC CPU_CTRL register. This function assumes that there is only one GIC
instance present and hence always uses the chip data for the primary GIC
controller. Although it is not common for there to be a secondary, some
devices do support a secondary. Therefore, fix this by passing
gic_cpu_if_up() a pointer to the appropriate chip data structure.

Similarly, the function gic_cpu_if_down() only assumes that there is a
single GIC instance present. Update this function so that an instance
number is passed for the appropriate GIC and return an error code on
failure. The vexpress TC2 (which has a single GIC) is currently the only
user of this function and so update it accordingly. Note that because the
TC2 only has a single GIC, the call to gic_cpu_if_down() should always
be successful.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
V2 changes:
- Rebased on v4.2-rc4
- Added test to ensure GIC instance is valid to gic_cpu_if_down() and
  updated gic_cpu_if_down() to return an error code on failure.

 arch/arm/mach-vexpress/tc2_pm.c |  2 +-
 drivers/irqchip/irq-gic.c       | 18 ++++++++++++------
 include/linux/irqchip/arm-gic.h |  2 +-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
index b3328cd46c33..1aa4ccece69f 100644
--- a/arch/arm/mach-vexpress/tc2_pm.c
+++ b/arch/arm/mach-vexpress/tc2_pm.c
@@ -80,7 +80,7 @@ static void tc2_pm_cpu_powerdown_prepare(unsigned int cpu, unsigned int cluster)
 	 * to the CPU by disabling the GIC CPU IF to prevent wfi
 	 * from completing execution behind power controller back
 	 */
-	gic_cpu_if_down();
+	gic_cpu_if_down(0);
 }
 
 static void tc2_pm_cluster_powerdown_prepare(unsigned int cluster)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index dc5090543eca..fdd1b0e6e5c3 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -355,9 +355,9 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
 	return mask;
 }
 
-static void gic_cpu_if_up(void)
+static void gic_cpu_if_up(struct gic_chip_data *gic)
 {
-	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
 	u32 bypass = 0;
 
 	/*
@@ -425,17 +425,23 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	gic_cpu_config(dist_base, NULL);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
-	gic_cpu_if_up();
+	gic_cpu_if_up(gic);
 }
 
-void gic_cpu_if_down(void)
+int gic_cpu_if_down(unsigned int gic_nr)
 {
-	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
+	void __iomem *cpu_base;
 	u32 val = 0;
 
+	if (gic_nr >= MAX_GIC_NR)
+		return -EINVAL;
+
+	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
 	val = readl(cpu_base + GIC_CPU_CTRL);
 	val &= ~GICC_ENABLE;
 	writel_relaxed(val, cpu_base + GIC_CPU_CTRL);
+
+	return 0;
 }
 
 #ifdef CONFIG_CPU_PM
@@ -571,7 +577,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 					dist_base + GIC_DIST_PRI + i * 4);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK);
-	gic_cpu_if_up();
+	gic_cpu_if_up(&gic_data[gic_nr]);
 }
 
 static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 9de976b4f9a7..da6aced1105e 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -99,7 +99,7 @@ void gic_set_irqchip_flags(unsigned long flags);
 void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
 		    u32 offset, struct device_node *);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
-void gic_cpu_if_down(void);
+int gic_cpu_if_down(unsigned int gic_nr);
 
 static inline void gic_init(unsigned int nr, int start,
 			    void __iomem *dist , void __iomem *cpu)
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map
  2015-07-31  8:44 [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map Jon Hunter
  2015-07-31  8:44 ` [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance Jon Hunter
@ 2015-08-03  9:58 ` Marc Zyngier
  2015-08-04 12:18 ` [tip:irq/core] " tip-bot for Jon Hunter
  2 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2015-08-03  9:58 UTC (permalink / raw)
  To: Jon Hunter, Russell King, nicolas.pitre@linaro.org,
	Thomas Gleixner, Jason Cooper
  Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org

On 31/07/15 09:44, Jon Hunter wrote:
> The gic_init_bases() function initialises an array that stores the mapping
> between the GIC and CPUs. This array is a global array that is
> unconditionally initialised on every call to gic_init_bases(). Although,
> it is not common for there to be more than one GIC instance, there are
> some devices that do support nested GIC controllers and gic_init_bases()
> can be called more than once.
> 
> A 2nd call to gic_init_bases() will clear the previous CPU mapping and
> will only setup the mapping again for the CPU calling gic_init_bases().
> Fix this by only allowing the CPU map to be configured for the primary GIC.
> 
> For secondary GICs the CPU map is not relevant because these GICs do not
> directly route the interrupts to the main CPU(s) but to other GICs or
> devices.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

Looks good to me:

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
> V2 changes:
> - Rebased on v4.2-rc4
> 
>  drivers/irqchip/irq-gic.c | 43 +++++++++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 4dd88264dff5..dc5090543eca 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -401,19 +401,26 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>  	int i;
>  
>  	/*
> -	 * Get what the GIC says our CPU mask is.
> +	 * Setting up the CPU map is only relevant for the primary GIC
> +	 * because any nested/secondary GICs do not directly interface
> +	 * with the CPU(s).
>  	 */
> -	BUG_ON(cpu >= NR_GIC_CPU_IF);
> -	cpu_mask = gic_get_cpumask(gic);
> -	gic_cpu_map[cpu] = cpu_mask;
> +	if (gic == &gic_data[0]) {
> +		/*
> +		 * Get what the GIC says our CPU mask is.
> +		 */
> +		BUG_ON(cpu >= NR_GIC_CPU_IF);
> +		cpu_mask = gic_get_cpumask(gic);
> +		gic_cpu_map[cpu] = cpu_mask;
>  
> -	/*
> -	 * Clear our mask from the other map entries in case they're
> -	 * still undefined.
> -	 */
> -	for (i = 0; i < NR_GIC_CPU_IF; i++)
> -		if (i != cpu)
> -			gic_cpu_map[i] &= ~cpu_mask;
> +		/*
> +		 * Clear our mask from the other map entries in case they're
> +		 * still undefined.
> +		 */
> +		for (i = 0; i < NR_GIC_CPU_IF; i++)
> +			if (i != cpu)
> +				gic_cpu_map[i] &= ~cpu_mask;
> +	}
>  
>  	gic_cpu_config(dist_base, NULL);
>  
> @@ -930,13 +937,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  	}
>  
>  	/*
> -	 * Initialize the CPU interface map to all CPUs.
> -	 * It will be refined as each CPU probes its ID.
> -	 */
> -	for (i = 0; i < NR_GIC_CPU_IF; i++)
> -		gic_cpu_map[i] = 0xff;
> -
> -	/*
>  	 * Find out how many interrupts are supported.
>  	 * The GIC only supports up to 1020 interrupt sources.
>  	 */
> @@ -981,6 +981,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  		return;
>  
>  	if (gic_nr == 0) {
> +		/*
> +		 * Initialize the CPU interface map to all CPUs.
> +		 * It will be refined as each CPU probes its ID.
> +		 * This is only necessary for the primary GIC.
> +		 */
> +		for (i = 0; i < NR_GIC_CPU_IF; i++)
> +			gic_cpu_map[i] = 0xff;
>  #ifdef CONFIG_SMP
>  		set_smp_cross_call(gic_raise_softirq);
>  		register_cpu_notifier(&gic_cpu_notifier);
> 


-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance
  2015-07-31  8:44 ` [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance Jon Hunter
@ 2015-08-03 10:02   ` Marc Zyngier
  2015-08-04 12:18   ` [tip:irq/core] " tip-bot for Jon Hunter
  1 sibling, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2015-08-03 10:02 UTC (permalink / raw)
  To: Jon Hunter, Russell King, nicolas.pitre@linaro.org,
	Thomas Gleixner, Jason Cooper
  Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org

On 31/07/15 09:44, Jon Hunter wrote:
> Commit 3228950621d9 ("irqchip: gic: Preserve gic V2 bypass bits in cpu
> ctrl register") added a new function, gic_cpu_if_up(), to program the
> GIC CPU_CTRL register. This function assumes that there is only one GIC
> instance present and hence always uses the chip data for the primary GIC
> controller. Although it is not common for there to be a secondary, some
> devices do support a secondary. Therefore, fix this by passing
> gic_cpu_if_up() a pointer to the appropriate chip data structure.
> 
> Similarly, the function gic_cpu_if_down() only assumes that there is a
> single GIC instance present. Update this function so that an instance
> number is passed for the appropriate GIC and return an error code on
> failure. The vexpress TC2 (which has a single GIC) is currently the only
> user of this function and so update it accordingly. Note that because the
> TC2 only has a single GIC, the call to gic_cpu_if_down() should always
> be successful.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

I also gave both patches a test run on Realview EB11MP and PB1176, and
both seem happy enough.

Thanks,

	M.

> ---
> V2 changes:
> - Rebased on v4.2-rc4
> - Added test to ensure GIC instance is valid to gic_cpu_if_down() and
>   updated gic_cpu_if_down() to return an error code on failure.
> 
>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>  drivers/irqchip/irq-gic.c       | 18 ++++++++++++------
>  include/linux/irqchip/arm-gic.h |  2 +-
>  3 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
> index b3328cd46c33..1aa4ccece69f 100644
> --- a/arch/arm/mach-vexpress/tc2_pm.c
> +++ b/arch/arm/mach-vexpress/tc2_pm.c
> @@ -80,7 +80,7 @@ static void tc2_pm_cpu_powerdown_prepare(unsigned int cpu, unsigned int cluster)
>  	 * to the CPU by disabling the GIC CPU IF to prevent wfi
>  	 * from completing execution behind power controller back
>  	 */
> -	gic_cpu_if_down();
> +	gic_cpu_if_down(0);
>  }
>  
>  static void tc2_pm_cluster_powerdown_prepare(unsigned int cluster)
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index dc5090543eca..fdd1b0e6e5c3 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -355,9 +355,9 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
>  	return mask;
>  }
>  
> -static void gic_cpu_if_up(void)
> +static void gic_cpu_if_up(struct gic_chip_data *gic)
>  {
> -	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
>  	u32 bypass = 0;
>  
>  	/*
> @@ -425,17 +425,23 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>  	gic_cpu_config(dist_base, NULL);
>  
>  	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
> -	gic_cpu_if_up();
> +	gic_cpu_if_up(gic);
>  }
>  
> -void gic_cpu_if_down(void)
> +int gic_cpu_if_down(unsigned int gic_nr)
>  {
> -	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> +	void __iomem *cpu_base;
>  	u32 val = 0;
>  
> +	if (gic_nr >= MAX_GIC_NR)
> +		return -EINVAL;
> +
> +	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
>  	val = readl(cpu_base + GIC_CPU_CTRL);
>  	val &= ~GICC_ENABLE;
>  	writel_relaxed(val, cpu_base + GIC_CPU_CTRL);
> +
> +	return 0;
>  }
>  
>  #ifdef CONFIG_CPU_PM
> @@ -571,7 +577,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>  					dist_base + GIC_DIST_PRI + i * 4);
>  
>  	writel_relaxed(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK);
> -	gic_cpu_if_up();
> +	gic_cpu_if_up(&gic_data[gic_nr]);
>  }
>  
>  static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 9de976b4f9a7..da6aced1105e 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -99,7 +99,7 @@ void gic_set_irqchip_flags(unsigned long flags);
>  void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
>  		    u32 offset, struct device_node *);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
> -void gic_cpu_if_down(void);
> +int gic_cpu_if_down(unsigned int gic_nr);
>  
>  static inline void gic_init(unsigned int nr, int start,
>  			    void __iomem *dist , void __iomem *cpu)
> 


-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:irq/core] irqchip/gic: Only allow the primary GIC to set the CPU map
  2015-07-31  8:44 [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map Jon Hunter
  2015-07-31  8:44 ` [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance Jon Hunter
  2015-08-03  9:58 ` [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map Marc Zyngier
@ 2015-08-04 12:18 ` tip-bot for Jon Hunter
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Jon Hunter @ 2015-08-04 12:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux, nicolas.pitre, mingo, tglx, linux-arm-kernel, jonathanh,
	marc.zyngier, hpa, jason, linux-kernel

Commit-ID:  567e5a014848c6aeb1d6fc862b1b5d0183760259
Gitweb:     http://git.kernel.org/tip/567e5a014848c6aeb1d6fc862b1b5d0183760259
Author:     Jon Hunter <jonathanh@nvidia.com>
AuthorDate: Fri, 31 Jul 2015 09:44:11 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 4 Aug 2015 14:14:06 +0200

irqchip/gic: Only allow the primary GIC to set the CPU map

The gic_init_bases() function initialises an array that stores the mapping
between the GIC and CPUs. This array is a global array that is
unconditionally initialised on every call to gic_init_bases(). Although,
it is not common for there to be more than one GIC instance, there are
some devices that do support nested GIC controllers and gic_init_bases()
can be called more than once.

A 2nd call to gic_init_bases() will clear the previous CPU mapping and
will only setup the mapping again for the CPU calling gic_init_bases().
Fix this by only allowing the CPU map to be configured for the primary GIC.

For secondary GICs the CPU map is not relevant because these GICs do not
directly route the interrupts to the main CPU(s) but to other GICs or
devices.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Link: http://lkml.kernel.org/r/1438332252-25248-1-git-send-email-jonathanh@nvidia.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/irq-gic.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 29c544d..84fc622 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -402,19 +402,26 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	int i;
 
 	/*
-	 * Get what the GIC says our CPU mask is.
+	 * Setting up the CPU map is only relevant for the primary GIC
+	 * because any nested/secondary GICs do not directly interface
+	 * with the CPU(s).
 	 */
-	BUG_ON(cpu >= NR_GIC_CPU_IF);
-	cpu_mask = gic_get_cpumask(gic);
-	gic_cpu_map[cpu] = cpu_mask;
+	if (gic == &gic_data[0]) {
+		/*
+		 * Get what the GIC says our CPU mask is.
+		 */
+		BUG_ON(cpu >= NR_GIC_CPU_IF);
+		cpu_mask = gic_get_cpumask(gic);
+		gic_cpu_map[cpu] = cpu_mask;
 
-	/*
-	 * Clear our mask from the other map entries in case they're
-	 * still undefined.
-	 */
-	for (i = 0; i < NR_GIC_CPU_IF; i++)
-		if (i != cpu)
-			gic_cpu_map[i] &= ~cpu_mask;
+		/*
+		 * Clear our mask from the other map entries in case they're
+		 * still undefined.
+		 */
+		for (i = 0; i < NR_GIC_CPU_IF; i++)
+			if (i != cpu)
+				gic_cpu_map[i] &= ~cpu_mask;
+	}
 
 	gic_cpu_config(dist_base, NULL);
 
@@ -926,13 +933,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 	}
 
 	/*
-	 * Initialize the CPU interface map to all CPUs.
-	 * It will be refined as each CPU probes its ID.
-	 */
-	for (i = 0; i < NR_GIC_CPU_IF; i++)
-		gic_cpu_map[i] = 0xff;
-
-	/*
 	 * Find out how many interrupts are supported.
 	 * The GIC only supports up to 1020 interrupt sources.
 	 */
@@ -977,6 +977,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 		return;
 
 	if (gic_nr == 0) {
+		/*
+		 * Initialize the CPU interface map to all CPUs.
+		 * It will be refined as each CPU probes its ID.
+		 * This is only necessary for the primary GIC.
+		 */
+		for (i = 0; i < NR_GIC_CPU_IF; i++)
+			gic_cpu_map[i] = 0xff;
 #ifdef CONFIG_SMP
 		set_smp_cross_call(gic_raise_softirq);
 		register_cpu_notifier(&gic_cpu_notifier);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:irq/core] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance
  2015-07-31  8:44 ` [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance Jon Hunter
  2015-08-03 10:02   ` Marc Zyngier
@ 2015-08-04 12:18   ` tip-bot for Jon Hunter
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Jon Hunter @ 2015-08-04 12:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: marc.zyngier, linux-kernel, hpa, jonathanh, tglx, jason, mingo,
	nicolas.pitre, linux-arm-kernel, linux

Commit-ID:  4c2880b31c700b03f3f115b5ca64be615783aa9c
Gitweb:     http://git.kernel.org/tip/4c2880b31c700b03f3f115b5ca64be615783aa9c
Author:     Jon Hunter <jonathanh@nvidia.com>
AuthorDate: Fri, 31 Jul 2015 09:44:12 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 4 Aug 2015 14:14:06 +0200

irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

Commit 3228950621d9 ("irqchip: gic: Preserve gic V2 bypass bits in cpu
ctrl register") added a new function, gic_cpu_if_up(), to program the
GIC CPU_CTRL register. This function assumes that there is only one GIC
instance present and hence always uses the chip data for the primary GIC
controller. Although it is not common for there to be a secondary, some
devices do support a secondary. Therefore, fix this by passing
gic_cpu_if_up() a pointer to the appropriate chip data structure.

Similarly, the function gic_cpu_if_down() only assumes that there is a
single GIC instance present. Update this function so that an instance
number is passed for the appropriate GIC and return an error code on
failure. The vexpress TC2 (which has a single GIC) is currently the only
user of this function and so update it accordingly. Note that because the
TC2 only has a single GIC, the call to gic_cpu_if_down() should always
be successful.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Link: http://lkml.kernel.org/r/1438332252-25248-2-git-send-email-jonathanh@nvidia.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/arm/mach-vexpress/tc2_pm.c |  2 +-
 drivers/irqchip/irq-gic.c       | 18 ++++++++++++------
 include/linux/irqchip/arm-gic.h |  2 +-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-vexpress/tc2_pm.c b/arch/arm/mach-vexpress/tc2_pm.c
index b3328cd..1aa4cce 100644
--- a/arch/arm/mach-vexpress/tc2_pm.c
+++ b/arch/arm/mach-vexpress/tc2_pm.c
@@ -80,7 +80,7 @@ static void tc2_pm_cpu_powerdown_prepare(unsigned int cpu, unsigned int cluster)
 	 * to the CPU by disabling the GIC CPU IF to prevent wfi
 	 * from completing execution behind power controller back
 	 */
-	gic_cpu_if_down();
+	gic_cpu_if_down(0);
 }
 
 static void tc2_pm_cluster_powerdown_prepare(unsigned int cluster)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 84fc622..aa3e7b8 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -356,9 +356,9 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
 	return mask;
 }
 
-static void gic_cpu_if_up(void)
+static void gic_cpu_if_up(struct gic_chip_data *gic)
 {
-	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
 	u32 bypass = 0;
 
 	/*
@@ -426,17 +426,23 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	gic_cpu_config(dist_base, NULL);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
-	gic_cpu_if_up();
+	gic_cpu_if_up(gic);
 }
 
-void gic_cpu_if_down(void)
+int gic_cpu_if_down(unsigned int gic_nr)
 {
-	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
+	void __iomem *cpu_base;
 	u32 val = 0;
 
+	if (gic_nr >= MAX_GIC_NR)
+		return -EINVAL;
+
+	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
 	val = readl(cpu_base + GIC_CPU_CTRL);
 	val &= ~GICC_ENABLE;
 	writel_relaxed(val, cpu_base + GIC_CPU_CTRL);
+
+	return 0;
 }
 
 #ifdef CONFIG_CPU_PM
@@ -572,7 +578,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 					dist_base + GIC_DIST_PRI + i * 4);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK);
-	gic_cpu_if_up();
+	gic_cpu_if_up(&gic_data[gic_nr]);
 }
 
 static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 61a2007..65da435 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -98,7 +98,7 @@ struct device_node;
 void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
 		    u32 offset, struct device_node *);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
-void gic_cpu_if_down(void);
+int gic_cpu_if_down(unsigned int gic_nr);
 
 static inline void gic_init(unsigned int nr, int start,
 			    void __iomem *dist , void __iomem *cpu)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-08-04 12:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-31  8:44 [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map Jon Hunter
2015-07-31  8:44 ` [PATCH V2 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance Jon Hunter
2015-08-03 10:02   ` Marc Zyngier
2015-08-04 12:18   ` [tip:irq/core] " tip-bot for Jon Hunter
2015-08-03  9:58 ` [PATCH V2 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map Marc Zyngier
2015-08-04 12:18 ` [tip:irq/core] " tip-bot for Jon Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox