CoreSight

coresight@lists.linaro.org

6 participants
2351 discussions

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, 1 month

[PATCH] coresight: cti: Check if the CPU activated for the CPU CTI

by Tao Zhang

Check whether the CPU corresponding to the CPU CTI is activated. If it is not activated, the CPU CTI node should not exist, and an error will be returned in the initialization function. Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> --- drivers/hwtracing/coresight/coresight-cti-core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 277c890..aaa83ae 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -899,10 +899,12 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) drvdata->config.hw_powered = true; /* set up device name - will depend if cpu bound or otherwise */ - if (drvdata->ctidev.cpu >= 0) + if (drvdata->ctidev.cpu >= 0) { + if (!cpu_active(drvdata->ctidev.cpu)) + return -ENXIO; cti_desc.name = devm_kasprintf(dev, GFP_KERNEL, "cti_cpu%d", drvdata->ctidev.cpu); - else + } else cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, dev); if (!cti_desc.name) return -ENOMEM; -- 2.7.4

2 years, 1 month

Re: [PATCH v2 3/3] Documentation: trace: Add documentation for Coresight Dummy Trace

by Suzuki K Poulose

On 24/03/2023 06:16, Hao Zhang wrote: > Add documentation for Coresight Dummy Trace under trace/coresight. > > Signed-off-by: Hao Zhang <quic_hazha(a)quicinc.com> > --- > .../trace/coresight/coresight-dummy.rst | 58 +++++++++++++++++++ > 1 file changed, 58 insertions(+) > create mode 100644 Documentation/trace/coresight/coresight-dummy.rst > > diff --git a/Documentation/trace/coresight/coresight-dummy.rst b/Documentation/trace/coresight/coresight-dummy.rst > new file mode 100644 > index 000000000000..819cabab8623 > --- /dev/null > +++ b/Documentation/trace/coresight/coresight-dummy.rst > @@ -0,0 +1,58 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +============================= > +Coresight Dummy Trace Module > +============================= > + > + :Author: Hao Zhang <quic_hazha(a)quicinc.com> > + :Date: March 2023 > + > +Introduction > +--------------------------- > + > +Coresight Dummy Trace Module is for the specific devices that HLOS don't Please do not use cryptic abbreviations, please use "kernel" > +have permission to access or configure. Such as Coresight sink EUD, some > +TPDMs etc. Say "e.g., CoreSight TPDMs on Qualcomm platforms.: So there need driver to register dummy devices as Coresight > +devices. Add: "It may also be used to define components that may not have any programming interfaces (e.g, static links), so that paths can be established in the driver. " 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. I think the following content may not be needed as they are part of the standard source/sink type devices, nothing specific to dummy devices. --- vvvvv --- > + > +Sysfs files and directories > +--------------------------- > + > +Root: ``/sys/bus/coresight/devices/dummy<N>`` > + > +---- > + > +:File: ``enable_source`` (RW) > +:Notes: > + - > 0 : enable the datasets of dummy source. > + > + - = 0 : disable the datasets of dummy source. > + > +:Syntax: > + ``echo 1 > enable_source`` > + > +---- > + > +:File: ``enable_sink`` (RW) > +:Notes: > + - > 0 : enable the datasets of dummy sink. > + > + - = 0 : disable the datasets of dummy sink. > + > +:Syntax: > + ``echo 1 > enable_sink`` > + > +---- > + --- You may remove the above ^^^ ---- > +Config details > +--------------------------- > + > +There are two types of nodes, dummy sink and dummy source. The nodes > +should be observed at the coresight path > +"/sys/bus/coresight/devices". > +e.g. > +/sys/bus/coresight/devices # ls -l | grep dummy > +dummy0 -> ../../../devices/platform/soc@0/soc@0:dummy_source/dummy0 > +dummy1 -> ../../../devices/platform/soc@0/soc@0:dummy_sink/dummy1 Suzuki

2 years, 1 month

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

by Suzuki K Poulose

On 24/03/2023 06:16, Hao Zhang wrote: > Add new coresight-dummy.yaml file describing the bindings required > to define coresight dummy trace in the device trees. > > 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 As mentioned in the previous email, please make this Arm CoreSight. This is not specific to Qcom, rather something the CoreSight driver exposes as a dummy framework. Otherwise looks good to me. Suzuki

