[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <520C07AD.9040003@codeaurora.org>
Date: Wed, 14 Aug 2013 15:41:49 -0700
From: Rohit Vaswani <rvaswani@...eaurora.org>
To: Kumar Gala <galak@...eaurora.org>
CC: David Brown <davidb@...eaurora.org>,
Rob Herring <rob.herring@...xeda.com>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Stephen Warren <swarren@...dotorg.org>,
Ian Campbell <ian.campbell@...rix.com>,
Rob Landley <rob@...dley.net>,
Russell King <linux@....linux.org.uk>,
Daniel Walker <dwalker@...o99.com>,
Bryan Huntsman <bryanh@...eaurora.org>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Nicolas Pitre <nico@...aro.org>, devicetree@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 3/4] ARM: msm: Add SMP support for 8960
On 8/2/2013 8:43 AM, Kumar Gala wrote:
> On Aug 1, 2013, at 9:15 PM, Rohit Vaswani wrote:
>
>> Add the cpus bindings and the Krait release sequence
>> to make SMP work for MSM8960
>>
>> Signed-off-by: Rohit Vaswani <rvaswani@...eaurora.org>
>> ---
>> Documentation/devicetree/bindings/arm/cpus.txt | 2 +
>> Documentation/devicetree/bindings/arm/msm/kpss.txt | 16 ++++++
>> arch/arm/boot/dts/msm8960-cdp.dts | 22 +++++++++
>> arch/arm/mach-msm/platsmp.c | 57 ++++++++++++++++++++++
>> arch/arm/mach-msm/scm-boot.h | 8 +--
>> 5 files changed, 102 insertions(+), 3 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/arm/msm/kpss.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
>> index 327aad2..1132eac 100644
>> --- a/Documentation/devicetree/bindings/arm/cpus.txt
>> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
>> @@ -45,11 +45,13 @@ For the ARM architecture every CPU node must contain the following properties:
>> "marvell,xsc3"
>> "marvell,xscale"
>> "qcom,scorpion"
>> + "qcom,krait"
>> - enable-method: Specifies the method used to enable or take the secondary cores
>> out of reset. This allows different reset sequence for
>> different types of cpus.
>> This should be one of:
>> "qcom,scss"
>> + "qcom,kpssv1"
>>
>> Example:
>>
>> diff --git a/Documentation/devicetree/bindings/arm/msm/kpss.txt b/Documentation/devicetree/bindings/arm/msm/kpss.txt
>> new file mode 100644
>> index 0000000..7272340
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/msm/kpss.txt
>> @@ -0,0 +1,16 @@
>> +* KPSS - Krait Processor Sub-system
>> +
>> +Properties
>> +
>> +- compatible : Should contain "qcom,kpss".
>> +
>> +- reg: Specifies the base address for the KPSS registers used for
>> + booting up secondary cores.
>> +
>> +Example:
>> +
>> + kpss@...8000 {
>> + compatible = "qcom,kpss";
>> + reg = <0x02088000 0x1000
>> + 0x02098000 0x2000>;
>> + };
>> diff --git a/arch/arm/boot/dts/msm8960-cdp.dts b/arch/arm/boot/dts/msm8960-cdp.dts
>> index db2060c..8c82d5e 100644
>> --- a/arch/arm/boot/dts/msm8960-cdp.dts
>> +++ b/arch/arm/boot/dts/msm8960-cdp.dts
>> @@ -7,6 +7,22 @@
>> compatible = "qcom,msm8960-cdp", "qcom,msm8960";
>> interrupt-parent = <&intc>;
>>
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "qcom,krait";
>> + device_type = "cpu";
>> + enable-method = "qcom,kpssv1";
>> +
>> + cpu@0 {
>> + reg = <0>;
>> + };
>> +
>> + cpu@1 {
>> + reg = <1>;
>> + };
>> + };
>> +
>> intc: interrupt-controller@...0000 {
>> compatible = "qcom,msm-qgic2";
>> interrupt-controller;
>> @@ -37,6 +53,12 @@
>> reg = <0xfd510000 0x4000>;
>> };
>>
>> + kpss@...8000 {
>> + compatible = "qcom,kpss";
>> + reg = <0x02088000 0x1000
>> + 0x02098000 0x2000>;
>> + };
>> +
>> serial@...40000 {
>> compatible = "qcom,msm-hsuart", "qcom,msm-uart";
>> reg = <0x16440000 0x1000>,
>> diff --git a/arch/arm/mach-msm/platsmp.c b/arch/arm/mach-msm/platsmp.c
>> index 17022e0..82eb079 100644
>> --- a/arch/arm/mach-msm/platsmp.c
>> +++ b/arch/arm/mach-msm/platsmp.c
>> @@ -74,6 +74,56 @@ static int scorpion_release_secondary(void)
>> return 0;
>> }
>>
>> +static int msm8960_release_secondary(unsigned int cpu)
>> +{
>> + void __iomem *reg;
>> + struct device_node *dn = NULL;
>> +
>> + if (cpu == 0 || cpu >= num_possible_cpus())
>> + return -EINVAL;
>> +
>> + dn = of_find_compatible_node(dn, NULL, "qcom,kpss");
>> + if (!dn) {
>> + pr_err("%s : Missing kpss node from device tree\n", __func__);
>> + return -ENXIO;
>> + }
>> +
>> + reg = of_iomap(dn, cpu);
>> + if (!reg)
>> + return -ENOMEM;
>> +
>> + pr_debug("Starting secondary CPU %d\n", cpu);
>> +
>> + /* Turn on CPU Rail */
>> + writel_relaxed(0xA4, reg+0x1014);
> Is there some reason we are using magic numbers for both values and offsets?
Yes. The names are not defined by hardware spec. Would you prefer having
multiple #defines for each value, register ?
>
>> + mb();
>> + udelay(512);
>> +
>> + /* Krait bring-up sequence */
>> + writel_relaxed(0x109, reg+0x04);
>> + writel_relaxed(0x101, reg+0x04);
>> + mb();
>> + ndelay(300);
>> +
>> + writel_relaxed(0x121, reg+0x04);
>> + mb();
>> + udelay(2);
>> +
>> + writel_relaxed(0x120, reg+0x04);
>> + mb();
>> + udelay(2);
>> +
>> + writel_relaxed(0x100, reg+0x04);
>> + mb();
>> + udelay(100);
>> +
>> + writel_relaxed(0x180, reg+0x04);
>> + mb();
>> +
>> + iounmap(reg);
>> + return 0;
>> +}
>> +
>> static DEFINE_PER_CPU(int, cold_boot_done);
>>
>> static void boot_cold_cpu(unsigned int cpu)
>> @@ -96,6 +146,11 @@ static void boot_cold_cpu(unsigned int cpu)
>> scorpion_release_secondary();
>> per_cpu(cold_boot_done, cpu) = true;
>> }
>> + } else if (!strcmp(enable_method, "qcom,kpssv1")) {
>> + if (per_cpu(cold_boot_done, cpu) == false) {
>> + msm8960_release_secondary(cpu);
> Is this really msm8960 specific, if so than we should be doing something other than comparing against "qcom,kpssv1" or we should change the function to kpssv1_release_secondary()
Will do.
>
>> + per_cpu(cold_boot_done, cpu) = true;
>> + }
>> } else {
>> pr_err("%s: Invalid enable-method property: %s\n",
>> __func__, enable_method);
>> @@ -151,6 +206,8 @@ static void __init msm_smp_init_cpus(void)
>> static const int cold_boot_flags[] __initconst = {
>> 0,
>> SCM_FLAG_COLDBOOT_CPU1,
>> + SCM_FLAG_COLDBOOT_CPU2,
>> + SCM_FLAG_COLDBOOT_CPU3,
>> };
>>
>> static void __init msm_smp_prepare_cpus(unsigned int max_cpus)
>> diff --git a/arch/arm/mach-msm/scm-boot.h b/arch/arm/mach-msm/scm-boot.h
>> index 7be32ff..6aabb24 100644
>> --- a/arch/arm/mach-msm/scm-boot.h
>> +++ b/arch/arm/mach-msm/scm-boot.h
>> @@ -13,9 +13,11 @@
>> #define __MACH_SCM_BOOT_H
>>
>> #define SCM_BOOT_ADDR 0x1
>> -#define SCM_FLAG_COLDBOOT_CPU1 0x1
>> -#define SCM_FLAG_WARMBOOT_CPU1 0x2
>> -#define SCM_FLAG_WARMBOOT_CPU0 0x4
>> +#define SCM_FLAG_COLDBOOT_CPU1 0x01
>> +#define SCM_FLAG_COLDBOOT_CPU2 0x08
>> +#define SCM_FLAG_COLDBOOT_CPU3 0x20
>> +#define SCM_FLAG_WARMBOOT_CPU0 0x04
>> +#define SCM_FLAG_WARMBOOT_CPU1 0x02
>>
>> int scm_set_boot_addr(phys_addr_t addr, int flags);
>>
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> hosted by The Linux Foundation
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
Rohit Vaswani
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists