enable Hisilicon HiP04

There's some difference between ARM GICv2 and HiP04 GIC.

* HiP04 GIC could support 16 cores at most, and ARM GIC could support 8 cores at most. So the defination on GIC_DIST_TARGET registers are different since CPU interfaces are increased from 8-bit to 16-bit.

* HiP04 GIC could support 510 interrupts at most, and ARM GIC could support 1020 interrupts at most.

Changelog:
v14:
* Mount function pointers to different implementation on standard GICv2 and Hisilicon HiP04 GIC.

Signed-off-by Haojian Zhuang <[email protected]>

---
 Documentation/devicetree/bindings/arm/gic.txt |   1 +
 drivers/irqchip/irq-gic.c                     | 436 +++++++++++++++++++++-----
 2 files changed, 350 insertions(+), 87 deletions(-)

Documentation/devicetree/bindings/arm/gic.txt +1 -0

index 5573c08..150f7d6 100644
@@ -16,6 +16,7 @@ Main node required properties:
        "arm,cortex-a9-gic"
        "arm,cortex-a7-gic"
        "arm,arm11mp-gic"
+       "hisilicon,hip04-gic"
 - interrupt-controller : Identifies the node as an interrupt controller
 - #interrupt-cells : Specifies the number of cells needed to encode an
   interrupt source.  The type shall be a <u32> and the value shall be 3.

drivers/irqchip/irq-gic.c +0 -0

index 508b815..b47243f 100644

drivers/irqchip/irq-gic.c +110 -87

+       u32 (*get_cpumask)(struct gic_chip_data *);
+       void (*set_dist_target)(struct gic_chip_data *, u32, u32);
+       void (*set_dist_softint)(struct gic_chip_data *, u32, u32);
+       void (*dist_init)(struct gic_chip_data *);
+       void (*dist_save)(unsigned int);
+       void (*dist_restore)(unsigned int);
+       u32 nr_cpu_if;
+       u32 max_nr_irq;
 };
 
 static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 
 /*
- * The GIC mapping of CPU interfaces does not necessarily match
- * the logical CPU numbering.  Let's use a mapping as returned
- * by the GIC itself.
- */
-#define NR_GIC_CPU_IF 8
-static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
-
-/*
  * Supported arch specific GIC irq extension.
  * Default make them NULL.
  */

@@ -222,23 +226,21 @@ static int gic_retrigger(struct irq_data *d)
 static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
                            bool force)
 {
-       void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3);
-       unsigned int cpu, shift = (gic_irq(d) % 4) * 8;
-       u32 val, mask, bit;
+       struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+       unsigned int cpu;
+       u32 bit;
 
        if (!force)
                cpu = cpumask_any_and(mask_val, cpu_online_mask);
        else
                cpu = cpumask_first(mask_val);
 
-       if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
+       if (gic_data->cpu_invalid(cpu) || cpu >= nr_cpu_ids)
                return -EINVAL;
 
        raw_spin_lock(&irq_controller_lock);
-       mask = 0xff << shift;
-       bit = gic_cpu_map[cpu] << shift;
-       val = readl_relaxed(reg) & ~mask;
-       writel_relaxed(val | bit, reg);
+       bit = gic_data->get_cpu_map(cpu);
+       gic_data->set_dist_target(gic_data, gic_irq(d), bit);
        raw_spin_unlock(&irq_controller_lock);
 
        return IRQ_SET_MASK_OK;

@@ -304,7 +306,7 @@ static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
                goto out;
 
        cascade_irq = irq_find_mapping(chip_data->domain, gic_irq);
-       if (unlikely(gic_irq < 32 || gic_irq > 1020))
+       if (unlikely(gic_irq < 32 || gic_irq > chip_data->max_nr_irq))
                handle_bad_irq(cascade_irq, desc);
        else
                generic_handle_irq(cascade_irq);

@@ -335,69 +337,31 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
        irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
-static u8 gic_get_cpumask(struct gic_chip_data *gic)
-{
-       void __iomem *base = gic_data_dist_base(gic);
-       u32 mask, i;
-
-       for (i = mask = 0; i < 32; i += 4) {
-               mask = readl_relaxed(base + GIC_DIST_TARGET + i);
-               mask |= mask >> 16;
-               mask |= mask >> 8;
-               if (mask)
-                       break;
-       }
-
-       if (!mask)
-               pr_crit("GIC CPU mask not found - kernel will fail to boot.\n");
-
-       return mask;
-}
-
-static void __init gic_dist_init(struct gic_chip_data *gic)
-{
-       unsigned int i;
-       u32 cpumask;
-       unsigned int gic_irqs = gic->gic_irqs;
-       void __iomem *base = gic_data_dist_base(gic);
-
-       writel_relaxed(0, base + GIC_DIST_CTRL);
-
-       /*
-        * Set all global interrupts to this CPU only.
-        */
-       cpumask = gic_get_cpumask(gic);
-       cpumask |= cpumask << 8;
-       cpumask |= cpumask << 16;
-       for (i = 32; i < gic_irqs; i += 4)
-               writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
-
-       gic_dist_config(base, gic_irqs, NULL);
-
-       writel_relaxed(1, base + GIC_DIST_CTRL);
-}
-
 static void gic_cpu_init(struct gic_chip_data *gic)
 {
        void __iomem *dist_base = gic_data_dist_base(gic);
        void __iomem *base = gic_data_cpu_base(gic);
        unsigned int cpu_mask, cpu = smp_processor_id();
        int i;
+       u32 data;
 
        /*
         * 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;
+       BUG_ON(gic->cpu_invalid(cpu));
+       cpu_mask = gic->get_cpumask(gic);
+       gic->set_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;
+       for (i = 0; i < gic->nr_cpu_if; i++) {
+               if (i != cpu) {
+                       data = gic->get_cpu_map(i);
+                       gic->set_cpu_map(i, data & ~cpu_mask);
+               }
+       }
 
        gic_cpu_config(dist_base, NULL);
 

@@ -489,6 +453,70 @@ static void gic_dist_restore(unsigned int gic_nr)
        writel_relaxed(1, dist_base + GIC_DIST_CTRL);
 }
 
+static void hip04_dist_save(unsigned int gic_nr)
+{
+       unsigned int gic_irqs;
+       void __iomem *dist_base;
+       int i;
+
+       if (gic_nr >= MAX_GIC_NR)
+               BUG();
+
+       gic_irqs = gic_data[gic_nr].gic_irqs;
+       dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+
+       if (!dist_base)
+               return;
+
+       for (i = 0; i < DIV_ROUND_UP(gic_irqs, 16); i++)
+               gic_data[gic_nr].saved_spi_conf[i] =
+                       readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
+
+       for (i = 0; i < DIV_ROUND_UP(gic_irqs, 2); i++)
+               gic_data[gic_nr].saved_spi_target[i] =
+                       readl_relaxed(dist_base + GIC_DIST_TARGET + i * 2);
+
+       for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+               gic_data[gic_nr].saved_spi_enable[i] =
+                       readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
+}
+
+static void hip04_dist_restore(unsigned int gic_nr)
+{
+       unsigned int gic_irqs;
+       unsigned int i;
+       void __iomem *dist_base;
+
+       if (gic_nr >= MAX_GIC_NR)
+               BUG();
+
+       gic_irqs = gic_data[gic_nr].gic_irqs;
+       dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+
+       if (!dist_base)
+               return;
+
+       writel_relaxed(0, 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,
+                       dist_base + GIC_DIST_PRI + i * 4);
+
+       for (i = 0; i < DIV_ROUND_UP(gic_irqs, 2); i++)
+               writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
+                       dist_base + GIC_DIST_TARGET + i * 2);
+
+       for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
+               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);
+}
+
 static void gic_cpu_save(unsigned int gic_nr)
 {
        int i;

@@ -565,11 +593,11 @@ static int gic_notifier(struct notifier_block *self, unsigned long cmd,   void *v)
                        gic_cpu_restore(i);
                        break;
                case CPU_CLUSTER_PM_ENTER:
-                       gic_dist_save(i);
+                       gic_data[i].dist_save(i);
                        break;
                case CPU_CLUSTER_PM_ENTER_FAILED:
                case CPU_CLUSTER_PM_EXIT:
-                       gic_dist_restore(i);
+                       gic_data[i].dist_restore(i);
                        break;
                }
        }

@@ -610,7 +638,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 
        /* Convert our logical CPU mask into a physical one. */
        for_each_cpu(cpu, mask)
-               map |= gic_cpu_map[cpu];
+               map |= gic_data[0].get_cpu_map(cpu);
 
        /*
         * Ensure that stores to Normal memory are visible to the

@@ -619,7 +647,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
        dmb(ishst);
 
        /* this always happens on GIC0 */
-       writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+       gic_data[0].set_dist_softint(&gic_data[0], irq, map);
 
        raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
 }

@@ -634,10 +662,9 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
  */
 void gic_send_sgi(unsigned int cpu_id, unsigned int irq)
 {
-       BUG_ON(cpu_id >= NR_GIC_CPU_IF);
-       cpu_id = 1 << cpu_id;
+       BUG_ON(gic_data[0].cpu_invalid(cpu_id));
        /* this always happens on GIC0 */
-       writel_relaxed((cpu_id << 16) | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+       gic_data[0].set_dist_softint(&gic_data[0], irq, 1 << cpu_id);
 }
 
 /*

@@ -653,9 +680,9 @@ int gic_get_cpu_id(unsigned int cpu)
 {
        unsigned int cpu_bit;
 
-       if (cpu >= NR_GIC_CPU_IF)
+       if (gic_data[0].cpu_invalid(cpu))
                return -1;
-       cpu_bit = gic_cpu_map[cpu];
+       cpu_bit = gic_data[0].get_cpu_map(cpu);
        if (cpu_bit & (cpu_bit - 1))
                return -1;
        return __ffs(cpu_bit);

@@ -673,6 +700,7 @@ int gic_get_cpu_id(unsigned int cpu)
  */
 void gic_migrate_target(unsigned int new_cpu_id)
 {
+       struct gic_chip_data *gic = &gic_data[gic_nr];
        unsigned int cur_cpu_id, gic_irqs, gic_nr = 0;
        void __iomem *dist_base;
        int i, ror_val, cpu = smp_processor_id();

@@ -681,19 +709,19 @@ void gic_migrate_target(unsigned int new_cpu_id)
        if (gic_nr >= MAX_GIC_NR)
                BUG();
 
-       dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+       dist_base = gic_data_dist_base(gic);
        if (!dist_base)
                return;
-       gic_irqs = gic_data[gic_nr].gic_irqs;
+       gic_irqs = gic->gic_irqs;
 
-       cur_cpu_id = __ffs(gic_cpu_map[cpu]);
+       cur_cpu_id = __ffs(gic->get_cpu_map(cpu));
        cur_target_mask = 0x01010101 << cur_cpu_id;
        ror_val = (cur_cpu_id - new_cpu_id) & 31;
 
        raw_spin_lock(&irq_controller_lock);
 
        /* Update the target interface for this logical CPU */
-       gic_cpu_map[cpu] = 1 << new_cpu_id;
+       gic_data->set_cpu_map(cpu, 1 << new_cpu_id);
 
        /*
         * Find all the peripheral interrupts targetting the current

@@ -730,8 +758,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
                writel_relaxed(val, dist_base + GIC_DIST_SGI_PENDING_CLEAR + i);
                for (j = i; j < i + 4; j++) {
                        if (val & 0xff)
-                               writel_relaxed((1 << (new_cpu_id + 16)) | j,
-                                               dist_base + GIC_DIST_SOFTINT);
+                               gic->set_dist_softint(gic, j, 1 << new_cpu_id);
                        val >>= 8;
                }
        }

@@ -883,7 +910,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 {
        irq_hw_number_t hwirq_base;
        struct gic_chip_data *gic;
-       int gic_irqs, irq_base, i;
+       int gic_irqs, irq_base;
        int nr_routable_irqs;
 
        BUG_ON(gic_nr >= MAX_GIC_NR);

@@ -924,8 +951,7 @@ 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;
+       gic->init_cpu_map();
 
        /*
         * For primary GICs, skip over SGIs.

@@ -941,12 +967,13 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 
        /*
         * Find out how many interrupts are supported.
-        * The GIC only supports up to 1020 interrupt sources.
+        * The ARM/Qualcomm GIC only supports up to 1020 interrupt sources.
+        * The HiP04 GIC only supports up to 510 interrupt sources.
         */
        gic_irqs = readl_relaxed(gic_data_dist_base(gic) + GIC_DIST_CTR) & 0x1f;
        gic_irqs = (gic_irqs + 1) * 32;
-       if (gic_irqs > 1020)
-               gic_irqs = 1020;
+       if (gic_irqs > gic->max_nr_irq)
+               gic_irqs = gic->max_nr_irq;
        gic->gic_irqs = gic_irqs;
 
        gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */

@@ -981,7 +1008,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
        }
 
        gic_chip.flags |= gic_arch_extn.flags;
-       gic_dist_init(gic);
+       gic->dist_init(gic);
        gic_cpu_init(gic);
        gic_pm_init(gic);
 }

