-----邮件原件----- 发件人: Leo Yan [mailto:leo.yan@linaro.org] 发送时间: 2019年3月1日 15:33 收件人: Shiwanglai shiwanglai@hisilicon.com 抄送: robh+dt@kernel.org; mark.rutland@arm.com; john.stultz@linaro.org; xuwei (O) xuwei5@huawei.com; linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; coresight@lists.linaro.org; Suzhuangluan suzhuangluan@hisilicon.com 主题: Re: [PATCH v2] dts: arm64: add CoreSight trace support for hi3660
Hi Wanglai,
On Thu, Feb 28, 2019 at 02:33:23PM +0800, Wanglai Shi wrote:
This patch adds devicetree entries for the CoreSight trace components on hi3660.
Signed-off-by: Wanglai Shi shiwanglai@hisilicon.com
.../arm64/boot/dts/hisilicon/hi3660-coresight.dtsi | 429 +++++++++++++++++++++ arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 + 2 files changed, 431 insertions(+) create mode 100644 arch/arm64/boot/dts/hisilicon/hi3660-coresight.dtsi
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-coresight.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660-coresight.dtsi new file mode 100644 index 0000000..d651a8b --- /dev/null +++ b/arch/arm64/boot/dts/hisilicon/hi3660-coresight.dtsi @@ -0,0 +1,429 @@ +// SPDX-License-Identifier: GPL-2.0
+/*
- dtsi for Hisilicon Hi3660 Coresight
- Copyright (C) 2016-2018 Hisilicon Ltd.
- Author: Wanglai Shi shiwanglai@hisilicon.com
- */
+/ {
- soc {
/* A53 cluster internals */etm@ecc40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xecc40000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu0>;out-ports {port {etm0_out: endpoint {remote-endpoint =<&cluster0_funnel_in0>;};};};};etm@ecd40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xecd40000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu1>;out-ports {port {etm1_out: endpoint {remote-endpoint =<&cluster0_funnel_in1>;};};};};etm@ece40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xece40000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu2>;out-ports {port {etm2_out: endpoint {remote-endpoint =<&cluster0_funnel_in2>;};};};};etm@ecf40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xecf40000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu3>;out-ports {port {etm3_out: endpoint {remote-endpoint =<&cluster0_funnel_in3>;};};};};funnel@ec801000 {compatible = "arm,coresight-funnel", "arm,primecell";reg = <0 0xec801000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";out-ports {port {cluster0_funnel_out: endpoint {remote-endpoint =<&cluster0_etf_in>;};};};in-ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;cluster0_funnel_in0: endpoint {remote-endpoint = <&etm0_out>;};};port@1 {reg = <1>;cluster0_funnel_in1: endpoint {remote-endpoint = <&etm1_out>;};};port@2 {reg = <2>;cluster0_funnel_in2: endpoint {remote-endpoint = <&etm2_out>;};};port@3 {reg = <3>;cluster0_funnel_in3: endpoint {remote-endpoint = <&etm3_out>;};};};};etf@ec802000 {compatible = "arm,coresight-tmc", "arm,primecell";reg = <0 0xec802000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";in-ports {port {cluster0_etf_in: endpoint {remote-endpoint =<&cluster0_funnel_out>;};};};out-ports {port {cluster0_etf_out: endpoint {remote-endpoint =<&top_funnel_in0>;};};};};/* A73 cluster internals */etm@ed440000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xed440000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu4>;out-ports {port {etm4_out: endpoint {remote-endpoint =<&cluster1_funnel_in0>;};};};};etm@ed540000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xed540000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu5>;out-ports {port {etm5_out: endpoint {remote-endpoint =<&cluster1_funnel_in1>;};};};};etm@ed640000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xed640000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu6>;out-ports {port {etm6_out: endpoint {remote-endpoint =<&cluster1_funnel_in2>;};};};};etm@ed740000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xed740000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";cpu = <&cpu7>;out-ports {port {etm7_out: endpoint {remote-endpoint =<&cluster1_funnel_in3>;};};};};funnel@ed001000 {compatible = "arm,coresight-funnel", "arm,primecell";reg = <0 0xed001000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";out-ports {port {cluster1_funnel_out: endpoint {remote-endpoint =<&cluster1_etf_in>;};};};in-ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;cluster1_funnel_in0: endpoint {remote-endpoint = <&etm4_out>;};};port@1 {reg = <1>;cluster1_funnel_in1: endpoint {remote-endpoint = <&etm5_out>;};};port@2 {reg = <2>;cluster1_funnel_in2: endpoint {remote-endpoint = <&etm6_out>;};};port@3 {reg = <3>;cluster1_funnel_in3: endpoint {remote-endpoint = <&etm7_out>;};};};};etf@ed002000 {compatible = "arm,coresight-tmc", "arm,primecell";reg = <0 0xed002000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";in-ports {port {cluster1_etf_in: endpoint {remote-endpoint =<&cluster1_funnel_out>;};};};out-ports {port {cluster1_etf_out: endpoint {remote-endpoint =<&top_funnel_in1>;};};};};/* Top internals */funnel@ec031000 {compatible = "arm,coresight-funnel", "arm,primecell";reg = <0 0xec031000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";out-ports {port {top_funnel_out: endpoint {remote-endpoint =<&top_etf_in>;};};};in-ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;top_funnel_in0: endpoint {remote-endpoint =<&cluster0_etf_out>;};};port@1 {reg = <0>;
Here should s/<0>/<1>; otherwise DTC will complain warning for mismatching between 'port@1' and 'reg = <0>'. -- if reg set to 1, then there's no data output from cluster 1 to top.
/* there's an invisible funnel combo *//* between clusters and top funnel */
-- for the above reason.
top_funnel_in1: endpoint {remote-endpoint =<&cluster1_etf_out>;};};};};etf@ec036000 {compatible = "arm,coresight-tmc", "arm,primecell";reg = <0 0xec036000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";in-ports {port {top_etf_in: endpoint {remote-endpoint =<&top_funnel_out>;};};};out-ports {port {top_etf_out: endpoint {remote-endpoint =<&replicator_in>;};};};};replicator {compatible = "arm,coresight-replicator";clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";in-ports {port {replicator_in: endpoint {remote-endpoint =<&top_etf_out>;};};};out-ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;replicator0_out0: endpoint {remote-endpoint = <&etr_in>;};};port@1 {reg = <1>;replicator0_out1: endpoint {remote-endpoint = <&tpiu_in>;};};};};etr@ec033000 {compatible = "arm,coresight-tmc", "arm,primecell";reg = <0 0xec033000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";in-ports {port {etr_in: endpoint {remote-endpoint =<&replicator0_out0>;};};};};tpiu@ec032000 {compatible = "arm,coresight-tpiu", "arm,primecell";reg = <0 0xec032000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";in-ports {port {tpiu_in: endpoint {remote-endpoint =<&replicator0_out1>;};};};};- };
+}; diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi index a4a3d08..8f2fede 100644 --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi @@ -1246,3 +1246,5 @@ }; }; };
+#include "hi3660-coresight.dtsi"
Except the up mentioned issue, this patch looks good to me.
After applying this patch though I can see Coresight devices under /sys/bus/coresight, but I cannot capture ETM trace data by using below two commands:
perf record -e cs_etm/@ec036000.etf/ --per-thread ./main perf record -e cs_etm/@ec033000.etr/ --per-thread ./main
I checked perf report command with '-D' option, I can see the perf.data file doesn't contain any trace data from ETM. could you tell me which test command you are using? At my side I will debug a bit on my Hikey960 board and will keep you posted if find anything. -- my test command -- echo 1 > /sys/bus/coresight/devices/ec033000.etr -- echo 1 > /sys/bus/coresight/devices/ecc40000.etm/enable_source
Thanks, Leo Yan
On Sat, Mar 02, 2019 at 09:45:22AM +0000, Shiwanglai wrote:
[...]
/* Top internals */funnel@ec031000 {compatible = "arm,coresight-funnel", "arm,primecell";reg = <0 0xec031000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";out-ports {port {top_funnel_out: endpoint {remote-endpoint =<&top_etf_in>;};};};in-ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;top_funnel_in0: endpoint {remote-endpoint =<&cluster0_etf_out>;};};port@1 {reg = <0>;Here should s/<0>/<1>; otherwise DTC will complain warning for mismatching between 'port@1' and 'reg = <0>'.
-- if reg set to 1, then there's no data output from cluster 1 to top.
Thanks for the info, Wanglai. Now I see why write as it is.
I can confirm if directly use your patch with perf with mainline kernel I can capture CoreSight trace data successfully on Hikey960 board.
But since this DT binding will introduce DTC warning, I personally think we can improve for this with below method:
We can create a funnel node named "funnel_combo", and we don't need to specify register address range for it; and cluster 0 and cluster 1 will output to "funnel_combo" and "funnel_combo" will output to the top funnel. Thus the DT binding will write as below.
To support for a funnel without any register address range (we have support replicator like this mode), we also need to extend the driver drivers/hwtracing/coresight/coresight-funnel.c.
Mathieu, Mike, Suzuki, could you help confirm this is the right direction we should move forward to?
---8<--- /* An invisible combo funnel between clusters and top funnel */ funnel_combo { compatible = "arm,coresight-funnel"; clocks = <&crg_ctrl HI3660_PCLK>; clock-names = "apb_pclk";
out-ports { port { combo_funnel_out: endpoint { remote-endpoint = <&top_funnel_in>; }; }; };
in-ports { #address-cells = <1>; #size-cells = <0>;
port@0 { reg = <0>; combo_funnel_in0: endpoint { remote-endpoint = <&cluster0_etf_out>; }; };
port@1 { reg = <1>; combo_funnel_in1: endpoint { remote-endpoint = <&cluster1_etf_out>; }; }; }; };
/* Top internals */ funnel@ec031000 { compatible = "arm,coresight-funnel", "arm,primecell"; reg = <0 0xec031000 0 0x1000>; clocks = <&crg_ctrl HI3660_PCLK>; clock-names = "apb_pclk";
out-ports { port { top_funnel_out: endpoint { remote-endpoint = <&top_etf_in>; }; }; };
in-ports { #address-cells = <1>; #size-cells = <0>;
port@0 { reg = <0>; top_funnel_in: endpoint { remote-endpoint = <&combo_funnel_out>; }; }; }; };
--->8---
[...]
Thanks, Leo Yan
On Sat, 2 Mar 2019 at 06:00, Leo Yan leo.yan@linaro.org wrote:
On Sat, Mar 02, 2019 at 09:45:22AM +0000, Shiwanglai wrote:
[...]
/* Top internals */funnel@ec031000 {compatible = "arm,coresight-funnel", "arm,primecell";reg = <0 0xec031000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";out-ports {port {top_funnel_out: endpoint {remote-endpoint =<&top_etf_in>;};};};in-ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;top_funnel_in0: endpoint {remote-endpoint =<&cluster0_etf_out>;};};port@1 {reg = <0>;Here should s/<0>/<1>; otherwise DTC will complain warning for mismatching between 'port@1' and 'reg = <0>'.
-- if reg set to 1, then there's no data output from cluster 1 to top.
Thanks for the info, Wanglai. Now I see why write as it is.
I can confirm if directly use your patch with perf with mainline kernel I can capture CoreSight trace data successfully on Hikey960 board.
But since this DT binding will introduce DTC warning, I personally think we can improve for this with below method:
We can create a funnel node named "funnel_combo", and we don't need to specify register address range for it; and cluster 0 and cluster 1 will output to "funnel_combo" and "funnel_combo" will output to the top funnel. Thus the DT binding will write as below.
To support for a funnel without any register address range (we have support replicator like this mode), we also need to extend the driver drivers/hwtracing/coresight/coresight-funnel.c.
Mathieu, Mike, Suzuki, could you help confirm this is the right direction we should move forward to?
Leo, thanks for testing this out. Shiwanglai, please add Suzuki and myself to future revision of this set - this will help you getting a timely response for your work.
As Leo pointed out we already have support for replicators that don't have a register map and the same thing should be done in this case. But contrary to what was done for replicators, I think we should keep the drivers in the same file as Russell did here[1]. That way we can keep all things funnel at the same place and reduce the amount of kernel configuration options.
Regards, Mathieu
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/amba-pl011...
---8<--- /* An invisible combo funnel between clusters and top funnel */ funnel_combo { compatible = "arm,coresight-funnel"; clocks = <&crg_ctrl HI3660_PCLK>; clock-names = "apb_pclk";
out-ports { port { combo_funnel_out: endpoint { remote-endpoint = <&top_funnel_in>; }; }; }; in-ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; combo_funnel_in0: endpoint { remote-endpoint = <&cluster0_etf_out>; }; }; port@1 { reg = <1>; combo_funnel_in1: endpoint { remote-endpoint = <&cluster1_etf_out>; }; }; }; }; /* Top internals */ funnel@ec031000 { compatible = "arm,coresight-funnel", "arm,primecell"; reg = <0 0xec031000 0 0x1000>; clocks = <&crg_ctrl HI3660_PCLK>; clock-names = "apb_pclk"; out-ports { port { top_funnel_out: endpoint { remote-endpoint = <&top_etf_in>; }; }; }; in-ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; top_funnel_in: endpoint { remote-endpoint = <&combo_funnel_out>; }; }; }; };--->8---
[...]
Thanks, Leo Yan
Hi,
On 04/03/2019 17:18, Mathieu Poirier wrote:
On Sat, 2 Mar 2019 at 06:00, Leo Yan leo.yan@linaro.org wrote:
On Sat, Mar 02, 2019 at 09:45:22AM +0000, Shiwanglai wrote:
[...]
/* Top internals */funnel@ec031000 {compatible = "arm,coresight-funnel", "arm,primecell";reg = <0 0xec031000 0 0x1000>;clocks = <&crg_ctrl HI3660_PCLK>;clock-names = "apb_pclk";out-ports {port {top_funnel_out: endpoint {remote-endpoint =<&top_etf_in>;};};};in-ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;top_funnel_in0: endpoint {remote-endpoint =<&cluster0_etf_out>;};};port@1 {reg = <0>;Here should s/<0>/<1>; otherwise DTC will complain warning for mismatching between 'port@1' and 'reg = <0>'.
-- if reg set to 1, then there's no data output from cluster 1 to top.
Thanks for the info, Wanglai. Now I see why write as it is.
I can confirm if directly use your patch with perf with mainline kernel I can capture CoreSight trace data successfully on Hikey960 board.
But since this DT binding will introduce DTC warning, I personally think we can improve for this with below method:
We can create a funnel node named "funnel_combo", and we don't need to specify register address range for it; and cluster 0 and cluster 1 will output to "funnel_combo" and "funnel_combo" will output to the top funnel. Thus the DT binding will write as below.
To support for a funnel without any register address range (we have support replicator like this mode), we also need to extend the driver drivers/hwtracing/coresight/coresight-funnel.c.
Mathieu, Mike, Suzuki, could you help confirm this is the right direction we should move forward to?
Leo, thanks for testing this out. Shiwanglai, please add Suzuki and myself to future revision of this set - this will help you getting a timely response for your work.
As Leo pointed out we already have support for replicators that don't have a register map and the same thing should be done in this case. But contrary to what was done for replicators, I think we should keep the drivers in the same file as Russell did here[1]. That way we can keep all things funnel at the same place and reduce the amount of kernel configuration options.
I back that. I have already merged the static and programmable replicators into one and this should be out here soon. So, I back the single driver approach for funnels.
Cheers Suzuki
Regards, Mathieu
[1]. https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/amba-pl011...
On Mon, Mar 04, 2019 at 05:29:22PM +0000, Suzuki K Poulose wrote:
[...]
To support for a funnel without any register address range (we have support replicator like this mode), we also need to extend the driver drivers/hwtracing/coresight/coresight-funnel.c.
Mathieu, Mike, Suzuki, could you help confirm this is the right direction we should move forward to?
Leo, thanks for testing this out. Shiwanglai, please add Suzuki and myself to future revision of this set - this will help you getting a timely response for your work.
Yeah, I didn't give a good example for this; just remind to Wanglai, usually we can add maintainers' email address in the "To:" list so this will be more friendly for email filters.
As Leo pointed out we already have support for replicators that don't have a register map and the same thing should be done in this case. But contrary to what was done for replicators, I think we should keep the drivers in the same file as Russell did here[1]. That way we can keep all things funnel at the same place and reduce the amount of kernel configuration options.
I back that. I have already merged the static and programmable replicators into one and this should be out here soon. So, I back the single driver approach for funnels.
That's great! Thanks for advices, Mathieu & Suzuki.
Will sync with you offline for proceeding.
Thanks, Leo Yan
-
Leo Yan -
Mathieu Poirier -
Shiwanglai -
Suzuki K Poulose