On Tue, Nov 12, 2013 at 12:49 AM, Haojian Zhuang
<haojian.zhuang@xxxxxxxxxx> wrote:
> On 7 November 2013 18:30, Dinh Nguyen <dinh.linux@xxxxxxxxx> wrote:
>>
>> On 11/7/13 2:41 AM, Haojian Zhuang wrote:
>>> From: Zhangfei Gao <zhangfei.gao@xxxxxxxxxx>
>>>
>>> Enable SMP support on hi3xxx platform
>>>
>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@xxxxxxxxxx>
>>> Tested-by: Zhang Mingjun <zhang.mingjun@xxxxxxxxxx>
>>> Tested-by: Li Xin <li.xin@xxxxxxxxxx>
>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@xxxxxxxxxx>
>>> ---
>>> .../bindings/arm/hisilicon/hisilicon.txt | 30 ++++++--
>>> arch/arm/boot/dts/hi3620.dtsi | 38 ++++++++++
>>> arch/arm/mach-hi3xxx/Kconfig | 3 +
>>> arch/arm/mach-hi3xxx/Makefile | 1 +
>>> arch/arm/mach-hi3xxx/core.h | 11 +++
>>> arch/arm/mach-hi3xxx/hi3xxx.c | 34 +++++++++
>>> arch/arm/mach-hi3xxx/platsmp.c | 84 ++++++++++++++++++++++
>>> 7 files changed, 197 insertions(+), 4 deletions(-)
>>> create mode 100644 arch/arm/mach-hi3xxx/core.h
>>> create mode 100644 arch/arm/mach-hi3xxx/platsmp.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>>> index 3be60c8..8c7a465 100644
>>> --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>>> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>>> @@ -1,10 +1,32 @@
>>> Hisilicon Platforms Device Tree Bindings
>>> ----------------------------------------------------
>>>
>>> -Hi3716 Development Board
>>> -Required root node properties:
>>> - - compatible = "hisilicon,hi3716-dkb";
>>> -
>>> Hi4511 Board
>>> Required root node properties:
>>> - compatible = "hisilicon,hi3620-hi4511";
>>> +
>>> +Hisilicon system controller
>>> +
>>> +Required properties:
>>> +- compatible : "hisilicon,sysctrl"
>>> +- reg : Register address and size
>>> +
>>> +Optional properties:
>>> +- smp-offset : offset in sysctrl for notifying slave cpu booting
>>> + cpu 1, reg;
>>> + cpu 2, reg + 0x4;
>>> + cpu 3, reg + 0x8;
>>> + 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
>>> +
>>> +Example:
>>> +
>>> + /* for Hi3620 */
>>> + sysctrl: system-controller@fc802000 {
>>> + compatible = "hisilicon,sysctrl";
>>> + reg = <0xfc802000 0x1000>;
>>> + smp-offset = <0x31c>;
>>> + resume-offset = <0x308>;
>>> + reboot-offset = <0x4>;
>>> + };
>>> diff --git a/arch/arm/boot/dts/hi3620.dtsi b/arch/arm/boot/dts/hi3620.dtsi
>>> index b9d8679..e311937 100644
>>> --- a/arch/arm/boot/dts/hi3620.dtsi
>>> +++ b/arch/arm/boot/dts/hi3620.dtsi
>>> @@ -39,6 +39,27 @@
>>> reg = <0x0>;
>>> next-level-cache = <&L2>;
>>> };
>>> +
>>> + cpu@1 {
>>> + compatible = "arm,cortex-a9";
>>> + device_type = "cpu";
>>> + reg = <1>;
>>> + next-level-cache = <&L2>;
>>> + };
>>> +
>>> + cpu@2 {
>>> + compatible = "arm,cortex-a9";
>>> + device_type = "cpu";
>>> + reg = <2>;
>>> + next-level-cache = <&L2>;
>>> + };
>>> +
>>> + cpu@3 {
>>> + compatible = "arm,cortex-a9";
>>> + device_type = "cpu";
>>> + reg = <3>;
>>> + next-level-cache = <&L2>;
>>> + };
>>> };
>>>
>>> amba {
>>> @@ -65,6 +86,17 @@
>>> reg = <0x1000 0x1000>, <0x100 0x100>;
>>> };
>>>
>>> + sysctrl: system-controller@802000 {
>>> + compatible = "hisilicon,sysctrl";
>>> + reg = <0x802000 0x1000>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + smp-offset = <0x31c>;
>>> + resume-offset = <0x308>;
>>> + reboot-offset = <0x4>;
>>> + };
>>> +
>>> dual_timer0: dual_timer@800000 {
>>> compatible = "arm,sp804", "arm,primecell";
>>> reg = <0x800000 0x1000>;
>>> @@ -115,6 +147,12 @@
>>> status = "disabled";
>>> };
>>>
>>> + timer5: timer@600 {
>>> + compatible = "arm,cortex-a9-twd-timer";
>>> + reg = <0x600 0x20>;
>>> + interrupts = <1 13 0xf01>;
>>> + };
>> Do you have a clocks node for this timer?
>
> As I mentioned in the 0th patch, clock binding are totally removed
> in this patch. And clock driver will be append in another patch set.
>
>>> +
>>> uart0: uart@b00000 {
>>> compatible = "arm,pl011", "arm,primecell";
>>> reg = <0xb00000 0x1000>;
>>> diff --git a/arch/arm/mach-hi3xxx/Kconfig b/arch/arm/mach-hi3xxx/Kconfig
>>> index 68bd26c..d0f298a 100644
>>> --- a/arch/arm/mach-hi3xxx/Kconfig
>>> +++ b/arch/arm/mach-hi3xxx/Kconfig
>>> @@ -6,6 +6,9 @@ config ARCH_HI3xxx
>>> select CACHE_L2X0
>>> select CLKSRC_OF
>>> select GENERIC_CLOCKEVENTS
>>> + select HAVE_ARM_SCU
>>> + select HAVE_ARM_TWD
>> Need "if SMP" to avoid a build failure for a non-smp kernel.
>>
>
> ARCH_HI3xxx always need SMP configuration. HAVE_SMP is already
> required. So it's impossible to build a non-smp kernel with this kernel
> configuration.
Then you need to have appropriate dependencies in Kconfig.
Right now, someone can do:
make hi3xxx_defconfig
edit .config, disable CONFIG_SMP
make
and get errors both from Kconfig:
warning: (ARCH_HI3xxx && SOC_OMAP5 && ARCH_SHMOBILE_MULTI) selects
HAVE_ARM_TWD which has unmet direct dependencies (SMP)
and from the make:
arch/arm/kernel/smp_twd.c: In function 'twd_local_timer_of_register':
arch/arm/kernel/smp_twd.c:391:20: error: 'setup_max_cpus' undeclared
(first use in this function)
if (!is_smp() || !setup_max_cpus)
... etc.
I.e. omap and shmobile have similar issues but we'll sort them out in
separate patches. You should still fix the hisilicon one.
>>> /*
>>> * This table is only for optimization. Since ioremap() could always share
>>> * the same mapping if it's defined as static IO mapping.
>>> @@ -29,6 +34,7 @@
>>> */
>>> static struct map_desc hi3620_io_desc[] __initdata = {
>>> {
>>> + /* sysctrl */
>> What's this comment for?
>>> .pfn = __phys_to_pfn(0xfc802000),
>>> .virtual = 0xfe802000,
>>> .length = 0x1000,
>>> @@ -48,6 +54,32 @@ static void __init hi3xxx_timer_init(void)
>>> clocksource_of_init();
>>> }
I know the code was added in a different patch, but I don't like the
lack of using constants here.
>>> +static void hi3xxx_restart(enum reboot_mode mode, const char *cmd)
>>> +{
>>> + struct device_node *np;
>>> + void __iomem *base;
>>> + int offset;
>>> +
>>> + np = of_find_compatible_node(NULL, NULL, "hisilicon,sysctrl");
>>> + if (!np) {
>>> + pr_err("failed to find hisilicon,sysctrl node\n");
>>> + return;
>>> + }
>>> + base = of_iomap(np, 0);
You can't do this ioremap at restart time, since you can't rely on
things being up enough for that when you're panicking, for example.
Please set up the ioremap beforehand so you can just do the
writel_relaxed, etc.
>>> diff --git a/arch/arm/mach-hi3xxx/platsmp.c b/arch/arm/mach-hi3xxx/platsmp.c
>>> new file mode 100644
>>> index 0000000..a4d04c0
>>> --- /dev/null
>>> +++ b/arch/arm/mach-hi3xxx/platsmp.c
>>> @@ -0,0 +1,84 @@
>>> +/*
>>> + * Copyright (c) 2013 Linaro Ltd.
>>> + * Copyright (c) 2013 Hisilicon Limited.
>>> + * Based on platsmp.c, Copyright (C) 2002 ARM Ltd.
Based on what platsmp? There are a lot of them under arch/arm. :)
-Olof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
