CoreSight

coresight@lists.linaro.org

9 participants
2400 discussions

Re: [BUG] perf: No samples found when using kcore + coresight

by James Clark

On 30/03/2023 00:25, Yang Shi wrote: > On Wed, Mar 29, 2023 at 9:08 AM James Clark <james.clark(a)arm.com> wrote: >> >> >> >> On 14/03/2023 00:36, Leo Yan wrote: >>> On Mon, Mar 13, 2023 at 11:15:44AM -0700, Yang Shi wrote: >>> >>> [...] >>> >>>>> Just a quick summary, here we have two issues: >>>>> >>>>> - With command: >>>>> perf record -e cs_etm/@tmc_etf63/k --kcore --per-thread \ >>>>> -- taskset --cpu-list 1 uname", >>>>> >>>>> perf doesn't enable "text poke" attribution. >>>> >>>> No, it enables "text poke" and perf fails to decode coresight trace >>>> data too. It doesn't matter whether "--kcore" is after or before "-e >>>> cs/etm/@tmc_etf63/k". >>> >>> Understand now. Thanks for correction, if so we can ignore this one. >>> >>> Leo >> >> To me it looks like it's only --per-thread and --kcore together that >> cause the issue. I can't see if that was mentioned previously in this >> thread. > > If "--pre-thread" is not passed in, perf record failed with "failed to > mmap with 12 (Cannot allocate memory)". Sorry for not mentioning this > in the first place. I was quite focused on --kcore and didn't realize > they may be related. That's unrelated. That's because you have specified a sink and without --per-thread it tries to open the event on all cores. If the sink can't be reached from all cores it will fail to open. You can make it work without --per-thread if you limit it to a valid core like this, although I don't know which ones exactly would be valid for your system: perf record -e cs_etm/@tmc_etf63/k --kcore -C 0 \ -- taskset --cpu-list 1 uname > >> >> If it is --per-thread that's causing the issue then I think I have an >> idea why it might be. There are some assumptions and different paths >> taken in decoding in that mode that aren't correct. It causes some other >> issues to do with ordering and timestamps as well and I wanted to fix it >> previously. I wouldn't say that the text-poke change has caused a >> regression, as decoding in this mode was always a bit buggy. >> >> Maybe this is another reason to fix it properly.

2 years, 3 months

[PATCH v2 0/9] coresight: Fix CTI module refcount leak by making it a helper device

by James Clark

Changes since v1: * Don't dereference handle in tmc_etr_get_buffer() when not in perf mode. * Fix some W=1 warnings * Add a commit to rename child/output in terms of local/remote ------------------- Currently there is a refcount leak in CTI when using system wide mode or tracing multithreaded applications. See the last commit for a reproducer. This prevents the module from being unloaded. Historically there have been a few issues and fixes attempted around here which have resulted in some extra logic and a member to keep track of CTI being enabled 'struct coresight_device->ect_enabled'. The fix in commit 665c157e0204 ("coresight: cti: Fix hang in cti_disable_hw()") was also related to CTI having its own enable/disable path which came later than other devices. If we make CTI a helper device and enable helper devices adjacent to the path we get very similar enable/disable behavior to now, but with more reuse of the existing reference counting logic in the coresight core code. This also affects CATU which can have a little bit of its hard coded enable/disable code removed. Enabling CATU on the generic path does require that input connections are tracked so that it can get its associated ETR buffer. Applies to coresight/next (669c4614236a7) but also requires the realloc_array patch here [1]. Also available in full here [2]. [1]: https://lore.kernel.org/linux-arm-kernel/20230306152723.3090195-1-james.cla… [2]: https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-cti-module-refcou… James Clark (9): coresight: Use enum type for cs_mode wherever possible coresight: Change name of pdata->conns coresight: Rename nr_outports to nr_outconns coresight: Rename connection members to allow for input connections coresight: Dynamically add connections coresight: Store in-connections as well as out-connections coresight: Refactor out buffer allocation function for ETR coresight: Enable and disable helper devices adjacent to the path coresight: Fix CTI module refcount leak by making it a helper device drivers/hwtracing/coresight/coresight-catu.c | 34 +- drivers/hwtracing/coresight/coresight-core.c | 312 +++++++++++------- .../hwtracing/coresight/coresight-cti-core.c | 56 ++-- .../hwtracing/coresight/coresight-cti-sysfs.c | 4 +- drivers/hwtracing/coresight/coresight-cti.h | 4 +- drivers/hwtracing/coresight/coresight-etb10.c | 3 +- .../coresight/coresight-etm3x-core.c | 6 +- .../coresight/coresight-etm4x-core.c | 6 +- .../hwtracing/coresight/coresight-platform.c | 178 +++++++--- drivers/hwtracing/coresight/coresight-priv.h | 9 +- drivers/hwtracing/coresight/coresight-stm.c | 6 +- drivers/hwtracing/coresight/coresight-sysfs.c | 9 +- .../hwtracing/coresight/coresight-tmc-etf.c | 2 +- .../hwtracing/coresight/coresight-tmc-etr.c | 89 ++--- drivers/hwtracing/coresight/coresight-tmc.h | 2 + drivers/hwtracing/coresight/coresight-tpdm.c | 4 +- drivers/hwtracing/coresight/coresight-tpiu.c | 3 +- drivers/hwtracing/coresight/coresight-trbe.c | 3 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 3 +- drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- include/linux/coresight.h | 109 +++--- 21 files changed, 530 insertions(+), 314 deletions(-) -- 2.34.1

2 years, 3 months

[PATCH v1 0/8] Add cmb dataset support for TPDM

by Mao Jinlong

Continuous multi-bit (CMB) is responsible for collection of CMB data sets. It monitors a bus of related signals (eg, a counter value) and an associated valid signal. This patch series adds support to config of CMB registers. Element Creation: CMB_CR.mode : Continuous mode or Trace on Change mode(cmb_mode) Pattern Match Output (usually to CTI): CMB_XPR : Trigger pattern register value(cmb_trig_patt_val) CMB_XPMR : Trigger pattern mask register value(cmb_trig_patt_mask) Timestamp Request Based on Input Pattern Match: CMB_TPR : Timestamp pattern register value(cmb_patt_val) CMB_TPMR : Timestamp pattern mask register value(cmb_patt_mask) CMB_TIER.patt_tsenab : Timestamps are requested upon CMB interface pattern match via setting this bit to 1(cmb_patt_ts) Timestamp Request Based on Input (usually from CTI): CMB_TIER.xtrig_tsenab : Timestamps are requested upon CMB cross trigger interface timestamp request via setting this bit to 1(cmb_trig_ts) Mux Select Registers: CMB_MSR : Configure Mux select registers(cmb_msr) Once this series patches are applied properly, the new tpdm nodes should be observed at the tpdm path. e.g. /sys/devices/platform/soc(a)0/10c29000.tpdm/tpdm1 # ls -l | grep cmb -rw-r--r-- 1 root 0 4096 Jan 1 00:00 cmb_mode -rw-r--r-- 1 root 0 4096 Jan 1 00:00 cmb_msr -rw-r--r-- 1 root 0 4096 Jan 1 00:00 cmb_patt_mask -rw-r--r-- 1 root 0 4096 Jan 1 00:00 cmb_patt_ts -rw-r--r-- 1 root 0 4096 Jan 1 00:22 cmb_patt_val -rw-r--r-- 1 root 0 4096 Jan 1 00:59 cmb_trig_patt_mask -rw-r--r-- 1 root 0 4096 Jan 1 00:57 cmb_trig_patt_val -rw-r--r-- 1 root 0 4096 Jan 1 00:00 cmb_trig_ts -rw-r--r-- 1 root 0 4096 Jan 1 00:58 cmb_ts_all This patch series depends on: [v3,0/11] Add support to configure TPDM DSB subunit https://patchwork.kernel.org/project/linux-arm-kernel/cover/1679551448-1916… Codelinaro link: https://git.codelinaro.org/clo/linux-kernel/coresight/-/commits/tpdm-cmb-v1 Mao Jinlong (8): coresight-tpdm: Add CMB dataset support coresight-tpdm: Add support to configure CMB collection mode coresight-tpdm: Add pattern registers support for CMB data set coresight-tpdm: Add timestamp control register support for the CMB coresight-tpdm: Add msr register support for CMB dt-bindings: arm: Add support for TPDM CMB MSR register coresight-tpda: Add support to configure CMB element size dt-bindings: arm: Add support for TPDM CMB element size .../testing/sysfs-bus-coresight-devices-tpdm | 63 +++ .../bindings/arm/qcom,coresight-tpdm.yaml | 26 + drivers/hwtracing/coresight/coresight-tpda.c | 33 +- drivers/hwtracing/coresight/coresight-tpda.h | 4 + drivers/hwtracing/coresight/coresight-tpdm.c | 447 +++++++++++++++++- drivers/hwtracing/coresight/coresight-tpdm.h | 60 +++ 6 files changed, 626 insertions(+), 7 deletions(-) -- 2.39.0

2 years, 3 months

Re: [PATCH v2 1/3] Coresight: Add coresight dummy driver

by Suzuki K Poulose

On 28/03/2023 12:25, Hao Zhang wrote: > Hi Mike, > > On 3/28/2023 6:06 PM, Mike Leach wrote: >> Hi, >> >> A few additional comments.... >> >> On Tue, 28 Mar 2023 at 10:24, Hao Zhang <quic_hazha(a)quicinc.com> wrote: >>> >>> Hi Suzuki, >>> >>> On 3/28/2023 4:35 PM, Suzuki K Poulose wrote: >>>> On 28/03/2023 08:22, Hao Zhang wrote: >>>>> Hi Mike, >>>>> >>>>> On 3/27/2023 11:58 PM, Mike Leach wrote: >>>>>> Hi, >>>>>> >>>>>> On Fri, 24 Mar 2023 at 06:16, Hao Zhang <quic_hazha(a)quicinc.com> >>>>>> wrote: >>>>>>> >>>>>>> Some Coresight devices that HLOS don't have permission to access >>>>>>> or configure. Such as Coresight sink EUD, some TPDMs etc. So there >>>>>>> need driver to register dummy devices as Coresight devices. Provide >>>>>>> Coresight API for dummy device operations, such as enabling and >>>>>>> disabling dummy devices. Build the Coresight path for dummy sink or >>>>>>> dummy source for debugging. >>>>>>> >>>>>>> Signed-off-by: Hao Zhang <quic_hazha(a)quicinc.com> >>>>>>> --- >>>>>>> drivers/hwtracing/coresight/Kconfig | 11 ++ >>>>>>> drivers/hwtracing/coresight/Makefile | 1 + >>>>>>> drivers/hwtracing/coresight/coresight-dummy.c | 176 >>>>>>> ++++++++++++++++++ >>>>>>> 3 files changed, 188 insertions(+) >>>>>>> create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c >>>>>>> >>>>>>> diff --git a/drivers/hwtracing/coresight/Kconfig >>>>>>> b/drivers/hwtracing/coresight/Kconfig >>>>>>> index 2b5bbfffbc4f..06f0a7594169 100644 >>>>>>> --- a/drivers/hwtracing/coresight/Kconfig >>>>>>> +++ b/drivers/hwtracing/coresight/Kconfig >>>>>>> @@ -236,4 +236,15 @@ config CORESIGHT_TPDA >>>>>>> >>>>>>> To compile this driver as a module, choose M here: the >>>>>>> module will be >>>>>>> called coresight-tpda. >>>>>>> + >>>>>>> +config CORESIGHT_DUMMY >>>>>>> + tristate "Dummy driver support" >>>>>>> + help >>>>>>> + Enables support for dummy driver. Dummy driver can be used >>>>>>> for >>>>>>> + CoreSight sources/sinks that are owned and configured >>>>>>> by some >>>>>>> + other subsystem and use Linux drivers to configure rest of >>>>>>> trace >>>>>>> + path. >>>>>>> + >>>>>>> + To compile this driver as a module, choose M here: the >>>>>>> module will be >>>>>>> + called coresight-dummy. >>>>>>> endif >>>>>>> diff --git a/drivers/hwtracing/coresight/Makefile >>>>>>> b/drivers/hwtracing/coresight/Makefile >>>>>>> index 33bcc3f7b8ae..995d3b2c76df 100644 >>>>>>> --- a/drivers/hwtracing/coresight/Makefile >>>>>>> +++ b/drivers/hwtracing/coresight/Makefile >>>>>>> @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o >>>>>>> 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 >>>>>>> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c >>>>>>> b/drivers/hwtracing/coresight/coresight-dummy.c >>>>>>> new file mode 100644 >>>>>>> index 000000000000..2d4eb3e546eb >>>>>>> --- /dev/null >>>>>>> +++ b/drivers/hwtracing/coresight/coresight-dummy.c >>>>>>> @@ -0,0 +1,176 @@ >>>>>>> +// SPDX-License-Identifier: GPL-2.0 >>>>>>> +/* >>>>>>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights >>>>>>> reserved. >>>>>>> + */ >>>>>>> + >>>>>>> +#include <linux/kernel.h> >>>>>>> +#include <linux/module.h> >>>>>>> +#include <linux/platform_device.h> >>>>>>> +#include <linux/coresight.h> >>>>>>> +#include <linux/of.h> >>>>>>> +#include <linux/pm_runtime.h> >>>>>>> + >>>>>>> +#include "coresight-priv.h" >>>>>>> +#include "coresight-trace-id.h" >>>>>>> + >>>>>>> +struct dummy_drvdata { >>>>>>> + struct device *dev; >>>>>>> + struct coresight_device *csdev; >>>>>>> + int traceid; >>>>>>> +}; >>>>>>> + >>>>>>> +DEFINE_CORESIGHT_DEVLIST(dummy_devs, "dummy"); >>>>>>> + >> >> minor nit: can we have dummy_source and dummy_sink as the device names >> to make it clear at the first level what these are without having to >> look at the attributes? >> > > This is a good advice, dummy_source and dummy_sink are two different > components, so it's better to separate it at the first level. I will > take your advice in the next version of patch. > >>>>>>> +static int dummy_source_enable(struct coresight_device *csdev, >>>>>>> + struct perf_event *event, u32 mode) >>>>>>> +{ >>>>>>> + struct dummy_drvdata *drvdata = >>>>>>> dev_get_drvdata(csdev->dev.parent); >>>>>>> + >>>>>>> + dev_info(drvdata->dev, "Dummy source enabled\n"); >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static void dummy_source_disable(struct coresight_device *csdev, >>>>>>> + struct perf_event *event) >>>>>>> +{ >>>>>>> + struct dummy_drvdata *drvdata = >>>>>>> dev_get_drvdata(csdev->dev.parent); >>>>>>> + >>>>>>> + dev_info(drvdata->dev, "Dummy source disabled\n"); >>>>>>> +} >>>>>>> + >>>>>>> +static int dummy_sink_enable(struct coresight_device *csdev, u32 >>>>>>> mode, >>>>>>> + void *data) >>>>>>> +{ >>>>>>> + struct dummy_drvdata *drvdata = >>>>>>> dev_get_drvdata(csdev->dev.parent); >>>>>>> + >>>>>>> + dev_info(drvdata->dev, "Dummy sink enabled\n"); >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static int dummy_sink_disable(struct coresight_device *csdev) >>>>>>> +{ >>>>>>> + struct dummy_drvdata *drvdata = >>>>>>> dev_get_drvdata(csdev->dev.parent); >>>>>>> + >>>>>>> + dev_info(drvdata->dev, "Dummy sink disabled\n"); >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static const struct coresight_ops_source dummy_source_ops = { >>>>>>> + .enable = dummy_source_enable, >>>>>>> + .disable = dummy_source_disable, >>>>>>> +}; >>>>>>> + >>>>>>> +static const struct coresight_ops_sink dummy_sink_ops = { >>>>>>> + .enable = dummy_sink_enable, >>>>>>> + .disable = dummy_sink_disable, >>>>>>> +}; >>>>>>> + >>>>>>> +static const struct coresight_ops dummy_cs_ops = { >>>>>>> + .source_ops = &dummy_source_ops, >>>>>>> + .sink_ops = &dummy_sink_ops, >>>>>>> +}; >>>>>>> + >>>>>>> +static int dummy_probe(struct platform_device *pdev) >>>>>>> +{ >>>>>>> + int ret, trace_id; >>>>>>> + struct device *dev = &pdev->dev; >>>>>>> + struct coresight_platform_data *pdata; >>>>>>> + struct dummy_drvdata *drvdata; >>>>>>> + struct coresight_desc desc = { 0 }; >>>>>>> + >>>>>>> + desc.name = coresight_alloc_device_name(&dummy_devs, dev); >>>>>>> + if (!desc.name) >>>>>>> + return -ENOMEM; >>>>>>> + >>>>>>> + pdata = coresight_get_platform_data(dev); >>>>>>> + if (IS_ERR(pdata)) >>>>>>> + return PTR_ERR(pdata); >>>>>>> + pdev->dev.platform_data = pdata; >>>>>>> + >>>>>>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >>>>>>> + if (!drvdata) >>>>>>> + return -ENOMEM; >>>>>>> + >>>>>>> + drvdata->dev = &pdev->dev; >>>>>>> + platform_set_drvdata(pdev, drvdata); >>>>>>> + >>>>>>> + if (of_property_read_bool(pdev->dev.of_node, >>>>>>> "qcom,dummy-source")) { >>>>>>> + desc.type = CORESIGHT_DEV_TYPE_SOURCE; >>>>>>> + desc.subtype.source_subtype = >>>>>>> + CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS; >>>>>>> + } else if (of_property_read_bool(pdev->dev.of_node, >>>>>>> + "qcom,dummy-sink")) { >> >> It would simplify things if the compatibles were >> arm,coresight-dummy-source and arm,coresight-dummy-sink - and drop the >> two additional attributes, using of_device_is_compatible() here. >> > > Yes, I will update it in the next version of patch. > >>>>>>> + desc.type = CORESIGHT_DEV_TYPE_SINK; >>>>>>> + desc.subtype.sink_subtype = >>>>>>> CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; >>>>>> >>>>>> This will break the automatic sink selection on a system where >>>>>> perf is >>>>>> looking for a default sink and the dummy sink is closest / first >>>>>> discovered. >>>>>> >>>>>> i.e. when perf record -e cs_etm// <options> >>>>>> is used to trace a program in linux, a dummy sink appearing in the >>>>>> coresight tree with this designation may be selected. >>>>>> >>>>>> This needs to be corrected, probably with a unique sub-type that >>>>>> appears before the CORESIGHT_DEV_SUBTYPE_SINK_BUFFER value in the >>>>>> enum >>>>>> as the selection is based on >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER. >>>>>> >>>> >>>> Good point Mike. >>>> >>>>>> By implication adding a new value - will possibly affect other code >>>>>> using the enum values so will need to be checked >>>>>> >>>>>> Regards >>>>>> >>>>>> Mike >>>>>> >>>>> >>>>> Thanks for your comments, I will add a new sub-type for dummy sink and >>>>> check the impact of it. >>>> >>>> Please keep this as the lowest priority, something like: >>>> >>>> enum coresight_dev_subtype_sink { >>>> + CORESIGHT_DEV_SUBTYPE_SINK_DUMMY, >>>> CORESIGHT_DEV_SUBTYPE_SINK_PORT, >>>> CORESIGHT_DEV_SUBTYPE_SINK_BUFFER, >>>> CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM, >>>> CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM, >>>> }; >>>> >>>> This should be fine without any impact on the existing code, as we >>>> expect the driver modules to be updated with the new core module. >>>> >>>> Suzuki >>>> >>> >>> Sure, I will take your advice in the next version of patch. >>> >>> Thanks, >>> Hao >>> >>>> >>>>> >>>>> Thanks, >>>>> Hao >>>>> >>>>>> >>>>>>> + } else { >>>>>>> + dev_info(dev, "Device type not set\n"); >>>>>>> + return -EINVAL; >>>>>>> + } >>>>>>> + >>>>>>> + desc.ops = &dummy_cs_ops; >>>>>>> + desc.pdata = pdev->dev.platform_data; >>>>>>> + desc.dev = &pdev->dev; >>>>>>> + drvdata->csdev = coresight_register(&desc); >>>>>>> + if (IS_ERR(drvdata->csdev)) >>>>>>> + return PTR_ERR(drvdata->csdev); >>>>>>> + >>>>>>> + trace_id = coresight_trace_id_get_system_id(); >>>>>>> + if (trace_id < 0) { >>>>>>> + ret = trace_id; >>>>>>> + goto cs_unregister; >>>>>>> + } >>>>>>> + drvdata->traceid = (u8)trace_id; >>>>>>> + >> >> Number of issues here:- >> 1) Why are sinks being given a trace ID? - they do not need them. >> 2) how is the trace ID communicated to the underlying hardware system? >> - there appears to be no way of doing this here. Without this you >> cannot guarantee that there will not be clashes. >> Although your use case may mitigate against this - for this to be a >> generic module there must be a way to ensure the IDs can be discovered >> externally. >> 3) Trace IDs are a limited resource - most sources allocate on enable, >> release on disable / reset - this would be preferable. >> >> >> Regards >> >> Mike Good points Mike. > > 1. It should not be given a trace ID for sink, I will correct it in the > next version of patch. > 2. There are other patches to transmit the trace ID to sub-processor. > But We have an upstream dependency on QMI project. We will sync with > them for the other related patches. > 3. The trace ID of dummy source need to be communicated to the > sub-processor, it's better to be allocated on probe, that would reduce > communications costs. On the other hand, there will be few dummy > sources. I'd perfer to allocate it on probe function. Could that be delayed to dynamic allocation when the device is enabled ? Also, do we need a property for the dummy-source to "allocate" a traceID? i.e., add a "property" (not compatible) "arm,coresight-dummy-source-traceid" ? Suzuki > > Thanks, > Hao > >> >>>>>>> + pm_runtime_enable(dev); >>>>>>> + dev_info(dev, "Dummy device initialized\n"); >>>>>>> + >>>>>>> + return 0; >>>>>>> + >>>>>>> +cs_unregister: >>>>>>> + coresight_unregister(drvdata->csdev); >>>>>>> + >>>>>>> + return ret; >>>>>>> +} >>>>>>> + >>>>>>> +static int dummy_remove(struct platform_device *pdev) >>>>>>> +{ >>>>>>> + struct dummy_drvdata *drvdata = platform_get_drvdata(pdev); >>>>>>> + struct device *dev = &pdev->dev; >>>>>>> + >>>>>>> + coresight_trace_id_put_system_id(drvdata->traceid); >>>>>>> + pm_runtime_disable(dev); >>>>>>> + coresight_unregister(drvdata->csdev); >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> +static const struct of_device_id dummy_match[] = { >>>>>>> + {.compatible = "qcom,coresight-dummy"}, >>>>>>> + {}, >>>>>>> +}; >>>>>>> + >>>>>>> +static struct platform_driver dummy_driver = { >>>>>>> + .probe = dummy_probe, >>>>>>> + .remove = dummy_remove, >>>>>>> + .driver = { >>>>>>> + .name = "coresight-dummy", >>>>>>> + .of_match_table = dummy_match, >>>>>>> + }, >>>>>>> +}; >>>>>>> + >>>>>>> +static int __init dummy_init(void) >>>>>>> +{ >>>>>>> + return platform_driver_register(&dummy_driver); >>>>>>> +} >>>>>>> +module_init(dummy_init); >>>>>>> + >>>>>>> +static void __exit dummy_exit(void) >>>>>>> +{ >>>>>>> + platform_driver_unregister(&dummy_driver); >>>>>>> +} >>>>>>> +module_exit(dummy_exit); >>>>>>> + >>>>>>> +MODULE_LICENSE("GPL"); >>>>>>> +MODULE_DESCRIPTION("CoreSight dummy source driver"); >>>>>>> -- >>>>>>> 2.17.1 >>>>>>> >>>>>> >>>>>> >>>> >> >> >>

2 years, 3 months

Re: [PATCH v3 01/11] dt-bindings: arm: Add support for DSB element size

by Tao Zhang

Hi Krzysztof, On 3/25/2023 7:35 PM, Krzysztof Kozlowski wrote: > On 24/03/2023 10:15, Tao Zhang wrote: >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + minimum: 32 >>>>> + maximum: 64 >>>> Shouldn't this be something like oneOf ? It is not a range, but one of >>>> those two specific values ? >>> Yes, "qcom,dsb-element-size" should be an optional option required in >>> TPDM >>> >>> devicetree. Other properties like "qcom,cmb-element-size", >>> "qcom,tc-element-size" >>> >>> and etc. will be added in a later patch series. >>> >>> I will update this doc according to your advice in the next version of >>> the patch. >>> >>> Tao >>> >> Correct my misunderstanding in the mail above. >> >> You are right, DSB element size should be 32-bit or 64-bit. I will >> update this in the next > Then 'enum', not 'oneOf'. Got it. Tao > > Best regards, > Krzysztof >

2 years, 3 months

Re: [PATCH v2 2/3] dt-bindings: arm: Add Coresight Dummy Trace YAML schema

by Mike Leach

Hi, As per my comments in the previous patch in this set.... On Mon, 27 Mar 2023 at 08:38, Hao Zhang <quic_hazha(a)quicinc.com> wrote: > > Hi Krzysztof, > > On 3/25/2023 7:49 PM, Krzysztof Kozlowski wrote: > > On 24/03/2023 07:16, Hao Zhang wrote: > >> Add new coresight-dummy.yaml file describing the bindings required > >> to define coresight dummy trace in the device trees. > >> > > > > Subject: drop second/last, redundant "YAML schema". The "dt-bindings" > > prefix is already stating that these are bindings and all new must be DT > > schema. You cannot add anything else, so this is redundant. > > > I will take your advice to drop redundant part of title in the next > version of patch. > > > >> Signed-off-by: Hao Zhang <quic_hazha(a)quicinc.com> > >> --- > >> .../bindings/arm/qcom,coresight-dummy.yaml | 118 ++++++++++++++++++ > >> 1 file changed, 118 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml > >> new file mode 100644 > >> index 000000000000..7b719b084d72 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-dummy.yaml > >> @@ -0,0 +1,118 @@ > >> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause > >> +# Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. > >> +%YAML 1.2 > >> +--- > >> +$id: http://devicetree.org/schemas/arm/qcom,coresight-dummy.yaml# > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: QCOM Coresight Dummy component > >> + > >> +description: | > >> + The Coresight Dummy component is for the specific devices that HLOS don't have > >> + permission to access or configure. Such as Coresight sink EUD, some TPDMs etc. > >> + So there need driver to register dummy devices as Coresight devices. Provide > >> + Coresight API for dummy device operations, such as enabling and disabling > >> + dummy devices. Build the Coresight path for dummy sink or dummy source for > >> + debugging. > >> + > >> + The primary use case of the coresight dummy is to build path for dummy sink or > >> + dummy source. > >> + > >> +maintainers: > >> + - Mao Jinlong <quic_jinlmao(a)quicinc.com> > >> + - Tao Zhang <quic_taozha(a)quicinc.com> > >> + - Hao Zhang <quic_hazha(a)quicinc.com> > >> + > >> +select: > >> + properties: > >> + compatible: > >> + contains: > >> + enum: > >> + - qcom,coresight-dummy Can we have coresight-dummy-source and coresight-dummy-sink? > >> + required: > >> + - compatible > > > > Why do you need the select? > > > This is a mistake, will remove it in the next version of patch. > >> + > >> +properties: > >> + $nodename: > >> + pattern: "^dummy_.*(sink|source)_[0-9]+.*$" > > > > We do not enforce node names in individual bindings. Why do you need it? > > Plus underscore is not even proper character... > > > I will remove this node. > > >> + compatible: > >> + items: > > > > Drop items. You have only one item, so no need for list. > > I will take your advice and update it in the next version of patch. > > >> + - const: qcom,coresight-dummy > >> + > >> + qcom,dummy-sink: > >> + type: boolean > >> + description: > >> + Indicates that the type of this coresight node is dummy sink. > > > > You just duplicated property name. Write something useful. > > > >> + > >> + qcom,dummy-source: > >> + type: boolean > >> + description: > >> + Indicates that the type of this coresight node is dummy source. > > > > You just duplicated property name. Write something useful. > > > These properties not required if the compatible name is more specific > Sure, I will add more details for it. > > >> + > >> + out-ports: > >> + description: | > > > > No need for | > > > >> + Output connections from the dummy source to Coresight Trace bus. > >> + $ref: /schemas/graph.yaml#/properties/ports > >> + > >> + properties: > >> + port: > >> + description: Output connection from the dummy source to Coresight > >> + Trace bus. > >> + $ref: /schemas/graph.yaml#/properties/port > >> + > >> + in-ports: > >> + description: | > > > > Ditto > > > I will remove it in the next version of patch. > > >> + Input connections from the CoreSight Trace bus to dummy sink. > >> + $ref: /schemas/graph.yaml#/properties/ports > >> + > >> + properties: > >> + port: > >> + description: Input connection from the Coresight Trace bus to > >> + dummy sink. > >> + $ref: /schemas/graph.yaml#/properties/port > >> + > >> +required: > >> + - compatible > >> + The binding should constrain out ports to dummy-source only, and in ports to dummy sink only. Regards Mike > >> +additionalProperties: false > >> + > >> +oneOf: > >> + - required: > >> + - qcom,dummy-sink > >> + - required: > >> + - qcom,dummy-source > >> + > >> +examples: > >> + # minimum dummy sink definition. dummy sink connect to coresight replicator. > >> + - | > >> + dummy_sink_1 { > > > > Node names should be generic, so "sink" > > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetr… > > > >> + compatible = "qcom,coresight-dummy"; > >> + qcom,dummy-sink; > >> + > >> + in-ports { > >> + port { > >> + eud_in_replicator_swao: endpoint { > >> + remote-endpoint = > >> + <&replicator_swao_out_eud>; > > > > Why line break after =? > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + # minimum dummy source definition. dummy source connect to coresight funnel. > > > > If you use sentences, then start with capital letter. > > > > I will update it according to your advice in the next version of patch. > > >> + - | > >> + dummy_source_1 { > >> + compatible = "qcom,coresight-dummy"; > >> + qcom,dummy-source; > >> + > >> + out-ports { > >> + port { > >> + dummy_riscv_out_funnel_swao: endpoint { > >> + remote-endpoint = > >> + <&funnel_swao_in_dummy_riscv>; > > > > Why line break? > > I copy it from device tree and keep the original format, will correct > the format in the next version of patch. > > Thanks, > Hao > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> +... > > > > Best regards, > > Krzysztof > > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

2 years, 3 months

Re: [PATCH v2 1/3] Coresight: Add coresight dummy driver

by Mike Leach

Hi, A few additional comments.... On Tue, 28 Mar 2023 at 10:24, Hao Zhang <quic_hazha(a)quicinc.com> wrote: > > Hi Suzuki, > > On 3/28/2023 4:35 PM, Suzuki K Poulose wrote: > > On 28/03/2023 08:22, Hao Zhang wrote: > >> Hi Mike, > >> > >> On 3/27/2023 11:58 PM, Mike Leach wrote: > >>> Hi, > >>> > >>> On Fri, 24 Mar 2023 at 06:16, Hao Zhang <quic_hazha(a)quicinc.com> wrote: > >>>> > >>>> Some Coresight devices that HLOS don't have permission to access > >>>> or configure. Such as Coresight sink EUD, some TPDMs etc. So there > >>>> need driver to register dummy devices as Coresight devices. Provide > >>>> Coresight API for dummy device operations, such as enabling and > >>>> disabling dummy devices. Build the Coresight path for dummy sink or > >>>> dummy source for debugging. > >>>> > >>>> Signed-off-by: Hao Zhang <quic_hazha(a)quicinc.com> > >>>> --- > >>>> drivers/hwtracing/coresight/Kconfig | 11 ++ > >>>> drivers/hwtracing/coresight/Makefile | 1 + > >>>> drivers/hwtracing/coresight/coresight-dummy.c | 176 > >>>> ++++++++++++++++++ > >>>> 3 files changed, 188 insertions(+) > >>>> create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c > >>>> > >>>> diff --git a/drivers/hwtracing/coresight/Kconfig > >>>> b/drivers/hwtracing/coresight/Kconfig > >>>> index 2b5bbfffbc4f..06f0a7594169 100644 > >>>> --- a/drivers/hwtracing/coresight/Kconfig > >>>> +++ b/drivers/hwtracing/coresight/Kconfig > >>>> @@ -236,4 +236,15 @@ config CORESIGHT_TPDA > >>>> > >>>> To compile this driver as a module, choose M here: the > >>>> module will be > >>>> called coresight-tpda. > >>>> + > >>>> +config CORESIGHT_DUMMY > >>>> + tristate "Dummy driver support" > >>>> + help > >>>> + Enables support for dummy driver. Dummy driver can be used > >>>> for > >>>> + CoreSight sources/sinks that are owned and configured by some > >>>> + other subsystem and use Linux drivers to configure rest of > >>>> trace > >>>> + path. > >>>> + > >>>> + To compile this driver as a module, choose M here: the > >>>> module will be > >>>> + called coresight-dummy. > >>>> endif > >>>> diff --git a/drivers/hwtracing/coresight/Makefile > >>>> b/drivers/hwtracing/coresight/Makefile > >>>> index 33bcc3f7b8ae..995d3b2c76df 100644 > >>>> --- a/drivers/hwtracing/coresight/Makefile > >>>> +++ b/drivers/hwtracing/coresight/Makefile > >>>> @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o > >>>> 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 > >>>> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c > >>>> b/drivers/hwtracing/coresight/coresight-dummy.c > >>>> new file mode 100644 > >>>> index 000000000000..2d4eb3e546eb > >>>> --- /dev/null > >>>> +++ b/drivers/hwtracing/coresight/coresight-dummy.c > >>>> @@ -0,0 +1,176 @@ > >>>> +// SPDX-License-Identifier: GPL-2.0 > >>>> +/* > >>>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights > >>>> reserved. > >>>> + */ > >>>> + > >>>> +#include <linux/kernel.h> > >>>> +#include <linux/module.h> > >>>> +#include <linux/platform_device.h> > >>>> +#include <linux/coresight.h> > >>>> +#include <linux/of.h> > >>>> +#include <linux/pm_runtime.h> > >>>> + > >>>> +#include "coresight-priv.h" > >>>> +#include "coresight-trace-id.h" > >>>> + > >>>> +struct dummy_drvdata { > >>>> + struct device *dev; > >>>> + struct coresight_device *csdev; > >>>> + int traceid; > >>>> +}; > >>>> + > >>>> +DEFINE_CORESIGHT_DEVLIST(dummy_devs, "dummy"); > >>>> + minor nit: can we have dummy_source and dummy_sink as the device names to make it clear at the first level what these are without having to look at the attributes? > >>>> +static int dummy_source_enable(struct coresight_device *csdev, > >>>> + struct perf_event *event, u32 mode) > >>>> +{ > >>>> + struct dummy_drvdata *drvdata = > >>>> dev_get_drvdata(csdev->dev.parent); > >>>> + > >>>> + dev_info(drvdata->dev, "Dummy source enabled\n"); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +static void dummy_source_disable(struct coresight_device *csdev, > >>>> + struct perf_event *event) > >>>> +{ > >>>> + struct dummy_drvdata *drvdata = > >>>> dev_get_drvdata(csdev->dev.parent); > >>>> + > >>>> + dev_info(drvdata->dev, "Dummy source disabled\n"); > >>>> +} > >>>> + > >>>> +static int dummy_sink_enable(struct coresight_device *csdev, u32 mode, > >>>> + void *data) > >>>> +{ > >>>> + struct dummy_drvdata *drvdata = > >>>> dev_get_drvdata(csdev->dev.parent); > >>>> + > >>>> + dev_info(drvdata->dev, "Dummy sink enabled\n"); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +static int dummy_sink_disable(struct coresight_device *csdev) > >>>> +{ > >>>> + struct dummy_drvdata *drvdata = > >>>> dev_get_drvdata(csdev->dev.parent); > >>>> + > >>>> + dev_info(drvdata->dev, "Dummy sink disabled\n"); > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +static const struct coresight_ops_source dummy_source_ops = { > >>>> + .enable = dummy_source_enable, > >>>> + .disable = dummy_source_disable, > >>>> +}; > >>>> + > >>>> +static const struct coresight_ops_sink dummy_sink_ops = { > >>>> + .enable = dummy_sink_enable, > >>>> + .disable = dummy_sink_disable, > >>>> +}; > >>>> + > >>>> +static const struct coresight_ops dummy_cs_ops = { > >>>> + .source_ops = &dummy_source_ops, > >>>> + .sink_ops = &dummy_sink_ops, > >>>> +}; > >>>> + > >>>> +static int dummy_probe(struct platform_device *pdev) > >>>> +{ > >>>> + int ret, trace_id; > >>>> + struct device *dev = &pdev->dev; > >>>> + struct coresight_platform_data *pdata; > >>>> + struct dummy_drvdata *drvdata; > >>>> + struct coresight_desc desc = { 0 }; > >>>> + > >>>> + desc.name = coresight_alloc_device_name(&dummy_devs, dev); > >>>> + if (!desc.name) > >>>> + return -ENOMEM; > >>>> + > >>>> + pdata = coresight_get_platform_data(dev); > >>>> + if (IS_ERR(pdata)) > >>>> + return PTR_ERR(pdata); > >>>> + pdev->dev.platform_data = pdata; > >>>> + > >>>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > >>>> + if (!drvdata) > >>>> + return -ENOMEM; > >>>> + > >>>> + drvdata->dev = &pdev->dev; > >>>> + platform_set_drvdata(pdev, drvdata); > >>>> + > >>>> + if (of_property_read_bool(pdev->dev.of_node, > >>>> "qcom,dummy-source")) { > >>>> + desc.type = CORESIGHT_DEV_TYPE_SOURCE; > >>>> + desc.subtype.source_subtype = > >>>> + CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS; > >>>> + } else if (of_property_read_bool(pdev->dev.of_node, > >>>> + "qcom,dummy-sink")) { It would simplify things if the compatibles were arm,coresight-dummy-source and arm,coresight-dummy-sink - and drop the two additional attributes, using of_device_is_compatible() here. > >>>> + desc.type = CORESIGHT_DEV_TYPE_SINK; > >>>> + desc.subtype.sink_subtype = > >>>> CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; > >>> > >>> This will break the automatic sink selection on a system where perf is > >>> looking for a default sink and the dummy sink is closest / first > >>> discovered. > >>> > >>> i.e. when perf record -e cs_etm// <options> > >>> is used to trace a program in linux, a dummy sink appearing in the > >>> coresight tree with this designation may be selected. > >>> > >>> This needs to be corrected, probably with a unique sub-type that > >>> appears before the CORESIGHT_DEV_SUBTYPE_SINK_BUFFER value in the enum > >>> as the selection is based on >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER. > >>> > > > > Good point Mike. > > > >>> By implication adding a new value - will possibly affect other code > >>> using the enum values so will need to be checked > >>> > >>> Regards > >>> > >>> Mike > >>> > >> > >> Thanks for your comments, I will add a new sub-type for dummy sink and > >> check the impact of it. > > > > Please keep this as the lowest priority, something like: > > > > enum coresight_dev_subtype_sink { > > + CORESIGHT_DEV_SUBTYPE_SINK_DUMMY, > > CORESIGHT_DEV_SUBTYPE_SINK_PORT, > > CORESIGHT_DEV_SUBTYPE_SINK_BUFFER, > > CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM, > > CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM, > > }; > > > > This should be fine without any impact on the existing code, as we > > expect the driver modules to be updated with the new core module. > > > > Suzuki > > > > Sure, I will take your advice in the next version of patch. > > Thanks, > Hao > > > > >> > >> Thanks, > >> Hao > >> > >>> > >>>> + } else { > >>>> + dev_info(dev, "Device type not set\n"); > >>>> + return -EINVAL; > >>>> + } > >>>> + > >>>> + desc.ops = &dummy_cs_ops; > >>>> + desc.pdata = pdev->dev.platform_data; > >>>> + desc.dev = &pdev->dev; > >>>> + drvdata->csdev = coresight_register(&desc); > >>>> + if (IS_ERR(drvdata->csdev)) > >>>> + return PTR_ERR(drvdata->csdev); > >>>> + > >>>> + trace_id = coresight_trace_id_get_system_id(); > >>>> + if (trace_id < 0) { > >>>> + ret = trace_id; > >>>> + goto cs_unregister; > >>>> + } > >>>> + drvdata->traceid = (u8)trace_id; > >>>> + Number of issues here:- 1) Why are sinks being given a trace ID? - they do not need them. 2) how is the trace ID communicated to the underlying hardware system? - there appears to be no way of doing this here. Without this you cannot guarantee that there will not be clashes. Although your use case may mitigate against this - for this to be a generic module there must be a way to ensure the IDs can be discovered externally. 3) Trace IDs are a limited resource - most sources allocate on enable, release on disable / reset - this would be preferable. Regards Mike > >>>> + pm_runtime_enable(dev); > >>>> + dev_info(dev, "Dummy device initialized\n"); > >>>> + > >>>> + return 0; > >>>> + > >>>> +cs_unregister: > >>>> + coresight_unregister(drvdata->csdev); > >>>> + > >>>> + return ret; > >>>> +} > >>>> + > >>>> +static int dummy_remove(struct platform_device *pdev) > >>>> +{ > >>>> + struct dummy_drvdata *drvdata = platform_get_drvdata(pdev); > >>>> + struct device *dev = &pdev->dev; > >>>> + > >>>> + coresight_trace_id_put_system_id(drvdata->traceid); > >>>> + pm_runtime_disable(dev); > >>>> + coresight_unregister(drvdata->csdev); > >>>> + return 0; > >>>> +} > >>>> + > >>>> +static const struct of_device_id dummy_match[] = { > >>>> + {.compatible = "qcom,coresight-dummy"}, > >>>> + {}, > >>>> +}; > >>>> + > >>>> +static struct platform_driver dummy_driver = { > >>>> + .probe = dummy_probe, > >>>> + .remove = dummy_remove, > >>>> + .driver = { > >>>> + .name = "coresight-dummy", > >>>> + .of_match_table = dummy_match, > >>>> + }, > >>>> +}; > >>>> + > >>>> +static int __init dummy_init(void) > >>>> +{ > >>>> + return platform_driver_register(&dummy_driver); > >>>> +} > >>>> +module_init(dummy_init); > >>>> + > >>>> +static void __exit dummy_exit(void) > >>>> +{ > >>>> + platform_driver_unregister(&dummy_driver); > >>>> +} > >>>> +module_exit(dummy_exit); > >>>> + > >>>> +MODULE_LICENSE("GPL"); > >>>> +MODULE_DESCRIPTION("CoreSight dummy source driver"); > >>>> -- > >>>> 2.17.1 > >>>> > >>> > >>> > > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

2 years, 3 months

Re: [BUG] perf: No samples found when using kcore + coresight

by James Clark

On 28/03/2023 01:53, Yang Shi wrote: > Hi Leo, > > Just follow up on this one. Any update? > Hi Yang, Sorry no update on this yet from me. I was just finishing off "coresight: Fix CTI module refcount leak by making it a helper device" which I hope to post in the next day or two and then I will start on this. James > Thanks, > Yang > > On Mon, Mar 13, 2023 at 5:36 PM Leo Yan <leo.yan(a)linaro.org> wrote: >> >> On Mon, Mar 13, 2023 at 11:15:44AM -0700, Yang Shi wrote: >> >> [...] >> >>>> Just a quick summary, here we have two issues: >>>> >>>> - With command: >>>> perf record -e cs_etm/@tmc_etf63/k --kcore --per-thread \ >>>> -- taskset --cpu-list 1 uname", >>>> >>>> perf doesn't enable "text poke" attribution. >>> >>> No, it enables "text poke" and perf fails to decode coresight trace >>> data too. It doesn't matter whether "--kcore" is after or before "-e >>> cs/etm/@tmc_etf63/k". >> >> Understand now. Thanks for correction, if so we can ignore this one. >> >> Leo

2 years, 3 months

Re: [PATCH v2 1/3] Coresight: Add coresight dummy driver

by Suzuki K Poulose

On 28/03/2023 08:22, Hao Zhang wrote: > Hi Mike, > > On 3/27/2023 11:58 PM, Mike Leach wrote: >> Hi, >> >> On Fri, 24 Mar 2023 at 06:16, Hao Zhang <quic_hazha(a)quicinc.com> wrote: >>> >>> Some Coresight devices that HLOS don't have permission to access >>> or configure. Such as Coresight sink EUD, some TPDMs etc. So there >>> need driver to register dummy devices as Coresight devices. Provide >>> Coresight API for dummy device operations, such as enabling and >>> disabling dummy devices. Build the Coresight path for dummy sink or >>> dummy source for debugging. >>> >>> Signed-off-by: Hao Zhang <quic_hazha(a)quicinc.com> >>> --- >>> drivers/hwtracing/coresight/Kconfig | 11 ++ >>> drivers/hwtracing/coresight/Makefile | 1 + >>> drivers/hwtracing/coresight/coresight-dummy.c | 176 ++++++++++++++++++ >>> 3 files changed, 188 insertions(+) >>> create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c >>> >>> diff --git a/drivers/hwtracing/coresight/Kconfig >>> b/drivers/hwtracing/coresight/Kconfig >>> index 2b5bbfffbc4f..06f0a7594169 100644 >>> --- a/drivers/hwtracing/coresight/Kconfig >>> +++ b/drivers/hwtracing/coresight/Kconfig >>> @@ -236,4 +236,15 @@ config CORESIGHT_TPDA >>> >>> To compile this driver as a module, choose M here: the >>> module will be >>> called coresight-tpda. >>> + >>> +config CORESIGHT_DUMMY >>> + tristate "Dummy driver support" >>> + help >>> + Enables support for dummy driver. Dummy driver can be used for >>> + CoreSight sources/sinks that are owned and configured by some >>> + other subsystem and use Linux drivers to configure rest of >>> trace >>> + path. >>> + >>> + To compile this driver as a module, choose M here: the >>> module will be >>> + called coresight-dummy. >>> endif >>> diff --git a/drivers/hwtracing/coresight/Makefile >>> b/drivers/hwtracing/coresight/Makefile >>> index 33bcc3f7b8ae..995d3b2c76df 100644 >>> --- a/drivers/hwtracing/coresight/Makefile >>> +++ b/drivers/hwtracing/coresight/Makefile >>> @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o >>> 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 >>> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c >>> b/drivers/hwtracing/coresight/coresight-dummy.c >>> new file mode 100644 >>> index 000000000000..2d4eb3e546eb >>> --- /dev/null >>> +++ b/drivers/hwtracing/coresight/coresight-dummy.c >>> @@ -0,0 +1,176 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights >>> reserved. >>> + */ >>> + >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/coresight.h> >>> +#include <linux/of.h> >>> +#include <linux/pm_runtime.h> >>> + >>> +#include "coresight-priv.h" >>> +#include "coresight-trace-id.h" >>> + >>> +struct dummy_drvdata { >>> + struct device *dev; >>> + struct coresight_device *csdev; >>> + int traceid; >>> +}; >>> + >>> +DEFINE_CORESIGHT_DEVLIST(dummy_devs, "dummy"); >>> + >>> +static int dummy_source_enable(struct coresight_device *csdev, >>> + struct perf_event *event, u32 mode) >>> +{ >>> + struct dummy_drvdata *drvdata = >>> dev_get_drvdata(csdev->dev.parent); >>> + >>> + dev_info(drvdata->dev, "Dummy source enabled\n"); >>> + >>> + return 0; >>> +} >>> + >>> +static void dummy_source_disable(struct coresight_device *csdev, >>> + struct perf_event *event) >>> +{ >>> + struct dummy_drvdata *drvdata = >>> dev_get_drvdata(csdev->dev.parent); >>> + >>> + dev_info(drvdata->dev, "Dummy source disabled\n"); >>> +} >>> + >>> +static int dummy_sink_enable(struct coresight_device *csdev, u32 mode, >>> + void *data) >>> +{ >>> + struct dummy_drvdata *drvdata = >>> dev_get_drvdata(csdev->dev.parent); >>> + >>> + dev_info(drvdata->dev, "Dummy sink enabled\n"); >>> + >>> + return 0; >>> +} >>> + >>> +static int dummy_sink_disable(struct coresight_device *csdev) >>> +{ >>> + struct dummy_drvdata *drvdata = >>> dev_get_drvdata(csdev->dev.parent); >>> + >>> + dev_info(drvdata->dev, "Dummy sink disabled\n"); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct coresight_ops_source dummy_source_ops = { >>> + .enable = dummy_source_enable, >>> + .disable = dummy_source_disable, >>> +}; >>> + >>> +static const struct coresight_ops_sink dummy_sink_ops = { >>> + .enable = dummy_sink_enable, >>> + .disable = dummy_sink_disable, >>> +}; >>> + >>> +static const struct coresight_ops dummy_cs_ops = { >>> + .source_ops = &dummy_source_ops, >>> + .sink_ops = &dummy_sink_ops, >>> +}; >>> + >>> +static int dummy_probe(struct platform_device *pdev) >>> +{ >>> + int ret, trace_id; >>> + struct device *dev = &pdev->dev; >>> + struct coresight_platform_data *pdata; >>> + struct dummy_drvdata *drvdata; >>> + struct coresight_desc desc = { 0 }; >>> + >>> + desc.name = coresight_alloc_device_name(&dummy_devs, dev); >>> + if (!desc.name) >>> + return -ENOMEM; >>> + >>> + pdata = coresight_get_platform_data(dev); >>> + if (IS_ERR(pdata)) >>> + return PTR_ERR(pdata); >>> + pdev->dev.platform_data = pdata; >>> + >>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >>> + if (!drvdata) >>> + return -ENOMEM; >>> + >>> + drvdata->dev = &pdev->dev; >>> + platform_set_drvdata(pdev, drvdata); >>> + >>> + if (of_property_read_bool(pdev->dev.of_node, >>> "qcom,dummy-source")) { >>> + desc.type = CORESIGHT_DEV_TYPE_SOURCE; >>> + desc.subtype.source_subtype = >>> + >>> CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS; >>> + } else if (of_property_read_bool(pdev->dev.of_node, >>> + "qcom,dummy-sink")) { >>> + desc.type = CORESIGHT_DEV_TYPE_SINK; >>> + desc.subtype.sink_subtype = >>> CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; >> >> This will break the automatic sink selection on a system where perf is >> looking for a default sink and the dummy sink is closest / first >> discovered. >> >> i.e. when perf record -e cs_etm// <options> >> is used to trace a program in linux, a dummy sink appearing in the >> coresight tree with this designation may be selected. >> >> This needs to be corrected, probably with a unique sub-type that >> appears before the CORESIGHT_DEV_SUBTYPE_SINK_BUFFER value in the enum >> as the selection is based on >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER. >> Good point Mike. >> By implication adding a new value - will possibly affect other code >> using the enum values so will need to be checked >> >> Regards >> >> Mike >> > > Thanks for your comments, I will add a new sub-type for dummy sink and > check the impact of it. Please keep this as the lowest priority, something like: enum coresight_dev_subtype_sink { + CORESIGHT_DEV_SUBTYPE_SINK_DUMMY, CORESIGHT_DEV_SUBTYPE_SINK_PORT, CORESIGHT_DEV_SUBTYPE_SINK_BUFFER, CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM, CORESIGHT_DEV_SUBTYPE_SINK_PERCPU_SYSMEM, }; This should be fine without any impact on the existing code, as we expect the driver modules to be updated with the new core module. Suzuki > > Thanks, > Hao > >> >>> + } else { >>> + dev_info(dev, "Device type not set\n"); >>> + return -EINVAL; >>> + } >>> + >>> + desc.ops = &dummy_cs_ops; >>> + desc.pdata = pdev->dev.platform_data; >>> + desc.dev = &pdev->dev; >>> + drvdata->csdev = coresight_register(&desc); >>> + if (IS_ERR(drvdata->csdev)) >>> + return PTR_ERR(drvdata->csdev); >>> + >>> + trace_id = coresight_trace_id_get_system_id(); >>> + if (trace_id < 0) { >>> + ret = trace_id; >>> + goto cs_unregister; >>> + } >>> + drvdata->traceid = (u8)trace_id; >>> + >>> + pm_runtime_enable(dev); >>> + dev_info(dev, "Dummy device initialized\n"); >>> + >>> + return 0; >>> + >>> +cs_unregister: >>> + coresight_unregister(drvdata->csdev); >>> + >>> + return ret; >>> +} >>> + >>> +static int dummy_remove(struct platform_device *pdev) >>> +{ >>> + struct dummy_drvdata *drvdata = platform_get_drvdata(pdev); >>> + struct device *dev = &pdev->dev; >>> + >>> + coresight_trace_id_put_system_id(drvdata->traceid); >>> + pm_runtime_disable(dev); >>> + coresight_unregister(drvdata->csdev); >>> + return 0; >>> +} >>> + >>> +static const struct of_device_id dummy_match[] = { >>> + {.compatible = "qcom,coresight-dummy"}, >>> + {}, >>> +}; >>> + >>> +static struct platform_driver dummy_driver = { >>> + .probe = dummy_probe, >>> + .remove = dummy_remove, >>> + .driver = { >>> + .name = "coresight-dummy", >>> + .of_match_table = dummy_match, >>> + }, >>> +}; >>> + >>> +static int __init dummy_init(void) >>> +{ >>> + return platform_driver_register(&dummy_driver); >>> +} >>> +module_init(dummy_init); >>> + >>> +static void __exit dummy_exit(void) >>> +{ >>> + platform_driver_unregister(&dummy_driver); >>> +} >>> +module_exit(dummy_exit); >>> + >>> +MODULE_LICENSE("GPL"); >>> +MODULE_DESCRIPTION("CoreSight dummy source driver"); >>> -- >>> 2.17.1 >>> >> >>

2 years, 3 months

Re: [PATCH v2 1/3] Coresight: Add coresight dummy driver

by Mike Leach

Hi, On Fri, 24 Mar 2023 at 06:16, Hao Zhang <quic_hazha(a)quicinc.com> wrote: > > Some Coresight devices that HLOS don't have permission to access > or configure. Such as Coresight sink EUD, some TPDMs etc. So there > need driver to register dummy devices as Coresight devices. Provide > Coresight API for dummy device operations, such as enabling and > disabling dummy devices. Build the Coresight path for dummy sink or > dummy source for debugging. > > Signed-off-by: Hao Zhang <quic_hazha(a)quicinc.com> > --- > drivers/hwtracing/coresight/Kconfig | 11 ++ > drivers/hwtracing/coresight/Makefile | 1 + > drivers/hwtracing/coresight/coresight-dummy.c | 176 ++++++++++++++++++ > 3 files changed, 188 insertions(+) > create mode 100644 drivers/hwtracing/coresight/coresight-dummy.c > > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig > index 2b5bbfffbc4f..06f0a7594169 100644 > --- a/drivers/hwtracing/coresight/Kconfig > +++ b/drivers/hwtracing/coresight/Kconfig > @@ -236,4 +236,15 @@ config CORESIGHT_TPDA > > To compile this driver as a module, choose M here: the module will be > called coresight-tpda. > + > +config CORESIGHT_DUMMY > + tristate "Dummy driver support" > + help > + Enables support for dummy driver. Dummy driver can be used for > + CoreSight sources/sinks that are owned and configured by some > + other subsystem and use Linux drivers to configure rest of trace > + path. > + > + To compile this driver as a module, choose M here: the module will be > + called coresight-dummy. > endif > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > index 33bcc3f7b8ae..995d3b2c76df 100644 > --- a/drivers/hwtracing/coresight/Makefile > +++ b/drivers/hwtracing/coresight/Makefile > @@ -30,3 +30,4 @@ obj-$(CONFIG_CORESIGHT_TPDA) += coresight-tpda.o > 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 > diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c > new file mode 100644 > index 000000000000..2d4eb3e546eb > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-dummy.c > @@ -0,0 +1,176 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/coresight.h> > +#include <linux/of.h> > +#include <linux/pm_runtime.h> > + > +#include "coresight-priv.h" > +#include "coresight-trace-id.h" > + > +struct dummy_drvdata { > + struct device *dev; > + struct coresight_device *csdev; > + int traceid; > +}; > + > +DEFINE_CORESIGHT_DEVLIST(dummy_devs, "dummy"); > + > +static int dummy_source_enable(struct coresight_device *csdev, > + struct perf_event *event, u32 mode) > +{ > + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + dev_info(drvdata->dev, "Dummy source enabled\n"); > + > + return 0; > +} > + > +static void dummy_source_disable(struct coresight_device *csdev, > + struct perf_event *event) > +{ > + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + dev_info(drvdata->dev, "Dummy source disabled\n"); > +} > + > +static int dummy_sink_enable(struct coresight_device *csdev, u32 mode, > + void *data) > +{ > + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + dev_info(drvdata->dev, "Dummy sink enabled\n"); > + > + return 0; > +} > + > +static int dummy_sink_disable(struct coresight_device *csdev) > +{ > + struct dummy_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + > + dev_info(drvdata->dev, "Dummy sink disabled\n"); > + > + return 0; > +} > + > +static const struct coresight_ops_source dummy_source_ops = { > + .enable = dummy_source_enable, > + .disable = dummy_source_disable, > +}; > + > +static const struct coresight_ops_sink dummy_sink_ops = { > + .enable = dummy_sink_enable, > + .disable = dummy_sink_disable, > +}; > + > +static const struct coresight_ops dummy_cs_ops = { > + .source_ops = &dummy_source_ops, > + .sink_ops = &dummy_sink_ops, > +}; > + > +static int dummy_probe(struct platform_device *pdev) > +{ > + int ret, trace_id; > + struct device *dev = &pdev->dev; > + struct coresight_platform_data *pdata; > + struct dummy_drvdata *drvdata; > + struct coresight_desc desc = { 0 }; > + > + desc.name = coresight_alloc_device_name(&dummy_devs, dev); > + if (!desc.name) > + return -ENOMEM; > + > + pdata = coresight_get_platform_data(dev); > + if (IS_ERR(pdata)) > + return PTR_ERR(pdata); > + pdev->dev.platform_data = pdata; > + > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > + if (!drvdata) > + return -ENOMEM; > + > + drvdata->dev = &pdev->dev; > + platform_set_drvdata(pdev, drvdata); > + > + if (of_property_read_bool(pdev->dev.of_node, "qcom,dummy-source")) { > + desc.type = CORESIGHT_DEV_TYPE_SOURCE; > + desc.subtype.source_subtype = > + CORESIGHT_DEV_SUBTYPE_SOURCE_OTHERS; > + } else if (of_property_read_bool(pdev->dev.of_node, > + "qcom,dummy-sink")) { > + desc.type = CORESIGHT_DEV_TYPE_SINK; > + desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; This will break the automatic sink selection on a system where perf is looking for a default sink and the dummy sink is closest / first discovered. i.e. when perf record -e cs_etm// <options> is used to trace a program in linux, a dummy sink appearing in the coresight tree with this designation may be selected. This needs to be corrected, probably with a unique sub-type that appears before the CORESIGHT_DEV_SUBTYPE_SINK_BUFFER value in the enum as the selection is based on >= CORESIGHT_DEV_SUBTYPE_SINK_BUFFER. By implication adding a new value - will possibly affect other code using the enum values so will need to be checked Regards Mike > + } else { > + dev_info(dev, "Device type not set\n"); > + return -EINVAL; > + } > + > + desc.ops = &dummy_cs_ops; > + desc.pdata = pdev->dev.platform_data; > + desc.dev = &pdev->dev; > + drvdata->csdev = coresight_register(&desc); > + if (IS_ERR(drvdata->csdev)) > + return PTR_ERR(drvdata->csdev); > + > + trace_id = coresight_trace_id_get_system_id(); > + if (trace_id < 0) { > + ret = trace_id; > + goto cs_unregister; > + } > + drvdata->traceid = (u8)trace_id; > + > + pm_runtime_enable(dev); > + dev_info(dev, "Dummy device initialized\n"); > + > + return 0; > + > +cs_unregister: > + coresight_unregister(drvdata->csdev); > + > + return ret; > +} > + > +static int dummy_remove(struct platform_device *pdev) > +{ > + struct dummy_drvdata *drvdata = platform_get_drvdata(pdev); > + struct device *dev = &pdev->dev; > + > + coresight_trace_id_put_system_id(drvdata->traceid); > + pm_runtime_disable(dev); > + coresight_unregister(drvdata->csdev); > + return 0; > +} > + > +static const struct of_device_id dummy_match[] = { > + {.compatible = "qcom,coresight-dummy"}, > + {}, > +}; > + > +static struct platform_driver dummy_driver = { > + .probe = dummy_probe, > + .remove = dummy_remove, > + .driver = { > + .name = "coresight-dummy", > + .of_match_table = dummy_match, > + }, > +}; > + > +static int __init dummy_init(void) > +{ > + return platform_driver_register(&dummy_driver); > +} > +module_init(dummy_init); > + > +static void __exit dummy_exit(void) > +{ > + platform_driver_unregister(&dummy_driver); > +} > +module_exit(dummy_exit); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("CoreSight dummy source driver"); > -- > 2.17.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

2 years, 3 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight