[PATCH v17 2/9] ARM: hisi: enable MCPM implementation
Haojian Zhuang
haojian.zhuang at linaro.org
Fri Aug 8 06:12:55 PDT 2014
On 7 August 2014 23:29, Nicolas Pitre <nicolas.pitre at linaro.org> wrote:
> On Thu, 7 Aug 2014, Haojian Zhuang wrote:
>
>> Multiple CPU clusters are used in Hisilicon HiP04 SoC. Now use MCPM
>> framework to manage power on HiP04 SoC.
>>
>> Changelog:
>> v17:
>> * Parse bootwrapper parameters in DTS file.
>> * Fix to use msleep() in spinlock region.
>> v16:
>> * Parse bootwrapper parameters in command line instead.
>> v13:
>> * Restore power down operation in MCPM.
>> * Fix disabling snoop filter issue in MCPM.
>> v12:
>> * Use wfi as power down state in MCPM.
>> * Remove wait_for_powerdown() in MCPM because wfi is used now.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang at linaro.org>
>
> This still has issues.
>
>
>
>> ---
>> arch/arm/mach-hisi/Makefile | 1 +
>> arch/arm/mach-hisi/platmcpm.c | 362 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 363 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 ee2506b..d64831e 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 headsmp.o
>> diff --git a/arch/arm/mach-hisi/platmcpm.c b/arch/arm/mach-hisi/platmcpm.c
>> new file mode 100644
>> index 0000000..2eabd52
>> --- /dev/null
>> +++ b/arch/arm/mach-hisi/platmcpm.c
>> @@ -0,0 +1,362 @@
>> +/*
>> + * 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/memblock.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 u32 fabric_phys_addr;
>> +/*
>> + * [0]: bootwrapper physical address
>> + * [1]: bootwrapper size
>> + * [2]: relocation address
>> + * [3]: relocation size
>> + */
>> +static u32 hip04_boot_method[4];
>> +
>> +static bool hip04_cluster_is_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)
>> + 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;
>> + }
>> + return;
>> +}
>> +
>> +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);
>> +
>> + if (hip04_cpu_table[cluster][cpu])
>> + goto out;
>> +
>> + /* Make secondary core out of reset. */
>> + writel_relaxed(hip04_boot_method[0], relocation);
>> + writel_relaxed(0xa5a5a5a5, relocation + 4); /* magic number */
>> + writel_relaxed(virt_to_phys(mcpm_entry_point), relocation + 8);
>> + writel_relaxed(0, relocation + 12);
>> +
>> + if (hip04_cluster_is_down(cluster)) {
>> + data = CLUSTER_DEBUG_RESET_BIT;
>> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> + do {
>> + cpu_relax();
>> + mask = CLUSTER_DEBUG_RESET_STATUS;
>> + data = readl_relaxed(sysctrl + \
>> + SC_CPU_RESET_STATUS(cluster));
>> + } while (data & mask);
>> + }
>> +
>> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> + CORE_DEBUG_RESET_BIT(cpu);
>> + writel_relaxed(data, sysctrl + SC_CPU_RESET_DREQ(cluster));
>> +out:
>> + hip04_cpu_table[cluster][cpu]++;
>> + spin_unlock_irq(&boot_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static void hip04_mcpm_power_down(void)
>> +{
>> + unsigned int mpidr, cpu, cluster;
>> + bool skip_wfi = 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_wfi = true;
>> + } else if (hip04_cpu_table[cluster][cpu] > 1) {
>> + pr_err("Cluster %d CPU%d boots multiple times\n", cluster, cpu);
>> + BUG();
>> + }
>> + spin_unlock(&boot_lock);
>> +
>> + if (hip04_cluster_is_down(cluster))
>> + v7_exit_coherency_flush(all);
>> + else
>> + v7_exit_coherency_flush(louis);
>
> This is not sufficient. To be safe, the MCPM cluster state must be
> controlled as well. And once the cache is flushed, the snoops must be
> disabled as well. And this should be controlled with the boot_lock
> still held. In other words here's what it should look like:
>
> if (hip04_cluster_is_down(cluster) &&
> __mcpm_outbound_enter_critical(cpu, cluster)) {
> spin_unlock(&boot_lock);
> v7_exit_coherency_flush(all);
> hip04_set_snoop_filter(cluster, 0);
> } else {
> spin_unlock(&boot_lock);
> v7_exit_coherency_flush(louis);
> }
>
I can make cpu online/offline successfully in these two cases.
1. Add msleep() in the end of power_up and move disabling snoop filter at here.
2. Disable snoop filter in wait_for_power_down after target core in WFI status.
Since case #1 isn't good, I choose the #2.
I also got some hisilicon documents. They said that setting target
core to WFI status is required before disabling snoop filter.
>> +
>> + __mcpm_cpu_down(cpu, cluster);
>> +
>> + if (!skip_wfi)
>> + wfi();
>> +}
>> +
>> +static int hip04_mcpm_wait_for_powerdown(unsigned int cpu, unsigned int cluster)
>> +{
>> + unsigned int data, tries, count;
>> +
>> + BUG_ON(cluster >= HIP04_MAX_CLUSTERS ||
>> + cpu >= HIP04_MAX_CPUS_PER_CLUSTER);
>> +
>> + count = TIMEOUT_MSEC / POLL_MSEC;
>> + spin_lock_irq(&boot_lock);
>> + for (tries = 0; tries < count; tries++) {
>> + data = readl_relaxed(sysctrl + SC_CPU_RESET_STATUS(cluster));
>> + if (!(data & CORE_WFI_STATUS(cpu))) {
>> + spin_unlock_irq(&boot_lock);
>> + msleep(POLL_MSEC);
>> + spin_lock_irq(&boot_lock);
>> + continue;
>
> The "continue" is redundant here.
>
I missed "break" in this version. If I got expected value, I should
break current loop.
> Also, as I mentioned in my previous review, you should verify that the
> value of hip04_cpu_table[cluster][cpu] is still 0 everytime the lock is
> taken, and bail out (with -EBUSY in that case instead of -ETIMEOUT) if
> not. I'd do it first thing in the loop.
>
OK
>> + }
>> + }
>> + if (tries >= count)
>> + goto err;
>> + data = CORE_RESET_BIT(cpu) | NEON_RESET_BIT(cpu) | \
>> + CORE_DEBUG_RESET_BIT(cpu);
>> + writel_relaxed(data, sysctrl + SC_CPU_RESET_REQ(cluster));
>> + for (tries = 0; tries < count; tries++) {
>> + data = readl_relaxed(sysctrl + SC_CPU_RESET_STATUS(cluster));
>> + if (!(data & CORE_RESET_STATUS(cpu))) {
>> + spin_unlock_irq(&boot_lock);
>> + msleep(POLL_MSEC);
>> + spin_lock_irq(&boot_lock);
>> + continue;
>> + }
>
> Ditto here.
>
OK
>> + }
>> + if (tries >= count)
>> + goto err;
>> + if (hip04_cluster_is_down(cluster))
>> + hip04_set_snoop_filter(cluster, 0);
>
> That won't be needed anymore.
>
> Also, I want a comment explaining why the CPU reset is handled in this
> function. I asked for that and explained why in my previous review as
> well.
>
I must need this at here. All cores in one cluster should be in WFI
status first. Then I could make them in reset mode. At last, I could
disable the snoop filter.
>> + spin_unlock_irq(&boot_lock);
>> + return 0;
>> +err:
>> + spin_unlock_irq(&boot_lock);
>> + 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);
>
> The value for hip04_cpu_table[cluster][cpu] must be set back to 1
> if it is still 0 here.
>
OK
>> + spin_unlock(&boot_lock);
>> +}
>> +
>> +static void __naked hip04_mcpm_power_up_setup(unsigned int affinity_level)
>> +{
>> + asm volatile (" \n"
>> +" cmp r0, #0 \n"
>> +" bxeq lr \n"
>> + /* calculate fabric phys address */
>> +" adr r2, 2f \n"
>> +" ldmia r2, {r1, r3} \n"
>> +" sub r0, r2, r1 \n"
>> +" ldr r2, [r0, r3] \n"
>> + /* get cluster id from MPIDR */
>> +" mrc p15, 0, r0, c0, c0, 5 \n"
>> +" ubfx r1, r0, #8, #8 \n"
>> + /* 1 << cluster id */
>> +" mov r0, #1 \n"
>> +" mov r3, r0, lsl r1 \n"
>> +" ldr r0, [r2, #"__stringify(FAB_SF_MODE)"] \n"
>> +" tst r0, r3 \n"
>> +" bxne lr \n"
>> +" orr r1, r0, r3 \n"
>> +" str r1, [r2, #"__stringify(FAB_SF_MODE)"] \n"
>> +"1: ldr r0, [r2, #"__stringify(FAB_SF_MODE)"] \n"
>> +" tst r0, r3 \n"
>> +" beq 1b \n"
>> +" bx lr \n"
>> +
>> +" .align 2 \n"
>> +"2: .word . \n"
>> +" .word fabric_phys_addr \n"
>> + );
>> +}
>> +
>> +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_sctl, *np_fab;
>> + struct resource fab_res;
>> + int ret = -ENODEV;
>> +
>> + np = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-bootwrapper");
>> + if (!np)
>> + goto err;
>> + ret = of_property_read_u32_array(np, "boot-method",
>> + &hip04_boot_method[0], 4);
>> + if (ret)
>> + goto err;
>> + np_sctl = of_find_compatible_node(NULL, NULL, "hisilicon,sysctrl");
>> + if (!np_sctl)
>> + goto err;
>> + np_fab = of_find_compatible_node(NULL, NULL, "hisilicon,hip04-fabric");
>> + if (!np_fab)
>> + goto err;
>> +
>> + ret = memblock_reserve(hip04_boot_method[0], hip04_boot_method[1]);
>> + if (ret)
>> + goto err;
>> +
>> + relocation = ioremap(hip04_boot_method[2], hip04_boot_method[3]);
>> + if (!relocation) {
>> + pr_err("failed to map relocation space\n");
>> + ret = -ENOMEM;
>> + goto err_reloc;
>> + }
>> + sysctrl = of_iomap(np_sctl, 0);
>> + if (!sysctrl) {
>> + pr_err("failed to get sysctrl base\n");
>> + ret = -ENOMEM;
>> + goto err_sysctrl;
>> + }
>> + ret = of_address_to_resource(np_fab, 0, &fab_res);
>> + if (ret) {
>> + pr_err("failed to get fabric base phys\n");
>> + goto err_fabric;
>> + }
>> + fabric_phys_addr = fab_res.start;
>> + sync_cache_w(&fabric_phys_addr);
>> + 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;
>
> Shouldn't you free allocated resources here as well, as you do with
> err_fabric and the others?
>
>> + ret = mcpm_platform_register(&hip04_mcpm_ops);
>> + if (!ret) {
>> + mcpm_sync_init(hip04_mcpm_power_up_setup);
>> + mcpm_smp_set_ops();
>> + pr_info("HiP04 MCPM initialized\n");
>> + }
>> + return ret;
>
> Same here in case of error.
>
>
>> +err_fabric:
>> + iounmap(sysctrl);
>> +err_sysctrl:
>> + iounmap(relocation);
>> +err_reloc:
>> + memblock_free(hip04_boot_method[0], hip04_boot_method[1]);
>> +err:
>> + return ret;
>> +}
>> +early_initcall(hip04_mcpm_init);
>> --
>> 1.9.1
>>
>>
More information about the linux-arm-kernel
mailing list