From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753645AbbG3QvI (ORCPT ); Thu, 30 Jul 2015 12:51:08 -0400 Received: from foss.arm.com ([217.140.101.70]:41417 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752148AbbG3QvH (ORCPT ); Thu, 30 Jul 2015 12:51:07 -0400 Message-ID: <55BA55F4.2060001@arm.com> Date: Thu, 30 Jul 2015 17:51:00 +0100 From: Marc Zyngier Organization: ARM Ltd User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.7.0 MIME-Version: 1.0 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" Subject: Re: [PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance References: <1438273573-27958-1-git-send-email-jonathanh@nvidia.com> <1438273573-27958-2-git-send-email-jonathanh@nvidia.com> In-Reply-To: <1438273573-27958-2-git-send-email-jonathanh@nvidia.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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... From mboxrd@z Thu Jan 1 00:00:00 1970 From: marc.zyngier@arm.com (Marc Zyngier) Date: Thu, 30 Jul 2015 17:51:00 +0100 Subject: [PATCH 2/2] irqchip/gic: Ensure gic_cpu_if_up/down() programs correct GIC instance In-Reply-To: <1438273573-27958-2-git-send-email-jonathanh@nvidia.com> References: <1438273573-27958-1-git-send-email-jonathanh@nvidia.com> <1438273573-27958-2-git-send-email-jonathanh@nvidia.com> Message-ID: <55BA55F4.2060001@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > --- > 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...