CoreSight April 2025

coresight@lists.linaro.org

10 participants
54 discussions

Re: [PATCH] [Patch v3]Add ETE devarch condition in etm4_init_iomem_access

by Mike Leach

Hi, Please stop using the unusual title format of [PATCH}[Patch v3] - use the git command line options to ensure the format is e.g. [PATCH v3] What is the reason that your device cannot use the system instructions to access the ETE? Using a memory interface, if implemented, is only recommended for external debuggers, or on systems where the implementation of system register access is not working. On Thu, 10 Apr 2025 at 10:52, yiru zhang <yiru.zhang(a)mediatek.com> wrote: > > Due to ETE supported, so add ETE devarch condition in etm4_init_iomem_access. > > Signed-off-by: yiru zhang <yiru.zhang(a)mediatek.com> > Reported-by: kernel test robot <lkp(a)intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202504101759.7Ls0Uy4o-lkp@intel.com/ > > v1->v2: use switch case way > v2->v3: clean build warning > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 2b8f10463840..4002a2823fd0 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -1135,11 +1135,15 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, > * with MMIO. But we cannot touch the OSLK until we are > * sure this is an ETM. So rely only on the TRCDEVARCH. > */ > - if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) { > - pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n"); > + switch (devarch & ETM_DEVARCH_ID_MASK) { > + case ETM_DEVARCH_ETMv4x_ARCH: > + case ETM_DEVARCH_ETE_ARCH: > + break; > + default: > + pr_warn_once("Unknown ETM architecture: 0x%lx\n", > + devarch & ETM_DEVARCH_ID_MASK); > return false; > } > - > drvdata->arch = etm_devarch_to_arch(devarch); > *csa = CSDEV_ACCESS_IOMEM(drvdata->base); > return true; > -- > 2.46.0 > Otherwise - Reviewed-by: Mike Leach <mike.leach(a)linaro.org> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

3 weeks

[PATCH v2 0/5] coresight: Add Coresight Trace NOC driver

by Yuanfang Zhang

The Trace NoC is an integration hierarchy which is a replacement of Dragonlink configuration. It brings together debug component like TPDA, funnel and interconnect Trace Noc which collects trace from subsystems and transfers to QDSS sink. Compared to DL, it has the following advantages: 1. Reduce wires between subsystems. 2. Continue cleaning the infrastructure. 3. Reduce Data overhead by transporting raw data from source to target. +--------------+ +-------------+ | SDCC5 TPDM | | SDCC5 TPDM | +--------------+ +-------------+ | | | | +----------|-------------------+ | | v | | | +----v----+ Dragon Link | v | |DLNT TPDA| North | +----------------------+ | +---------+ | | TRACE NOC AG | | | | | | | v-------------+ | +----------------------+ | | | | | +------v-----+ | | | | DLNT Funnel| | | | +------------+ | | | | | | +-------------------|----------+ | <-----+ | | | | | v v +----------------+ +---------------+ | QDSS | | QDSS | +----------------+ +---------------+ Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com> --- --- Changes in v2: 1. Modified the format of DT binging file. 2. Fix compile warnings. - Link to v1: https://lore.kernel.org/r/20250221-trace-noc-driver-v1-0-0a23fc643217@quici… --- Yuanfang Zhang (5): dt-bindings: arm: Add Coresight device Trace NOC definition coresight: add coresight Trace NOC driver coresight-tnoc: add nodes to configure flush coresight-tnoc: add node to configure flag type coresight-tnoc: add nodes to configure freq packet .../bindings/arm/qcom,coresight-tnoc.yaml | 116 ++++++ drivers/hwtracing/coresight/Kconfig | 13 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/coresight-tnoc.c | 400 +++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tnoc.h | 57 +++ 5 files changed, 587 insertions(+) --- base-commit: 92514ef226f511f2ca1fb1b8752966097518edc0 change-id: 20250212-trace-noc-driver-9e75d78114fa Best regards, -- Yuanfang Zhang <quic_yuanfang(a)quicinc.com>

3 weeks, 2 days

Re: [PATCH] [Patch v1]Add ETE devarch condition in etm4_init_iomem_access

by Suzuki K Poulose

Hi Yiru On 08/04/2025 08:36, yiru zhang wrote: > Due to ETE supported, so add ETE devarch condition in etm4_init_iomem_access. Is there a reason why you cannot use the "system instructions to access the ETE" ? The patch as such is fine by me (withe some minor styling nits). But, we do not recommend using the MMIO for ETE, when it can be accessed directly by sysreg. FWIW, if you remove the "mmio base" address from the DT/ACPI, it should automatically use the system instructions for ETE. > Signed-off-by: yiru zhang <yiru.zhang(a)mediatek.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 2b8f10463840..971b9f0fe5e4 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -1135,8 +1135,9 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, > * with MMIO. But we cannot touch the OSLK until we are > * sure this is an ETM. So rely only on the TRCDEVARCH. > */ > - if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) { > - pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n"); > + if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH \ > + && (devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETE_ARCH) { We could use a switch case ? switch (devarch & ETM_DEVARCH_ID_MASK) { case ETM_DEVARCH_ETMv4x_ARCH: case ETM_DEVARCH_ETE_ARCH: break; default: pr_warn_once("Unknown ETM architecture: %x\n", devarch & ETM_DEVARCH_ID_MASK); return false; } Suzuki > + pr_warn_once("TRCDEVARCH doesn't match ETMv4&ETE architecture\n"); > return false; > } >

3 weeks, 3 days

Re: [PATCH v2 1/3] coresight: catu: Introduce refcount and spinlock for enabling/disabling

by Suzuki K Poulose

On 03/04/2025 18:53, Yabin Cui wrote: > On Wed, Apr 2, 2025 at 6:01 AM Leo Yan <leo.yan(a)arm.com> wrote: >> >> Hi Yabin, >> >> On Tue, Apr 01, 2025 at 06:21:59PM -0700, Yabin Cui wrote: >> >> [...] >> >>>> @@ -486,12 +491,17 @@ static int catu_disable_hw(struct catu_drvdata *drvdata) >>>> >>>> static int catu_disable(struct coresight_device *csdev, void *__unused) >>>> { >>>> - int rc; >>>> + int rc = 0; >>>> struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev); >>>> + guard(raw_spinlock_irqsave)(&catu_drvdata->spinlock); >>>> >>>> - CS_UNLOCK(catu_drvdata->base); >>>> - rc = catu_disable_hw(catu_drvdata); >>>> - CS_LOCK(catu_drvdata->base); >>>> + if (--csdev->refcnt == 0) { >>>> + CS_UNLOCK(catu_drvdata->base); >>>> + rc = catu_disable_hw(catu_drvdata); >>>> + CS_LOCK(catu_drvdata->base); >>>> + } else { >>>> + rc = -EBUSY; >> >> This is not an error if the decremented reference counter is not zero. >> It should return 0. Otherwise, the change looks good to me. > > In coresight_disable_helpers(), the return value of catu_disable() > isn't checked. > The -EBUSY return was used for consistency with other refcounted > disable functions > like tmc_disable_etf_sink() and tmc_disable_etr_sink(). I'm happy to > change it back > to 0 if you believe that would be the more accurate return value here. Please stick to 0 here. This indicates there was no errors w.r.t the current session. This is similar to what we do for TMC ETR for e.g. Suzuki > >> >> Thanks, >> Leo

4 weeks

[PATCH] coresight: Break to error handler if connection can't be made

by James Clark

Change the return to a break so that the of_node_put()s in the error handler are hit on failure. Fixes: 3d4ff657e454 ("coresight: Dynamically add connections") Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> Closes: https://lore.kernel.org/linux-arm-kernel/3b026b3f-2cb2-49ef-aa20-8b14220f53… Signed-off-by: James Clark <james.clark(a)linaro.org> --- drivers/hwtracing/coresight/coresight-platform.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c index 8192ba3279f0..39851a13ff89 100644 --- a/drivers/hwtracing/coresight/coresight-platform.c +++ b/drivers/hwtracing/coresight/coresight-platform.c @@ -267,7 +267,8 @@ static int of_coresight_parse_endpoint(struct device *dev, new_conn = coresight_add_out_conn(dev, pdata, &conn); if (IS_ERR_VALUE(new_conn)) { fwnode_handle_put(conn.dest_fwnode); - return PTR_ERR(new_conn); + ret = PTR_ERR(new_conn); + break; } /* Connection record updated */ } while (0); --- base-commit: 5442d22da7dbff3ba8c6720fc6f23ea4934d402d change-id: 20250402-james-cs-ret-break-fix-ad38103af5f6 Best regards, -- James Clark <james.clark(a)linaro.org>

4 weeks, 1 day

Re: [PATCH v2 3/3] coresight: core: Disable helpers for devices that fail to enable

by Leo Yan

On Tue, Apr 01, 2025 at 06:22:46PM -0700, Yabin Cui wrote: > On Tue, Apr 1, 2025 at 6:19 PM Yabin Cui <yabinc(a)google.com> wrote: > > > > When enabling a SINK or LINK type coresight device fails, the > > associated helpers should be disabled. > > > > Signed-off-by: Yabin Cui <yabinc(a)google.com> > > Suggested-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> > > --- > > drivers/hwtracing/coresight/coresight-core.c | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > > index fb43ef6a3b1f..e3270d9b46c9 100644 > > --- a/drivers/hwtracing/coresight/coresight-core.c > > +++ b/drivers/hwtracing/coresight/coresight-core.c > > @@ -465,7 +465,7 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode, > > /* Enable all helpers adjacent to the path first */ > > ret = coresight_enable_helpers(csdev, mode, path); > > if (ret) > > - goto err; > > + goto err_helper; > > /* > > * ETF devices are tricky... They can be a link or a sink, > > * depending on how they are configured. If an ETF has been > > @@ -480,14 +480,8 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode, > > switch (type) { > > case CORESIGHT_DEV_TYPE_SINK: > > ret = coresight_enable_sink(csdev, mode, sink_data); > > - /* > > - * Sink is the first component turned on. If we > > - * failed to enable the sink, there are no components > > - * that need disabling. Disabling the path here > > - * would mean we could disrupt an existing session. > > - */ > > if (ret) > > - goto out; > > + goto err; > > break; > > case CORESIGHT_DEV_TYPE_SOURCE: > > /* sources are enabled from either sysFS or Perf */ > > @@ -507,6 +501,8 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode, > > out: > > return ret; > > err: > > + coresight_disable_helpers(csdev); Given it is unconditionally to enable helpers, I would suggest we disable helper unconditionally. Thus, we don't need to add a new 'err_helper' tag, simply reuse the 'err' tag would be fine. I would like to know if mainatiners have different opinions. Thanks, Leo > > +err_helper: > > coresight_disable_path_from(path, nd); > > goto out; > > } > > -- > > 2.49.0.472.ge94155a9ec-goog > >

4 weeks, 1 day

Re: [PATCH v2 2/3] coresight: catu: Prevent concurrent PERF and SYSFS mode enablement

by Leo Yan

On Tue, Apr 01, 2025 at 06:18:31PM -0700, Yabin Cui wrote: > A CATU device can be enabled for either PERF mode or SYSFS mode, but > not both simultaneously. Consider the following race condition: > > CPU 1 enables CATU for PERF mode. > CPU 2 enables CATU for SYSFS mode. It only increases refcnt. > CPU 2 enables ETR for SYSFS mode. > CPU 1 fails to enable ETR for PERF mode. > > This results in a CATU being enabled in PERF mode and an ETR enabled > in SYSFS mode, leading to unknown behavior. > > To prevent this situation, this patch checks enabled mode of a > CATU device before increasing its refcnt. Yes. We have also observed the same issue. I am currently working on refactoring the Arm CoreSight locking, my plan is to use coresight_path to maintain mode. Given it is uncommon case to use two modes concurrently in Arm CoreSight, I assume this is not an urgent issue. Could we defer this patch? My understanding is that this issue will be prevented with checking the path mode, rather than checking modes in seperate modules. To be clear, I am fine with other two patches in the series. Thanks, Leo > Signed-off-by: Yabin Cui <yabinc(a)google.com> > Suggested-by: James Clark <james.clark(a)linaro.org> > Suggested-by: Leo Yan <leo.yan(a)arm.com> > --- > drivers/hwtracing/coresight/coresight-catu.c | 6 +++++- > drivers/hwtracing/coresight/coresight-catu.h | 1 + > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c > index b1d490dd7249..0caf3a3e75ce 100644 > --- a/drivers/hwtracing/coresight/coresight-catu.c > +++ b/drivers/hwtracing/coresight/coresight-catu.c > @@ -466,7 +466,10 @@ static int catu_enable(struct coresight_device *csdev, enum cs_mode mode, > CS_UNLOCK(catu_drvdata->base); > rc = catu_enable_hw(catu_drvdata, mode, data); > CS_LOCK(catu_drvdata->base); > - } > + if (!rc) > + catu_drvdata->mode = mode; > + } else if (catu_drvdata->mode != mode) > + rc = -EBUSY; > if (!rc) > csdev->refcnt++; > return rc; > @@ -499,6 +502,7 @@ static int catu_disable(struct coresight_device *csdev, void *__unused) > CS_UNLOCK(catu_drvdata->base); > rc = catu_disable_hw(catu_drvdata); > CS_LOCK(catu_drvdata->base); > + catu_drvdata->mode = CS_MODE_DISABLED; > } else { > rc = -EBUSY; > } > diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h > index 755776cd19c5..ea406464f0a8 100644 > --- a/drivers/hwtracing/coresight/coresight-catu.h > +++ b/drivers/hwtracing/coresight/coresight-catu.h > @@ -66,6 +66,7 @@ struct catu_drvdata { > struct coresight_device *csdev; > int irq; > raw_spinlock_t spinlock; > + enum cs_mode mode; > }; > > #define CATU_REG32(name, offset) \ > -- > 2.49.0.472.ge94155a9ec-goog >

4 weeks, 1 day

Re: [PATCH v2 1/3] coresight: catu: Introduce refcount and spinlock for enabling/disabling

by Leo Yan

Hi Yabin, On Tue, Apr 01, 2025 at 06:21:59PM -0700, Yabin Cui wrote: [...] > > @@ -486,12 +491,17 @@ static int catu_disable_hw(struct catu_drvdata *drvdata) > > > > static int catu_disable(struct coresight_device *csdev, void *__unused) > > { > > - int rc; > > + int rc = 0; > > struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev); > > + guard(raw_spinlock_irqsave)(&catu_drvdata->spinlock); > > > > - CS_UNLOCK(catu_drvdata->base); > > - rc = catu_disable_hw(catu_drvdata); > > - CS_LOCK(catu_drvdata->base); > > + if (--csdev->refcnt == 0) { > > + CS_UNLOCK(catu_drvdata->base); > > + rc = catu_disable_hw(catu_drvdata); > > + CS_LOCK(catu_drvdata->base); > > + } else { > > + rc = -EBUSY; This is not an error if the decremented reference counter is not zero. It should return 0. Otherwise, the change looks good to me. Thanks, Leo

4 weeks, 1 day

[PATCH v3 0/6] 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 updates buffer on AUX pause occasion, which can mitigate the trace data lose issue. Patch 07 documents the AUX pause usages with Arm CoreSight. 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. Changes from v2: - Rebased on CoreSight next branch. - Dropped the uAPI 'update_buf_on_pause' and updated document respectively (Suzuki). - Renamed ETM callbacks to .pause_perf() and .resume_perf() (Suzuki). - Minor improvement for error handling in the AUX resume flow. Changes from v1: - Added validation function pointers in pause and resume APIs (Mike). Leo Yan (6): 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: perf: Update buffer on AUX pause Documentation: coresight: Document AUX pause and resume .../trace/coresight/coresight-perf.rst | 31 ++++ drivers/hwtracing/coresight/coresight-core.c | 22 +++ .../hwtracing/coresight/coresight-etm-perf.c | 84 +++++++++- .../coresight/coresight-etm4x-core.c | 143 +++++++++++++----- drivers/hwtracing/coresight/coresight-etm4x.h | 2 + drivers/hwtracing/coresight/coresight-priv.h | 2 + include/linux/coresight.h | 4 + 7 files changed, 246 insertions(+), 42 deletions(-) -- 2.34.1

4 weeks, 1 day

Re: [bug report] coresight: Dynamically add connections

by James Clark

On 02/04/2025 12:00 pm, Dan Carpenter wrote: > Hello James Clark, > > Commit 3d4ff657e454 ("coresight: Dynamically add connections") from > Apr 25, 2023 (linux-next), leads to the following Smatch static > checker warning: > > drivers/hwtracing/coresight/coresight-platform.c:279 of_coresight_parse_endpoint() > warn: inconsistent refcounting 'rep->kobj.kref.refcount.refs.counter': > > drivers/hwtracing/coresight/coresight-platform.c > 196 static int of_coresight_parse_endpoint(struct device *dev, > 197 struct device_node *ep, > 198 struct coresight_platform_data *pdata) > 199 { > 200 int ret = 0; > 201 struct of_endpoint endpoint, rendpoint; > 202 struct device_node *rparent = NULL; > 203 struct device_node *rep = NULL; > 204 struct device *rdev = NULL; > 205 struct fwnode_handle *rdev_fwnode; > 206 struct coresight_connection conn = {}; > 207 struct coresight_connection *new_conn; > 208 > 209 do { > 210 /* Parse the local port details */ > 211 if (of_graph_parse_endpoint(ep, &endpoint)) > 212 break; > 213 /* > 214 * Get a handle on the remote endpoint and the device it is > 215 * attached to. > 216 */ > 217 rep = of_graph_get_remote_endpoint(ep); > 218 if (!rep) > 219 break; > 220 rparent = of_coresight_get_port_parent(rep); > 221 if (!rparent) > 222 break; > 223 if (of_graph_parse_endpoint(rep, &rendpoint)) > 224 break; > 225 > 226 rdev_fwnode = of_fwnode_handle(rparent); > 227 /* If the remote device is not available, defer probing */ > 228 rdev = coresight_find_device_by_fwnode(rdev_fwnode); > 229 if (!rdev) { > 230 ret = -EPROBE_DEFER; > 231 break; > 232 } > 233 > 234 conn.src_port = endpoint.port; > 235 /* > 236 * Hold the refcount to the target device. This could be > 237 * released via: > 238 * 1) coresight_release_platform_data() if the probe fails or > 239 * this device is unregistered. > 240 * 2) While removing the target device via > 241 * coresight_remove_match() > 242 */ > 243 conn.dest_fwnode = fwnode_handle_get(rdev_fwnode); > 244 conn.dest_port = rendpoint.port; > 245 > 246 /* > 247 * Get the firmware node of the filter source through the > 248 * reference. This could be used to filter the source in > 249 * building path. > 250 */ > 251 conn.filter_src_fwnode = > 252 fwnode_find_reference(&ep->fwnode, "filter-source", 0); > 253 if (IS_ERR(conn.filter_src_fwnode)) { > 254 conn.filter_src_fwnode = NULL; > 255 } else { > 256 conn.filter_src_dev = > 257 coresight_find_csdev_by_fwnode(conn.filter_src_fwnode); > 258 if (conn.filter_src_dev && > 259 !coresight_is_device_source(conn.filter_src_dev)) { > 260 dev_warn(dev, "port %d: Filter handle is not a trace source : %s\n", > 261 conn.src_port, dev_name(&conn.filter_src_dev->dev)); > 262 conn.filter_src_dev = NULL; > 263 conn.filter_src_fwnode = NULL; > 264 } > 265 } > 266 > 267 new_conn = coresight_add_out_conn(dev, pdata, &conn); > 268 if (IS_ERR_VALUE(new_conn)) { > 269 fwnode_handle_put(conn.dest_fwnode); > 270 return PTR_ERR(new_conn); > > Should this be a break statement instead of return statement? > Hi Dan, Yes it probably should, I'll send a patch. Thanks James

4 weeks, 1 day

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight April 2025