This patch series cleans up hex number in the gic driver and then adds
the code to preserve GIC v2 bypass disable bits in the GIC driver.
V2 Change:
seem my send email was not working correctly, resending this with
rebase pull.
- had to pull HaoJian's change out of arm-gic.h to keep consistency.
- replace GIC defines as noted by Marc
- remove GIC_CPU_DISABLE since it no longer used.
- fix gic_cpu_if_down as noted by Marc
PM path gic cpu ctrl register restore is not added here, I will have to
test that seperately once all the PM code is fully in place and submit it
afterward.
Feng Kan (2):
irqchip: gic: replace hex numbers with defines.
irqchip: gic: preserve gic V2 bypass bits in cpu ctrl register
drivers/irqchip/irq-gic.c | 82 +++++++++++++++++++++++++++++++++++------------
1 file changed, 62 insertions(+), 20 deletions(-)
--
1.9.1
This change is made to preserve the GIC v2 bypass bits in the
GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
This code will preserve all bits configured by the bootloader regarding
v2 bypass group bits. In the X-Gene platform, the bypass functionality
is not used and bypass bits should not be changed by the kernel gic
code as it could lead to incorrect behavior.
Signed-off-by: Vinayak Kale <[email protected]>
Signed-off-by: Feng Kan <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
---
drivers/irqchip/irq-gic.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 9ec3f4c..e70ef38 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -64,6 +64,7 @@
#define GIC_CPU_INT_PRI_THRESHOLD 0xf0
#define GIC_CPU_INT_SPURIOUS 1023
#define GIC_CPU_INT_ID_MASK 0x3ff
+#define GIC_CPU_DIS_BYPASS_MASK 0x1e0
union gic_base {
void __iomem *common_base;
@@ -394,6 +395,20 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
return mask;
}
+static void gic_cpu_if_up(void)
+{
+ void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
+ u32 bypass;
+
+ /*
+ * Preserve bypass disable bits to be written back later
+ */
+ bypass = readl(cpu_base + GIC_CPU_CTRL);
+ bypass &= GIC_CPU_DIS_BYPASS_MASK;
+
+ writel_relaxed(bypass | GIC_CPU_ENABLE, cpu_base + GIC_CPU_CTRL);
+}
+
static void __init gic_dist_init(struct gic_chip_data *gic)
{
unsigned int i;
@@ -476,13 +491,17 @@ static void gic_cpu_init(struct gic_chip_data *gic)
dist_base + GIC_DIST_PRI + i * 4 / 4);
writel_relaxed(GIC_CPU_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
- writel_relaxed(GIC_CPU_ENABLE, base + GIC_CPU_CTRL);
+ gic_cpu_if_up();
}
void gic_cpu_if_down(void)
{
void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
- writel_relaxed(0, cpu_base + GIC_CPU_CTRL);
+ u32 val;
+
+ val = readl(cpu_base + GIC_CPU_CTRL);
+ val &= ~GIC_CPU_ENABLE;
+ writel_relaxed(val, cpu_base + GIC_CPU_CTRL);
}
#ifdef CONFIG_CPU_PM
@@ -618,7 +637,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
dist_base + GIC_DIST_PRI + i * 4);
writel_relaxed(GIC_CPU_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(GIC_CPU_ENABLE, cpu_base + GIC_CPU_CTRL);
+ gic_cpu_if_up();
}
static int gic_notifier(struct notifier_block *self, unsigned long cmd, void *v)
--
1.9.1
This is to cleanup some hex numbers used in the code and replace
then with defines to make the code cleaner.
Signed-off-by: Feng Kan <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
---
drivers/irqchip/irq-gic.c | 62 ++++++++++++++++++++++++++++-------------
include/linux/irqchip/arm-gic.h | 2 --
2 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 7e11c9d..9ec3f4c 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -48,6 +48,23 @@
#include "irqchip.h"
+#define GIC_DIST_ENABLE 0x1
+#define GIC_DIST_DISABLE 0x0
+#define GIC_DIST_INT_CLR_EN_32 0xffffffff
+#define GIC_DIST_INT_SET_EN_SGI 0x0000ffff
+#define GIC_DIST_INT_CLR_EN_PPI 0xffff0000
+#define GIC_DIST_INT_LVL_TRIGGER 0x0
+#define GIC_DIST_INT_DEF_PRI 0xa0
+#define GIC_DIST_INT_DEF_PRI_4 ((GIC_DIST_INT_DEF_PRI << 24) |\
+ (GIC_DIST_INT_DEF_PRI << 16) |\
+ (GIC_DIST_INT_DEF_PRI << 8) |\
+ GIC_DIST_INT_DEF_PRI)
+
+#define GIC_CPU_ENABLE 0x1
+#define GIC_CPU_INT_PRI_THRESHOLD 0xf0
+#define GIC_CPU_INT_SPURIOUS 1023
+#define GIC_CPU_INT_ID_MASK 0x3ff
+
union gic_base {
void __iomem *common_base;
void __percpu * __iomem *percpu_base;
@@ -291,7 +308,7 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
do {
irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
- irqnr = irqstat & GICC_IAR_INT_ID_MASK;
+ irqnr = irqstat & GIC_CPU_INT_ID_MASK;
if (likely(irqnr > 15 && irqnr < 1021)) {
irqnr = irq_find_mapping(gic->domain, irqnr);
@@ -322,8 +339,8 @@ static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
status = readl_relaxed(gic_data_cpu_base(chip_data) + GIC_CPU_INTACK);
raw_spin_unlock(&irq_controller_lock);
- gic_irq = (status & 0x3ff);
- if (gic_irq == 1023)
+ gic_irq = (status & GIC_CPU_INT_ID_MASK);
+ if (gic_irq == GIC_CPU_INT_SPURIOUS)
goto out;
cascade_irq = irq_find_mapping(chip_data->domain, gic_irq);
@@ -384,13 +401,14 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
unsigned int gic_irqs = gic->gic_irqs;
void __iomem *base = gic_data_dist_base(gic);
- writel_relaxed(0, base + GIC_DIST_CTRL);
+ writel_relaxed(GIC_DIST_DISABLE, base + GIC_DIST_CTRL);
/*
* Set all global interrupts to be level triggered, active low.
*/
for (i = 32; i < gic_irqs; i += 16)
- writel_relaxed(0, base + GIC_DIST_CONFIG + i * 4 / 16);
+ writel_relaxed(GIC_DIST_INT_LVL_TRIGGER,
+ base + GIC_DIST_CONFIG + i * 4 / 16);
/*
* Set all global interrupts to this CPU only.
@@ -405,16 +423,18 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
* Set priority on all global interrupts.
*/
for (i = 32; i < gic_irqs; i += 4)
- writel_relaxed(0xa0a0a0a0, base + GIC_DIST_PRI + i * 4 / 4);
+ writel_relaxed(GIC_DIST_INT_DEF_PRI_4,
+ base + GIC_DIST_PRI + i * 4 / 4);
/*
* Disable all interrupts. Leave the PPI and SGIs alone
* as these enables are banked registers.
*/
for (i = 32; i < gic_irqs; i += 32)
- writel_relaxed(0xffffffff, base + GIC_DIST_ENABLE_CLEAR + i * 4 / 32);
+ writel_relaxed(GIC_DIST_INT_CLR_EN_32,
+ base + GIC_DIST_ENABLE_CLEAR + i * 4 / 32);
- writel_relaxed(1, base + GIC_DIST_CTRL);
+ writel_relaxed(GIC_DIST_ENABLE, base + GIC_DIST_CTRL);
}
static void gic_cpu_init(struct gic_chip_data *gic)
@@ -443,17 +463,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
* Deal with the banked PPI and SGI interrupts - disable all
* PPI interrupts, ensure all SGI interrupts are enabled.
*/
- writel_relaxed(0xffff0000, dist_base + GIC_DIST_ENABLE_CLEAR);
- writel_relaxed(0x0000ffff, dist_base + GIC_DIST_ENABLE_SET);
+ writel_relaxed(GIC_DIST_INT_CLR_EN_PPI,
+ dist_base + GIC_DIST_ENABLE_CLEAR);
+ writel_relaxed(GIC_DIST_INT_SET_EN_SGI,
+ dist_base + GIC_DIST_ENABLE_SET);
/*
* Set priority on PPI and SGI interrupts
*/
for (i = 0; i < 32; i += 4)
- writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
+ writel_relaxed(GIC_DIST_INT_DEF_PRI_4,
+ dist_base + GIC_DIST_PRI + i * 4 / 4);
- writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
- writel_relaxed(1, base + GIC_CPU_CTRL);
+ writel_relaxed(GIC_CPU_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
+ writel_relaxed(GIC_CPU_ENABLE, base + GIC_CPU_CTRL);
}
void gic_cpu_if_down(void)
@@ -519,14 +542,14 @@ static void gic_dist_restore(unsigned int gic_nr)
if (!dist_base)
return;
- writel_relaxed(0, dist_base + GIC_DIST_CTRL);
+ writel_relaxed(GIC_DIST_DISABLE, dist_base + GIC_DIST_CTRL);
for (i = 0; i < DIV_ROUND_UP(gic_irqs, 16); i++)
writel_relaxed(gic_data[gic_nr].saved_spi_conf[i],
dist_base + GIC_DIST_CONFIG + i * 4);
for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
- writel_relaxed(0xa0a0a0a0,
+ writel_relaxed(GIC_DIST_INT_DEF_PRI_4,
dist_base + GIC_DIST_PRI + i * 4);
for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
@@ -537,7 +560,7 @@ static void gic_dist_restore(unsigned int gic_nr)
writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
dist_base + GIC_DIST_ENABLE_SET + i * 4);
- writel_relaxed(1, dist_base + GIC_DIST_CTRL);
+ writel_relaxed(GIC_DIST_ENABLE, dist_base + GIC_DIST_CTRL);
}
static void gic_cpu_save(unsigned int gic_nr)
@@ -591,10 +614,11 @@ static void gic_cpu_restore(unsigned int gic_nr)
writel_relaxed(ptr[i], dist_base + GIC_DIST_CONFIG + i * 4);
for (i = 0; i < DIV_ROUND_UP(32, 4); i++)
- writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
+ writel_relaxed(GIC_DIST_INT_DEF_PRI_4,
+ dist_base + GIC_DIST_PRI + i * 4);
- writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
- writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+ writel_relaxed(GIC_CPU_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK);
+ writel_relaxed(GIC_CPU_ENABLE, cpu_base + GIC_CPU_CTRL);
}
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 45e2d8c..7ed92d0 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -21,8 +21,6 @@
#define GIC_CPU_ACTIVEPRIO 0xd0
#define GIC_CPU_IDENT 0xfc
-#define GICC_IAR_INT_ID_MASK 0x3ff
-
#define GIC_DIST_CTRL 0x000
#define GIC_DIST_CTR 0x004
#define GIC_DIST_IGROUP 0x080
--
1.9.1
On Wed, 25 Jun 2014, Feng Kan wrote:
> This change is made to preserve the GIC v2 bypass bits in the
> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
> This code will preserve all bits configured by the bootloader regarding
> v2 bypass group bits. In the X-Gene platform, the bypass functionality
> is not used and bypass bits should not be changed by the kernel gic
> code as it could lead to incorrect behavior.
>
> Signed-off-by: Vinayak Kale <[email protected]>
> Signed-off-by: Feng Kan <[email protected]>
Who wrote the patch? According to the SOB chain it's Vinayak, but your
patch is missing the:
From: Vinayak Kale <[email protected]>
before the actual changelog starts.
On Wed, Jun 25, 2014 at 6:05 PM, Thomas Gleixner <[email protected]> wrote:
> On Wed, 25 Jun 2014, Feng Kan wrote:
>
>> This change is made to preserve the GIC v2 bypass bits in the
>> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
>> This code will preserve all bits configured by the bootloader regarding
>> v2 bypass group bits. In the X-Gene platform, the bypass functionality
>> is not used and bypass bits should not be changed by the kernel gic
>> code as it could lead to incorrect behavior.
>>
>> Signed-off-by: Vinayak Kale <[email protected]>
>> Signed-off-by: Feng Kan <[email protected]>
>
> Who wrote the patch? According to the SOB chain it's Vinayak, but your
> patch is missing the:
I wrote the patch which was based on Vinayak's original change. I can
change it to
Reviewed-by. I was trying to give him credit.
>
> From: Vinayak Kale <[email protected]>
>
> before the actual changelog starts.
On Thu, Jun 26, 2014 at 09:48:25AM -0700, Feng Kan wrote:
> On Wed, Jun 25, 2014 at 6:05 PM, Thomas Gleixner <[email protected]> wrote:
> > On Wed, 25 Jun 2014, Feng Kan wrote:
> >
> >> This change is made to preserve the GIC v2 bypass bits in the
> >> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
> >> This code will preserve all bits configured by the bootloader regarding
> >> v2 bypass group bits. In the X-Gene platform, the bypass functionality
> >> is not used and bypass bits should not be changed by the kernel gic
> >> code as it could lead to incorrect behavior.
> >>
> >> Signed-off-by: Vinayak Kale <[email protected]>
> >> Signed-off-by: Feng Kan <[email protected]>
> >
> > Who wrote the patch? According to the SOB chain it's Vinayak, but your
> > patch is missing the:
>
> I wrote the patch which was based on Vinayak's original change. I can
> change it to
> Reviewed-by. I was trying to give him credit.
>
Then please leave the S-o-b's alone and append a note describing the
changes you made to his original patch.
eg:
----->8-------------
From: Vinayak Kale <[email protected]>
This change is made to preserve the GIC v2 bypass bits in the
GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
This code will preserve all bits configured by the bootloader regarding
v2 bypass group bits. In the X-Gene platform, the bypass functionality
is not used and bypass bits should not be changed by the kernel gic
code as it could lead to incorrect behavior.
[Feng Kan: Rebased on magic number removal patch, added feature X, fixed
bug Y.]
Signed-off-by: Vinayak Kale <[email protected]>
Signed-off-by: Feng Kan <[email protected]>
----->8-------------
'git am' will use the info from the last 'From:' as the author of the
commit.
It goes without saying (even though I'm mentioning it ;-) ) the S-o-b
means a very specific thing. It is *not* there to enhance credit. It
is there to indicate that the person referenced (yourself and Vinayak)
have read, understood, and consent to the Developer's Certificate of
Origin [1]. Nothing else.
The commit message tags have one purpose: To accurately render
information about a commit. Who wrote it, who applied it, who reviewed
it, who let it go by a different tree than the default for a subsystem,
etc.
Attempts to (mis)use them as a corporate evaluation metric only leads to
developers trying to game the system. More importantly, it wastes
maintainers time because now we have to question whether the appended
tags are indeed factual. :(
thx,
Jason.
[1] Documentation/SubmittingPatches, Section 12.