Message ID | 1536661032-30481-1-git-send-email-suzuki.poulose@arm.com |
---|---|
Headers | show |
Series | dts: Update coresight device tree bindings | expand |
On 09/11/2018 06:01 PM, Sudeep Holla wrote: > On Tue, Sep 11, 2018 at 11:17:12AM +0100, Suzuki K Poulose wrote: >> Switch to the new coresight bindings >> > > I still see the below warnings: > > vexpress-v2p-ca15_a7.dtb: Warning (graph_child_address): > /replicator/in-ports: graph node has single child node 'port@0', > #address-cells/#size-cells are not necessary > vexpress-v2p-ca15_a7.dtb: Warning (graph_child_address): > /funnel@20040000/out-ports: graph node has single child node 'port@0', > #address-cells/#size-cells are not necessary > > I need the below patch to fix them, let me know if it looks OK, I can > amend and apply. Thanks for reporting. I purposefully added the "address-cells" and followed the format everywhere in the series thinking that, that is indeed the formal way of doing it, rather than having implicit port numbers. I can send an updated series fixing it everywhere. Regards Suzuki > > Regards, > Sudeep > > -->8 > > diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts > index 3a5090616bc6..8b926c30ccd1 100644 > --- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts > +++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts > @@ -443,11 +443,7 @@ > }; > > in-ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > replicator_in_port0: endpoint { > remote-endpoint = <&funnel_out_port0>; > }; > @@ -462,11 +458,7 @@ > clocks = <&oscclk6a>; > clock-names = "apb_pclk"; > out-ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > funnel_out_port0: endpoint { > remote-endpoint = > <&replicator_in_port0>; >
On 11/09/18 18:15, Suzuki K Poulose wrote: > On 09/11/2018 06:01 PM, Sudeep Holla wrote: >> On Tue, Sep 11, 2018 at 11:17:12AM +0100, Suzuki K Poulose wrote: >>> Switch to the new coresight bindings >>> >> >> I still see the below warnings: >> >> vexpress-v2p-ca15_a7.dtb: Warning (graph_child_address): >> /replicator/in-ports: graph node has single child node 'port@0', >> #address-cells/#size-cells are not necessary >> vexpress-v2p-ca15_a7.dtb: Warning (graph_child_address): >> /funnel@20040000/out-ports: graph node has single child node >> 'port@0', >> #address-cells/#size-cells are not necessary >> >> I need the below patch to fix them, let me know if it looks OK, I can >> amend and apply. > > Thanks for reporting. I purposefully added the "address-cells" and > followed the format everywhere in the series thinking that, that is > indeed the formal way of doing it, rather than having implicit port > numbers. I can send an updated series fixing it everywhere. > No need to post the update for TC2 unless it's different from what I have proposed.
On 09/11/2018 06:23 PM, Sudeep Holla wrote: > > > On 11/09/18 18:15, Suzuki K Poulose wrote: >> On 09/11/2018 06:01 PM, Sudeep Holla wrote: >>> On Tue, Sep 11, 2018 at 11:17:12AM +0100, Suzuki K Poulose wrote: >>>> Switch to the new coresight bindings >>>> >>> >>> I still see the below warnings: >>> >>> vexpress-v2p-ca15_a7.dtb: Warning (graph_child_address): >>> /replicator/in-ports: graph node has single child node 'port@0', >>> #address-cells/#size-cells are not necessary >>> vexpress-v2p-ca15_a7.dtb: Warning (graph_child_address): >>> /funnel@20040000/out-ports: graph node has single child node >>> 'port@0', >>> #address-cells/#size-cells are not necessary >>> >>> I need the below patch to fix them, let me know if it looks OK, I can >>> amend and apply. >> >> Thanks for reporting. I purposefully added the "address-cells" and >> followed the format everywhere in the series thinking that, that is >> indeed the formal way of doing it, rather than having implicit port >> numbers. I can send an updated series fixing it everywhere. >> > No need to post the update for TC2 unless it's different from what I > have proposed. > Yes, the changes look good. Thanks Sudeep. I will drop this patch from the next version then. Btw, my kernel build didn't trigger those warnings. Thanks Suzuki
On 09/11/2018 11:17 AM, Suzuki K Poulose wrote: > Coresight DT bindings have been updated to obey the DTS rules > for label/address matching for graph nodes. The changes are in > coresight/next tree scheduled for v4.20. This series updates the > in kernel dts to match the new bindings along with updating a couple > of new examples (e.,g CATU) in the Documentation (which were missed > as they were still in flight when we created the series). > > Please note that this should not be pulled for v4.19, which I assume > is a safe assumption. But please do pull it for v4.20. > The dt updates for the Juno boards were sent earlier with the original > DT update series and has been queued for v4.20. > > Applies on coresight/next (which is based on v4.19) and should apply > cleanly on v4.19-rc3. > All, There are some additional warnings triggered by this series, as reported by Sudeep [0]. I intend to fix the warnings and send and updated version soon. So kindly ignore this thread. Apologies. [0] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-September/601121.html Suzuki
On Tue, Sep 11, 2018 at 11:17:07AM +0100, Suzuki K Poulose wrote: > Switch to the updated coresight bindings. > > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Fabio Estevam <fabio.estevam@nxp.com> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> As per the convention we use for subject prefix, I suggest you use 'ARM: dts: imx7: ...' Shawn > --- > arch/arm/boot/dts/imx7d.dtsi | 11 ++++--- > arch/arm/boot/dts/imx7s.dtsi | 78 ++++++++++++++++++++++++++------------------ > 2 files changed, 53 insertions(+), 36 deletions(-) > > diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi > index 7cbc2ff..4ced17c 100644 > --- a/arch/arm/boot/dts/imx7d.dtsi > +++ b/arch/arm/boot/dts/imx7d.dtsi > @@ -63,9 +63,11 @@ > clocks = <&clks IMX7D_MAIN_AXI_ROOT_CLK>; > clock-names = "apb_pclk"; > > - port { > - etm1_out_port: endpoint { > - remote-endpoint = <&ca_funnel_in_port1>; > + out-ports { > + port { > + etm1_out_port: endpoint { > + remote-endpoint = <&ca_funnel_in_port1>; > + }; > }; > }; > }; > @@ -148,11 +150,10 @@ > }; > }; > > -&ca_funnel_ports { > +&ca_funnel_in_ports { > port@1 { > reg = <1>; > ca_funnel_in_port1: endpoint { > - slave-mode; > remote-endpoint = <&etm1_out_port>; > }; > }; > diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi > index a052198..9176885 100644 > --- a/arch/arm/boot/dts/imx7s.dtsi > +++ b/arch/arm/boot/dts/imx7s.dtsi > @@ -106,7 +106,7 @@ > */ > compatible = "arm,coresight-replicator"; > > - ports { > + out-ports { > #address-cells = <1>; > #size-cells = <0>; > /* replicator output ports */ > @@ -123,12 +123,15 @@ > remote-endpoint = <&etr_in_port>; > }; > }; > + }; > > - /* replicator input port */ > - port@2 { > + in-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > reg = <0>; > replicator_in_port0: endpoint { > - slave-mode; > remote-endpoint = <&etf_out_port>; > }; > }; > @@ -168,28 +171,31 @@ > clocks = <&clks IMX7D_MAIN_AXI_ROOT_CLK>; > clock-names = "apb_pclk"; > > - ca_funnel_ports: ports { > + ca_funnel_in_ports: in-ports { > #address-cells = <1>; > #size-cells = <0>; > > - /* funnel input ports */ > port@0 { > reg = <0>; > ca_funnel_in_port0: endpoint { > - slave-mode; > remote-endpoint = <&etm0_out_port>; > }; > }; > > - /* funnel output port */ > - port@2 { > + /* the other input ports are not connect to anything */ > + }; > + > + out-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > reg = <0>; > ca_funnel_out_port0: endpoint { > remote-endpoint = <&hugo_funnel_in_port0>; > }; > }; > > - /* the other input ports are not connect to anything */ > }; > }; > > @@ -200,9 +206,11 @@ > clocks = <&clks IMX7D_MAIN_AXI_ROOT_CLK>; > clock-names = "apb_pclk"; > > - port { > - etm0_out_port: endpoint { > - remote-endpoint = <&ca_funnel_in_port0>; > + out-ports { > + port { > + etm0_out_port: endpoint { > + remote-endpoint = <&ca_funnel_in_port0>; > + }; > }; > }; > }; > @@ -213,15 +221,13 @@ > clocks = <&clks IMX7D_MAIN_AXI_ROOT_CLK>; > clock-names = "apb_pclk"; > > - ports { > + in-ports { > #address-cells = <1>; > #size-cells = <0>; > > - /* funnel input ports */ > port@0 { > reg = <0>; > hugo_funnel_in_port0: endpoint { > - slave-mode; > remote-endpoint = <&ca_funnel_out_port0>; > }; > }; > @@ -229,18 +235,22 @@ > port@1 { > reg = <1>; > hugo_funnel_in_port1: endpoint { > - slave-mode; /* M4 input */ > + /* M4 input */ > }; > }; > + /* the other input ports are not connect to anything */ > + }; > > - port@2 { > + out-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > reg = <0>; > hugo_funnel_out_port0: endpoint { > remote-endpoint = <&etf_in_port>; > }; > }; > - > - /* the other input ports are not connect to anything */ > }; > }; > > @@ -250,19 +260,23 @@ > clocks = <&clks IMX7D_MAIN_AXI_ROOT_CLK>; > clock-names = "apb_pclk"; > > - ports { > + in-ports { > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > reg = <0>; > etf_in_port: endpoint { > - slave-mode; > remote-endpoint = <&hugo_funnel_out_port0>; > }; > }; > + }; > > - port@1 { > + out-ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > reg = <0>; > etf_out_port: endpoint { > remote-endpoint = <&replicator_in_port0>; > @@ -277,10 +291,11 @@ > clocks = <&clks IMX7D_MAIN_AXI_ROOT_CLK>; > clock-names = "apb_pclk"; > > - port { > - etr_in_port: endpoint { > - slave-mode; > - remote-endpoint = <&replicator_out_port1>; > + in-ports { > + port { > + etr_in_port: endpoint { > + remote-endpoint = <&replicator_out_port1>; > + }; > }; > }; > }; > @@ -291,10 +306,11 @@ > clocks = <&clks IMX7D_MAIN_AXI_ROOT_CLK>; > clock-names = "apb_pclk"; > > - port { > - tpiu_in_port: endpoint { > - slave-mode; > - remote-endpoint = <&replicator_out_port0>; > + in-ports { > + port { > + tpiu_in_port: endpoint { > + remote-endpoint = <&replicator_out_port0>; > + }; > }; > }; > }; > -- > 2.7.4 >
On 12/09/18 03:21, Shawn Guo wrote: > On Tue, Sep 11, 2018 at 11:17:07AM +0100, Suzuki K Poulose wrote: >> Switch to the updated coresight bindings. >> >> Cc: Shawn Guo <shawnguo@kernel.org> >> Cc: Sascha Hauer <s.hauer@pengutronix.de> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> >> Cc: Fabio Estevam <fabio.estevam@nxp.com> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > As per the convention we use for subject prefix, I suggest you use > > 'ARM: dts: imx7: ...' Shawn Thanks for the suggestion. As I mentioned in the reply to the cover letter, I will update the series with this addressed. Thanks Suzuki
Hi Suzuki, On Tue, Sep 11, 2018 at 11:17:05AM +0100, Suzuki K Poulose wrote: > Switch to updated coresight bindings for hw ports > > Cc: Andy Gross <andy.gross@linaro.org> > Cc: David Brown <david.brown@linaro.org> > Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > arch/arm64/boot/dts/qcom/msm8916.dtsi | 98 ++++++++++++++++++++++------------- > 1 file changed, 63 insertions(+), 35 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi > index 7b32b89..5bed52b 100644 > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi > @@ -1099,10 +1099,11 @@ > clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>; > clock-names = "apb_pclk", "atclk"; > > - port { > - tpiu_in: endpoint { > - slave-mode; > - remote-endpoint = <&replicator_out1>; > + out-ports { Here should be in-ports for tpiu node? BTW, if upper comment is valid, this means the DT binding doc also gives wrong example for tpiu node [1]. I tested this patch with upper changing to 'in-ports', etm with perf mode can work well on DB410c board. [...] Thanks, Leo Yan [1] https://git.linaro.org/kernel/coresight.git/tree/Documentation/devicetree/bindings/arm/coresight.txt?h=next#n135
Hi Leo, On 12/09/18 11:17, leo.yan@linaro.org wrote: > Hi Suzuki, > > On Tue, Sep 11, 2018 at 11:17:05AM +0100, Suzuki K Poulose wrote: >> Switch to updated coresight bindings for hw ports >> >> Cc: Andy Gross <andy.gross@linaro.org> >> Cc: David Brown <david.brown@linaro.org> >> Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> arch/arm64/boot/dts/qcom/msm8916.dtsi | 98 ++++++++++++++++++++++------------- >> 1 file changed, 63 insertions(+), 35 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi >> index 7b32b89..5bed52b 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi >> @@ -1099,10 +1099,11 @@ >> clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>; >> clock-names = "apb_pclk", "atclk"; >> >> - port { >> - tpiu_in: endpoint { >> - slave-mode; >> - remote-endpoint = <&replicator_out1>; >> + out-ports { > > Here should be in-ports for tpiu node? > > BTW, if upper comment is valid, this means the DT binding doc also > gives wrong example for tpiu node [1]. You are right on both counts. > > I tested this patch with upper changing to 'in-ports', etm with perf > mode can work well on DB410c board. > Thanks a lot for the testing. I will fix both the problems. Suzuki
On Tue, Sep 11, 2018 at 11:17:03AM +0100, Suzuki K Poulose wrote: > Switch to updated coresight bindings for hw ports. As Shawn suggested, please change subject as "arm64: dts: hi6220: ...." > Cc: xuwei5@hisilicon.com > Cc: lipengcheng8@huawei.com > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > .../arm64/boot/dts/hisilicon/hi6220-coresight.dtsi | 147 ++++++++++++--------- > 1 file changed, 85 insertions(+), 62 deletions(-) > > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi > index 7afee5d..2202816 100644 > --- a/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi [...] > @@ -302,10 +315,12 @@ > > cpu = <&cpu3>; > > - port { > - etm3_out: endpoint { > - remote-endpoint = > - <&acpu_funnel_in3>; > + out-ports { > + port { > + etm3_out: endpoint { > + remote-endpoint = > + <&acpu_funnel_in3>; > + }; > }; > }; > }; > @@ -319,10 +334,12 @@ > > cpu = <&cpu4>; > > - port { > + out-ports { > + port { > etm4_out: endpoint { Indent? After applied this patch and tested on Hikey board; FWIW: Tested-by: Leo Yan <leo.yan@linaro.org> > - remote-endpoint = > - <&acpu_funnel_in4>; > + remote-endpoint = > + <&acpu_funnel_in4>; > + }; > }; > }; > }; > @@ -336,10 +353,12 @@ > > cpu = <&cpu5>; > > - port { > - etm5_out: endpoint { > - remote-endpoint = > - <&acpu_funnel_in5>; > + out-ports { > + port { > + etm5_out: endpoint { > + remote-endpoint = > + <&acpu_funnel_in5>; > + }; > }; > }; > }; > @@ -353,10 +372,12 @@ > > cpu = <&cpu6>; > > - port { > - etm6_out: endpoint { > - remote-endpoint = > - <&acpu_funnel_in6>; > + out-ports { > + port { > + etm6_out: endpoint { > + remote-endpoint = > + <&acpu_funnel_in6>; > + }; > }; > }; > }; > @@ -370,10 +391,12 @@ > > cpu = <&cpu7>; > > - port { > - etm7_out: endpoint { > - remote-endpoint = > - <&acpu_funnel_in7>; > + out-ports { > + port { > + etm7_out: endpoint { > + remote-endpoint = > + <&acpu_funnel_in7>; > + }; > }; > }; > }; > -- > 2.7.4 >
Hi Leo, On 12/09/18 11:47, leo.yan@linaro.org wrote: > On Tue, Sep 11, 2018 at 11:17:03AM +0100, Suzuki K Poulose wrote: >> Switch to updated coresight bindings for hw ports. > > As Shawn suggested, please change subject as "arm64: dts: hi6220: > ...." > Sure, will do. >> Cc: xuwei5@hisilicon.com >> Cc: lipengcheng8@huawei.com >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> .../arm64/boot/dts/hisilicon/hi6220-coresight.dtsi | 147 ++++++++++++--------- >> 1 file changed, 85 insertions(+), 62 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi >> index 7afee5d..2202816 100644 >> --- a/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi >> +++ b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi > > [...] > >> @@ -302,10 +315,12 @@ >> >> cpu = <&cpu3>; >> >> - port { >> - etm3_out: endpoint { >> - remote-endpoint = >> - <&acpu_funnel_in3>; >> + out-ports { >> + port { >> + etm3_out: endpoint { >> + remote-endpoint = >> + <&acpu_funnel_in3>; >> + }; >> }; >> }; >> }; >> @@ -319,10 +334,12 @@ >> >> cpu = <&cpu4>; >> >> - port { >> + out-ports { >> + port { >> etm4_out: endpoint { > > Indent? Yes, I have fixed this locally for v2. > > After applied this patch and tested on Hikey board; FWIW: > > Tested-by: Leo Yan <leo.yan@linaro.org> Thanks for the testing ! Cheers Suzuki
On Tue, 11 Sep 2018 11:17:02 +0100, Suzuki K Poulose wrote: > While we updated the coresight DT bindings, some of the > new examples were not updated due to the order in which they > were merged. Let us update all the missed out ones to the > new bindings to avoid confusion. > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Rob Herring <robh@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > Documentation/devicetree/bindings/arm/coresight.txt | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>