2 years, 1 month

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

by Suzuki K Poulose

On 24/03/2023 06:16, Hao Zhang 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"); Please use dev_dbg everywher. > + > + 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")) { I don't see any reason why this should be qcom,... Please could we use : "arm,coresight-", everywhere including the "dt" compatible ? > + 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; > + } 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"}, As mentioned above, "arm,coresight-dummy-device" ? This has nothing to do with qcom IP. qcom has a use for this. So, I would like to keep this "coresight" subsystem specific compatibles. May be we could even add other types too : i.e, arm,coresight-dummy-link-split, arm,coresight-dummy-link-merge Suzuki > + {}, > +}; > + > +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 years, 1 month

[GIT PULL] coresight: Fixes for v6.3

by Suzuki K Poulose

Greg, Please find a couple of fixes for coresight self-hosted tracing for v6.3. Kindly consider pulling. Suzuki The following changes since commit eeac8ede17557680855031c6f305ece2378af326: Linux 6.3-rc2 (2023-03-12 16:36:44 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git tags/coresight-fixes-v6.3 for you to fetch changes up to 735e7b30a53a1679c050cddb73f5e5316105d2e3: coresight: etm4x: Do not access TRCIDR1 for identification (2023-03-21 12:31:02 +0000) ---------------------------------------------------------------- coresight: Fixes for v6.3 Fixes for coresight subsystem includes: - Fix etm4_enable_hw to program all the address comparator pairs (instead of half of them) - Do not access TRCIDR1 register without OSLK cleared in etm4_probe for mmio access. Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> ---------------------------------------------------------------- Steve Clevenger (1): coresight-etm4: Fix for() loop drvdata->nr_addr_cmp range bug Suzuki K Poulose (1): coresight: etm4x: Do not access TRCIDR1 for identification drivers/hwtracing/coresight/coresight-etm4x-core.c | 24 +++++++++------------- drivers/hwtracing/coresight/coresight-etm4x.h | 20 ++++++------------ 2 files changed, 16 insertions(+), 28 deletions(-)

2 years, 1 month

Re: [PATCH 0/7] coresight: etm4x: Migrate AMBA devices to platform driver

by Suzuki K Poulose

On 20/03/2023 14:17, Rob Herring wrote: > On Thu, Mar 16, 2023 at 10:05 PM Anshuman Khandual > <anshuman.khandual(a)arm.com> wrote: >> >> CoreSight ETM4x devices could be accessed either via MMIO (handled via >> amba_driver) or CPU system instructions (handled via platform driver). But >> this has the following issues : >> >> - Each new CPU comes up with its own PID and thus we need to keep on >> adding the "known" PIDs to get it working with AMBA driver. While >> the ETM4 architecture (and CoreSight architecture) defines way to >> identify a device as ETM4. Thus older kernels won't be able to >> "discover" a newer CPU, unless we add the PIDs. > > But v8.4 discourages MMIO access, so this problem will go away on its > own. Even if not, adding IDs to stable kernels is standard practice > whether it is PCI VID/PID, compatible string or AMBA PID. Yes, it would eventually go away. As for adding the PIDs, the fundamental issue is, unlike other drivers, except for the "PIDs" everything else is architected and each CPU has this PID alone different and we have plenty of CPUs implementaions out there. But all that said, since we added this as an AMBA driver in the first place (all for simply getting the apb_clk management), I am happy to choose the "Add PIDs to stable kernel approach" for this problem. > >> - With ACPI, the ETM4x devices have the same HID to identify the device >> irrespective of the mode of access. This creates a problem where two >> different drivers (both AMBA based driver and platform driver) would >> hook into the "HID" and could conflict. e.g., if AMBA driver gets >> hold of a non-MMIO device, the probe fails. If we have single driver >> hooked into the given "HID", we could handle them seamlessly, >> irrespective of the mode of access. > > Why are we changing DT for ACPI? Just always use the platform driver > for ACPI and leave DT systems alone. This was mainly due to (1), given we have a platform driver anyway for ACPI. As mentioned above, we could leave the DT alone. > >> - CoreSight is heavily dependent on the runtime power management. With >> ACPI, amba_driver doesn't get us anywhere with handling the power >> and thus one need to always turn the power ON to use them. Moving to >> platform driver gives us the power management for free. > > This sounds like an issue for any amba driver. If this is an issue, > solve it for everyone, not just work around it in one driver. This alone wouldn't be sufficient. We need a platform driver anyway to handle the two different modes in ACPI for ETMs. But this will be a an option for the other CoreSight components which are always MMIO. Thanks Suzuki > > When someone puts another primecell device into an ACPI system, are we > going to go do the same one-off change in that driver too? (We kind of > already did with SBSA UART...) > > Rob

2 years, 1 month

[PATCH v2] coresight: etm4x: Do not access TRCIDR1 for identification

by Suzuki K Poulose

CoreSight ETM4x architecture clearly provides ways to identify a device via registers in the "Management" class, TRCDEVARCH and TRCDEVTYPE. These registers can be accessed without the Trace domain being powered on. We additionally added TRCIDR1 as fallback in order to cover for any ETMs that may not have implemented TRCDEVARCH. So far, nobody has reported hitting a WARNING we placed to catch such systems. Also, more importantly it is problematic to access TRCIDR1, which is a "Trace" register via MMIO access, without clearing the OSLK. But we cannot mess with the OSLK until we know for sure that this is an ETMv4 device. Thus, this kind of creates a chicken and egg problem unnecessarily for systems "which are compliant" to the ETMv4 architecture. Let us remove the TRCIDR1 fall back check and rely only on TRCDEVARCH. Reported-by: Steve Clevenger <scclevenger(a)os.amperecomputing.com> Link: https://lore.kernel.org/all/143540e5623d4c7393d24833f2b80600d8d745d2.167788… Cc: Mike Leach <mike.leach(a)linaro.org> Cc: James Clark <james.clark(a)arm.com> Fixes: 8b94db1edaee ("coresight: etm4x: Use TRCDEVARCH for component discovery") Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> Reviewed-by: Mike Leach <mike.leach(a)linaro.org> --- Changes since v2: - Remove unused etm_tridr_to_arch() helper - Add comment to explain why TRCIDR1 cannot be used. --- .../coresight/coresight-etm4x-core.c | 22 ++++++++----------- drivers/hwtracing/coresight/coresight-etm4x.h | 20 +++++------------ 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 104333c2c8a3..4c15fae534f3 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1070,25 +1070,21 @@ static bool etm4_init_iomem_access(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { u32 devarch = readl_relaxed(drvdata->base + TRCDEVARCH); - u32 idr1 = readl_relaxed(drvdata->base + TRCIDR1); /* * All ETMs must implement TRCDEVARCH to indicate that - * the component is an ETMv4. To support any broken - * implementations we fall back to TRCIDR1 check, which - * is not really reliable. + * the component is an ETMv4. Even though TRCIDR1 also + * contains the information, it is part of the "Trace" + * register and must be accessed with the OSLK cleared, + * 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) { - drvdata->arch = etm_devarch_to_arch(devarch); - } else { - pr_warn("CPU%d: ETM4x incompatible TRCDEVARCH: %x, falling back to TRCIDR1\n", - smp_processor_id(), devarch); - - if (ETM_TRCIDR1_ARCH_MAJOR(idr1) != ETM_TRCIDR1_ARCH_ETMv4) - return false; - drvdata->arch = etm_trcidr_to_arch(idr1); + if ((devarch & ETM_DEVARCH_ID_MASK) != ETM_DEVARCH_ETMv4x_ARCH) { + pr_warn_once("TRCDEVARCH doesn't match ETMv4 architecture\n"); + return false; } + drvdata->arch = etm_devarch_to_arch(devarch); *csa = CSDEV_ACCESS_IOMEM(drvdata->base); return true; } diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 434f4e95ee17..27c8a9901868 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -753,14 +753,12 @@ * TRCDEVARCH - CoreSight architected register * - Bits[15:12] - Major version * - Bits[19:16] - Minor version - * TRCIDR1 - ETM architected register - * - Bits[11:8] - Major version - * - Bits[7:4] - Minor version - * We must rely on TRCDEVARCH for the version information, - * however we don't want to break the support for potential - * old implementations which might not implement it. Thus - * we fall back to TRCIDR1 if TRCDEVARCH is not implemented - * for memory mapped components. + * + * We must rely only on TRCDEVARCH for the version information. Even though, + * TRCIDR1 also provides the architecture version, it is a "Trace" register + * and as such must be accessed only with Trace power domain ON. This may + * not be available at probe time. + * * Now to make certain decisions easier based on the version * we use an internal representation of the version in the * driver, as follows : @@ -786,12 +784,6 @@ static inline u8 etm_devarch_to_arch(u32 devarch) ETM_DEVARCH_REVISION(devarch)); } -static inline u8 etm_trcidr_to_arch(u32 trcidr1) -{ - return ETM_ARCH_VERSION(ETM_TRCIDR1_ARCH_MAJOR(trcidr1), - ETM_TRCIDR1_ARCH_MINOR(trcidr1)); -} - enum etm_impdef_type { ETM4_IMPDEF_HISI_CORE_COMMIT, ETM4_IMPDEF_FEATURE_MAX, -- 2.34.1

2 years, 1 month

[PATCH] coresight: etm4x: Do not access TRCIDR1 for identification

by Suzuki K Poulose

2 years, 1 month

Re: [PATCH 6/7] of/platform: Skip coresight etm4x devices from AMBA bus

by Suzuki K Poulose

On 17/03/2023 20:06, Rob Herring wrote: > On Fri, Mar 17, 2023 at 11:03 AM Suzuki K Poulose > <suzuki.poulose(a)arm.com> wrote: >> >> Hi Rob >> >> Thanks for your response. >> >> On 17/03/2023 14:52, Rob Herring wrote: >>> On Thu, Mar 16, 2023 at 10:06 PM Anshuman Khandual >>> <anshuman.khandual(a)arm.com> wrote: >>>> >>>> Allow other drivers to claim a device, disregarding the "priority" of >>>> "arm,primecell". e.g., CoreSight ETM4x devices could be accessed via MMIO >>>> (AMBA Bus) or via CPU system instructions. >>> >>> The OS can pick which one, use both, or this is a system integration >>> time decision? >> >> Not an OS choice. Historically, this has always been MMIO accessed but >> with v8.4 TraceFiltering support, CPUs are encouraged to use system >> instructions and obsolete MMIO. So, yes, MMIO is still possible but >> something that is discouraged and have to be decided at system >> integration time. >> >>> >>>> The CoreSight ETM4x platform >>>> driver can now handle both types of devices. In order to make sure the >>>> driver gets to handle the "MMIO based" devices, which always had the >>>> "arm,primecell" compatible, we have two options : >>>> >>>> 1) Remove the "arm,primecell" from the DTS. But this may be problematic >>>> for an older kernel without the support. >>>> >>>> 2) The other option is to allow OF code to "ignore" the arm,primecell >>>> priority for a selected list of compatibles. This would make sure that >>>> both older kernels and the new kernels work fine without breaking >>>> the functionality. The new DTS could always have the "arm,primecell" >>>> removed. >>> >>> 3) Drop patches 6 and 7 and just register as both AMBA and platform >>> drivers. It's just some extra boilerplate. I would also do different >>> compatible strings for CPU system instruction version (assuming this >>> is an integration time decision). >> >> The system instruction (and the reigster layouts) are all part of the >> ETMv4/ETE architecture and specific capabilities/features are >> discoverable, just like the Arm CPUs. Thus we don't need special >> versions within the ETMv4x or ETE minor versions. As of now, we have >> one for etm4x and another for ete. > > I just meant 2 new compatible strings. One each for ETMv4x and ETE, > but different from the 2 existing ones. It is different h/w presented > to the OS, so different compatible. > Sorry, was not very clear here. Right now, we have : 1) arm,coresight-etm4x && arm,primecell - For AMBA based devices 2) arm,coresight-etm4x-sysreg - For system instruction based 3) arm,embedded-trace-extension - For ETE >> One problem with the AMBA driver in place is having to keep on adding >> new PIDs for the CPUs. The other option is to have a blanket mask >> for matching the PIDs with AMBA_UCI_ID checks. > > But if MMIO access is discouraged, then new h/w would use the platform > driver(s), not the amba driver, and you won't have to add PIDs. Yes for v8.4 onwards. Alternatively, the newer DTS could skip arm,primecell in the entry and that would kick the platform driver in. So, that should be fine I guess. Kind regards Suzuki > > Rob

2 years, 1 month

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight