GIC: Fix setting interrupt configuration

  - Interrupt configuration is a 2-bit field, so the field shift has to
    be double that of the bit number.

  - Interrupt configuration (level- or edge-trigger) is specified in the
    MSB of the field, not LSB.

Fixes applied to both GICv2 and GICv3 drivers.

Fixes ARM-software/tf-issues#570

Change-Id: Ia6ae6ed9ba9fb0e3eb0f921a833af48e365ba359
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>

diff --git a/drivers/arm/gic/common/gic_common.c b/drivers/arm/gic/common/gic_common.c
index d523772b1f4b852b5161524933caed6b38045e68..07ed63d7467bb5c04e2ba0a68b181e047c89c604 100644 (file)
--- a/drivers/arm/gic/common/gic_common.c
+++ b/drivers/arm/gic/common/gic_common.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015-2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2018, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -302,12 +302,15 @@ void gicd_set_ipriorityr(uintptr_t base, unsigned int id, unsigned int pri)
 
 void gicd_set_icfgr(uintptr_t base, unsigned int id, unsigned int cfg)
 {
-       unsigned bit_num = id & ((1 << ICFGR_SHIFT) - 1);
+       /* Interrupt configuration is a 2-bit field */
+       unsigned int bit_num = id & ((1 << ICFGR_SHIFT) - 1);
+       unsigned int bit_shift = bit_num << 1;
+
        uint32_t reg_val = gicd_read_icfgr(base, id);
 
        /* Clear the field, and insert required configuration */
-       reg_val &= ~(GIC_CFG_MASK << bit_num);
-       reg_val |= ((cfg & GIC_CFG_MASK) << bit_num);
+       reg_val &= ~(GIC_CFG_MASK << bit_shift);
+       reg_val |= ((cfg & GIC_CFG_MASK) << bit_shift);
 
        gicd_write_icfgr(base, id, reg_val);
 }

diff --git a/drivers/arm/gic/v3/gicv3_helpers.c b/drivers/arm/gic/v3/gicv3_helpers.c
index dee63f18a9beb5bc968faf4ba3050ce694a4d0fa..69c6951281d3eb46ee6deecdf62f523fc84fb624 100644 (file)
--- a/drivers/arm/gic/v3/gicv3_helpers.c
+++ b/drivers/arm/gic/v3/gicv3_helpers.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015-2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2018, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -232,12 +232,15 @@ void gicr_set_ipriorityr(uintptr_t base, unsigned int id, unsigned int pri)
  */
 void gicr_set_icfgr0(uintptr_t base, unsigned int id, unsigned int cfg)
 {
-       unsigned bit_num = id & ((1 << ICFGR_SHIFT) - 1);
+       /* Interrupt configuration is a 2-bit field */
+       unsigned int bit_num = id & ((1 << ICFGR_SHIFT) - 1);
+       unsigned int bit_shift = bit_num << 1;
+
        uint32_t reg_val = gicr_read_icfgr0(base);
 
        /* Clear the field, and insert required configuration */
-       reg_val &= ~(GIC_CFG_MASK << bit_num);
-       reg_val |= ((cfg & GIC_CFG_MASK) << bit_num);
+       reg_val &= ~(GIC_CFG_MASK << bit_shift);
+       reg_val |= ((cfg & GIC_CFG_MASK) << bit_shift);
 
        gicr_write_icfgr0(base, reg_val);
 }
@@ -248,12 +251,15 @@ void gicr_set_icfgr0(uintptr_t base, unsigned int id, unsigned int cfg)
  */
 void gicr_set_icfgr1(uintptr_t base, unsigned int id, unsigned int cfg)
 {
-       unsigned bit_num = id & ((1 << ICFGR_SHIFT) - 1);
+       /* Interrupt configuration is a 2-bit field */
+       unsigned int bit_num = id & ((1 << ICFGR_SHIFT) - 1);
+       unsigned int bit_shift = bit_num << 1;
+
        uint32_t reg_val = gicr_read_icfgr1(base);
 
        /* Clear the field, and insert required configuration */
-       reg_val &= ~(GIC_CFG_MASK << bit_num);
-       reg_val |= ((cfg & GIC_CFG_MASK) << bit_num);
+       reg_val &= ~(GIC_CFG_MASK << bit_shift);
+       reg_val |= ((cfg & GIC_CFG_MASK) << bit_shift);
 
        gicr_write_icfgr1(base, reg_val);
 }

diff --git a/include/drivers/arm/gic_common.h b/include/drivers/arm/gic_common.h
index 001f57360d996ce401df4e6bad7c3b360daed01c..6e953a0dcf7587ca388a7a27f043f01b9b0acdb3 100644 (file)
--- a/include/drivers/arm/gic_common.h
+++ b/include/drivers/arm/gic_common.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015-2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2018, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -29,9 +29,9 @@
 /* Constant to indicate a spurious interrupt in all GIC versions */
 #define GIC_SPURIOUS_INTERRUPT         1023
 
-/* Interrupt configurations */
-#define GIC_INTR_CFG_LEVEL             0
-#define GIC_INTR_CFG_EDGE              1
+/* Interrupt configurations: 2-bit fields with LSB reserved */
+#define GIC_INTR_CFG_LEVEL             (0 << 1)
+#define GIC_INTR_CFG_EDGE              (1 << 1)
 
 /* Constants to categorise priorities */
 #define GIC_HIGHEST_SEC_PRIORITY       0x0