drivers/irqchip/irq-gic.c +147 -0

+/*
+ * The GIC mapping of CPU interfaces does not necessarily match
+ * the logical CPU numbering.  Let's use a mapping as returned
+ * by the GIC itself.
+ */
+#define NR_GIC_CPU_IF 8
+static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
+
+static void gic_init_cpu_map(void)
+{
+       int i;
+       for (i = 0; i < NR_GIC_CPU_IF; i++)
+               gic_cpu_map[i] = 0xff;
+}
+
+static u32 gic_get_cpu_map(u32 i)
+{
+       return gic_cpu_map[i];
+}
+
+static void gic_set_cpu_map(u32 i, u32 data)
+{
+       gic_cpu_map[i] = data & 0xff;
+}
+
+static bool gic_cpu_invalid(u32 cpu)
+{
+       return cpu >= NR_GIC_CPU_IF;
+}
+
+static u32 gic_get_cpumask(struct gic_chip_data *gic)
+{
+       void __iomem *base = gic_data_dist_base(gic);
+       u32 mask, i;
+
+       for (i = mask = 0; i < 32; i += 4) {
+               mask = readl_relaxed(base + GIC_DIST_TARGET + i);
+               mask |= mask >> 16;
+               mask |= mask >> 8;
+               if (mask)
+                       break;
+       }
+
+       if (!mask)
+               pr_crit("GIC CPU mask not found - kernel will fail to boot.\n");
+
+       return mask & 0xff;
+}
+
+static void gic_set_dist_target(struct gic_chip_data *gic, u32 irq, u32 data)
+{
+       void __iomem *base = gic_data_dist_base(gic);
+       u32 val, mask, offset, shift = (irq % 4) * 8;
+
+       mask = 0xff << shift;
+       offset = irq & ~3U;
+       val = readl_relaxed(base + GIC_DIST_TARGET + offset) & ~mask;
+       val |= data << shift;
+       writel_relaxed(val, base + GIC_DIST_TARGET + offset);
+}
+
+static void gic_set_dist_softint(struct gic_chip_data *gic, u32 irq, u32 data)
+{
+       void __iomem *base = gic_data_dist_base(gic);
+
+       data = data << 16;
+       writel_relaxed(data | irq, base + GIC_DIST_SOFTINT);
+}
+
+static void gic_dist_init(struct gic_chip_data *gic)
+{
+       unsigned int i;
+       u32 cpumask;
+       unsigned int gic_irqs = gic->gic_irqs;
+       void __iomem *base = gic_data_dist_base(gic);
+
+       writel_relaxed(0, base + GIC_DIST_CTRL);
+
+       /*
+        * Set all global interrupts to this CPU only.
+        */
+       cpumask = gic_get_cpumask(gic);
+       cpumask |= cpumask << 8;
+       cpumask |= cpumask << 16;
+       for (i = 32; i < gic_irqs; i += 4)
+               writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 4 / 4);
+
+       gic_dist_config(base, gic_irqs, NULL);
+
+       writel_relaxed(1, base + GIC_DIST_CTRL);
+}
+
 static int __init
 gic_of_init(struct device_node *node, struct device_node *parent)
 {

@@ -1009,6 +1128,148 @@ gic_of_init(struct device_node *node, struct device_node *parent)
        if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
                percpu_offset = 0;
 
+       gic_data[gic_cnt].nr_cpu_if = 8;
+       gic_data[gic_cnt].init_cpu_map = gic_init_cpu_map;
+       gic_data[gic_cnt].get_cpu_map = gic_get_cpu_map;
+       gic_data[gic_cnt].set_cpu_map = gic_set_cpu_map;
+       gic_data[gic_cnt].cpu_invalid = gic_cpu_invalid;
+       gic_data[gic_cnt].get_cpumask = gic_get_cpumask;
+       gic_data[gic_cnt].dist_init = gic_dist_init;
+       gic_data[gic_cnt].dist_save = gic_dist_save;
+       gic_data[gic_cnt].dist_restore = gic_dist_restore;
+       gic_data[gic_cnt].set_dist_target = gic_set_dist_target;
+       gic_data[gic_cnt].set_dist_softint = gic_set_dist_softint;
+       gic_data[gic_cnt].max_nr_irq = 1020;
+       gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
+       if (!gic_cnt)
+               gic_init_physaddr(node);
+
+       if (parent) {
+               irq = irq_of_parse_and_map(node, 0);
+               gic_cascade_irq(gic_cnt, irq);
+       }
+       gic_cnt++;
+       return 0;
+}
+
+/* HiP04 extends the number of CPU interface from 8 to 16 */
+#define NR_HIP04_CPU_IF        16
+static u16 hip04_cpu_map[NR_HIP04_CPU_IF] __read_mostly;
+
+static void hip04_init_cpu_map(void)
+{
+       int i;
+       for (i = 0; i < NR_HIP04_CPU_IF; i++)
+               hip04_cpu_map[i] = 0xffff;
+}
+
+static u32 hip04_get_cpu_map(u32 i)
+{
+       return hip04_cpu_map[i];
+}
+
+static void hip04_set_cpu_map(u32 i, u32 data)
+{
+       hip04_cpu_map[i] = data & 0xffff;
+}
+
+static bool hip04_cpu_invalid(u32 cpu)
+{
+       return cpu >= NR_HIP04_CPU_IF;
+}
+
+static u32 hip04_get_cpumask(struct gic_chip_data *gic)
+{
+       void __iomem *base = gic_data_dist_base(gic);
+       u32 mask, i;
+

drivers/irqchip/irq-gic.c +78 -0

+
+       return mask & 0xffff;
+}
+
+static void hip04_set_dist_target(struct gic_chip_data *gic, u32 irq, u32 data)
+{
+       void __iomem *base = gic_data_dist_base(gic);
+       u32 val, mask, offset, shift = (irq % 2) * 16;
+
+       mask = 0xffff << shift;
+       offset = (irq * 2) & ~3U;
+       val = readl_relaxed(base + GIC_DIST_TARGET + offset) & ~mask;
+       val |= data << shift;
+       writel_relaxed(val, base + GIC_DIST_TARGET + offset);
+}
+
+static void hip04_set_dist_softint(struct gic_chip_data *gic, u32 irq, u32 data)
+{
+       void __iomem *base = gic_data_dist_base(gic);
+
+       data = data << 8;
+       writel_relaxed(data | irq, base + GIC_DIST_SOFTINT);
+}
+
+static void hip04_dist_init(struct gic_chip_data *gic)
+{
+       unsigned int i;
+       u32 cpumask;
+       unsigned int gic_irqs = gic->gic_irqs;
+       void __iomem *base = gic_data_dist_base(gic);
+
+       writel_relaxed(0, base + GIC_DIST_CTRL);
+
+       /*
+        * Set all global interrupts to this CPU only.
+        */
+       cpumask = hip04_get_cpumask(gic);
+       cpumask |= cpumask << 16;
+       for (i = 32; i < gic_irqs; i += 2)
+               writel_relaxed(cpumask, base + GIC_DIST_TARGET + i * 2);
+
+       gic_dist_config(base, gic_irqs, NULL);
+
+       writel_relaxed(1, base + GIC_DIST_CTRL);
+}
+
+static int __init
+hip04_of_init(struct device_node *node, struct device_node *parent)
+{
+       void __iomem *cpu_base;
+       void __iomem *dist_base;
+       u32 percpu_offset;
+       int irq;
+
+       if (WARN_ON(!node))
+               return -ENODEV;
+
+       dist_base = of_iomap(node, 0);
+       WARN(!dist_base, "unable to map gic dist registers\n");
+
+       cpu_base = of_iomap(node, 1);
+       WARN(!cpu_base, "unable to map gic cpu registers\n");
+
+       if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
+               percpu_offset = 0;
+
+       gic_data[gic_cnt].nr_cpu_if = 16;
+       gic_data[gic_cnt].init_cpu_map = hip04_init_cpu_map;
+       gic_data[gic_cnt].get_cpu_map = hip04_get_cpu_map;
+       gic_data[gic_cnt].set_cpu_map = hip04_set_cpu_map;
+       gic_data[gic_cnt].cpu_invalid = hip04_cpu_invalid;
+       gic_data[gic_cnt].get_cpumask = hip04_get_cpumask;
+       gic_data[gic_cnt].dist_init = hip04_dist_init;
+       gic_data[gic_cnt].dist_save = hip04_dist_save;
+       gic_data[gic_cnt].dist_restore = hip04_dist_restore;
+       gic_data[gic_cnt].set_dist_target = hip04_set_dist_target;
+       gic_data[gic_cnt].set_dist_softint = hip04_set_dist_softint;
+       gic_data[gic_cnt].max_nr_irq = 510;
        gic_init_bases(gic_cnt, -1, dist_base, cpu_base, percpu_offset, node);
        if (!gic_cnt)

                gic_init_physaddr(node);

Overall, this code should be able to sitting within a #ifdef/#endif block, only selected if this platform is enabled, and possibly find a way not to impact all the other platforms when this is not selected.

Finally, I'd like to outline how much I dislike the way the GIC architecture has been abused here. Yes, this solves a particular problem, at a given point in time, but this also feels extremely short sighted from whoever has put this thing together. This really feels like a short-term HW hack that is already addressed by GICv3. Merging support for non architecture compliant HW is never the best solution.

Quoted 10 lines · On Monday 28 July 2014 18:38:58 Marc Zyngier wrote:

Overall, this code should be able to sitting within a #ifdef/#endif block, only selected if this platform is enabled, and possibly find a way not to impact all the other platforms when this is not selected.

Finally, I'd like to outline how much I dislike the way the GIC architecture has been abused here. Yes, this solves a particular problem, at a given point in time, but this also feels extremely short sighted from whoever has put this thing together. This really feels like a short-term HW hack that is already addressed by GICv3. Merging support for non architecture compliant HW is never the best solution.

Not merging support for shipping hardware is also not a solution, and we already support any number of irqchip drivers and have all the logic we need to sort that out at runtime.

My feeling is that the original approach taken in the early version of the patch set was actually better, given all the problems with integrating this nicely into the gic driver.

Would you be happier with that? The downside of that would be that some driver bugs would get fixed in one driver but not the other one, which is why we normally try to avoid it. On the other hand, it means that the normal GIC driver does not get polluted with hacks that are only needed for the hip04 version and there is an obvious way to disable the driver at compile time when you build for other platforms.

Quoted 10 lines · On Monday 28 July 2014 18:38:58 Marc Zyngier wrote:

Overall, this code should be able to sitting within a #ifdef/#endif block, only selected if this platform is enabled, and possibly find a way not to impact all the other platforms when this is not selected.

Finally, I'd like to outline how much I dislike the way the GIC architecture has been abused here. Yes, this solves a particular problem, at a given point in time, but this also feels extremely short sighted from whoever has put this thing together. This really feels like a short-term HW hack that is already addressed by GICv3. Merging support for non architecture compliant HW is never the best solution.

Not merging support for shipping hardware is also not a solution, and we already support any number of irqchip drivers and have all the logic we need to sort that out at runtime.

My feeling is that the original approach taken in the early version of the patch set was actually better, given all the problems with integrating this nicely into the gic driver.

Would you be happier with that? The downside of that would be that some driver bugs would get fixed in one driver but not the other one, which is why we normally try to avoid it. On the other hand, it means that the normal GIC driver does not get polluted with hacks that are only needed for the hip04 version and there is an obvious way to disable the driver at compile time when you build for other platforms.

The DT binding for Hisilicon HiP04 clock driver.

Signed-off-by Haojian Zhuang <[email protected]>

---
 .../devicetree/bindings/clock/hip04-clock.txt        | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +0 -0

new file mode 100644
index 0000000..4d31ae3

Quoted 17 lines · Haojian Zhuang

The DT binding for Hisilicon HiP04 clock driver.

Signed-off-by Haojian Zhuang <[email protected]>

---
 .../devicetree/bindings/clock/hip04-clock.txt        | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +3 -0

new file mode 100644
index 0000000..4d31ae3
@@ -0,0 +1,20 @@
+* Hisilicon HiP04 Clock Controller
+
+The HiP04 clock controller generates and supplies clock to various

Documentation/devicetree/bindings/clock/hip04-clock.txt line 9

Quoted 6 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +6 -0

+controllers within the HiP04 SoC.
+
+Required Properties:
+
+- compatible: should be one of the following.
+  - "hisilicon,hip04-clock" - controller compatible with HiP04 SoC.

There's only one clock controller on-board and it's custom for HiP04?

Documentation/devicetree/bindings/clock/hip04-clock.txt line 20

Quoted 11 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +11 -0

+
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+- #clock-cells: should be 1.
+
+
+Each clock is assigned an identifier and client nodes use this identifier
+to specify the clock which they consume.
+
+All these identifier could be found in <dt-bindings/clock/hip04-clock.h>.

This must come in the same patch.

Mark.

Quoted 18 lines · Mark Rutland

Quoted 17 lines · Haojian Zhuang

The DT binding for Hisilicon HiP04 clock driver.

Signed-off-by Haojian Zhuang <[email protected]>

---
 .../devicetree/bindings/clock/hip04-clock.txt        | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +3 -0

new file mode 100644
index 0000000..4d31ae3
@@ -0,0 +1,20 @@
+* Hisilicon HiP04 Clock Controller
+
+The HiP04 clock controller generates and supplies clock to various

Quoted 9 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt line 9

Quoted 6 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +6 -0

+controllers within the HiP04 SoC.
+
+Required Properties:
+
+- compatible: should be one of the following.
+  - "hisilicon,hip04-clock" - controller compatible with HiP04 SoC.

There's only one clock controller on-board and it's custom for HiP04?

Since all those PLL settings are configured in bootloader, kernel don't need to configure it again.

By the way, I don't have the manual to configure those PLLs. All the clocks are defined as fixed in kernel side.

Quoted 29 lines · On Tuesday 29 July 2014 10:25:49 Haojian Zhuang wrote:

Quoted 18 lines · Mark Rutland

Quoted 17 lines · Haojian Zhuang

The DT binding for Hisilicon HiP04 clock driver.

Signed-off-by Haojian Zhuang <[email protected]>

---
 .../devicetree/bindings/clock/hip04-clock.txt        | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +3 -0

new file mode 100644
index 0000000..4d31ae3
@@ -0,0 +1,20 @@
+* Hisilicon HiP04 Clock Controller
+
+The HiP04 clock controller generates and supplies clock to various

Quoted 9 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt line 9

Quoted 6 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +6 -0

+controllers within the HiP04 SoC.
+
+Required Properties:
+
+- compatible: should be one of the following.
+  - "hisilicon,hip04-clock" - controller compatible with HiP04 SoC.

There's only one clock controller on-board and it's custom for HiP04?

Quoted 6 lines

Since all those PLL settings are configured in bootloader, kernel don't need to configure it again.

By the way, I don't have the manual to configure those PLLs. All the clocks are defined as fixed in kernel side.

If all the clocks are fixed-rate, you don't actually need a driver for them, just use the regular fixed-rate clock and define one node per clock.

Arnd

Quoted 18 lines · Mark Rutland

Quoted 17 lines · Haojian Zhuang

The DT binding for Hisilicon HiP04 clock driver.

Signed-off-by Haojian Zhuang <[email protected]>

---
 .../devicetree/bindings/clock/hip04-clock.txt        | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +3 -0

new file mode 100644
index 0000000..4d31ae3
@@ -0,0 +1,20 @@
+* Hisilicon HiP04 Clock Controller
+
+The HiP04 clock controller generates and supplies clock to various

Quoted 9 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt line 9

Quoted 6 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +6 -0

+controllers within the HiP04 SoC.
+
+Required Properties:
+
+- compatible: should be one of the following.
+  - "hisilicon,hip04-clock" - controller compatible with HiP04 SoC.

There's only one clock controller on-board and it's custom for HiP04?

Since all those PLL settings are configured in bootloader, kernel don't need to configure it again.

By the way, I don't have the manual to configure those PLLs. All the clocks are defined as fixed in kernel side.

Quoted 14 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt line 20

Quoted 11 lines

Documentation/devicetree/bindings/clock/hip04-clock.txt new file +11 -0

+
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+- #clock-cells: should be 1.
+
+
+Each clock is assigned an identifier and client nodes use this identifier
+to specify the clock which they consume.
+
+All these identifier could be found in <dt-bindings/clock/hip04-clock.h>.

This must come in the same patch.

Quoted 1 line

Mark.

HiP04 clock driver is already merged into mainline.

Add hip04-d01.dts & hip04.dtsi for hip04 SoC platform.

Signed-off-by Haojian Zhuang <[email protected]>

---
 arch/arm/boot/dts/Makefile      |   1 +
 arch/arm/boot/dts/hip04-d01.dts |  39 ++++++
 arch/arm/boot/dts/hip04.dtsi    | 267 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 arch/arm/boot/dts/hip04-d01.dts
 create mode 100644 arch/arm/boot/dts/hip04.dtsi

arch/arm/boot/dts/Makefile +1 -0

index 721525e..6587bbf 100644
@@ -86,6 +86,7 @@ dtb-$(CONFIG_ARCH_HI3xxx) += hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIX5HD2) += hix5hd2-dkb.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += highbank.dtb \
        ecx-2000.dtb
