[PATCH 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map

[PATCH 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map

[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>
---
 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 a530d9a9b810..7566fe259d27 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -416,19 +416,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);
 
@@ -977,13 +984,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.
 	 */
@@ -1028,6 +1028,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

[PATCH 1/2] irqchip/gic: Only allow the primary GIC to set the CPU map

[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>
---
 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 a530d9a9b810..7566fe259d27 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -416,19 +416,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);
 
@@ -977,13 +984,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.
 	 */
@@ -1028,6 +1028,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

[PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[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. The vexpress TC2 (which has
a single GIC) is currently the only user of this function and so update
it accordingly.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
I was hoping to make the gic_cpu_if_up/down function more symmetric as we
discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
from all the places called without making more changes. However, given
that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
may be it does not matter too much.

 arch/arm/mach-vexpress/tc2_pm.c |  2 +-
 drivers/irqchip/irq-gic.c       | 14 +++++++-------
 include/linux/irqchip/arm-gic.h |  2 +-
 3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
+	void __iomem *dist_base = gic_data_dist_base(gic);
 	u32 bypass = 0;
 
 	/*
@@ -451,12 +451,12 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	}
 
 	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)
+void gic_cpu_if_down(unsigned int gic_nr)
 {
-	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
+	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
 	u32 val = 0;
 
 	val = readl(cpu_base + GIC_CPU_CTRL);
@@ -608,7 +608,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 	}
 
 	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 f52a9024be9a..9728bf4ca31b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -100,7 +100,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);
+void 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

[PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[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. The vexpress TC2 (which has
a single GIC) is currently the only user of this function and so update
it accordingly.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
I was hoping to make the gic_cpu_if_up/down function more symmetric as we
discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
from all the places called without making more changes. However, given
that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
may be it does not matter too much.

 arch/arm/mach-vexpress/tc2_pm.c |  2 +-
 drivers/irqchip/irq-gic.c       | 14 +++++++-------
 include/linux/irqchip/arm-gic.h |  2 +-
 3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
+	void __iomem *dist_base = gic_data_dist_base(gic);
 	u32 bypass = 0;
 
 	/*
@@ -451,12 +451,12 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	}
 
 	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)
+void gic_cpu_if_down(unsigned int gic_nr)
 {
-	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
+	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
 	u32 val = 0;
 
 	val = readl(cpu_base + GIC_CPU_CTRL);
@@ -608,7 +608,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 	}
 
 	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 f52a9024be9a..9728bf4ca31b 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -100,7 +100,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);
+void 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

Re: [PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Marc Zyngier]

On 30/07/15 17:26, 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. The vexpress TC2 (which has
> a single GIC) is currently the only user of this function and so update
> it accordingly.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
> from all the places called without making more changes. However, given
> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
> may be it does not matter too much.
> 
>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>  include/linux/irqchip/arm-gic.h |  2 +-
>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
> +	void __iomem *dist_base = gic_data_dist_base(gic);

Which tree is that against? I don't have a dist_base in mainline...

Thanks,

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

[PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Marc Zyngier]

On 30/07/15 17:26, 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. The vexpress TC2 (which has
> a single GIC) is currently the only user of this function and so update
> it accordingly.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
> from all the places called without making more changes. However, given
> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
> may be it does not matter too much.
> 
>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>  include/linux/irqchip/arm-gic.h |  2 +-
>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
> +	void __iomem *dist_base = gic_data_dist_base(gic);

Which tree is that against? I don't have a dist_base in mainline...

Thanks,

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

Re: [PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Jon Hunter]

On 30/07/15 17:51, Marc Zyngier wrote:
> On 30/07/15 17:26, 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. The vexpress TC2 (which has
>> a single GIC) is currently the only user of this function and so update
>> it accordingly.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
>> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
>> from all the places called without making more changes. However, given
>> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
>> may be it does not matter too much.
>>
>>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>>  include/linux/irqchip/arm-gic.h |  2 +-
>>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
>> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
>> +	void __iomem *dist_base = gic_data_dist_base(gic);
> 
> Which tree is that against? I don't have a dist_base in mainline...

It is based upon linux-next. I can rebase on the current mainline if you
want them for v4.2.

Jon

[PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Jon Hunter]

On 30/07/15 17:51, Marc Zyngier wrote:
> On 30/07/15 17:26, 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. The vexpress TC2 (which has
>> a single GIC) is currently the only user of this function and so update
>> it accordingly.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
>> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
>> from all the places called without making more changes. However, given
>> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
>> may be it does not matter too much.
>>
>>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>>  include/linux/irqchip/arm-gic.h |  2 +-
>>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
>> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
>> +	void __iomem *dist_base = gic_data_dist_base(gic);
> 
> Which tree is that against? I don't have a dist_base in mainline...

It is based upon linux-next. I can rebase on the current mainline if you
want them for v4.2.

Jon

Re: [PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Marc Zyngier]

On 30/07/15 18:56, Jon Hunter wrote:
> 
> On 30/07/15 17:51, Marc Zyngier wrote:
>> On 30/07/15 17:26, 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. The vexpress TC2 (which has
>>> a single GIC) is currently the only user of this function and so update
>>> it accordingly.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
>>> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
>>> from all the places called without making more changes. However, given
>>> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
>>> may be it does not matter too much.
>>>
>>>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>>>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>>>  include/linux/irqchip/arm-gic.h |  2 +-
>>>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
>>> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
>>> +	void __iomem *dist_base = gic_data_dist_base(gic);
>>
>> Which tree is that against? I don't have a dist_base in mainline...
> 
> It is based upon linux-next. I can rebase on the current mainline if you
> want them for v4.2.

It'd be good to fix it in mainline ASAP, as the Realview platforms are a
tiny bit dead at the moment, so a 4.2-rc would be good. The conflict
with Russell's FIQ work won't be hard to solve anyway.

Thanks,

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

[PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Marc Zyngier]

On 30/07/15 18:56, Jon Hunter wrote:
> 
> On 30/07/15 17:51, Marc Zyngier wrote:
>> On 30/07/15 17:26, 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. The vexpress TC2 (which has
>>> a single GIC) is currently the only user of this function and so update
>>> it accordingly.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
>>> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
>>> from all the places called without making more changes. However, given
>>> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
>>> may be it does not matter too much.
>>>
>>>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>>>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>>>  include/linux/irqchip/arm-gic.h |  2 +-
>>>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
>>> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
>>> +	void __iomem *dist_base = gic_data_dist_base(gic);
>>
>> Which tree is that against? I don't have a dist_base in mainline...
> 
> It is based upon linux-next. I can rebase on the current mainline if you
> want them for v4.2.

It'd be good to fix it in mainline ASAP, as the Realview platforms are a
tiny bit dead at the moment, so a 4.2-rc would be good. The conflict
with Russell's FIQ work won't be hard to solve anyway.

Thanks,

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

Re: [PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Jon Hunter]

On 30/07/15 17:26, 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. The vexpress TC2 (which has
> a single GIC) is currently the only user of this function and so update
> it accordingly.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
> from all the places called without making more changes. However, given
> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
> may be it does not matter too much.
> 
>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>  include/linux/irqchip/arm-gic.h |  2 +-
>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
> +	void __iomem *dist_base = gic_data_dist_base(gic);
>  	u32 bypass = 0;
>  
>  	/*
> @@ -451,12 +451,12 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>  	}
>  
>  	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)
> +void gic_cpu_if_down(unsigned int gic_nr)
>  {
> -	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> +	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
>  	u32 val = 0;

Well, this is rubbish. I need to check the gic_nr is valid. Will update ...

Jon

[PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Jon Hunter]

On 30/07/15 17:26, 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. The vexpress TC2 (which has
> a single GIC) is currently the only user of this function and so update
> it accordingly.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
> from all the places called without making more changes. However, given
> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
> may be it does not matter too much.
> 
>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>  include/linux/irqchip/arm-gic.h |  2 +-
>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
> +	void __iomem *dist_base = gic_data_dist_base(gic);
>  	u32 bypass = 0;
>  
>  	/*
> @@ -451,12 +451,12 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>  	}
>  
>  	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)
> +void gic_cpu_if_down(unsigned int gic_nr)
>  {
> -	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> +	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
>  	u32 val = 0;

Well, this is rubbish. I need to check the gic_nr is valid. Will update ...

Jon

Re: [PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Jon Hunter]

On 30/07/15 19:11, Marc Zyngier wrote:
> On 30/07/15 18:56, Jon Hunter wrote:
>>
>> On 30/07/15 17:51, Marc Zyngier wrote:
>>> On 30/07/15 17:26, 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. The vexpress TC2 (which has
>>>> a single GIC) is currently the only user of this function and so update
>>>> it accordingly.
>>>>
>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>>> ---
>>>> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
>>>> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
>>>> from all the places called without making more changes. However, given
>>>> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
>>>> may be it does not matter too much.
>>>>
>>>>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>>>>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>>>>  include/linux/irqchip/arm-gic.h |  2 +-
>>>>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
>>>> --- a/drivers/irqchip/irq-gic.c
>>>> +++ b/drivers/irqchip/irq-gic.c
>>>> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
>>>> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
>>>> +	void __iomem *dist_base = gic_data_dist_base(gic);
>>>
>>> Which tree is that against? I don't have a dist_base in mainline...
>>
>> It is based upon linux-next. I can rebase on the current mainline if you
>> want them for v4.2.
> 
> It'd be good to fix it in mainline ASAP, as the Realview platforms are a
> tiny bit dead at the moment, so a 4.2-rc would be good. The conflict
> with Russell's FIQ work won't be hard to solve anyway.

Ok, will rebase on 4.2-rc4. I will also fix-up my incompetence, I just
pointed out in the above.

Jon

[PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance

[Jon Hunter]

On 30/07/15 19:11, Marc Zyngier wrote:
> On 30/07/15 18:56, Jon Hunter wrote:
>>
>> On 30/07/15 17:51, Marc Zyngier wrote:
>>> On 30/07/15 17:26, 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. The vexpress TC2 (which has
>>>> a single GIC) is currently the only user of this function and so update
>>>> it accordingly.
>>>>
>>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>>> ---
>>>> I was hoping to make the gic_cpu_if_up/down function more symmetric as we
>>>> discussed but it is not possible to pass the gic_nr to gic_cpu_if_up()
>>>> from all the places called without making more changes. However, given
>>>> that gic_cpu_if_up() is a local function and gic_cpu_if_down() is public,
>>>> may be it does not matter too much.
>>>>
>>>>  arch/arm/mach-vexpress/tc2_pm.c |  2 +-
>>>>  drivers/irqchip/irq-gic.c       | 14 +++++++-------
>>>>  include/linux/irqchip/arm-gic.h |  2 +-
>>>>  3 files changed, 9 insertions(+), 9 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 7566fe259d27..cf9aca22120f 100644
>>>> --- a/drivers/irqchip/irq-gic.c
>>>> +++ b/drivers/irqchip/irq-gic.c
>>>> @@ -356,10 +356,10 @@ 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 *dist_base = gic_data_dist_base(&gic_data[0]);
>>>> +	void __iomem *cpu_base = gic_data_cpu_base(gic);
>>>> +	void __iomem *dist_base = gic_data_dist_base(gic);
>>>
>>> Which tree is that against? I don't have a dist_base in mainline...
>>
>> It is based upon linux-next. I can rebase on the current mainline if you
>> want them for v4.2.
> 
> It'd be good to fix it in mainline ASAP, as the Realview platforms are a
> tiny bit dead at the moment, so a 4.2-rc would be good. The conflict
> with Russell's FIQ work won't be hard to solve anyway.

Ok, will rebase on 4.2-rc4. I will also fix-up my incompetence, I just
pointed out in the above.

Jon