Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932940AbdDGJia (ORCPT ); Fri, 7 Apr 2017 05:38:30 -0400 Received: from mail-pg0-f48.google.com ([74.125.83.48]:33177 "EHLO mail-pg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752775AbdDGJiV (ORCPT ); Fri, 7 Apr 2017 05:38:21 -0400 Date: Fri, 7 Apr 2017 17:38:11 +0800 From: Leo Yan To: Li Pengcheng Cc: xuwei5@hisilicon.com, robh+dt@kernel.org, mark.rutland@arm.com, catalin.marinas@arm.com, will.deacon@arm.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, suzhuangluan@hisilicon.com, dan.zhao@hisilicon.com, lizhong11@hisilicon.com, Mathieu Poirier Subject: Re: [PATCH] arm64: dts: Add coresight DT nodes for hi6220-hikey Message-ID: <20170407093811.GA17455@leoy-linaro> References: <1491552418-74386-1-git-send-email-lipengcheng8@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1491552418-74386-1-git-send-email-lipengcheng8@huawei.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9758 Lines: 425 Hi Pengcheng, [ + Mathieu ] On Fri, Apr 07, 2017 at 04:06:58PM +0800, Li Pengcheng wrote: > Add coresight DT nodes for hikey board. > > Signed-off-by: Li Pengcheng > Signed-off-by: Li Zhong > --- > .../arm64/boot/dts/hisilicon/hi6220-coresight.dtsi | 318 +++++++++++++++++++++ > arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 1 + > 2 files changed, 319 insertions(+) > create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi > > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi > new file mode 100644 > index 0000000..a523e43 > --- /dev/null > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi > @@ -0,0 +1,318 @@ > +/* > + * Hisilicon Ltd. Hi6220 SoC > + * > + * Copyright (C) 2015-2016 Hisilicon Ltd. > + * Author: lipengcheng > + * > + * 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. > + */ > +/ { > + amba { Here I'm curious if can use the similiar implementation with Juno? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi?id=refs/tags/v4.11-rc5 So finally can include this file into hi6220.dtsi 'soc' section; this can reflect the hardware topology more clearly: soc { [...] #include "hi6220-coresight.dtsi" } > + /* A53 cluster0 internal coresight */ > + #address-cells = <2>; > + #size-cells = <2>; > + compatible = "arm,amba-bus"; Usually the compatible string should be documented in Documentation/devicetree/bindings/, so please drop it. I think "#address-cells" and "#size-cells" also can remove. This is defined by its parent node. > + ranges; > + etm@0,f659c000 { Change to etm@f659c000? > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf659c000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu0>; > + port { > + etm0_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port0>; > + }; > + }; > + }; > + > + etm@1,f659d000 { Same with above. > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf659d000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu1>; > + port { > + etm1_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port1>; > + }; > + }; > + > + }; > + > + etm@2,f659e000 { Same with above. > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf659e000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu2>; > + port { > + etm2_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port2>; > + }; > + }; > + }; > + > + etm@3,f659f000 { Same with above. > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf659f000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu3>; > + port { > + etm3_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port3>; > + }; > + }; > + }; > + > + /* A53 cluster1 internal coresight */ > + etm@4,f65dc000 { Same with above. > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf65dc000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu4>; > + port { > + etm4_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port4>; Wrong indent. > + }; > + }; > + }; > + > + etm@5,f65dd000 { Same with above. > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf65dd000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu5>; > + port { > + etm5_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port5>; Wrong indent. > + }; > + }; > + }; > + > + etm@6,f65de000 { Change to etm@f65de000? > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf65de000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu6>; > + port { > + etm6_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port6>; > + }; > + }; > + }; > + > + etm@7,f65df000 { Same with above. > + compatible = "arm,coresight-etm4x","arm,primecell"; > + reg = <0 0xf65df000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + cpu = <&cpu7>; > + port { > + etm7_out_port: endpoint { > + remote-endpoint = <&funnel0_in_port7>; > + }; > + }; > + }; > + > + funnel0:funnel@0,f6501000 { funnel0@f6501000. > + compatible = "arm,coresight-funnel","arm,primecell"; > + reg = <0 0xf6501000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + /* funnel output port */ > + port@0 { > + reg = <0>; > + funnel0_out_port: endpoint { > + remote-endpoint = <&funnel1_in_port>; > + }; > + }; > + > + /* funnel input ports */ > + port@1 { > + reg = <0>; > + funnel0_in_port0: endpoint { > + slave-mode; > + remote-endpoint = <&etm0_out_port>; > + }; > + }; > + > + port@2 { > + reg = <1>; > + funnel0_in_port1: endpoint { > + slave-mode; > + remote-endpoint = <&etm1_out_port>; > + }; > + }; > + > + port@3 { > + reg = <2>; > + funnel0_in_port2: endpoint { > + slave-mode; > + remote-endpoint = <&etm2_out_port>; > + }; > + }; > + > + port@4 { > + reg = <3>; > + funnel0_in_port3: endpoint { > + slave-mode; > + remote-endpoint = <&etm3_out_port>; > + }; > + }; > + > + port@5 { > + reg = <4>; > + funnel0_in_port4: endpoint { > + slave-mode; > + remote-endpoint = <&etm4_out_port>; > + }; > + }; > + > + port@6 { > + reg = <5>; > + funnel0_in_port5: endpoint { > + slave-mode; > + remote-endpoint = <&etm5_out_port>; > + }; > + }; > + > + port@7 { > + reg = <6>; > + funnel0_in_port6: endpoint { > + slave-mode; > + remote-endpoint = <&etm6_out_port>; > + }; > + }; > + > + port@8 { > + reg = <7>; > + funnel0_in_port7: endpoint { > + slave-mode; > + remote-endpoint = <&etm7_out_port>; > + }; > + }; > + }; > + }; > + > + funnel1:funnel@1,f6401000 { funnel1@f6401000 > + compatible = "arm,coresight-funnel","arm,primecell"; > + reg = <0 0xf6401000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + /* funnel1 output port */ > + port@0 { > + reg = <0>; > + funnel1_out_port: endpoint { > + remote-endpoint = <&etf_in_port>; > + }; > + }; > + > + /* funnel1 input port */ > + port@1 { > + reg = <0>; > + funnel1_in_port: endpoint { > + slave-mode; > + remote-endpoint = <&funnel0_out_port>; > + }; > + }; > + }; > + }; Extra blank line. > + etf:etf@0,f6402000 { > + compatible = "arm,coresight-tmc","arm,primecell"; > + reg = <0 0xf6402000 0 0x1000>; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + /* etf input port */ > + port@0 { > + reg = <0>; > + etf_in_port: endpoint { > + slave-mode; > + remote-endpoint = <&funnel1_out_port>; > + }; > + }; > + /* etf output port */ > + port@1 { > + reg = <0>; > + etf_out_port: endpoint { > + remote-endpoint = <&replicator0_in_port>; > + }; > + }; > + }; > + }; > + > + replicator@0{ > + compatible = "arm,coresight-replicator"; > + > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + /* replicator input port */ > + port@0 { > + reg = <0>; > + replicator0_in_port: endpoint{ > + slave-mode; > + remote-endpoint = <&etf_out_port>; Wrong indent. > + }; > + }; > + /* replicator out port */ > + port@1 { > + reg = <0>; > + replicator0_out_port: endpoint { > + remote-endpoint = <&etr0_in_port>; > + }; > + }; > + }; > + }; > + > + etr@0,f6404000 { > + compatible = "arm,coresight-tmc","arm,primecell"; > + reg = <0 0xf6404000 0 0x1000>; > + > + coresight-default-sink; > + clocks = <&sys_ctrl HI6220_CS_ATB>; > + clock-names = "apb_pclk"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + /* etr input port */ > + port@0 { > + etr0_in_port: endpoint{ > + slave-mode; > + remote-endpoint = <&replicator0_out_port>; Wrong indent. > + }; > + }; > + }; > + }; > + }; > +}; > + > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > index dba3c13..fb70c9b 100644 > --- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts > @@ -8,6 +8,7 @@ > /dts-v1/; > #include "hi6220.dtsi" > #include "hikey-pinctrl.dtsi" > +#include "hi6220-coresight.dtsi" Please review upper comments for adding it into 'soc' node. BTW, it's good to use script ./scripts/checkpatch.pl to check the patch. > #include > > / { > -- > 2.1.0 >