+dtb-$(CONFIG_ARCH_HIP04) += hip04-d01.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += integratorap.dtb \
        integratorcp.dtb
 dtb-$(CONFIG_ARCH_KEYSTONE) += k2hk-evm.dtb \

arch/arm/boot/dts/hip04-d01.dts new file +3 -0

new file mode 100644
index 0000000..661c8e5
@@ -0,0 +1,39 @@
+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>

arch/arm/boot/dts/hip04-d01.dts new file +9 -0

+
+#include "hip04.dtsi"
+
+/ {
+       /* memory bus is 64-bit */
+       #address-cells = <2>;
+       #size-cells = <2>;
+       model = "Hisilicon D01 Development Board";
+       compatible = "hisilicon,hip04-d01";

arch/arm/boot/dts/hip04-d01.dts new file +7 -0

+
+       soc {
+               uart0: uart at 4007000 {
+                       status = "ok";
+               };
+       };
+};

arch/arm/boot/dts/hip04.dtsi new file +1 -0

new file mode 100644
index 0000000..30942be
@@ -0,0 +1,267 @@
+/*

arch/arm/boot/dts/hip04.dtsi new file +19 -0

+               #clock-cells = <1>;
+       };
+
+       timer {
+               compatible = "arm,armv7-timer";
+               interrupt-parent = <&gic>;
+               interrupts = <1 13 0xf08>,
+                            <1 14 0xf08>,
+                            <1 11 0xf08>,
+                            <1 10 0xf08>;
+       };
+
+       soc {
+               /* It's a 32-bit SoC. */
+               #address-cells = <1>;
+               #size-cells = <1>;
+               compatible = "simple-bus";
+               interrupt-parent = <&gic>;
+               ranges = <0 0 0xe0000000 0x10000000>;

Quoted 32 lines · Haojian Zhuang

Add hip04-d01.dts & hip04.dtsi for hip04 SoC platform.

Signed-off-by Haojian Zhuang <[email protected]>

---
 arch/arm/boot/dts/Makefile      |   1 +
 arch/arm/boot/dts/hip04-d01.dts |  39 ++++++
 arch/arm/boot/dts/hip04.dtsi    | 267 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 arch/arm/boot/dts/hip04-d01.dts
 create mode 100644 arch/arm/boot/dts/hip04.dtsi

arch/arm/boot/dts/Makefile +1 -0

index 721525e..6587bbf 100644
@@ -86,6 +86,7 @@ dtb-$(CONFIG_ARCH_HI3xxx) += hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIX5HD2) += hix5hd2-dkb.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += highbank.dtb \
        ecx-2000.dtb
+dtb-$(CONFIG_ARCH_HIP04) += hip04-d01.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += integratorap.dtb \
        integratorcp.dtb
 dtb-$(CONFIG_ARCH_KEYSTONE) += k2hk-evm.dtb \

arch/arm/boot/dts/hip04-d01.dts new file +3 -0

new file mode 100644
index 0000000..661c8e5
@@ -0,0 +1,39 @@
+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 10 lines

arch/arm/boot/dts/hip04-d01.dts new file +10 -0

+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

Quoted 13 lines

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+
+#include "hip04.dtsi"
+
+/ {
+       /* memory bus is 64-bit */
+       #address-cells = <2>;
+       #size-cells = <2>;
+       model = "Hisilicon D01 Development Board";
+       compatible = "hisilicon,hip04-d01";
+
+       memory at 00000000,10000000 {
+               device_type = "memory";
+               reg = <0x00000000 0x10000000 0x00000000 0xc0000000>;

arch/arm/boot/dts/hip04-d01.dts line 32

Quoted 6 lines

arch/arm/boot/dts/hip04-d01.dts new file +6 -0

+       };
+
+       memory at 00000004,c0000000 {
+               device_type = "memory";
+               reg = <0x00000004 0xc0000000 0x00000003 0x40000000>;
+       };

You can fold these into a single node.

Quoted 18 lines

arch/arm/boot/dts/hip04-d01.dts new file +7 -0

+
+       soc {
+               uart0: uart at 4007000 {
+                       status = "ok";
+               };
+       };
+};

arch/arm/boot/dts/hip04.dtsi new file +5 -0

new file mode 100644
index 0000000..30942be
@@ -0,0 +1,267 @@
+/*
+ * Hisilicon Ltd. HiP04 SoC
+ *
+ * Copyright (C) 2013-2014 Hisilicon Ltd.
+ * Copyright (C) 2013-2014 Linaro Ltd.

arch/arm/boot/dts/hip04.dtsi line 11

Quoted 6 lines

arch/arm/boot/dts/hip04.dtsi new file +6 -0

+ *
+ * Author: Haojian Zhuang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.

s/hh/h/

[...]

arch/arm/boot/dts/hip04.dtsi line 175

Quoted 7 lines

arch/arm/boot/dts/hip04.dtsi new file +7 -0

+       clock: clock {
+               compatible = "hisilicon,hip04-clock";
+               /* dummy register.
+                * Don't need to access clock registers since they're
+                * configured in firmware already.
+                */
+               reg = <0 0 0 0x1000>;

Huh? Whether or not you need to access the registers should be up to the kernel, not the DT.

Why can the kernel not access these? This sounds like a hack.

Quoted 24 lines

arch/arm/boot/dts/hip04.dtsi new file +24 -0

+               #clock-cells = <1>;
+       };
+
+       timer {
+               compatible = "arm,armv7-timer";
+               interrupt-parent = <&gic>;
+               interrupts = <1 13 0xf08>,
+                            <1 14 0xf08>,
+                            <1 11 0xf08>,
+                            <1 10 0xf08>;
+       };
+
+       soc {
+               /* It's a 32-bit SoC. */
+               #address-cells = <1>;
+               #size-cells = <1>;
+               compatible = "simple-bus";
+               interrupt-parent = <&gic>;
+               ranges = <0 0 0xe0000000 0x10000000>;
+
+               gic: interrupt-controller at c01000 {
+                       compatible = "hisilicon,hip04-gic";
+                       #interrupt-cells = <3>;
+                       #address-cells = <0>;

arch/arm/boot/dts/hip04.dtsi line 204

Quoted 5 lines

arch/arm/boot/dts/hip04.dtsi new file +5 -0

+                       interrupt-controller;
+                       interrupts = <1 9 0xf04>;
+
+                       reg = <0xc01000 0x1000>, <0xc02000 0x1000>,
+                             <0xc04000 0x2000>, <0xc06000 0x2000>;

Please place these on separate lines. It's easier to read and will match what you've done for every other node.

arch/arm/boot/dts/hip04.dtsi line 214

Quoted 10 lines

arch/arm/boot/dts/hip04.dtsi new file +10 -0

+               };
+
+               sysctrl: sysctrl {
+                       compatible = "hisilicon,sysctrl";
+                       reg = <0x3e00000 0x00100000>;
+                       relocation-entry = <0xe0000100>;
+                       relocation-size = <0x1000>;
+                       bootwrapper-phys = <0x10c00000>;
+                       bootwrapper-size = <0x10000>;
+                       bootwrapper-magic = <0xa5a5a5a5>;

Are these absolute addresses, or translated per ranges above?

Why are they related to the system controller?

arch/arm/boot/dts/hip04.dtsi line 219

Quoted 5 lines

arch/arm/boot/dts/hip04.dtsi new file +5 -0

+               };
+
+               fabric: fabric {
+                       compatible = "hisilicon,hip04-fabric";
+                       reg = <0x302a000 0x1000>;

How is this going to be used?

arch/arm/boot/dts/hip04.dtsi line 228

Quoted 9 lines

arch/arm/boot/dts/hip04.dtsi new file +9 -0

+               };
+
+               dual_timer0: dual_timer at 3000000 {
+                       compatible = "arm,sp804", "arm,primecell";
+                       reg = <0x3000000 0x1000>;
+                       interrupts = <0 224 4>;
+                       clocks = <&clock HIP04_CLK_50M>;
+                       clock-names = "apb_pclk";
+               };

I thought sp804 had two clocks (one for AMBA and one for the actual timer). What's going on here?

Quoted 17 lines · Mark Rutland

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 13 lines · Haojian Zhuang

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

Quoted 5 lines

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

Quoted 13 lines

Quoted 13 lines

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+
+#include "hip04.dtsi"
+
+/ {
+       /* memory bus is 64-bit */
+       #address-cells = <2>;
+       #size-cells = <2>;
+       model = "Hisilicon D01 Development Board";
+       compatible = "hisilicon,hip04-d01";
+
+       memory at 00000000,10000000 {
+               device_type = "memory";
+               reg = <0x00000000 0x10000000 0x00000000 0xc0000000>;

Quoted 9 lines

arch/arm/boot/dts/hip04-d01.dts line 32

Quoted 6 lines

arch/arm/boot/dts/hip04-d01.dts new file +6 -0

+       };
+
+       memory at 00000004,c0000000 {
+               device_type = "memory";
+               reg = <0x00000004 0xc0000000 0x00000003 0x40000000>;
+       };

You can fold these into a single node.

The address spaces of these two memory node are different. I can't fold them into a single node.

Quoted 18 lines

Quoted 18 lines

arch/arm/boot/dts/hip04-d01.dts new file +7 -0

+
+       soc {
+               uart0: uart at 4007000 {
+                       status = "ok";
+               };
+       };
+};

arch/arm/boot/dts/hip04.dtsi new file +5 -0

new file mode 100644
index 0000000..30942be
@@ -0,0 +1,267 @@
+/*
+ * Hisilicon Ltd. HiP04 SoC
+ *
+ * Copyright (C) 2013-2014 Hisilicon Ltd.
+ * Copyright (C) 2013-2014 Linaro Ltd.

Quoted 9 lines

arch/arm/boot/dts/hip04.dtsi line 11

Quoted 6 lines

arch/arm/boot/dts/hip04.dtsi new file +6 -0

+ *
+ * Author: Haojian Zhuang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.

s/hh/h/

Thanks. I'll fix it.

Quoted 10 lines

[...]

Quoted 7 lines

arch/arm/boot/dts/hip04.dtsi new file +7 -0

+       clock: clock {
+               compatible = "hisilicon,hip04-clock";
+               /* dummy register.
+                * Don't need to access clock registers since they're
+                * configured in firmware already.
+                */
+               reg = <0 0 0 0x1000>;

Quoted 5 lines

Huh? Whether or not you need to access the registers should be up to the kernel, not the DT.

Why can the kernel not access these? This sounds like a hack.

Because I didn't get these materials yet. All clocks are enabled in bootloader.

Quoted 11 lines

Quoted 10 lines

arch/arm/boot/dts/hip04.dtsi new file +10 -0

+               };
+
+               sysctrl: sysctrl {
+                       compatible = "hisilicon,sysctrl";
+                       reg = <0x3e00000 0x00100000>;
+                       relocation-entry = <0xe0000100>;
+                       relocation-size = <0x1000>;
+                       bootwrapper-phys = <0x10c00000>;
+                       bootwrapper-size = <0x10000>;
+                       bootwrapper-magic = <0xa5a5a5a5>;

Quoted 4 lines

Are these absolute addresses, or translated per ranges above?

Why are they related to the system controller?

I need these parameters. But I can't find a better place.

Quoted 8 lines

arch/arm/boot/dts/hip04.dtsi line 219

Quoted 5 lines

arch/arm/boot/dts/hip04.dtsi new file +5 -0

+               };
+
+               fabric: fabric {
+                       compatible = "hisilicon,hip04-fabric";
+                       reg = <0x302a000 0x1000>;

How is this going to be used?

Fabric could configure snoop filter of multiple cluster. I don't have the manual. I only know this.

Hi,

Quoted 18 lines · Haojian Zhuang

Quoted 17 lines · Mark Rutland

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 13 lines · Haojian Zhuang

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

Quoted 8 lines

Quoted 5 lines

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

What is this memory used for? Is this where the cores are parked on the spin table, or something else? Does your platform have runtime firmware?

Quoted 23 lines

Quoted 13 lines

Quoted 13 lines

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+
+#include "hip04.dtsi"
+
+/ {
+       /* memory bus is 64-bit */
+       #address-cells = <2>;
+       #size-cells = <2>;
+       model = "Hisilicon D01 Development Board";
+       compatible = "hisilicon,hip04-d01";
+
+       memory at 00000000,10000000 {
+               device_type = "memory";
+               reg = <0x00000000 0x10000000 0x00000000 0xc0000000>;

Quoted 9 lines

arch/arm/boot/dts/hip04-d01.dts line 32

Quoted 6 lines

arch/arm/boot/dts/hip04-d01.dts new file +6 -0

+       };
+
+       memory at 00000004,c0000000 {
+               device_type = "memory";
+               reg = <0x00000004 0xc0000000 0x00000003 0x40000000>;
+       };

You can fold these into a single node.

Quoted 2 lines

The address spaces of these two memory node are different. I can't fold them into a single node.

There's no functional difference today between having two memory nodes or one node with two ranges. It would be:

        memory at 0 {
                device_type = "memory";
                reg = <0x00000000 0x10000000 0x00000000 0xc0000000>,
                      <0x00000004 0xc0000000 0x00000003 0x40000000>;

Quoted 46 lines

Quoted 18 lines

Quoted 18 lines

arch/arm/boot/dts/hip04-d01.dts new file +7 -0

+
+       soc {
+               uart0: uart at 4007000 {
+                       status = "ok";
+               };
+       };
+};

arch/arm/boot/dts/hip04.dtsi new file +5 -0

new file mode 100644
index 0000000..30942be
@@ -0,0 +1,267 @@
+/*
+ * Hisilicon Ltd. HiP04 SoC
+ *
+ * Copyright (C) 2013-2014 Hisilicon Ltd.
+ * Copyright (C) 2013-2014 Linaro Ltd.

Quoted 9 lines

arch/arm/boot/dts/hip04.dtsi line 11

Quoted 6 lines

arch/arm/boot/dts/hip04.dtsi new file +6 -0

+ *
+ * Author: Haojian Zhuang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.

s/hh/h/

Thanks. I'll fix it.

Quoted 15 lines

[...]

arch/arm/boot/dts/hip04.dtsi line 175

Quoted 7 lines

arch/arm/boot/dts/hip04.dtsi new file +7 -0

+       clock: clock {
+               compatible = "hisilicon,hip04-clock";
+               /* dummy register.
+                * Don't need to access clock registers since they're
+                * configured in firmware already.
+                */
+               reg = <0 0 0 0x1000>;

Huh? Whether or not you need to access the registers should be up to the kernel, not the DT.

Why can the kernel not access these? This sounds like a hack.

Quoted 1 line

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

arch/arm/boot/dts/hip04.dtsi new file +4 -0

+               };
+
+               sysctrl: sysctrl {
+                       compatible = "hisilicon,sysctrl";

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

arch/arm/boot/dts/hip04.dtsi new file +6 -0

+                       reg = <0x3e00000 0x00100000>;
+                       relocation-entry = <0xe0000100>;
+                       relocation-size = <0x1000>;
+                       bootwrapper-phys = <0x10c00000>;
+                       bootwrapper-size = <0x10000>;
+                       bootwrapper-magic = <0xa5a5a5a5>;

Quoted 6 lines

Quoted 4 lines

Are these absolute addresses, or translated per ranges above?

Why are they related to the system controller?

I need these parameters. But I can't find a better place.

Again, what is the boot wrapper in this case? And what do you need them for?

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

arch/arm/boot/dts/hip04.dtsi new file +6 -0

+                       reg = <0x3e00000 0x00100000>;
+                       relocation-entry = <0xe0000100>;
+                       relocation-size = <0x1000>;
+                       bootwrapper-phys = <0x10c00000>;
+                       bootwrapper-size = <0x10000>;
+                       bootwrapper-magic = <0xa5a5a5a5>;

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Are these absolute addresses, or translated per ranges above?

Why are they related to the system controller?

I need these parameters. But I can't find a better place.

Again, what is the boot wrapper in this case? And what do you need them for?

Ok, looking at patch 3/12, it seems that the "bootwrapper" base address is used to specify where the CPU goes when released by the system controller.

Isn't there an address that can be written in the system controller that specifies this base address, such that you can just handle this dynamically at runtime? It seems really, really odd that the platform will expect this one page in the middle of ram to just be untouched otherwise.

Hi,

Quoted 24 lines · Haojian Zhuang

Quoted 22 lines · Mark Rutland

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 13 lines · Haojian Zhuang

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

Quoted 2 lines

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

What is this memory used for? Is this where the cores are parked on the spin table, or something else? Does your platform have runtime firmware?

Quoted 23 lines

Quoted 13 lines

Quoted 13 lines

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+
+#include "hip04.dtsi"
+
+/ {
+       /* memory bus is 64-bit */
+       #address-cells = <2>;
+       #size-cells = <2>;
+       model = "Hisilicon D01 Development Board";
+       compatible = "hisilicon,hip04-d01";
+
+       memory at 00000000,10000000 {
+               device_type = "memory";
+               reg = <0x00000000 0x10000000 0x00000000 0xc0000000>;

Quoted 9 lines

arch/arm/boot/dts/hip04-d01.dts line 32

Quoted 6 lines

arch/arm/boot/dts/hip04-d01.dts new file +6 -0

+       };
+
+       memory at 00000004,c0000000 {
+               device_type = "memory";
+               reg = <0x00000004 0xc0000000 0x00000003 0x40000000>;
+       };

You can fold these into a single node.

Quoted 2 lines

The address spaces of these two memory node are different. I can't fold them into a single node.

There's no functional difference today between having two memory nodes or one node with two ranges. It would be:

        memory at 0 {
                device_type = "memory";
                reg = <0x00000000 0x10000000 0x00000000 0xc0000000>,
                      <0x00000004 0xc0000000 0x00000003 0x40000000>;

Quoted 40 lines

Quoted 18 lines

Quoted 18 lines

arch/arm/boot/dts/hip04-d01.dts new file +7 -0

+
+       soc {
+               uart0: uart at 4007000 {
+                       status = "ok";
+               };
+       };
+};

arch/arm/boot/dts/hip04.dtsi new file +5 -0

new file mode 100644
index 0000000..30942be
@@ -0,0 +1,267 @@
+/*
+ * Hisilicon Ltd. HiP04 SoC
+ *
+ * Copyright (C) 2013-2014 Hisilicon Ltd.
+ * Copyright (C) 2013-2014 Linaro Ltd.

Quoted 9 lines

arch/arm/boot/dts/hip04.dtsi line 11

Quoted 6 lines

arch/arm/boot/dts/hip04.dtsi new file +6 -0

+ *
+ * Author: Haojian Zhuang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.

s/hh/h/

Thanks. I'll fix it.

Quoted 10 lines

[...]

Quoted 7 lines

arch/arm/boot/dts/hip04.dtsi new file +7 -0

+       clock: clock {
+               compatible = "hisilicon,hip04-clock";
+               /* dummy register.
+                * Don't need to access clock registers since they're
+                * configured in firmware already.
+                */
+               reg = <0 0 0 0x1000>;

Quoted 7 lines

Quoted 5 lines

Huh? Whether or not you need to access the registers should be up to the kernel, not the DT.

Why can the kernel not access these? This sounds like a hack.

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

Quoted 11 lines

Quoted 11 lines

Quoted 10 lines

arch/arm/boot/dts/hip04.dtsi new file +10 -0

+               };
+
+               sysctrl: sysctrl {
+                       compatible = "hisilicon,sysctrl";
+                       reg = <0x3e00000 0x00100000>;
+                       relocation-entry = <0xe0000100>;
+                       relocation-size = <0x1000>;
+                       bootwrapper-phys = <0x10c00000>;
+                       bootwrapper-size = <0x10000>;
+                       bootwrapper-magic = <0xa5a5a5a5>;

Quoted 6 lines

Quoted 4 lines

Are these absolute addresses, or translated per ranges above?

Why are they related to the system controller?

I need these parameters. But I can't find a better place.

Again, what is the boot wrapper in this case? And what do you need them for?

Quoted 9 lines

Quoted 8 lines

arch/arm/boot/dts/hip04.dtsi line 219

Quoted 5 lines

arch/arm/boot/dts/hip04.dtsi new file +5 -0

+               };
+
+               fabric: fabric {
+                       compatible = "hisilicon,hip04-fabric";
+                       reg = <0x302a000 0x1000>;

How is this going to be used?

Quoted 2 lines

Fabric could configure snoop filter of multiple cluster. I don't have the manual. I only know this.

What driver is going to bind against this? It probably makes sense to leave this out until the consumer is also upstreamed, to have something to have as a base for discussion.

Quoted 31 lines · Olof Johansson

Hi,

Quoted 26 lines · Haojian Zhuang

Quoted 17 lines · Mark Rutland

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 13 lines · Haojian Zhuang

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

Quoted 5 lines

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

Quoted 3 lines

What is this memory used for? Is this where the cores are parked on the spin table, or something else? Does your platform have runtime firmware?

This memory stores the instructions that secondary cores will execute initially.

They will switch cores into non-secure & hyp mode.

There's no runtime firmware in this platform. And there's no schedule to support it.

Quoted 27 lines

Quoted 24 lines

Quoted 14 lines

Quoted 14 lines

arch/arm/boot/dts/hip04-d01.dts new file +13 -0

+
+#include "hip04.dtsi"
+
+/ {
+       /* memory bus is 64-bit */
+       #address-cells = <2>;
+       #size-cells = <2>;
+       model = "Hisilicon D01 Development Board";
+       compatible = "hisilicon,hip04-d01";
+
+       memory at 00000000,10000000 {
+               device_type = "memory";
+               reg = <0x00000000 0x10000000 0x00000000 0xc0000000>;

Quoted 9 lines

arch/arm/boot/dts/hip04-d01.dts line 32

Quoted 6 lines

arch/arm/boot/dts/hip04-d01.dts new file +6 -0

+       };
+
+       memory at 00000004,c0000000 {
+               device_type = "memory";
+               reg = <0x00000004 0xc0000000 0x00000003 0x40000000>;
+       };

You can fold these into a single node.

Quoted 2 lines

The address spaces of these two memory node are different. I can't fold them into a single node.

Quoted 7 lines

There's no functional difference today between having two memory nodes or one node with two ranges. It would be:

        memory at 0 {
                device_type = "memory";
                reg = <0x00000000 0x10000000 0x00000000 0xc0000000>,
                      <0x00000004 0xc0000000 0x00000003 0x40000000>;

OK.

Quoted 66 lines

Quoted 40 lines

Quoted 18 lines

Quoted 18 lines

arch/arm/boot/dts/hip04-d01.dts new file +7 -0

+
+       soc {
+               uart0: uart at 4007000 {
+                       status = "ok";
+               };
+       };
+};

arch/arm/boot/dts/hip04.dtsi new file +5 -0

new file mode 100644
index 0000000..30942be
@@ -0,0 +1,267 @@
+/*
+ * Hisilicon Ltd. HiP04 SoC
+ *
+ * Copyright (C) 2013-2014 Hisilicon Ltd.
+ * Copyright (C) 2013-2014 Linaro Ltd.

Quoted 9 lines

arch/arm/boot/dts/hip04.dtsi line 11

Quoted 6 lines

arch/arm/boot/dts/hip04.dtsi new file +6 -0

+ *
+ * Author: Haojian Zhuang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.

s/hh/h/

Thanks. I'll fix it.

Quoted 10 lines

[...]

Quoted 7 lines

arch/arm/boot/dts/hip04.dtsi new file +7 -0

+       clock: clock {
+               compatible = "hisilicon,hip04-clock";
+               /* dummy register.
+                * Don't need to access clock registers since they're
+                * configured in firmware already.
+                */
+               reg = <0 0 0 0x1000>;

Quoted 7 lines

Quoted 5 lines

Huh? Whether or not you need to access the registers should be up to the kernel, not the DT.

Why can the kernel not access these? This sounds like a hack.

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

Quoted 11 lines

Quoted 11 lines

Quoted 10 lines

arch/arm/boot/dts/hip04.dtsi new file +10 -0

+               };
+
+               sysctrl: sysctrl {
+                       compatible = "hisilicon,sysctrl";
+                       reg = <0x3e00000 0x00100000>;
+                       relocation-entry = <0xe0000100>;
+                       relocation-size = <0x1000>;
+                       bootwrapper-phys = <0x10c00000>;
+                       bootwrapper-size = <0x10000>;
+                       bootwrapper-magic = <0xa5a5a5a5>;

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

Are these absolute addresses, or translated per ranges above?

Why are they related to the system controller?

I need these parameters. But I can't find a better place.

Again, what is the boot wrapper in this case? And what do you need them for?

Now UEFI didn't switch secondary cores to non-secure & hyp mode. The bootwrapper is used to handle this job. Then bootwrapper will jump to kernel to continue boot SMP. So the relocation area is used to store the SMP entrance.

SMP feature will fail without bootwrapper in this platform.

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

arch/arm/boot/dts/hip04.dtsi line 219

Quoted 5 lines

arch/arm/boot/dts/hip04.dtsi new file +5 -0

+               };
+
+               fabric: fabric {
+                       compatible = "hisilicon,hip04-fabric";
+                       reg = <0x302a000 0x1000>;

How is this going to be used?

Quoted 6 lines

Quoted 2 lines

Fabric could configure snoop filter of multiple cluster. I don't have the manual. I only know this.

What driver is going to bind against this? It probably makes sense to leave this out until the consumer is also upstreamed, to have something to have as a base for discussion.

Only one register is configured in the fabric by kernel. Do we need a driver?

Quoted 1 line · Haojian Zhuang

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

Quoted 9 lines · Olof Johansson

Quoted 1 line · Haojian Zhuang

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

I completely agree. Furthermore, given that this SoC implements a dodgy custom GIC, I don't think it's unreasonable for us to ask for documentation describing that too, otherwise it's going to make maintaining a complicated irqchip driver that much worse.

Will

Quoted 11 lines · Will Deacon

Quoted 9 lines · Olof Johansson

Quoted 1 line · Haojian Zhuang

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

Quoted 5 lines

I completely agree. Furthermore, given that this SoC implements a dodgy custom GIC, I don't think it's unreasonable for us to ask for documentation describing that too, otherwise it's going to make maintaining a complicated irqchip driver that much worse.

I don't have enough documents on clocks. And all clocks are already enabled in UEFI. There're also no requirement on change the clock frequency & disable them. So I set all these clocks as fixed rate.

It's different from GIC. If I don't have documents on GIC, I can't write code for HIP04 GIC. HiP04 GIC supports 16-cores. ARM GICv2 supports 8-cores. So some registers offsets and bit fields are different from GICv2.

Quoted 17 lines · Haojian Zhuang

Quoted 11 lines · Will Deacon

Quoted 9 lines · Olof Johansson

Quoted 1 line · Haojian Zhuang

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

Quoted 5 lines

I completely agree. Furthermore, given that this SoC implements a dodgy custom GIC, I don't think it's unreasonable for us to ask for documentation describing that too, otherwise it's going to make maintaining a complicated irqchip driver that much worse.

Quoted 8 lines

I don't have enough documents on clocks. And all clocks are already enabled in UEFI. There're also no requirement on change the clock frequency & disable them. So I set all these clocks as fixed rate.

It's different from GIC. If I don't have documents on GIC, I can't write code for HIP04 GIC. HiP04 GIC supports 16-cores. ARM GICv2 supports 8-cores. So some registers offsets and bit fields are different from GICv2.

Sure, and I'm asking for those documents to be made publicly available somewhere so that we can refer to them if we need to change this code in future.

Will

Quoted 27 lines · Will Deacon

Quoted 25 lines · Haojian Zhuang

Quoted 11 lines · Will Deacon

Quoted 9 lines · Olof Johansson

Quoted 1 line · Haojian Zhuang

Because I didn't get these materials yet. All clocks are enabled in bootloader.

What materials? Technical documentation for the CPU you're upstreaming?

It seems like a very bad idea to upstream a DT for a CPU that you don't have documentation for. It seems appropriate to wait until you have documentation so you can make sure that the hardware you're describing is actually what is there, and not something made-up or misdescribed.

Quoted 5 lines

I completely agree. Furthermore, given that this SoC implements a dodgy custom GIC, I don't think it's unreasonable for us to ask for documentation describing that too, otherwise it's going to make maintaining a complicated irqchip driver that much worse.

I don't have enough documents on clocks. And all clocks are already enabled in UEFI. There're also no requirement on change the clock frequency & disable them. So I set all these clocks as fixed rate.

It's different from GIC. If I don't have documents on GIC, I can't write code for HIP04 GIC. HiP04 GIC supports 16-cores. ARM GICv2 supports 8-cores. So some registers offsets and bit fields are different from GICv2.

Quoted 4 lines

Sure, and I'm asking for those documents to be made publicly available somewhere so that we can refer to them if we need to change this code in future.

I can send the copy to you since it's already public. But the main issue is that this document is written in Chinese. I hope it could be written in English.

Quoted 46 lines · Mark Rutland

Quoted 32 lines · Haojian Zhuang

Add hip04-d01.dts & hip04.dtsi for hip04 SoC platform.

Signed-off-by Haojian Zhuang <[email protected]>

---
 arch/arm/boot/dts/Makefile      |   1 +
 arch/arm/boot/dts/hip04-d01.dts |  39 ++++++
 arch/arm/boot/dts/hip04.dtsi    | 267 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 arch/arm/boot/dts/hip04-d01.dts
 create mode 100644 arch/arm/boot/dts/hip04.dtsi

arch/arm/boot/dts/Makefile +1 -0

index 721525e..6587bbf 100644
@@ -86,6 +86,7 @@ dtb-$(CONFIG_ARCH_HI3xxx) += hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIX5HD2) += hix5hd2-dkb.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += highbank.dtb \
        ecx-2000.dtb
+dtb-$(CONFIG_ARCH_HIP04) += hip04-d01.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += integratorap.dtb \
        integratorcp.dtb
 dtb-$(CONFIG_ARCH_KEYSTONE) += k2hk-evm.dtb \

arch/arm/boot/dts/hip04-d01.dts new file +3 -0

new file mode 100644
index 0000000..661c8e5
@@ -0,0 +1,39 @@
+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 10 lines

arch/arm/boot/dts/hip04-d01.dts new file +10 -0

+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

Quoted 5 lines

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

Quoted 8 lines

arch/arm/boot/dts/hip04-d01.dts line 18

Quoted 5 lines

arch/arm/boot/dts/hip04-d01.dts new file +5 -0

+               };
+
+               fabric: fabric {
+                       compatible = "hisilicon,hip04-fabric";
+                       reg = <0x302a000 0x1000>;

How is this going to be used?

Fabric could configure snoop filter of multiple cluster. I don't have the manual. I only know this.

Quoted 10 lines

Quoted 9 lines

arch/arm/boot/dts/hip04-d01.dts new file +9 -0

+               };
+
+               dual_timer0: dual_timer at 3000000 {
+                       compatible = "arm,sp804", "arm,primecell";
+                       reg = <0x3000000 0x1000>;
+                       interrupts = <0 224 4>;
+                       clocks = <&clock HIP04_CLK_50M>;
+                       clock-names = "apb_pclk";
+               };

Quoted 3 lines

I thought sp804 had two clocks (one for AMBA and one for the actual timer). What's going on here?

If only one clock is configured at here, sp804 driver believes that clk2 is same as clk1.

Quoted 47 lines · Haojian Zhuang

Quoted 46 lines · Mark Rutland

Quoted 32 lines · Haojian Zhuang

Add hip04-d01.dts & hip04.dtsi for hip04 SoC platform.

Signed-off-by Haojian Zhuang <[email protected]>

---
 arch/arm/boot/dts/Makefile      |   1 +
 arch/arm/boot/dts/hip04-d01.dts |  39 ++++++
 arch/arm/boot/dts/hip04.dtsi    | 267 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 arch/arm/boot/dts/hip04-d01.dts
 create mode 100644 arch/arm/boot/dts/hip04.dtsi

arch/arm/boot/dts/Makefile +1 -0

index 721525e..6587bbf 100644
@@ -86,6 +86,7 @@ dtb-$(CONFIG_ARCH_HI3xxx) += hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIX5HD2) += hix5hd2-dkb.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += highbank.dtb \
        ecx-2000.dtb
+dtb-$(CONFIG_ARCH_HIP04) += hip04-d01.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += integratorap.dtb \
        integratorcp.dtb
 dtb-$(CONFIG_ARCH_KEYSTONE) += k2hk-evm.dtb \

arch/arm/boot/dts/hip04-d01.dts new file +3 -0

new file mode 100644
index 0000000..661c8e5
@@ -0,0 +1,39 @@
+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 10 lines

arch/arm/boot/dts/hip04-d01.dts new file +10 -0

+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

Quoted 8 lines

Quoted 5 lines

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

What loads the boot wrapper into memory, and from where?

In another reply you mentioned you're using UEFI. Is this wrapper considered part of your UEFI implementation? If so it should be reserved in the UEFI memory map, and there should be no need of memory nodes or memreserves -- all that should come from UEFI.

What exactly is the boot wrapper in charge of?

What is the split in responsiblity between the wrapper and the kernel?

Quoted 9 lines

Quoted 8 lines

arch/arm/boot/dts/hip04-d01.dts line 18

Quoted 5 lines

arch/arm/boot/dts/hip04-d01.dts new file +5 -0

+               };
+
+               fabric: fabric {
+                       compatible = "hisilicon,hip04-fabric";
+                       reg = <0x302a000 0x1000>;

How is this going to be used?

Quoted 2 lines

Fabric could configure snoop filter of multiple cluster. I don't have the manual. I only know this.

Is this accessible to the non-secure side?

If the firmware is currently programming things correctly then we shouldn't need this for now.

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

arch/arm/boot/dts/hip04-d01.dts new file +9 -0

+               };
+
+               dual_timer0: dual_timer at 3000000 {
+                       compatible = "arm,sp804", "arm,primecell";
+                       reg = <0x3000000 0x1000>;
+                       interrupts = <0 224 4>;
+                       clocks = <&clock HIP04_CLK_50M>;
+                       clock-names = "apb_pclk";
+               };

Quoted 6 lines

Quoted 3 lines

I thought sp804 had two clocks (one for AMBA and one for the actual timer). What's going on here?

If only one clock is configured at here, sp804 driver believes that clk2 is same as clk1.

I'm not keen on relying on that. It's arguably a bug in the driver.

Quoted 57 lines · Mark Rutland

Quoted 55 lines · Haojian Zhuang

Quoted 46 lines · Mark Rutland

Quoted 32 lines · Haojian Zhuang

Add hip04-d01.dts & hip04.dtsi for hip04 SoC platform.

Signed-off-by Haojian Zhuang <[email protected]>

---
 arch/arm/boot/dts/Makefile      |   1 +
 arch/arm/boot/dts/hip04-d01.dts |  39 ++++++
 arch/arm/boot/dts/hip04.dtsi    | 267 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 arch/arm/boot/dts/hip04-d01.dts
 create mode 100644 arch/arm/boot/dts/hip04.dtsi

arch/arm/boot/dts/Makefile +1 -0

index 721525e..6587bbf 100644
@@ -86,6 +86,7 @@ dtb-$(CONFIG_ARCH_HI3xxx) += hi3620-hi4511.dtb
 dtb-$(CONFIG_ARCH_HIX5HD2) += hix5hd2-dkb.dtb
 dtb-$(CONFIG_ARCH_HIGHBANK) += highbank.dtb \
        ecx-2000.dtb
+dtb-$(CONFIG_ARCH_HIP04) += hip04-d01.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += integratorap.dtb \
        integratorcp.dtb
 dtb-$(CONFIG_ARCH_KEYSTONE) += k2hk-evm.dtb \

arch/arm/boot/dts/hip04-d01.dts new file +3 -0

new file mode 100644
index 0000000..661c8e5
@@ -0,0 +1,39 @@
+/*
+ *  Copyright (C) 2013-2014 Linaro Ltd.
+ *  Author: Haojian Zhuang <[email protected]>

arch/arm/boot/dts/hip04-d01.dts line 13

Quoted 10 lines

arch/arm/boot/dts/hip04-d01.dts new file +10 -0

+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+
+/* For bootwrapper */
+/memreserve/ 0x10c00000 0x00010000;

How exactly is this bootwrapper used? Is the kernel compiled into it?

Quoted 5 lines

It might make more sense for the wrapper build system to inject bootwrapper-related properties. Then the DTB is less likely to amalgamate hacks to workaround differences between versions, and can be used on systems without a wrapper without throwing away some memory.

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

Quoted 6 lines

What loads the boot wrapper into memory, and from where?

In another reply you mentioned you're using UEFI. Is this wrapper considered part of your UEFI implementation? If so it should be reserved in the UEFI memory map, and there should be no need of memory nodes or memreserves -- all that should come from UEFI.

There's no UEFI memory map parsing on 32-bit, is there?

Quoted 3 lines

What exactly is the boot wrapper in charge of?

What is the split in responsiblity between the wrapper and the kernel?

Sounds like it's just the trampoline code that's executed at CPU spin-up time. I wouldn't be surprised if there's a register somewhere on the same controller that you poke the cpu to life that you can write what address for it to start executing at, or at least some other location where it can be redirected somewhere else. It's very unlikely a wired-in address in hardware. Or so I hope. :-)

Quoted 12 lines

Quoted 11 lines

Quoted 8 lines

arch/arm/boot/dts/hip04-d01.dts line 18

Quoted 5 lines

arch/arm/boot/dts/hip04-d01.dts new file +5 -0

+               };
+
+               fabric: fabric {
+                       compatible = "hisilicon,hip04-fabric";
+                       reg = <0x302a000 0x1000>;

How is this going to be used?

Fabric could configure snoop filter of multiple cluster. I don't have the manual. I only know this.

Quoted 4 lines

Is this accessible to the non-secure side?

If the firmware is currently programming things correctly then we shouldn't need this for now.

with MCPM clusters get powered up and down, it's not uncommon to have to take them out of coherence domains.

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

arch/arm/boot/dts/hip04-d01.dts line 27

Quoted 9 lines

arch/arm/boot/dts/hip04-d01.dts new file +9 -0

+               };
+
+               dual_timer0: dual_timer at 3000000 {
+                       compatible = "arm,sp804", "arm,primecell";
+                       reg = <0x3000000 0x1000>;
+                       interrupts = <0 224 4>;
+                       clocks = <&clock HIP04_CLK_50M>;
+                       clock-names = "apb_pclk";
+               };

I thought sp804 had two clocks (one for AMBA and one for the actual timer). What's going on here?

Quoted 4 lines

Quoted 2 lines

If only one clock is configured at here, sp804 driver believes that clk2 is same as clk1.

I'm not keen on relying on that. It's arguably a bug in the driver.

The driver should warn, but we can't make it fail on it now. :(

Quoted 9 lines

Quoted 2 lines

In this platform, we relied on the bootwrapper. If I discard this, I'll fail to bring up all secondary cores.

What loads the boot wrapper into memory, and from where?

In another reply you mentioned you're using UEFI. Is this wrapper considered part of your UEFI implementation? If so it should be reserved in the UEFI memory map, and there should be no need of memory nodes or memreserves -- all that should come from UEFI.

There's no UEFI memory map parsing on 32-bit, is there?

Append multi_v7_lpae_config. In this default configuration, CONFIG_ARCH_MULTI_V6 is disabled. CONFIG_ARM_LPAE is enabled.

Quoted 2 lines · Haojian Zhuang

Append multi_v7_lpae_config. In this default configuration, CONFIG_ARCH_MULTI_V6 is disabled. CONFIG_ARM_LPAE is enabled.

NACK. This is the nth time I have had to nack this patch.

You are enabling platforms here that don't have LPAE. Don't do that.

It's trivial to create a LPAE version of multi_v7 defconfig locally.

-Olof

Quoted 11 lines · Olof Johansson

Quoted 2 lines · Haojian Zhuang

Append multi_v7_lpae_config. In this default configuration, CONFIG_ARCH_MULTI_V6 is disabled. CONFIG_ARM_LPAE is enabled.

NACK. This is the nth time I have had to nack this patch.

You are enabling platforms here that don't have LPAE. Don't do that.

It's trivial to create a LPAE version of multi_v7 defconfig locally.

Quoted 1 line

-Olof

I tried to remove those SoCs that can't support LPAE. But I don't know every platform in details.

How about that I only enable HIP04 in multi_v7_lpae_defconfig. If you think any platform could be added, please help to list it. Then I could append them into the multi_v7_lpae_defconfig.

Quoted 12 lines · Haojian Zhuang

Quoted 11 lines · Olof Johansson

Quoted 2 lines · Haojian Zhuang

Append multi_v7_lpae_config. In this default configuration, CONFIG_ARCH_MULTI_V6 is disabled. CONFIG_ARM_LPAE is enabled.

NACK. This is the nth time I have had to nack this patch.

You are enabling platforms here that don't have LPAE. Don't do that.

It's trivial to create a LPAE version of multi_v7 defconfig locally.

Quoted 8 lines

Quoted 1 line

-Olof

I tried to remove those SoCs that can't support LPAE. But I don't know every platform in details.

How about that I only enable HIP04 in multi_v7_lpae_defconfig. If you think any platform could be added, please help to list it. Then I could append them into the multi_v7_lpae_defconfig.

I really don't like having to repeat this over and over: Just add LPAE to a multi_v7_defconfig build when you need to turn it on.

The alternative is to do this right from the beginning, and move multi_v7 over to be a v6_v7 defconfig, and also have an lpae+kvm defconfig for A7/12/15/17 (and mvebu/krait) platforms. But there's been no need for that yet since all platforms to date can boot without LPAE just fine -- in fact most of them don't actually need it since nearly all of them have less memory and for the few of them that do you just turn on LPAE manually like I suggested.

-Olof

Quoted 35 lines · Olof Johansson

Quoted 12 lines · Haojian Zhuang

Quoted 11 lines · Olof Johansson

Quoted 2 lines · Haojian Zhuang

Append multi_v7_lpae_config. In this default configuration, CONFIG_ARCH_MULTI_V6 is disabled. CONFIG_ARM_LPAE is enabled.

NACK. This is the nth time I have had to nack this patch.

You are enabling platforms here that don't have LPAE. Don't do that.

It's trivial to create a LPAE version of multi_v7 defconfig locally.

Quoted 8 lines

Quoted 1 line

-Olof

I tried to remove those SoCs that can't support LPAE. But I don't know every platform in details.

How about that I only enable HIP04 in multi_v7_lpae_defconfig. If you think any platform could be added, please help to list it. Then I could append them into the multi_v7_lpae_defconfig.

I really don't like having to repeat this over and over: Just add LPAE to a multi_v7_defconfig build when you need to turn it on.

The alternative is to do this right from the beginning, and move multi_v7 over to be a v6_v7 defconfig, and also have an lpae+kvm defconfig for A7/12/15/17 (and mvebu/krait) platforms. But there's been no need for that yet since all platforms to date can boot without LPAE just fine -- in fact most of them don't actually need it since nearly all of them have less memory and for the few of them that do you just turn on LPAE manually like I suggested.

Quoted 1 line

-Olof

I can drop both hip04_defconfig & multi_v7_lpae_defconfig. But the problem is that 16GB memory is used in hip04 platform. 64bit memory address is also used in hip04 dts file. So I can't boot kernel without LPAE configuration.

If I integrate ARCH_HIP04 & LPAE into hi3xxx_defconfig, both ARCH_HI3XXX & ARCH_X5HD2 don't support LPAE.

Quoted 43 lines · Haojian Zhuang

Quoted 35 lines · Olof Johansson

Quoted 12 lines · Haojian Zhuang

Quoted 11 lines · Olof Johansson

Quoted 2 lines · Haojian Zhuang

Append multi_v7_lpae_config. In this default configuration, CONFIG_ARCH_MULTI_V6 is disabled. CONFIG_ARM_LPAE is enabled.

NACK. This is the nth time I have had to nack this patch.

You are enabling platforms here that don't have LPAE. Don't do that.

It's trivial to create a LPAE version of multi_v7 defconfig locally.

Quoted 8 lines

Quoted 1 line

-Olof

I tried to remove those SoCs that can't support LPAE. But I don't know every platform in details.

How about that I only enable HIP04 in multi_v7_lpae_defconfig. If you think any platform could be added, please help to list it. Then I could append them into the multi_v7_lpae_defconfig.

I really don't like having to repeat this over and over: Just add LPAE to a multi_v7_defconfig build when you need to turn it on.

The alternative is to do this right from the beginning, and move multi_v7 over to be a v6_v7 defconfig, and also have an lpae+kvm defconfig for A7/12/15/17 (and mvebu/krait) platforms. But there's been no need for that yet since all platforms to date can boot without LPAE just fine -- in fact most of them don't actually need it since nearly all of them have less memory and for the few of them that do you just turn on LPAE manually like I suggested.

Quoted 1 line

-Olof

I can drop both hip04_defconfig & multi_v7_lpae_defconfig. But the problem is that 16GB memory is used in hip04 platform. 64bit memory address is also used in hip04 dts file. So I can't boot kernel without LPAE configuration.

Quoted 2 lines

If I integrate ARCH_HIP04 & LPAE into hi3xxx_defconfig, both ARCH_HI3XXX & ARCH_X5HD2 don't support LPAE.

There is memory at 0x10000000-0xd0000000 and all I/O is below 4GB. You should be able to boot just fine without LPAE, but you won't see all memory available.

Maybe I didn't express this well. Let's check the memory layout in hip04.

<0x00000000-10000000, 0x00000000-c0000000> & <0x00000004-0xc0000000, 0x00000003-40000000> are two memory regions.

These memory address is 64-bit. When ARM_LPAE is disabled, memblock_add() parses memory base address & size with 32-bit. Since ARCH_PHYS_ADDR_T_64BIT is highly depend on ARM_LPAE.

Then these two memory regions become <0x10000000, 0xc0000000> & <0xc0000000, 0xffffffff>. Lots of IO space are in <0xe0000000, 0xefffffff>. So memory space conflicts when ARM_LPAE is disabled.

Kernel panic will come since it believes IO space is memory.

Only two solutions are available in below.
1. Use hip04_defconfig to declare ARM_LPAE.
2. Use hi3xxx_defconfig, and only declare 2.9GB memory in DTS file without ARM_LPAE.

I think solution #1 is better. What's your opinion?

Quoted 12 lines · Haojian Zhuang

Maybe I didn't express this well. Let's check the memory layout in hip04.

<0x00000000-10000000, 0x00000000-c0000000> & <0x00000004-0xc0000000, 0x00000003-40000000> are two memory regions.

These memory address is 64-bit. When ARM_LPAE is disabled, memblock_add() parses memory base address & size with 32-bit. Since ARCH_PHYS_ADDR_T_64BIT is highly depend on ARM_LPAE.

Then these two memory regions become <0x10000000, 0xc0000000> & <0xc0000000, 0xffffffff>. Lots of IO space are in <0xe0000000, 0xefffffff>. So memory space conflicts when ARM_LPAE is disabled.

Quoted 9 lines

Kernel panic will come since it believes IO space is memory.

Only two solutions are available in below.
1. Use hip04_defconfig to declare ARM_LPAE.
2. Use hi3xxx_defconfig, and only declare 2.9GB memory in DTS file without ARM_LPAE.

I think solution #1 is better. What's your opinion?

I think it's a bug in the DT parsing code if incorrect memory regions get added. It's supposed to parse the memory nodes using "long enough" (u64 or arbitrary-length) data types and then skip every range that doesn't fit into phys_addr_t.

Arnd

Select HiP04 SoC configuration by hip04_defconfig.

From: Kefeng Wang <[email protected]>

The commit 93dc688 (ARM: 7684/1: errata: Workaround for Cortex-A15 erratum 798181 (TLBI/DSB operations)) introduced a workaround for Cortex-A15 erratum 798181. Enable it for HIP04(Cortex-a15 r3p2).

Signed-off-by Kefeng Wang <[email protected]>

Singed-off-by Haojian Zhuang <[email protected]>

Quoted 6 lines · Haojian Zhuang

From: Kefeng Wang <[email protected]>

The commit 93dc688 (ARM: 7684/1: errata: Workaround for Cortex-A15 erratum 798181 (TLBI/DSB operations)) introduced a workaround for Cortex-A15 erratum 798181. Enable it for HIP04(Cortex-a15 r3p2).

Quoted 2 lines

Signed-off-by Kefeng Wang <[email protected]>

Singed-off-by Haojian Zhuang <[email protected]>

There's no need to do this as a separate patch if you just added this entry in a different patch in the same series -- just fold this in with that.

-Olof

In ARM standard GIC, GICH_APR offset is 0xf0 & GICH_LR0 offset is 0x100. In HiP04 GIC, GICH_APR offset is 0x70 & GICH_LR0 offset is 0x80.

Now reuse the nr_lr field in struct vgic_cpu. Bit[31:16] is used to store GICH_APR offset in HiP04, and bit[15:0] is used to store real nr_lr variable. In ARM standard GIC, don't set bit[31:16]. So we could avoid to change the VGIC implementation in arm64.

Signed-off-by Haojian Zhuang <[email protected]>

---
 arch/arm/kernel/asm-offsets.c   |  2 +-
 arch/arm/kvm/interrupts_head.S  | 29 +++++++++++++++++++++++------
 arch/arm64/kernel/asm-offsets.c |  2 +-
 arch/arm64/kvm/hyp.S            |  4 ++--
 include/kvm/arm_vgic.h          |  7 +++++--
 include/linux/irqchip/arm-gic.h |  6 ++++++
 virt/kvm/arm/vgic.c             | 37 ++++++++++++++++++++++++++-----------
 7 files changed, 64 insertions(+), 23 deletions(-)

arch/arm/kernel/asm-offsets.c +1 -1

index 85598b5..166cc98 100644
@@ -189,7 +189,7 @@ int main(void)
   DEFINE(VGIC_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_elrsr));
   DEFINE(VGIC_CPU_APR,		offsetof(struct vgic_cpu, vgic_apr));
   DEFINE(VGIC_CPU_LR,		offsetof(struct vgic_cpu, vgic_lr));
-  DEFINE(VGIC_CPU_NR_LR,	offsetof(struct vgic_cpu, nr_lr));
+  DEFINE(VGIC_CPU_HW_CFG,	offsetof(struct vgic_cpu, hw_cfg));
 #ifdef CONFIG_KVM_ARM_TIMER
   DEFINE(VCPU_TIMER_CNTV_CTL,	offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_ctl));
   DEFINE(VCPU_TIMER_CNTV_CVAL,	offsetof(struct kvm_vcpu, arch.timer_cpu.cntv_cval));

arch/arm/kvm/interrupts_head.S +3 -2

index 76af9302..9fbbf99 100644
@@ -419,7 +419,9 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	ldr	r7, [r2, #GICH_EISR1]
 	ldr	r8, [r2, #GICH_ELRSR0]
 	ldr	r9, [r2, #GICH_ELRSR1]
-	ldr	r10, [r2, #GICH_APR]
+	ldr	r10, [r11, #VGIC_CPU_HW_CFG]
+	mov	r10, r10, lsr #HWCFG_APR_SHIFT
+	ldr	r10, [r2, r10]
 
 	str	r3, [r11, #VGIC_CPU_HCR]
 	str	r4, [r11, #VGIC_CPU_VMCR]

@@ -435,9 +437,15 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	str	r5, [r2, #GICH_HCR]
 
 	/* Save list registers */
-	add	r2, r2, #GICH_LR0

arch/arm/kvm/interrupts_head.S +4 -2

 1:	ldr	r6, [r2], #4
 	str	r6, [r3], #4
 	subs	r4, r4, #1

@@ -469,12 +477,21 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 
 	str	r3, [r2, #GICH_HCR]
 	str	r4, [r2, #GICH_VMCR]
-	str	r8, [r2, #GICH_APR]
+	ldr	r6, [r11, #VGIC_CPU_HW_CFG]
+	mov	r6, r6, lsr #HWCFG_APR_SHIFT
+	str	r8, [r2, r6]
 
 	/* Restore list registers */
-	add	r2, r2, #GICH_LR0
+	ldr	r4, [r11, #VGIC_CPU_HW_CFG]

arch/arm/kvm/interrupts_head.S +0 -0

 1:	ldr	r6, [r3], #4
 	str	r6, [r2], #4
 	subs	r4, r4, #1

arch/arm64/kernel/asm-offsets.c +1 -1

index 646f888..2422358 100644
@@ -136,7 +136,7 @@ int main(void)
   DEFINE(VGIC_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_elrsr));
   DEFINE(VGIC_CPU_APR,		offsetof(struct vgic_cpu, vgic_apr));
   DEFINE(VGIC_CPU_LR,		offsetof(struct vgic_cpu, vgic_lr));
-  DEFINE(VGIC_CPU_NR_LR,	offsetof(struct vgic_cpu, nr_lr));
+  DEFINE(VGIC_CPU_HW_CFG,	offsetof(struct vgic_cpu, hw_cfg));
   DEFINE(KVM_VTTBR,		offsetof(struct kvm, arch.vttbr));
   DEFINE(KVM_VGIC_VCTRL,	offsetof(struct kvm, arch.vgic.vctrl_base));
 #endif

arch/arm64/kvm/hyp.S +0 -0

index b0d1512..2dbe337 100644

arch/arm64/kvm/hyp.S +0 -0

 	add	x3, x3, #VGIC_CPU_LR
 1:	ldr	w5, [x3], #4
 CPU_BE(	rev	w5, w5 )

include/kvm/arm_vgic.h +5 -2

index f27000f..eba4b51 100644
@@ -122,8 +122,11 @@ struct vgic_cpu {
 	/* Bitmap of used/free list registers */
 	DECLARE_BITMAP(	lr_used, VGIC_MAX_LRS);
 
-	/* Number of list registers on this CPU */
-	int		nr_lr;
+	/*
+	 * bit[31:16]: GICH_APR offset
+	 * bit[15:0]:  Number of list registers on this CPU
+	 */
+	u32		hw_cfg;
 
 	/* CPU vif control registers for world switch */
 	u32		vgic_hcr;

include/linux/irqchip/arm-gic.h +6 -0

index 45e2d8c..b055f92 100644
@@ -49,6 +49,8 @@
 #define GICH_ELRSR1 			0x34
 #define GICH_APR			0xf0
 #define GICH_LR0			0x100
+#define HIP04_GICH_APR			0x70
+/* GICH_LR0 offset in HiP04 is 0x80 */
 
 #define GICH_HCR_EN			(1 << 0)
 #define GICH_HCR_UIE			(1 << 1)

@@ -73,6 +75,10 @@
 #define GICH_MISR_EOI			(1 << 0)
 #define GICH_MISR_U			(1 << 1)
 
+#define HWCFG_NR_LR_MASK	0xffff
+#define HWCFG_APR_SHIFT		16
+#define HWCFG_APR_MASK		(0xffff << HWCFG_APR_SHIFT)
+
 #ifndef __ASSEMBLY__
 
 struct device_node;

virt/kvm/arm/vgic.c +25 -11

index 56ff9be..6c0ee89 100644
@@ -97,7 +97,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
 static void vgic_update_state(struct kvm *kvm);
 static void vgic_kick_vcpus(struct kvm *kvm);
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
-static u32 vgic_nr_lr;
+static u32 vgic_hw_cfg, vgic_nr_lr;
 
 static unsigned int vgic_maint_irq;
 

@@ -625,7 +625,7 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
 	int i, irq, source_cpu;
 	u32 *lr;
 
-	for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
+	for_each_set_bit(i, vgic_cpu->lr_used, vgic_nr_lr) {
 		lr = &vgic_cpu->vgic_lr[i];
 		irq = LR_IRQID(*lr);
 		source_cpu = LR_CPUID(*lr);

@@ -1006,7 +1006,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	int lr;
 
-	for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
+	for_each_set_bit(lr, vgic_cpu->lr_used, vgic_nr_lr) {
 		int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
 
 		if (!vgic_irq_is_enabled(vcpu, irq)) {

@@ -1047,8 +1047,8 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
 
 	/* Try to use another LR for this interrupt */
 	lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used,
-			       vgic_cpu->nr_lr);
-	if (lr >= vgic_cpu->nr_lr)
+				 vgic_nr_lr);
+	if (lr >= vgic_nr_lr)
 		return false;
 
 	kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id);

@@ -1183,7 +1183,7 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
 		int lr, irq;
 
 		for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr,
-				 vgic_cpu->nr_lr) {
+				 vgic_nr_lr) {
 			irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
 
 			vgic_irq_clear_active(vcpu, irq);

@@ -1227,7 +1227,7 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
 
 	/* Clear mappings for empty LRs */
 	for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr,
-			 vgic_cpu->nr_lr) {
+			 vgic_nr_lr) {
 		int irq;
 
 		if (!test_and_clear_bit(lr, vgic_cpu->lr_used))

@@ -1241,8 +1241,8 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
 
 	/* Check if we still have something up our sleeve... */
 	pending = find_first_zero_bit((unsigned long *)vgic_cpu->vgic_elrsr,
-				      vgic_cpu->nr_lr);
-	if (level_pending || pending < vgic_cpu->nr_lr)
+				      vgic_nr_lr);
+	if (level_pending || pending < vgic_nr_lr)
 		set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
 }
 

@@ -1438,7 +1438,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	 */
 	vgic_cpu->vgic_vmcr = 0;
 
-	vgic_cpu->nr_lr = vgic_nr_lr;
+	vgic_cpu->hw_cfg = vgic_hw_cfg;
 	vgic_cpu->vgic_hcr = GICH_HCR_EN; /* Get the show on the road... */
 
 	return 0;

@@ -1470,17 +1470,31 @@ static struct notifier_block vgic_cpu_nb = {
 	.notifier_call = vgic_cpu_notify,
 };
 
+static const struct of_device_id of_vgic_ids[] = {
+	{
+		.compatible = "arm,cortex-a15-gic",
+		.data = (void *)GICH_APR,
+	}, {
+		.compatible = "hisilicon,hip04-gic",
+		.data = (void *)HIP04_GICH_APR,
+	}, {
+	},
+};
+
 int kvm_vgic_hyp_init(void)
 {
 	int ret;
 	struct resource vctrl_res;
 	struct resource vcpu_res;
+	const struct of_device_id *match;
 
-	vgic_node = of_find_compatible_node(NULL, NULL, "arm,cortex-a15-gic");
+	vgic_node = of_find_matching_node_and_match(NULL, of_vgic_ids, &match);
 	if (!vgic_node) {
 		kvm_err("error: no compatible vgic node in DT\n");
 		return -ENODEV;
 	}
+	/* High word of vgic_hw_cfg is the offset of GICH_APR. */
+	vgic_hw_cfg = (unsigned long)match->data << HWCFG_APR_SHIFT;
 
 	vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
 	if (!vgic_maint_irq) {