CoreSight

coresight@lists.linaro.org

6 participants
2338 discussions

Re: [PATCH 1/5] dt-bindings: arm: Add Coresight device Trace NOC definition

by Yuanfang Zhang

On 2/22/2025 6:47 PM, Krzysztof Kozlowski wrote: > On 21/02/2025 08:40, Yuanfang Zhang wrote: >> Adds new coresight-tnoc.yaml file describing the bindings required >> to define Trace NOC in the device trees. >> >> Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> > > > So you just sent the same v1, ignoring previous review. That's not how > it works. > sorry for this incorrect process. because i just update --to-cc list and no other change, i forced the version to V1, hoped it would work like resend, but the result was not as expected. > Provide proper changelog, implement ENTIRE feedback and do no ask > maintainers do point the same issues TWICE. > > NAK > > <form letter> > It seems my or other reviewer's previous comments were not fully > addressed. Maybe the feedback got lost between the quotes, maybe you > just forgot to apply it. Please go back to the previous discussion and > either implement all requested changes or keep discussing them. > > Thank you. > </form letter> > > Best regards, > Krzysztof >

2 months, 1 week

Re: [PATCH 2/5] coresight: add coresight Trace NOC driver

by Yuanfang Zhang

On 2/22/2025 6:54 PM, Krzysztof Kozlowski wrote: > On 21/02/2025 08:40, Yuanfang Zhang wrote: >> Add driver to support Coresight device Trace NOC(Network On Chip). >> Trace NOC is an integration hierarchy which is a replacement of >> Dragonlink configuration. It brings together debug components like >> TPDA, funnel and interconnect Trace Noc. >> >> It sits in the different subsystem of SOC and aggregates the trace >> and transports to QDSS trace bus. >> >> Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> >> --- >> drivers/hwtracing/coresight/Kconfig | 10 ++ >> drivers/hwtracing/coresight/Makefile | 1 + >> drivers/hwtracing/coresight/coresight-tnoc.c | 191 +++++++++++++++++++++++++++ >> drivers/hwtracing/coresight/coresight-tnoc.h | 53 ++++++++ >> 4 files changed, 255 insertions(+) >> >> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig >> index 06f0a7594169c5f03ca5f893b7debd294587de78..712b2469e37610e6fc5f15cedb2535bf570f99aa 100644 >> --- a/drivers/hwtracing/coresight/Kconfig >> +++ b/drivers/hwtracing/coresight/Kconfig >> @@ -247,4 +247,14 @@ config CORESIGHT_DUMMY >> >> To compile this driver as a module, choose M here: the module will be >> called coresight-dummy. >> + >> +config CORESIGHT_TNOC >> + tristate "Coresight Trace Noc driver" >> + help >> + This driver provides support for Trace NoC component. >> + Trace NoC is a interconnect that is used to collect trace from >> + various subsystems and transport it QDSS trace sink.It sits in >> + the different tiles of SOC and aggregates the trace local to the >> + tile and transports it another tile or to QDSS trace sink eventually. >> + >> endif >> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile >> index 4ba478211b318ea5305f9f98dda40a041759f09f..ab1cff8f027495fabe3872d52f8c0877e39f0ea8 100644 >> --- a/drivers/hwtracing/coresight/Makefile >> +++ b/drivers/hwtracing/coresight/Makefile >> @@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ >> coresight-cti-sysfs.o >> obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o >> obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o >> +obj-$(CONFIG_CORESIGHT_TNOC) += coresight-tnoc.o > > Why do you keep adding entries to the end instead to some logically > ordered place? > > Dummy driver, before tpda (obviously tpda should go after tpdm) and now > this... This is just unnecessarily making simultaneous edits difficult. > sure, add it after funnel/replicator before etm, since it work as a link. >> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c >> new file mode 100644 >> index 0000000000000000000000000000000000000000..11b9a7fd1efdc9fff7c1e9666bda14acb41786cb >> --- /dev/null >> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c >> @@ -0,0 +1,191 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/device.h> >> +#include <linux/platform_device.h> >> +#include <linux/amba/bus.h> >> +#include <linux/io.h> >> +#include <linux/coresight.h> >> +#include <linux/of.h> >> + >> +#include "coresight-priv.h" >> +#include "coresight-tnoc.h" >> +#include "coresight-trace-id.h" >> + > > >> + >> + drvdata->base = devm_ioremap_resource(dev, &adev->res); >> + if (!drvdata->base) >> + return -ENOMEM; >> + >> + spin_lock_init(&drvdata->spinlock); >> + >> + ret = trace_noc_init_default_data(drvdata); >> + if (ret) >> + return ret; >> + >> + desc.ops = &trace_noc_cs_ops; >> + desc.type = CORESIGHT_DEV_TYPE_LINK; >> + desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG; >> + desc.pdata = adev->dev.platform_data; >> + desc.dev = &adev->dev; >> + desc.access = CSDEV_ACCESS_IOMEM(drvdata->base); >> + drvdata->csdev = coresight_register(&desc); >> + if (IS_ERR(drvdata->csdev)) >> + return PTR_ERR(drvdata->csdev); >> + >> + pm_runtime_put(&adev->dev); >> + >> + dev_dbg(drvdata->dev, "Trace Noc initialized\n"); > > > Drop. There is really no need to tell that function finished. > > Please run standard kernel tools for static analysis, like coccinelle, > smatch and sparse, and fix reported warnings. Also please check for > warnings when building with W=1. Most of these commands (checks or W=1 > build) can build specific targets, like some directory, to narrow the > scope to only your code. The code here looks like it needs a fix. Feel > free to get in touch if the warning is not clear. > Done. > > Best regards, > Krzysztof >

2 months, 1 week

[PATCH v4] coresight-etm4x: add isb() before reading the TRCSTATR

by Yuanfang Zhang

As recommended by section 4.3.7 ("Synchronization when using system instructions to progrom the trace unit") of ARM IHI 0064H.b, the self-hosted trace analyzer must perform a Context synchronization event between writing to the TRCPRGCTLR and reading the TRCSTATR. Additionally, add an ISB between the each read of TRCSTATR on coresight_timeout() when using system instructions to program the trace unit. Fixes: 1ab3bb9df5e3 ("coresight: etm4x: Add necessary synchronization for sysreg access") Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> --- drivers/hwtracing/coresight/coresight-core.c | 20 ++++++--- drivers/hwtracing/coresight/coresight-etm4x-core.c | 48 +++++++++++++++++++--- include/linux/coresight.h | 4 ++ 3 files changed, 62 insertions(+), 10 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ea38ecf26fcbfb383eb40a222788bf5d7976013d..c42aa9fddab9b752361b84deab6b10be045b7104 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1017,18 +1017,20 @@ static void coresight_remove_conns(struct coresight_device *csdev) } /** - * coresight_timeout - loop until a bit has changed to a specific register - * state. + * coresight_timeout_action - loop until a bit has changed to a specific register + * state, with a callback after every trial. * @csa: coresight device access for the device * @offset: Offset of the register from the base of the device. * @position: the position of the bit of interest. * @value: the value the bit should have. + * @cb: Call back after each trial. * * Return: 0 as soon as the bit has taken the desired state or -EAGAIN if * TIMEOUT_US has elapsed, which ever happens first. */ -int coresight_timeout(struct csdev_access *csa, u32 offset, - int position, int value) +int coresight_timeout_action(struct csdev_access *csa, u32 offset, + int position, int value, + coresight_timeout_cb_t cb) { int i; u32 val; @@ -1044,7 +1046,8 @@ int coresight_timeout(struct csdev_access *csa, u32 offset, if (!(val & BIT(position))) return 0; } - + if (cb) + cb(csa, offset, position, value); /* * Delay is arbitrary - the specification doesn't say how long * we are expected to wait. Extra check required to make sure @@ -1056,6 +1059,13 @@ int coresight_timeout(struct csdev_access *csa, u32 offset, return -EAGAIN; } +EXPORT_SYMBOL_GPL(coresight_timeout_action); + +int coresight_timeout(struct csdev_access *csa, u32 offset, + int position, int value) +{ + return coresight_timeout_action(csa, offset, position, value, NULL); +} EXPORT_SYMBOL_GPL(coresight_timeout); u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index dd8c74f893dbdfe1a35d811ef2dc476b81ce07ae..b933cdc4e52fd5ecd16d3ad97cf0d3f1a3fd87e4 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -399,6 +399,29 @@ static void etm4_check_arch_features(struct etmv4_drvdata *drvdata, } #endif /* CONFIG_ETM4X_IMPDEF_FEATURE */ +static void etm4x_sys_ins_barrier(struct csdev_access *csa, u32 offset, int pos, int val) +{ + if (!csa->io_mem) + isb(); +} + +/* + * etm4x_wait_status: Poll for TRCSTATR.<pos> == <val>. While using system + * instruction to access the trace unit, each access must be separated by a + * synchronization barrier. See ARM IHI0064H.b section "4.3.7 Synchronization of + * register updates", for system instructions section, in "Notes": + * + * "In particular, whenever disabling or enabling the trace unit, a poll of + * TRCSTATR needs explicit synchronization between each read of TRCSTATR" + */ +static int etm4x_wait_status(struct csdev_access *csa, int pos, int val) +{ + if (!csa->io_mem) + return coresight_timeout_action(csa, TRCSTATR, pos, val, + etm4x_sys_ins_barrier); + return coresight_timeout(csa, TRCSTATR, pos, val); +} + static int etm4_enable_hw(struct etmv4_drvdata *drvdata) { int i, rc; @@ -430,7 +453,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) isb(); /* wait for TRCSTATR.IDLE to go up */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) + if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); if (drvdata->nr_pe) @@ -523,7 +546,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) isb(); /* wait for TRCSTATR.IDLE to go back down to '0' */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 0)) + if (etm4x_wait_status(csa, TRCSTATR_IDLE_BIT, 0)) dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); @@ -906,10 +929,25 @@ static void etm4_disable_hw(void *info) tsb_csync(); etm4x_relaxed_write32(csa, control, TRCPRGCTLR); + /* + * As recommended by section 4.3.7 ("Synchronization when using system + * instructions to progrom the trace unit") of ARM IHI 0064H.b, the + * self-hosted trace analyzer must perform a Context synchronization + * event between writing to the TRCPRGCTLR and reading the TRCSTATR. + */ + if (!csa->io_mem) + isb(); + /* wait for TRCSTATR.PMSTABLE to go to '1' */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) + if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for PM stable Trace Status\n"); + /* + * As recommended by section 4.3.7 (Synchronization of register updates) + * of ARM IHI 0064H.b. + */ + isb(); + /* read the status of the single shot comparators */ for (i = 0; i < drvdata->nr_ss_cmp; i++) { config->ss_status[i] = @@ -1711,7 +1749,7 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) etm4_os_lock(drvdata); /* wait for TRCSTATR.PMSTABLE to go up */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) { + if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) { dev_err(etm_dev, "timeout while waiting for PM Stable Status\n"); etm4_os_unlock(drvdata); @@ -1802,7 +1840,7 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) state->trcpdcr = etm4x_read32(csa, TRCPDCR); /* wait for TRCSTATR.IDLE to go up */ - if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) { + if (etm4x_wait_status(csa, TRCSTATR_PMSTABLE_BIT, 1)) { dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); etm4_os_unlock(drvdata); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index c1334259427850173ccc5872a6ced550a99f8e1c..f106b1025111892e9d44ac643f7409552952d4da 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -639,6 +639,10 @@ extern int coresight_enable_sysfs(struct coresight_device *csdev); extern void coresight_disable_sysfs(struct coresight_device *csdev); extern int coresight_timeout(struct csdev_access *csa, u32 offset, int position, int value); +typedef void (*coresight_timeout_cb_t) (struct csdev_access *, u32, int, int); +extern int coresight_timeout_action(struct csdev_access *csa, u32 offset, + int position, int value, + coresight_timeout_cb_t cb); extern int coresight_claim_device(struct coresight_device *csdev); extern int coresight_claim_device_unlocked(struct coresight_device *csdev); --- base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4 change-id: 20250116-etm_sync-7be51cc6cbcd Best regards, -- Yuanfang Zhang <quic_yuanfang(a)quicinc.com>

2 months, 1 week

Re: [PATCH v14 00/10] Coresight: Add Coresight TMC Control Unit driver

by James Clark

On 26/02/2025 7:05 am, Jie Gan wrote: > > > On 2/26/2025 12:13 PM, Jie Gan wrote: >> The Coresight TMC Control Unit(CTCU) device hosts miscellaneous >> configuration >> registers to control various features related to TMC ETR device. >> > > [...] > Hi, James > > Sorry for the mistake, I just found I forget to add the co-developed-by > tag to patch(5/10), patch(6/10) after the division. > > Do I need resend the patch series? > > Jie > No, no worries. If you end up sending another version you can fix it otherwise it's not worth it. >> >> Sincere thanks to James Clark for providing an excellent idea to handle >> the trace_id of the path. >> >> --- >> Changes in V14: >> 1. Drop the reviewed-by tag for previous patch: Coresight-Introduce-a- >> new-struct-coresight_path >> due to a massive modification. >> 2. Split the patch, Coresight-Introduce-a-new-struct-coresight_path, into >> four patches. >> - Coresight-Introduce-a-new-struct-coresight_path >> - Coresight-Allocate-trace-ID-after-building-the-path >> - Coresight-Change-to-read-the-trace-ID-from-coresight_path >> - Coresight-Change-functions-to-accept-the-coresight_path >> 3. Change the type of the coresight_path_assign_trace_id function to >> void. >> 4. Change the type of the path_list from struct list_head * to struct >> list_head to avoid >> extra memory allocate/free. >> 5. Rename the file coresight-ctcu.c to coresight-ctcu-core.c to >> improve scalibility. >> 6. Add pm_ops for CTCU driver. >> 7. Rename the struct ctcu_atid_config to ctcu_etr_config to improve >> scalibility. >> 8. Optimize following functions of the CTCU driver to improve >> readability. >> - ctcu_program_atid_register >> - __ctcu_set_etr_traceid >> 9. Change the way to get the port number. The new solution is searching >> the sink device from CTCU's view. >> 10. Add desc.access for CTCU driver. >> Link to V13 - https://lore.kernel.org/linux-arm- >> msm/20250221060543.2898845-1-quic_jiegan(a)quicinc.com/ >> --- >> >> --- >> Changes in V13: >> 1. Move the trace_id callback to coresight_ops to simplify the code. >> Link to V12 - https://lore.kernel.org/linux-arm- >> msm/20250217093024.1133096-1-quic_jiegan(a)quicinc.com/ >> --- >> >> --- >> 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(a)quicinc.com/ >> --- >> >> --- >> Changes in V11: >> 1. Add reviewed-by tag to patch(2/7), (4/7), (6/7). Patch(3/7) is >> contributed by James, so didnot add reviewed-by tag of James. >> 2. Fix warning reported by kernel bot and verified with build(W=1). >> 3. Restore to the original logic that responsible for allocate trace_id >> of ETM device in perf mode according to James' comment. >> Link to V10 - https://lore.kernel.org/linux-arm- >> msm/20250207064213.2314482-1-quic_jiegan(a)quicinc.com/ >> --- >> >> --- >> 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(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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_jiegan(a)quicinc.com/ >> >> 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(a)quicinc.com/T/#t >> >> 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(a)quicinc.com/T/#t >> --- >> >> James Clark (1): >> Coresight: Use coresight_etm_get_trace_id() in traceid_show() >> >> Jie Gan (9): >> Coresight: Add support for new APB clock name >> Coresight: Add trace_id function to retrieving the trace ID >> Coresight: Introduce a new struct coresight_path >> Coresight: Move trace_id to coresight_path and allocate it after >> building the path >> Coresight: Change to read the trace ID from coresight_path >> Coresight: Change functions to accept the 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 | 2 + >> drivers/hwtracing/coresight/coresight-core.c | 122 +++++-- >> .../hwtracing/coresight/coresight-ctcu-core.c | 326 ++++++++++++++++++ >> drivers/hwtracing/coresight/coresight-ctcu.h | 39 +++ >> drivers/hwtracing/coresight/coresight-dummy.c | 15 +- >> .../hwtracing/coresight/coresight-etm-perf.c | 27 +- >> .../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 | 14 +- >> drivers/hwtracing/coresight/coresight-stm.c | 13 +- >> drivers/hwtracing/coresight/coresight-sysfs.c | 17 +- >> drivers/hwtracing/coresight/coresight-tpda.c | 11 + >> drivers/hwtracing/coresight/coresight-tpdm.c | 2 +- >> include/linux/coresight.h | 27 +- >> 22 files changed, 824 insertions(+), 161 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/arm/ >> qcom,coresight-ctcu.yaml >> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-core.c >> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.h >> >

2 months, 1 week

Re: [PATCH RESEND] coresight: etm4x: don't include '<linux/pm_wakeup.h>' directly

by Suzuki K Poulose

On Mon, 10 Feb 2025 12:36:36 +0100, Wolfram Sang wrote: > The header clearly states that it does not want to be included directly, > only via '<linux/(platform_)?device.h>'. Which is already present, so > delete the superfluous include. > > Applied, thanks! [1/1] coresight: etm4x: don't include '<linux/pm_wakeup.h>' directly commit: b2d67616fcf9f0a21940b2ba8d80af2fb0c81bc1 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

2 months, 1 week

Re: [PATCH v4 0/9] coresight: change some driver' spinlock type to raw_spinlock_t

by Suzuki K Poulose

Hi Levi, On 20/12/2024 10:45, Yeoreum Yun wrote: > In some coresight drivers, drvdata->spinlock can be held during __schedule() > by perf_event_task_sched_out()/in(). > > Since drvdata->spinlock type is spinlock_t and > perf_event_task_sched_out()/in() is called after acquiring rq_lock, > which is raw_spinlock_t (an unsleepable lock), > this poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable. > > To address this,change type drvdata->spinlock in some coresight drivers, > which can be called by perf_event_task_sched_out()/in(), > from spinlock_t to raw_spinlock_t. > > Reviewed-by: James Clark <james.clark(a)linaro.org> This doesn't apply cleanly on coresight next branch. Please could you rebase on coresight /next ? Cheers Suzuki > > Yeoreum Yun (9): > coresight: change coresight_device lock type to raw_spinlock_t > coresight-etm4x: change etmv4_drvdata spinlock type to raw_spinlock_t > coresight: change coresight_trace_id_map's lock type to > raw_spinlock_t > coresight-cti: change cti_drvdata spinlock's type to raw_spinlock_t > coresight-etb10: change etb_drvdata spinlock's type to raw_spinlock_t > coresight-funnel: change funnel_drvdata spinlock's type to > raw_spinlock_t > coresight-replicator: change replicator_drvdata spinlock's type to > raw_spinlock_t > coresight-tmc: change tmc_drvdata spinlock's type to raw_spinlock_t > coresight/ultrasoc: change smb_drv_data spinlock's type to > raw_spinlock_t > > .../hwtracing/coresight/coresight-config.c | 8 +- > .../hwtracing/coresight/coresight-config.h | 2 +- > drivers/hwtracing/coresight/coresight-core.c | 2 +- > .../hwtracing/coresight/coresight-cti-core.c | 44 +-- > .../hwtracing/coresight/coresight-cti-sysfs.c | 76 +++--- > drivers/hwtracing/coresight/coresight-cti.h | 2 +- > drivers/hwtracing/coresight/coresight-etb10.c | 26 +- > .../coresight/coresight-etm4x-core.c | 18 +- > .../coresight/coresight-etm4x-sysfs.c | 250 +++++++++--------- > drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- > .../hwtracing/coresight/coresight-funnel.c | 12 +- > .../coresight/coresight-replicator.c | 12 +- > .../hwtracing/coresight/coresight-syscfg.c | 26 +- > .../hwtracing/coresight/coresight-tmc-core.c | 6 +- > .../hwtracing/coresight/coresight-tmc-etf.c | 48 ++-- > .../hwtracing/coresight/coresight-tmc-etr.c | 40 +-- > drivers/hwtracing/coresight/coresight-tmc.h | 2 +- > .../hwtracing/coresight/coresight-trace-id.c | 22 +- > drivers/hwtracing/coresight/ultrasoc-smb.c | 12 +- > drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- > include/linux/coresight.h | 4 +- > 21 files changed, 308 insertions(+), 308 deletions(-) > > -- > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

2 months, 1 week

[PATCH v1 0/8] Arm CoreSight: Support AUX pause and resume

by Leo Yan

This series is to enable AUX pause and resume on Arm CoreSight. The first patch extracts the trace unit controlling operations to two functions. These two functions will be used by AUX pause and resume. Patches 02 and 03 change the ETMv4 driver to prepare callback functions for AUX pause and resume. Patch 04 changes the ETM perf layer to support AUX pause and resume in a perf session. The patches 05 and 06 offers an extra feature for updating buffer on AUX pause occasion, which can mitigate the trace data lose issue. Patch 07 documents the AUX pause usages with Arm CoreSight. The last patch syncs headers between user space and the kernel. This patch set has been verified on the Hikey960 board and TC platform. The previous one uses ETR and the later uses TRBE as sink. It is suggested to disable CPUIdle (add `nohlt` option in Linux command line) when verifying this series. ETM and funnel drivers are found issues during CPU suspend and resume which will be addressed separately. Leo Yan (8): coresight: etm4x: Extract the trace unit controlling coresight: Introduce pause and resume APIs for source coresight: etm4x: Hook pause and resume callbacks coresight: perf: Support AUX trace pause and resume coresight: etm: Add an attribute for updating buffer coresight: perf: Update buffer on AUX pause Documentation: coresight: Document AUX pause and resume perf cs-etm: Sync kernel coresight-pmu.h header .../trace/coresight/coresight-perf.rst | 50 ++++++ drivers/hwtracing/coresight/coresight-core.c | 12 ++ .../hwtracing/coresight/coresight-etm-perf.c | 94 +++++++++- .../hwtracing/coresight/coresight-etm-perf.h | 2 + .../coresight/coresight-etm4x-core.c | 166 ++++++++++++------ drivers/hwtracing/coresight/coresight-etm4x.h | 2 + drivers/hwtracing/coresight/coresight-priv.h | 2 + include/linux/coresight-pmu.h | 1 + include/linux/coresight.h | 4 + tools/include/linux/coresight-pmu.h | 1 + 10 files changed, 281 insertions(+), 53 deletions(-) -- 2.34.1

2 months, 1 week

Re: [PATCH v12 6/7] Coresight: Add Coresight TMC Control Unit driver

by Suzuki K Poulose

On 24/02/2025 03:32, Jie Gan wrote: > > > On 2/21/2025 7:39 PM, Suzuki K Poulose wrote: >> On 17/02/2025 09:30, Jie Gan wrote: >>> The Coresight TMC Control Unit hosts miscellaneous configuration >>> registers >>> which control various features related to TMC ETR sink. >>> >>> Based on the trace ID, which is programmed in the related CTCU ATID >>> register of a specific ETR, trace data with that trace ID gets into >>> the ETR buffer, while other trace data gets dropped. >>> >>> Enabling source device sets one bit of the ATID register based on >>> source device's trace ID. >>> Disabling source device resets the bit according to the source >>> device's trace ID. >>> >>> Reviewed-by: James Clark <james.clark(a)linaro.org> >>> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> >>> --- >>> drivers/hwtracing/coresight/Kconfig | 12 + >>> drivers/hwtracing/coresight/Makefile | 1 + >>> drivers/hwtracing/coresight/coresight-ctcu.c | 268 +++++++++++++++++++ >>> drivers/hwtracing/coresight/coresight-ctcu.h | 24 ++ >>> include/linux/coresight.h | 3 +- >>> 5 files changed, 307 insertions(+), 1 deletion(-) >>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.c >>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.h >>> >>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/ >>> coresight/Kconfig >>> index 06f0a7594169..ecd7086a5b83 100644 >>> --- a/drivers/hwtracing/coresight/Kconfig >>> +++ b/drivers/hwtracing/coresight/Kconfig >>> @@ -133,6 +133,18 @@ config CORESIGHT_STM >>> To compile this driver as a module, choose M here: the >>> module will be called coresight-stm. >>> +config CORESIGHT_CTCU >>> + tristate "CoreSight TMC Control Unit driver" >>> + depends on CORESIGHT_LINK_AND_SINK_TMC >>> + help >>> + This driver provides support for CoreSight TMC Control Unit >>> + that hosts miscellaneous configuration registers. This is >>> + primarily used for controlling the behaviors of the TMC >>> + ETR device. >>> + >>> + To compile this driver as a module, choose M here: the >>> + module will be called coresight-ctcu. >>> + >>> config CORESIGHT_CPU_DEBUG >>> tristate "CoreSight CPU Debug driver" >>> depends on ARM || ARM64 >>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/ >>> hwtracing/ coresight/Makefile >>> index 4ba478211b31..1b7869910a12 100644 >>> --- a/drivers/hwtracing/coresight/Makefile >>> +++ b/drivers/hwtracing/coresight/Makefile >>> @@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight- >>> cti-platform.o \ >>> coresight-cti-sysfs.o >>> obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o >>> obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o >>> +obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o >>> diff --git a/drivers/hwtracing/coresight/coresight-ctcu.c b/drivers/ >>> hwtracing/coresight/coresight-ctcu.c >>> new file mode 100644 >>> index 000000000000..e1460a627c4d >>> --- /dev/null >>> +++ b/drivers/hwtracing/coresight/coresight-ctcu.c >>> @@ -0,0 +1,268 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/* >>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All >>> rights reserved. >>> + */ >>> + >>> +#include <linux/clk.h> >>> +#include <linux/coresight.h> >>> +#include <linux/device.h> >>> +#include <linux/err.h> >>> +#include <linux/kernel.h> >>> +#include <linux/init.h> >>> +#include <linux/io.h> >>> +#include <linux/module.h> >>> +#include <linux/mutex.h> >>> +#include <linux/of.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/slab.h> >>> + >>> +#include "coresight-ctcu.h" >>> +#include "coresight-priv.h" >>> + >>> +DEFINE_CORESIGHT_DEVLIST(ctcu_devs, "ctcu"); >>> + >>> +#define ctcu_writel(drvdata, val, offset) __raw_writel((val), >>> drvdata->base + offset) >>> +#define ctcu_readl(drvdata, offset) __raw_readl(drvdata->base >>> + offset) >>> + >>> +/* >>> + * The TMC Coresight Control Unit uses four ATID registers to >>> control the data >>> + * filter function based on the trace ID for each TMC ETR sink. The >>> length of >>> + * each ATID register is 32 bits. Therefore, the ETR has a related >>> field in >>> + * CTCU that is 128 bits long. Each trace ID is represented by one >>> bit in that >>> + * filed. >>> + * e.g. ETR0ATID0 layout, set bit 5 for traceid 5 >>> + * bit5 >>> + * ------------------------------------------------------ >>> + * | |28| |24| |20| |16| |12| |8| 1|4| |0| >>> + * ------------------------------------------------------ >>> + * >>> + * e.g. ETR0: >>> + * 127 0 from ATID_offset for ETR0ATID0 >>> + * ------------------------- >>> + * |ATID3|ATID2|ATID1|ATID0| >>> + */ >>> +#define CTCU_ATID_REG_OFFSET(traceid, atid_offset) \ >>> + ((traceid / 32) * 4 + atid_offset) >>> + >>> +#define CTCU_ATID_REG_BIT(traceid) (traceid % 32) >>> +#define CTCU_ATID_REG_SIZE 0x10 >>> + >>> +struct ctcu_atid_config { >>> + const u32 atid_offset; >>> + const u32 port_num; >>> +}; >>> + >>> +struct ctcu_config { >>> + const struct ctcu_atid_config *atid_config; >>> + int num_atid_config; >>> +}; >>> + >>> +static const struct ctcu_atid_config sa8775p_atid_cfgs[] = { >>> + {0xf8, 0}, >>> + {0x108, 1}, >>> +}; >>> + >>> +static const struct ctcu_config sa8775p_cfgs = { >>> + .atid_config = sa8775p_atid_cfgs, >>> + .num_atid_config = ARRAY_SIZE(sa8775p_atid_cfgs), >>> +}; >>> + >>> +static void ctcu_program_atid_register(struct ctcu_drvdata *drvdata, >>> u32 reg_offset, >>> + u8 bit, bool enable) >>> +{ >>> + u32 val; >>> + >>> + CS_UNLOCK(drvdata->base); >>> + val = ctcu_readl(drvdata, reg_offset); >>> + val = enable? (val | BIT(bit)) : (val & ~BIT(bit)); >> >> minor nit: If possible do not use the ternary operator like this. It >> is much better readable as: >> >> if (enable) >> val |= BIT(bit); >> else >> val &= ~BIT(bit); >> > > Will do this way. > >>> + ctcu_writel(drvdata, val, reg_offset); >>> + CS_LOCK(drvdata->base); >>> +} >>> + >>> +/* >>> + * __ctcu_set_etr_traceid: Set bit in the ATID register based on >>> trace ID when enable is true. >>> + * Reset the bit of the ATID register based on trace ID when enable >>> is false. >>> + * >>> + * @csdev: coresight_device struct related to the device >>> + * @traceid: trace ID of the source tracer. >>> + * @port_num: port number from TMC ETR sink. >>> + * @enable: True for set bit and false for reset bit. >>> + * >>> + * Returns 0 indicates success. Non-zero result means failure. >>> + */ >>> +static int __ctcu_set_etr_traceid(struct coresight_device *csdev, u8 >>> traceid, int port_num, >>> + bool enable) >>> +{ >>> + struct ctcu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >>> + u32 atid_offset, reg_offset; >>> + u8 refcnt, bit; >>> + >>> + atid_offset = drvdata->atid_offset[port_num]; >>> + if (atid_offset == 0) >>> + return -EINVAL; >>> + >>> + bit = CTCU_ATID_REG_BIT(traceid); >>> + reg_offset = CTCU_ATID_REG_OFFSET(traceid, atid_offset); >>> + if (reg_offset - atid_offset > CTCU_ATID_REG_SIZE) >>> + return -EINVAL; >>> + >>> + guard(raw_spinlock_irqsave)(&drvdata->spin_lock); >>> + refcnt = drvdata->traceid_refcnt[port_num][traceid]; >>> + /* Only program the atid register when the refcnt value is 0 or >>> 1 */ >> >> A normal trace source won't be enabled more than once (e.g., ETM). The >> only odd one out is the STM, which may be driven by multiple agents. >> So this refcounting looks necessary. >> > > Besides, for the TPDMs which shared the trace_id of the TPDA also need > the refcnt. Consider we have TPDM1 and TPDM2 connected to the same TPDA > device. Once we disable one of the TPDM without checking the refcnt, the > filter function will be disabled for another TPDM. > >>> + if (enable && (++refcnt == 1)) >>> + ctcu_program_atid_register(drvdata, reg_offset, bit, enable); >>> + else if (!enable && (--refcnt == 0)) >>> + ctcu_program_atid_register(drvdata, reg_offset, bit, enable); >> >> minor nit: >> >> if ((enable && !refcount++) || >> (!enable && --refcount)) >> ctcu_program_atid_register(drvdata, reg_offset, bit, enable); >> >> > > I did (enable && (++refcnt == 1)) just because I think we only need > program the register when refcnt is equal to 1. We dont need reprogram > the register with same value when refcnt greater than 1. So I think it's > better for the performance? The code above is similar to yours. It would "set" only for the first time, when enable == 0, refcount == 0 now, but will be incremented to 1. Suzuki

2 months, 1 week

Re: [PATCH v12 6/7] Coresight: Add Coresight TMC Control Unit driver

by James Clark

On 24/02/2025 3:32 am, Jie Gan wrote: > > > On 2/21/2025 7:39 PM, Suzuki K Poulose wrote: >> On 17/02/2025 09:30, Jie Gan wrote: >>> The Coresight TMC Control Unit hosts miscellaneous configuration >>> registers >>> which control various features related to TMC ETR sink. >>> >>> Based on the trace ID, which is programmed in the related CTCU ATID >>> register of a specific ETR, trace data with that trace ID gets into >>> the ETR buffer, while other trace data gets dropped. >>> >>> Enabling source device sets one bit of the ATID register based on >>> source device's trace ID. >>> Disabling source device resets the bit according to the source >>> device's trace ID. >>> >>> Reviewed-by: James Clark <james.clark(a)linaro.org> >>> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> >>> --- >>> drivers/hwtracing/coresight/Kconfig | 12 + >>> drivers/hwtracing/coresight/Makefile | 1 + >>> drivers/hwtracing/coresight/coresight-ctcu.c | 268 +++++++++++++++++++ >>> drivers/hwtracing/coresight/coresight-ctcu.h | 24 ++ >>> include/linux/coresight.h | 3 +- >>> 5 files changed, 307 insertions(+), 1 deletion(-) >>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.c >>> create mode 100644 drivers/hwtracing/coresight/coresight-ctcu.h >>> >>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/ >>> coresight/Kconfig >>> index 06f0a7594169..ecd7086a5b83 100644 >>> --- a/drivers/hwtracing/coresight/Kconfig >>> +++ b/drivers/hwtracing/coresight/Kconfig >>> @@ -133,6 +133,18 @@ config CORESIGHT_STM >>> To compile this driver as a module, choose M here: the >>> module will be called coresight-stm. >>> +config CORESIGHT_CTCU >>> + tristate "CoreSight TMC Control Unit driver" >>> + depends on CORESIGHT_LINK_AND_SINK_TMC >>> + help >>> + This driver provides support for CoreSight TMC Control Unit >>> + that hosts miscellaneous configuration registers. This is >>> + primarily used for controlling the behaviors of the TMC >>> + ETR device. >>> + >>> + To compile this driver as a module, choose M here: the >>> + module will be called coresight-ctcu. >>> + >>> config CORESIGHT_CPU_DEBUG >>> tristate "CoreSight CPU Debug driver" >>> depends on ARM || ARM64 >>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/ >>> hwtracing/ coresight/Makefile >>> index 4ba478211b31..1b7869910a12 100644 >>> --- a/drivers/hwtracing/coresight/Makefile >>> +++ b/drivers/hwtracing/coresight/Makefile >>> @@ -51,3 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight- >>> cti-platform.o \ >>> coresight-cti-sysfs.o >>> obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o >>> obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o >>> +obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o >>> diff --git a/drivers/hwtracing/coresight/coresight-ctcu.c b/drivers/ >>> hwtracing/coresight/coresight-ctcu.c >>> new file mode 100644 >>> index 000000000000..e1460a627c4d >>> --- /dev/null >>> +++ b/drivers/hwtracing/coresight/coresight-ctcu.c >>> @@ -0,0 +1,268 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/* >>> + * Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All >>> rights reserved. >>> + */ >>> + >>> +#include <linux/clk.h> >>> +#include <linux/coresight.h> >>> +#include <linux/device.h> >>> +#include <linux/err.h> >>> +#include <linux/kernel.h> >>> +#include <linux/init.h> >>> +#include <linux/io.h> >>> +#include <linux/module.h> >>> +#include <linux/mutex.h> >>> +#include <linux/of.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/slab.h> >>> + >>> +#include "coresight-ctcu.h" >>> +#include "coresight-priv.h" >>> + >>> +DEFINE_CORESIGHT_DEVLIST(ctcu_devs, "ctcu"); >>> + >>> +#define ctcu_writel(drvdata, val, offset) __raw_writel((val), >>> drvdata->base + offset) >>> +#define ctcu_readl(drvdata, offset) __raw_readl(drvdata->base >>> + offset) >>> + >>> +/* >>> + * The TMC Coresight Control Unit uses four ATID registers to >>> control the data >>> + * filter function based on the trace ID for each TMC ETR sink. The >>> length of >>> + * each ATID register is 32 bits. Therefore, the ETR has a related >>> field in >>> + * CTCU that is 128 bits long. Each trace ID is represented by one >>> bit in that >>> + * filed. >>> + * e.g. ETR0ATID0 layout, set bit 5 for traceid 5 >>> + * bit5 >>> + * ------------------------------------------------------ >>> + * | |28| |24| |20| |16| |12| |8| 1|4| |0| >>> + * ------------------------------------------------------ >>> + * >>> + * e.g. ETR0: >>> + * 127 0 from ATID_offset for ETR0ATID0 >>> + * ------------------------- >>> + * |ATID3|ATID2|ATID1|ATID0| >>> + */ >>> +#define CTCU_ATID_REG_OFFSET(traceid, atid_offset) \ >>> + ((traceid / 32) * 4 + atid_offset) >>> + >>> +#define CTCU_ATID_REG_BIT(traceid) (traceid % 32) >>> +#define CTCU_ATID_REG_SIZE 0x10 >>> + >>> +struct ctcu_atid_config { >>> + const u32 atid_offset; >>> + const u32 port_num; >>> +}; >>> + >>> +struct ctcu_config { >>> + const struct ctcu_atid_config *atid_config; >>> + int num_atid_config; >>> +}; >>> + >>> +static const struct ctcu_atid_config sa8775p_atid_cfgs[] = { >>> + {0xf8, 0}, >>> + {0x108, 1}, >>> +}; >>> + >>> +static const struct ctcu_config sa8775p_cfgs = { >>> + .atid_config = sa8775p_atid_cfgs, >>> + .num_atid_config = ARRAY_SIZE(sa8775p_atid_cfgs), >>> +}; >>> + >>> +static void ctcu_program_atid_register(struct ctcu_drvdata *drvdata, >>> u32 reg_offset, >>> + u8 bit, bool enable) >>> +{ >>> + u32 val; >>> + >>> + CS_UNLOCK(drvdata->base); >>> + val = ctcu_readl(drvdata, reg_offset); >>> + val = enable? (val | BIT(bit)) : (val & ~BIT(bit)); >> >> minor nit: If possible do not use the ternary operator like this. It >> is much better readable as: >> >> if (enable) >> val |= BIT(bit); >> else >> val &= ~BIT(bit); >> > > Will do this way. > >>> + ctcu_writel(drvdata, val, reg_offset); >>> + CS_LOCK(drvdata->base); >>> +} >>> + >>> +/* >>> + * __ctcu_set_etr_traceid: Set bit in the ATID register based on >>> trace ID when enable is true. >>> + * Reset the bit of the ATID register based on trace ID when enable >>> is false. >>> + * >>> + * @csdev: coresight_device struct related to the device >>> + * @traceid: trace ID of the source tracer. >>> + * @port_num: port number from TMC ETR sink. >>> + * @enable: True for set bit and false for reset bit. >>> + * >>> + * Returns 0 indicates success. Non-zero result means failure. >>> + */ >>> +static int __ctcu_set_etr_traceid(struct coresight_device *csdev, u8 >>> traceid, int port_num, >>> + bool enable) >>> +{ >>> + struct ctcu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); >>> + u32 atid_offset, reg_offset; >>> + u8 refcnt, bit; >>> + >>> + atid_offset = drvdata->atid_offset[port_num]; >>> + if (atid_offset == 0) >>> + return -EINVAL; >>> + >>> + bit = CTCU_ATID_REG_BIT(traceid); >>> + reg_offset = CTCU_ATID_REG_OFFSET(traceid, atid_offset); >>> + if (reg_offset - atid_offset > CTCU_ATID_REG_SIZE) >>> + return -EINVAL; >>> + >>> + guard(raw_spinlock_irqsave)(&drvdata->spin_lock); >>> + refcnt = drvdata->traceid_refcnt[port_num][traceid]; >>> + /* Only program the atid register when the refcnt value is 0 or >>> 1 */ >> >> A normal trace source won't be enabled more than once (e.g., ETM). The >> only odd one out is the STM, which may be driven by multiple agents. >> So this refcounting looks necessary. >> > > Besides, for the TPDMs which shared the trace_id of the TPDA also need > the refcnt. Consider we have TPDM1 and TPDM2 connected to the same TPDA > device. Once we disable one of the TPDM without checking the refcnt, the > filter function will be disabled for another TPDM. > >>> + if (enable && (++refcnt == 1)) >>> + ctcu_program_atid_register(drvdata, reg_offset, bit, enable); >>> + else if (!enable && (--refcnt == 0)) >>> + ctcu_program_atid_register(drvdata, reg_offset, bit, enable); >> >> minor nit: >> >> if ((enable && !refcount++) || >> (!enable && --refcount)) >> ctcu_program_atid_register(drvdata, reg_offset, bit, enable); >> >> > > I did (enable && (++refcnt == 1)) just because I think we only need > program the register when refcnt is equal to 1. We dont need reprogram > the register with same value when refcnt greater than 1. So I think it's > better for the performance? > >> Also, see my comment the bottom for "refcount" being u8 . > > Sure, will check. > >> >> >>> + >>> + drvdata->traceid_refcnt[port_num][traceid] = refcnt; >>> + >>> + return 0; >>> +} >>> + >>> +static int ctcu_get_active_port(struct coresight_device *sink, >>> struct coresight_device *helper) >>> +{ >>> + int i; >>> + >>> + for (i = 0; i < sink->pdata->nr_outconns; ++i) { >>> + if (sink->pdata->out_conns[i]->dest_dev) >>> + return sink->pdata->out_conns[i]->dest_port; >> >> Don't we need to make sure it matches the helper ? What if there are >> multiple helpers ? e.g, a CATU connected to the ETR and CTCU ? >> Or even try the opposite ? i.e. search the helper and find the port >> matching the sink ? Good catch, looks like this should be done the opposite way around. >> >> struct coresight_platform_data *pdata = helper->pdata; >> >> for (i = 0; i < pdata->nr_inconns; ++i) >> if (pdata->in_conns[i]->dest_dev == sink) >> return pdata->in_conns[i]->src_port; >> >> Not sure if that works with the helper device connection, James ? Yeah connections are always made in both directions. > Can we check the subtype of the helper device? We should only have one > CTCU helper device for each ETR. > > enum coresight_dev_subtype_helper subtype; > > for (i = 0; i < sink->pdata->nr_outconns; ++i) { > subtype = sink->pdata->out_conns[i]->dest_dev->subtype.helper_subtype; > if (subtype == CORESIGHT_DEV_SUBTYPE_HELPER_CTCU) > return sink->pdata->out_conns[i]->dest_port; > I don't think we need to check the type, just search all the CTCU's in_conns until you find the sink. As Suzuki says, by looking at the out_conns of the sink you might find a different helper device. Checking that it really is connected to the sink is probably more robust that relying on the type anyway.

2 months, 1 week

Re: [PATCH 0/3] hwtracing: coresight: Minor const fixes

by Suzuki K Poulose

On Sat, 22 Feb 2025 12:38:56 +0100, Krzysztof Kozlowski wrote: > Just few improvements. > > Best regards, > Krzysztof > Applied, thanks! [1/3] coresight: catu: Constify amba_id table https://git.kernel.org/coresight/c/2ecdbebe [2/3] coresight: tpda: Constify amba_id table https://git.kernel.org/coresight/c/7b6d52e8 [3/3] coresight: tpdm: Constify amba_id table https://git.kernel.org/coresight/c/2197cbc2 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

2 months, 1 week

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight