* [PATCH v6 01/15] ARM: debug: add HiP04 debug uart
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
@ 2014-05-11 8:05 ` Haojian Zhuang
2014-05-11 8:05 ` [PATCH v6 02/15] irq: gic: use mask field in GICC_IAR Haojian Zhuang
` (14 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:05 UTC (permalink / raw)
To: linux-arm-kernel
Add the support of Hisilicon HiP04 debug uart.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/Kconfig.debug | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 4a2fc0b..5a311af 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -223,6 +223,14 @@ choice
Say Y here if you want kernel low-level debugging support
on HI3716 UART.
+ config DEBUG_HIP04_UART
+ bool "Hisilicon HiP04 Debug UART"
+ depends on ARCH_HIP04
+ select DEBUG_UART_8250
+ help
+ Say Y here if you want kernel low-level debugging support
+ on HIP04 UART.
+
config DEBUG_HIGHBANK_UART
bool "Kernel low-level debugging messages via Highbank UART"
depends on ARCH_HIGHBANK
@@ -1044,6 +1052,7 @@ config DEBUG_UART_PHYS
default 0xd4017000 if DEBUG_MMP_UART2
default 0xd4018000 if DEBUG_MMP_UART3
default 0xe0000000 if ARCH_SPEAR13XX
+ default 0xe4007000 if DEBUG_HIP04_UART
default 0xf0000be0 if ARCH_EBSA110
default 0xf1012000 if DEBUG_MVEBU_UART_ALTERNATE
default 0xf1012000 if ARCH_DOVE || ARCH_KIRKWOOD || ARCH_MV78XX0 || \
@@ -1076,6 +1085,7 @@ config DEBUG_UART_VIRT
default 0xf4090000 if ARCH_LPC32XX
default 0xf4200000 if ARCH_GEMINI
default 0xf7fc9000 if DEBUG_BERLIN_UART
+ default 0xf8007000 if DEBUG_HIP04_UART
default 0xf8009000 if DEBUG_VEXPRESS_UART0_CA9
default 0xf8090000 if DEBUG_VEXPRESS_UART0_RS1
default 0xfb009000 if DEBUG_REALVIEW_STD_PORT
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 02/15] irq: gic: use mask field in GICC_IAR
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
2014-05-11 8:05 ` [PATCH v6 01/15] ARM: debug: add HiP04 debug uart Haojian Zhuang
@ 2014-05-11 8:05 ` Haojian Zhuang
2014-05-19 0:40 ` Jason Cooper
2014-05-11 8:05 ` [PATCH v6 03/15] irq: gic: support hip04 gic Haojian Zhuang
` (13 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:05 UTC (permalink / raw)
To: linux-arm-kernel
Bit[9:0] is interrupt ID field in GICC_IAR. Bit[12:10] is CPU ID field,
and others are reserved.
So we should use GICC_IAR_INT_ID_MASK to get interrupt ID. It's not a good way
to use ~0x1c00 (CPU ID field) to get interrupt ID.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
drivers/irqchip/irq-gic.c | 2 +-
include/linux/irqchip/arm-gic.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4300b66..f711fb6 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -287,7 +287,7 @@ static void __exception_irq_entry gic_handle_irq(struct pt_regs *regs)
do {
irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
- irqnr = irqstat & ~0x1c00;
+ irqnr = irqstat & GICC_IAR_INT_ID_MASK;
if (likely(irqnr > 15 && irqnr < 1021)) {
irqnr = irq_find_mapping(gic->domain, irqnr);
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 7ed92d0..45e2d8c 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -21,6 +21,8 @@
#define GIC_CPU_ACTIVEPRIO 0xd0
#define GIC_CPU_IDENT 0xfc
+#define GICC_IAR_INT_ID_MASK 0x3ff
+
#define GIC_DIST_CTRL 0x000
#define GIC_DIST_CTR 0x004
#define GIC_DIST_IGROUP 0x080
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 02/15] irq: gic: use mask field in GICC_IAR
2014-05-11 8:05 ` [PATCH v6 02/15] irq: gic: use mask field in GICC_IAR Haojian Zhuang
@ 2014-05-19 0:40 ` Jason Cooper
0 siblings, 0 replies; 50+ messages in thread
From: Jason Cooper @ 2014-05-19 0:40 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, May 11, 2014 at 04:05:58PM +0800, Haojian Zhuang wrote:
> Bit[9:0] is interrupt ID field in GICC_IAR. Bit[12:10] is CPU ID field,
> and others are reserved.
>
> So we should use GICC_IAR_INT_ID_MASK to get interrupt ID. It's not a good way
> to use ~0x1c00 (CPU ID field) to get interrupt ID.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
> drivers/irqchip/irq-gic.c | 2 +-
> include/linux/irqchip/arm-gic.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
Applied to irqchip/core.
Amended subject line to "irqchip: gic: Use mask field in GICC_IAR"
thx,
Jason.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
2014-05-11 8:05 ` [PATCH v6 01/15] ARM: debug: add HiP04 debug uart Haojian Zhuang
2014-05-11 8:05 ` [PATCH v6 02/15] irq: gic: use mask field in GICC_IAR Haojian Zhuang
@ 2014-05-11 8:05 ` Haojian Zhuang
2014-05-15 9:34 ` Marc Zyngier
2014-05-19 2:05 ` Jason Cooper
2014-05-11 8:06 ` [PATCH v6 04/15] ARM: mcpm: support 4 clusters Haojian Zhuang
` (12 subsequent siblings)
15 siblings, 2 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:05 UTC (permalink / raw)
To: linux-arm-kernel
There's a little difference between ARM GIC and HiP04 GIC.
* HiP04 GIC could support 16 cores at most, and ARM GIC could support
8 cores at most. So the difination 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.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
Documentation/devicetree/bindings/arm/gic.txt | 1 +
drivers/irqchip/irq-gic.c | 160 ++++++++++++++++++++------
2 files changed, 129 insertions(+), 32 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index 5573c08..150f7d6 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -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.
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index f711fb6..d1d1430 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -68,6 +68,7 @@ struct gic_chip_data {
#ifdef CONFIG_GIC_NON_BANKED
void __iomem *(*get_base)(union gic_base *);
#endif
+ u32 nr_cpu_if;
};
static DEFINE_RAW_SPINLOCK(irq_controller_lock);
@@ -76,9 +77,11 @@ 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.
+ *
+ * Hisilicon HiP04 extends the number of CPU interface from 8 to 16.
*/
-#define NR_GIC_CPU_IF 8
-static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
+#define NR_GIC_CPU_IF 16
+static u16 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
/*
* Supported arch specific GIC irq extension.
@@ -241,20 +244,64 @@ static int gic_retrigger(struct irq_data *d)
return 0;
}
+static inline bool gic_is_standard(struct gic_chip_data *gic)
+{
+ return (gic->nr_cpu_if == 8);
+}
+
+static inline int gic_irqs_per_target_reg(struct gic_chip_data *gic)
+{
+ return 32 / gic->nr_cpu_if;
+}
+
#ifdef CONFIG_SMP
+static inline u32 irq_to_target_reg(struct irq_data *d)
+{
+ struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+ unsigned int i = gic_irq(d);
+
+ if (gic_is_standard(gic_data))
+ i = i & ~3U;
+ else
+ i = (i << 1) & ~3U;
+ return (i + GIC_DIST_TARGET);
+}
+
+static inline u32 irq_to_core_shift(struct irq_data *d)
+{
+ struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+ unsigned int i = gic_irq(d);
+
+ if (gic_is_standard(gic_data))
+ return ((i % 4) << 3);
+ return ((i % 2) << 4);
+}
+
+static inline u32 irq_to_core_mask(struct irq_data *d)
+{
+ struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+ u32 mask;
+ /* ARM GIC, nr_cpu_if == 8; HiP04 GIC, nr_cpu_if == 16 */
+ mask = (1 << gic_data->nr_cpu_if) - 1;
+ return (mask << irq_to_core_shift(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 shift = (gic_irq(d) % 4) * 8;
+ void __iomem *reg;
+ struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+ unsigned int shift = irq_to_core_shift(d);
unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
u32 val, mask, bit;
- if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
+ if (cpu >= gic_data->nr_cpu_if || cpu >= nr_cpu_ids)
return -EINVAL;
+ reg = gic_dist_base(d) + irq_to_target_reg(d);
+
raw_spin_lock(&irq_controller_lock);
- mask = 0xff << shift;
+ mask = irq_to_core_mask(d);
bit = gic_cpu_map[cpu] << shift;
val = readl_relaxed(reg) & ~mask;
writel_relaxed(val | bit, reg);
@@ -354,15 +401,24 @@ 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)
+static u16 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;
+ /*
+ * ARM GIC uses 8 registers for interrupt 0-31,
+ * HiP04 GIC uses 16 registers for interrupt 0-31.
+ */
+ for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
+ if (gic_is_standard(gic)) {
+ mask = readl_relaxed(base + GIC_DIST_TARGET + i);
+ mask |= mask >> 16;
+ mask |= mask >> 8;
+ } else { /* HiP04 GIC */
+ mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
+ mask |= mask >> 16;
+ }
if (mask)
break;
}
@@ -370,6 +426,10 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
if (!mask)
pr_crit("GIC CPU mask not found - kernel will fail to boot.\n");
+ /* ARM GIC needs 8-bit cpu mask, HiP04 GIC needs 16-bit cpu mask. */
+ if (gic_is_standard(gic))
+ mask &= 0xff;
+
return mask;
}
@@ -392,10 +452,17 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
* Set all global interrupts to this CPU only.
*/
cpumask = gic_get_cpumask(gic);
- cpumask |= cpumask << 8;
+ if (gic_is_standard(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);
+ for (i = 32; i < gic_irqs; i += gic_irqs_per_target_reg(gic)) {
+ if (gic_is_standard(gic))
+ writel_relaxed(cpumask,
+ base + GIC_DIST_TARGET + i / 4 * 4);
+ else
+ writel_relaxed(cpumask,
+ base + GIC_DIST_TARGET + i / 2 * 4);
+ }
/*
* Set priority on all global interrupts.
@@ -423,7 +490,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
/*
* Get what the GIC says our CPU mask is.
*/
- BUG_ON(cpu >= NR_GIC_CPU_IF);
+ BUG_ON(cpu >= gic->nr_cpu_if);
cpu_mask = gic_get_cpumask(gic);
gic_cpu_map[cpu] = cpu_mask;
@@ -431,7 +498,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
* Clear our mask from the other map entries in case they're
* still undefined.
*/
- for (i = 0; i < NR_GIC_CPU_IF; i++)
+ for (i = 0; i < gic->nr_cpu_if; i++)
if (i != cpu)
gic_cpu_map[i] &= ~cpu_mask;
@@ -467,7 +534,7 @@ void gic_cpu_if_down(void)
*/
static void gic_dist_save(unsigned int gic_nr)
{
- unsigned int gic_irqs;
+ unsigned int gic_irqs, nr_irq_per_target;
void __iomem *dist_base;
int i;
@@ -476,6 +543,7 @@ static void gic_dist_save(unsigned int gic_nr)
gic_irqs = gic_data[gic_nr].gic_irqs;
dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+ nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
if (!dist_base)
return;
@@ -484,7 +552,7 @@ static void gic_dist_save(unsigned int gic_nr)
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, 4); i++)
+ for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
gic_data[gic_nr].saved_spi_target[i] =
readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
@@ -502,7 +570,7 @@ static void gic_dist_save(unsigned int gic_nr)
*/
static void gic_dist_restore(unsigned int gic_nr)
{
- unsigned int gic_irqs;
+ unsigned int gic_irqs, nr_irq_per_target;
unsigned int i;
void __iomem *dist_base;
@@ -511,6 +579,7 @@ static void gic_dist_restore(unsigned int gic_nr)
gic_irqs = gic_data[gic_nr].gic_irqs;
dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+ nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
if (!dist_base)
return;
@@ -525,7 +594,7 @@ static void gic_dist_restore(unsigned int gic_nr)
writel_relaxed(0xa0a0a0a0,
dist_base + GIC_DIST_PRI + i * 4);
- for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
+ for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
dist_base + GIC_DIST_TARGET + i * 4);
@@ -665,9 +734,19 @@ 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);
-
+ /*
+ * CPUTargetList -- bit[23:16] in GIC_DIST_SOFTINT in ARM GIC.
+ * bit[23:8] in GIC_DIST_SOFTINT in HiP04 GIC.
+ * NSATT -- bit[15] in GIC_DIST_SOFTINT in ARM GIC.
+ * bit[7] in GIC_DIST_SOFTINT in HiP04 GIC.
+ * this always happens on GIC0
+ */
+ if (gic_is_standard(&gic_data[0]))
+ map = map << 16;
+ else
+ map = map << 8;
+ writel_relaxed(map | irq,
+ gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
}
#endif
@@ -681,10 +760,15 @@ 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);
+ BUG_ON(cpu_id >= gic_data[0].nr_cpu_if);
cpu_id = 1 << cpu_id;
/* this always happens on GIC0 */
- writel_relaxed((cpu_id << 16) | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+ if (gic_is_standard(&gic_data[0]))
+ cpu_id = cpu_id << 16;
+ else
+ cpu_id = cpu_id << 8;
+ writel_relaxed(cpu_id | irq,
+ gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
}
/*
@@ -700,7 +784,7 @@ int gic_get_cpu_id(unsigned int cpu)
{
unsigned int cpu_bit;
- if (cpu >= NR_GIC_CPU_IF)
+ if (cpu >= gic_data[0].nr_cpu_if)
return -1;
cpu_bit = gic_cpu_map[cpu];
if (cpu_bit & (cpu_bit - 1))
@@ -931,7 +1015,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 nr_routable_irqs;
+ int nr_routable_irqs, max_nr_irq;
BUG_ON(gic_nr >= MAX_GIC_NR);
@@ -967,12 +1051,22 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
gic_set_base_accessor(gic, gic_get_common_base);
}
+ if (of_device_is_compatible(node, "hisilicon,hip04-gic")) {
+ /* HiP04 GIC supports 16 CPUs at most */
+ gic->nr_cpu_if = 16;
+ max_nr_irq = 510;
+ } else {
+ /* ARM/Qualcomm GIC supports 8 CPUs at most */
+ gic->nr_cpu_if = 8;
+ max_nr_irq = 1020;
+ }
+
/*
* 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;
+ for (i = 0; i < gic->nr_cpu_if; i++)
+ gic_cpu_map[i] = (1 << gic->nr_cpu_if) - 1;
/*
* For primary GICs, skip over SGIs.
@@ -988,12 +1082,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 > max_nr_irq)
+ gic_irqs = max_nr_irq;
gic->gic_irqs = gic_irqs;
gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
@@ -1069,6 +1164,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
}
IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
+IRQCHIP_DECLARE(hip04_gic, "hisilicon,hip04-gic", gic_of_init);
IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-11 8:05 ` [PATCH v6 03/15] irq: gic: support hip04 gic Haojian Zhuang
@ 2014-05-15 9:34 ` Marc Zyngier
2014-05-15 11:14 ` Arnd Bergmann
2014-05-20 3:24 ` Haojian Zhuang
2014-05-19 2:05 ` Jason Cooper
1 sibling, 2 replies; 50+ messages in thread
From: Marc Zyngier @ 2014-05-15 9:34 UTC (permalink / raw)
To: linux-arm-kernel
On 11/05/14 09:05, Haojian Zhuang wrote:
> There's a little difference between ARM GIC and HiP04 GIC.
>
> * HiP04 GIC could support 16 cores at most, and ARM GIC could support
> 8 cores at most. So the difination 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.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
> Documentation/devicetree/bindings/arm/gic.txt | 1 +
> drivers/irqchip/irq-gic.c | 160 ++++++++++++++++++++------
> 2 files changed, 129 insertions(+), 32 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08..150f7d6 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -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.
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index f711fb6..d1d1430 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -68,6 +68,7 @@ struct gic_chip_data {
> #ifdef CONFIG_GIC_NON_BANKED
> void __iomem *(*get_base)(union gic_base *);
> #endif
> + u32 nr_cpu_if;
> };
>
> static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> @@ -76,9 +77,11 @@ 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.
> + *
> + * Hisilicon HiP04 extends the number of CPU interface from 8 to 16.
> */
> -#define NR_GIC_CPU_IF 8
> -static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
> +#define NR_GIC_CPU_IF 16
> +static u16 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
>
> /*
> * Supported arch specific GIC irq extension.
> @@ -241,20 +244,64 @@ static int gic_retrigger(struct irq_data *d)
> return 0;
> }
>
> +static inline bool gic_is_standard(struct gic_chip_data *gic)
Please loose all of the inlines. The compiler can do this by itself.
> +{
> + return (gic->nr_cpu_if == 8);
> +}
> +
> +static inline int gic_irqs_per_target_reg(struct gic_chip_data *gic)
> +{
> + return 32 / gic->nr_cpu_if;
> +}
> +
> #ifdef CONFIG_SMP
> +static inline u32 irq_to_target_reg(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + unsigned int i = gic_irq(d);
> +
> + if (gic_is_standard(gic_data))
> + i = i & ~3U;
> + else
> + i = (i << 1) & ~3U;
> + return (i + GIC_DIST_TARGET);
> +}
> +
> +static inline u32 irq_to_core_shift(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + unsigned int i = gic_irq(d);
> +
> + if (gic_is_standard(gic_data))
> + return ((i % 4) << 3);
> + return ((i % 2) << 4);
> +}
> +
> +static inline u32 irq_to_core_mask(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + u32 mask;
> + /* ARM GIC, nr_cpu_if == 8; HiP04 GIC, nr_cpu_if == 16 */
> + mask = (1 << gic_data->nr_cpu_if) - 1;
> + return (mask << irq_to_core_shift(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 shift = (gic_irq(d) % 4) * 8;
> + void __iomem *reg;
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + unsigned int shift = irq_to_core_shift(d);
> unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
> u32 val, mask, bit;
>
> - if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
> + if (cpu >= gic_data->nr_cpu_if || cpu >= nr_cpu_ids)
> return -EINVAL;
>
> + reg = gic_dist_base(d) + irq_to_target_reg(d);
> +
> raw_spin_lock(&irq_controller_lock);
> - mask = 0xff << shift;
> + mask = irq_to_core_mask(d);
> bit = gic_cpu_map[cpu] << shift;
> val = readl_relaxed(reg) & ~mask;
> writel_relaxed(val | bit, reg);
> @@ -354,15 +401,24 @@ 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)
> +static u16 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;
> + /*
> + * ARM GIC uses 8 registers for interrupt 0-31,
> + * HiP04 GIC uses 16 registers for interrupt 0-31.
> + */
> + for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
> + if (gic_is_standard(gic)) {
> + mask = readl_relaxed(base + GIC_DIST_TARGET + i);
> + mask |= mask >> 16;
> + mask |= mask >> 8;
> + } else { /* HiP04 GIC */
> + mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
> + mask |= mask >> 16;
> + }
You have irq_to_target_reg now, and you can rewrite most of this without
duplication (see my previous review comment).
> if (mask)
> break;
> }
> @@ -370,6 +426,10 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
> if (!mask)
> pr_crit("GIC CPU mask not found - kernel will fail to boot.\n");
>
> + /* ARM GIC needs 8-bit cpu mask, HiP04 GIC needs 16-bit cpu mask. */
> + if (gic_is_standard(gic))
> + mask &= 0xff;
> +
> return mask;
> }
>
> @@ -392,10 +452,17 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
> * Set all global interrupts to this CPU only.
> */
> cpumask = gic_get_cpumask(gic);
> - cpumask |= cpumask << 8;
> + if (gic_is_standard(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);
> + for (i = 32; i < gic_irqs; i += gic_irqs_per_target_reg(gic)) {
> + if (gic_is_standard(gic))
> + writel_relaxed(cpumask,
> + base + GIC_DIST_TARGET + i / 4 * 4);
> + else
> + writel_relaxed(cpumask,
> + base + GIC_DIST_TARGET + i / 2 * 4);
> + }
Same here.
>
> /*
> * Set priority on all global interrupts.
> @@ -423,7 +490,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> /*
> * Get what the GIC says our CPU mask is.
> */
> - BUG_ON(cpu >= NR_GIC_CPU_IF);
> + BUG_ON(cpu >= gic->nr_cpu_if);
> cpu_mask = gic_get_cpumask(gic);
> gic_cpu_map[cpu] = cpu_mask;
>
> @@ -431,7 +498,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> * Clear our mask from the other map entries in case they're
> * still undefined.
> */
> - for (i = 0; i < NR_GIC_CPU_IF; i++)
> + for (i = 0; i < gic->nr_cpu_if; i++)
> if (i != cpu)
> gic_cpu_map[i] &= ~cpu_mask;
>
> @@ -467,7 +534,7 @@ void gic_cpu_if_down(void)
> */
> static void gic_dist_save(unsigned int gic_nr)
> {
> - unsigned int gic_irqs;
> + unsigned int gic_irqs, nr_irq_per_target;
> void __iomem *dist_base;
> int i;
>
> @@ -476,6 +543,7 @@ static void gic_dist_save(unsigned int gic_nr)
>
> gic_irqs = gic_data[gic_nr].gic_irqs;
> dist_base = gic_data_dist_base(&gic_data[gic_nr]);
> + nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
>
> if (!dist_base)
> return;
> @@ -484,7 +552,7 @@ static void gic_dist_save(unsigned int gic_nr)
> 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, 4); i++)
> + for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
> gic_data[gic_nr].saved_spi_target[i] =
> readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
>
> @@ -502,7 +570,7 @@ static void gic_dist_save(unsigned int gic_nr)
> */
> static void gic_dist_restore(unsigned int gic_nr)
> {
> - unsigned int gic_irqs;
> + unsigned int gic_irqs, nr_irq_per_target;
> unsigned int i;
> void __iomem *dist_base;
>
> @@ -511,6 +579,7 @@ static void gic_dist_restore(unsigned int gic_nr)
>
> gic_irqs = gic_data[gic_nr].gic_irqs;
> dist_base = gic_data_dist_base(&gic_data[gic_nr]);
> + nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
>
> if (!dist_base)
> return;
> @@ -525,7 +594,7 @@ static void gic_dist_restore(unsigned int gic_nr)
> writel_relaxed(0xa0a0a0a0,
> dist_base + GIC_DIST_PRI + i * 4);
>
> - for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
> + for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
> writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
> dist_base + GIC_DIST_TARGET + i * 4);
>
> @@ -665,9 +734,19 @@ 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);
> -
> + /*
> + * CPUTargetList -- bit[23:16] in GIC_DIST_SOFTINT in ARM GIC.
> + * bit[23:8] in GIC_DIST_SOFTINT in HiP04 GIC.
> + * NSATT -- bit[15] in GIC_DIST_SOFTINT in ARM GIC.
> + * bit[7] in GIC_DIST_SOFTINT in HiP04 GIC.
> + * this always happens on GIC0
> + */
> + if (gic_is_standard(&gic_data[0]))
> + map = map << 16;
> + else
> + map = map << 8;
> + writel_relaxed(map | irq,
> + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
> }
> #endif
> @@ -681,10 +760,15 @@ 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);
> + BUG_ON(cpu_id >= gic_data[0].nr_cpu_if);
> cpu_id = 1 << cpu_id;
> /* this always happens on GIC0 */
> - writel_relaxed((cpu_id << 16) | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> + if (gic_is_standard(&gic_data[0]))
> + cpu_id = cpu_id << 16;
> + else
> + cpu_id = cpu_id << 8;
> + writel_relaxed(cpu_id | irq,
> + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> }
>
> /*
> @@ -700,7 +784,7 @@ int gic_get_cpu_id(unsigned int cpu)
> {
> unsigned int cpu_bit;
>
> - if (cpu >= NR_GIC_CPU_IF)
> + if (cpu >= gic_data[0].nr_cpu_if)
> return -1;
> cpu_bit = gic_cpu_map[cpu];
> if (cpu_bit & (cpu_bit - 1))
> @@ -931,7 +1015,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 nr_routable_irqs;
> + int nr_routable_irqs, max_nr_irq;
>
> BUG_ON(gic_nr >= MAX_GIC_NR);
>
> @@ -967,12 +1051,22 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> gic_set_base_accessor(gic, gic_get_common_base);
> }
>
> + if (of_device_is_compatible(node, "hisilicon,hip04-gic")) {
> + /* HiP04 GIC supports 16 CPUs at most */
> + gic->nr_cpu_if = 16;
> + max_nr_irq = 510;
> + } else {
> + /* ARM/Qualcomm GIC supports 8 CPUs at most */
> + gic->nr_cpu_if = 8;
> + max_nr_irq = 1020;
> + }
> +
> /*
> * 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;
> + for (i = 0; i < gic->nr_cpu_if; i++)
> + gic_cpu_map[i] = (1 << gic->nr_cpu_if) - 1;
>
> /*
> * For primary GICs, skip over SGIs.
> @@ -988,12 +1082,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 > max_nr_irq)
> + gic_irqs = max_nr_irq;
> gic->gic_irqs = gic_irqs;
>
> gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> @@ -1069,6 +1164,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> }
> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> +IRQCHIP_DECLARE(hip04_gic, "hisilicon,hip04-gic", gic_of_init);
> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>
> --
> 1.9.1
>
>
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-15 9:34 ` Marc Zyngier
@ 2014-05-15 11:14 ` Arnd Bergmann
2014-05-15 12:08 ` Christoffer Dall
2014-05-20 3:24 ` Haojian Zhuang
1 sibling, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2014-05-15 11:14 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 15 May 2014 10:34:35 Marc Zyngier wrote:
> > +static inline bool gic_is_standard(struct gic_chip_data *gic)
>
> Please loose all of the inlines. The compiler can do this by itself.
>
> > +{
> > + return (gic->nr_cpu_if == 8);
> > +}
> > +
I think it's actually the common style to spell out the 'inline'
for trivial helpers like this. The compiler doesn't need it, but
it also doesn't hurt and most people write it.
Arnd
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-15 11:14 ` Arnd Bergmann
@ 2014-05-15 12:08 ` Christoffer Dall
2014-05-15 12:13 ` Arnd Bergmann
0 siblings, 1 reply; 50+ messages in thread
From: Christoffer Dall @ 2014-05-15 12:08 UTC (permalink / raw)
To: linux-arm-kernel
On 15 May 2014 12:14, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 15 May 2014 10:34:35 Marc Zyngier wrote:
>> > +static inline bool gic_is_standard(struct gic_chip_data *gic)
>>
>> Please loose all of the inlines. The compiler can do this by itself.
>>
>> > +{
>> > + return (gic->nr_cpu_if == 8);
>> > +}
>> > +
>
> I think it's actually the common style to spell out the 'inline'
> for trivial helpers like this. The compiler doesn't need it, but
> it also doesn't hurt and most people write it.
>
Only in header files, no?
I've been chastised previously for putting this in .c files.
-Christoffer
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-15 12:08 ` Christoffer Dall
@ 2014-05-15 12:13 ` Arnd Bergmann
2014-05-15 12:16 ` Christoffer Dall
0 siblings, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2014-05-15 12:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 15 May 2014 13:08:17 Christoffer Dall wrote:
> On 15 May 2014 12:14, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 15 May 2014 10:34:35 Marc Zyngier wrote:
> >> > +static inline bool gic_is_standard(struct gic_chip_data *gic)
> >>
> >> Please loose all of the inlines. The compiler can do this by itself.
> >>
> >> > +{
> >> > + return (gic->nr_cpu_if == 8);
> >> > +}
> >> > +
> >
> > I think it's actually the common style to spell out the 'inline'
> > for trivial helpers like this. The compiler doesn't need it, but
> > it also doesn't hurt and most people write it.
> >
> Only in header files, no?
>
> I've been chastised previously for putting this in .c files.
It's crazy to make it a hard rule for C files one way or the other,
given that the compiler doesn't care.
I normally spell out the the inline part in my code because I find
that clearer to read.
Arnd
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-15 12:13 ` Arnd Bergmann
@ 2014-05-15 12:16 ` Christoffer Dall
0 siblings, 0 replies; 50+ messages in thread
From: Christoffer Dall @ 2014-05-15 12:16 UTC (permalink / raw)
To: linux-arm-kernel
On 15 May 2014 13:13, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 15 May 2014 13:08:17 Christoffer Dall wrote:
>> On 15 May 2014 12:14, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Thursday 15 May 2014 10:34:35 Marc Zyngier wrote:
>> >> > +static inline bool gic_is_standard(struct gic_chip_data *gic)
>> >>
>> >> Please loose all of the inlines. The compiler can do this by itself.
>> >>
>> >> > +{
>> >> > + return (gic->nr_cpu_if == 8);
>> >> > +}
>> >> > +
>> >
>> > I think it's actually the common style to spell out the 'inline'
>> > for trivial helpers like this. The compiler doesn't need it, but
>> > it also doesn't hurt and most people write it.
>> >
>> Only in header files, no?
>>
>> I've been chastised previously for putting this in .c files.
>
> It's crazy to make it a hard rule for C files one way or the other,
> given that the compiler doesn't care.
>
> I normally spell out the the inline part in my code because I find
> that clearer to read.
>
if that's the general consensus, I'll stop caring about it then.
-Christoffer
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-15 9:34 ` Marc Zyngier
2014-05-15 11:14 ` Arnd Bergmann
@ 2014-05-20 3:24 ` Haojian Zhuang
2014-05-20 9:02 ` Marc Zyngier
1 sibling, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-20 3:24 UTC (permalink / raw)
To: linux-arm-kernel
On 15 May 2014 17:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 11/05/14 09:05, Haojian Zhuang wrote:
>>
>> +static inline bool gic_is_standard(struct gic_chip_data *gic)
>
> Please loose all of the inlines. The compiler can do this by itself.
>
Since others also agree on inline, I'll keep to use it.
>> -static u8 gic_get_cpumask(struct gic_chip_data *gic)
>> +static u16 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;
>> + /*
>> + * ARM GIC uses 8 registers for interrupt 0-31,
>> + * HiP04 GIC uses 16 registers for interrupt 0-31.
>> + */
>> + for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
>> + if (gic_is_standard(gic)) {
>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i);
>> + mask |= mask >> 16;
>> + mask |= mask >> 8;
>> + } else { /* HiP04 GIC */
>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
>> + mask |= mask >> 16;
>> + }
>
> You have irq_to_target_reg now, and you can rewrite most of this without
> duplication (see my previous review comment).
>
At here, the offset from GIC_DIST_TARGET is got directly.
In irq_to_target_reg(), the parameter is struct irq_data. These two
cases are different.
How could I reuse the irq_to_target_reg() at here?
>> @@ -392,10 +452,17 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>> * Set all global interrupts to this CPU only.
>> */
>> cpumask = gic_get_cpumask(gic);
>> - cpumask |= cpumask << 8;
>> + if (gic_is_standard(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);
>> + for (i = 32; i < gic_irqs; i += gic_irqs_per_target_reg(gic)) {
>> + if (gic_is_standard(gic))
>> + writel_relaxed(cpumask,
>> + base + GIC_DIST_TARGET + i / 4 * 4);
>> + else
>> + writel_relaxed(cpumask,
>> + base + GIC_DIST_TARGET + i / 2 * 4);
>> + }
>
> Same here.
>
Same reason that I can't use irq_to_target_reg(). There's no struct
irq_data at here.
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-20 3:24 ` Haojian Zhuang
@ 2014-05-20 9:02 ` Marc Zyngier
2014-05-20 9:35 ` Haojian Zhuang
0 siblings, 1 reply; 50+ messages in thread
From: Marc Zyngier @ 2014-05-20 9:02 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 20 2014 at 4:24:08 am BST, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
> On 15 May 2014 17:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 11/05/14 09:05, Haojian Zhuang wrote:
>>>
>>> +static inline bool gic_is_standard(struct gic_chip_data *gic)
>>
>> Please loose all of the inlines. The compiler can do this by itself.
>>
> Since others also agree on inline, I'll keep to use it.
>
>>> -static u8 gic_get_cpumask(struct gic_chip_data *gic)
>>> +static u16 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;
>>> + /*
>>> + * ARM GIC uses 8 registers for interrupt 0-31,
>>> + * HiP04 GIC uses 16 registers for interrupt 0-31.
>>> + */
>>> + for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
>>> + if (gic_is_standard(gic)) {
>>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i);
>>> + mask |= mask >> 16;
>>> + mask |= mask >> 8;
>>> + } else { /* HiP04 GIC */
>>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
>>> + mask |= mask >> 16;
>>> + }
>>
>> You have irq_to_target_reg now, and you can rewrite most of this without
>> duplication (see my previous review comment).
>>
>
> At here, the offset from GIC_DIST_TARGET is got directly.
> In irq_to_target_reg(), the parameter is struct irq_data. These two
> cases are different.
>
> How could I reuse the irq_to_target_reg() at here?
By using your imagination, and redefining irq_to_target_reg to this:
static inline u32 irq_to_target_reg(struct gic_chip_data *gic, int irq)
{
if (!gic_is_standard(gic))
i *= 2;
irq &= ~3U;
return (i + GIC_DIST_TARGET);
}
You could then try modifying the only existing caller, and then rewrite
the above hunk as such:
for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
mask = readl_relaxed(base + irq_to_target_reg(gic, i));
mask |= mask >> 16;
if (gic_is_standard(gic))
mask |= mask >> 8;
}
>
>>> @@ -392,10 +452,17 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>>> * Set all global interrupts to this CPU only.
>>> */
>>> cpumask = gic_get_cpumask(gic);
>>> - cpumask |= cpumask << 8;
>>> + if (gic_is_standard(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);
>>> + for (i = 32; i < gic_irqs; i += gic_irqs_per_target_reg(gic)) {
>>> + if (gic_is_standard(gic))
>>> + writel_relaxed(cpumask,
>>> + base + GIC_DIST_TARGET + i / 4 * 4);
>>> + else
>>> + writel_relaxed(cpumask,
>>> + base + GIC_DIST_TARGET + i / 2 * 4);
>>> + }
>>
>> Same here.
>>
> Same reason that I can't use irq_to_target_reg(). There's no struct
> irq_data at here.
for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic))
writel_relaxed(cpumask, base + irq_to_target_reg(gic, i));
You might need to move irq_to_target_reg out of the CONFIG_SMP section
as well.
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-20 9:02 ` Marc Zyngier
@ 2014-05-20 9:35 ` Haojian Zhuang
2014-05-20 9:42 ` Marc Zyngier
0 siblings, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-20 9:35 UTC (permalink / raw)
To: linux-arm-kernel
On 20 May 2014 17:02, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On Tue, May 20 2014 at 4:24:08 am BST, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
>> On 15 May 2014 17:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> On 11/05/14 09:05, Haojian Zhuang wrote:
>>>>
>>>> +static inline bool gic_is_standard(struct gic_chip_data *gic)
>>>
>>> Please loose all of the inlines. The compiler can do this by itself.
>>>
>> Since others also agree on inline, I'll keep to use it.
>>
>>>> -static u8 gic_get_cpumask(struct gic_chip_data *gic)
>>>> +static u16 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;
>>>> + /*
>>>> + * ARM GIC uses 8 registers for interrupt 0-31,
>>>> + * HiP04 GIC uses 16 registers for interrupt 0-31.
>>>> + */
>>>> + for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
>>>> + if (gic_is_standard(gic)) {
>>>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i);
>>>> + mask |= mask >> 16;
>>>> + mask |= mask >> 8;
>>>> + } else { /* HiP04 GIC */
>>>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
>>>> + mask |= mask >> 16;
>>>> + }
>>>
>>> You have irq_to_target_reg now, and you can rewrite most of this without
>>> duplication (see my previous review comment).
>>>
>>
>> At here, the offset from GIC_DIST_TARGET is got directly.
>> In irq_to_target_reg(), the parameter is struct irq_data. These two
>> cases are different.
>>
>> How could I reuse the irq_to_target_reg() at here?
>
> By using your imagination, and redefining irq_to_target_reg to this:
>
> static inline u32 irq_to_target_reg(struct gic_chip_data *gic, int irq)
> {
> if (!gic_is_standard(gic))
> i *= 2;
> irq &= ~3U;
> return (i + GIC_DIST_TARGET);
> }
>
> You could then try modifying the only existing caller, and then rewrite
> the above hunk as such:
>
> for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
> mask = readl_relaxed(base + irq_to_target_reg(gic, i));
> mask |= mask >> 16;
> if (gic_is_standard(gic))
> mask |= mask >> 8;
> }
>
But why should I name it as irq_to_target_reg()? There's nothing related
to irq at here.
So I'll append a new function to handle it.
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-20 9:35 ` Haojian Zhuang
@ 2014-05-20 9:42 ` Marc Zyngier
0 siblings, 0 replies; 50+ messages in thread
From: Marc Zyngier @ 2014-05-20 9:42 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 20 2014 at 10:35:42 am BST, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
> On 20 May 2014 17:02, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On Tue, May 20 2014 at 4:24:08 am BST, Haojian Zhuang
>> <haojian.zhuang@linaro.org> wrote:
>>> On 15 May 2014 17:34, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> On 11/05/14 09:05, Haojian Zhuang wrote:
>>>>>
>>>>> +static inline bool gic_is_standard(struct gic_chip_data *gic)
>>>>
>>>> Please loose all of the inlines. The compiler can do this by itself.
>>>>
>>> Since others also agree on inline, I'll keep to use it.
>>>
>>>>> -static u8 gic_get_cpumask(struct gic_chip_data *gic)
>>>>> +static u16 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;
>>>>> + /*
>>>>> + * ARM GIC uses 8 registers for interrupt 0-31,
>>>>> + * HiP04 GIC uses 16 registers for interrupt 0-31.
>>>>> + */
>>>>> + for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
>>>>> + if (gic_is_standard(gic)) {
>>>>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i);
>>>>> + mask |= mask >> 16;
>>>>> + mask |= mask >> 8;
>>>>> + } else { /* HiP04 GIC */
>>>>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
>>>>> + mask |= mask >> 16;
>>>>> + }
>>>>
>>>> You have irq_to_target_reg now, and you can rewrite most of this without
>>>> duplication (see my previous review comment).
>>>>
>>>
>>> At here, the offset from GIC_DIST_TARGET is got directly.
>>> In irq_to_target_reg(), the parameter is struct irq_data. These two
>>> cases are different.
>>>
>>> How could I reuse the irq_to_target_reg() at here?
>>
>> By using your imagination, and redefining irq_to_target_reg to this:
>>
>> static inline u32 irq_to_target_reg(struct gic_chip_data *gic, int irq)
>> {
>> if (!gic_is_standard(gic))
>> i *= 2;
>> irq &= ~3U;
>> return (i + GIC_DIST_TARGET);
>> }
>>
>> You could then try modifying the only existing caller, and then rewrite
>> the above hunk as such:
>>
>> for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
>> mask = readl_relaxed(base + irq_to_target_reg(gic, i));
>> mask |= mask >> 16;
>> if (gic_is_standard(gic))
>> mask |= mask >> 8;
>> }
>>
>
> But why should I name it as irq_to_target_reg()? There's nothing related
> to irq at here.
What do you think the second parameter is? If the name troubles you,
feel free to suggest one you find more suitable.
> So I'll append a new function to handle it.
There is no need for that.
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-11 8:05 ` [PATCH v6 03/15] irq: gic: support hip04 gic Haojian Zhuang
2014-05-15 9:34 ` Marc Zyngier
@ 2014-05-19 2:05 ` Jason Cooper
2014-05-20 3:35 ` Haojian Zhuang
1 sibling, 1 reply; 50+ messages in thread
From: Jason Cooper @ 2014-05-19 2:05 UTC (permalink / raw)
To: linux-arm-kernel
Haojian,
On Sun, May 11, 2014 at 04:05:59PM +0800, Haojian Zhuang wrote:
> There's a little difference between ARM GIC and HiP04 GIC.
>
> * HiP04 GIC could support 16 cores at most, and ARM GIC could support
> 8 cores at most. So the difination 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.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
> Documentation/devicetree/bindings/arm/gic.txt | 1 +
> drivers/irqchip/irq-gic.c | 160 ++++++++++++++++++++------
> 2 files changed, 129 insertions(+), 32 deletions(-)
As I'm still coming up to speed on this driver, I'm just going to make
some general comments about this patch. If I appear to be off-base on
something, please don't assume I know better ;-)
I have no strong preference regarding using 'inline'. In my own code, I
prefer to be explicit, but I also prefer to use macros in some of the
situations below.
I would try to avoid runtime calculations of known values (register
offsets, masks, etc) where possible. I've tried to highlight some of
them below:
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08..150f7d6 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -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.
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index f711fb6..d1d1430 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -68,6 +68,7 @@ struct gic_chip_data {
> #ifdef CONFIG_GIC_NON_BANKED
> void __iomem *(*get_base)(union gic_base *);
> #endif
> + u32 nr_cpu_if;
> };
>
> static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> @@ -76,9 +77,11 @@ 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.
> + *
> + * Hisilicon HiP04 extends the number of CPU interface from 8 to 16.
> */
> -#define NR_GIC_CPU_IF 8
> -static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
> +#define NR_GIC_CPU_IF 16
> +static u16 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly;
>
> /*
> * Supported arch specific GIC irq extension.
> @@ -241,20 +244,64 @@ static int gic_retrigger(struct irq_data *d)
> return 0;
> }
>
> +static inline bool gic_is_standard(struct gic_chip_data *gic)
> +{
> + return (gic->nr_cpu_if == 8);
> +}
> +
> +static inline int gic_irqs_per_target_reg(struct gic_chip_data *gic)
> +{
> + return 32 / gic->nr_cpu_if;
> +}
> +
> #ifdef CONFIG_SMP
> +static inline u32 irq_to_target_reg(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + unsigned int i = gic_irq(d);
> +
> + if (gic_is_standard(gic_data))
> + i = i & ~3U;
> + else
> + i = (i << 1) & ~3U;
> + return (i + GIC_DIST_TARGET);
> +}
> +
> +static inline u32 irq_to_core_shift(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + unsigned int i = gic_irq(d);
> +
> + if (gic_is_standard(gic_data))
> + return ((i % 4) << 3);
> + return ((i % 2) << 4);
> +}
> +
> +static inline u32 irq_to_core_mask(struct irq_data *d)
> +{
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + u32 mask;
> + /* ARM GIC, nr_cpu_if == 8; HiP04 GIC, nr_cpu_if == 16 */
> + mask = (1 << gic_data->nr_cpu_if) - 1;
> + return (mask << irq_to_core_shift(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 shift = (gic_irq(d) % 4) * 8;
> + void __iomem *reg;
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> + unsigned int shift = irq_to_core_shift(d);
> unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask);
> u32 val, mask, bit;
>
> - if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
> + if (cpu >= gic_data->nr_cpu_if || cpu >= nr_cpu_ids)
> return -EINVAL;
>
> + reg = gic_dist_base(d) + irq_to_target_reg(d);
> +
> raw_spin_lock(&irq_controller_lock);
> - mask = 0xff << shift;
> + mask = irq_to_core_mask(d);
You're inside a spinlock here, calculating a mask value (in
irq_to_core_mask()) that never changes after boot. It, in turn, calls
irq_to_core_shift() and checks gic_is_standard() to use two values that
also never change after boot.
Sure, these functions are inlined, but the compiler can't optimize for
something that's determined at runtime. eg the mask, and X and Y in
((i % X) << Y).
> bit = gic_cpu_map[cpu] << shift;
> val = readl_relaxed(reg) & ~mask;
> writel_relaxed(val | bit, reg);
> @@ -354,15 +401,24 @@ 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)
> +static u16 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;
> + /*
> + * ARM GIC uses 8 registers for interrupt 0-31,
> + * HiP04 GIC uses 16 registers for interrupt 0-31.
> + */
> + for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
> + if (gic_is_standard(gic)) {
> + mask = readl_relaxed(base + GIC_DIST_TARGET + i);
> + mask |= mask >> 16;
> + mask |= mask >> 8;
> + } else { /* HiP04 GIC */
> + mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
> + mask |= mask >> 16;
> + }
> if (mask)
> break;
> }
> @@ -370,6 +426,10 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
> if (!mask)
> pr_crit("GIC CPU mask not found - kernel will fail to boot.\n");
>
> + /* ARM GIC needs 8-bit cpu mask, HiP04 GIC needs 16-bit cpu mask. */
> + if (gic_is_standard(gic))
> + mask &= 0xff;
> +
> return mask;
> }
>
> @@ -392,10 +452,17 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
> * Set all global interrupts to this CPU only.
> */
> cpumask = gic_get_cpumask(gic);
> - cpumask |= cpumask << 8;
> + if (gic_is_standard(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);
> + for (i = 32; i < gic_irqs; i += gic_irqs_per_target_reg(gic)) {
> + if (gic_is_standard(gic))
> + writel_relaxed(cpumask,
> + base + GIC_DIST_TARGET + i / 4 * 4);
> + else
> + writel_relaxed(cpumask,
> + base + GIC_DIST_TARGET + i / 2 * 4);
> + }
>
> /*
> * Set priority on all global interrupts.
> @@ -423,7 +490,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> /*
> * Get what the GIC says our CPU mask is.
> */
> - BUG_ON(cpu >= NR_GIC_CPU_IF);
> + BUG_ON(cpu >= gic->nr_cpu_if);
> cpu_mask = gic_get_cpumask(gic);
> gic_cpu_map[cpu] = cpu_mask;
>
> @@ -431,7 +498,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> * Clear our mask from the other map entries in case they're
> * still undefined.
> */
> - for (i = 0; i < NR_GIC_CPU_IF; i++)
> + for (i = 0; i < gic->nr_cpu_if; i++)
> if (i != cpu)
> gic_cpu_map[i] &= ~cpu_mask;
>
> @@ -467,7 +534,7 @@ void gic_cpu_if_down(void)
> */
> static void gic_dist_save(unsigned int gic_nr)
> {
> - unsigned int gic_irqs;
> + unsigned int gic_irqs, nr_irq_per_target;
> void __iomem *dist_base;
> int i;
>
> @@ -476,6 +543,7 @@ static void gic_dist_save(unsigned int gic_nr)
>
> gic_irqs = gic_data[gic_nr].gic_irqs;
> dist_base = gic_data_dist_base(&gic_data[gic_nr]);
> + nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
gic_dist_save() isn't called too often (suspend/idle), but this is
another example of calculating something that doesn't change after boot.
>
> if (!dist_base)
> return;
> @@ -484,7 +552,7 @@ static void gic_dist_save(unsigned int gic_nr)
> 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, 4); i++)
> + for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
> gic_data[gic_nr].saved_spi_target[i] =
> readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
>
> @@ -502,7 +570,7 @@ static void gic_dist_save(unsigned int gic_nr)
> */
> static void gic_dist_restore(unsigned int gic_nr)
> {
> - unsigned int gic_irqs;
> + unsigned int gic_irqs, nr_irq_per_target;
> unsigned int i;
> void __iomem *dist_base;
>
> @@ -511,6 +579,7 @@ static void gic_dist_restore(unsigned int gic_nr)
>
> gic_irqs = gic_data[gic_nr].gic_irqs;
> dist_base = gic_data_dist_base(&gic_data[gic_nr]);
> + nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
Same.
>
> if (!dist_base)
> return;
> @@ -525,7 +594,7 @@ static void gic_dist_restore(unsigned int gic_nr)
> writel_relaxed(0xa0a0a0a0,
> dist_base + GIC_DIST_PRI + i * 4);
>
> - for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
> + for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
> writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
> dist_base + GIC_DIST_TARGET + i * 4);
>
> @@ -665,9 +734,19 @@ 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);
> -
> + /*
> + * CPUTargetList -- bit[23:16] in GIC_DIST_SOFTINT in ARM GIC.
> + * bit[23:8] in GIC_DIST_SOFTINT in HiP04 GIC.
> + * NSATT -- bit[15] in GIC_DIST_SOFTINT in ARM GIC.
> + * bit[7] in GIC_DIST_SOFTINT in HiP04 GIC.
> + * this always happens on GIC0
> + */
> + if (gic_is_standard(&gic_data[0]))
> + map = map << 16;
> + else
> + map = map << 8;
ditto. Inside a spinlock as well.
> + writel_relaxed(map | irq,
> + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
> }
> #endif
> @@ -681,10 +760,15 @@ 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);
> + BUG_ON(cpu_id >= gic_data[0].nr_cpu_if);
> cpu_id = 1 << cpu_id;
> /* this always happens on GIC0 */
> - writel_relaxed((cpu_id << 16) | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> + if (gic_is_standard(&gic_data[0]))
> + cpu_id = cpu_id << 16;
> + else
> + cpu_id = cpu_id << 8;
ditto.
> + writel_relaxed(cpu_id | irq,
> + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> }
>
> /*
> @@ -700,7 +784,7 @@ int gic_get_cpu_id(unsigned int cpu)
> {
> unsigned int cpu_bit;
>
> - if (cpu >= NR_GIC_CPU_IF)
> + if (cpu >= gic_data[0].nr_cpu_if)
> return -1;
> cpu_bit = gic_cpu_map[cpu];
> if (cpu_bit & (cpu_bit - 1))
> @@ -931,7 +1015,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 nr_routable_irqs;
> + int nr_routable_irqs, max_nr_irq;
>
> BUG_ON(gic_nr >= MAX_GIC_NR);
>
> @@ -967,12 +1051,22 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> gic_set_base_accessor(gic, gic_get_common_base);
> }
>
> + if (of_device_is_compatible(node, "hisilicon,hip04-gic")) {
> + /* HiP04 GIC supports 16 CPUs at most */
> + gic->nr_cpu_if = 16;
> + max_nr_irq = 510;
> + } else {
> + /* ARM/Qualcomm GIC supports 8 CPUs at most */
> + gic->nr_cpu_if = 8;
> + max_nr_irq = 1020;
> + }
> +
> /*
> * 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;
> + for (i = 0; i < gic->nr_cpu_if; i++)
> + gic_cpu_map[i] = (1 << gic->nr_cpu_if) - 1;
>
> /*
> * For primary GICs, skip over SGIs.
> @@ -988,12 +1082,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 > max_nr_irq)
> + gic_irqs = max_nr_irq;
> gic->gic_irqs = gic_irqs;
>
> gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> @@ -1069,6 +1164,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> }
> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> +IRQCHIP_DECLARE(hip04_gic, "hisilicon,hip04-gic", gic_of_init);
> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
I would re-evaluate your use of gic_is_standard() and attempt to
eliminate all post-bootup callers of it.
thx,
Jason.
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 03/15] irq: gic: support hip04 gic
2014-05-19 2:05 ` Jason Cooper
@ 2014-05-20 3:35 ` Haojian Zhuang
0 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-20 3:35 UTC (permalink / raw)
To: linux-arm-kernel
On 19 May 2014 10:05, Jason Cooper <jason@lakedaemon.net> wrote:
> Haojian,
>
> On Sun, May 11, 2014 at 04:05:59PM +0800, Haojian Zhuang wrote:
>> There's a little difference between ARM GIC and HiP04 GIC.
>>
>> * HiP04 GIC could support 16 cores at most, and ARM GIC could support
>> 8 cores at most. So the difination 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.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>> Documentation/devicetree/bindings/arm/gic.txt | 1 +
>> drivers/irqchip/irq-gic.c | 160 ++++++++++++++++++++------
>> 2 files changed, 129 insertions(+), 32 deletions(-)
>
> As I'm still coming up to speed on this driver, I'm just going to make
> some general comments about this patch. If I appear to be off-base on
> something, please don't assume I know better ;-)
>
> I have no strong preference regarding using 'inline'. In my own code, I
> prefer to be explicit, but I also prefer to use macros in some of the
> situations below.
>
> I would try to avoid runtime calculations of known values (register
> offsets, masks, etc) where possible. I've tried to highlight some of
> them below:
>
>>
>> - if (cpu >= NR_GIC_CPU_IF || cpu >= nr_cpu_ids)
>> + if (cpu >= gic_data->nr_cpu_if || cpu >= nr_cpu_ids)
>> return -EINVAL;
>>
>> + reg = gic_dist_base(d) + irq_to_target_reg(d);
>> +
>> raw_spin_lock(&irq_controller_lock);
>> - mask = 0xff << shift;
>> + mask = irq_to_core_mask(d);
>
> You're inside a spinlock here, calculating a mask value (in
> irq_to_core_mask()) that never changes after boot. It, in turn, calls
> irq_to_core_shift() and checks gic_is_standard() to use two values that
> also never change after boot.
>
> Sure, these functions are inlined, but the compiler can't optimize for
> something that's determined at runtime. eg the mask, and X and Y in
> ((i % X) << Y).
>
Do you mean that inlined is useless at here? So I should drop inlined
at here instead.
>> bit = gic_cpu_map[cpu] << shift;
>> val = readl_relaxed(reg) & ~mask;
>> writel_relaxed(val | bit, reg);
>> @@ -354,15 +401,24 @@ 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)
>> +static u16 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;
>> + /*
>> + * ARM GIC uses 8 registers for interrupt 0-31,
>> + * HiP04 GIC uses 16 registers for interrupt 0-31.
>> + */
>> + for (i = mask = 0; i < 32; i += gic_irqs_per_target_reg(gic)) {
>> + if (gic_is_standard(gic)) {
>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i);
>> + mask |= mask >> 16;
>> + mask |= mask >> 8;
>> + } else { /* HiP04 GIC */
>> + mask = readl_relaxed(base + GIC_DIST_TARGET + i * 2);
>> + mask |= mask >> 16;
>> + }
>> if (mask)
>> break;
>> }
>> @@ -370,6 +426,10 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
>> if (!mask)
>> pr_crit("GIC CPU mask not found - kernel will fail to boot.\n");
>>
>> + /* ARM GIC needs 8-bit cpu mask, HiP04 GIC needs 16-bit cpu mask. */
>> + if (gic_is_standard(gic))
>> + mask &= 0xff;
>> +
>> return mask;
>> }
>>
>> @@ -392,10 +452,17 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>> * Set all global interrupts to this CPU only.
>> */
>> cpumask = gic_get_cpumask(gic);
>> - cpumask |= cpumask << 8;
>> + if (gic_is_standard(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);
>> + for (i = 32; i < gic_irqs; i += gic_irqs_per_target_reg(gic)) {
>> + if (gic_is_standard(gic))
>> + writel_relaxed(cpumask,
>> + base + GIC_DIST_TARGET + i / 4 * 4);
>> + else
>> + writel_relaxed(cpumask,
>> + base + GIC_DIST_TARGET + i / 2 * 4);
>> + }
>>
>> /*
>> * Set priority on all global interrupts.
>> @@ -423,7 +490,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>> /*
>> * Get what the GIC says our CPU mask is.
>> */
>> - BUG_ON(cpu >= NR_GIC_CPU_IF);
>> + BUG_ON(cpu >= gic->nr_cpu_if);
>> cpu_mask = gic_get_cpumask(gic);
>> gic_cpu_map[cpu] = cpu_mask;
>>
>> @@ -431,7 +498,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>> * Clear our mask from the other map entries in case they're
>> * still undefined.
>> */
>> - for (i = 0; i < NR_GIC_CPU_IF; i++)
>> + for (i = 0; i < gic->nr_cpu_if; i++)
>> if (i != cpu)
>> gic_cpu_map[i] &= ~cpu_mask;
>>
>> @@ -467,7 +534,7 @@ void gic_cpu_if_down(void)
>> */
>> static void gic_dist_save(unsigned int gic_nr)
>> {
>> - unsigned int gic_irqs;
>> + unsigned int gic_irqs, nr_irq_per_target;
>> void __iomem *dist_base;
>> int i;
>>
>> @@ -476,6 +543,7 @@ static void gic_dist_save(unsigned int gic_nr)
>>
>> gic_irqs = gic_data[gic_nr].gic_irqs;
>> dist_base = gic_data_dist_base(&gic_data[gic_nr]);
>> + nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
>
> gic_dist_save() isn't called too often (suspend/idle), but this is
> another example of calculating something that doesn't change after boot.
>
>>
>> if (!dist_base)
>> return;
>> @@ -484,7 +552,7 @@ static void gic_dist_save(unsigned int gic_nr)
>> 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, 4); i++)
>> + for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
>> gic_data[gic_nr].saved_spi_target[i] =
>> readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
>>
>> @@ -502,7 +570,7 @@ static void gic_dist_save(unsigned int gic_nr)
>> */
>> static void gic_dist_restore(unsigned int gic_nr)
>> {
>> - unsigned int gic_irqs;
>> + unsigned int gic_irqs, nr_irq_per_target;
>> unsigned int i;
>> void __iomem *dist_base;
>>
>> @@ -511,6 +579,7 @@ static void gic_dist_restore(unsigned int gic_nr)
>>
>> gic_irqs = gic_data[gic_nr].gic_irqs;
>> dist_base = gic_data_dist_base(&gic_data[gic_nr]);
>> + nr_irq_per_target = gic_irqs_per_target_reg(&gic_data[gic_nr]);
>
> Same.
>
>>
>> if (!dist_base)
>> return;
>> @@ -525,7 +594,7 @@ static void gic_dist_restore(unsigned int gic_nr)
>> writel_relaxed(0xa0a0a0a0,
>> dist_base + GIC_DIST_PRI + i * 4);
>>
>> - for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
>> + for (i = 0; i < DIV_ROUND_UP(gic_irqs, nr_irq_per_target); i++)
>> writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
>> dist_base + GIC_DIST_TARGET + i * 4);
>>
>> @@ -665,9 +734,19 @@ 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);
>> -
>> + /*
>> + * CPUTargetList -- bit[23:16] in GIC_DIST_SOFTINT in ARM GIC.
>> + * bit[23:8] in GIC_DIST_SOFTINT in HiP04 GIC.
>> + * NSATT -- bit[15] in GIC_DIST_SOFTINT in ARM GIC.
>> + * bit[7] in GIC_DIST_SOFTINT in HiP04 GIC.
>> + * this always happens on GIC0
>> + */
>> + if (gic_is_standard(&gic_data[0]))
>> + map = map << 16;
>> + else
>> + map = map << 8;
>
> ditto. Inside a spinlock as well.
>
>> + writel_relaxed(map | irq,
>> + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
>> raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
>> }
>> #endif
>> @@ -681,10 +760,15 @@ 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);
>> + BUG_ON(cpu_id >= gic_data[0].nr_cpu_if);
>> cpu_id = 1 << cpu_id;
>> /* this always happens on GIC0 */
>> - writel_relaxed((cpu_id << 16) | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
>> + if (gic_is_standard(&gic_data[0]))
>> + cpu_id = cpu_id << 16;
>> + else
>> + cpu_id = cpu_id << 8;
>
> ditto.
>
>> + writel_relaxed(cpu_id | irq,
>> + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
>> }
>>
>> /*
>> @@ -700,7 +784,7 @@ int gic_get_cpu_id(unsigned int cpu)
>> {
>> unsigned int cpu_bit;
>>
>> - if (cpu >= NR_GIC_CPU_IF)
>> + if (cpu >= gic_data[0].nr_cpu_if)
>> return -1;
>> cpu_bit = gic_cpu_map[cpu];
>> if (cpu_bit & (cpu_bit - 1))
>> @@ -931,7 +1015,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 nr_routable_irqs;
>> + int nr_routable_irqs, max_nr_irq;
>>
>> BUG_ON(gic_nr >= MAX_GIC_NR);
>>
>> @@ -967,12 +1051,22 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>> gic_set_base_accessor(gic, gic_get_common_base);
>> }
>>
>> + if (of_device_is_compatible(node, "hisilicon,hip04-gic")) {
>> + /* HiP04 GIC supports 16 CPUs at most */
>> + gic->nr_cpu_if = 16;
>> + max_nr_irq = 510;
>> + } else {
>> + /* ARM/Qualcomm GIC supports 8 CPUs at most */
>> + gic->nr_cpu_if = 8;
>> + max_nr_irq = 1020;
>> + }
>> +
>> /*
>> * 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;
>> + for (i = 0; i < gic->nr_cpu_if; i++)
>> + gic_cpu_map[i] = (1 << gic->nr_cpu_if) - 1;
>>
>> /*
>> * For primary GICs, skip over SGIs.
>> @@ -988,12 +1082,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 > max_nr_irq)
>> + gic_irqs = max_nr_irq;
>> gic->gic_irqs = gic_irqs;
>>
>> gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>> @@ -1069,6 +1164,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>> }
>> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>> +IRQCHIP_DECLARE(hip04_gic, "hisilicon,hip04-gic", gic_of_init);
>> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>
> I would re-evaluate your use of gic_is_standard() and attempt to
> eliminate all post-bootup callers of it.
>
I didn't get your point. Do you mean that I should use macro or global
variable instead? All inlined should be dropped, since they're embedded
in spin_lock_irqsave().
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 04/15] ARM: mcpm: support 4 clusters
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (2 preceding siblings ...)
2014-05-11 8:05 ` [PATCH v6 03/15] irq: gic: support hip04 gic Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 05/15] ARM: hisi: add ARCH_HISI Haojian Zhuang
` (11 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Add the CONFIG_MCPM_QUAD_CLUSTER configuration to enlarge cluster number
from 2 to 4.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Reviewed-by: Nicolas Pitre <nico@linaro.org>
---
arch/arm/Kconfig | 9 +++++++++
arch/arm/include/asm/mcpm.h | 5 +++++
2 files changed, 14 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index ab438cb..6ce4a49 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1564,6 +1564,15 @@ config MCPM
for (multi-)cluster based systems, such as big.LITTLE based
systems.
+config MCPM_QUAD_CLUSTER
+ bool
+ depends on MCPM
+ help
+ To avoid wasting resources unnecessarily, MCPM only supports up
+ to 2 clusters by default.
+ Platforms with 3 or 4 clusters that use MCPM must select this
+ option to allow the additional clusters to be managed.
+
config BIG_LITTLE
bool "big.LITTLE support (Experimental)"
depends on CPU_V7 && SMP
diff --git a/arch/arm/include/asm/mcpm.h b/arch/arm/include/asm/mcpm.h
index 608516e..fc8d70d 100644
--- a/arch/arm/include/asm/mcpm.h
+++ b/arch/arm/include/asm/mcpm.h
@@ -20,7 +20,12 @@
* to consider dynamic allocation.
*/
#define MAX_CPUS_PER_CLUSTER 4
+
+#ifdef CONFIG_MCPM_QUAD_CLUSTER
+#define MAX_NR_CLUSTERS 4
+#else
#define MAX_NR_CLUSTERS 2
+#endif
#ifndef __ASSEMBLY__
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 05/15] ARM: hisi: add ARCH_HISI
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (3 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 04/15] ARM: mcpm: support 4 clusters Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 06/15] ARM: hisi: enable MCPM implementation Haojian Zhuang
` (10 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Since multiple ARCH configuration will be appended into mach-hisi
directory, add ARCH_HISI as common configuration for different ARCH in
mach-hisi.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/Makefile | 2 +-
arch/arm/mach-hisi/Kconfig | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 41c1931..4c2798a 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -154,7 +154,7 @@ machine-$(CONFIG_ARCH_EP93XX) += ep93xx
machine-$(CONFIG_ARCH_EXYNOS) += exynos
machine-$(CONFIG_ARCH_GEMINI) += gemini
machine-$(CONFIG_ARCH_HIGHBANK) += highbank
-machine-$(CONFIG_ARCH_HI3xxx) += hisi
+machine-$(CONFIG_ARCH_HISI) += hisi
machine-$(CONFIG_ARCH_INTEGRATOR) += integrator
machine-$(CONFIG_ARCH_IOP13XX) += iop13xx
machine-$(CONFIG_ARCH_IOP32X) += iop32x
diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index feee4db..da16efd 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -1,8 +1,16 @@
-config ARCH_HI3xxx
- bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
+config ARCH_HISI
+ bool "Hisilicon SoC Support"
+ depends on ARCH_MULTIPLATFORM
select ARM_AMBA
select ARM_GIC
select ARM_TIMER_SP804
+
+if ARCH_HISI
+
+menu "Hisilicon platform type"
+
+config ARCH_HI3xxx
+ bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
select CACHE_L2X0
select HAVE_ARM_SCU if SMP
select HAVE_ARM_TWD if SMP
@@ -10,3 +18,7 @@ config ARCH_HI3xxx
select PINCTRL_SINGLE
help
Support for Hisilicon Hi36xx/Hi37xx processor family
+
+endmenu
+
+endif
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 06/15] ARM: hisi: enable MCPM implementation
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (4 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 05/15] ARM: hisi: add ARCH_HISI Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-13 8:28 ` Dave Martin
2014-05-13 11:44 ` [PATCH v7 " Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 07/15] ARM: hisi: enable HiP04 Haojian Zhuang
` (9 subsequent siblings)
15 siblings, 2 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
framework to manage power on HiP04 SoC.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/mach-hisi/Makefile | 1 +
arch/arm/mach-hisi/platmcpm.c | 309 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 310 insertions(+)
create mode 100644 arch/arm/mach-hisi/platmcpm.c
diff --git a/arch/arm/mach-hisi/Makefile b/arch/arm/mach-hisi/Makefile
index 2ae1b59..e7a8640 100644
--- a/arch/arm/mach-hisi/Makefile
+++ b/arch/arm/mach-hisi/Makefile
@@ -3,4 +3,5 @@
#
obj-y += hisilicon.o
+obj-$(CONFIG_MCPM) += platmcpm.o
obj-$(CONFIG_SMP) += platsmp.o hotplug.o
diff --git a/arch/arm/mach-hisi/platmcpm.c b/arch/arm/mach-hisi/platmcpm.c
new file mode 100644
index 0000000..6208bc6
--- /dev/null
+++ b/arch/arm/mach-hisi/platmcpm.c
@@ -0,0 +1,309 @@
+/*
+ * Copyright (c) 2013-2014 Linaro Ltd.
+ * Copyright (c) 2013-2014 Hisilicon Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+
+#include <asm/cputype.h>
+#include <asm/cp15.h>
+#include <asm/mcpm.h>
+
+#include "core.h"
+
+/* bits definition in SC_CPU_RESET_REQ[x]/SC_CPU_RESET_DREQ[x]
+ * 1 -- unreset; 0 -- reset
+ */
+#define CORE_RESET_BIT(x) (1 << x)
+#define NEON_RESET_BIT(x) (1 << (x + 4))
+#define CORE_DEBUG_RESET_BIT(x) (1 << (x + 9))
+#define CLUSTER_L2_RESET_BIT (1 << 8)
+#define CLUSTER_DEBUG_RESET_BIT (1 << 13)
+
+/*
+ * bits definition in SC_CPU_RESET_STATUS[x]
+ * 1 -- reset status; 0 -- unreset status
+ */
+#define CORE_RESET_STATUS(x) (1 << x)
+#define NEON_RESET_STATUS(x) (1 << (x + 4))
+#define CORE_DEBUG_RESET_STATUS(x) (1 << (x + 9))
+#define CLUSTER_L2_RESET_STATUS (1 << 8)
+#define CLUSTER_DEBUG_RESET_STATUS (1 << 13)
+#define CORE_WFI_STATUS(x) (1 << (x + 16))
+#define CORE_WFE_STATUS(x) (1 << (x + 20))
+#define CORE_DEBUG_ACK(x) (1 << (x + 24))
+
+#define SC_CPU_RESET_REQ(x) (0x520 + (x << 3)) /* reset */
+#define SC_CPU_RESET_DREQ(x) (0x524 + (x << 3)) /* unreset */
+#define SC_CPU_RESET_STATUS(x) (0x1520 + (x << 3))
+
+#define FAB_SF_MODE 0x0c
+#define FAB_SF_INVLD 0x10
+
+/* bits definition in FB_SF_INVLD */
+#define FB_SF_INVLD_START (1 << 8)
+
+#define HIP04_MAX_CLUSTERS 4
+#define HIP04_MAX_CPUS_PER_CLUSTER 4
+
+#define POLL_MSEC 10
+#define TIMEOUT_MSEC 1000
+
+struct hip04_secondary_cpu_data {
+ u32 bootwrapper_phys;
+ u32 bootwrapper_size;
+ u32 bootwrapper_magic;
+ u32 relocation_entry;
+ u32 relocation_size;
+};
+
+static void __iomem *relocation, *sysctrl, *fabric;
+static int hip04_cpu_table[HIP04_MAX_CLUSTERS][HIP04_MAX_CPUS_PER_CLUSTER];
+static DEFINE_SPINLOCK(boot_lock);
+static struct hip04_secondary_cpu_data hip04_boot;
+
+static bool hip04_cluster_down(unsigned int cluster)
+{
+ int i;
+
+ for (i = 0; i < HIP04_MAX_CPUS_PER_CLUSTER; i++)
+ if (hip04_cpu_table[cluster][i])
+ return false;
+ return true;
+}
+
+static void hip04_set_snoop_filter(unsigned int cluster, unsigned int on)
+{
+ unsigned long data;
+
+ if (!fabric)
+ return;
+ data = readl_relaxed(fabric + FAB_SF_MODE);
+ if (on)
+ data |= 1 << cluster;
+ else
+ data &= ~(1 << cluster);
+ writel_relaxed(data, fabric + FAB_SF_MODE);
+ while (1) {
+ if (data == readl_relaxed(fabric + FAB_SF_MODE))
+ break;
+ }
+}
+
+static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
+{
+ unsigned long data, mask;
+
+ if (!relocation || !sysctrl)
+ return -ENODEV;
+ if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
+ return -EINVAL;
+
+ spin_lock_irq(&boot_lock);
+ writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
+ writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
+ writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
+ writel_relaxed(0, relocation + 12);
+
+ if (hip04_cluster_down(cluster)) {
+ data = CLUSTER_DEBUG_RESET_BIT;
+ writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
+ do {
+ mask = CLUSTER_DEBUG_RESET_STATUS;
+ data = readl_relaxed(sysctrl + \
+ SC_CPU_RESET_STATUS(cluster));
+ } while (data & mask);
+ hip04_set_snoop_filter(cluster, 1);
+ }
+
+ hip04_cpu_table[cluster][cpu]++;
+
+ data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
+ CORE_DEBUG_RESET_BIT(cpu);
+ writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
+ spin_unlock_irq(&boot_lock);
+ msleep(POLL_MSEC);
+
+ return 0;
+}
+
+static void hip04_mcpm_power_down(void)
+{
+ unsigned int mpidr, cpu, cluster, data = 0;
+ bool skip_reset = false;
+
+ mpidr = read_cpuid_mpidr();
+ cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+ __mcpm_cpu_going_down(cpu, cluster);
+
+ spin_lock(&boot_lock);
+ BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
+ hip04_cpu_table[cluster][cpu]--;
+ if (hip04_cpu_table[cluster][cpu] == 1) {
+ /* A power_up request went ahead of us. */
+ skip_reset = true;
+ } else if (hip04_cpu_table[cluster][cpu] > 1) {
+ pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
+ BUG();
+ }
+
+ spin_unlock(&boot_lock);
+
+ v7_exit_coherency_flush(louis);
+
+ __mcpm_cpu_down(cpu, cluster);
+
+ if (!skip_reset) {
+ data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
+ CORE_DEBUG_RESET_BIT(cpu);
+ writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
+ }
+}
+
+static int hip04_mcpm_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
+{
+ unsigned int data, tries;
+
+ BUG_ON(cluster >= HIP04_MAX_CLUSTERS ||
+ cpu >= HIP04_MAX_CPUS_PER_CLUSTER);
+
+ for (tries = 0; tries < TIMEOUT_MSEC / POLL_MSEC; tries++) {
+ data = readl_relaxed(sysctrl + SC_CPU_RESET_STATUS(cluster));
+ if (!(data & CORE_RESET_STATUS(cpu))) {
+ msleep(POLL_MSEC);
+ continue;
+ }
+ return 0;
+ }
+ return -ETIMEDOUT;
+}
+
+static void hip04_mcpm_powered_up(void)
+{
+ if (!relocation)
+ return;
+ spin_lock(&boot_lock);
+ writel_relaxed(0, relocation);
+ writel_relaxed(0, relocation + 4);
+ writel_relaxed(0, relocation + 8);
+ writel_relaxed(0, relocation + 12);
+ spin_unlock(&boot_lock);
+}
+
+static const struct mcpm_platform_ops hip04_mcpm_ops = {
+ .power_up = hip04_mcpm_power_up,
+ .power_down = hip04_mcpm_power_down,
+ .wait_for_powerdown = hip04_mcpm_wait_for_powerdown,
+ .powered_up = hip04_mcpm_powered_up,
+};
+
+static bool __init hip04_cpu_table_init(void)
+{
+ unsigned int mpidr, cpu, cluster;
+
+ mpidr = read_cpuid_mpidr();
+ cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+ if (cluster >= HIP04_MAX_CLUSTERS ||
+ cpu >= HIP04_MAX_CPUS_PER_CLUSTER) {
+ pr_err("%s: boot CPU is out of bound!\n", __func__);
+ return false;
+ }
+ hip04_set_snoop_filter(cluster, 1);
+ hip04_cpu_table[cluster][cpu] = 1;
+ return true;
+}
+
+static int __init hip04_mcpm_init(void)
+{
+ struct device_node *np, *np_fab;
+ int ret = -ENODEV;
+
+ np = of_find_compatible_node(NULL, NULL, "hisilicon,sysctrl");
+ if (!np)
+ goto err;
+ np_fab = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-fabric");
+ if (!np_fab)
+ goto err;
+
+ if (of_property_read_u32(np, "bootwrapper-phys",
+ &hip04_boot.bootwrapper_phys)) {
+ pr_err("failed to get bootwrapper-phys\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "bootwrapper-size",
+ &hip04_boot.bootwrapper_size)) {
+ pr_err("failed to get bootwrapper-size\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "bootwrapper-magic",
+ &hip04_boot.bootwrapper_magic)) {
+ pr_err("failed to get bootwrapper-magic\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "relocation-entry",
+ &hip04_boot.relocation_entry)) {
+ pr_err("failed to get relocation-entry\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "relocation-size",
+ &hip04_boot.relocation_size)) {
+ pr_err("failed to get relocation-size\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ relocation = ioremap(hip04_boot.relocation_entry,
+ hip04_boot.relocation_size);
+ if (!relocation) {
+ pr_err("failed to map relocation space\n");
+ ret = -ENOMEM;
+ goto err;
+ }
+ sysctrl = of_iomap(np, 0);
+ if (!sysctrl) {
+ pr_err("failed to get sysctrl base\n");
+ ret = -ENOMEM;
+ goto err_sysctrl;
+ }
+ fabric = of_iomap(np_fab, 0);
+ if (!fabric) {
+ pr_err("failed to get fabric base\n");
+ ret = -ENOMEM;
+ goto err_fabric;
+ }
+
+ if (!hip04_cpu_table_init())
+ return -EINVAL;
+ ret = mcpm_platform_register(&hip04_mcpm_ops);
+ if (!ret) {
+ mcpm_sync_init(NULL);
+ pr_info("HiP04 MCPM initialized\n");
+ }
+ return ret;
+err_fabric:
+ iounmap(sysctrl);
+err_sysctrl:
+ iounmap(relocation);
+err:
+ return ret;
+}
+early_initcall(hip04_mcpm_init);
+
+bool __init hip04_smp_init_ops(void)
+{
+ mcpm_smp_set_ops();
+ return true;
+}
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 06/15] ARM: hisi: enable MCPM implementation
2014-05-11 8:06 ` [PATCH v6 06/15] ARM: hisi: enable MCPM implementation Haojian Zhuang
@ 2014-05-13 8:28 ` Dave Martin
2014-05-13 9:46 ` Haojian Zhuang
2014-05-13 11:44 ` [PATCH v7 " Haojian Zhuang
1 sibling, 1 reply; 50+ messages in thread
From: Dave Martin @ 2014-05-13 8:28 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, May 11, 2014 at 04:06:02PM +0800, Haojian Zhuang wrote:
> Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
> framework to manage power on HiP04 SoC.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
> arch/arm/mach-hisi/Makefile | 1 +
> arch/arm/mach-hisi/platmcpm.c | 309 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 310 insertions(+)
> create mode 100644 arch/arm/mach-hisi/platmcpm.c
>
> diff --git a/arch/arm/mach-hisi/Makefile b/arch/arm/mach-hisi/Makefile
> index 2ae1b59..e7a8640 100644
> --- a/arch/arm/mach-hisi/Makefile
> +++ b/arch/arm/mach-hisi/Makefile
> @@ -3,4 +3,5 @@
> #
>
> obj-y += hisilicon.o
> +obj-$(CONFIG_MCPM) += platmcpm.o
> obj-$(CONFIG_SMP) += platsmp.o hotplug.o
> diff --git a/arch/arm/mach-hisi/platmcpm.c b/arch/arm/mach-hisi/platmcpm.c
> new file mode 100644
> index 0000000..6208bc6
> --- /dev/null
> +++ b/arch/arm/mach-hisi/platmcpm.c
> @@ -0,0 +1,309 @@
[...]
> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
> +{
> + unsigned long data, mask;
> +
> + if (!relocation || !sysctrl)
> + return -ENODEV;
> + if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
> + return -EINVAL;
> +
> + spin_lock_irq(&boot_lock);
> + writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
> + writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
> + writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
> + writel_relaxed(0, relocation + 12);
> +
> + if (hip04_cluster_down(cluster)) {
> + data = CLUSTER_DEBUG_RESET_BIT;
> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> + do {
> + mask = CLUSTER_DEBUG_RESET_STATUS;
> + data = readl_relaxed(sysctrl + \
> + SC_CPU_RESET_STATUS(cluster));
> + } while (data & mask);
> + hip04_set_snoop_filter(cluster, 1);
Did you find the answer regarding whether it's safe to enable snoops to
a powered-down CPU?
Does HiP04 use CCI, or some different interconnect? As Nico pointed out,
the above code would definitely not be safe for CCI.
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 06/15] ARM: hisi: enable MCPM implementation
2014-05-13 8:28 ` Dave Martin
@ 2014-05-13 9:46 ` Haojian Zhuang
0 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-13 9:46 UTC (permalink / raw)
To: linux-arm-kernel
On 13 May 2014 16:28, Dave Martin <Dave.Martin@arm.com> wrote:
> On Sun, May 11, 2014 at 04:06:02PM +0800, Haojian Zhuang wrote:
>> Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
>> framework to manage power on HiP04 SoC.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>> arch/arm/mach-hisi/Makefile | 1 +
>> arch/arm/mach-hisi/platmcpm.c | 309 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 310 insertions(+)
>> create mode 100644 arch/arm/mach-hisi/platmcpm.c
>>
>> diff --git a/arch/arm/mach-hisi/Makefile b/arch/arm/mach-hisi/Makefile
>> index 2ae1b59..e7a8640 100644
>> --- a/arch/arm/mach-hisi/Makefile
>> +++ b/arch/arm/mach-hisi/Makefile
>> @@ -3,4 +3,5 @@
>> #
>>
>> obj-y += hisilicon.o
>> +obj-$(CONFIG_MCPM) += platmcpm.o
>> obj-$(CONFIG_SMP) += platsmp.o hotplug.o
>> diff --git a/arch/arm/mach-hisi/platmcpm.c b/arch/arm/mach-hisi/platmcpm.c
>> new file mode 100644
>> index 0000000..6208bc6
>> --- /dev/null
>> +++ b/arch/arm/mach-hisi/platmcpm.c
>> @@ -0,0 +1,309 @@
>
> [...]
>
>> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
>> +{
>> + unsigned long data, mask;
>> +
>> + if (!relocation || !sysctrl)
>> + return -ENODEV;
>> + if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
>> + return -EINVAL;
>> +
>> + spin_lock_irq(&boot_lock);
>> + writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
>> + writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
>> + writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
>> + writel_relaxed(0, relocation + 12);
>> +
>> + if (hip04_cluster_down(cluster)) {
>> + data = CLUSTER_DEBUG_RESET_BIT;
>> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> + do {
>> + mask = CLUSTER_DEBUG_RESET_STATUS;
>> + data = readl_relaxed(sysctrl + \
>> + SC_CPU_RESET_STATUS(cluster));
>> + } while (data & mask);
>> + hip04_set_snoop_filter(cluster, 1);
>
> Did you find the answer regarding whether it's safe to enable snoops to
> a powered-down CPU?
>
> Does HiP04 use CCI, or some different interconnect? As Nico pointed out,
> the above code would definitely not be safe for CCI.
>
First, it's safe.
In the second, HiP04 doesn't use CCI. They designed their own fabric instead.
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v7 06/15] ARM: hisi: enable MCPM implementation
2014-05-11 8:06 ` [PATCH v6 06/15] ARM: hisi: enable MCPM implementation Haojian Zhuang
2014-05-13 8:28 ` Dave Martin
@ 2014-05-13 11:44 ` Haojian Zhuang
2014-05-13 19:43 ` Nicolas Pitre
1 sibling, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-13 11:44 UTC (permalink / raw)
To: linux-arm-kernel
Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
framework to manage power on HiP04 SoC.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/mach-hisi/Makefile | 1 +
arch/arm/mach-hisi/platmcpm.c | 304 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 305 insertions(+)
create mode 100644 arch/arm/mach-hisi/platmcpm.c
diff --git a/arch/arm/mach-hisi/Makefile b/arch/arm/mach-hisi/Makefile
index 2ae1b59..e7a8640 100644
--- a/arch/arm/mach-hisi/Makefile
+++ b/arch/arm/mach-hisi/Makefile
@@ -3,4 +3,5 @@
#
obj-y += hisilicon.o
+obj-$(CONFIG_MCPM) += platmcpm.o
obj-$(CONFIG_SMP) += platsmp.o hotplug.o
diff --git a/arch/arm/mach-hisi/platmcpm.c b/arch/arm/mach-hisi/platmcpm.c
new file mode 100644
index 0000000..3b42977
--- /dev/null
+++ b/arch/arm/mach-hisi/platmcpm.c
@@ -0,0 +1,304 @@
+/*
+ * Copyright (c) 2013-2014 Linaro Ltd.
+ * Copyright (c) 2013-2014 Hisilicon Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+
+#include <asm/cputype.h>
+#include <asm/cp15.h>
+#include <asm/mcpm.h>
+
+#include "core.h"
+
+/* bits definition in SC_CPU_RESET_REQ[x]/SC_CPU_RESET_DREQ[x]
+ * 1 -- unreset; 0 -- reset
+ */
+#define CORE_RESET_BIT(x) (1 << x)
+#define NEON_RESET_BIT(x) (1 << (x + 4))
+#define CORE_DEBUG_RESET_BIT(x) (1 << (x + 9))
+#define CLUSTER_L2_RESET_BIT (1 << 8)
+#define CLUSTER_DEBUG_RESET_BIT (1 << 13)
+
+/*
+ * bits definition in SC_CPU_RESET_STATUS[x]
+ * 1 -- reset status; 0 -- unreset status
+ */
+#define CORE_RESET_STATUS(x) (1 << x)
+#define NEON_RESET_STATUS(x) (1 << (x + 4))
+#define CORE_DEBUG_RESET_STATUS(x) (1 << (x + 9))
+#define CLUSTER_L2_RESET_STATUS (1 << 8)
+#define CLUSTER_DEBUG_RESET_STATUS (1 << 13)
+#define CORE_WFI_STATUS(x) (1 << (x + 16))
+#define CORE_WFE_STATUS(x) (1 << (x + 20))
+#define CORE_DEBUG_ACK(x) (1 << (x + 24))
+
+#define SC_CPU_RESET_REQ(x) (0x520 + (x << 3)) /* reset */
+#define SC_CPU_RESET_DREQ(x) (0x524 + (x << 3)) /* unreset */
+#define SC_CPU_RESET_STATUS(x) (0x1520 + (x << 3))
+
+#define FAB_SF_MODE 0x0c
+#define FAB_SF_INVLD 0x10
+
+/* bits definition in FB_SF_INVLD */
+#define FB_SF_INVLD_START (1 << 8)
+
+#define HIP04_MAX_CLUSTERS 4
+#define HIP04_MAX_CPUS_PER_CLUSTER 4
+
+#define POLL_MSEC 10
+#define TIMEOUT_MSEC 1000
+
+struct hip04_secondary_cpu_data {
+ u32 bootwrapper_phys;
+ u32 bootwrapper_size;
+ u32 bootwrapper_magic;
+ u32 relocation_entry;
+ u32 relocation_size;
+};
+
+static void __iomem *relocation, *sysctrl, *fabric;
+static int hip04_cpu_table[HIP04_MAX_CLUSTERS][HIP04_MAX_CPUS_PER_CLUSTER];
+static DEFINE_SPINLOCK(boot_lock);
+static struct hip04_secondary_cpu_data hip04_boot;
+
+static bool hip04_cluster_down(unsigned int cluster)
+{
+ int i;
+
+ for (i = 0; i < HIP04_MAX_CPUS_PER_CLUSTER; i++)
+ if (hip04_cpu_table[cluster][i])
+ return false;
+ return true;
+}
+
+static void hip04_set_snoop_filter(unsigned int cluster, unsigned int on)
+{
+ unsigned long data;
+
+ if (!fabric)
+ return;
+ data = readl_relaxed(fabric + FAB_SF_MODE);
+ if (on)
+ data |= 1 << cluster;
+ else
+ data &= ~(1 << cluster);
+ writel_relaxed(data, fabric + FAB_SF_MODE);
+ while (1) {
+ if (data == readl_relaxed(fabric + FAB_SF_MODE))
+ break;
+ }
+}
+
+static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
+{
+ unsigned long data, mask;
+
+ if (!relocation || !sysctrl)
+ return -ENODEV;
+ if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
+ return -EINVAL;
+
+ spin_lock_irq(&boot_lock);
+ writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
+ writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
+ writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
+ writel_relaxed(0, relocation + 12);
+
+ if (hip04_cluster_down(cluster)) {
+ data = CLUSTER_DEBUG_RESET_BIT;
+ writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
+ do {
+ mask = CLUSTER_DEBUG_RESET_STATUS;
+ data = readl_relaxed(sysctrl + \
+ SC_CPU_RESET_STATUS(cluster));
+ } while (data & mask);
+ hip04_set_snoop_filter(cluster, 1);
+ }
+
+ hip04_cpu_table[cluster][cpu]++;
+
+ data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
+ CORE_DEBUG_RESET_BIT(cpu);
+ writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
+ spin_unlock_irq(&boot_lock);
+ msleep(POLL_MSEC);
+
+ return 0;
+}
+
+static void hip04_mcpm_power_down(void)
+{
+ unsigned int mpidr, cpu, cluster, data = 0;
+ bool skip_reset = false;
+
+ mpidr = read_cpuid_mpidr();
+ cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+ __mcpm_cpu_going_down(cpu, cluster);
+
+ spin_lock(&boot_lock);
+ BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
+ hip04_cpu_table[cluster][cpu]--;
+ if (hip04_cpu_table[cluster][cpu] == 1) {
+ /* A power_up request went ahead of us. */
+ skip_reset = true;
+ } else if (hip04_cpu_table[cluster][cpu] > 1) {
+ pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
+ BUG();
+ }
+
+ spin_unlock(&boot_lock);
+
+ v7_exit_coherency_flush(louis);
+
+ __mcpm_cpu_down(cpu, cluster);
+
+ if (!skip_reset) {
+ data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
+ CORE_DEBUG_RESET_BIT(cpu);
+ writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
+ }
+}
+
+static int hip04_mcpm_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
+{
+ unsigned int data, tries;
+
+ BUG_ON(cluster >= HIP04_MAX_CLUSTERS ||
+ cpu >= HIP04_MAX_CPUS_PER_CLUSTER);
+
+ for (tries = 0; tries < TIMEOUT_MSEC / POLL_MSEC; tries++) {
+ data = readl_relaxed(sysctrl + SC_CPU_RESET_STATUS(cluster));
+ if (!(data & CORE_RESET_STATUS(cpu))) {
+ msleep(POLL_MSEC);
+ continue;
+ }
+ return 0;
+ }
+ return -ETIMEDOUT;
+}
+
+static void hip04_mcpm_powered_up(void)
+{
+ if (!relocation)
+ return;
+ spin_lock(&boot_lock);
+ writel_relaxed(0, relocation);
+ writel_relaxed(0, relocation + 4);
+ writel_relaxed(0, relocation + 8);
+ writel_relaxed(0, relocation + 12);
+ spin_unlock(&boot_lock);
+}
+
+static const struct mcpm_platform_ops hip04_mcpm_ops = {
+ .power_up = hip04_mcpm_power_up,
+ .power_down = hip04_mcpm_power_down,
+ .wait_for_powerdown = hip04_mcpm_wait_for_powerdown,
+ .powered_up = hip04_mcpm_powered_up,
+};
+
+static bool __init hip04_cpu_table_init(void)
+{
+ unsigned int mpidr, cpu, cluster;
+
+ mpidr = read_cpuid_mpidr();
+ cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+ cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+ if (cluster >= HIP04_MAX_CLUSTERS ||
+ cpu >= HIP04_MAX_CPUS_PER_CLUSTER) {
+ pr_err("%s: boot CPU is out of bound!\n", __func__);
+ return false;
+ }
+ hip04_set_snoop_filter(cluster, 1);
+ hip04_cpu_table[cluster][cpu] = 1;
+ return true;
+}
+
+static int __init hip04_mcpm_init(void)
+{
+ struct device_node *np, *np_fab;
+ int ret = -ENODEV;
+
+ np = of_find_compatible_node(NULL, NULL, "hisilicon,sysctrl");
+ if (!np)
+ goto err;
+ np_fab = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-fabric");
+ if (!np_fab)
+ goto err;
+
+ if (of_property_read_u32(np, "bootwrapper-phys",
+ &hip04_boot.bootwrapper_phys)) {
+ pr_err("failed to get bootwrapper-phys\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "bootwrapper-size",
+ &hip04_boot.bootwrapper_size)) {
+ pr_err("failed to get bootwrapper-size\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "bootwrapper-magic",
+ &hip04_boot.bootwrapper_magic)) {
+ pr_err("failed to get bootwrapper-magic\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "relocation-entry",
+ &hip04_boot.relocation_entry)) {
+ pr_err("failed to get relocation-entry\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ if (of_property_read_u32(np, "relocation-size",
+ &hip04_boot.relocation_size)) {
+ pr_err("failed to get relocation-size\n");
+ ret = -EINVAL;
+ goto err;
+ }
+
+ relocation = ioremap(hip04_boot.relocation_entry,
+ hip04_boot.relocation_size);
+ if (!relocation) {
+ pr_err("failed to map relocation space\n");
+ ret = -ENOMEM;
+ goto err;
+ }
+ sysctrl = of_iomap(np, 0);
+ if (!sysctrl) {
+ pr_err("failed to get sysctrl base\n");
+ ret = -ENOMEM;
+ goto err_sysctrl;
+ }
+ fabric = of_iomap(np_fab, 0);
+ if (!fabric) {
+ pr_err("failed to get fabric base\n");
+ ret = -ENOMEM;
+ goto err_fabric;
+ }
+
+ if (!hip04_cpu_table_init())
+ return -EINVAL;
+ ret = mcpm_platform_register(&hip04_mcpm_ops);
+ if (!ret) {
+ mcpm_sync_init(NULL);
+ pr_info("HiP04 MCPM initialized\n");
+ }
+ mcpm_smp_set_ops();
+ return ret;
+err_fabric:
+ iounmap(sysctrl);
+err_sysctrl:
+ iounmap(relocation);
+err:
+ return ret;
+}
+early_initcall(hip04_mcpm_init);
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v7 06/15] ARM: hisi: enable MCPM implementation
2014-05-13 11:44 ` [PATCH v7 " Haojian Zhuang
@ 2014-05-13 19:43 ` Nicolas Pitre
2014-05-15 6:23 ` Haojian Zhuang
0 siblings, 1 reply; 50+ messages in thread
From: Nicolas Pitre @ 2014-05-13 19:43 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 13 May 2014, Haojian Zhuang wrote:
> Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
> framework to manage power on HiP04 SoC.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Some more comments...
[...]
> +static void hip04_set_snoop_filter(unsigned int cluster, unsigned int on)
> +{
> + unsigned long data;
> +
> + if (!fabric)
> + return;
How could this validly be NULL?
> + data = readl_relaxed(fabric + FAB_SF_MODE);
> + if (on)
> + data |= 1 << cluster;
> + else
> + data &= ~(1 << cluster);
> + writel_relaxed(data, fabric + FAB_SF_MODE);
> + while (1) {
> + if (data == readl_relaxed(fabric + FAB_SF_MODE))
> + break;
> + }
> +}
The above could be easily coded in assembly for the power_up_setup
callback thusly:
hip04_power_up_setup:
cmp r0, #0 @ check affinity level
bxeq lr @ nothing to do at CPU level
mrc p15, 0, r0, c0, c0, 5 @ get MPIDR
ubfx r0, r0, #8, #8 @ extract cluster number
adr r1, .LC0
ldmia r1, {r2, r3}
sub r2, r2, r1 @ virt_addr - phys_addr
ldr r1, [r2, r3] @ get fabric_phys_addr
mov r2, #1
ldr r3, [r1, #FAB_SF_MODE] @ read "data"
orr r3, r3, r2, lsl r0 @ set cluster bit
str r3, [r1, #FAB_SF_MODE] @ write it back
1: ldr r2, [r1, #FAB_SF_MODE] @ read register content
cmp r2, r3 @ make sure it matches
bne 1b @ otherwise retry
bx lr
:LC0: .word .
.word fabric_phys_addr - .LC0
That should be it.
> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
> +{
> + unsigned long data, mask;
> +
> + if (!relocation || !sysctrl)
> + return -ENODEV;
> + if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
> + return -EINVAL;
> +
> + spin_lock_irq(&boot_lock);
> + writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
> + writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
> + writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
> + writel_relaxed(0, relocation + 12);
Shouldn't you do the above writes only when
hip04_cpu_table[cluster][cpu] is zero? Please see the comment in
mcpm_cpu_power_down() about unordered calls.
> + if (hip04_cluster_down(cluster)) {
> + data = CLUSTER_DEBUG_RESET_BIT;
> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> + do {
> + mask = CLUSTER_DEBUG_RESET_STATUS;
> + data = readl_relaxed(sysctrl + \
> + SC_CPU_RESET_STATUS(cluster));
> + } while (data & mask);
> + hip04_set_snoop_filter(cluster, 1);
> + }
> +
> + hip04_cpu_table[cluster][cpu]++;
> +
> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
> + CORE_DEBUG_RESET_BIT(cpu);
> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> + spin_unlock_irq(&boot_lock);
> + msleep(POLL_MSEC);
> +
> + return 0;
> +}
> +
> +static void hip04_mcpm_power_down(void)
> +{
> + unsigned int mpidr, cpu, cluster, data = 0;
> + bool skip_reset = false;
> +
> + mpidr = read_cpuid_mpidr();
> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> + __mcpm_cpu_going_down(cpu, cluster);
> +
> + spin_lock(&boot_lock);
> + BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
> + hip04_cpu_table[cluster][cpu]--;
> + if (hip04_cpu_table[cluster][cpu] == 1) {
> + /* A power_up request went ahead of us. */
> + skip_reset = true;
> + } else if (hip04_cpu_table[cluster][cpu] > 1) {
> + pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
This message is misleading. If execution gets here, that means
mcpm_cpu_power_up() was called more than twice in a row for the same CPU
which should never happen.
> + BUG();
> + }
> +
> + spin_unlock(&boot_lock);
> +
> + v7_exit_coherency_flush(louis);
> +
> + __mcpm_cpu_down(cpu, cluster);
> +
> + if (!skip_reset) {
> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
> + CORE_DEBUG_RESET_BIT(cpu);
> + writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
You should not perform this outside the lock protected region as this
could race with hip04_mcpm_power_up(). Instead, this should be done
above when hip04_cpu_table[cluster][cpu] == 0 after being decremented.
> +}
> +
> +static int hip04_mcpm_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
> +{
> + unsigned int data, tries;
> +
> + BUG_ON(cluster >= HIP04_MAX_CLUSTERS ||
> + cpu >= HIP04_MAX_CPUS_PER_CLUSTER);
> +
> + for (tries = 0; tries < TIMEOUT_MSEC / POLL_MSEC; tries++) {
> + data = readl_relaxed(sysctrl + SC_CPU_RESET_STATUS(cluster));
> + if (!(data & CORE_RESET_STATUS(cpu))) {
> + msleep(POLL_MSEC);
> + continue;
> + }
> + return 0;
> + }
> + return -ETIMEDOUT;
> +}
> +
> +static void hip04_mcpm_powered_up(void)
> +{
> + if (!relocation)
> + return;
> + spin_lock(&boot_lock);
> + writel_relaxed(0, relocation);
> + writel_relaxed(0, relocation + 4);
> + writel_relaxed(0, relocation + 8);
> + writel_relaxed(0, relocation + 12);
> + spin_unlock(&boot_lock);
> +}
> +
> +static const struct mcpm_platform_ops hip04_mcpm_ops = {
> + .power_up = hip04_mcpm_power_up,
> + .power_down = hip04_mcpm_power_down,
> + .wait_for_powerdown = hip04_mcpm_wait_for_powerdown,
> + .powered_up = hip04_mcpm_powered_up,
> +};
> +
> +static bool __init hip04_cpu_table_init(void)
> +{
> + unsigned int mpidr, cpu, cluster;
> +
> + mpidr = read_cpuid_mpidr();
> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> + if (cluster >= HIP04_MAX_CLUSTERS ||
> + cpu >= HIP04_MAX_CPUS_PER_CLUSTER) {
> + pr_err("%s: boot CPU is out of bound!\n", __func__);
> + return false;
> + }
> + hip04_set_snoop_filter(cluster, 1);
> + hip04_cpu_table[cluster][cpu] = 1;
> + return true;
> +}
> +
> +static int __init hip04_mcpm_init(void)
> +{
> + struct device_node *np, *np_fab;
> + int ret = -ENODEV;
> +
> + np = of_find_compatible_node(NULL, NULL, "hisilicon,sysctrl");
> + if (!np)
> + goto err;
> + np_fab = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-fabric");
> + if (!np_fab)
> + goto err;
> +
> + if (of_property_read_u32(np, "bootwrapper-phys",
> + &hip04_boot.bootwrapper_phys)) {
> + pr_err("failed to get bootwrapper-phys\n");
> + ret = -EINVAL;
> + goto err;
> + }
> + if (of_property_read_u32(np, "bootwrapper-size",
> + &hip04_boot.bootwrapper_size)) {
> + pr_err("failed to get bootwrapper-size\n");
> + ret = -EINVAL;
> + goto err;
> + }
> + if (of_property_read_u32(np, "bootwrapper-magic",
> + &hip04_boot.bootwrapper_magic)) {
> + pr_err("failed to get bootwrapper-magic\n");
> + ret = -EINVAL;
> + goto err;
> + }
> + if (of_property_read_u32(np, "relocation-entry",
> + &hip04_boot.relocation_entry)) {
> + pr_err("failed to get relocation-entry\n");
> + ret = -EINVAL;
> + goto err;
> + }
> + if (of_property_read_u32(np, "relocation-size",
> + &hip04_boot.relocation_size)) {
> + pr_err("failed to get relocation-size\n");
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + relocation = ioremap(hip04_boot.relocation_entry,
> + hip04_boot.relocation_size);
> + if (!relocation) {
> + pr_err("failed to map relocation space\n");
> + ret = -ENOMEM;
> + goto err;
> + }
> + sysctrl = of_iomap(np, 0);
> + if (!sysctrl) {
> + pr_err("failed to get sysctrl base\n");
> + ret = -ENOMEM;
> + goto err_sysctrl;
> + }
> + fabric = of_iomap(np_fab, 0);
> + if (!fabric) {
> + pr_err("failed to get fabric base\n");
> + ret = -ENOMEM;
> + goto err_fabric;
> + }
> +
> + if (!hip04_cpu_table_init())
> + return -EINVAL;
> + ret = mcpm_platform_register(&hip04_mcpm_ops);
> + if (!ret) {
> + mcpm_sync_init(NULL);
> + pr_info("HiP04 MCPM initialized\n");
> + }
> + mcpm_smp_set_ops();
> + return ret;
> +err_fabric:
> + iounmap(sysctrl);
> +err_sysctrl:
> + iounmap(relocation);
> +err:
> + return ret;
> +}
> +early_initcall(hip04_mcpm_init);
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v7 06/15] ARM: hisi: enable MCPM implementation
2014-05-13 19:43 ` Nicolas Pitre
@ 2014-05-15 6:23 ` Haojian Zhuang
2014-05-15 20:01 ` Nicolas Pitre
0 siblings, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-15 6:23 UTC (permalink / raw)
To: linux-arm-kernel
On 14 May 2014 03:43, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Tue, 13 May 2014, Haojian Zhuang wrote:
>
>> Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
>> framework to manage power on HiP04 SoC.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>
> Some more comments...
>
> [...]
>> +static void hip04_set_snoop_filter(unsigned int cluster, unsigned int on)
>> +{
>> + unsigned long data;
>> +
>> + if (!fabric)
>> + return;
>
> How could this validly be NULL?
>
OK. I'll make it report BUG.
>> + data = readl_relaxed(fabric + FAB_SF_MODE);
>> + if (on)
>> + data |= 1 << cluster;
>> + else
>> + data &= ~(1 << cluster);
>> + writel_relaxed(data, fabric + FAB_SF_MODE);
>> + while (1) {
>> + if (data == readl_relaxed(fabric + FAB_SF_MODE))
>> + break;
>> + }
>> +}
>
> The above could be easily coded in assembly for the power_up_setup
> callback thusly:
>
> hip04_power_up_setup:
>
> cmp r0, #0 @ check affinity level
> bxeq lr @ nothing to do at CPU level
>
> mrc p15, 0, r0, c0, c0, 5 @ get MPIDR
> ubfx r0, r0, #8, #8 @ extract cluster number
>
> adr r1, .LC0
> ldmia r1, {r2, r3}
> sub r2, r2, r1 @ virt_addr - phys_addr
> ldr r1, [r2, r3] @ get fabric_phys_addr
> mov r2, #1
> ldr r3, [r1, #FAB_SF_MODE] @ read "data"
> orr r3, r3, r2, lsl r0 @ set cluster bit
> str r3, [r1, #FAB_SF_MODE] @ write it back
>
> 1: ldr r2, [r1, #FAB_SF_MODE] @ read register content
> cmp r2, r3 @ make sure it matches
> bne 1b @ otherwise retry
>
> bx lr
>
> :LC0: .word .
> .word fabric_phys_addr - .LC0
>
> That should be it.
>
No. These code should be executed before new CPU on. If I transfer
them to assembler code, it means that code will be executed after
new CPU on.
Then it results me failing to make new CPU online.
>> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
>> +{
>> + unsigned long data, mask;
>> +
>> + if (!relocation || !sysctrl)
>> + return -ENODEV;
>> + if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
>> + return -EINVAL;
>> +
>> + spin_lock_irq(&boot_lock);
>> + writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
>> + writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
>> + writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
>> + writel_relaxed(0, relocation + 12);
>
> Shouldn't you do the above writes only when
> hip04_cpu_table[cluster][cpu] is zero? Please see the comment in
> mcpm_cpu_power_down() about unordered calls.
>
OK. I can add the check.
>> + if (hip04_cluster_down(cluster)) {
>> + data = CLUSTER_DEBUG_RESET_BIT;
>> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> + do {
>> + mask = CLUSTER_DEBUG_RESET_STATUS;
>> + data = readl_relaxed(sysctrl + \
>> + SC_CPU_RESET_STATUS(cluster));
>> + } while (data & mask);
>> + hip04_set_snoop_filter(cluster, 1);
>> + }
>> +
>> + hip04_cpu_table[cluster][cpu]++;
>> +
>> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> + CORE_DEBUG_RESET_BIT(cpu);
>> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> + spin_unlock_irq(&boot_lock);
>> + msleep(POLL_MSEC);
>> +
>> + return 0;
>> +}
>> +
>> +static void hip04_mcpm_power_down(void)
>> +{
>> + unsigned int mpidr, cpu, cluster, data = 0;
>> + bool skip_reset = false;
>> +
>> + mpidr = read_cpuid_mpidr();
>> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>> +
>> + __mcpm_cpu_going_down(cpu, cluster);
>> +
>> + spin_lock(&boot_lock);
>> + BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
>> + hip04_cpu_table[cluster][cpu]--;
>> + if (hip04_cpu_table[cluster][cpu] == 1) {
>> + /* A power_up request went ahead of us. */
>> + skip_reset = true;
>> + } else if (hip04_cpu_table[cluster][cpu] > 1) {
>> + pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
>
> This message is misleading. If execution gets here, that means
> mcpm_cpu_power_up() was called more than twice in a row for the same CPU
> which should never happen.
>
OK. I'll replace the comments.
>> + BUG();
>> + }
>> +
>> + spin_unlock(&boot_lock);
>> +
>> + v7_exit_coherency_flush(louis);
>> +
>> + __mcpm_cpu_down(cpu, cluster);
>> +
>> + if (!skip_reset) {
>> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> + CORE_DEBUG_RESET_BIT(cpu);
>> + writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
>
> You should not perform this outside the lock protected region as this
> could race with hip04_mcpm_power_up(). Instead, this should be done
> above when hip04_cpu_table[cluster][cpu] == 0 after being decremented.
>
No. power_down() is executed on the specified CPU. If spin_unlock() is
placed after reset operation, it means that there's no chance to
execute
the spin_unlock(). Because CPU is already in reset mode at this time.
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v7 06/15] ARM: hisi: enable MCPM implementation
2014-05-15 6:23 ` Haojian Zhuang
@ 2014-05-15 20:01 ` Nicolas Pitre
2014-05-20 4:43 ` Haojian Zhuang
0 siblings, 1 reply; 50+ messages in thread
From: Nicolas Pitre @ 2014-05-15 20:01 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 15 May 2014, Haojian Zhuang wrote:
> On 14 May 2014 03:43, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Tue, 13 May 2014, Haojian Zhuang wrote:
> >
> >> + data = readl_relaxed(fabric + FAB_SF_MODE);
> >> + if (on)
> >> + data |= 1 << cluster;
> >> + else
> >> + data &= ~(1 << cluster);
> >> + writel_relaxed(data, fabric + FAB_SF_MODE);
> >> + while (1) {
> >> + if (data == readl_relaxed(fabric + FAB_SF_MODE))
> >> + break;
> >> + }
> >> +}
> >
> > The above could be easily coded in assembly for the power_up_setup
> > callback thusly:
> >
> > hip04_power_up_setup:
> >
> > cmp r0, #0 @ check affinity level
> > bxeq lr @ nothing to do at CPU level
> >
> > mrc p15, 0, r0, c0, c0, 5 @ get MPIDR
> > ubfx r0, r0, #8, #8 @ extract cluster number
> >
> > adr r1, .LC0
> > ldmia r1, {r2, r3}
> > sub r2, r2, r1 @ virt_addr - phys_addr
> > ldr r1, [r2, r3] @ get fabric_phys_addr
> > mov r2, #1
> > ldr r3, [r1, #FAB_SF_MODE] @ read "data"
> > orr r3, r3, r2, lsl r0 @ set cluster bit
> > str r3, [r1, #FAB_SF_MODE] @ write it back
> >
> > 1: ldr r2, [r1, #FAB_SF_MODE] @ read register content
> > cmp r2, r3 @ make sure it matches
> > bne 1b @ otherwise retry
> >
> > bx lr
> >
> > :LC0: .word .
> > .word fabric_phys_addr - .LC0
> >
> > That should be it.
> >
>
> No. These code should be executed before new CPU on. If I transfer
> them to assembler code, it means that code will be executed after
> new CPU on.
Exact.
> Then it results me failing to make new CPU online.
The assembly code could be wrong as well. Are you sure this is not the
actual reason?
Is there some documentation for this stuff?
> >> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
> >> +{
> >> + unsigned long data, mask;
> >> +
> >> + if (!relocation || !sysctrl)
> >> + return -ENODEV;
> >> + if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
> >> + return -EINVAL;
> >> +
> >> + spin_lock_irq(&boot_lock);
> >> + writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
> >> + writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
> >> + writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
> >> + writel_relaxed(0, relocation + 12);
> >
> > Shouldn't you do the above writes only when
> > hip04_cpu_table[cluster][cpu] is zero? Please see the comment in
> > mcpm_cpu_power_down() about unordered calls.
> >
> OK. I can add the check.
>
> >> + if (hip04_cluster_down(cluster)) {
> >> + data = CLUSTER_DEBUG_RESET_BIT;
> >> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> >> + do {
> >> + mask = CLUSTER_DEBUG_RESET_STATUS;
> >> + data = readl_relaxed(sysctrl + \
> >> + SC_CPU_RESET_STATUS(cluster));
> >> + } while (data & mask);
> >> + hip04_set_snoop_filter(cluster, 1);
> >> + }
> >> +
> >> + hip04_cpu_table[cluster][cpu]++;
> >> +
> >> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
> >> + CORE_DEBUG_RESET_BIT(cpu);
> >> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
> >> + spin_unlock_irq(&boot_lock);
> >> + msleep(POLL_MSEC);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void hip04_mcpm_power_down(void)
> >> +{
> >> + unsigned int mpidr, cpu, cluster, data = 0;
> >> + bool skip_reset = false;
> >> +
> >> + mpidr = read_cpuid_mpidr();
> >> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> >> + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> >> +
> >> + __mcpm_cpu_going_down(cpu, cluster);
> >> +
> >> + spin_lock(&boot_lock);
> >> + BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
> >> + hip04_cpu_table[cluster][cpu]--;
> >> + if (hip04_cpu_table[cluster][cpu] == 1) {
> >> + /* A power_up request went ahead of us. */
> >> + skip_reset = true;
> >> + } else if (hip04_cpu_table[cluster][cpu] > 1) {
> >> + pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
> >
> > This message is misleading. If execution gets here, that means
> > mcpm_cpu_power_up() was called more than twice in a row for the same CPU
> > which should never happen.
> >
> OK. I'll replace the comments.
>
> >> + BUG();
> >> + }
> >> +
> >> + spin_unlock(&boot_lock);
> >> +
> >> + v7_exit_coherency_flush(louis);
> >> +
> >> + __mcpm_cpu_down(cpu, cluster);
> >> +
> >> + if (!skip_reset) {
> >> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
> >> + CORE_DEBUG_RESET_BIT(cpu);
> >> + writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
> >
> > You should not perform this outside the lock protected region as this
> > could race with hip04_mcpm_power_up(). Instead, this should be done
> > above when hip04_cpu_table[cluster][cpu] == 0 after being decremented.
> >
>
> No. power_down() is executed on the specified CPU. If spin_unlock() is
> placed after reset operation, it means that there's no chance to
> execute the spin_unlock(). Because CPU is already in reset mode at
> this time.
Normally, reset is effective only when WFI is later executed. Are you
sure this is not the case on hip04 as well?
Nicolas
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v7 06/15] ARM: hisi: enable MCPM implementation
2014-05-15 20:01 ` Nicolas Pitre
@ 2014-05-20 4:43 ` Haojian Zhuang
2014-05-21 10:02 ` Dave Martin
2014-05-21 13:52 ` Nicolas Pitre
0 siblings, 2 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-20 4:43 UTC (permalink / raw)
To: linux-arm-kernel
On 16 May 2014 04:01, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Thu, 15 May 2014, Haojian Zhuang wrote:
>
>> On 14 May 2014 03:43, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>> > On Tue, 13 May 2014, Haojian Zhuang wrote:
>> >
>> >> + data = readl_relaxed(fabric + FAB_SF_MODE);
>> >> + if (on)
>> >> + data |= 1 << cluster;
>> >> + else
>> >> + data &= ~(1 << cluster);
>> >> + writel_relaxed(data, fabric + FAB_SF_MODE);
>> >> + while (1) {
>> >> + if (data == readl_relaxed(fabric + FAB_SF_MODE))
>> >> + break;
>> >> + }
>> >> +}
>> >
>> > The above could be easily coded in assembly for the power_up_setup
>> > callback thusly:
>> >
>> > hip04_power_up_setup:
>> >
>> > cmp r0, #0 @ check affinity level
>> > bxeq lr @ nothing to do at CPU level
>> >
>> > mrc p15, 0, r0, c0, c0, 5 @ get MPIDR
>> > ubfx r0, r0, #8, #8 @ extract cluster number
>> >
>> > adr r1, .LC0
>> > ldmia r1, {r2, r3}
>> > sub r2, r2, r1 @ virt_addr - phys_addr
>> > ldr r1, [r2, r3] @ get fabric_phys_addr
>> > mov r2, #1
>> > ldr r3, [r1, #FAB_SF_MODE] @ read "data"
>> > orr r3, r3, r2, lsl r0 @ set cluster bit
>> > str r3, [r1, #FAB_SF_MODE] @ write it back
>> >
>> > 1: ldr r2, [r1, #FAB_SF_MODE] @ read register content
>> > cmp r2, r3 @ make sure it matches
>> > bne 1b @ otherwise retry
>> >
>> > bx lr
>> >
>> > :LC0: .word .
>> > .word fabric_phys_addr - .LC0
>> >
>> > That should be it.
>> >
>>
>> No. These code should be executed before new CPU on. If I transfer
>> them to assembler code, it means that code will be executed after
>> new CPU on.
>
> Exact.
>
>> Then it results me failing to make new CPU online.
>
> The assembly code could be wrong as well. Are you sure this is not the
> actual reason?
>
> Is there some documentation for this stuff?
>
There's no problem in assembly code. I even rewrite your assembly code.
If I keep my c code with assembly code, new CPU could be online right.
If I only use assembly code, I only get the kernel panic. So it's not
caused by assembly code. It's caused by executing code after new CPU
on.
There's no documentation on this. They didn't prepare well on documents.
I think they'll improve it in the future.
>> >> +static int hip04_mcpm_power_up(unsigned int cpu, unsigned int cluster)
>> >> +{
>> >> + unsigned long data, mask;
>> >> +
>> >> + if (!relocation || !sysctrl)
>> >> + return -ENODEV;
>> >> + if (cluster >= HIP04_MAX_CLUSTERS || cpu >= HIP04_MAX_CPUS_PER_CLUSTER)
>> >> + return -EINVAL;
>> >> +
>> >> + spin_lock_irq(&boot_lock);
>> >> + writel_relaxed(hip04_boot.bootwrapper_phys, relocation);
>> >> + writel_relaxed(hip04_boot.bootwrapper_magic, relocation + 4);
>> >> + writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
>> >> + writel_relaxed(0, relocation + 12);
>> >
>> > Shouldn't you do the above writes only when
>> > hip04_cpu_table[cluster][cpu] is zero? Please see the comment in
>> > mcpm_cpu_power_down() about unordered calls.
>> >
>> OK. I can add the check.
>>
>> >> + if (hip04_cluster_down(cluster)) {
>> >> + data = CLUSTER_DEBUG_RESET_BIT;
>> >> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> >> + do {
>> >> + mask = CLUSTER_DEBUG_RESET_STATUS;
>> >> + data = readl_relaxed(sysctrl + \
>> >> + SC_CPU_RESET_STATUS(cluster));
>> >> + } while (data & mask);
>> >> + hip04_set_snoop_filter(cluster, 1);
>> >> + }
>> >> +
>> >> + hip04_cpu_table[cluster][cpu]++;
>> >> +
>> >> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> >> + CORE_DEBUG_RESET_BIT(cpu);
>> >> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> >> + spin_unlock_irq(&boot_lock);
>> >> + msleep(POLL_MSEC);
>> >> +
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static void hip04_mcpm_power_down(void)
>> >> +{
>> >> + unsigned int mpidr, cpu, cluster, data = 0;
>> >> + bool skip_reset = false;
>> >> +
>> >> + mpidr = read_cpuid_mpidr();
>> >> + cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> >> + cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>> >> +
>> >> + __mcpm_cpu_going_down(cpu, cluster);
>> >> +
>> >> + spin_lock(&boot_lock);
>> >> + BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
>> >> + hip04_cpu_table[cluster][cpu]--;
>> >> + if (hip04_cpu_table[cluster][cpu] == 1) {
>> >> + /* A power_up request went ahead of us. */
>> >> + skip_reset = true;
>> >> + } else if (hip04_cpu_table[cluster][cpu] > 1) {
>> >> + pr_err("Cluster %d CPU%d is still running\n", cluster, cpu);
>> >
>> > This message is misleading. If execution gets here, that means
>> > mcpm_cpu_power_up() was called more than twice in a row for the same CPU
>> > which should never happen.
>> >
>> OK. I'll replace the comments.
>>
>> >> + BUG();
>> >> + }
>> >> +
>> >> + spin_unlock(&boot_lock);
>> >> +
>> >> + v7_exit_coherency_flush(louis);
>> >> +
>> >> + __mcpm_cpu_down(cpu, cluster);
>> >> +
>> >> + if (!skip_reset) {
>> >> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> >> + CORE_DEBUG_RESET_BIT(cpu);
>> >> + writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
>> >
>> > You should not perform this outside the lock protected region as this
>> > could race with hip04_mcpm_power_up(). Instead, this should be done
>> > above when hip04_cpu_table[cluster][cpu] == 0 after being decremented.
>> >
>>
>> No. power_down() is executed on the specified CPU. If spin_unlock() is
>> placed after reset operation, it means that there's no chance to
>> execute the spin_unlock(). Because CPU is already in reset mode at
>> this time.
>
> Normally, reset is effective only when WFI is later executed. Are you
> sure this is not the case on hip04 as well?
>
>
Oh. it's different. cpu_v7_reset() likes to give a reset pulse signal to CPU
core logic.
The operation on SC_CPU_RESET_REQ register likes make CPU out of
reset mode. After system is power on, all CPUs except for CPU0 stay
in reset mode.
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v7 06/15] ARM: hisi: enable MCPM implementation
2014-05-20 4:43 ` Haojian Zhuang
@ 2014-05-21 10:02 ` Dave Martin
2014-05-21 13:52 ` Nicolas Pitre
1 sibling, 0 replies; 50+ messages in thread
From: Dave Martin @ 2014-05-21 10:02 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 20, 2014 at 12:43:59PM +0800, Haojian Zhuang wrote:
> On 16 May 2014 04:01, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Thu, 15 May 2014, Haojian Zhuang wrote:
> >
> >> On 14 May 2014 03:43, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >> > On Tue, 13 May 2014, Haojian Zhuang wrote:
> >> >
> >> >> + data = readl_relaxed(fabric + FAB_SF_MODE);
> >> >> + if (on)
> >> >> + data |= 1 << cluster;
> >> >> + else
> >> >> + data &= ~(1 << cluster);
> >> >> + writel_relaxed(data, fabric + FAB_SF_MODE);
> >> >> + while (1) {
> >> >> + if (data == readl_relaxed(fabric + FAB_SF_MODE))
> >> >> + break;
> >> >> + }
> >> >> +}
> >> >
> >> > The above could be easily coded in assembly for the power_up_setup
> >> > callback thusly:
> >> >
> >> > hip04_power_up_setup:
> >> >
> >> > cmp r0, #0 @ check affinity level
> >> > bxeq lr @ nothing to do at CPU level
> >> >
> >> > mrc p15, 0, r0, c0, c0, 5 @ get MPIDR
> >> > ubfx r0, r0, #8, #8 @ extract cluster number
> >> >
> >> > adr r1, .LC0
> >> > ldmia r1, {r2, r3}
> >> > sub r2, r2, r1 @ virt_addr - phys_addr
> >> > ldr r1, [r2, r3] @ get fabric_phys_addr
> >> > mov r2, #1
> >> > ldr r3, [r1, #FAB_SF_MODE] @ read "data"
> >> > orr r3, r3, r2, lsl r0 @ set cluster bit
> >> > str r3, [r1, #FAB_SF_MODE] @ write it back
> >> >
> >> > 1: ldr r2, [r1, #FAB_SF_MODE] @ read register content
> >> > cmp r2, r3 @ make sure it matches
> >> > bne 1b @ otherwise retry
> >> >
> >> > bx lr
> >> >
> >> > :LC0: .word .
> >> > .word fabric_phys_addr - .LC0
> >> >
> >> > That should be it.
> >> >
> >>
> >> No. These code should be executed before new CPU on. If I transfer
> >> them to assembler code, it means that code will be executed after
> >> new CPU on.
> >
> > Exact.
> >
> >> Then it results me failing to make new CPU online.
> >
> > The assembly code could be wrong as well. Are you sure this is not the
> > actual reason?
> >
> > Is there some documentation for this stuff?
> >
>
> There's no problem in assembly code. I even rewrite your assembly code.
>
> If I keep my c code with assembly code, new CPU could be online right.
> If I only use assembly code, I only get the kernel panic. So it's not
> caused by assembly code. It's caused by executing code after new CPU
> on.
>
> There's no documentation on this. They didn't prepare well on documents.
> I think they'll improve it in the future.
It's essential to understand what the hardware is actually doing here.
If we don't understand exactly what toggling those bits in FAB_SF_MODE
actually does, then it's impossible to judge on how to do it safely.
Cheers
---Dave
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v7 06/15] ARM: hisi: enable MCPM implementation
2014-05-20 4:43 ` Haojian Zhuang
2014-05-21 10:02 ` Dave Martin
@ 2014-05-21 13:52 ` Nicolas Pitre
1 sibling, 0 replies; 50+ messages in thread
From: Nicolas Pitre @ 2014-05-21 13:52 UTC (permalink / raw)
To: linux-arm-kernel
[ I somehow missed this email yesterday. Sorry if I asked the same
questions for which you already had provided answers. ]
On Tue, 20 May 2014, Haojian Zhuang wrote:
> On 16 May 2014 04:01, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Thu, 15 May 2014, Haojian Zhuang wrote:
> >
> >> On 14 May 2014 03:43, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >> > On Tue, 13 May 2014, Haojian Zhuang wrote:
> >> >
> >> >> + data = readl_relaxed(fabric + FAB_SF_MODE);
> >> >> + if (on)
> >> >> + data |= 1 << cluster;
> >> >> + else
> >> >> + data &= ~(1 << cluster);
> >> >> + writel_relaxed(data, fabric + FAB_SF_MODE);
> >> >> + while (1) {
> >> >> + if (data == readl_relaxed(fabric + FAB_SF_MODE))
> >> >> + break;
> >> >> + }
> >> >> +}
> >> >
> >> > The above could be easily coded in assembly for the power_up_setup
> >> > callback thusly:
> >> >
> >> > hip04_power_up_setup:
> >> >
> >> > cmp r0, #0 @ check affinity level
> >> > bxeq lr @ nothing to do at CPU level
> >> >
> >> > mrc p15, 0, r0, c0, c0, 5 @ get MPIDR
> >> > ubfx r0, r0, #8, #8 @ extract cluster number
> >> >
> >> > adr r1, .LC0
> >> > ldmia r1, {r2, r3}
> >> > sub r2, r2, r1 @ virt_addr - phys_addr
> >> > ldr r1, [r2, r3] @ get fabric_phys_addr
> >> > mov r2, #1
> >> > ldr r3, [r1, #FAB_SF_MODE] @ read "data"
> >> > orr r3, r3, r2, lsl r0 @ set cluster bit
> >> > str r3, [r1, #FAB_SF_MODE] @ write it back
> >> >
> >> > 1: ldr r2, [r1, #FAB_SF_MODE] @ read register content
> >> > cmp r2, r3 @ make sure it matches
> >> > bne 1b @ otherwise retry
> >> >
> >> > bx lr
> >> >
> >> > :LC0: .word .
> >> > .word fabric_phys_addr - .LC0
> >> >
> >> > That should be it.
> >> >
> >>
> >> No. These code should be executed before new CPU on. If I transfer
> >> them to assembler code, it means that code will be executed after
> >> new CPU on.
> >
> > Exact.
> >
> >> Then it results me failing to make new CPU online.
> >
> > The assembly code could be wrong as well. Are you sure this is not the
> > actual reason?
> >
> > Is there some documentation for this stuff?
> >
>
> There's no problem in assembly code. I even rewrite your assembly code.
>
> If I keep my c code with assembly code, new CPU could be online right.
> If I only use assembly code, I only get the kernel panic. So it's not
> caused by assembly code. It's caused by executing code after new CPU
> on.
Beware. The assembly code, when invoked via the MCPM layer during early
boot of a CPU, is executing with the MMU still disabled. That means all
addresses must be physical addresses. This is where things myght be
tricky. And then that code should not work if invoked from C code
because it then has to deal with virtual addresses. So if you tested the
assembly code by calling it from C code and it worked then the assembly
code is wrong.
To be sure please post the code you tested (mine wasn't complete) so we
could tell you if it is right.
> cpu_v7_reset() likes to give a reset pulse signal to CPU core logic.
> The operation on SC_CPU_RESET_REQ register likes make CPU out of reset
> mode. After system is power on, all CPUs except for CPU0 stay in reset
> mode.
Sorry, I don't fully understand the above.
I also note in your code that you write the same bits to
SC_CPU_RESET_REQ in both the power_up and power_down methods. So if
this is about sending a reset pulse only, how do you keep a CPU down for
a long period?
Nicolas
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 07/15] ARM: hisi: enable HiP04
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (5 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 06/15] ARM: hisi: enable MCPM implementation Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-13 11:45 ` [PATCH v7 " Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 08/15] document: dt: add the binding on HiP04 Haojian Zhuang
` (8 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Support HiP04 SoC what supports 16 cores. And it relies on MCPM
framework.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/mach-hisi/Kconfig | 10 +++++++++-
arch/arm/mach-hisi/core.h | 2 ++
arch/arm/mach-hisi/hisilicon.c | 12 ++++++++++++
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index da16efd..3f1ece7 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -17,7 +17,15 @@ config ARCH_HI3xxx
select PINCTRL
select PINCTRL_SINGLE
help
- Support for Hisilicon Hi36xx/Hi37xx processor family
+ Support for Hisilicon Hi36xx/Hi37xx SoC family
+
+config ARCH_HIP04
+ bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7
+ select HAVE_ARM_ARCH_TIMER
+ select MCPM if SMP
+ select MCPM_QUAD_CLUSTER if SMP
+ help
+ Support for Hisilicon HiP04 SoC family
endmenu
diff --git a/arch/arm/mach-hisi/core.h b/arch/arm/mach-hisi/core.h
index af23ec2..1e60795 100644
--- a/arch/arm/mach-hisi/core.h
+++ b/arch/arm/mach-hisi/core.h
@@ -12,4 +12,6 @@ extern void hi3xxx_cpu_die(unsigned int cpu);
extern int hi3xxx_cpu_kill(unsigned int cpu);
extern void hi3xxx_set_cpu(int cpu, bool enable);
+extern bool __init hip04_smp_init_ops(void);
+
#endif
diff --git a/arch/arm/mach-hisi/hisilicon.c b/arch/arm/mach-hisi/hisilicon.c
index 741faf3..6489e57 100644
--- a/arch/arm/mach-hisi/hisilicon.c
+++ b/arch/arm/mach-hisi/hisilicon.c
@@ -88,3 +88,15 @@ DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
.smp = smp_ops(hi3xxx_smp_ops),
.restart = hi3xxx_restart,
MACHINE_END
+
+#ifdef CONFIG_ARCH_HIP04
+static const char *hip04_compat[] __initconst = {
+ "hisilicon,hip04-d01",
+ NULL,
+};
+
+DT_MACHINE_START(HIP04, "Hisilicon HiP04 (Flattened Device Tree)")
+ .dt_compat = hip04_compat,
+ .smp_init = smp_init_ops(hip04_smp_init_ops),
+MACHINE_END
+#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v7 07/15] ARM: hisi: enable HiP04
2014-05-11 8:06 ` [PATCH v6 07/15] ARM: hisi: enable HiP04 Haojian Zhuang
@ 2014-05-13 11:45 ` Haojian Zhuang
0 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-13 11:45 UTC (permalink / raw)
To: linux-arm-kernel
Support HiP04 SoC what supports 16 cores. And it relies on MCPM
framework.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/mach-hisi/Kconfig | 10 +++++++++-
arch/arm/mach-hisi/hisilicon.c | 9 +++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index da16efd..3f1ece7 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -17,7 +17,15 @@ config ARCH_HI3xxx
select PINCTRL
select PINCTRL_SINGLE
help
- Support for Hisilicon Hi36xx/Hi37xx processor family
+ Support for Hisilicon Hi36xx/Hi37xx SoC family
+
+config ARCH_HIP04
+ bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7
+ select HAVE_ARM_ARCH_TIMER
+ select MCPM if SMP
+ select MCPM_QUAD_CLUSTER if SMP
+ help
+ Support for Hisilicon HiP04 SoC family
endmenu
diff --git a/arch/arm/mach-hisi/hisilicon.c b/arch/arm/mach-hisi/hisilicon.c
index 741faf3..a9f648f 100644
--- a/arch/arm/mach-hisi/hisilicon.c
+++ b/arch/arm/mach-hisi/hisilicon.c
@@ -88,3 +88,12 @@ DT_MACHINE_START(HI3620, "Hisilicon Hi3620 (Flattened Device Tree)")
.smp = smp_ops(hi3xxx_smp_ops),
.restart = hi3xxx_restart,
MACHINE_END
+
+static const char *hip04_compat[] __initconst = {
+ "hisilicon,hip04-d01",
+ NULL,
+};
+
+DT_MACHINE_START(HIP04, "Hisilicon HiP04 (Flattened Device Tree)")
+ .dt_compat = hip04_compat,
+MACHINE_END
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH v6 08/15] document: dt: add the binding on HiP04
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (6 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 07/15] ARM: hisi: enable HiP04 Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 09/15] document: dt: add the binding on HiP04 clock Haojian Zhuang
` (7 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Add bootwrapper-phys, bootwrapper-size, bootwrapper-magic properties for
Hisilicon HiP04 SoC.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
.../devicetree/bindings/arm/hisilicon/hisilicon.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
index df0a452..5024992 100644
--- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
+++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
@@ -4,6 +4,10 @@ Hisilicon Platforms Device Tree Bindings
Hi4511 Board
Required root node properties:
- compatible = "hisilicon,hi3620-hi4511";
+HiP04 D01 Board
+Required root node properties:
+ - compatible = "hisilicon,hip04-d01";
+
Hisilicon system controller
@@ -19,6 +23,15 @@ Optional properties:
If reg value is not zero, cpun exit wfi and go
- resume-offset : offset in sysctrl for notifying cpu0 when resume
- reboot-offset : offset in sysctrl for system reboot
+- relocation-entry : relocation address of secondary cpu boot code
+- relocation-size : relocation size of secondary cpu boot code
+- bootwrapper-phys : physical address of boot wrapper
+- bootwrapper-size : size of boot wrapper
+- bootwrapper-magic : magic number for secondary cpu in boot wrapper
+The memory area of [bootwrapper-phys : bootwrapper-phys+bootwrapper-size]
+should be reserved. This should be set in /memreserve/ node in DTS file.
+bootwrapper-phys, bootwrapper-size, bootwrapper-magic is used in HiP04
+DTS file.
Example:
@@ -31,6 +44,7 @@ Example:
reboot-offset = <0x4>;
};
+
PCTRL: Peripheral misc control register
Required Properties:
@@ -44,3 +58,10 @@ Example:
compatible = "hisilicon,pctrl";
reg = <0xfca09000 0x1000>;
};
+
+
+Fabric:
+
+Required Properties:
+- compatible: "hisilicon,hip04-fabric";
+- reg: Address and size of Fabric
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 09/15] document: dt: add the binding on HiP04 clock
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (7 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 08/15] document: dt: add the binding on HiP04 Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 10/15] ARM: dts: append hip04 dts Haojian Zhuang
` (6 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
The DT binding for Hisilicon HiP04 clock driver.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
.../devicetree/bindings/clock/hip04-clock.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/hip04-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/hip04-clock.txt b/Documentation/devicetree/bindings/clock/hip04-clock.txt
new file mode 100644
index 0000000..4d31ae3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/hip04-clock.txt
@@ -0,0 +1,20 @@
+* Hisilicon HiP04 Clock Controller
+
+The HiP04 clock controller generates and supplies clock to various
+controllers within the HiP04 SoC.
+
+Required Properties:
+
+- compatible: should be one of the following.
+ - "hisilicon,hip04-clock" - controller compatible with HiP04 SoC.
+
+- 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>.
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 10/15] ARM: dts: append hip04 dts
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (8 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 09/15] document: dt: add the binding on HiP04 clock Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 11/15] ARM: config: append lpae configuration Haojian Zhuang
` (5 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Add hip04-d01.dts & hip04.dtsi for hip04 SoC platform.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/hip04-d01.dts | 39 ++++++
arch/arm/boot/dts/hip04.dtsi | 260 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 300 insertions(+)
create mode 100644 arch/arm/boot/dts/hip04-d01.dts
create mode 100644 arch/arm/boot/dts/hip04.dtsi
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 35c146f..7119bca 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -80,6 +80,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
dtb-$(CONFIG_ARCH_HI3xxx) += hi3620-hi4511.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 \
diff --git a/arch/arm/boot/dts/hip04-d01.dts b/arch/arm/boot/dts/hip04-d01.dts
new file mode 100644
index 0000000..661c8e5
--- /dev/null
+++ b/arch/arm/boot/dts/hip04-d01.dts
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2013-2014 Linaro Ltd.
+ * Author: Haojian Zhuang <haojian.zhuang@linaro.org>
+ *
+ * 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;
+
+#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>;
+ };
+
+ memory at 00000004,c0000000 {
+ device_type = "memory";
+ reg = <0x00000004 0xc0000000 0x00000003 0x40000000>;
+ };
+
+ soc {
+ uart0: uart at 4007000 {
+ status = "ok";
+ };
+ };
+};
diff --git a/arch/arm/boot/dts/hip04.dtsi b/arch/arm/boot/dts/hip04.dtsi
new file mode 100644
index 0000000..eee69e5
--- /dev/null
+++ b/arch/arm/boot/dts/hip04.dtsi
@@ -0,0 +1,260 @@
+/*
+ * Hisilicon Ltd. HiP01 SoC
+ *
+ * Copyright (C) 2013-2014 Hisilicon Ltd.
+ * Copyright (C) 2013-2014 Linaro Ltd.
+ *
+ * Author: Haojian Zhuang <haojian.zhuang@linaro.org>
+ *
+ * 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.
+ */
+
+#include <dt-bindings/clock/hip04-clock.h>
+
+/ {
+ /* memory bus is 64-bit */
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu-map {
+ cluster0 {
+ core0 {
+ cpu = <&CPU0>;
+ };
+ core1 {
+ cpu = <&CPU1>;
+ };
+ core2 {
+ cpu = <&CPU2>;
+ };
+ core3 {
+ cpu = <&CPU3>;
+ };
+ };
+ cluster1 {
+ core0 {
+ cpu = <&CPU4>;
+ };
+ core1 {
+ cpu = <&CPU5>;
+ };
+ core2 {
+ cpu = <&CPU6>;
+ };
+ core3 {
+ cpu = <&CPU7>;
+ };
+ };
+ cluster2 {
+ core0 {
+ cpu = <&CPU8>;
+ };
+ core1 {
+ cpu = <&CPU9>;
+ };
+ core2 {
+ cpu = <&CPU10>;
+ };
+ core3 {
+ cpu = <&CPU11>;
+ };
+ };
+ cluster3 {
+ core0 {
+ cpu = <&CPU12>;
+ };
+ core1 {
+ cpu = <&CPU13>;
+ };
+ core2 {
+ cpu = <&CPU14>;
+ };
+ core3 {
+ cpu = <&CPU15>;
+ };
+ };
+ };
+ CPU0: cpu at 0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0>;
+ };
+ CPU1: cpu at 1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <1>;
+ };
+ CPU2: cpu at 2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <2>;
+ };
+ CPU3: cpu at 3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <3>;
+ };
+ CPU4: cpu at 100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x100>;
+ };
+ CPU5: cpu at 101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x101>;
+ };
+ CPU6: cpu at 102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x102>;
+ };
+ CPU7: cpu at 103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x103>;
+ };
+ CPU8: cpu at 200 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x200>;
+ };
+ CPU9: cpu at 201 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x201>;
+ };
+ CPU10: cpu at 202 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x202>;
+ };
+ CPU11: cpu at 203 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x203>;
+ };
+ CPU12: cpu at 300 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x300>;
+ };
+ CPU13: cpu at 301 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x301>;
+ };
+ CPU14: cpu at 302 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x302>;
+ };
+ CPU15: cpu at 303 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a15";
+ reg = <0x303>;
+ };
+ };
+
+ 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>;
+ #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>;
+ interrupt-controller;
+ interrupts = <1 9 0xf04>;
+
+ reg = <0xc01000 0x1000>, <0xc02000 0x1000>,
+ <0xc04000 0x2000>, <0xc06000 0x2000>;
+ };
+
+ sysctrl: sysctrl {
+ compatible = "hisilicon,sysctrl";
+ reg = <0x3e00000 0x00100000>;
+ relocation-entry = <0xe0000100>;
+ relocation-size = <0x1000>;
+ bootwrapper-phys = <0x10c00000>;
+ bootwrapper-size = <0x10000>;
+ bootwrapper-magic = <0xa5a5a5a5>;
+ };
+
+ fabric: fabric {
+ compatible = "hisilicon,hip04-fabric";
+ reg = <0x302a000 0x1000>;
+ };
+
+ 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";
+ };
+
+ arm-pmu {
+ compatible = "arm,cortex-a15-pmu";
+ interrupts = <0 64 4>,
+ <0 65 4>,
+ <0 66 4>,
+ <0 67 4>,
+ <0 68 4>,
+ <0 69 4>,
+ <0 70 4>,
+ <0 71 4>,
+ <0 72 4>,
+ <0 73 4>,
+ <0 74 4>,
+ <0 75 4>,
+ <0 76 4>,
+ <0 77 4>,
+ <0 78 4>,
+ <0 79 4>;
+ };
+
+ uart0: uart at 4007000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x4007000 0x1000>;
+ interrupts = <0 381 4>;
+ clocks = <&clock HIP04_CLK_168M>;
+ clock-names = "uartclk";
+ reg-shift = <2>;
+ status = "disabled";
+ };
+ };
+};
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 11/15] ARM: config: append lpae configuration
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (9 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 10/15] ARM: dts: append hip04 dts Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 12/15] ARM: config: append hip04_defconfig Haojian Zhuang
` (4 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Append multi_v7_lpae_config. In this default configuration,
CONFIG_ARCH_MULTI_V6 is disabled. CONFIG_ARM_LPAE is enabled.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/configs/multi_v7_lpae_defconfig | 351 +++++++++++++++++++++++++++++++
1 file changed, 351 insertions(+)
create mode 100644 arch/arm/configs/multi_v7_lpae_defconfig
diff --git a/arch/arm/configs/multi_v7_lpae_defconfig b/arch/arm/configs/multi_v7_lpae_defconfig
new file mode 100644
index 0000000..59fcefc
--- /dev/null
+++ b/arch/arm/configs/multi_v7_lpae_defconfig
@@ -0,0 +1,351 @@
+CONFIG_SYSVIPC=y
+CONFIG_FHANDLE=y
+CONFIG_IRQ_DOMAIN_DEBUG=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_EMBEDDED=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
+CONFIG_PARTITION_ADVANCED=y
+# CONFIG_ARCH_MULTI_V6 is not set
+CONFIG_ARCH_MULTI_V7=y
+CONFIG_ARM_LPAE=y
+CONFIG_ARCH_MVEBU=y
+CONFIG_MACH_ARMADA_370=y
+CONFIG_MACH_ARMADA_375=y
+CONFIG_MACH_ARMADA_38X=y
+CONFIG_MACH_ARMADA_XP=y
+CONFIG_MACH_DOVE=y
+CONFIG_ARCH_BCM=y
+CONFIG_ARCH_BCM_5301X=y
+CONFIG_ARCH_BCM_MOBILE=y
+CONFIG_ARCH_BERLIN=y
+CONFIG_MACH_BERLIN_BG2=y
+CONFIG_MACH_BERLIN_BG2CD=y
+CONFIG_GPIO_PCA953X=y
+CONFIG_ARCH_HIGHBANK=y
+CONFIG_ARCH_HISI=y
+CONFIG_ARCH_HI3xxx=y
+CONFIG_ARCH_HIP04=y
+CONFIG_ARCH_KEYSTONE=y
+CONFIG_ARCH_MXC=y
+CONFIG_MACH_IMX51_DT=y
+CONFIG_SOC_IMX53=y
+CONFIG_SOC_IMX6Q=y
+CONFIG_SOC_IMX6SL=y
+CONFIG_SOC_VF610=y
+CONFIG_ARCH_OMAP3=y
+CONFIG_ARCH_OMAP4=y
+CONFIG_SOC_OMAP5=y
+CONFIG_SOC_AM33XX=y
+CONFIG_SOC_DRA7XX=y
+CONFIG_SOC_AM43XX=y
+CONFIG_ARCH_QCOM=y
+CONFIG_ARCH_MSM8X60=y
+CONFIG_ARCH_MSM8960=y
+CONFIG_ARCH_MSM8974=y
+CONFIG_ARCH_ROCKCHIP=y
+CONFIG_ARCH_SOCFPGA=y
+CONFIG_PLAT_SPEAR=y
+CONFIG_ARCH_SPEAR13XX=y
+CONFIG_MACH_SPEAR1310=y
+CONFIG_MACH_SPEAR1340=y
+CONFIG_ARCH_STI=y
+CONFIG_ARCH_SUNXI=y
+CONFIG_ARCH_SIRF=y
+CONFIG_ARCH_TEGRA=y
+CONFIG_ARCH_TEGRA_2x_SOC=y
+CONFIG_ARCH_TEGRA_3x_SOC=y
+CONFIG_ARCH_TEGRA_114_SOC=y
+CONFIG_ARCH_TEGRA_124_SOC=y
+CONFIG_TEGRA_EMC_SCALING_ENABLE=y
+CONFIG_ARCH_U8500=y
+CONFIG_MACH_HREFV60=y
+CONFIG_MACH_SNOWBALL=y
+CONFIG_MACH_UX500_DT=y
+CONFIG_ARCH_VEXPRESS=y
+CONFIG_ARCH_VEXPRESS_CA9X4=y
+CONFIG_ARCH_VIRT=y
+CONFIG_ARCH_WM8850=y
+CONFIG_ARCH_ZYNQ=y
+CONFIG_NEON=y
+CONFIG_TRUSTED_FOUNDATIONS=y
+CONFIG_PCI=y
+CONFIG_PCI_MSI=y
+CONFIG_PCI_MVEBU=y
+CONFIG_PCI_TEGRA=y
+CONFIG_SMP=y
+CONFIG_HIGHMEM=y
+CONFIG_HIGHPTE=y
+CONFIG_CMA=y
+CONFIG_ARM_APPENDED_DTB=y
+CONFIG_ARM_ATAG_DTB_COMPAT=y
+CONFIG_KEXEC=y
+CONFIG_CPU_FREQ=y
+CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
+CONFIG_CPU_IDLE=y
+CONFIG_NET=y
+CONFIG_PACKET=y
+CONFIG_UNIX=y
+CONFIG_INET=y
+CONFIG_IP_PNP=y
+CONFIG_IP_PNP_DHCP=y
+CONFIG_IP_PNP_BOOTP=y
+CONFIG_IP_PNP_RARP=y
+CONFIG_IPV6_ROUTER_PREF=y
+CONFIG_IPV6_OPTIMISTIC_DAD=y
+CONFIG_INET6_AH=m
+CONFIG_INET6_ESP=m
+CONFIG_INET6_IPCOMP=m
+CONFIG_IPV6_MIP6=m
+CONFIG_IPV6_TUNNEL=m
+CONFIG_IPV6_MULTIPLE_TABLES=y
+CONFIG_CFG80211=m
+CONFIG_MAC80211=m
+CONFIG_RFKILL=y
+CONFIG_RFKILL_INPUT=y
+CONFIG_RFKILL_GPIO=y
+CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
+CONFIG_DMA_CMA=y
+CONFIG_CMA_SIZE_MBYTES=64
+CONFIG_OMAP_OCP2SCP=y
+CONFIG_MTD=y
+CONFIG_MTD_M25P80=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_ICS932S401=y
+CONFIG_APDS9802ALS=y
+CONFIG_ISL29003=y
+CONFIG_BLK_DEV_SD=y
+CONFIG_BLK_DEV_SR=y
+CONFIG_SCSI_MULTI_LUN=y
+CONFIG_ATA=y
+CONFIG_SATA_AHCI_PLATFORM=y
+CONFIG_SATA_HIGHBANK=y
+CONFIG_SATA_MV=y
+CONFIG_NETDEVICES=y
+CONFIG_SUN4I_EMAC=y
+CONFIG_NET_CALXEDA_XGMAC=y
+CONFIG_MV643XX_ETH=y
+CONFIG_MVNETA=y
+CONFIG_KS8851=y
+CONFIG_R8169=y
+CONFIG_SMSC911X=y
+CONFIG_STMMAC_ETH=y
+CONFIG_TI_CPSW=y
+CONFIG_AT803X_PHY=y
+CONFIG_MARVELL_PHY=y
+CONFIG_ICPLUS_PHY=y
+CONFIG_USB_PEGASUS=y
+CONFIG_USB_USBNET=y
+CONFIG_USB_NET_SMSC75XX=y
+CONFIG_USB_NET_SMSC95XX=y
+CONFIG_BRCMFMAC=m
+CONFIG_RT2X00=m
+CONFIG_RT2800USB=m
+CONFIG_INPUT_EVDEV=y
+CONFIG_KEYBOARD_GPIO=y
+CONFIG_KEYBOARD_TEGRA=y
+CONFIG_KEYBOARD_SPEAR=y
+CONFIG_KEYBOARD_CROS_EC=y
+CONFIG_MOUSE_PS2_ELANTECH=y
+CONFIG_INPUT_MISC=y
+CONFIG_INPUT_MPU3050=y
+CONFIG_SERIO_AMBAKMI=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_8250_DW=y
+CONFIG_SERIAL_AMBA_PL011=y
+CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
+CONFIG_SERIAL_SIRFSOC=y
+CONFIG_SERIAL_SIRFSOC_CONSOLE=y
+CONFIG_SERIAL_TEGRA=y
+CONFIG_SERIAL_IMX=y
+CONFIG_SERIAL_IMX_CONSOLE=y
+CONFIG_SERIAL_MSM=y
+CONFIG_SERIAL_MSM_CONSOLE=y
+CONFIG_SERIAL_VT8500=y
+CONFIG_SERIAL_VT8500_CONSOLE=y
+CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_SERIAL_OMAP=y
+CONFIG_SERIAL_OMAP_CONSOLE=y
+CONFIG_SERIAL_XILINX_PS_UART=y
+CONFIG_SERIAL_XILINX_PS_UART_CONSOLE=y
+CONFIG_SERIAL_FSL_LPUART=y
+CONFIG_SERIAL_FSL_LPUART_CONSOLE=y
+CONFIG_SERIAL_ST_ASC=y
+CONFIG_SERIAL_ST_ASC_CONSOLE=y
+CONFIG_I2C_CHARDEV=y
+CONFIG_I2C_MUX=y
+CONFIG_I2C_MUX_PCA954x=y
+CONFIG_I2C_MUX_PINCTRL=y
+CONFIG_I2C_DESIGNWARE_PLATFORM=y
+CONFIG_I2C_MV64XXX=y
+CONFIG_I2C_SIRF=y
+CONFIG_I2C_TEGRA=y
+CONFIG_SPI=y
+CONFIG_SPI_OMAP24XX=y
+CONFIG_SPI_ORION=y
+CONFIG_SPI_PL022=y
+CONFIG_SPI_SIRF=y
+CONFIG_SPI_TEGRA114=y
+CONFIG_SPI_TEGRA20_SFLASH=y
+CONFIG_SPI_TEGRA20_SLINK=y
+CONFIG_PINCTRL_AS3722=y
+CONFIG_PINCTRL_PALMAS=y
+CONFIG_GPIO_SYSFS=y
+CONFIG_GPIO_GENERIC_PLATFORM=y
+CONFIG_GPIO_PCA953X_IRQ=y
+CONFIG_GPIO_TWL4030=y
+CONFIG_GPIO_PALMAS=y
+CONFIG_GPIO_TPS6586X=y
+CONFIG_GPIO_TPS65910=y
+CONFIG_BATTERY_SBS=y
+CONFIG_CHARGER_TPS65090=y
+CONFIG_POWER_RESET_AS3722=y
+CONFIG_POWER_RESET_GPIO=y
+CONFIG_SENSORS_LM90=y
+CONFIG_THERMAL=y
+CONFIG_DOVE_THERMAL=y
+CONFIG_ARMADA_THERMAL=y
+CONFIG_WATCHDOG=y
+CONFIG_ORION_WATCHDOG=y
+CONFIG_MFD_AS3722=y
+CONFIG_MFD_CROS_EC=y
+CONFIG_MFD_CROS_EC_SPI=y
+CONFIG_MFD_MAX8907=y
+CONFIG_MFD_PALMAS=y
+CONFIG_MFD_TPS65090=y
+CONFIG_MFD_TPS6586X=y
+CONFIG_MFD_TPS65910=y
+CONFIG_REGULATOR_VIRTUAL_CONSUMER=y
+CONFIG_REGULATOR_AB8500=y
+CONFIG_REGULATOR_AS3722=y
+CONFIG_REGULATOR_GPIO=y
+CONFIG_REGULATOR_MAX8907=y
+CONFIG_REGULATOR_PALMAS=y
+CONFIG_REGULATOR_TPS51632=y
+CONFIG_REGULATOR_TPS62360=y
+CONFIG_REGULATOR_TPS65090=y
+CONFIG_REGULATOR_TPS6586X=y
+CONFIG_REGULATOR_TPS65910=y
+CONFIG_REGULATOR_TWL4030=y
+CONFIG_REGULATOR_VEXPRESS=y
+CONFIG_MEDIA_SUPPORT=y
+CONFIG_MEDIA_CAMERA_SUPPORT=y
+CONFIG_MEDIA_USB_SUPPORT=y
+CONFIG_USB_VIDEO_CLASS=y
+CONFIG_USB_GSPCA=y
+CONFIG_DRM=y
+CONFIG_DRM_TEGRA=y
+CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_FB_ARMCLCD=y
+CONFIG_FB_WM8505=y
+CONFIG_FB_SIMPLE=y
+CONFIG_BACKLIGHT_LCD_SUPPORT=y
+CONFIG_BACKLIGHT_CLASS_DEVICE=y
+CONFIG_BACKLIGHT_PWM=y
+CONFIG_FRAMEBUFFER_CONSOLE=y
+CONFIG_SOUND=y
+CONFIG_SND=y
+CONFIG_SND_SOC=y
+CONFIG_SND_SOC_TEGRA=y
+CONFIG_SND_SOC_TEGRA_RT5640=y
+CONFIG_SND_SOC_TEGRA_WM8753=y
+CONFIG_SND_SOC_TEGRA_WM8903=y
+CONFIG_SND_SOC_TEGRA_TRIMSLICE=y
+CONFIG_SND_SOC_TEGRA_ALC5632=y
+CONFIG_SND_SOC_TEGRA_MAX98090=y
+CONFIG_USB=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_TEGRA=y
+CONFIG_USB_EHCI_HCD_PLATFORM=y
+CONFIG_USB_ISP1760_HCD=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_CHIPIDEA=y
+CONFIG_USB_CHIPIDEA_HOST=y
+CONFIG_AB8500_USB=y
+CONFIG_OMAP_USB3=y
+CONFIG_SAMSUNG_USB2PHY=y
+CONFIG_SAMSUNG_USB3PHY=y
+CONFIG_USB_GPIO_VBUS=y
+CONFIG_USB_ISP1301=y
+CONFIG_USB_MXS_PHY=y
+CONFIG_MMC=y
+CONFIG_MMC_BLOCK_MINORS=16
+CONFIG_MMC_ARMMMCI=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_ESDHC_IMX=y
+CONFIG_MMC_SDHCI_TEGRA=y
+CONFIG_MMC_SDHCI_DOVE=y
+CONFIG_MMC_SDHCI_SPEAR=y
+CONFIG_MMC_SDHCI_BCM_KONA=y
+CONFIG_MMC_OMAP=y
+CONFIG_MMC_OMAP_HS=y
+CONFIG_MMC_MVSDIO=y
+CONFIG_EDAC=y
+CONFIG_EDAC_MM_EDAC=y
+CONFIG_EDAC_HIGHBANK_MC=y
+CONFIG_EDAC_HIGHBANK_L2=y
+CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_AS3722=y
+CONFIG_RTC_DRV_MAX8907=y
+CONFIG_RTC_DRV_PALMAS=y
+CONFIG_RTC_DRV_TWL4030=y
+CONFIG_RTC_DRV_TPS6586X=y
+CONFIG_RTC_DRV_TPS65910=y
+CONFIG_RTC_DRV_EM3027=y
+CONFIG_RTC_DRV_PL031=y
+CONFIG_RTC_DRV_VT8500=y
+CONFIG_RTC_DRV_MV=y
+CONFIG_RTC_DRV_TEGRA=y
+CONFIG_DMADEVICES=y
+CONFIG_DW_DMAC=y
+CONFIG_MV_XOR=y
+CONFIG_TEGRA20_APB_DMA=y
+CONFIG_STE_DMA40=y
+CONFIG_SIRF_DMA=y
+CONFIG_TI_EDMA=y
+CONFIG_PL330_DMA=y
+CONFIG_IMX_SDMA=y
+CONFIG_IMX_DMA=y
+CONFIG_MXS_DMA=y
+CONFIG_DMA_OMAP=y
+CONFIG_STAGING=y
+CONFIG_SENSORS_ISL29018=y
+CONFIG_SENSORS_ISL29028=y
+CONFIG_MFD_NVEC=y
+CONFIG_KEYBOARD_NVEC=y
+CONFIG_SERIO_NVEC_PS2=y
+CONFIG_NVEC_POWER=y
+CONFIG_COMMON_CLK_QCOM=y
+CONFIG_MSM_GCC_8660=y
+CONFIG_MSM_MMCC_8960=y
+CONFIG_MSM_MMCC_8974=y
+CONFIG_TEGRA_IOMMU_GART=y
+CONFIG_TEGRA_IOMMU_SMMU=y
+CONFIG_MEMORY=y
+CONFIG_IIO=y
+CONFIG_AK8975=y
+CONFIG_PWM=y
+CONFIG_PWM_TEGRA=y
+CONFIG_PWM_VT8500=y
+CONFIG_OMAP_USB2=y
+CONFIG_EXT4_FS=y
+CONFIG_VFAT_FS=y
+CONFIG_TMPFS=y
+CONFIG_SQUASHFS=y
+CONFIG_SQUASHFS_LZO=y
+CONFIG_SQUASHFS_XZ=y
+CONFIG_NFS_FS=y
+CONFIG_NFS_V3_ACL=y
+CONFIG_NFS_V4=y
+CONFIG_ROOT_NFS=y
+CONFIG_PRINTK_TIME=y
+CONFIG_DEBUG_FS=y
+CONFIG_MAGIC_SYSRQ=y
+CONFIG_LOCKUP_DETECTOR=y
+CONFIG_CRYPTO_DEV_TEGRA_AES=y
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 12/15] ARM: config: append hip04_defconfig
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (10 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 11/15] ARM: config: append lpae configuration Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 13/15] ARM: config: select ARCH_HISI in hi3xxx_defconfig Haojian Zhuang
` (3 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Select HiP04 SoC configuration by hip04_defconfig.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/configs/hip04_defconfig | 74 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
create mode 100644 arch/arm/configs/hip04_defconfig
diff --git a/arch/arm/configs/hip04_defconfig b/arch/arm/configs/hip04_defconfig
new file mode 100644
index 0000000..5c471b2
--- /dev/null
+++ b/arch/arm/configs/hip04_defconfig
@@ -0,0 +1,74 @@
+CONFIG_SYSVIPC=y
+CONFIG_POSIX_MQUEUE=y
+CONFIG_IRQ_DOMAIN_DEBUG=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_BSD_PROCESS_ACCT=y
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_RD_GZIP=y
+# CONFIG_ARCH_MULTI_V6 is not set
+CONFIG_ARCH_MULTI_V7=y
+CONFIG_ARCH_HISI=y
+CONFIG_ARCH_HIP04=y
+CONFIG_ARM_LPAE=y
+CONFIG_SMP=y
+CONFIG_NR_CPUS=16
+CONFIG_MCPM=y
+CONFIG_MCPM_QUAD_CLUSTER=y
+CONFIG_PREEMPT=y
+CONFIG_AEABI=y
+CONFIG_ARM_APPENDED_DTB=y
+CONFIG_ARM_ATAG_DTB_COMPAT=y
+CONFIG_ARM_ATAG_DTB_COMPAT_CMDLINE_FROM_BOOTLOADER=y
+CONFIG_HIGHMEM=y
+CONFIG_VFP=y
+CONFIG_NEON=y
+CONFIG_NET=y
+CONFIG_UNIX=y
+CONFIG_INET=y
+CONFIG_IP_PNP=y
+CONFIG_IP_PNP_DHCP=y
+CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
+CONFIG_BLK_DEV=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_BLK_DEV_LOOP_MIN_COUNT=8
+CONFIG_BLK_DEV_RAM=y
+CONFIG_BLK_DEV_RAM_COUNT=16
+CONFIG_BLK_DEV_RAM_SIZE=4096
+CONFIG_BLK_DEV_SD=y
+CONFIG_ATA=y
+CONFIG_SATA_AHCI_PLATFORM=y
+CONFIG_NETDEVICES=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_8250_NR_UARTS=2
+CONFIG_SERIAL_8250_RUNTIME_UARTS=2
+CONFIG_SERIAL_8250_DW=y
+CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_I2C_DESIGNWARE_PLATFORM=y
+CONFIG_PINCTRL_SINGLE=y
+CONFIG_GPIO_GENERIC_PLATFORM=y
+CONFIG_REGULATOR_GPIO=y
+CONFIG_DRM=y
+CONFIG_FB_SIMPLE=y
+CONFIG_RTC_CLASS=y
+CONFIG_EXT4_FS=y
+CONFIG_TMPFS=y
+CONFIG_NFS_FS=y
+CONFIG_NFS_V3_ACL=y
+CONFIG_NFS_V4=y
+CONFIG_ROOT_NFS=y
+CONFIG_PRINTK_TIME=y
+CONFIG_DEBUG_FS=y
+CONFIG_DEBUG_KERNEL=y
+CONFIG_DEBUG_LL=y
+CONFIG_DEBUG_UART_8250=y
+CONFIG_EARLY_PRINTK=y
+CONFIG_LOCKUP_DETECTOR=y
+CONFIG_DEBUG_USER=y
+CONFIG_VIRTUALIZATION=y
+CONFIG_KVM=y
+CONFIG_KVM_ARM_HOST=y
+CONFIG_KVM_ARM_MAX_VCPUS=4
+CONFIG_KVM_ARM_VGIC=y
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 13/15] ARM: config: select ARCH_HISI in hi3xxx_defconfig
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (11 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 12/15] ARM: config: append hip04_defconfig Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 14/15] ARM: hisi: enable erratum 798181 of A15 on HiP04 Haojian Zhuang
` (2 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Since ARCH_HISI is added as common configuration of both ARCH_HI3xxx and
ARCH_HIP04, update it into hi3xxx_defconfig.
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/configs/hi3xxx_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/hi3xxx_defconfig b/arch/arm/configs/hi3xxx_defconfig
index f186bdf..553e1b6 100644
--- a/arch/arm/configs/hi3xxx_defconfig
+++ b/arch/arm/configs/hi3xxx_defconfig
@@ -3,10 +3,12 @@ CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_RD_LZMA=y
+CONFIG_ARCH_HISI=y
CONFIG_ARCH_HI3xxx=y
CONFIG_SMP=y
CONFIG_PREEMPT=y
CONFIG_AEABI=y
+CONFIG_HIGHMEM=y
CONFIG_ARM_APPENDED_DTB=y
CONFIG_NET=y
CONFIG_UNIX=y
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 14/15] ARM: hisi: enable erratum 798181 of A15 on HiP04
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (12 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 13/15] ARM: config: select ARCH_HISI in hi3xxx_defconfig Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-11 8:06 ` [PATCH v6 15/15] virt: arm: support hip04 gic Haojian Zhuang
2014-05-14 18:37 ` [PATCH v6 00/15] enable HiP04 SoC Arnd Bergmann
15 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
From: Kefeng Wang <kefeng.wang@linaro.org>
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 <kefeng.wang@linaro.org>
Singed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
arch/arm/mach-hisi/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index 3f1ece7..0a5a49d 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -21,6 +21,7 @@ config ARCH_HI3xxx
config ARCH_HIP04
bool "Hisilicon HiP04 Cortex A15 family" if ARCH_MULTI_V7
+ select ARM_ERRATA_798181 if SMP
select HAVE_ARM_ARCH_TIMER
select MCPM if SMP
select MCPM_QUAD_CLUSTER if SMP
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 15/15] virt: arm: support hip04 gic
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (13 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 14/15] ARM: hisi: enable erratum 798181 of A15 on HiP04 Haojian Zhuang
@ 2014-05-11 8:06 ` Haojian Zhuang
2014-05-15 9:42 ` Marc Zyngier
2014-05-14 18:37 ` [PATCH v6 00/15] enable HiP04 SoC Arnd Bergmann
15 siblings, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-11 8:06 UTC (permalink / raw)
To: linux-arm-kernel
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 <haojian.zhuang@linaro.org>
---
arch/arm/kvm/interrupts_head.S | 32 ++++++++++++++++++++++------
include/kvm/arm_vgic.h | 5 ++++-
include/linux/irqchip/arm-gic.h | 6 ++++++
virt/kvm/arm/vgic.c | 47 +++++++++++++++++++++++++++++------------
4 files changed, 69 insertions(+), 21 deletions(-)
diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
index 76af9302..7aacaff 100644
--- a/arch/arm/kvm/interrupts_head.S
+++ b/arch/arm/kvm/interrupts_head.S
@@ -419,7 +419,10 @@ 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_NR_LR]
+ movs r10, r10, lsr #HWCFG_APR_SHIFT
+ ldrne r10, [r2, r10]
+ ldreq r10, [r2, #GICH_APR]
str r3, [r11, #VGIC_CPU_HCR]
str r4, [r11, #VGIC_CPU_VMCR]
@@ -435,9 +438,16 @@ vcpu .req r0 @ vcpu pointer always in r0
str r5, [r2, #GICH_HCR]
/* Save list registers */
- add r2, r2, #GICH_LR0
- add r3, r11, #VGIC_CPU_LR
ldr r4, [r11, #VGIC_CPU_NR_LR]
+ addeq r2, r2, #GICH_LR0
+ movne r10, r4, lsr #HWCFG_APR_SHIFT
+ /* the offset between GICH_APR & GICH_LR0 is 0x10 */
+ addne r10, r10, #0x10
+ addne r2, r2, r10
+ add r3, r11, #VGIC_CPU_LR
+ /* Get NR_LR from VGIC_CPU_NR_LR */
+ ldr r6, =HWCFG_NR_LR_MASK
+ and r4, r4, r6
1: ldr r6, [r2], #4
str r6, [r3], #4
subs r4, r4, #1
@@ -469,12 +479,22 @@ 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_NR_LR]
+ movs r6, r6, lsr #HWCFG_APR_SHIFT
+ strne r8, [r2, r6]
+ streq r8, [r2, #GICH_APR]
/* Restore list registers */
- add r2, r2, #GICH_LR0
- add r3, r11, #VGIC_CPU_LR
ldr r4, [r11, #VGIC_CPU_NR_LR]
+ addeq r2, r2, #GICH_LR0
+ movne r6, r4, lsr #HWCFG_APR_SHIFT
+ /* the offset between GICH_APR & GICH_LR0 is 0x10 */
+ addne r6, r6, #0x10
+ addne r2, r2, r6
+ /* Get NR_LR from VGIC_CPU_NR_LR */
+ add r3, r11, #VGIC_CPU_LR
+ ldr r6, =HWCFG_NR_LR_MASK
+ and r4, r4, r6
1: ldr r6, [r3], #4
str r6, [r2], #4
subs r4, r4, #1
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index f27000f..089de25 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -122,7 +122,10 @@ struct vgic_cpu {
/* Bitmap of used/free list registers */
DECLARE_BITMAP( lr_used, VGIC_MAX_LRS);
- /* Number of list registers on this CPU */
+ /*
+ * bit[31:16]: GICH_APR offset
+ * bit[15:0]: Number of list registers on this CPU
+ */
int nr_lr;
/* CPU vif control registers for world switch */
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 45e2d8c..5530388 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -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_MASK 0xffff
+#define HWCFG_APR_SHIFT 16
+
#ifndef __ASSEMBLY__
struct device_node;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 47b2983..e635b9b 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -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;
static unsigned int vgic_maint_irq;
@@ -624,9 +624,9 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
int vcpu_id = vcpu->vcpu_id;
int i, irq, source_cpu;
- u32 *lr;
+ u32 *lr, nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
- for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
+ for_each_set_bit(i, vgic_cpu->lr_used, nr_lr) {
lr = &vgic_cpu->vgic_lr[i];
irq = LR_IRQID(*lr);
source_cpu = LR_CPUID(*lr);
@@ -1005,8 +1005,9 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
{
struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
int lr;
+ int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
- for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
+ for_each_set_bit(lr, vgic_cpu->lr_used, nr_lr) {
int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
if (!vgic_irq_is_enabled(vcpu, irq)) {
@@ -1025,6 +1026,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
{
struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
int lr;
+ int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
/* Sanitize the input... */
BUG_ON(sgi_source_id & ~7);
@@ -1046,9 +1048,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)
+ lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used, nr_lr);
+ if (lr >= nr_lr)
return false;
kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id);
@@ -1181,9 +1182,10 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
* active bit.
*/
int lr, irq;
+ int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr,
- vgic_cpu->nr_lr) {
+ nr_lr) {
irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
vgic_irq_clear_active(vcpu, irq);
@@ -1221,13 +1223,13 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
int lr, pending;
+ int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
bool level_pending;
level_pending = vgic_process_maintenance(vcpu);
/* Clear mappings for empty LRs */
- for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr,
- vgic_cpu->nr_lr) {
+ for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr, nr_lr) {
int irq;
if (!test_and_clear_bit(lr, vgic_cpu->lr_used))
@@ -1241,8 +1243,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)
+ nr_lr);
+ if (level_pending || pending < nr_lr)
set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
}
@@ -1438,7 +1440,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
*/
vgic_cpu->vgic_vmcr = 0;
- vgic_cpu->nr_lr = vgic_nr_lr;
+ vgic_cpu->nr_lr = vgic_hw_cfg;
vgic_cpu->vgic_hcr = GICH_HCR_EN; /* Get the show on the road... */
return 0;
@@ -1470,17 +1472,33 @@ 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",
+ }, {
+ .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;
+ u32 vgic_nr_lr;
- 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;
}
+ /* If it's standard ARM GIC, high word in vgic_hw_cfg is 0.
+ * Otherwise, it's the offset of non-standard GICH_APR (in HiP04).
+ */
+ vgic_hw_cfg = (unsigned int)match->data << HWCFG_APR_SHIFT;
vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
if (!vgic_maint_irq) {
@@ -1517,6 +1535,7 @@ int kvm_vgic_hyp_init(void)
vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
vgic_nr_lr = (vgic_nr_lr & 0x3f) + 1;
+ vgic_hw_cfg |= vgic_nr_lr;
ret = create_hyp_io_mappings(vgic_vctrl_base,
vgic_vctrl_base + resource_size(&vctrl_res),
--
1.9.1
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v6 15/15] virt: arm: support hip04 gic
2014-05-11 8:06 ` [PATCH v6 15/15] virt: arm: support hip04 gic Haojian Zhuang
@ 2014-05-15 9:42 ` Marc Zyngier
2014-05-15 13:09 ` Haojian Zhuang
0 siblings, 1 reply; 50+ messages in thread
From: Marc Zyngier @ 2014-05-15 9:42 UTC (permalink / raw)
To: linux-arm-kernel
On 11/05/14 09:06, Haojian Zhuang wrote:
> 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 <haojian.zhuang@linaro.org>
> ---
> arch/arm/kvm/interrupts_head.S | 32 ++++++++++++++++++++++------
> include/kvm/arm_vgic.h | 5 ++++-
> include/linux/irqchip/arm-gic.h | 6 ++++++
> virt/kvm/arm/vgic.c | 47 +++++++++++++++++++++++++++++------------
> 4 files changed, 69 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> index 76af9302..7aacaff 100644
> --- a/arch/arm/kvm/interrupts_head.S
> +++ b/arch/arm/kvm/interrupts_head.S
> @@ -419,7 +419,10 @@ 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_NR_LR]
Please rename this field to something else, now that it contains more
than the number of LRs.
> + movs r10, r10, lsr #HWCFG_APR_SHIFT
> + ldrne r10, [r2, r10]
> + ldreq r10, [r2, #GICH_APR]
No. Just encode the offset there always. No need for a test.
> str r3, [r11, #VGIC_CPU_HCR]
> str r4, [r11, #VGIC_CPU_VMCR]
> @@ -435,9 +438,16 @@ vcpu .req r0 @ vcpu pointer always in r0
> str r5, [r2, #GICH_HCR]
>
> /* Save list registers */
> - add r2, r2, #GICH_LR0
> - add r3, r11, #VGIC_CPU_LR
> ldr r4, [r11, #VGIC_CPU_NR_LR]
> + addeq r2, r2, #GICH_LR0
> + movne r10, r4, lsr #HWCFG_APR_SHIFT
> + /* the offset between GICH_APR & GICH_LR0 is 0x10 */
> + addne r10, r10, #0x10
> + addne r2, r2, r10
> + add r3, r11, #VGIC_CPU_LR
> + /* Get NR_LR from VGIC_CPU_NR_LR */
> + ldr r6, =HWCFG_NR_LR_MASK
> + and r4, r4, r6
And the above simplifies all this complicated mess.
> 1: ldr r6, [r2], #4
> str r6, [r3], #4
> subs r4, r4, #1
> @@ -469,12 +479,22 @@ 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_NR_LR]
> + movs r6, r6, lsr #HWCFG_APR_SHIFT
> + strne r8, [r2, r6]
> + streq r8, [r2, #GICH_APR]
>
> /* Restore list registers */
> - add r2, r2, #GICH_LR0
> - add r3, r11, #VGIC_CPU_LR
> ldr r4, [r11, #VGIC_CPU_NR_LR]
> + addeq r2, r2, #GICH_LR0
> + movne r6, r4, lsr #HWCFG_APR_SHIFT
> + /* the offset between GICH_APR & GICH_LR0 is 0x10 */
> + addne r6, r6, #0x10
> + addne r2, r2, r6
> + /* Get NR_LR from VGIC_CPU_NR_LR */
> + add r3, r11, #VGIC_CPU_LR
> + ldr r6, =HWCFG_NR_LR_MASK
> + and r4, r4, r6
Same here.
> 1: ldr r6, [r3], #4
> str r6, [r2], #4
> subs r4, r4, #1
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index f27000f..089de25 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -122,7 +122,10 @@ struct vgic_cpu {
> /* Bitmap of used/free list registers */
> DECLARE_BITMAP( lr_used, VGIC_MAX_LRS);
>
> - /* Number of list registers on this CPU */
> + /*
> + * bit[31:16]: GICH_APR offset
> + * bit[15:0]: Number of list registers on this CPU
> + */
> int nr_lr;
Make it a u32 to reflect the encoding, rename it to reflect the change
in functionality.
>
> /* CPU vif control registers for world switch */
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 45e2d8c..5530388 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -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_MASK 0xffff
> +#define HWCFG_APR_SHIFT 16
HWCFG_APR_MASK is wrong, as it should be shifted.
> #ifndef __ASSEMBLY__
>
> struct device_node;
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 47b2983..e635b9b 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -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;
>
> static unsigned int vgic_maint_irq;
>
> @@ -624,9 +624,9 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> int vcpu_id = vcpu->vcpu_id;
> int i, irq, source_cpu;
> - u32 *lr;
> + u32 *lr, nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
Define an accessor (vgic_nr_lr(vcpu)). and use it everywhere you've
introduced this construct.
>
> - for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
> + for_each_set_bit(i, vgic_cpu->lr_used, nr_lr) {
> lr = &vgic_cpu->vgic_lr[i];
> irq = LR_IRQID(*lr);
> source_cpu = LR_CPUID(*lr);
> @@ -1005,8 +1005,9 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
> {
> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> int lr;
> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>
> - for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
> + for_each_set_bit(lr, vgic_cpu->lr_used, nr_lr) {
> int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>
> if (!vgic_irq_is_enabled(vcpu, irq)) {
> @@ -1025,6 +1026,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
> {
> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> int lr;
> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>
> /* Sanitize the input... */
> BUG_ON(sgi_source_id & ~7);
> @@ -1046,9 +1048,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)
> + lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used, nr_lr);
> + if (lr >= nr_lr)
> return false;
>
> kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id);
> @@ -1181,9 +1182,10 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
> * active bit.
> */
> int lr, irq;
> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>
> for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr,
> - vgic_cpu->nr_lr) {
> + nr_lr) {
> irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>
> vgic_irq_clear_active(vcpu, irq);
> @@ -1221,13 +1223,13 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
> int lr, pending;
> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
> bool level_pending;
>
> level_pending = vgic_process_maintenance(vcpu);
>
> /* Clear mappings for empty LRs */
> - for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr,
> - vgic_cpu->nr_lr) {
> + for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr, nr_lr) {
> int irq;
>
> if (!test_and_clear_bit(lr, vgic_cpu->lr_used))
> @@ -1241,8 +1243,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)
> + nr_lr);
> + if (level_pending || pending < nr_lr)
> set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
> }
>
> @@ -1438,7 +1440,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
> */
> vgic_cpu->vgic_vmcr = 0;
>
> - vgic_cpu->nr_lr = vgic_nr_lr;
> + vgic_cpu->nr_lr = vgic_hw_cfg;
> vgic_cpu->vgic_hcr = GICH_HCR_EN; /* Get the show on the road... */
>
> return 0;
> @@ -1470,17 +1472,33 @@ 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",
> + }, {
> + .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;
> + u32 vgic_nr_lr;
>
> - 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;
> }
> + /* If it's standard ARM GIC, high word in vgic_hw_cfg is 0.
> + * Otherwise, it's the offset of non-standard GICH_APR (in HiP04).
> + */
> + vgic_hw_cfg = (unsigned int)match->data << HWCFG_APR_SHIFT;
>
> vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
> if (!vgic_maint_irq) {
> @@ -1517,6 +1535,7 @@ int kvm_vgic_hyp_init(void)
>
> vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
> vgic_nr_lr = (vgic_nr_lr & 0x3f) + 1;
> + vgic_hw_cfg |= vgic_nr_lr;
>
> ret = create_hyp_io_mappings(vgic_vctrl_base,
> vgic_vctrl_base + resource_size(&vctrl_res),
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 15/15] virt: arm: support hip04 gic
2014-05-15 9:42 ` Marc Zyngier
@ 2014-05-15 13:09 ` Haojian Zhuang
2014-05-15 13:26 ` Marc Zyngier
2014-05-20 10:51 ` Christoffer Dall
0 siblings, 2 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-15 13:09 UTC (permalink / raw)
To: linux-arm-kernel
On 15 May 2014 17:42, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 11/05/14 09:06, Haojian Zhuang wrote:
>> 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 <haojian.zhuang@linaro.org>
>> ---
>> arch/arm/kvm/interrupts_head.S | 32 ++++++++++++++++++++++------
>> include/kvm/arm_vgic.h | 5 ++++-
>> include/linux/irqchip/arm-gic.h | 6 ++++++
>> virt/kvm/arm/vgic.c | 47 +++++++++++++++++++++++++++++------------
>> 4 files changed, 69 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
>> index 76af9302..7aacaff 100644
>> --- a/arch/arm/kvm/interrupts_head.S
>> +++ b/arch/arm/kvm/interrupts_head.S
>> @@ -419,7 +419,10 @@ 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_NR_LR]
>
> Please rename this field to something else, now that it contains more
> than the number of LRs.
Since vgic driver is shared between arm & arm64, I only use high word
in VGIC_CPU_NR_LR register if SoC is HiP04. Then I could avoid to
change the vgic implementation in arm64.
Do you want to me change arm64 implementation at the same time?
>
>> + movs r10, r10, lsr #HWCFG_APR_SHIFT
>> + ldrne r10, [r2, r10]
>> + ldreq r10, [r2, #GICH_APR]
>
> No. Just encode the offset there always. No need for a test.
>
It's the same reason above.
>> str r3, [r11, #VGIC_CPU_HCR]
>> str r4, [r11, #VGIC_CPU_VMCR]
>> @@ -435,9 +438,16 @@ vcpu .req r0 @ vcpu pointer always in r0
>> str r5, [r2, #GICH_HCR]
>>
>> /* Save list registers */
>> - add r2, r2, #GICH_LR0
>> - add r3, r11, #VGIC_CPU_LR
>> ldr r4, [r11, #VGIC_CPU_NR_LR]
>> + addeq r2, r2, #GICH_LR0
>> + movne r10, r4, lsr #HWCFG_APR_SHIFT
>> + /* the offset between GICH_APR & GICH_LR0 is 0x10 */
>> + addne r10, r10, #0x10
>> + addne r2, r2, r10
>> + add r3, r11, #VGIC_CPU_LR
>> + /* Get NR_LR from VGIC_CPU_NR_LR */
>> + ldr r6, =HWCFG_NR_LR_MASK
>> + and r4, r4, r6
>
> And the above simplifies all this complicated mess.
>
Same reason.
>> 1: ldr r6, [r2], #4
>> str r6, [r3], #4
>> subs r4, r4, #1
>> @@ -469,12 +479,22 @@ 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_NR_LR]
>> + movs r6, r6, lsr #HWCFG_APR_SHIFT
>> + strne r8, [r2, r6]
>> + streq r8, [r2, #GICH_APR]
>>
>> /* Restore list registers */
>> - add r2, r2, #GICH_LR0
>> - add r3, r11, #VGIC_CPU_LR
>> ldr r4, [r11, #VGIC_CPU_NR_LR]
>> + addeq r2, r2, #GICH_LR0
>> + movne r6, r4, lsr #HWCFG_APR_SHIFT
>> + /* the offset between GICH_APR & GICH_LR0 is 0x10 */
>> + addne r6, r6, #0x10
>> + addne r2, r2, r6
>> + /* Get NR_LR from VGIC_CPU_NR_LR */
>> + add r3, r11, #VGIC_CPU_LR
>> + ldr r6, =HWCFG_NR_LR_MASK
>> + and r4, r4, r6
>
> Same here.
>
Same reason.
>> 1: ldr r6, [r3], #4
>> str r6, [r2], #4
>> subs r4, r4, #1
>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>> index f27000f..089de25 100644
>> --- a/include/kvm/arm_vgic.h
>> +++ b/include/kvm/arm_vgic.h
>> @@ -122,7 +122,10 @@ struct vgic_cpu {
>> /* Bitmap of used/free list registers */
>> DECLARE_BITMAP( lr_used, VGIC_MAX_LRS);
>>
>> - /* Number of list registers on this CPU */
>> + /*
>> + * bit[31:16]: GICH_APR offset
>> + * bit[15:0]: Number of list registers on this CPU
>> + */
>> int nr_lr;
>
> Make it a u32 to reflect the encoding, rename it to reflect the change
> in functionality.
>
OK. I'll change it to u32. And rename it in struct vgic_cpu.
>>
>> /* CPU vif control registers for world switch */
>> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
>> index 45e2d8c..5530388 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -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_MASK 0xffff
>> +#define HWCFG_APR_SHIFT 16
>
> HWCFG_APR_MASK is wrong, as it should be shifted.
>
OK. I'll change it.
>> #ifndef __ASSEMBLY__
>>
>> struct device_node;
>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>> index 47b2983..e635b9b 100644
>> --- a/virt/kvm/arm/vgic.c
>> +++ b/virt/kvm/arm/vgic.c
>> @@ -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;
>>
>> static unsigned int vgic_maint_irq;
>>
>> @@ -624,9 +624,9 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
>> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>> int vcpu_id = vcpu->vcpu_id;
>> int i, irq, source_cpu;
>> - u32 *lr;
>> + u32 *lr, nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>
> Define an accessor (vgic_nr_lr(vcpu)). and use it everywhere you've
> introduced this construct.
>
>>
>> - for_each_set_bit(i, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
>> + for_each_set_bit(i, vgic_cpu->lr_used, nr_lr) {
>> lr = &vgic_cpu->vgic_lr[i];
>> irq = LR_IRQID(*lr);
>> source_cpu = LR_CPUID(*lr);
>> @@ -1005,8 +1005,9 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
>> {
>> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>> int lr;
>> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>>
>> - for_each_set_bit(lr, vgic_cpu->lr_used, vgic_cpu->nr_lr) {
>> + for_each_set_bit(lr, vgic_cpu->lr_used, nr_lr) {
>> int irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>>
>> if (!vgic_irq_is_enabled(vcpu, irq)) {
>> @@ -1025,6 +1026,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
>> {
>> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>> int lr;
>> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>>
>> /* Sanitize the input... */
>> BUG_ON(sgi_source_id & ~7);
>> @@ -1046,9 +1048,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)
>> + lr = find_first_zero_bit((unsigned long *)vgic_cpu->lr_used, nr_lr);
>> + if (lr >= nr_lr)
>> return false;
>>
>> kvm_debug("LR%d allocated for IRQ%d %x\n", lr, irq, sgi_source_id);
>> @@ -1181,9 +1182,10 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
>> * active bit.
>> */
>> int lr, irq;
>> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>>
>> for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_eisr,
>> - vgic_cpu->nr_lr) {
>> + nr_lr) {
>> irq = vgic_cpu->vgic_lr[lr] & GICH_LR_VIRTUALID;
>>
>> vgic_irq_clear_active(vcpu, irq);
>> @@ -1221,13 +1223,13 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
>> struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
>> int lr, pending;
>> + int nr_lr = vgic_cpu->nr_lr & HWCFG_NR_LR_MASK;
>> bool level_pending;
>>
>> level_pending = vgic_process_maintenance(vcpu);
>>
>> /* Clear mappings for empty LRs */
>> - for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr,
>> - vgic_cpu->nr_lr) {
>> + for_each_set_bit(lr, (unsigned long *)vgic_cpu->vgic_elrsr, nr_lr) {
>> int irq;
>>
>> if (!test_and_clear_bit(lr, vgic_cpu->lr_used))
>> @@ -1241,8 +1243,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)
>> + nr_lr);
>> + if (level_pending || pending < nr_lr)
>> set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
>> }
>>
>> @@ -1438,7 +1440,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
>> */
>> vgic_cpu->vgic_vmcr = 0;
>>
>> - vgic_cpu->nr_lr = vgic_nr_lr;
>> + vgic_cpu->nr_lr = vgic_hw_cfg;
>> vgic_cpu->vgic_hcr = GICH_HCR_EN; /* Get the show on the road... */
>>
>> return 0;
>> @@ -1470,17 +1472,33 @@ 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",
>> + }, {
>> + .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;
>> + u32 vgic_nr_lr;
>>
>> - 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;
>> }
>> + /* If it's standard ARM GIC, high word in vgic_hw_cfg is 0.
>> + * Otherwise, it's the offset of non-standard GICH_APR (in HiP04).
>> + */
>> + vgic_hw_cfg = (unsigned int)match->data << HWCFG_APR_SHIFT;
>>
>> vgic_maint_irq = irq_of_parse_and_map(vgic_node, 0);
>> if (!vgic_maint_irq) {
>> @@ -1517,6 +1535,7 @@ int kvm_vgic_hyp_init(void)
>>
>> vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
>> vgic_nr_lr = (vgic_nr_lr & 0x3f) + 1;
>> + vgic_hw_cfg |= vgic_nr_lr;
>>
>> ret = create_hyp_io_mappings(vgic_vctrl_base,
>> vgic_vctrl_base + resource_size(&vctrl_res),
>>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 15/15] virt: arm: support hip04 gic
2014-05-15 13:09 ` Haojian Zhuang
@ 2014-05-15 13:26 ` Marc Zyngier
2014-07-01 8:57 ` Haojian Zhuang
2014-05-20 10:51 ` Christoffer Dall
1 sibling, 1 reply; 50+ messages in thread
From: Marc Zyngier @ 2014-05-15 13:26 UTC (permalink / raw)
To: linux-arm-kernel
On 15/05/14 14:09, Haojian Zhuang wrote:
> On 15 May 2014 17:42, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 11/05/14 09:06, Haojian Zhuang wrote:
>>> 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 <haojian.zhuang@linaro.org>
>>> ---
>>> arch/arm/kvm/interrupts_head.S | 32 ++++++++++++++++++++++------
>>> include/kvm/arm_vgic.h | 5 ++++-
>>> include/linux/irqchip/arm-gic.h | 6 ++++++
>>> virt/kvm/arm/vgic.c | 47 +++++++++++++++++++++++++++++------------
>>> 4 files changed, 69 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
>>> index 76af9302..7aacaff 100644
>>> --- a/arch/arm/kvm/interrupts_head.S
>>> +++ b/arch/arm/kvm/interrupts_head.S
>>> @@ -419,7 +419,10 @@ 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_NR_LR]
>>
>> Please rename this field to something else, now that it contains more
>> than the number of LRs.
>
> Since vgic driver is shared between arm & arm64, I only use high word
> in VGIC_CPU_NR_LR register if SoC is HiP04. Then I could avoid to
> change the vgic implementation in arm64.
>
> Do you want to me change arm64 implementation at the same time?
Is there anything that guarantees we'll never see the same GIC connected
to an arm64 CPU? Probably not. So you might as well do it completely. At
the minimum, mask out the top bits of the nr_lr word.
Also, there is a number of things that are not quite clear about this
GICH implementation. Given how extensive the changes are in GICD, do the
LRs have the exact same format as GICv2 (i.e. supporting only 8 vcpus,
1020 interrupts)?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 15/15] virt: arm: support hip04 gic
2014-05-15 13:26 ` Marc Zyngier
@ 2014-07-01 8:57 ` Haojian Zhuang
0 siblings, 0 replies; 50+ messages in thread
From: Haojian Zhuang @ 2014-07-01 8:57 UTC (permalink / raw)
To: linux-arm-kernel
On 15 May 2014 21:26, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 15/05/14 14:09, Haojian Zhuang wrote:
>> On 15 May 2014 17:42, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> On 11/05/14 09:06, Haojian Zhuang wrote:
>>>> 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 <haojian.zhuang@linaro.org>
>>>> ---
>>>> arch/arm/kvm/interrupts_head.S | 32 ++++++++++++++++++++++------
>>>> include/kvm/arm_vgic.h | 5 ++++-
>>>> include/linux/irqchip/arm-gic.h | 6 ++++++
>>>> virt/kvm/arm/vgic.c | 47 +++++++++++++++++++++++++++++------------
>>>> 4 files changed, 69 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
>>>> index 76af9302..7aacaff 100644
>>>> --- a/arch/arm/kvm/interrupts_head.S
>>>> +++ b/arch/arm/kvm/interrupts_head.S
>>>> @@ -419,7 +419,10 @@ 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_NR_LR]
>>>
>>> Please rename this field to something else, now that it contains more
>>> than the number of LRs.
>>
>> Since vgic driver is shared between arm & arm64, I only use high word
>> in VGIC_CPU_NR_LR register if SoC is HiP04. Then I could avoid to
>> change the vgic implementation in arm64.
>>
>> Do you want to me change arm64 implementation at the same time?
>
> Is there anything that guarantees we'll never see the same GIC connected
> to an arm64 CPU? Probably not. So you might as well do it completely. At
> the minimum, mask out the top bits of the nr_lr word.
>
I checked that this GIC is only connected to an arm32 CPU. So we don't need
to worry about the same offset issue on arm64.
> Also, there is a number of things that are not quite clear about this
> GICH implementation. Given how extensive the changes are in GICD, do the
> LRs have the exact same format as GICv2 (i.e. supporting only 8 vcpus,
> 1020 interrupts)?
In HiP04 GICH_LR0, the CPUID is in bit[13:10]. Other bits are same as GICv2.
If I only support 4 or 8 vcpus, I needn't to change code to support
it. Am I right?
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 15/15] virt: arm: support hip04 gic
2014-05-15 13:09 ` Haojian Zhuang
2014-05-15 13:26 ` Marc Zyngier
@ 2014-05-20 10:51 ` Christoffer Dall
1 sibling, 0 replies; 50+ messages in thread
From: Christoffer Dall @ 2014-05-20 10:51 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 15, 2014 at 09:09:05PM +0800, Haojian Zhuang wrote:
> On 15 May 2014 17:42, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 11/05/14 09:06, Haojian Zhuang wrote:
> >> 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 <haojian.zhuang@linaro.org>
> >> ---
> >> arch/arm/kvm/interrupts_head.S | 32 ++++++++++++++++++++++------
> >> include/kvm/arm_vgic.h | 5 ++++-
> >> include/linux/irqchip/arm-gic.h | 6 ++++++
> >> virt/kvm/arm/vgic.c | 47 +++++++++++++++++++++++++++++------------
> >> 4 files changed, 69 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S
> >> index 76af9302..7aacaff 100644
> >> --- a/arch/arm/kvm/interrupts_head.S
> >> +++ b/arch/arm/kvm/interrupts_head.S
> >> @@ -419,7 +419,10 @@ 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_NR_LR]
> >
> > Please rename this field to something else, now that it contains more
> > than the number of LRs.
>
> Since vgic driver is shared between arm & arm64, I only use high word
> in VGIC_CPU_NR_LR register if SoC is HiP04. Then I could avoid to
> change the vgic implementation in arm64.
>
> Do you want to me change arm64 implementation at the same time?
>
> >
> >> + movs r10, r10, lsr #HWCFG_APR_SHIFT
> >> + ldrne r10, [r2, r10]
> >> + ldreq r10, [r2, #GICH_APR]
> >
> > No. Just encode the offset there always. No need for a test.
> >
We don't want all these conditionals in the critical path and it makes
the code impossible to read, please follow Marc's advice.
Thanks,
-Christoffer
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 00/15] enable HiP04 SoC
2014-05-11 8:05 [PATCH v6 00/15] enable HiP04 SoC Haojian Zhuang
` (14 preceding siblings ...)
2014-05-11 8:06 ` [PATCH v6 15/15] virt: arm: support hip04 gic Haojian Zhuang
@ 2014-05-14 18:37 ` Arnd Bergmann
2014-05-15 9:20 ` Haojian Zhuang
15 siblings, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2014-05-14 18:37 UTC (permalink / raw)
To: linux-arm-kernel
On Sunday 11 May 2014 16:05:56 Haojian Zhuang wrote:
> Changelog:
> v6:
> * Split HiP04 enabling patch into patches on document, mcpm & hiP04.
> * Move reset operation in HiP04 MCPM implementation.
> * Remove ARCH_MULTI_V7_NONLPAE & ARCH_MULTI_V7_LPAE according to olof's
> comment.
The latest version looks good to me, at least if all the remaining
comments for the mcpm implementation have been addressed.
I would suggest to split the series into two, and submit all the gic
related changes through Russell's patch tracker but have a pull
request for the platform changes.
As far as I can tell, there are no compile-time dependencies between
the two, so we don't get a broken build during bisection, but
of course the platform will only be usable if both parts are merged.
Arnd
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 00/15] enable HiP04 SoC
2014-05-14 18:37 ` [PATCH v6 00/15] enable HiP04 SoC Arnd Bergmann
@ 2014-05-15 9:20 ` Haojian Zhuang
2014-05-15 9:31 ` Marc Zyngier
0 siblings, 1 reply; 50+ messages in thread
From: Haojian Zhuang @ 2014-05-15 9:20 UTC (permalink / raw)
To: linux-arm-kernel
On 15 May 2014 02:37, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday 11 May 2014 16:05:56 Haojian Zhuang wrote:
>> Changelog:
>> v6:
>> * Split HiP04 enabling patch into patches on document, mcpm & hiP04.
>> * Move reset operation in HiP04 MCPM implementation.
>> * Remove ARCH_MULTI_V7_NONLPAE & ARCH_MULTI_V7_LPAE according to olof's
>> comment.
>
> The latest version looks good to me, at least if all the remaining
> comments for the mcpm implementation have been addressed.
>
> I would suggest to split the series into two, and submit all the gic
> related changes through Russell's patch tracker but have a pull
> request for the platform changes.
>
> As far as I can tell, there are no compile-time dependencies between
> the two, so we don't get a broken build during bisection, but
> of course the platform will only be usable if both parts are merged.
>
> Arnd
Hi Arnd,
Thanks. I'll submit gic patches into Russell's patch tracker.
By the way, will anybody give the Acked-by or Reviewed-by on these
patches? I already received the comments without any Acked-by.
Regards
Haojian
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 00/15] enable HiP04 SoC
2014-05-15 9:20 ` Haojian Zhuang
@ 2014-05-15 9:31 ` Marc Zyngier
2014-05-16 5:07 ` Jason Cooper
0 siblings, 1 reply; 50+ messages in thread
From: Marc Zyngier @ 2014-05-15 9:31 UTC (permalink / raw)
To: linux-arm-kernel
On 15/05/14 10:20, Haojian Zhuang wrote:
> On 15 May 2014 02:37, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Sunday 11 May 2014 16:05:56 Haojian Zhuang wrote:
>>> Changelog:
>>> v6:
>>> * Split HiP04 enabling patch into patches on document, mcpm & hiP04.
>>> * Move reset operation in HiP04 MCPM implementation.
>>> * Remove ARCH_MULTI_V7_NONLPAE & ARCH_MULTI_V7_LPAE according to olof's
>>> comment.
>>
>> The latest version looks good to me, at least if all the remaining
>> comments for the mcpm implementation have been addressed.
>>
>> I would suggest to split the series into two, and submit all the gic
>> related changes through Russell's patch tracker but have a pull
>> request for the platform changes.
>>
>> As far as I can tell, there are no compile-time dependencies between
>> the two, so we don't get a broken build during bisection, but
>> of course the platform will only be usable if both parts are merged.
>>
>> Arnd
>
> Hi Arnd,
>
> Thanks. I'll submit gic patches into Russell's patch tracker.
Please hold on on the GIC stuff, I still have comments on the main
changes (I should have sent them earlier), and the KVM stuff is still
quite wrong.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 00/15] enable HiP04 SoC
2014-05-15 9:31 ` Marc Zyngier
@ 2014-05-16 5:07 ` Jason Cooper
2014-05-16 7:57 ` Marc Zyngier
0 siblings, 1 reply; 50+ messages in thread
From: Jason Cooper @ 2014-05-16 5:07 UTC (permalink / raw)
To: linux-arm-kernel
Haojian,
On Thu, May 15, 2014 at 10:31:30AM +0100, Marc Zyngier wrote:
> On 15/05/14 10:20, Haojian Zhuang wrote:
> > On 15 May 2014 02:37, Arnd Bergmann <arnd@arndb.de> wrote:
> >> On Sunday 11 May 2014 16:05:56 Haojian Zhuang wrote:
> >>> Changelog:
> >>> v6:
> >>> * Split HiP04 enabling patch into patches on document, mcpm & hiP04.
> >>> * Move reset operation in HiP04 MCPM implementation.
> >>> * Remove ARCH_MULTI_V7_NONLPAE & ARCH_MULTI_V7_LPAE according to olof's
> >>> comment.
> >>
> >> The latest version looks good to me, at least if all the remaining
> >> comments for the mcpm implementation have been addressed.
> >>
> >> I would suggest to split the series into two, and submit all the gic
> >> related changes through Russell's patch tracker but have a pull
> >> request for the platform changes.
> >>
> >> As far as I can tell, there are no compile-time dependencies between
> >> the two, so we don't get a broken build during bisection, but
> >> of course the platform will only be usable if both parts are merged.
> >>
> >> Arnd
> >
> > Hi Arnd,
> >
> > Thanks. I'll submit gic patches into Russell's patch tracker.
>
> Please hold on on the GIC stuff, I still have comments on the main
> changes (I should have sent them earlier), and the KVM stuff is still
> quite wrong.
I've just started assisting Thomas with review / wrangling patches for
drivers/irqchip. If you could Cc me on the next revision, I'd appreciate
it.
thx,
Jason.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 00/15] enable HiP04 SoC
2014-05-16 5:07 ` Jason Cooper
@ 2014-05-16 7:57 ` Marc Zyngier
2014-05-16 13:31 ` Jason Cooper
2014-05-16 19:09 ` Jason Cooper
0 siblings, 2 replies; 50+ messages in thread
From: Marc Zyngier @ 2014-05-16 7:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jason,
> I've just started assisting Thomas with review / wrangling patches
> for
> drivers/irqchip. If you could Cc me on the next revision, I'd
> appreciate
> it.
That's excellent news! If you didn't have enough on your plate already,
feel free to have a look at the GICv3 patches I reposted yesterday! :-)
Thanks,
M.
--
Fast, cheap, reliable. Pick two.
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v6 00/15] enable HiP04 SoC
2014-05-16 7:57 ` Marc Zyngier
@ 2014-05-16 13:31 ` Jason Cooper
2014-05-16 19:09 ` Jason Cooper
1 sibling, 0 replies; 50+ messages in thread
From: Jason Cooper @ 2014-05-16 13:31 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, May 16, 2014 at 08:57:53AM +0100, Marc Zyngier wrote:
> Hi Jason,
>
> >I've just started assisting Thomas with review / wrangling patches
> >for
> >drivers/irqchip. If you could Cc me on the next revision, I'd
> >appreciate
> >it.
>
> That's excellent news! If you didn't have enough on your plate
> already, feel free to have a look at the GICv3 patches I reposted
> yesterday! :-)
During my first skim, I had noted to put Haojian in touch with you since
you were both working on GIC for >8 cores. Then I saw you were already
there. :)
This is the first I've looked at irq-gic.c, (mvebu uses irq-orion.c and
irq-armada-370-xp.c) I'm going to try to bring myself up to speed this
weekend and then post some more thoughtful comments.
I know it sucks when you've reached v6 of a series and someone jumps in
out of the blue with changes and suggestions. So I'll try to do it in
one go. Like pulling off a band-aid.
thx,
Jason.
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v6 00/15] enable HiP04 SoC
2014-05-16 7:57 ` Marc Zyngier
2014-05-16 13:31 ` Jason Cooper
@ 2014-05-16 19:09 ` Jason Cooper
1 sibling, 0 replies; 50+ messages in thread
From: Jason Cooper @ 2014-05-16 19:09 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, May 16, 2014 at 08:57:53AM +0100, Marc Zyngier wrote:
> Hi Jason,
>
> >I've just started assisting Thomas with review / wrangling patches
> >for
> >drivers/irqchip. If you could Cc me on the next revision, I'd
> >appreciate
> >it.
>
> That's excellent news! If you didn't have enough on your plate
> already, feel free to have a look at the GICv3 patches I reposted
> yesterday! :-)
During my first skim, I had noted to put Haojian in touch with you since
you were both working on GIC for >8 cores. Then I saw you were already
there. :)
This is the first I've looked at irq-gic.c, (mvebu uses irq-orion.c and
irq-armada-370-xp.c) I'm going to try to bring myself up to speed this
weekend and then post some more thoughtful comments.
I know it sucks when you've reached v6 of a series and someone jumps in
out of the blue with changes and suggestions. So I'll try to do it in
one go. Like pulling off a band-aid.
thx,
Jason.
^ permalink raw reply [flat|nested] 50+ messages in thread