Hi Suzuki,
thanks for the reply! The CPUs of the boards I am using are all based on Arm-v8(.2), but I found the components' addresses in the manuals of the SoCs.
I managed to modify the Devicetree by writing my own .dtsi file (see attachment) and finally got the CoreSight devices in /sys/devices/.
However, dmesg shows the following:
[ 0.000000] Booting Linux on physical CPU 0x0
[ 0.000000] Linux version 4.9.253-coresight (user@user-desktop) (gcc version 7.5.0 (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) ) #1 SMP PREEMPT Wed Jan 1 18:45:04 CET 2025
[ 0.000000] Boot CPU: AArch64 Processor [411fd071]
(omitted 87 lines)
[ 0.212039] DTS File Name: /home/user/Downloads/Linux_for_Tegra/source/public/kernel/kernel-4.9/arch/arm64/boot/dts/../../../../../../hardware/nvidia/platform/t210/porg/kernel-dts/tegra210-p3448-0000-p3449-0000-a02.dts
[ 0.212045] DTB Build time: Jan 1 2025 16:04:45
(omitted 35 lines)
[ 0.420616] DTS File Name: /home/user/Downloads/Linux_for_Tegra/source/public/kernel/kernel-4.9/arch/arm64/boot/dts/../../../../../../hardware/nvidia/platform/t210/porg/kernel-dts/tegra210-p3448-0000-p3449-0000-a02.dts
[ 0.420622] DTB Build time: Jan 1 2025 16:04:45
(omitted 75 lines)
[ 0.524166] OF: amba_device_add() failed (-19) for /funnel_bccplex@73001000
(omitted 367 lines)
[ 1.330484] OF: graph: no port node found in /etf@72030000
[ 1.330757] OF: graph: no port node found in /etr@72050000
[ 1.330987] OF: graph: no port node found in /funnel_major@72010000
[ 1.331238] OF: graph: no port node found in /ptm0@73440000
[ 1.331451] coresight-etm4x 73440000.ptm0: CPU0: Cortex-A57 ETM v4.0 initialized
[ 1.331482] OF: graph: no port node found in /ptm1@73540000
[ 1.331689] coresight-etm4x 73540000.ptm1: CPU1: Cortex-A57 ETM v4.0 initialized
[ 1.331719] OF: graph: no port node found in /ptm2@73640000
[ 1.331938] coresight-etm4x 73640000.ptm2: CPU2: Cortex-A57 ETM v4.0 initialized
[ 1.331944] extcon-disp-state extcon:disp-state: cable 47 state 0
[ 1.331946] Extcon AUX1(HDMI) disable
[ 1.331976] OF: graph: no port node found in /ptm3@73740000
[ 1.332192] coresight-etm4x 73740000.ptm3: CPU3: Cortex-A57 ETM v4.0 initialized
[ 1.332250] OF: graph: no port node found in /replicator@72040000
[ 1.332305] coresight-replicator-qcom 72040000.replicator: REPLICATOR 1.0 initialized
[ 1.332350] OF: graph: no port node found in /stm@72070000
[ 1.332386] coresight-stm 72070000.stm: stm_register_device failed, probing deffered
(omitted 64 lines)
[ 1.411751] OF: graph: no port node found in /stm@72070000
[ 1.412025] coresight-stm 72070000.stm: STM32 initialized
(omitted 212 lines)
Do you have an idea what I did wrong? In the end, I want to be able to follow the steps described here:
https://docs.nvidia.com/jetson/archives/l4t-archived/l4t-3275/index.html#pa…
Best regards,
Vincent
(P.S. There was a problem sending this email a first time, but it should work now)
On 17/02/2025 9:30 am, Jie Gan wrote:
> From: Jie Gan <jie.gan(a)oss.qualcomm.com>
>
> The Coresight TMC Control Unit(CTCU) device hosts miscellaneous configuration
> registers to control various features related to TMC ETR device.
>
> The CTCU device works as a helper device physically connected to the TMC ETR device.
> ---------------------------------------------------------
> |ETR0| |ETR1|
> . \ / .
> . \ / .
> . \ / .
> . \ / .
> ---------------------------------------------------
> ETR0ATID0-ETR0ATID3 CTCU ETR1ATID0-ETR1ATID3
> ---------------------------------------------------
> Each ETR has four ATID registers with 128 bits long in total.
> e.g. ETR0ATID0-ETR0ATID3 registers are used by ETR0 device.
>
> Based on the trace id which is programed in CTCU ATID register of
> specific ETR, trace data with that trace id can get into ETR's buffer
> while other trace data gets ignored. The number of CTCU ATID registers
> depends on the number of defined TMC ETR devices. For example, two TMC
> ETR devices need eight ATID registers. ETR0 with ETR0ATID0-ETR0ATID3
> and ETR1 with ETR1ATID0-ETRATID3.
>
> The significant challenge in enabling the data filter function is how
> to collect the trace ID of the source device. The introduction of
> trace_id callback function addresses this challenge. The callback function
> collects trace ID of the device and return it back. The trace ID will be
> stored in the structure called coresight_path and transmitted to helper
> and sink devices.
>
> The coresight_path structure is created to address how to transmit
> parameters needs by coresight_enable_path/coresight_disbale_path
> functions.
>
> Here is the definition of the struct coresight_path:
> /**
> * struct coresight_path - data needed by enable/disable path
> * @path: path from source to sink.
> * @trace_id: trace_id of the whole path.
> */
> struct coresight_path {
> struct list_head *path;
> u8 trace_id;
> };
>
> The atid_offset mentioned before is the offset to ATID register in CTCU
> device.
>
> Enabling the source device will configure one bit in the ATID register based
> on its trace ID.
> Disabling the source devices will reset the bit in the AITD register
> based on its trace ID.
>
> Useage:
> Enable:
> STM device with trace ID 5 and ETR0 is activated.
> Bitmap before the enablement:
> ETR0ATID0:
> 31..................543210
> ==========================
> 0000000000000000000000...0
> ==========================
>
> Bitmap after the enablement:
> 31..................543210
> ==========================
> 0000000000000...0000100000
> ==========================
>
> The bit 5 of the ETR0ATID0 register is configured to 1 when enabling the
> STM device.
>
> Disable:
> STM device with trace ID 5 and ETR0 is activated.
> Bitmap before the disablement:
> ETR0ATID0:
> 31................6543210
> =========================
> 000000000010111...0100000
> =========================
>
> Bitmap after the disablement
> ETR0ATID0:
> 31................6543210
> =========================
> 000000000010111...0000000
> =========================
>
> The bit 5 of the ETR0ATID0 register is reset to 0 when disabling the STM
> device.
>
> Sincere thanks to James Clark for providing an excellent idea to handle
> the trace_id of the path.
>
> Changes in V12:
> 1. Update the method for allocating trace_id for perf mode.
> Link to V11 - https://lore.kernel.org/linux-arm-msm/20250214024021.249655-1-quic_jiegan@q…
>
I tested the latest change, looks good to me.
On 17/02/2025 1:14 am, Jie Gan wrote:
>
>
> On 2/14/2025 7:09 PM, James Clark wrote:
>>
>>
>> On 14/02/2025 1:34 am, Jie Gan wrote:
>>>
>>>
>>> On 2/14/2025 12:00 AM, James Clark wrote:
>>>>
>>>>
>>>> On 07/02/2025 6:42 am, Jie Gan wrote:
>>>>> Add 'struct coresight_path' to store the data that is needed by
>>>>> coresight_enable_path/coresight_disable_path. The structure will be
>>>>> transmitted to any required devices to enable related
>>>>> funcationalities.
>>>>>
>>>>> The trace_id will be allocated after the path is built. Consequently,
>>>>> The ETM3x and ETM4x devices will directly read the trace_id from path
>>>>> which result in etm_read_alloc_trace_id and etm4_read_alloc_trace_id
>>>>> being deleted.
>>>>>
>>>>> Co-developed-by: James Clark <james.clark(a)linaro.org>
>>>>> Signed-off-by: James Clark <james.clark(a)linaro.org>
>>>>> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com>
>>>>> ---
>>>>> drivers/hwtracing/coresight/coresight-core.c | 106 ++++++++++++
>>>>> +-----
>>>>> drivers/hwtracing/coresight/coresight-dummy.c | 5 +-
>>>>> .../hwtracing/coresight/coresight-etm-perf.c | 30 +++--
>>>>> .../hwtracing/coresight/coresight-etm-perf.h | 2 +-
>>>>> drivers/hwtracing/coresight/coresight-etm.h | 1 -
>>>>> .../coresight/coresight-etm3x-core.c | 54 ++-------
>>>>> .../coresight/coresight-etm4x-core.c | 54 ++-------
>>>>> drivers/hwtracing/coresight/coresight-etm4x.h | 1 -
>>>>> drivers/hwtracing/coresight/coresight-priv.h | 12 +-
>>>>> drivers/hwtracing/coresight/coresight-stm.c | 3 +-
>>>>> drivers/hwtracing/coresight/coresight-sysfs.c | 17 ++-
>>>>> drivers/hwtracing/coresight/coresight-tpdm.c | 3 +-
>>>>> include/linux/coresight.h | 12 +-
>>>>> 13 files changed, 143 insertions(+), 157 deletions(-)
>>>>>
>>>> [...]
>>>>> @@ -352,7 +352,7 @@ static void *etm_setup_aux(struct perf_event
>>>>> *event, void **pages,
>>>>> * CPUs, we can handle it and fail the session.
>>>>> */
>>>>> for_each_cpu(cpu, mask) {
>>>>> - struct list_head *path;
>>>>> + struct coresight_path *path;
>>>>> struct coresight_device *csdev;
>>>>> csdev = per_cpu(csdev_src, cpu);
>>>>> @@ -405,15 +405,15 @@ static void *etm_setup_aux(struct perf_event
>>>>> *event, void **pages,
>>>>> cpumask_clear_cpu(cpu, mask);
>>>>> continue;
>>>>> }
>>>>> -
>>>>> /* ensure we can allocate a trace ID for this CPU */
>>>>> - trace_id = coresight_trace_id_get_cpu_id_map(cpu, &sink-
>>>>> >perf_sink_id_map);
>>>>> - if (!IS_VALID_CS_TRACE_ID(trace_id)) {
>>>>> + trace_id = coresight_path_assign_trace_id(path,
>>>>> CS_MODE_PERF);
>>>>> +
>>>>> + /* Can be 0 and valid, ETE doesn't need an ID */
>>>>> + if (trace_id < 0) {
>>>>
>>>> Not sure why I wrote it like this, but I think we should leave it as
>>>> it was with !IS_VALID_CS_TRACE_ID(). Even with ETE it calls the
>>>> trace ID allocator, so nothing has changed here.
>>>>
>>> Sure, Will restore. For ETE or ETM, we dont need traverse the path,
>>> just directly allocate the trace id based on cpu id.
>>>
>>> Jie
>>>
>>>
>>
>> Sorry I meant to only keep the !IS_VALID_CS_TRACE_ID() bit. We still
>> need to call the new coresight_path_assign_trace_id() otherwise it
>> doesn't get assigned to the path. I saw that got removed in v11.
>>
>>
> Sorry for the misunderstanding, I was focused on "nothing has changed
> here", lol.
>
> I got your point here.
> So the updated codes should be:
> ...
> /* ensure we can allocate a trace ID for this CPU */
> trace_id = coresight_path_assign_trace_id(path,
> CS_MODE_PERF);
> if (!IS_VALID_CS_TRACE_ID(trace_id)) {
> cpumask_clear_cpu(cpu, mask);
> coresight_release_path(path);
> continue;
> }
> ...
>
>
> Thanks,
> Jie
Yes that looks good.
On 14/02/2025 2:40 am, Jie Gan wrote:
> From: Jie Gan <jie.gan(a)oss.qualcomm.com>
>
> The Coresight TMC Control Unit(CTCU) device hosts miscellaneous configuration
> registers to control various features related to TMC ETR device.
>
> The CTCU device works as a helper device physically connected to the TMC ETR device.
> ---------------------------------------------------------
> |ETR0| |ETR1|
> . \ / .
> . \ / .
> . \ / .
> . \ / .
> ---------------------------------------------------
> ETR0ATID0-ETR0ATID3 CTCU ETR1ATID0-ETR1ATID3
> ---------------------------------------------------
> Each ETR has four ATID registers with 128 bits long in total.
> e.g. ETR0ATID0-ETR0ATID3 registers are used by ETR0 device.
>
> Based on the trace id which is programed in CTCU ATID register of
> specific ETR, trace data with that trace id can get into ETR's buffer
> while other trace data gets ignored. The number of CTCU ATID registers
> depends on the number of defined TMC ETR devices. For example, two TMC
> ETR devices need eight ATID registers. ETR0 with ETR0ATID0-ETR0ATID3
> and ETR1 with ETR1ATID0-ETRATID3.
>
> The significant challenge in enabling the data filter function is how
> to collect the trace ID of the source device. The introduction of
> trace_id callback function addresses this challenge. The callback function
> collects trace ID of the device and return it back. The trace ID will be
> stored in the structure called coresight_path and transmitted to helper
> and sink devices.
>
> The coresight_path structure is created to address how to transmit
> parameters needs by coresight_enable_path/coresight_disbale_path
> functions.
>
> Here is the definition of the struct coresight_path:
> /**
> * struct coresight_path - data needed by enable/disable path
> * @path: path from source to sink.
> * @trace_id: trace_id of the whole path.
> */
> struct coresight_path {
> struct list_head *path;
> u8 trace_id;
> };
>
> The atid_offset mentioned before is the offset to ATID register in CTCU
> device.
>
> Enabling the source device will configure one bit in the ATID register based
> on its trace ID.
> Disabling the source devices will reset the bit in the AITD register
> based on its trace ID.
>
> Useage:
> Enable:
> STM device with trace ID 5 and ETR0 is activated.
> Bitmap before the enablement:
> ETR0ATID0:
> 31..................543210
> ==========================
> 0000000000000000000000...0
> ==========================
>
> Bitmap after the enablement:
> 31..................543210
> ==========================
> 0000000000000...0000100000
> ==========================
>
> The bit 5 of the ETR0ATID0 register is configured to 1 when enabling the
> STM device.
>
> Disable:
> STM device with trace ID 5 and ETR0 is activated.
> Bitmap before the disablement:
> ETR0ATID0:
> 31................6543210
> =========================
> 000000000010111...0100000
> =========================
>
> Bitmap after the disablement
> ETR0ATID0:
> 31................6543210
> =========================
> 000000000010111...0000000
> =========================
>
> The bit 5 of the ETR0ATID0 register is reset to 0 when disabling the STM
> device.
>
> Sincere thanks to James Clark for providing an excellent idea to handle
> the trace_id of the path.
>
No worries! Thanks for working on Coresight too.
On 14/02/2025 1:34 am, Jie Gan wrote:
>
>
> On 2/14/2025 12:00 AM, James Clark wrote:
>>
>>
>> On 07/02/2025 6:42 am, Jie Gan wrote:
>>> Add 'struct coresight_path' to store the data that is needed by
>>> coresight_enable_path/coresight_disable_path. The structure will be
>>> transmitted to any required devices to enable related funcationalities.
>>>
>>> The trace_id will be allocated after the path is built. Consequently,
>>> The ETM3x and ETM4x devices will directly read the trace_id from path
>>> which result in etm_read_alloc_trace_id and etm4_read_alloc_trace_id
>>> being deleted.
>>>
>>> Co-developed-by: James Clark <james.clark(a)linaro.org>
>>> Signed-off-by: James Clark <james.clark(a)linaro.org>
>>> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com>
>>> ---
>>> drivers/hwtracing/coresight/coresight-core.c | 106 +++++++++++++-----
>>> drivers/hwtracing/coresight/coresight-dummy.c | 5 +-
>>> .../hwtracing/coresight/coresight-etm-perf.c | 30 +++--
>>> .../hwtracing/coresight/coresight-etm-perf.h | 2 +-
>>> drivers/hwtracing/coresight/coresight-etm.h | 1 -
>>> .../coresight/coresight-etm3x-core.c | 54 ++-------
>>> .../coresight/coresight-etm4x-core.c | 54 ++-------
>>> drivers/hwtracing/coresight/coresight-etm4x.h | 1 -
>>> drivers/hwtracing/coresight/coresight-priv.h | 12 +-
>>> drivers/hwtracing/coresight/coresight-stm.c | 3 +-
>>> drivers/hwtracing/coresight/coresight-sysfs.c | 17 ++-
>>> drivers/hwtracing/coresight/coresight-tpdm.c | 3 +-
>>> include/linux/coresight.h | 12 +-
>>> 13 files changed, 143 insertions(+), 157 deletions(-)
>>>
>> [...]
>>> @@ -352,7 +352,7 @@ static void *etm_setup_aux(struct perf_event
>>> *event, void **pages,
>>> * CPUs, we can handle it and fail the session.
>>> */
>>> for_each_cpu(cpu, mask) {
>>> - struct list_head *path;
>>> + struct coresight_path *path;
>>> struct coresight_device *csdev;
>>> csdev = per_cpu(csdev_src, cpu);
>>> @@ -405,15 +405,15 @@ static void *etm_setup_aux(struct perf_event
>>> *event, void **pages,
>>> cpumask_clear_cpu(cpu, mask);
>>> continue;
>>> }
>>> -
>>> /* ensure we can allocate a trace ID for this CPU */
>>> - trace_id = coresight_trace_id_get_cpu_id_map(cpu, &sink-
>>> >perf_sink_id_map);
>>> - if (!IS_VALID_CS_TRACE_ID(trace_id)) {
>>> + trace_id = coresight_path_assign_trace_id(path, CS_MODE_PERF);
>>> +
>>> + /* Can be 0 and valid, ETE doesn't need an ID */
>>> + if (trace_id < 0) {
>>
>> Not sure why I wrote it like this, but I think we should leave it as
>> it was with !IS_VALID_CS_TRACE_ID(). Even with ETE it calls the trace
>> ID allocator, so nothing has changed here.
>>
> Sure, Will restore. For ETE or ETM, we dont need traverse the path, just
> directly allocate the trace id based on cpu id.
>
> Jie
>
>
Sorry I meant to only keep the !IS_VALID_CS_TRACE_ID() bit. We still
need to call the new coresight_path_assign_trace_id() otherwise it
doesn't get assigned to the path. I saw that got removed in v11.
On 07/02/2025 6:42 am, Jie Gan wrote:
> The Coresight TMC Control Unit(CTCU) device hosts miscellaneous configuration
> registers to control various features related to TMC ETR device.
>
> The CTCU device works as a helper device physically connected to the TMC ETR device.
> ---------------------------------------------------------
> |ETR0| |ETR1|
> . \ / .
> . \ / .
> . \ / .
> . \ / .
> ---------------------------------------------------
> ETR0ATID0-ETR0ATID3 CTCU ETR1ATID0-ETR1ATID3
> ---------------------------------------------------
> Each ETR has four ATID registers with 128 bits long in total.
> e.g. ETR0ATID0-ETR0ATID3 registers are used by ETR0 device.
>
> Based on the trace id which is programed in CTCU ATID register of
> specific ETR, trace data with that trace id can get into ETR's buffer
> while other trace data gets ignored. The number of CTCU ATID registers
> depends on the number of defined TMC ETR devices. For example, two TMC
> ETR devices need eight ATID registers. ETR0 with ETR0ATID0-ETR0ATID3
> and ETR1 with ETR1ATID0-ETRATID3.
>
> The significant challenge in enabling the data filter function is how
> to collect the trace ID of the source device. The introduction of
> trace_id callback function addresses this challenge. The callback function
> collects trace ID of the device and return it back. The trace ID will be
> stored in the structure called coresight_path and transmitted to helper
> and sink devices.
>
> The coresight_path structure is created to address how to transmit
> parameters needs by coresight_enable_path/coresight_disbale_path
> functions.
>
> Here is the definition of the struct coresight_path:
> /**
> * struct coresight_path - data needed by enable/disable path
> * @path: path from source to sink.
> * @trace_id: trace_id of the whole path.
> */
> struct coresight_path {
> struct list_head *path;
> u8 trace_id;
> };
>
> The atid_offset mentioned before is the offset to ATID register in CTCU
> device.
>
> Enabling the source device will configure one bit in the ATID register based
> on its trace ID.
> Disabling the source devices will reset the bit in the AITD register
> based on its trace ID.
>
> Useage:
> Enable:
> STM device with trace ID 5 and ETR0 is activated.
> Bitmap before the enablement:
> ETR0ATID0:
> 31..................543210
> ==========================
> 0000000000000000000000...0
> ==========================
>
> Bitmap after the enablement:
> 31..................543210
> ==========================
> 0000000000000...0000100000
> ==========================
>
> The bit 5 of the ETR0ATID0 register is configured to 1 when enabling the
> STM device.
>
> Disable:
> STM device with trace ID 5 and ETR0 is activated.
> Bitmap before the disablement:
> ETR0ATID0:
> 31................6543210
> =========================
> 000000000010111...0100000
> =========================
>
> Bitmap after the disablement
> ETR0ATID0:
> 31................6543210
> =========================
> 000000000010111...0000000
> =========================
>
> The bit 5 of the ETR0ATID0 register is reset to 0 when disabling the STM
> device.
>
> Sincere thanks to James Clark for providing an excellent idea to handle
> the trace_id of the path.
>
> Changes in V2:
> 1. Rename the device to Coresight Control Unit.
> 2. Introduce the trace_id function pointer to address the challeng how to
> properly collect the trace ID of the device.
> 3. Introduce a new way to define the qcom,ccu-atid-offset property in
> device tree.
> 4. Disabling the filter function blocked on acquiring the ATID-offset,
> which will be addressed in a separate patch once it’s ready.
> Link to V1 - https://lore.kernel.org/lkml/20240618072726.3767974-1-quic_jiegan@quicinc.c…
>
> Changes in V3:
> 1. Rename the device to Coresight TMC Control Unit(CTCU).
> 2. Introduce a new way to define the platform related configs. The new
> structure, qcom_ctcu_config, is used to store configurations specific
> to a platform. Each platform should have its own qcom_ctcu_config structure.
> 3. In perf mode, the ETM devices allocate their trace IDs using the
> perf_sink_id_map. In sysfs mode, the ETM devices allocate their trace
> IDs using the id_map_default.
> 4. Considering the scenario where both ETR devices might be enabled simultaneously
> with multiple sources, retrieving and using trace IDs instead of id_map is more effective
> for the CTCU device in sysfs mode. For example, We can configure one ETR as sink for high
> throughput trace data like ETM and another ETR for low throughput trace data like STM.
> In this case, STM data won’t be flushed out by ETM data quickly. However, if we use id_map to
> manage the trace IDs, we need to create a separate id_map for each ETR device. Addtionally, We
> would need to iterate through the entire id_map for each configuration.
> 5. Add support for apb's clock name "apb". If the function fails to obtain the clock with
> the name "apb_pclk", it will attempt to acquire the clock with the name "apb".
> Link to V2 - https://lore.kernel.org/linux-arm-msm/20240705090049.1656986-1-quic_jiegan@…
>
> Changes in V4:
> 1. Add TMC description in binding file.
> 2. Restrict the number of ports for the CTCU device to a range of 0 to 1 in the binding file,
> because the maximum number of CTCU devices is 2 for existing projects.
> Link to V3 - https://lore.kernel.org/linux-arm-kernel/20240812024141.2867655-1-quic_jieg…
>
> Changes in V5:
> 1. Fix the format issue for description paragrah in dt binding file.
> 2. Previous discussion for why use "in-ports" property instead of "ports".
> Link to V4 - https://lore.kernel.org/linux-arm-msm/20240828012706.543605-1-quic_jiegan@q…
>
> Changes in V6:
> 1. Collected reviewed-by tag from Rob for dt-binding patch.
> 2. Rebased on tag next-20241008.
> 3. Dropped all depends-on tags.
> Link to V5 - https://lore.kernel.org/linux-arm-msm/20240909033458.3118238-1-quic_jiegan@…
>
> Changes in V7:
> 1. Rebased on tag next-20241204.
> 2. Fix format issue for dts patch.
> - Padding the address part to 8 digits
> Link to V6 - https://lore.kernel.org/linux-arm-msm/20241009112503.1851585-1-quic_jiegan@…
>
> Changes in V8:
> 1. Rebased on tag next-20241220.
> 2. Use raw_spinlock_t instead of spinlock_t.
> 3. Remove redundant codes in CTCU driver:
> - Eliminate unnecessary parameter validations.
> - Correct log level when an error occurs.
> - Optimize codes.
> 4. Correct the subject prefix for DT patch.
> 5. Collected reviewed-by tag from Konrad Dybcib for DT patch.
> Link to V7 - https://lore.kernel.org/all/20241210031545.3468561-1-quic_jiegan@quicinc.co…
>
> Changes in V9:
> 1. Rebased on tag next-20250113.
> 2. Separate the previous trace_id patch (patch 2/5 Coresight: Add trace_id function to
> retrieving the trace ID) into two patches.
> 3. Introduce a new struct coresight_path instead of cs_sink_data which was
> created in previous version. The coresight_path will be initialized
> and constructed in coresight_build_path function and released by
> coresight_release_path function.
> Detail of the struct coresight_path is shown below:
> /**
> * struct coresight_path - data needed by enable/disable path
> * @path: path from source to sink.
> * @trace_id: trace_id of the whole path.
> */
> struct coresight_path {
> struct list_head *path;
> u8 trace_id;
> };
>
> 4. Introduce an array of atomic in CTCU driver to represent the refcnt or each
> enabled trace_id for each sink. The reason is there is a scenario that more
> than one TPDM device physically connected to the same TPDA device has
> been enabled. The CTCU driver must verify the refcnt before resetting the
> bit of the atid register according to the trace_id of the TPDA device.
> 5. Remove redundant codes in CTCU driver.
> 6. Add reviewed-by tag to the commit message for APB clock path(patch
> 1/5).
> Link to V8 - https://lore.kernel.org/all/20241226011022.1477160-1-quic_jiegan@quicinc.co…
>
> Changes in V10:
> 1. Introduce a new API to allocate and read trace_id after path is built.
> 2. Introduce a new API to allocate and read trace_id of ETM device.
> 3. Add a new patch: [PATCH v10 3/7] Coresight: Use coresight_etm_get_trace_id() in traceid_show()
> 4. Remove perf handle from coresight_path.
> 5. Use u8 instead of atomic_t for traceid_refcnt.
> 6. Optimize the part of code in CTCU drvier that is responsible for program atid register.
> Link to V9 - https://lore.kernel.org/all/20250124072537.1801030-1-quic_jiegan@quicinc.co…
>
> Jie Gan (7):
> Coresight: Add support for new APB clock name
> Coresight: Add trace_id function to retrieving the trace ID
> Coresight: Use coresight_etm_get_trace_id() in traceid_show()
> Coresight: Introduce a new struct coresight_path
> dt-bindings: arm: Add Coresight TMC Control Unit hardware
> Coresight: Add Coresight TMC Control Unit driver
> arm64: dts: qcom: sa8775p: Add CTCU and ETR nodes
>
> .../bindings/arm/qcom,coresight-ctcu.yaml | 84 ++++++
> arch/arm64/boot/dts/qcom/sa8775p.dtsi | 153 ++++++++++
> drivers/hwtracing/coresight/Kconfig | 12 +
> drivers/hwtracing/coresight/Makefile | 1 +
> drivers/hwtracing/coresight/coresight-core.c | 133 +++++++--
> drivers/hwtracing/coresight/coresight-ctcu.c | 268 ++++++++++++++++++
> drivers/hwtracing/coresight/coresight-ctcu.h | 24 ++
> drivers/hwtracing/coresight/coresight-dummy.c | 16 +-
> .../hwtracing/coresight/coresight-etm-perf.c | 30 +-
> .../hwtracing/coresight/coresight-etm-perf.h | 2 +-
> drivers/hwtracing/coresight/coresight-etm.h | 1 -
> .../coresight/coresight-etm3x-core.c | 55 +---
> .../coresight/coresight-etm3x-sysfs.c | 3 +-
> .../coresight/coresight-etm4x-core.c | 55 +---
> .../coresight/coresight-etm4x-sysfs.c | 4 +-
> drivers/hwtracing/coresight/coresight-etm4x.h | 1 -
> drivers/hwtracing/coresight/coresight-priv.h | 12 +-
> drivers/hwtracing/coresight/coresight-stm.c | 14 +-
> drivers/hwtracing/coresight/coresight-sysfs.c | 17 +-
> drivers/hwtracing/coresight/coresight-tpda.c | 11 +
> drivers/hwtracing/coresight/coresight-tpdm.c | 3 +-
> include/linux/coresight.h | 30 +-
> 22 files changed, 765 insertions(+), 164 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml
> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.c
> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.h
>
Just one small comment, and the kernel test bot report to fix. Otherwise
looks good to me.
Reviewed-by: James Clark <james.clark(a)linaro.org>
On 07/02/2025 6:42 am, Jie Gan wrote:
> Add 'struct coresight_path' to store the data that is needed by
> coresight_enable_path/coresight_disable_path. The structure will be
> transmitted to any required devices to enable related funcationalities.
>
> The trace_id will be allocated after the path is built. Consequently,
> The ETM3x and ETM4x devices will directly read the trace_id from path
> which result in etm_read_alloc_trace_id and etm4_read_alloc_trace_id
> being deleted.
>
> Co-developed-by: James Clark <james.clark(a)linaro.org>
> Signed-off-by: James Clark <james.clark(a)linaro.org>
> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 106 +++++++++++++-----
> drivers/hwtracing/coresight/coresight-dummy.c | 5 +-
> .../hwtracing/coresight/coresight-etm-perf.c | 30 +++--
> .../hwtracing/coresight/coresight-etm-perf.h | 2 +-
> drivers/hwtracing/coresight/coresight-etm.h | 1 -
> .../coresight/coresight-etm3x-core.c | 54 ++-------
> .../coresight/coresight-etm4x-core.c | 54 ++-------
> drivers/hwtracing/coresight/coresight-etm4x.h | 1 -
> drivers/hwtracing/coresight/coresight-priv.h | 12 +-
> drivers/hwtracing/coresight/coresight-stm.c | 3 +-
> drivers/hwtracing/coresight/coresight-sysfs.c | 17 ++-
> drivers/hwtracing/coresight/coresight-tpdm.c | 3 +-
> include/linux/coresight.h | 12 +-
> 13 files changed, 143 insertions(+), 157 deletions(-)
>
[...]
> @@ -352,7 +352,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
> * CPUs, we can handle it and fail the session.
> */
> for_each_cpu(cpu, mask) {
> - struct list_head *path;
> + struct coresight_path *path;
> struct coresight_device *csdev;
>
> csdev = per_cpu(csdev_src, cpu);
> @@ -405,15 +405,15 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
> cpumask_clear_cpu(cpu, mask);
> continue;
> }
> -
> /* ensure we can allocate a trace ID for this CPU */
> - trace_id = coresight_trace_id_get_cpu_id_map(cpu, &sink->perf_sink_id_map);
> - if (!IS_VALID_CS_TRACE_ID(trace_id)) {
> + trace_id = coresight_path_assign_trace_id(path, CS_MODE_PERF);
> +
> + /* Can be 0 and valid, ETE doesn't need an ID */
> + if (trace_id < 0) {
Not sure why I wrote it like this, but I think we should leave it as it
was with !IS_VALID_CS_TRACE_ID(). Even with ETE it calls the trace ID
allocator, so nothing has changed here.
Hi,
We are not seeing these problems on other systems with multiple ETMs.
How are you using the system?
Perf will strictly disable sources before sinks - which would mean
that the ETMs will flush before the TMC requests a manual flush on
stop.
For sysfs use, we recommend the same, but even halting the TMC first
should result in all sources being requested to flush at the same time
(subject to propagation through connected trace funnels).
On Fri, 7 Feb 2025 at 07:25, Yuanfang Zhang <quic_yuanfang(a)quicinc.com> wrote:
>
>
>
> On 2/6/2025 6:32 PM, Leo Yan wrote:
> > Hi Yuanfang,
> >
> > On Fri, Jan 03, 2025 at 04:01:54PM +0800, Yuanfang Zhang wrote:
> >>
> >> When multiple ETMs are enabled simultaneously, the time required
> >> to complete a flush in the process of reading the TMC device node
> >> may exceed the default wait time of 100us.
> >
How do you know that it is the ETM flush process that is the issue?
TMC ready can be delayed by issues in the memory system that prevent
write to the memory buffer.
The TMC flush complete will indicate completion of flush for all
connected devices - so if flush complete is indicated by the TMC then
a flush request cannot be the issue.
With multiple ETMs, then the connected trace funnels arbitration will
ensure flush is indicated complete on all connected inputs before
transmitting to the output.
The driver code to stop the TMC will first wait on flush complete
before waiting on TMC ready.
> > Have you tried how long time would be safe to wait for the
> > TMCREADY_BIT bit to be set on your platform?
> yes, 400000us.
This is a huge amount of latency for a coresight system. This
suggests some sort of contention issue.
> >
> >> If the TMC capture is stopped while any ETM has not completed its
> >> flush, it can cause the corresponding CPU to hang.
> >>
There is nothing in the ETMv4 hardware specification /design that
should cause an CPU to hang. There is an implementation dependent
stall mechanism that if implemented can stall a PE when the ETM fifo
approaches overflow, but this is switch off on the drivers, if
implemented on the hardware.
The flush process for the ETM is a purely internal process of clearing
out all trace currently in the ETM, and is not dependent on the PE.
> >> Fix the by checking the TMCReady bit after the flush. If TMCReady
> >> bit is set, TraceCaptEn bit can be clear; otherwise, return directly
> >> and stop the TMC read.
> >
> > When a timeout for TMC flush and stop is detected, if the driver
> > arbitrarily bails out without disabling TMC, this would leave the
> > hardware always on.
> >
I agree that this is not an acceptable solution - we cannot leave the
TMC in an indeterminate state.
Where clients such as perf are using the hardware there is no
mechanism for re-trying if halt fails.
The functions you have changed seem to relate to reading the buffer
via sysfs. Do you see similar problems using perf to collect trace
data?
> > I would like first know if a longer timeout can fix the issue.
> >
> > Thanks,
> > Leo
> >
> yes, longer timeout can fix this issue.
>
I think that a longer timeout will mask some underlying contention
issue, but it should be possible to make the coresight driver timeout
configurable, and set the value appropriately for your system.
There is a new TMC logging patchset here:
https://lists.linaro.org/archives/list/coresight@lists.linaro.org/thread/7Y…
which improves the error reporting around timeouts. This might help
establish if the error is in waiting for flush complete, or TMC ready.
This API could be extended to add a timeout value parameter, that can
be configured from the TMC driver, set via sysfs for the TMC in the
initial case to further debugging of this issue on your system
Regards
Mike
> >> Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
> >> ---
> >> drivers/hwtracing/coresight/coresight-tmc-etf.c | 17 +++++++++++++++--
> >> drivers/hwtracing/coresight/coresight-tmc-etr.c | 22 +++++++++++++++++-----
> >> 2 files changed, 32 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> >> index d4f641cd9de69488fe3d1c1dc9b5a9eafb55ed59..bded290c42891d782344d9a6e63ebdbed6719133 100644
> >> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> >> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> >> @@ -80,11 +80,21 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
> >> return;
> >> }
> >>
> >> -static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> >> +static int __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> >> {
> >> + int rc;
> >> +
> >> CS_UNLOCK(drvdata->base);
> >>
> >> tmc_flush_and_stop(drvdata);
> >> +
> >> + rc = tmc_wait_for_tmcready(drvdata);
> >> + if (rc) {
> >> + dev_err(&drvdata->csdev->dev,
> >> + "Failed to disable : TMC is not ready\n");
> >> + CS_LOCK(drvdata->base);
> >> + return rc;
> >> + }
> >> /*
> >> * When operating in sysFS mode the content of the buffer needs to be
> >> * read before the TMC is disabled.
> >> @@ -94,6 +104,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> >> tmc_disable_hw(drvdata);
> >>
> >> CS_LOCK(drvdata->base);
> >> + return 0;
> >> }
> >>
> >> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> >> @@ -650,7 +661,9 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> >> ret = -EINVAL;
> >> goto out;
> >> }
> >> - __tmc_etb_disable_hw(drvdata);
> >> + ret = __tmc_etb_disable_hw(drvdata);
> >> + if (ret)
> >> + goto out;
> >> }
> >>
> >> drvdata->reading = true;
> >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> >> index a48bb85d0e7f44a25b813f3c828cc3d705d16012..63a1f7501562fa0b5c2fe6ea53dce4d82842bec3 100644
> >> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> >> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> >> @@ -1135,11 +1135,21 @@ static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata)
> >> }
> >> }
> >>
> >> -static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> >> +static int __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> >> {
> >> + int rc;
> >> +
> >> CS_UNLOCK(drvdata->base);
> >>
> >> tmc_flush_and_stop(drvdata);
> >> +
> >> + rc = tmc_wait_for_tmcready(drvdata);
> >> + if (rc) {
> >> + dev_err(&drvdata->csdev->dev,
> >> + "Failed to disable : TMC is not ready\n");
> >> + CS_LOCK(drvdata->base);
> >> + return rc;
> >> + }
> >> /*
> >> * When operating in sysFS mode the content of the buffer needs to be
> >> * read before the TMC is disabled.
> >> @@ -1150,7 +1160,7 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> >> tmc_disable_hw(drvdata);
> >>
> >> CS_LOCK(drvdata->base);
> >> -
> >> + return 0;
> >> }
> >>
> >> void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> >> @@ -1779,9 +1789,11 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> >> }
> >>
> >> /* Disable the TMC if we are trying to read from a running session. */
> >> - if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS)
> >> - __tmc_etr_disable_hw(drvdata);
> >> -
> >> + if (coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS) {
> >> + ret = __tmc_etr_disable_hw(drvdata);
> >> + if (ret)
> >> + goto out;
> >> + }
> >> drvdata->reading = true;
> >> out:
> >> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> >>
> >> ---
> >> base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
> >> change-id: 20250103-fix_cpu_hung-b5a95179ada4
> >>
> >> Best regards,
> >> --
> >> Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
> >>
> >>
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
