On Mon, 3 Feb 2020 at 00:12, Michal Simek michal.simek@xilinx.com wrote:
On 31. 01. 20 20:37, Wojciech Żmuda wrote:
Hello Mathieu, thank you for quick review! Please see my responses inline
On 31/01/2020, 19:47, "Mathieu Poirier" mathieu.poirier@linaro.org wrote:
On Fri, 31 Jan 2020 at 11:36, Mathieu Poirier mathieu.poirier@linaro.org wrote:
Hi Wojciech,
On Thu, Jan 30, 2020 at 03:36:27PM +0000, Wojciech Żmuda wrote:
From: Wojciech Zmuda wzmuda@n7space.com
Add nodes for the following CoreSight components:
- ETMs for A53 cores
- debug components for A53 cores
- funnel gathering outputs from A53 ETMs and SoC-wide funnels
- the only replicator
- all TMCs: 4k ETF, 8k ETF and ETR
- TPIU
Signed-off-by: Wojciech Zmuda wzmuda@n7space.com
arch/arm64/boot/dts/xilinx/zynqmp-coresight.dtsi | 272 +++++++++++++++++++++++ arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 2 + 2 files changed, 274 insertions(+) create mode 100644 arch/arm64/boot/dts/xilinx/zynqmp-coresight.dtsi
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-coresight.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp-coresight.dtsi new file mode 100644 index 000000000000..8b7579ad89cc --- /dev/null +++ b/arch/arm64/boot/dts/xilinx/zynqmp-coresight.dtsi @@ -0,0 +1,272 @@ +// SPDX-License-Identifier: GPL-2.0+
Looking at other Xilinx DT files there is no space between the SPDX identifier and the header of the file.
Ok, I'll remove the extra space.
+/*
- dtsi for Xilinx Ultrascale+ MPSoC CoreSight components
- Copyright (C) 2019-2020 N7 Space Sp. z o.o.
- Author: Wojciech Zmuda wzmuda@n7space.com
- */
+/ {
etm0@fec40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xfec40000 0 0x1000>;cpu = <&cpu0>;clocks = <&clk100>;clock-names = "apb_pclk";out-ports {port {etm0_out_port: endpoint {remote-endpoint = <&funnel1_in_port0>;};};};};etm1@fed40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xfed40000 0 0x1000>;cpu = <&cpu1>;clocks = <&clk100>;clock-names = "apb_pclk";out-ports {port {etm1_out_port: endpoint {remote-endpoint = <&funnel1_in_port1>;};};};};etm2@fee40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xfee40000 0 0x1000>;cpu = <&cpu2>;clocks = <&clk100>;clock-names = "apb_pclk";out-ports {port {etm2_out_port: endpoint {remote-endpoint = <&funnel1_in_port2>;};};};};etm3@fef40000 {compatible = "arm,coresight-etm4x", "arm,primecell";reg = <0 0xfef40000 0 0x1000>;cpu = <&cpu3>;clocks = <&clk100>;clock-names = "apb_pclk";out-ports {port {etm3_out_port: endpoint {remote-endpoint = <&funnel1_in_port3>;};};};};debug0@fec10000 {compatible = "arm,coresight-cpu-debug", "arm,primecell";reg = <0 0xfec10000 0 0x1000>;cpu = <&cpu0>;clocks = <&clk100>;clock-names = "apb_pclk";};debug1@fed10000 {compatible = "arm,coresight-cpu-debug", "arm,primecell";reg = <0 0xfed10000 0 0x1000>;cpu = <&cpu1>;clocks = <&clk100>;clock-names = "apb_pclk";};debug2@fee10000 {compatible = "arm,coresight-cpu-debug", "arm,primecell";reg = <0 0xfee10000 0 0x1000>;cpu = <&cpu2>;clocks = <&clk100>;clock-names = "apb_pclk";};debug3@fee10000 {compatible = "arm,coresight-cpu-debug", "arm,primecell";reg = <0 0xfef10000 0 0x1000>;cpu = <&cpu3>;clocks = <&clk100>;clock-names = "apb_pclk";};funnel1@fe920000 {compatible = "arm,coresight-dynamic-funnel", "arm,primecell";reg = <0 0xfe920000 0 0x1000>;clocks = <&clk100>;clock-names = "apb_pclk";in-ports {#address-cells = <0x1>;#size-cells = <0x0>;port@0 {reg = <0x0>;funnel1_in_port0: endpoint {remote-endpoint = <&etm0_out_port>;};};port@1 {reg = <0x1>;funnel1_in_port1: endpoint {remote-endpoint = <&etm1_out_port>;};};port@2 {reg = <0x2>;funnel1_in_port2: endpoint {remote-endpoint = <&etm2_out_port>;};};port@3 {reg = <0x3>;funnel1_in_port3: endpoint {remote-endpoint = <&etm3_out_port>;};};};out-ports {port {funnel1_out_port0: endpoint {remote-endpoint = <&etf1_in_port>;};};};};funnel2@fe930000 {compatible = "arm,coresight-dynamic-funnel", "arm,primecell";reg = <0 0xfe930000 0 0x1000>;clocks = <&clk100>;clock-names = "apb_pclk";in-ports {#address-cells = <0x1>;#size-cells = <0x0>;port@2 {reg = <0x2>;funnel2_in_port2: endpoint {remote-endpoint = <&etf1_out_port>;};};// Funnel2 has another input port connected to// funnel0's output. Funnel0 gathers Cortex-R5 ETMs.C++ style comments.
Ok, I'll go with: /*
- Funnel2 has another input port connected to
- funnel0's output. Funnel0 gathers Cortex-R5 ETMs.
*/
};out-ports {port {funnel2_out_port0: endpoint {remote-endpoint = <&etf2_in_port>;};};};};etf1@fe940000 {compatible = "arm,coresight-tmc", "arm,primecell";reg = <0 0xfe940000 0 0x1000>;clocks = <&clk100>;clock-names = "apb_pclk";in-ports {port {etf1_in_port: endpoint {remote-endpoint = <&funnel1_out_port0>;};};};out-ports {port {etf1_out_port: endpoint {remote-endpoint = <&funnel2_in_port2>;};};};};etf2@fe950000 {compatible = "arm,coresight-tmc", "arm,primecell";reg = <0 0xfe950000 0 0x1000>;clocks = <&clk100>;clock-names = "apb_pclk";in-ports {port {etf2_in_port: endpoint {remote-endpoint = <&funnel2_out_port0>;};};};out-ports {port {etf2_out_port: endpoint {remote-endpoint =<&replicator_in_port0>;};};};};replicator {compatible = "arm,coresight-static-replicator";in-ports {port {replicator_in_port0: endpoint {remote-endpoint = <&etf2_out_port>;};};};out-ports {#address-cells = <0x1>;#size-cells = <0x0>;port@0 {reg = <0x0>;replicator_out_port0: endpoint {remote-endpoint = <&etr_in_port>;};};port@1 {reg = <0x1>;replicator_out_port1: endpoint {remote-endpoint = <&tpiu_in_port>;};};};};etr@fe970000 {compatible = "arm,coresight-tmc", "arm,primecell";reg = <0 0xfe970000 0 0x1000>;clocks = <&clk100>;clock-names = "apb_pclk";in-ports {port {etr_in_port: endpoint {remote-endpoint =<&replicator_out_port0>;};};};};tpiu@fe980000 {compatible = "arm,coresight-tpiu", "arm,primecell";reg = <0 0xfe980000 0 0x1000>;clocks = <&clk100>;clock-names = "apb_pclk";in-ports {port {tpiu_in_port: endpoint {remote-endpoint =<&replicator_out_port1>;};};};};+};
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi index 3c731e73903a..ca0a6b9f4445 100644 --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi @@ -12,6 +12,8 @@
- the License, or (at your option) any later version.
*/
+#include "zynqmp-coresight.dtsi"
Those bindings are correctly used. If Michal doesn't mind the nit-picks I have highlighted above then
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
Otherwise feel free to add my tag to your next revision, which I advise you to send out once v5.6-rc1 comes out.
Great, thanks. I'll keep an eye for v5.6-rc1.
I forgot... How does power management work on this board? Is the power domain where the CS blocks are powered up by the FW? And what about state retention when processors go idle? Should this be taken care of in the drivers or is the PMIC properly handling low retention states? Failure to properly address both cases will likely hang the board (at boot time of when processors are idled).
To be honest, I don't know. I use kernel with CPU idle permanently disabled, as was advised somewhere in CoreSight-related documents I found at the beginning of my experiments. It might've been a presentation from Linaro Connect, either yours or Leo's. I didn't do any extra steps to power CS blocks on, so I guess they're either powered on by default, or it's taken care of by either FSBL, U-boot or ATF. I use those three provided by Xilinx. I didn't experiment with vanilla U-boot or ATF.
Can you please suggest some experiments I in this matter? Would turning off CPU idle in kernel's config and tracing a migrating process be sufficient for such test?
This is a fairly new feature that Andrew finished in September [1]. It allows to save/restore the coresight tracers' configuration when CPUs enter idle states, allowing coresight to be used when CPUIdle is enabled. This is necessary when the HW line driven by the PU bit of configuration register TRCPDCR is not connected to a PMIC or the PMIC's firmware is ignoring the signal. It is very likely that either one of those scenarios are enacted on this platform.
To deal with power management issues on the ETM blocks, re-enable CPUIdle and add "arm,coresight-loses-context-with-cpu;" to each of the ETM node specification in the DT (see bindings for details). That should take care of the ETMs only, which are usually in the CPU power domain. The rest of the coresight blocks are generally located in the debug power domain, something I can't help with. It is either enabled at boot time by the boot loader or the kernel, or handled dynamically at run time. See the "power-domains" property in the Juno coresight specification[2].
[1]. https://www.spinics.net/lists/devicetree/msg309139.html [2]. https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/arm/juno-...
Jolly: Can you please take a look how this block is powered and how power stuff should be handled?
Thanks, Michal