From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752029Ab3GIWFd (ORCPT ); Tue, 9 Jul 2013 18:05:33 -0400 Received: from moutng.kundenserver.de ([212.227.17.10]:54242 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751229Ab3GIWFc (ORCPT ); Tue, 9 Jul 2013 18:05:32 -0400 From: Arnd Bergmann To: Neil Zhang Subject: Re: [PATCH V3 3/3] ARM: mmp: bring up pxa988 with device tree support Date: Wed, 10 Jul 2013 00:05:21 +0200 User-Agent: KMail/1.12.2 (Linux/3.8.0-22-generic; KDE/4.3.2; x86_64; ; ) Cc: grant.likely@linaro.org, haojian.zhuang@gmail.com, devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Chao Xie References: <1373352166-10064-1-git-send-email-zhangwm@marvell.com> <1373352166-10064-4-git-send-email-zhangwm@marvell.com> In-Reply-To: <1373352166-10064-4-git-send-email-zhangwm@marvell.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201307100005.21769.arnd@arndb.de> X-Provags-ID: V02:K0:JlthFFyEEpBOkx83tXrKzQrJCBDFubuQyex/Xpc1RRt kH2mc3vFwOO+FhBrUAsh3i6EZ0NOE7/jl2sO4yWgJnFhjYk/Ai hVYlpaBgpLNi0uJYDhbgqAvlHqG7cdYhuPOUUJFIcSVVglmpgU DA2y4DPjMsCTjOHnUuFogchzFFHI8t59HFBhZNwOypZWx7d6oV K7KTNsmNayR5v/JVk/BkL/GzTQ8ltUqmq+JiA+pXSwyYfnihN6 /5mXQb4fSOcWM9UZnBwSMIs15tLjhT3lyiDkL+/z8WXCupo9XI OyHe9qAiGN5ve64eQgwyX1ceCevkzVeTmS5yvjt1pWtL5UAt+4 fimY/gwuzOMQ3U+5NqDQ= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 09 July 2013, Neil Zhang wrote: > + soc { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupt-parent = <&gic>; > + ranges; > + > + gic: interrupt-controller@d1dfe100 { > + compatible = "arm,cortex-a9-gic"; > + #interrupt-cells = <3>; > + #address-cells = <1>; > + interrupt-controller; > + reg = <0xd1dff000 0x1000>, > + <0xd1dfe100 0x0100>; > + }; > + > + L2: l2-cache-controller@d1dfb000 { > + compatible = "arm,pl310-cache"; > + reg = <0xd1dfb000 0x1000>; > + arm,data-latency = <2 1 1>; > + arm,tag-latency = <2 1 1>; > + arm,pwr-dynamic-clk-gating; > + arm,pwr-standby-mode; > + cache-unified; > + cache-level = <2>; > + }; > + > + local-timer@d1dfe600 { > + compatible = "arm,cortex-a9-twd-timer"; > + reg = <0xd1dfe600 0x20>; > + interrupts = <1 13 0x304>; > + }; > + > + axi@d4200000 { /* AXI */ > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0xd4200000 0xd4200000 0x00200000>; > + > + intc: wakeupgen@d4282000 { > + compatible = "marvell,mmp-intc"; > + reg = <0xd4282000 0x1000>; > + marvell,intc-wakeup = <0x114 0x3 > + 0x144 0x3>; > + }; > + }; I am guessing that the structure does not actually reflect the hardware. Shouldn't AXI be the top-level bus, with the other stuff under it? > + > + > + uart1: uart@d4017000 { > + compatible = "marvell,mmp-uart"; > + reg = <0xd4017000 0x1000>; > + interrupts = <0 27 0x4>; > + status = "disabled"; > + }; The uart node should be called "serial@d4017000" instead of "uart@d4017000". > diff --git a/arch/arm/mach-mmp/reset.c b/arch/arm/mach-mmp/reset.c > new file mode 100644 > index 0000000..b90ec54 > --- /dev/null > +++ b/arch/arm/mach-mmp/reset.c > @@ -0,0 +1,66 @@ > +/* > + * linux/arch/arm/mach-mmp/reset.c I think this could just be part of the smp.c file. > + * > + * Author: Neil Zhang > + * Copyright: (C) 2012 Marvell International Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include > +#include > + > +#include > +#include > +#include > + > +#include > + > +#include "reset.h" > + > +#define PMU_CC2_AP APMU_REG(0x0100) > +#define CIU_CA9_WARM_RESET_VECTOR CIU_REG(0x00d8) You should not hardcode the addresses here, better find them from the device tree. > + > +#define CPU_CORE_RST(n) (1 << ((n) * 4 + 16)) > +#define CPU_DBG_RST(n) (1 << ((n) * 4 + 18)) > +#define CPU_WDOG_RST(n) (1 << ((n) * 4 + 19)) This should probably go into a reset controller driver, in drivers/reset/ Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Wed, 10 Jul 2013 00:05:21 +0200 Subject: [PATCH V3 3/3] ARM: mmp: bring up pxa988 with device tree support In-Reply-To: <1373352166-10064-4-git-send-email-zhangwm@marvell.com> References: <1373352166-10064-1-git-send-email-zhangwm@marvell.com> <1373352166-10064-4-git-send-email-zhangwm@marvell.com> Message-ID: <201307100005.21769.arnd@arndb.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tuesday 09 July 2013, Neil Zhang wrote: > + soc { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupt-parent = <&gic>; > + ranges; > + > + gic: interrupt-controller at d1dfe100 { > + compatible = "arm,cortex-a9-gic"; > + #interrupt-cells = <3>; > + #address-cells = <1>; > + interrupt-controller; > + reg = <0xd1dff000 0x1000>, > + <0xd1dfe100 0x0100>; > + }; > + > + L2: l2-cache-controller at d1dfb000 { > + compatible = "arm,pl310-cache"; > + reg = <0xd1dfb000 0x1000>; > + arm,data-latency = <2 1 1>; > + arm,tag-latency = <2 1 1>; > + arm,pwr-dynamic-clk-gating; > + arm,pwr-standby-mode; > + cache-unified; > + cache-level = <2>; > + }; > + > + local-timer at d1dfe600 { > + compatible = "arm,cortex-a9-twd-timer"; > + reg = <0xd1dfe600 0x20>; > + interrupts = <1 13 0x304>; > + }; > + > + axi at d4200000 { /* AXI */ > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0xd4200000 0xd4200000 0x00200000>; > + > + intc: wakeupgen at d4282000 { > + compatible = "marvell,mmp-intc"; > + reg = <0xd4282000 0x1000>; > + marvell,intc-wakeup = <0x114 0x3 > + 0x144 0x3>; > + }; > + }; I am guessing that the structure does not actually reflect the hardware. Shouldn't AXI be the top-level bus, with the other stuff under it? > + > + > + uart1: uart at d4017000 { > + compatible = "marvell,mmp-uart"; > + reg = <0xd4017000 0x1000>; > + interrupts = <0 27 0x4>; > + status = "disabled"; > + }; The uart node should be called "serial at d4017000" instead of "uart at d4017000". > diff --git a/arch/arm/mach-mmp/reset.c b/arch/arm/mach-mmp/reset.c > new file mode 100644 > index 0000000..b90ec54 > --- /dev/null > +++ b/arch/arm/mach-mmp/reset.c > @@ -0,0 +1,66 @@ > +/* > + * linux/arch/arm/mach-mmp/reset.c I think this could just be part of the smp.c file. > + * > + * Author: Neil Zhang > + * Copyright: (C) 2012 Marvell International Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include > +#include > + > +#include > +#include > +#include > + > +#include > + > +#include "reset.h" > + > +#define PMU_CC2_AP APMU_REG(0x0100) > +#define CIU_CA9_WARM_RESET_VECTOR CIU_REG(0x00d8) You should not hardcode the addresses here, better find them from the device tree. > + > +#define CPU_CORE_RST(n) (1 << ((n) * 4 + 16)) > +#define CPU_DBG_RST(n) (1 << ((n) * 4 + 18)) > +#define CPU_WDOG_RST(n) (1 << ((n) * 4 + 19)) This should probably go into a reset controller driver, in drivers/reset/ Arnd