Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1493382 > unrolled thread
| Started by | Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> |
|---|---|
| First post | 2016-09-29 13:00 +0200 |
| Last post | 2016-10-03 16:20 +0200 |
| Articles | 4 — 1 participant |
Back to article view | Back to linux.kernel
[PATCH] Adding Support for Coresight Components on Zynq 7000. Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> - 2016-09-29 13:00 +0200
Re: [PATCH] Adding Support for Coresight Components on Zynq 7000. Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> - 2016-09-30 09:50 +0200
Re: [PATCH] Adding Support for Coresight Components on Zynq 7000. Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> - 2016-10-03 15:00 +0200
Re: [PATCH] Adding Support for Coresight Components on Zynq 7000. Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> - 2016-10-03 16:20 +0200
| From | Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> |
|---|---|
| Date | 2016-09-29 13:00 +0200 |
| Subject | [PATCH] Adding Support for Coresight Components on Zynq 7000. |
| Message-ID | <smHei-33A-31@gated-at.bofh.it> |
The Coresight components are present on the Zynq SoC but the corresponding
device tree entries are missing. This patch adds device tree entries for
coresight components while explaining how it was done in order to allow
porting towards other boards easily.
By adding the entries for Coresight components in the device tree: if no
files are created in sysfile system, you need to contact the board designer
to sort out the problem. On some boards, Coresight components are not
powered on boot.
Signed-off-by: Muhammad Abdul Wahab <muhammadabdul.wahab@centralesupelec.fr>
---
The documentation file was very helpful
(Documentation/devicetree/bindings/arm/coresight.txt). However, few details
can be added to make it more clear for beginners.
Things to modify in device tree when changing the board are mainly:
- address
- `clocks` field
- some references in other entries may be missing (e.g. for `CPU` field in
ETM/PTM component, references need to be created)
Furthermore, the `reg` field should be adapted according to
`#address-cells` and `#size-cells`. It may appear obvious, not for
beginners.
## Testing
The trace sink components need to be enabled by accessing through sysfile
system.
echo 1 > /sys/bus/coresight/devices/@addr.etb/enable\_sink
Then enable the CS source component:
echo 1 > /sys/bus/coresight/devices/@addr.ptm/enable\_source
By default, CS Source components are configured to trace the kernel.
Then the trace can be read by dumping ETB.
dd if=/dev/@addr.etb of=trace_kernel.bin
The trace can be visualized by:
hexdump -C trace_kernel.bin
Or stored using:
hexdump -C trace_kernel.bin > trace_kernel.txt
The trace need to be decoded to be readable. All these above steps can now
be performed with Perf Library which was not available at the time I was
playing with DT entries.
--- linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi.orig 2016-07-24
21:23:50.000000000 +0200
+++ linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi 2016-09-28
19:13:52.651881000 +0200
@@ -363,5 +363,159 @@
reg = <0xf8005000 0x1000>;
timeout-sec = <10>;
};
+
+ etb@F8801000 {
+ compatible = "arm,coresight-etb10", "arm,primecell";
+ reg = <0xf8801000 0x1000>;
+ coresight-default-sink;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+
+ endpoint@0 {
+ slave-mode;
+ remote-endpoint = <0x8>;
+ linux,phandle = <0xd>;
+ phandle = <0xd>;
+ };
+ };
+ };
+
+ tpiu@F8803000 {
+ compatible = "arm,coresight-tpiu", "arm,primecell";
+ reg = <0xf8803000 0x1000>;
+ clocks = <&clkc 47>, <&clkc 16>;
+ clock-names = "apb_pclk", "fclk1";
+ clock-frequency=<0xee6b280>;
+
+ port {
+
+ endpoint@0 {
+ slave-mode;
+ remote-endpoint = <0x9>;
+ linux,phandle = <0xe>;
+ phandle = <0xe>;
+ };
+ };
+ };
+
+ funnel@F8804000 {
+ compatible = "arm,coresight-funnel", "arm,primecell";
+ reg = <0xf8804000 0x1000>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+
+ port@0 {
+ reg = <0x0>;
+
+ endpoint {
+ remote-endpoint = <0xa>;
+ linux,phandle = <0xf>;
+ phandle = <0xf>;
+ };
+ };
+
+ port@1 {
+ reg = <0x0>;
+
+ endpoint {
+ slave-mode;
+ remote-endpoint = <0xb>;
+ linux,phandle = <0x11>;
+ phandle = <0x11>;
+ };
+ };
+
+ port@2 {
+ reg = <0x1>;
+
+ endpoint {
+ slave-mode;
+ remote-endpoint = <0xc>;
+ linux,phandle = <0x13>;
+ phandle = <0x13>;
+ };
+ };
+ };
+ };
+
+ replicator {
+ compatible = "arm,coresight-replicator";
+
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+
+ port@0 {
+ reg = <0x0>;
+
+ endpoint {
+ remote-endpoint = <0xd>;
+ linux,phandle = <0x8>;
+ phandle = <0x8>;
+ };
+ };
+
+ port@1 {
+ reg = <0x1>;
+
+ endpoint {
+ remote-endpoint = <0xe>;
+ linux,phandle = <0x9>;
+ phandle = <0x9>;
+ };
+ };
+
+ port@2 {
+ reg = <0x0>;
+
+ endpoint {
+ slave-mode;
+ remote-endpoint = <0xf>;
+ linux,phandle = <0xa>;
+ phandle = <0xa>;
+ };
+ };
+ };
+ };
+
+ ptm0@F889C000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889c000 0x1000>;
+ cpu = <0x10>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+
+ endpoint {
+ remote-endpoint = <0x11>;
+ linux,phandle = <0xb>;
+ phandle = <0xb>;
+ };
+ };
+ };
+
+ ptm1@F889D000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889d000 0x1000>;
+ cpu = <0x12>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+
+ endpoint {
+ remote-endpoint = <0x13>;
+ linux,phandle = <0xc>;
+ phandle = <0xc>;
+ };
+ };
+ };
};
};
[toc] | [next] | [standalone]
| From | Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> |
|---|---|
| Date | 2016-09-30 09:50 +0200 |
| Message-ID | <sn0JX-7gi-25@gated-at.bofh.it> |
| In reply to | #1493382 |
Hi Sören,
Thank you for your remarks. I corrected a few things as you suggested.
> I'm curious, did you test that with external debug tools. I have the
> feeling the kernel using the debug HW could interfere with JTAG
> debuggers, external trace tools, etc.
I did not test with any external debug tools. For testing, I obtained
trace for simple function (e.g. loop) on Xilinx standalone and I then
traced the same function under Linux. Then, I compare both traces
obtained under standalone and under Linux. I decoded the trace to make
sure that the trace I get corresponds to my function. The instructions
for testing here only allows users to see that coresight components
are enabled and generating traces. The trace still need to be decoded
to make sure whether the trace is correct or not. It can be done by
openCSD[1] library.
> Use labels please.
I changed the entries to use labels. I don't need phandle anymore.
>> + tpiu@F8803000 {
>>>
>>> + compatible = "arm,coresight-tpiu", "arm,primecell";
>>> + reg = <0xf8803000 0x1000>;
>>> + clocks = <&clkc 47>, <&clkc 16>;
>>
>
> I'm not sure this is correct for every setup. Sorry, that I don't recall
> all the details, I haven't used tracing in a long time. But I guess this
> clock is configurable as you're referring an fclk here. The other thing
> that makes me a little suspicious is, that nothing in here uses the
> 'dbg_trc' clock that the clock controller provides.
The TPIU setup included in my patch is specific to my configuration of TPIU.
The second clock is configurable but I didn't know how to do so. As in
Vivado setup I chose "fclk1" as the clock for TPIU, I decided to put fclk1.
But, I am not sure about this. I am having some problems when I recover the
trace from TPIU: it is not the same as in ETB even though it resembles a
lot.
I don't know if the problem is coming from the device tree or from the
driver.
I will have a look at 'dbg_trc' clock.
>>
>> + clock-names = "apb_pclk", "fclk1";
>>
> Those names (at least fclk1) is not a good name for tpiu to identify
> it's input. fclk1 is a zynq-specific clock, and as mentioned above, it
> seems likely that this could easily become a different one. The
> clock-names are meant to identify an input from the consumer's
> perspective. The correct names should be documented in the DT binding.
The first name was chosen as "apb_pclk" because it was recommended in
Documentation. On the second name, I agree with you but again for TPIU DT
entry, I am not sure how to make this clock configurable. So, that's why
I put the same clock name as I had in my Vivado setup. If you have any
leads on how to make it configurable, I will be happy to take a look
into it.
>> + clock-frequency=<0xee6b280>;
>>
> I cannot find this property in the binding.
>
This property was described in clock binding (in an example [2]) and the
value here (250MHz) corresponds to the value I had chosen in Vivado for
TPIU input clock.
> I think nodes were ordered alphabetically in our DTs.
Yes, I modified the patch to take care of it. Thank you.
Muhammad Abdul WAHAB
[1]: https://github.com/Linaro/OpenCSD
[2]: Documentation/devicetree/bindings/clock/clock-bindings.txt
---
--- linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi.orig 2016-07-24
21:23:50.000000000 +0200
+++ linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi 2016-09-29
20:42:44.286322433 +0200
@@ -96,6 +96,57 @@
rx-fifo-depth = <0x40>;
};
+ etb@F8801000 {
+ compatible = "arm,coresight-etb10", "arm,primecell";
+ reg = <0xf8801000 0x1000>;
+
+ coresight-default-sink;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+ etb_in_port: endpoint@0 {
+ slave-mode;
+ remote-endpoint = <&replicator_out_port0>;
+ };
+ };
+ };
+
+ funnel@F8804000 {
+ compatible = "arm,coresight-funnel", "arm,primecell";
+ reg = <0xf8804000 0x1000>;
+
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+
+ port@0 {
+ reg = <0x0>;
+ funnel_out_port0: endpoint {
+ remote-endpoint = <&replicator_in_port0>;
+ };
+ };
+
+ port@1 {
+ reg = <0x0>;
+ funnel_in_port0: endpoint {
+ slave-mode;
+ remote-endpoint = <&ptm0_out_port>;
+ };
+ };
+
+ port@2 {
+ reg = <0x1>;
+ funnel_in_port1: endpoint {
+ slave-mode;
+ remote-endpoint = <&ptm1_out_port>;
+ };
+ };
+ };
+ };
+
gpio0: gpio@e000a000 {
compatible = "xlnx,zynq-gpio-1.0";
#gpio-cells = <2>;
@@ -311,6 +362,82 @@
clocks = <&clkc 4>;
};
+ ptm0@F889C000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889c000 0x1000>;
+
+ cpu = <&cpu0>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+ ptm0_out_port: endpoint {
+ remote-endpoint = <&funnel_in_port0>;
+ };
+ };
+ };
+
+ ptm1@F889D000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889d000 0x1000>;
+
+ cpu = <&cpu1>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+ ptm1_out_port: endpoint {
+ remote-endpoint = <&funnel_in_port1>;
+ };
+ };
+ };
+
+ replicator {
+ compatible = "arm,coresight-replicator";
+
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+
+ port@0 {
+ reg = <0x0>;
+ replicator_out_port0: endpoint {
+ remote-endpoint = <&etb_in_port>;
+ };
+ };
+
+ port@1 {
+ reg = <0x1>;
+ replicator_out_port1: endpoint {
+ remote-endpoint = <&tpiu_in_port>;
+ };
+ };
+
+ port@2 {
+ reg = <0x0>;
+ replicator_in_port0: endpoint {
+ slave-mode;
+ remote-endpoint = <&funnel_out_port0>;
+ };
+ };
+ };
+ };
+
+ tpiu@F8803000 {
+ compatible = "arm,coresight-tpiu", "arm,primecell";
+ reg = <0xf8803000 0x1000>;
+
+ clocks = <&clkc 47>, <&clkc 46>;
+ clock-names = "apb_pclk", "configurable_clk";
+ clock-frequency=<0xee6b280>;
+
+ port {
+ tpiu_in_port: endpoint@0 {
+ slave-mode;
+ remote-endpoint = <&replicator_out_port0>;
+ };
+ };
+ };
+
ttc0: timer@f8001000 {
interrupt-parent = <&intc>;
interrupts = <0 10 4>, <0 11 4>, <0 12 4>;
[toc] | [prev] | [next] | [standalone]
| From | Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> |
|---|---|
| Date | 2016-10-03 15:00 +0200 |
| Message-ID | <sob0B-52c-1@gated-at.bofh.it> |
| In reply to | #1493931 |
Hi Sören,
> I tried to refresh my Zynq knowledge a bit. The clkc provides the
> dbg_trc clock, and that is the clock you need (not fclk). I couldn't
> find it in the binding (I guess I messed that up), but apparently,
> you can provide a 'trace_emio_clk' as input to the clkc node in the
> Zynq DT. Then, with the muxes correctly configured (FSBL should do
> that if you select the EMIO trace clock in Vivado), the dbg_trc
> output of the clkc should be that EMIO clock. And the dbg_trc output
> of the clkc is what should be consumed by the tpiu node. Though, as
> I see it the binding/driver for the TPIU do not support that.
>
> I.e.
> In the clkc description you'd have to add 'trace_emio_clk' to the
> clock-names property together with a matching reference in the 'clocks'
> property. As this change would be specific to local setups, this is not
> really appropriate for upstream.
>
> Then, for the trace clock, ideally the TPIU would consume and enable it
> as needed.
Thank you very much for this. I will have a look into it.
Below is the patch without TPIU, is it possible to submit it ? I will
submit the TPIU part very soon once I manage to get it working.
> Unfortunately, this is not how it works. The DT bindings are not a
> recommendation. The DT description must follow the binding, otherwise
> drivers will not work correctly, or best case, just ignore what you put
> there.
Thanks. The idea of including TPIU part was to get feedback as I am far
from being an expert on DT.
> As I don't see this in the coresight binding, I doubt that it has any
> effect or should be here.
That's what I was thinking also. I will re-look into TPIU part and send
it soon. Besides, if I want to ask you a question regarding TPIU or DT,
can I contact you alone or should I keep sending it to all the CS/DT
maintainers ?
Thanks,
M.Abdul WAHAB
---
--- linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi.orig 2016-07-24
21:23:50.000000000 +0200
+++ linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi 2016-10-03
14:38:00.164515838 +0200
@@ -96,6 +96,57 @@
rx-fifo-depth = <0x40>;
};
+ etb@F8801000 {
+ compatible = "arm,coresight-etb10", "arm,primecell";
+ reg = <0xf8801000 0x1000>;
+
+ coresight-default-sink;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+ etb_in_port: endpoint@0 {
+ slave-mode;
+ remote-endpoint = <&replicator_out_port0>;
+ };
+ };
+ };
+
+ funnel@F8804000 {
+ compatible = "arm,coresight-funnel", "arm,primecell";
+ reg = <0xf8804000 0x1000>;
+
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+
+ port@0 {
+ reg = <0x0>;
+ funnel_out_port0: endpoint {
+ remote-endpoint = <&replicator_in_port0>;
+ };
+ };
+
+ port@1 {
+ reg = <0x0>;
+ funnel_in_port0: endpoint {
+ slave-mode;
+ remote-endpoint = <&ptm0_out_port>;
+ };
+ };
+
+ port@2 {
+ reg = <0x1>;
+ funnel_in_port1: endpoint {
+ slave-mode;
+ remote-endpoint = <&ptm1_out_port>;
+ };
+ };
+ };
+ };
+
gpio0: gpio@e000a000 {
compatible = "xlnx,zynq-gpio-1.0";
#gpio-cells = <2>;
@@ -311,6 +362,67 @@
clocks = <&clkc 4>;
};
+ ptm0@F889C000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889c000 0x1000>;
+
+ cpu = <&cpu0>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+ ptm0_out_port: endpoint {
+ remote-endpoint = <&funnel_in_port0>;
+ };
+ };
+ };
+
+ ptm1@F889D000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889d000 0x1000>;
+
+ cpu = <&cpu1>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+
+ port {
+ ptm1_out_port: endpoint {
+ remote-endpoint = <&funnel_in_port1>;
+ };
+ };
+ };
+
+ replicator {
+ compatible = "arm,coresight-replicator";
+
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+
+ port@0 {
+ reg = <0x0>;
+ replicator_out_port0: endpoint {
+ remote-endpoint = <&etb_in_port>;
+ };
+ };
+
+ port@1 {
+ reg = <0x1>;
+ replicator_out_port1: endpoint {
+ remote-endpoint = <&tpiu_in_port>;
+ };
+ };
+
+ port@2 {
+ reg = <0x0>;
+ replicator_in_port0: endpoint {
+ slave-mode;
+ remote-endpoint = <&funnel_out_port0>;
+ };
+ };
+ };
+ };
+
ttc0: timer@f8001000 {
interrupt-parent = <&intc>;
interrupts = <0 10 4>,
[toc] | [prev] | [next] | [standalone]
| From | Muhammad Abdul WAHAB <muhammadabdul.wahab@supelec.fr> |
|---|---|
| Date | 2016-10-03 16:20 +0200 |
| Message-ID | <socg1-62h-7@gated-at.bofh.it> |
| In reply to | #1494864 |
Hi again Sören,
> Sounds good. AFAICT, the change below should be OK. Probably some
> stylistic changes to make it blend in with the rest of the DT (e.g.
> use lower case characters in the address parts of the node name).
The change to low characters has been made for address part. I also
deleted some empty lines to respect the style of the rest of the DT.
> I'd say that depends on what it is about. If it is about DT and the TPIU
> Linux driver, I'd say, keep it on list and probably even include the
> authors of that driver (the folks the get_maintainers script is
> identifying for that driver).
>
> If it's specific to Zynq, the Xilinx forums can be quite helpful as
> there are a lot of people familiar with the device
> (https://forums.xilinx.com/t5/Embedded-Linux/bd-p/ELINUX).
>
> But when in doubt, feel free to reach out to me directly.
OK. Thank you !
M.Abdul WAHAB
---
--- linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi.orig 2016-07-24
21:23:50.000000000 +0200
+++ linux-4.7/arch/arm/boot/dts/zynq-7000.dtsi 2016-10-03
15:54:35.228460164 +0200
@@ -96,6 +96,51 @@
rx-fifo-depth = <0x40>;
};
+ etb@f8801000 {
+ compatible = "arm,coresight-etb10", "arm,primecell";
+ reg = <0xf8801000 0x1000>;
+ coresight-default-sink;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+ port {
+ etb_in_port: endpoint@0 {
+ slave-mode;
+ remote-endpoint = <&replicator_out_port0>;
+ };
+ };
+ };
+
+ funnel@f8804000 {
+ compatible = "arm,coresight-funnel", "arm,primecell";
+ reg = <0xf8804000 0x1000>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+ port@0 {
+ reg = <0x0>;
+ funnel_out_port0: endpoint {
+ remote-endpoint = <&replicator_in_port0>;
+ };
+ };
+ port@1 {
+ reg = <0x0>;
+ funnel_in_port0: endpoint {
+ slave-mode;
+ remote-endpoint = <&ptm0_out_port>;
+ };
+ };
+ port@2 {
+ reg = <0x1>;
+ funnel_in_port1: endpoint {
+ slave-mode;
+ remote-endpoint = <&ptm1_out_port>;
+ };
+ };
+ };
+ };
+
gpio0: gpio@e000a000 {
compatible = "xlnx,zynq-gpio-1.0";
#gpio-cells = <2>;
@@ -311,6 +356,59 @@
clocks = <&clkc 4>;
};
+ ptm0@f889c000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889c000 0x1000>;
+ cpu = <&cpu0>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+ port {
+ ptm0_out_port: endpoint {
+ remote-endpoint = <&funnel_in_port0>;
+ };
+ };
+ };
+
+ ptm1@f889d000 {
+ compatible = "arm,coresight-etm3x", "arm,primecell";
+ reg = <0xf889d000 0x1000>;
+ cpu = <&cpu1>;
+ clocks = <&clkc 47>;
+ clock-names = "apb_pclk";
+ port {
+ ptm1_out_port: endpoint {
+ remote-endpoint = <&funnel_in_port1>;
+ };
+ };
+ };
+
+ replicator {
+ compatible = "arm,coresight-replicator";
+ ports {
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+ port@0 {
+ reg = <0x0>;
+ replicator_out_port0: endpoint {
+ remote-endpoint = <&etb_in_port>;
+ };
+ };
+ port@1 {
+ reg = <0x1>;
+ replicator_out_port1: endpoint {
+ remote-endpoint = <&tpiu_in_port>;
+ };
+ };
+ port@2 {
+ reg = <0x0>;
+ replicator_in_port0: endpoint {
+ slave-mode;
+ remote-endpoint = <&funnel_out_port0>;
+ };
+ };
+ };
+ };
+
ttc0: timer@f8001000 {
interrupt-parent = <&intc>;
interrupts = <0 10 4>, <0 11 4>, <0 12 4>;
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web